linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: Rob Herring <robherring2@gmail.com>
Cc: devicetree-discuss@lists.ozlabs.org,
	Thierry Reding <thierry.reding@avionic-design.de>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] of: require a match on all fields of of_device_id
Date: Mon, 23 Jul 2012 10:52:54 -0500	[thread overview]
Message-ID: <500D7356.5080706@freescale.com> (raw)
In-Reply-To: <500CAF49.1060003@gmail.com>

On 07/22/2012 08:56 PM, Rob Herring wrote:
> On 07/18/2012 11:04 AM, Scott Wood wrote:
>> On 07/17/2012 09:38 PM, Rob Herring wrote:
>>> On 07/17/2012 08:11 PM, Scott Wood wrote:
>>>> Commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6 ("of: match by compatible
>>>> property first") breaks the gianfar ethernet driver found on various
>>>> Freescale PPC chips.
>>>
>>> You do know this is reverted, right?
>>
>> No, I didn't.  I got it via Kumar's next branch, and saw that it was
>> still in your fixes-for-grant branch, and didn't see any revert-related
>> e-mail activity on the devicetree-discuss list about it.  I now see that
>> it was reverted directly in Linus's tree (I didn't see either the
>> original or the revert in Linus's tree when I checked, but apparently I
>> hadn't fetched that as recently as I thought).
>>
>>> Here's my fix (untested) which is a bit simpler. I'm assuming if we care
>>> about which compatible string we are matching to, then we require name
>>> and type are blank and we only care about compatible strings.
>>
>> Any particular reason for making that assumption?  We should be avoiding
>> the need for .name or .type matching in new bindings, but this seems
>> like unnecessarily inconsistent behavior.
> 
> Only to ensure we don't change existing behavior and I think trying to
> cover all possibilities will be nearly impossible. For example, what if
> we match on both a specific compatible prop alone and a less specific
> compatible prop plus name and/or type. Which one do we pick as the
> better match?

Whichever one has a compatible string that is listed earlier.  Having an
additional type/name field doesn't necessarily make it a better match --
it's just there to resolve ambiguity (or sometimes for no good reason at
all).

You're going to change existing behavior in some case no matter what,
short of a match table flag saying "it's OK to not keep depending on
link order", or just not making the change.  Might as well change it to
something sane.

-Scott

      reply	other threads:[~2012-07-23 15:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-18  1:11 [PATCH] of: require a match on all fields of of_device_id Scott Wood
2012-07-18  1:57 ` Tabi Timur-B04825
2012-07-18  2:38 ` Rob Herring
2012-07-18 16:04   ` Scott Wood
2012-07-23  1:56     ` Rob Herring
2012-07-23 15:52       ` Scott Wood [this message]

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=500D7356.5080706@freescale.com \
    --to=scottwood@freescale.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=robherring2@gmail.com \
    --cc=thierry.reding@avionic-design.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).