| Message ID | 1436865374-22027-1-git-send-email-jthumshirn@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2015-07-14 at 11:16 +0200, Johannes Thumshirn wrote: > Destroy IDRs on module exit, freeing the resources for > * bq2415x_charger.c > * ds2782_battery.c > * ltc2941-battery-gauge.c > > The drivers had to be converted to "ordinary" > module_init()/module_exit() > style drivers instead of using module_i2c_driver. That is a drawback. How about adding a pointer to the idr to "struct i2c_driver" and destroy it in generic code? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 14 July 2015 11:16:14 Johannes Thumshirn wrote: > Destroy IDRs on module exit, freeing the resources for > * bq2415x_charger.c > * ds2782_battery.c > * ltc2941-battery-gauge.c > > The drivers had to be converted to "ordinary" module_init()/module_exit() > style drivers instead of using module_i2c_driver. > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > drivers/power/bq2415x_charger.c | 20 +++++++++++++++++++- > drivers/power/ds2782_battery.c | 20 +++++++++++++++++++- > drivers/power/ltc2941-battery-gauge.c | 20 +++++++++++++++++++- > 3 files changed, 57 insertions(+), 3 deletions(-) > > diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c > index e98dcb6..38f4208 100644 > --- a/drivers/power/bq2415x_charger.c > +++ b/drivers/power/bq2415x_charger.c > @@ -1773,7 +1773,25 @@ static struct i2c_driver bq2415x_driver = { > .remove = bq2415x_remove, > .id_table = bq2415x_i2c_id_table, > }; > -module_i2c_driver(bq2415x_driver); > + > +static int __init bq2415x_init(void) > +{ > + int ret; > + > + ret = i2c_add_driver(&bq2415x_driver); > + if (ret) > + printk(KERN_ERR "Unable to register bq2415x-battery driver\n"); > + > + return ret; > +} > +module_init(bq2415x_init); > + > +static void __exit bq2415x_exit(void) > +{ > + i2c_del_driver(&bq2415x_driver); > + idr_destroy(&bq2415x_id); > +} > +module_exit(bq2415x_exit); > > MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); > MODULE_DESCRIPTION("bq2415x charger driver"); > diff --git a/drivers/power/ds2782_battery.c b/drivers/power/ds2782_battery.c > index ed4d756..410bc9d 100644 > --- a/drivers/power/ds2782_battery.c > +++ b/drivers/power/ds2782_battery.c > @@ -468,7 +468,25 @@ static struct i2c_driver ds278x_battery_driver = { > .remove = ds278x_battery_remove, > .id_table = ds278x_id, > }; > -module_i2c_driver(ds278x_battery_driver); > + > +static int __init ds2782_init(void) > +{ > + int ret; > + > + ret = i2c_add_driver(&ds278x_battery_driver); > + if (ret) > + printk(KERN_ERR "Unable to register ds2782-battery driver\n"); > + > + return ret; > +} > +module_init(ds2782_init); > + > +static void __exit ds2782_exit(void) > +{ > + i2c_del_driver(&ds278x_battery_driver); > + idr_destroy(&battery_id); > +} > +module_exit(ds2782_exit); > > MODULE_AUTHOR("Ryan Mallon"); > MODULE_DESCRIPTION("Maxim/Dallas DS2782 Stand-Alone Fuel Gauage IC driver"); > diff --git a/drivers/power/ltc2941-battery-gauge.c b/drivers/power/ltc2941-battery-gauge.c > index daeb086..6fbbcd2 100644 > --- a/drivers/power/ltc2941-battery-gauge.c > +++ b/drivers/power/ltc2941-battery-gauge.c > @@ -544,7 +544,25 @@ static struct i2c_driver ltc294x_driver = { > .remove = ltc294x_i2c_remove, > .id_table = ltc294x_i2c_id, > }; > -module_i2c_driver(ltc294x_driver); > + > +static int __init ltc294x_init(void) > +{ > + int ret; > + > + ret = i2c_add_driver(<c294x_driver); > + if (ret) > + printk(KERN_ERR "Unable to register bq2415x-battery driver\n"); > + > + return ret; > +} > +module_init(ltc294x_init); > + > +static void __exit ltc294x_exit(void) > +{ > + i2c_del_driver(<c294x_driver); > + idr_destroy(<c294x_id); > +} > +module_exit(ltc294x_exit); > > MODULE_AUTHOR("Auryn Verwegen, Topic Embedded Systems"); > MODULE_AUTHOR("Mike Looijmans, Topic Embedded Products"); I'm feeling that there is something wrong with idr API. There is no idr code in __init functions, so why it is needed for __exit? idr functions are used in devices, not in global module.
[+CC: Tejun Heo <tj@kernel.org>, for being very active in lib/idr.c] Hi, On Tue, Jul 14, 2015 at 11:16:14AM +0200, Johannes Thumshirn wrote: > Destroy IDRs on module exit, freeing the resources for > * bq2415x_charger.c > * ds2782_battery.c > * ltc2941-battery-gauge.c You missed bq27x00_battery.c. Maybe a coccinelle script should be written? > The drivers had to be converted to "ordinary" module_init()/module_exit() > style drivers instead of using module_i2c_driver. mh I would prefer another solution. How about adding something like this: static void idr_remove_and_destroy(struct idr *idp, int id) { idr_remove(idp, id); if (idr_is_empty(idp) idr_destroy(idp); } If that is called by the drivers instead of idr_remove(), there should be no need for adding idr_destroy to module_exit(). -- Sebastian
Sebastian Reichel <sre@kernel.org> writes: > [+CC: Tejun Heo <tj@kernel.org>, for being very active in lib/idr.c] > > Hi, > > On Tue, Jul 14, 2015 at 11:16:14AM +0200, Johannes Thumshirn wrote: >> Destroy IDRs on module exit, freeing the resources for >> * bq2415x_charger.c >> * ds2782_battery.c >> * ltc2941-battery-gauge.c > > You missed bq27x00_battery.c. Maybe a coccinelle script should be > written? I already did this one that last week (I think), it should've been in your INBOX (I hope). >> The drivers had to be converted to "ordinary" module_init()/module_exit() >> style drivers instead of using module_i2c_driver. > > mh I would prefer another solution. How about adding something > like this: > > static void idr_remove_and_destroy(struct idr *idp, int id) > { > idr_remove(idp, id); > if (idr_is_empty(idp) > idr_destroy(idp); > } > > If that is called by the drivers instead of idr_remove(), there > should be no need for adding idr_destroy to module_exit(). > This would be an option. [Copied a part of Palin's reply in here as well] Palin> I'm feeling that there is something wrong with idr API. There is no idr Palin> code in __init functions, so why it is needed for __exit? idr functions Palin> are used in devices, not in global module. but I start getting a bit puzzled about the whole IDR thing. Further grepping through the tree revealed even more drivers doing DEFINE_ID{A,R} but not all doing the destroy calls afterwards. Maybe Tejun can clarify on this. Johannes -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Jul 14, 2015 at 12:48:31PM +0200, Johannes Thumshirn wrote: > Sebastian Reichel <sre@kernel.org> writes: > > On Tue, Jul 14, 2015 at 11:16:14AM +0200, Johannes Thumshirn wrote: > >> Destroy IDRs on module exit, freeing the resources for > >> * bq2415x_charger.c > >> * ds2782_battery.c > >> * ltc2941-battery-gauge.c > > > > You missed bq27x00_battery.c. Maybe a coccinelle script should be > > written? > > I already did this one that last week (I think), it should've been in > your INBOX (I hope). Right. I marked that one as read assuming you would write a new version of the patch changing all drivers. Please change all drivers in one patch in the next version. -- Sebastian
diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c index e98dcb6..38f4208 100644 --- a/drivers/power/bq2415x_charger.c +++ b/drivers/power/bq2415x_charger.c @@ -1773,7 +1773,25 @@ static struct i2c_driver bq2415x_driver = { .remove = bq2415x_remove, .id_table = bq2415x_i2c_id_table, }; -module_i2c_driver(bq2415x_driver); + +static int __init bq2415x_init(void) +{ + int ret; + + ret = i2c_add_driver(&bq2415x_driver); + if (ret) + printk(KERN_ERR "Unable to register bq2415x-battery driver\n"); + + return ret; +} +module_init(bq2415x_init); + +static void __exit bq2415x_exit(void) +{ + i2c_del_driver(&bq2415x_driver); + idr_destroy(&bq2415x_id); +} +module_exit(bq2415x_exit); MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>"); MODULE_DESCRIPTION("bq2415x charger driver"); diff --git a/drivers/power/ds2782_battery.c b/drivers/power/ds2782_battery.c index ed4d756..410bc9d 100644 --- a/drivers/power/ds2782_battery.c +++ b/drivers/power/ds2782_battery.c @@ -468,7 +468,25 @@ static struct i2c_driver ds278x_battery_driver = { .remove = ds278x_battery_remove, .id_table = ds278x_id, }; -module_i2c_driver(ds278x_battery_driver); + +static int __init ds2782_init(void) +{ + int ret; + + ret = i2c_add_driver(&ds278x_battery_driver); + if (ret) + printk(KERN_ERR "Unable to register ds2782-battery driver\n"); + + return ret; +} +module_init(ds2782_init); + +static void __exit ds2782_exit(void) +{ + i2c_del_driver(&ds278x_battery_driver); + idr_destroy(&battery_id); +} +module_exit(ds2782_exit); MODULE_AUTHOR("Ryan Mallon"); MODULE_DESCRIPTION("Maxim/Dallas DS2782 Stand-Alone Fuel Gauage IC driver"); diff --git a/drivers/power/ltc2941-battery-gauge.c b/drivers/power/ltc2941-battery-gauge.c index daeb086..6fbbcd2 100644 --- a/drivers/power/ltc2941-battery-gauge.c +++ b/drivers/power/ltc2941-battery-gauge.c @@ -544,7 +544,25 @@ static struct i2c_driver ltc294x_driver = { .remove = ltc294x_i2c_remove, .id_table = ltc294x_i2c_id, }; -module_i2c_driver(ltc294x_driver); + +static int __init ltc294x_init(void) +{ + int ret; + + ret = i2c_add_driver(<c294x_driver); + if (ret) + printk(KERN_ERR "Unable to register bq2415x-battery driver\n"); + + return ret; +} +module_init(ltc294x_init); + +static void __exit ltc294x_exit(void) +{ + i2c_del_driver(<c294x_driver); + idr_destroy(<c294x_id); +} +module_exit(ltc294x_exit); MODULE_AUTHOR("Auryn Verwegen, Topic Embedded Systems"); MODULE_AUTHOR("Mike Looijmans, Topic Embedded Products");
Destroy IDRs on module exit, freeing the resources for * bq2415x_charger.c * ds2782_battery.c * ltc2941-battery-gauge.c The drivers had to be converted to "ordinary" module_init()/module_exit() style drivers instead of using module_i2c_driver. Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- drivers/power/bq2415x_charger.c | 20 +++++++++++++++++++- drivers/power/ds2782_battery.c | 20 +++++++++++++++++++- drivers/power/ltc2941-battery-gauge.c | 20 +++++++++++++++++++- 3 files changed, 57 insertions(+), 3 deletions(-)