[PATCH v2] implement a generic PWM framework

[PATCH v2] implement a generic PWM framework

[Sascha Hauer]

Thanks for the comments to the last version, hopefully I addressed
them all. Here is another round of the patches.

changes since last version:

- only kfree() in case of errors
- add missing kfree() in pwmchip_remove()
- drop dynamic pwm id support
- fix typos, remove unused variables
- instantiate pwm_base_common in mxs-pwm driver from resources

The following changes since commit b0af8dfdd67699e25083478c63eedef2e72ebd85:

  Linux 3.0-rc5 (2011-06-27 19:12:22 -0700)

are available in the git repository at:
  git://git.pengutronix.de/git/imx/linux-2.6.git pwm

Sascha Hauer (2):
      PWM: add pwm framework support
      pwm: Add a i.MX23/28 pwm driver

 Documentation/pwm.txt |   56 +++++++++
 MAINTAINERS           |    5 +
 drivers/Kconfig       |    2 +
 drivers/Makefile      |    1 +
 drivers/pwm/Kconfig   |   16 +++
 drivers/pwm/Makefile  |    2 +
 drivers/pwm/core.c    |  220 +++++++++++++++++++++++++++++++++
 drivers/pwm/mxs-pwm.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h   |   37 ++++++
 9 files changed, 660 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/pwm.txt
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/core.c
 create mode 100644 drivers/pwm/mxs-pwm.c

[PATCH 1/2] PWM: add pwm framework support

[Sascha Hauer]

This patch adds framework support for PWM (pulse width modulation) devices.

The is a barebone PWM API already in the kernel under include/linux/pwm.h,
but it does not allow for multiple drivers as each of them implements the
pwm_*() functions.

There are other PWM framework patches around from Bill Gatliff. Unlike
his framework this one does not change the existing API for PWMs so that
this framework can act as a drop in replacement for the existing API.

Why another framework?

Several people argue that there should not be another framework for PWMs
but they should be integrated into one of the existing frameworks like led
or hwmon. Unlike these frameworks the PWM framework is agnostic to the
purpose of the PWM. In fact, a PWM can drive a LED, but this makes the
LED framework a user of a PWM, like already done in leds-pwm.c. The gpio
framework also is not suitable for PWMs. Every gpio could be turned into
a PWM using timer based toggling, but on the other hand not every PWM hardware
device can be turned into a gpio due to the lack of hardware capabilities.

This patch does not try to improve the PWM API yet, this could be done in
subsequent patches.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/pwm.txt |   56 +++++++++++++
 MAINTAINERS           |    5 +
 drivers/Kconfig       |    2 +
 drivers/Makefile      |    1 +
 drivers/pwm/Kconfig   |   12 +++
 drivers/pwm/Makefile  |    1 +
 drivers/pwm/core.c    |  220 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h   |   37 ++++++++
 8 files changed, 334 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/pwm.txt
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/core.c

diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
new file mode 100644
index 0000000..c7c5cb1
--- /dev/null
+++ b/Documentation/pwm.txt
@@ -0,0 +1,56 @@
+Pulse Width Modulation (PWM) interface
+
+This provides an overview about the Linux PWM interface
+
+PWMs are commonly used for controlling LEDs, fans or vibrators in
+cell phones. PWMs with a fixed purpose have no need implementing
+the Linux PWM API (although they could). However, PWMs are often
+found as discrete devices on SoCs which have no fixed purpose. It's
+up to the board designer to connect them to LEDs or fans. To provide
+this kind of flexibility the generic PWM API exists.
+
+Identifying PWMs
+----------------
+
+PWMs are identified by unique ids throughout the system. A platform
+should call pwmchip_reserve() during init time to reserve the id range
+for internal PWMs so that users have a fixed id to refer to specific
+PWMs.
+
+Using PWMs
+----------
+
+A PWM can be requested using pwm_request() and freed after usage with
+pwm_free(). After being requested a PWM has to be configured using
+
+int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
+
+To start/stop toggling the PWM output use pwm_enable()/pwm_disable().
+
+Implementing a PWM driver
+-------------------------
+
+Currently there are two ways to implement pwm drivers. Traditionally
+there only has been the barebone API meaning that each driver has
+to implement the pwm_*() functions itself. This means that it's impossible
+to have multiple PWM drivers in the system. For this reason it's mandatory
+for new drivers to use the generic PWM framework.
+A new PWM device can be added using pwmchip_add() and removed again with
+pwmchip_remove(). pwmchip_add() takes a filled in struct pwm_chip as
+argument which provides the ops and the pwm id to the framework.
+
+Locking
+-------
+
+The PWM core list manipulations are protected by a mutex, so pwm_request()
+and pwm_free() may not be called from an atomic context. Currently the
+PWM core does not enforce any locking to pwm_enable(), pwm_disable() and
+pwm_config(), so the calling context is currently driver specific. This
+is an issue derived from the former barebone API and should be fixed soon.
+
+Helpers
+-------
+
+Currently a PWM can only be configured with period_ns and duty_ns. For several
+use cases freq_hz and duty_percent might be better. Instead of calculating 
+this in your driver please consider adding appropriate helpers to the framework.
diff --git a/MAINTAINERS b/MAINTAINERS
index f0358cd..5a3a713 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5142,6 +5142,11 @@ M:	Robert Jarzmik <robert.jarzmik@free.fr>
 L:	rtc-linux@googlegroups.com
 S:	Maintained
 
+PWM core
+M:	Sascha Hauer <s.hauer@pengutronix.de>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+
 QLOGIC QLA1280 SCSI DRIVER
 M:	Michael Reed <mdr@sgi.com>
 L:	linux-scsi@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 3bb154d..67f5c27 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -126,4 +126,6 @@ source "drivers/hwspinlock/Kconfig"
 
 source "drivers/clocksource/Kconfig"
 
+source "drivers/pwm/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 09f3232..c321763 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -6,6 +6,7 @@
 #
 
 obj-y				+= gpio/
