All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
Date: Wed, 24 Oct 2012 10:45:13 -0600	[thread overview]
Message-ID: <50881B19.3080609@wwwdotorg.org> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1210241233040.1282-100000@iolanthe.rowland.org>

On 10/24/2012 10:38 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, 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)
> 
> Yes, it does.
> 
>> 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.
> 
> Which might be difficult since the inner loop would be controlled by
> the outer code in the driver core.
> 
> How do we determine which existing drivers claim to support usb-ehci?  
> A quick search under arch/ and drivers/ turns up nothing but
> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> match should be easy enough, and then "usb-ehci" would be safe to use
> in ehci-platform.c.

It's not drivers that claim to support usb-ehci, but device tree files
that contain nodes that include "usb-ehci" in their compatible value,
yet need to be handled by a driver that isn't the generic USB EHCI
driver, and that driver binds to the other more specific compatible value:

> $ grep -HrnI usb-ehci arch/*/boot/dts
> arch/arm/boot/dts/spear3xx.dtsi:76:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9x5.dtsi:399:			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:145:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:152:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:215:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:224:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:232:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:88:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:96:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9g45.dtsi:392:			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/powerpc/boot/dts/sequoia.dts:156:			compatible = "ibm,usb-ehci-440epx", "usb-ehci";
> arch/powerpc/boot/dts/wii.dts:120:			compatible = "nintendo,hollywood-usb-ehci",
> arch/powerpc/boot/dts/wii.dts:121:					"usb-ehci";
> arch/powerpc/boot/dts/canyonlands.dts:167:			compatible = "ibm,usb-ehci-460ex", "usb-ehci";

and then search for all those other compatible values in drivers. The
ARM instances certainly all have this issue (although perhaps it's worth
investigating if a generic could work for them instead). The PPC
instances need more investigation; the values don't show up in of match
tables but rather in code.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Cc: Sebastian Andrzej Siewior
	<sebastian-E0PNVn5OA6ohrxcnuTQ+TQ@public.gmane.org>,
	Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Tony Prisk <linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>,
	Greg KH
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	USB list <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	Florian Fainelli
	<florian-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
Date: Wed, 24 Oct 2012 10:45:13 -0600	[thread overview]
Message-ID: <50881B19.3080609@wwwdotorg.org> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1210241233040.1282-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

On 10/24/2012 10:38 AM, Alan Stern wrote:
> On Wed, 24 Oct 2012, 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)
> 
> Yes, it does.
> 
>> 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.
> 
> Which might be difficult since the inner loop would be controlled by
> the outer code in the driver core.
> 
> How do we determine which existing drivers claim to support usb-ehci?  
> A quick search under arch/ and drivers/ turns up nothing but
> drivers/usb/host/ehci-ppc-of.c.  Changing it to a more HW-specific
> match should be easy enough, and then "usb-ehci" would be safe to use
> in ehci-platform.c.

It's not drivers that claim to support usb-ehci, but device tree files
that contain nodes that include "usb-ehci" in their compatible value,
yet need to be handled by a driver that isn't the generic USB EHCI
driver, and that driver binds to the other more specific compatible value:

> $ grep -HrnI usb-ehci arch/*/boot/dts
> arch/arm/boot/dts/spear3xx.dtsi:76:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9x5.dtsi:399:			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:145:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear13xx.dtsi:152:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:215:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:224:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/tegra20.dtsip:232:		compatible = "nvidia,tegra20-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:88:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/spear600.dtsi:96:			compatible = "st,spear600-ehci", "usb-ehci";
> arch/arm/boot/dts/at91sam9g45.dtsi:392:			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";
> arch/powerpc/boot/dts/sequoia.dts:156:			compatible = "ibm,usb-ehci-440epx", "usb-ehci";
> arch/powerpc/boot/dts/wii.dts:120:			compatible = "nintendo,hollywood-usb-ehci",
> arch/powerpc/boot/dts/wii.dts:121:					"usb-ehci";
> arch/powerpc/boot/dts/canyonlands.dts:167:			compatible = "ibm,usb-ehci-460ex", "usb-ehci";

and then search for all those other compatible values in drivers. The
ARM instances certainly all have this issue (although perhaps it's worth
investigating if a generic could work for them instead). The PPC
instances need more investigation; the values don't show up in of match
tables but rather in code.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2012-10-24 16:45 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-20 22:10 [PATCH v2 0/2] Update ehci-platform driver to support devicetree Tony Prisk
2012-10-20 22:10 ` Tony Prisk
2012-10-20 22:10 ` [PATCH v2 1/2] USB: Update EHCI-platform driver to devicetree Tony Prisk
2012-10-20 22:10   ` Tony Prisk
2012-10-21  2:02   ` Alan Stern
2012-10-21  2:02     ` Alan Stern
     [not found]   ` <1350771032-11527-2-git-send-email-linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>
