[PATCHv2 0/7] PWM core driver for pwm based led and backlight driver

[PATCHv2 0/7] PWM core driver for pwm based led and backlight driver

[Arun Murthy]

The series of patch add a new pwm core driver.
Align the existing pwm drivers to make use of the pwm core driver.

Arun Murthy (7):
  pwm: Add pwm core driver
  backlight:pwm: add an element 'name' to platform data
  leds: pwm: add a new element 'name' to platform data
  pwm: Align existing pwm drivers with pwm-core driver
  platform: Update the pwm based led and backlight platform data
  pwm: move existing pwm driver to drivers/pwm
  pwm: Modify backlight and led Kconfig aligning to pwm core

 arch/arm/mach-pxa/cm-x300.c               |    1 +
 arch/arm/mach-pxa/colibri-pxa270-income.c |    1 +
 arch/arm/mach-pxa/ezx.c                   |    1 +
 arch/arm/mach-pxa/hx4700.c                |    1 +
 arch/arm/mach-pxa/lpd270.c                |    1 +
 arch/arm/mach-pxa/magician.c              |    1 +
 arch/arm/mach-pxa/mainstone.c             |    1 +
 arch/arm/mach-pxa/mioa701.c               |    1 +
 arch/arm/mach-pxa/palm27x.c               |    1 +
 arch/arm/mach-pxa/palmtc.c                |    1 +
 arch/arm/mach-pxa/palmte2.c               |    1 +
 arch/arm/mach-pxa/pcm990-baseboard.c      |    1 +
 arch/arm/mach-pxa/raumfeld.c              |    1 +
 arch/arm/mach-pxa/tavorevb.c              |    2 +
 arch/arm/mach-pxa/viper.c                 |    1 +
 arch/arm/mach-pxa/z2.c                    |    2 +
 arch/arm/mach-pxa/zylonite.c              |    1 +
 arch/arm/mach-s3c2410/mach-h1940.c        |    1 +
 arch/arm/mach-s3c2440/mach-rx1950.c       |    1 +
 arch/arm/mach-s3c64xx/mach-hmt.c          |    1 +
 arch/arm/mach-s3c64xx/mach-smartq.c       |    1 +
 arch/arm/plat-mxc/pwm.c                   |  166 +++++++++------------
 arch/arm/plat-pxa/pwm.c                   |  210 ++++++++++++--------------
 arch/arm/plat-samsung/pwm.c               |  235 +++++++++++++----------------
 arch/mips/jz4740/pwm.c                    |    2 +-
 drivers/Kconfig                           |    2 +
 drivers/Makefile                          |    1 +
 drivers/leds/Kconfig                      |    2 +-
 drivers/leds/leds-pwm.c                   |    4 +-
 drivers/mfd/Kconfig                       |    9 -
 drivers/mfd/Makefile                      |    1 -
 drivers/mfd/twl-core.c                    |   13 ++
 drivers/mfd/twl6030-pwm.c                 |  163 --------------------
 drivers/misc/Kconfig                      |    9 -
 drivers/misc/Makefile                     |    1 -
 drivers/misc/ab8500-pwm.c                 |  168 --------------------
 drivers/pwm/Kconfig                       |   33 ++++
 drivers/pwm/Makefile                      |    4 +
 drivers/pwm/pwm-ab8500.c                  |  157 +++++++++++++++++++
 drivers/pwm/pwm-core.c                    |  124 +++++++++++++++
 drivers/pwm/pwm-twl6040.c                 |  196 ++++++++++++++++++++++++
 drivers/video/backlight/Kconfig           |    2 +-
 drivers/video/backlight/pwm_bl.c          |    4 +-
 include/linux/leds_pwm.h                  |    1 +
 include/linux/pwm.h                       |   29 ++++-
 include/linux/pwm_backlight.h             |    1 +
 46 files changed, 864 insertions(+), 696 deletions(-)
 delete mode 100644 drivers/mfd/twl6030-pwm.c
 delete mode 100644 drivers/misc/ab8500-pwm.c
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/pwm-ab8500.c
 create mode 100644 drivers/pwm/pwm-core.c
 create mode 100644 drivers/pwm/pwm-twl6040.c

-- 
1.7.2.dirty

[PATCH 1/7] pwm: Add pwm core driver

[Arun Murthy]

The existing pwm based led and backlight driver makes use of the
pwm(include/linux/pwm.h). So all the board specific pwm drivers will
be exposing the same set of function name as in include/linux/pwm.h.
As a result build fails.

In order to overcome this issue all the pwm drivers must register to
some core pwm driver with function pointers for pwm operations (i.e
pwm_config, pwm_enable, pwm_disable).

The clients of pwm device will have to call pwm_request, wherein
they will get the pointer to struct pwm_ops. This structure include
function pointers for pwm_config, pwm_enable and pwm_disable.

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/Kconfig        |    2 +
 drivers/Makefile       |    1 +
 drivers/pwm/Kconfig    |   16 ++++++
 drivers/pwm/Makefile   |    1 +
 drivers/pwm/pwm-core.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h    |   12 ++++-
 6 files changed, 160 insertions(+), 1 deletions(-)
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/pwm-core.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index a2b902f..e042f27 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -111,4 +111,6 @@ source "drivers/xen/Kconfig"
 source "drivers/staging/Kconfig"
 
 source "drivers/platform/Kconfig"
+
+source "drivers/pwm/Kconfig"
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 4ca727d..0061ec4 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -116,3 +116,4 @@ obj-$(CONFIG_STAGING)		+= staging/
 obj-y				+= platform/
 obj-y				+= ieee802154/
 obj-y				+= vbus/
+obj-$(CONFIG_PWM_DEVICES)	+= pwm/
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
new file mode 100644
index 0000000..5d10106
--- /dev/null
+++ b/drivers/pwm/Kconfig
@@ -0,0 +1,16 @@
+#
+# PWM devices
+#
+
+menuconfig PWM_DEVICES
+	bool "PWM devices"
+	default y
+	---help---
+	  Say Y here to get to see options for device drivers from various
+	  different categories. This option alone does not add any kernel code.
+
+	  If you say N, all options in this submenu will be skipped and disabled.
+
+if PWM_DEVICES
+
+endif # PWM_DEVICES
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
new file mode 100644
index 0000000..552f969
--- /dev/null
+++ b/drivers/pwm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PWM_DEVICES)	+= pwm-core.o
diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
new file mode 100644
index 0000000..b84027a
--- /dev/null
+++ b/drivers/pwm/pwm-core.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) ST-Ericsson SA 2010
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Arun R Murthy <arun.murthy@stericsson.com>
+ */
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/rwsem.h>
+#include <linux/err.h>
+#include <linux/pwm.h>
+
+struct pwm_device {
+	struct pwm_ops *pops;
+	int pwm_id;
+};
+
+struct pwm_dev_info {
+	struct pwm_device *pwm_dev;
+	struct list_head list;
+};
+static struct pwm_dev_info *di;
+
+DECLARE_RWSEM(pwm_list_lock);
+
+void __deprecated pwm_free(struct pwm_device *pwm)
+{
+}
+
+int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+{
+	return pwm->pops->pwm_config(pwm, duty_ns, period_ns);
+}
+EXPORT_SYMBOL(pwm_config);
+
+int pwm_enable(struct pwm_device *pwm)
+{
+	return pwm->pops->pwm_enable(pwm);
+}
+EXPORT_SYMBOL(pwm_enable);
+
+void pwm_disable(struct pwm_device *pwm)
+{
+	pwm->pops->pwm_disable(pwm);
+}
+EXPORT_SYMBOL(pwm_disable);
+
+int pwm_device_register(struct pwm_device *pwm_dev)
+{
+	struct pwm_dev_info *pwm;
+
+	down_write(&pwm_list_lock);
+	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
+	if (!pwm) {
+		up_write(&pwm_list_lock);
+		return -ENOMEM;
+	}
+	pwm->pwm_dev = pwm_dev;
+	list_add_tail(&pwm->list, &di->list);
+	up_write(&pwm_list_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(pwm_device_register);
+
+int pwm_device_unregister(struct pwm_device *pwm_dev)
+{
+	struct pwm_dev_info *tmp;
+	struct list_head *pos, *tmp_lst;
+
+	down_write(&pwm_list_lock);
+	list_for_each_safe(pos, tmp_lst, &di->list) {
+		tmp = list_entry(pos, struct pwm_dev_info, list);
+		if (tmp->pwm_dev == pwm_dev) {
+			list_del(pos);
+			kfree(tmp);
+			up_write(&pwm_list_lock);
+			return 0;
+		}
+	}
+	up_write(&pwm_list_lock);
+	return -ENOENT;
+}
+EXPORT_SYMBOL(pwm_device_unregister);
+
+struct pwm_device *pwm_request(int pwm_id, const char *name)
+{
+	struct pwm_dev_info *pwm;
+	struct list_head *pos;
+
+	down_read(&pwm_list_lock);
+	list_for_each(pos, &di->list) {
+		pwm = list_entry(pos, struct pwm_dev_info, list);
+		if ((!strcmp(pwm->pwm_dev->pops->name, name)) &&
+				(pwm->pwm_dev->pwm_id == pwm_id)) {
+			up_read(&pwm_list_lock);
+			return pwm->pwm_dev;
+		}
+	}
+	up_read(&pwm_list_lock);
+	return ERR_PTR(-ENOENT);
+}
+EXPORT_SYMBOL(pwm_request);
+
+static int __init pwm_init(void)
+{
+	struct pwm_dev_info *pwm;
+
+	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
+	if (!pwm)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&pwm->list);
+	di = pwm;
+	return 0;
+}
+
+static void __exit pwm_exit(void)
+{
+	kfree(di);
+}
+
+subsys_initcall(pwm_init);
+module_exit(pwm_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Arun R Murthy");
+MODULE_ALIAS("core:pwm");
+MODULE_DESCRIPTION("Core pwm driver");
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 7c77575..6e7da1f 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -3,6 +3,13 @@
 
 struct pwm_device;
 
+struct pwm_ops {
+	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int period_ns);
+	int (*pwm_enable)(struct pwm_device *pwm);
+	int (*pwm_disable)(struct pwm_device *pwm);
+	char *name;
+};
+
 /*
  * pwm_request - request a PWM device
  */
@@ -11,7 +18,7 @@ struct pwm_device *pwm_request(int pwm_id, const char *label);
 /*
  * pwm_free - free a PWM device
  */
-void pwm_free(struct pwm_device *pwm);
+void __deprecated pwm_free(struct pwm_device *pwm);
 
 /*
  * pwm_config - change a PWM device configuration
@@ -28,4 +35,7 @@ int pwm_enable(struct pwm_device *pwm);
  */
 void pwm_disable(struct pwm_device *pwm);
 
+int pwm_device_register(struct pwm_device *pwm_dev);
+int pwm_device_unregister(struct pwm_device *pwm_dev);
+
 #endif /* __LINUX_PWM_H */
-- 
1.7.2.dirty

[PATCH 1/7] pwm: Add pwm core driver

[Vasily Khoruzhick]

On 28 of September 2010 10:40:42 Arun Murthy wrote:
> The existing pwm based led and backlight driver makes use of the
> pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> be exposing the same set of function name as in include/linux/pwm.h.
> As a result build fails.

Which build fails? One with multi-SoC support? Please be more specific.

> In order to overcome this issue all the pwm drivers must register to
> some core pwm driver with function pointers for pwm operations (i.e
> pwm_config, pwm_enable, pwm_disable).
> 
> The clients of pwm device will have to call pwm_request, wherein
> they will get the pointer to struct pwm_ops. This structure include
> function pointers for pwm_config, pwm_enable and pwm_disable.
> 
> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/Kconfig        |    2 +
>  drivers/Makefile       |    1 +
>  drivers/pwm/Kconfig    |   16 ++++++
>  drivers/pwm/Makefile   |    1 +
>  drivers/pwm/pwm-core.c |  129
> ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pwm.h    | 
>  12 ++++-
>  6 files changed, 160 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/pwm/Kconfig
>  create mode 100644 drivers/pwm/Makefile
>  create mode 100644 drivers/pwm/pwm-core.c
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a2b902f..e042f27 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -111,4 +111,6 @@ source "drivers/xen/Kconfig"
>  source "drivers/staging/Kconfig"
> 
>  source "drivers/platform/Kconfig"
> +
> +source "drivers/pwm/Kconfig"
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 4ca727d..0061ec4 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -116,3 +116,4 @@ obj-$(CONFIG_STAGING)		+= staging/
>  obj-y				+= platform/
>  obj-y				+= ieee802154/
>  obj-y				+= vbus/
> +obj-$(CONFIG_PWM_DEVICES)	+= pwm/
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> new file mode 100644
> index 0000000..5d10106
> --- /dev/null
> +++ b/drivers/pwm/Kconfig
> @@ -0,0 +1,16 @@
> +#
> +# PWM devices
> +#
> +
> +menuconfig PWM_DEVICES
> +	bool "PWM devices"
> +	default y
> +	---help---
> +	  Say Y here to get to see options for device drivers from various
> +	  different categories. This option alone does not add any kernel code.
> +
> +	  If you say N, all options in this submenu will be skipped and disabled.
> +
> +if PWM_DEVICES
> +
> +endif # PWM_DEVICES
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> new file mode 100644
> index 0000000..552f969
> --- /dev/null
> +++ b/drivers/pwm/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PWM_DEVICES)	+= pwm-core.o
> diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
> new file mode 100644
> index 0000000..b84027a
> --- /dev/null
> +++ b/drivers/pwm/pwm-core.c
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2010
> + *
> + * License Terms: GNU General Public License v2
> + * Author: Arun R Murthy <arun.murthy@stericsson.com>
> + */
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/rwsem.h>
> +#include <linux/err.h>
> +#include <linux/pwm.h>
> +
> +struct pwm_device {
> +	struct pwm_ops *pops;
> +	int pwm_id;
> +};
> +
> +struct pwm_dev_info {
> +	struct pwm_device *pwm_dev;
> +	struct list_head list;
> +};
> +static struct pwm_dev_info *di;
> +
> +DECLARE_RWSEM(pwm_list_lock);
> +
> +void __deprecated pwm_free(struct pwm_device *pwm)
> +{
> +}

Why pwm_free is marked __deprecated? What is its successor?

> +
> +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +{
> +	return pwm->pops->pwm_config(pwm, duty_ns, period_ns);
> +}
> +EXPORT_SYMBOL(pwm_config);
> +
> +int pwm_enable(struct pwm_device *pwm)
> +{
> +	return pwm->pops->pwm_enable(pwm);
> +}
> +EXPORT_SYMBOL(pwm_enable);
> +
> +void pwm_disable(struct pwm_device *pwm)
> +{
> +	pwm->pops->pwm_disable(pwm);
> +}
> +EXPORT_SYMBOL(pwm_disable);
> +
> +int pwm_device_register(struct pwm_device *pwm_dev)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	down_write(&pwm_list_lock);
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm) {
> +		up_write(&pwm_list_lock);
> +		return -ENOMEM;
> +	}
> +	pwm->pwm_dev = pwm_dev;
> +	list_add_tail(&pwm->list, &di->list);
> +	up_write(&pwm_list_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(pwm_device_register);
> +
> +int pwm_device_unregister(struct pwm_device *pwm_dev)
> +{
> +	struct pwm_dev_info *tmp;
> +	struct list_head *pos, *tmp_lst;
> +
> +	down_write(&pwm_list_lock);
> +	list_for_each_safe(pos, tmp_lst, &di->list) {
> +		tmp = list_entry(pos, struct pwm_dev_info, list);
> +		if (tmp->pwm_dev == pwm_dev) {
> +			list_del(pos);
> +			kfree(tmp);
> +			up_write(&pwm_list_lock);
> +			return 0;
> +		}
> +	}
> +	up_write(&pwm_list_lock);
> +	return -ENOENT;
> +}
> +EXPORT_SYMBOL(pwm_device_unregister);
> +
> +struct pwm_device *pwm_request(int pwm_id, const char *name)
> +{
> +	struct pwm_dev_info *pwm;
> +	struct list_head *pos;
> +
> +	down_read(&pwm_list_lock);
> +	list_for_each(pos, &di->list) {
> +		pwm = list_entry(pos, struct pwm_dev_info, list);
> +		if ((!strcmp(pwm->pwm_dev->pops->name, name)) &&
> +				(pwm->pwm_dev->pwm_id == pwm_id)) {
> +			up_read(&pwm_list_lock);
> +			return pwm->pwm_dev;
> +		}
> +	}
> +	up_read(&pwm_list_lock);
> +	return ERR_PTR(-ENOENT);
> +}
> +EXPORT_SYMBOL(pwm_request);
> +
> +static int __init pwm_init(void)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&pwm->list);
> +	di = pwm;
> +	return 0;
> +}
> +
> +static void __exit pwm_exit(void)
> +{
> +	kfree(di);
> +}
> +
> +subsys_initcall(pwm_init);
> +module_exit(pwm_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Arun R Murthy");
> +MODULE_ALIAS("core:pwm");
> +MODULE_DESCRIPTION("Core pwm driver");
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 7c77575..6e7da1f 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -3,6 +3,13 @@
> 
>  struct pwm_device;
> 
> +struct pwm_ops {
> +	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int period_ns);
> +	int (*pwm_enable)(struct pwm_device *pwm);
> +	int (*pwm_disable)(struct pwm_device *pwm);
> +	char *name;
> +};
> +
>  /*
>   * pwm_request - request a PWM device
>   */
> @@ -11,7 +18,7 @@ struct pwm_device *pwm_request(int pwm_id, const char
> *label); /*
>   * pwm_free - free a PWM device
>   */
> -void pwm_free(struct pwm_device *pwm);
> +void __deprecated pwm_free(struct pwm_device *pwm);
> 
>  /*
>   * pwm_config - change a PWM device configuration
> @@ -28,4 +35,7 @@ int pwm_enable(struct pwm_device *pwm);
>   */
>  void pwm_disable(struct pwm_device *pwm);
> 
> +int pwm_device_register(struct pwm_device *pwm_dev);
> +int pwm_device_unregister(struct pwm_device *pwm_dev);
> +
>  #endif /* __LINUX_PWM_H */

[PATCH 1/7] pwm: Add pwm core driver

[Vasily Khoruzhick]

On 28 of September 2010 11:38:26 Arun MURTHY wrote:

> > 
> > Why pwm_free is marked __deprecated? What is its successor?
> 
> This function is to be removed.

What should be used as replacement?

Regards
Vasily

[PATCH 1/7] pwm: Add pwm core driver

[Hemanth V]

>
>> On 28 of September 2010 10:40:42 Arun Murthy wrote:
>> > The existing pwm based led and backlight driver makes use of the
>> > pwm(include/linux/pwm.h). So all the board specific pwm drivers will
>> > be exposing the same set of function name as in include/linux/pwm.h.
>> > As a result build fails.
>>
>> Which build fails? One with multi-SoC support? Please be more specific.
> Sure will add this in v2.
>

Could you clarify for the benefit of all, which specific issues you are
trying to address with this patch series

[PATCH 1/7] pwm: Add pwm core driver

[Hemanth V]

>> >> On 28 of September 2010 10:40:42 Arun Murthy wrote:
>> >> > The existing pwm based led and backlight driver makes use of the
>> >> > pwm(include/linux/pwm.h). So all the board specific pwm drivers
>> will
>> >> > be exposing the same set of function name as in
>> include/linux/pwm.h.
>> >> > As a result build fails.
>> >>
>> >> Which build fails? One with multi-SoC support? Please be more
>> specific.
>> > Sure will add this in v2.
>> >
>>
>> Could you clarify for the benefit of all, which specific issues you are
>> trying to address with this patch series
> 1. Now since all the pwm driver export same set of function(pwm_enable, pwm_disable,..), if it happens that there are two pwm driver enabled this
> leads to re-declaration and results in build failure. The proper way to handle this would be to have a pwm core function, and let all the pwm
> drivers register to the pwm core driver.
> 2. The above scenario also occurs in place of multi-soc environment. Lets say OMAP has a pwm module and that is being used for primary lcd backlight
> and twl has a backlight that is being used for controlling the charging led brightness. In this case there exists 2 pwm drivers and one pwm driver
> will be used by pwm_bl.c and other by leds-pwm.c

Speaking specifically of OMAP4, twl6030 supports multiple PWMs i.e for display/keypad backlight, charging
led. But there should not be need for multiple drivers since twl6030-pwm should be able to support
all these (currently it doesnot though). So there would single pwm_enable, pwm_disable exported and driver
internally takes care configuring the correct PWM based on id. Would it not be similar
situation for other hardware also.

Thanks
Hemanth

[PATCH 1/7] pwm: Add pwm core driver

[Hemanth V]

>> >> >> On 28 of September 2010 10:40:42 Arun Murthy wrote:
>> >> >> > The existing pwm based led and backlight driver makes use of
>> the
>> >> >> > pwm(include/linux/pwm.h). So all the board specific pwm drivers
>> >> will
>> >> >> > be exposing the same set of function name as in
>> >> include/linux/pwm.h.
>> >> >> > As a result build fails.
>> >> >>
>> >> >> Which build fails? One with multi-SoC support? Please be more
>> >> specific.
>> >> > Sure will add this in v2.
>> >> >
>> >>
>> >> Could you clarify for the benefit of all, which specific issues you
>> are
>> >> trying to address with this patch series
>> > 1. Now since all the pwm driver export same set of
>> function(pwm_enable, pwm_disable,..), if it happens that there are two
>> pwm driver enabled this
>> > leads to re-declaration and results in build failure. The proper way
>> to handle this would be to have a pwm core function, and let all the
>> pwm
>> > drivers register to the pwm core driver.
>> > 2. The above scenario also occurs in place of multi-soc environment.
>> Lets say OMAP has a pwm module and that is being used for primary lcd
>> backlight
>> > and twl has a backlight that is being used for controlling the
>> charging led brightness. In this case there exists 2 pwm drivers and
>> one pwm driver
>> > will be used by pwm_bl.c and other by leds-pwm.c
>>
>> Speaking specifically of OMAP4, twl6030 supports multiple PWMs i.e for
>> display/keypad backlight, charging
>> led. But there should not be need for multiple drivers since twl6030-
>> pwm should be able to support
>> all these (currently it doesnot though). So there would single
>> pwm_enable, pwm_disable exported and driver
>> internally takes care configuring the correct PWM based on id. Would it
>> not be similar
>> situation for other hardware also.
>>
> You are right, there is only one pwm module in twl4030/twl6030 and this module might have any number or pwm's line PWM1, PWM2, PWM3 etc.
> My consideration is when you have two separate pwm modules on different soc. Its not in case of OMAP boards. But that was just an example that I
> gave.
>
> Let me be more specific, consider an environment where there is an APE and Power Management subsystem(separate IC but on same board/platform)
> APE has a pwm module and Power Management SubSystem also has pwm module. Both are part of the platform.
> Not there exists two drivers in a single platform and both of them are enabled. Building such a kernel results in re-declaration build error.
>

Is this something specific to ST chipsets or do you know of any other chips which might use this feature.

Thanks
Hemanth

[PATCH 1/7] pwm: Add pwm core driver

[Lars-Peter Clausen]

Arun Murthy wrote:
> The existing pwm based led and backlight driver makes use of the
> pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> be exposing the same set of function name as in include/linux/pwm.h.
> As a result build fails.
> 
> In order to overcome this issue all the pwm drivers must register to
> some core pwm driver with function pointers for pwm operations (i.e
> pwm_config, pwm_enable, pwm_disable).
> 
> The clients of pwm device will have to call pwm_request, wherein
> they will get the pointer to struct pwm_ops. This structure include
> function pointers for pwm_config, pwm_enable and pwm_disable.
> 
> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  drivers/Kconfig        |    2 +
>  drivers/Makefile       |    1 +
>  drivers/pwm/Kconfig    |   16 ++++++
>  drivers/pwm/Makefile   |    1 +
>  drivers/pwm/pwm-core.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pwm.h    |   12 ++++-
>  6 files changed, 160 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/pwm/Kconfig
>  create mode 100644 drivers/pwm/Makefile
>  create mode 100644 drivers/pwm/pwm-core.c
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a2b902f..e042f27 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -111,4 +111,6 @@ source "drivers/xen/Kconfig"
>  source "drivers/staging/Kconfig"
>  
>  source "drivers/platform/Kconfig"
> +
> +source "drivers/pwm/Kconfig"
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 4ca727d..0061ec4 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -116,3 +116,4 @@ obj-$(CONFIG_STAGING)		+= staging/
>  obj-y				+= platform/
>  obj-y				+= ieee802154/
>  obj-y				+= vbus/
> +obj-$(CONFIG_PWM_DEVICES)	+= pwm/
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> new file mode 100644
> index 0000000..5d10106
> --- /dev/null
> +++ b/drivers/pwm/Kconfig
> @@ -0,0 +1,16 @@
> +#
> +# PWM devices
> +#
> +
> +menuconfig PWM_DEVICES
> +	bool "PWM devices"
> +	default y
> +	---help---
> +	  Say Y here to get to see options for device drivers from various
> +	  different categories. This option alone does not add any kernel code.
> +
> +	  If you say N, all options in this submenu will be skipped and disabled.
> +

Shouldn't PWM_DEVICES select HAVE_PWM?

> +if PWM_DEVICES
> +
> +endif # PWM_DEVICES
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> new file mode 100644
> index 0000000..552f969
> --- /dev/null
> +++ b/drivers/pwm/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PWM_DEVICES)	+= pwm-core.o
> diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
> new file mode 100644
> index 0000000..b84027a
> --- /dev/null
> +++ b/drivers/pwm/pwm-core.c
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2010
> + *
> + * License Terms: GNU General Public License v2
> + * Author: Arun R Murthy <arun.murthy@stericsson.com>
> + */
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/rwsem.h>
> +#include <linux/err.h>
> +#include <linux/pwm.h>
> +
> +struct pwm_device {
> +	struct pwm_ops *pops;
> +	int pwm_id;
> +};
> +
> +struct pwm_dev_info {
> +	struct pwm_device *pwm_dev;
> +	struct list_head list;
> +};
> +static struct pwm_dev_info *di;

Why not embed the list head into pwm_device. That would certainly make
pwm_device_unregister much simpler.

> +
> +DECLARE_RWSEM(pwm_list_lock);
> +
> +void __deprecated pwm_free(struct pwm_device *pwm)
> +{
> +}
> +
> +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +{
> +	return pwm->pops->pwm_config(pwm, duty_ns, period_ns);
> +}
> +EXPORT_SYMBOL(pwm_config);
> +
> +int pwm_enable(struct pwm_device *pwm)
> +{
> +	return pwm->pops->pwm_enable(pwm);
> +}
> +EXPORT_SYMBOL(pwm_enable);
> +
> +void pwm_disable(struct pwm_device *pwm)
> +{
> +	pwm->pops->pwm_disable(pwm);
> +}
> +EXPORT_SYMBOL(pwm_disable);
> +
> +int pwm_device_register(struct pwm_device *pwm_dev)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	down_write(&pwm_list_lock);
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm) {
> +		up_write(&pwm_list_lock);
> +		return -ENOMEM;
> +	}
> +	pwm->pwm_dev = pwm_dev;
> +	list_add_tail(&pwm->list, &di->list);
> +	up_write(&pwm_list_lock);
> +

I guess you only need to lock the list when accessing the list and adding the new
pwm_dev.

> +	return 0;
> +}
> +EXPORT_SYMBOL(pwm_device_register);
> +
> +int pwm_device_unregister(struct pwm_device *pwm_dev)
> +{
> +	struct pwm_dev_info *tmp;
> +	struct list_head *pos, *tmp_lst;
> +
> +	down_write(&pwm_list_lock);
> +	list_for_each_safe(pos, tmp_lst, &di->list) {
> +		tmp = list_entry(pos, struct pwm_dev_info, list);
> +		if (tmp->pwm_dev == pwm_dev) {
> +			list_del(pos);
> +			kfree(tmp);
> +			up_write(&pwm_list_lock);
> +			return 0;
> +		}
> +	}
> +	up_write(&pwm_list_lock);
> +	return -ENOENT;
> +}
> +EXPORT_SYMBOL(pwm_device_unregister);
> +
> +struct pwm_device *pwm_request(int pwm_id, const char *name)
> +{
> +	struct pwm_dev_info *pwm;
> +	struct list_head *pos;
> +
> +	down_read(&pwm_list_lock);
> +	list_for_each(pos, &di->list) {
> +		pwm = list_entry(pos, struct pwm_dev_info, list);
> +		if ((!strcmp(pwm->pwm_dev->pops->name, name)) &&
> +				(pwm->pwm_dev->pwm_id == pwm_id)) {
> +			up_read(&pwm_list_lock);
> +			return pwm->pwm_dev;
> +		}
> +	}
> +	up_read(&pwm_list_lock);
> +	return ERR_PTR(-ENOENT);
> +}
> +EXPORT_SYMBOL(pwm_request);
> +
> +static int __init pwm_init(void)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&pwm->list);
> +	di = pwm;
> +	return 0;
> +}
> +
> +static void __exit pwm_exit(void)
> +{
> +	kfree(di);
> +}
> +
> +subsys_initcall(pwm_init);
> +module_exit(pwm_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Arun R Murthy");
> +MODULE_ALIAS("core:pwm");
> +MODULE_DESCRIPTION("Core pwm driver");
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 7c77575..6e7da1f 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -3,6 +3,13 @@
>  
>  struct pwm_device;
>  
> +struct pwm_ops {
> +	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int period_ns);
> +	int (*pwm_enable)(struct pwm_device *pwm);
> +	int (*pwm_disable)(struct pwm_device *pwm);
> +	char *name;
> +};
> +

Shouldn't name be part of the pwm_device? That would allow the ops to be shared
between different devices.

