| Message ID | 1391518634-6472-1-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Feb 5, 2014 at 6:01 AM, Jingoo Han <jg1.han@samsung.com> wrote: > On Tuesday, February 04, 2014 9:57 PM, Linus Walleij wrote: >> >> In some compilations the LM3630A and LP855X backlight drivers >> fail like this: >> >> drivers/built-in.o: In function `lm3630a_pwm_ctrl': >> drivers/video/backlight/lm3630a_bl.c:168: undefined reference to `pwm_config' >> drivers/video/backlight/lm3630a_bl.c:172: undefined reference to `pwm_disable' >> drivers/video/backlight/lm3630a_bl.c:170: undefined reference to `pwm_enable' >> drivers/built-in.o: In function `lp855x_pwm_ctrl': >> drivers/video/backlight/lp855x_bl.c:249: undefined reference to `pwm_config' >> drivers/video/backlight/lp855x_bl.c:253: undefined reference to `pwm_disable' >> drivers/video/backlight/lp855x_bl.c:251: undefined reference to `pwm_enable' >> >> This is because both drivers depend on the PWM framework, so >> add this dependency to their Kconfig entries. > > However, even though, when CONFIG_PWM is not enabled, the problem > should not happen. pwm_config(),pwm_disable(), and pwm_enable() > are already defined for CONFIG_PWM=n case as below. So you may think but it does happen :-) I reproduced this with the defconfig for ARM pxa255-idp and enabling all boards for that platform, then enabling all available backlight drivers as compiled-in objects (y). > ./include/linux/pwm.h > #if IS_ENABLED(CONFIG_PWM) || IS_ENABLED(CONFIG_HAVE_PWM) > ..... > #else Hm PXA that I am using defines CONFIG_HAVE_PWM, but doesn't provide the required signatures (pwm_config/pwm_disable/pwm_enable). One of two things is wrong: - Either the PXA platform is breaking the CONFIG_HAVE_PWM contract by not providing pwm_config/pwm_disable/pwm_enable functions. Then HAVE_PWM should be removed from the PXA Kconfig selects. Or: - There is no such contract that these functions must exist if CONFIG_HAVE_PWM is defined, and the #if IS_ENABLED(CONFIG_HAVE_PWM) should be removed from <linux/pwm.h> Does anyone know which one it is? PWM subsystem maintainer? :-) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Thu, Feb 6, 2014 at 8:23 AM, Jingoo Han <jg1.han@samsung.com> wrote: > In the case of "CONFIG_HAVE_PWM=y && CONFIG_PWM=n", it makes > the problem. > > The HAVE_PWM symbol is only for legacy platforms that provide > the PWM API without using the generic framework. PXA looks to > use the generic PWM framework. Then, how about removing > "select HAVE_PWM" from PXA as below? > > --- a/arch/arm/mach-pxa/Kconfig > +++ b/arch/arm/mach-pxa/Kconfig > @@ -7,7 +7,6 @@ comment "Intel/Marvell Dev Platforms (sorted by hardware release time)" > config MACH_PXA3XX_DT > bool "Support PXA3xx platforms from device tree" > select CPU_PXA300 > - select HAVE_PWM > select POWER_SUPPLY > select PXA3xx > select USE_OF > @@ -23,12 +22,10 @@ config ARCH_LUBBOCK > > config MACH_MAINSTONE > bool "Intel HCDDBBVA0 Development Platform (aka Mainstone)" > - select HAVE_PWM > select PXA27x > > config MACH_ZYLONITE > bool > - select HAVE_PWM > select PXA3xx Looks like the right solution to me. Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, Feb 10, 2014 at 11:40 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > it seems like at least BACKLIGHT_LP8788 is missing a corresponding > dependency as well. > > I have applied Sascha's patch to remove the obsolete HAVE_PWM symbol, > and this will fix at least the build issues. However it will also cause > the driver to fail at runtime because the pwm_*() functions won't work. So it definately needs that API, not just stubs. But isn't it proper for Kconfig to allow you to break things like that by configuring out stuff and have stubs come in? I'm a bit torn here. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index 5a3eb2ecb525..0604c3348761 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -371,6 +371,7 @@ config BACKLIGHT_AAT2870 config BACKLIGHT_LM3630A tristate "Backlight Driver for LM3630A" depends on BACKLIGHT_CLASS_DEVICE && I2C + depends on PWM select REGMAP_I2C help This supports TI LM3630A Backlight Driver @@ -387,6 +388,7 @@ config BACKLIGHT_LM3639 config BACKLIGHT_LP855X tristate "Backlight driver for TI LP855X" depends on BACKLIGHT_CLASS_DEVICE && I2C + depends on PWM help This supports TI LP8550, LP8551, LP8552, LP8553, LP8555, LP8556 and LP8557 backlight driver.
In some compilations the LM3630A and LP855X backlight drivers fail like this: drivers/built-in.o: In function `lm3630a_pwm_ctrl': drivers/video/backlight/lm3630a_bl.c:168: undefined reference to `pwm_config' drivers/video/backlight/lm3630a_bl.c:172: undefined reference to `pwm_disable' drivers/video/backlight/lm3630a_bl.c:170: undefined reference to `pwm_enable' drivers/built-in.o: In function `lp855x_pwm_ctrl': drivers/video/backlight/lp855x_bl.c:249: undefined reference to `pwm_config' drivers/video/backlight/lp855x_bl.c:253: undefined reference to `pwm_disable' drivers/video/backlight/lp855x_bl.c:251: undefined reference to `pwm_enable' This is because both drivers depend on the PWM framework, so add this dependency to their Kconfig entries. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/video/backlight/Kconfig | 2 ++ 1 file changed, 2 insertions(+)