Patchwork pwm: Get rid of HAVE_PWM

login
register
mail settings
Submitter Thierry Reding
Date Oct. 4, 2012, 6:06 a.m.
Message ID <1349330819-11924-1-git-send-email-thierry.reding@avionic-design.de>
Download mbox | patch
Permalink /patch/189051/
State New
Headers show

Comments

Thierry Reding - Oct. 4, 2012, 6:06 a.m.
Now that all drivers have been moved to the PWM subsystem, remove the
legacy HAVE_PWM symbol and replace it with the new PWM symbol. While at
it, select the PWM subsystem and corresponding PWM driver on boards that
require PWM functionality.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Bryan Wu <bryan.wu@canonical.com>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Ashish Jangam <ashish.jangam@kpitcummins.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-input@vger.kernel.org
Cc: linux-leds@vger.kernel.org
---
 arch/arm/Kconfig           |  6 ++----
 arch/arm/mach-mxs/Kconfig  |  6 ++++--
 arch/arm/mach-pxa/Kconfig  | 42 ++++++++++++++++++++++++++++--------------
 arch/mips/Kconfig          |  3 ++-
 drivers/input/misc/Kconfig |  4 ++--
 drivers/leds/Kconfig       |  2 +-
 include/linux/pwm.h        |  2 +-
 7 files changed, 40 insertions(+), 25 deletions(-)
Lars-Peter Clausen - Oct. 4, 2012, 3 p.m.
On 10/04/2012 08:06 AM, Thierry Reding wrote:
> [...]
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 331d574..b38f23d 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -219,7 +219,8 @@ config MACH_JZ4740
>  	select GENERIC_GPIO
>  	select ARCH_REQUIRE_GPIOLIB
>  	select SYS_HAS_EARLY_PRINTK
> -	select HAVE_PWM
> +	select PWM
> +	select PWM_JZ4740
>  	select HAVE_CLK
>  	select GENERIC_IRQ_CHIP

I'm not sure if this is such a good idea, not all jz4740 based board
necessarily require PWM.

- Lars
Thierry Reding - Oct. 4, 2012, 6:29 p.m.
On Thu, Oct 04, 2012 at 05:00:23PM +0200, Lars-Peter Clausen wrote:
> On 10/04/2012 08:06 AM, Thierry Reding wrote:
> > [...]
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index 331d574..b38f23d 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -219,7 +219,8 @@ config MACH_JZ4740
> >  	select GENERIC_GPIO
> >  	select ARCH_REQUIRE_GPIOLIB
> >  	select SYS_HAS_EARLY_PRINTK
> > -	select HAVE_PWM
> > +	select PWM
> > +	select PWM_JZ4740
> >  	select HAVE_CLK
> >  	select GENERIC_IRQ_CHIP
> 
> I'm not sure if this is such a good idea, not all jz4740 based board
> necessarily require PWM.

This really only restores previous behaviour. But I agree that this is
potentially not what we want. Maybe we should just not select this for
any boards but rather leave it up to some default configuration. If so
the patch can be made simpler by just removing the HAVE_PWM entries.

Thierry
Lars-Peter Clausen - Oct. 4, 2012, 6:48 p.m.
On 10/04/2012 08:29 PM, Thierry Reding wrote:
> On Thu, Oct 04, 2012 at 05:00:23PM +0200, Lars-Peter Clausen wrote:
>> On 10/04/2012 08:06 AM, Thierry Reding wrote:
>>> [...]
>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>> index 331d574..b38f23d 100644
>>> --- a/arch/mips/Kconfig
>>> +++ b/arch/mips/Kconfig
>>> @@ -219,7 +219,8 @@ config MACH_JZ4740
>>>  	select GENERIC_GPIO
>>>  	select ARCH_REQUIRE_GPIOLIB
>>>  	select SYS_HAS_EARLY_PRINTK
>>> -	select HAVE_PWM
>>> +	select PWM
>>> +	select PWM_JZ4740
>>>  	select HAVE_CLK
>>>  	select GENERIC_IRQ_CHIP
>>
>> I'm not sure if this is such a good idea, not all jz4740 based board
>> necessarily require PWM.
> 
> This really only restores previous behaviour. But I agree that this is
> potentially not what we want. Maybe we should just not select this for
> any boards but rather leave it up to some default configuration. If so
> the patch can be made simpler by just removing the HAVE_PWM entries.

