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: Mon, 22 Oct 2012 16:10:56 -0600	[thread overview]
Message-ID: <5085C470.4040707@wwwdotorg.org> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1210221443580.1724-100000@iolanthe.rowland.org>

On 10/22/2012 01:00 PM, Alan Stern wrote:
> On Mon, 22 Oct 2012, Stephen Warren wrote:
> 
>>>>> +- has-tt : controller has transaction translator(s).
>>>>> +- has-synopsys-hc-bug : controller has the synopsys hc bug
>>>>
>>>> That would normally be determined by the driver based on the particular
>>>> compatible value that is in device tree.
>>>
>>> I don't understand this comment.  Isn't "has-synopsys-hc-bug" the 
>>> compatible value in question?
>>
>> "compatible value" in this context means that value of the property
>> named "compatible".
> 
> I see.  But why would it be done this way instead having a separate 
> property?

Well, I did say normally:-)

I can certainly see an argument for representing these differences using
custom properties, rather than deriving the information from the
compatible value. It's probably be OK to do so for something generic
like this; it's just perhaps not always the default choice.

Do note that even though this binding document dictates a particular
value for the compatible property, every device tree should additionally
add a separate value alongside it to indicate the specific HW model
that's actually present, so that if some device-specific bug-fix or
workaround needs to be applied, the model can be identified anyway.

So, rather than:

compatible = "usb-ehci";

You should always have e.g.:

compatible = "nvidia,tegra20-ehci", "usb-ehci";

Given that, there is then always enough information in the device tree
for the driver to be able to derive the other values from the compatible
value.

Whether you want to derive the information, or whether you want to
explicitly represent it via properties, is a decision to make based on
the trade-offs.

Oh, and I note that quite a few device trees already use compatible
value "usb-ehci" in their device-trees. Care needs to be taken not to
usurp that value from any existing device drivers if that was to be
picked as the compatible value required by this binding.

> And doesn't the same reasoning apply to has-tt?  Doesn't that mean the 
> driver would have to match four different hardware types?  What happens 
> if a third characteristic like these comes around; would the driver 
> then have to check against eight different types?

No, the compatible value represents the model, so you'd have a table like:

compatible          -> bugX has_tt
nvidia,tegra20-ehci -> 0    1
vendor1,foo-ehci    -> 0    1
vendor2,bar-ehci    -> 1    1
vendor3,baz-ehci    -> 0    1
vendor4,qux-ehci    -> 0    1
...

So the table size isn't related to the number of options. The table size
is probably bigger than subset of options combinations that make sense.

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: Tony Prisk <linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>,
	Greg KH
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Florian Fainelli
	<florian-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Subject: Re: [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
Date: Mon, 22 Oct 2012 16:10:56 -0600	[thread overview]
Message-ID: <5085C470.4040707@wwwdotorg.org> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1210221443580.1724-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

On 10/22/2012 01:00 PM, Alan Stern wrote:
> On Mon, 22 Oct 2012, Stephen Warren wrote:
> 
>>>>> +- has-tt : controller has transaction translator(s).
>>>>> +- has-synopsys-hc-bug : controller has the synopsys hc bug
>>>>
>>>> That would normally be determined by the driver based on the particular
>>>> compatible value that is in device tree.
>>>
>>> I don't understand this comment.  Isn't "has-synopsys-hc-bug" the 
>>> compatible value in question?
>>
>> "compatible value" in this context means that value of the property
>> named "compatible".
> 
> I see.  But why would it be done this way instead having a separate 
> property?

Well, I did say normally:-)

I can certainly see an argument for representing these differences using
custom properties, rather than deriving the information from the
compatible value. It's probably be OK to do so for something generic
like this; it's just perhaps not always the default choice.

Do note that even though this binding document dictates a particular
value for the compatible property, every device tree should additionally
add a separate value alongside it to indicate the specific HW model
that's actually present, so that if some device-specific bug-fix or
workaround needs to be applied, the model can be identified anyway.

So, rather than:

compatible = "usb-ehci";

You should always have e.g.:

compatible = "nvidia,tegra20-ehci", "usb-ehci";

Given that, there is then always enough information in the device tree
for the driver to be able to derive the other values from the compatible
value.

Whether you want to derive the information, or whether you want to
explicitly represent it via properties, is a decision to make based on
the trade-offs.

Oh, and I note that quite a few device trees already use compatible
value "usb-ehci" in their device-trees. Care needs to be taken not to
usurp that value from any existing device drivers if that was to be
picked as the compatible value required by this binding.

> And doesn't the same reasoning apply to has-tt?  Doesn't that mean the 
> driver would have to match four different hardware types?  What happens 
> if a third characteristic like these comes around; would the driver 
> then have to check against eight different types?

No, the compatible value represents the model, so you'd have a table like:

compatible          -> bugX has_tt
nvidia,tegra20-ehci -> 0    1
vendor1,foo-ehci    -> 0    1
vendor2,bar-ehci    -> 1    1
vendor3,baz-ehci    -> 0    1
vendor4,qux-ehci    -> 0    1
...

So the table size isn't related to the number of options. The table size
is probably bigger than subset of options combinations that make sense.
--
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

  reply	other threads:[~2012-10-22 22:10 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 [this message]
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
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=5085C470.4040707@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.