[PATCH RFC 0/2] Extend device_get_match_data() to struct bus_type

[PATCH RFC 0/2] Extend device_get_match_data() to struct bus_type

[Biju Das]

This patch series extend device_get_match_data() to struct bus_type,
so that buses like I2C can get matched data.

Biju Das (2):
  drivers: fwnode: Extend device_get_match_data() to struct bus_type
  i2c: Add i2c_device_get_match_data() callback

 drivers/base/property.c     |  8 +++++++-
 drivers/i2c/i2c-core-base.c | 38 +++++++++++++++++++++++++++++--------
 include/linux/device/bus.h  |  3 +++
 3 files changed, 40 insertions(+), 9 deletions(-)

-- 
2.25.1

[PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

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().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/base/property.c    | 8 +++++++-
 include/linux/device/bus.h | 3 +++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8c40abed7852..cc0bf7bb6f3a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1277,7 +1277,13 @@ EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
 
 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 && dev->bus && dev->bus->get_match_data)
+		data = dev->bus->get_match_data(dev);
+
+	return data;
 }
 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

[PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Biju Das]

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().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/i2c/i2c-core-base.c | 38 +++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 60746652fd52..80259111355b 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 }
 EXPORT_SYMBOL_GPL(i2c_match_id);
 
+static void *i2c_get_match_data_helper(struct i2c_driver *driver,
+				       const struct i2c_client *client)
+{
+	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)
+{
+	const struct i2c_client *client = to_i2c_client(dev);
+	const struct i2c_driver *driver;
+
+	if (!dev->driver)
+		return NULL;
+
+	driver = to_i2c_driver(dev->driver);
+	if (!driver)
+		return NULL;
+
+	return i2c_get_match_data_helper(driver, client);
+}
+
 const void *i2c_get_match_data(const struct i2c_client *client)
 {
 	struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
-	const struct i2c_device_id *match;
 	const void *data;
 
 	data = device_get_match_data(&client->dev);
-	if (!data) {
-		match = i2c_match_id(driver->id_table, client);
-		if (!match)
-			return NULL;
-
-		data = (const void *)match->driver_data;
-	}
+	if (!data)
+		data = i2c_get_match_data_helper(driver, client);
 
 	return data;
 }
@@ -695,6 +716,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

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[kernel test robot]