At least for JZ4740 I think that is the better solution.

Thanks,
- Lars
Thierry Reding - Oct. 4, 2012, 7:13 p.m.
On Thu, Oct 04, 2012 at 08:48:54PM +0200, Lars-Peter Clausen wrote:
> On 10/04/2012 08:29 PM, Thierry Reding wrote:
> > On Thu, Oct 04, 2012 at 05:00:23PM +0200, Lars-Peter Clausen wrote:
> >> On 10/04/2012 08:06 AM, Thierry Reding wrote:
> >>> [...]
> >>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> >>> index 331d574..b38f23d 100644
> >>> --- a/arch/mips/Kconfig
> >>> +++ b/arch/mips/Kconfig
> >>> @@ -219,7 +219,8 @@ config MACH_JZ4740
> >>>  	select GENERIC_GPIO
> >>>  	select ARCH_REQUIRE_GPIOLIB
> >>>  	select SYS_HAS_EARLY_PRINTK
> >>> -	select HAVE_PWM
> >>> +	select PWM
> >>> +	select PWM_JZ4740
> >>>  	select HAVE_CLK
> >>>  	select GENERIC_IRQ_CHIP
> >>
> >> I'm not sure if this is such a good idea, not all jz4740 based board
> >> necessarily require PWM.
> > 
> > This really only restores previous behaviour. But I agree that this is
> > potentially not what we want. Maybe we should just not select this for
> > any boards but rather leave it up to some default configuration. If so
> > the patch can be made simpler by just removing the HAVE_PWM entries.
> 
> At least for JZ4740 I think that is the better solution.

Okay, I'll give other people the chance to comment on this. Especially
for PXA there are many boards that use the PWM for backlight and it
would be interesting to hear from them how they want this to be handled.

Thierry
Shawn Guo - Oct. 7, 2012, 4:17 a.m.
On Thu, Oct 04, 2012 at 08:06:59AM +0200, Thierry Reding wrote:
> Now that all drivers have been moved to the PWM subsystem, remove the
> legacy HAVE_PWM symbol and replace it with the new PWM symbol. While at
> it, select the PWM subsystem and corresponding PWM driver on boards that
> require PWM functionality.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Eric Miao <eric.y.miao@gmail.com>
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Bryan Wu <bryan.wu@canonical.com>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Ashish Jangam <ashish.jangam@kpitcummins.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: linux-input@vger.kernel.org
> Cc: linux-leds@vger.kernel.org
> ---
>  arch/arm/Kconfig           |  6 ++----
>  arch/arm/mach-mxs/Kconfig  |  6 ++++--

Shawn Guo <shawn.guo@linaro.org>

