diff mbox series

[RFC,2/2] iio: pressure: bmp280: convert to i2c's .probe_new()

Message ID af8ed10a85d48531c50823163e6c55b2a72371ef.1667151588.git.ang.iglesiasg@gmail.com
State New
Headers
Series i2c: core: Introduce i2c_client_get_device_id helper |

Commit Message

Angel Iglesias Oct. 30, 2022, 5:53 p.m. UTC 
  Use i2c_client_get_device_id() to get the i2c_device_id* parameter in the
.new_probe() callback.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
---
 drivers/iio/pressure/bmp280-i2c.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Uwe Kleine-König Nov. 1, 2022, 9:52 p.m. UTC | #1 
Hello,

On Sun, Oct 30, 2022 at 06:53:11PM +0100, Angel Iglesias wrote:
> Use i2c_client_get_device_id() to get the i2c_device_id* parameter in the
> .new_probe() callback.
> 
> Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> ---
>  drivers/iio/pressure/bmp280-i2c.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> index 0c27211f3ea0..20073b09b3e3 100644
> --- a/drivers/iio/pressure/bmp280-i2c.c
> +++ b/drivers/iio/pressure/bmp280-i2c.c
> @@ -5,11 +5,11 @@
>  
>  #include "bmp280.h"
>  
> -static int bmp280_i2c_probe(struct i2c_client *client,
> -			    const struct i2c_device_id *id)
> +static int bmp280_i2c_probe(struct i2c_client *client)
>  {
> -	struct regmap *regmap;
> +	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	const struct regmap_config *regmap_config;
> +	struct regmap *regmap;
>  
>  	switch (id->driver_data) {
>  	case BMP180_CHIP_ID:

What is the motivation for moving regmap? I thought reverse christmas
tree is only a thing in network code? I would have left the regmap
declaration where it is.

> @@ -65,7 +65,7 @@ static struct i2c_driver bmp280_i2c_driver = {
>  		.of_match_table = bmp280_of_i2c_match,
>  		.pm = pm_ptr(&bmp280_dev_pm_ops),
>  	},
> -	.probe		= bmp280_i2c_probe,
> +	.probe_new	= bmp280_i2c_probe,
>  	.id_table	= bmp280_i2c_id,
>  };
>  module_i2c_driver(bmp280_i2c_driver);

Best regards
Uwe
Angel Iglesias Nov. 2, 2022, 12:16 a.m. UTC | #2 
On Tue, 2022-11-01 at 22:52 +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Sun, Oct 30, 2022 at 06:53:11PM +0100, Angel Iglesias wrote:
> > Use i2c_client_get_device_id() to get the i2c_device_id* parameter in the
> > .new_probe() callback.
> > 
> > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > ---
> >  drivers/iio/pressure/bmp280-i2c.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > b/drivers/iio/pressure/bmp280-i2c.c
> > index 0c27211f3ea0..20073b09b3e3 100644
> > --- a/drivers/iio/pressure/bmp280-i2c.c
> > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > @@ -5,11 +5,11 @@
> >  
> >  #include "bmp280.h"
> >  
> > -static int bmp280_i2c_probe(struct i2c_client *client,
> > -                           const struct i2c_device_id *id)
> > +static int bmp280_i2c_probe(struct i2c_client *client)
> >  {
> > -       struct regmap *regmap;
> > +       const struct i2c_device_id *id = i2c_client_get_device_id(client);
> >         const struct regmap_config *regmap_config;
> > +       struct regmap *regmap;
> >  
> >         switch (id->driver_data) {
> >         case BMP180_CHIP_ID:
> 
> What is the motivation for moving regmap? I thought reverse christmas
> tree is only a thing in network code? I would have left the regmap
> declaration where it is.

Long story short, I worked previously on a small refactor of this driver to add
support for a new family of sensors. During the different iterations of the
patchset, one thing that was agreed was unifying the driver coding style to
reverse xmas tree. For some extra context, here's the thread:
https://lore.kernel.org/all/20220814145249.701f1261@jic23-huawei/

> > @@ -65,7 +65,7 @@ static struct i2c_driver bmp280_i2c_driver = {
> >                 .of_match_table = bmp280_of_i2c_match,
> >                 .pm = pm_ptr(&bmp280_dev_pm_ops),
> >         },
> > -       .probe          = bmp280_i2c_probe,
> > +       .probe_new      = bmp280_i2c_probe,
> >         .id_table       = bmp280_i2c_id,
> >  };
> >  module_i2c_driver(bmp280_i2c_driver);
> 
> Best regards
> Uwe
> 
Kind regards
Angel
Jonathan Cameron Nov. 5, 2022, 2:54 p.m. UTC | #3 
On Wed, 02 Nov 2022 01:16:44 +0100
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> On Tue, 2022-11-01 at 22:52 +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Sun, Oct 30, 2022 at 06:53:11PM +0100, Angel Iglesias wrote:  
> > > Use i2c_client_get_device_id() to get the i2c_device_id* parameter in the
> > > .new_probe() callback.
> > > 
> > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > > ---
> > >  drivers/iio/pressure/bmp280-i2c.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/iio/pressure/bmp280-i2c.c
> > > b/drivers/iio/pressure/bmp280-i2c.c
> > > index 0c27211f3ea0..20073b09b3e3 100644
> > > --- a/drivers/iio/pressure/bmp280-i2c.c
> > > +++ b/drivers/iio/pressure/bmp280-i2c.c
> > > @@ -5,11 +5,11 @@
> > >  
> > >  #include "bmp280.h"
> > >  
> > > -static int bmp280_i2c_probe(struct i2c_client *client,
> > > -                           const struct i2c_device_id *id)
> > > +static int bmp280_i2c_probe(struct i2c_client *client)
> > >  {
> > > -       struct regmap *regmap;
> > > +       const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > >         const struct regmap_config *regmap_config;
> > > +       struct regmap *regmap;
> > >  
> > >         switch (id->driver_data) {
> > >         case BMP180_CHIP_ID:  
> > 
> > What is the motivation for moving regmap? I thought reverse christmas
> > tree is only a thing in network code? I would have left the regmap
> > declaration where it is.  
> 
> Long story short, I worked previously on a small refactor of this driver to add
> support for a new family of sensors. During the different iterations of the
> patchset, one thing that was agreed was unifying the driver coding style to
> reverse xmas tree. For some extra context, here's the thread:
> https://lore.kernel.org/all/20220814145249.701f1261@jic23-huawei/

Not something I feel strongly enough about either way, but has benefit of
consistency. However, it's an unrelated change in this patch, so drop it
to avoid the noise in a patch where you have more significant changes.

Jonathan

> 
> > > @@ -65,7 +65,7 @@ static struct i2c_driver bmp280_i2c_driver = {
> > >                 .of_match_table = bmp280_of_i2c_match,
> > >                 .pm = pm_ptr(&bmp280_dev_pm_ops),
> > >         },
> > > -       .probe          = bmp280_i2c_probe,
> > > +       .probe_new      = bmp280_i2c_probe,
> > >         .id_table       = bmp280_i2c_id,
> > >  };
> > >  module_i2c_driver(bmp280_i2c_driver);  
> > 
> > Best regards
> > Uwe
> >   
> Kind regards
> Angel
diff mbox series

Patch

diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index 0c27211f3ea0..20073b09b3e3 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -5,11 +5,11 @@ 
 
 #include "bmp280.h"
 
-static int bmp280_i2c_probe(struct i2c_client *client,
-			    const struct i2c_device_id *id)
+static int bmp280_i2c_probe(struct i2c_client *client)
 {
-	struct regmap *regmap;
+	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	const struct regmap_config *regmap_config;
+	struct regmap *regmap;
 
 	switch (id->driver_data) {
 	case BMP180_CHIP_ID:
@@ -65,7 +65,7 @@  static struct i2c_driver bmp280_i2c_driver = {
 		.of_match_table = bmp280_of_i2c_match,
 		.pm = pm_ptr(&bmp280_dev_pm_ops),
 	},
-	.probe		= bmp280_i2c_probe,
+	.probe_new	= bmp280_i2c_probe,
 	.id_table	= bmp280_i2c_id,
 };
 module_i2c_driver(bmp280_i2c_driver);