[PATCH v2 0/3] iio: pressure: bmp280: Small driver cleanup

[PATCH v2 0/3] iio: pressure: bmp280: Small driver cleanup

[Angel Iglesias]

Minor cleanup of BMP280 i2c code and migration to the new helper functions
i2c_get_match_data() and spi_get_device_match_data().

Revise driver coding style to follow reverse christmas tree.

Changelog v2:
- Added patch 3 adopting spi_get_device_match_data().
- Updated patch 2 following advice from Uwe Kleine-König
  <u.kleine-koenig@pengutronix.de> regarding unintentioned driver semantic
  changes.
- Revised thoroughfully driver coding style and update more functions.

v1:
https://lore.kernel.org/all/cover.1691276610.git.ang.iglesiasg@gmail.com/

Angel Iglesias (3):
  iio: pressure: bmp280: i2c: Rearrange vars in reverse xmas tree order
  iio: pressure: bmp280: Use i2c_get_match_data
  iio: pressure: bmp280: Use spi_get_device_match_data()

 drivers/iio/pressure/bmp280-core.c |  4 ++--
 drivers/iio/pressure/bmp280-i2c.c  |  8 +++-----
 drivers/iio/pressure/bmp280-spi.c  | 10 +++-------
 3 files changed, 8 insertions(+), 14 deletions(-)


base-commit: 14b7447cec15ee8dfdfe0da66ba1e280ded7e00a
-- 
2.41.0

[PATCH v2 1/3] iio: pressure: bmp280: i2c: Rearrange vars in reverse xmas tree order

[Angel Iglesias]