>  arch/arm/mach-pxa/Kconfig  | 42 ++++++++++++++++++++++++++++--------------
>  arch/mips/Kconfig          |  3 ++-
>  drivers/input/misc/Kconfig |  4 ++--
>  drivers/leds/Kconfig       |  2 +-
>  include/linux/pwm.h        |  2 +-
>  7 files changed, 40 insertions(+), 25 deletions(-)
Eric Miao - Oct. 8, 2012, 2:45 a.m.
On Fri, Oct 5, 2012 at 3:13 AM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Thu, Oct 04, 2012 at 08:48:54PM +0200, Lars-Peter Clausen wrote:
>> On 10/04/2012 08:29 PM, Thierry Reding wrote:
>> > On Thu, Oct 04, 2012 at 05:00:23PM +0200, Lars-Peter Clausen wrote:
>> >> On 10/04/2012 08:06 AM, Thierry Reding wrote:
>> >>> [...]
>> >>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> >>> index 331d574..b38f23d 100644
>> >>> --- a/arch/mips/Kconfig
>> >>> +++ b/arch/mips/Kconfig
>> >>> @@ -219,7 +219,8 @@ config MACH_JZ4740
>> >>>   select GENERIC_GPIO
>> >>>   select ARCH_REQUIRE_GPIOLIB
>> >>>   select SYS_HAS_EARLY_PRINTK
>> >>> - select HAVE_PWM
>> >>> + select PWM
>> >>> + select PWM_JZ4740
>> >>>   select HAVE_CLK
>> >>>   select GENERIC_IRQ_CHIP
>> >>
>> >> I'm not sure if this is such a good idea, not all jz4740 based board
>> >> necessarily require PWM.
>> >
>> > This really only restores previous behaviour. But I agree that this is
>> > potentially not what we want. Maybe we should just not select this for
>> > any boards but rather leave it up to some default configuration. If so
>> > the patch can be made simpler by just removing the HAVE_PWM entries.
>>
>> At least for JZ4740 I think that is the better solution.
>
> Okay, I'll give other people the chance to comment on this. Especially
> for PXA there are many boards that use the PWM for backlight and it
> would be interesting to hear from them how they want this to be handled.

I'd say the original intention to introduce HAVE_PWM (although PXA
makes a lot use of this, but I remember Russell was the first to propose
this), is for board config to indicate its potential usage of the PWM or
rather if PWM is available for use on the board, and then leave the
developer another config option to build or not-to-build.

Personally I think removing HAVE_PWM will simplify the code a bit
and that's a good thing, on the other hand, how to keep a certain level
of flexibility mentioned above remains a small question. I guess with
device tree, this can be mitigated a bit.
Thierry Reding - Oct. 8, 2012, 5:45 a.m.
On Mon, Oct 08, 2012 at 10:45:44AM +0800, Eric Miao wrote:
> On Fri, Oct 5, 2012 at 3:13 AM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > On Thu, Oct 04, 2012 at 08:48:54PM +0200, Lars-Peter Clausen wrote:
> >> On 10/04/2012 08:29 PM, Thierry Reding wrote:
> >> > On Thu, Oct 04, 2012 at 05:00:23PM +0200, Lars-Peter Clausen wrote:
> >> >> On 10/04/2012 08:06 AM, Thierry Reding wrote:
> >> >>> [...]
> >> >>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> >> >>> index 331d574..b38f23d 100644
> >> >>> --- a/arch/mips/Kconfig
> >> >>> +++ b/arch/mips/Kconfig
> >> >>> @@ -219,7 +219,8 @@ config MACH_JZ4740
> >> >>>   select GENERIC_GPIO
> >> >>>   select ARCH_REQUIRE_GPIOLIB
> >> >>>   select SYS_HAS_EARLY_PRINTK
> >> >>> - select HAVE_PWM
> >> >>> + select PWM
> >> >>> + select PWM_JZ4740
> >> >>>   select HAVE_CLK
> >> >>>   select GENERIC_IRQ_CHIP
> >> >>
> >> >> I'm not sure if this is such a good idea, not all jz4740 based board
> >> >> necessarily require PWM.
> >> >
> >> > This really only restores previous behaviour. But I agree that this is
> >> > potentially not what we want. Maybe we should just not select this for
> >> > any boards but rather leave it up to some default configuration. If so
> >> > the patch can be made simpler by just removing the HAVE_PWM entries.
> >>
> >> At least for JZ4740 I think that is the better solution.
> >
> > Okay, I'll give other people the chance to comment on this. Especially
> > for PXA there are many boards that use the PWM for backlight and it
> > would be interesting to hear from them how they want this to be handled.
> 
> I'd say the original intention to introduce HAVE_PWM (although PXA
> makes a lot use of this, but I remember Russell was the first to propose
> this), is for board config to indicate its potential usage of the PWM or
> rather if PWM is available for use on the board, and then leave the
> developer another config option to build or not-to-build.

