[v2,1/2] i2c: core: Introduce i2c_client_get_device_id helper function

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

Commit Message

Angel Iglesias Nov. 6, 2022, 4:42 p.m. UTC
  Introduces new helper function to aid in .probe_new() refactors. In order
to use existing i2c_get_device_id() on the probe callback, the device
match table needs to be accessible in that function, which would require
bigger refactors in some drivers using the deprecated .probe callback.

This issue was discussed in more detail in the IIO mailing list.

Link: https://lore.kernel.org/all/20221023132302.911644-11-u.kleine-koenig@pengutronix.de/
Suggested-by: Nuno Sá <noname.nuno@gmail.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
  

Comments

Jonathan Cameron Nov. 6, 2022, 5:22 p.m. UTC | #1
On Sun,  6 Nov 2022 17:42:24 +0100
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> Introduces new helper function to aid in .probe_new() refactors. In order
> to use existing i2c_get_device_id() on the probe callback, the device
> match table needs to be accessible in that function, which would require
> bigger refactors in some drivers using the deprecated .probe callback.
> 
> This issue was discussed in more detail in the IIO mailing list.
> 
> Link: https://lore.kernel.org/all/20221023132302.911644-11-u.kleine-koenig@pengutronix.de/
> Suggested-by: Nuno Sá <noname.nuno@gmail.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Trivial comment inline - otherwise lgtm
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Thanks,

> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index b4edf10e8fd0..920676e62c22 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2236,6 +2236,21 @@ int i2c_get_device_id(const struct i2c_client *client,
>  }
>  EXPORT_SYMBOL_GPL(i2c_get_device_id);
>  
> +/**
> + * i2c_client_get_device_id - get the driver match table entry of a device
> + * @client: the device to query. The device must be bound to a driver or
> + *	    the function oopses.

Maybe don't state what happens otherwise if something changes to make it
instead do something else unfortunate, this comment will need updating.
"The device must be bound to a driver."


> + *
> + * Returns a pointer to the matching entry if found, NULL otherwise.
> + */
> +const struct i2c_device_id *i2c_client_get_device_id(const struct i2c_client *client)
> +{
> +	const struct i2c_driver *drv = to_i2c_driver(client->dev.driver);
> +
> +	return i2c_match_id(drv->id_table, client);
> +}
> +EXPORT_SYMBOL_GPL(i2c_client_get_device_id);
> +
>  /* ----------------------------------------------------
>   * the i2c address scanning function
>   * Will not work for 10-bit addresses!
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index f7c49bbdb8a1..d84e0e99f084 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -189,6 +189,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>  					      u8 *values);
>  int i2c_get_device_id(const struct i2c_client *client,
>  		      struct i2c_device_identity *id);
> +const struct i2c_device_id *i2c_client_get_device_id(const struct i2c_client *client);
>  #endif /* I2C */
>  
>  /**
  
Angel Iglesias Nov. 6, 2022, 5:50 p.m. UTC | #2
On Sun, 2022-11-06 at 17:22 +0000, Jonathan Cameron wrote:
> On Sun,  6 Nov 2022 17:42:24 +0100
> Angel Iglesias <ang.iglesiasg@gmail.com> wrote:
> 
> > Introduces new helper function to aid in .probe_new() refactors. In order
> > to use existing i2c_get_device_id() on the probe callback, the device
> > match table needs to be accessible in that function, which would require
> > bigger refactors in some drivers using the deprecated .probe callback.
> > 
> > This issue was discussed in more detail in the IIO mailing list.
> > 
> > Link:
> > https://lore.kernel.org/all/20221023132302.911644-11-u.kleine-koenig@pengutronix.de/
> > Suggested-by: Nuno Sá <noname.nuno@gmail.com>
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Trivial comment inline - otherwise lgtm
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Thanks,
> 
> > 
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index b4edf10e8fd0..920676e62c22 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -2236,6 +2236,21 @@ int i2c_get_device_id(const struct i2c_client
> > *client,
> >  }
> >  EXPORT_SYMBOL_GPL(i2c_get_device_id);
> >  
> > +/**
> > + * i2c_client_get_device_id - get the driver match table entry of a device
> > + * @client: the device to query. The device must be bound to a driver or
> > + *         the function oopses.
> 
> Maybe don't state what happens otherwise if something changes to make it
> instead do something else unfortunate, this comment will need updating.
> "The device must be bound to a driver."

Sure, sorry for the trouble. I'll wait a bit for more comments and then send v3
fixed.

Thanks for your time,
Angel

> 
> > + *
> > + * Returns a pointer to the matching entry if found, NULL otherwise.
> > + */
> > +const struct i2c_device_id *i2c_client_get_device_id(const struct
> > i2c_client *client)
> > +{
> > +       const struct i2c_driver *drv = to_i2c_driver(client->dev.driver);
> > +
> > +       return i2c_match_id(drv->id_table, client);
> > +}
> > +EXPORT_SYMBOL_GPL(i2c_client_get_device_id);
> > +
> >  /* ----------------------------------------------------
> >   * the i2c address scanning function
> >   * Will not work for 10-bit addresses!
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index f7c49bbdb8a1..d84e0e99f084 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -189,6 +189,7 @@ s32 i2c_smbus_read_i2c_block_data_or_emulated(const
> > struct i2c_client *client,
> >                                               u8 *values);
> >  int i2c_get_device_id(const struct i2c_client *client,
> >                       struct i2c_device_identity *id);
> > +const struct i2c_device_id *i2c_client_get_device_id(const struct
> > i2c_client *client);
> >  #endif /* I2C */
> >  
> >  /**
>
  

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index b4edf10e8fd0..920676e62c22 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2236,6 +2236,21 @@  int i2c_get_device_id(const struct i2c_client *client,
 }
 EXPORT_SYMBOL_GPL(i2c_get_device_id);
 
+/**
+ * i2c_client_get_device_id - get the driver match table entry of a device
+ * @client: the device to query. The device must be bound to a driver or
+ *	    the function oopses.
+ *
+ * Returns a pointer to the matching entry if found, NULL otherwise.
+ */
+const struct i2c_device_id *i2c_client_get_device_id(const struct i2c_client *client)
+{
+	const struct i2c_driver *drv = to_i2c_driver(client->dev.driver);
+
+	return i2c_match_id(drv->id_table, client);
+}
+EXPORT_SYMBOL_GPL(i2c_client_get_device_id);
+
 /* ----------------------------------------------------
  * the i2c address scanning function
  * Will not work for 10-bit addresses!
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index f7c49bbdb8a1..d84e0e99f084 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -189,6 +189,7 @@  s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
 					      u8 *values);
 int i2c_get_device_id(const struct i2c_client *client,
 		      struct i2c_device_identity *id);
+const struct i2c_device_id *i2c_client_get_device_id(const struct i2c_client *client);
 #endif /* I2C */
 
 /**