[PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
florian at openwrt.org
Wed Oct 24 12:36:08 EDT 2012
On Wednesday 24 October 2012 10:16:31 Stephen Warren wrote:
> On 10/24/2012 09:26 AM, Sebastian Andrzej Siewior wrote:
> > On Wed, Oct 24, 2012 at 10:57:00AM -0400, Alan Stern wrote:
> >> Under the circumstances, do we really need a new binding document for
> >> the ehci-platform driver?
> It seems reasonable to add the new properties to usb-ehci.txt, since
> they do describe the HW.
> >> We should be able to use the existing
> >> usb-ehci binding, perhaps with some new properties added:
> >> has-synopsys-hc-bug
> >> no-io-watchdog
> >> has-tt
> That sounds fine to me.
> However, there is an implementation issue here. I believe the way Linux
> searches for a driver for a particular node is:
> for every driver that's registered
> if the driver's supported compatible list matches the device
> use the driver
> (See drivers/base/platform.c:platform_match() which implements the if
> statement above, and I assume the driver core implements the outer for
> loop above)
> That means that if the generic driver supports compatible="usb-ehci", it
> may "steal" device nodes that have
> compatible="something-custom","usb-ehci", even if there's a driver
> specifically for "something-custom". We would need to re-arrange the
> driver matching code to:
> for each compatible value in the node:
> for each driver that's registered:
> if the driver supports the compatible value:
> use the driver.
> > On the PCI side we have VID, PID which is used for quirks. Sometimes we have a
> > revision ID which can be used to figure out if "this quirk" is still required.
> > The PCI driver probes by class so the ehci driver does not have a large table
> > of VID/PID for each controller out there. And the USB controller in two
> > different Intel boards has a different PID so a quirk, if required, could be
> > applied only to the specific mainboard.
> > Based on this we need atleast two compatible entries one "HW-Specific"
> > followed by a generic one (similar to PCI class).
> > Doing it the PCI way we would have to have table and figure out which
> > HW-specific compatible entry sets the TT flag and which one does the
> > no-io-watchdog. Having has-tt in compatible isn't right.
> Yes, the driver could easily bind to anything compatible with
> "usb-ehci", then use the HW-specific compatible value to index into a
> quirk table in the same way that specific PCI IDs index into a quirk table.
> I agree that having separate compatible values like usb-ehci and
> usb-ehci-with-tt probably doesn't make sense here.
> > I'm all with Alan here, to make a shortcut and allow Linux specific properties
> > which describe a specific quirk in less code lines. Other OS can just ignore
> > those and build their table based on the compatible entry if they want to.
> We should absolutely avoid Linux-specific properties where possible.
> That said, what Linux-specific properties are you talking about? The
> properties discussed here (has-synopsys-hc-bug, no-io-watchdog, has-tt)
> are all purely a description of HW, aren't they.
has-tt and has-synopsys-hc-bug are certainly hardware properties, while
no-io-watchdog is a Linux driver software workaround for a hardware issue,
so that's kind of in a grey zone to decide whether this describes hardware or
not. Let's just assume that this is a hardware issue :)
> > So usb-ehci should be fine. It is a generic USB-EHCI controller after all.
> > Quirks or no quirks, the register layout is the same, the functionality is the
> > same. If you can't map memory >4GiB then you need a quirk for this but you may
> > discover it way too late.
> > If your platform driver requires extra code for the PHY then it is still an
> > EHCI controller. The PHY wasn't setup by the bootloader / bios so Linux has to
> > deal with it.
> >> We probably can omit has-synopsys-hc-bug, as that is specific to one
> >> type of MIPS ATH79 controller. The driver can check for it explicitly,
> >> if necessary.
> >> In fact, it's not clear that these new properties need to be added now.
> >> They can be added over time as needed, as existing drivers are
> >> converted over to DT. Or is it preferable to document these things
> >> now, preemptively as it were?
> It's best to define the binding up-front so it doesn't churn, where
> possible. This will also ensure that multiple people don't try to update
> the binding document to add the same feature in different ways.
Agreed, we do support these properties in the non-DT case, so I see no reason
why we should not document them in the binding too.
> > I would add it only if required / has users.
More information about the linux-arm-kernel