>  /*
>   * pwm_request - request a PWM device
>   */
> @@ -11,7 +18,7 @@ struct pwm_device *pwm_request(int pwm_id, const char *label);
>  /*
>   * pwm_free - free a PWM device
>   */
> -void pwm_free(struct pwm_device *pwm);
> +void __deprecated pwm_free(struct pwm_device *pwm);
>  
>  /*
>   * pwm_config - change a PWM device configuration
> @@ -28,4 +35,7 @@ int pwm_enable(struct pwm_device *pwm);
>   */
>  void pwm_disable(struct pwm_device *pwm);
>  
> +int pwm_device_register(struct pwm_device *pwm_dev);
> +int pwm_device_unregister(struct pwm_device *pwm_dev);
> +
>  #endif /* __LINUX_PWM_H */

It might be also a good idea to add a device class for pwm devices.

[PATCH 1/7] pwm: Add pwm core driver

[Lars-Peter Clausen]

Arun MURTHY wrote:
>>> +menuconfig PWM_DEVICES
>>> +	bool "PWM devices"
>>> +	default y
>>> +	---help---
>>> +	  Say Y here to get to see options for device drivers from
>> various
>>> +	  different categories. This option alone does not add any kernel
>> code.
>>> +
>>> +	  If you say N, all options in this submenu will be skipped and
>> disabled.
>>> +
>> Shouldn't PWM_DEVICES select HAVE_PWM?
> 
> 
> No not required, the entire concept is to remove HAVE_PWM and use PWM_CORE.

Well in patch 4 you say that PWM_CORE is currently limited to ARM. Furthermore you
change the pwm-backlight and pwm-led Kconfig entries to depend on HAVE_PWM ||
PWM_CORE. Adding a select HAVE_PWM here would make those changes unnecessary.
HAVE_PWM should be set, when pwm_* functions are available. When your pwm-core driver
is selected they are available.

> 
>>> +struct pwm_dev_info {
>>> +	struct pwm_device *pwm_dev;
>>> +	struct list_head list;
>>> +};
>>> +static struct pwm_dev_info *di;
>> Why not embed the list head into pwm_device. That would certainly make
>> pwm_device_unregister much simpler.
> pwm_device will be passed to each and every pwm driver that are registered as client with pwm core.
> The list consists of the registered pwm drivers and is to be handled by pwm core.
> Why should each and every pwm driver get to know about the entire pwm driver list?
Declare the list field to be private, by saying that it should only be touched by the
core. Right now you allocate a rather small additional structure for each registered
device. This could be easily be avoided by embedding the list field into the
pwm_device struct.

> And also since the pwm_request/register/unregister are the one which require this list having this list inst in local/static device information structure seems to be right.
> 
>>> +	}
>>> +	pwm->pwm_dev = pwm_dev;
>>> +	list_add_tail(&pwm->list, &di->list);
>>> +	up_write(&pwm_list_lock);
>>> +
>> I guess you only need to lock the list when accessing the list and
>> adding the new
>> pwm_dev.
> 
> Oops, thanks for pointing out, will implement this in the v2 patch.
>>> +struct pwm_ops {
>>> +	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int
>> period_ns);
>>> +	int (*pwm_enable)(struct pwm_device *pwm);
>>> +	int (*pwm_disable)(struct pwm_device *pwm);
>>> +	char *name;
>>> +};
>>> +
>> Shouldn't name be part of the pwm_device? That would allow the ops to
>> be shared
>> between different devices.
> Good catch, the reason being that 2 or more devices can share the same ops and get registered to pwm core.
> But the catch lies while identifying the pwm device while the clients are requesting for.
> The pwm backlight will request the pwm driver by name. This is parameter that distinguishes among different pwm devices irrespective of same ops or not.
Yes. And thats why it should go into the pwm_device struct itself.

If an additional ops struct is allocated for each device anyway we would be better of
embedding it directly into the device struct instead of just holding a pointer to it.

> 
>>> +int pwm_device_register(struct pwm_device *pwm_dev);
>>> +int pwm_device_unregister(struct pwm_device *pwm_dev);
>>> +
>>>  #endif /* __LINUX_PWM_H */
>> It might be also a good idea to add a device class for pwm devices.
> Sure, but can you please explain with an example the use case.
>
Well, for one it helps to keep data structured.
And there would be functions to traverse all devices of a class, so you could get rid
of your "di" list.


> Thanks and Regards,
> Arun R Murthy
> -------------
> 

- Lars

[PATCH 1/7] pwm: Add pwm core driver

[Lars-Peter Clausen]

Arun MURTHY wrote:
>>>> Shouldn't PWM_DEVICES select HAVE_PWM?
>>>
>>> No not required, the entire concept is to remove HAVE_PWM and use
>> PWM_CORE.
>>
>> Well in patch 4 you say that PWM_CORE is currently limited to ARM.
>> Furthermore you
>> change the pwm-backlight and pwm-led Kconfig entries to depend on
>> HAVE_PWM ||
>> PWM_CORE. Adding a select HAVE_PWM here would make those changes
>> unnecessary.
> HAVE_PWM is retained just because the mips pwm driver is not aligned with the pwm core driver.
> On mips pwm driver aligning to the pwm core driver HAVE_PWM will be replaced by PWM_CORE.
> 
>> HAVE_PWM should be set, when pwm_* functions are available. When your
>> pwm-core driver
>> is selected they are available.
> On applying this patch set pwm_* function will be exported in pwm_core driver and in mips pwm driver.
> Since mips pwm driver is not aligned with the pwm core, HAVE_PWM is retained and removed in places where pwm drivers register to pwm core driver.

pwm_{enable,disable,request,free} are the interface of the pwm api. Your pwm-core is
one implementation of that interface. A somewhat special though, because it tries to
be a generic implementation.
There are still other implementations though. For example right now the mips jz4740 one.
In my opinion HAVE_PWM should be defined if there is a implementation for the pwm
interface is available.
I know that your plan is that in the end pwm-core is the only implementation of the
pwm interface.

But right now it is not and on the other hand some SoC implementors might choose that
they want to provide their own minimal pwm interface implementation.
Furthermore this would allow you to start with pwm-core for one SoC which you have on
your desk and where you can properly test things and keep the patches clean from
clutter changing all the different archs.
Once pwm-core is in a proper shape you or other people can start porting all the
different SoC support code to pwm-core.

Similar behavior is for example true for the gpio api. There is gpiolib which is the
generic implementation which allows having gpio chips outside of the chip. On the
other hand there are still archs which choose to have their own gpio api implementation.

> 
>>> pwm_device will be passed to each and every pwm driver that are
>> registered as client with pwm core.
>>> The list consists of the registered pwm drivers and is to be handled
>> by pwm core.
>>> Why should each and every pwm driver get to know about the entire pwm
>> driver list?
>> Declare the list field to be private, by saying that it should only be
>> touched by the
>> core. Right now you allocate a rather small additional structure for
>> each registered
>> device. This could be easily be avoided by embedding the list field
>> into the
>> pwm_device struct.
> 
> The one that is being allocated in register is the pwm_device and this has to. Because each pwm driver will have their own data related to ops, pwm_id.
> Also note that there exists an element "data" that points to the pwm device specific information. Hence this allocation is required.
> 
>>>>> +	}
>>>>> +	pwm->pwm_dev = pwm_dev;
>>>>> +	list_add_tail(&pwm->list, &di->list);
>>>>> +	up_write(&pwm_list_lock);
>>>>> +
>>>> I guess you only need to lock the list when accessing the list and
>>>> adding the new
>>>> pwm_dev.
>>> Oops, thanks for pointing out, will implement this in the v2 patch.
> Coming back to this, I guess the locking has to be done while traversing the list also, as my present pointer in the list my get over written by the time I add an element to list. Please let me know if I am wrong.
> 
>>>>> +struct pwm_ops {
>>>>> +	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int
>>>> period_ns);
>>>>> +	int (*pwm_enable)(struct pwm_device *pwm);
>>>>> +	int (*pwm_disable)(struct pwm_device *pwm);
>>>>> +	char *name;
>>>>> +};
>>>>> +
>>>> Shouldn't name be part of the pwm_device? That would allow the ops
>> to
>>>> be shared
>>>> between different devices.
>>> Good catch, the reason being that 2 or more devices can share the
>> same ops and get registered to pwm core.
>>> But the catch lies while identifying the pwm device while the clients
>> are requesting for.
>>> The pwm backlight will request the pwm driver by name. This is
>> parameter that distinguishes among different pwm devices irrespective
>> of same ops or not.
>> Yes. And thats why it should go into the pwm_device struct itself.
>>
>> If an additional ops struct is allocated for each device anyway we
>> would be better of
>> embedding it directly into the device struct instead of just holding a
>> pointer to it.
> Yes ops structure will be allocated. But how can we get access to the ops structure of another driver?
> And moreover two pwm driver sharing same ops ideally means a single pwm module. If not everything atleast the pwm registers of two different modules changes. So this scenario can never occur.
> 
>>>>>  #endif /* __LINUX_PWM_H */
>>>> It might be also a good idea to add a device class for pwm devices.
>>> Sure, but can you please explain with an example the use case.
>>>
>> Well, for one it helps to keep data structured.
>> And there would be functions to traverse all devices of a class, so you
>> could get rid
>> of your "di" list.
> Sorry, I didn't get you can you please elaborate more?
> 
Sure. You would create a "struct class" device class for pwm devices. For each
registered pwm device there would then be a "struct device" representing the pwm
device whithin the linux device tree.
This has serveral advantages:
For one you can use this for keeping track of the all the pwm devices instead of
having your custom list. You can use class_for_each_device and class_find_device
instead of traversing your list. This would make the core much simpler and more readable.
Also you can use the device structure for refcounting of modules and devices. Right
now if a pwm-core driver, like the twl6040, is build as a module you can remove the
module while another driver, for example pwm-backlight, is using a pwm device from
the pwm-core driver. Then upon accessing the pwm device from the pwm-backlight driver
the code would crash, because it would access already freed memory.

- Lars


> Thanks and Regards,
> Arun R Murthy
> -------------

[PATCH 1/7] pwm: Add pwm core driver

[Arun MURTHY]

> Arun MURTHY wrote:
> >>>> Shouldn't PWM_DEVICES select HAVE_PWM?
> >>>
> >>> No not required, the entire concept is to remove HAVE_PWM and use
> >> PWM_CORE.
> >>
> >> Well in patch 4 you say that PWM_CORE is currently limited to ARM.
> >> Furthermore you
> >> change the pwm-backlight and pwm-led Kconfig entries to depend on
> >> HAVE_PWM ||
> >> PWM_CORE. Adding a select HAVE_PWM here would make those changes
> >> unnecessary.
> > HAVE_PWM is retained just because the mips pwm driver is not aligned
> with the pwm core driver.
> > On mips pwm driver aligning to the pwm core driver HAVE_PWM will be
> replaced by PWM_CORE.
> >
> >> HAVE_PWM should be set, when pwm_* functions are available. When
> your
> >> pwm-core driver
> >> is selected they are available.
> > On applying this patch set pwm_* function will be exported in
> pwm_core driver and in mips pwm driver.
> > Since mips pwm driver is not aligned with the pwm core, HAVE_PWM is
> retained and removed in places where pwm drivers register to pwm core
> driver.
> 
> pwm_{enable,disable,request,free} are the interface of the pwm api.
> Your pwm-core is
> one implementation of that interface. A somewhat special though,
> because it tries to
> be a generic implementation.
> There are still other implementations though. For example right now the
> mips jz4740 one.
> In my opinion HAVE_PWM should be defined if there is a implementation
> for the pwm
> interface is available.
> I know that your plan is that in the end pwm-core is the only
> implementation of the
> pwm interface.
Yes for that reason, I still have retained the HAVE_PWM, once that is
also aligned with pwm core, HAVE_PWM will be totally removed.
I am in the process of doing the same in this series of patch, but was
blocked by mips jz4740 pwm driver.
Will do that very soon in near future.

> 
> But right now it is not and on the other hand some SoC implementors
> might choose that
> they want to provide their own minimal pwm interface implementation.
> Furthermore this would allow you to start with pwm-core for one SoC
> which you have on
> your desk and where you can properly test things and keep the patches
> clean from
> clutter changing all the different archs.
> Once pwm-core is in a proper shape you or other people can start
> porting all the
> different SoC support code to pwm-core.
That's the exact reason for me to come up with this core driver, which has
the minimal part and the one that are common in all pwm drivers.
Not only common most if not all pwm drivers just make use of only these
functions.
Moreover I have aligned all existing pwm driver in my patch 4/7 :-)

> 
> Similar behavior is for example true for the gpio api. There is gpiolib
> which is the
> generic implementation which allows having gpio chips outside of the
> chip. On the
> other hand there are still archs which choose to have their own gpio
> api implementation.
> 
> >
> >>> pwm_device will be passed to each and every pwm driver that are
> >> registered as client with pwm core.
> >>> The list consists of the registered pwm drivers and is to be
> handled
> >> by pwm core.
> >>> Why should each and every pwm driver get to know about the entire
> pwm
> >> driver list?
> >> Declare the list field to be private, by saying that it should only
> be
> >> touched by the
> >> core. Right now you allocate a rather small additional structure for
> >> each registered
> >> device. This could be easily be avoided by embedding the list field
> >> into the
> >> pwm_device struct.
> >
> > The one that is being allocated in register is the pwm_device and
> this has to. Because each pwm driver will have their own data related
> to ops, pwm_id.
> > Also note that there exists an element "data" that points to the pwm
> device specific information. Hence this allocation is required.
> >
> >>>>> +	}
> >>>>> +	pwm->pwm_dev = pwm_dev;
> >>>>> +	list_add_tail(&pwm->list, &di->list);
> >>>>> +	up_write(&pwm_list_lock);
> >>>>> +
> >>>> I guess you only need to lock the list when accessing the list and
> >>>> adding the new
> >>>> pwm_dev.
> >>> Oops, thanks for pointing out, will implement this in the v2 patch.
> > Coming back to this, I guess the locking has to be done while
> traversing the list also, as my present pointer in the list my get over
> written by the time I add an element to list. Please let me know if I
> am wrong.
> >
> >>>>> +struct pwm_ops {
> >>>>> +	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int
> >>>> period_ns);
> >>>>> +	int (*pwm_enable)(struct pwm_device *pwm);
> >>>>> +	int (*pwm_disable)(struct pwm_device *pwm);
> >>>>> +	char *name;
> >>>>> +};
> >>>>> +
> >>>> Shouldn't name be part of the pwm_device? That would allow the ops
> >> to
> >>>> be shared
> >>>> between different devices.
> >>> Good catch, the reason being that 2 or more devices can share the
> >> same ops and get registered to pwm core.
> >>> But the catch lies while identifying the pwm device while the
> clients
> >> are requesting for.
> >>> The pwm backlight will request the pwm driver by name. This is
> >> parameter that distinguishes among different pwm devices
> irrespective
> >> of same ops or not.
> >> Yes. And thats why it should go into the pwm_device struct itself.
> >>
> >> If an additional ops struct is allocated for each device anyway we
> >> would be better of
> >> embedding it directly into the device struct instead of just holding
> a
> >> pointer to it.
> > Yes ops structure will be allocated. But how can we get access to the
> ops structure of another driver?
> > And moreover two pwm driver sharing same ops ideally means a single
> pwm module. If not everything atleast the pwm registers of two
> different modules changes. So this scenario can never occur.
> >
> >>>>>  #endif /* __LINUX_PWM_H */
> >>>> It might be also a good idea to add a device class for pwm
> devices.
> >>> Sure, but can you please explain with an example the use case.
> >>>
> >> Well, for one it helps to keep data structured.
> >> And there would be functions to traverse all devices of a class, so
> you
> >> could get rid
> >> of your "di" list.
> > Sorry, I didn't get you can you please elaborate more?
> >
> Sure. You would create a "struct class" device class for pwm devices.
> For each
> registered pwm device there would then be a "struct device"
> representing the pwm
> device whithin the linux device tree.
> This has serveral advantages:
> For one you can use this for keeping track of the all the pwm devices
> instead of
> having your custom list. You can use class_for_each_device and
> class_find_device
> instead of traversing your list. This would make the core much simpler
> and more readable.
> Also you can use the device structure for refcounting of modules and
> devices. Right
> now if a pwm-core driver, like the twl6040, is build as a module you
> can remove the
> module while another driver, for example pwm-backlight, is using a pwm
> device from
> the pwm-core driver. Then upon accessing the pwm device from the pwm-
> backlight driver
> the code would crash, because it would access already freed memory.
> 
Can this be taken after this patch is merged?

Thanks and Regards,
Arun R Murthy
------------

[PATCH 1/7] pwm: Add pwm core driver

[Trilok Soni]

Hi Arun,

Adding Bill Gatliff (anyway, CC list already crowded)

On Wed, Sep 29, 2010 at 10:19 AM, Arun MURTHY
<arun.murthy@stericsson.com> wrote:
>> Arun MURTHY wrote:
>> >>>> Shouldn't PWM_DEVICES select HAVE_PWM?
>> >>>
>> >>> No not required, the entire concept is to remove HAVE_PWM and use
>> >> PWM_CORE.

There is already nice and clean framework written by Bill for PWM, if
you grep the LKML and linux-embedded mailing list archive then you
will get his patches, and it seems that he had promised to send the
updated version few week back, but not heard from him (may be because
he was travelling as per FB status).

Please evaluate that framework too.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

[PATCH 1/7] pwm: Add pwm core driver

[Arun MURTHY]

Hi Trilok,

> Hi Arun,
> 
> Adding Bill Gatliff (anyway, CC list already crowded)
> 
> On Wed, Sep 29, 2010 at 10:19 AM, Arun MURTHY
> <arun.murthy@stericsson.com> wrote:
> >> Arun MURTHY wrote:
> >> >>>> Shouldn't PWM_DEVICES select HAVE_PWM?
> >> >>>
> >> >>> No not required, the entire concept is to remove HAVE_PWM and
> use
> >> >> PWM_CORE.
> 
> There is already nice and clean framework written by Bill for PWM, if
> you grep the LKML and linux-embedded mailing list archive then you
> will get his patches, and it seems that he had promised to send the
> updated version few week back, but not heard from him (may be because
> he was travelling as per FB status).
> 
> Please evaluate that framework too.
> 
Thanks for this information, I did search in linux-embedded mailing list
archive. Below are my views on that patch set.
Many of the functions that has been defined in pwm core driver
written by Bill Gatliff is not being used by the most of the pwm drivers
except Atmel PWM driver. I rather felt the pwm core driver was an attempt
made to generalize the Atmel pwm driver.
And moreover this was posted long back somewhere in the beginning of this
year i.e Feb and the thread is dead thereafter.

This patch has been submitted focusing all the existing pwm drivers and
only these are the functions that are being used by pwm drivers.
This patch set also included patch to align all the existing pwm driver
with the pwm core driver.
So it is an attempt to generalize most of the pwm drivers and
conclude with a pwm core driver.

Thanks and Regards,
Arun R Murthy
-------------

[PATCH 1/7] pwm: Add pwm core driver

[Trilok Soni]

Hi Arun,

On Fri, Oct 1, 2010 at 8:55 AM, Arun MURTHY <arun.murthy@stericsson.com> wrote:
> Hi Trilok,
>
>> Hi Arun,
>>
>> Adding Bill Gatliff (anyway, CC list already crowded)
>>
>> On Wed, Sep 29, 2010 at 10:19 AM, Arun MURTHY
>> <arun.murthy@stericsson.com> wrote:
>> >> Arun MURTHY wrote:
>> >> >>>> Shouldn't PWM_DEVICES select HAVE_PWM?
>> >> >>>
>> >> >>> No not required, the entire concept is to remove HAVE_PWM and
>> use
>> >> >> PWM_CORE.
>>
>> There is already nice and clean framework written by Bill for PWM, if
>> you grep the LKML and linux-embedded mailing list archive then you
>> will get his patches, and it seems that he had promised to send the
>> updated version few week back, but not heard from him (may be because
>> he was travelling as per FB status).
>>
>> Please evaluate that framework too.
>>
> Thanks for this information, I did search in linux-embedded mailing list
> archive. Below are my views on that patch set.
> Many of the functions that has been defined in pwm core driver
> written by Bill Gatliff is not being used by the most of the pwm drivers
> except Atmel PWM driver. I rather felt the pwm core driver was an attempt
> made to generalize the Atmel pwm driver.
> And moreover this was posted long back somewhere in the beginning of this
> year i.e Feb and the thread is dead thereafter.
>
> This patch has been submitted focusing all the existing pwm drivers and
> only these are the functions that are being used by pwm drivers.
> This patch set also included patch to align all the existing pwm driver
> with the pwm core driver.
> So it is an attempt to generalize most of the pwm drivers and
> conclude with a pwm core driver.

I don't agree that Bill had only atmel drivers view. The PWM framework
was discussed in-depth and at that time reviewers also requested once
to provide more example drivers using these drivers, someone said "we
atleast need three drivers as rule of thumb". Let's wait until Bill
reviews your framework, I am sure we don't need to end up the same
problems faced by Bill while designing that framework in your code
too.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

[PATCH 1/7] pwm: Add pwm core driver

[Arun MURTHY]

Hi Trilok,

> Hi Arun,
> 
> On Fri, Oct 1, 2010 at 8:55 AM, Arun MURTHY
> <arun.murthy@stericsson.com> wrote:
> > Hi Trilok,
> >
> >> Hi Arun,
> >>
> >> Adding Bill Gatliff (anyway, CC list already crowded)
> >>
> >> On Wed, Sep 29, 2010 at 10:19 AM, Arun MURTHY
> >> <arun.murthy@stericsson.com> wrote:
> >> >> Arun MURTHY wrote:
> >> >> >>>> Shouldn't PWM_DEVICES select HAVE_PWM?
> >> >> >>>
> >> >> >>> No not required, the entire concept is to remove HAVE_PWM and
> >> use
> >> >> >> PWM_CORE.
> >>
> >> There is already nice and clean framework written by Bill for PWM,
> if
> >> you grep the LKML and linux-embedded mailing list archive then you
> >> will get his patches, and it seems that he had promised to send the
> >> updated version few week back, but not heard from him (may be
> because
> >> he was travelling as per FB status).
> >>
> >> Please evaluate that framework too.
> >>
> > Thanks for this information, I did search in linux-embedded mailing
> list
> > archive. Below are my views on that patch set.
> > Many of the functions that has been defined in pwm core driver
> > written by Bill Gatliff is not being used by the most of the pwm
> drivers
> > except Atmel PWM driver. I rather felt the pwm core driver was an
> attempt
> > made to generalize the Atmel pwm driver.
> > And moreover this was posted long back somewhere in the beginning of
> this
> > year i.e Feb and the thread is dead thereafter.
> >
> > This patch has been submitted focusing all the existing pwm drivers
> and
> > only these are the functions that are being used by pwm drivers.
> > This patch set also included patch to align all the existing pwm
> driver
> > with the pwm core driver.
> > So it is an attempt to generalize most of the pwm drivers and
> > conclude with a pwm core driver.
> 
> I don't agree that Bill had only atmel drivers view. The PWM framework
> was discussed in-depth and at that time reviewers also requested once
> to provide more example drivers using these drivers, someone said "we
> atleast need three drivers as rule of thumb". Let's wait until Bill
> reviews your framework, I am sure we don't need to end up the same
> problems faced by Bill while designing that framework in your code
> too.
> 
You can have a look at the pwm_config_nosleep(),pwm_set_polarity(),
pwm_synchronize(),pwm_unsynchronize(), pwm_set_handler() etc.
These are not being used by the exsting pwm drivers except Atmel pwm.
I mean not the functions but the functionality.
PWM is a simple device and most of its clients are controlling intensity
of backlight, leds, vibrator etc.
I don't think these complex functionality are required.
And moreover it also refers to GPIO pins, in that case it comes under
a different classification. The one that I have suggested is a generic
pwm core driver.
You can have a look at the existing pwm drivers in drivers/mfd/twl6030-pwm.c,
arch/arm/plat-samsung/pwm.c, arch/arm/plat-mxc/pwm.c, arch/arm/plat-pxa/pwm.c,
arch/mips/jz4740/pwm.c.
None of these include the function provided the patch " [PWM PATCH 1/5] API
to consolidate PWM devices behind a common user and kernel interface "
except pwm_enable, pwm_config, pwm_disable.
I have focused on all these and come up with this design.
And moreover Bill's patch set for pwm core driver, becomes incompatible with
pwm based backlight and led driver(drivers/leds/leds-pwm.c,
drivers/video/backlight/pwm_bl.c) and drivers/input/misc/pwm-beeper.c.

I don't mind waiting for Bill's review on my patch, but he is not active
since Feb 2010.

Thanks and Regards,
Arun R Murthy
-------------

[PATCH 1/7] pwm: Add pwm core driver

[Jassi Brar]

On Fri, Oct 1, 2010 at 4:25 PM, Arun MURTHY <arun.murthy@stericsson.com> wrote:
> You can have a look at the pwm_config_nosleep(),pwm_set_polarity(),
> pwm_synchronize(),pwm_unsynchronize(), pwm_set_handler() etc.
> These are not being used by the exsting pwm drivers except Atmel pwm.
How would your 'simple' driver handle Atmel then ?
What if future's SoCs start providing those 'advance' features like Atmel's ?

> I mean not the functions but the functionality.
> PWM is a simple device and most of its clients are controlling intensity
> of backlight, leds, vibrator etc.
> I don't think these complex functionality are required.
oh dear !

[PATCH 1/7] pwm: Add pwm core driver

[Arun MURTHY]

> On Fri, Oct 1, 2010 at 4:25 PM, Arun MURTHY
> <arun.murthy@stericsson.com> wrote:
> > You can have a look at the pwm_config_nosleep(),pwm_set_polarity(),
> > pwm_synchronize(),pwm_unsynchronize(), pwm_set_handler() etc.
> > These are not being used by the exsting pwm drivers except Atmel pwm.
> How would your 'simple' driver handle Atmel then ?
> What if future's SoCs start providing those 'advance' features like
> Atmel's ?
> 
The pwm core driver is the intersection of all pwm drivers and not union
of all pwm driver. I refer this as simple pwm core driver / framework.
Atmel pwm is of a separate classification.
It includes GPIO also. Though, Atmel can use the pwm core driver framework
for functionalities like pwm_enable, pwm_disable, pwm_config, etc and remaining
functionalities specific to Atmel will be handled in Atlmel pwm driver and
will not be exposed to the entire kernel.
Its that the present day pwm device that has been made easy though, by providing
the same functionality.

> > I mean not the functions but the functionality.
> > PWM is a simple device and most of its clients are controlling
> intensity
> > of backlight, leds, vibrator etc.
> > I don't think these complex functionality are required.
> oh dear !
Here I mean why should all those function be exposed to the entire kernel,
as most of the pwm devices do not use them.

Thanks and Regards,
Arun R Murthy
------------

[PATCH 1/7] pwm: Add pwm core driver

[Jassi Brar]

On Fri, Oct 1, 2010 at 5:46 PM, Arun MURTHY <arun.murthy@stericsson.com> wrote:
>> On Fri, Oct 1, 2010 at 4:25 PM, Arun MURTHY
>> <arun.murthy@stericsson.com> wrote:
>> > You can have a look at the pwm_config_nosleep(),pwm_set_polarity(),
>> > pwm_synchronize(),pwm_unsynchronize(), pwm_set_handler() etc.
>> > These are not being used by the exsting pwm drivers except Atmel pwm.
>> How would your 'simple' driver handle Atmel then ?
>> What if future's SoCs start providing those 'advance' features like
>> Atmel's ?
>>
> The pwm core driver is the intersection of all pwm drivers and not union
> of all pwm driver. I refer this as simple pwm core driver / framework.
> Atmel pwm is of a separate classification.
> It includes GPIO also. Though, Atmel can use the pwm core driver framework
> for functionalities like pwm_enable, pwm_disable, pwm_config, etc and remaining
> functionalities specific to Atmel will be handled in Atlmel pwm driver and
> will not be exposed to the entire kernel.
> Its that the present day pwm device that has been made easy though, by providing
> the same functionality.
It's sad that Bill Gatliff didn't/couldn't take his work to conclusive end.
The work was apparently better
http://www.mail-archive.com/linux-embedded at vger.kernel.org/msg02599.html

Best of luck.

[PATCH 1/7] pwm: Add pwm core driver

[Mark Brown]

On Fri, Oct 01, 2010 at 10:46:15AM +0200, Arun MURTHY wrote:
> > On Fri, Oct 1, 2010 at 4:25 PM, Arun MURTHY

> > > I mean not the functions but the functionality.
> > > PWM is a simple device and most of its clients are controlling
> > intensity
> > > of backlight, leds, vibrator etc.
> > > I don't think these complex functionality are required.
> > oh dear !
> Here I mean why should all those function be exposed to the entire kernel,
> as most of the pwm devices do not use them.

While many PWM uses are very simple that doesn't mean that we'll never
need to support more advanced uses.  Normally we try to design APIs so
that they both scale down to the simplest use cases and also up to more
complex ones.

[PATCH 1/7] pwm: Add pwm core driver

[Arun MURTHY]

> On Fri, Oct 01, 2010 at 10:46:15AM +0200, Arun MURTHY wrote:
> > > On Fri, Oct 1, 2010 at 4:25 PM, Arun MURTHY
> 
> > > > I mean not the functions but the functionality.
> > > > PWM is a simple device and most of its clients are controlling
> > > intensity
> > > > of backlight, leds, vibrator etc.
> > > > I don't think these complex functionality are required.
> > > oh dear !
> > Here I mean why should all those function be exposed to the entire
> kernel,
> > as most of the pwm devices do not use them.
> 
> While many PWM uses are very simple that doesn't mean that we'll never
> need to support more advanced uses.  Normally we try to design APIs so
> that they both scale down to the simplest use cases and also up to more
> complex ones.
My intention is just to enable two pwm drivers and build successfully.
This patch can be considered for this reason and taken up until Bill's
patches are merged.

Thanks and Regards,
Arun R Murthy
-------------

[PATCH 1/7] pwm: Add pwm core driver

[Mark Brown]