Minor cleanup reordering local variable declarations following reverse
christmas tree convention.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 6089f3f9d8f4..ea02a623bb58 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -766,8 +766,8 @@ static const struct iio_info bmp280_info = {
 
 static int bmp280_chip_config(struct bmp280_data *data)
 {
-	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
-		  FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1);
+	u8 osrs = FIELD_PREP(BMP280_OSRS_PRESS_MASK, data->oversampling_press + 1) |
+		  FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1);
 	int ret;
 
 	ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index dbe630ad05b5..693eb1975fdc 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -7,9 +7,9 @@
 
 static int bmp280_i2c_probe(struct i2c_client *client)
 {
-	struct regmap *regmap;
-	const struct bmp280_chip_info *chip_info;
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	const struct bmp280_chip_info *chip_info;
+	struct regmap *regmap;
 
 	chip_info = device_get_match_data(&client->dev);
 	if (!chip_info)
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 1dff9bb7c4e9..1c9c01f1b1c7 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -14,8 +14,7 @@
 static int bmp280_regmap_spi_write(void *context, const void *data,
                                    size_t count)
 {
-	struct device *dev = context;
-	struct spi_device *spi = to_spi_device(dev);
+	struct spi_device *spi = to_spi_device(context);
 	u8 buf[2];
 
 	memcpy(buf, data, 2);
@@ -31,8 +30,7 @@ static int bmp280_regmap_spi_write(void *context, const void *data,
 static int bmp280_regmap_spi_read(void *context, const void *reg,
                                   size_t reg_size, void *val, size_t val_size)
 {
-	struct device *dev = context;
-	struct spi_device *spi = to_spi_device(dev);
+	struct spi_device *spi = to_spi_device(context);
 
 	return spi_write_then_read(spi, reg, reg_size, val, val_size);
 }
-- 
2.41.0

[PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Angel Iglesias]

Replaces device_get_match_data() and fallback match_id logic by new
unified helper function i2c_get_match_data().

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index 693eb1975fdc..34e3bc758493 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client *client)
 	const struct bmp280_chip_info *chip_info;
 	struct regmap *regmap;
 
-	chip_info = device_get_match_data(&client->dev);
-	if (!chip_info)
-		chip_info = (const struct bmp280_chip_info *) id->driver_data;
+	chip_info = i2c_get_match_data(client);
 
 	regmap = devm_regmap_init_i2c(client, chip_info->regmap_config);
 	if (IS_ERR(regmap)) {
-- 
2.41.0

Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Angel Iglesias]

On Sun, 2023-08-13 at 21:03 +0200, Angel Iglesias wrote:
> Replaces device_get_match_data() and fallback match_id logic by new
> unified helper function i2c_get_match_data().
> 
> Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> 
> diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-
> i2c.c
> index 693eb1975fdc..34e3bc758493 100644
> --- a/drivers/iio/pressure/bmp280-i2c.c
> +++ b/drivers/iio/pressure/bmp280-i2c.c
> @@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client *client)
>         const struct bmp280_chip_info *chip_info;
>         struct regmap *regmap;
>  
> -       chip_info = device_get_match_data(&client->dev);
> -       if (!chip_info)
> -               chip_info = (const struct bmp280_chip_info *) id->driver_data;
> +       chip_info = i2c_get_match_data(client);
>  
>         regmap = devm_regmap_init_i2c(client, chip_info->regmap_config);
>         if (IS_ERR(regmap)) {

Hi,

I noticed I submitted this change that was also submitted by Biju Das on another
patch:
https://lore.kernel.org/all/20230812175808.236405-1-biju.das.jz@bp.renesas.com/

Should I drop this patch from the series?

Kind regards,
Angel

RE: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Biju Das]

Hi Angel Iglesias,


> Subject: Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data
> 
> On Sun, 2023-08-13 at 21:03 +0200, Angel Iglesias wrote:
> > Replaces device_get_match_data() and fallback match_id logic by new
> > unified helper function i2c_get_match_data().
> >
> > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> >
> > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > b/drivers/iio/pressure/bmp280- i2c.c index 693eb1975fdc..34e3bc758493
> > 100644
> > --- a/drivers/iio/pressure/bmp280-i2c.c
> > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > @@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client
> > *client)
> >         const struct bmp280_chip_info *chip_info;
> >         struct regmap *regmap;
> >
> > -       chip_info = device_get_match_data(&client->dev);
> > -       if (!chip_info)
> > -               chip_info = (const struct bmp280_chip_info *)
> > id->driver_data;
> > +       chip_info = i2c_get_match_data(client);
> >
> >         regmap = devm_regmap_init_i2c(client,
> > chip_info->regmap_config);
> >         if (IS_ERR(regmap)) {
> 
> Hi,
> 
> I noticed I submitted this change that was also submitted by Biju Das on
> another
> patch:

> Should I drop this patch from the series?

I think it is ok. Andy is suggesting to use unified table for SPI/I2C

Is it something do able and testable in your environment? see [1],
If yes, please create another patch for using unified table for both i2c and spi.

https://lore.kernel.org/linux-renesas-soc/CAHp75VeX+T=hAN+PgtHTdv4b6UtDVgveUUww1b1kuOngzDinFw@mail.gmail.com/T/#t

Cheers,
Biju

Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Angel Iglesias]