But with HAVE_PWM there was never a different configuration option to
enable the PWM driver. All architectures that used the HAVE_PWM symbol
built the driver based on its selection. The meaning seemed more like
"the PWM API is available" rather than anything else.

Given that, HAVE_PWM would now be replaced by just PWM since that makes
the API available. Client drivers would still be able to compile
properly but fail at runtime if no driver actually provided any PWM
devices.

Note that starting with 3.7 the API will even be available in the form
of dummy implementations if PWM is not selected.

> Personally I think removing HAVE_PWM will simplify the code a bit
> and that's a good thing, on the other hand, how to keep a certain level
> of flexibility mentioned above remains a small question. I guess with
> device tree, this can be mitigated a bit.

I think, given the above, that the best solution would be to not select
any PWM-related symbols from boards and handle this via the board
default configurations. I was just worrying that we were changing
existing behaviour. On the other hand behaviour was already changed by
making the drivers no longer conditional on HAVE_PWM.

If nobody has any objections, my tendency would be to just not replace
HAVE_PWM but just get rid of it. Of course, as I mentioned above, it
would require the board default configurations to be updated and select
the PWM and corresponding driver symbol in order to restore previous
behaviour.

Thierry

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e91c7cd..a0cebf8 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -69,9 +69,6 @@  config ARM_DMA_USE_IOMMU
 	select ARM_HAS_SG_CHAIN
 	bool
 
-config HAVE_PWM
-	bool
-
 config MIGHT_HAVE_PCI
 	bool
 
@@ -610,7 +607,8 @@  config ARCH_LPC32XX
 	select CLKDEV_LOOKUP
 	select GENERIC_CLOCKEVENTS
 	select USE_OF
-	select HAVE_PWM
+	select PWM
+	select PWM_LPC32XX
 	help
 	  Support for the NXP LPC32XX family of processors
 
diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
index 9a8bbda..2fc01bd 100644
--- a/arch/arm/mach-mxs/Kconfig
+++ b/arch/arm/mach-mxs/Kconfig
@@ -6,7 +6,8 @@  config SOC_IMX23
 	bool
 	select ARM_AMBA
 	select CPU_ARM926T
-	select HAVE_PWM
+	select PWM
+	select PWM_MXS
 	select PINCTRL_IMX23
 
 config SOC_IMX28
@@ -14,7 +15,8 @@  config SOC_IMX28
 	select ARM_AMBA
 	select CPU_ARM926T
 	select HAVE_CAN_FLEXCAN if CAN
-	select HAVE_PWM
+	select PWM
+	select PWM_MXS
 	select PINCTRL_IMX28
 
 comment "MXS platforms:"
diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
index fe2d1f8..14f41c3 100644
--- a/arch/arm/mach-pxa/Kconfig
+++ b/arch/arm/mach-pxa/Kconfig
@@ -33,12 +33,14 @@  config ARCH_LUBBOCK
 config MACH_MAINSTONE
 	bool "Intel HCDDBBVA0 Development Platform (aka Mainstone)"
 	select PXA27x
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_ZYLONITE
 	bool
 	select PXA3xx
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_ZYLONITE300
 	bool "PXA3xx Development Platform (aka Zylonite) PXA300/310"
@@ -78,7 +80,8 @@  config ARCH_VIPER
 	select PXA25x
 	select ISA
 	select I2C_GPIO
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 	select PXA_HAVE_ISA_IRQS
 	select ARCOM_PCMCIA
 
@@ -128,7 +131,8 @@  config MACH_CM_X300
 	select PXA3xx
 	select CPU_PXA300
 	select CPU_PXA310
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_CAPC7117
 	bool "Embedian CAPC-7117 evaluation kit based on the MXM-8x10 CoM"
@@ -220,7 +224,8 @@  config TRIZEPS_PCMCIA
 config MACH_LOGICPD_PXA270
 	bool "LogicPD PXA270 Card Engine Development Platform"
 	select PXA27x
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_PCM027
 	bool "Phytec phyCORE-PXA270 CPU module (PCM-027)"
