Subject: Re: [PATCH 4/5] hwmon: (ibmpowernv) change create_hwmon_attr_name() prototype

From: Guenter Roeck <[email-masked]>
Date: Wed, 18 Mar 2015 21:02:02 -0700

On 03/18/2015 08:47 AM, Cédric Le Goater wrote:
> It simplifies the creation of the hwmon attributes and will help when
> support for a new device tree layout is added. The patch also changes
> the name of the routine to parse_opal_node_name().
>
> Signed-off-by: Cédric Le Goater <[email-masked]>
> ---
>   drivers/hwmon/ibmpowernv.c |   32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index edcc4ffa5ad0..570d2360b698 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -155,28 +155,22 @@ static const char *convert_opal_attr_name(enum sensors type,
>    * which need to be mapped as fan2_input, temp1_max respectively before
>    * populating them inside hwmon device class.
>    */
> -static int create_hwmon_attr_name(struct device *dev, enum sensors type,
> -					 const char *node_name,
> -					 char *hwmon_attr_name)
> +static int parse_opal_node_name(const char *node_name, enum sensors type,
> +	u32 *index, const char **hwmon_attr_name)
>   {

Please have the function return const char * and merge the error code with ERR_PTR.

>   	char attr_suffix[MAX_ATTR_LEN];
>   	const char *attr_name;
> -	u32 index;
>   	int err;
>
> -	err = get_sensor_index_attr(node_name, &index, attr_suffix);
> -	if (err) {
> -		dev_err(dev, "Sensor device node name '%s' is invalid\n",
> -			node_name);
> +	err = get_sensor_index_attr(node_name, index, attr_suffix);
> +	if (err)
>   		return err;
> -	}
>
>   	attr_name = convert_opal_attr_name(type, attr_suffix);
>   	if (!attr_name)
>   		return -ENOENT;
>
> -	snprintf(hwmon_attr_name, MAX_ATTR_LEN, "%s%d_%s",
> -		 sensor_groups[type].name, index, attr_name);
> +	*hwmon_attr_name = attr_name;
>   	return 0;
>   }
>
> @@ -252,6 +246,9 @@ static int create_device_attrs(struct platform_device *pdev)
>   	}
>
>   	for_each_child_of_node(opal, np) {
> +		const char *attr_name;
> +		u32 opal_index;
> +
>   		if (np->name == NULL)
>   			continue;
>
> @@ -268,10 +265,17 @@ static int create_device_attrs(struct platform_device *pdev)
>
>   		sdata[count].id = sensor_id;
>   		sdata[count].type = type;
> -		err = create_hwmon_attr_name(&pdev->dev, type, np->name,
> -					     sdata[count].name);
> -		if (err)
> +
> +		err = parse_opal_node_name(np->name, type, &opal_index,
> +					   &attr_name);
> +		if (err) {
> +			dev_err(&pdev->dev, "Sensor device node name '%s' is invalid\n",
> +				np->name);
>   			goto exit_put_node;
> +		}
> +
> +		snprintf(sdata[count].name, MAX_ATTR_LEN, "%s%d_%s",
> +			 sensor_groups[type].name, opal_index, attr_name);
>
>   		sysfs_attr_init(&sdata[count].dev_attr.attr);
>   		sdata[count].dev_attr.attr.name = sdata[count].name;
>