Re: [PATCH 1/2] of: base: add support to get machine model name
From: Rob Herring
Date: Fri Dec 09 2016 - 11:03:39 EST
On Wed, Nov 23, 2016 at 4:25 AM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> On 22/11/16 21:35, Rob Herring wrote:
>> On Tue, Nov 22, 2016 at 12:44 PM, Frank Rowand <frowand.list@xxxxxxxxx>
>>> This patch adds a function that leads to conflating the "model" property
>>> and the "compatible" property. This leads to opaque, confusing and
>>> 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?
>> Yes, I will revert.
I looked at this again and the users. They are all informational, so
I'm not worried if a compatible string could be returned with this
change. The function returns the best name for the machine and having
consistency is a good thing.
I was considering not reverting (as I'd not yet gotten around to it),
but I'm still going to revert for the naming.
>>> If not, will you accept a patch to change the function name to more
>>> clearly indicate what it does? (One possible name would be
>> I took it as there's already the FDT equivalent function.
> Yes it was mainly for non of_flat_* replacement for
I would suggest just of_get_machine_name().
You might also add a fallback to return "unknown", and drop some of
the custom strings. I don't think anyone should care about the actual
string. However, it's an error to have a DT with no model or top level
compatible, so maybe a WARN.