Page MenuHomeFreeBSD
Differential D27612

include: Use INCSGROUPS for a few files
ClosedPublic
Actions

Authored by manu on Dec 14 2020, 6:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 14, 5:21 AM
Unknown Object (File)
Thu, Mar 6, 12:01 AM
Unknown Object (File)
Wed, Mar 5, 11:29 PM
Unknown Object (File)
Wed, Mar 5, 4:04 PM
Unknown Object (File)
Sat, Mar 1, 4:25 PM
Unknown Object (File)
Sat, Mar 1, 4:22 AM
Unknown Object (File)
Feb 23 2025, 9:22 AM
Unknown Object (File)
Jan 5 2025, 4:03 PM
View All 79 Files
Subscribers
imp

Details

Reviewers
kp
bapt
Group Reviewers
pkgbase
Commits
rGf61a3898bb98: include: Use INCSGROUPS for a few files
Summary

Instead of using install directly use INCSGROUPS for them.
All those files are the ones we installed when the directory have more .h
files that we don't want to install so they aren't using the LSUBDIR stuff.

Bonus point for pkgbase users files are now in the -dev packages.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
include/
193 lines

Diff 81629

View Options

include/Makefile

  • This file was added.
/*-
* Copyright (c) 2013 Ruslan Bukin <br@bsdpad.com>
* Copyright (c) 2013 Semihalf
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/bus.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/smp.h>
#include <sys/cpuset.h>
#include <machine/smp.h>
#include <machine/fdt.h>
#include <machine/intr.h>
#include <dev/fdt/fdt_common.h>
#define AL_CPU_RESUME_WATERMARK_REG 0x00
#define AL_CPU_RESUME_FLAGS_REG 0x04
#define AL_CPU_RESUME_PCPU_RADDR_REG(cpu) (0x08 + 0x04 + 8*(cpu))
#define AL_CPU_RESUME_PCPU_FLAGS(cpu) (0x08 + 8*(cpu))
/* Per-CPU flags */
#define AL_CPU_RESUME_FLG_PERCPU_DONT_RESUME (1 << 2)
/* The expected magic number for validating the resume addresses */
#define AL_CPU_RESUME_MAGIC_NUM 0xf0e1d200
#define AL_CPU_RESUME_MAGIC_NUM_MASK 0xffffff00
/* The expected minimal version number for validating the capabilities */
#define AL_CPU_RESUME_MIN_VER 0x000000c3
#define AL_CPU_RESUME_MIN_VER_MASK 0x000000ff
/* Field controlling the boot-up of companion cores */
#define AL_NB_INIT_CONTROL (0x8)
#define AL_NB_CONFIG_STATUS_PWR_CTRL(cpu) (0x2020 + (cpu)*0x100)
extern void mpentry(void);
int alpine_serdes_resource_get(uint32_t group, bus_space_tag_t *tag,
bus_addr_t *baddr);
static int platform_mp_get_core_cnt(void);
static int alpine_get_cpu_resume_base(uint32_t *pbase, uint32_t *psize);
static int alpine_get_nb_base(uint32_t *pbase, uint32_t *psize);
static int alpine_get_serdes_base(uint32_t *pbase, uint32_t *psize);
int alpine_serdes_resource_get(uint32_t group, bus_space_tag_t *tag,
bus_addr_t *baddr);
static int
platform_mp_get_core_cnt(void)
{
static int ncores = 0;
phandle_t node;
phandle_t child;
int nchilds;
/* Calculate ncores value only once */
impUnsubmitted
Done
Inline Actions

Shouldn't this be lower case?

imp: Shouldn't this be lower case?
jpa-semihalf.comAuthorUnsubmitted
Done
Inline Actions

While this is case insensitive comparison you are right that the practice is lower case. Will fix it.

jpa-semihalf.com: While this is case insensitive comparison you are right that the practice is lower case. Will…
if (ncores)
return (ncores);
__asm __volatile("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (ncores));
ncores = ((ncores >> 24) & 3) + 1;
andrewUnsubmitted
Done
Inline Actions

This needs a newline:

boolean_t
alpine_validate_cpu(...)

Should it be static?

andrew: This needs a newline: boolean_t alpine_validate_cpu(...) Should it be static?
/* Check if number of CPU cores should be limited in SMP mode */
andrewUnsubmitted
Done
Inline Actions

You should use ofw_cpu_early_foreach for this.

andrew: You should use ofw_cpu_early_foreach for this.
if ((node = OF_finddevice("/cpus")) == -1)
goto end;
nchilds = 0;
for (child = OF_child(node); child != 0; child = OF_peer(child)) {
if (fdt_is_compatible(child, "ARM,CORTEX-A15")) {
andrewUnsubmitted
Done
Inline Actions

This should use a function from cpu-v6.h

andrew: This should use a function from cpu-v6.h
nchilds++;
andrewUnsubmitted
Done
Inline Actions

You should add something like the following to armreg.h, then use it here.

#define L2CTLR_NPROC_SHIFT 24
#define L2CTLR_NPROC(r) ((((r) >> L2CTLR_NPROC_SHIFT) & 3) + 1)
andrew: You should add something like the following to armreg.h, then use it here. #define…
}
}
/* Limit CPUs if DTS has configured less than available */
if ((nchilds > 0) && (nchilds < ncores)) {
printf("SMP: limiting number of active CPUs to %d out of %d\n",
nchilds, ncores);
ncores = nchilds;
}
ianUnsubmitted
Done
Inline Actions

the number of cores should also be limited by the hw.ncpu tunable so that you can control it from loader(8).

ian: the number of cores should also be limited by the hw.ncpu tunable so that you can control it…
jpa-semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

We are not supporting loader(8) so that would be dead code.

jpa-semihalf.com: We are not supporting loader(8) so that would be dead code.
ianUnsubmitted
Done
Inline Actions

I don't view "we are not supporting loader(8)" as a valid position without some sort of really strong justification. It's not 2011 anymore where we commit code for a new platform as soon as it can boot a bare kernel.bin that supports a serial console.

loader(8) is part of freebsd and is the standard way to load a kernel. why do you claim to not support it?

ian: I don't view "we are not supporting loader(8)" as a valid position without some sort of really…
impUnsubmitted
Done
Inline Actions

These boards are embedded network processors that live on a PCIe card. They have very limited space for extra stuff not related to the task at hand... While I generally agree we need loader(8) support, this board may be a reasonable exception to that rule.

imp: These boards are embedded network processors that live on a PCIe card. They have very limited…
ianUnsubmitted
Done
Inline Actions

Ah, I didn't realize these were not general-purpose systems oriented towards the end user. For that case, compiling in the dtb and skipping loader(8) seems reasonable.

ian: Ah, I didn't realize these were not general-purpose systems oriented towards the end user. For…
end:
return (ncores);
}
void
platform_mp_init_secondary(void)
{
arm_init_secondary_ic();
}
void
platform_mp_setmaxid(void)
{
int core_cnt;
core_cnt = platform_mp_get_core_cnt();
mp_maxid = core_cnt - 1;
}
int
platform_mp_probe(void)
{
mp_ncpus = platform_mp_get_core_cnt();
return (1);
}
static int
alpine_get_cpu_resume_base(uint32_t *pbase, uint32_t *psize)
{
phandle_t node;
u_long base=0;
u_long size=0;
if (!pbase || !psize)
impUnsubmitted
Not Done
Inline Actions

Shouldn't this be a kassert?

imp: Shouldn't this be a kassert?
jpa-semihalf.comAuthorUnsubmitted
Not Done
Inline Actions

In general that would be the way to go, but other errors are handled with same panic message anyway, so I would rather not duplicate code unless necessary.

jpa-semihalf.com: In general that would be the way to go, but other errors are handled with same panic message…
return (EINVAL);
if ((node = OF_finddevice("/")) == -1)
return (EFAULT);
andrewUnsubmitted
Done
Inline Actions

Do these need to be fdt_ functions, or could you call an ofw_ function?

andrew: Do these need to be fdt_ functions, or could you call an ofw_ function?
jpa-semihalf.comAuthorUnsubmitted
Done
Inline Actions

I don't have device yet, so the alternative option is to use pure OF_ set of functions to find one, for example wrapped into ofw_search_tree_compatible_device("/simple-bus/al-cpu-resume");

jpa-semihalf.com: I don't have device yet, so the alternative option is to use pure OF_ set of functions to find…
if ((node = fdt_find_compatible(node, "simple-bus", 1)) == 0)
return (EFAULT);
if ((node =
fdt_find_compatible(node, "annapurna-labs,al-cpu-resume", 1)) == 0)
return (EFAULT);
if (fdt_regsize(node, &base, &size))
return (EFAULT);
*pbase = base;
*psize = size;
return (0);
}
static int
alpine_get_nb_base(uint32_t *pbase, uint32_t *psize)
{
phandle_t node;
u_long base=0;
u_long size=0;
if (!pbase || !psize)
impUnsubmitted
Done
Inline Actions

kassert?

imp: kassert?
jpa-semihalf.comAuthorUnsubmitted
Done
Inline Actions

Same as above.

jpa-semihalf.com: Same as above.
return (EINVAL);
if ((node = OF_finddevice("/")) == -1)
andrewUnsubmitted
Done
Inline Actions

Missing space, should be base = 0;, and the same for size.

andrew: Missing space, should be `base = 0;`, and the same for size.
return (EFAULT);
andrewUnsubmitted
Done
Inline Actions
if (pbase == NULL || psize == NULL)
andrew: if (pbase == NULL || psize == NULL)
if ((node = fdt_find_compatible(node, "simple-bus", 1)) == 0)
return (EFAULT);
if ((node =
fdt_find_compatible(node, "annapurna-labs,al-nb-service", 1)) == 0)
return (EFAULT);
if (fdt_regsize(node, &base, &size))
return (EFAULT);
*pbase = base;
*psize = size;
return (0);
}
void
platform_mp_start_ap(void)
{
uint32_t physaddr;
vm_offset_t vaddr;
uint32_t val;
uint32_t start_mask;
uint32_t cpu_resume_base;
uint32_t nb_base;
andrewUnsubmitted
Done
Inline Actions

uint32_t is the wrong type for a number of these.

andrew: uint32_t is the wrong type for a number of these.
uint32_t cpu_resume_size;
uint32_t nb_size;
bus_addr_t cpu_resume_baddr;
bus_addr_t nb_baddr;
int a;
if (alpine_get_cpu_resume_base(&cpu_resume_base, &cpu_resume_size))
panic("Couldn't resolve cpu_resume_base address\n");
if (alpine_get_nb_base(&nb_base, &nb_size))
panic("Couldn't resolve_nb_base address\n");
/* Proceed with start addresses for additional CPUs */
ianUnsubmitted
Done
Inline Actions

Unless this platform has mapping control registers that allow repositioning the window for accessing hardware regs, none of these bus_space_map() calls should be using fdt_immr_pa, they should just be using the physical addresses from the fdt data.

As far as I know, only Marvel chips have such a feature (and maybe not even all of them). fdt_immr_pa should never appear outside of arm/mv (and some polution where it leaks out into dev/fdt that we really need to fix).

ian: Unless this platform has mapping control registers that allow repositioning the window for…
jpa-semihalf.comAuthorUnsubmitted
Done
Inline Actions

I removed all references to fdt_immr_pa and used the preferred way of setting addresses (arm_devmap_add_entry())

jpa-semihalf.com: I removed all references to fdt_immr_pa and used the preferred way of setting addresses…
if (bus_space_map(fdtbus_bs_tag, fdt_immr_pa + cpu_resume_base,
cpu_resume_size, 0, &cpu_resume_baddr))
panic("Couldn't map CPU-resume area");
if (bus_space_map(fdtbus_bs_tag, fdt_immr_pa + nb_base,
nb_size, 0, &nb_baddr))
panic("Couldn't map NB-service area");
/* Proceed with start addresses for additional CPUs */
val = bus_space_read_4(fdtbus_bs_tag, cpu_resume_baddr,
AL_CPU_RESUME_WATERMARK_REG);
if (((val & AL_CPU_RESUME_MAGIC_NUM_MASK) != AL_CPU_RESUME_MAGIC_NUM) ||
((val & AL_CPU_RESUME_MIN_VER_MASK) < AL_CPU_RESUME_MIN_VER)) {
panic("CPU-resume device is not compatible");
}
vaddr = (vm_offset_t)mpentry;
physaddr = pmap_kextract(vaddr);
for (a = 1; a < platform_mp_get_core_cnt(); a++) {
/* Power up the core */
bus_space_write_4(fdtbus_bs_tag, nb_baddr,
AL_NB_CONFIG_STATUS_PWR_CTRL(a), 0);
mb();
/* Enable resume */
val = bus_space_read_4(fdtbus_bs_tag, cpu_resume_baddr,
AL_CPU_RESUME_PCPU_FLAGS(a));
val &= ~AL_CPU_RESUME_FLG_PERCPU_DONT_RESUME;
bus_space_write_4(fdtbus_bs_tag, cpu_resume_baddr,
AL_CPU_RESUME_PCPU_FLAGS(a), val);
mb();
/* Set resume physical address */
bus_space_write_4(fdtbus_bs_tag, cpu_resume_baddr,
AL_CPU_RESUME_PCPU_RADDR_REG(a), physaddr);
mb();
}
/* Release cores from reset */
if (bus_space_map(fdtbus_bs_tag, fdt_immr_pa + nb_base,
nb_size, 0, &nb_baddr))
panic("Couldn't map NB-service area");
start_mask = (1 << platform_mp_get_core_cnt()) - 1;
/* Release cores from reset */
val = bus_space_read_4(fdtbus_bs_tag, nb_baddr, AL_NB_INIT_CONTROL);
val |= start_mask;
andrewUnsubmitted
Done
Inline Actions

Is a dmb enough here?

andrew: Is a dmb enough here?
jpa-semihalf.comAuthorUnsubmitted
Done
Inline Actions

armv7_sev() issues dsb internally, so I think it is even not needed here, but for the sake of future changes to armv7_sev() I will replace the mb() accordingly.

jpa-semihalf.com: armv7_sev() issues dsb internally, so I think it is even not needed here, but for the sake of…
bus_space_write_4(fdtbus_bs_tag, nb_baddr, AL_NB_INIT_CONTROL, val);
ianUnsubmitted
Done
Inline Actions

Why do we even do a SEV here? I have the feeling this has been pasted into every soc mp file and doesn't do anything useful on any of them.

ian: Why do we even do a SEV here? I have the feeling this has been pasted into every soc mp file…
jpa-semihalf.comAuthorUnsubmitted
Done
Inline Actions

I removed it and so far so good. On the other hand I kept the dsb().

jpa-semihalf.com: I removed it and so far so good. On the other hand I kept the dsb().
mb();
armv7_sev();
bus_space_unmap(fdtbus_bs_tag, nb_baddr, nb_size);
bus_space_unmap(fdtbus_bs_tag, cpu_resume_baddr, cpu_resume_size);
}
void
platform_ipi_send(cpuset_t cpus, u_int ipi)
{
pic_ipi_send(cpus, ipi);
}
andrewUnsubmitted
Done
Inline Actions

This has the same style issues as alpine_get_nb_base.

andrew: This has the same style issues as alpine_get_nb_base.