Add support for I3C device in the tmp108 driver to handle the P3T1085
sensor. Register the I3C device driver to enable I3C functionality for the
sensor.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change from v2 to v3
- change kconfig to select REGMAP_I3C if enable i3c
- remove i3c/master.h
- remove , after {}
- use #ifdef CONFIG_I3C about i3c register code
I2C I3C
Y Y support both
Y N i3c part code will not be compiled
N Y whole TPM108 will not be compiled
N N whole TPM108 will not be compiled
---
drivers/hwmon/Kconfig | 1 +
drivers/hwmon/tmp108.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index d43ca7aa4a548..9579db7849e1f 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2298,6 +2298,7 @@ config SENSORS_TMP108
tristate "Texas Instruments TMP108"
depends on I2C
select REGMAP_I2C
+ select REGMAP_I3C if I3C
help
If you say yes here you get support for Texas Instruments TMP108
sensor chips and NXP P3T1085.
diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
index bfbea6349a95f..deb1505321335 100644
--- a/drivers/hwmon/tmp108.c
+++ b/drivers/hwmon/tmp108.c
@@ -13,6 +13,7 @@
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/i2c.h>
+#include <linux/i3c/device.h>
#include <linux/init.h>
#include <linux/jiffies.h>
#include <linux/regmap.h>
@@ -442,6 +443,36 @@ static struct i2c_driver tmp108_driver = {
module_i2c_driver(tmp108_driver);
+#ifdef CONFIG_I3C
+static const struct i3c_device_id p3t1085_i3c_ids[] = {
+ I3C_DEVICE(0x011b, 0x1529, NULL),
+ {}
+};
+MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
+
+static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
+{
+ struct device *dev = i3cdev_to_dev(i3cdev);
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dev, PTR_ERR(regmap),
+ "Failed to register i3c regmap\n");
+
+ return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
+}
+
+static struct i3c_driver p3t1085_driver = {
+ .driver = {
+ .name = "p3t1085_i3c",
+ },
+ .probe = p3t1085_i3c_probe,
+ .id_table = p3t1085_i3c_ids,
+};
+module_i3c_driver(p3t1085_driver);
+#endif
+
MODULE_AUTHOR("John Muir <john@jmuir.com>");
MODULE_DESCRIPTION("Texas Instruments TMP108 temperature sensor driver");
MODULE_LICENSE("GPL");
--
2.34.1Hi Frank,
kernel test robot noticed the following build errors:
[auto build test ERROR on 74741a050b79d31d8d2eeee12c77736596d0a6b2]
url: https://github.com/intel-lab-lkp/linux/commits/Frank-Li/dt-bindings-hwmon-ti-tmp108-Add-nxp-p3t1085-compatible-string/20241112-013721
base: 74741a050b79d31d8d2eeee12c77736596d0a6b2
patch link: https://lore.kernel.org/r/20241111-p3t1085-v3-4-bff511550aad%40nxp.com
patch subject: [PATCH v3 4/5] hwmon: tmp108: Add support for I3C device
config: arc-randconfig-001-20241114 (https://download.01.org/0day-ci/archive/20241114/202411141530.qTxjCzf7-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241114/202411141530.qTxjCzf7-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/202411141530.qTxjCzf7-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from drivers/hwmon/tmp108.c:8:
>> include/linux/module.h:131:49: error: redefinition of '__inittest'
131 | static inline initcall_t __maybe_unused __inittest(void) \
| ^~~~~~~~~~
include/linux/device/driver.h:262:1: note: in expansion of macro 'module_init'
262 | module_init(__driver##_init); \
| ^~~~~~~~~~~
include/linux/i3c/device.h:233:9: note: in expansion of macro 'module_driver'
233 | module_driver(__drv, i3c_driver_register, i3c_driver_unregister)
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:473:1: note: in expansion of macro 'module_i3c_driver'
473 | module_i3c_driver(p3t1085_driver);
| ^~~~~~~~~~~~~~~~~
include/linux/module.h:131:49: note: previous definition of '__inittest' with type 'int (*(void))(void)'
131 | static inline initcall_t __maybe_unused __inittest(void) \
| ^~~~~~~~~~
include/linux/device/driver.h:262:1: note: in expansion of macro 'module_init'
262 | module_init(__driver##_init); \
| ^~~~~~~~~~~
include/linux/i2c.h:965:9: note: in expansion of macro 'module_driver'
965 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:444:1: note: in expansion of macro 'module_i2c_driver'
444 | module_i2c_driver(tmp108_driver);
| ^~~~~~~~~~~~~~~~~
>> include/linux/module.h:133:13: error: redefinition of 'init_module'
133 | int init_module(void) __copy(initfn) \
| ^~~~~~~~~~~
include/linux/device/driver.h:262:1: note: in expansion of macro 'module_init'
262 | module_init(__driver##_init); \
| ^~~~~~~~~~~
include/linux/i3c/device.h:233:9: note: in expansion of macro 'module_driver'
233 | module_driver(__drv, i3c_driver_register, i3c_driver_unregister)
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:473:1: note: in expansion of macro 'module_i3c_driver'
473 | module_i3c_driver(p3t1085_driver);
| ^~~~~~~~~~~~~~~~~
include/linux/module.h:133:13: note: previous definition of 'init_module' with type 'int(void)'
133 | int init_module(void) __copy(initfn) \
| ^~~~~~~~~~~
include/linux/device/driver.h:262:1: note: in expansion of macro 'module_init'
262 | module_init(__driver##_init); \
| ^~~~~~~~~~~
include/linux/i2c.h:965:9: note: in expansion of macro 'module_driver'
965 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:444:1: note: in expansion of macro 'module_i2c_driver'
444 | module_i2c_driver(tmp108_driver);
| ^~~~~~~~~~~~~~~~~
>> include/linux/module.h:139:49: error: redefinition of '__exittest'
139 | static inline exitcall_t __maybe_unused __exittest(void) \
| ^~~~~~~~~~
include/linux/device/driver.h:267:1: note: in expansion of macro 'module_exit'
267 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
include/linux/i3c/device.h:233:9: note: in expansion of macro 'module_driver'
233 | module_driver(__drv, i3c_driver_register, i3c_driver_unregister)
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:473:1: note: in expansion of macro 'module_i3c_driver'
473 | module_i3c_driver(p3t1085_driver);
| ^~~~~~~~~~~~~~~~~
include/linux/module.h:139:49: note: previous definition of '__exittest' with type 'void (*(void))(void)'
139 | static inline exitcall_t __maybe_unused __exittest(void) \
| ^~~~~~~~~~
include/linux/device/driver.h:267:1: note: in expansion of macro 'module_exit'
267 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
include/linux/i2c.h:965:9: note: in expansion of macro 'module_driver'
965 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:444:1: note: in expansion of macro 'module_i2c_driver'
444 | module_i2c_driver(tmp108_driver);
| ^~~~~~~~~~~~~~~~~
>> include/linux/module.h:141:14: error: redefinition of 'cleanup_module'
141 | void cleanup_module(void) __copy(exitfn) \
| ^~~~~~~~~~~~~~
include/linux/device/driver.h:267:1: note: in expansion of macro 'module_exit'
267 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
include/linux/i3c/device.h:233:9: note: in expansion of macro 'module_driver'
233 | module_driver(__drv, i3c_driver_register, i3c_driver_unregister)
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:473:1: note: in expansion of macro 'module_i3c_driver'
473 | module_i3c_driver(p3t1085_driver);
| ^~~~~~~~~~~~~~~~~
include/linux/module.h:141:14: note: previous definition of 'cleanup_module' with type 'void(void)'
141 | void cleanup_module(void) __copy(exitfn) \
| ^~~~~~~~~~~~~~
include/linux/device/driver.h:267:1: note: in expansion of macro 'module_exit'
267 | module_exit(__driver##_exit);
| ^~~~~~~~~~~
include/linux/i2c.h:965:9: note: in expansion of macro 'module_driver'
965 | module_driver(__i2c_driver, i2c_add_driver, \
| ^~~~~~~~~~~~~
drivers/hwmon/tmp108.c:444:1: note: in expansion of macro 'module_i2c_driver'
444 | module_i2c_driver(tmp108_driver);
| ^~~~~~~~~~~~~~~~~
vim +/__inittest +131 include/linux/module.h
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 128
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 129 /* Each module must use one module_init(). */
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 130 #define module_init(initfn) \
1f318a8bafcfba9 Arnd Bergmann 2017-02-01 @131 static inline initcall_t __maybe_unused __inittest(void) \
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 132 { return initfn; } \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 @133 int init_module(void) __copy(initfn) \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 134 __attribute__((alias(#initfn))); \
92efda8eb15295a Sami Tolvanen 2022-09-08 135 ___ADDRESSABLE(init_module, __initdata);
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 136
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 137 /* This is only required if you want to be unloadable. */
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 138 #define module_exit(exitfn) \
1f318a8bafcfba9 Arnd Bergmann 2017-02-01 @139 static inline exitcall_t __maybe_unused __exittest(void) \
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 140 { return exitfn; } \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 @141 void cleanup_module(void) __copy(exitfn) \
cf68fffb66d60d9 Sami Tolvanen 2021-04-08 142 __attribute__((alias(#exitfn))); \
92efda8eb15295a Sami Tolvanen 2022-09-08 143 ___ADDRESSABLE(cleanup_module, __exitdata);
0fd972a7d91d6e1 Paul Gortmaker 2015-05-01 144
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 11/11/24 09:32, Frank Li wrote:
> Add support for I3C device in the tmp108 driver to handle the P3T1085
> sensor. Register the I3C device driver to enable I3C functionality for the
> sensor.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v2 to v3
> - change kconfig to select REGMAP_I3C if enable i3c
> - remove i3c/master.h
> - remove , after {}
> - use #ifdef CONFIG_I3C about i3c register code
>
> I2C I3C
> Y Y support both
> Y N i3c part code will not be compiled
> N Y whole TPM108 will not be compiled
> N N whole TPM108 will not be compiled
> ---
> drivers/hwmon/Kconfig | 1 +
> drivers/hwmon/tmp108.c | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index d43ca7aa4a548..9579db7849e1f 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2298,6 +2298,7 @@ config SENSORS_TMP108
> tristate "Texas Instruments TMP108"
> depends on I2C
> select REGMAP_I2C
> + select REGMAP_I3C if I3C
> help
> If you say yes here you get support for Texas Instruments TMP108
> sensor chips and NXP P3T1085.
> diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
> index bfbea6349a95f..deb1505321335 100644
> --- a/drivers/hwmon/tmp108.c
> +++ b/drivers/hwmon/tmp108.c
> @@ -13,6 +13,7 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/i2c.h>
> +#include <linux/i3c/device.h>
> #include <linux/init.h>
> #include <linux/jiffies.h>
> #include <linux/regmap.h>
> @@ -442,6 +443,36 @@ static struct i2c_driver tmp108_driver = {
>
> module_i2c_driver(tmp108_driver);
>
> +#ifdef CONFIG_I3C
> +static const struct i3c_device_id p3t1085_i3c_ids[] = {
> + I3C_DEVICE(0x011b, 0x1529, NULL),
> + {}
> +};
> +MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
> +
> +static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
> +{
> + struct device *dev = i3cdev_to_dev(i3cdev);
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
It is a bit kludgy, but maybe
#ifdef REGMAP_I3C
regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
#else
regmap = ERR_PTR(-ENODEV);
#endif
and then using module_i3c_i2c_driver() would work.
Guenter
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "Failed to register i3c regmap\n");
> +
> + return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
> +}
> +
> +static struct i3c_driver p3t1085_driver = {
> + .driver = {
> + .name = "p3t1085_i3c",
> + },
> + .probe = p3t1085_i3c_probe,
> + .id_table = p3t1085_i3c_ids,
> +};
> +module_i3c_driver(p3t1085_driver);
> +#endif
> +
> MODULE_AUTHOR("John Muir <john@jmuir.com>");
> MODULE_DESCRIPTION("Texas Instruments TMP108 temperature sensor driver");
> MODULE_LICENSE("GPL");
>
On 11/11/24 09:32, Frank Li wrote:
> Add support for I3C device in the tmp108 driver to handle the P3T1085
> sensor. Register the I3C device driver to enable I3C functionality for the
> sensor.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change from v2 to v3
> - change kconfig to select REGMAP_I3C if enable i3c
> - remove i3c/master.h
> - remove , after {}
> - use #ifdef CONFIG_I3C about i3c register code
>
> I2C I3C
> Y Y support both
> Y N i3c part code will not be compiled
> N Y whole TPM108 will not be compiled
> N N whole TPM108 will not be compiled
> ---
> drivers/hwmon/Kconfig | 1 +
> drivers/hwmon/tmp108.c | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index d43ca7aa4a548..9579db7849e1f 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2298,6 +2298,7 @@ config SENSORS_TMP108
> tristate "Texas Instruments TMP108"
> depends on I2C
> select REGMAP_I2C
> + select REGMAP_I3C if I3C
> help
> If you say yes here you get support for Texas Instruments TMP108
> sensor chips and NXP P3T1085.
> diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
> index bfbea6349a95f..deb1505321335 100644
> --- a/drivers/hwmon/tmp108.c
> +++ b/drivers/hwmon/tmp108.c
> @@ -13,6 +13,7 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/i2c.h>
> +#include <linux/i3c/device.h>
> #include <linux/init.h>
> #include <linux/jiffies.h>
> #include <linux/regmap.h>
> @@ -442,6 +443,36 @@ static struct i2c_driver tmp108_driver = {
>
> module_i2c_driver(tmp108_driver);
>
> +#ifdef CONFIG_I3C
> +static const struct i3c_device_id p3t1085_i3c_ids[] = {
> + I3C_DEVICE(0x011b, 0x1529, NULL),
> + {}
> +};
> +MODULE_DEVICE_TABLE(i3c, p3t1085_i3c_ids);
> +
> +static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
> +{
> + struct device *dev = i3cdev_to_dev(i3cdev);
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "Failed to register i3c regmap\n");
> +
> + return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
> +}
> +
> +static struct i3c_driver p3t1085_driver = {
> + .driver = {
> + .name = "p3t1085_i3c",
> + },
> + .probe = p3t1085_i3c_probe,
> + .id_table = p3t1085_i3c_ids,
> +};
> +module_i3c_driver(p3t1085_driver);
> +#endif
While looking at i3c code, I found module_i3c_i2c_driver(). Can we use
that function to register both i2c and i3c in one call ?
Thanks,
Guenter
On 11/11/24 10:04, Guenter Roeck wrote:
[ ... ]
>> +static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
>> +{
>> + struct device *dev = i3cdev_to_dev(i3cdev);
>> + struct regmap *regmap;
>> +
>> + regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
>> + if (IS_ERR(regmap))
>> + return dev_err_probe(dev, PTR_ERR(regmap),
>> + "Failed to register i3c regmap\n");
>> +
>> + return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
>> +}
>> +
>> +static struct i3c_driver p3t1085_driver = {
>> + .driver = {
>> + .name = "p3t1085_i3c",
>> + },
>> + .probe = p3t1085_i3c_probe,
>> + .id_table = p3t1085_i3c_ids,
>> +};
>> +module_i3c_driver(p3t1085_driver);
>> +#endif
>
> While looking at i3c code, I found module_i3c_i2c_driver(). Can we use
> that function to register both i2c and i3c in one call ?
>
Answering my own question: No, because devm_regmap_init_i3c()
does not provide a dummy function if i3C is not enabled.
Guenter
On 11/11/24 10:10, Guenter Roeck wrote:
> On 11/11/24 10:04, Guenter Roeck wrote:
> [ ... ]
>>> +static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
>>> +{
>>> + struct device *dev = i3cdev_to_dev(i3cdev);
>>> + struct regmap *regmap;
>>> +
>>> + regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
>>> + if (IS_ERR(regmap))
>>> + return dev_err_probe(dev, PTR_ERR(regmap),
>>> + "Failed to register i3c regmap\n");
>>> +
>>> + return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
>>> +}
>>> +
>>> +static struct i3c_driver p3t1085_driver = {
>>> + .driver = {
>>> + .name = "p3t1085_i3c",
>>> + },
>>> + .probe = p3t1085_i3c_probe,
>>> + .id_table = p3t1085_i3c_ids,
>>> +};
>>> +module_i3c_driver(p3t1085_driver);
>>> +#endif
>>
>> While looking at i3c code, I found module_i3c_i2c_driver(). Can we use
>> that function to register both i2c and i3c in one call ?
>>
> Answering my own question: No, because devm_regmap_init_i3c()
> does not provide a dummy function if i3C is not enabled.
>
I do have another concern, though: What happens if the i2c part of the driver
registers and the i3c part fails to register ? module_i3c_i2c_driver() handles
that situation by unregistering the i2c driver, but I don't really know
what happens if a single module registers two drivers and one of them fails.
Thanks,
Guenter
On Mon, Nov 11, 2024 at 10:37:04AM -0800, Guenter Roeck wrote:
> On 11/11/24 10:10, Guenter Roeck wrote:
> > On 11/11/24 10:04, Guenter Roeck wrote:
> > [ ... ]
> > > > +static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
> > > > +{
> > > > + struct device *dev = i3cdev_to_dev(i3cdev);
> > > > + struct regmap *regmap;
> > > > +
> > > > + regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
> > > > + if (IS_ERR(regmap))
> > > > + return dev_err_probe(dev, PTR_ERR(regmap),
> > > > + "Failed to register i3c regmap\n");
> > > > +
> > > > + return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
> > > > +}
> > > > +
> > > > +static struct i3c_driver p3t1085_driver = {
> > > > + .driver = {
> > > > + .name = "p3t1085_i3c",
> > > > + },
> > > > + .probe = p3t1085_i3c_probe,
> > > > + .id_table = p3t1085_i3c_ids,
> > > > +};
> > > > +module_i3c_driver(p3t1085_driver);
> > > > +#endif
> > >
> > > While looking at i3c code, I found module_i3c_i2c_driver(). Can we use
> > > that function to register both i2c and i3c in one call ?
> > >
> > Answering my own question: No, because devm_regmap_init_i3c()
> > does not provide a dummy function if i3C is not enabled.
> >
>
> I do have another concern, though: What happens if the i2c part of the driver
> registers and the i3c part fails to register ? module_i3c_i2c_driver() handles
> that situation by unregistering the i2c driver, but I don't really know
> what happens if a single module registers two drivers and one of them fails.
After use module_i3c_i2c_driver(), and remove #ifdef I3C, and disable I3C
in config, build passed.
It possible cause by
static inline int i3c_i2c_driver_register(struct i3c_driver *i3cdrv,
struct i2c_driver *i2cdrv)
{
int ret;
ret = i2c_add_driver(i2cdrv);
if (ret || !IS_ENABLED(CONFIG_I3C))
return ret;
^^^ !IS_ENABLED(CONFIG_I3C) is true, so linker skip below part. So no
ref to i3cdrv, so linker remove all related codes.
I use aarch64-linux-gnu-gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0.
Did you met error if use module_i3c_i2c_driver()?
ret = i3c_driver_register(i3cdrv);
if (ret)
i2c_del_driver(i2cdrv);
return ret;
}
>
> Thanks,
> Guenter
>
On 11/11/24 15:20, Frank Li wrote:
> On Mon, Nov 11, 2024 at 10:37:04AM -0800, Guenter Roeck wrote:
>> On 11/11/24 10:10, Guenter Roeck wrote:
>>> On 11/11/24 10:04, Guenter Roeck wrote:
>>> [ ... ]
>>>>> +static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
>>>>> +{
>>>>> + struct device *dev = i3cdev_to_dev(i3cdev);
>>>>> + struct regmap *regmap;
>>>>> +
>>>>> + regmap = devm_regmap_init_i3c(i3cdev, &tmp108_regmap_config);
>>>>> + if (IS_ERR(regmap))
>>>>> + return dev_err_probe(dev, PTR_ERR(regmap),
>>>>> + "Failed to register i3c regmap\n");
>>>>> +
>>>>> + return tmp108_common_probe(dev, regmap, "p3t1085_i3c");
>>>>> +}
>>>>> +
>>>>> +static struct i3c_driver p3t1085_driver = {
>>>>> + .driver = {
>>>>> + .name = "p3t1085_i3c",
>>>>> + },
>>>>> + .probe = p3t1085_i3c_probe,
>>>>> + .id_table = p3t1085_i3c_ids,
>>>>> +};
>>>>> +module_i3c_driver(p3t1085_driver);
>>>>> +#endif
>>>>
>>>> While looking at i3c code, I found module_i3c_i2c_driver(). Can we use
>>>> that function to register both i2c and i3c in one call ?
>>>>
>>> Answering my own question: No, because devm_regmap_init_i3c()
>>> does not provide a dummy function if i3C is not enabled.
>>>
>>
>> I do have another concern, though: What happens if the i2c part of the driver
>> registers and the i3c part fails to register ? module_i3c_i2c_driver() handles
>> that situation by unregistering the i2c driver, but I don't really know
>> what happens if a single module registers two drivers and one of them fails.
>
> After use module_i3c_i2c_driver(), and remove #ifdef I3C, and disable I3C
> in config, build passed.
>
> It possible cause by
>
> static inline int i3c_i2c_driver_register(struct i3c_driver *i3cdrv,
> struct i2c_driver *i2cdrv)
> {
> int ret;
>
> ret = i2c_add_driver(i2cdrv);
> if (ret || !IS_ENABLED(CONFIG_I3C))
> return ret;
>
> ^^^ !IS_ENABLED(CONFIG_I3C) is true, so linker skip below part. So no
> ref to i3cdrv, so linker remove all related codes.
>
Yes, but I don't think we can rely on the compiler removing the call to
devm_regmap_init_i3c().
Thanks,
Guenter
© 2016 - 2024 Red Hat, Inc.