Re: [PATCH v3 04/18] iio: accel: bma220: split original driver

[Petre Rodan <petre.rodan@subdimension.ro>]

[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]


hi Andy,

On Sun, Sep 14, 2025 at 03:45:07PM +0300, Andy Shevchenko wrote:
> >           To compile this driver as a module, choose M here: the
> > -         module will be called bma220_spi.
> > +         module will be called bma220_core and you will also get
> > +         bma220_spi if SPI is enabled.
> 
> I'm not sure the last part is practical, as one needs to list all
> buses next time a new one will be added.

it's just a friendly note to the user of how the modules are called.
the sensor supports only spi and i2c, so the list is short.

> > +++ b/drivers/iio/accel/bma220.h
> 
> > +#ifndef _BMA220_H
> > +#define _BMA220_H
> > +
> > +#include <linux/pm.h>
> > +
> > +extern const struct dev_pm_ops bma220_pm_ops;
> 
> > +struct spi_device;
> 
> Besides the location of this (I would expect it to follow up include
> linux/*) convert the existing driver to regmap first and remove this
> unneeded churn.

[..]

> I haven't reviewed the rest as I believe it's just ~1:1 copy of the
> existing code, but I still think that the result will be better if
> this series starts from small fixes, like kernel doc, and other
> things, followed by the regmap conversion and only _after_ the split
> is made.

either way you look at it it can be seen as churn.

scenario 1
    split is done early: ~300 lines from _spi.c have to move to _core.c.
    I tried to do a 1:1 copy so it can be easily diffed, but small tweaks are needed here and there.

scenario 2
   split is done after regmap: much more than 300 lines need to be moved AND the code would diverge way too much from my target.

as I see it scenario 2 is worse from a reviewer's perspective and a nightmare from my pov.

I prefer to stick with scenario 1, adding a few prerequisite patches if you so prefer.
It's much easier for me to cherry pick modifications and copy them from my target code once the file structure in the patches have a _core.c, _spi.c, etc.

this is what I had in mind for v4:
split prerequisites (2-4p) -> split -> regmap prerequisites (4p) -> regmap -> regmap cleanup (2p) -> i2c -> features (5p).

best regards,
petre rodan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]