* [PATCH] mfd: ab8500: update kconfig for ab8500 core driver
@ 2010-09-15 11:23 Arun Murthy
2010-09-15 15:20 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Arun Murthy @ 2010-09-15 11:23 UTC (permalink / raw)
To: sameo
Cc: linux-kernel, linus.walleij, mattias.wallin, srinidhi.kasagar,
arun.murthy
This patch add a dependancy for ab8500-core driver so as to depend on
u8500 platform.
This patch also fixes the build issues(powerpc_allyesconfig) for the
patch 03f582a93ecca6e9584b622570022abf08ed03ec (misc: Add ab8500 pwm
driver)
Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
drivers/mfd/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index fbfe14a..841ce47 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -475,7 +475,7 @@ config EZX_PCAP
config AB8500_CORE
bool "ST-Ericsson AB8500 Mixed Signal Power Management chip"
- depends on GENERIC_HARDIRQS && ABX500_CORE
+ depends on GENERIC_HARDIRQS && ABX500_CORE && ARCH_U8500
select MFD_CORE
help
Select this option to enable access to AB8500 power management
--
1.7.2.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver
2010-09-15 11:23 [PATCH] mfd: ab8500: update kconfig for ab8500 core driver Arun Murthy
@ 2010-09-15 15:20 ` Randy Dunlap
2010-09-15 17:12 ` Arun MURTHY
0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2010-09-15 15:20 UTC (permalink / raw)
To: Arun Murthy
Cc: sameo, linux-kernel, linus.walleij, mattias.wallin,
srinidhi.kasagar
On Wed, 15 Sep 2010 16:53:43 +0530 Arun Murthy wrote:
> This patch add a dependancy for ab8500-core driver so as to depend on
> u8500 platform.
>
> This patch also fixes the build issues(powerpc_allyesconfig) for the
> patch 03f582a93ecca6e9584b622570022abf08ed03ec (misc: Add ab8500 pwm
> driver)
Please list build errors/warnings that are fixed in your patch description.
This patch will certainly keep i386 builds from failing, like I saw
yesterday. However, since this driver uses some spi_*() functions,
it should also depend on SPI_MASTER IMO.
See this patch: http://lkml.org/lkml/2010/9/14/412
> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
> drivers/mfd/Kconfig | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index fbfe14a..841ce47 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -475,7 +475,7 @@ config EZX_PCAP
>
> config AB8500_CORE
> bool "ST-Ericsson AB8500 Mixed Signal Power Management chip"
> - depends on GENERIC_HARDIRQS && ABX500_CORE
> + depends on GENERIC_HARDIRQS && ABX500_CORE && ARCH_U8500
> select MFD_CORE
> help
> Select this option to enable access to AB8500 power management
> --
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver
2010-09-15 15:20 ` Randy Dunlap
@ 2010-09-15 17:12 ` Arun MURTHY
2010-09-15 17:41 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Arun MURTHY @ 2010-09-15 17:12 UTC (permalink / raw)
To: Randy Dunlap
Cc: sameo@linux.intel.com, linux-kernel@vger.kernel.org,
Linus WALLEIJ, Mattias WALLIN, Srinidhi KASAGAR
Hi Randy,
> On Wed, 15 Sep 2010 16:53:43 +0530 Arun Murthy wrote:
>
> > This patch add a dependancy for ab8500-core driver so as to depend on
> > u8500 platform.
> >
> > This patch also fixes the build issues(powerpc_allyesconfig) for the
> > patch 03f582a93ecca6e9584b622570022abf08ed03ec (misc: Add ab8500 pwm
> > driver)
>
> Please list build errors/warnings that are fixed in your patch
> description.
>
Please find the build error log below (http://www.spinics.net/lists/kernel/msg1083549.html)
drivers/mfd/built-in.o: In function `__crc_pwm_free':
(*ABS*+0x24ba6d1e): multiple definition of `__crc_pwm_free'
drivers/mfd/built-in.o: In function `.pwm_free':
(.text+0x1ab3c): multiple definition of `.pwm_free'
drivers/misc/built-in.o:(.text+0x2620c): first defined here
drivers/mfd/built-in.o: In function `__crc_pwm_request':
(*ABS*+0xc1f4ec93): multiple definition of `__crc_pwm_request'
drivers/mfd/built-in.o: In function `__crc_pwm_enable':
(*ABS*+0x9d09808d): multiple definition of `__crc_pwm_enable'
drivers/mfd/built-in.o: In function `.pwm_enable':
(.text+0x1abc8): multiple definition of `.pwm_enable'
drivers/misc/built-in.o:(.text+0x26278): first defined here
drivers/mfd/built-in.o: In function `.pwm_request':
(.text+0x1a774): multiple definition of `.pwm_request'
drivers/misc/built-in.o:(.text+0x2604c): first defined here
drivers/mfd/built-in.o: In function `pwm_config':
(.opd+0x1ce0): multiple definition of `pwm_config'
drivers/misc/built-in.o:(.opd+0x3000): first defined here
drivers/mfd/built-in.o: In function `pwm_free':
(.opd+0x1d28): multiple definition of `pwm_free'
drivers/misc/built-in.o:(.opd+0x2fd0): first defined here
drivers/mfd/built-in.o: In function `pwm_request':
(.opd+0x1cf8): multiple definition of `pwm_request'
drivers/misc/built-in.o:(.opd+0x2f58): first defined here
drivers/mfd/built-in.o: In function `__crc_pwm_disable':
(*ABS*+0xb0493b18): multiple definition of `__crc_pwm_disable'
drivers/mfd/built-in.o: In function `.pwm_disable':
(.text+0x1aa24): multiple definition of `.pwm_disable'
drivers/misc/built-in.o:(.text+0x2613c): first defined here
drivers/mfd/built-in.o: In function `pwm_enable':
(.opd+0x1d40): multiple definition of `pwm_enable'
drivers/misc/built-in.o:(.opd+0x2fe8): first defined here
drivers/mfd/built-in.o: In function `.pwm_config':
(.text+0x1a648): multiple definition of `.pwm_config'
drivers/misc/built-in.o:(.text+0x26358): first defined here
drivers/mfd/built-in.o: In function `pwm_disable':
(.opd+0x1d10): multiple definition of `pwm_disable'
drivers/misc/built-in.o:(.opd+0x2fb8): first defined here
drivers/mfd/built-in.o: In function `__crc_pwm_config':
(*ABS*+0xc23f5b9): multiple definition of `__crc_pwm_config
> This patch will certainly keep i386 builds from failing, like I saw
> yesterday. However, since this driver uses some spi_*() functions,
> it should also depend on SPI_MASTER IMO.
> See this patch: http://lkml.org/lkml/2010/9/14/412
>
I saw your patch for fixing the same @ (http://www.spinics.net/lists/kernel/msg1083970.html)
Hence I have'nt included this.
Thanks and Regards,
Arun R Murthy
--------------
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver
2010-09-15 17:12 ` Arun MURTHY
@ 2010-09-15 17:41 ` Randy Dunlap
2010-09-15 18:21 ` Samuel Ortiz
0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2010-09-15 17:41 UTC (permalink / raw)
To: Arun MURTHY
Cc: sameo@linux.intel.com, linux-kernel@vger.kernel.org,
Linus WALLEIJ, Mattias WALLIN, Srinidhi KASAGAR
On Wed, 15 Sep 2010 19:12:33 +0200 Arun MURTHY wrote:
[resending, first time bounced for some reason]
> Hi Randy,
>
> > On Wed, 15 Sep 2010 16:53:43 +0530 Arun Murthy wrote:
> >
> > > This patch add a dependancy for ab8500-core driver so as to depend on
> > > u8500 platform.
> > >
> > > This patch also fixes the build issues(powerpc_allyesconfig) for the
> > > patch 03f582a93ecca6e9584b622570022abf08ed03ec (misc: Add ab8500 pwm
> > > driver)
> >
> > Please list build errors/warnings that are fixed in your patch
> > description.
> >
> Please find the build error log below (http://www.spinics.net/lists/kernel/msg1083549.html)
>
> drivers/mfd/built-in.o: In function `__crc_pwm_free':
> (*ABS*+0x24ba6d1e): multiple definition of `__crc_pwm_free'
> drivers/mfd/built-in.o: In function `.pwm_free':
> (.text+0x1ab3c): multiple definition of `.pwm_free'
> drivers/misc/built-in.o:(.text+0x2620c): first defined here
> drivers/mfd/built-in.o: In function `__crc_pwm_request':
> (*ABS*+0xc1f4ec93): multiple definition of `__crc_pwm_request'
> drivers/mfd/built-in.o: In function `__crc_pwm_enable':
> (*ABS*+0x9d09808d): multiple definition of `__crc_pwm_enable'
> drivers/mfd/built-in.o: In function `.pwm_enable':
> (.text+0x1abc8): multiple definition of `.pwm_enable'
> drivers/misc/built-in.o:(.text+0x26278): first defined here
> drivers/mfd/built-in.o: In function `.pwm_request':
> (.text+0x1a774): multiple definition of `.pwm_request'
> drivers/misc/built-in.o:(.text+0x2604c): first defined here
> drivers/mfd/built-in.o: In function `pwm_config':
> (.opd+0x1ce0): multiple definition of `pwm_config'
> drivers/misc/built-in.o:(.opd+0x3000): first defined here
> drivers/mfd/built-in.o: In function `pwm_free':
> (.opd+0x1d28): multiple definition of `pwm_free'
> drivers/misc/built-in.o:(.opd+0x2fd0): first defined here
> drivers/mfd/built-in.o: In function `pwm_request':
> (.opd+0x1cf8): multiple definition of `pwm_request'
> drivers/misc/built-in.o:(.opd+0x2f58): first defined here
> drivers/mfd/built-in.o: In function `__crc_pwm_disable':
> (*ABS*+0xb0493b18): multiple definition of `__crc_pwm_disable'
> drivers/mfd/built-in.o: In function `.pwm_disable':
> (.text+0x1aa24): multiple definition of `.pwm_disable'
> drivers/misc/built-in.o:(.text+0x2613c): first defined here
> drivers/mfd/built-in.o: In function `pwm_enable':
> (.opd+0x1d40): multiple definition of `pwm_enable'
> drivers/misc/built-in.o:(.opd+0x2fe8): first defined here
> drivers/mfd/built-in.o: In function `.pwm_config':
> (.text+0x1a648): multiple definition of `.pwm_config'
> drivers/misc/built-in.o:(.text+0x26358): first defined here
> drivers/mfd/built-in.o: In function `pwm_disable':
> (.opd+0x1d10): multiple definition of `pwm_disable'
> drivers/misc/built-in.o:(.opd+0x2fb8): first defined here
> drivers/mfd/built-in.o: In function `__crc_pwm_config':
> (*ABS*+0xc23f5b9): multiple definition of `__crc_pwm_config
I don't know what the __crc_* symbols are (I can't find them anywhere).
The other functions (pwm_config, pwm_free, pwm_request, pwm_disable,
pwm_enable) exist in multiple places. This is not good.
They are very generically named. The instances of these that are
provided by platform code are OK (these):
./include/linux/pwm.h:19:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
./arch/arm/plat-pxa/pwm.c:64:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
./arch/arm/plat-pxa/pwm.c:101:EXPORT_SYMBOL(pwm_config);
./arch/arm/plat-samsung/pwm.c:194:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
./arch/arm/plat-samsung/pwm.c:281:EXPORT_SYMBOL(pwm_config);
./arch/arm/plat-mxc/pwm.c:55:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
./arch/arm/plat-mxc/pwm.c:114:EXPORT_SYMBOL(pwm_config);
./arch/mips/jz4740/pwm.c:94:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
but the instances of these that are defined in
drivers/mfd/twl6030-pwm.c should not be named so generically.
Changing (Fixing) the function names in twl6030-pwm.c should fix the build problem
that you reported, I think. And it will still allow the ab8500 driver to be
built on other platforms, which is what we prefer when that is possible.
> > This patch will certainly keep i386 builds from failing, like I saw
> > yesterday. However, since this driver uses some spi_*() functions,
> > it should also depend on SPI_MASTER IMO.
> > See this patch: http://lkml.org/lkml/2010/9/14/412
> >
> I saw your patch for fixing the same @ (http://www.spinics.net/lists/kernel/msg1083970.html)
> Hence I have'nt included this.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver
2010-09-15 17:41 ` Randy Dunlap
@ 2010-09-15 18:21 ` Samuel Ortiz
2010-09-15 18:25 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Samuel Ortiz @ 2010-09-15 18:21 UTC (permalink / raw)
To: Randy Dunlap
Cc: Arun MURTHY, linux-kernel@vger.kernel.org, Linus WALLEIJ,
Mattias WALLIN, Srinidhi KASAGAR
Hi Randy,
On Wed, Sep 15, 2010 at 10:41:05AM -0700, Randy Dunlap wrote:
> I don't know what the __crc_* symbols are (I can't find them anywhere).
>
> The other functions (pwm_config, pwm_free, pwm_request, pwm_disable,
> pwm_enable) exist in multiple places. This is not good.
> They are very generically named. The instances of these that are
> provided by platform code are OK (these):
>
> ./include/linux/pwm.h:19:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> ./arch/arm/plat-pxa/pwm.c:64:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ./arch/arm/plat-pxa/pwm.c:101:EXPORT_SYMBOL(pwm_config);
> ./arch/arm/plat-samsung/pwm.c:194:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ./arch/arm/plat-samsung/pwm.c:281:EXPORT_SYMBOL(pwm_config);
> ./arch/arm/plat-mxc/pwm.c:55:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ./arch/arm/plat-mxc/pwm.c:114:EXPORT_SYMBOL(pwm_config);
> ./arch/mips/jz4740/pwm.c:94:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>
> but the instances of these that are defined in
> drivers/mfd/twl6030-pwm.c should not be named so generically.
>
> Changing (Fixing) the function names in twl6030-pwm.c should fix the build problem
> that you reported, I think.
The pwm "API" relies on someone providing the pwm_* symbols, and then you have
leds_pwm or backlight_pwm calling those symbols out of the blue.
If we change the twl6030-pwm.c function names, no pwm users (backlight, led or
input) will actually be able to use the twl6030 PWM driver.
The pwm API is very poorly designed in my opinion and should provide a way for
pwm drivers to register against it. pwm users will then call into the pwm
framework who would select which driver to use.
> And it will still allow the ab8500 driver to be
> built on other platforms, which is what we prefer when that is possible.
I agree with that, but couldnt see any other fix with the current pwm
situation.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: ab8500: update kconfig for ab8500 core driver
2010-09-15 18:21 ` Samuel Ortiz
@ 2010-09-15 18:25 ` Randy Dunlap
0 siblings, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2010-09-15 18:25 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Arun MURTHY, linux-kernel@vger.kernel.org, Linus WALLEIJ,
Mattias WALLIN, Srinidhi KASAGAR
On 09/15/10 11:21, Samuel Ortiz wrote:
> Hi Randy,
>
> On Wed, Sep 15, 2010 at 10:41:05AM -0700, Randy Dunlap wrote:
>> I don't know what the __crc_* symbols are (I can't find them anywhere).
>>
>> The other functions (pwm_config, pwm_free, pwm_request, pwm_disable,
>> pwm_enable) exist in multiple places. This is not good.
>> They are very generically named. The instances of these that are
>> provided by platform code are OK (these):
>>
>> ./include/linux/pwm.h:19:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
>> ./arch/arm/plat-pxa/pwm.c:64:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>> ./arch/arm/plat-pxa/pwm.c:101:EXPORT_SYMBOL(pwm_config);
>> ./arch/arm/plat-samsung/pwm.c:194:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>> ./arch/arm/plat-samsung/pwm.c:281:EXPORT_SYMBOL(pwm_config);
>> ./arch/arm/plat-mxc/pwm.c:55:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>> ./arch/arm/plat-mxc/pwm.c:114:EXPORT_SYMBOL(pwm_config);
>> ./arch/mips/jz4740/pwm.c:94:int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>>
>> but the instances of these that are defined in
>> drivers/mfd/twl6030-pwm.c should not be named so generically.
>>
>> Changing (Fixing) the function names in twl6030-pwm.c should fix the build problem
>> that you reported, I think.
> The pwm "API" relies on someone providing the pwm_* symbols, and then you have
> leds_pwm or backlight_pwm calling those symbols out of the blue.
> If we change the twl6030-pwm.c function names, no pwm users (backlight, led or
> input) will actually be able to use the twl6030 PWM driver.
> The pwm API is very poorly designed in my opinion and should provide a way for
> pwm drivers to register against it. pwm users will then call into the pwm
> framework who would select which driver to use.
So twl6030-pwm is a provider, just like some of the arch/ providers?
OK, I couldn't see that.
>> And it will still allow the ab8500 driver to be
>> built on other platforms, which is what we prefer when that is possible.
> I agree with that, but couldnt see any other fix with the current pwm
> situation.
OK, thanks. Do whatever you have to do.
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-15 18:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-15 11:23 [PATCH] mfd: ab8500: update kconfig for ab8500 core driver Arun Murthy
2010-09-15 15:20 ` Randy Dunlap
2010-09-15 17:12 ` Arun MURTHY
2010-09-15 17:41 ` Randy Dunlap
2010-09-15 18:21 ` Samuel Ortiz
2010-09-15 18:25 ` Randy Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).