* [PATCH v4 0/3] Extend device_get_match_data() to struct bus_type
@ 2023-08-02 11:23 Biju Das
2023-08-02 11:23 ` [PATCH v4 1/3] drivers: fwnode: " Biju Das
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Biju Das @ 2023-08-02 11:23 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Biju Das, linux-acpi, Dmitry Torokhov, Wolfram Sang,
Geert Uytterhoeven, linux-i2c, linux-renesas-soc
This patch series extend device_get_match_data() to struct bus_type,
so that buses like I2C can get matched data.
v3->v4:
* Documented corner case for device_get_match_data()
* Dropped struct i2c_driver parameter from i2c_get_match_data_helper()
* Split I2C sysfs handling in seperate patch(patch#3)
* Added space after of_device_id for i2c_of_match_device_sysfs()
* Added const parameter for struct i2c_client, to prevent overriding it's
pointer.
* Moved declaration from public i2c.h->i2c-core.h
v2->v3:
* Added Rb tag from Andy for patch#1.
* Extended to support i2c_of_match_device() as suggested by Andy.
* Changed i2c_of_match_device_sysfs() as non-static function as it is
needed for i2c_device_get_match_data().
* Added a TODO comment to use i2c_verify_client() when it accepts const
pointer.
* Added multiple returns to make code path for device_get_match_data()
faster in i2c_get_match_data().
RFC v1->v2:
* Replaced "Signed-off-by"->"Suggested-by" tag for Dmitry.
* Documented device_get_match_data().
* Added multiple returns to make code path for generic fwnode-based
lookup faster.
* Fixed build warnings reported by kernel test robot <lkp@intel.com>
* Added const qualifier to return type and parameter struct i2c_driver
in i2c_get_match_data_helper().
* Added const qualifier to struct i2c_driver in i2c_get_match_data()
* Dropped driver variable from i2c_device_get_match_data()
* Replaced to_i2c_client with logic for assigning verify_client as it
returns non const pointer.
Biju Das (3):
drivers: fwnode: Extend device_get_match_data() to struct bus_type
i2c: Add i2c_device_get_match_data() callback
i2c: Extend i2c_device_get_match_data() to support i2c sysfs
drivers/base/property.c | 27 ++++++++++++++++++++-
drivers/i2c/i2c-core-base.c | 48 ++++++++++++++++++++++++++++++-------
drivers/i2c/i2c-core-of.c | 5 ++--
drivers/i2c/i2c-core.h | 9 +++++++
include/linux/device/bus.h | 3 +++
5 files changed, 81 insertions(+), 11 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/3] drivers: fwnode: Extend device_get_match_data() to struct bus_type
2023-08-02 11:23 [PATCH v4 0/3] Extend device_get_match_data() to struct bus_type Biju Das
@ 2023-08-02 11:23 ` Biju Das
2023-08-02 11:23 ` [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback Biju Das
2023-08-02 11:23 ` [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs Biju Das
2 siblings, 0 replies; 10+ messages in thread
From: Biju Das @ 2023-08-02 11:23 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Biju Das, linux-acpi, Dmitry Torokhov, Wolfram Sang,
Geert Uytterhoeven, linux-renesas-soc
Extend device_get_match_data() to buses (for eg: I2C) by adding a
callback device_get_match_data() to struct bus_type() and call this method
as a fallback for generic fwnode based device_get_match_data().
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3->v4:
* Documented corner case.
v2->v3:
* Added Rb tag from Andy.
RFC v1-> v2:
* Replaced "Signed-off-by"->"Suggested-by" tag for Dmitry.
* Documented device_get_match_data().
* Added multiple returns to make code path for generic fwnode-based
lookup faster.
---
drivers/base/property.c | 27 ++++++++++++++++++++++++++-
include/linux/device/bus.h | 3 +++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8c40abed7852..a3c188cf68bb 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1275,9 +1275,34 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
}
EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
+/**
+ * device_get_match_data - get match data from OF/ACPI/Bus match tables
+ * @dev: device to find the match data
+ *
+ * Find match data using generic fwnode-based lookup and if there is no
+ * match, call the bus->get_match_data() for finding match data.
+ *
+ * Return: a match data pointer or NULL if there is no match in the matching
+ * table.
+ *
+ * Besides the fact that some drivers abuse the device ID driver_data type
+ * and claim it to be integer, for the bus specific ID tables the driver_data
+ * may be defined as kernel_ulong_t. For these tables 0 is a valid response,
+ * but not for this function. It's recommended to convert those either to avoid
+ * 0 or use a real pointer to the predefined driver data.
+ */
const void *device_get_match_data(const struct device *dev)
{
- return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
+ const void *data;
+
+ data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
+ if (data)
+ return data;
+
+ if (dev->bus && dev->bus->get_match_data)
+ return dev->bus->get_match_data(dev);
+
+ return NULL;
}
EXPORT_SYMBOL_GPL(device_get_match_data);
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index ae10c4322754..2e15b0ae5384 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -60,6 +60,7 @@ struct fwnode_handle;
* this bus.
* @dma_cleanup: Called to cleanup DMA configuration on a device on
* this bus.
+ * @get_match_data: Called to get match data on a device on this bus.
* @pm: Power management operations of this bus, callback the specific
* device driver's pm-ops.
* @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU
@@ -102,6 +103,8 @@ struct bus_type {
int (*dma_configure)(struct device *dev);
void (*dma_cleanup)(struct device *dev);
+ const void *(*get_match_data)(const struct device *dev);
+
const struct dev_pm_ops *pm;
const struct iommu_ops *iommu_ops;
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback
2023-08-02 11:23 [PATCH v4 0/3] Extend device_get_match_data() to struct bus_type Biju Das
2023-08-02 11:23 ` [PATCH v4 1/3] drivers: fwnode: " Biju Das
@ 2023-08-02 11:23 ` Biju Das
2023-08-02 15:08 ` Andy Shevchenko
2023-08-02 11:23 ` [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs Biju Das
2 siblings, 1 reply; 10+ messages in thread
From: Biju Das @ 2023-08-02 11:23 UTC (permalink / raw)
To: Wolfram Sang
Cc: Biju Das, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
Andy Shevchenko, linux-renesas-soc
Add i2c_device_get_match_data() callback to struct bus_type().
While at it, introduced i2c_get_match_data_helper() to avoid code
duplication with i2c_get_match_data().
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
* Dropped struct i2c_driver parameter from i2c_get_match_data_helper()
* Split I2C sysfs handling in seperate patch.
v2->v3:
* Extended to support i2c_of_match_device() as suggested by Andy.
* Changed i2c_of_match_device_sysfs() as non-static function as it is
needed for i2c_device_get_match_data().
* Added a TODO comment to use i2c_verify_client() when it accepts const
pointer.
* Added multiple returns to make code path for device_get_match_data()
faster in i2c_get_match_data().
RFC v1->v2:
* Replaced "Signed-off-by"->"Suggested-by" tag for Dmitry.
* Fixed build warnings reported by kernel test robot <lkp@intel.com>
* Added const qualifier to return type and parameter struct i2c_driver
in i2c_get_match_data_helper().
* Added const qualifier to struct i2c_driver in i2c_get_match_data()
* Dropped driver variable from i2c_device_get_match_data()
* Replaced to_i2c_client with logic for assigning verify_client as it
returns non const pointer.
---
drivers/i2c/i2c-core-base.c | 38 +++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 60746652fd52..2436f23e63af 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -114,22 +114,39 @@ const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
}
EXPORT_SYMBOL_GPL(i2c_match_id);
-const void *i2c_get_match_data(const struct i2c_client *client)
+static const void *i2c_get_match_data_helper(const struct i2c_client *client)
{
- struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
+ const struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
const struct i2c_device_id *match;
+
+ match = i2c_match_id(driver->id_table, client);
+ if (!match)
+ return NULL;
+
+ return (const void *)match->driver_data;
+}
+
+static const void *i2c_device_get_match_data(const struct device *dev)
+{
+ /* TODO: use i2c_verify_client() when it accepts const pointer */
+ const struct i2c_client *client = (dev->type == &i2c_client_type) ?
+ to_i2c_client(dev) : NULL;
+
+ if (!client || !dev->driver)
+ return NULL;
+
+ return i2c_get_match_data_helper(client);
+}
+
+const void *i2c_get_match_data(const struct i2c_client *client)
+{
const void *data;
data = device_get_match_data(&client->dev);
- if (!data) {
- match = i2c_match_id(driver->id_table, client);
- if (!match)
- return NULL;
+ if (data)
+ return data;
- data = (const void *)match->driver_data;
- }
-
- return data;
+ return i2c_get_match_data_helper(client);
}
EXPORT_SYMBOL(i2c_get_match_data);
@@ -695,6 +712,7 @@ struct bus_type i2c_bus_type = {
.probe = i2c_device_probe,
.remove = i2c_device_remove,
.shutdown = i2c_device_shutdown,
+ .get_match_data = i2c_device_get_match_data,
};
EXPORT_SYMBOL_GPL(i2c_bus_type);
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs
2023-08-02 11:23 [PATCH v4 0/3] Extend device_get_match_data() to struct bus_type Biju Das
2023-08-02 11:23 ` [PATCH v4 1/3] drivers: fwnode: " Biju Das
2023-08-02 11:23 ` [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback Biju Das
@ 2023-08-02 11:23 ` Biju Das
2023-08-02 18:00 ` Dmitry Torokhov
2023-08-02 21:39 ` Andy Shevchenko
2 siblings, 2 replies; 10+ messages in thread
From: Biju Das @ 2023-08-02 11:23 UTC (permalink / raw)
To: Wolfram Sang
Cc: Biju Das, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
Andy Shevchenko, linux-renesas-soc
Extend i2c_device_get_match_data() to i2c sysfs interface by changing
i2c_of_match_device_sysfs() to non-static and call this function as a
fallback for i2c_get_match_data_helper().
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v4:
* split from patch #2
* Added space after of_device_id for i2c_of_match_device_sysfs()
* Added const parameter for struct i2c_client, to prevent overriding it's
pointer.
* Moved declaration from public i2c.h->i2c-core.h
---
drivers/i2c/i2c-core-base.c | 16 +++++++++++++++-
drivers/i2c/i2c-core-of.c | 5 +++--
drivers/i2c/i2c-core.h | 9 +++++++++
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 2436f23e63af..d002c5a1cc84 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -131,11 +131,25 @@ static const void *i2c_device_get_match_data(const struct device *dev)
/* TODO: use i2c_verify_client() when it accepts const pointer */
const struct i2c_client *client = (dev->type == &i2c_client_type) ?
to_i2c_client(dev) : NULL;
+ const void *data;
if (!client || !dev->driver)
return NULL;
- return i2c_get_match_data_helper(client);
+ data = i2c_get_match_data_helper(client);
+ if (data)
+ return data;
+
+ if (dev->driver->of_match_table) {
+ const struct of_device_id *match;
+
+ match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
+ client);
+ if (match)
+ return match->data;
+ }
+
+ return data;
}
const void *i2c_get_match_data(const struct i2c_client *client)
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index a6c407d36800..179b3486904a 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -113,9 +113,9 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
of_node_put(bus);
}
-static const struct of_device_id*
+const struct of_device_id *
i2c_of_match_device_sysfs(const struct of_device_id *matches,
- struct i2c_client *client)
+ const struct i2c_client *client)
{
const char *name;
@@ -141,6 +141,7 @@ i2c_of_match_device_sysfs(const struct of_device_id *matches,
return NULL;
}
+EXPORT_SYMBOL_GPL(i2c_of_match_device_sysfs);
const struct of_device_id
*i2c_of_match_device(const struct of_device_id *matches,
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index 1247e6e6e975..e4d397b67989 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -82,8 +82,17 @@ static inline void i2c_acpi_remove_space_handler(struct i2c_adapter *adapter) {
#ifdef CONFIG_OF
void of_i2c_register_devices(struct i2c_adapter *adap);
+const struct of_device_id *
+i2c_of_match_device_sysfs(const struct of_device_id *matches,
+ const struct i2c_client *client);
#else
static inline void of_i2c_register_devices(struct i2c_adapter *adap) { }
+static inline const struct of_device_id *
+i2c_of_match_device_sysfs(const struct of_device_id *matches,
+ const struct i2c_client *client)
+{
+ return NULL;
+}
#endif
extern struct notifier_block i2c_of_notifier;
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback
2023-08-02 11:23 ` [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback Biju Das
@ 2023-08-02 15:08 ` Andy Shevchenko
2023-08-03 6:01 ` Biju Das
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2023-08-02 15:08 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
linux-renesas-soc
On Wed, Aug 02, 2023 at 12:23:16PM +0100, Biju Das wrote:
> Add i2c_device_get_match_data() callback to struct bus_type().
>
> While at it, introduced i2c_get_match_data_helper() to avoid code
> duplication with i2c_get_match_data().
...
> +static const void *i2c_get_match_data_helper(const struct i2c_client *client)
> {
> - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
> + const struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
> const struct i2c_device_id *match;
> +
> + match = i2c_match_id(driver->id_table, client);
> + if (!match)
> + return NULL;
> +
> + return (const void *)match->driver_data;
> +}
Yes, perfect!
...
> +static const void *i2c_device_get_match_data(const struct device *dev)
> +{
> + /* TODO: use i2c_verify_client() when it accepts const pointer */
> + const struct i2c_client *client = (dev->type == &i2c_client_type) ?
> + to_i2c_client(dev) : NULL;
> +
> + if (!client || !dev->driver)
> + return NULL;
> +
> + return i2c_get_match_data_helper(client);
I believe below looks better from readability and maintenance perspectives.
const struct i2c_client *client;
/* ...comment as in Dmitry's reply in v3 thread on why we need this check... */
if (!dev->driver)
return NULL;
/* TODO: use i2c_verify_client() when it accepts const pointer */
client = (dev->type == &i2c_client_type) ? to_i2c_client(dev) : NULL;
if (!client)
return NULL;
> + return i2c_get_match_data_helper(client);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs
2023-08-02 11:23 ` [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs Biju Das
@ 2023-08-02 18:00 ` Dmitry Torokhov
2023-08-03 6:05 ` Biju Das
2023-08-02 21:39 ` Andy Shevchenko
1 sibling, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2023-08-02 18:00 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Andy Shevchenko,
linux-renesas-soc
Hi Niju,
On Wed, Aug 02, 2023 at 12:23:17PM +0100, Biju Das wrote:
> @@ -141,6 +141,7 @@ i2c_of_match_device_sysfs(const struct of_device_id *matches,
>
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(i2c_of_match_device_sysfs);
This is private to i2c core, you do not need to export it to modules.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs
2023-08-02 11:23 ` [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs Biju Das
2023-08-02 18:00 ` Dmitry Torokhov
@ 2023-08-02 21:39 ` Andy Shevchenko
2023-08-03 6:04 ` Biju Das
1 sibling, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2023-08-02 21:39 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
linux-renesas-soc
On Wed, Aug 02, 2023 at 12:23:17PM +0100, Biju Das wrote:
> Extend i2c_device_get_match_data() to i2c sysfs interface by changing
> i2c_of_match_device_sysfs() to non-static and call this function as a
> fallback for i2c_get_match_data_helper().
...
> /* TODO: use i2c_verify_client() when it accepts const pointer */
> const struct i2c_client *client = (dev->type == &i2c_client_type) ?
> to_i2c_client(dev) : NULL;
> + const void *data;
Oh, yeah, definitely, please follow advice for previous patch I have given.
> if (!client || !dev->driver)
> return NULL;
>
> - return i2c_get_match_data_helper(client);
> + data = i2c_get_match_data_helper(client);
> + if (data)
> + return data;
> +
> + if (dev->driver->of_match_table) {
> + const struct of_device_id *match;
> +
> + match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
> + client);
So, with this it makes sense to have
strucct device_driver *drv = dev->driver;
...
if (!drv)
return NULL;
The above for the first patch even. And here
match = i2c_of_match_device_sysfs(drv->of_match_table, client);
> + if (match)
> + return match->data;
> + }
> + return data;
This actually should be
return NULL;
to avoid (potentially) stale pointers.
> }
...
> -static const struct of_device_id*
> +const struct of_device_id *
> i2c_of_match_device_sysfs(const struct of_device_id *matches,
> - struct i2c_client *client)
> + const struct i2c_client *client)
> {
> const char *name;
>
> @@ -141,6 +141,7 @@ i2c_of_match_device_sysfs(const struct of_device_id *matches,
>
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(i2c_of_match_device_sysfs);
>
> const struct of_device_id
> *i2c_of_match_device(const struct of_device_id *matches,
> diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
> index 1247e6e6e975..e4d397b67989 100644
> --- a/drivers/i2c/i2c-core.h
> +++ b/drivers/i2c/i2c-core.h
> @@ -82,8 +82,17 @@ static inline void i2c_acpi_remove_space_handler(struct i2c_adapter *adapter) {
>
> #ifdef CONFIG_OF
> void of_i2c_register_devices(struct i2c_adapter *adap);
> +const struct of_device_id *
> +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> + const struct i2c_client *client);
> #else
> static inline void of_i2c_register_devices(struct i2c_adapter *adap) { }
> +static inline const struct of_device_id *
> +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> + const struct i2c_client *client)
> +{
> + return NULL;
> +}
> #endif
> extern struct notifier_block i2c_of_notifier;
That's what above I meant to see as a separate change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback
2023-08-02 15:08 ` Andy Shevchenko
@ 2023-08-03 6:01 ` Biju Das
0 siblings, 0 replies; 10+ messages in thread
From: Biju Das @ 2023-08-03 6:01 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
linux-renesas-soc
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v4 2/3] i2c: Add i2c_device_get_match_data()
> callback
>
> On Wed, Aug 02, 2023 at 12:23:16PM +0100, Biju Das wrote:
> > Add i2c_device_get_match_data() callback to struct bus_type().
> >
> > While at it, introduced i2c_get_match_data_helper() to avoid code
> > duplication with i2c_get_match_data().
>
> ...
>
> > +static const void *i2c_get_match_data_helper(const struct i2c_client
> > +*client)
> > {
> > - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
> > + const struct i2c_driver *driver = to_i2c_driver(client-
> >dev.driver);
> > const struct i2c_device_id *match;
> > +
> > + match = i2c_match_id(driver->id_table, client);
> > + if (!match)
> > + return NULL;
> > +
> > + return (const void *)match->driver_data; }
>
> Yes, perfect!
>
> ...
>
> > +static const void *i2c_device_get_match_data(const struct device
> > +*dev) {
> > + /* TODO: use i2c_verify_client() when it accepts const pointer */
> > + const struct i2c_client *client = (dev->type ==
> &i2c_client_type) ?
> > + to_i2c_client(dev) : NULL;
> > +
> > + if (!client || !dev->driver)
> > + return NULL;
> > +
> > + return i2c_get_match_data_helper(client);
>
>
> I believe below looks better from readability and maintenance
> perspectives.
Agreed. Will Incorporate latest comment from v4 as well.
Cheers,
Biju
>
> const struct i2c_client *client;
>
> /* ...comment as in Dmitry's reply in v3 thread on why we need
> this check... */
> if (!dev->driver)
> return NULL;
>
> /* TODO: use i2c_verify_client() when it accepts const pointer */
> client = (dev->type == &i2c_client_type) ? to_i2c_client(dev) :
> NULL;
> if (!client)
> return NULL;
>
> > + return i2c_get_match_data_helper(client);
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs
2023-08-02 21:39 ` Andy Shevchenko
@ 2023-08-03 6:04 ` Biju Das
0 siblings, 0 replies; 10+ messages in thread
From: Biju Das @ 2023-08-03 6:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
linux-renesas-soc
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to
> support i2c sysfs
>
> On Wed, Aug 02, 2023 at 12:23:17PM +0100, Biju Das wrote:
> > Extend i2c_device_get_match_data() to i2c sysfs interface by changing
> > i2c_of_match_device_sysfs() to non-static and call this function as a
> > fallback for i2c_get_match_data_helper().
>
> ...
>
> > /* TODO: use i2c_verify_client() when it accepts const pointer */
> > const struct i2c_client *client = (dev->type ==
> &i2c_client_type) ?
> > to_i2c_client(dev) : NULL;
> > + const void *data;
>
> Oh, yeah, definitely, please follow advice for previous patch I have
> given.
OK.
>
> > if (!client || !dev->driver)
> > return NULL;
> >
> > - return i2c_get_match_data_helper(client);
> > + data = i2c_get_match_data_helper(client);
> > + if (data)
> > + return data;
> > +
> > + if (dev->driver->of_match_table) {
> > + const struct of_device_id *match;
> > +
> > + match = i2c_of_match_device_sysfs(dev->driver-
> >of_match_table,
> > + client);
>
> So, with this it makes sense to have
>
> strucct device_driver *drv = dev->driver;
> ...
> if (!drv)
> return NULL;
>
> The above for the first patch even. And here
Agreed.
>
> match = i2c_of_match_device_sysfs(drv->of_match_table,
> client);
>
> > + if (match)
> > + return match->data;
> > + }
>
> > + return data;
>
> This actually should be
>
> return NULL;
>
> to avoid (potentially) stale pointers.
OK.
>
> > }
>
> ...
>
> > -static const struct of_device_id*
> > +const struct of_device_id *
> > i2c_of_match_device_sysfs(const struct of_device_id *matches,
> > - struct i2c_client *client)
> > + const struct i2c_client *client)
> > {
> > const char *name;
> >
> > @@ -141,6 +141,7 @@ i2c_of_match_device_sysfs(const struct
> > of_device_id *matches,
> >
> > return NULL;
> > }
> > +EXPORT_SYMBOL_GPL(i2c_of_match_device_sysfs);
> >
> > const struct of_device_id
> > *i2c_of_match_device(const struct of_device_id *matches, diff --git
> > a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h index
> > 1247e6e6e975..e4d397b67989 100644
> > --- a/drivers/i2c/i2c-core.h
> > +++ b/drivers/i2c/i2c-core.h
> > @@ -82,8 +82,17 @@ static inline void
> > i2c_acpi_remove_space_handler(struct i2c_adapter *adapter) {
> >
> > #ifdef CONFIG_OF
> > void of_i2c_register_devices(struct i2c_adapter *adap);
> > +const struct of_device_id *
> > +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> > + const struct i2c_client *client);
> > #else
> > static inline void of_i2c_register_devices(struct i2c_adapter *adap)
> > { }
> > +static inline const struct of_device_id *
> > +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> > + const struct i2c_client *client) {
> > + return NULL;
> > +}
> > #endif
> > extern struct notifier_block i2c_of_notifier;
>
> That's what above I meant to see as a separate change.
OK will create a separate patch for this change.
Cheers,
Biju
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs
2023-08-02 18:00 ` Dmitry Torokhov
@ 2023-08-03 6:05 ` Biju Das
0 siblings, 0 replies; 10+ messages in thread
From: Biju Das @ 2023-08-03 6:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Andy Shevchenko,
linux-renesas-soc
Hi Dmitry Torokhov,
Thanks for the feedback.
> Subject: Re: [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to
> support i2c sysfs
>
> Hi Niju,
>
> On Wed, Aug 02, 2023 at 12:23:17PM +0100, Biju Das wrote:
> > @@ -141,6 +141,7 @@ i2c_of_match_device_sysfs(const struct
> of_device_id *matches,
> >
> > return NULL;
> > }
> > +EXPORT_SYMBOL_GPL(i2c_of_match_device_sysfs);
>
> This is private to i2c core, you do not need to export it to modules.
Agreed. Drop this export in next version.
Cheers,
Biju
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-03 6:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-02 11:23 [PATCH v4 0/3] Extend device_get_match_data() to struct bus_type Biju Das
2023-08-02 11:23 ` [PATCH v4 1/3] drivers: fwnode: " Biju Das
2023-08-02 11:23 ` [PATCH v4 2/3] i2c: Add i2c_device_get_match_data() callback Biju Das
2023-08-02 15:08 ` Andy Shevchenko
2023-08-03 6:01 ` Biju Das
2023-08-02 11:23 ` [PATCH v4 3/3] i2c: Extend i2c_device_get_match_data() to support i2c sysfs Biju Das
2023-08-02 18:00 ` Dmitry Torokhov
2023-08-03 6:05 ` Biju Das
2023-08-02 21:39 ` Andy Shevchenko
2023-08-03 6:04 ` Biju Das
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.