From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755769AbcKVSpi (ORCPT ); Tue, 22 Nov 2016 13:45:38 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:33668 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755401AbcKVSph (ORCPT ); Tue, 22 Nov 2016 13:45:37 -0500 Subject: Re: [PATCH 1/2] of: base: add support to get machine model name To: Rob Herring , Sudeep Holla References: <1479396775-32033-1-git-send-email-sudeep.holla@arm.com> <20161118144651.275xz4gu6jaefhp7@rob-hp-laptop> <582F5DC0.4080804@gmail.com> Cc: linux-kernel@vger.kernel.org, Arnd Bergmann , devicetree@vger.kernel.org From: Frank Rowand Message-ID: <5834921F.2020809@gmail.com> Date: Tue, 22 Nov 2016 10:44:47 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <582F5DC0.4080804@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rob, On 11/18/16 12:00, Frank Rowand wrote: > On 11/18/16 06:46, Rob Herring wrote: >> On Thu, Nov 17, 2016 at 03:32:54PM +0000, Sudeep Holla wrote: >>> Currently platforms/drivers needing to get the machine model name are >>> replicating the same snippet of code. In some case, the OF reference >>> counting is either missing or incorrect. >>> >>> This patch adds support to read the machine model name either using >>> the "model" or the "compatible" property in the device tree root node >>> to the core OF/DT code. >>> >>> This can be used to remove all the duplicate code snippets doing exactly >>> same thing later. >>> >>> Cc: Rob Herring >>> Cc: Frank Rowand >>> Cc: Arnd Bergmann >>> Signed-off-by: Sudeep Holla >>> --- >>> drivers/of/base.c | 32 ++++++++++++++++++++++++++++++++ >>> include/linux/of.h | 6 ++++++ >>> 2 files changed, 38 insertions(+) >>> >>> Hi Rob, >>> >>> It would be good if we can target this for v4.10, so that we have no >>> dependencies to push PATCH 2/2 in v4.11 >> >> Applied. >> >> Rob >> > > A little fast on the trigger Rob. > > -Frank This patch adds a function that leads to conflating the "model" property and the "compatible" property. This leads to opaque, confusing and unclear code where ever it is used. I think it is not good for the device tree framework to contribute to writing unclear code. Further, only two of the proposed users of this new function appear to be proper usage. I do not think that the small amount of reduced lines of code is a good trade off for the reduced code clarity and for the potential for future mis-use of this function. Can I convince you to revert this patch? If not, will you accept a patch to change the function name to more clearly indicate what it does? (One possible name would be of_model_or_1st_compatible().) -Frank From mboxrd@z Thu Jan 1 00:00:00 1970 From: Frank Rowand Subject: Re: [PATCH 1/2] of: base: add support to get machine model name Date: Tue, 22 Nov 2016 10:44:47 -0800 Message-ID: <5834921F.2020809@gmail.com> References: <1479396775-32033-1-git-send-email-sudeep.holla@arm.com> <20161118144651.275xz4gu6jaefhp7@rob-hp-laptop> <582F5DC0.4080804@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <582F5DC0.4080804-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Rob Herring , Sudeep Holla Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Arnd Bergmann , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org Hi Rob, On 11/18/16 12:00, Frank Rowand wrote: > On 11/18/16 06:46, Rob Herring wrote: >> On Thu, Nov 17, 2016 at 03:32:54PM +0000, Sudeep Holla wrote: >>> Currently platforms/drivers needing to get the machine model name are >>> replicating the same snippet of code. In some case, the OF reference >>> counting is either missing or incorrect. >>> >>> This patch adds support to read the machine model name either using >>> the "model" or the "compatible" property in the device tree root node >>> to the core OF/DT code. >>> >>> This can be used to remove all the duplicate code snippets doing exactly >>> same thing later. >>> >>> Cc: Rob Herring >>> Cc: Frank Rowand >>> Cc: Arnd Bergmann >>> Signed-off-by: Sudeep Holla >>> --- >>> drivers/of/base.c | 32 ++++++++++++++++++++++++++++++++ >>> include/linux/of.h | 6 ++++++ >>> 2 files changed, 38 insertions(+) >>> >>> Hi Rob, >>> >>> It would be good if we can target this for v4.10, so that we have no >>> dependencies to push PATCH 2/2 in v4.11 >> >> Applied. >> >> Rob >> > > A little fast on the trigger Rob. > > -Frank This patch adds a function that leads to conflating the "model" property and the "compatible" property. This leads to opaque, confusing and unclear code where ever it is used. I think it is not good for the device tree framework to contribute to writing unclear code. Further, only two of the proposed users of this new function appear to be proper usage. I do not think that the small amount of reduced lines of code is a good trade off for the reduced code clarity and for the potential for future mis-use of this function. Can I convince you to revert this patch? If not, will you accept a patch to change the function name to more clearly indicate what it does? (One possible name would be of_model_or_1st_compatible().) -Frank -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html