On Tue, Sep 28, 2010 at 01:10:42PM +0530, Arun Murthy wrote:

> The existing pwm based led and backlight driver makes use of the
> pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> be exposing the same set of function name as in include/linux/pwm.h.
> As a result build fails.

As others have said it's *really* not clear what the problem is here...

Please also take a look at the work which Bill Gatliff was doing on a
similar PWM core API and the resulting discussion - how does your code
differ from his, and is any of the feedback on his proposal relevant to
yours?

> +void __deprecated pwm_free(struct pwm_device *pwm)
> +{
> +}
> +

Shouldn't this either be an inline function directly in the header or
exported?

> +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +{
> +	return pwm->pops->pwm_config(pwm, duty_ns, period_ns);
> +}
> +EXPORT_SYMBOL(pwm_config);

I'd expect some handling of fixed function PWMs (though I'd expect those
to be rare).

> +	down_write(&pwm_list_lock);
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm) {
> +		up_write(&pwm_list_lock);
> +		return -ENOMEM;
> +	}

No need to take the lock until the allocation succeeded.

> +static int __init pwm_init(void)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&pwm->list);
> +	di = pwm;
> +	return 0;

Why not just use static data for the list head?

> +subsys_initcall(pwm_init);
> +module_exit(pwm_exit);

Usually these are located next to the functions.

[PATCH 1/7] pwm: Add pwm core driver

[Ryan Mallon]

On 09/28/2010 08:40 PM, Arun Murthy wrote:
> The existing pwm based led and backlight driver makes use of the
> pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> be exposing the same set of function name as in include/linux/pwm.h.
> As a result build fails.
> 
> In order to overcome this issue all the pwm drivers must register to
> some core pwm driver with function pointers for pwm operations (i.e
> pwm_config, pwm_enable, pwm_disable).

The other major benefit of this patch set is that it allows non-soc
pwms, such as those provided on an i2c or spi device, to be added easily.

> The clients of pwm device will have to call pwm_request, wherein
> they will get the pointer to struct pwm_ops. This structure include
> function pointers for pwm_config, pwm_enable and pwm_disable.
> 
> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---

> +menuconfig PWM_DEVICES
> +	bool "PWM devices"
> +	default y
> +	---help---
> +	  Say Y here to get to see options for device drivers from various
> +	  different categories. This option alone does not add any kernel code.

This help text doesn't really explain what the option does.

> +struct pwm_device {
> +	struct pwm_ops *pops;
> +	int pwm_id;
> +};
> +
> +struct pwm_dev_info {
> +	struct pwm_device *pwm_dev;
> +	struct list_head list;
> +};

These two structures can be merged into one which will make the code
much simpler.

> +static struct pwm_dev_info *di;

This appears to be used as just a list, and could be replaced by:

static LIST_HEAD(pwm_list);

> +struct pwm_device *pwm_request(int pwm_id, const char *name)
> +{
> +	struct pwm_dev_info *pwm;
> +	struct list_head *pos;
> +
> +	down_read(&pwm_list_lock);
> +	list_for_each(pos, &di->list) {
> +		pwm = list_entry(pos, struct pwm_dev_info, list);
> +		if ((!strcmp(pwm->pwm_dev->pops->name, name)) &&
> +				(pwm->pwm_dev->pwm_id == pwm_id)) {
> +			up_read(&pwm_list_lock);
> +			return pwm->pwm_dev;
> +		}
> +	}
> +	up_read(&pwm_list_lock);
> +	return ERR_PTR(-ENOENT);
> +}

Is it by design that multiple users can request and use the same pwm or
should pwms have a use count and return -EBUSY if already requested?

> +static int __init pwm_init(void)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&pwm->list);
> +	di = pwm;
> +	return 0;

If di is changed to a list as suggested above this function does not
need to exist.