Hi Biju,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on driver-core/driver-core-next driver-core/driver-core-linus wsa/i2c/for-next linus/master v6.5-rc2 next-20230721]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Biju-Das/drivers-fwnode-Extend-device_get_match_data-to-struct-bus_type/20230723-163825
base:   driver-core/driver-core-testing
patch link:    https://lore.kernel.org/r/20230723083721.35384-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback
config: nios2-randconfig-r033-20230723 (https://download.01.org/0day-ci/archive/20230723/202307231907.FiS4VQie-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230723/202307231907.FiS4VQie-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307231907.FiS4VQie-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/i2c/i2c-core-base.c: In function 'i2c_get_match_data_helper':
>> drivers/i2c/i2c-core-base.c:126:16: warning: return discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     126 |         return (const void *)match->driver_data;
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/i2c-core-base.c: In function 'i2c_device_get_match_data':
>> drivers/i2c/i2c-core-base.c:141:42: warning: passing argument 1 of 'i2c_get_match_data_helper' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     141 |         return i2c_get_match_data_helper(driver, client);
         |                                          ^~~~~~
   drivers/i2c/i2c-core-base.c:117:59: note: expected 'struct i2c_driver *' but argument is of type 'const struct i2c_driver *'
     117 | static void *i2c_get_match_data_helper(struct i2c_driver *driver,
         |                                        ~~~~~~~~~~~~~~~~~~~^~~~~~


vim +/const +126 drivers/i2c/i2c-core-base.c

   116	
   117	static void *i2c_get_match_data_helper(struct i2c_driver *driver,
   118					       const struct i2c_client *client)
   119	{
   120		const struct i2c_device_id *match;
   121	
   122		match = i2c_match_id(driver->id_table, client);
   123		if (!match)
   124			return NULL;
   125	
 > 126		return (const void *)match->driver_data;
   127	}
   128	
   129	static const void *i2c_device_get_match_data(const struct device *dev)
   130	{
   131		const struct i2c_client *client = to_i2c_client(dev);
   132		const struct i2c_driver *driver;
   133	
   134		if (!dev->driver)
   135			return NULL;
   136	
   137		driver = to_i2c_driver(dev->driver);
   138		if (!driver)
   139			return NULL;
   140	
 > 141		return i2c_get_match_data_helper(driver, client);
   142	}
   143	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[kernel test robot]

Hi Biju,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on driver-core/driver-core-next driver-core/driver-core-linus wsa/i2c/for-next linus/master v6.5-rc2 next-20230721]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Biju-Das/drivers-fwnode-Extend-device_get_match_data-to-struct-bus_type/20230723-163825
base:   driver-core/driver-core-testing
patch link:    https://lore.kernel.org/r/20230723083721.35384-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback
config: sh-randconfig-r023-20230723 (https://download.01.org/0day-ci/archive/20230723/202307231900.3fDdJkXn-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230723/202307231900.3fDdJkXn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307231900.3fDdJkXn-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/i2c/i2c-core-base.c: In function 'i2c_get_match_data_helper':
>> drivers/i2c/i2c-core-base.c:126:16: warning: return discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     126 |         return (const void *)match->driver_data;
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/i2c/i2c-core-base.c: In function 'i2c_device_get_match_data':
>> drivers/i2c/i2c-core-base.c:141:42: warning: passing argument 1 of 'i2c_get_match_data_helper' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     141 |         return i2c_get_match_data_helper(driver, client);
         |                                          ^~~~~~
   drivers/i2c/i2c-core-base.c:117:59: note: expected 'struct i2c_driver *' but argument is of type 'const struct i2c_driver *'
     117 | static void *i2c_get_match_data_helper(struct i2c_driver *driver,
         |                                        ~~~~~~~~~~~~~~~~~~~^~~~~~


vim +/const +126 drivers/i2c/i2c-core-base.c

   116	
   117	static void *i2c_get_match_data_helper(struct i2c_driver *driver,
   118					       const struct i2c_client *client)
   119	{
   120		const struct i2c_device_id *match;
   121	
   122		match = i2c_match_id(driver->id_table, client);
   123		if (!match)
   124			return NULL;
   125	
 > 126		return (const void *)match->driver_data;
   127	}
   128	
   129	static const void *i2c_device_get_match_data(const struct device *dev)
   130	{
   131		const struct i2c_client *client = to_i2c_client(dev);
   132		const struct i2c_driver *driver;
   133	
   134		if (!dev->driver)
   135			return NULL;
   136	
   137		driver = to_i2c_driver(dev->driver);
   138		if (!driver)
   139			return NULL;
   140	
 > 141		return i2c_get_match_data_helper(driver, client);
   142	}
   143	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Biju Das]

Hi kernel test robot,

> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Sent: Sunday, July 23, 2023 12:19 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: oe-kbuild-all@lists.linux.dev
> Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> callback
> 
> Hi Biju,
> 
> [This is a private test report for your RFC patch.] kernel test robot
> noticed the following build warnings:
> 
> [auto build test WARNING on driver-core/driver-core-testing] [also build
> test WARNING on driver-core/driver-core-next driver-core/driver-core-
> linus wsa/i2c/for-next linus/master v6.5-rc2 next-20230721] [cannot
> apply to sailus-media-tree/streams] [If your patch is applied to the
> wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> 
> 
> url:
> patch link:
> 
> patch subject: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> callback
> config: nios2-randconfig-r033-20230723
> 
> compiler: nios2-linux-gcc (GCC) 12.3.0
> reproduce:
> If you fix the issue in a separate patch/commit (i.e. not just a new
> version of the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes:
> | https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> | .kernel.org%2Foe-kbuild-all%2F202307231907.FiS4VQie-lkp%40intel.com%2F
> | &data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7Ca41dac8cc153434a166508d
> | b8b6e99e7%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638257079370000
> | 053%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> | I6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iy644qCgKSJ5WOIgu1q9DLb
> | w%2FZDcAdH6roPahE4L9Bo%3D&reserved=0
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/i2c/i2c-core-base.c: In function 'i2c_get_match_data_helper':
> >> drivers/i2c/i2c-core-base.c:126:16: warning: return discards 'const'
> >> qualifier from pointer target type [-Wdiscarded-qualifiers]
>      126 |         return (const void *)match->driver_data;
>          |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/i2c/i2c-core-base.c: In function 'i2c_device_get_match_data':
> >> drivers/i2c/i2c-core-base.c:141:42: warning: passing argument 1 of
> >> 'i2c_get_match_data_helper' discards 'const' qualifier from pointer
> >> target type [-Wdiscarded-qualifiers]
>      141 |         return i2c_get_match_data_helper(driver, client);
>          |                                          ^~~~~~
>    drivers/i2c/i2c-core-base.c:117:59: note: expected 'struct i2c_driver
> *' but argument is of type 'const struct i2c_driver *'
>      117 | static void *i2c_get_match_data_helper(struct i2c_driver
> *driver,
>          |
> ~~~~~~~~~~~~~~~~~~~^~~~~~
> 
> 
> vim +/const +126 drivers/i2c/i2c-core-base.c
> 
>    116
>    117	static void *i2c_get_match_data_helper(struct i2c_driver
> *driver,
>    118					       const struct i2c_client
> *client)
>    119	{
>    120		const struct i2c_device_id *match;
>    121
>    122		match = i2c_match_id(driver->id_table, client);
>    123		if (!match)
>    124			return NULL;
>    125
>  > 126		return (const void *)match->driver_data;
>    127	}
>    128
>    129	static const void *i2c_device_get_match_data(const struct
> device *dev)
>    130	{
>    131		const struct i2c_client *client = to_i2c_client(dev);
>    132		const struct i2c_driver *driver;
>    133
>    134		if (!dev->driver)
>    135			return NULL;
>    136
>    137		driver = to_i2c_driver(dev->driver);
>    138		if (!driver)
>    139			return NULL;
>    140
>  > 141		return i2c_get_match_data_helper(driver, client);
>    142	}

Ok, will update once there is any feedback on RFC.

+static const void *i2c_get_match_data_helper(const struct i2c_driver *driver,
+                                            const struct i2c_client *client)
 {
        const struct i2c_device_id *match;
 
@@ -143,7 +143,7 @@ static const void *i2c_device_get_match_data(const struct device *dev)
 
 const void *i2c_get_match_data(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 void *data;
 
        data = device_get_match_data(&client->dev);

Cheers,
biju

RE: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Biju Das]

Hi kernel test robot,

> -----Original Message-----
> From: kernel test robot <lkp@intel.com>
> Sent: Sunday, July 23, 2023 12:29 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: oe-kbuild-all@lists.linux.dev
> Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> callback
> 
> Hi Biju,
> 
> [This is a private test report for your RFC patch.] kernel test robot
> noticed the following build warnings:

> patch subject: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> callback
> config: sh-randconfig-r023-20230723
> 
> compiler: sh4-linux-gcc (GCC) 12.3.0
> reproduce:
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new
> version of the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes:
> | 
> All warnings (new ones prefixed by >>):
> 
>    drivers/i2c/i2c-core-base.c: In function 'i2c_get_match_data_helper':
> >> drivers/i2c/i2c-core-base.c:126:16: warning: return discards 'const'
> >> qualifier from pointer target type [-Wdiscarded-qualifiers]
>      126 |         return (const void *)match->driver_data;
>          |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/i2c/i2c-core-base.c: In function 'i2c_device_get_match_data':
> >> drivers/i2c/i2c-core-base.c:141:42: warning: passing argument 1 of
> >> 'i2c_get_match_data_helper' discards 'const' qualifier from pointer
> >> target type [-Wdiscarded-qualifiers]
>      141 |         return i2c_get_match_data_helper(driver, client);
>          |                                          ^~~~~~
>    drivers/i2c/i2c-core-base.c:117:59: note: expected 'struct i2c_driver
> *' but argument is of type 'const struct i2c_driver *'
>      117 | static void *i2c_get_match_data_helper(struct i2c_driver
> *driver,
>          |
> ~~~~~~~~~~~~~~~~~~~^~~~~~
> 
> 
> vim +/const +126 drivers/i2c/i2c-core-base.c
> 
>    116
>    117	static void *i2c_get_match_data_helper(struct i2c_driver
> *driver,
>    118					       const struct i2c_client
> *client)
>    119	{
>    120		const struct i2c_device_id *match;
>    121
>    122		match = i2c_match_id(driver->id_table, client);
>    123		if (!match)
>    124			return NULL;
>    125
>  > 126		return (const void *)match->driver_data;
>    127	}
>    128
>    129	static const void *i2c_device_get_match_data(const struct
> device *dev)
>    130	{
>    131		const struct i2c_client *client = to_i2c_client(dev);
>    132		const struct i2c_driver *driver;
>    133
>    134		if (!dev->driver)
>    135			return NULL;
>    136
>    137		driver = to_i2c_driver(dev->driver);
>    138		if (!driver)
>    139			return NULL;
>    140
>  > 141		return i2c_get_match_data_helper(driver, client);
>    142	}
>    143

Ok, will update once there is any feedback on RFC.

+static const void *i2c_get_match_data_helper(const struct i2c_driver *driver,
+                                            const struct i2c_client *client)
 {
        const struct i2c_device_id *match;
 
@@ -143,7 +143,7 @@ static const void *i2c_device_get_match_data(const struct device *dev)
 
 const void *i2c_get_match_data(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 void *data;
 
        data = device_get_match_data(&client->dev);

Cheers,
biju

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[kernel test robot]

Hi Biju,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on driver-core/driver-core-next driver-core/driver-core-linus wsa/i2c/for-next linus/master v6.5-rc2 next-20230721]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Biju-Das/drivers-fwnode-Extend-device_get_match_data-to-struct-bus_type/20230723-163825
base:   driver-core/driver-core-testing
patch link:    https://lore.kernel.org/r/20230723083721.35384-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback
config: hexagon-randconfig-r041-20230723 (https://download.01.org/0day-ci/archive/20230723/202307232145.kmTzXn7a-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230723/202307232145.kmTzXn7a-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307232145.kmTzXn7a-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/i2c/i2c-core-base.c:23:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/i2c/i2c-core-base.c:23:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/i2c/i2c-core-base.c:23:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/i2c/i2c-core-base.c:126:9: error: returning 'const void *' from a function with result type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
     126 |         return (const void *)match->driver_data;
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/i2c/i2c-core-base.c:141:35: error: passing 'const struct i2c_driver *' to parameter of type 'struct i2c_driver *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
     141 |         return i2c_get_match_data_helper(driver, client);
         |                                          ^~~~~~
   drivers/i2c/i2c-core-base.c:117:59: note: passing argument to parameter 'driver' here
     117 | static void *i2c_get_match_data_helper(struct i2c_driver *driver,
         |                                                           ^
   6 warnings and 2 errors generated.


vim +126 drivers/i2c/i2c-core-base.c

   116	
   117	static void *i2c_get_match_data_helper(struct i2c_driver *driver,
   118					       const struct i2c_client *client)
   119	{
   120		const struct i2c_device_id *match;
   121	
   122		match = i2c_match_id(driver->id_table, client);
   123		if (!match)
   124			return NULL;
   125	
 > 126		return (const void *)match->driver_data;
   127	}
   128	
   129	static const void *i2c_device_get_match_data(const struct device *dev)
   130	{
   131		const struct i2c_client *client = to_i2c_client(dev);
   132		const struct i2c_driver *driver;
   133	
   134		if (!dev->driver)
   135			return NULL;
   136	
   137		driver = to_i2c_driver(dev->driver);
   138		if (!driver)
   139			return NULL;
   140	
 > 141		return i2c_get_match_data_helper(driver, client);
   142	}
   143	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Dmitry Torokhov]

On Sun, Jul 23, 2023 at 09:37:21AM +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().
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/i2c/i2c-core-base.c | 38 +++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 60746652fd52..80259111355b 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
>  }
>  EXPORT_SYMBOL_GPL(i2c_match_id);
>  
> +static void *i2c_get_match_data_helper(struct i2c_driver *driver,
> +				       const struct i2c_client *client)
> +{
> +	const struct i2c_device_id *match;
> +
> +	match = i2c_match_id(driver->id_table, client);
> +	if (!match)
> +		return NULL;
> +
> +	return (const void *)match->driver_data;

Not sure if this helper is needed given that in the end (hopefully) we
can simply remove i2c_get_match_data() and leave only
i2c_device_get_match_data().

i2c_get_match_data() and leave only i2c_device_get_match_data().

Thanks.

-- 
Dmitry

Re: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Andy Shevchenko]

On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:

Thank you for your contribution!
My comments below.

> 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().

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

You can't just throw one's SoB tag without clear understanding what's going on
here (either wrong authorship or missing Co-developed-by or...?).

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

...

>  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 && dev->bus && dev->bus->get_match_data)
> +		data = dev->bus->get_match_data(dev);
> +
> +	return data;

Much better looking is

	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;

>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Andy Shevchenko]

On Mon, Jul 24, 2023 at 02:06:07PM +0300, Andy Shevchenko wrote:
> On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:
> 
> Thank you for your contribution!
> My comments below.
> 
> > 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().
> 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> You can't just throw one's SoB tag without clear understanding what's going on
> here (either wrong authorship or missing Co-developed-by or...?).
> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

...

> >  const void *device_get_match_data(const struct device *dev)

Btw, this needs a documentation update to explain how it works now.

> >  {
> > -	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 && dev->bus && dev->bus->get_match_data)
> > +		data = dev->bus->get_match_data(dev);
> > +
> > +	return data;
> 
> Much better looking is
> 
> 	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;
> 
> >  }

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

Hi Andy,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> device_get_match_data() to struct bus_type
> 
> On Mon, Jul 24, 2023 at 02:06:07PM +0300, Andy Shevchenko wrote:
> > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:
> >
> > Thank you for your contribution!
> > My comments below.
> >
> > > 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().
> >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> > You can't just throw one's SoB tag without clear understanding what's
> > going on here (either wrong authorship or missing Co-developed-by
> or...?).
> >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> ...
> 
> > >  const void *device_get_match_data(const struct device *dev)
> 
> Btw, this needs a documentation update to explain how it works now.

Can you please point me to the location where I need to update?

Cheers,
Biju

> 
> > >  {
> > > -	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 && dev->bus && dev->bus->get_match_data)
> > > +		data = dev->bus->get_match_data(dev);
> > > +
> > > +	return data;
> >
> > Much better looking is
> >
> > 	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;
> >
> > >  }
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

RE: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

Hi Andy Shevchenko,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> device_get_match_data() to struct bus_type
> 
> On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:
> 
> Thank you for your contribution!
> My comments below.
> 
> > 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().
> 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> You can't just throw one's SoB tag without clear understanding what's
> going on here (either wrong authorship or missing Co-developed-by
> or...?).

Dmitry feels instead of having separate bus based match_data() like i2c_get_match_data[2] and spi_get_device_match_data[3], it is
better to have a generic approach like a single API device_get_match_data()
for getting match_data for OF/ACPI/I2C/SPI tables.

So, he came with a proposal and shared some code here[1].
Since,I have send this patch, I put my signed -off.

If this patch is accepted, then we can get rid of bus based match_data.

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25436207

[2] https://elixir.bootlin.com/linux/v6.5-rc3/source/drivers/i2c/i2c-core-base.c#L117

[3] https://elixir.bootlin.com/linux/v6.5-rc3/source/drivers/spi/spi.c#L364

Cheers,
Biju


> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> ...
> 
> >  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 && dev->bus && dev->bus->get_match_data)
> > +		data = dev->bus->get_match_data(dev);
> > +
> > +	return data;
> 
> Much better looking is
> 
> 	data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data,
> dev);
> 	if (data)
> 		return data;

OK.

Cheers,
Biju

> 
> 	if (dev->bus && dev->bus->get_match_data)
> 		return dev->bus->get_match_data(dev);
> 
> 	return NULL;
> 
> >  }
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Re: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Andy Shevchenko]

