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 <robh+dt@xxxxxxxxxx> >>> Cc: Frank Rowand <frowand.list@xxxxxxxxx> >>> Cc: Arnd Bergmann <arnd@xxxxxxxx> >>> Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx> >>> --- >>> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