> +static void __exit pwm_exit(void)
> +{
> +	kfree(di);

Do you need to ensure the list is empty first or do module dependencies
ensure that?

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

[PATCH 2/7] backlight:pwm: add an element 'name' to platform data

[Arun Murthy]

A new element 'name' is added to pwm backlight platform data structure.
This is required to identify the pwm device.

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/video/backlight/pwm_bl.c |    4 +++-
 include/linux/pwm_backlight.h    |    1 +
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 5504435..b0978a8 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -94,7 +94,9 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	pb->notify = data->notify;
 	pb->dev = &pdev->dev;
 
-	pb->pwm = pwm_request(data->pwm_id, "backlight");
+	if (!data->name)
+		data->name = "backlight";
+	pb->pwm = pwm_request(data->pwm_id, data->name);
 	if (IS_ERR(pb->pwm)) {
 		dev_err(&pdev->dev, "unable to request PWM for backlight\n");
 		ret = PTR_ERR(pb->pwm);
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 01b3d75..c2ce8f8 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -6,6 +6,7 @@
 
 struct platform_pwm_backlight_data {
 	int pwm_id;
+	char *name;
 	unsigned int max_brightness;
 	unsigned int dft_brightness;
 	unsigned int pwm_period_ns;
-- 
1.7.2.dirty

[PATCH 2/7] backlight:pwm: add an element 'name' to platform data

[Mark Brown]

On Tue, Sep 28, 2010 at 01:10:43PM +0530, Arun Murthy wrote:
> A new element 'name' is added to pwm backlight platform data structure.
> This is required to identify the pwm device.

> -	pb->pwm = pwm_request(data->pwm_id, "backlight");
> +	if (!data->name)
> +		data->name = "backlight";
> +	pb->pwm = pwm_request(data->pwm_id, data->name);

If we're going to go through and require that all PWM API users be
updated to take platform data for the name might it not be better to
switch over to the clock API style request by device interface?

[PATCH 3/7] leds: pwm: add a new element 'name' to platform data

[Arun Murthy]

A new element 'name' is added to pwm led platform data structure.
This is required to identify the pwm device.

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/leds/leds-pwm.c  |    4 +++-
 include/linux/leds_pwm.h |    1 +
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index da3fa8d..8da2be6 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -66,8 +66,10 @@ static int led_pwm_probe(struct platform_device *pdev)
 		cur_led = &pdata->leds[i];
 		led_dat = &leds_data[i];
 
+		if (!pdata->name)
+			pdata->name = cur_led->name;
 		led_dat->pwm = pwm_request(cur_led->pwm_id,
-				cur_led->name);
+				pdata->name);
 		if (IS_ERR(led_dat->pwm)) {
 			dev_err(&pdev->dev, "unable to request PWM %d\n",
 					cur_led->pwm_id);
diff --git a/include/linux/leds_pwm.h b/include/linux/leds_pwm.h
index 33a0711..7a847a0 100644
--- a/include/linux/leds_pwm.h
+++ b/include/linux/leds_pwm.h
@@ -16,6 +16,7 @@ struct led_pwm {
 struct led_pwm_platform_data {
 	int			num_leds;
 	struct led_pwm	*leds;
+	char *name;
 };
 
 #endif
-- 
1.7.2.dirty

[PATCH 3/7] leds: pwm: add a new element 'name' to platform data

[Eric Miao]

On Tue, Sep 28, 2010 at 3:40 PM, Arun Murthy <arun.murthy@stericsson.com> wrote:
> A new element 'name' is added to pwm led platform data structure.
> This is required to identify the pwm device.

I cannot see what this name is used for?

>
> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
> ?drivers/leds/leds-pwm.c ?| ? ?4 +++-
> ?include/linux/leds_pwm.h | ? ?1 +
> ?2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index da3fa8d..8da2be6 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -66,8 +66,10 @@ static int led_pwm_probe(struct platform_device *pdev)
> ? ? ? ? ? ? ? ?cur_led = &pdata->leds[i];
> ? ? ? ? ? ? ? ?led_dat = &leds_data[i];
>
> + ? ? ? ? ? ? ? if (!pdata->name)
> + ? ? ? ? ? ? ? ? ? ? ? pdata->name = cur_led->name;
> ? ? ? ? ? ? ? ?led_dat->pwm = pwm_request(cur_led->pwm_id,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cur_led->name);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pdata->name);
> ? ? ? ? ? ? ? ?if (IS_ERR(led_dat->pwm)) {
> ? ? ? ? ? ? ? ? ? ? ? ?dev_err(&pdev->dev, "unable to request PWM %d\n",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cur_led->pwm_id);
> diff --git a/include/linux/leds_pwm.h b/include/linux/leds_pwm.h
> index 33a0711..7a847a0 100644
> --- a/include/linux/leds_pwm.h
> +++ b/include/linux/leds_pwm.h
> @@ -16,6 +16,7 @@ struct led_pwm {
> ?struct led_pwm_platform_data {
> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? num_leds;
> ? ? ? ?struct led_pwm ?*leds;
> + ? ? ? char *name;
> ?};
>
> ?#endif
> --
> 1.7.2.dirty
>
>

[PATCH 3/7] leds: pwm: add a new element 'name' to platform data

[Arun MURTHY]

> On Tue, Sep 28, 2010 at 3:40 PM, Arun Murthy
> <arun.murthy@stericsson.com> wrote:
> > A new element 'name' is added to pwm led platform data structure.
> > This is required to identify the pwm device.
> 
> I cannot see what this name is used for?
> 
This name is referenced in leds-pwm.c and pwm_bl.c. It is passed as a parameter in pwm_request() function.(please refer the next patch i.e PATHC 4/7)
It is used to identify the pwm driver by name.

Thanks and Regards,
Arun R Murthy
-------------

[PATCH 4/7] pwm: Align existing pwm drivers with pwm-core driver

[Arun Murthy]

pwm-core: make the driver visible for ARM only

	Align ab8500 pwm with the pwm core driver
	Align twl6030 pwm driver with pwm core driver
	Align Freescale mxc pwm driver with pwm core driver
	Align pxa pwm driver with pwm core driver
	Align samsung(s3c) pwm driver with pwm core driver

mips-jz4740: pwm: Align with new pwm core driver

PWM core driver has been added and has been enabled only for ARM
platform. The same can be utilised for mips also.
Please align with the pwm core driver(drivers/pwm-core.c).

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 arch/arm/plat-mxc/pwm.c     |  166 +++++++++++++-----------------
 arch/arm/plat-pxa/pwm.c     |  210 ++++++++++++++++++--------------------
 arch/arm/plat-samsung/pwm.c |  235 +++++++++++++++++++------------------------
 arch/mips/jz4740/pwm.c      |    2 +-
 drivers/mfd/twl-core.c      |   13 +++
 drivers/mfd/twl6030-pwm.c   |  111 +++++++++++++-------
 drivers/misc/ab8500-pwm.c   |   87 +++++++---------
 drivers/pwm/Kconfig         |    1 +
 drivers/pwm/pwm-core.c      |    9 +--
 include/linux/pwm.h         |   21 ++++-
 10 files changed, 418 insertions(+), 437 deletions(-)

diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
index c36f263..b259ba9 100644
--- a/arch/arm/plat-mxc/pwm.c
+++ b/arch/arm/plat-mxc/pwm.c
@@ -38,22 +38,16 @@
 
 
 
-struct pwm_device {
-	struct list_head	node;
-	struct platform_device *pdev;
-
-	const char	*label;
+struct mxc_pwm_device {
 	struct clk	*clk;
-
 	int		clk_enabled;
 	void __iomem	*mmio_base;
-
-	unsigned int	use_count;
-	unsigned int	pwm_id;
 };
 
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+static int mxc_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
+	struct mxc_pwm_device *mxc_pwm = pwm->data;
+
 	if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
 		return -EINVAL;
 
@@ -62,7 +56,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 		unsigned long period_cycles, duty_cycles, prescale;
 		u32 cr;
 
-		c = clk_get_rate(pwm->clk);
+		c = clk_get_rate(mxc_pwm->clk);
 		c = c * period_ns;
 		do_div(c, 1000000000);
 		period_cycles = c;
@@ -74,8 +68,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 		do_div(c, period_ns);
 		duty_cycles = c;
 
-		writel(duty_cycles, pwm->mmio_base + MX3_PWMSAR);
-		writel(period_cycles, pwm->mmio_base + MX3_PWMPR);
+		writel(duty_cycles, mxc_pwm->mmio_base + MX3_PWMSAR);
+		writel(period_cycles, mxc_pwm->mmio_base + MX3_PWMPR);
 
 		cr = MX3_PWMCR_PRESCALER(prescale) | MX3_PWMCR_EN;
 
@@ -84,7 +78,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 		else
 			cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
 
-		writel(cr, pwm->mmio_base + MX3_PWMCR);
+		writel(cr, mxc_pwm->mmio_base + MX3_PWMCR);
 	} else if (cpu_is_mx1() || cpu_is_mx21()) {
 		/* The PWM subsystem allows for exact frequencies. However,
 		 * I cannot connect a scope on my device to the PWM line and
@@ -102,110 +96,76 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 		 * both the prescaler (/1 .. /128) and then by CLKSEL
 		 * (/2 .. /16).
 		 */
-		u32 max = readl(pwm->mmio_base + MX1_PWMP);
+		u32 max = readl(mxc_pwm->mmio_base + MX1_PWMP);
 		u32 p = max * duty_ns / period_ns;
-		writel(max - p, pwm->mmio_base + MX1_PWMS);
+		writel(max - p, mxc_pwm->mmio_base + MX1_PWMS);
 	} else {
 		BUG();
 	}
 
 	return 0;
 }
-EXPORT_SYMBOL(pwm_config);
 
-int pwm_enable(struct pwm_device *pwm)
+static int mxc_pwm_enable(struct pwm_device *pwm)
 {
+	struct mxc_pwm_device *mxc_pwm = pwm->data;
 	int rc = 0;
 
-	if (!pwm->clk_enabled) {
-		rc = clk_enable(pwm->clk);
+	if (!mxc_pwm->clk_enabled) {
+		rc = clk_enable(mxc_pwm->clk);
 		if (!rc)
-			pwm->clk_enabled = 1;
+			mxc_pwm->clk_enabled = 1;
 	}
 	return rc;
 }
-EXPORT_SYMBOL(pwm_enable);
-
-void pwm_disable(struct pwm_device *pwm)
-{
-	writel(0, pwm->mmio_base + MX3_PWMCR);
-
-	if (pwm->clk_enabled) {
-		clk_disable(pwm->clk);
-		pwm->clk_enabled = 0;
-	}
-}
-EXPORT_SYMBOL(pwm_disable);
-
-static DEFINE_MUTEX(pwm_lock);
-static LIST_HEAD(pwm_list);
 
-struct pwm_device *pwm_request(int pwm_id, const char *label)
+static int mxc_pwm_disable(struct pwm_device *pwm)
 {
-	struct pwm_device *pwm;
-	int found = 0;
+	struct mxc_pwm_device *mxc_pwm = pwm->data;
 
-	mutex_lock(&pwm_lock);
+	writel(0, mxc_pwm->mmio_base + MX3_PWMCR);
 
-	list_for_each_entry(pwm, &pwm_list, node) {
-		if (pwm->pwm_id == pwm_id) {
-			found = 1;
-			break;
-		}
+	if (mxc_pwm->clk_enabled) {
+		clk_disable(mxc_pwm->clk);
+		mxc_pwm->clk_enabled = 0;
 	}
-
-	if (found) {
-		if (pwm->use_count == 0) {
-			pwm->use_count++;
-			pwm->label = label;
-		} else
-			pwm = ERR_PTR(-EBUSY);
-	} else
-		pwm = ERR_PTR(-ENOENT);
-
-	mutex_unlock(&pwm_lock);
-	return pwm;
-}
-EXPORT_SYMBOL(pwm_request);
-
-void pwm_free(struct pwm_device *pwm)
-{
-	mutex_lock(&pwm_lock);
-
-	if (pwm->use_count) {
-		pwm->use_count--;
-		pwm->label = NULL;
-	} else
-		pr_warning("PWM device already freed\n");
-
-	mutex_unlock(&pwm_lock);
+	return 0;
 }
-EXPORT_SYMBOL(pwm_free);
 
 static int __devinit mxc_pwm_probe(struct platform_device *pdev)
 {
+	struct mxc_pwm_device *mxc_pwm;
 	struct pwm_device *pwm;
+	struct pwm_ops *pops;
 	struct resource *r;
 	int ret = 0;
 
+	mxc_pwm = kzalloc(sizeof(struct mxc_pwm_device), GFP_KERNEL);
+	if (mxc_pwm == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
 	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
 	if (pwm == NULL) {
 		dev_err(&pdev->dev, "failed to allocate memory\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_free1;
+	}
+	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
+	if (pops == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		ret = -ENOMEM;
+		goto err_free2;
 	}
 
-	pwm->clk = clk_get(&pdev->dev, "pwm");
+	mxc_pwm->clk = clk_get(&pdev->dev, "pwm");
 
-	if (IS_ERR(pwm->clk)) {
-		ret = PTR_ERR(pwm->clk);
-		goto err_free;
+	if (IS_ERR(mxc_pwm->clk)) {
+		ret = PTR_ERR(mxc_pwm->clk);
+		goto err_free3;
 	}
 
-	pwm->clk_enabled = 0;
-
-	pwm->use_count = 0;
-	pwm->pwm_id = pdev->id;
-	pwm->pdev = pdev;
+	mxc_pwm->clk_enabled = 0;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (r == NULL) {
@@ -221,16 +181,27 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
 		goto err_free_clk;
 	}
 
-	pwm->mmio_base = ioremap(r->start, r->end - r->start + 1);
-	if (pwm->mmio_base == NULL) {
+	mxc_pwm->mmio_base = ioremap(r->start, r->end - r->start + 1);
+	if (mxc_pwm->mmio_base == NULL) {
 		dev_err(&pdev->dev, "failed to ioremap() registers\n");
 		ret = -ENODEV;
 		goto err_free_mem;
 	}
 
-	mutex_lock(&pwm_lock);
-	list_add_tail(&pwm->node, &pwm_list);
-	mutex_unlock(&pwm_lock);
+	pops->pwm_config = mxc_pwm_config;
+	pops->pwm_enable = mxc_pwm_enable;
+	pops->pwm_disable = mxc_pwm_disable;
+	pops->name = pdev->name;
+
+	pwm->pwm_id = pdev->id;
+	pwm->dev = &pdev->dev;
+	pwm->pops = pops;
+	pwm->data = mxc_pwm;
+	ret = pwm_device_register(pwm);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register pwm device\n");
+		goto err_free_mem;
+	}
 
 	platform_set_drvdata(pdev, pwm);
 	return 0;
@@ -238,33 +209,38 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
 err_free_mem:
 	release_mem_region(r->start, r->end - r->start + 1);
 err_free_clk:
-	clk_put(pwm->clk);
-err_free:
+	clk_put(mxc_pwm->clk);
+err_free3:
+	kfree(pops);
+err_free2:
 	kfree(pwm);
+err_free1:
+	kfree(mxc_pwm);
 	return ret;
 }
 
 static int __devexit mxc_pwm_remove(struct platform_device *pdev)
 {
 	struct pwm_device *pwm;
+	struct mxc_pwm_device *mxc_pwm;
 	struct resource *r;
 
 	pwm = platform_get_drvdata(pdev);
 	if (pwm == NULL)
 		return -ENODEV;
+	mxc_pwm = pwm->data;
 
-	mutex_lock(&pwm_lock);
-	list_del(&pwm->node);
-	mutex_unlock(&pwm_lock);
-
-	iounmap(pwm->mmio_base);
+	pwm_device_unregister(pwm);
+	iounmap(mxc_pwm->mmio_base);
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(r->start, r->end - r->start + 1);
 
-	clk_put(pwm->clk);
+	clk_put(mxc_pwm->clk);
 
+	kfree(pwm->pops);
 	kfree(pwm);
+	kfree(mxc_pwm);
 	return 0;
 }
 
diff --git a/arch/arm/plat-pxa/pwm.c b/arch/arm/plat-pxa/pwm.c
index ef32686..1de902a 100644
--- a/arch/arm/plat-pxa/pwm.c
+++ b/arch/arm/plat-pxa/pwm.c
@@ -43,33 +43,27 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
 #define PWMCR_SD	(1 << 6)
 #define PWMDCR_FD	(1 << 10)
 
-struct pwm_device {
-	struct list_head	node;
-	struct pwm_device	*secondary;
-	struct platform_device	*pdev;
-
-	const char	*label;
+struct pxa_pwm_device {
+	struct pxa_pwm_device *sec;
 	struct clk	*clk;
 	int		clk_enabled;
 	void __iomem	*mmio_base;
-
-	unsigned int	use_count;
-	unsigned int	pwm_id;
 };
 
 /*
  * period_ns = 10^9 * (PRESCALE + 1) * (PV + 1) / PWM_CLK_RATE
  * duty_ns   = 10^9 * (PRESCALE + 1) * DC / PWM_CLK_RATE
  */
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+int pxa_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
 	unsigned long long c;
 	unsigned long period_cycles, prescale, pv, dc;
+	struct pxa_pwm_device *pxa_pwm = pwm->data;
 
 	if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
 		return -EINVAL;
 
-	c = clk_get_rate(pwm->clk);
+	c = clk_get_rate(pxa_pwm->clk);
 	c = c * period_ns;
 	do_div(c, 1000000000);
 	period_cycles = c;
@@ -90,94 +84,45 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	/* NOTE: the clock to PWM has to be enabled first
 	 * before writing to the registers
 	 */
-	clk_enable(pwm->clk);
-	__raw_writel(prescale, pwm->mmio_base + PWMCR);
-	__raw_writel(dc, pwm->mmio_base + PWMDCR);
-	__raw_writel(pv, pwm->mmio_base + PWMPCR);
-	clk_disable(pwm->clk);
+	clk_enable(pxa_pwm->clk);
+	__raw_writel(prescale, pxa_pwm->mmio_base + PWMCR);
+	__raw_writel(dc, pxa_pwm->mmio_base + PWMDCR);
+	__raw_writel(pv, pxa_pwm->mmio_base + PWMPCR);
+	clk_disable(pxa_pwm->clk);
 
 	return 0;
 }
-EXPORT_SYMBOL(pwm_config);
 
-int pwm_enable(struct pwm_device *pwm)
+int pxa_pwm_enable(struct pwm_device *pwm)
 {
+	struct pxa_pwm_device *pxa_pwm = pwm->data;
 	int rc = 0;
 
-	if (!pwm->clk_enabled) {
-		rc = clk_enable(pwm->clk);
+	if (!pxa_pwm->clk_enabled) {
+		rc = clk_enable(pxa_pwm->clk);
 		if (!rc)
-			pwm->clk_enabled = 1;
+			pxa_pwm->clk_enabled = 1;
 	}
 	return rc;
 }
-EXPORT_SYMBOL(pwm_enable);
 
-void pwm_disable(struct pwm_device *pwm)
+int pxa_pwm_disable(struct pwm_device *pwm)
 {
-	if (pwm->clk_enabled) {
-		clk_disable(pwm->clk);
-		pwm->clk_enabled = 0;
-	}
-}
-EXPORT_SYMBOL(pwm_disable);
-
-static DEFINE_MUTEX(pwm_lock);
-static LIST_HEAD(pwm_list);
+	struct pxa_pwm_device *pxa_pwm = pwm->data;
 
-struct pwm_device *pwm_request(int pwm_id, const char *label)
-{
-	struct pwm_device *pwm;
-	int found = 0;
-
-	mutex_lock(&pwm_lock);
-
-	list_for_each_entry(pwm, &pwm_list, node) {
-		if (pwm->pwm_id == pwm_id) {
-			found = 1;
-			break;
-		}
+	if (pxa_pwm->clk_enabled) {
+		clk_disable(pxa_pwm->clk);
+		pxa_pwm->clk_enabled = 0;
 	}
-
-	if (found) {
-		if (pwm->use_count == 0) {
-			pwm->use_count++;
-			pwm->label = label;
-		} else
-			pwm = ERR_PTR(-EBUSY);
-	} else
-		pwm = ERR_PTR(-ENOENT);
-
-	mutex_unlock(&pwm_lock);
-	return pwm;
-}
-EXPORT_SYMBOL(pwm_request);
-
-void pwm_free(struct pwm_device *pwm)
-{
-	mutex_lock(&pwm_lock);
-
-	if (pwm->use_count) {
-		pwm->use_count--;
-		pwm->label = NULL;
-	} else
-		pr_warning("PWM device already freed\n");
-
-	mutex_unlock(&pwm_lock);
-}
-EXPORT_SYMBOL(pwm_free);
-
-static inline void __add_pwm(struct pwm_device *pwm)
-{
-	mutex_lock(&pwm_lock);
-	list_add_tail(&pwm->node, &pwm_list);
-	mutex_unlock(&pwm_lock);
+	return 0;
 }
 
 static int __devinit pwm_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
+	struct pxa_pwm_device *pxa_pwm, *pxa_pwm_sec;
 	struct pwm_device *pwm, *secondary = NULL;
+	struct pwm_ops *pops;
 	struct resource *r;
 	int ret = 0;
 
@@ -186,17 +131,26 @@ static int __devinit pwm_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to allocate memory\n");
 		return -ENOMEM;
 	}
+	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
+	if (pops == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		kfree(pwm);
+		return -ENOMEM;
+	}
+	pxa_pwm = kzalloc(sizeof(struct pxa_pwm_device), GFP_KERNEL);
+	if (pxa_pwm == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		kfree(pops);
+		kfree(pwm);
+		return -ENOMEM;
+	}
 
-	pwm->clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(pwm->clk)) {
-		ret = PTR_ERR(pwm->clk);
+	pxa_pwm->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pxa_pwm->clk)) {
+		ret = PTR_ERR(pxa_pwm->clk);
 		goto err_free;
 	}
-	pwm->clk_enabled = 0;
-
-	pwm->use_count = 0;
-	pwm->pwm_id = PWM_ID_BASE(id->driver_data) + pdev->id;
-	pwm->pdev = pdev;
+	pxa_pwm->clk_enabled = 0;
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (r == NULL) {
@@ -212,69 +166,105 @@ static int __devinit pwm_probe(struct platform_device *pdev)
 		goto err_free_clk;
 	}
 
-	pwm->mmio_base = ioremap(r->start, resource_size(r));
-	if (pwm->mmio_base == NULL) {
+	pxa_pwm->mmio_base = ioremap(r->start, resource_size(r));
+	if (pxa_pwm->mmio_base == NULL) {
 		dev_err(&pdev->dev, "failed to ioremap() registers\n");
 		ret = -ENODEV;
 		goto err_free_mem;
 	}
 
+	pops->pwm_config = pxa_pwm_config;
+	pops->pwm_enable = pxa_pwm_enable;
+	pops->pwm_disable = pxa_pwm_disable;
+	pops->name = pdev->name;
+
+	pwm->pwm_id = PWM_ID_BASE(id->driver_data) + pdev->id;
+	pwm->dev = &pdev->dev;
+	pwm->pops = pops;
+	pwm->data = pxa_pwm;
+
+	ret = pwm_device_register(pwm);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register pwm device\n");
+		goto err_free_mem;
+	}
+
 	if (id->driver_data & HAS_SECONDARY_PWM) {
 		secondary = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
 		if (secondary == NULL) {
 			ret = -ENOMEM;
-			goto err_free_mem;
+			goto err_pwm;
+		}
+		pxa_pwm_sec = kzalloc(sizeof(struct pxa_pwm_device),
+								GFP_KERNEL);
+		if (pxa_pwm_sec == NULL) {
+			ret = -ENOMEM;
+			goto err_free_mem2;
 		}
 
 		*secondary = *pwm;
-		pwm->secondary = secondary;
+		*pxa_pwm_sec = *pxa_pwm;
+		pxa_pwm->sec = pxa_pwm_sec;
 
 		/* registers for the second PWM has offset of 0x10 */
-		secondary->mmio_base = pwm->mmio_base + 0x10;
+		pxa_pwm_sec->mmio_base = pxa_pwm->mmio_base + 0x10;
 		secondary->pwm_id = pdev->id + 2;
-	}
+		secondary->data = pxa_pwm_sec;
 
-	__add_pwm(pwm);
-	if (secondary)
-		__add_pwm(secondary);
+		ret = pwm_device_register(secondary);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to register pwm device\n");
+			goto err_free_mem3;
+		}
+	}
 
 	platform_set_drvdata(pdev, pwm);
 	return 0;
-
+err_free_mem3:
+	kfree(pxa_pwm_sec);
+err_free_mem2:
+	kfree(secondary);
+err_pwm:
+	pwm_device_unregister(pwm);
 err_free_mem:
 	release_mem_region(r->start, resource_size(r));
 err_free_clk:
-	clk_put(pwm->clk);
+	clk_put(pxa_pwm->clk);
 err_free:
+	kfree(pxa_pwm);
+	kfree(pops);
 	kfree(pwm);
 	return ret;
 }
 
 static int __devexit pwm_remove(struct platform_device *pdev)
 {
-	struct pwm_device *pwm;
+	struct pwm_device *pwm, *secondary;
+	struct pxa_pwm_device *pxa_pwm, *pxa_pwm_sec;
 	struct resource *r;
 
 	pwm = platform_get_drvdata(pdev);
 	if (pwm == NULL)
 		return -ENODEV;
-
-	mutex_lock(&pwm_lock);
-
-	if (pwm->secondary) {
-		list_del(&pwm->secondary->node);
-		kfree(pwm->secondary);
+	pxa_pwm = pwm->data;
+	secondary = pwm_request((pdev->id + 2), pdev->name);
+	pxa_pwm_sec = secondary->data;
+
+	pwm_device_unregister(pwm);
+	iounmap(pxa_pwm->mmio_base);
+	if (secondary) {
+		pwm_device_unregister(secondary);
+		iounmap(pxa_pwm->mmio_base);
+		kfree(pxa_pwm_sec);
+		kfree(secondary);
 	}
 
-	list_del(&pwm->node);
-	mutex_unlock(&pwm_lock);
-
-	iounmap(pwm->mmio_base);
-
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(r->start, resource_size(r));
 
-	clk_put(pwm->clk);
+	clk_put(pxa_pwm->clk);
+	kfree(pxa_pwm);
+	kfree(pwm->pops);
 	kfree(pwm);
 	return 0;
 }
diff --git a/arch/arm/plat-samsung/pwm.c b/arch/arm/plat-samsung/pwm.c
index 2eeb49f..63fba01 100644
--- a/arch/arm/plat-samsung/pwm.c
+++ b/arch/arm/plat-samsung/pwm.c
@@ -26,25 +26,19 @@
 #include <plat/devs.h>
 #include <plat/regs-timer.h>
 
-struct pwm_device {
-	struct list_head	 list;
+struct s3c_pwm_device {
 	struct platform_device	*pdev;
 
 	struct clk		*clk_div;
 	struct clk		*clk;
-	const char		*label;
 
 	unsigned int		 period_ns;
 	unsigned int		 duty_ns;
 
 	unsigned char		 tcon_base;
 	unsigned char		 running;
-	unsigned char		 use_count;
-	unsigned char		 pwm_id;
 };
 
-#define pwm_dbg(_pwm, msg...) dev_dbg(&(_pwm)->pdev->dev, msg)
-
 static struct clk *clk_scaler[2];
 
 /* Standard setup for a timer block. */
@@ -78,108 +72,61 @@ struct platform_device s3c_device_timer[] = {
 	[4] = { DEFINE_S3C_TIMER(4, IRQ_TIMER4) },
 };
 
-static inline int pwm_is_tdiv(struct pwm_device *pwm)
+static inline int pwm_is_tdiv(struct s3c_pwm_device *s3c_pwm)
 {
-	return clk_get_parent(pwm->clk) == pwm->clk_div;
+	return clk_get_parent(s3c_pwm->clk) == s3c_pwm->clk_div;
 }
 
-static DEFINE_MUTEX(pwm_lock);
-static LIST_HEAD(pwm_list);
+#define pwm_tcon_start(s3c_pwm) (1 << (s3c_pwm->tcon_base + 0))
+#define pwm_tcon_invert(s3c_pwm) (1 << (s3c_pwm->tcon_base + 2))
+#define pwm_tcon_autoreload(s3c_pwm) (1 << (s3c_pwm->tcon_base + 3))
+#define pwm_tcon_manulupdate(s3c_pwm) (1 << (s3c_pwm->tcon_base + 1))
 
-struct pwm_device *pwm_request(int pwm_id, const char *label)
-{
-	struct pwm_device *pwm;
-	int found = 0;
-
-	mutex_lock(&pwm_lock);
-
-	list_for_each_entry(pwm, &pwm_list, list) {
-		if (pwm->pwm_id == pwm_id) {
-			found = 1;
-			break;
-		}
-	}
-
-	if (found) {
-		if (pwm->use_count == 0) {
-			pwm->use_count = 1;
-			pwm->label = label;
-		} else
-			pwm = ERR_PTR(-EBUSY);
-	} else
-		pwm = ERR_PTR(-ENOENT);
-
-	mutex_unlock(&pwm_lock);
-	return pwm;
-}
-
-EXPORT_SYMBOL(pwm_request);
-
-
-void pwm_free(struct pwm_device *pwm)
-{
-	mutex_lock(&pwm_lock);
-
-	if (pwm->use_count) {
-		pwm->use_count--;
-		pwm->label = NULL;
-	} else
-		printk(KERN_ERR "PWM%d device already freed\n", pwm->pwm_id);
-
-	mutex_unlock(&pwm_lock);
-}
-
-EXPORT_SYMBOL(pwm_free);
-
-#define pwm_tcon_start(pwm) (1 << (pwm->tcon_base + 0))
-#define pwm_tcon_invert(pwm) (1 << (pwm->tcon_base + 2))
-#define pwm_tcon_autoreload(pwm) (1 << (pwm->tcon_base + 3))
-#define pwm_tcon_manulupdate(pwm) (1 << (pwm->tcon_base + 1))
-
-int pwm_enable(struct pwm_device *pwm)
+int s3c_pwm_enable(struct pwm_device *pwm)
 {
 	unsigned long flags;
 	unsigned long tcon;
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 
 	local_irq_save(flags);
 
 	tcon = __raw_readl(S3C2410_TCON);
-	tcon |= pwm_tcon_start(pwm);
+	tcon |= pwm_tcon_start(s3c_pwm);
 	__raw_writel(tcon, S3C2410_TCON);
 
 	local_irq_restore(flags);
 
-	pwm->running = 1;
+	s3c_pwm->running = 1;
 	return 0;
 }
 
-EXPORT_SYMBOL(pwm_enable);
-
-void pwm_disable(struct pwm_device *pwm)
+int s3c_pwm_disable(struct pwm_device *pwm)
 {
 	unsigned long flags;
 	unsigned long tcon;
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 
 	local_irq_save(flags);
 
 	tcon = __raw_readl(S3C2410_TCON);
-	tcon &= ~pwm_tcon_start(pwm);
+	tcon &= ~pwm_tcon_start(s3c_pwm);
 	__raw_writel(tcon, S3C2410_TCON);
 
 	local_irq_restore(flags);
 
-	pwm->running = 0;
+	s3c_pwm->running = 0;
+	return 0;
 }
 
-EXPORT_SYMBOL(pwm_disable);
-
-static unsigned long pwm_calc_tin(struct pwm_device *pwm, unsigned long freq)
+static unsigned long pwm_calc_tin(struct pwm_device *pwm,
+		unsigned long freq)
 {
 	unsigned long tin_parent_rate;
 	unsigned int div;
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 
-	tin_parent_rate = clk_get_rate(clk_get_parent(pwm->clk_div));
-	pwm_dbg(pwm, "tin parent at %lu\n", tin_parent_rate);
+	tin_parent_rate = clk_get_rate(clk_get_parent(s3c_pwm->clk_div));
+	dev_dbg(pwm->dev, "tin parent at %lu\n", tin_parent_rate);
 
 	for (div = 2; div <= 16; div *= 2) {
 		if ((tin_parent_rate / (div << 16)) < freq)
@@ -191,7 +138,7 @@ static unsigned long pwm_calc_tin(struct pwm_device *pwm, unsigned long freq)
 
 #define NS_IN_HZ (1000000000UL)
 
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+int s3c_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
 	unsigned long tin_rate;
 	unsigned long tin_ns;
@@ -200,6 +147,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	unsigned long tcon;
 	unsigned long tcnt;
 	long tcmp;
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 
 	/* We currently avoid using 64bit arithmetic by using the
 	 * fact that anything faster than 1Hz is easily representable
@@ -211,8 +159,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	if (duty_ns > period_ns)
 		return -EINVAL;
 
-	if (period_ns == pwm->period_ns &&
-	    duty_ns == pwm->duty_ns)
+	if (period_ns == s3c_pwm->period_ns &&
+	    duty_ns == s3c_pwm->duty_ns)
 		return 0;
 
 	/* The TCMP and TCNT can be read without a lock, they're not
@@ -223,26 +171,26 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 
 	period = NS_IN_HZ / period_ns;
 
-	pwm_dbg(pwm, "duty_ns=%d, period_ns=%d (%lu)\n",
+	dev_dbg(pwm->dev, "duty_ns=%d, period_ns=%d (%lu)\n",
 		duty_ns, period_ns, period);
 
 	/* Check to see if we are changing the clock rate of the PWM */
 
-	if (pwm->period_ns != period_ns) {
-		if (pwm_is_tdiv(pwm)) {
+	if (s3c_pwm->period_ns != period_ns) {
+		if (pwm_is_tdiv(s3c_pwm)) {
 			tin_rate = pwm_calc_tin(pwm, period);
-			clk_set_rate(pwm->clk_div, tin_rate);
+			clk_set_rate(s3c_pwm->clk_div, tin_rate);
 		} else
-			tin_rate = clk_get_rate(pwm->clk);
+			tin_rate = clk_get_rate(s3c_pwm->clk);
 
-		pwm->period_ns = period_ns;
+		s3c_pwm->period_ns = period_ns;
 
-		pwm_dbg(pwm, "tin_rate=%lu\n", tin_rate);
+		dev_dbg(pwm->dev, "tin_rate=%lu\n", tin_rate);
 
 		tin_ns = NS_IN_HZ / tin_rate;
 		tcnt = period_ns / tin_ns;
 	} else
-		tin_ns = NS_IN_HZ / clk_get_rate(pwm->clk);
+		tin_ns = NS_IN_HZ / clk_get_rate(s3c_pwm->clk);
 
 	/* Note, counters count down */
 
@@ -253,7 +201,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	if (tcmp == tcnt)
 		tcmp--;
 
-	pwm_dbg(pwm, "tin_ns=%lu, tcmp=%ld/%lu\n", tin_ns, tcmp, tcnt);
+	dev_dbg(pwm->dev, "tin_ns=%lu, tcmp=%ld/%lu\n", tin_ns, tcmp, tcnt);
 
 	if (tcmp < 0)
 		tcmp = 0;
@@ -266,11 +214,11 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	__raw_writel(tcnt, S3C2410_TCNTB(pwm->pwm_id));
 
 	tcon = __raw_readl(S3C2410_TCON);
-	tcon |= pwm_tcon_manulupdate(pwm);
-	tcon |= pwm_tcon_autoreload(pwm);
+	tcon |= pwm_tcon_manulupdate(s3c_pwm);
+	tcon |= pwm_tcon_autoreload(s3c_pwm);
 	__raw_writel(tcon, S3C2410_TCON);
 
-	tcon &= ~pwm_tcon_manulupdate(pwm);
+	tcon &= ~pwm_tcon_manulupdate(s3c_pwm);
 	__raw_writel(tcon, S3C2410_TCON);
 
 	local_irq_restore(flags);
@@ -278,103 +226,122 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	return 0;
 }
 
-EXPORT_SYMBOL(pwm_config);
-
-static int pwm_register(struct pwm_device *pwm)
-{
-	pwm->duty_ns = -1;
-	pwm->period_ns = -1;
-
-	mutex_lock(&pwm_lock);
-	list_add_tail(&pwm->list, &pwm_list);
-	mutex_unlock(&pwm_lock);
-
-	return 0;
-}
-
 static int s3c_pwm_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct s3c_pwm_device *s3c_pwm;
 	struct pwm_device *pwm;
+	struct pwm_ops *pops;
 	unsigned long flags;
 	unsigned long tcon;
 	unsigned int id = pdev->id;
-	int ret;
+	int ret = 0;
 
 	if (id == 4) {
 		dev_err(dev, "TIMER4 is currently not supported\n");
 		return -ENXIO;
 	}
 
+	s3c_pwm = kzalloc(sizeof(struct s3c_pwm_device), GFP_KERNEL);
+	if (s3c_pwm == NULL) {
+		dev_err(dev, "failed to allocate pwm_device\n");
+		return -ENOMEM;
+	}
+	s3c_pwm->pdev = pdev;
 	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
 	if (pwm == NULL) {
 		dev_err(dev, "failed to allocate pwm_device\n");
-		return -ENOMEM;
+		goto err_alloc;
+		ret = -ENOMEM;
+	}
+	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
+	if (pops == NULL) {
+		dev_err(dev, "failed to allocate memory\n");
+		goto err_alloc1;
+		ret = -ENOMEM;
 	}
-
-	pwm->pdev = pdev;
-	pwm->pwm_id = id;
 
 	/* calculate base of control bits in TCON */
-	pwm->tcon_base = id == 0 ? 0 : (id * 4) + 4;
+	s3c_pwm->tcon_base = id == 0 ? 0 : (id * 4) + 4;
 
-	pwm->clk = clk_get(dev, "pwm-tin");
-	if (IS_ERR(pwm->clk)) {
+	s3c_pwm->clk = clk_get(dev, "pwm-tin");
+	if (IS_ERR(s3c_pwm->clk)) {
 		dev_err(dev, "failed to get pwm tin clk\n");
-		ret = PTR_ERR(pwm->clk);
-		goto err_alloc;
+		ret = PTR_ERR(s3c_pwm->clk);
+		goto err_alloc2;
 	}
 
-	pwm->clk_div = clk_get(dev, "pwm-tdiv");
-	if (IS_ERR(pwm->clk_div)) {
+	s3c_pwm->clk_div = clk_get(dev, "pwm-tdiv");
+	if (IS_ERR(s3c_pwm->clk_div)) {
 		dev_err(dev, "failed to get pwm tdiv clk\n");
-		ret = PTR_ERR(pwm->clk_div);
+		ret = PTR_ERR(s3c_pwm->clk_div);
 		goto err_clk_tin;
 	}
 
 	local_irq_save(flags);
 
 	tcon = __raw_readl(S3C2410_TCON);
-	tcon |= pwm_tcon_invert(pwm);
+	tcon |= pwm_tcon_invert(s3c_pwm);
 	__raw_writel(tcon, S3C2410_TCON);
 
 	local_irq_restore(flags);
 
+	pops->pwm_config = s3c_pwm_config;
+	pops->pwm_enable = s3c_pwm_enable;
+	pops->pwm_disable = s3c_pwm_disable;
+	pops->name = pdev->name;
+
+	pwm->dev = dev;
+	pwm->pwm_id = id;
+	pwm->pops = pops;
+	pwm->data = s3c_pwm;
 
-	ret = pwm_register(pwm);
+	s3c_pwm->duty_ns = -1;
+	s3c_pwm->period_ns = -1;
+	ret = pwm_device_register(pwm);
 	if (ret) {
 		dev_err(dev, "failed to register pwm\n");
 		goto err_clk_tdiv;
 	}
 
-	pwm_dbg(pwm, "config bits %02x\n",
-		(__raw_readl(S3C2410_TCON) >> pwm->tcon_base) & 0x0f);
+	dev_dbg(dev, "config bits %02x\n",
+		(__raw_readl(S3C2410_TCON) >> s3c_pwm->tcon_base) & 0x0f);
 
 	dev_info(dev, "tin at %lu, tdiv at %lu, tin=%sclk, base %d\n",
-		 clk_get_rate(pwm->clk),
-		 clk_get_rate(pwm->clk_div),
-		 pwm_is_tdiv(pwm) ? "div" : "ext", pwm->tcon_base);
+		 clk_get_rate(s3c_pwm->clk),
+		 clk_get_rate(s3c_pwm->clk_div),
+		 pwm_is_tdiv(s3c_pwm) ? "div" : "ext", s3c_pwm->tcon_base);
 
 	platform_set_drvdata(pdev, pwm);
 	return 0;
 
- err_clk_tdiv:
-	clk_put(pwm->clk_div);
+err_clk_tdiv:
+	clk_put(s3c_pwm->clk_div);
 
- err_clk_tin:
-	clk_put(pwm->clk);
+err_clk_tin:
+	clk_put(s3c_pwm->clk);
 
- err_alloc:
+err_alloc2:
+	kfree(pops);
+
+err_alloc1:
 	kfree(pwm);
+
+err_alloc:
+	kfree(s3c_pwm);
 	return ret;
 }
 
 static int __devexit s3c_pwm_remove(struct platform_device *pdev)
 {
 	struct pwm_device *pwm = platform_get_drvdata(pdev);
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 
-	clk_put(pwm->clk_div);
-	clk_put(pwm->clk);
+	pwm_device_unregister(pwm);
+	clk_put(s3c_pwm->clk_div);
+	clk_put(s3c_pwm->clk);
+	kfree(s3c_pwm);
+	kfree(pwm->pops);
 	kfree(pwm);
 
 	return 0;
@@ -384,13 +351,14 @@ static int __devexit s3c_pwm_remove(struct platform_device *pdev)
 static int s3c_pwm_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct pwm_device *pwm = platform_get_drvdata(pdev);
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 
 	/* No one preserve these values during suspend so reset them
 	 * Otherwise driver leaves PWM unconfigured if same values
 	 * passed to pwm_config
 	 */
-	pwm->period_ns = 0;
-	pwm->duty_ns = 0;
+	s3c_pwm->period_ns = 0;
+	s3c_pwm->duty_ns = 0;
 
 	return 0;
 }
@@ -398,11 +366,12 @@ static int s3c_pwm_suspend(struct platform_device *pdev, pm_message_t state)
 static int s3c_pwm_resume(struct platform_device *pdev)
 {
 	struct pwm_device *pwm = platform_get_drvdata(pdev);
+	struct s3c_pwm_device *s3c_pwm = pwm->data;
 	unsigned long tcon;
 
 	/* Restore invertion */
 	tcon = __raw_readl(S3C2410_TCON);
-	tcon |= pwm_tcon_invert(pwm);
+	tcon |= pwm_tcon_invert(s3c_pwm);
 	__raw_writel(tcon, S3C2410_TCON);
 
 	return 0;
diff --git a/arch/mips/jz4740/pwm.c b/arch/mips/jz4740/pwm.c
index a26a6fa..9f46767 100644
--- a/arch/mips/jz4740/pwm.c
+++ b/arch/mips/jz4740/pwm.c
@@ -152,7 +152,7 @@ int pwm_enable(struct pwm_device *pwm)
 	return 0;
 }
 
-void pwm_disable(struct pwm_device *pwm)
+int pwm_disable(struct pwm_device *pwm)
 {
 	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->id);
 
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index b0f2c00..6a6ea41 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -129,6 +129,12 @@
 #define twl_has_pwrbutton()	false
 #endif
 
+#if defined CONFIG_TWL6030_PWM
+#define twl_has_pwm()	true
+#else
+#define twl_has_pwm()	false
+#endif
+
 #define SUB_CHIP_ID0 0
 #define SUB_CHIP_ID1 1
 #define SUB_CHIP_ID2 2
@@ -825,6 +831,13 @@ add_children(struct twl4030_platform_data *pdata, unsigned long features)
 		if (IS_ERR(child))
 			return PTR_ERR(child);
 	}
+	if (twl_has_pwm()) {
+		child = add_child(SUB_CHIP_ID2, "twl6030_pwm",
+				NULL, 0,
+				false, 0, 0);
+		if (IS_ERR(child))
+			return PTR_ERR(child);
+	}
 
 	return 0;
 }
diff --git a/drivers/mfd/twl6030-pwm.c b/drivers/mfd/twl6030-pwm.c
index 5d25bdc..b78324b 100644
--- a/drivers/mfd/twl6030-pwm.c
+++ b/drivers/mfd/twl6030-pwm.c
@@ -20,8 +20,10 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/i2c/twl.h>
 #include <linux/slab.h>
+#include <linux/pwm.h>
+#include <linux/err.h>
+#include <linux/i2c/twl.h>
 
 #define LED_PWM_CTRL1	0xF4
 #define LED_PWM_CTRL2	0xF5
@@ -45,15 +47,10 @@
 
 #define PWM_CTRL2_MODE_MASK	0x3
 
-struct pwm_device {
-	const char *label;
-	unsigned int pwm_id;
-};
-
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+int twl6030_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
 	u8 duty_cycle;
-	int ret;
+	int ret = 0;
 
 	if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
 		return -EINVAL;
@@ -69,12 +66,11 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	}
 	return 0;
 }
-EXPORT_SYMBOL(pwm_config);
 
-int pwm_enable(struct pwm_device *pwm)
+int twl6030_pwm_enable(struct pwm_device *pwm)
 {
 	u8 val;
-	int ret;
+	int ret = 0;
 
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
 	if (ret < 0) {
@@ -95,18 +91,17 @@ int pwm_enable(struct pwm_device *pwm)
 	twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
 	return 0;
 }
-EXPORT_SYMBOL(pwm_enable);
 
-void pwm_disable(struct pwm_device *pwm)
+int twl6030_pwm_disable(struct pwm_device *pwm)
 {
 	u8 val;
-	int ret;
+	int ret = 0;
 
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
 	if (ret < 0) {
 		pr_err("%s: Failed to disable PWM, Error %d\n",
 			pwm->label, ret);
-		return;
+		return ret;
 	}
 
 	val &= ~PWM_CTRL2_MODE_MASK;
@@ -116,48 +111,86 @@ void pwm_disable(struct pwm_device *pwm)
 	if (ret < 0) {
 		pr_err("%s: Failed to disable PWM, Error %d\n",
 			pwm->label, ret);
-		return;
 	}
-	return;
+	return ret;
 }
-EXPORT_SYMBOL(pwm_disable);
 
-struct pwm_device *pwm_request(int pwm_id, const char *label)
+static int __devinit twl6030_pwm_probe(struct platform_device *pdev)
 {
-	u8 val;
-	int ret;
 	struct pwm_device *pwm;
+	struct pwm_ops *pops;
+	int ret;
+	u8 val;
 
 	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
 	if (pwm == NULL) {
-		pr_err("%s: failed to allocate memory\n", label);
-		return NULL;
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
+	if (pops == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		kfree(pwm);
+		return -ENOMEM;
 	}
 
-	pwm->label = label;
-	pwm->pwm_id = pwm_id;
-
+	pops->pwm_config = twl6030_pwm_config;
+	pops->pwm_enable = twl6030_pwm_enable;
+	pops->pwm_disable = twl6030_pwm_disable;
+	pops->name = &pdev->name;
+	pwm->dev = &pdev->dev;
+	pwm->pwm_id = pdev->id;
+	pwm->pops = pops;
+	ret = pwm_device_register(pwm);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register pwm device\n");
+		kfree(pwm);
+		kfree(pops);
+		return ret;
+	}
+	platform_set_drvdata(pdev, pwm);
 	/* Configure PWM */
 	val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VAC |
-		PWM_CTRL2_MODE_HW;
+							PWM_CTRL2_MODE_HW;
 
 	ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
-
 	if (ret < 0) {
-		pr_err("%s: Failed to configure PWM, Error %d\n",
-			 pwm->label, ret);
-
-		kfree(pwm);
-		return NULL;
+		dev_err(&pdev->dev, "Failed to configure PWM, Error %d\n", ret);
+		return ret;
 	}
-
-	return pwm;
+	dev_dbg(&pdev->dev, "pwm probe successful\n");
+	return ret;
 }
-EXPORT_SYMBOL(pwm_request);
 
-void pwm_free(struct pwm_device *pwm)
+static int __devexit twl6030_pwm_remove(struct platform_device *pdev)
 {
-	pwm_disable(pwm);
+	struct pwm_device *pwm = platform_get_drvdata(pdev);
+
+	pwm_device_unregister(pwm);
+	kfree(pwm->pops);
 	kfree(pwm);
+	dev_dbg(&pdev->dev, "pwm driver removed\n");
+	return 0;
 }
-EXPORT_SYMBOL(pwm_free);
+
+static struct platform_driver twl6030_pwm_driver = {
+	.driver = {
+		.name = "twl6030_pwm",
+		.owner = THIS_MODULE,
+	},
+	.probe = twl6030_pwm_probe,
+	.remove = __devexit_p(twl6030_pwm_remove),
+};
+
+static int __init twl6030_pwm_init(void)
+{
+	return platform_driver_register(&twl6030_pwm_driver);
+}
+
+static void __exit twl6030_pwm_deinit(void)
+{
+	platform_driver_unregister(&twl6030_pwm_driver);
+}
+
+subsys_initcall(twl6030_pwm_init);
+module_exit(twl6030_pwm_deinit);
diff --git a/drivers/misc/ab8500-pwm.c b/drivers/misc/ab8500-pwm.c
index 54e3d05..d2b23b6 100644
--- a/drivers/misc/ab8500-pwm.c
+++ b/drivers/misc/ab8500-pwm.c
@@ -23,16 +23,9 @@
 #define ENABLE_PWM			1
 #define DISABLE_PWM			0
 
-struct pwm_device {
-	struct device *dev;
-	struct list_head node;
-	const char *label;
-	unsigned int pwm_id;
-};
-
 static LIST_HEAD(pwm_list);
 
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+int ab8500_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 {
 	int ret = 0;
 	unsigned int higher_val, lower_val;
@@ -60,23 +53,21 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 
 	return ret;
 }
-EXPORT_SYMBOL(pwm_config);
 
-int pwm_enable(struct pwm_device *pwm)
+int ab8500_pwm_enable(struct pwm_device *pwm)
 {
 	int ret;
 
 	ret = abx500_mask_and_set_register_interruptible(pwm->dev,
 				AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
-				1 << (pwm->pwm_id-1), ENABLE_PWM);
+				1 << (pwm->pwm_id-1), 1 << (pwm->pwm_id-1));
 	if (ret < 0)
 		dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
 							pwm->label, ret);
 	return ret;
 }
-EXPORT_SYMBOL(pwm_enable);
 
-void pwm_disable(struct pwm_device *pwm)
+int ab8500_pwm_disable(struct pwm_device *pwm)
 {
 	int ret;
 
@@ -86,58 +77,56 @@ void pwm_disable(struct pwm_device *pwm)
 	if (ret < 0)
 		dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
 							pwm->label, ret);
-	return;
-}
-EXPORT_SYMBOL(pwm_disable);
-
-struct pwm_device *pwm_request(int pwm_id, const char *label)
-{
-	struct pwm_device *pwm;
-
-	list_for_each_entry(pwm, &pwm_list, node) {
-		if (pwm->pwm_id == pwm_id) {
-			pwm->label = label;
-			pwm->pwm_id = pwm_id;
-			return pwm;
-		}
-	}
-
-	return ERR_PTR(-ENOENT);
-}
-EXPORT_SYMBOL(pwm_request);
-
-void pwm_free(struct pwm_device *pwm)
-{
-	pwm_disable(pwm);
+	return ret;
 }
-EXPORT_SYMBOL(pwm_free);
 
 static int __devinit ab8500_pwm_probe(struct platform_device *pdev)
 {
-	struct pwm_device *pwm;
+	int ret = 0;
+	struct pwm_ops *pops;
+	struct pwm_device *pwm_dev;
 	/*
 	 * Nothing to be done in probe, this is required to get the
 	 * device which is required for ab8500 read and write
 	 */
-	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
-	if (pwm == NULL) {
+	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
+	if (pops == NULL) {
 		dev_err(&pdev->dev, "failed to allocate memory\n");
 		return -ENOMEM;
 	}
-	pwm->dev = &pdev->dev;
-	pwm->pwm_id = pdev->id;
-	list_add_tail(&pwm->node, &pwm_list);
-	platform_set_drvdata(pdev, pwm);
-	dev_dbg(pwm->dev, "pwm probe successful\n");
-	return 0;
+	pwm_dev = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
+	if (pwm_dev == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		kfree(pops);
+		return -ENOMEM;
+	}
+	pops->pwm_config = ab8500_pwm_config;
+	pops->pwm_enable = ab8500_pwm_enable;
+	pops->pwm_disable = ab8500_pwm_disable;
+	pops->name = "ab8500";
+	pwm_dev->dev = &pdev->dev;
+	pwm_dev->pwm_id = pdev->id;
+	pwm_dev->pops = pops;
+	ret = pwm_device_register(pwm_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register pwm device\n");
+		kfree(pwm_dev);
+		kfree(pops);
+		return ret;
+	}
+	platform_set_drvdata(pdev, pwm_dev);
+	dev_dbg(&pdev->dev, "pwm probe successful\n");
+	return ret;
 }
 
 static int __devexit ab8500_pwm_remove(struct platform_device *pdev)
 {
-	struct pwm_device *pwm = platform_get_drvdata(pdev);
-	list_del(&pwm->node);
+	struct pwm_device *pwm_dev = platform_get_drvdata(pdev);
+
+	pwm_device_unregister(pwm_dev);
 	dev_dbg(&pdev->dev, "pwm driver removed\n");
-	kfree(pwm);
+	kfree(pwm_dev->pops);
+	kfree(pwm_dev);
 	return 0;
 }
 
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 5d10106..a88640c 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -4,6 +4,7 @@
 
 menuconfig PWM_DEVICES
 	bool "PWM devices"
+	depends on ARM
 	default y
 	---help---
 	  Say Y here to get to see options for device drivers from various
diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
index b84027a..3a0d426 100644
--- a/drivers/pwm/pwm-core.c
+++ b/drivers/pwm/pwm-core.c
@@ -11,11 +11,6 @@
 #include <linux/err.h>
 #include <linux/pwm.h>
 
-struct pwm_device {
-	struct pwm_ops *pops;
-	int pwm_id;
-};
-
 struct pwm_dev_info {
 	struct pwm_device *pwm_dev;
 	struct list_head list;
@@ -40,9 +35,9 @@ int pwm_enable(struct pwm_device *pwm)
 }
 EXPORT_SYMBOL(pwm_enable);
 
-void pwm_disable(struct pwm_device *pwm)
+int pwm_disable(struct pwm_device *pwm)
 {
-	pwm->pops->pwm_disable(pwm);
+	return pwm->pops->pwm_disable(pwm);
 }
 EXPORT_SYMBOL(pwm_disable);
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 6e7da1f..4344c0b 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -1,14 +1,29 @@
 #ifndef __LINUX_PWM_H
 #define __LINUX_PWM_H
 
-struct pwm_device;
+/*
+ * TODO: #if defined CONFIG_PWM_CORE has to be removed after mips jz4740
+ * pwm driver aligning with pwm-core.c driver.
+ */
+#if defined CONFIG_PWM_CORE
+struct pwm_device {
+	struct pwm_ops *pops;
+	struct device *dev;
+	struct list_head node;
+	const char *label;
+	unsigned int pwm_id;
+	void *data;
+};
 
 struct pwm_ops {
 	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int period_ns);
 	int (*pwm_enable)(struct pwm_device *pwm);
 	int (*pwm_disable)(struct pwm_device *pwm);
-	char *name;
+	const char *name;
 };
+#else
+struct pwm_device;
+#endif
 
 /*
  * pwm_request - request a PWM device
@@ -33,7 +48,7 @@ int pwm_enable(struct pwm_device *pwm);
 /*
  * pwm_disable - stop a PWM output toggling
  */
-void pwm_disable(struct pwm_device *pwm);
+int pwm_disable(struct pwm_device *pwm);
 
 int pwm_device_register(struct pwm_device *pwm_dev);
 int pwm_device_unregister(struct pwm_device *pwm_dev);
-- 
1.7.2.dirty

[PATCH 4/7] pwm: Align existing pwm drivers with pwm-core driver

[Lars-Peter Clausen]

Arun Murthy wrote:
> pwm-core: make the driver visible for ARM only
> 
> 	Align ab8500 pwm with the pwm core driver
> 	Align twl6030 pwm driver with pwm core driver
> 	Align Freescale mxc pwm driver with pwm core driver
> 	Align pxa pwm driver with pwm core driver
> 	Align samsung(s3c) pwm driver with pwm core driver
> 
> mips-jz4740: pwm: Align with new pwm core driver
> 
> PWM core driver has been added and has been enabled only for ARM
> platform. The same can be utilised for mips also.
> Please align with the pwm core driver(drivers/pwm-core.c).


Is there any reason for artificially limiting it to ARM?

> 
> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
>  arch/arm/plat-mxc/pwm.c     |  166 +++++++++++++-----------------
>  arch/arm/plat-pxa/pwm.c     |  210 ++++++++++++++++++--------------------
>  arch/arm/plat-samsung/pwm.c |  235 +++++++++++++++++++------------------------
>  arch/mips/jz4740/pwm.c      |    2 +-
>  drivers/mfd/twl-core.c      |   13 +++
>  drivers/mfd/twl6030-pwm.c   |  111 +++++++++++++-------
>  drivers/misc/ab8500-pwm.c   |   87 +++++++---------
>  drivers/pwm/Kconfig         |    1 +
>  drivers/pwm/pwm-core.c      |    9 +--
>  include/linux/pwm.h         |   21 ++++-
>  10 files changed, 418 insertions(+), 437 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
> index c36f263..b259ba9 100644
> --- a/arch/arm/plat-mxc/pwm.c
> +++ b/arch/arm/plat-mxc/pwm.c
> @@ -38,22 +38,16 @@
>  
>  
>  
> -struct pwm_device {
> -	struct list_head	node;
> -	struct platform_device *pdev;
> -
> -	const char	*label;
> +struct mxc_pwm_device {
>  	struct clk	*clk;
> -
>  	int		clk_enabled;
>  	void __iomem	*mmio_base;
> -
> -	unsigned int	use_count;
> -	unsigned int	pwm_id;
>  };
>  
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +static int mxc_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  {
> +	struct mxc_pwm_device *mxc_pwm = pwm->data;
> +
>  	if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
>  		return -EINVAL;
>  
> @@ -62,7 +56,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  		unsigned long period_cycles, duty_cycles, prescale;
>  		u32 cr;
>  
> -		c = clk_get_rate(pwm->clk);
> +		c = clk_get_rate(mxc_pwm->clk);
>  		c = c * period_ns;
>  		do_div(c, 1000000000);
>  		period_cycles = c;
> @@ -74,8 +68,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  		do_div(c, period_ns);
>  		duty_cycles = c;
>  
> -		writel(duty_cycles, pwm->mmio_base + MX3_PWMSAR);
> -		writel(period_cycles, pwm->mmio_base + MX3_PWMPR);
> +		writel(duty_cycles, mxc_pwm->mmio_base + MX3_PWMSAR);
> +		writel(period_cycles, mxc_pwm->mmio_base + MX3_PWMPR);
>  
>  		cr = MX3_PWMCR_PRESCALER(prescale) | MX3_PWMCR_EN;
>  
> @@ -84,7 +78,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  		else
>  			cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
>  
> -		writel(cr, pwm->mmio_base + MX3_PWMCR);
> +		writel(cr, mxc_pwm->mmio_base + MX3_PWMCR);
>  	} else if (cpu_is_mx1() || cpu_is_mx21()) {
>  		/* The PWM subsystem allows for exact frequencies. However,
>  		 * I cannot connect a scope on my device to the PWM line and
> @@ -102,110 +96,76 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  		 * both the prescaler (/1 .. /128) and then by CLKSEL
>  		 * (/2 .. /16).
>  		 */
> -		u32 max = readl(pwm->mmio_base + MX1_PWMP);
> +		u32 max = readl(mxc_pwm->mmio_base + MX1_PWMP);
>  		u32 p = max * duty_ns / period_ns;
> -		writel(max - p, pwm->mmio_base + MX1_PWMS);
> +		writel(max - p, mxc_pwm->mmio_base + MX1_PWMS);
>  	} else {
>  		BUG();
>  	}
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(pwm_config);
>  
> -int pwm_enable(struct pwm_device *pwm)
> +static int mxc_pwm_enable(struct pwm_device *pwm)
>  {
> +	struct mxc_pwm_device *mxc_pwm = pwm->data;
>  	int rc = 0;
>  
> -	if (!pwm->clk_enabled) {
> -		rc = clk_enable(pwm->clk);
> +	if (!mxc_pwm->clk_enabled) {
> +		rc = clk_enable(mxc_pwm->clk);
>  		if (!rc)
> -			pwm->clk_enabled = 1;
> +			mxc_pwm->clk_enabled = 1;
>  	}
>  	return rc;
>  }
> -EXPORT_SYMBOL(pwm_enable);
> -
> -void pwm_disable(struct pwm_device *pwm)
> -{
> -	writel(0, pwm->mmio_base + MX3_PWMCR);
> -
> -	if (pwm->clk_enabled) {
> -		clk_disable(pwm->clk);
> -		pwm->clk_enabled = 0;
> -	}
> -}
> -EXPORT_SYMBOL(pwm_disable);
> -
> -static DEFINE_MUTEX(pwm_lock);
> -static LIST_HEAD(pwm_list);
>  
> -struct pwm_device *pwm_request(int pwm_id, const char *label)
> +static int mxc_pwm_disable(struct pwm_device *pwm)
>  {
> -	struct pwm_device *pwm;
> -	int found = 0;
> +	struct mxc_pwm_device *mxc_pwm = pwm->data;
>  
> -	mutex_lock(&pwm_lock);
> +	writel(0, mxc_pwm->mmio_base + MX3_PWMCR);
>  
> -	list_for_each_entry(pwm, &pwm_list, node) {
> -		if (pwm->pwm_id == pwm_id) {
> -			found = 1;
> -			break;
> -		}
> +	if (mxc_pwm->clk_enabled) {
> +		clk_disable(mxc_pwm->clk);
> +		mxc_pwm->clk_enabled = 0;
>  	}
> -
> -	if (found) {
> -		if (pwm->use_count == 0) {
> -			pwm->use_count++;
> -			pwm->label = label;
> -		} else
> -			pwm = ERR_PTR(-EBUSY);
> -	} else
> -		pwm = ERR_PTR(-ENOENT);
> -
> -	mutex_unlock(&pwm_lock);
> -	return pwm;
> -}
> -EXPORT_SYMBOL(pwm_request);
> -
> -void pwm_free(struct pwm_device *pwm)
> -{
> -	mutex_lock(&pwm_lock);
> -
> -	if (pwm->use_count) {
> -		pwm->use_count--;
> -		pwm->label = NULL;
> -	} else
> -		pr_warning("PWM device already freed\n");
> -
> -	mutex_unlock(&pwm_lock);
> +	return 0;
>  }
> -EXPORT_SYMBOL(pwm_free);
>  
>  static int __devinit mxc_pwm_probe(struct platform_device *pdev)
>  {
> +	struct mxc_pwm_device *mxc_pwm;
>  	struct pwm_device *pwm;
> +	struct pwm_ops *pops;
>  	struct resource *r;
>  	int ret = 0;
>  
> +	mxc_pwm = kzalloc(sizeof(struct mxc_pwm_device), GFP_KERNEL);
> +	if (mxc_pwm == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		return -ENOMEM;
> +	}
>  	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
>  	if (pwm == NULL) {
>  		dev_err(&pdev->dev, "failed to allocate memory\n");
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto err_free1;
> +	}
> +	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> +	if (pops == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		ret = -ENOMEM;
> +		goto err_free2;
>  	}
>  
> -	pwm->clk = clk_get(&pdev->dev, "pwm");
> +	mxc_pwm->clk = clk_get(&pdev->dev, "pwm");
>  
> -	if (IS_ERR(pwm->clk)) {
> -		ret = PTR_ERR(pwm->clk);
> -		goto err_free;
> +	if (IS_ERR(mxc_pwm->clk)) {
> +		ret = PTR_ERR(mxc_pwm->clk);
> +		goto err_free3;
>  	}
>  
> -	pwm->clk_enabled = 0;
> -
> -	pwm->use_count = 0;
> -	pwm->pwm_id = pdev->id;
> -	pwm->pdev = pdev;
> +	mxc_pwm->clk_enabled = 0;
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (r == NULL) {
> @@ -221,16 +181,27 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
>  		goto err_free_clk;
>  	}
>  
> -	pwm->mmio_base = ioremap(r->start, r->end - r->start + 1);
> -	if (pwm->mmio_base == NULL) {
> +	mxc_pwm->mmio_base = ioremap(r->start, r->end - r->start + 1);
> +	if (mxc_pwm->mmio_base == NULL) {
>  		dev_err(&pdev->dev, "failed to ioremap() registers\n");
>  		ret = -ENODEV;
>  		goto err_free_mem;
>  	}
>  
> -	mutex_lock(&pwm_lock);
> -	list_add_tail(&pwm->node, &pwm_list);
> -	mutex_unlock(&pwm_lock);
> +	pops->pwm_config = mxc_pwm_config;
> +	pops->pwm_enable = mxc_pwm_enable;
> +	pops->pwm_disable = mxc_pwm_disable;
> +	pops->name = pdev->name;
> +
> +	pwm->pwm_id = pdev->id;
> +	pwm->dev = &pdev->dev;
> +	pwm->pops = pops;
> +	pwm->data = mxc_pwm;
> +	ret = pwm_device_register(pwm);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register pwm device\n");
> +		goto err_free_mem;
> +	}
>  
>  	platform_set_drvdata(pdev, pwm);
>  	return 0;
> @@ -238,33 +209,38 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
>  err_free_mem:
>  	release_mem_region(r->start, r->end - r->start + 1);
>  err_free_clk:
> -	clk_put(pwm->clk);
> -err_free:
> +	clk_put(mxc_pwm->clk);
> +err_free3:
> +	kfree(pops);
> +err_free2:
>  	kfree(pwm);
> +err_free1:
> +	kfree(mxc_pwm);
>  	return ret;
>  }
>  
>  static int __devexit mxc_pwm_remove(struct platform_device *pdev)
>  {
>  	struct pwm_device *pwm;
> +	struct mxc_pwm_device *mxc_pwm;
>  	struct resource *r;
>  
>  	pwm = platform_get_drvdata(pdev);
>  	if (pwm == NULL)
>  		return -ENODEV;
> +	mxc_pwm = pwm->data;
>  
> -	mutex_lock(&pwm_lock);
> -	list_del(&pwm->node);
> -	mutex_unlock(&pwm_lock);
> -
> -	iounmap(pwm->mmio_base);
> +	pwm_device_unregister(pwm);
> +	iounmap(mxc_pwm->mmio_base);
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(r->start, r->end - r->start + 1);
>  
> -	clk_put(pwm->clk);
> +	clk_put(mxc_pwm->clk);
>  
> +	kfree(pwm->pops);
>  	kfree(pwm);
> +	kfree(mxc_pwm);
>  	return 0;
>  }
>  
> diff --git a/arch/arm/plat-pxa/pwm.c b/arch/arm/plat-pxa/pwm.c
> index ef32686..1de902a 100644
> --- a/arch/arm/plat-pxa/pwm.c
> +++ b/arch/arm/plat-pxa/pwm.c
> @@ -43,33 +43,27 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
>  #define PWMCR_SD	(1 << 6)
>  #define PWMDCR_FD	(1 << 10)
>  
> -struct pwm_device {
> -	struct list_head	node;
> -	struct pwm_device	*secondary;
> -	struct platform_device	*pdev;
> -
> -	const char	*label;
> +struct pxa_pwm_device {
> +	struct pxa_pwm_device *sec;
>  	struct clk	*clk;
>  	int		clk_enabled;
>  	void __iomem	*mmio_base;
> -
> -	unsigned int	use_count;
> -	unsigned int	pwm_id;
>  };
>  
>  /*
>   * period_ns = 10^9 * (PRESCALE + 1) * (PV + 1) / PWM_CLK_RATE
>   * duty_ns   = 10^9 * (PRESCALE + 1) * DC / PWM_CLK_RATE
>   */
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +int pxa_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  {
>  	unsigned long long c;
>  	unsigned long period_cycles, prescale, pv, dc;
> +	struct pxa_pwm_device *pxa_pwm = pwm->data;
>  
>  	if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
>  		return -EINVAL;
>  
> -	c = clk_get_rate(pwm->clk);
> +	c = clk_get_rate(pxa_pwm->clk);
>  	c = c * period_ns;
>  	do_div(c, 1000000000);
>  	period_cycles = c;
> @@ -90,94 +84,45 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	/* NOTE: the clock to PWM has to be enabled first
>  	 * before writing to the registers
>  	 */
> -	clk_enable(pwm->clk);
> -	__raw_writel(prescale, pwm->mmio_base + PWMCR);
> -	__raw_writel(dc, pwm->mmio_base + PWMDCR);
> -	__raw_writel(pv, pwm->mmio_base + PWMPCR);
> -	clk_disable(pwm->clk);
> +	clk_enable(pxa_pwm->clk);
> +	__raw_writel(prescale, pxa_pwm->mmio_base + PWMCR);
> +	__raw_writel(dc, pxa_pwm->mmio_base + PWMDCR);
> +	__raw_writel(pv, pxa_pwm->mmio_base + PWMPCR);
> +	clk_disable(pxa_pwm->clk);
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL(pwm_config);
>  
> -int pwm_enable(struct pwm_device *pwm)
> +int pxa_pwm_enable(struct pwm_device *pwm)
>  {
> +	struct pxa_pwm_device *pxa_pwm = pwm->data;
>  	int rc = 0;
>  
> -	if (!pwm->clk_enabled) {
> -		rc = clk_enable(pwm->clk);
> +	if (!pxa_pwm->clk_enabled) {
> +		rc = clk_enable(pxa_pwm->clk);
>  		if (!rc)
> -			pwm->clk_enabled = 1;
> +			pxa_pwm->clk_enabled = 1;
>  	}
>  	return rc;
>  }
> -EXPORT_SYMBOL(pwm_enable);
>  
> -void pwm_disable(struct pwm_device *pwm)
> +int pxa_pwm_disable(struct pwm_device *pwm)
>  {
> -	if (pwm->clk_enabled) {
> -		clk_disable(pwm->clk);
> -		pwm->clk_enabled = 0;
> -	}
> -}
> -EXPORT_SYMBOL(pwm_disable);
> -
> -static DEFINE_MUTEX(pwm_lock);
> -static LIST_HEAD(pwm_list);
> +	struct pxa_pwm_device *pxa_pwm = pwm->data;
>  
> -struct pwm_device *pwm_request(int pwm_id, const char *label)
> -{
> -	struct pwm_device *pwm;
> -	int found = 0;
> -
> -	mutex_lock(&pwm_lock);
> -
> -	list_for_each_entry(pwm, &pwm_list, node) {
> -		if (pwm->pwm_id == pwm_id) {
> -			found = 1;
> -			break;
> -		}
> +	if (pxa_pwm->clk_enabled) {
> +		clk_disable(pxa_pwm->clk);
> +		pxa_pwm->clk_enabled = 0;
>  	}
> -
> -	if (found) {
> -		if (pwm->use_count == 0) {
> -			pwm->use_count++;
> -			pwm->label = label;
> -		} else
> -			pwm = ERR_PTR(-EBUSY);
> -	} else
> -		pwm = ERR_PTR(-ENOENT);
> -
> -	mutex_unlock(&pwm_lock);
> -	return pwm;
> -}
> -EXPORT_SYMBOL(pwm_request);
> -
> -void pwm_free(struct pwm_device *pwm)
> -{
> -	mutex_lock(&pwm_lock);
> -
> -	if (pwm->use_count) {
> -		pwm->use_count--;
> -		pwm->label = NULL;
> -	} else
> -		pr_warning("PWM device already freed\n");
> -
> -	mutex_unlock(&pwm_lock);
> -}
> -EXPORT_SYMBOL(pwm_free);
> -
> -static inline void __add_pwm(struct pwm_device *pwm)
> -{
> -	mutex_lock(&pwm_lock);
> -	list_add_tail(&pwm->node, &pwm_list);
> -	mutex_unlock(&pwm_lock);
> +	return 0;
>  }
>  
>  static int __devinit pwm_probe(struct platform_device *pdev)
>  {
>  	const struct platform_device_id *id = platform_get_device_id(pdev);
> +	struct pxa_pwm_device *pxa_pwm, *pxa_pwm_sec;
>  	struct pwm_device *pwm, *secondary = NULL;
> +	struct pwm_ops *pops;
>  	struct resource *r;
>  	int ret = 0;
>  
> @@ -186,17 +131,26 @@ static int __devinit pwm_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "failed to allocate memory\n");
>  		return -ENOMEM;
>  	}
> +	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> +	if (pops == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		kfree(pwm);
> +		return -ENOMEM;
> +	}
> +	pxa_pwm = kzalloc(sizeof(struct pxa_pwm_device), GFP_KERNEL);
> +	if (pxa_pwm == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		kfree(pops);
> +		kfree(pwm);
> +		return -ENOMEM;
> +	}
>  
> -	pwm->clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(pwm->clk)) {
> -		ret = PTR_ERR(pwm->clk);
> +	pxa_pwm->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(pxa_pwm->clk)) {
> +		ret = PTR_ERR(pxa_pwm->clk);
>  		goto err_free;
>  	}
> -	pwm->clk_enabled = 0;
> -
> -	pwm->use_count = 0;
> -	pwm->pwm_id = PWM_ID_BASE(id->driver_data) + pdev->id;
> -	pwm->pdev = pdev;
> +	pxa_pwm->clk_enabled = 0;
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (r == NULL) {
> @@ -212,69 +166,105 @@ static int __devinit pwm_probe(struct platform_device *pdev)
>  		goto err_free_clk;
>  	}
>  
> -	pwm->mmio_base = ioremap(r->start, resource_size(r));
> -	if (pwm->mmio_base == NULL) {
> +	pxa_pwm->mmio_base = ioremap(r->start, resource_size(r));
> +	if (pxa_pwm->mmio_base == NULL) {
>  		dev_err(&pdev->dev, "failed to ioremap() registers\n");
>  		ret = -ENODEV;
>  		goto err_free_mem;
>  	}
>  
> +	pops->pwm_config = pxa_pwm_config;
> +	pops->pwm_enable = pxa_pwm_enable;
> +	pops->pwm_disable = pxa_pwm_disable;
> +	pops->name = pdev->name;
> +
> +	pwm->pwm_id = PWM_ID_BASE(id->driver_data) + pdev->id;
> +	pwm->dev = &pdev->dev;
> +	pwm->pops = pops;
> +	pwm->data = pxa_pwm;
> +
> +	ret = pwm_device_register(pwm);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register pwm device\n");
> +		goto err_free_mem;
> +	}
> +
>  	if (id->driver_data & HAS_SECONDARY_PWM) {
>  		secondary = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
>  		if (secondary == NULL) {
>  			ret = -ENOMEM;
> -			goto err_free_mem;
> +			goto err_pwm;
> +		}
> +		pxa_pwm_sec = kzalloc(sizeof(struct pxa_pwm_device),
> +								GFP_KERNEL);
> +		if (pxa_pwm_sec == NULL) {
> +			ret = -ENOMEM;
> +			goto err_free_mem2;
>  		}
>  
>  		*secondary = *pwm;
> -		pwm->secondary = secondary;
> +		*pxa_pwm_sec = *pxa_pwm;
> +		pxa_pwm->sec = pxa_pwm_sec;
>  
>  		/* registers for the second PWM has offset of 0x10 */
> -		secondary->mmio_base = pwm->mmio_base + 0x10;
> +		pxa_pwm_sec->mmio_base = pxa_pwm->mmio_base + 0x10;
>  		secondary->pwm_id = pdev->id + 2;
> -	}
> +		secondary->data = pxa_pwm_sec;
>  
> -	__add_pwm(pwm);
> -	if (secondary)
> -		__add_pwm(secondary);
> +		ret = pwm_device_register(secondary);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "failed to register pwm device\n");
> +			goto err_free_mem3;
> +		}
> +	}
>  
>  	platform_set_drvdata(pdev, pwm);
>  	return 0;
> -
> +err_free_mem3:
> +	kfree(pxa_pwm_sec);
> +err_free_mem2:
> +	kfree(secondary);
> +err_pwm:
> +	pwm_device_unregister(pwm);
>  err_free_mem:
>  	release_mem_region(r->start, resource_size(r));
>  err_free_clk:
> -	clk_put(pwm->clk);
> +	clk_put(pxa_pwm->clk);
>  err_free:
> +	kfree(pxa_pwm);
> +	kfree(pops);
>  	kfree(pwm);
>  	return ret;
>  }
>  
>  static int __devexit pwm_remove(struct platform_device *pdev)
>  {
> -	struct pwm_device *pwm;
> +	struct pwm_device *pwm, *secondary;
> +	struct pxa_pwm_device *pxa_pwm, *pxa_pwm_sec;
>  	struct resource *r;
>  
>  	pwm = platform_get_drvdata(pdev);
>  	if (pwm == NULL)
>  		return -ENODEV;
> -
> -	mutex_lock(&pwm_lock);
> -
> -	if (pwm->secondary) {
> -		list_del(&pwm->secondary->node);
> -		kfree(pwm->secondary);
> +	pxa_pwm = pwm->data;
> +	secondary = pwm_request((pdev->id + 2), pdev->name);
> +	pxa_pwm_sec = secondary->data;
> +
> +	pwm_device_unregister(pwm);
> +	iounmap(pxa_pwm->mmio_base);
> +	if (secondary) {
> +		pwm_device_unregister(secondary);
> +		iounmap(pxa_pwm->mmio_base);
> +		kfree(pxa_pwm_sec);
> +		kfree(secondary);
>  	}
>  
> -	list_del(&pwm->node);
> -	mutex_unlock(&pwm_lock);
> -
> -	iounmap(pwm->mmio_base);
> -
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(r->start, resource_size(r));
>  
> -	clk_put(pwm->clk);
> +	clk_put(pxa_pwm->clk);
> +	kfree(pxa_pwm);
> +	kfree(pwm->pops);
>  	kfree(pwm);
>  	return 0;
>  }
> diff --git a/arch/arm/plat-samsung/pwm.c b/arch/arm/plat-samsung/pwm.c
> index 2eeb49f..63fba01 100644
> --- a/arch/arm/plat-samsung/pwm.c
> +++ b/arch/arm/plat-samsung/pwm.c
> @@ -26,25 +26,19 @@
>  #include <plat/devs.h>
>  #include <plat/regs-timer.h>
>  
> -struct pwm_device {
> -	struct list_head	 list;
> +struct s3c_pwm_device {
>  	struct platform_device	*pdev;
>  
>  	struct clk		*clk_div;
>  	struct clk		*clk;
> -	const char		*label;
>  
>  	unsigned int		 period_ns;
>  	unsigned int		 duty_ns;
>  
>  	unsigned char		 tcon_base;
>  	unsigned char		 running;
> -	unsigned char		 use_count;
> -	unsigned char		 pwm_id;
>  };
>  
> -#define pwm_dbg(_pwm, msg...) dev_dbg(&(_pwm)->pdev->dev, msg)
> -
>  static struct clk *clk_scaler[2];
>  
>  /* Standard setup for a timer block. */
> @@ -78,108 +72,61 @@ struct platform_device s3c_device_timer[] = {
>  	[4] = { DEFINE_S3C_TIMER(4, IRQ_TIMER4) },
>  };
>  
> -static inline int pwm_is_tdiv(struct pwm_device *pwm)
> +static inline int pwm_is_tdiv(struct s3c_pwm_device *s3c_pwm)
>  {
> -	return clk_get_parent(pwm->clk) == pwm->clk_div;
> +	return clk_get_parent(s3c_pwm->clk) == s3c_pwm->clk_div;
>  }
>  
> -static DEFINE_MUTEX(pwm_lock);
> -static LIST_HEAD(pwm_list);
> +#define pwm_tcon_start(s3c_pwm) (1 << (s3c_pwm->tcon_base + 0))
> +#define pwm_tcon_invert(s3c_pwm) (1 << (s3c_pwm->tcon_base + 2))
> +#define pwm_tcon_autoreload(s3c_pwm) (1 << (s3c_pwm->tcon_base + 3))
> +#define pwm_tcon_manulupdate(s3c_pwm) (1 << (s3c_pwm->tcon_base + 1))
>  
> -struct pwm_device *pwm_request(int pwm_id, const char *label)
> -{
> -	struct pwm_device *pwm;
> -	int found = 0;
> -
> -	mutex_lock(&pwm_lock);
> -
> -	list_for_each_entry(pwm, &pwm_list, list) {
> -		if (pwm->pwm_id == pwm_id) {
> -			found = 1;
> -			break;
> -		}
> -	}
> -
> -	if (found) {
> -		if (pwm->use_count == 0) {
> -			pwm->use_count = 1;
> -			pwm->label = label;
> -		} else
> -			pwm = ERR_PTR(-EBUSY);
> -	} else
> -		pwm = ERR_PTR(-ENOENT);
> -
> -	mutex_unlock(&pwm_lock);
> -	return pwm;
> -}
> -
> -EXPORT_SYMBOL(pwm_request);
> -
> -
> -void pwm_free(struct pwm_device *pwm)
> -{
> -	mutex_lock(&pwm_lock);
> -
> -	if (pwm->use_count) {
> -		pwm->use_count--;
> -		pwm->label = NULL;
> -	} else
> -		printk(KERN_ERR "PWM%d device already freed\n", pwm->pwm_id);
> -
> -	mutex_unlock(&pwm_lock);
> -}
> -
> -EXPORT_SYMBOL(pwm_free);
> -
> -#define pwm_tcon_start(pwm) (1 << (pwm->tcon_base + 0))
> -#define pwm_tcon_invert(pwm) (1 << (pwm->tcon_base + 2))
> -#define pwm_tcon_autoreload(pwm) (1 << (pwm->tcon_base + 3))
> -#define pwm_tcon_manulupdate(pwm) (1 << (pwm->tcon_base + 1))
> -
> -int pwm_enable(struct pwm_device *pwm)
> +int s3c_pwm_enable(struct pwm_device *pwm)
>  {
>  	unsigned long flags;
>  	unsigned long tcon;
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  
>  	local_irq_save(flags);
>  
>  	tcon = __raw_readl(S3C2410_TCON);
> -	tcon |= pwm_tcon_start(pwm);
> +	tcon |= pwm_tcon_start(s3c_pwm);
>  	__raw_writel(tcon, S3C2410_TCON);
>  
>  	local_irq_restore(flags);
>  
> -	pwm->running = 1;
> +	s3c_pwm->running = 1;
>  	return 0;
>  }
>  
> -EXPORT_SYMBOL(pwm_enable);
> -
> -void pwm_disable(struct pwm_device *pwm)
> +int s3c_pwm_disable(struct pwm_device *pwm)
>  {
>  	unsigned long flags;
>  	unsigned long tcon;
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  
>  	local_irq_save(flags);
>  
>  	tcon = __raw_readl(S3C2410_TCON);
> -	tcon &= ~pwm_tcon_start(pwm);
> +	tcon &= ~pwm_tcon_start(s3c_pwm);
>  	__raw_writel(tcon, S3C2410_TCON);
>  
>  	local_irq_restore(flags);
>  
> -	pwm->running = 0;
> +	s3c_pwm->running = 0;
> +	return 0;
>  }
>  
> -EXPORT_SYMBOL(pwm_disable);
> -
> -static unsigned long pwm_calc_tin(struct pwm_device *pwm, unsigned long freq)
> +static unsigned long pwm_calc_tin(struct pwm_device *pwm,
> +		unsigned long freq)
>  {
>  	unsigned long tin_parent_rate;
>  	unsigned int div;
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  
> -	tin_parent_rate = clk_get_rate(clk_get_parent(pwm->clk_div));
> -	pwm_dbg(pwm, "tin parent at %lu\n", tin_parent_rate);
> +	tin_parent_rate = clk_get_rate(clk_get_parent(s3c_pwm->clk_div));
> +	dev_dbg(pwm->dev, "tin parent at %lu\n", tin_parent_rate);
>  
>  	for (div = 2; div <= 16; div *= 2) {
>  		if ((tin_parent_rate / (div << 16)) < freq)
> @@ -191,7 +138,7 @@ static unsigned long pwm_calc_tin(struct pwm_device *pwm, unsigned long freq)
>  
>  #define NS_IN_HZ (1000000000UL)
>  
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +int s3c_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  {
>  	unsigned long tin_rate;
>  	unsigned long tin_ns;
> @@ -200,6 +147,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	unsigned long tcon;
>  	unsigned long tcnt;
>  	long tcmp;
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  
>  	/* We currently avoid using 64bit arithmetic by using the
>  	 * fact that anything faster than 1Hz is easily representable
> @@ -211,8 +159,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	if (duty_ns > period_ns)
>  		return -EINVAL;
>  
> -	if (period_ns == pwm->period_ns &&
> -	    duty_ns == pwm->duty_ns)
> +	if (period_ns == s3c_pwm->period_ns &&
> +	    duty_ns == s3c_pwm->duty_ns)
>  		return 0;
>  
>  	/* The TCMP and TCNT can be read without a lock, they're not
> @@ -223,26 +171,26 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  
>  	period = NS_IN_HZ / period_ns;
>  
> -	pwm_dbg(pwm, "duty_ns=%d, period_ns=%d (%lu)\n",
> +	dev_dbg(pwm->dev, "duty_ns=%d, period_ns=%d (%lu)\n",
>  		duty_ns, period_ns, period);
>  
>  	/* Check to see if we are changing the clock rate of the PWM */
>  
> -	if (pwm->period_ns != period_ns) {
> -		if (pwm_is_tdiv(pwm)) {
> +	if (s3c_pwm->period_ns != period_ns) {
> +		if (pwm_is_tdiv(s3c_pwm)) {
>  			tin_rate = pwm_calc_tin(pwm, period);
> -			clk_set_rate(pwm->clk_div, tin_rate);
> +			clk_set_rate(s3c_pwm->clk_div, tin_rate);
>  		} else
> -			tin_rate = clk_get_rate(pwm->clk);
> +			tin_rate = clk_get_rate(s3c_pwm->clk);
>  
> -		pwm->period_ns = period_ns;
> +		s3c_pwm->period_ns = period_ns;
>  
> -		pwm_dbg(pwm, "tin_rate=%lu\n", tin_rate);
> +		dev_dbg(pwm->dev, "tin_rate=%lu\n", tin_rate);
>  
>  		tin_ns = NS_IN_HZ / tin_rate;
>  		tcnt = period_ns / tin_ns;
>  	} else
> -		tin_ns = NS_IN_HZ / clk_get_rate(pwm->clk);
> +		tin_ns = NS_IN_HZ / clk_get_rate(s3c_pwm->clk);
>  
>  	/* Note, counters count down */
>  
> @@ -253,7 +201,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	if (tcmp == tcnt)
>  		tcmp--;
>  
> -	pwm_dbg(pwm, "tin_ns=%lu, tcmp=%ld/%lu\n", tin_ns, tcmp, tcnt);
> +	dev_dbg(pwm->dev, "tin_ns=%lu, tcmp=%ld/%lu\n", tin_ns, tcmp, tcnt);
>  
>  	if (tcmp < 0)
>  		tcmp = 0;
> @@ -266,11 +214,11 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	__raw_writel(tcnt, S3C2410_TCNTB(pwm->pwm_id));
>  
>  	tcon = __raw_readl(S3C2410_TCON);
> -	tcon |= pwm_tcon_manulupdate(pwm);
> -	tcon |= pwm_tcon_autoreload(pwm);
> +	tcon |= pwm_tcon_manulupdate(s3c_pwm);
> +	tcon |= pwm_tcon_autoreload(s3c_pwm);
>  	__raw_writel(tcon, S3C2410_TCON);
>  
> -	tcon &= ~pwm_tcon_manulupdate(pwm);
> +	tcon &= ~pwm_tcon_manulupdate(s3c_pwm);
>  	__raw_writel(tcon, S3C2410_TCON);
>  
>  	local_irq_restore(flags);
> @@ -278,103 +226,122 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	return 0;
>  }
>  
> -EXPORT_SYMBOL(pwm_config);
> -
> -static int pwm_register(struct pwm_device *pwm)
> -{
> -	pwm->duty_ns = -1;
> -	pwm->period_ns = -1;
> -
> -	mutex_lock(&pwm_lock);
> -	list_add_tail(&pwm->list, &pwm_list);
> -	mutex_unlock(&pwm_lock);
> -
> -	return 0;
> -}
> -
>  static int s3c_pwm_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct s3c_pwm_device *s3c_pwm;
>  	struct pwm_device *pwm;
> +	struct pwm_ops *pops;
>  	unsigned long flags;
>  	unsigned long tcon;
>  	unsigned int id = pdev->id;
> -	int ret;
> +	int ret = 0;
>  
>  	if (id == 4) {
>  		dev_err(dev, "TIMER4 is currently not supported\n");
>  		return -ENXIO;
>  	}
>  
> +	s3c_pwm = kzalloc(sizeof(struct s3c_pwm_device), GFP_KERNEL);
> +	if (s3c_pwm == NULL) {
> +		dev_err(dev, "failed to allocate pwm_device\n");
> +		return -ENOMEM;
> +	}
> +	s3c_pwm->pdev = pdev;
>  	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
>  	if (pwm == NULL) {
>  		dev_err(dev, "failed to allocate pwm_device\n");
> -		return -ENOMEM;
> +		goto err_alloc;
> +		ret = -ENOMEM;
> +	}
> +	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> +	if (pops == NULL) {
> +		dev_err(dev, "failed to allocate memory\n");
> +		goto err_alloc1;
> +		ret = -ENOMEM;
>  	}
> -
> -	pwm->pdev = pdev;
> -	pwm->pwm_id = id;
>  
>  	/* calculate base of control bits in TCON */
> -	pwm->tcon_base = id == 0 ? 0 : (id * 4) + 4;
> +	s3c_pwm->tcon_base = id == 0 ? 0 : (id * 4) + 4;
>  
> -	pwm->clk = clk_get(dev, "pwm-tin");
> -	if (IS_ERR(pwm->clk)) {
> +	s3c_pwm->clk = clk_get(dev, "pwm-tin");
> +	if (IS_ERR(s3c_pwm->clk)) {
>  		dev_err(dev, "failed to get pwm tin clk\n");
> -		ret = PTR_ERR(pwm->clk);
> -		goto err_alloc;
> +		ret = PTR_ERR(s3c_pwm->clk);
> +		goto err_alloc2;
>  	}
>  
> -	pwm->clk_div = clk_get(dev, "pwm-tdiv");
> -	if (IS_ERR(pwm->clk_div)) {
> +	s3c_pwm->clk_div = clk_get(dev, "pwm-tdiv");
> +	if (IS_ERR(s3c_pwm->clk_div)) {
>  		dev_err(dev, "failed to get pwm tdiv clk\n");
> -		ret = PTR_ERR(pwm->clk_div);
> +		ret = PTR_ERR(s3c_pwm->clk_div);
>  		goto err_clk_tin;
>  	}
>  
>  	local_irq_save(flags);
>  
>  	tcon = __raw_readl(S3C2410_TCON);
> -	tcon |= pwm_tcon_invert(pwm);
> +	tcon |= pwm_tcon_invert(s3c_pwm);
>  	__raw_writel(tcon, S3C2410_TCON);
>  
>  	local_irq_restore(flags);
>  
> +	pops->pwm_config = s3c_pwm_config;
> +	pops->pwm_enable = s3c_pwm_enable;
> +	pops->pwm_disable = s3c_pwm_disable;
> +	pops->name = pdev->name;
> +
> +	pwm->dev = dev;
> +	pwm->pwm_id = id;
> +	pwm->pops = pops;
> +	pwm->data = s3c_pwm;
>  
> -	ret = pwm_register(pwm);
> +	s3c_pwm->duty_ns = -1;
> +	s3c_pwm->period_ns = -1;
> +	ret = pwm_device_register(pwm);
>  	if (ret) {
>  		dev_err(dev, "failed to register pwm\n");
>  		goto err_clk_tdiv;
>  	}
>  
> -	pwm_dbg(pwm, "config bits %02x\n",
> -		(__raw_readl(S3C2410_TCON) >> pwm->tcon_base) & 0x0f);
> +	dev_dbg(dev, "config bits %02x\n",
> +		(__raw_readl(S3C2410_TCON) >> s3c_pwm->tcon_base) & 0x0f);
>  
>  	dev_info(dev, "tin at %lu, tdiv at %lu, tin=%sclk, base %d\n",
> -		 clk_get_rate(pwm->clk),
> -		 clk_get_rate(pwm->clk_div),
> -		 pwm_is_tdiv(pwm) ? "div" : "ext", pwm->tcon_base);
> +		 clk_get_rate(s3c_pwm->clk),
> +		 clk_get_rate(s3c_pwm->clk_div),
> +		 pwm_is_tdiv(s3c_pwm) ? "div" : "ext", s3c_pwm->tcon_base);
>  
>  	platform_set_drvdata(pdev, pwm);
>  	return 0;
>  
> - err_clk_tdiv:
> -	clk_put(pwm->clk_div);
> +err_clk_tdiv:
> +	clk_put(s3c_pwm->clk_div);
>  
> - err_clk_tin:
> -	clk_put(pwm->clk);
> +err_clk_tin:
> +	clk_put(s3c_pwm->clk);
>  
> - err_alloc:
> +err_alloc2:
> +	kfree(pops);
> +
> +err_alloc1:
>  	kfree(pwm);
> +
> +err_alloc:
> +	kfree(s3c_pwm);
>  	return ret;
>  }
>  
>  static int __devexit s3c_pwm_remove(struct platform_device *pdev)
>  {
>  	struct pwm_device *pwm = platform_get_drvdata(pdev);
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  
> -	clk_put(pwm->clk_div);
> -	clk_put(pwm->clk);
> +	pwm_device_unregister(pwm);
> +	clk_put(s3c_pwm->clk_div);
> +	clk_put(s3c_pwm->clk);
> +	kfree(s3c_pwm);
> +	kfree(pwm->pops);
>  	kfree(pwm);
>  
>  	return 0;
> @@ -384,13 +351,14 @@ static int __devexit s3c_pwm_remove(struct platform_device *pdev)
>  static int s3c_pwm_suspend(struct platform_device *pdev, pm_message_t state)
>  {
>  	struct pwm_device *pwm = platform_get_drvdata(pdev);
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  
>  	/* No one preserve these values during suspend so reset them
>  	 * Otherwise driver leaves PWM unconfigured if same values
>  	 * passed to pwm_config
>  	 */
> -	pwm->period_ns = 0;
> -	pwm->duty_ns = 0;
> +	s3c_pwm->period_ns = 0;
> +	s3c_pwm->duty_ns = 0;
>  
>  	return 0;
>  }
> @@ -398,11 +366,12 @@ static int s3c_pwm_suspend(struct platform_device *pdev, pm_message_t state)
>  static int s3c_pwm_resume(struct platform_device *pdev)
>  {
>  	struct pwm_device *pwm = platform_get_drvdata(pdev);
> +	struct s3c_pwm_device *s3c_pwm = pwm->data;
>  	unsigned long tcon;
>  
>  	/* Restore invertion */
>  	tcon = __raw_readl(S3C2410_TCON);
> -	tcon |= pwm_tcon_invert(pwm);
> +	tcon |= pwm_tcon_invert(s3c_pwm);
>  	__raw_writel(tcon, S3C2410_TCON);
>  
>  	return 0;
> diff --git a/arch/mips/jz4740/pwm.c b/arch/mips/jz4740/pwm.c
> index a26a6fa..9f46767 100644
> --- a/arch/mips/jz4740/pwm.c
> +++ b/arch/mips/jz4740/pwm.c
> @@ -152,7 +152,7 @@ int pwm_enable(struct pwm_device *pwm)
>  	return 0;
>  }
>  
> -void pwm_disable(struct pwm_device *pwm)
> +int pwm_disable(struct pwm_device *pwm)
>  {
>  	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->id);
>  
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index b0f2c00..6a6ea41 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -129,6 +129,12 @@
>  #define twl_has_pwrbutton()	false
>  #endif
>  
> +#if defined CONFIG_TWL6030_PWM
> +#define twl_has_pwm()	true
> +#else
> +#define twl_has_pwm()	false
> +#endif
> +
>  #define SUB_CHIP_ID0 0
>  #define SUB_CHIP_ID1 1
>  #define SUB_CHIP_ID2 2
> @@ -825,6 +831,13 @@ add_children(struct twl4030_platform_data *pdata, unsigned long features)
>  		if (IS_ERR(child))
>  			return PTR_ERR(child);
>  	}
> +	if (twl_has_pwm()) {
> +		child = add_child(SUB_CHIP_ID2, "twl6030_pwm",
> +				NULL, 0,
> +				false, 0, 0);
> +		if (IS_ERR(child))
> +			return PTR_ERR(child);
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/mfd/twl6030-pwm.c b/drivers/mfd/twl6030-pwm.c
> index 5d25bdc..b78324b 100644
> --- a/drivers/mfd/twl6030-pwm.c
> +++ b/drivers/mfd/twl6030-pwm.c
> @@ -20,8 +20,10 @@
>  
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> -#include <linux/i2c/twl.h>
>  #include <linux/slab.h>
> +#include <linux/pwm.h>
> +#include <linux/err.h>
> +#include <linux/i2c/twl.h>
>  
>  #define LED_PWM_CTRL1	0xF4
>  #define LED_PWM_CTRL2	0xF5
> @@ -45,15 +47,10 @@
>  
>  #define PWM_CTRL2_MODE_MASK	0x3
>  
> -struct pwm_device {
> -	const char *label;
> -	unsigned int pwm_id;
> -};
> -
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +int twl6030_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  {
>  	u8 duty_cycle;
> -	int ret;
> +	int ret = 0;
>  
>  	if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
>  		return -EINVAL;
> @@ -69,12 +66,11 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  	}
>  	return 0;
>  }
> -EXPORT_SYMBOL(pwm_config);
>  
> -int pwm_enable(struct pwm_device *pwm)
> +int twl6030_pwm_enable(struct pwm_device *pwm)
>  {
>  	u8 val;
> -	int ret;
> +	int ret = 0;
>  
>  	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
>  	if (ret < 0) {
> @@ -95,18 +91,17 @@ int pwm_enable(struct pwm_device *pwm)
>  	twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
>  	return 0;
>  }
> -EXPORT_SYMBOL(pwm_enable);
>  
> -void pwm_disable(struct pwm_device *pwm)
> +int twl6030_pwm_disable(struct pwm_device *pwm)
>  {
>  	u8 val;
> -	int ret;
> +	int ret = 0;
>  
>  	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
>  	if (ret < 0) {
>  		pr_err("%s: Failed to disable PWM, Error %d\n",
>  			pwm->label, ret);
> -		return;
> +		return ret;
>  	}
>  
>  	val &= ~PWM_CTRL2_MODE_MASK;
> @@ -116,48 +111,86 @@ void pwm_disable(struct pwm_device *pwm)
>  	if (ret < 0) {
>  		pr_err("%s: Failed to disable PWM, Error %d\n",
>  			pwm->label, ret);
> -		return;
>  	}
> -	return;
> +	return ret;
>  }
> -EXPORT_SYMBOL(pwm_disable);
>  
> -struct pwm_device *pwm_request(int pwm_id, const char *label)
> +static int __devinit twl6030_pwm_probe(struct platform_device *pdev)
>  {
> -	u8 val;
> -	int ret;
>  	struct pwm_device *pwm;
> +	struct pwm_ops *pops;
> +	int ret;
> +	u8 val;
>  
>  	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
>  	if (pwm == NULL) {
> -		pr_err("%s: failed to allocate memory\n", label);
> -		return NULL;
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		return -ENOMEM;
> +	}
> +	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> +	if (pops == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		kfree(pwm);
> +		return -ENOMEM;
>  	}
>  
> -	pwm->label = label;
> -	pwm->pwm_id = pwm_id;
> -
> +	pops->pwm_config = twl6030_pwm_config;
> +	pops->pwm_enable = twl6030_pwm_enable;
> +	pops->pwm_disable = twl6030_pwm_disable;
> +	pops->name = &pdev->name;
> +	pwm->dev = &pdev->dev;
> +	pwm->pwm_id = pdev->id;
> +	pwm->pops = pops;
> +	ret = pwm_device_register(pwm);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register pwm device\n");
> +		kfree(pwm);
> +		kfree(pops);
> +		return ret;
> +	}
> +	platform_set_drvdata(pdev, pwm);
>  	/* Configure PWM */
>  	val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VAC |
> -		PWM_CTRL2_MODE_HW;
> +							PWM_CTRL2_MODE_HW;
>  
>  	ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> -
>  	if (ret < 0) {
> -		pr_err("%s: Failed to configure PWM, Error %d\n",
> -			 pwm->label, ret);
> -
> -		kfree(pwm);
> -		return NULL;
> +		dev_err(&pdev->dev, "Failed to configure PWM, Error %d\n", ret);
> +		return ret;
>  	}
> -
> -	return pwm;
> +	dev_dbg(&pdev->dev, "pwm probe successful\n");
> +	return ret;
>  }
> -EXPORT_SYMBOL(pwm_request);
>  
> -void pwm_free(struct pwm_device *pwm)
> +static int __devexit twl6030_pwm_remove(struct platform_device *pdev)
>  {
> -	pwm_disable(pwm);
> +	struct pwm_device *pwm = platform_get_drvdata(pdev);
> +
> +	pwm_device_unregister(pwm);
> +	kfree(pwm->pops);
>  	kfree(pwm);
> +	dev_dbg(&pdev->dev, "pwm driver removed\n");
> +	return 0;
>  }
> -EXPORT_SYMBOL(pwm_free);
> +
> +static struct platform_driver twl6030_pwm_driver = {
> +	.driver = {
> +		.name = "twl6030_pwm",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = twl6030_pwm_probe,
> +	.remove = __devexit_p(twl6030_pwm_remove),
> +};
> +
> +static int __init twl6030_pwm_init(void)
> +{
> +	return platform_driver_register(&twl6030_pwm_driver);
> +}
> +
> +static void __exit twl6030_pwm_deinit(void)
> +{
> +	platform_driver_unregister(&twl6030_pwm_driver);
> +}
> +
> +subsys_initcall(twl6030_pwm_init);
> +module_exit(twl6030_pwm_deinit);
> diff --git a/drivers/misc/ab8500-pwm.c b/drivers/misc/ab8500-pwm.c
> index 54e3d05..d2b23b6 100644
> --- a/drivers/misc/ab8500-pwm.c
> +++ b/drivers/misc/ab8500-pwm.c
> @@ -23,16 +23,9 @@
>  #define ENABLE_PWM			1
>  #define DISABLE_PWM			0
>  
> -struct pwm_device {
> -	struct device *dev;
> -	struct list_head node;
> -	const char *label;
> -	unsigned int pwm_id;
> -};
> -
>  static LIST_HEAD(pwm_list);
>  
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +int ab8500_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  {
>  	int ret = 0;
>  	unsigned int higher_val, lower_val;
> @@ -60,23 +53,21 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(pwm_config);
>  
> -int pwm_enable(struct pwm_device *pwm)
> +int ab8500_pwm_enable(struct pwm_device *pwm)
>  {
>  	int ret;
>  
>  	ret = abx500_mask_and_set_register_interruptible(pwm->dev,
>  				AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
> -				1 << (pwm->pwm_id-1), ENABLE_PWM);
> +				1 << (pwm->pwm_id-1), 1 << (pwm->pwm_id-1));
>  	if (ret < 0)
>  		dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
>  							pwm->label, ret);
>  	return ret;
>  }
> -EXPORT_SYMBOL(pwm_enable);
>  
> -void pwm_disable(struct pwm_device *pwm)
> +int ab8500_pwm_disable(struct pwm_device *pwm)
>  {
>  	int ret;
>  
> @@ -86,58 +77,56 @@ void pwm_disable(struct pwm_device *pwm)
>  	if (ret < 0)
>  		dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
>  							pwm->label, ret);
> -	return;
> -}
> -EXPORT_SYMBOL(pwm_disable);
> -
> -struct pwm_device *pwm_request(int pwm_id, const char *label)
> -{
> -	struct pwm_device *pwm;
> -
> -	list_for_each_entry(pwm, &pwm_list, node) {
> -		if (pwm->pwm_id == pwm_id) {
> -			pwm->label = label;
> -			pwm->pwm_id = pwm_id;
> -			return pwm;
> -		}
> -	}
> -
> -	return ERR_PTR(-ENOENT);
> -}
> -EXPORT_SYMBOL(pwm_request);
> -
> -void pwm_free(struct pwm_device *pwm)
> -{
> -	pwm_disable(pwm);
> +	return ret;
>  }
> -EXPORT_SYMBOL(pwm_free);
>  
>  static int __devinit ab8500_pwm_probe(struct platform_device *pdev)
>  {
> -	struct pwm_device *pwm;
> +	int ret = 0;
> +	struct pwm_ops *pops;
> +	struct pwm_device *pwm_dev;
>  	/*
>  	 * Nothing to be done in probe, this is required to get the
>  	 * device which is required for ab8500 read and write
>  	 */
> -	pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
> -	if (pwm == NULL) {
> +	pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> +	if (pops == NULL) {
>  		dev_err(&pdev->dev, "failed to allocate memory\n");
>  		return -ENOMEM;
>  	}
> -	pwm->dev = &pdev->dev;
> -	pwm->pwm_id = pdev->id;
> -	list_add_tail(&pwm->node, &pwm_list);
> -	platform_set_drvdata(pdev, pwm);
> -	dev_dbg(pwm->dev, "pwm probe successful\n");
> -	return 0;
> +	pwm_dev = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
> +	if (pwm_dev == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		kfree(pops);
> +		return -ENOMEM;
> +	}
> +	pops->pwm_config = ab8500_pwm_config;
> +	pops->pwm_enable = ab8500_pwm_enable;
> +	pops->pwm_disable = ab8500_pwm_disable;
> +	pops->name = "ab8500";
> +	pwm_dev->dev = &pdev->dev;
> +	pwm_dev->pwm_id = pdev->id;
> +	pwm_dev->pops = pops;
> +	ret = pwm_device_register(pwm_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register pwm device\n");
> +		kfree(pwm_dev);
> +		kfree(pops);
> +		return ret;
> +	}
> +	platform_set_drvdata(pdev, pwm_dev);
> +	dev_dbg(&pdev->dev, "pwm probe successful\n");
> +	return ret;
>  }
>  
>  static int __devexit ab8500_pwm_remove(struct platform_device *pdev)
>  {
> -	struct pwm_device *pwm = platform_get_drvdata(pdev);
> -	list_del(&pwm->node);
> +	struct pwm_device *pwm_dev = platform_get_drvdata(pdev);
> +
> +	pwm_device_unregister(pwm_dev);
>  	dev_dbg(&pdev->dev, "pwm driver removed\n");
> -	kfree(pwm);
> +	kfree(pwm_dev->pops);
> +	kfree(pwm_dev);
>  	return 0;
>  }
>  
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 5d10106..a88640c 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -4,6 +4,7 @@
>  
>  menuconfig PWM_DEVICES
>  	bool "PWM devices"
> +	depends on ARM
>  	default y
>  	---help---
>  	  Say Y here to get to see options for device drivers from various
> diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
> index b84027a..3a0d426 100644



Why can't these changes be in the initial patch which adds pwm-core?

> --- a/drivers/pwm/pwm-core.c
> +++ b/drivers/pwm/pwm-core.c
> @@ -11,11 +11,6 @@
>  #include <linux/err.h>
>  #include <linux/pwm.h>
>  
> -struct pwm_device {
> -	struct pwm_ops *pops;
> -	int pwm_id;
> -};
> -
>  struct pwm_dev_info {
>  	struct pwm_device *pwm_dev;
>  	struct list_head list;
> @@ -40,9 +35,9 @@ int pwm_enable(struct pwm_device *pwm)
>  }
>  EXPORT_SYMBOL(pwm_enable);
>  
> -void pwm_disable(struct pwm_device *pwm)
> +int pwm_disable(struct pwm_device *pwm)
>  {
> -	pwm->pops->pwm_disable(pwm);
> +	return pwm->pops->pwm_disable(pwm);
>  }
>  EXPORT_SYMBOL(pwm_disable);
>  
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 6e7da1f..4344c0b 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -1,14 +1,29 @@
>  #ifndef __LINUX_PWM_H
>  #define __LINUX_PWM_H
>  
> -struct pwm_device;
> +/*
> + * TODO: #if defined CONFIG_PWM_CORE has to be removed after mips jz4740
> + * pwm driver aligning with pwm-core.c driver.
> + */
> +#if defined CONFIG_PWM_CORE
> +struct pwm_device {
> +	struct pwm_ops *pops;
> +	struct device *dev;
> +	struct list_head node;
> +	const char *label;
> +	unsigned int pwm_id;
> +	void *data;
> +};
>  
>  struct pwm_ops {
>  	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int period_ns);
>  	int (*pwm_enable)(struct pwm_device *pwm);
>  	int (*pwm_disable)(struct pwm_device *pwm);
> -	char *name;
> +	const char *name;
>  };
> +#else
> +struct pwm_device;
> +#endif
>  
>  /*
>   * pwm_request - request a PWM device
> @@ -33,7 +48,7 @@ int pwm_enable(struct pwm_device *pwm);
>  /*
>   * pwm_disable - stop a PWM output toggling
>   */
> -void pwm_disable(struct pwm_device *pwm);
> +int pwm_disable(struct pwm_device *pwm);
>  
>  int pwm_device_register(struct pwm_device *pwm_dev);
>  int pwm_device_unregister(struct pwm_device *pwm_dev);

[PATCH 4/7] pwm: Align existing pwm drivers with pwm-core driver

[Lars-Peter Clausen]

Arun MURTHY wrote:
>>> mips-jz4740: pwm: Align with new pwm core driver
>>>
>>> PWM core driver has been added and has been enabled only for ARM
>>> platform. The same can be utilised for mips also.
>>> Please align with the pwm core driver(drivers/pwm-core.c).
>>
>> Is there any reason for artificially limiting it to ARM?
> 
> No not at all, right now I have aligned all existing pwm drivers in ARM to make use of the pwm core driver.
> But faced difficulty in aligning the mips-jz4740 pwm driver, without having much knowledge about the device/data sheet.
> Hence I have let it to the maintainer of that driver to align and thereafter this limitation will be removed.
> Have also comments the same as TODO in the driver.
> 

Ok, I'll take care of adjusting the jz4740 pwm driver once the pwm-core is in proper
shape.
But I still think it would be better to have a config symbol which would be selected
by SoC code and on which PWM_CORE would depend. Then it would be possible for SoC
implementation to device whether it wants to provide it's own PWM API implementation
or use pwm-core.

>>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>>> index 5d10106..a88640c 100644
>>> --- a/drivers/pwm/Kconfig
>>> +++ b/drivers/pwm/Kconfig
>>> @@ -4,6 +4,7 @@
>>>
>>>  menuconfig PWM_DEVICES
>>>  	bool "PWM devices"
>>> +	depends on ARM
>>>  	default y
>>>  	---help---
>>>  	  Say Y here to get to see options for device drivers from
>> various
>>> diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
>>> index b84027a..3a0d426 100644
>>
>>
>> Why can't these changes be in the initial patch which adds pwm-core?
>>
> Since by default this driver is enabled, and if there is some other pwm driver enabled, both happen to export the same function(pwm_enable/pwm_disable,..) After applying the first patch build may fail.
> 
I would understand that if you were just moving code around, but the pwm_device
struct looks completly different now.


> Thanks and Regards,
> Arun R Murthy
> -------------
> 

[PATCH 5/7] platform: Update the pwm based led and backlight platform data

[Arun Murthy]

	mxc-pwm: Update the platform data with pwm name for backlight
	s3c24xx-pwm: update platform data for backlight with pwm name

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 arch/arm/mach-pxa/cm-x300.c               |    1 +
 arch/arm/mach-pxa/colibri-pxa270-income.c |    1 +
 arch/arm/mach-pxa/ezx.c                   |    1 +
 arch/arm/mach-pxa/hx4700.c                |    1 +
 arch/arm/mach-pxa/lpd270.c                |    1 +
 arch/arm/mach-pxa/magician.c              |    1 +
 arch/arm/mach-pxa/mainstone.c             |    1 +
 arch/arm/mach-pxa/mioa701.c               |    1 +
 arch/arm/mach-pxa/palm27x.c               |    1 +
 arch/arm/mach-pxa/palmtc.c                |    1 +
 arch/arm/mach-pxa/palmte2.c               |    1 +
 arch/arm/mach-pxa/pcm990-baseboard.c      |    1 +
 arch/arm/mach-pxa/raumfeld.c              |    1 +
 arch/arm/mach-pxa/tavorevb.c              |    2 ++
 arch/arm/mach-pxa/viper.c                 |    1 +
 arch/arm/mach-pxa/z2.c                    |    2 ++
 arch/arm/mach-pxa/zylonite.c              |    1 +
 arch/arm/mach-s3c2410/mach-h1940.c        |    1 +
 arch/arm/mach-s3c2440/mach-rx1950.c       |    1 +
 arch/arm/mach-s3c64xx/mach-hmt.c          |    1 +
 arch/arm/mach-s3c64xx/mach-smartq.c       |    1 +
 21 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-pxa/cm-x300.c b/arch/arm/mach-pxa/cm-x300.c
index c70e6c2..ddf763b 100644
--- a/arch/arm/mach-pxa/cm-x300.c
+++ b/arch/arm/mach-pxa/cm-x300.c
@@ -301,6 +301,7 @@ static inline void cm_x300_init_lcd(void) {}
 #if defined(CONFIG_BACKLIGHT_PWM) || defined(CONFIG_BACKLIGHT_PWM_MODULE)
 static struct platform_pwm_backlight_data cm_x300_backlight_data = {
 	.pwm_id		= 2,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 100,
 	.dft_brightness	= 100,
 	.pwm_period_ns	= 10000,
diff --git a/arch/arm/mach-pxa/colibri-pxa270-income.c b/arch/arm/mach-pxa/colibri-pxa270-income.c
index 37f0f3e..d5b5874 100644
--- a/arch/arm/mach-pxa/colibri-pxa270-income.c
+++ b/arch/arm/mach-pxa/colibri-pxa270-income.c
@@ -234,6 +234,7 @@ static inline void income_lcd_init(void) {}
 #if defined(CONFIG_BACKLIGHT_PWM) || defined(CONFIG_BACKLIGHT_PWM__MODULE)
 static struct platform_pwm_backlight_data income_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 0x3ff,
 	.dft_brightness	= 0x1ff,
 	.pwm_period_ns	= 1000000,
diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
index 626c82b..747f217 100644
--- a/arch/arm/mach-pxa/ezx.c
+++ b/arch/arm/mach-pxa/ezx.c
@@ -49,6 +49,7 @@
 
 static struct platform_pwm_backlight_data ezx_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 1023,
 	.dft_brightness	= 1023,
 	.pwm_period_ns	= 78770,
diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c
index 848c861..8e4905a 100644
--- a/arch/arm/mach-pxa/hx4700.c
+++ b/arch/arm/mach-pxa/hx4700.c
@@ -565,6 +565,7 @@ static struct platform_device hx4700_lcd = {
 
 static struct platform_pwm_backlight_data backlight_data = {
 	.pwm_id         = 1,
+	.name		= "pxa25x-pwm",
 	.max_brightness = 200,
 	.dft_brightness = 100,
 	.pwm_period_ns  = 30923,
diff --git a/arch/arm/mach-pxa/lpd270.c b/arch/arm/mach-pxa/lpd270.c
index d279507..91efade 100644
--- a/arch/arm/mach-pxa/lpd270.c
+++ b/arch/arm/mach-pxa/lpd270.c
@@ -273,6 +273,7 @@ static struct platform_device lpd270_flash_device[2] = {
 
 static struct platform_pwm_backlight_data lpd270_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 1,
 	.dft_brightness	= 1,
 	.pwm_period_ns	= 78770,
diff --git a/arch/arm/mach-pxa/magician.c b/arch/arm/mach-pxa/magician.c
index e81dd0c..bb657a4 100644
--- a/arch/arm/mach-pxa/magician.c
+++ b/arch/arm/mach-pxa/magician.c
@@ -382,6 +382,7 @@ static void magician_backlight_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data backlight_data = {
 	.pwm_id         = 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness = 272,
 	.dft_brightness = 100,
 	.pwm_period_ns  = 30923,
diff --git a/arch/arm/mach-pxa/mainstone.c b/arch/arm/mach-pxa/mainstone.c
index 5543c64..cbd359c 100644
--- a/arch/arm/mach-pxa/mainstone.c
+++ b/arch/arm/mach-pxa/mainstone.c
@@ -342,6 +342,7 @@ static struct platform_device mst_flash_device[2] = {
 #if defined(CONFIG_FB_PXA) || defined(CONFIG_FB_PXA_MODULE)
 static struct platform_pwm_backlight_data mainstone_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 1023,
 	.dft_brightness	= 1023,
 	.pwm_period_ns	= 78770,
diff --git a/arch/arm/mach-pxa/mioa701.c b/arch/arm/mach-pxa/mioa701.c
index dc66942..e442088 100644
--- a/arch/arm/mach-pxa/mioa701.c
+++ b/arch/arm/mach-pxa/mioa701.c
@@ -224,6 +224,7 @@ static void mio_gpio_free(struct gpio_ress *gpios, int size)
 /* LCD Screen and Backlight */
 static struct platform_pwm_backlight_data mioa701_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 100,
 	.dft_brightness	= 50,
 	.pwm_period_ns	= 4000 * 1024,	/* Fl = 250kHz */
diff --git a/arch/arm/mach-pxa/palm27x.c b/arch/arm/mach-pxa/palm27x.c
index 77ad6d3..46677a4 100644
--- a/arch/arm/mach-pxa/palm27x.c
+++ b/arch/arm/mach-pxa/palm27x.c
@@ -321,6 +321,7 @@ static void palm27x_backlight_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data palm27x_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 0xfe,
 	.dft_brightness	= 0x7e,
 	.pwm_period_ns	= 3500,
diff --git a/arch/arm/mach-pxa/palmtc.c b/arch/arm/mach-pxa/palmtc.c
index ce1104d..385a0b5 100644
--- a/arch/arm/mach-pxa/palmtc.c
+++ b/arch/arm/mach-pxa/palmtc.c
@@ -180,6 +180,7 @@ static void palmtc_backlight_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data palmtc_backlight_data = {
 	.pwm_id		= 1,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= PALMTC_MAX_INTENSITY,
 	.dft_brightness	= PALMTC_MAX_INTENSITY,
 	.pwm_period_ns	= PALMTC_PERIOD_NS,
diff --git a/arch/arm/mach-pxa/palmte2.c b/arch/arm/mach-pxa/palmte2.c
index 93c11a0..b7e95f4 100644
--- a/arch/arm/mach-pxa/palmte2.c
+++ b/arch/arm/mach-pxa/palmte2.c
@@ -177,6 +177,7 @@ static void palmte2_backlight_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data palmte2_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= PALMTE2_MAX_INTENSITY,
 	.dft_brightness	= PALMTE2_MAX_INTENSITY,
 	.pwm_period_ns	= PALMTE2_PERIOD_NS,
diff --git a/arch/arm/mach-pxa/pcm990-baseboard.c b/arch/arm/mach-pxa/pcm990-baseboard.c
index f56ae10..29c7e88 100644
--- a/arch/arm/mach-pxa/pcm990-baseboard.c
+++ b/arch/arm/mach-pxa/pcm990-baseboard.c
@@ -138,6 +138,7 @@ static struct pxafb_mach_info pcm990_fbinfo __initdata = {
 
 static struct platform_pwm_backlight_data pcm990_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 1023,
 	.dft_brightness	= 1023,
 	.pwm_period_ns	= 78770,
diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
index 67e04f4..98dc2e3 100644
--- a/arch/arm/mach-pxa/raumfeld.c
+++ b/arch/arm/mach-pxa/raumfeld.c
@@ -535,6 +535,7 @@ static void __init raumfeld_w1_init(void)
 /* PWM controlled backlight */
 static struct platform_pwm_backlight_data raumfeld_pwm_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 100,
 	.dft_brightness	= 100,
 	/* 10000 ns = 10 ms ^= 100 kHz */
diff --git a/arch/arm/mach-pxa/tavorevb.c b/arch/arm/mach-pxa/tavorevb.c
index f02dcb5..3164de8 100644
--- a/arch/arm/mach-pxa/tavorevb.c
+++ b/arch/arm/mach-pxa/tavorevb.c
@@ -168,6 +168,7 @@ static struct platform_pwm_backlight_data tavorevb_backlight_data[] = {
 	[0] = {
 		/* primary backlight */
 		.pwm_id		= 2,
+		.name		= "pxa25x-pwm",
 		.max_brightness	= 100,
 		.dft_brightness	= 100,
 		.pwm_period_ns	= 100000,
@@ -175,6 +176,7 @@ static struct platform_pwm_backlight_data tavorevb_backlight_data[] = {
 	[1] = {
 		/* secondary backlight */
 		.pwm_id		= 0,
+		.name		= "pxa25x-pwm",
 		.max_brightness	= 100,
 		.dft_brightness	= 100,
 		.pwm_period_ns	= 100000,
diff --git a/arch/arm/mach-pxa/viper.c b/arch/arm/mach-pxa/viper.c
index e90114a..fdb768c 100644
--- a/arch/arm/mach-pxa/viper.c
+++ b/arch/arm/mach-pxa/viper.c
@@ -397,6 +397,7 @@ static void viper_backlight_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data viper_backlight_data = {
 	.pwm_id		= 0,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 100,
 	.dft_brightness	= 100,
 	.pwm_period_ns	= 1000000,
diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c
index f0d0228..bb3d821 100644
--- a/arch/arm/mach-pxa/z2.c
+++ b/arch/arm/mach-pxa/z2.c
@@ -204,6 +204,7 @@ static struct platform_pwm_backlight_data z2_backlight_data[] = {
 	[0] = {
 		/* Keypad Backlight */
 		.pwm_id		= 1,
+		.name		= "pxa25x-pwm",
 		.max_brightness	= 1023,
 		.dft_brightness	= 512,
 		.pwm_period_ns	= 1260320,
@@ -211,6 +212,7 @@ static struct platform_pwm_backlight_data z2_backlight_data[] = {
 	[1] = {
 		/* LCD Backlight */
 		.pwm_id		= 2,
+		.name		= "pxa25x-pwm",
 		.max_brightness	= 1023,
 		.dft_brightness	= 512,
 		.pwm_period_ns	= 1260320,
diff --git a/arch/arm/mach-pxa/zylonite.c b/arch/arm/mach-pxa/zylonite.c
index 5ba9d99..29492bf 100644
--- a/arch/arm/mach-pxa/zylonite.c
+++ b/arch/arm/mach-pxa/zylonite.c
@@ -122,6 +122,7 @@ static inline void zylonite_init_leds(void) {}
 #if defined(CONFIG_FB_PXA) || defined(CONFIG_FB_PXA_MODULE)
 static struct platform_pwm_backlight_data zylonite_backlight_data = {
 	.pwm_id		= 3,
+	.name		= "pxa25x-pwm",
 	.max_brightness	= 100,
 	.dft_brightness	= 100,
 	.pwm_period_ns	= 10000,
diff --git a/arch/arm/mach-s3c2410/mach-h1940.c b/arch/arm/mach-s3c2410/mach-h1940.c
index 3ba3bab..357342f 100644
--- a/arch/arm/mach-s3c2410/mach-h1940.c
+++ b/arch/arm/mach-s3c2410/mach-h1940.c
@@ -224,6 +224,7 @@ static void h1940_backlight_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data backlight_data = {
 	.pwm_id         = 0,
+	.name		= "s3c24xx-pwm",
 	.max_brightness = 100,
 	.dft_brightness = 50,
 	/* tcnt = 0x31 */
diff --git a/arch/arm/mach-s3c2440/mach-rx1950.c b/arch/arm/mach-s3c2440/mach-rx1950.c
index 142d1f9..6d993de 100644
--- a/arch/arm/mach-s3c2440/mach-rx1950.c
+++ b/arch/arm/mach-s3c2440/mach-rx1950.c
@@ -291,6 +291,7 @@ static int rx1950_backlight_notify(struct device *dev, int brightness)
 
 static struct platform_pwm_backlight_data rx1950_backlight_data = {
 	.pwm_id = 0,
+	.name = "s3c24xx-pwm",
 	.max_brightness = 24,
 	.dft_brightness = 4,
 	.pwm_period_ns = 48000,
diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c b/arch/arm/mach-s3c64xx/mach-hmt.c
index fba9022..14e9011 100644
--- a/arch/arm/mach-s3c64xx/mach-hmt.c
+++ b/arch/arm/mach-s3c64xx/mach-hmt.c
@@ -109,6 +109,7 @@ static void hmt_bl_exit(struct device *dev)
 
 static struct platform_pwm_backlight_data hmt_backlight_data = {
 	.pwm_id		= 1,
+	.name		= "s3c24xx-pwm",
 	.max_brightness	= 100 * 256,
 	.dft_brightness	= 40 * 256,
 	.pwm_period_ns	= 1000000000 / (100 * 256 * 20),
diff --git a/arch/arm/mach-s3c64xx/mach-smartq.c b/arch/arm/mach-s3c64xx/mach-smartq.c
index cb1ebeb..20999d5 100644
--- a/arch/arm/mach-s3c64xx/mach-smartq.c
+++ b/arch/arm/mach-s3c64xx/mach-smartq.c
@@ -145,6 +145,7 @@ static int smartq_bl_init(struct device *dev)
 
 static struct platform_pwm_backlight_data smartq_backlight_data = {
 	.pwm_id		= 1,
+	.name		= "s3c24xx-pwm",
 	.max_brightness	= 1000,
 	.dft_brightness	= 600,
 	.pwm_period_ns	= 1000000000 / (1000 * 20),
-- 
1.7.2.dirty

[PATCH 7/7] pwm: Modify backlight and led Kconfig aligning to pwm core

[Arun Murthy]

PWM based backlight and led driver will not be calling the pwm drivers
through the pwm core driver and hence adding dependancy on the same.

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/leds/Kconfig            |    2 +-
 drivers/pwm/Kconfig             |    2 --
 drivers/video/backlight/Kconfig |    2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e411262..eba388f 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -244,7 +244,7 @@ config LEDS_DAC124S085
 
 config LEDS_PWM
 	tristate "PWM driven LED Support"
-	depends on HAVE_PWM
+	depends on HAVE_PWM || PWM_CORE
 	help
 	  This option enables support for pwm driven LEDs
 
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e347365..1f8cbc0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -17,7 +17,6 @@ if PWM_DEVICES
 config AB8500_PWM
 	bool "AB8500 PWM support"
 	depends on AB8500_CORE
-	select HAVE_PWM
 	help
 	  This driver exports functions to enable/disble/config/free Pulse
 	  Width Modulation in the Analog Baseband Chip AB8500.
@@ -26,7 +25,6 @@ config AB8500_PWM
 config TWL6030_PWM
 	tristate "TWL6030 PWM (Pulse Width Modulator) Support"
 	depends on TWL4030_CORE
-	select HAVE_PWM
 	default n
 	help
 	  Say yes here if you want support for TWL6030 PWM.
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index e54a337..73fc17b 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -217,7 +217,7 @@ config BACKLIGHT_CARILLO_RANCH
 
 config BACKLIGHT_PWM
 	tristate "Generic PWM based Backlight Driver"
-	depends on HAVE_PWM
+	depends on HAVE_PWM || PWM_CORE
 	help
 	  If you have a LCD backlight adjustable by PWM, say Y to enable
 	  this driver.
-- 
1.7.2.dirty

[PATCH 6/7] pwm: move existing pwm driver to drivers/pwm

[Eric Miao]

On Tue, Sep 28, 2010 at 3:40 PM, Arun Murthy <arun.murthy@stericsson.com> wrote:
> As of now only ab8500 and twl6030 are moved.
>

You can use 'git format-patch -M' to reduce the patch size when renaming
is involved.

> Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> ---
> ?drivers/mfd/Kconfig ? ? ? | ? ?9 --
> ?drivers/mfd/Makefile ? ? ?| ? ?1 -
> ?drivers/mfd/twl6030-pwm.c | ?196 ---------------------------------------------
> ?drivers/misc/Kconfig ? ? ?| ? ?9 --
> ?drivers/misc/Makefile ? ? | ? ?1 -
> ?drivers/misc/ab8500-pwm.c | ?157 ------------------------------------
> ?drivers/pwm/Kconfig ? ? ? | ? 18 ++++
> ?drivers/pwm/Makefile ? ? ?| ? ?3 +
> ?drivers/pwm/pwm-ab8500.c ?| ?157 ++++++++++++++++++++++++++++++++++++
> ?drivers/pwm/pwm-twl6040.c | ?196 +++++++++++++++++++++++++++++++++++++++++++++
> ?10 files changed, 374 insertions(+), 373 deletions(-)
> ?delete mode 100644 drivers/mfd/twl6030-pwm.c
> ?delete mode 100644 drivers/misc/ab8500-pwm.c
> ?create mode 100644 drivers/pwm/pwm-ab8500.c
> ?create mode 100644 drivers/pwm/pwm-twl6040.c
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 256fabd..ab1d376 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -186,15 +186,6 @@ config TWL4030_CODEC
> ? ? ? ?select MFD_CORE
> ? ? ? ?default n
>
> -config TWL6030_PWM
> - ? ? ? tristate "TWL6030 PWM (Pulse Width Modulator) Support"
> - ? ? ? depends on TWL4030_CORE
> - ? ? ? select HAVE_PWM
> - ? ? ? default n
> - ? ? ? help
> - ? ? ? ? Say yes here if you want support for TWL6030 PWM.
> - ? ? ? ? This is used to control charging LED brightness.
> -
> ?config MFD_STMPE
> ? ? ? ?bool "Support STMicroelectronics STMPE"
> ? ? ? ?depends on I2C=y && GENERIC_HARDIRQS
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index d5968cd..1a89dbf 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -37,7 +37,6 @@ obj-$(CONFIG_MENELAUS) ? ? ? ? ? ? ? ?+= menelaus.o
> ?obj-$(CONFIG_TWL4030_CORE) ? ? += twl-core.o twl4030-irq.o twl6030-irq.o
> ?obj-$(CONFIG_TWL4030_POWER) ? ?+= twl4030-power.o
> ?obj-$(CONFIG_TWL4030_CODEC) ? ?+= twl4030-codec.o
> -obj-$(CONFIG_TWL6030_PWM) ? ? ?+= twl6030-pwm.o
>
> ?obj-$(CONFIG_MFD_MC13783) ? ? ?+= mc13783-core.o
>
> diff --git a/drivers/mfd/twl6030-pwm.c b/drivers/mfd/twl6030-pwm.c
> deleted file mode 100644
> index b78324b..0000000
> --- a/drivers/mfd/twl6030-pwm.c
> +++ /dev/null
> @@ -1,196 +0,0 @@
> -/*
> - * twl6030_pwm.c
> - * Driver for PHOENIX (TWL6030) Pulse Width Modulator
> - *
> - * Copyright (C) 2010 Texas Instruments
> - * Author: Hemanth V <hemanthv@ti.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License version 2 as published by
> - * the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE. ?See the GNU General Public License for
> - * more details.
> - *
> - * You should have received a copy of the GNU General Public License along with
> - * this program. ?If not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> -#include <linux/pwm.h>
> -#include <linux/err.h>
> -#include <linux/i2c/twl.h>
> -
> -#define LED_PWM_CTRL1 ?0xF4
> -#define LED_PWM_CTRL2 ?0xF5
> -
> -/* Max value for CTRL1 register */
> -#define PWM_CTRL1_MAX ?255
> -
> -/* Pull down disable */
> -#define PWM_CTRL2_DIS_PD ? ? ? (1 << 6)
> -
> -/* Current control 2.5 milli Amps */
> -#define PWM_CTRL2_CURR_02 ? ? ?(2 << 4)
> -
> -/* LED supply source */
> -#define PWM_CTRL2_SRC_VAC ? ? ?(1 << 2)
> -
> -/* LED modes */
> -#define PWM_CTRL2_MODE_HW ? ? ?(0 << 0)
> -#define PWM_CTRL2_MODE_SW ? ? ?(1 << 0)
> -#define PWM_CTRL2_MODE_DIS ? ? (2 << 0)
> -
> -#define PWM_CTRL2_MODE_MASK ? ?0x3
> -
> -int twl6030_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> -{
> - ? ? ? u8 duty_cycle;
> - ? ? ? int ret = 0;
> -
> - ? ? ? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
> - ? ? ? ? ? ? ? return -EINVAL;
> -
> - ? ? ? duty_cycle = (duty_ns * PWM_CTRL1_MAX) / period_ns;
> -
> - ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, duty_cycle, LED_PWM_CTRL1);
> -
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? pr_err("%s: Failed to configure PWM, Error %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> - ? ? ? return 0;
> -}
> -
> -int twl6030_pwm_enable(struct pwm_device *pwm)
> -{
> - ? ? ? u8 val;
> - ? ? ? int ret = 0;
> -
> - ? ? ? ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? pr_err("%s: Failed to enable PWM, Error %d\n", pwm->label, ret);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> -
> - ? ? ? /* Change mode to software control */
> - ? ? ? val &= ~PWM_CTRL2_MODE_MASK;
> - ? ? ? val |= PWM_CTRL2_MODE_SW;
> -
> - ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? pr_err("%s: Failed to enable PWM, Error %d\n", pwm->label, ret);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> -
> - ? ? ? twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
> - ? ? ? return 0;
> -}
> -
> -int twl6030_pwm_disable(struct pwm_device *pwm)
> -{
> - ? ? ? u8 val;
> - ? ? ? int ret = 0;
> -
> - ? ? ? ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? pr_err("%s: Failed to disable PWM, Error %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> -
> - ? ? ? val &= ~PWM_CTRL2_MODE_MASK;
> - ? ? ? val |= PWM_CTRL2_MODE_HW;
> -
> - ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? pr_err("%s: Failed to disable PWM, Error %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> - ? ? ? }
> - ? ? ? return ret;
> -}
> -
> -static int __devinit twl6030_pwm_probe(struct platform_device *pdev)
> -{
> - ? ? ? struct pwm_device *pwm;
> - ? ? ? struct pwm_ops *pops;
> - ? ? ? int ret;
> - ? ? ? u8 val;
> -
> - ? ? ? pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
> - ? ? ? if (pwm == NULL) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> - ? ? ? ? ? ? ? return -ENOMEM;
> - ? ? ? }
> - ? ? ? pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> - ? ? ? if (pops == NULL) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> - ? ? ? ? ? ? ? kfree(pwm);
> - ? ? ? ? ? ? ? return -ENOMEM;
> - ? ? ? }
> -
> - ? ? ? pops->pwm_config = twl6030_pwm_config;
> - ? ? ? pops->pwm_enable = twl6030_pwm_enable;
> - ? ? ? pops->pwm_disable = twl6030_pwm_disable;
> - ? ? ? pops->name = &pdev->name;
> - ? ? ? pwm->dev = &pdev->dev;
> - ? ? ? pwm->pwm_id = pdev->id;
> - ? ? ? pwm->pops = pops;
> - ? ? ? ret = pwm_device_register(pwm);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to register pwm device\n");
> - ? ? ? ? ? ? ? kfree(pwm);
> - ? ? ? ? ? ? ? kfree(pops);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> - ? ? ? platform_set_drvdata(pdev, pwm);
> - ? ? ? /* Configure PWM */
> - ? ? ? val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VAC |
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PWM_CTRL2_MODE_HW;
> -
> - ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "Failed to configure PWM, Error %d\n", ret);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> - ? ? ? dev_dbg(&pdev->dev, "pwm probe successful\n");
> - ? ? ? return ret;
> -}
> -
> -static int __devexit twl6030_pwm_remove(struct platform_device *pdev)
> -{
> - ? ? ? struct pwm_device *pwm = platform_get_drvdata(pdev);
> -
> - ? ? ? pwm_device_unregister(pwm);
> - ? ? ? kfree(pwm->pops);
> - ? ? ? kfree(pwm);
> - ? ? ? dev_dbg(&pdev->dev, "pwm driver removed\n");
> - ? ? ? return 0;
> -}
> -
> -static struct platform_driver twl6030_pwm_driver = {
> - ? ? ? .driver = {
> - ? ? ? ? ? ? ? .name = "twl6030_pwm",
> - ? ? ? ? ? ? ? .owner = THIS_MODULE,
> - ? ? ? },
> - ? ? ? .probe = twl6030_pwm_probe,
> - ? ? ? .remove = __devexit_p(twl6030_pwm_remove),
> -};
> -
> -static int __init twl6030_pwm_init(void)
> -{
> - ? ? ? return platform_driver_register(&twl6030_pwm_driver);
> -}
> -
> -static void __exit twl6030_pwm_deinit(void)
> -{
> - ? ? ? platform_driver_unregister(&twl6030_pwm_driver);
> -}
> -
> -subsys_initcall(twl6030_pwm_init);
> -module_exit(twl6030_pwm_deinit);
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index ff8ea55..2c38d4e 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -62,15 +62,6 @@ config ATMEL_PWM
> ? ? ? ? ?purposes including software controlled power-efficient backlights
> ? ? ? ? ?on LCD displays, motor control, and waveform generation.
>
> -config AB8500_PWM
> - ? ? ? bool "AB8500 PWM support"
> - ? ? ? depends on AB8500_CORE
> - ? ? ? select HAVE_PWM
> - ? ? ? help
> - ? ? ? ? This driver exports functions to enable/disble/config/free Pulse
> - ? ? ? ? Width Modulation in the Analog Baseband Chip AB8500.
> - ? ? ? ? It is used by led and backlight driver to control the intensity.
> -
> ?config ATMEL_TCLIB
> ? ? ? ?bool "Atmel AT32/AT91 Timer/Counter Library"
> ? ? ? ?depends on (AVR32 || ARCH_AT91)
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 5da82965..21b4761 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -35,5 +35,4 @@ obj-y ? ? ? ? ? ? ? ? ? ? ? ? += eeprom/
> ?obj-y ? ? ? ? ? ? ? ? ? ? ? ? ?+= cb710/
> ?obj-$(CONFIG_VMWARE_BALLOON) ? += vmware_balloon.o
> ?obj-$(CONFIG_ARM_CHARLCD) ? ? ?+= arm-charlcd.o
> -obj-$(CONFIG_AB8500_PWM) ? ? ? += ab8500-pwm.o
> ?obj-$(CONFIG_PCH_PHUB) ? ? ? ? += pch_phub.o
> diff --git a/drivers/misc/ab8500-pwm.c b/drivers/misc/ab8500-pwm.c
> deleted file mode 100644
> index d2b23b6..0000000
> --- a/drivers/misc/ab8500-pwm.c
> +++ /dev/null
> @@ -1,157 +0,0 @@
> -/*
> - * Copyright (C) ST-Ericsson SA 2010
> - *
> - * Author: Arun R Murthy <arun.murthy@stericsson.com>
> - * License terms: GNU General Public License (GPL) version 2
> - */
> -#include <linux/err.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> -#include <linux/pwm.h>
> -#include <linux/mfd/ab8500.h>
> -#include <linux/mfd/abx500.h>
> -
> -/*
> - * PWM Out generators
> - * Bank: 0x10
> - */
> -#define AB8500_PWM_OUT_CTRL1_REG ? ? ? 0x60
> -#define AB8500_PWM_OUT_CTRL2_REG ? ? ? 0x61
> -#define AB8500_PWM_OUT_CTRL7_REG ? ? ? 0x66
> -
> -/* backlight driver constants */
> -#define ENABLE_PWM ? ? ? ? ? ? ? ? ? ? 1
> -#define DISABLE_PWM ? ? ? ? ? ? ? ? ? ?0
> -
> -static LIST_HEAD(pwm_list);
> -
> -int ab8500_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> -{
> - ? ? ? int ret = 0;
> - ? ? ? unsigned int higher_val, lower_val;
> - ? ? ? u8 reg;
> -
> - ? ? ? /*
> - ? ? ? ?* get the first 8 bits that are be written to
> - ? ? ? ?* AB8500_PWM_OUT_CTRL1_REG[0:7]
> - ? ? ? ?*/
> - ? ? ? lower_val = duty_ns & 0x00FF;
> - ? ? ? /*
> - ? ? ? ?* get bits [9:10] that are to be written to
> - ? ? ? ?* AB8500_PWM_OUT_CTRL2_REG[0:1]
> - ? ? ? ?*/
> - ? ? ? higher_val = ((duty_ns & 0x0300) >> 8);
> -
> - ? ? ? reg = AB8500_PWM_OUT_CTRL1_REG + ((pwm->pwm_id - 1) * 2);
> -
> - ? ? ? ret = abx500_set_register_interruptible(pwm->dev, AB8500_MISC,
> - ? ? ? ? ? ? ? ? ? ? ? reg, (u8)lower_val);
> - ? ? ? if (ret < 0)
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? ret = abx500_set_register_interruptible(pwm->dev, AB8500_MISC,
> - ? ? ? ? ? ? ? ? ? ? ? (reg + 1), (u8)higher_val);
> -
> - ? ? ? return ret;
> -}
> -
> -int ab8500_pwm_enable(struct pwm_device *pwm)
> -{
> - ? ? ? int ret;
> -
> - ? ? ? ret = abx500_mask_and_set_register_interruptible(pwm->dev,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 1 << (pwm->pwm_id-1), 1 << (pwm->pwm_id-1));
> - ? ? ? if (ret < 0)
> - ? ? ? ? ? ? ? dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> - ? ? ? return ret;
> -}
> -
> -int ab8500_pwm_disable(struct pwm_device *pwm)
> -{
> - ? ? ? int ret;
> -
> - ? ? ? ret = abx500_mask_and_set_register_interruptible(pwm->dev,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 1 << (pwm->pwm_id-1), DISABLE_PWM);
> - ? ? ? if (ret < 0)
> - ? ? ? ? ? ? ? dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> - ? ? ? return ret;
> -}
> -
> -static int __devinit ab8500_pwm_probe(struct platform_device *pdev)
> -{
> - ? ? ? int ret = 0;
> - ? ? ? struct pwm_ops *pops;
> - ? ? ? struct pwm_device *pwm_dev;
> - ? ? ? /*
> - ? ? ? ?* Nothing to be done in probe, this is required to get the
> - ? ? ? ?* device which is required for ab8500 read and write
> - ? ? ? ?*/
> - ? ? ? pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> - ? ? ? if (pops == NULL) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> - ? ? ? ? ? ? ? return -ENOMEM;
> - ? ? ? }
> - ? ? ? pwm_dev = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
> - ? ? ? if (pwm_dev == NULL) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> - ? ? ? ? ? ? ? kfree(pops);
> - ? ? ? ? ? ? ? return -ENOMEM;
> - ? ? ? }
> - ? ? ? pops->pwm_config = ab8500_pwm_config;
> - ? ? ? pops->pwm_enable = ab8500_pwm_enable;
> - ? ? ? pops->pwm_disable = ab8500_pwm_disable;
> - ? ? ? pops->name = "ab8500";
> - ? ? ? pwm_dev->dev = &pdev->dev;
> - ? ? ? pwm_dev->pwm_id = pdev->id;
> - ? ? ? pwm_dev->pops = pops;
> - ? ? ? ret = pwm_device_register(pwm_dev);
> - ? ? ? if (ret < 0) {
> - ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to register pwm device\n");
> - ? ? ? ? ? ? ? kfree(pwm_dev);
> - ? ? ? ? ? ? ? kfree(pops);
> - ? ? ? ? ? ? ? return ret;
> - ? ? ? }
> - ? ? ? platform_set_drvdata(pdev, pwm_dev);
> - ? ? ? dev_dbg(&pdev->dev, "pwm probe successful\n");
> - ? ? ? return ret;
> -}
> -
> -static int __devexit ab8500_pwm_remove(struct platform_device *pdev)
> -{
> - ? ? ? struct pwm_device *pwm_dev = platform_get_drvdata(pdev);
> -
> - ? ? ? pwm_device_unregister(pwm_dev);
> - ? ? ? dev_dbg(&pdev->dev, "pwm driver removed\n");
> - ? ? ? kfree(pwm_dev->pops);
> - ? ? ? kfree(pwm_dev);
> - ? ? ? return 0;
> -}
> -
> -static struct platform_driver ab8500_pwm_driver = {
> - ? ? ? .driver = {
> - ? ? ? ? ? ? ? .name = "ab8500-pwm",
> - ? ? ? ? ? ? ? .owner = THIS_MODULE,
> - ? ? ? },
> - ? ? ? .probe = ab8500_pwm_probe,
> - ? ? ? .remove = __devexit_p(ab8500_pwm_remove),
> -};
> -
> -static int __init ab8500_pwm_init(void)
> -{
> - ? ? ? return platform_driver_register(&ab8500_pwm_driver);
> -}
> -
> -static void __exit ab8500_pwm_exit(void)
> -{
> - ? ? ? platform_driver_unregister(&ab8500_pwm_driver);
> -}
> -
> -subsys_initcall(ab8500_pwm_init);
> -module_exit(ab8500_pwm_exit);
> -MODULE_AUTHOR("Arun MURTHY <arun.murthy@stericsson.com>");
> -MODULE_DESCRIPTION("AB8500 Pulse Width Modulation Driver");
> -MODULE_ALIAS("AB8500 PWM driver");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index a88640c..e347365 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -14,4 +14,22 @@ menuconfig PWM_DEVICES
>
> ?if PWM_DEVICES
>
> +config AB8500_PWM
> + ? ? ? bool "AB8500 PWM support"
> + ? ? ? depends on AB8500_CORE
> + ? ? ? select HAVE_PWM
> + ? ? ? help
> + ? ? ? ? This driver exports functions to enable/disble/config/free Pulse
> + ? ? ? ? Width Modulation in the Analog Baseband Chip AB8500.
> + ? ? ? ? It is used by led and backlight driver to control the intensity.
> +
> +config TWL6030_PWM
> + ? ? ? tristate "TWL6030 PWM (Pulse Width Modulator) Support"
> + ? ? ? depends on TWL4030_CORE
> + ? ? ? select HAVE_PWM
> + ? ? ? default n
> + ? ? ? help
> + ? ? ? ? Say yes here if you want support for TWL6030 PWM.
> + ? ? ? ? This is used to control charging LED brightness.
> +
> ?endif # PWM_DEVICES
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 552f969..f35afb4 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -1 +1,4 @@
> ?obj-$(CONFIG_PWM_DEVICES) ? ? ?+= pwm-core.o
> +
> +obj-$(CONFIG_AB8500_PWM) ? ? ? += pwm-ab8500.o
> +obj-$(CONFIG_TWL6030_PWM) ? ? ?+= pwm-twl6030.o
> diff --git a/drivers/pwm/pwm-ab8500.c b/drivers/pwm/pwm-ab8500.c
> new file mode 100644
> index 0000000..d2b23b6
> --- /dev/null
> +++ b/drivers/pwm/pwm-ab8500.c
> @@ -0,0 +1,157 @@
> +/*
> + * Copyright (C) ST-Ericsson SA 2010
> + *
> + * Author: Arun R Murthy <arun.murthy@stericsson.com>
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/pwm.h>
> +#include <linux/mfd/ab8500.h>
> +#include <linux/mfd/abx500.h>
> +
> +/*
> + * PWM Out generators
> + * Bank: 0x10
> + */
> +#define AB8500_PWM_OUT_CTRL1_REG ? ? ? 0x60
> +#define AB8500_PWM_OUT_CTRL2_REG ? ? ? 0x61
> +#define AB8500_PWM_OUT_CTRL7_REG ? ? ? 0x66
> +
> +/* backlight driver constants */
> +#define ENABLE_PWM ? ? ? ? ? ? ? ? ? ? 1
> +#define DISABLE_PWM ? ? ? ? ? ? ? ? ? ?0
> +
> +static LIST_HEAD(pwm_list);
> +
> +int ab8500_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +{
> + ? ? ? int ret = 0;
> + ? ? ? unsigned int higher_val, lower_val;
> + ? ? ? u8 reg;
> +
> + ? ? ? /*
> + ? ? ? ?* get the first 8 bits that are be written to
> + ? ? ? ?* AB8500_PWM_OUT_CTRL1_REG[0:7]
> + ? ? ? ?*/
> + ? ? ? lower_val = duty_ns & 0x00FF;
> + ? ? ? /*
> + ? ? ? ?* get bits [9:10] that are to be written to
> + ? ? ? ?* AB8500_PWM_OUT_CTRL2_REG[0:1]
> + ? ? ? ?*/
> + ? ? ? higher_val = ((duty_ns & 0x0300) >> 8);
> +
> + ? ? ? reg = AB8500_PWM_OUT_CTRL1_REG + ((pwm->pwm_id - 1) * 2);
> +
> + ? ? ? ret = abx500_set_register_interruptible(pwm->dev, AB8500_MISC,
> + ? ? ? ? ? ? ? ? ? ? ? reg, (u8)lower_val);
> + ? ? ? if (ret < 0)
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? ret = abx500_set_register_interruptible(pwm->dev, AB8500_MISC,
> + ? ? ? ? ? ? ? ? ? ? ? (reg + 1), (u8)higher_val);
> +
> + ? ? ? return ret;
> +}
> +
> +int ab8500_pwm_enable(struct pwm_device *pwm)
> +{
> + ? ? ? int ret;
> +
> + ? ? ? ret = abx500_mask_and_set_register_interruptible(pwm->dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 1 << (pwm->pwm_id-1), 1 << (pwm->pwm_id-1));
> + ? ? ? if (ret < 0)
> + ? ? ? ? ? ? ? dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> + ? ? ? return ret;
> +}
> +
> +int ab8500_pwm_disable(struct pwm_device *pwm)
> +{
> + ? ? ? int ret;
> +
> + ? ? ? ret = abx500_mask_and_set_register_interruptible(pwm->dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? AB8500_MISC, AB8500_PWM_OUT_CTRL7_REG,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 1 << (pwm->pwm_id-1), DISABLE_PWM);
> + ? ? ? if (ret < 0)
> + ? ? ? ? ? ? ? dev_err(pwm->dev, "%s: Failed to disable PWM, Error %d\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> + ? ? ? return ret;
> +}
> +
> +static int __devinit ab8500_pwm_probe(struct platform_device *pdev)
> +{
> + ? ? ? int ret = 0;
> + ? ? ? struct pwm_ops *pops;
> + ? ? ? struct pwm_device *pwm_dev;
> + ? ? ? /*
> + ? ? ? ?* Nothing to be done in probe, this is required to get the
> + ? ? ? ?* device which is required for ab8500 read and write
> + ? ? ? ?*/
> + ? ? ? pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> + ? ? ? if (pops == NULL) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> + ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> + ? ? ? pwm_dev = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
> + ? ? ? if (pwm_dev == NULL) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> + ? ? ? ? ? ? ? kfree(pops);
> + ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> + ? ? ? pops->pwm_config = ab8500_pwm_config;
> + ? ? ? pops->pwm_enable = ab8500_pwm_enable;
> + ? ? ? pops->pwm_disable = ab8500_pwm_disable;
> + ? ? ? pops->name = "ab8500";
> + ? ? ? pwm_dev->dev = &pdev->dev;
> + ? ? ? pwm_dev->pwm_id = pdev->id;
> + ? ? ? pwm_dev->pops = pops;
> + ? ? ? ret = pwm_device_register(pwm_dev);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to register pwm device\n");
> + ? ? ? ? ? ? ? kfree(pwm_dev);
> + ? ? ? ? ? ? ? kfree(pops);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> + ? ? ? platform_set_drvdata(pdev, pwm_dev);
> + ? ? ? dev_dbg(&pdev->dev, "pwm probe successful\n");
> + ? ? ? return ret;
> +}
> +
> +static int __devexit ab8500_pwm_remove(struct platform_device *pdev)
> +{
> + ? ? ? struct pwm_device *pwm_dev = platform_get_drvdata(pdev);
> +
> + ? ? ? pwm_device_unregister(pwm_dev);
> + ? ? ? dev_dbg(&pdev->dev, "pwm driver removed\n");
> + ? ? ? kfree(pwm_dev->pops);
> + ? ? ? kfree(pwm_dev);
> + ? ? ? return 0;
> +}
> +
> +static struct platform_driver ab8500_pwm_driver = {
> + ? ? ? .driver = {
> + ? ? ? ? ? ? ? .name = "ab8500-pwm",
> + ? ? ? ? ? ? ? .owner = THIS_MODULE,
> + ? ? ? },
> + ? ? ? .probe = ab8500_pwm_probe,
> + ? ? ? .remove = __devexit_p(ab8500_pwm_remove),
> +};
> +
> +static int __init ab8500_pwm_init(void)
> +{
> + ? ? ? return platform_driver_register(&ab8500_pwm_driver);
> +}
> +
> +static void __exit ab8500_pwm_exit(void)
> +{
> + ? ? ? platform_driver_unregister(&ab8500_pwm_driver);
> +}
> +
> +subsys_initcall(ab8500_pwm_init);
> +module_exit(ab8500_pwm_exit);
> +MODULE_AUTHOR("Arun MURTHY <arun.murthy@stericsson.com>");
> +MODULE_DESCRIPTION("AB8500 Pulse Width Modulation Driver");
> +MODULE_ALIAS("AB8500 PWM driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pwm/pwm-twl6040.c b/drivers/pwm/pwm-twl6040.c
> new file mode 100644
> index 0000000..b78324b
> --- /dev/null
> +++ b/drivers/pwm/pwm-twl6040.c
> @@ -0,0 +1,196 @@
> +/*
> + * twl6030_pwm.c
> + * Driver for PHOENIX (TWL6030) Pulse Width Modulator
> + *
> + * Copyright (C) 2010 Texas Instruments
> + * Author: Hemanth V <hemanthv@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. ?See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. ?If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/pwm.h>
> +#include <linux/err.h>
> +#include <linux/i2c/twl.h>
> +
> +#define LED_PWM_CTRL1 ?0xF4
> +#define LED_PWM_CTRL2 ?0xF5
> +
> +/* Max value for CTRL1 register */
> +#define PWM_CTRL1_MAX ?255
> +
> +/* Pull down disable */
> +#define PWM_CTRL2_DIS_PD ? ? ? (1 << 6)
> +
> +/* Current control 2.5 milli Amps */
> +#define PWM_CTRL2_CURR_02 ? ? ?(2 << 4)
> +
> +/* LED supply source */
> +#define PWM_CTRL2_SRC_VAC ? ? ?(1 << 2)
> +
> +/* LED modes */
> +#define PWM_CTRL2_MODE_HW ? ? ?(0 << 0)
> +#define PWM_CTRL2_MODE_SW ? ? ?(1 << 0)
> +#define PWM_CTRL2_MODE_DIS ? ? (2 << 0)
> +
> +#define PWM_CTRL2_MODE_MASK ? ?0x3
> +
> +int twl6030_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> +{
> + ? ? ? u8 duty_cycle;
> + ? ? ? int ret = 0;
> +
> + ? ? ? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? duty_cycle = (duty_ns * PWM_CTRL1_MAX) / period_ns;
> +
> + ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, duty_cycle, LED_PWM_CTRL1);
> +
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? pr_err("%s: Failed to configure PWM, Error %d\n",
> + ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> + ? ? ? return 0;
> +}
> +
> +int twl6030_pwm_enable(struct pwm_device *pwm)
> +{
> + ? ? ? u8 val;
> + ? ? ? int ret = 0;
> +
> + ? ? ? ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? pr_err("%s: Failed to enable PWM, Error %d\n", pwm->label, ret);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? /* Change mode to software control */
> + ? ? ? val &= ~PWM_CTRL2_MODE_MASK;
> + ? ? ? val |= PWM_CTRL2_MODE_SW;
> +
> + ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? pr_err("%s: Failed to enable PWM, Error %d\n", pwm->label, ret);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
> + ? ? ? return 0;
> +}
> +
> +int twl6030_pwm_disable(struct pwm_device *pwm)
> +{
> + ? ? ? u8 val;
> + ? ? ? int ret = 0;
> +
> + ? ? ? ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? pr_err("%s: Failed to disable PWM, Error %d\n",
> + ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> +
> + ? ? ? val &= ~PWM_CTRL2_MODE_MASK;
> + ? ? ? val |= PWM_CTRL2_MODE_HW;
> +
> + ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? pr_err("%s: Failed to disable PWM, Error %d\n",
> + ? ? ? ? ? ? ? ? ? ? ? pwm->label, ret);
> + ? ? ? }
> + ? ? ? return ret;
> +}
> +
> +static int __devinit twl6030_pwm_probe(struct platform_device *pdev)
> +{
> + ? ? ? struct pwm_device *pwm;
> + ? ? ? struct pwm_ops *pops;
> + ? ? ? int ret;
> + ? ? ? u8 val;
> +
> + ? ? ? pwm = kzalloc(sizeof(struct pwm_device), GFP_KERNEL);
> + ? ? ? if (pwm == NULL) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> + ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> + ? ? ? pops = kzalloc(sizeof(struct pwm_ops), GFP_KERNEL);
> + ? ? ? if (pops == NULL) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to allocate memory\n");
> + ? ? ? ? ? ? ? kfree(pwm);
> + ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> +
> + ? ? ? pops->pwm_config = twl6030_pwm_config;
> + ? ? ? pops->pwm_enable = twl6030_pwm_enable;
> + ? ? ? pops->pwm_disable = twl6030_pwm_disable;
> + ? ? ? pops->name = &pdev->name;
> + ? ? ? pwm->dev = &pdev->dev;
> + ? ? ? pwm->pwm_id = pdev->id;
> + ? ? ? pwm->pops = pops;
> + ? ? ? ret = pwm_device_register(pwm);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "failed to register pwm device\n");
> + ? ? ? ? ? ? ? kfree(pwm);
> + ? ? ? ? ? ? ? kfree(pops);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> + ? ? ? platform_set_drvdata(pdev, pwm);
> + ? ? ? /* Configure PWM */
> + ? ? ? val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VAC |
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PWM_CTRL2_MODE_HW;
> +
> + ? ? ? ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2);
> + ? ? ? if (ret < 0) {
> + ? ? ? ? ? ? ? dev_err(&pdev->dev, "Failed to configure PWM, Error %d\n", ret);
> + ? ? ? ? ? ? ? return ret;
> + ? ? ? }
> + ? ? ? dev_dbg(&pdev->dev, "pwm probe successful\n");
> + ? ? ? return ret;
> +}
> +
> +static int __devexit twl6030_pwm_remove(struct platform_device *pdev)
> +{
> + ? ? ? struct pwm_device *pwm = platform_get_drvdata(pdev);
> +
> + ? ? ? pwm_device_unregister(pwm);
> + ? ? ? kfree(pwm->pops);
> + ? ? ? kfree(pwm);
> + ? ? ? dev_dbg(&pdev->dev, "pwm driver removed\n");
> + ? ? ? return 0;
> +}
> +
> +static struct platform_driver twl6030_pwm_driver = {
> + ? ? ? .driver = {
> + ? ? ? ? ? ? ? .name = "twl6030_pwm",
> + ? ? ? ? ? ? ? .owner = THIS_MODULE,
> + ? ? ? },
> + ? ? ? .probe = twl6030_pwm_probe,
> + ? ? ? .remove = __devexit_p(twl6030_pwm_remove),
> +};
> +
> +static int __init twl6030_pwm_init(void)
> +{
> + ? ? ? return platform_driver_register(&twl6030_pwm_driver);
> +}
> +
> +static void __exit twl6030_pwm_deinit(void)
> +{
> + ? ? ? platform_driver_unregister(&twl6030_pwm_driver);
> +}
> +
> +subsys_initcall(twl6030_pwm_init);
> +module_exit(twl6030_pwm_deinit);
> --
> 1.7.2.dirty
>
>

[PATCH 0/7] PWM core driver for pwm based led and backlight driver

[Arun Murthy]

The series of patch add a new pwm core driver.
Align the existing pwm drivers to make use of the pwm core driver.

Arun Murthy (7):
  pwm: Add pwm core driver
  backlight:pwm: add an element 'name' to platform data
  leds: pwm: add a new element 'name' to platform data
  pwm: Align existing pwm drivers with pwm-core driver
  platform: Update the pwm based led and backlight platform data
  pwm: move existing pwm driver to drivers/pwm
  pwm: Modify backlight and led Kconfig aligning to pwm core

 arch/arm/mach-pxa/cm-x300.c               |    1 +
 arch/arm/mach-pxa/colibri-pxa270-income.c |    1 +
 arch/arm/mach-pxa/ezx.c                   |    1 +
 arch/arm/mach-pxa/hx4700.c                |    1 +
 arch/arm/mach-pxa/lpd270.c                |    1 +
 arch/arm/mach-pxa/magician.c              |    1 +
 arch/arm/mach-pxa/mainstone.c             |    1 +
 arch/arm/mach-pxa/mioa701.c               |    1 +
 arch/arm/mach-pxa/palm27x.c               |    1 +
 arch/arm/mach-pxa/palmtc.c                |    1 +
 arch/arm/mach-pxa/palmte2.c               |    1 +
 arch/arm/mach-pxa/pcm990-baseboard.c      |    1 +
 arch/arm/mach-pxa/raumfeld.c              |    1 +
 arch/arm/mach-pxa/tavorevb.c              |    2 +
 arch/arm/mach-pxa/viper.c                 |    1 +
 arch/arm/mach-pxa/z2.c                    |    2 +
 arch/arm/mach-pxa/zylonite.c              |    1 +
 arch/arm/mach-s3c2410/mach-h1940.c        |    1 +
 arch/arm/mach-s3c2440/mach-rx1950.c       |    1 +
 arch/arm/mach-s3c64xx/mach-hmt.c          |    1 +
 arch/arm/mach-s3c64xx/mach-smartq.c       |    1 +
 arch/arm/plat-mxc/pwm.c                   |  166 +++++++++------------
 arch/arm/plat-pxa/pwm.c                   |  210 ++++++++++++--------------
 arch/arm/plat-samsung/pwm.c               |  235 +++++++++++++----------------
 arch/mips/jz4740/pwm.c                    |    2 +-
 drivers/Kconfig                           |    2 +
 drivers/Makefile                          |    1 +
 drivers/leds/Kconfig                      |    2 +-
 drivers/leds/leds-pwm.c                   |    4 +-
 drivers/mfd/Kconfig                       |    9 -
 drivers/mfd/Makefile                      |    1 -
 drivers/mfd/twl-core.c                    |   13 ++
 drivers/mfd/twl6030-pwm.c                 |  163 --------------------
 drivers/misc/Kconfig                      |    9 -
 drivers/misc/Makefile                     |    1 -
 drivers/misc/ab8500-pwm.c                 |  168 --------------------
 drivers/pwm/Kconfig                       |   33 ++++
 drivers/pwm/Makefile                      |    4 +
 drivers/pwm/pwm-ab8500.c                  |  157 +++++++++++++++++++
 drivers/pwm/pwm-core.c                    |  124 +++++++++++++++
 drivers/pwm/pwm-twl6040.c                 |  196 ++++++++++++++++++++++++
 drivers/video/backlight/Kconfig           |    2 +-
 drivers/video/backlight/pwm_bl.c          |    4 +-
 include/linux/leds_pwm.h                  |    1 +
 include/linux/pwm.h                       |   29 ++++-
 include/linux/pwm_backlight.h             |    1 +
 46 files changed, 864 insertions(+), 696 deletions(-)
 delete mode 100644 drivers/mfd/twl6030-pwm.c
 delete mode 100644 drivers/misc/ab8500-pwm.c
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/pwm-ab8500.c
 create mode 100644 drivers/pwm/pwm-core.c
 create mode 100644 drivers/pwm/pwm-twl6040.c

-- 
1.7.2.dirty

[PATCH 1/7] pwm: Add pwm core driver

[Arun Murthy]

The existing pwm based led and backlight driver makes use of the
pwm(include/linux/pwm.h). So all the board specific pwm drivers will
be exposing the same set of function name as in include/linux/pwm.h.
As a result build fails in case of multi soc environments where each soc
has a pwm device in it.

In order to overcome this issue all the pwm drivers must register to
some core pwm driver with function pointers for pwm operations (i.e
pwm_config, pwm_enable, pwm_disable).

The clients of pwm device will have to call pwm_request, wherein
they will get the pointer to struct pwm_ops. This structure include
function pointers for pwm_config, pwm_enable and pwm_disable.

Signed-off-by: Arun Murthy <arun.murthy@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 drivers/Kconfig        |    2 +
 drivers/Makefile       |    1 +
 drivers/pwm/Kconfig    |   16 ++++++
 drivers/pwm/Makefile   |    1 +
 drivers/pwm/pwm-core.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h    |   12 ++++-
 6 files changed, 160 insertions(+), 1 deletions(-)
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/pwm-core.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index a2b902f..e042f27 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -111,4 +111,6 @@ source "drivers/xen/Kconfig"
 source "drivers/staging/Kconfig"
 
 source "drivers/platform/Kconfig"
+
+source "drivers/pwm/Kconfig"
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 4ca727d..0061ec4 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -116,3 +116,4 @@ obj-$(CONFIG_STAGING)		+= staging/
 obj-y				+= platform/
 obj-y				+= ieee802154/
 obj-y				+= vbus/
+obj-$(CONFIG_PWM_DEVICES)	+= pwm/
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
new file mode 100644
index 0000000..5d10106
--- /dev/null
+++ b/drivers/pwm/Kconfig
@@ -0,0 +1,16 @@
+#
+# PWM devices
+#
+
+menuconfig PWM_DEVICES
+	bool "PWM devices"
+	default y
+	---help---
+	  Say Y here to get to see options for device drivers from various
+	  different categories. This option alone does not add any kernel code.
+
+	  If you say N, all options in this submenu will be skipped and disabled.
+
+if PWM_DEVICES
+
+endif # PWM_DEVICES
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
new file mode 100644
index 0000000..552f969
--- /dev/null
+++ b/drivers/pwm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PWM_DEVICES)	+= pwm-core.o
diff --git a/drivers/pwm/pwm-core.c b/drivers/pwm/pwm-core.c
new file mode 100644
index 0000000..b84027a
--- /dev/null
+++ b/drivers/pwm/pwm-core.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) ST-Ericsson SA 2010
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Arun R Murthy <arun.murthy@stericsson.com>
+ */
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/rwsem.h>
+#include <linux/err.h>
+#include <linux/pwm.h>
+
+struct pwm_device {
+	struct pwm_ops *pops;
+	int pwm_id;
+};
+
+struct pwm_dev_info {
+	struct pwm_device *pwm_dev;
+	struct list_head list;
+};
+static struct pwm_dev_info *di;
+
+DECLARE_RWSEM(pwm_list_lock);
+
+void __deprecated pwm_free(struct pwm_device *pwm)
+{
+}
+
+int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+{
+	return pwm->pops->pwm_config(pwm, duty_ns, period_ns);
+}
+EXPORT_SYMBOL(pwm_config);
+
+int pwm_enable(struct pwm_device *pwm)
+{
+	return pwm->pops->pwm_enable(pwm);
+}
+EXPORT_SYMBOL(pwm_enable);
+
+void pwm_disable(struct pwm_device *pwm)
+{
+	pwm->pops->pwm_disable(pwm);
+}
+EXPORT_SYMBOL(pwm_disable);
+
+int pwm_device_register(struct pwm_device *pwm_dev)
+{
+	struct pwm_dev_info *pwm;
+
+	down_write(&pwm_list_lock);
+	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
+	if (!pwm) {
+		up_write(&pwm_list_lock);
+		return -ENOMEM;
+	}
+	pwm->pwm_dev = pwm_dev;
+	list_add_tail(&pwm->list, &di->list);
+	up_write(&pwm_list_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(pwm_device_register);
+
+int pwm_device_unregister(struct pwm_device *pwm_dev)
+{
+	struct pwm_dev_info *tmp;
+	struct list_head *pos, *tmp_lst;
+
+	down_write(&pwm_list_lock);
+	list_for_each_safe(pos, tmp_lst, &di->list) {
+		tmp = list_entry(pos, struct pwm_dev_info, list);
+		if (tmp->pwm_dev == pwm_dev) {
+			list_del(pos);
+			kfree(tmp);
+			up_write(&pwm_list_lock);
+			return 0;
+		}
+	}
+	up_write(&pwm_list_lock);
+	return -ENOENT;
+}
+EXPORT_SYMBOL(pwm_device_unregister);
+
+struct pwm_device *pwm_request(int pwm_id, const char *name)
+{
+	struct pwm_dev_info *pwm;
+	struct list_head *pos;
+
+	down_read(&pwm_list_lock);
+	list_for_each(pos, &di->list) {
+		pwm = list_entry(pos, struct pwm_dev_info, list);
+		if ((!strcmp(pwm->pwm_dev->pops->name, name)) &&
+				(pwm->pwm_dev->pwm_id == pwm_id)) {
+			up_read(&pwm_list_lock);
+			return pwm->pwm_dev;
+		}
+	}
+	up_read(&pwm_list_lock);
+	return ERR_PTR(-ENOENT);
+}
+EXPORT_SYMBOL(pwm_request);
+
+static int __init pwm_init(void)
+{
+	struct pwm_dev_info *pwm;
+
+	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
+	if (!pwm)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&pwm->list);
+	di = pwm;
+	return 0;
+}
+
+static void __exit pwm_exit(void)
+{
+	kfree(di);
+}
+
+subsys_initcall(pwm_init);
+module_exit(pwm_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Arun R Murthy");
+MODULE_ALIAS("core:pwm");
+MODULE_DESCRIPTION("Core pwm driver");
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 7c77575..6e7da1f 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -3,6 +3,13 @@
 
 struct pwm_device;
 
+struct pwm_ops {
+	int (*pwm_config)(struct pwm_device *pwm, int duty_ns, int period_ns);
+	int (*pwm_enable)(struct pwm_device *pwm);
+	int (*pwm_disable)(struct pwm_device *pwm);
+	char *name;
+};
+
 /*
  * pwm_request - request a PWM device
  */
@@ -11,7 +18,7 @@ struct pwm_device *pwm_request(int pwm_id, const char *label);
 /*
  * pwm_free - free a PWM device
  */
-void pwm_free(struct pwm_device *pwm);
+void __deprecated pwm_free(struct pwm_device *pwm);
 
 /*
  * pwm_config - change a PWM device configuration
@@ -28,4 +35,7 @@ int pwm_enable(struct pwm_device *pwm);
  */
 void pwm_disable(struct pwm_device *pwm);
 
+int pwm_device_register(struct pwm_device *pwm_dev);
+int pwm_device_unregister(struct pwm_device *pwm_dev);
+
 #endif /* __LINUX_PWM_H */
-- 
1.7.2.dirty

[PATCH 1/7] pwm: Add pwm core driver

[Hemanth V]

----- Original Message ----- 
From: "Arun Murthy" <arun.murthy@stericsson.com>


> The existing pwm based led and backlight driver makes use of the
> pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> be exposing the same set of function name as in include/linux/pwm.h.
> As a result build fails in case of multi soc environments where each soc
> has a pwm device in it.

This seems very specific to ST environment,  looking at the driver list from
( [PATCH 4/7] pwm: Align existing pwm drivers with pwm-core ) it seems
most multi SOC environments might support PWM in either one of the SOC.

arch/arm/plat-mxc/pwm.c
arch/arm/plat-pxa/pwm.c
arch/arm/plat-samsung/pwm.c
arch/mips/jz4740/pwm.c
drivers/mfd/twl6030-pwm.c

Unless people have examples of other SOCs which might use this,
the better approach might be to go for a custom driver rather than changing
the framework.

Thanks
Hemanth 

[PATCH 1/7] pwm: Add pwm core driver

[Samuel Ortiz]

On Tue, Sep 28, 2010 at 06:23:24PM +0530, Hemanth V wrote:
> ----- Original Message ----- From: "Arun Murthy"
> <arun.murthy@stericsson.com>
> 
> 
> >The existing pwm based led and backlight driver makes use of the
> >pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> >be exposing the same set of function name as in include/linux/pwm.h.
> >As a result build fails in case of multi soc environments where each soc
> >has a pwm device in it.
> 
> This seems very specific to ST environment,  
No it's not. It's an issue Arun has hit while enabling one of the ST MFD chip,
but he's tackling a generic issue.

> looking at the driver list from
> ( [PATCH 4/7] pwm: Align existing pwm drivers with pwm-core ) it seems
> most multi SOC environments might support PWM in either one of the SOC.
> 
> arch/arm/plat-mxc/pwm.c
> arch/arm/plat-pxa/pwm.c
> arch/arm/plat-samsung/pwm.c
> arch/mips/jz4740/pwm.c
> drivers/mfd/twl6030-pwm.c
> 
> Unless people have examples of other SOCs which might use this,
> the better approach might be to go for a custom driver rather than changing
> the framework.
I wouldn't call the current pwm code a framework. It's a bunch of header
definitions that happens to work in the specific case of 1 pwm per
sub architecture.
What Arun is proposing is an actual framework. And it seems to be clean and
simple enough.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH 1/7] pwm: Add pwm core driver

[Felipe Balbi]

On Tue, Sep 28, 2010 at 08:06:11AM -0500, Samuel Ortiz wrote:
>On Tue, Sep 28, 2010 at 06:23:24PM +0530, Hemanth V wrote:
>> ----- Original Message ----- From: "Arun Murthy"
>> <arun.murthy@stericsson.com>
>>
>>
>> >The existing pwm based led and backlight driver makes use of the
>> >pwm(include/linux/pwm.h). So all the board specific pwm drivers will
>> >be exposing the same set of function name as in include/linux/pwm.h.
>> >As a result build fails in case of multi soc environments where each soc
>> >has a pwm device in it.
>>
>> This seems very specific to ST environment,
>No it's not. It's an issue Arun has hit while enabling one of the ST MFD chip,
>but he's tackling a generic issue.
>
>> looking at the driver list from
>> ( [PATCH 4/7] pwm: Align existing pwm drivers with pwm-core ) it seems
>> most multi SOC environments might support PWM in either one of the SOC.
>>
>> arch/arm/plat-mxc/pwm.c
>> arch/arm/plat-pxa/pwm.c
>> arch/arm/plat-samsung/pwm.c
>> arch/mips/jz4740/pwm.c
>> drivers/mfd/twl6030-pwm.c
>>
>> Unless people have examples of other SOCs which might use this,
>> the better approach might be to go for a custom driver rather than changing
>> the framework.
>I wouldn't call the current pwm code a framework. It's a bunch of header
>definitions that happens to work in the specific case of 1 pwm per
>sub architecture.
>What Arun is proposing is an actual framework. And it seems to be clean and
>simple enough.

FWIW, I agree with you Sam. Sooner or later, this will hit other SoCs.

-- 
balbi