On Thu, Apr 07, 2016 at 06:31:22PM +0200, Patrick Wildt wrote:
> On Thu, Apr 07, 2016 at 06:00:25PM +0200, Mark Kettenis wrote:
> > > Date: Thu, 7 Apr 2016 16:54:19 +0200
> > > From: Patrick Wildt <[email protected]>
> > >
> > > Hi,
> > >
> > > after a bit of talking with jsg@ we have found a way forward on how to
> > > integrate FDT into ARM.
> > >
> > > An aspect of this is having an MI FDT bus residing in sys/dev/ofw/,
> > > where the fdt subroutines currently already are. Then we'll convert
> > > the vexpress platform to use FDT and continue with Exynos later on.
> > >
> > > As a first step I would rename fdt.c to fdt_subr.c. The actual fdt bus
> > > will then be committed to fdt.c. Also, create sys/dev/ofw/files.fdt to
> > > later on declare the new fdt bus in there.
> > >
> > > As we'll soon declare fdt at mainbus, I think we should also move the
> > > include of files.fdt to be after mainbus on socppc. Even if it won't
> > > be used on socppc.
> > >
> > > ok?
> >
> > I have no objections to rename fdt.c fdt_subr.c. However, I do object
> > against having an "fdt" bus, at least for socppc. The fdt code should
>
> I don't propose an fdt bus for socppc.
>
> > just be the enumeration mechanism for "mainbus". And I think armv7
> > should be the same.
>
> That sounds more like implementing the fdtbus functionality, but just
> call it mainbus.
>
> The current situation is that we have a mainbus0 on arm that attaches
> anything that wants to be attached. It's not really the mainbus you
> expect it to be. What is does is attach cpu0 at mainbus0 and depending
> on the SoC an obio0 at mainbus0, pxaip0 at mainbus0, or imx0 at
> mainbus0, exynos0 at mainbus0 and so on. These are the actual busses
> that create their own bus space and bus dma tag and pass it to the
> controller driver instance. mainbus0 does (not yet) do anything in that
> regard. This also means all drivers are ... imxuart0 at imx0, not at
> mainbus0.
>
> Looks like:
>
> mainbus0 at root
> cpu0 at mainbus0
> cortex0 at mainbus0
> ampintc0 at mainbus0
> imx0 at mainbus0
> imxuart0 at imx0
> imxiic0 at imx0
> ehci0 at imx0
>
> Goal was:
>
> mainbus0 at root
> cpu0 at mainbus0
> ampintc0 at fdt0
> imxuart0 at fdt0
>
> So the idea was to replace imx0, exynos0, vexpress0, etc. on ARMv7 with
> an fdt0 that does all that what e. g. imx0 used to do, just with FDT
> only.
>
> Attaching controllers from FDT is not just going through the tree and
> attaching all nodes that are there. You have to take care of interrupt-
> controller hierarchy and clocks. Those need to be attached first. This
> adds a bit of complexity and should probably not be duplicated for a new
> arch (arm64 for instance). Which is why we thought it would be nice to
> have that in sys/dev/ofw/.
>
> I could though completely replace mainbus.c with the fdtbus, call it
> mainbus, implement it for armv7 only, and fall back to the old version
> for the legacy platforms armish, zaurus and non-fdt armv7. Is that the
> way to go?
>
Hi,
it could very well be "imx0 at root", and rest on imx0.
i don't see the point of making fdt the primary, when improving the
code for both fdt and 'hardcoded' could happen together.
i see fdt as mostly great opportunity to learn about soc details by eye,
and possibly great source of bug reports from users of boards untested
by devs, which might not sound bad, but giving absolute answers without
the actual source for the fdt blob used will be hard =leading to additional
effort in solving some likely bugs in the future.. gl hf :D
i'll show some snippets of what improving the structures used to describe
soc specifics may lead to look like, with sun7i md parts as example at
the end. drivers get to use confargs etc.. :)
something like below would allow keeping the fact fdt is in play as hidden
as wanted, possibly only manipulating the hardcoded device list of devices
KNOWN to work on the SOC being ran on (and board details).
i don't see any great advance by addition of fdt-nativeness since lack
of working drivers is caused by something totally else than lack of
addresses of peripherals for drivers to map and such.
personally, i'd hate to see the uglyness of fdt0-the-fakebus in dmesg.
do read(if you care) as through s/arm88k/armv7/, and yeah, timestamps
did prove this way faster than allowing autoconf do the job completely.
-Artturi
_________________________________________________________________
dev/mainbus.c:
static int mainbus_attached;
vaddr_t last_bs_va = MACHINE_IO_AREA_VBASE;
bus_addr_t bs_io_start = 0xffffffffU;
bus_addr_t bs_io_end;
struct extent *bs_extent;
#define ADD_2_IOBS(xd) \
if ((xd)->addr < bs_io_start) \
bs_io_start = (xd)->addr; \
if (((xd)->addr + (xd)->size) > bs_io_end) \
bs_io_end = (xd)->addr + (xd)->size
/* called from xxx_early_boot() */
void
arm88k_init_devs(void)
{
struct arm88k_soc_dev *sd;
struct arm88k_soc_dev gic_sd;
struct devmem *dm;
if (CORTEX_IS(_A9) || CORTEX_IS(_A7) || CORTEX_IS(_A15)) {
gic_sd.addr = cpu_periphbase();
gic_sd.size = 0x3000; /* off + maxsize we might use */
sd = &gic_sd;
mmu_drain_writebuf();
ADD_2_IOBS(sd);
}
for (sd = platform->soc_devs; sd->addr != ~0u; sd++) {
if (!sd->addr || !sd->size)
goto check4init;
ADD_2_IOBS(sd);
if (sd->eres != NULL)
for (dm = sd->eres; dm->addr != ~0u &&
dm->addr > ARM88K_SD_ADDR_IRQ; dm++) {
ADD_2_IOBS(dm);
}
check4init:
if ((sd->flags & 0xff) != 0)
continue; /* only run init on unit0 */
if (sd->init)
sd->init();
}
dbg_print_ts(0); /* XXX */
printf(" init_devs done %#lx %#lx\n", bs_io_start, bs_io_end);
}
/*
* Configuration glue
*/
struct cfdriver mainbus_cd = {
NULL, "'get rid of assumption', make it extern in sys/kern ;)", DV_DULL
};
static void *mainbusdev;
void
arm88k_attach(struct device *parent, struct device *self, void *args)
{
struct confargs oca[2];
struct cfdata *cf;
struct arm88k_soc_dev *sd;
mainbus_attached = 1;
/* hack.. but i like it */
mainbusdev = self;
mainbus_cd.cd_devs = &mainbusdev;
mainbus_cd.cd_ndevs = 1;
/* XXX got fdt? hook in here. */
printf(": %s\n", platform->board_name);
/* XXX DBG */
printf("arm88k_attach() ");
dbg_print_ts(1);
bs_extent = extent_create("bus_space", bs_io_start,
bs_io_end, M_DEVBUF, NULL, 0, EX_NOWAIT);
if (bs_extent == NULL)
panic("unable to allocate bus_space extent");
bzero(&oca[0], sizeof(struct confargs));
oca[0].ca_iot = &mainbus_bustag;
oca[0].ca_dmat = &arm88k_bus_dma_tag;
oca[0].ca_paddr = ~0u;
oca[0].ca_irq = -1;
for (sd = platform->soc_devs; sd->addr != ~0u; sd++) {
if ((sd->flags >> 16 & 0xff) & ARM88K_SD_DISABLED)
continue;
if ((sd->flags >> 16 & 0xff) & ARM88K_SD_CONFIGURED)
continue;
if (!sd->ca)
continue;
for (cf = cfdata; cf->cf_attach; cf++) {
if (cf->cf_fstate == FSTATE_FOUND ||
cf->cf_fstate == FSTATE_DNOTFOUND ||
cf->cf_fstate == FSTATE_DSTAR)
continue;
/* XXX check that we're in the parents also? */
if (cf->cf_attach == sd->ca &&
cf->cf_unit == (sd->flags & 0xff))
goto found;
}
/* XXX print not found? */
continue;
found:
memcpy(&oca[1], &oca[0], sizeof(struct confargs));
oca[1].ca_dev = sd;
config_attach(self, cf, &oca[1], arm88k_print);
}
}
_________________________________________________________________
include/autoconf.h:
/*
* the order of these in <soc>_soc_devs[] does matter, because:
* 1. cpu should be the first[0], dmesg looks better.
* 2. for same reason as above and because most devices establish interrupt
* handlers, interrupt controller should be after cpu in [1].
* 3. timer as third because it's one of the few drivers that must have
* working .init(), which means anything needed by these in
* <driver>_attach shall be provided in time during early
* bootstrap&|cpu attach. XXX
* then, things should be ordered so that no panics appear
* during the boot :P
* hint: most of the code handling _soc_devs[] is found in
* dev/mainbus.c: arm88k_attach()
*/
struct arm88k_soc_dev {
paddr_t addr;
#define ARM88K_SD_ADDR_INV ~0
#define ARM88K_SD_ADDR_IRQ 1
psize_t size;
u_int flags;
#define ARM88K_SD_UNIT_MASK 0x000000ff
#define ARM88K_SD_UNIT_SHIFT 0
#define ARM88K_SD_TYPE_MASK 0x0000ff00
#define ARM88K_SD_TYPE_SHIFT 8
#define ARM88K_SD_DISABLED 1
#define ARM88K_SD_CONFIGURED 2
#define ARM88K_SD_CONSPROBE 4
#if !defined(_STANDALONE)
struct cfattach *ca;
#else
void *ca;
#endif
int irq;
void *eres; /* extra resource */
void (*init)(void);
};
struct confargs {
bus_space_tag_t ca_iot;
bus_dma_tag_t ca_dmat;
paddr_t ca_paddr; /* physical address */
int ca_irq; /* interrupt number */
const char *ca_name; /* device name */
struct arm88k_soc_dev *ca_dev;
};
_________________________________________________________________
conf/generic.sunxi(included in GENERIC):
# main bus device
sunxi0 at root
# Sunxi A1x/A20 on-chip devices
cpu* at sunxi?
gic* at sunxi? # generic interrupt controller
agtimer* at sunxi?
sxiintc* at sunxi? # sun4i interrupt controller
sxipio* at sunxi? # GPIO pins for leds & PHYs
sxiccmu* at sunxi? # Clock Control Module/Unit
sxitimer* at sunxi?
sxidog* at sunxi? # watchdog timer
sxirtc* at sunxi? # Real Time Clock
sxie* at sunxi?
ahci* at sunxi? # AHCI/SATA (shim)
ehci* at sunxi? # EHCI (shim)
#ohci* at sunxi? # OHCI (shim)
gpio* at sxipio?
uart* at sunxi?
_________________________________________________________________
sunxi/files.sunxi:
define sunxi {}
device sunxi: sunxi
attach sunxi at root
file arch/arm88k/sunxi/sun4i.c sunxi
file arch/arm88k/sunxi/sun7i.c sunxi
file arch/arm88k/sunxi/sunxi_machdep.c sunxi
_________________________________________________________________
sunxi/sunxi_machdep.c:
#include <sys/param.h>
#include <sys/types.h>
#include <sys/systm.h>
#include <machine/autoconf.h>
#include <machine/bus.h>
#include <machine/board.h>
#include <arm88k/sunxi/sunxireg.h>
#include <arm88k/sunxi/sunxiext.h>
#include <dev/cons.h>
#if !defined(_STANDALONE)
int sxi_match(struct device *, void *, void *);
struct cfdriver sunxi_cd = { NULL, "sunxi", DV_DULL };
struct cfattach sunxi_ca = { sizeof(struct device), sxi_match, arm88k_attach };
struct cfattach sxicpu_ca = { sizeof(struct device), NULL, arm88k_cpu_attach };
int
sxi_match(struct device *parent, void *cf, void *args)
{
if (ICV_IS(_SUNXI))
return 1;
return 0;
}
#endif
void
sunxi_powerdown(void)
{
}
void
sunxi_reboot(void)
{
#if !defined(_STANDALONE)
sxidog_reset();
#else
bus_space_tag_t bst = NULL;
bus_space_write_4(bst, WDOG_ADDR, 0x04, 3);
bus_space_write_4(bst, WDOG_ADDR, 0x00, 0x14af);
#endif
}
/* called from arm88k_bootstrap()(=initarm()), before consinit() */
void
sunxi_early_boot(void)
{
if (SOC_IS(_SUN4I))
platform->soc_devs = sun4i_devs;
else if (SOC_IS(_SUN7I))
platform->soc_devs = sun7i_devs;
else
return;
/* generic, possibly adjusted in _brd_init() */
platform->mainbus = "sunxi";
platform->memstart = 0x40000000;
platform->memsize = 0x40000000;
platform->freq = 24 * 1000 * 1000;
platform->cnfreq = 24 * 1000 * 1000;
platform->reboot = sunxi_reboot;
platform->powerdown = sunxi_powerdown;
sunxi_brd_init();
arm88k_init_devs();
}
void
sunxi_brd_init(void)
{
/* XXX got fdt? hook in here. */
switch (brdtyp & BOARD_MASK) {
case _CUBIE:
platform->board_name = "Allwinner A10 Cubieboard";
break;
case _CUBIE2:
platform->board_name = "Allwinner A20 Cubieboard2";
break;
case _CTRUCK:
platform->board_name = "Allwinner A20 CubieTruck";
break;
default:
platform->board_name =
SOC_IS(_SUN4I) ? "unknown A10" : "unknown A20";
break;
}
}
_________________________________________________________________
sunxi/sun7i.c:
#include <sys/param.h>
#include <sys/systm.h>
#include <machine/autoconf.h>
#include <machine/bus.h>
#include <machine/board.h>
#include <arm88k/sunxi/sunxireg.h>
#include <arm88k/sunxi/sunxiext.h>
extern struct cfattach agtimer_ca;
extern void agtimer_init(void);
#if !defined(_STANDALONE)
static struct devmem sxie_extra[] = {
{ SXIESRAM_ADDR, SXIESRAM_SIZE },
{ ~0u, 0 }
};
#endif /* !_STANDALONE */
struct arm88k_soc_dev sun7i_soc_devs[] = {
#if !defined(_STANDALONE)
{ .flags = 0,
.ca = &sxicpu_ca,
},
{ .flags = 0,
.init = gic_init,
.ca = &gic_ca,
},
#endif /* !_STANDALONE */
{ .flags = 0,
.init = agtimer_init,
.ca = &agtimer_ca,
},
#if !defined(_STANDALONE)
{ .addr = CCMU_ADDR,
.size = CCMU_SIZE,
.flags = 0,
.init = sxiccmu_init,
.ca = &sxiccmu_ca,
},
{ .addr = PIO_ADDR,
.size = PIOx_SIZE,
.irq = PIO_IRQ,
.flags = 0,
.init = sxipio_init,
.ca = &sxipio_ca,
},
{ .addr = WDOG_ADDR,
.size = WDOG_SIZE,
.flags = 0,
.init = sxidog_init,
.ca = &sxidog_ca,
},
{ .addr = RTC_ADDR,
.size = RTC_SIZE,
.flags = 0,
.ca = &sxirtc_ca,
},
#endif /* !_STANDALONE */
{ .addr = UART0_ADDR,
.size = UARTx_SIZE,
.irq = UART0_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 0,
.ca = &sxiuart_ca,
},
{ .addr = UART1_ADDR,
.size = UARTx_SIZE,
.irq = UART1_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 1,
.ca = &sxiuart_ca,
},
{ .addr = UART2_ADDR,
.size = UARTx_SIZE,
.irq = UART2_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 2,
.ca = &sxiuart_ca,
},
{ .addr = UART3_ADDR,
.size = UARTx_SIZE,
.irq = UART3_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 3,
.ca = &sxiuart_ca,
},
{ .addr = UART4_ADDR,
.size = UARTx_SIZE,
.irq = UART4_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 4,
.ca = &sxiuart_ca,
},
{ .addr = UART5_ADDR,
.size = UARTx_SIZE,
.irq = UART5_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 5,
.ca = &sxiuart_ca,
},
{ .addr = UART6_ADDR,
.size = UARTx_SIZE,
.irq = UART6_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 6,
.ca = &sxiuart_ca,
},
{ .addr = UART7_ADDR,
.size = UARTx_SIZE,
.irq = UART7_IRQ,
.flags = ARM88K_SD_CONSPROBE << 16 | 7,
.ca = &sxiuart_ca,
},
#if !defined(_STANDALONE)
{ .addr = EMAC_ADDR,
.size = EMAC_SIZE,
.irq = EMAC_IRQ,
.eres = &sxie_extra,
.flags = 0,
.ca = &sxie_ca,
},
{ .addr = SATA_ADDR,
.size = SATA_SIZE,
.irq = SATA_IRQ,
.flags = 0,
.ca = &sxiahci_ca,
},
{ .addr = USB1_ADDR,
.size = USBx_SIZE,
.irq = USB1_IRQ,
.flags = 0,
.ca = &sxiehci_ca,
},
{ .addr = USB2_ADDR,
.size = USBx_SIZE,
.irq = USB2_IRQ,
.flags = 1,
.ca = &sxiehci_ca,
},
#endif /* !_STANDALONE */
{ .addr = ~0u,
.ca = NULL,
}
};
struct arm88k_soc_dev *sun7i_devs =
(struct arm88k_soc_dev *)&sun7i_soc_devs[0];
> >
> > It should be relatively easy to add an ma_node member to the arm
> > struct mainbus_attach_args and use the fdt to attach the right drivers
> > if it is present and fall back on mainbussearch() if the fdt
> > information is not available yet (or if a particular platform has not
> > been converted yet). You just have to make mainbussearch() set
> > ma_node to -1 (or some other magic value) and check for that in the
> > existing "platform" drivers,
> >
> > I think that implies that we don't need sys/dev/ofw/files.fdt either.
> >
> > Cheers,
> >
> > Mark
> >
> > > diff --git sys/arch/armv7/conf/files.armv7 sys/arch/armv7/conf/files.armv7
> > > index c5d022c..fc0a14f 100644
> > > --- sys/arch/armv7/conf/files.armv7
> > > +++ sys/arch/armv7/conf/files.armv7
> > > @@ -10,7 +10,8 @@ major {rd = 18}
> > >
> > > define fdt {}
> > > file arch/armv7/fdt/fdt_machdep.c fdt needs-flag
> > > -file dev/ofw/fdt.c
> > > +
> > > +include "dev/ofw/files.fdt"
> > >
> > > file arch/arm/arm/conf.c
> > >
> > > diff --git sys/arch/socppc/conf/files.socppc
> > > sys/arch/socppc/conf/files.socppc
> > > index 4aa9526..3d02ddb 100644
> > > --- sys/arch/socppc/conf/files.socppc
> > > +++ sys/arch/socppc/conf/files.socppc
> > > @@ -14,10 +14,8 @@ file arch/socppc/socppc/disksubr.c
> > > disk
> > > file arch/socppc/socppc/machdep.c
> > > file arch/socppc/socppc/mem.c
> > > file dev/cninit.c
> > > -file dev/ofw/fdt.c
> > > file arch/socppc/socppc/n1200_dts.S
> > >
> > > -
> > > define mainbus {}
> > > device mainbus
> > > attach mainbus at root
> > > @@ -27,6 +25,8 @@ device cpu
> > > attach cpu at mainbus
> > > file arch/socppc/socppc/cpu.c
> > >
> > > +include "dev/ofw/files.fdt"
> > > +
> > > # MPC8349E on-board devices
> > > device obio {[addr = 0], [ivec = -1], [phy = -1]}
> > > attach obio at mainbus
> > > diff --git sys/dev/ofw/fdt.c sys/dev/ofw/fdt_subr.c
> > > similarity index 100%
> > > rename from sys/dev/ofw/fdt.c
> > > rename to sys/dev/ofw/fdt_subr.c
> > > diff --git sys/dev/ofw/files.fdt sys/dev/ofw/files.fdt
> > > new file mode 100644
> > > index 0000000..0ff8192
> > > --- /dev/null
> > > +++ sys/dev/ofw/files.fdt
> > > @@ -0,0 +1,3 @@
> > > +# $OpenBSD$
> > > +
> > > +file dev/ofw/fdt_subr.c
> > >
> > >
> >
>