+obj-y				+= pwm/
 obj-$(CONFIG_PCI)		+= pci/
 obj-$(CONFIG_PARISC)		+= parisc/
 obj-$(CONFIG_RAPIDIO)		+= rapidio/
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
new file mode 100644
index 0000000..93c1052
--- /dev/null
+++ b/drivers/pwm/Kconfig
@@ -0,0 +1,12 @@
+menuconfig PWM
+	bool "PWM Support"
+	help
+	  This enables PWM support through the generic PWM framework.
+	  You only need to enable this, if you also want to enable
+	  one or more of the PWM drivers below.
+
+	  If unsure, say N.
+
+if PWM
+
+endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
new file mode 100644
index 0000000..3469c3d
--- /dev/null
+++ b/drivers/pwm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PWM)		+= core.o
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
new file mode 100644
index 0000000..71de479
--- /dev/null
+++ b/drivers/pwm/core.c
@@ -0,0 +1,220 @@
+/*
+ * Generic pwmlib implementation
+ *
+ * Copyright (C) 2011 Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2, or (at your option)
+ *  any later version.
+ *
+ *  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; see the file COPYING.  If not, write to
+ *  the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+#include <linux/module.h>
+#include <linux/pwm.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+
+struct pwm_device {
+	struct			pwm_chip *chip;
+	const char		*label;
+	unsigned long		flags;
+#define FLAG_REQUESTED	0
+#define FLAG_ENABLED	1
+	struct list_head	node;
+};
+
+static LIST_HEAD(pwm_list);
+
+static DEFINE_MUTEX(pwm_lock);
+
+static struct pwm_device *_find_pwm(int pwm_id)
+{
+	struct pwm_device *pwm;
+
+	list_for_each_entry(pwm, &pwm_list, node) {
+		if (pwm->chip->pwm_id == pwm_id)
+			return pwm;
+	}
+
+	return NULL;
+}
+
+/**
+ * pwmchip_add() - register a new pwm
+ * @chip: the pwm
+ *
+ * register a new pwm. pwm->pwm_id must be initialized. if pwm_id < 0 then
+ * a dynamically assigned id will be used, otherwise the id specified,
+ */
+int pwmchip_add(struct pwm_chip *chip)
+{
+	struct pwm_device *pwm;
+	int ret = 0;
+
+	pwm = kzalloc(sizeof(*pwm), GFP_KERNEL);
+	if (!pwm)
+		return -ENOMEM;
+
+	pwm->chip = chip;
+
+	mutex_lock(&pwm_lock);
+
+	if (chip->pwm_id >= 0 && _find_pwm(chip->pwm_id)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	list_add_tail(&pwm->node, &pwm_list);
+out:
+	mutex_unlock(&pwm_lock);
+
+	if (ret)
+		kfree(pwm);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pwmchip_add);
+
+/**
+ * pwmchip_remove() - remove a pwm
+ * @chip: the pwm
+ *
+ * remove a pwm. This function may return busy if the pwm is still requested.
+ */
+int pwmchip_remove(struct pwm_chip *chip)
+{
+	struct pwm_device *pwm;
+	int ret = 0;
+
+	mutex_lock(&pwm_lock);
+
+	pwm = _find_pwm(chip->pwm_id);
+	if (!pwm) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	if (test_bit(FLAG_REQUESTED, &pwm->flags)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	list_del(&pwm->node);
+
+	kfree(pwm);
+out:
+	mutex_unlock(&pwm_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pwmchip_remove);
+
+/*
+ * pwm_request - request a PWM device
+ */
+struct pwm_device *pwm_request(int pwm_id, const char *label)
+{
+	struct pwm_device *pwm;
+	int ret;
+
+	mutex_lock(&pwm_lock);
+
+	pwm = _find_pwm(pwm_id);
+	if (!pwm) {
+		pwm = ERR_PTR(-ENOENT);
+		goto out;
+	}
+
+	if (test_bit(FLAG_REQUESTED, &pwm->flags)) {
+		pwm = ERR_PTR(-EBUSY);
+		goto out;
+	}
+
+	if (!try_module_get(pwm->chip->ops->owner)) {
+		pwm = ERR_PTR(-ENODEV);
+		goto out;
+	}
+
+	if (pwm->chip->ops->request) {
+		ret = pwm->chip->ops->request(pwm->chip);
+		if (ret) {
+			pwm = ERR_PTR(ret);
+			goto out_put;
+		}
+	}
+
+	pwm->label = label;
+	set_bit(FLAG_REQUESTED, &pwm->flags);
+
+	goto out;
+
+out_put:
+	module_put(pwm->chip->ops->owner);
+out:
+	mutex_unlock(&pwm_lock);
+
+	return pwm;
+}
+EXPORT_SYMBOL_GPL(pwm_request);
+
+/*
+ * pwm_free - free a PWM device
+ */
+void pwm_free(struct pwm_device *pwm)
+{
+	mutex_lock(&pwm_lock);
+
+	if (!test_and_clear_bit(FLAG_REQUESTED, &pwm->flags)) {
+		pr_warning("PWM device already freed\n");
+		goto out;
+	}
+
+	pwm->label = NULL;
+
+	module_put(pwm->chip->ops->owner);
+out:
+	mutex_unlock(&pwm_lock);
+}
+EXPORT_SYMBOL_GPL(pwm_free);
+
+/*
+ * pwm_config - change a PWM device configuration
+ */
+int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+{
+	return pwm->chip->ops->config(pwm->chip, duty_ns, period_ns);
+}
+EXPORT_SYMBOL_GPL(pwm_config);
+
+/*
+ * pwm_enable - start a PWM output toggling
+ */
+int pwm_enable(struct pwm_device *pwm)
+{
+	if (!test_and_set_bit(FLAG_ENABLED, &pwm->flags))
+		return pwm->chip->ops->enable(pwm->chip);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwm_enable);
+
+/*
+ * pwm_disable - stop a PWM output toggling
+ */
+void pwm_disable(struct pwm_device *pwm)
+{
+	if (test_and_clear_bit(FLAG_ENABLED, &pwm->flags))
+		pwm->chip->ops->disable(pwm->chip);
+}
+EXPORT_SYMBOL_GPL(pwm_disable);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 7c77575..df9681b 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -28,4 +28,41 @@ int pwm_enable(struct pwm_device *pwm);
  */
 void pwm_disable(struct pwm_device *pwm);
 
+#ifdef CONFIG_PWM
+struct pwm_chip;
+
+/**
+ * struct pwm_ops - PWM operations
+ * @request: optional hook for requesting a PWM
+ * @free: optional hook for freeing a PWM
+ * @config: configure duty cycles and period length for this PWM
+ * @enable: enable PWM output toggling
+ * @disable: disable PWM output toggling
+ */
+struct pwm_ops {
+	int			(*request)(struct pwm_chip *chip);
+	void			(*free)(struct pwm_chip *chip);
+	int			(*config)(struct pwm_chip *chip, int duty_ns,
+						int period_ns);
+	int			(*enable)(struct pwm_chip *chip);
+	void			(*disable)(struct pwm_chip *chip);
+	struct module		*owner;
+};
+
+/**
+ * struct pwm_chip - abstract a PWM
+ * @label: for diagnostics
+ * @owner: helps prevent removal of modules exporting active PWMs
+ * @ops: The callbacks for this PWM
+ */
+struct pwm_chip {
+	int			pwm_id;
+	const char		*label;
+	struct pwm_ops		*ops;
+};
+
+int pwmchip_add(struct pwm_chip *chip);
+int pwmchip_remove(struct pwm_chip *chip);
+#endif
+
 #endif /* __LINUX_PWM_H */
-- 
1.7.5.3

Re: [PATCH 1/2] PWM: add pwm framework support

[Kurt Van Dijck]

On Wed, Jun 29, 2011 at 11:03:34AM +0200, Sascha Hauer wrote:
> This patch adds framework support for PWM (pulse width modulation) devices.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

Re: [PATCH 1/2] PWM: add pwm framework support

[Arnd Bergmann]

On Wednesday 29 June 2011, Sascha Hauer wrote:
> This patch adds framework support for PWM (pulse width modulation) devices.

Hi Sascha,

The code looks all good now, but it seems you missed the three trivial remarks
I had about the documentation. It would be good to just fix them, but not
essential.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 1/2] PWM: add pwm framework support

[Matthias Kaehlcke]

El Wed, Jun 29, 2011 at 11:03:34AM +0200 Sascha Hauer ha dit:

> This patch adds framework support for PWM (pulse width modulation) devices.
> 
> The is a barebone PWM API already in the kernel under include/linux/pwm.h,
> but it does not allow for multiple drivers as each of them implements the
> pwm_*() functions.
> 
> There are other PWM framework patches around from Bill Gatliff. Unlike
> his framework this one does not change the existing API for PWMs so that
> this framework can act as a drop in replacement for the existing API.
> 
> Why another framework?
> 
> Several people argue that there should not be another framework for PWMs
> but they should be integrated into one of the existing frameworks like led
> or hwmon. Unlike these frameworks the PWM framework is agnostic to the
> purpose of the PWM. In fact, a PWM can drive a LED, but this makes the
> LED framework a user of a PWM, like already done in leds-pwm.c. The gpio
> framework also is not suitable for PWMs. Every gpio could be turned into
> a PWM using timer based toggling, but on the other hand not every PWM hardware
> device can be turned into a gpio due to the lack of hardware capabilities.
> 
> This patch does not try to improve the PWM API yet, this could be done in
> subsequent patches.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Matthias Kaehlcke <matthias@kaehlcke.net>

-- 
Matthias Kaehlcke
Embedded Linux Developer
Amsterdam

            You must have long-range goals to keep you from
                being frustrated by short-range failure
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

[PATCH 2/2] pwm: Add a i.MX23/28 pwm driver

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/Kconfig   |    6 +-
 drivers/pwm/Makefile  |    1 +
 drivers/pwm/mxs-pwm.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 327 insertions(+), 1 deletions(-)
 create mode 100644 drivers/pwm/mxs-pwm.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 93c1052..5694574 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -2,11 +2,15 @@ menuconfig PWM
 	bool "PWM Support"
 	help
 	  This enables PWM support through the generic PWM framework.
-	  You only need to enable this, if you also want to enable
+	  You only need to enable this if you also want to enable
 	  one or more of the PWM drivers below.
 
 	  If unsure, say N.
 
 if PWM
 
+config PWM_MXS
+	tristate "MXS pwm support"
+	depends on ARCH_MXS
+
 endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 3469c3d..2cadd50 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_PWM)		+= core.o
+obj-$(CONFIG_PWM_MXS)		+= mxs-pwm.o
diff --git a/drivers/pwm/mxs-pwm.c b/drivers/pwm/mxs-pwm.c
new file mode 100644
index 0000000..6788f48
--- /dev/null
+++ b/drivers/pwm/mxs-pwm.c
@@ -0,0 +1,321 @@
+/*
+ * Copyright (C) 2011 Pengutronix
+ * Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * simple driver for PWM (Pulse Width Modulator) controller
+ *
+ * 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.
+ *
+ * Derived from pxa PWM driver by eric miao <eric.miao@marvell.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/pwm.h>
+#include <mach/hardware.h>
+#include <asm/div64.h>
+
+struct mxs_pwm_device {
+	struct device		*dev;
+
+	struct clk	*clk;
+
+	int		enabled;
+	void __iomem	*mmio_base;
+
+	unsigned int	pwm_id;
+
+	u32		val_active;
+	u32		val_period;
+	int		period_us;
+	struct pwm_chip chip;
+};
+
+/* common register space */
+#define REG_PWM_CTRL		0x0
+#define REG_PWM_CTRL_SET	0x4
+#define REG_PWM_CTRL_CLEAR	0x8
+#define PWM_SFTRST	(1 << 31)
+#define PWM_CLKGATE	(1 << 30)
+#define PWM_ENABLE(p)	(1 << (p))
+
+/* per pwm register space */
+#define REG_ACTIVE	0x0
+#define REG_PERIOD	0x10
+
+#define PERIOD_PERIOD(p)	((p) & 0xffff)
+#define PERIOD_ACTIVE_HIGH	(3 << 16)
+#define PERIOD_INACTIVE_LOW	(2 << 18)
+#define PERIOD_CDIV(div)	(((div) & 0x7) << 20)
+
+static void pwm_update(struct mxs_pwm_device *pwm)
+{
+	writel(pwm->val_active, pwm->mmio_base + REG_ACTIVE);
+	writel(pwm->val_period, pwm->mmio_base + REG_PERIOD);
+}
+
+#define to_mxs_pwm_device(chip)	container_of(chip, struct mxs_pwm_device, chip)
+
+static int mxs_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+	int div = 0;
+	unsigned long rate;
+	unsigned long long c;
+	unsigned long period_cycles, duty_cycles;
+
+	rate = clk_get_rate(pwm->clk);
+
+	dev_dbg(pwm->dev, "config: duty_ns: %d, period_ns: %d (clkrate %ld)\n",
+			duty_ns, period_ns, rate);
+
+	while (1) {
+		c = rate / (1 << div);
+		c = c * period_ns;
+		do_div(c, 1000000000);
+		if (c < 0x10000)
+			break;
+		div++;
+
+		if (div > 8)
+			return -EINVAL;
+	}
+
+	period_cycles = c;
+	duty_cycles = period_cycles * duty_ns / period_ns;
+
+	dev_dbg(pwm->dev, "config period_cycles: %ld duty_cycles: %ld\n",
+			period_cycles, duty_cycles);
+
+	pwm->val_active = period_cycles << 16 | duty_cycles;
+	pwm->val_period = PERIOD_PERIOD(period_cycles) | PERIOD_ACTIVE_HIGH |
+			PERIOD_INACTIVE_LOW | PERIOD_CDIV(div);
+	pwm->period_us = period_ns / 1000;
+
+	pwm_update(pwm);
+
+	return 0;
+}
+
+static void __iomem *pwm_base_common;
+static int num_instances;
+static DEFINE_MUTEX(pwm_common_mutex);
+
+/*
+ * each pwm has a separate register space but all share a common
+ * enable register.
+ */
+static int mxs_pwm_common_get(struct platform_device *pdev)
+{
+	struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	resource_size_t start = r->start & ~0xfff;
+	int ret = 0;
+
+	mutex_lock(&pwm_common_mutex);
+
+	if (!num_instances) {
+		r = request_mem_region(start, 0x10, "mxs-pwm-common");
+		if (!r)
+			goto err_request;
+
+		pwm_base_common = ioremap(start, 0x10);
+		if (!pwm_base_common) {
+			ret = -ENOMEM;
+			goto err_ioremap;
+		}
+	}
+
+	writel(PWM_SFTRST | PWM_CLKGATE, pwm_base_common + REG_PWM_CTRL_CLEAR);
+
+	num_instances++;
+
+	mutex_unlock(&pwm_common_mutex);
+
+	return 0;
+
+err_ioremap:
+	release_mem_region(start, 0x10);
+err_request:
+	mutex_unlock(&pwm_common_mutex);
+	return ret;
+}
+
+static void mxs_pwm_common_put(struct platform_device *pdev)
+{
+	struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	resource_size_t start = r->start & ~0xfff;
+
+	mutex_lock(&pwm_common_mutex);
+
+	num_instances--;
+
+	if (!num_instances) {
+		iounmap(pwm_base_common);
+		release_mem_region(start, 0x10);
+
+		writel(PWM_CLKGATE, pwm_base_common + REG_PWM_CTRL_SET);
+	}
+
+	mutex_unlock(&pwm_common_mutex);
+}
+
+static void __pwm_enable(struct mxs_pwm_device *pwm, int enable)
+{
+	void __iomem *reg;
+
+	if (enable)
+		reg = pwm_base_common + REG_PWM_CTRL_SET;
+	else
+		reg = pwm_base_common + REG_PWM_CTRL_CLEAR;
+
+	writel(PWM_ENABLE(pwm->chip.pwm_id), reg);
+}
+
+static int mxs_pwm_enable(struct pwm_chip *chip)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+	int ret = 0;
+
+	dev_dbg(pwm->dev, "enable\n");
+
+	if (!pwm->enabled) {
+		ret = clk_enable(pwm->clk);
+		if (!ret) {
+			pwm->enabled = 1;
+			__pwm_enable(pwm, 1);
+			pwm_update(pwm);
+		}
+	}
+	return ret;
+}
+
+static void mxs_pwm_disable(struct pwm_chip *chip)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+
+	dev_dbg(pwm->dev, "disable\n");
+
+	if (pwm->enabled) {
+		/*
+		 * We need a little delay here, it takes one period for
+		 * the last pwm_config call to take effect. If we disable
+		 * the pwm too early it just freezes the current output
+		 * state.
+		 */
+		usleep_range(pwm->period_us, pwm->period_us * 2);
+		__pwm_enable(pwm, 0);
+		clk_disable(pwm->clk);
+		pwm->enabled = 0;
+	}
+}
+
+static struct pwm_ops mxs_pwm_ops = {
+	.enable = mxs_pwm_enable,
+	.disable = mxs_pwm_disable,
+	.config = mxs_pwm_config,
+	.owner = THIS_MODULE,
+};
+
+static int __devinit mxs_pwm_probe(struct platform_device *pdev)
+{
+	struct mxs_pwm_device *pwm;
+	struct resource *r;
+	int ret = 0;
+
+	pwm = devm_kzalloc(&pdev->dev, sizeof(struct mxs_pwm_device), GFP_KERNEL);
+	if (pwm == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	pwm->chip.ops = &mxs_pwm_ops;
+	pwm->chip.pwm_id = pdev->id;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r)
+		return -ENODEV;
+
+	r = devm_request_mem_region(&pdev->dev, r->start, resource_size(r),
+			pdev->name);
+	if (!r)
+		return -EBUSY;
+
+	pwm->mmio_base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (!pwm->mmio_base) {
+		dev_err(&pdev->dev, "failed to ioremap() registers\n");
+		return -ENOMEM;
+	}
+
+	pwm->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pwm->clk)) {
+		ret = PTR_ERR(pwm->clk);
+		goto err_out;
+	}
+
+	ret = mxs_pwm_common_get(pdev);
+	if (ret)
+		goto err_free_clk;
+
+	ret = pwmchip_add(&pwm->chip);
+	if (ret)
+		goto err_remove_common;
+
+	platform_set_drvdata(pdev, pwm);
+	return 0;
+
+err_remove_common:
+	mxs_pwm_common_put(pdev);
+err_free_clk:
+	clk_put(pwm->clk);
+err_out:
+	return ret;
+}
+
+static int __devexit mxs_pwm_remove(struct platform_device *pdev)
+{
+	struct mxs_pwm_device *pwm;
+	int ret;
+
+	pwm = platform_get_drvdata(pdev);
+
+	ret = pwmchip_remove(&pwm->chip);
+	if (ret)
+		return ret;
+
+	clk_put(pwm->clk);
+
+	mxs_pwm_common_put(pdev);
+
+	return 0;
+}
+
+static struct platform_driver mxs_pwm_driver = {
+	.driver		= {
+		.name	= "mxs-pwm",
+	},
+	.probe		= mxs_pwm_probe,
+	.remove		= __devexit_p(mxs_pwm_remove),
+};
+
+static int __init mxs_pwm_init(void)
+{
+	return platform_driver_register(&mxs_pwm_driver);
+}
+arch_initcall(mxs_pwm_init);
+
+static void __exit mxs_pwm_exit(void)
+{
+	platform_driver_unregister(&mxs_pwm_driver);
+}
+module_exit(mxs_pwm_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
-- 
1.7.5.3

Re: [PATCH 2/2] pwm: Add a i.MX23/28 pwm driver

[Arnd Bergmann]

On Wednesday 29 June 2011, Sascha Hauer wrote:
> +/*
> + * each pwm has a separate register space but all share a common
> + * enable register.
> + */
> +static int mxs_pwm_common_get(struct platform_device *pdev)
> +{
> +       struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       resource_size_t start = r->start & ~0xfff;
> +       int ret = 0;
> +
> +       if (!num_instances) {
> +               r = request_mem_region(start, 0x10, "mxs-pwm-common");
> +               if (!r)
> +                       goto err_request;

Yes, this looks better than the original approach, but it still feels
a bit awkward: 

You are requesting a region outside of the platform device resource.
This will cause problems with the device tree conversion, where the
idea is to list all registers that are used for each device.
It also becomes a problem if a system has multiple PWM regions
that are a page long each -- you only map one of them currently,
so the first one would win.

When you model the pwm device in the device tree, the most logical
representation IMHO would be to have a nested device, like:

/amba/pwm_core@0fa0000/pwm@0
                      /pwm@1
                      /pwm@2

The pwm_core then has the MMIO registers and provides an interface
for the individual pwms to access the registers, like an MFD
device. The resources for the slave devices are not MMIO ranges
but simply offsets. The pwm_enable function will then do something
like 

static void __pwm_enable(struct mxs_pwm_device *pwm, int enable)
{
	struct device *parent = &pwm->chip.dev.parent->parent;
	void __iomem *pwm_base_common = dev_get_drvdata(parent);

        if (enable)
                reg = pwm_base_common + REG_PWM_CTRL_SET;
        else
                reg = pwm_base_common + REG_PWM_CTRL_CLEAR;
 
        writel(PWM_ENABLE(pwm->chip.pwm_id), reg);
}

The pwm driver obviously has to register for both device types,
the parent and the child, and do different things in the two
cases, e.g.

static int __devinit mxs_pwm_probe(struct platform_device *pdev)
{
	switch (pdev->id_entry->driver_data) {
	case MXS_PWM_MASTER:
		return mxs_pwm_map_master_resources(pdev);
	case MXS_PWM_SLAVE:
		return mxs_pwm_register_pwmchip(pdev, to_platform_device(pdev->dev.parent));
	}
	return -ENODEV;
}

I'm normally not that picky, but I think we should have the best
possible solution for this in the mx23 driver, because it will
likely be used as an example for other drivers that have the
same problem.

	Arnd

[RFC] implement a generic PWM framework - once again

[Sascha Hauer]

Hi All,

The following implements a generic pwm framework and adds a user
for it. I already posted this series back in January. Based on
the comments I received I added some details about the motivation
for adding such a framework and not using the led or hwmon framework
to patch 1/2.
I also added some documentation to Documentation/pwm.txt.

This patch does not change the user API for PWMs, in particular
it does not enforce any sleep/nonsleep context to the PWM users.
The patch merely puts the status quo into a core wrapper to be able
to register multiple PWM drivers in the system. Improvements to
the API can still be made later once we have at least a place
in the kernel to collect the existing PWM drivers.

Sascha

The following changes since commit b0af8dfdd67699e25083478c63eedef2e72ebd85:

  Linux 3.0-rc5 (2011-06-27 19:12:22 -0700)

are available in the git repository at:
  git://git.pengutronix.de/git/imx/linux-2.6.git pwm

Sascha Hauer (2):
      PWM: add pwm framework support
      pwm: Add a i.MX23/28 pwm driver

 Documentation/pwm.txt |   56 ++++++++++
 MAINTAINERS           |    5 +
 drivers/Kconfig       |    2 +
 drivers/Makefile      |    1 +
 drivers/pwm/Kconfig   |   12 ++
 drivers/pwm/Makefile  |    1 +
 drivers/pwm/core.c    |  246 +++++++++++++++++++++++++++++++++++++++++++
 drivers/pwm/mxs-pwm.c |  275 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h   |   38 +++++++
 9 files changed, 636 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/pwm.txt
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/core.c
 create mode 100644 drivers/pwm/mxs-pwm.c

[PATCH 2/2] pwm: Add a i.MX23/28 pwm driver

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/mxs-pwm.c |  275 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 275 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/mxs-pwm.c

diff --git a/drivers/pwm/mxs-pwm.c b/drivers/pwm/mxs-pwm.c
new file mode 100644
index 0000000..052cb0c
--- /dev/null
+++ b/drivers/pwm/mxs-pwm.c
@@ -0,0 +1,275 @@
+/*
+ * Copyright (C) 2011 Pengutronix
+ * Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * simple driver for PWM (Pulse Width Modulator) controller
+ *
+ * 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.
+ *
+ * Derived from pxa PWM driver by eric miao <eric.miao@marvell.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/pwm.h>
+#include <mach/hardware.h>
+#include <mach/mxs.h>
+#include <mach/mx23.h>
+#include <mach/mx28.h>
+#include <asm/div64.h>
+
+struct mxs_pwm_device {
+	struct device		*dev;
+
+	struct clk	*clk;
+
+	int		enabled;
+	void __iomem	*mmio_base;
+
+	unsigned int	pwm_id;
+
+	u32		val_active;
+	u32		val_period;
+	int		period_us;
+	struct pwm_chip chip;
+};
+
+/* common register space */
+static void __iomem *pwm_base_common;
+#define REG_PWM_CTRL	0x0
+#define PWM_SFTRST	(1 << 31)
+#define PWM_CLKGATE	(1 << 30)
+#define PWM_ENABLE(p)	(1 << (p))
+
+/* per pwm register space */
+#define REG_ACTIVE	0x0
+#define REG_PERIOD	0x10
+
+#define PERIOD_PERIOD(p)	((p) & 0xffff)
+#define PERIOD_ACTIVE_HIGH	(3 << 16)
+#define PERIOD_INACTIVE_LOW	(2 << 18)
+#define PERIOD_CDIV(div)	(((div) & 0x7) << 20)
+
+static void pwm_update(struct mxs_pwm_device *pwm)
+{
+	writel(pwm->val_active, pwm->mmio_base + REG_ACTIVE);
+	writel(pwm->val_period, pwm->mmio_base + REG_PERIOD);
+}
+
+#define to_mxs_pwm_device(chip)	container_of(chip, struct mxs_pwm_device, chip)
+
+static int mxs_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+	int div = 0;
+	unsigned long rate;
+	unsigned long long c;
+	unsigned long period_cycles, duty_cycles;
+
+	rate = clk_get_rate(pwm->clk);
+
+	dev_dbg(pwm->dev, "config: duty_ns: %d, period_ns: %d (clkrate %ld)\n",
+			duty_ns, period_ns, rate);
+
+	while (1) {
+		c = rate / (1 << div);
+		c = c * period_ns;
+		do_div(c, 1000000000);
+		if (c < 0x10000)
+			break;
+		div++;
+
+		if (div > 8)
+			return -EINVAL;
+	}
+
+	period_cycles = c;
+	duty_cycles = period_cycles * duty_ns / period_ns;
+
+	dev_dbg(pwm->dev, "config period_cycles: %ld duty_cycles: %ld\n",
+			period_cycles, duty_cycles);
+
+	pwm->val_active = period_cycles << 16 | duty_cycles;
+	pwm->val_period = PERIOD_PERIOD(period_cycles) | PERIOD_ACTIVE_HIGH |
+			PERIOD_INACTIVE_LOW | PERIOD_CDIV(div);
+	pwm->period_us = period_ns / 1000;
+
+	pwm_update(pwm);
+
+	return 0;
+}
+
+static void __pwm_enable(struct mxs_pwm_device *pwm, int enable)
+{
+	if (enable)
+		__mxs_setl(PWM_ENABLE(pwm->chip.pwm_id), pwm_base_common + REG_PWM_CTRL);
+	else
+		__mxs_clrl(PWM_ENABLE(pwm->chip.pwm_id), pwm_base_common + REG_PWM_CTRL);
+}
+
+static int mxs_pwm_enable(struct pwm_chip *chip)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+	int ret = 0;
+
+	dev_dbg(pwm->dev, "enable\n");
+
+	if (!pwm->enabled) {
+		ret = clk_enable(pwm->clk);
+		if (!ret) {
+			pwm->enabled = 1;
+			__pwm_enable(pwm, 1);
+			pwm_update(pwm);
+		}
+	}
+	return ret;
+}
+
+static void mxs_pwm_disable(struct pwm_chip *chip)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+
+	dev_dbg(pwm->dev, "disable\n");
+
+	if (pwm->enabled) {
+		/*
+		 * We need a little delay here, it takes one period for
+		 * the last pwm_config call to take effect. If we disable
+		 * the pwm too early it just freezes the current output
+		 * state.
+		 */
+		usleep_range(pwm->period_us, pwm->period_us * 2);
+		__pwm_enable(pwm, 0);
+		clk_disable(pwm->clk);
+		pwm->enabled = 0;
+	}
+}
+
+static struct pwm_ops mxs_pwm_ops = {
+	.enable = mxs_pwm_enable,
+	.disable = mxs_pwm_disable,
+	.config = mxs_pwm_config,
+};
+
+static int __devinit mxs_pwm_probe(struct platform_device *pdev)
+{
+	struct mxs_pwm_device *pwm;
+	struct resource *r;
+	int ret = 0;
+
+	pwm = devm_kzalloc(&pdev->dev, sizeof(struct mxs_pwm_device), GFP_KERNEL);
+	if (pwm == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	pwm->clk = clk_get(&pdev->dev, NULL);
+
+	if (IS_ERR(pwm->clk)) {
+		ret = PTR_ERR(pwm->clk);
+		goto err_free;
+	}
+
+	pwm->chip.ops = &mxs_pwm_ops;
+
+	pwm->enabled = 0;
+
+	pwm->chip.pwm_id = pdev->id;
+	pwm->chip.owner = THIS_MODULE;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		dev_err(&pdev->dev, "no memory resource defined\n");
+		ret = -ENODEV;
+		goto err_free_clk;
+	}
+
+	r = request_mem_region(r->start, resource_size(r), pdev->name);
+	if (r == NULL) {
+		dev_err(&pdev->dev, "failed to request memory resource\n");
+		ret = -EBUSY;
+		goto err_free_clk;
+	}
+
+	pwm->mmio_base = ioremap(r->start, resource_size(r));
+	if (pwm->mmio_base == NULL) {
+		dev_err(&pdev->dev, "failed to ioremap() registers\n");
+		ret = -ENODEV;
+		goto err_free_mem;
+	}
+
+	ret = pwmchip_add(&pwm->chip);
+	if (ret)
+		goto err_free_mem;
+
+	platform_set_drvdata(pdev, pwm);
+	return 0;
+
+err_free_mem:
+	release_mem_region(r->start, resource_size(r));
+err_free_clk:
+	clk_put(pwm->clk);
+err_free:
+	kfree(pwm);
+	return ret;
+}
+
+static int __devexit mxs_pwm_remove(struct platform_device *pdev)
+{
+	struct mxs_pwm_device *pwm;
+	struct resource *r;
+	int ret;
+
+	pwm = platform_get_drvdata(pdev);
+
+	ret = pwmchip_remove(&pwm->chip);
+	if (ret)
+		return ret;
+
+	iounmap(pwm->mmio_base);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(r->start, resource_size(r));
+
+	clk_put(pwm->clk);
+
+	return 0;
+}
+
+static struct platform_driver mxs_pwm_driver = {
+	.driver		= {
+		.name	= "mxs-pwm",
+	},
+	.probe		= mxs_pwm_probe,
+	.remove		= __devexit_p(mxs_pwm_remove),
+};
+
+static int __init mxs_pwm_init(void)
+{
+	if (cpu_is_mx28())
+		pwm_base_common = MX28_IO_ADDRESS(MX28_PWM_BASE_ADDR);
+	else
+		pwm_base_common = MX23_IO_ADDRESS(MX23_PWM_BASE_ADDR);
+
+	__mxs_clrl(PWM_SFTRST | PWM_CLKGATE, pwm_base_common + REG_PWM_CTRL);
+
+	return platform_driver_register(&mxs_pwm_driver);
+}
+arch_initcall(mxs_pwm_init);
+
+static void __exit mxs_pwm_exit(void)
+{
+	platform_driver_unregister(&mxs_pwm_driver);
+}
+module_exit(mxs_pwm_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
-- 
1.7.5.3

Re: [PATCH 2/2] pwm: Add a i.MX23/28 pwm driver

[Lothar Waßmann]

Hi,

just some nitpicks...

Sascha Hauer writes:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/pwm/mxs-pwm.c |  275 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 275 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/pwm/mxs-pwm.c
> 
> diff --git a/drivers/pwm/mxs-pwm.c b/drivers/pwm/mxs-pwm.c
> new file mode 100644
> index 0000000..052cb0c
> --- /dev/null
> +++ b/drivers/pwm/mxs-pwm.c
> @@ -0,0 +1,275 @@
> +/*
> + * Copyright (C) 2011 Pengutronix
> + * Sascha Hauer <s.hauer@pengutronix.de>
> + *
> + * simple driver for PWM (Pulse Width Modulator) controller
> + *
> + * 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.
> + *
> + * Derived from pxa PWM driver by eric miao <eric.miao@marvell.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/pwm.h>
> +#include <mach/hardware.h>
> +#include <mach/mxs.h>
> +#include <mach/mx23.h>
> +#include <mach/mx28.h>
> +#include <asm/div64.h>
> +
> +struct mxs_pwm_device {
> +	struct device		*dev;
> +
> +	struct clk	*clk;
> +
> +	int		enabled;
> +	void __iomem	*mmio_base;
> +
> +	unsigned int	pwm_id;
> +
> +	u32		val_active;
> +	u32		val_period;
> +	int		period_us;
> +	struct pwm_chip chip;
> +};
> +
> +/* common register space */
> +static void __iomem *pwm_base_common;
> +#define REG_PWM_CTRL	0x0
> +#define PWM_SFTRST	(1 << 31)
> +#define PWM_CLKGATE	(1 << 30)
> +#define PWM_ENABLE(p)	(1 << (p))
> +
> +/* per pwm register space */
> +#define REG_ACTIVE	0x0
> +#define REG_PERIOD	0x10
> +
> +#define PERIOD_PERIOD(p)	((p) & 0xffff)
> +#define PERIOD_ACTIVE_HIGH	(3 << 16)
> +#define PERIOD_INACTIVE_LOW	(2 << 18)
> +#define PERIOD_CDIV(div)	(((div) & 0x7) << 20)
> +
[...]
> +static int __devinit mxs_pwm_probe(struct platform_device *pdev)
> +{
> +	struct mxs_pwm_device *pwm;
> +	struct resource *r;
> +	int ret = 0;
> +
> +	pwm = devm_kzalloc(&pdev->dev, sizeof(struct mxs_pwm_device), GFP_KERNEL);
> +	if (pwm == NULL) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	pwm->clk = clk_get(&pdev->dev, NULL);
> +
> +	if (IS_ERR(pwm->clk)) {
Why an empty line here, but not in other similar places?

> +		ret = PTR_ERR(pwm->clk);
> +		goto err_free;
> +	}
> +
> +	pwm->chip.ops = &mxs_pwm_ops;
> +
> +	pwm->enabled = 0;
is already 0 due to kzalloc().

> +
> +	pwm->chip.pwm_id = pdev->id;
> +	pwm->chip.owner = THIS_MODULE;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (r == NULL) {
> +		dev_err(&pdev->dev, "no memory resource defined\n");
> +		ret = -ENODEV;
> +		goto err_free_clk;
> +	}
> +
> +	r = request_mem_region(r->start, resource_size(r), pdev->name);
> +	if (r == NULL) {
> +		dev_err(&pdev->dev, "failed to request memory resource\n");
> +		ret = -EBUSY;
> +		goto err_free_clk;
> +	}
> +
> +	pwm->mmio_base = ioremap(r->start, resource_size(r));
> +	if (pwm->mmio_base == NULL) {
> +		dev_err(&pdev->dev, "failed to ioremap() registers\n");
> +		ret = -ENODEV;
-ENOMEM?.

> +	if (ret)
> +		goto err_free_mem;
> +
> +	platform_set_drvdata(pdev, pwm);
> +	return 0;
> +
> +err_free_mem:
> +	release_mem_region(r->start, resource_size(r));
> +err_free_clk:
> +	clk_put(pwm->clk);
> +err_free:
> +	kfree(pwm);
> +	return ret;
> +}
> +
> +static int __devexit mxs_pwm_remove(struct platform_device *pdev)
> +{
> +	struct mxs_pwm_device *pwm;
> +	struct resource *r;
> +	int ret;
> +
> +	pwm = platform_get_drvdata(pdev);
> +
> +	ret = pwmchip_remove(&pwm->chip);
> +	if (ret)
> +		return ret;
> +
> +	iounmap(pwm->mmio_base);
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	release_mem_region(r->start, resource_size(r));
> +
> +	clk_put(pwm->clk);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mxs_pwm_driver = {
> +	.driver		= {
> +		.name	= "mxs-pwm",
> +	},
> +	.probe		= mxs_pwm_probe,
> +	.remove		= __devexit_p(mxs_pwm_remove),
> +};
> +
> +static int __init mxs_pwm_init(void)
> +{
> +	if (cpu_is_mx28())
> +		pwm_base_common = MX28_IO_ADDRESS(MX28_PWM_BASE_ADDR);
> +	else
> +		pwm_base_common = MX23_IO_ADDRESS(MX23_PWM_BASE_ADDR);
> +
> +	__mxs_clrl(PWM_SFTRST | PWM_CLKGATE, pwm_base_common + REG_PWM_CTRL);
> +
Shouldn't CLKGATE be set again in the remove() function?



Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

Re: [PATCH 2/2] pwm: Add a i.MX23/28 pwm driver

[Arnd Bergmann]

Hi Sascha,

On Tuesday 28 June 2011, Sascha Hauer wrote:
> +
> +/* common register space */
> +static void __iomem *pwm_base_common;
> +#define REG_PWM_CTRL   0x0
> +#define PWM_SFTRST     (1 << 31)
> +#define PWM_CLKGATE    (1 << 30)
> +#define PWM_ENABLE(p)  (1 << (p))
> +

The driver looks pretty good overall, but the global pwm_base_common register
is rather ugly. I think this should really be passed down through resources
from  the platform device in one way or another.

	Arnd

[RFC] add pwmlib support

[Sascha Hauer]

Hi all,

While implementing just another pwm driver I thought it's time to implement
generic pwm support. The following series adds drivers/pwm/pwmlib.c and
a i.MX23/28 pwm driver which serves as a usage example for pwmlib. The
code is inspired by gpiolib support and tested using the backlight pwm
driver by Eric Miao.

Currently the pwm_request, pwm_add, pwm_remove and pwm_free operations are
protected with a single mutex wherea the pwm_enable, pwm_disable and
pwm_config operations are unlocked. It is assumed that the owners of the
pwm handle the serialization of the pwm accesses. This may not be enough, so
i'd like to discuss the locking (and type of locking) here.

I Cced the people working with PWMs in the kernel in the hope that they can
give input on what's missing / wrong in this implementation

Sascha


The following changes since commit 2b1caf6ed7b888c95a1909d343799672731651a5:

  Merge branch 'core-fixes-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip (2011-01-20 18:30:37 -0800)

are available in the git repository at:

  git://git.pengutronix.de/git/imx/linux-2.6.git pwmlib

Sascha Hauer (2):
      pwmlib: add pwm support
      pwm: Add a i.MX23/28 pwm driver

 drivers/Kconfig       |    2 +
 drivers/Makefile      |    1 +
 drivers/pwm/Kconfig   |   16 +++
 drivers/pwm/Makefile  |    3 +
 drivers/pwm/mxs-pwm.c |  275 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pwm/pwmlib.c  |  246 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm.h   |   38 +++++++
 7 files changed, 581 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/mxs-pwm.c
 create mode 100644 drivers/pwm/pwmlib.c

[PATCH 2/2] pwm: Add a i.MX23/28 pwm driver

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/Kconfig   |    5 +
 drivers/pwm/Makefile  |    2 +
 drivers/pwm/mxs-pwm.c |  275 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 282 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/mxs-pwm.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 04ddbfd..f9a728b 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -8,4 +8,9 @@ menuconfig PWMLIB
 	  If unsure, say N.
 
 if PWMLIB
+
+config PWM_MXS
+	tristate "MXS pwm support"
+	depends on ARCH_MXS
+
 endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 389c049..1c6441c 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -1 +1,3 @@
 obj-$(CONFIG_PWMLIB)		+= pwmlib.o
+
+obj-$(CONFIG_PWM_MXS)		+= mxs-pwm.o
diff --git a/drivers/pwm/mxs-pwm.c b/drivers/pwm/mxs-pwm.c
new file mode 100644
index 0000000..052cb0c
--- /dev/null
+++ b/drivers/pwm/mxs-pwm.c
@@ -0,0 +1,275 @@
+/*
+ * Copyright (C) 2011 Pengutronix
+ * Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * simple driver for PWM (Pulse Width Modulator) controller
+ *
+ * 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.
+ *
+ * Derived from pxa PWM driver by eric miao <eric.miao@marvell.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/pwm.h>
+#include <mach/hardware.h>
+#include <mach/mxs.h>
+#include <mach/mx23.h>
+#include <mach/mx28.h>
+#include <asm/div64.h>
+
+struct mxs_pwm_device {
+	struct device		*dev;
+
+	struct clk	*clk;
+
+	int		enabled;
+	void __iomem	*mmio_base;
+
+	unsigned int	pwm_id;
+
+	u32		val_active;
+	u32		val_period;
+	int		period_us;
+	struct pwm_chip chip;
+};
+
+/* common register space */
+static void __iomem *pwm_base_common;
+#define REG_PWM_CTRL	0x0
+#define PWM_SFTRST	(1 << 31)
+#define PWM_CLKGATE	(1 << 30)
+#define PWM_ENABLE(p)	(1 << (p))
+
+/* per pwm register space */
+#define REG_ACTIVE	0x0
+#define REG_PERIOD	0x10
+
+#define PERIOD_PERIOD(p)	((p) & 0xffff)
+#define PERIOD_ACTIVE_HIGH	(3 << 16)
+#define PERIOD_INACTIVE_LOW	(2 << 18)
+#define PERIOD_CDIV(div)	(((div) & 0x7) << 20)
+
+static void pwm_update(struct mxs_pwm_device *pwm)
+{
+	writel(pwm->val_active, pwm->mmio_base + REG_ACTIVE);
+	writel(pwm->val_period, pwm->mmio_base + REG_PERIOD);
+}
+
+#define to_mxs_pwm_device(chip)	container_of(chip, struct mxs_pwm_device, chip)
+
+static int mxs_pwm_config(struct pwm_chip *chip, int duty_ns, int period_ns)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+	int div = 0;
+	unsigned long rate;
+	unsigned long long c;
+	unsigned long period_cycles, duty_cycles;
+
+	rate = clk_get_rate(pwm->clk);
+
+	dev_dbg(pwm->dev, "config: duty_ns: %d, period_ns: %d (clkrate %ld)\n",
+			duty_ns, period_ns, rate);
+
+	while (1) {
+		c = rate / (1 << div);
+		c = c * period_ns;
+		do_div(c, 1000000000);
+		if (c < 0x10000)
+			break;
+		div++;
+
+		if (div > 8)
+			return -EINVAL;
+	}
+
+	period_cycles = c;
+	duty_cycles = period_cycles * duty_ns / period_ns;
+
+	dev_dbg(pwm->dev, "config period_cycles: %ld duty_cycles: %ld\n",
+			period_cycles, duty_cycles);
+
+	pwm->val_active = period_cycles << 16 | duty_cycles;
+	pwm->val_period = PERIOD_PERIOD(period_cycles) | PERIOD_ACTIVE_HIGH |
+			PERIOD_INACTIVE_LOW | PERIOD_CDIV(div);
+	pwm->period_us = period_ns / 1000;
+
+	pwm_update(pwm);
+
+	return 0;
+}
+
+static void __pwm_enable(struct mxs_pwm_device *pwm, int enable)
+{
+	if (enable)
+		__mxs_setl(PWM_ENABLE(pwm->chip.pwm_id), pwm_base_common + REG_PWM_CTRL);
+	else
+		__mxs_clrl(PWM_ENABLE(pwm->chip.pwm_id), pwm_base_common + REG_PWM_CTRL);
+}
+
+static int mxs_pwm_enable(struct pwm_chip *chip)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+	int ret = 0;
+
+	dev_dbg(pwm->dev, "enable\n");
+
+	if (!pwm->enabled) {
+		ret = clk_enable(pwm->clk);
+		if (!ret) {
+			pwm->enabled = 1;
+			__pwm_enable(pwm, 1);
+			pwm_update(pwm);
+		}
+	}
+	return ret;
+}
+
+static void mxs_pwm_disable(struct pwm_chip *chip)
+{
+	struct mxs_pwm_device *pwm = to_mxs_pwm_device(chip);
+
+	dev_dbg(pwm->dev, "disable\n");
+
+	if (pwm->enabled) {
+		/*
+		 * We need a little delay here, it takes one period for
+		 * the last pwm_config call to take effect. If we disable
+		 * the pwm too early it just freezes the current output
+		 * state.
+		 */
+		usleep_range(pwm->period_us, pwm->period_us * 2);
+		__pwm_enable(pwm, 0);
+		clk_disable(pwm->clk);
+		pwm->enabled = 0;
+	}
+}
+
+static struct pwm_ops mxs_pwm_ops = {
+	.enable = mxs_pwm_enable,
+	.disable = mxs_pwm_disable,
+	.config = mxs_pwm_config,
+};
+
+static int __devinit mxs_pwm_probe(struct platform_device *pdev)
+{
+	struct mxs_pwm_device *pwm;
+	struct resource *r;
+	int ret = 0;
+
+	pwm = devm_kzalloc(&pdev->dev, sizeof(struct mxs_pwm_device), GFP_KERNEL);
+	if (pwm == NULL) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	pwm->clk = clk_get(&pdev->dev, NULL);
+
+	if (IS_ERR(pwm->clk)) {
+		ret = PTR_ERR(pwm->clk);
+		goto err_free;
+	}
+
+	pwm->chip.ops = &mxs_pwm_ops;
+
+	pwm->enabled = 0;
+
+	pwm->chip.pwm_id = pdev->id;
+	pwm->chip.owner = THIS_MODULE;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r == NULL) {
+		dev_err(&pdev->dev, "no memory resource defined\n");
+		ret = -ENODEV;
+		goto err_free_clk;
+	}
+
+	r = request_mem_region(r->start, resource_size(r), pdev->name);
+	if (r == NULL) {
+		dev_err(&pdev->dev, "failed to request memory resource\n");
+		ret = -EBUSY;
+		goto err_free_clk;
+	}
+
+	pwm->mmio_base = ioremap(r->start, resource_size(r));
+	if (pwm->mmio_base == NULL) {
+		dev_err(&pdev->dev, "failed to ioremap() registers\n");
+		ret = -ENODEV;
+		goto err_free_mem;
+	}
+
+	ret = pwmchip_add(&pwm->chip);
+	if (ret)
+		goto err_free_mem;
+
+	platform_set_drvdata(pdev, pwm);
+	return 0;
+
+err_free_mem:
+	release_mem_region(r->start, resource_size(r));
+err_free_clk:
+	clk_put(pwm->clk);
+err_free:
+	kfree(pwm);
+	return ret;
+}
+
+static int __devexit mxs_pwm_remove(struct platform_device *pdev)
+{
+	struct mxs_pwm_device *pwm;
+	struct resource *r;
+	int ret;
+
+	pwm = platform_get_drvdata(pdev);
+
+	ret = pwmchip_remove(&pwm->chip);
+	if (ret)
+		return ret;
+
+	iounmap(pwm->mmio_base);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(r->start, resource_size(r));
+
+	clk_put(pwm->clk);
+
+	return 0;
+}
+
+static struct platform_driver mxs_pwm_driver = {
+	.driver		= {
+		.name	= "mxs-pwm",
+	},
+	.probe		= mxs_pwm_probe,
+	.remove		= __devexit_p(mxs_pwm_remove),
+};
+
+static int __init mxs_pwm_init(void)
+{
+	if (cpu_is_mx28())
+		pwm_base_common = MX28_IO_ADDRESS(MX28_PWM_BASE_ADDR);
+	else
+		pwm_base_common = MX23_IO_ADDRESS(MX23_PWM_BASE_ADDR);
+
+	__mxs_clrl(PWM_SFTRST | PWM_CLKGATE, pwm_base_common + REG_PWM_CTRL);
+
+	return platform_driver_register(&mxs_pwm_driver);
+}
+arch_initcall(mxs_pwm_init);
+
+static void __exit mxs_pwm_exit(void)
+{
+	platform_driver_unregister(&mxs_pwm_driver);
+}
+module_exit(mxs_pwm_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
-- 
1.7.2.3