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 >