Re: [PATCH 1/2] of: base: add support to get machine model name

From: Frank Rowand
Date: Thu Nov 17 2016 - 17:13:53 EST


On 11/17/16 13:00, Frank Rowand wrote:
> On 11/17/16 07:32, 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.
>
> I find five instances of reading only property "model":
>
> arch/arm/mach-imx/cpu.c
> arch/arm/mach-mxs/mach-mxs.c
> arch/c6x/kernel/setup.c
> arch/mips/cavium-octeon/setup.c
> arch/sh/boards/of-generic.c

My initial search was a little too strict. With a less restrictive
search I find 16 more instances of reading property "model" and
not reading property "compatible".

>
> I find one instance of reading property "model", then if
> that does not exist, property "compatible":
>
> arch/mips/generic/proc.c
>
> The proposed patch matches the code used in one place, and thus
> current usage does not match the patch description.
>
> Is my search bad? Are you planning to add additional instances
> of reading "model" then "compatible"?
>
> -Frank
>
>>
>> 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
>>
>> Regards,
>> Sudeep
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index a0bccb54a9bd..0810c5ecf1aa 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -546,6 +546,38 @@ int of_machine_is_compatible(const char *compat)
>> EXPORT_SYMBOL(of_machine_is_compatible);
>>
>> /**
>> + * of_machine_get_model_name - Find and read the model name or the compatible
>> + * value for the machine.
>> + * @model: pointer to null terminated return string, modified only if
>> + * return value is 0.
>> + *
>> + * Returns a string containing either the model name or the compatible value
>> + * of the machine if found, else return error.
>> + *
>> + * Search for a machine model name or the compatible if model name is missing
>> + * in a device tree node and retrieve a null terminated string value (pointer
>> + * to data, not a copy). Returns 0 on success, -EINVAL if root of the device
>> + * tree is not found and other error returned by of_property_read_string on
>> + * failure.
>> + */
>> +int of_machine_get_model_name(const char **model)
>> +{
>> + int error;
>> +
>> + if (!of_node_get(of_root))
>> + return -EINVAL;
>> +
>> + error = of_property_read_string(of_root, "model", model);
>> + if (error)
>> + error = of_property_read_string_index(of_root, "compatible",
>> + 0, model);
>> + of_node_put(of_root);
>> +
>> + return error;
>> +}
>> +EXPORT_SYMBOL(of_machine_get_model_name);
>> +
>> +/**
>> * __of_device_is_available - check if a device is available for use
>> *
>> * @device: Node to check for availability, with locks already held
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index d72f01009297..13fc66531f1b 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -367,6 +367,7 @@ extern int of_alias_get_id(struct device_node *np, const char *stem);
>> extern int of_alias_get_highest_id(const char *stem);
>>
>> extern int of_machine_is_compatible(const char *compat);
>> +extern int of_machine_get_model_name(const char **model);
>>
>> extern int of_add_property(struct device_node *np, struct property *prop);
>> extern int of_remove_property(struct device_node *np, struct property *prop);
>> @@ -788,6 +789,11 @@ static inline int of_machine_is_compatible(const char *compat)
>> return 0;
>> }
>>
>> +static inline int of_machine_get_model_name(const char **model)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> static inline bool of_console_check(const struct device_node *dn, const char *name, int index)
>> {
>> return false;
>> --
>> 2.7.4
>>
>>
>
>