* [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).