* [PATCH v3 0/2] Extend device_get_match_data() to struct bus_type
@ 2023-08-01 17:03 Biju Das
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
0 siblings, 1 reply; 16+ messages in thread
From: Biju Das @ 2023-08-01 17:03 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.
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 (2):
drivers: fwnode: Extend device_get_match_data() to struct bus_type
i2c: Add i2c_device_get_match_data() callback
drivers/base/property.c | 21 +++++++++++++++-
drivers/i2c/i2c-core-base.c | 49 +++++++++++++++++++++++++++++++------
drivers/i2c/i2c-core-of.c | 3 ++-
include/linux/device/bus.h | 3 +++
include/linux/i2c.h | 11 +++++++++
5 files changed, 77 insertions(+), 10 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 [PATCH v3 0/2] Extend device_get_match_data() to struct bus_type Biju Das
@ 2023-08-01 17:03 ` Biju Das
2023-08-01 19:28 ` Andy Shevchenko
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Biju Das @ 2023-08-01 17:03 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>
---
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 | 49 +++++++++++++++++++++++++++++++------
drivers/i2c/i2c-core-of.c | 3 ++-
include/linux/i2c.h | 11 +++++++++
3 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 60746652fd52..7db881a923b6 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -114,23 +114,55 @@ 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_driver *driver,
+ const struct i2c_client *client)
{
- 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;
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 (!dev->driver)
+ return NULL;
+
+ data = i2c_get_match_data_helper(to_i2c_driver(dev->driver), client);
+ if (data)
+ return data;
- data = (const void *)match->driver_data;
+ if (dev->driver->of_match_table) {
+ const struct of_device_id *match;
+
+ match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
+ (struct i2c_client *)client);
+ if (match)
+ data = match->data;
}
return data;
}
+
+const void *i2c_get_match_data(const struct i2c_client *client)
+{
+ const struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
+ const void *data;
+
+ data = device_get_match_data(&client->dev);
+ if (data)
+ return data;
+
+ return i2c_get_match_data_helper(driver, client);
+}
EXPORT_SYMBOL(i2c_get_match_data);
static int i2c_device_match(struct device *dev, struct device_driver *drv)
@@ -695,6 +727,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);
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index a6c407d36800..514bf8cddb81 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -113,7 +113,7 @@ 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)
{
@@ -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/include/linux/i2c.h b/include/linux/i2c.h
index 3430cc2b05a6..597aa0f1ed0b 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -999,6 +999,10 @@ static inline struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node
return i2c_get_adapter_by_fwnode(of_fwnode_handle(node));
}
+const struct of_device_id*
+i2c_of_match_device_sysfs(const struct of_device_id *matches,
+ struct i2c_client *client);
+
const struct of_device_id
*i2c_of_match_device(const struct of_device_id *matches,
struct i2c_client *client);
@@ -1030,6 +1034,13 @@ static inline const struct of_device_id
return NULL;
}
+static inline const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+ struct i2c_client *client)
+{
+ return NULL;
+}
+
static inline int of_i2c_get_board_info(struct device *dev,
struct device_node *node,
struct i2c_board_info *info)
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
@ 2023-08-01 19:28 ` Andy Shevchenko
2023-08-01 20:41 ` Dmitry Torokhov
2023-08-02 7:59 ` Biju Das
2023-08-01 19:29 ` Andy Shevchenko
` (4 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-08-01 19:28 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
linux-renesas-soc
On Tue, Aug 01, 2023 at 06:03:18PM +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().
Have you used --patience when prepared the patch for sending?
...
> -const void *i2c_get_match_data(const struct i2c_client *client)
> +static const void *i2c_get_match_data_helper(const struct i2c_driver *driver,
> + const struct i2c_client *client)
> {
> - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
Does it make sense to remove and add an additional parameter? In one case it's
a copy, in another it probably the same, just hidden in the code.
> 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;
> const void *data;
> + if (!dev->driver)
> + return NULL;
When can this be true?
> + data = i2c_get_match_data_helper(to_i2c_driver(dev->driver), client);
> + if (data)
> + return data;
>
> - data = (const void *)match->driver_data;
> + if (dev->driver->of_match_table) {
> + const struct of_device_id *match;
> +
> + match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
> + (struct i2c_client *)client);
> + if (match)
> + data = match->data;
> }
>
> return data;
> }
...
> -static const struct of_device_id*
> +const struct of_device_id*
While here, add a missing space after of_device_id.
...
> +const struct of_device_id*
Ditto.
Or use below (weird) style in case it occurs more often than usual one.
> +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> + struct i2c_client *client);
> +
> const struct of_device_id
> *i2c_of_match_device(const struct of_device_id *matches,
> struct i2c_client *client);
...
> +static inline const struct of_device_id
> +*i2c_of_match_device(const struct of_device_id *matches,
As per above.
> + struct i2c_client *client)
> +{
> + return NULL;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
2023-08-01 19:28 ` Andy Shevchenko
@ 2023-08-01 19:29 ` Andy Shevchenko
2023-08-02 6:34 ` Biju Das
2023-08-02 4:12 ` kernel test robot
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2023-08-01 19:29 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c, Geert Uytterhoeven, Dmitry Torokhov,
linux-renesas-soc
On Tue, Aug 01, 2023 at 06:03:18PM +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().
...
> * Changed i2c_of_match_device_sysfs() as non-static function as it is
> needed for i2c_device_get_match_data().
Btw, this can be split to a separate change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 19:28 ` Andy Shevchenko
@ 2023-08-01 20:41 ` Dmitry Torokhov
2023-08-02 9:34 ` Biju Das
2023-08-02 7:59 ` Biju Das
1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2023-08-01 20:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Biju Das, Wolfram Sang, linux-i2c, Geert Uytterhoeven,
linux-renesas-soc
On Tue, Aug 01, 2023 at 10:28:38PM +0300, Andy Shevchenko wrote:
> On Tue, Aug 01, 2023 at 06:03:18PM +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().
>
> Have you used --patience when prepared the patch for sending?
>
> ...
>
> > -const void *i2c_get_match_data(const struct i2c_client *client)
> > +static const void *i2c_get_match_data_helper(const struct i2c_driver *driver,
> > + const struct i2c_client *client)
> > {
>
> > - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
>
> Does it make sense to remove and add an additional parameter? In one case it's
> a copy, in another it probably the same, just hidden in the code.
>
> > 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;
> > const void *data;
>
> > + if (!dev->driver)
> > + return NULL;
>
> When can this be true?
It is not guaranteed that the function is always called on a device
bound to a driver (even though we normally expect this to be the case).
>
> > + data = i2c_get_match_data_helper(to_i2c_driver(dev->driver), client);
> > + if (data)
> > + return data;
> >
> > - data = (const void *)match->driver_data;
> > + if (dev->driver->of_match_table) {
> > + const struct of_device_id *match;
> > +
> > + match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
> > + (struct i2c_client *)client);
Can we make i2c_of_match_device_sysfs() take a const pointer to a
client? Casting away constness is something that we should avoid.
> > + if (match)
> > + data = match->data;
> > }
> >
> > return data;
> > }
>
> ...
>
> > -static const struct of_device_id*
> > +const struct of_device_id*
>
> While here, add a missing space after of_device_id.
>
> ...
>
> > +const struct of_device_id*
>
> Ditto.
>
> Or use below (weird) style in case it occurs more often than usual one.
>
> > +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> > + struct i2c_client *client);
> > +
> > const struct of_device_id
> > *i2c_of_match_device(const struct of_device_id *matches,
> > struct i2c_client *client);
>
> ...
>
> > +static inline const struct of_device_id
> > +*i2c_of_match_device(const struct of_device_id *matches,
>
> As per above.
Was it supposed to be i2c_of_match_device_sysfs()? Also, this should be
in drivers/i2c/i2c-core.h, not in public header.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
2023-08-01 19:28 ` Andy Shevchenko
2023-08-01 19:29 ` Andy Shevchenko
@ 2023-08-02 4:12 ` kernel test robot
2023-08-02 11:04 ` Biju Das
2023-08-02 4:22 ` kernel test robot
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: kernel test robot @ 2023-08-02 4:12 UTC (permalink / raw)
To: Biju Das, Wolfram Sang
Cc: llvm, oe-kbuild-all, Biju Das, linux-i2c, Geert Uytterhoeven,
Dmitry Torokhov, Andy Shevchenko, linux-renesas-soc
Hi Biju,
kernel test robot noticed the following build errors:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.5-rc4 next-20230801]
[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/20230802-010931
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230801170318.82682-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
config: hexagon-randconfig-r024-20230731 (https://download.01.org/0day-ci/archive/20230802/202308021149.cnDNnUAh-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce: (https://download.01.org/0day-ci/archive/20230802/202308021149.cnDNnUAh-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/202308021149.cnDNnUAh-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/i2c/i2c-core-slave.c:12:
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]
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]
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'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/i2c/i2c-core-slave.c:12:
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]
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'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/i2c/i2c-core-slave.c:12:
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]
__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]
__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]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
In file included from drivers/i2c/i2c-core-slave.c:12:
>> include/linux/i2c.h:1038:2: error: redefinition of 'i2c_of_match_device'
*i2c_of_match_device(const struct of_device_id *matches,
^
include/linux/i2c.h:1031:2: note: previous definition is here
*i2c_of_match_device(const struct of_device_id *matches,
^
6 warnings and 1 error generated.
--
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]
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]
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'
#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]
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'
#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]
__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]
__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]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
In file included from drivers/i2c/i2c-core-base.c:23:
>> include/linux/i2c.h:1038:2: error: redefinition of 'i2c_of_match_device'
*i2c_of_match_device(const struct of_device_id *matches,
^
include/linux/i2c.h:1031:2: note: previous definition is here
*i2c_of_match_device(const struct of_device_id *matches,
^
>> drivers/i2c/i2c-core-base.c:146:11: error: implicit declaration of function 'i2c_of_match_device_sysfs' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
^
drivers/i2c/i2c-core-base.c:146:11: note: did you mean 'i2c_of_match_device'?
include/linux/i2c.h:1038:2: note: 'i2c_of_match_device' declared here
*i2c_of_match_device(const struct of_device_id *matches,
^
>> drivers/i2c/i2c-core-base.c:146:9: warning: incompatible integer to pointer conversion assigning to 'const struct of_device_id *' from 'int' [-Wint-conversion]
match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7 warnings and 2 errors generated.
vim +/i2c_of_match_device +1038 include/linux/i2c.h
1036
1037 static inline const struct of_device_id
> 1038 *i2c_of_match_device(const struct of_device_id *matches,
1039 struct i2c_client *client)
1040 {
1041 return NULL;
1042 }
1043
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
` (2 preceding siblings ...)
2023-08-02 4:12 ` kernel test robot
@ 2023-08-02 4:22 ` kernel test robot
2023-08-02 4:22 ` kernel test robot
2023-08-02 5:25 ` kernel test robot
5 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-08-02 4:22 UTC (permalink / raw)
To: Biju Das, Wolfram Sang
Cc: oe-kbuild-all, Biju Das, linux-i2c, Geert Uytterhoeven,
Dmitry Torokhov, Andy Shevchenko, linux-renesas-soc
Hi Biju,
kernel test robot noticed the following build errors:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.5-rc4 next-20230801]
[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/20230802-010931
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230801170318.82682-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
config: parisc-randconfig-r026-20230731 (https://download.01.org/0day-ci/archive/20230802/202308021202.uZeI7Cnt-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230802/202308021202.uZeI7Cnt-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/202308021202.uZeI7Cnt-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from drivers/i2c/i2c-core-base.c:23:
include/linux/i2c.h:1038:2: error: redefinition of 'i2c_of_match_device'
1038 | *i2c_of_match_device(const struct of_device_id *matches,
| ^~~~~~~~~~~~~~~~~~~
include/linux/i2c.h:1031:2: note: previous definition of 'i2c_of_match_device' with type 'const struct of_device_id *(const struct of_device_id *, struct i2c_client *)'
1031 | *i2c_of_match_device(const struct of_device_id *matches,
| ^~~~~~~~~~~~~~~~~~~
drivers/i2c/i2c-core-base.c: In function 'i2c_device_get_match_data':
>> drivers/i2c/i2c-core-base.c:146:25: error: implicit declaration of function 'i2c_of_match_device_sysfs'; did you mean 'i2c_of_match_device'? [-Werror=implicit-function-declaration]
146 | match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| i2c_of_match_device
>> drivers/i2c/i2c-core-base.c:146:23: warning: assignment to 'const struct of_device_id *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
146 | match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
| ^
cc1: some warnings being treated as errors
vim +146 drivers/i2c/i2c-core-base.c
128
129 static const void *i2c_device_get_match_data(const struct device *dev)
130 {
131 /* TODO: use i2c_verify_client() when it accepts const pointer */
132 const struct i2c_client *client = (dev->type == &i2c_client_type) ?
133 to_i2c_client(dev) : NULL;
134 const void *data;
135
136 if (!dev->driver)
137 return NULL;
138
139 data = i2c_get_match_data_helper(to_i2c_driver(dev->driver), client);
140 if (data)
141 return data;
142
143 if (dev->driver->of_match_table) {
144 const struct of_device_id *match;
145
> 146 match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
147 (struct i2c_client *)client);
148 if (match)
149 data = match->data;
150 }
151
152 return data;
153 }
154
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
` (3 preceding siblings ...)
2023-08-02 4:22 ` kernel test robot
@ 2023-08-02 4:22 ` kernel test robot
2023-08-02 5:25 ` kernel test robot
5 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-08-02 4:22 UTC (permalink / raw)
To: Biju Das, Wolfram Sang
Cc: llvm, oe-kbuild-all, Biju Das, linux-i2c, Geert Uytterhoeven,
Dmitry Torokhov, Andy Shevchenko, linux-renesas-soc
Hi Biju,
kernel test robot noticed the following build errors:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.5-rc4 next-20230801]
[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/20230802-010931
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230801170318.82682-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
config: i386-randconfig-r014-20230731 (https://download.01.org/0day-ci/archive/20230802/202308021233.ztWtV82e-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230802/202308021233.ztWtV82e-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/202308021233.ztWtV82e-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/i2c/i2c-core-base.c:23:
>> include/linux/i2c.h:1038:2: error: redefinition of 'i2c_of_match_device'
*i2c_of_match_device(const struct of_device_id *matches,
^
include/linux/i2c.h:1031:2: note: previous definition is here
*i2c_of_match_device(const struct of_device_id *matches,
^
>> drivers/i2c/i2c-core-base.c:146:11: error: call to undeclared function 'i2c_of_match_device_sysfs'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
^
drivers/i2c/i2c-core-base.c:146:11: note: did you mean 'i2c_of_match_device'?
include/linux/i2c.h:1038:2: note: 'i2c_of_match_device' declared here
*i2c_of_match_device(const struct of_device_id *matches,
^
>> drivers/i2c/i2c-core-base.c:146:9: error: incompatible integer to pointer conversion assigning to 'const struct of_device_id *' from 'int' [-Wint-conversion]
match = i2c_of_match_device_sysfs(dev->driver->of_match_table,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
--
In file included from drivers/i2c/i2c-smbus.c:11:
>> include/linux/i2c.h:1038:2: error: redefinition of 'i2c_of_match_device'
*i2c_of_match_device(const struct of_device_id *matches,
^
include/linux/i2c.h:1031:2: note: previous definition is here
*i2c_of_match_device(const struct of_device_id *matches,
^
1 error generated.
vim +/i2c_of_match_device +1038 include/linux/i2c.h
1036
1037 static inline const struct of_device_id
> 1038 *i2c_of_match_device(const struct of_device_id *matches,
1039 struct i2c_client *client)
1040 {
1041 return NULL;
1042 }
1043
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
` (4 preceding siblings ...)
2023-08-02 4:22 ` kernel test robot
@ 2023-08-02 5:25 ` kernel test robot
5 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-08-02 5:25 UTC (permalink / raw)
To: Biju Das, Wolfram Sang
Cc: oe-kbuild-all, Biju Das, linux-i2c, Geert Uytterhoeven,
Dmitry Torokhov, Andy Shevchenko, linux-renesas-soc
Hi Biju,
kernel test robot noticed the following build warnings:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.5-rc4 next-20230801]
[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/20230802-010931
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230801170318.82682-3-biju.das.jz%40bp.renesas.com
patch subject: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230802/202308021316.lB5HzyKZ-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230802/202308021316.lB5HzyKZ-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/202308021316.lB5HzyKZ-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/fb.h:6,
from include/linux/fb.h:7,
from include/linux/vga_switcheroo.h:34,
from sound/pci/hda/hda_intel.c:52:
include/linux/i2c.h:1038:2: error: redefinition of 'i2c_of_match_device'
1038 | *i2c_of_match_device(const struct of_device_id *matches,
| ^~~~~~~~~~~~~~~~~~~
include/linux/i2c.h:1031:2: note: previous definition of 'i2c_of_match_device' with type 'const struct of_device_id *(const struct of_device_id *, struct i2c_client *)'
1031 | *i2c_of_match_device(const struct of_device_id *matches,
| ^~~~~~~~~~~~~~~~~~~
In file included from include/linux/container_of.h:5,
from include/linux/kernel.h:21,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/current.h:10,
from include/linux/sched.h:12,
from include/linux/delay.h:23,
from sound/pci/hda/hda_intel.c:23:
include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
| ^
include/linux/compiler.h:231:33: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
231 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
| ^~~~~~~~~~~~~~~~~
include/linux/kernel.h:56:59: note: in expansion of macro '__must_be_array'
56 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
| ^~~~~~~~~~~~~~~
include/linux/moduleparam.h:517:20: note: in expansion of macro 'ARRAY_SIZE'
517 | = { .max = ARRAY_SIZE(array), .num = nump, \
| ^~~~~~~~~~
include/linux/moduleparam.h:501:9: note: in expansion of macro 'module_param_array_named'
501 | module_param_array_named(name, name, type, nump, perm)
| ^~~~~~~~~~~~~~~~~~~~~~~~
sound/pci/hda/hda_intel.c:125:1: note: in expansion of macro 'module_param_array'
125 | module_param_array(index, int, NULL, 0444);
| ^~~~~~~~~~~~~~~~~~
>> sound/pci/hda/hda_intel.c:104:12: warning: 'index' defined but not used [-Wunused-variable]
104 | static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
| ^~~~~
vim +/index +104 sound/pci/hda/hda_intel.c
33124929a23c5b5 Takashi Iwai 2014-06-26 102
^1da177e4c3f415 Linus Torvalds 2005-04-16 103
5aba4f8ec72b2b8 Takashi Iwai 2008-01-07 @104 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
5aba4f8ec72b2b8 Takashi Iwai 2008-01-07 105 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;
a67ff6a54095e27 Rusty Russell 2011-12-15 106 static bool enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE_PNP;
5aba4f8ec72b2b8 Takashi Iwai 2008-01-07 107 static char *model[SNDRV_CARDS];
1dac6695c683c66 Takashi Iwai 2012-09-13 108 static int position_fix[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1};
5c0d7bc103dd1ae Takashi Iwai 2008-06-10 109 static int bdl_pos_adj[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1};
5aba4f8ec72b2b8 Takashi Iwai 2008-01-07 110 static int probe_mask[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1};
d4d9cd0338892e7 Takashi Iwai 2008-12-19 111 static int probe_only[SNDRV_CARDS];
26a6cb6cca225f6 David Henningsson 2012-10-09 112 static int jackpoll_ms[SNDRV_CARDS];
41438f1314b0f6f Takashi Iwai 2017-01-12 113 static int single_cmd = -1;
71623855e20c3fe Takashi Iwai 2009-09-28 114 static int enable_msi = -1;
4ea6fbc8eb23c3a Takashi Iwai 2009-06-17 115 #ifdef CONFIG_SND_HDA_PATCH_LOADER
4ea6fbc8eb23c3a Takashi Iwai 2009-06-17 116 static char *patch[SNDRV_CARDS];
4ea6fbc8eb23c3a Takashi Iwai 2009-06-17 117 #endif
2dca0bba70ce3c2 Jaroslav Kysela 2009-11-13 118 #ifdef CONFIG_SND_HDA_INPUT_BEEP
0920c9b4c4d8960 Takashi Iwai 2012-07-03 119 static bool beep_mode[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] =
2dca0bba70ce3c2 Jaroslav Kysela 2009-11-13 120 CONFIG_SND_HDA_INPUT_BEEP_MODE};
2dca0bba70ce3c2 Jaroslav Kysela 2009-11-13 121 #endif
7fba6aea4472f01 Takashi Iwai 2020-01-09 122 static bool dmic_detect = 1;
d045bceff5a904b Jaroslav Kysela 2023-02-02 123 static bool ctl_dev_id = IS_ENABLED(CONFIG_SND_HDA_CTL_DEV_ID) ? 1 : 0;
^1da177e4c3f415 Linus Torvalds 2005-04-16 124
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 19:29 ` Andy Shevchenko
@ 2023-08-02 6:34 ` Biju Das
2023-08-02 14:44 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Biju Das @ 2023-08-02 6:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, linux-i2c@vger.kernel.org, Geert Uytterhoeven,
Dmitry Torokhov, linux-renesas-soc@vger.kernel.org
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data()
> callback
>
> On Tue, Aug 01, 2023 at 06:03:18PM +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().
>
> ...
>
> > * Changed i2c_of_match_device_sysfs() as non-static function as it is
> > needed for i2c_device_get_match_data().
>
> Btw, this can be split to a separate change.
OK, first patch is callback with I2C table match and
Second patch is for handling i2c_of_match_device().
Cheers,
Biju
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 19:28 ` Andy Shevchenko
2023-08-01 20:41 ` Dmitry Torokhov
@ 2023-08-02 7:59 ` Biju Das
2023-08-02 14:41 ` Andy Shevchenko
1 sibling, 1 reply; 16+ messages in thread
From: Biju Das @ 2023-08-02 7:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wolfram Sang, linux-i2c@vger.kernel.org, Geert Uytterhoeven,
Dmitry Torokhov, linux-renesas-soc@vger.kernel.org
Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data()
> callback
>
> On Tue, Aug 01, 2023 at 06:03:18PM +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().
>
> Have you used --patience when prepared the patch for sending?
Normally, I use "git format-patch -n --subject-prefix="PATCH vY" --cover-letter" for preparing patch.
I see there is a difference with "git format-patch -n --patience *".
I will send this patch series with --patience option.
>
> ...
>
> > -const void *i2c_get_match_data(const struct i2c_client *client)
> > +static const void *i2c_get_match_data_helper(const struct i2c_driver
> *driver,
> > + const struct i2c_client *client)
> > {
>
> > - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
>
> Does it make sense to remove and add an additional parameter? In one
> case it's a copy, in another it probably the same, just hidden in the
> code.
Ok, you mean add the below check in i2c_device_get_match_data() and
drop *driver parameter from i2c_get_match_data_helper().
if (!client || !dev->driver)
return NULL;
>
> > 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;
> > const void *data;
>
> > + if (!dev->driver)
> > + return NULL;
>
> When can this be true?
>
> > + data = i2c_get_match_data_helper(to_i2c_driver(dev->driver),
> client);
> > + if (data)
> > + return data;
> >
> > - data = (const void *)match->driver_data;
> > + if (dev->driver->of_match_table) {
> > + const struct of_device_id *match;
> > +
> > + match = i2c_of_match_device_sysfs(dev->driver-
> >of_match_table,
> > + (struct i2c_client *)client);
> > + if (match)
> > + data = match->data;
> > }
> >
> > return data;
> > }
>
> ...
>
> > -static const struct of_device_id*
> > +const struct of_device_id*
>
> While here, add a missing space after of_device_id.
OK.
>
> ...
>
> > +const struct of_device_id*
>
> Ditto.
>
> Or use below (weird) style in case it occurs more often than usual one.
It is one of case. So, I will use space after of_device_id.
>
> > +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> > + struct i2c_client *client);
> > +
> > const struct of_device_id
> > *i2c_of_match_device(const struct of_device_id *matches,
> > struct i2c_client *client);
>
> ...
>
> > +static inline const struct of_device_id *i2c_of_match_device(const
> > +struct of_device_id *matches,
>
> As per above.
OK, This will be moved to i2c-core.h.
Cheers,
Biju
>
> > + struct i2c_client *client)
> > +{
> > + return NULL;
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-01 20:41 ` Dmitry Torokhov
@ 2023-08-02 9:34 ` Biju Das
2023-08-02 14:23 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Biju Das @ 2023-08-02 9:34 UTC (permalink / raw)
To: Dmitry Torokhov, Andy Shevchenko
Cc: Wolfram Sang, linux-i2c@vger.kernel.org, Geert Uytterhoeven,
linux-renesas-soc@vger.kernel.org
Hi Dmitry Torokhov,
Thanks for the feedback.
> Subject: Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data()
> callback
>
> On Tue, Aug 01, 2023 at 10:28:38PM +0300, Andy Shevchenko wrote:
> > On Tue, Aug 01, 2023 at 06:03:18PM +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().
> >
> > Have you used --patience when prepared the patch for sending?
> >
> > ...
> >
> > > -const void *i2c_get_match_data(const struct i2c_client *client)
> > > +static const void *i2c_get_match_data_helper(const struct
> i2c_driver *driver,
> > > + const struct i2c_client *client)
> > > {
> >
> > > - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
> >
> > Does it make sense to remove and add an additional parameter? In one
> > case it's a copy, in another it probably the same, just hidden in the
> code.
> >
> > > 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;
> > > const void *data;
> >
> > > + if (!dev->driver)
> > > + return NULL;
> >
> > When can this be true?
>
> It is not guaranteed that the function is always called on a device
> bound to a driver (even though we normally expect this to be the case).
>
> >
> > > + data = i2c_get_match_data_helper(to_i2c_driver(dev->driver),
> client);
> > > + if (data)
> > > + return data;
> > >
> > > - data = (const void *)match->driver_data;
> > > + if (dev->driver->of_match_table) {
> > > + const struct of_device_id *match;
> > > +
> > > + match = i2c_of_match_device_sysfs(dev->driver-
> >of_match_table,
> > > + (struct i2c_client *)client);
>
> Can we make i2c_of_match_device_sysfs() take a const pointer to a
> client? Casting away constness is something that we should avoid.
I agree, we are not supposed to modify client pointer inside
i2c_of_match_device_sysfs(), so const makes sense here.
Wolfram, I guess you are ok with it.
>
> > > + if (match)
> > > + data = match->data;
> > > }
> > >
> > > return data;
> > > }
> >
> > ...
> >
> > > -static const struct of_device_id*
> > > +const struct of_device_id*
> >
> > While here, add a missing space after of_device_id.
> >
> > ...
> >
> > > +const struct of_device_id*
> >
> > Ditto.
> >
> > Or use below (weird) style in case it occurs more often than usual
> one.
> >
> > > +i2c_of_match_device_sysfs(const struct of_device_id *matches,
> > > + struct i2c_client *client);
> > > +
> > > const struct of_device_id
> > > *i2c_of_match_device(const struct of_device_id *matches,
> > > struct i2c_client *client);
> >
> > ...
> >
> > > +static inline const struct of_device_id *i2c_of_match_device(const
> > > +struct of_device_id *matches,
> >
> > As per above.
>
> Was it supposed to be i2c_of_match_device_sysfs()? Also, this should be
> in drivers/i2c/i2c-core.h, not in public header.
Agreed.
Cheers,
Biju
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-02 4:12 ` kernel test robot
@ 2023-08-02 11:04 ` Biju Das
0 siblings, 0 replies; 16+ messages in thread
From: Biju Das @ 2023-08-02 11:04 UTC (permalink / raw)
To: kernel test robot, Wolfram Sang
Cc: llvm@lists.linux.dev, oe-kbuild-all@lists.linux.dev,
linux-i2c@vger.kernel.org, Geert Uytterhoeven, Dmitry Torokhov,
Andy Shevchenko, linux-renesas-soc@vger.kernel.org
I will send V4 fixing this error and also fixing review comments from Andy and Dmitry.
Cheers,
Biju
> Subject: Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data()
> callback
>
> Hi Biju,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on wsa/i2c/for-next] [also build test ERROR on
> driver-core/driver-core-testing driver-core/driver-core-next driver-
> core/driver-core-linus linus/master v6.5-rc4 next-20230801] [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%2Fdocs%2Fgit-format-
> patch%23_base_tree_information&data=05%7C01%7Cbiju.das.jz%40bp.renesas.c
> om%7C0a6df48bde964a9d1df208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a
> %7C0%7C0%7C638265463936796169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=p
> KKEfl6xAPN%2F3isXNWiWAvVClYEajI1Yih0NrnrohWI%3D&reserved=0]
>
> url:
> https://github/
> .com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FBiju-Das%2Fdrivers-fwnode-
> Extend-device_get_match_data-to-struct-bus_type%2F20230802-
> 010931&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C0a6df48bde964a9d1df
> 208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C6382654639367
> 96169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> I6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CUEi5wumZLT0TK8CAzNxKJ6tW
> csSgwP1y3m9IHDbAlo%3D&reserved=0
> base:
> https://git.ke/
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fwsa%2Flinux.git&data=05%7C
> 01%7Cbiju.das.jz%40bp.renesas.com%7C0a6df48bde964a9d1df208db930ec91c%7C5
> 3d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638265463936796169%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
> 6Mn0%3D%7C3000%7C%7C%7C&sdata=m7eaBOcvLLPUFOm4TZkwztRu68DeRSUm0k5I%2FFck
> 2Ok%3D&reserved=0 i2c/for-next
> patch link:
> https://lore.k/
> ernel.org%2Fr%2F20230801170318.82682-3-
> biju.das.jz%2540bp.renesas.com&data=05%7C01%7Cbiju.das.jz%40bp.renesas.c
> om%7C0a6df48bde964a9d1df208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a
> %7C0%7C0%7C638265463936796169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=2
> 8xs5DcJNWd52QHMFU1Mp%2F%2FWz4rSWjbzA5EXBozJpbw%3D&reserved=0
> patch subject: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data()
> callback
> config: hexagon-randconfig-r024-20230731
> (https://downl/
> oad.01.org%2F0day-ci%2Farchive%2F20230802%2F202308021149.cnDNnUAh-
> lkp%40intel.com%2Fconfig&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C0
> a6df48bde964a9d1df208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7
> C0%7C638265463936796169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=LH08RuO
> 8H23e6niXnY39LvZP5opxVNOlcrCWcbUSlLA%3D&reserved=0)
> compiler: clang version 14.0.6
> (https://githu/
> b.com%2Fllvm%2Fllvm-
> project.git&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C0a6df48bde964a
> 9d1df208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C63826546
> 3936796169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL
> CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=mcqSYepniOPtvFzx7hv%
> 2FzYAkNGk%2BhzFjP3K4xY6ojPM%3D&reserved=0
> f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce:
> (https://downl/
> oad.01.org%2F0day-ci%2Farchive%2F20230802%2F202308021149.cnDNnUAh-
> lkp%40intel.com%2Freproduce&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%
> 7C0a6df48bde964a9d1df208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a%7C
> 0%7C0%7C638265463936796169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jDK%
> 2Be3Ghf14uVCvz%2BzPtNQrCxQ4PpxzqRJrnE3BTzLU%3D&reserved=0)
>
> 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%2Foe-kbuild-all%2F202308021149.cnDNnUAh-lkp%40intel.com%2F
> | &data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C0a6df48bde964a9d1df208d
> | b930ec91c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638265463936796
> | 169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTi
> | I6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Dmp8go7rkus5Z5pTxiss9OL
> | j49ja%2F3DPGWFcwzn397M%3D&reserved=0
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from drivers/i2c/i2c-core-slave.c:12:
> 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]
> 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]
> 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'
> #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
> ^
> In file included from drivers/i2c/i2c-core-slave.c:12:
> 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]
> 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'
> #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
> ^
> In file included from drivers/i2c/i2c-core-slave.c:12:
> 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]
> __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]
> __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]
> __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE +
> addr);
> ~~~~~~~~~~ ^
> In file included from drivers/i2c/i2c-core-slave.c:12:
> >> include/linux/i2c.h:1038:2: error: redefinition of
> 'i2c_of_match_device'
> *i2c_of_match_device(const struct of_device_id *matches,
> ^
> include/linux/i2c.h:1031:2: note: previous definition is here
> *i2c_of_match_device(const struct of_device_id *matches,
> ^
> 6 warnings and 1 error generated.
> --
> 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]
> 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]
> 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'
> #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]
> 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'
> #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]
> __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]
> __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]
> __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE +
> addr);
> ~~~~~~~~~~ ^
> In file included from drivers/i2c/i2c-core-base.c:23:
> >> include/linux/i2c.h:1038:2: error: redefinition of
> 'i2c_of_match_device'
> *i2c_of_match_device(const struct of_device_id *matches,
> ^
> include/linux/i2c.h:1031:2: note: previous definition is here
> *i2c_of_match_device(const struct of_device_id *matches,
> ^
> >> drivers/i2c/i2c-core-base.c:146:11: error: implicit declaration of
> >> function 'i2c_of_match_device_sysfs' is invalid in C99
> >> [-Werror,-Wimplicit-function-declaration]
> match = i2c_of_match_device_sysfs(dev->driver-
> >of_match_table,
> ^
> drivers/i2c/i2c-core-base.c:146:11: note: did you mean
> 'i2c_of_match_device'?
> include/linux/i2c.h:1038:2: note: 'i2c_of_match_device' declared here
> *i2c_of_match_device(const struct of_device_id *matches,
> ^
> >> drivers/i2c/i2c-core-base.c:146:9: warning: incompatible integer to
> >> pointer conversion assigning to 'const struct of_device_id *' from
> >> 'int' [-Wint-conversion]
> match = i2c_of_match_device_sysfs(dev->driver-
> >of_match_table,
> ^
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 7 warnings and 2 errors generated.
>
>
> vim +/i2c_of_match_device +1038 include/linux/i2c.h
>
> 1036
> 1037 static inline const struct of_device_id
> > 1038 *i2c_of_match_device(const struct of_device_id *matches,
> 1039 struct i2c_client *client)
> 1040 {
> 1041 return NULL;
> 1042 }
> 1043
>
> --
> 0-DAY CI Kernel Test Service
> https://github/
> .com%2Fintel%2Flkp-
> tests%2Fwiki&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C0a6df48bde964
> a9d1df208db930ec91c%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C6382654
> 63936796169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi
> LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=RfyYpwqM0HOM%2BLuRc
> vB4%2F1qYFLQwDKwj9bCbA6Y965c%3D&reserved=0
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-02 9:34 ` Biju Das
@ 2023-08-02 14:23 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-08-02 14:23 UTC (permalink / raw)
To: Biju Das
Cc: Dmitry Torokhov, Wolfram Sang, linux-i2c@vger.kernel.org,
Geert Uytterhoeven, linux-renesas-soc@vger.kernel.org
On Wed, Aug 02, 2023 at 09:34:18AM +0000, Biju Das wrote:
...
> > Can we make i2c_of_match_device_sysfs() take a const pointer to a
> > client? Casting away constness is something that we should avoid.
>
> I agree, we are not supposed to modify client pointer inside
> i2c_of_match_device_sysfs(), so const makes sense here.
> Wolfram, I guess you are ok with it.
Don't forget to do that in a separate patch as well!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-02 7:59 ` Biju Das
@ 2023-08-02 14:41 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-08-02 14:41 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c@vger.kernel.org, Geert Uytterhoeven,
Dmitry Torokhov, linux-renesas-soc@vger.kernel.org
On Wed, Aug 02, 2023 at 07:59:21AM +0000, Biju Das wrote:
> > On Tue, Aug 01, 2023 at 06:03:18PM +0100, Biju Das wrote:
...
> > > - struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
> >
> > Does it make sense to remove and add an additional parameter? In one
> > case it's a copy, in another it probably the same, just hidden in the
> > code.
>
> Ok, you mean add the below check in i2c_device_get_match_data() and
> drop *driver parameter from i2c_get_match_data_helper().
Right.
> if (!client || !dev->driver)
> return NULL;
Not sure if you need this here in this static helper.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback
2023-08-02 6:34 ` Biju Das
@ 2023-08-02 14:44 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-08-02 14:44 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-i2c@vger.kernel.org, Geert Uytterhoeven,
Dmitry Torokhov, linux-renesas-soc@vger.kernel.org
On Wed, Aug 02, 2023 at 06:34:56AM +0000, Biju Das wrote:
> > On Tue, Aug 01, 2023 at 06:03:18PM +0100, Biju Das wrote:
...
> > > * Changed i2c_of_match_device_sysfs() as non-static function as it is
> > > needed for i2c_device_get_match_data().
> >
> > Btw, this can be split to a separate change.
>
> OK, first patch is callback with I2C table match and
> Second patch is for handling i2c_of_match_device().
One patch making API non-static, second, third, ... patches -- other things.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-08-02 15:01 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-01 17:03 [PATCH v3 0/2] Extend device_get_match_data() to struct bus_type Biju Das
2023-08-01 17:03 ` [PATCH v3 2/2] i2c: Add i2c_device_get_match_data() callback Biju Das
2023-08-01 19:28 ` Andy Shevchenko
2023-08-01 20:41 ` Dmitry Torokhov
2023-08-02 9:34 ` Biju Das
2023-08-02 14:23 ` Andy Shevchenko
2023-08-02 7:59 ` Biju Das
2023-08-02 14:41 ` Andy Shevchenko
2023-08-01 19:29 ` Andy Shevchenko
2023-08-02 6:34 ` Biju Das
2023-08-02 14:44 ` Andy Shevchenko
2023-08-02 4:12 ` kernel test robot
2023-08-02 11:04 ` Biju Das
2023-08-02 4:22 ` kernel test robot
2023-08-02 4:22 ` kernel test robot
2023-08-02 5:25 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox