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

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: <aMeeku8OseSrTqW1@lipo>
Date: Mon, 15 Sep 2025 08:05:22 +0300
From: Petre Rodan <petre.rodan@...dimension.ro>
To: Andy Shevchenko <andy.shevchenko@...il.com>
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 v3 04/18] iio: accel: bma220: split original driver


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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)