On Mon, Jul 24, 2023 at 11:46:44AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > device_get_match_data() to struct bus_type
> > On Mon, Jul 24, 2023 at 02:06:07PM +0300, Andy Shevchenko wrote:
> > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:

...

> > > >  const void *device_get_match_data(const struct device *dev)

(1)

> > Btw, this needs a documentation update to explain how it works now.
> 
> Can you please point me to the location where I need to update?

Sure. It's just on top of the (1). It looks like no documentation
yet existed, so you need to create one.

Not sure if fwnode.h has to be updated. At least it doesn't contradict
with the added code, while not describing all details.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Andy Shevchenko]

On Mon, Jul 24, 2023 at 12:02:27PM +0000, Biju Das wrote:
> > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > device_get_match_data() to struct bus_type
> > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:

...

> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > You can't just throw one's SoB tag without clear understanding what's
> > going on here (either wrong authorship or missing Co-developed-by
> > or...?).
> 
> Dmitry feels instead of having separate bus based match_data() like
> i2c_get_match_data[2] and spi_get_device_match_data[3], it is better to have
> a generic approach like a single API device_get_match_data() for getting
> match_data for OF/ACPI/I2C/SPI tables.
> 
> So, he came with a proposal and shared some code here[1].

Yes, I'm pretty much following the discussion.

