power_supply: only register tosa_battery driver on tosa
PATCH 1/1 power_supply: only register tosa_battery driver on tosa
There are already several wm97xx-battery drivers. Do register the tosa one only on tosa machines.
--- drivers/power/tosa_battery.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/power/tosa_battery.c +3 -0
@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +return platform_driver_register (&tosa_bat_driver); }
Signature · 1 line
1.5.6.5
PATCH 1/1 power_supply: change the way how wm97xx-bat driver is registered
Do register the driver from wm97xx_bat_set_pdata instead of module init. This has several positive outcome points:
1) This driver for wm97xx-battery is now registered on demand, thus allowing other driver (e.g. tosa-battery) to claim the battery.
2) After dropping __init from wm97xx_bat_set_pdata(), the wm97xx_bat driver can be correctly built as a module. Then one can request it and set the battery data.
--- drivers/power/Kconfig | 4 ++-- drivers/power/wm97xx_battery.c | 12 +++++------- include/linux/wm97xx_batt.h | 7 +++++-- 3 files changed, 12 insertions(+), 11 deletions(-)
drivers/power/Kconfig +2 -2
@@ -57,8 +57,8 @@ config BATTERY_TOSA SL-6000 (tosa) models. config BATTERY_WM97XX - bool "WM97xx generic battery driver" - depends on TOUCHSCREEN_WM97XX=y + tristate "WM97xx generic battery driver" + depends on TOUCHSCREEN_WM97XX help Say Y to enable support for battery measured by WM97xx aux port.
drivers/power/wm97xx_battery.c +5 -7
@@ -248,23 +248,21 @@ static struct platform_driver wm97xx_bat_driver = { .resume = wm97xx_bat_resume, }; -static int __init wm97xx_bat_init (void ) -{ -return platform_driver_register (&wm97xx_bat_driver); -} - static void __exit wm97xx_bat_exit(void) {platform_driver_unregister (&wm97xx_bat_driver); } -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) { +if (pdata) +return -EEXIST ; + pdata = data; +return platform_driver_register (&wm97xx_bat_driver); }EXPORT_SYMBOL_GPL (wm97xx_bat_set_pdata); -module_init (wm97xx_bat_init);module_exit (wm97xx_bat_exit);MODULE_LICENSE ("GPL" );
include/linux/wm97xx_batt.h +5 -2
@@ -18,9 +18,12 @@ struct wm97xx_batt_info { };#ifdef CONFIG_BATTERY_WM97XX -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data); +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data);#else -static inline void wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +{ +return -ENODEV ; +}#endif #endif
Signature · 1 line
1.5.6.5
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 8 lines
Do register the driver from
wm97xx_bat_set_pdatainstead of module init. This has several positive outcome points:
1) This driver for wm97xx-battery is now registered on demand, thus allowing other driver (e.g. tosa-battery) to claim the battery.
2) After dropping__initfromwm97xx_bat_set_pdata(), thewm97xx_batdriver can be correctly built as a module. Then one can request it and set the battery data.
Quoted 7 lines
Signed-off-by Dmitry Baryshkov <[email protected]>Cc Marek Vasut <[email protected]>--- drivers/power/Kconfig | 4 ++-- drivers/power/wm97xx_battery.c | 12 +++++------- include/linux/wm97xx_batt.h | 7 +++++-- 3 files changed, 12 insertions(+), 11 deletions(-)
What about this patch?
Signature · 2 lines
With best wishes Dmitry
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
Quoted 17 lines · On Sunday 02 of November 2008 21:36:58 Dmitry wrote:
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 8 lines
Do register the driver from
wm97xx_bat_set_pdatainstead of module init. This has several positive outcome points:
1) This driver for wm97xx-battery is now registered on demand, thus allowing other driver (e.g. tosa-battery) to claim the battery.
2) After dropping__initfromwm97xx_bat_set_pdata(), thewm97xx_batdriver can be correctly built as a module. Then one can request it and set the battery data.Quoted 7 lines
Signed-off-by Dmitry Baryshkov <[email protected]>Cc Marek Vasut <[email protected]>--- drivers/power/Kconfig | 4 ++-- drivers/power/wm97xx_battery.c | 12 +++++------- include/linux/wm97xx_batt.h | 7 +++++-- 3 files changed, 12 insertions(+), 11 deletions(-)
Quoted 1 line
What about this patch?
Im sure you know what you are doing. I cant test it right now though, since I have load of university work to do, sorry.
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
In reply to Dmitry BaryshkovQuoted 71 lines · Dmitry Baryshkov
Do register the driver from
wm97xx_bat_set_pdatainstead of module init. This has several positive outcome points:
1) This driver for wm97xx-battery is now registered on demand, thus allowing other driver (e.g. tosa-battery) to claim the battery.
2) After dropping__initfromwm97xx_bat_set_pdata(), thewm97xx_batdriver can be correctly built as a module. Then one can request it and set the battery data.Signed-off-by Dmitry Baryshkov <[email protected]>Cc Marek Vasut <[email protected]>--- drivers/power/Kconfig | 4 ++-- drivers/power/wm97xx_battery.c | 12 +++++------- include/linux/wm97xx_batt.h | 7 +++++-- 3 files changed, 12 insertions(+), 11 deletions(-)drivers/power/Kconfig +2
-2@@ -57,8 +57,8 @@ config BATTERY_TOSA SL-6000 (tosa) models. config BATTERY_WM97XX - bool "WM97xx generic battery driver" - depends on TOUCHSCREEN_WM97XX=y + tristate "WM97xx generic battery driver" + depends on TOUCHSCREEN_WM97XX help Say Y to enable support for battery measured by WM97xx aux port.drivers/power/wm97xx_battery.c +5
-7@@ -248,23 +248,21 @@ static struct platform_driver wm97xx_bat_driver = { .resume = wm97xx_bat_resume, }; -static int __init wm97xx_bat_init (void ) -{ -return platform_driver_register (&wm97xx_bat_driver); -} - static void __exit wm97xx_bat_exit(void) {platform_driver_unregister (&wm97xx_bat_driver); } -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) { +if (pdata) +return -EEXIST ; + pdata = data; +return platform_driver_register (&wm97xx_bat_driver); }EXPORT_SYMBOL_GPL (wm97xx_bat_set_pdata); -module_init (wm97xx_bat_init);module_exit (wm97xx_bat_exit);MODULE_LICENSE ("GPL" );include/linux/wm97xx_batt.h +0
-0@@ -18,9 +18,12 @@ struct wm97xx_batt_info { };
include/linux/wm97xx_batt.h struct wm97xx_batt_info { · line 27
Quoted 10 lines
include/linux/wm97xx_batt.h +5
-2#ifdef CONFIG_BATTERY_WM97XX -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data); +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data);#else -static inline void wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +{ +return -ENODEV ; +}#endif
How that supposed to work when BATTERY_WM97XX is a module? #ifdef CONFIG_BATTERY_WM97XX will evaluate to false, and you'll have dummy wm97xx_bat_set_pdata() that returns -ENODEV...
The whole wm97xx stuff is broken wrt device-driver model... Yeah, I know why -- we don't have the ADC API/Subsystem. It would be great if somebody could find time to forward-port the subsystem from the handhelds.org tree...
I don't know what happened to the industrialio subsystem though
(http://lkml.org/lkml/2008/7/23/163), but it looked needlessly complicated, and probably Jonathan gave up on it... :-/
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
Quoted 87 lines · Anton Vorontsov
Quoted 71 lines · Dmitry Baryshkov
Do register the driver from
wm97xx_bat_set_pdatainstead of module init. This has several positive outcome points:
1) This driver for wm97xx-battery is now registered on demand, thus allowing other driver (e.g. tosa-battery) to claim the battery.
2) After dropping__initfromwm97xx_bat_set_pdata(), thewm97xx_batdriver can be correctly built as a module. Then one can request it and set the battery data.Signed-off-by Dmitry Baryshkov <[email protected]>Cc Marek Vasut <[email protected]>--- drivers/power/Kconfig | 4 ++-- drivers/power/wm97xx_battery.c | 12 +++++------- include/linux/wm97xx_batt.h | 7 +++++-- 3 files changed, 12 insertions(+), 11 deletions(-)drivers/power/Kconfig +2
-2@@ -57,8 +57,8 @@ config BATTERY_TOSA SL-6000 (tosa) models. config BATTERY_WM97XX - bool "WM97xx generic battery driver" - depends on TOUCHSCREEN_WM97XX=y + tristate "WM97xx generic battery driver" + depends on TOUCHSCREEN_WM97XX help Say Y to enable support for battery measured by WM97xx aux port.drivers/power/wm97xx_battery.c +5
-7@@ -248,23 +248,21 @@ static struct platform_driver wm97xx_bat_driver = { .resume = wm97xx_bat_resume, }; -static int __init wm97xx_bat_init (void ) -{ -return platform_driver_register (&wm97xx_bat_driver); -} - static void __exit wm97xx_bat_exit(void) {platform_driver_unregister (&wm97xx_bat_driver); } -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) { +if (pdata) +return -EEXIST ; + pdata = data; +return platform_driver_register (&wm97xx_bat_driver); }EXPORT_SYMBOL_GPL (wm97xx_bat_set_pdata); -module_init (wm97xx_bat_init);module_exit (wm97xx_bat_exit);MODULE_LICENSE ("GPL" );include/linux/wm97xx_batt.h +0
-0@@ -18,9 +18,12 @@ struct wm97xx_batt_info { };include/linux/wm97xx_batt.h struct wm97xx_batt_info { · line 27
Quoted 10 lines
include/linux/wm97xx_batt.h +5
-2#ifdef CONFIG_BATTERY_WM97XX -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data); +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data);#else -static inline void wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +{ +return -ENODEV ; +}#endif How that supposed to work when
BATTERY_WM97XXis a module? #ifdefCONFIG_BATTERY_WM97XXwill evaluate to false, and you'll have dummywm97xx_bat_set_pdata()that returns-ENODEV...
Quoted 9 lines
The whole wm97xx stuff is broken wrt device-driver model... Yeah, I know why -- we don't have the ADC API/Subsystem. It would be great if somebody could find time to forward-port the subsystem from the handhelds.org tree...
I don't know what happened to the industrialio subsystem though
(http://lkml.org/lkml/2008/7/23/163), but it looked needlessly complicated, and probably Jonathan gave up on it... :-/
It is needlessly complex for this sort of thing, but that's not it's purpose. (though that's not to say it won't be able to do this sort of thing). It's gotten a smigeon delayed due to a change of my own requirements for what it does. As a reminder, the purpose of that subsystem was at least partly to provide reasonably high performance data capture facilities (ring buffers, triggered sampling etc alongside suitably powerful userspace interfaces.) Possibly my apps are somewhat unusual, but the complexity is absolutely necessary for what I'm doing (annoyingly!)
The complexity is partly due to trying to elegantly allow much reduced
functionality such as this sort of app requires. (leads to a lot
of fiddly testing!)
Anyhow, definitely not given up on it. The intermediate versions are getting a fair bit of testing, but isn't ready (in my view) to go towards mainline yet.
Jonathan
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
On Mon, Nov 03, 2008 at 04:54:57PM +0000, Jonathan Cameron wrote:
[...]
Quoted 9 lines
It is needlessly complex for this sort of thing, but that's not it's purpose. (though that's not to say it won't be able to do this sort of thing). It's gotten a smigeon delayed due to a change of my own requirements for what it does. As a reminder, the purpose of that subsystem was at least partly to provide reasonably high performance data capture facilities (ring buffers, triggered sampling etc alongside suitably powerful userspace interfaces.) Possibly my apps are somewhat unusual, but the complexity is absolutely necessary for what I'm doing (annoyingly!)
Yeah, I understand that. But when you need simple driver for very simple ADC device, the subsystem is a bit scary. Maybe we could just implement "simple API" on top of it, that would hide the complexity.
Something like
value = adc_sample_pin(adc_device, "voltage");
And
struct adc_pin pins[2];
pins[0].name = "x-axis";
pins[0].num_samples = 5;
pins[1].name = "y-axis";
pins[2].num_samples = 5;
adc_sample_pins(adc_device, pins);
for (i = 0; i < ARRAY_SIZE(pins); i++) {
for (j = 0; j < pins[i].num_samples; i++)
process(pins[i].values[j].value);
}That would work for most in-kernel ADC users (batteries, touchscreens).
Quoted 1 line
Anyhow, definitely not given up on it.
Great, looking forward to your patches.
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
In reply to Anton VorontsovQuoted 15 lines · Anton Vorontsov
Review
Quoted 10 lines · Dmitry Baryshkov
#ifdef
CONFIG_BATTERY_WM97XX(quoted diff) +5
-2-void __init wm97xx_bat_set_pdata(struct wm97xx_batt_info *data); +int wm97xx_bat_set_pdata(struct wm97xx_batt_info *data); #else -static inline void wm97xx_bat_set_pdata(struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata(struct wm97xx_batt_info *data) +{ + return -ENODEV; +} #endifHow that supposed to work when
BATTERY_WM97XXis a module? #ifdefCONFIG_BATTERY_WM97XXwill evaluate to false, and you'll have dummywm97xx_bat_set_pdata()that returns-ENODEV...
This won't of course fix the wm97xx driver model, but the module issue should be fixed. What about this patch?
Quoted 1 line
From
f4b2e46cbac0d3ce77643c008d44fdaea8864366Mon Sep 17 00:00:00 2001
From: Dmitry Baryshkov <[email protected]>
Date: Wed, 29 Oct 2008 12:00:57 +0300
Subject: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
Do register the driver from wm97xx_bat_set_pdata instead of module init. This has several positive outcome points:
1) This driver for wm97xx-battery is now registered on demand, thus allowing other driver (e.g. tosa-battery) to claim the battery.
2) After dropping __init from wm97xx_bat_set_pdata(), the wm97xx_bat driver can be correctly built as a module. Then one can request it and set the battery data.
--- drivers/power/Kconfig | 4 ++-- drivers/power/wm97xx_battery.c | 12 +++++------- include/linux/wm97xx_batt.h | 9 ++++++--- 3 files changed, 13 insertions(+), 12 deletions(-)
drivers/power/Kconfig +2 -2
@@ -57,8 +57,8 @@ config BATTERY_TOSA SL-6000 (tosa) models. config BATTERY_WM97XX - bool "WM97xx generic battery driver" - depends on TOUCHSCREEN_WM97XX=y + tristate "WM97xx generic battery driver" + depends on TOUCHSCREEN_WM97XX help Say Y to enable support for battery measured by WM97xx aux port.
drivers/power/wm97xx_battery.c +5 -7
@@ -248,23 +248,21 @@ static struct platform_driver wm97xx_bat_driver = { .resume = wm97xx_bat_resume, }; -static int __init wm97xx_bat_init (void ) -{ -return platform_driver_register (&wm97xx_bat_driver); -} - static void __exit wm97xx_bat_exit(void) {platform_driver_unregister (&wm97xx_bat_driver); } -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) { +if (pdata) +return -EEXIST ; + pdata = data; +return platform_driver_register (&wm97xx_bat_driver); }EXPORT_SYMBOL_GPL (wm97xx_bat_set_pdata); -module_init (wm97xx_bat_init);module_exit (wm97xx_bat_exit);MODULE_LICENSE ("GPL" );
include/linux/wm97xx_batt.h +6 -3
@@ -17,10 +17,13 @@ struct wm97xx_batt_info { char *batt_name; }; -#ifdef CONFIG_BATTERY_WM97XX -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data); +#if defined(CONFIG_BATTERY_WM97XX ) || defined(CONFIG_BATTERY_WM97XX_MODULE ) +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data);#else -static inline void wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +{ +return -ENODEV ; +}#endif #endif
Signature · 6 lines
1.5.6.5 -- With best wishes Dmitry
Re: [PATCH] power_supply: change the way how wm97xx-bat driver is registered
Quoted 19 lines · Dmitry Baryshkov
Quoted 15 lines · Anton Vorontsov
include/linux/wm97xx_batt.h struct wm97xx_batt_info { · line 27
Quoted 10 lines · Dmitry Baryshkov
#ifdef
CONFIG_BATTERY_WM97XXinclude/linux/wm97xx_batt.h +5
-2-void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data); +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data);#else -static inline void wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +{ +return -ENODEV ; +}#endif How that supposed to work when
BATTERY_WM97XXis a module? #ifdefCONFIG_BATTERY_WM97XXwill evaluate to false, and you'll have dummywm97xx_bat_set_pdata()that returns-ENODEV...This won't of course fix the wm97xx driver model, but the module issue should be fixed. What about this patch?
[...]
include/linux/wm97xx_batt.h struct wm97xx_batt_info { · line 21
Quoted 4 lines
include/linux/wm97xx_batt.h +2
-2-#ifdef CONFIG_BATTERY_WM97XX -void __init wm97xx_bat_set_pdata (struct wm97xx_batt_info *data); +#if defined(CONFIG_BATTERY_WM97XX ) || defined(CONFIG_BATTERY_WM97XX_MODULE ) +int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data);
In case of CONFIG_BATTERY_WM97XX_MODULE, the build will break at link time, since wm97xx_bat_set_pdata() might be called from the built-in code (i.e. arch/arm/mach-pxa/palmtx.c).
Quoted 11 lines
#else
include/linux/wm97xx_batt.h +4
-1-static inline void wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) {} +static inline int wm97xx_bat_set_pdata (struct wm97xx_batt_info *data) +{ +return -ENODEV ; +}#endif #endif Signature · 1 line
1.5.6.5
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 7 lines
There are already several wm97xx-battery drivers. Do register the tosa one only on tosa machines.
Signed-off-by Dmitry Baryshkov <[email protected]>--- drivers/power/tosa_battery.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
What about this patch? IMO it can and should be merged during 2.6.28-rc cycle.
Signature · 2 lines
With best wishes Dmitry
Quoted 10 lines · Dmitry
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 7 lines
There are already several wm97xx-battery drivers. Do register the tosa one only on tosa machines.
Signed-off-by Dmitry Baryshkov <[email protected]>--- drivers/power/tosa_battery.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)What about this patch?
Sorry for the delay.
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 474
Quoted 13 lines
IMO it can and should be merged during 2.6.28-rc cycle.
drivers/power/tosa_battery.c +3
-0@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +
I tend to reject this approach. You should rename the driver instead.
I.e.
-.driver.name = "wm97xx-battery",
-.driver.name = "tosa-battery",
And make sure that this won't break users of that driver (though I don't see any).
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 476
Quoted 2 lines
drivers/power/tosa_battery.c +0
-0return platform_driver_register (&tosa_bat_driver); }
Thanks,
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 30 lines · Dmitry
Quoted 10 lines
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 7 lines
There are already several wm97xx-battery drivers. Do register the tosa one only on tosa machines.
Signed-off-by Dmitry Baryshkov <[email protected]>--- drivers/power/tosa_battery.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)What about this patch?
Sorry for the delay.
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 474
Quoted 13 lines
IMO it can and should be merged during 2.6.28-rc cycle.
drivers/power/tosa_battery.c +3
-0@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +I tend to reject this approach. You should rename the driver instead.
Quoted 6 lines
I.e.
-.driver.name = "wm97xx-battery",
-.driver.name = "tosa-battery",And make sure that this won't break users of that driver (though I don't see any).
The wm97xx-battery device is registered bydrivers/input/toucscreen/wm97xx-core.c
Quoted 11 lines
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 476
Quoted 3 lines
drivers/power/tosa_battery.c +0
-0return platform_driver_register (&tosa_bat_driver); }Thanks,
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
Signature · 2 lines
With best wishes Dmitry
Quoted 35 lines · Dmitry
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 34 lines · Dmitry
Quoted 10 lines
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 7 lines
There are already several wm97xx-battery drivers. Do register the tosa one only on tosa machines.
Signed-off-by Dmitry Baryshkov <[email protected]>--- drivers/power/tosa_battery.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)What about this patch?
Sorry for the delay.
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 474
Quoted 13 lines
IMO it can and should be merged during 2.6.28-rc cycle.
drivers/power/tosa_battery.c +3
-0@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +I tend to reject this approach. You should rename the driver instead.
I.e.
-.driver.name = "wm97xx-battery",
-.driver.name = "tosa-battery",
Quoted 5 lines
Quoted 2 lines
And make sure that this won't break users of that driver (though I don't see any).
The wm97xx-battery device is registered by
drivers/input/toucscreen/wm97xx-core.c
Hmm.. So you can't rename the tosa driver, because then it won't work, correct?
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 36 lines · Dmitry
Quoted 35 lines
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 34 lines · Dmitry
Quoted 10 lines
2008/10/29 Dmitry Baryshkov <[email protected]>:
Quoted 7 lines
There are already several wm97xx-battery drivers. Do register the tosa one only on tosa machines.
Signed-off-by Dmitry Baryshkov <[email protected]>--- drivers/power/tosa_battery.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)What about this patch?
Sorry for the delay.
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 474
Quoted 13 lines
IMO it can and should be merged during 2.6.28-rc cycle.
drivers/power/tosa_battery.c +3
-0@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +I tend to reject this approach. You should rename the driver instead.
I.e.
-.driver.name = "wm97xx-battery",
-.driver.name = "tosa-battery",
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
And make sure that this won't break users of that driver (though I don't see any).
The wm97xx-battery device is registered by
drivers/input/toucscreen/wm97xx-core.cHmm.. So you can't rename the tosa driver, because then it won't work, correct?
Yup.
I see few ways to resolve this:
* write better wm97xx interface. Dunno if that is feasible or possible. One of the possible solutions is to pass battery and ts device names and data from within board data via ac97 layer to wm97xx-core. This will provide several benefits (e.g. then we can drop lots of parameters from wm97xx-core, which are really board parameters).
* Merge most of functionality of wm97xx-battery driver for Palms (by Marek), my tosa battery driver, name that The Only wm97xx-battery Driver. However on this path we will have to put lot's of code into hooks, which is
pretty much
ugly IMO.
Signature · 2 lines
With best wishes Dmitry
On Mon, Nov 03, 2008 at 07:41:47PM +0300, Dmitry wrote:
[...]
Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 474
Quoted 11 lines
drivers/power/tosa_battery.c +3
-0@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +I tend to reject this approach. You should rename the driver instead.
I.e.
-.driver.name = "wm97xx-battery",
-.driver.name = "tosa-battery",Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
And make sure that this won't break users of that driver (though I don't see any).
The wm97xx-battery device is registered by
drivers/input/toucscreen/wm97xx-core.cHmm.. So you can't rename the tosa driver, because then it won't work, correct?
Quoted 7 lines
Yup.
I see few ways to resolve this:
* write better wm97xx interface. Dunno if that is feasible or possible. One of the possible solutions is to pass battery and ts device names and data from within board data via ac97 layer to wm97xx-core. This will provide several benefits (e.g. then we can drop lots of parameters from wm97xx-core, which are really board parameters).
That would be great indeed. But for now, just don't compile the two drivers on the tosa platform. No need for machine_is_*() hacks...
Thanks,
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 37 lines · Dmitry
[...]
Quoted 27 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
drivers/power/tosa_battery.c static struct platform_driver tosa_bat_driver = { · line 474
Quoted 11 lines
drivers/power/tosa_battery.c +3
-0@@ -469,6 +469,9 @@ static struct platform_driver tosa_bat_driver = {static int __init tosa_bat_init (void ) { +if (!machine_is_tosa ()) +return -EINVAL ; +I tend to reject this approach. You should rename the driver instead.
I.e.
-.driver.name = "wm97xx-battery",
-.driver.name = "tosa-battery",Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
And make sure that this won't break users of that driver (though I don't see any).
The wm97xx-battery device is registered by
drivers/input/toucscreen/wm97xx-core.cHmm.. So you can't rename the tosa driver, because then it won't work, correct?
Quoted 7 lines
Yup.
I see few ways to resolve this:
* write better wm97xx interface. Dunno if that is feasible or possible. One of the possible solutions is to pass battery and ts device names and data from within board data via ac97 layer to wm97xx-core. This will provide several benefits (e.g. then we can drop lots of parameters from wm97xx-core, which are really board parameters).
Quoted 2 lines
That would be great indeed. But for now, just don't compile the two drivers on the tosa platform. No need for machine_is_*() hacks...
That is a bit strange requirement. During several past months there was a lot of efforts put into supporting multi-machine PXA kernel images. And this requirement goes against this.
Signature · 2 lines
With best wishes Dmitry
On Mon, Nov 03, 2008 at 08:09:14PM +0300, Dmitry wrote:
[...]
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Yup.
I see few ways to resolve this:
* write better wm97xx interface. Dunno if that is feasible or possible. One of the possible solutions is to pass battery and ts device names and data from within board data via ac97 layer to wm97xx-core. This will provide several benefits (e.g. then we can drop lots of parameters from wm97xx-core, which are really board parameters).
Quoted 5 lines
Quoted 2 lines
That would be great indeed. But for now, just don't compile the two drivers on the tosa platform. No need for machine_is_*() hacks...
That is a bit strange requirement. During several past months there was a lot of efforts put into supporting multi-machine PXA kernel images.
Yes, I know this.
But you're implementing hacks instead of fixing the problem.
Btw, you inserted the machine_is() into the tosa driver, but wm97xx driver registers unconditionally. That could be a problem when wm97xx loads first, and then you try to load the tosa_battery module on tosa machine.
Let's insert machine_is() hack into the wm97xx driver as well?
Signature · 3 lines
Anton Vorontsov email: [email protected] irc://irc.freenode.net/bd2
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 18 lines · Dmitry
[...]
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Yup.
I see few ways to resolve this:
* write better wm97xx interface. Dunno if that is feasible or possible. One of the possible solutions is to pass battery and ts device names and data from within board data via ac97 layer to wm97xx-core. This will provide several benefits (e.g. then we can drop lots of parameters from wm97xx-core, which are really board parameters).Quoted 5 lines
Quoted 2 lines
That would be great indeed. But for now, just don't compile the two drivers on the tosa platform. No need for machine_is_*() hacks...
That is a bit strange requirement. During several past months there was a lot of efforts put into supporting multi-machine PXA kernel images.
Yes, I know this.
Quoted 8 lines
But you're implementing hacks instead of fixing the problem.
Btw, you inserted the
machine_is()into the tosa driver, but wm97xx driver registers unconditionally. That could be a problem when wm97xx loads first, and then you try to load thetosa_batterymodule on tosa machine.Let's insert
machine_is()hack into the wm97xx driver as well?
That's why I suggested moving driver registration into _set_pdata() function in that driver.
Quoted 6 lines
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
Signature · 2 lines
With best wishes Dmitry
Quoted 10 lines · Dmitry
2008/11/3 Anton Vorontsov <[email protected]>:
Quoted 2 lines
Hmm.. So you can't rename the tosa driver, because then it won't work, correct?
* write better wm97xx interface. Dunno if that is feasible or possible. One of the possible solutions is to pass battery and ts device names and data from within board data via ac97 layer to wm97xx-core. This will provide several benefits (e.g. then we can drop lots of parameters from wm97xx-core, which are really board parameters).
There's only trivial writing required to do this in the WM97xx driver itself - all that needs to be done is to allow the existing machine data to be passed in as platform data and then add a couple of additional hooks to that to allow instantation of things like the battery drivers from machine code rather than doing it unconditionally as is done now. The blocker currently is that it's not possible to pass any platform data to an AC97 codec.
There was some work on doing that from Sebastian Siewior (CCed) but it doesn't look like it ever made it to mainline. The last I saw of it[1] it got stalled due to concerns about the difficulty in doing something that is both type safe and general enough to cover complex cases like the WM97xx parts that want to pass in lots of highly device and system specific stuff. It did go into external ALSA for at least a while, though.
It'd be really good to see some way of getting platform data to AC97 devices. It would make the WM97xx drivers a lot easier to use if we could do it - I keep hoping to find the cycles to actively pursue this myself but it's not happened yet.