On Mon, 2023-08-14 at 06:57 +0000, Biju Das wrote:
> Hi Angel Iglesias,
> 
> 
> > Subject: Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data
> > 
> > On Sun, 2023-08-13 at 21:03 +0200, Angel Iglesias wrote:
> > > Replaces device_get_match_data() and fallback match_id logic by new
> > > unified helper function i2c_get_match_data().
> > > 
> > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > > 
> > > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > > b/drivers/iio/pressure/bmp280- i2c.c index 693eb1975fdc..34e3bc758493
> > > 100644
> > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > @@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client
> > > *client)
> > >         const struct bmp280_chip_info *chip_info;
> > >         struct regmap *regmap;
> > > 
> > > -       chip_info = device_get_match_data(&client->dev);
> > > -       if (!chip_info)
> > > -               chip_info = (const struct bmp280_chip_info *)
> > > id->driver_data;
> > > +       chip_info = i2c_get_match_data(client);
> > > 
> > >         regmap = devm_regmap_init_i2c(client,
> > > chip_info->regmap_config);
> > >         if (IS_ERR(regmap)) {
> > 
> > Hi,
> > 
> > I noticed I submitted this change that was also submitted by Biju Das on
> > another
> > patch:
> 
> > Should I drop this patch from the series?
> 
> I think it is ok. Andy is suggesting to use unified table for SPI/I2C
> 
> Is it something do able and testable in your environment? see [1],
> If yes, please create another patch for using unified table for both i2c and
> spi.

I have around a BMP390 with the SPI pins available to test it out. In the case
of the bmp280 we could unify the of_match table as they're almost the same. In
the case of the spi_device_id and i2c_device_id tables, as they're different
structs I'm not sure if they can be unified.

Regarding Andy's comment, I think he's referring to the duplicated chip infos.
In the case of the bmp280, the chip_infos are defined on the common driver code
and used for both SPI and I2C match tables.

> 
> https://lore.kernel.org/linux-renesas-soc/CAHp75VeX+T=hAN+PgtHTdv4b6UtDVgveUUww1b1kuOngzDinFw@mail.gmail.com/T/#t
> 
> Cheers,
> Biju

Kind regards,
Angel

Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Jonathan Cameron]

On Mon, 14 Aug 2023 18:43:49 +0200
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> On Mon, 2023-08-14 at 06:57 +0000, Biju Das wrote:
> > Hi Angel Iglesias,
> > 
> >   
> > > Subject: Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data
> > > 
> > > On Sun, 2023-08-13 at 21:03 +0200, Angel Iglesias wrote:  
> > > > Replaces device_get_match_data() and fallback match_id logic by new
> > > > unified helper function i2c_get_match_data().
> > > > 
> > > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > > > 
> > > > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > > > b/drivers/iio/pressure/bmp280- i2c.c index 693eb1975fdc..34e3bc758493
> > > > 100644
> > > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > > @@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client
> > > > *client)
> > > >         const struct bmp280_chip_info *chip_info;
> > > >         struct regmap *regmap;
> > > > 
> > > > -       chip_info = device_get_match_data(&client->dev);
> > > > -       if (!chip_info)
> > > > -               chip_info = (const struct bmp280_chip_info *)
> > > > id->driver_data;
> > > > +       chip_info = i2c_get_match_data(client);
> > > > 
> > > >         regmap = devm_regmap_init_i2c(client,
> > > > chip_info->regmap_config);
> > > >         if (IS_ERR(regmap)) {  
> > > 
> > > Hi,
> > > 
> > > I noticed I submitted this change that was also submitted by Biju Das on
> > > another
> > > patch:  
> >   
> > > Should I drop this patch from the series?  
> > 
> > I think it is ok. Andy is suggesting to use unified table for SPI/I2C
> > 
> > Is it something do able and testable in your environment? see [1],
> > If yes, please create another patch for using unified table for both i2c and
> > spi.  
> 
> I have around a BMP390 with the SPI pins available to test it out. In the case
> of the bmp280 we could unify the of_match table as they're almost the same. In
> the case of the spi_device_id and i2c_device_id tables, as they're different
> structs I'm not sure if they can be unified.
> 
> Regarding Andy's comment, I think he's referring to the duplicated chip infos.
> In the case of the bmp280, the chip_infos are defined on the common driver code
> and used for both SPI and I2C match tables.
Hi,

I'm loosing track of where we are with this driver as multiple people are
working on it.

Angel, as most of the work is yours, please could you manage the flow of patches
for this one so I get series with clear statement of what they are dependent on.

Thanks,

Jonathan

> 
> > 
> > https://lore.kernel.org/linux-renesas-soc/CAHp75VeX+T=hAN+PgtHTdv4b6UtDVgveUUww1b1kuOngzDinFw@mail.gmail.com/T/#t
> > 
> > Cheers,
> > Biju  
> 
> Kind regards,
> Angel
> 

Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Angel Iglesias]

