* [PATCH] power: Destroy IDRs on module exit
@ 2015-07-14 9:16 Johannes Thumshirn
2015-07-14 9:17 ` Oliver Neukum
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2015-07-14 9:16 UTC (permalink / raw)
To: Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse
Cc: linux-pm, linux-kernel, Johannes Thumshirn
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");
--
2.4.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] power: Destroy IDRs on module exit
2015-07-14 9:16 [PATCH] power: Destroy IDRs on module exit Johannes Thumshirn
@ 2015-07-14 9:17 ` Oliver Neukum
2015-07-14 9:20 ` Pali Rohár
2015-07-14 10:19 ` Sebastian Reichel
2 siblings, 0 replies; 6+ messages in thread
From: Oliver Neukum @ 2015-07-14 9:17 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Pali Rohár, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, linux-pm, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] power: Destroy IDRs on module exit
2015-07-14 9:16 [PATCH] power: Destroy IDRs on module exit Johannes Thumshirn
2015-07-14 9:17 ` Oliver Neukum
@ 2015-07-14 9:20 ` Pali Rohár
2015-07-14 10:19 ` Sebastian Reichel
2 siblings, 0 replies; 6+ messages in thread
From: Pali Rohár @ 2015-07-14 9:20 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-pm, linux-kernel
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.
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] power: Destroy IDRs on module exit
2015-07-14 9:16 [PATCH] power: Destroy IDRs on module exit Johannes Thumshirn
2015-07-14 9:17 ` Oliver Neukum
2015-07-14 9:20 ` Pali Rohár
@ 2015-07-14 10:19 ` Sebastian Reichel
2015-07-14 10:48 ` Johannes Thumshirn
2 siblings, 1 reply; 6+ messages in thread
From: Sebastian Reichel @ 2015-07-14 10:19 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Pali Rohár, Dmitry Eremin-Solenikov, David Woodhouse,
linux-pm, linux-kernel, Tejun Heo
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
[+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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] power: Destroy IDRs on module exit
2015-07-14 10:19 ` Sebastian Reichel
@ 2015-07-14 10:48 ` Johannes Thumshirn
2015-07-14 12:14 ` Sebastian Reichel
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Thumshirn @ 2015-07-14 10:48 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pali Rohár, Dmitry Eremin-Solenikov, David Woodhouse,
linux-pm, linux-kernel, Tejun Heo, Johannes Thumshirn
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] power: Destroy IDRs on module exit
2015-07-14 10:48 ` Johannes Thumshirn
@ 2015-07-14 12:14 ` Sebastian Reichel
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2015-07-14 12:14 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 718 bytes --]
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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-14 12:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-14 9:16 [PATCH] power: Destroy IDRs on module exit Johannes Thumshirn
2015-07-14 9:17 ` Oliver Neukum
2015-07-14 9:20 ` Pali Rohár
2015-07-14 10:19 ` Sebastian Reichel
2015-07-14 10:48 ` Johannes Thumshirn
2015-07-14 12:14 ` Sebastian Reichel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.