| Message ID | 55f8dc02de16a353f0449bc1c7cb487bd776dfaf.1691952005.git.ang.iglesiasg@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: pressure: bmp280: Small driver cleanup | expand |
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
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
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
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 >
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 > > >
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
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)) {
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>