> Since,I have send this patch, I put my signed -off.

I'm not talking about this. There is no evidence that Dmitry gives you
any approval to use or clear SoB tag. Again, you may not do like this.

> If this patch is accepted, then we can get rid of bus based match_data.

This is unrelated to what I talking about.

> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25436207

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

Hi Andy,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> device_get_match_data() to struct bus_type
> 
> On Mon, Jul 24, 2023 at 12:02:27PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023 at
> > > 09:37:20AM +0100, Biju Das wrote:
> 
> ...
> 
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > >
> > > You can't just throw one's SoB tag without clear understanding
> > > what's going on here (either wrong authorship or missing
> > > Co-developed-by or...?).
> >
> > Dmitry feels instead of having separate bus based match_data() like
> > i2c_get_match_data[2] and spi_get_device_match_data[3], it is better
> > to have a generic approach like a single API device_get_match_data()
> > for getting match_data for OF/ACPI/I2C/SPI tables.
> >
> > So, he came with a proposal and shared some code here[1].
> 
> Yes, I'm pretty much following the discussion.
> 
> > Since,I have send this patch, I put my signed -off.
> 
> I'm not talking about this. There is no evidence that Dmitry gives you
> any approval to use or clear SoB tag. Again, you may not do like this.

Here Dmitry is acknowledging, he is ok with the patch I posted.

