| Message ID | 1616411413-7177-2-git-send-email-yangyicong@hisilicon.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for HiSilicon I2C controller | expand |
On Mon, Mar 22, 2021 at 07:10:11PM +0800, Yicong Yang wrote: > Some I2C controller drivers will only unregister the I2C > adapter in their .remove() callback, which can be done > by simply using a managed variant to add the I2C adapter. > > So add the managed functions for adding the I2C adapter. ... > +static void devm_i2c_del_adapter(struct device *dev, void *ptr); Can we instead of forward declaration simply move below to be after devm_i2c_del_adapter()? > +/** > + * devm_i2c_add_adapter - device-managed variant of i2c_add_adapter() > + * @dev: managing device for adding this I2C adapter > + * @adapter: the adapter to add > + * Context: can sleep > + * > + * Add adapter with dynamic bus number, same with i2c_add_adapter() > + * but the adapter will be auto deleted on driver detach. > + */ > +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter) > +{ > + struct i2c_adapter **ptr; > + int ret; > + > + ptr = devres_alloc(devm_i2c_del_adapter, sizeof(*ptr), GFP_KERNEL); > + if (!ptr) > + return -ENOMEM; > + > + ret = i2c_add_adapter(adapter); > + if (!ret) { > + *ptr = adapter; > + devres_add(dev, ptr); > + } else { > + devres_free(ptr); > + } > + > + return ret; > +}
On 2021/3/23 0:35, Andy Shevchenko wrote: > On Mon, Mar 22, 2021 at 07:10:11PM +0800, Yicong Yang wrote: >> Some I2C controller drivers will only unregister the I2C >> adapter in their .remove() callback, which can be done >> by simply using a managed variant to add the I2C adapter. >> >> So add the managed functions for adding the I2C adapter. > > ... > >> +static void devm_i2c_del_adapter(struct device *dev, void *ptr); > > Can we instead of forward declaration simply move below to be after > devm_i2c_del_adapter()? i just want to put the devm variant near i2c_add_adapter, i'll address this if it's unnecessary. Thanks, Yicong > >> +/** >> + * devm_i2c_add_adapter - device-managed variant of i2c_add_adapter() >> + * @dev: managing device for adding this I2C adapter >> + * @adapter: the adapter to add >> + * Context: can sleep >> + * >> + * Add adapter with dynamic bus number, same with i2c_add_adapter() >> + * but the adapter will be auto deleted on driver detach. >> + */ >> +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter) >> +{ >> + struct i2c_adapter **ptr; >> + int ret; >> + >> + ptr = devres_alloc(devm_i2c_del_adapter, sizeof(*ptr), GFP_KERNEL); >> + if (!ptr) >> + return -ENOMEM; >> + >> + ret = i2c_add_adapter(adapter); >> + if (!ret) { >> + *ptr = adapter; >> + devres_add(dev, ptr); >> + } else { >> + devres_free(ptr); >> + } >> + >> + return ret; >> +} >
On 2021/3/23 0:45, Dmitry Osipenko wrote: > 22.03.2021 14:10, Yicong Yang пишет: >> Some I2C controller drivers will only unregister the I2C >> adapter in their .remove() callback, which can be done >> by simply using a managed variant to add the I2C adapter. >> >> So add the managed functions for adding the I2C adapter. >> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> --- >> drivers/i2c/i2c-core-base.c | 39 +++++++++++++++++++++++++++++++++++++++ >> include/linux/i2c.h | 1 + >> 2 files changed, 40 insertions(+) >> >> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c >> index 63ebf72..61486dc 100644 >> --- a/drivers/i2c/i2c-core-base.c >> +++ b/drivers/i2c/i2c-core-base.c >> @@ -1550,6 +1550,38 @@ int i2c_add_adapter(struct i2c_adapter *adapter) >> } >> EXPORT_SYMBOL(i2c_add_adapter); >> >> +static void devm_i2c_del_adapter(struct device *dev, void *ptr); >> + >> +/** >> + * devm_i2c_add_adapter - device-managed variant of i2c_add_adapter() >> + * @dev: managing device for adding this I2C adapter >> + * @adapter: the adapter to add >> + * Context: can sleep >> + * >> + * Add adapter with dynamic bus number, same with i2c_add_adapter() >> + * but the adapter will be auto deleted on driver detach. >> + */ >> +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter) >> +{ >> + struct i2c_adapter **ptr; >> + int ret; >> + >> + ptr = devres_alloc(devm_i2c_del_adapter, sizeof(*ptr), GFP_KERNEL); >> + if (!ptr) >> + return -ENOMEM; >> + >> + ret = i2c_add_adapter(adapter); >> + if (!ret) { >> + *ptr = adapter; >> + devres_add(dev, ptr); >> + } else { >> + devres_free(ptr); >> + } > > This could be simplified using devm_add_action_or_reset(). thanks for the hint. i'll see whether i can change to that. > >> + return ret; >> +} >> +EXPORT_SYMBOL(devm_i2c_add_adapter); > > EXPORT_SYMBOL_GPL considering i2c_add_adapter exported with no GPL, i think the variant should keep the same. Thanks, Yicong > > . >
On Wed, Mar 24, 2021 at 04:26:54PM +0800, Yicong Yang wrote: > On 2021/3/23 0:35, Andy Shevchenko wrote: > > On Mon, Mar 22, 2021 at 07:10:11PM +0800, Yicong Yang wrote: > >> Some I2C controller drivers will only unregister the I2C > >> adapter in their .remove() callback, which can be done > >> by simply using a managed variant to add the I2C adapter. > >> > >> So add the managed functions for adding the I2C adapter. > > > > ... > > > >> +static void devm_i2c_del_adapter(struct device *dev, void *ptr); > > > > Can we instead of forward declaration simply move below to be after > > devm_i2c_del_adapter()? > > i just want to put the devm variant near i2c_add_adapter, i'll address > this if it's unnecessary. Please, address. -- With Best Regards, Andy Shevchenko
On Wed, Mar 24, 2021 at 04:29:26PM +0800, Yicong Yang wrote: > On 2021/3/23 0:45, Dmitry Osipenko wrote: > > 22.03.2021 14:10, Yicong Yang пишет: ... > >> +EXPORT_SYMBOL(devm_i2c_add_adapter); > > > > EXPORT_SYMBOL_GPL > > considering i2c_add_adapter exported with no GPL, i think the variant should > keep the same. Nope. You should follow other devm_*() APIs. Hint: they are using struct device which is GPL. Please, fix. -- With Best Regards, Andy Shevchenko
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 63ebf72..61486dc 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1550,6 +1550,38 @@ int i2c_add_adapter(struct i2c_adapter *adapter) } EXPORT_SYMBOL(i2c_add_adapter); +static void devm_i2c_del_adapter(struct device *dev, void *ptr); + +/** + * devm_i2c_add_adapter - device-managed variant of i2c_add_adapter() + * @dev: managing device for adding this I2C adapter + * @adapter: the adapter to add + * Context: can sleep + * + * Add adapter with dynamic bus number, same with i2c_add_adapter() + * but the adapter will be auto deleted on driver detach. + */ +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter) +{ + struct i2c_adapter **ptr; + int ret; + + ptr = devres_alloc(devm_i2c_del_adapter, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return -ENOMEM; + + ret = i2c_add_adapter(adapter); + if (!ret) { + *ptr = adapter; + devres_add(dev, ptr); + } else { + devres_free(ptr); + } + + return ret; +} +EXPORT_SYMBOL(devm_i2c_add_adapter); + /** * i2c_add_numbered_adapter - declare i2c adapter, use static bus number * @adap: the adapter to register (with adap->nr initialized) @@ -1703,6 +1735,13 @@ void i2c_del_adapter(struct i2c_adapter *adap) } EXPORT_SYMBOL(i2c_del_adapter); +static void devm_i2c_del_adapter(struct device *dev, void *ptr) +{ + struct i2c_adapter *adapter = *((struct i2c_adapter **)ptr); + + i2c_del_adapter(adapter); +} + static void i2c_parse_timing(struct device *dev, char *prop_name, u32 *cur_val_p, u32 def_val, bool use_def) { diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 5662265..10bd0b0 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -844,6 +844,7 @@ static inline void i2c_mark_adapter_resumed(struct i2c_adapter *adap) */ #if IS_ENABLED(CONFIG_I2C) int i2c_add_adapter(struct i2c_adapter *adap); +int devm_i2c_add_adapter(struct device *dev, struct i2c_adapter *adapter); void i2c_del_adapter(struct i2c_adapter *adap); int i2c_add_numbered_adapter(struct i2c_adapter *adap);
Some I2C controller drivers will only unregister the I2C adapter in their .remove() callback, which can be done by simply using a managed variant to add the I2C adapter. So add the managed functions for adding the I2C adapter. Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/i2c/i2c-core-base.c | 39 +++++++++++++++++++++++++++++++++++++++ include/linux/i2c.h | 1 + 2 files changed, 40 insertions(+)