Received-SPF: pass (google.com: domain of linux-kernel+bounces-129476-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <1d777161-7d86-4d45-91bc-c7653504b890@gmail.com>
Date: Wed, 3 Apr 2024 13:01:51 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/6] iio: adc: ad7173: refactor channel configuration
 parsing
To: David Lechner <dlechner@baylibre.com>, dumitru.ceclan@analog.com
Cc: Lars-Peter Clausen <lars@metafoo.de>,
 Michael Hennerich <Michael.Hennerich@analog.com>,
 Jonathan Cameron <jic23@kernel.org>, Rob Herring <robh@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240401-ad4111-v1-0-34618a9cc502@analog.com>
 <20240401-ad4111-v1-3-34618a9cc502@analog.com>
 <CAMknhBHo3xkkZymVfvauL7nBPn9+c1ZUoPwyk4mJO4syRJEhiQ@mail.gmail.com>
Content-Language: en-US
From: "Ceclan, Dumitru" <mitrutzceclan@gmail.com>
In-Reply-To: <CAMknhBHo3xkkZymVfvauL7nBPn9+c1ZUoPwyk4mJO4syRJEhiQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
"
https://lore.kernel.org/all/20240303162148.3ad91aa2@jic23-huawei/