https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25437032

Cheers,
Biju

RE: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

Hi Andy Shevchenko,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> device_get_match_data() to struct bus_type
> 
> On Mon, Jul 24, 2023 at 11:46:44AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 at
> > > 02:06:07PM +0300, Andy Shevchenko wrote:
> > > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote:
> 
> ...
> 
> > > > >  const void *device_get_match_data(const struct device *dev)
> 
> (1)
> 
> > > Btw, this needs a documentation update to explain how it works now.
> >
> > Can you please point me to the location where I need to update?
> 
> Sure. It's just on top of the (1). It looks like no documentation yet
> existed, so you need to create one.
> 
> Not sure if fwnode.h has to be updated. At least it doesn't contradict
> with the added code, while not describing all details.

OK, will do.

Cheers,
Biju

Re: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Andy Shevchenko]

On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote:
> > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > device_get_match_data() to struct bus_type
> > On Mon, Jul 24, 2023 at 12:02:27PM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023 at
> > > > 09:37:20AM +0100, Biju Das wrote:

...

> > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > >
> > > > You can't just throw one's SoB tag without clear understanding
> > > > what's going on here (either wrong authorship or missing
> > > > Co-developed-by or...?).
> > >
> > > Dmitry feels instead of having separate bus based match_data() like
> > > i2c_get_match_data[2] and spi_get_device_match_data[3], it is better
> > > to have a generic approach like a single API device_get_match_data()
> > > for getting match_data for OF/ACPI/I2C/SPI tables.
> > >
> > > So, he came with a proposal and shared some code here[1].
> > 
> > Yes, I'm pretty much following the discussion.
> > 
> > > Since,I have send this patch, I put my signed -off.
> > 
> > I'm not talking about this. There is no evidence that Dmitry gives you
> > any approval to use or clear SoB tag. Again, you may not do like this.
> 
> Here Dmitry is acknowledging, he is ok with the patch I posted.
> 
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25437032

No, you just misinterpreted his message.

See https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
for the explanation. The SoB has to be explicitly given. Dmitry had _not_
put it like this.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