On Mon, 2023-08-28 at 12:39 +0100, Jonathan Cameron wrote:
> On Mon, 14 Aug 2023 18:43:49 +0200
> Angel Iglesias <ang.iglesiasg@gmail.com> wrote:
> 
> > On Mon, 2023-08-14 at 06:57 +0000, Biju Das wrote:
> > > Hi Angel Iglesias,
> > > 
> > >   
> > > > Subject: Re: [PATCH v2 2/3] iio: pressure: bmp280: Use
> > > > i2c_get_match_data
> > > > 
> > > > On Sun, 2023-08-13 at 21:03 +0200, Angel Iglesias wrote:  
> > > > > Replaces device_get_match_data() and fallback match_id logic by new
> > > > > unified helper function i2c_get_match_data().
> > > > > 
> > > > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > > > > 
> > > > > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > > > > b/drivers/iio/pressure/bmp280- i2c.c index 693eb1975fdc..34e3bc758493
> > > > > 100644
> > > > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > > > @@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct i2c_client
> > > > > *client)
> > > > >         const struct bmp280_chip_info *chip_info;
> > > > >         struct regmap *regmap;
> > > > > 
> > > > > -       chip_info = device_get_match_data(&client->dev);
> > > > > -       if (!chip_info)
> > > > > -               chip_info = (const struct bmp280_chip_info *)
> > > > > id->driver_data;
> > > > > +       chip_info = i2c_get_match_data(client);
> > > > > 
> > > > >         regmap = devm_regmap_init_i2c(client,
> > > > > chip_info->regmap_config);
> > > > >         if (IS_ERR(regmap)) {  
> > > > 
> > > > Hi,
> > > > 
> > > > I noticed I submitted this change that was also submitted by Biju Das on
> > > > another
> > > > patch:  
> > >   
> > > > Should I drop this patch from the series?  
> > > 
> > > I think it is ok. Andy is suggesting to use unified table for SPI/I2C
> > > 
> > > Is it something do able and testable in your environment? see [1],
> > > If yes, please create another patch for using unified table for both i2c
> > > and
> > > spi.  
> > 
> > I have around a BMP390 with the SPI pins available to test it out. In the
> > case
> > of the bmp280 we could unify the of_match table as they're almost the same.
> > In
> > the case of the spi_device_id and i2c_device_id tables, as they're different
> > structs I'm not sure if they can be unified.
> > 
> > Regarding Andy's comment, I think he's referring to the duplicated chip
> > infos.
> > In the case of the bmp280, the chip_infos are defined on the common driver
> > code
> > and used for both SPI and I2C match tables.
> Hi,
> 
> I'm loosing track of where we are with this driver as multiple people are
> working on it.
> 
> Angel, as most of the work is yours, please could you manage the flow of
> patches
> for this one so I get series with clear statement of what they are dependent
> on.

Sure. If Biju is okay with it, maybe I should squash toghether this two series
of mine:
https://patchwork.kernel.org/project/linux-iio/cover/cover.1691952005.git.ang.iglesiasg@gmail.com/
https://patchwork.kernel.org/project/linux-iio/cover/cover.1692805377.git.ang.iglesiasg@gmail.com/

And pull this patch from Biju:
https://patchwork.kernel.org/project/linux-iio/patch/20230812175808.236405-1-biju.das.jz@bp.renesas.com/

Sorry again from the noise introduced in the mail-list,

Kind regards,
Angel

> Thanks,
> 
> Jonathan
> 
> > 
> > > 
> > > https://lore.kernel.org/linux-renesas-soc/CAHp75VeX+T=hAN+PgtHTdv4b6UtDVgveUUww1b1kuOngzDinFw@mail.gmail.com/T/#t
> > > 
> > > Cheers,
> > > Biju  
> > 
> > Kind regards,
> > Angel
> > 
> 

RE: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data

[Biju Das]

Hi Angel Iglesias,

> Subject: Re: [PATCH v2 2/3] iio: pressure: bmp280: Use i2c_get_match_data
> 
> On Mon, 2023-08-28 at 12:39 +0100, Jonathan Cameron wrote:
> > On Mon, 14 Aug 2023 18:43:49 +0200
> > Angel Iglesias <ang.iglesiasg@gmail.com> wrote:
> >
> > > On Mon, 2023-08-14 at 06:57 +0000, Biju Das wrote:
> > > > Hi Angel Iglesias,
> > > >
> > > >
> > > > > Subject: Re: [PATCH v2 2/3] iio: pressure: bmp280: Use
> > > > > i2c_get_match_data
> > > > >
> > > > > On Sun, 2023-08-13 at 21:03 +0200, Angel Iglesias wrote:
> > > > > > Replaces device_get_match_data() and fallback match_id logic
> > > > > > by new unified helper function i2c_get_match_data().
> > > > > >
> > > > > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > > > > >
> > > > > > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > > > > > b/drivers/iio/pressure/bmp280- i2c.c index
> > > > > > 693eb1975fdc..34e3bc758493
> > > > > > 100644
> > > > > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > > > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > > > > @@ -11,9 +11,7 @@ static int bmp280_i2c_probe(struct
> > > > > > i2c_client
> > > > > > *client)
> > > > > >         const struct bmp280_chip_info *chip_info;
> > > > > >         struct regmap *regmap;
> > > > > >
> > > > > > -       chip_info = device_get_match_data(&client->dev);
> > > > > > -       if (!chip_info)
> > > > > > -               chip_info = (const struct bmp280_chip_info *)
> > > > > > id->driver_data;
> > > > > > +       chip_info = i2c_get_match_data(client);
> > > > > >
> > > > > >         regmap = devm_regmap_init_i2c(client,
> > > > > > chip_info->regmap_config);
> > > > > >         if (IS_ERR(regmap)) {
> > > > >
> > > > > Hi,
> > > > >
> > > > > I noticed I submitted this change that was also submitted by
> > > > > Biju Das on another
> > > > > patch:
> > > >
> > > > > Should I drop this patch from the series?
> > > >
> > > > I think it is ok. Andy is suggesting to use unified table for
> > > > SPI/I2C
> > > >
> > > > Is it something do able and testable in your environment? see [1],
> > > > If yes, please create another patch for using unified table for
> > > > both i2c and spi.
> > >
> > > I have around a BMP390 with the SPI pins available to test it out.
> > > In the case of the bmp280 we could unify the of_match table as
> > > they're almost the same.
> > > In
> > > the case of the spi_device_id and i2c_device_id tables, as they're
> > > different structs I'm not sure if they can be unified.
> > >
> > > Regarding Andy's comment, I think he's referring to the duplicated
> > > chip infos.
> > > In the case of the bmp280, the chip_infos are defined on the common
> > > driver code and used for both SPI and I2C match tables.
> > Hi,
> >
> > I'm loosing track of where we are with this driver as multiple people
> > are working on it.
> >
> > Angel, as most of the work is yours, please could you manage the flow
> > of patches for this one so I get series with clear statement of what
> > they are dependent on.
> 
> Sure. If Biju is okay with it, maybe I should squash toghether this two
> series of mine:

I am ok with it, as I don't have bandwidth as well as board for testing it. Please feel free to post.

Cheers,
Biju

[PATCH v2 3/3] iio: pressure: bmp280: Use spi_get_device_match_data()

[Angel Iglesias]

Use the spi_get_device_match_data() helper instead of
device_get_match_data() and the fallback match_id logic.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index 1c9c01f1b1c7..433d6fac83c4 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -56,9 +56,7 @@ static int bmp280_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	chip_info = device_get_match_data(&spi->dev);
-	if (!chip_info)
-		chip_info = (const struct bmp280_chip_info *) id->driver_data;
+	chip_info = spi_get_device_match_data(spi);
 
 	regmap = devm_regmap_init(&spi->dev,
 				  &bmp280_regmap_bus,
-- 
2.41.0