Created attachment 165548 [details] Move Atmel's USB OHCI controller FDT driver Currently, the at91 specific USB OHCI driver is named simply ohci_fdt.c, which is misleading and does not leave space for an attempt at a generic ohci_fdt implementation (e.g., possibly based on the dwc_otg FDT support). The attached patch renames ohci_fdt.c to at91ohci_fdt.c in sys/dev/usb/controller and also replaces the reference to ohci_fdt.c in sys/arm/at91/files.at91 with at91ohci_fdt.c. Once/if a generic OHCI FDT driver is added, the at91 specific one may be retired if at91 can use the generic one.
Comment on attachment 165548 [details] Move Atmel's USB OHCI controller FDT driver A generic version of this may not be possible since each front end needs to do SoC specific things, especially around turning on and tuning different clocks for different SoCs even within the same family. And often it isn't just clocks. It would be better, instead, to refactor so we can share the code better between these different FDT front ends. This patch is generally good, however, as far as it goes.
Acutally, the rename isn't quite right. Generally, for SoCs, we put the code in the SoC directory. While there are some exceptions like USB, I think it is a mistake to continue them as exceptions. This should be moved to sys/arm/at91/at91_ohci_fdt.c. Likewise the sys/dev/usb/controller/ohci_atmelarm.c should be moved as well (and likely retired once we de-orbit !FDT suppport). The naming isn't right at all and is the odd-man out relative to other recent additions. And the ohci_s3c24x0.c likely should move as well, but that's beyond the scope of my advice as the Atmel dude. I'll commit the right changes instead.
Hi, Thank you for your patch. I'll have a look at it tomorrow. --HPS
(In reply to Warner Losh from comment #2) Warner: Feel free to fix this as you think is best.
(In reply to Warner Losh from comment #1) How about we split the SoC-specific stuff into a, say, USB-PHY layer, either totally separate or as a "bus" to which USB OHCI/EHCI/XHCI/OTG/etc driver would attach? If USB-PHY is totally separate it would need to be loaded before the USB driver and, if needed, unloaded after it, which may be a bit of a pain to handle. If USB-PHY is a bus, to which USB driver(s) would attach, then things seem cleaner from this perspective, but in this case aren't we just going to end up moving code duplication from one place to the other? At least with a bus approach, it seems to me, we could have a default implementation that simply goes and attaches its children (hinted via FDT for example). And for SoCs that require some additional magic before attaching their children we could simply override suitable bus methods? Or am I way off here?
A commit references this bug: Author: imp Date: Sat Jan 16 04:47:33 UTC 2016 New revision: 294131 URL: https://svnweb.freebsd.org/changeset/base/294131 Log: Move ohci files to their proper place in the tree for atmel. Fix when it is included (we don't have a at91rm9200 device). From a similar patch in the PR, with tweaked names. PR: 206229 Changes: head/sys/arm/at91/at91_ohci.c head/sys/arm/at91/at91_ohci_fdt.c head/sys/arm/at91/files.at91 head/sys/conf/files head/sys/dev/usb/controller/ohci_atmelarm.c head/sys/dev/usb/controller/ohci_fdt.c