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

Krzysztof Kozlowski
2025-09-11T13:07:10Z

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@subdimension.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