Re: [PATCH 3/6] iio: adc: ad7173: refactor channel configuration parsing

Ceclan, Dumitru
2024-04-03T10:01:56Z

On 01/04/2024 22:39, David Lechner wrote:
> On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>
>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>
>> Move configurations regarding number of channels from
>> *_fw_parse_device_config to *_fw_parse_channel_config.
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
>> ---
>
> Commit messages need to explain _why_ the change is being made [1]. It
> is not obvious to me why this needs to be moved.
>
> [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

Jonathan Cameron:

"
> + if (num_channels == 0)
> + return dev_err_probe(dev, -ENODATA, "No channels specified\n");
> + indio_dev->num_channels = num_channels;
> + st->num_channels = num_channels;

I'm not seeing benefit of duplication here really and logically it feels like
a lot of this last chunk would sit better in ad7173_fw_parse_channel_config()

Perhaps that's a job for a future tidying up patch.
"