Re: [PATCH v6 6/7] iio: ssp_sensors: Use dev_err_probe

[Sanjay Chitroda]

On 15 April 2026 3:09:56 pm IST, Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
>On Wed, Apr 15, 2026 at 10:37:48AM +0530, Sanjay Chitroda wrote:
>
>> dev_err_probe() makes error code handling simpler and handle
>> deferred probe nicely (avoid spamming logs).
>
>...
>
>>  	struct ssp_data *data;
>
>While at it, introduce a temporary variable
>
>	struct device *dev = &spi->dev;
>
>and reuse it below. Note, you may add a new patch to reuse it in the lines that
>are not related to this patch but may utilise the local variable as well.
>
>Actually, you can introduce it earlier in the series, exempli gratia when you
>moved probe to use managed resources.
>

Thank you for the input.
I will add a change to use dev, before devm changes in next series.

>>  	data = ssp_parse_dt(&spi->dev);
>> -	if (!data) {
>> -		dev_err(&spi->dev, "Failed to find platform data\n");
>> -		return -ENODEV;
>> -	}
>> +	if (!data)
>> +		return dev_err_probe(&spi->dev, -ENODEV,
>> +				     "Failed to find platform data\n");
>
>		return dev_err_probe(dev, -ENODEV, "Failed to find platform data\n");
>
>(And you can leave it one line as trailing string literals are fine to be
> longer 80 even in strict mode. You may even double check with checkpatch,
> it won't complain.)
>
>...and so on...
>

Yes, I'm bit obsessed with 72.
I am considering all new changes with possible single line since last week, again thank you for drawing my attention.
>...
>
>>  	if (data->fw_dl_state == SSP_FW_DL_STATE_NONE) {
>>  		ret = ssp_initialize_mcu(data);
>> -		if (ret < 0) {
>> -			dev_err(&spi->dev, "Initialize_mcu failed\n");
>> -			return ret;
>> -		}
>> -	} else {
>> -		dev_err(&spi->dev, "Firmware version not supported\n");
>> -		return -EPERM;
>> -	}
>> +		if (ret < 0)
>> +			return dev_err_probe(&spi->dev, ret,
>> +					     "Initialize_mcu failed\n");
>> +	} else
>> +		return dev_err_probe(&spi->dev, -EPERM,
>> +				     "Firmware version not supported\n");
>
>I would now negate the conditional and drop redundant 'else'. Also note
>the missing {} in the 'else' branch as per Coding Style.
>
>	if (data->fw_dl_state != SSP_FW_DL_STATE_NONE)
>		return dev_err_probe(dev, -EPERM, "Firmware version not supported\n");
>
>	ret = ssp_initialize_mcu(data);
>	if (ret < 0)
>		return dev_err_probe(dev, ret, "Initialize_mcu failed\n");
>

Noted, {} to match with if block style.

I will send changes in next series, drop redundant else and simplify the logic.