Hi Andy,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> device_get_match_data() to struct bus_type
> 
> On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 at
> > > 12:02:27PM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023
> > > > > at 09:37:20AM +0100, Biju Das wrote:
> 
> ...
> 
> > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > >
> > > > > You can't just throw one's SoB tag without clear understanding
> > > > > what's going on here (either wrong authorship or missing
> > > > > Co-developed-by or...?).
> > > >
> > > > Dmitry feels instead of having separate bus based match_data()
> > > > like i2c_get_match_data[2] and spi_get_device_match_data[3], it is
> > > > better to have a generic approach like a single API
> > > > device_get_match_data() for getting match_data for OF/ACPI/I2C/SPI
> tables.
> > > >
> > > > So, he came with a proposal and shared some code here[1].
> > >
> > > Yes, I'm pretty much following the discussion.
> > >
> > > > Since,I have send this patch, I put my signed -off.
> > >
> > > I'm not talking about this. There is no evidence that Dmitry gives
> > > you any approval to use or clear SoB tag. Again, you may not do like
> this.
> >
> > Here Dmitry is acknowledging, he is ok with the patch I posted.
> >
> 
> No, you just misinterpreted his message.
> 

Dmitry,

As you are the author of code, either you post a patch or provide your SoB as per the guideline mentioned here to avoid confusion.

 https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Cheers,
Biju

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Geert Uytterhoeven]

Hi Biju,

On Sun, Jul 23, 2023 at 10:37 AM Biju Das <biju.das.jz@bp.renesas.com> 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().
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
>  }
>  EXPORT_SYMBOL_GPL(i2c_match_id);
>
> +static void *i2c_get_match_data_helper(struct i2c_driver *driver,

static const void *

> +                                      const struct i2c_client *client)
> +{
> +       const struct i2c_device_id *match;
> +
> +       match = i2c_match_id(driver->id_table, client);
> +       if (!match)
> +               return NULL;
> +
> +       return (const void *)match->driver_data;

I guess your compiler didn't complain about the const/non-const
conversion when returning because it inlined the function?

> +}
> +
> +static const void *i2c_device_get_match_data(const struct device *dev)
> +{
> +       const struct i2c_client *client = to_i2c_client(dev);
> +       const struct i2c_driver *driver;
> +
> +       if (!dev->driver)
> +               return NULL;
> +
> +       driver = to_i2c_driver(dev->driver);
> +       if (!driver)
> +               return NULL;

"driver" can never be NULL here.

> +
> +       return i2c_get_match_data_helper(driver, client);
> +}
> +

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> callback
> 
> Hi Biju,
> 
> On Sun, Jul 23, 2023 at 10:37 AM Biju Das <biju.das.jz@bp.renesas.com>
> 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().
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const
> > struct i2c_device_id *id,  }  EXPORT_SYMBOL_GPL(i2c_match_id);
> >
> > +static void *i2c_get_match_data_helper(struct i2c_driver *driver,
> 
> static const void *

I missed this.

> 
> > +                                      const struct i2c_client
> > +*client) {
> > +       const struct i2c_device_id *match;
> > +
> > +       match = i2c_match_id(driver->id_table, client);
> > +       if (!match)
> > +               return NULL;
> > +
> > +       return (const void *)match->driver_data;
> 
> I guess your compiler didn't complain about the const/non-const
> conversion when returning because it inlined the function?

It complained. Somehow, I didn't notice that warning before sending the patch.

> 
> > +}
> > +
> > +static const void *i2c_device_get_match_data(const struct device
> > +*dev) {
> > +       const struct i2c_client *client = to_i2c_client(dev);

Not sure, non-const i2c_verify_client(dev)to be used here??

> > +       const struct i2c_driver *driver;
> > +
> > +       if (!dev->driver)
> > +               return NULL;
> > +
> > +       driver = to_i2c_driver(dev->driver);
> > +       if (!driver)
> > +               return NULL;
> 
> "driver" can never be NULL here.

OK, will remove this.

Cheers,
Biju

> 
> > +
> > +       return i2c_get_match_data_helper(driver, client); }
> > +
> 

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Dmitry Torokhov]

On Mon, Jul 24, 2023 at 03:06:50PM +0000, Biju Das wrote:
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> > callback
> > 
> > Hi Biju,
> > 
> > On Sun, Jul 23, 2023 at 10:37 AM Biju Das <biju.das.jz@bp.renesas.com>
> > 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().
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > 
> > Thanks for your patch!
> > 
> > > --- a/drivers/i2c/i2c-core-base.c
> > > +++ b/drivers/i2c/i2c-core-base.c
> > > @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const
> > > struct i2c_device_id *id,  }  EXPORT_SYMBOL_GPL(i2c_match_id);
> > >
> > > +static void *i2c_get_match_data_helper(struct i2c_driver *driver,
> > 
> > static const void *
> 
> I missed this.
> 
> > 
> > > +                                      const struct i2c_client
> > > +*client) {
> > > +       const struct i2c_device_id *match;
> > > +
> > > +       match = i2c_match_id(driver->id_table, client);
> > > +       if (!match)
> > > +               return NULL;
> > > +
> > > +       return (const void *)match->driver_data;
> > 
> > I guess your compiler didn't complain about the const/non-const
> > conversion when returning because it inlined the function?
> 
> It complained. Somehow, I didn't notice that warning before sending the patch.
> 
> > 
> > > +}
> > > +
> > > +static const void *i2c_device_get_match_data(const struct device
> > > +*dev) {
> > > +       const struct i2c_client *client = to_i2c_client(dev);
> 
> Not sure, non-const i2c_verify_client(dev)to be used here??

Good call, it actually should, as i2c bus contains instances of both
i2c_client and i2c_adapter.

Unfortunately i2c_verify_client() right now is a function, we might need
to turn it into a macro to allow transparently handle const/non-const
device argument... If this is too hard at the moment we could open-code
i2c_verify_client() in i2c_device_get_match_data() and first check on
the device type before doing to_i2c_client() conversion.

Also I see i2c_device_remove() uses to_i2c_client(). I guess it only
works because i2c adapters never have drivers assigned to them, but it
would be nice to use i2c_verify_client() and maybe put a big fat warning
if we get wrong type of device there.

Thanks.

-- 
Dmitry

Re: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Dmitry Torokhov]