@@ -229,7 +234,8 @@  config MACH_PCM027
 
 config MACH_PCM990_BASEBOARD
 	bool "PHYTEC PCM-990 development board"
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 	depends on MACH_PCM027
 
 choice
@@ -255,7 +261,8 @@  config MACH_COLIBRI_PXA270_INCOME
 	bool "Income s.r.o. PXA270 SBC"
 	depends on MACH_COLIBRI
 	select PXA27x
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_COLIBRI300
 	bool "Toradex Colibri PXA300/310"
@@ -285,7 +292,8 @@  config MACH_H4700
 	bool "HP iPAQ hx4700"
 	select PXA27x
 	select IWMMXT
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_H5000
 	bool "HP iPAQ h5000"
@@ -299,13 +307,15 @@  config MACH_MAGICIAN
 	bool "Enable HTC Magician Support"
 	select PXA27x
 	select IWMMXT
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_MIOA701
 	bool "Mitac Mio A701 Support"
 	select PXA27x
 	select IWMMXT
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 	select GPIO_SYSFS
 	help
 	  Say Y here if you intend to run this kernel on a
@@ -316,7 +326,8 @@  config PXA_EZX
 	bool "Motorola EZX Platform"
 	select PXA27x
 	select IWMMXT
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_EZX_A780
 	bool "Motorola EZX A780"
@@ -354,7 +365,8 @@  config MACH_MP900C
 
 config ARCH_PXA_PALM
 	bool "PXA based Palm PDAs"
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_PALM27X
 	bool
@@ -454,7 +466,8 @@  config MACH_RAUMFELD_RC
 	select PXA3xx
 	select CPU_PXA300
 	select POWER_SUPPLY
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 
 config MACH_RAUMFELD_CONNECTOR
 	bool "Raumfeld Connector"
@@ -617,7 +630,8 @@  config MACH_E800
 config MACH_ZIPIT2
 	bool "Zipit Z2 Handheld"
 	select PXA27x
-	select HAVE_PWM
+	select PWM
+	select PWM_PXA
 endif
 endmenu
 
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 331d574..b38f23d 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -219,7 +219,8 @@  config MACH_JZ4740
 	select GENERIC_GPIO
 	select ARCH_REQUIRE_GPIOLIB
 	select SYS_HAS_EARLY_PRINTK
-	select HAVE_PWM
+	select PWM
+	select PWM_JZ4740
 	select HAVE_CLK
 	select GENERIC_IRQ_CHIP
 
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 7c0f1ec..af6188b 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -146,7 +146,7 @@  config INPUT_MAX8925_ONKEY
 
 config INPUT_MAX8997_HAPTIC
 	tristate "MAXIM MAX8997 haptic controller support"
-	depends on HAVE_PWM && MFD_MAX8997
+	depends on PWM && MFD_MAX8997
 	select INPUT_FF_MEMLESS
 	help
 	  This option enables device driver support for the haptic controller
@@ -444,7 +444,7 @@  config INPUT_PCF8574
 
 config INPUT_PWM_BEEPER
 	tristate "PWM beeper support"
-	depends on HAVE_PWM
+	depends on PWM
 	help
 	  Say Y here to get support for PWM based beeper devices.
 
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index c96bbaa..a720d99 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -298,7 +298,7 @@  config LEDS_DAC124S085
 config LEDS_PWM
 	tristate "PWM driven LED Support"
 	depends on LEDS_CLASS
-	depends on HAVE_PWM
+	depends on PWM
 	help
 	  This option enables support for pwm driven LEDs
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 112b314..eea3f26 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -7,7 +7,7 @@ 
 struct pwm_device;
 struct seq_file;
 
-#if IS_ENABLED(CONFIG_PWM) || IS_ENABLED(CONFIG_HAVE_PWM)
+#if IS_ENABLED(CONFIG_PWM)
 /*
  * pwm_request - request a PWM device
  */