On Fri, Apr 08, 2016 at 06:30:31PM +0200, Martin Pieuchot wrote:
> On 08/04/16(Fri) 17:23, Patrick Wildt wrote:
> > On Fri, Apr 08, 2016 at 03:30:24PM +0200, Mark Kettenis wrote:
> > > > On Fri, Apr 08, 2016 at 03:29:05PM +0300, Artturi Alm wrote:
> > > > > On Fri, Apr 08, 2016 at 01:44:11PM +0200, Patrick Wildt wrote:
> > > >
> > > > There are a few drivers that need special handling. EHCI and AHCI need
> > > > an attachment driver, as ehci* at mainbus? can only be handled by one
> > > > driver. So you need something like.
> > > >
> > > > imxehci* at mainbus?
> > > > ehci* at imxehci?
> > > >
> > > > That's a bit of an overhead and steals about 16 lines in
> > > > imxehci.c/imxahci.c.
> > > >
> > > > You don't anymore need an imx0, exynos0 or sunxi0 bus. Those can all be
> > > > replaced by the mainbus.
> > >
> > > Actually, I don't think that's the right approach either. The FDT is
> > > a device tree, and the OpenBSD device tree should follow that. If you
> > > look at the device trees for the imx6 and exynos boards for example,
> > > you see that there is a "soc" node. The ehci and ahci are below that
> > > "soc" node in the device tree. You can differentiate between
> > > different SoCs based on the properties of this "soc" node, and attach
> > > different drivers based on that. Then you can have specific ahci and
> > > ehci attachments for each SoC.
> > >
> > > So you'd have:
> > >
> > > imx* at mainbus?
> > > ehci* at imx?
> > >
> > > and in your files.xxx you'd have something like:
> > >
> > > attach ehci at imx with ehci_imx
> > > file arch/armv7/imx/ehci_imx.c ehci_imx
> > >
> >
> > I don't think that's a good idea. Let's take the FreeScale (NXP)
> > LS1021A SoC and look at the device tree. There's a USB controller:
> >
> > usb3@3100000 {
> > compatible = "snps,dwc3";
> > [...]
> > };
> >
> > This is the Synopsis DesignWare Controller. It's not NXP-specific.
> > The same controller is used in other SoCs, for instance:
> >
> > exynos5420.dtsi: compatible = "snps,dwc3";
> > exynos5420.dtsi: compatible = "snps,dwc3";
> > omap5.dtsi: compatible = "snps,dwc3";
> >
> > This would mean that I would need one SoC bus per supported SoC
> > and one xhci driver per SoC. In practice, there's no difference
> > between the OMAP and the LS1 controller.
>
> Does that mean that the glue code for these two SoC can be completely
> abstracted? If I look at the existing glue code for ehci(4) on all
> the ARMv7 SoC that we run on they all need specific initialization.
And those will have separate drivers, as they are separate controllers.
Other controllers, which are the same, don't need anything specific
apart from:
quirks: sometimes you might need one bit differently, that is often
done by declaring the node using another compatible string.
gpios: sometimes you need to reset a device using a GPIO pin. this
can be abstracted with an API.
clocks: clocks need to run, or you need to extract a frequency. this
can also be abstracted.
This sounds like added complexity, and it is. I'm not proposing to add
the abstraction right now, but depending on how the platform support
will evolve, these might be more and more necessary.
>
> So having a generic abstraction in this case will lead to per-SoC
> code in your ehci_fdt.c. Or am I missing something?
I suppose most controllers will still be SoC-specific, but there are
more and more "generic" controllers, like those DesignWare.
So maybe it looks more like... ehci_imx.c, xhci_dwc3.c.
>
> > Implementing a new board would mean that I also need to write code
> > for a new SoC bus and it wouldn't just work out-of-the-box?
>
> Either way you'll have to write specific code since the lack of
> standard way to enumerate and enable devices requires it.
>
