From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <500D7356.5080706@freescale.com> Date: Mon, 23 Jul 2012 10:52:54 -0500 From: Scott Wood MIME-Version: 1.0 To: Rob Herring Subject: Re: [PATCH] of: require a match on all fields of of_device_id References: <20120718011151.GA6119@tyr.buserror.net> <50062199.7090904@gmail.com> <5006DE87.7020503@freescale.com> <500CAF49.1060003@gmail.com> In-Reply-To: <500CAF49.1060003@gmail.com> Content-Type: text/plain; charset="UTF-8" Cc: devicetree-discuss@lists.ozlabs.org, Thierry Reding , linuxppc-dev@lists.ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH] of: require a match on all fields of of_device_id Date: Mon, 23 Jul 2012 10:52:54 -0500 Message-ID: <500D7356.5080706@freescale.com> References: <20120718011151.GA6119@tyr.buserror.net> <50062199.7090904@gmail.com> <5006DE87.7020503@freescale.com> <500CAF49.1060003@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <500CAF49.1060003-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Rob Herring Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org List-Id: devicetree@vger.kernel.org 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