On Mon, Jul 24, 2023 at 01:58:55PM +0000, Biju Das wrote:
> Hi Andy,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > device_get_match_data() to struct bus_type
> > 
> > On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 at
> > > > 12:02:27PM +0000, Biju Das wrote:
> > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023
> > > > > > at 09:37:20AM +0100, Biju Das wrote:
> > 
> > ...
> > 
> > > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > > >
> > > > > > You can't just throw one's SoB tag without clear understanding
> > > > > > what's going on here (either wrong authorship or missing
> > > > > > Co-developed-by or...?).
> > > > >
> > > > > Dmitry feels instead of having separate bus based match_data()
> > > > > like i2c_get_match_data[2] and spi_get_device_match_data[3], it is
> > > > > better to have a generic approach like a single API
> > > > > device_get_match_data() for getting match_data for OF/ACPI/I2C/SPI
> > tables.
> > > > >
> > > > > So, he came with a proposal and shared some code here[1].
> > > >
> > > > Yes, I'm pretty much following the discussion.
> > > >
> > > > > Since,I have send this patch, I put my signed -off.
> > > >
> > > > I'm not talking about this. There is no evidence that Dmitry gives
> > > > you any approval to use or clear SoB tag. Again, you may not do like
> > this.
> > >
> > > Here Dmitry is acknowledging, he is ok with the patch I posted.
> > >
> > 
> > No, you just misinterpreted his message.
> > 
> 
> Dmitry,
> 
> As you are the author of code, either you post a patch or provide your SoB as per the guideline mentioned here to avoid confusion.
> 
>  https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

It was not really proper patch, consider it as an email with parts
written in unified diff, as sometimes it is easier than to explain in
words, and I do not want to take much credit for it.

If you wish you can put "Suggested-by" for me, or just drop my name off
the patch description altogether.

Thanks.

-- 
Dmitry

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Geert Uytterhoeven]

Hi Dmitry,