2012-10-22 14:51     ` Alan Stern
2012-10-20 22:10 ` [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver Tony Prisk
2012-10-20 22:10   ` Tony Prisk
2012-10-21 17:34   ` Florian Fainelli
2012-10-21 17:34     ` Florian Fainelli
2012-10-22 16:07   ` Stephen Warren
2012-10-22 16:07     ` Stephen Warren
2012-10-22 17:34     ` Alan Stern
2012-10-22 17:34       ` Alan Stern
2012-10-22 17:48       ` Stephen Warren
2012-10-22 17:48         ` Stephen Warren
2012-10-22 19:00         ` Alan Stern
2012-10-22 19:00           ` Alan Stern
2012-10-22 22:10           ` Stephen Warren
2012-10-22 22:10             ` Stephen Warren
2012-10-23 14:10             ` Alan Stern
2012-10-23 14:10               ` Alan Stern
2012-10-23 16:15               ` Stephen Warren
2012-10-23 16:15                 ` Stephen Warren
2012-10-23 17:59                 ` Alan Stern
2012-10-23 17:59                   ` Alan Stern
2012-10-23 18:47                   ` Stephen Warren
2012-10-23 18:47                     ` Stephen Warren
2012-10-23 19:33                     ` Alan Stern
2012-10-23 19:33                       ` Alan Stern
2012-10-23 20:06                       ` Rob Herring
2012-10-23 20:06                         ` Rob Herring
2012-10-24 14:57                         ` Alan Stern
2012-10-24 14:57                           ` Alan Stern
2012-10-24 15:26                           ` Sebastian Andrzej Siewior
2012-10-24 15:26                             ` Sebastian Andrzej Siewior
2012-10-24 16:16                             ` Stephen Warren
2012-10-24 16:16                               ` Stephen Warren
2012-10-24 16:36                               ` Florian Fainelli
2012-10-24 16:36                                 ` Florian Fainelli
2012-10-24 16:38                               ` Alan Stern
2012-10-24 16:38                                 ` Alan Stern
2012-10-24 16:44                                 ` Florian Fainelli
2012-10-24 16:44                                   ` Florian Fainelli
2012-10-24 18:04                                   ` Alan Stern
2012-10-24 18:04                                     ` Alan Stern
2012-10-24 18:18                                     ` Florian Fainelli
2012-10-24 18:18                                       ` Florian Fainelli
2012-10-24 16:45                                 ` Stephen Warren [this message]
2012-10-24 16:45                                   ` Stephen Warren
2012-10-24 17:46                                   ` Alan Stern
2012-10-24 17:46                                     ` Alan Stern
2012-10-24 18:09                                     ` Stephen Warren
2012-10-24 18:09                                       ` Stephen Warren
2012-10-24 18:55                                       ` Mitch Bradley
2012-10-24 18:55                                         ` Mitch Bradley
2012-10-24 19:30                                         ` Alan Stern
2012-10-24 19:30                                           ` Alan Stern
2012-10-25 10:23                                         ` Sebastian Andrzej Siewior
2012-10-25 10:23                                           ` Sebastian Andrzej Siewior
2012-10-25 14:36                                           ` Alan Stern
2012-10-25 14:36                                             ` Alan Stern
2012-10-26  8:02                                             ` Sebastian Andrzej Siewior
2012-10-26  8:02                                               ` Sebastian Andrzej Siewior
2012-10-26 14:54                                               ` Alan Stern
2012-10-26 14:54                                                 ` Alan Stern
2012-10-25 15:53                                           ` Stephen Warren
2012-10-25 15:53                                             ` Stephen Warren
2012-10-24 19:41                                       ` Alan Stern
2012-10-24 19:41                                         ` Alan Stern
2012-10-24 16:44                               ` Alan Stern
2012-10-24 16:44                                 ` Alan Stern
2012-10-24 16:48                                 ` Stephen Warren
2012-10-24 16:48                                   ` Stephen Warren
2012-10-24 17:42                                 ` Rob Herring
2012-10-24 17:42                                   ` Rob Herring
2012-10-24 17:57                                   ` Alan Stern
2012-10-24 17:57                                     ` Alan Stern
2012-10-24 16:28                           ` Stephen Warren
2012-10-24 16:28                             ` Stephen Warren
2012-10-24 16:54                             ` Alan Stern
2012-10-24 16:54                               ` Alan Stern
2012-10-24 17:37                               ` Florian Fainelli
2012-10-24 17:37                                 ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50881B19.3080609@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.