linux-kernel - Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init stage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <391229ff-d85a-4707-8e7c-ea64e0e3d7cb@kernel.org>
Date: Thu, 11 Sep 2025 15:07:05 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Petre Rodan <petre.rodan@...dimension.ro>
Cc: Jonathan Cameron <jic23@...nel.org>, David Lechner
 <dlechner@...libre.com>, Nuno S?? <nuno.sa@...log.com>,
 Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 07/14] iio: accel: bma220: reset registers during init
 stage

On 11/09/2025 14:36, Petre Rodan wrote:
> 
> Hi Krzysztof,
> 
> On Thu, Sep 11, 2025 at 09:35:52AM +0200, Krzysztof Kozlowski wrote:
>> On 10/09/2025 09:57, Petre Rodan wrote:
>>> Bring all configuration registers to default values during device probe().
>>>
>>> Signed-off-by: Petre Rodan <petre.rodan@...dimension.ro>
>>> ---
>>>  drivers/iio/accel/bma220_core.c | 71 ++++++++++++++++++++++++++++-------------
>>>  1 file changed, 49 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/bma220_core.c b/drivers/iio/accel/bma220_core.c
>>> index b6f1374a9cca52966c1055113710061a7284cf5a..322df516c90a7c645eeca579cae9803eb31caad1 100644
>>> --- a/drivers/iio/accel/bma220_core.c
>>> -static int bma220_init(struct spi_device *spi)
>>> +static int bma220_reset(struct spi_device *spi, bool up)
>>>  {
>>> -	int ret;
>>> -	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
>>> +	int i, ret;
>>>  
>>> -	ret = devm_regulator_bulk_get_enable(&spi->dev,
>>
>>
>> You just added this code in patch 6. Don't add code which immediately
>> you remove. I understand you re-add this later, so basically it is a
>> move, but such patch diff is still confusing.
> 
> sorry, but this is an artefact of 'git diff' I don't think I have no control of.


Don't think so. Before bma220_init() was above bma220_power(). After
your patch bma220_init() is BELOW bma220_power(), so that's a move.



> 
> the bma220_reset() function was added to bma220_core.c with this patch and the
> diff process merged lines from this new function with lines from bma220_init()
> causing the apparent removal of the lines added in the previous patch.
> if you look a few lines below your cut, the bma220_init() function contains the
> code:
> 
> +static int bma220_init(struct spi_device *spi)
> +{
> +	int ret;
> +	static const char * const regulator_names[] = { "vddd", "vddio", "vdda" };
> +
> +	ret = devm_regulator_bulk_get_enable(&spi->dev,
> +					     ARRAY_SIZE(regulator_names),
> +					     regulator_names);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret, "Failed to get regulators\n");
> [..]
> 
> Just for my curiosity, do reviewers apply the patches one by one to (a branch of)
> the tree itself or do they provide feedback directly based on the diffs?


Each commit must stand on its own, is reviewed independently and entire
patchset must be 100% bisectable.


Best regards,
Krzysztof