Re: [PATCH v1 1/1] iio: adc: qcom-spmi-adc5: Fix the channel name

[Marijn Suijten]

On 2023-01-17 11:39:44, Andy Shevchenko wrote:
> The node name can contain an address part which is not used by
> the driver. Cut it out before assigning the channel name.

This explanation doesn't cut it.  It's not that the driver "doesn't use"
the address part, it is that this string is propagated into the
userspace label, sysfs /filenames/ *and breaking ABI*.

> Fixes: 4f47a236a23d ("iio: adc: qcom-spmi-adc5: convert to device properties")
> Reported-by: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/iio/adc/qcom-spmi-adc5.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
> index e90c299c913a..c52bf6722a6a 100644
> --- a/drivers/iio/adc/qcom-spmi-adc5.c
> +++ b/drivers/iio/adc/qcom-spmi-adc5.c
> @@ -628,12 +628,19 @@ static int adc5_get_fw_channel_data(struct adc5_chip *adc,
>  				    struct fwnode_handle *fwnode,
>  				    const struct adc5_data *data)
>  {
> -	const char *name = fwnode_get_name(fwnode), *channel_name;
> +	const char *name, *channel_name;

I don't think this'll compile as name is still a pointer to const data,
while you're assigning (a '\0' char) to it below.

- Marijn

>  	u32 chan, value, varr[2];
>  	u32 sid = 0;
>  	int ret;
>  	struct device *dev = adc->dev;
>  
> +	name = devm_kasprintf(dev, GFP_KERNEL, "%pfwP", fwnode);
> +	if (!name)
> +		return -ENOMEM;
> +
> +	/* Cut the address part */
> +	name[strchrnul(name, '@') - name] = '\0';
> +
>  	ret = fwnode_property_read_u32(fwnode, "reg", &chan);
>  	if (ret) {
>  		dev_err(dev, "invalid channel number %s\n", name);
> -- 
> 2.39.0
>