On Mon, Jul 24, 2023 at 6:35 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Jul 24, 2023 at 03:06:50PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> > > callback
> > > On Sun, Jul 23, 2023 at 10:37 AM Biju Das <biju.das.jz@bp.renesas.com>
> > > 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().
> > > >
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/i2c/i2c-core-base.c
> > > > +++ b/drivers/i2c/i2c-core-base.c
> > > > @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const
> > > > struct i2c_device_id *id,  }  EXPORT_SYMBOL_GPL(i2c_match_id);
> > > >
> > > > +static void *i2c_get_match_data_helper(struct i2c_driver *driver,
> > >
> > > static const void *
> >
> > I missed this.
> >
> > >
> > > > +                                      const struct i2c_client
> > > > +*client) {
> > > > +       const struct i2c_device_id *match;
> > > > +
> > > > +       match = i2c_match_id(driver->id_table, client);
> > > > +       if (!match)
> > > > +               return NULL;
> > > > +
> > > > +       return (const void *)match->driver_data;
> > >
> > > I guess your compiler didn't complain about the const/non-const
> > > conversion when returning because it inlined the function?
> >
> > It complained. Somehow, I didn't notice that warning before sending the patch.
> >
> > >
> > > > +}
> > > > +
> > > > +static const void *i2c_device_get_match_data(const struct device
> > > > +*dev) {
> > > > +       const struct i2c_client *client = to_i2c_client(dev);
> >
> > Not sure, non-const i2c_verify_client(dev)to be used here??
>
> Good call, it actually should, as i2c bus contains instances of both
> i2c_client and i2c_adapter.
>
> Unfortunately i2c_verify_client() right now is a function, we might need
> to turn it into a macro to allow transparently handle const/non-const
> device argument... If this is too hard at the moment we could open-code
> i2c_verify_client() in i2c_device_get_match_data() and first check on
> the device type before doing to_i2c_client() conversion.

Tadah, we have _Generic()! See container_of_const():
https://elixir.bootlin.com/linux/latest/source/include/linux/container_of.h#L25

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data() callback

[Dmitry Torokhov]

On Mon, Jul 24, 2023 at 06:43:30PM +0200, Geert Uytterhoeven wrote:
> Hi Dmitry,
> 
> On Mon, Jul 24, 2023 at 6:35 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Mon, Jul 24, 2023 at 03:06:50PM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH RFC 2/2] i2c: Add i2c_device_get_match_data()
> > > > callback
> > > > On Sun, Jul 23, 2023 at 10:37 AM Biju Das <biju.das.jz@bp.renesas.com>
> > > > 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().
> > > > >
> > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > >
> > > > Thanks for your patch!
> > > >
> > > > > --- a/drivers/i2c/i2c-core-base.c
> > > > > +++ b/drivers/i2c/i2c-core-base.c
> > > > > @@ -114,20 +114,41 @@ const struct i2c_device_id *i2c_match_id(const
> > > > > struct i2c_device_id *id,  }  EXPORT_SYMBOL_GPL(i2c_match_id);
> > > > >
> > > > > +static void *i2c_get_match_data_helper(struct i2c_driver *driver,
> > > >
> > > > static const void *
> > >
> > > I missed this.
> > >
> > > >
> > > > > +                                      const struct i2c_client
> > > > > +*client) {
> > > > > +       const struct i2c_device_id *match;
> > > > > +
> > > > > +       match = i2c_match_id(driver->id_table, client);
> > > > > +       if (!match)
> > > > > +               return NULL;
> > > > > +
> > > > > +       return (const void *)match->driver_data;
> > > >
> > > > I guess your compiler didn't complain about the const/non-const
> > > > conversion when returning because it inlined the function?
> > >
> > > It complained. Somehow, I didn't notice that warning before sending the patch.
> > >
> > > >
> > > > > +}
> > > > > +
> > > > > +static const void *i2c_device_get_match_data(const struct device
> > > > > +*dev) {
> > > > > +       const struct i2c_client *client = to_i2c_client(dev);
> > >
> > > Not sure, non-const i2c_verify_client(dev)to be used here??
> >
> > Good call, it actually should, as i2c bus contains instances of both
> > i2c_client and i2c_adapter.
> >
> > Unfortunately i2c_verify_client() right now is a function, we might need
> > to turn it into a macro to allow transparently handle const/non-const
> > device argument... If this is too hard at the moment we could open-code
> > i2c_verify_client() in i2c_device_get_match_data() and first check on
> > the device type before doing to_i2c_client() conversion.
> 
> Tadah, we have _Generic()! See container_of_const():
> https://elixir.bootlin.com/linux/latest/source/include/linux/container_of.h#L25

I think we might want to go through all buses and switch to_XXX()
implementations there to use container_of_const() instead of old
container_of() there.

This still does not help with i2c_verify_client() being currently a
function, so it still needs to be converted to a macro to be able to
return const and nont-const pointers, depending on the argument type.

Thanks.

-- 
Dmitry

RE: [PATCH RFC 1/2] drivers: fwnode: Extend device_get_match_data() to struct bus_type

[Biju Das]

> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> device_get_match_data() to struct bus_type
> 
> On Mon, Jul 24, 2023 at 01:58:55PM +0000, Biju Das wrote:
> > Hi Andy,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > device_get_match_data() to struct bus_type
> > >
> > > On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023
> > > > > at 12:02:27PM +0000, Biju Das wrote:
> > > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend
> > > > > > > device_get_match_data() to struct bus_type On Sun, Jul 23,
> > > > > > > 2023 at 09:37:20AM +0100, Biju Das wrote:
> > >
> > > ...
> > >
> > > > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > > > >
> > > > > > > You can't just throw one's SoB tag without clear
> > > > > > > understanding what's going on here (either wrong authorship
> > > > > > > or missing Co-developed-by or...?).
> > > > > >
> > > > > > Dmitry feels instead of having separate bus based match_data()
> > > > > > like i2c_get_match_data[2] and spi_get_device_match_data[3],
> > > > > > it is better to have a generic approach like a single API
> > > > > > device_get_match_data() for getting match_data for
> > > > > > OF/ACPI/I2C/SPI
> > > tables.
> > > > > >
> > > > > > So, he came with a proposal and shared some code here[1].
> > > > >
> > > > > Yes, I'm pretty much following the discussion.
> > > > >
> > > > > > Since,I have send this patch, I put my signed -off.
> > > > >
> > > > > I'm not talking about this. There is no evidence that Dmitry
> > > > > gives you any approval to use or clear SoB tag. Again, you may
> > > > > not do like
> > > this.
> > > >
> > > > Here Dmitry is acknowledging, he is ok with the patch I posted.
> > > >
> > >
> > > No, you just misinterpreted his message.
> > >
> >
> > Dmitry,
> >
> > As you are the author of code, either you post a patch or provide your
> SoB as per the guideline mentioned here to avoid confusion.
> >
> >
> It was not really proper patch, consider it as an email with parts
> written in unified diff, as sometimes it is easier than to explain in
> words, and I do not want to take much credit for it.
> 
> If you wish you can put "Suggested-by" for me, or just drop my name off
> the patch description altogether.

Sure, will add Suggested-by tag.

Cheers,
Biju