Re: [PATCH v5 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[David Lechner]

On 9/16/25 3:14 AM, Matti Vaittinen wrote:
> On 16/09/2025 11:02, Jonathan Cameron wrote:
>> On Tue, 16 Sep 2025 07:52:07 +0300
>> Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
>>
>>> On 15/09/2025 23:13, Jonathan Cameron wrote:
>>>> On Mon, 15 Sep 2025 17:12:34 +0300
>>>> Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
>>>>   
>>>>> On Mon, Sep 15, 2025 at 10:12:43AM +0300, Matti Vaittinen wrote:
> 
>>>> --- a/drivers/iio/adc/rohm-bd79112.c
>>>> +++ b/drivers/iio/adc/rohm-bd79112.c
>>>> @@ -454,12 +454,18 @@ static int bd79112_probe(struct spi_device *spi)
>>>>           data->read_xfer[1].rx_buf = &data->read_rx;
>>>>           data->read_xfer[1].len = sizeof(data->read_rx);
>>>>           spi_message_init_with_transfers(&data->read_msg, data->read_xfer, 2);
>>>> -       devm_spi_optimize_message(dev, spi, &data->read_msg);
>>>> +       ret = devm_spi_optimize_message(dev, spi, &data->read_msg);
>>>> +       if (ret < 0)
>>>> +               return dev_err_probe(dev, ret,
>>>> +                                    "Failed to optimize SPI read message\n");
>>>>      
>>>
>>> I am not really sure under what conditions the
>>> devm_spi_optimize_message() could fail. It might be enough to print a
>>> warning and proceed, but I don't think returning is a problem either.
>>
>> No. Don't proceed on an unexpected failure whatever it is.  That's
>> storing up problems that may surface in a weird way later that is much
>> harder to debug.
> 
> Just a generic note, not disagreeing in this case.
> 
> I have had similar discussions before - and I have been on the both sides of the table. Hence, I don't have as strong stance on this as you. On some situations it is better to just try proceeding as aborting the operation brings no sane corrective actions but just reduces a device unusable.
> 
> On the other hand, as you say, usually bailing out loud and early is the best way to pinpoint the problem and get things fixed.
> 
> I still think that logging a warning should be a decent hint for someone doing the debugging.
> 
> Well, as I said, returning here is Ok for me - thanks for taking care of it! :)
> 
> Yours,
>     -- Matti

For devm_spi_optimize_message() specifically, there is no
point to continue after an error. The call of this will
just be deferred until the first SPI transfer and the same
error will happen again. If there is an error, it means there
is a programmer error and the SPI message is malformed
(or could be memory allocation failure). So better to fail
early anyway.