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

[Frank Rowand <frowand.list@gmail.com>]

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@kernel.org>
>> Cc: Frank Rowand <frowand.list@gmail.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  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
>>
>>
> 
>