[RFC 1/6] [PWM] Generic PWM API implementation

[RFC 1/6] [PWM] Generic PWM API implementation

[Bill Gatliff]

Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
---
 drivers/pwm/pwm.c |  667 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 667 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/pwm.c

diff --git a/drivers/pwm/pwm.c b/drivers/pwm/pwm.c
new file mode 100644
index 0000000..2f28c20
--- /dev/null
+++ b/drivers/pwm/pwm.c
@@ -0,0 +1,667 @@
+/*
+ * drivers/pwm/pwm.c
+ *
+ * Copyright (C) 2008 Bill Gatliff
+ *
+ * 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 of the License, 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; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/spinlock.h>
+#include <linux/fs.h>
+#include <linux/completion.h>
+#include <linux/workqueue.h>
+#include <linux/pwm.h>
+
+
+static int __pwm_create_sysfs(struct pwm_device *pwm);
+
+static LIST_HEAD(pwm_device_list);
+static DEFINE_MUTEX(device_list_mutex);
+static struct class pwm_class;
+static struct workqueue_struct *pwm_handler_workqueue;
+
+
+int pwm_register(struct pwm_device *pwm)
+{
+	struct pwm_channel *p;
+	int wchan;
+	int ret = 0;
+
+	spin_lock_init(&pwm->list_lock);
+
+	p = kcalloc(pwm->nchan, sizeof(struct pwm_channel), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	for (wchan = 0; wchan < pwm->nchan; wchan++) {
+		spin_lock_init(&p[wchan].lock);
+		init_completion(&p[wchan].complete);
+		p[wchan].chan = wchan;
+		p[wchan].pwm = pwm;
+	}
+
+	pwm->channels = p;
+
+	mutex_lock(&device_list_mutex);
+
+	list_add_tail(&pwm->list, &pwm_device_list);
+	ret = __pwm_create_sysfs(pwm);
+	if (ret) {
+		mutex_unlock(&device_list_mutex);
+		goto err_create_sysfs;
+	}
+
+	mutex_unlock(&device_list_mutex);
+
+	pr_info("%s: %d channels\n", pwm->bus_id, pwm->nchan);
+	return 0;
+
+err_create_sysfs:
+	kfree(p);
+
+	return ret;
+}
+EXPORT_SYMBOL(pwm_register);
+
+
+static int __match_device(struct device *dev, void *data)
+{
+	return dev_get_drvdata(dev) == data;
+}
+
+
+int pwm_unregister(struct pwm_device *pwm)
+{
+	int wchan;
+	struct device *dev;
+
+	mutex_lock(&device_list_mutex);
+
+	for (wchan = 0; wchan < pwm->nchan; wchan++) {
+		if (pwm->channels[wchan].flags & FLAG_REQUESTED) {
+			mutex_unlock(&device_list_mutex);
+			return -EBUSY;
+		}
+	}
+
+	for (wchan = 0; wchan < pwm->nchan; wchan++) {
+		dev = class_find_device(&pwm_class, NULL,
+					&pwm->channels[wchan],
+					__match_device);
+		if (dev) {
+			put_device(dev);
+			device_unregister(dev);
+		}
+	}
+
+	kfree(pwm->channels);
+	list_del(&pwm->list);
+	mutex_unlock(&device_list_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL(pwm_unregister);
+
+
+static struct pwm_device *
+__pwm_find_device(const char *bus_id)
+{
+	struct pwm_device *p;
+
+	list_for_each_entry(p, &pwm_device_list, list)
+	{
+		if (!strcmp(bus_id, p->bus_id))
+			return p;
+	}
+	return NULL;
+}
+
+
+static int
+__pwm_request_channel(struct pwm_channel *p,
+		      const char *requester)
+{
+	int ret;
+
+	if (test_and_set_bit(FLAG_REQUESTED, &p->flags))
+		return -EBUSY;
+
+	if (p->pwm->request) {
+		ret = p->pwm->request(p);
+		if (ret) {
+			clear_bit(FLAG_REQUESTED, &p->flags);
+			return ret;
+		}
+	}
+
+	p->requester = requester;
+	return 0;
+}
+
+
+struct pwm_channel *
+pwm_request(const char *bus_id,
+	    int chan,
+	    const char *requester)
+{
+	struct pwm_device *p;
+	int ret;
+
+	mutex_lock(&device_list_mutex);
+
+	p = __pwm_find_device(bus_id);
+	if (!p || chan >= p->nchan)
+		goto err_no_device;
+
+	if (!try_module_get(p->owner))
+		goto err_module_get_failed;
+
+	ret = __pwm_request_channel(&p->channels[chan], requester);
+	if (ret)
+		goto err_request_failed;
+
+	mutex_unlock(&device_list_mutex);
+
+	pr_debug("%s: %s:%d returns %p\n", __func__,
+		 bus_id, chan, &p->channels[chan]);
+
+	return &p->channels[chan];
+
+err_request_failed:
+	module_put(p->owner);
+err_module_get_failed:
+err_no_device:
+
+	mutex_unlock(&device_list_mutex);
+
+	pr_debug("%s: %s:%d returns NULL\n",
+		 __func__, bus_id, chan);
+
+	return NULL;
+}
+EXPORT_SYMBOL(pwm_request);
+
+
+void pwm_free(struct pwm_channel *p)
+{
+	mutex_lock(&device_list_mutex);
+
+	if (!test_and_clear_bit(FLAG_REQUESTED, &p->flags))
+		goto done;
+
+	pwm_stop(p);
+	pwm_unsynchronize(p, NULL);
+	pwm_set_handler(p, NULL, NULL);
+
+	if (p->pwm->free)
+		p->pwm->free(p);
+	module_put(p->pwm->owner);
+
+	pr_debug("%s: %s:%d free\n",
+		 __func__, p->pwm->bus_id, p->chan);
+
+done:
+	mutex_unlock(&device_list_mutex);
+}
+EXPORT_SYMBOL(pwm_free);
+
+
+unsigned long pwm_ns_to_ticks(struct pwm_channel *p,
+			      unsigned long nsecs)
+{
+	unsigned long long ticks;
+
+	ticks = nsecs;
+	ticks *= p->tick_hz;
+	do_div(ticks, 1000000000);
+	return ticks;
+}
+EXPORT_SYMBOL(pwm_ns_to_ticks);
+
+
+unsigned long pwm_ticks_to_ns(struct pwm_channel *p,
+			      unsigned long ticks)
+{
+	unsigned long long ns;
+
+	if (!p->tick_hz)
+		return 0;
+
+	ns = ticks;
+	ns *= 1000000000UL;
+	do_div(ns, p->tick_hz);
+	return ns;
+}
+EXPORT_SYMBOL(pwm_ticks_to_ns);
+
+
+static void
+pwm_config_ns_to_ticks(struct pwm_channel *p,
+		       struct pwm_channel_config *c)
+{
+	if (c->config_mask & PWM_CONFIG_PERIOD_NS) {
+		c->period_ticks = pwm_ns_to_ticks(p, c->period_ns);
+		c->config_mask &= ~PWM_CONFIG_PERIOD_NS;
+		c->config_mask |= PWM_CONFIG_PERIOD_TICKS;
+	}
+
+	if (c->config_mask & PWM_CONFIG_DUTY_NS) {
+		c->duty_ticks = pwm_ns_to_ticks(p, c->duty_ns);
+		c->config_mask &= ~PWM_CONFIG_DUTY_NS;
+		c->config_mask |= PWM_CONFIG_DUTY_TICKS;
+	}
+}
+
+
+static void
+pwm_config_percent_to_ticks(struct pwm_channel *p,
+			    struct pwm_channel_config *c)
+{
+	if (c->config_mask & PWM_CONFIG_DUTY_PERCENT) {
+		if (c->config_mask & PWM_CONFIG_PERIOD_TICKS)
+			c->duty_ticks = c->period_ticks;
+		else
+			c->duty_ticks = p->period_ticks;
+
+		c->duty_ticks *= c->duty_percent;
+		c->duty_ticks /= 100;
+		c->config_mask &= ~PWM_CONFIG_DUTY_PERCENT;
+		c->config_mask |= PWM_CONFIG_DUTY_TICKS;
+	}
+}
+
+
+int pwm_config_nosleep(struct pwm_channel *p,
+		       struct pwm_channel_config *c)
+{
+	if (!p->pwm->config_nosleep)
+		return -EINVAL;
+
+	pwm_config_ns_to_ticks(p, c);
+	pwm_config_percent_to_ticks(p, c);
+
+	return p->pwm->config_nosleep(p, c);
+}
+EXPORT_SYMBOL(pwm_config_nosleep);
+
+
+int pwm_config(struct pwm_channel *p,
+	       struct pwm_channel_config *c)
+{
+	int ret = 0;
+
+	if (unlikely(!p->pwm->config)) {
+		pr_debug("%s: %s:%d has no config handler (-EINVAL)\n",
+			 __func__, p->pwm->bus_id, p->chan);
+		return -EINVAL;
+	}
+
+	pwm_config_ns_to_ticks(p, c);
+	pwm_config_percent_to_ticks(p, c);
+
+	switch (c->config_mask & (PWM_CONFIG_PERIOD_TICKS
+				  | PWM_CONFIG_DUTY_TICKS)) {
+	case PWM_CONFIG_PERIOD_TICKS:
+		if (p->duty_ticks > c->period_ticks) {
+			ret = -EINVAL;
+			goto err;
+		}
+		break;
+	case PWM_CONFIG_DUTY_TICKS:
+		if (p->period_ticks < c->duty_ticks) {
+			ret = -EINVAL;
+			goto err;
+		}
+		break;
+	case PWM_CONFIG_DUTY_TICKS | PWM_CONFIG_PERIOD_TICKS:
+		if (c->duty_ticks > c->period_ticks) {
+			ret = -EINVAL;
+			goto err;
+		}
+		break;
+	default:
+		break;
+	}
+
+err:
+	pr_debug("%s: config_mask %d period_ticks %lu duty_ticks %lu"
+		 " polarity %d duty_ns %lu period_ns %lu duty_percent %d\n",
+		 __func__, c->config_mask, c->period_ticks, c->duty_ticks,
+		 c->polarity, c->duty_ns, c->period_ns, c->duty_percent);
+
+	if (ret)
+		return ret;
+	return p->pwm->config(p, c);
+}
+EXPORT_SYMBOL(pwm_config);
+
+
+int pwm_period_ns(struct pwm_channel *p,
+		  unsigned long period_ns)
+{
+	struct pwm_channel_config c = {
+		.config_mask = PWM_CONFIG_PERIOD_TICKS,
+		.period_ticks = pwm_ns_to_ticks(p, period_ns),
+	};
+
+	return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_period_ns);
+
+
+int pwm_duty_ns(struct pwm_channel *p,
+		unsigned long duty_ns)
+{
+	struct pwm_channel_config c = {
+		.config_mask = PWM_CONFIG_DUTY_TICKS,
+		.duty_ticks = pwm_ns_to_ticks(p, duty_ns),
+	};
+	return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_duty_ns);
+
+
+int pwm_duty_percent(struct pwm_channel *p,
+		     int percent)
+{
+	struct pwm_channel_config c = {
+		.config_mask = PWM_CONFIG_DUTY_PERCENT,
+		.duty_percent = percent,
+	};
+	return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_duty_percent);
+
+
+int pwm_polarity(struct pwm_channel *p,
+		 int active_high)
+{
+	struct pwm_channel_config c = {
+		.config_mask = PWM_CONFIG_POLARITY,
+		.polarity = active_high,
+	};
+	return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_polarity);
+
+
+int pwm_start(struct pwm_channel *p)
+{
+	struct pwm_channel_config c = {
+		.config_mask = PWM_CONFIG_START,
+	};
+	return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_start);
+
+
+int pwm_stop(struct pwm_channel *p)
+{
+	struct pwm_channel_config c = {
+		.config_mask = PWM_CONFIG_STOP,
+	};
+	return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_stop);
+
+
+int pwm_synchronize(struct pwm_channel *p,
+		    struct pwm_channel *to_p)
+{
+	if (p->pwm != to_p->pwm) {
+		/* TODO: support cross-device synchronization */
+		return -EINVAL;
+	}
+
+	if (!p->pwm->synchronize)
+		return -EINVAL;
+
+	return p->pwm->synchronize(p, to_p);
+}
+EXPORT_SYMBOL(pwm_synchronize);
+
+
+int pwm_unsynchronize(struct pwm_channel *p,
+		      struct pwm_channel *from_p)
+{
+	if (from_p && (p->pwm != from_p->pwm)) {
+		/* TODO: support cross-device synchronization */
+		return -EINVAL;
+	}
+
+	if (!p->pwm->unsynchronize)
+		return -EINVAL;
+
+	return p->pwm->unsynchronize(p, from_p);
+}
+EXPORT_SYMBOL(pwm_unsynchronize);
+
+
+static void pwm_handler(struct work_struct *w)
+{
+	struct pwm_channel *p = container_of(w, struct pwm_channel,
+					     handler_work);
+	if (p->handler && p->handler(p, p->handler_data))
+		pwm_stop(p);
+}
+
+
+static void __pwm_callback(struct pwm_channel *p)
+{
+	queue_work(pwm_handler_workqueue, &p->handler_work);
+	pr_debug("%s:%d handler %p scheduled with data %p\n",
+		 p->pwm->bus_id, p->chan, p->handler, p->handler_data);
+}
+
+
+int pwm_set_handler(struct pwm_channel *p,
+		    pwm_handler_t handler,
+		    void *data)
+{
+	if (p->pwm->set_callback) {
+		p->handler_data = data;
+		p->handler = handler;
+		INIT_WORK(&p->handler_work, pwm_handler);
+		return p->pwm->set_callback(p, handler ? __pwm_callback : NULL);
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL(pwm_set_handler);
+
+
+static ssize_t pwm_run_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf,
+			     size_t len)
+{
+	struct pwm_channel *p = dev_get_drvdata(dev);
+	if (sysfs_streq(buf, "1"))
+		pwm_start(p);
+	else if (sysfs_streq(buf, "0"))
+		pwm_stop(p);
+	return len;
+}
+static DEVICE_ATTR(run, 0200, NULL, pwm_run_store);
+
+
+static ssize_t pwm_duty_ns_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct pwm_channel *p = dev_get_drvdata(dev);
+	return sprintf(buf, "%lu\n", pwm_ticks_to_ns(p, p->duty_ticks));
+}
+
+static ssize_t pwm_duty_ns_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf,
+				 size_t len)
+{
+	unsigned long duty_ns;
+	struct pwm_channel *p = dev_get_drvdata(dev);
+
+	if (1 == sscanf(buf, "%lu", &duty_ns))
+		pwm_duty_ns(p, duty_ns);
+	return len;
+}
+static DEVICE_ATTR(duty_ns, 0644, pwm_duty_ns_show, pwm_duty_ns_store);
+
+
+static ssize_t pwm_period_ns_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct pwm_channel *p = dev_get_drvdata(dev);
+	return sprintf(buf, "%lu\n", pwm_ticks_to_ns(p, p->period_ticks));
+}
+
+
+static ssize_t pwm_period_ns_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf,
+				   size_t len)
+{
+	unsigned long period_ns;
+	struct pwm_channel *p = dev_get_drvdata(dev);
+
+	if (1 == sscanf(buf, "%lu", &period_ns))
+		pwm_period_ns(p, period_ns);
+	return len;
+}
+static DEVICE_ATTR(period_ns, 0644, pwm_period_ns_show, pwm_period_ns_store);
+
+
+static ssize_t pwm_polarity_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct pwm_channel *p = dev_get_drvdata(dev);
+	return sprintf(buf, "%d\n", p->active_low ? 0 : 1);
+}
+
+
+static ssize_t pwm_polarity_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t len)
+{
+	int polarity;
+	struct pwm_channel *p = dev_get_drvdata(dev);
+
+	if (1 == sscanf(buf, "%d", &polarity))
+		pwm_polarity(p, polarity);
+	return len;
+}
+static DEVICE_ATTR(polarity, 0644, pwm_polarity_show, pwm_polarity_store);
+
+
+static ssize_t pwm_request_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct pwm_channel *p = dev_get_drvdata(dev);
+	mutex_lock(&device_list_mutex);
+	__pwm_request_channel(p, "sysfs");
+	mutex_unlock(&device_list_mutex);
+
+	return sprintf(buf, "%s\n", p->requester);
+}
+
+
+static ssize_t pwm_request_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf,
+				 size_t len)
+{
+	struct pwm_channel *p = dev_get_drvdata(dev);
+	pwm_free(p);
+	return len;
+}
+static DEVICE_ATTR(request, 0644, pwm_request_show, pwm_request_store);
+
+
+static const struct attribute *pwm_attrs[] =
+{
+	&dev_attr_run.attr,
+	&dev_attr_polarity.attr,
+	&dev_attr_duty_ns.attr,
+	&dev_attr_period_ns.attr,
+	&dev_attr_request.attr,
+	NULL,
+};
+
+
+static const struct attribute_group pwm_device_attr_group = {
+	.attrs = (struct attribute **)pwm_attrs,
+};
+
+
+static int __pwm_create_sysfs(struct pwm_device *pwm)
+{
+	int ret = 0;
+	struct device *dev;
+	int wchan;
+
+	for (wchan = 0; wchan < pwm->nchan; wchan++) {
+		dev = device_create(&pwm_class, pwm->dev, MKDEV(0, 0),
+				    pwm->channels + wchan,
+				    "%s:%d", pwm->bus_id, wchan);
+		if (!dev)
+			goto err_dev_create;
+		ret = sysfs_create_group(&dev->kobj, &pwm_device_attr_group);
+		if (ret)
+			goto err_dev_group_create;
+	}
+
+	return ret;
+
+err_dev_group_create:
+err_dev_create:
+	/* TODO: undo all the successful device_create calls */
+	return -ENODEV;
+}
+
+
+static struct class_attribute pwm_class_attrs[] = {
+	__ATTR_NULL,
+};
+
+static struct class pwm_class = {
+	.name = "pwm",
+	.owner = THIS_MODULE,
+
+	.class_attrs = pwm_class_attrs,
+};
+
+
+static int __init pwm_init(void)
+{
+	int ret;
+
+	/* TODO: how to deal with devices that register very early? */
+
+	ret = class_register(&pwm_class);
+	if (ret < 0)
+		return ret;
+
+	pwm_handler_workqueue = create_workqueue("pwmd");
+
+	return 0;
+}
+postcore_initcall(pwm_init);
-- 
1.5.6.5

[RFC 2/6] [PWM] Changes to existing include/linux/pwm.h to adapt to generic PWM API

[Bill Gatliff]

Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
---
 include/linux/pwm.h |  168 ++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 147 insertions(+), 21 deletions(-)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 3945f80..d3d18f7 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -1,31 +1,157 @@
 #ifndef __LINUX_PWM_H
 #define __LINUX_PWM_H
 
-struct pwm_device;
-
 /*
- * pwm_request - request a PWM device
+ * include/linux/pwm.h
+ *
+ * Copyright (C) 2008 Bill Gatliff
+ *
+ * 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 of the License, 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; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
-struct pwm_device *pwm_request(int pwm_id, const char *label);
 
-/*
- * pwm_free - free a PWM device
- */
-void pwm_free(struct pwm_device *pwm);
+enum {
+	PWM_CONFIG_DUTY_TICKS = BIT(0),
+	PWM_CONFIG_PERIOD_TICKS = BIT(1),
+	PWM_CONFIG_POLARITY = BIT(2),
+	PWM_CONFIG_START = BIT(3),
+	PWM_CONFIG_STOP = BIT(4),
 
-/*
- * pwm_config - change a PWM device configuration
- */
-int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
+	PWM_CONFIG_HANDLER = BIT(5),
 
-/*
- * pwm_enable - start a PWM output toggling
- */
-int pwm_enable(struct pwm_device *pwm);
+	PWM_CONFIG_DUTY_NS = BIT(6),
+	PWM_CONFIG_DUTY_PERCENT = BIT(7),
+	PWM_CONFIG_PERIOD_NS = BIT(8),
+};
+
+struct pwm_channel;
+struct work_struct;
+
+typedef int (*pwm_handler_t)(struct pwm_channel *p, void *data);
+typedef void (*pwm_callback_t)(struct pwm_channel *p);
+
+struct pwm_channel_config {
+	int config_mask;
+	unsigned long duty_ticks;
+	unsigned long period_ticks;
+	int polarity;
+
+	pwm_handler_t handler;
+
+	unsigned long duty_ns;
+	unsigned long period_ns;
+	int duty_percent;
+};
+
+struct pwm_device {
+	struct list_head list;
+	spinlock_t list_lock;
+	struct device *dev;
+	struct module *owner;
+	struct pwm_channel *channels;
+
+	const char *bus_id;
+	int nchan;
+
+	int	(*request)	(struct pwm_channel *p);
+	void	(*free)		(struct pwm_channel *p);
+	int	(*config)	(struct pwm_channel *p,
+				 struct pwm_channel_config *c);
+	int	(*config_nosleep)(struct pwm_channel *p,
+				  struct pwm_channel_config *c);
+	int	(*synchronize)	(struct pwm_channel *p,
+				 struct pwm_channel *to_p);
+	int	(*unsynchronize)(struct pwm_channel *p,
+				 struct pwm_channel *from_p);
+	int	(*set_callback)	(struct pwm_channel *p,
+				 pwm_callback_t callback);
+};
+
+int pwm_register(struct pwm_device *pwm);
+int pwm_unregister(struct pwm_device *pwm);
+
+enum {
+	FLAG_REQUESTED = 0,
+	FLAG_STOP = 1,
+};
+
+struct pwm_channel {
+	struct list_head list;
+	struct pwm_device *pwm;
+	const char *requester;
+	int chan;
+	unsigned long flags;
+	unsigned long tick_hz;
+
+	spinlock_t lock;
+	struct completion complete;
+
+	pwm_callback_t callback;
+
+	struct work_struct handler_work;
+	pwm_handler_t handler;
+	void *handler_data;
+
+	int active_low;
+	unsigned long period_ticks;
+	unsigned long duty_ticks;
+};
+
+struct pwm_channel *
+pwm_request(const char *bus_id, int chan,
+	    const char *requester);
+
+void pwm_free(struct pwm_channel *pwm);
+
+int pwm_config_nosleep(struct pwm_channel *pwm,
+		       struct pwm_channel_config *c);
+
+int pwm_config(struct pwm_channel *pwm,
+	       struct pwm_channel_config *c);
+
+unsigned long pwm_ns_to_ticks(struct pwm_channel *pwm,
+			      unsigned long nsecs);
+
+unsigned long pwm_ticks_to_ns(struct pwm_channel *pwm,
+			      unsigned long ticks);
+
+int pwm_period_ns(struct pwm_channel *pwm,
+		  unsigned long period_ns);
+
+int pwm_duty_ns(struct pwm_channel *pwm,
+		unsigned long duty_ns);
+
+int pwm_duty_percent(struct pwm_channel *pwm,
+		     int percent);
+
+int pwm_polarity(struct pwm_channel *pwm,
+		 int active_high);
+
+int pwm_start(struct pwm_channel *pwm);
+
+int pwm_stop(struct pwm_channel *pwm);
+
+int pwm_set_handler(struct pwm_channel *pwm,
+		    pwm_handler_t handler,
+		    void *data);
+
+int pwm_synchronize(struct pwm_channel *p,
+		    struct pwm_channel *to_p);
+
+
+int pwm_unsynchronize(struct pwm_channel *p,
+		      struct pwm_channel *from_p);
 
-/*
- * pwm_disable - stop a PWM output toggling
- */
-void pwm_disable(struct pwm_device *pwm);
 
-#endif /* __ASM_ARCH_PWM_H */
+#endif /* __LINUX_PWM_H */
-- 
1.5.6.5

[RFC 3/6] [PWM] Documentation

[Bill Gatliff]

Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
---
 Documentation/pwm.txt |  258 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 258 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/pwm.txt

diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
new file mode 100644
index 0000000..b8932dd
--- /dev/null
+++ b/Documentation/pwm.txt
@@ -0,0 +1,258 @@
+                       Generic PWM Device API
+
+                          October 8, 2008
+                           Bill Gatliff
+                       <bgat@billgatliff.com>
+
+
+
+The code in drivers/pwm and include/linux/pwm.h implements an API for
+applications involving pulse-width-modulation signals.  This document
+describes how the API implementation facilitates both PWM-generating
+devices, and users of those devices.
+
+
+
+Motivation
+
+The primary goals for implementing the "generic PWM API" are to
+consolidate the various PWM implementations within a consistent and
+redundancy-reducing framework, and to facilitate the use of
+hotpluggable PWM devices.
+
+Previous PWM-related implementations within the Linux kernel achieved
+their consistency via cut-and-paste, but did not need to (and didn't)
+facilitate more than one PWM-generating device within the system---
+hotplug or otherwise.  The Generic PWM Device API might be most
+appropriately viewed as an update to those implementations, rather
+than a complete rewrite.
+
+
+
+Challenges
+
+One of the difficulties in implementing a generic PWM framework is the
+fact that pulse-width-modulation applications involve real-world
+signals, which often must be carefully managed to prevent destruction
+of hardware that is linked to those signals.  A DC motor that
+experiences a brief interruption in the PWM signal controlling it
+might destructively overheat; it could change speed, losing
+synchronization with a sensor; it could even suddenly change direction
+or torque, breaking the mechanical device connected to it.
+
+(A generic PWM device framework is not directly responsible for
+preventing the above scenarios: that responsibility lies with the
+hardware designer and the application and driver authors.  But it must
+to the greatest extent possible make it easy to avoid such problems).
+
+A generic PWM device framework must accomodate the substantial
+differences between available PWM-generating hardware devices, without
+becoming sub-optimal for any of them.
+
+Finally, a generic PWM device framework must be relatively
+lightweight, computationally speaking.  Some PWM users demand
+high-speed outputs, plus the ability to regulate those outputs
+quickly.  A device framework must be able to "keep up" with such
+hardware, while still leaving time to do real work.
+
+The Generic PWM Device API is an attempt to meet all of the above
+requirements.  At its initial publication, the API was already in use
+managing small DC motors through a custom-designed, optically-isolated
+H-bridge driver.
+
+
+
+Functional Overview
+
+The Generic PWM Device API framework is implemented in
+include/linux/pwm.h and drivers/pwm/pwm.c.  The functions therein use
+information from pwm_device, pwm_channel and pwm_channel_config
+structures to invoke services in PWM peripheral device drivers.
+Consult drivers/pwm/atmel-pwm.c for an example driver.
+
+There are two classes of adopters of the PWM framework:
+
+  "Users" -- those wishing to employ the API merely to produce PWM
+  signals; once they have identified the appropriate physical output
+  on the platform in question, they don't care about the details of
+  the underlying hardware
+
+  "Driver authors" -- those wishing to bind devices that can generate
+  PWM signals to the Generic PWM Device API, so that the services of
+  those devices become available to users; assuming the hardware can
+  support the needs of a user, driver authors don't care about the
+  details of the user's application
+
+Generally speaking, users will first invoke pwm_request() to obtain a
+handle to a PWM device.  They will then pass that handle to functions
+like pwm_duty_ns() and pwm_period_ns() to set the duty cycle and
+period of the PWM signal, respectively.  They will also invoke
+pwm_start() and pwm_stop() to turn the signal on and off.
+
+The framework also provides a sysfs interface to PWM devices, which is
+adequate for basic needs and testing.
+
+Driver authors fill out a pwm_device structure, which describes the
+capabilities of the PWM hardware being driven--- including the number
+of distinct output "channels" the peripheral offers.  They then invoke
+pwm_register() (usually from within their device's probe() handler) to
+make the PWM API aware of their device.  The framework will call back
+to the methods described in the pwm_device structure to configure and
+use the hardware.
+
+Note that PWM signals can be produced by a variety of peripherals,
+beyond the true "PWM hardware" offered by many system-on-chip devices.
+Other possibilities include timer/counters with compare-match
+capabilities, carefully-programmed synchronous serial ports
+(e.g. SPI), and GPIO pins driven by kernel interval timers.  With a
+proper pwm_device structure, these devices and pseudo-devices can all
+be accomodated by the Generic PWM Device API framework.
+
+
+
+Using the API to Generate PWM Signals -- Basic Functions for Users
+
+
+pwm_request() -- Returns a pwm_channel pointer, which is subsequently
+passed to the other user-related PWM functions.  Once requested, a PWM
+channel is marked as in-use and subsequent requests prior to
+pwm_free() will fail.
+
+The names used to refer to PWM devices are defined by driver authors.
+Typically they are platform device bus identifiers, and this
+convention is encouraged for consistency.
+
+
+pwm_free() -- Marks a PWM channel as no longer in use.  The PWM device
+is stopped before it is released by the API.
+
+
+pwm_period_ns() -- Specifies the PWM signal's period, in nanoseconds.
+
+
+pwm_duty_ns() -- Specifies the PWM signal's active duration, in nanoseconds.
+
+
+pwm_duty_percent() -- Specifies the PWM signal's active duration, as a
+percentage of the current period of the signal.  NOTE: this value is
+not recalculated if the period of the signal is subsequently changed.
+
+
+pwm_start(), pwm_stop() -- Turns the PWM signal on and off.  Except
+where stated otherwise by a driver author, signals are stopped at the
+end of the current period, at which time the output is set to its
+inactive state.
+
+
+pwm_polarity() -- Defines whether the PWM signal output's active
+region is "1" or "0".  A 10% duty-cycle, polarity=1 signal will
+conventionally be at 5V (or 3.3V, or 1000V, or whatever the platform
+hardware does) for 10% of the period.  The same configuration of a
+polarity=0 signal will be at 5V (or 3.3V, or ...) for 90% of the
+period.
+
+
+
+Using the API to Generate PWM Signals -- Advanced Functions
+
+
+pwm_config() -- Passes a pwm_channel_config structure to the
+associated device driver.  This function is invoked by pwm_start(),
+pwm_duty_ns(), etc. and is one of two main entry points to the PWM
+driver for the hardware being used.  The configuration change is
+guaranteed atomic if multiple configuration changes are specified.
+This function might sleep, depending on what the device driver has to
+do to satisfy the request.  All PWM device drivers must support this
+entry point.
+
+
+pwm_config_nosleep() -- Passes a pwm_channel_config structure to the
+associated device driver.  If the driver must sleep in order to
+implement the requested configuration change, -EWOULDBLOCK is
+returned.  Users may call this function from interrupt handlers, for
+example.  This is the other main entry point into the PWM hardware
+driver, but not all device drivers support this entry point.
+
+
+pwm_synchronize(), pwm_unsynchronize() -- "Synchronizes" two or more
+PWM channels, if the underlying hardware permits.  (If it doesn't, the
+framework facilitates emulating this capability but it is not yet
+implemented).  Synchronized channels will start and stop
+simultaneously when any single channel in the group is started or
+stopped.  Use pwm_unsynchronize(..., NULL) to completely detach a
+channel from any other synchronized channels.
+
+
+pwm_set_handler() -- Defines an end-of-period callback.  The indicated
+function will be invoked in a worker thread at the end of each PWM
+period, and can subsequently invoke pwm_config(), etc.  Must be used
+with extreme care for high-speed PWM outputs.  Set the handler
+function to NULL to un-set the handler.
+
+
+
+Implementing a PWM Device API Driver -- Functions for Driver Authors
+
+
+Fill out the appropriate fields in a pwm_device structure, and submit
+to pwm_register():
+
+
+bus_id -- the plaintext name of the device.  Users will bind to a
+channel on the device using this name plus the channel number.  For
+example, the Atmel PWMC's bus_id is "atmel_pwmc", the same as used by
+the platform device driver (recommended).  The first device registered
+thereby receives bus_id "atmel_pwmc.0", which is what you put in
+pwm_device.bus_id.  Channels are then named "atmel_pwmc.0:[0-3]".
+(Hint: just use pdev->dev.bus_id in your probe() method).
+
+
+nchan -- the number of distinct output channels provided by the device.
+
+
+request -- (optional) Invoked each time a user requests a channel.
+Use to turn on clocks, clean up register states, etc.  The framework
+takes care of device locking/unlocking; you will see only successful
+requests.
+
+
+free -- (optional) Callback for each time a user relinquishes a
+channel.  The framework will have already stopped, unsynchronized and
+un-handled the channel.  Use to turn off clocks, etc. as necessary.
+
+
+synchronize, unsynchronize -- (optional) Callbacks to
+synchronize/unsynchronize channels.  Some devices provide this
+capability in hardware; for others, it can be emulated (see
+atmel_pwmc.c's sync_mask for an example).
+
+
+set_callback -- (optional) Invoked when a user requests a handler.  If
+the hardware supports an end-of-period interrupt, invoke the function
+indicated during your interrupt handler.  The callback function itself
+is always internal to the API, and does not map directly to the user's
+callback function.
+
+
+config -- Invoked to change the device configuration, always from a
+sleep-capable context.  All the changes indicated must be performed
+atomically, ideally synchronized to an end-of-period event (so that
+you avoid short or long output pulses).  You may sleep, etc. as
+necessary within this function.
+
+
+config_nosleep -- (optional) Invoked to change device configuration
+from within a context that is not allowed to sleep.  If you cannot
+perform the requested configuration changes without sleeping, return
+-EWOULDBLOCK.
+
+
+
+Acknowledgements
+
+
+The author expresses his gratitude to the countless developers who
+have reviewed and submitted feedback on the various versions of the
+Generic PWM Device API code, and those who have submitted drivers and
+applications that use the framework.  You know who you are.  ;)
+
-- 
1.5.6.5

[RFC 4/6] [PWM] Driver for Atmel PWMC peripheral

[Bill Gatliff]

Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
---
 drivers/pwm/atmel-pwm.c |  631 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 631 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/atmel-pwm.c

diff --git a/drivers/pwm/atmel-pwm.c b/drivers/pwm/atmel-pwm.c
new file mode 100644
index 0000000..b65e84f
--- /dev/null
+++ b/drivers/pwm/atmel-pwm.c
@@ -0,0 +1,631 @@
+/*
+ * drivers/pwm/atmel-pwm.c
+ *
+ * Copyright (C) 2008 Bill Gatliff
+ * Copyright (C) 2007 David Brownell
+ *
+ * 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 of the License, 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; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+
+enum {
+	/* registers common to the PWMC peripheral */
+	PWMC_MR = 0,
+	PWMC_ENA = 4,
+	PWMC_DIS = 8,
+	PWMC_SR = 0xc,
+	PWMC_IER = 0x10,
+	PWMC_IDR = 0x14,
+	PWMC_IMR = 0x18,
+	PWMC_ISR = 0x1c,
+
+	/* registers per each PWMC channel */
+	PWMC_CMR = 0,
+	PWMC_CDTY = 4,
+	PWMC_CPRD = 8,
+	PWMC_CCNT = 0xc,
+	PWMC_CUPD = 0x10,
+
+	/* how to find each channel */
+	PWMC_CHAN_BASE = 0x200,
+	PWMC_CHAN_STRIDE = 0x20,
+
+	/* CMR bits of interest */
+	PWMC_CMR_CPD = 10,
+	PWMC_CMR_CPOL = 9,
+	PWMC_CMR_CALG = 8,
+	PWMC_CMR_CPRE_MASK = 0xf,
+};
+
+struct atmel_pwm {
+	struct pwm_device pwm;
+	spinlock_t lock;
+	void __iomem *iobase;
+	struct clk *clk;
+	u32 *sync_mask;
+	int irq;
+	u32 ccnt_mask;
+};
+
+
+static inline void
+pwmc_writel(const struct atmel_pwm *p,
+	    unsigned offset, u32 val)
+{
+	__raw_writel(val, p->iobase + offset);
+}
+
+
+static inline u32
+pwmc_readl(const struct atmel_pwm *p,
+	   unsigned offset)
+{
+	return __raw_readl(p->iobase + offset);
+}
+
+
+static inline void
+pwmc_chan_writel(const struct pwm_channel *p,
+		 u32 offset, u32 val)
+{
+	const struct atmel_pwm *ap
+		= container_of(p->pwm, struct atmel_pwm, pwm);
+
+	if (PWMC_CMR == offset)
+		val &= ((1 << PWMC_CMR_CPD)
+			| (1 << PWMC_CMR_CPOL)
+			| (1 << PWMC_CMR_CALG)
+			| (PWMC_CMR_CPRE_MASK));
+	else
+		val &= ap->ccnt_mask;
+
+	pwmc_writel(ap, offset + PWMC_CHAN_BASE
+		    + (p->chan * PWMC_CHAN_STRIDE), val);
+}
+
+
+static inline u32
+pwmc_chan_readl(const struct pwm_channel *p,
+		u32 offset)
+{
+	const struct atmel_pwm *ap
+		= container_of(p->pwm, struct atmel_pwm, pwm);
+
+	return pwmc_readl(ap, offset + PWMC_CHAN_BASE
+			  + (p->chan * PWMC_CHAN_STRIDE));
+}
+
+
+static inline int
+__atmel_pwm_is_on(struct pwm_channel *p)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	return (pwmc_readl(ap, PWMC_SR) & (1 << p->chan)) ? 1 : 0;
+}
+
+
+static inline void
+__atmel_pwm_unsynchronize(struct pwm_channel *p,
+			  struct pwm_channel *to_p)
+{
+	const struct atmel_pwm *ap
+		= container_of(p->pwm, struct atmel_pwm, pwm);
+	int wchan;
+
+	if (to_p) {
+		ap->sync_mask[p->chan] &= ~(1 << to_p->chan);
+		ap->sync_mask[to_p->chan] &= ~(1 << p->chan);
+		goto done;
+	}
+
+	ap->sync_mask[p->chan] = 0;
+	for (wchan = 0; wchan < ap->pwm.nchan; wchan++)
+		ap->sync_mask[wchan] &= ~(1 << p->chan);
+done:
+	pr_debug("%s:%d sync_mask %x\n",
+		 p->pwm->bus_id, p->chan, ap->sync_mask[p->chan]);
+}
+
+
+static inline void
+__atmel_pwm_synchronize(struct pwm_channel *p,
+			struct pwm_channel *to_p)
+{
+	const struct atmel_pwm *ap
+		= container_of(p->pwm, struct atmel_pwm, pwm);
+
+	if (!to_p)
+		return;
+
+	ap->sync_mask[p->chan] |= (1 << to_p->chan);
+	ap->sync_mask[to_p->chan] |= (1 << p->chan);
+
+	pr_debug("%s:%d sync_mask %x\n",
+		 p->pwm->bus_id, p->chan, ap->sync_mask[p->chan]);
+}
+
+
+static inline void
+__atmel_pwm_stop(struct pwm_channel *p)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	u32 chid = 1 << p->chan;
+
+	pwmc_writel(ap, PWMC_DIS, ap->sync_mask[p->chan] | chid);
+}
+
+
+static inline void
+__atmel_pwm_start(struct pwm_channel *p)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	u32 chid = 1 << p->chan;
+
+	pwmc_writel(ap, PWMC_ENA, ap->sync_mask[p->chan] | chid);
+}
+
+
+static int
+atmel_pwm_synchronize(struct pwm_channel *p,
+		      struct pwm_channel *to_p)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&p->lock, flags);
+	__atmel_pwm_synchronize(p, to_p);
+	spin_unlock_irqrestore(&p->lock, flags);
+	return 0;
+}
+
+
+static int
+atmel_pwm_unsynchronize(struct pwm_channel *p,
+			struct pwm_channel *from_p)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&p->lock, flags);
+	__atmel_pwm_unsynchronize(p, from_p);
+	spin_unlock_irqrestore(&p->lock, flags);
+	return 0;
+}
+
+
+static inline int
+__atmel_pwm_config_polarity(struct pwm_channel *p,
+			    struct pwm_channel_config *c)
+{
+	u32 cmr = pwmc_chan_readl(p, PWMC_CMR);
+
+	if (c->polarity)
+		cmr &= ~BIT(PWMC_CMR_CPOL);
+	else
+		cmr |= BIT(PWMC_CMR_CPOL);
+	pwmc_chan_writel(p, PWMC_CMR, cmr);
+
+	pr_debug("%s:%d polarity %d\n", p->pwm->bus_id,
+		 p->chan, c->polarity);
+	return 0;
+}
+
+
+static inline int
+__atmel_pwm_config_duty_ticks(struct pwm_channel *p,
+			      struct pwm_channel_config *c)
+{
+	u32 cmr, cprd, cpre, cdty;
+
+	cmr = pwmc_chan_readl(p, PWMC_CMR);
+	cprd = pwmc_chan_readl(p, PWMC_CPRD);
+
+	cpre = cmr & PWMC_CMR_CPRE_MASK;
+	cmr &= ~BIT(PWMC_CMR_CPD);
+
+	cdty = cprd - (c->duty_ticks >> cpre);
+
+	p->duty_ticks = c->duty_ticks;
+
+	if (__atmel_pwm_is_on(p)) {
+		pwmc_chan_writel(p, PWMC_CMR, cmr);
+		pwmc_chan_writel(p, PWMC_CUPD, cdty);
+	} else
+		pwmc_chan_writel(p, PWMC_CDTY, cdty);
+
+	pr_debug("%s:%d duty_ticks = %lu cprd = %x"
+		 " cdty = %x cpre = %x\n",
+		 p->pwm->bus_id, p->chan, p->duty_ticks,
+		 cprd, cdty, cpre);
+
+	return 0;
+}
+
+
+static inline int
+__atmel_pwm_config_period_ticks(struct pwm_channel *p,
+				struct pwm_channel_config *c)
+{
+	u32 cmr, cprd, cpre;
+
+	cpre = fls(c->period_ticks);
+	if (cpre < 16)
+		cpre = 0;
+	else {
+		cpre -= 15;
+		if (cpre > 10)
+			return -EINVAL;
+	}
+
+	cmr = pwmc_chan_readl(p, PWMC_CMR);
+	cmr &= ~PWMC_CMR_CPRE_MASK;
+	cmr |= cpre;
+
+	cprd = c->period_ticks >> cpre;
+
+	pwmc_chan_writel(p, PWMC_CMR, cmr);
+	pwmc_chan_writel(p, PWMC_CPRD, cprd);
+	p->period_ticks = c->period_ticks;
+
+	pr_debug("%s:%d period_ticks = %lu cprd = %x cpre = %x\n",
+		 p->pwm->bus_id, p->chan, p->period_ticks, cprd, cpre);
+
+	return 0;
+}
+
+
+
+static int
+atmel_pwm_config_nosleep(struct pwm_channel *p,
+			 struct pwm_channel_config *c)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&p->lock, flags);
+
+	switch (c->config_mask) {
+
+	case PWM_CONFIG_DUTY_TICKS:
+		__atmel_pwm_config_duty_ticks(p, c);
+		break;
+
+	case PWM_CONFIG_STOP:
+		__atmel_pwm_stop(p);
+		pr_debug("%s:%d stop\n", p->pwm->bus_id, p->chan);
+		break;
+
+	case PWM_CONFIG_START:
+		__atmel_pwm_start(p);
+		pr_debug("%s:%d start\n", p->pwm->bus_id, p->chan);
+		break;
+
+	case PWM_CONFIG_POLARITY:
+		__atmel_pwm_config_polarity(p, c);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	spin_unlock_irqrestore(&p->lock, flags);
+	return ret;
+}
+
+
+static int
+atmel_pwm_stop_sync(struct pwm_channel *p)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	int ret;
+	int was_on = __atmel_pwm_is_on(p);
+
+	if (was_on) {
+		do {
+			init_completion(&p->complete);
+			set_bit(FLAG_STOP, &p->flags);
+			pwmc_writel(ap, PWMC_IER, 1 << p->chan);
+
+			pr_debug("%s:%d waiting on stop_sync completion...\n",
+				 p->pwm->bus_id, p->chan);
+
+			ret = wait_for_completion_interruptible(&p->complete);
+
+			pr_debug("%s:%d stop_sync complete (%d)\n",
+				 p->pwm->bus_id, p->chan, ret);
+
+			if (ret)
+				return ret;
+		} while (p->flags & BIT(FLAG_STOP));
+	}
+
+	pr_debug("%s:%d stop_sync returning %d\n",
+		 p->pwm->bus_id, p->chan, was_on);
+
+	return was_on;
+}
+
+
+static int
+atmel_pwm_config(struct pwm_channel *p,
+		 struct pwm_channel_config *c)
+{
+	int was_on = 0;
+
+	might_sleep();
+
+	if (p->pwm->config_nosleep) {
+		if (!p->pwm->config_nosleep(p, c))
+			return 0;
+	}
+
+	pr_debug("%s:%d config_mask %x\n",
+		 p->pwm->bus_id, p->chan, c->config_mask);
+
+	was_on = atmel_pwm_stop_sync(p);
+	if (was_on < 0)
+		return was_on;
+
+	if (c->config_mask & PWM_CONFIG_PERIOD_TICKS) {
+		__atmel_pwm_config_period_ticks(p, c);
+		if (!(c->config_mask & PWM_CONFIG_DUTY_TICKS)) {
+			struct pwm_channel_config d = {
+				.config_mask = PWM_CONFIG_DUTY_TICKS,
+				.duty_ticks = p->duty_ticks,
+			};
+			__atmel_pwm_config_duty_ticks(p, &d);
+		}
+	}
+
+	if (c->config_mask & PWM_CONFIG_DUTY_TICKS)
+		__atmel_pwm_config_duty_ticks(p, c);
+
+	if (c->config_mask & PWM_CONFIG_POLARITY)
+		__atmel_pwm_config_polarity(p, c);
+
+	if ((c->config_mask & PWM_CONFIG_START)
+	    || (was_on && !(c->config_mask & PWM_CONFIG_STOP)))
+		__atmel_pwm_start(p);
+
+	return 0;
+}
+
+
+static void
+__atmel_pwm_set_callback(struct pwm_channel *p,
+			 pwm_callback_t callback)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+
+	p->callback = callback;
+	pwmc_writel(ap, p->callback ? PWMC_IER : PWMC_IDR, 1 << p->chan);
+	pr_debug("%s:%d set_callback %p\n", p->pwm->bus_id, p->chan, callback);
+}
+
+
+static int
+atmel_pwm_set_callback(struct pwm_channel *p,
+		       pwm_callback_t callback)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ap->lock, flags);
+	__atmel_pwm_set_callback(p, callback);
+	spin_unlock_irqrestore(&ap->lock, flags);
+
+	return 0;
+}
+
+
+static int
+atmel_pwm_request(struct pwm_channel *p)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	unsigned long flags;
+
+	spin_lock_irqsave(&p->lock, flags);
+	clk_enable(ap->clk);
+	p->tick_hz = clk_get_rate(ap->clk);
+	__atmel_pwm_unsynchronize(p, NULL);
+	__atmel_pwm_stop(p);
+	spin_unlock_irqrestore(&p->lock, flags);
+
+	return 0;
+}
+
+
+static void
+atmel_pwm_free(struct pwm_channel *p)
+{
+	struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
+	clk_disable(ap->clk);
+}
+
+
+static irqreturn_t
+atmel_pwmc_irq(int irq, void *data)
+{
+	struct atmel_pwm *ap = data;
+	struct pwm_channel *p;
+	u32 isr;
+	int chid;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ap->lock, flags);
+
+	isr = pwmc_readl(ap, PWMC_ISR);
+	for (chid = 0; isr; chid++, isr >>= 1) {
+		p = &ap->pwm.channels[chid];
+		if (isr & 1) {
+			if (p->callback)
+				p->callback(p);
+			if (p->flags & BIT(FLAG_STOP)) {
+				__atmel_pwm_stop(p);
+				clear_bit(FLAG_STOP, &p->flags);
+			}
+			complete_all(&p->complete);
+		}
+	}
+
+	spin_unlock_irqrestore(&ap->lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+
+static int __init
+atmel_pwmc_probe(struct platform_device *pdev)
+{
+	struct atmel_pwm *ap;
+	struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	int ret = 0;
+
+	ap = kzalloc(sizeof(*ap), GFP_KERNEL);
+	if (!ap) {
+		ret = -ENOMEM;
+		goto err_atmel_pwm_alloc;
+	}
+
+	spin_lock_init(&ap->lock);
+	platform_set_drvdata(pdev, ap);
+
+	ap->pwm.bus_id = pdev->dev.bus_id;
+
+	ap->pwm.nchan = 4; /* TODO: true only for SAM9263 and AP7000 */
+	ap->ccnt_mask = 0xffffUL; /* TODO: true only for SAM9263 */
+
+	ap->sync_mask = kzalloc(ap->pwm.nchan * sizeof(u32), GFP_KERNEL);
+	if (!ap->sync_mask) {
+		ret = -ENOMEM;
+		goto err_alloc_sync_masks;
+	}
+
+	ap->pwm.owner = THIS_MODULE;
+	ap->pwm.request = atmel_pwm_request;
+	ap->pwm.free = atmel_pwm_free;
+	ap->pwm.config_nosleep = atmel_pwm_config_nosleep;
+	ap->pwm.config = atmel_pwm_config;
+	ap->pwm.synchronize = atmel_pwm_synchronize;
+	ap->pwm.unsynchronize = atmel_pwm_unsynchronize;
+	ap->pwm.set_callback = atmel_pwm_set_callback;
+
+	ap->clk = clk_get(&pdev->dev, "pwmc_clk");
+	if (!ap->clk) {
+		ret = -ENODEV;
+		goto err_clk_get;
+	}
+
+	ap->iobase = ioremap_nocache(r->start, r->end - r->start + 1);
+	if (!ap->iobase) {
+		ret = -ENODEV;
+		goto err_ioremap;
+	}
+
+	clk_enable(ap->clk);
+	pwmc_writel(ap, PWMC_DIS, -1);
+	pwmc_writel(ap, PWMC_IDR, -1);
+	clk_disable(ap->clk);
+
+	ap->irq = platform_get_irq(pdev, 0);
+	if (ap->irq != -ENXIO) {
+		ret = request_irq(ap->irq, atmel_pwmc_irq, 0,
+				  ap->pwm.bus_id, ap);
+		if (ret)
+			goto err_request_irq;
+	}
+
+	ret = pwm_register(&ap->pwm);
+	if (ret)
+		goto err_pwm_register;
+
+	return 0;
+
+err_pwm_register:
+	if (ap->irq != -ENXIO)
+		free_irq(ap->irq, ap);
+err_request_irq:
+	iounmap(ap->iobase);
+err_ioremap:
+	clk_put(ap->clk);
+err_clk_get:
+	platform_set_drvdata(pdev, NULL);
+err_alloc_sync_masks:
+	kfree(ap);
+err_atmel_pwm_alloc:
+	return ret;
+}
+
+
+static int __devexit
+atmel_pwmc_remove(struct platform_device *pdev)
+{
+	struct atmel_pwm *ap = platform_get_drvdata(pdev);
+	int ret;
+
+	/* TODO: what can we do if this fails? */
+	ret = pwm_unregister(&ap->pwm);
+
+	clk_enable(ap->clk);
+	pwmc_writel(ap, PWMC_IDR, -1);
+	pwmc_writel(ap, PWMC_DIS, -1);
+	clk_disable(ap->clk);
+
+	if (ap->irq != -ENXIO)
+		free_irq(ap->irq, ap);
+
+	clk_put(ap->clk);
+	iounmap(ap->iobase);
+
+	kfree(ap);
+
+	return 0;
+}
+
+
+static struct platform_driver atmel_pwm_driver = {
+	.driver = {
+		.name = "atmel_pwmc",
+		.owner = THIS_MODULE,
+	},
+	.probe = atmel_pwmc_probe,
+	.remove = __devexit_p(atmel_pwmc_remove),
+};
+
+
+static int __init atmel_pwm_init(void)
+{
+	return platform_driver_register(&atmel_pwm_driver);
+}
+module_init(atmel_pwm_init);
+
+
+static void atmel_pwm_exit(void)
+{
+	platform_driver_unregister(&atmel_pwm_driver);
+}
+module_exit(atmel_pwm_exit);
+
+
+MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
+MODULE_DESCRIPTION("Driver for Atmel PWMC peripheral");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atmel_pwmc");
-- 
1.5.6.5

[RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Bill Gatliff]

Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
---
 arch/arm/Kconfig         |    2 +
 drivers/Makefile         |    2 +
 drivers/misc/Kconfig     |    9 -
 drivers/misc/Makefile    |    1 -
 drivers/misc/atmel_pwm.c |  409 ----------------------------------------------
 drivers/pwm/Kconfig      |   24 +++
 drivers/pwm/Makefile     |    6 +
 7 files changed, 34 insertions(+), 419 deletions(-)
 delete mode 100644 drivers/misc/atmel_pwm.c
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 70dba16..fed3eef 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1196,6 +1196,8 @@ source "drivers/spi/Kconfig"
 
 source "drivers/gpio/Kconfig"
 
+source "drivers/pwm/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/power/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 2735bde..f242fc6 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -6,6 +6,8 @@
 #
 
 obj-y				+= gpio/
+obj-$(CONFIG_GENERIC_PWM)	+= pwm/
+
 obj-$(CONFIG_PCI)		+= pci/
 obj-$(CONFIG_PARISC)		+= parisc/
 obj-$(CONFIG_RAPIDIO)		+= rapidio/
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a726f3b..cdea0bb 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -13,15 +13,6 @@ menuconfig MISC_DEVICES
 
 if MISC_DEVICES
 
-config ATMEL_PWM
-	tristate "Atmel AT32/AT91 PWM support"
-	depends on AVR32 || ARCH_AT91
-	help
-	  This option enables device driver support for the PWM channels
-	  on certain Atmel prcoessors.  Pulse Width Modulation is used for
-	  purposes including software controlled power-efficent backlights
-	  on LCD displays, motor control, and waveform generation.
-
 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 c6c13f6..9e67012 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -10,7 +10,6 @@ obj-$(CONFIG_EEEPC_LAPTOP)	+= eeepc-laptop.o
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
-obj-$(CONFIG_ATMEL_PWM)		+= atmel_pwm.o
 obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
 obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
 obj-$(CONFIG_HP_WMI)		+= hp-wmi.o
diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
deleted file mode 100644
index 6aa5294..0000000
--- a/drivers/misc/atmel_pwm.c
+++ /dev/null
@@ -1,409 +0,0 @@
-#include <linux/module.h>
-#include <linux/clk.h>
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/interrupt.h>
-#include <linux/platform_device.h>
-#include <linux/atmel_pwm.h>
-
-
-/*
- * This is a simple driver for the PWM controller found in various newer
- * Atmel SOCs, including the AVR32 series and the AT91sam9263.
- *
- * Chips with current Linux ports have only 4 PWM channels, out of max 32.
- * AT32UC3A and AT32UC3B chips have 7 channels (but currently no Linux).
- * Docs are inconsistent about the width of the channel counter registers;
- * it's at least 16 bits, but several places say 20 bits.
- */
-#define	PWM_NCHAN	4		/* max 32 */
-
-struct pwm {
-	spinlock_t		lock;
-	struct platform_device	*pdev;
-	u32			mask;
-	int			irq;
-	void __iomem		*base;
-	struct clk		*clk;
-	struct pwm_channel	*channel[PWM_NCHAN];
-	void			(*handler[PWM_NCHAN])(struct pwm_channel *);
-};
-
-
-/* global PWM controller registers */
-#define PWM_MR		0x00
-#define PWM_ENA		0x04
-#define PWM_DIS		0x08
-#define PWM_SR		0x0c
-#define PWM_IER		0x10
-#define PWM_IDR		0x14
-#define PWM_IMR		0x18
-#define PWM_ISR		0x1c
-
-static inline void pwm_writel(const struct pwm *p, unsigned offset, u32 val)
-{
-	__raw_writel(val, p->base + offset);
-}
-
-static inline u32 pwm_readl(const struct pwm *p, unsigned offset)
-{
-	return __raw_readl(p->base + offset);
-}
-
-static inline void __iomem *pwmc_regs(const struct pwm *p, int index)
-{
-	return p->base + 0x200 + index * 0x20;
-}
-
-static struct pwm *pwm;
-
-static void pwm_dumpregs(struct pwm_channel *ch, char *tag)
-{
-	struct device	*dev = &pwm->pdev->dev;
-
-	dev_dbg(dev, "%s: mr %08x, sr %08x, imr %08x\n",
-		tag,
-		pwm_readl(pwm, PWM_MR),
-		pwm_readl(pwm, PWM_SR),
-		pwm_readl(pwm, PWM_IMR));
-	dev_dbg(dev,
-		"pwm ch%d - mr %08x, dty %u, prd %u, cnt %u\n",
-		ch->index,
-		pwm_channel_readl(ch, PWM_CMR),
-		pwm_channel_readl(ch, PWM_CDTY),
-		pwm_channel_readl(ch, PWM_CPRD),
-		pwm_channel_readl(ch, PWM_CCNT));
-}
-
-
-/**
- * pwm_channel_alloc - allocate an unused PWM channel
- * @index: identifies the channel
- * @ch: structure to be initialized
- *
- * Drivers allocate PWM channels according to the board's wiring, and
- * matching board-specific setup code.  Returns zero or negative errno.
- */
-int pwm_channel_alloc(int index, struct pwm_channel *ch)
-{
-	unsigned long	flags;
-	int		status = 0;
-
-	/* insist on PWM init, with this signal pinned out */
-	if (!pwm || !(pwm->mask & 1 << index))
-		return -ENODEV;
-
-	if (index < 0 || index >= PWM_NCHAN || !ch)
-		return -EINVAL;
-	memset(ch, 0, sizeof *ch);
-
-	spin_lock_irqsave(&pwm->lock, flags);
-	if (pwm->channel[index])
-		status = -EBUSY;
-	else {
-		clk_enable(pwm->clk);
-
-		ch->regs = pwmc_regs(pwm, index);
-		ch->index = index;
-
-		/* REVISIT: ap7000 seems to go 2x as fast as we expect!! */
-		ch->mck = clk_get_rate(pwm->clk);
-
-		pwm->channel[index] = ch;
-		pwm->handler[index] = NULL;
-
-		/* channel and irq are always disabled when we return */
-		pwm_writel(pwm, PWM_DIS, 1 << index);
-		pwm_writel(pwm, PWM_IDR, 1 << index);
-	}
-	spin_unlock_irqrestore(&pwm->lock, flags);
-	return status;
-}
-EXPORT_SYMBOL(pwm_channel_alloc);
-
-static int pwmcheck(struct pwm_channel *ch)
-{
-	int		index;
-
-	if (!pwm)
-		return -ENODEV;
-	if (!ch)
-		return -EINVAL;
-	index = ch->index;
-	if (index < 0 || index >= PWM_NCHAN || pwm->channel[index] != ch)
-		return -EINVAL;
-
-	return index;
-}
-
-/**
- * pwm_channel_free - release a previously allocated channel
- * @ch: the channel being released
- *
- * The channel is completely shut down (counter and IRQ disabled),
- * and made available for re-use.  Returns zero, or negative errno.
- */
-int pwm_channel_free(struct pwm_channel *ch)
-{
-	unsigned long	flags;
-	int		t;
-
-	spin_lock_irqsave(&pwm->lock, flags);
-	t = pwmcheck(ch);
-	if (t >= 0) {
-		pwm->channel[t] = NULL;
-		pwm->handler[t] = NULL;
-
-		/* channel and irq are always disabled when we return */
-		pwm_writel(pwm, PWM_DIS, 1 << t);
-		pwm_writel(pwm, PWM_IDR, 1 << t);
-
-		clk_disable(pwm->clk);
-		t = 0;
-	}
-	spin_unlock_irqrestore(&pwm->lock, flags);
-	return t;
-}
-EXPORT_SYMBOL(pwm_channel_free);
-
-int __pwm_channel_onoff(struct pwm_channel *ch, int enabled)
-{
-	unsigned long	flags;
-	int		t;
-
-	/* OMITTED FUNCTIONALITY:  starting several channels in synch */
-
-	spin_lock_irqsave(&pwm->lock, flags);
-	t = pwmcheck(ch);
-	if (t >= 0) {
-		pwm_writel(pwm, enabled ? PWM_ENA : PWM_DIS, 1 << t);
-		t = 0;
-		pwm_dumpregs(ch, enabled ? "enable" : "disable");
-	}
-	spin_unlock_irqrestore(&pwm->lock, flags);
-
-	return t;
-}
-EXPORT_SYMBOL(__pwm_channel_onoff);
-
-/**
- * pwm_clk_alloc - allocate and configure CLKA or CLKB
- * @prescale: from 0..10, the power of two used to divide MCK
- * @div: from 1..255, the linear divisor to use
- *
- * Returns PWM_CPR_CLKA, PWM_CPR_CLKB, or negative errno.  The allocated
- * clock will run with a period of (2^prescale * div) / MCK, or twice as
- * long if center aligned PWM output is used.  The clock must later be
- * deconfigured using pwm_clk_free().
- */
-int pwm_clk_alloc(unsigned prescale, unsigned div)
-{
-	unsigned long	flags;
-	u32		mr;
-	u32		val = (prescale << 8) | div;
-	int		ret = -EBUSY;
-
-	if (prescale >= 10 || div == 0 || div > 255)
-		return -EINVAL;
-
-	spin_lock_irqsave(&pwm->lock, flags);
-	mr = pwm_readl(pwm, PWM_MR);
-	if ((mr & 0xffff) == 0) {
-		mr |= val;
-		ret = PWM_CPR_CLKA;
-	} else if ((mr & (0xffff << 16)) == 0) {
-		mr |= val << 16;
-		ret = PWM_CPR_CLKB;
-	}
-	if (ret > 0)
-		pwm_writel(pwm, PWM_MR, mr);
-	spin_unlock_irqrestore(&pwm->lock, flags);
-	return ret;
-}
-EXPORT_SYMBOL(pwm_clk_alloc);
-
-/**
- * pwm_clk_free - deconfigure and release CLKA or CLKB
- *
- * Reverses the effect of pwm_clk_alloc().
- */
-void pwm_clk_free(unsigned clk)
-{
-	unsigned long	flags;
-	u32		mr;
-
-	spin_lock_irqsave(&pwm->lock, flags);
-	mr = pwm_readl(pwm, PWM_MR);
-	if (clk == PWM_CPR_CLKA)
-		pwm_writel(pwm, PWM_MR, mr & ~(0xffff << 0));
-	if (clk == PWM_CPR_CLKB)
-		pwm_writel(pwm, PWM_MR, mr & ~(0xffff << 16));
-	spin_unlock_irqrestore(&pwm->lock, flags);
-}
-EXPORT_SYMBOL(pwm_clk_free);
-
-/**
- * pwm_channel_handler - manage channel's IRQ handler
- * @ch: the channel
- * @handler: the handler to use, possibly NULL
- *
- * If the handler is non-null, the handler will be called after every
- * period of this PWM channel.  If the handler is null, this channel
- * won't generate an IRQ.
- */
-int pwm_channel_handler(struct pwm_channel *ch,
-		void (*handler)(struct pwm_channel *ch))
-{
-	unsigned long	flags;
-	int		t;
-
-	spin_lock_irqsave(&pwm->lock, flags);
-	t = pwmcheck(ch);
-	if (t >= 0) {
-		pwm->handler[t] = handler;
-		pwm_writel(pwm, handler ? PWM_IER : PWM_IDR, 1 << t);
-		t = 0;
-	}
-	spin_unlock_irqrestore(&pwm->lock, flags);
-
-	return t;
-}
-EXPORT_SYMBOL(pwm_channel_handler);
-
-static irqreturn_t pwm_irq(int id, void *_pwm)
-{
-	struct pwm	*p = _pwm;
-	irqreturn_t	handled = IRQ_NONE;
-	u32		irqstat;
-	int		index;
-
-	spin_lock(&p->lock);
-
-	/* ack irqs, then handle them */
-	irqstat = pwm_readl(pwm, PWM_ISR);
-
-	while (irqstat) {
-		struct pwm_channel *ch;
-		void (*handler)(struct pwm_channel *ch);
-
-		index = ffs(irqstat) - 1;
-		irqstat &= ~(1 << index);
-		ch = pwm->channel[index];
-		handler = pwm->handler[index];
-		if (handler && ch) {
-			spin_unlock(&p->lock);
-			handler(ch);
-			spin_lock(&p->lock);
-			handled = IRQ_HANDLED;
-		}
-	}
-
-	spin_unlock(&p->lock);
-	return handled;
-}
-
-static int __init pwm_probe(struct platform_device *pdev)
-{
-	struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	int irq = platform_get_irq(pdev, 0);
-	u32 *mp = pdev->dev.platform_data;
-	struct pwm *p;
-	int status = -EIO;
-
-	if (pwm)
-		return -EBUSY;
-	if (!r || irq < 0 || !mp || !*mp)
-		return -ENODEV;
-	if (*mp & ~((1<<PWM_NCHAN)-1)) {
-		dev_warn(&pdev->dev, "mask 0x%x ... more than %d channels\n",
-			*mp, PWM_NCHAN);
-		return -EINVAL;
-	}
-
-	p = kzalloc(sizeof(*p), GFP_KERNEL);
-	if (!p)
-		return -ENOMEM;
-
-	spin_lock_init(&p->lock);
-	p->pdev = pdev;
-	p->mask = *mp;
-	p->irq = irq;
-	p->base = ioremap(r->start, r->end - r->start + 1);
-	if (!p->base)
-		goto fail;
-	p->clk = clk_get(&pdev->dev, "pwm_clk");
-	if (IS_ERR(p->clk)) {
-		status = PTR_ERR(p->clk);
-		p->clk = NULL;
-		goto fail;
-	}
-
-	status = request_irq(irq, pwm_irq, 0, pdev->name, p);
-	if (status < 0)
-		goto fail;
-
-	pwm = p;
-	platform_set_drvdata(pdev, p);
-
-	return 0;
-
-fail:
-	if (p->clk)
-		clk_put(p->clk);
-	if (p->base)
-		iounmap(p->base);
-
-	kfree(p);
-	return status;
-}
-
-static int __exit pwm_remove(struct platform_device *pdev)
-{
-	struct pwm *p = platform_get_drvdata(pdev);
-
-	if (p != pwm)
-		return -EINVAL;
-
-	clk_enable(pwm->clk);
-	pwm_writel(pwm, PWM_DIS, (1 << PWM_NCHAN) - 1);
-	pwm_writel(pwm, PWM_IDR, (1 << PWM_NCHAN) - 1);
-	clk_disable(pwm->clk);
-
-	pwm = NULL;
-
-	free_irq(p->irq, p);
-	clk_put(p->clk);
-	iounmap(p->base);
-	kfree(p);
-
-	return 0;
-}
-
-static struct platform_driver atmel_pwm_driver = {
-	.driver = {
-		.name = "atmel_pwm",
-		.owner = THIS_MODULE,
-	},
-	.remove = __exit_p(pwm_remove),
-
-	/* NOTE: PWM can keep running in AVR32 "idle" and "frozen" states;
-	 * and all AT91sam9263 states, albeit at reduced clock rate if
-	 * MCK becomes the slow clock (i.e. what Linux labels STR).
-	 */
-};
-
-static int __init pwm_init(void)
-{
-	return platform_driver_probe(&atmel_pwm_driver, pwm_probe);
-}
-module_init(pwm_init);
-
-static void __exit pwm_exit(void)
-{
-	platform_driver_unregister(&atmel_pwm_driver);
-}
-module_exit(pwm_exit);
-
-MODULE_DESCRIPTION("Driver for AT32/AT91 PWM module");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:atmel_pwm");
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
new file mode 100644
index 0000000..933bb2c
--- /dev/null
+++ b/drivers/pwm/Kconfig
@@ -0,0 +1,24 @@
+#
+# PWM infrastructure and devices
+#
+
+menuconfig GENERIC_PWM
+	tristate "PWM Support"
+	help
+	  This enables PWM support through the generic PWM library.
+	  If unsure, say N.
+
+if GENERIC_PWM
+
+config ATMEL_PWM
+	tristate "Atmel AT32/AT91 PWM support"
+	depends on AVR32 || ARCH_AT91
+	help
+	  This option enables device driver support for the PWMC
+	  peripheral channels found on certain Atmel processors.
+	  Pulse Width Modulation is used many for purposes, including
+	  software controlled power-efficent backlights on LCD
+	  displays, motor control, and waveform generation.  If
+	  unsure, say N.
+
+endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
new file mode 100644
index 0000000..21634f6
--- /dev/null
+++ b/drivers/pwm/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for pwm devices
+#
+obj-y := pwm.o
+
+obj-$(CONFIG_ATMEL_PWM)		+= atmel-pwm.o
-- 
1.5.6.5

[RFC 6/6] [PWM] New LED driver and trigger that use PWM API

[Bill Gatliff]

Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
---
 drivers/leds/Kconfig       |   21 ++++--
 drivers/leds/Makefile      |    2 +
 drivers/leds/leds-pwm.c    |  141 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/leds/ledtrig-dim.c |   95 +++++++++++++++++++++++++++++
 include/linux/pwm-led.h    |   34 +++++++++++
 5 files changed, 286 insertions(+), 7 deletions(-)
 create mode 100644 drivers/leds/leds-pwm.c
 create mode 100644 drivers/leds/ledtrig-dim.c
 create mode 100644 include/linux/pwm-led.h

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 9556262..019c2e8 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -17,13 +17,6 @@ config LEDS_CLASS
 
 comment "LED drivers"
 
-config LEDS_ATMEL_PWM
-	tristate "LED Support using Atmel PWM outputs"
-	depends on LEDS_CLASS && ATMEL_PWM
-	help
-	  This option enables support for LEDs driven using outputs
-	  of the dedicated PWM controller found on newer Atmel SOCs.
-
 config LEDS_CORGI
 	tristate "LED Support for the Sharp SL-C7x0 series"
 	depends on LEDS_CLASS && PXA_SHARP_C7xx
@@ -119,6 +112,12 @@ config LEDS_GPIO
 	  outputs. To be useful the particular board must have LEDs
 	  and they must be connected to the GPIO lines.
 
+config LEDS_PWM
+       tristate "LED Support for PWM connected LEDs"
+       depends on LEDS_CLASS && GENERIC_PWM
+       help
+         Enables support for LEDs connected to PWM outputs.
+
 config LEDS_CM_X270
 	tristate "LED Support for the CM-X270 LEDs"
 	depends on LEDS_CLASS && MACH_ARMCORE
@@ -190,6 +189,14 @@ config LEDS_TRIGGER_IDE_DISK
 	  This allows LEDs to be controlled by IDE disk activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_DIM
+	tristate "LED Dimmer Trigger"
+	depends on LEDS_TRIGGERS
+	help
+	  Regulates the brightness of an LED based on the 1-minute CPU
+	  load average.  Ideal for PWM-driven LEDs.
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_HEARTBEAT
 	tristate "LED Heartbeat Trigger"
 	depends on LEDS_TRIGGERS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index ff7982b..1031086 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_LEDS_COBALT_QUBE)		+= leds-cobalt-qube.o
 obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
 obj-$(CONFIG_LEDS_PCA9532)		+= leds-pca9532.o
 obj-$(CONFIG_LEDS_GPIO)			+= leds-gpio.o
+obj-$(CONFIG_LEDS_PWM)					+= leds-pwm.o
 obj-$(CONFIG_LEDS_CM_X270)              += leds-cm-x270.o
 obj-$(CONFIG_LEDS_CLEVO_MAIL)		+= leds-clevo-mail.o
 obj-$(CONFIG_LEDS_HP6XX)		+= leds-hp6xx.o
@@ -27,5 +28,6 @@ obj-$(CONFIG_LEDS_PCA955X)		+= leds-pca955x.o
 # LED Triggers
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
 obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK)	+= ledtrig-ide-disk.o
+obj-$(CONFIG_LEDS_TRIGGER_DIM)		+= ledtrig-dim.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o
 obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
new file mode 100644
index 0000000..3bd9afb
--- /dev/null
+++ b/drivers/leds/leds-pwm.c
@@ -0,0 +1,141 @@
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/leds.h>
+#include <linux/io.h>
+#include <linux/pwm.h>
+#include <linux/pwm-led.h>
+
+
+struct led_pwm {
+	struct led_classdev	led;
+	struct pwm_channel	*pwm;
+	int percent;
+};
+
+
+static void
+led_pwm_brightness_set(struct led_classdev *c,
+		       enum led_brightness b)
+{
+	struct led_pwm *led;
+	int percent;
+
+	percent = (b * 100) / (LED_FULL - LED_OFF);
+	led = container_of(c, struct led_pwm, led);
+	led->percent = percent;
+	pwm_duty_percent(led->pwm, percent);
+}
+
+
+static enum led_brightness
+led_pwm_brightness_get(struct led_classdev *c)
+{
+	struct led_pwm *led;
+	led = container_of(c, struct led_pwm, led);
+	return led->percent;
+}
+
+
+static int __init
+led_pwm_probe(struct platform_device *pdev)
+{
+	struct pwm_led_platform_data *pdata = pdev->dev.platform_data;
+	struct led_pwm *led;
+	struct device *d = &pdev->dev;
+	int ret;
+
+	if (!pdata || !pdata->led_info)
+		return -EINVAL;
+
+	if (!try_module_get(d->driver->owner))
+		return -ENODEV;
+
+	led = kzalloc(sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	led->pwm = pwm_request(pdata->bus_id, pdata->chan,
+			       pdata->led_info->name);
+	if (!led->pwm) {
+		ret = -EINVAL;
+		goto err_pwm_request;
+	}
+
+	platform_set_drvdata(pdev, led);
+
+	led->led.name = pdata->led_info->name;
+	led->led.default_trigger = pdata->led_info->default_trigger;
+	led->led.brightness_set = led_pwm_brightness_set;
+	led->led.brightness_get = led_pwm_brightness_get;
+	led->led.brightness = LED_OFF;
+
+	ret = pwm_config(led->pwm, pdata->config);
+	if (ret)
+		goto err_pwm_config;
+	pwm_start(led->pwm);
+
+	ret = led_classdev_register(&pdev->dev, &led->led);
+	if (ret < 0)
+		goto err_classdev_register;
+
+	return 0;
+
+err_classdev_register:
+	pwm_stop(led->pwm);
+err_pwm_config:
+	pwm_free(led->pwm);
+err_pwm_request:
+	kfree(led);
+
+	return ret;
+}
+
+
+static int
+led_pwm_remove(struct platform_device *pdev)
+{
+	struct led_pwm *led = platform_get_drvdata(pdev);
+	struct device *d = &pdev->dev;
+
+	led_classdev_unregister(&led->led);
+
+	if (led->pwm) {
+		pwm_stop(led->pwm);
+		pwm_free(led->pwm);
+	}
+
+	kfree(led);
+	module_put(d->driver->owner);
+
+	return 0;
+}
+
+
+static struct platform_driver led_pwm_driver = {
+	.driver = {
+		.name =		"leds-pwm",
+		.owner =	THIS_MODULE,
+	},
+	.probe = led_pwm_probe,
+	.remove = led_pwm_remove,
+};
+
+
+static int __init led_pwm_modinit(void)
+{
+	return platform_driver_register(&led_pwm_driver);
+}
+module_init(led_pwm_modinit);
+
+
+static void __exit led_pwm_modexit(void)
+{
+	platform_driver_unregister(&led_pwm_driver);
+}
+module_exit(led_pwm_modexit);
+
+
+MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
+MODULE_DESCRIPTION("Driver for LEDs with PWM-controlled brightness");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-pwm");
diff --git a/drivers/leds/ledtrig-dim.c b/drivers/leds/ledtrig-dim.c
new file mode 100644
index 0000000..299865b
--- /dev/null
+++ b/drivers/leds/ledtrig-dim.c
@@ -0,0 +1,95 @@
+/*
+ * LED Dim Trigger
+ *
+ * Copyright (C) 2008 Bill Gatliff <bgat@billgatliff.com>
+ *
+ * "Dims" an LED based on system load.  Derived from Atsushi Nemoto's
+ * ledtrig-heartbeat.c.
+ *
+ * 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.
+ *
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/sched.h>
+#include <linux/leds.h>
+
+#include "leds.h"
+
+struct dim_trig_data {
+	struct timer_list timer;
+};
+
+
+static void
+led_dim_function(unsigned long data)
+{
+	struct led_classdev *led_cdev = (struct led_classdev *)data;
+	struct dim_trig_data *dim_data = led_cdev->trigger_data;
+	unsigned int brightness;
+
+	brightness = ((LED_FULL - LED_OFF) * avenrun[0]) / EXP_1;
+	if (brightness > LED_FULL)
+		brightness = LED_FULL;
+
+	led_set_brightness(led_cdev, brightness);
+	mod_timer(&dim_data->timer, jiffies + msecs_to_jiffies(500));
+}
+
+
+static void
+dim_trig_activate(struct led_classdev *led_cdev)
+{
+	struct dim_trig_data *dim_data;
+
+	dim_data = kzalloc(sizeof(*dim_data), GFP_KERNEL);
+	if (!dim_data)
+		return;
+
+	led_cdev->trigger_data = dim_data;
+	setup_timer(&dim_data->timer,
+		    led_dim_function, (unsigned long)led_cdev);
+	led_dim_function(dim_data->timer.data);
+}
+
+
+static void
+dim_trig_deactivate(struct led_classdev *led_cdev)
+{
+	struct dim_trig_data *dim_data = led_cdev->trigger_data;
+
+	if (dim_data) {
+		del_timer_sync(&dim_data->timer);
+		kfree(dim_data);
+	}
+}
+
+
+static struct led_trigger dim_led_trigger = {
+	.name     = "dim",
+	.activate = dim_trig_activate,
+	.deactivate = dim_trig_deactivate,
+};
+
+
+static int __init dim_trig_init(void)
+{
+	return led_trigger_register(&dim_led_trigger);
+}
+module_init(dim_trig_init);
+
+
+static void __exit dim_trig_exit(void)
+{
+	led_trigger_unregister(&dim_led_trigger);
+}
+module_exit(dim_trig_exit);
+
+
+MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
+MODULE_DESCRIPTION("Dim LED trigger");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pwm-led.h b/include/linux/pwm-led.h
new file mode 100644
index 0000000..92363c8
--- /dev/null
+++ b/include/linux/pwm-led.h
@@ -0,0 +1,34 @@
+#ifndef __LINUX_PWM_LED_H
+#define __LINUX_PWM_LED_H
+
+/*
+ * include/linux/pwm-led.h
+ *
+ * Copyright (C) 2008 Bill Gatliff
+ *
+ * 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 of the License, 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; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+struct led_info;
+struct pwm_channel_config;
+
+struct pwm_led_platform_data {
+	const char *bus_id;
+	int chan;
+	struct pwm_channel_config *config;
+	struct led_info *led_info;
+};
+
+#endif /* __LINUX_PWM_LED_H */
-- 
1.5.6.5

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Mike Frysinger]

On Wed, Oct 8, 2008 at 12:43, Bill Gatliff wrote:
> This series proposes a "generic PWM" driver API.

seems that the API is solely geared to handle PWM as an output signal.
 what about input ?

all the utility config functions lack "set" in their name.  it's a
little confusing as to whether the function is a get or set at first
glance.  rather than expecting drivers to poke directly into the
structure, a set of "get" functions would work better (even if they're
simply #define's that poke into the structure) and line up better with
how the GPIO framework operates.
-mike

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Mike Frysinger wrote:
> On Wed, Oct 8, 2008 at 12:43, Bill Gatliff wrote:
>> This series proposes a "generic PWM" driver API.
> 
> seems that the API is solely geared to handle PWM as an output signal.

True.  The peripherals I'm currently targeting are output-only devices, and the
API reflects that.

>  what about input ?

Well, the SAM9263 has timer/counters that could be used to measure PWM period
and duty cycle.  But they are a different peripheral entirely.  I haven't done
an exhaustive survey, but I'm not aware of any PWM-generating hardware that is
simultaneously PWM-measuring hardware as well.  Seems like they are either one
or the other.

Are you proposing that the API accommodate both input and output devices?

> all the utility config functions lack "set" in their name.  it's a
> little confusing as to whether the function is a get or set at first
> glance.  rather than expecting drivers to poke directly into the
> structure, a set of "get" functions would work better (even if they're
> simply #define's that poke into the structure) and line up better with
> how the GPIO framework operates.

Good point.

Originally, I was thinking along the lines of a set-and-forget use case.  Do you
use "get" functionality much when generating PWM signals in your applications?


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Bill Gatliff wrote:

> 
>> all the utility config functions lack "set" in their name.  it's a
>> little confusing as to whether the function is a get or set at first
>> glance.


For the record, I can support adding a "set" to the function names.


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Mike Frysinger]

On Wed, Oct 8, 2008 at 22:23, Bill Gatliff wrote:
> Mike Frysinger wrote:
>> On Wed, Oct 8, 2008 at 12:43, Bill Gatliff wrote:
>>> This series proposes a "generic PWM" driver API.
>>
>> seems that the API is solely geared to handle PWM as an output signal.
>
> True.  The peripherals I'm currently targeting are output-only devices, and the
> API reflects that.
>
>>  what about input ?
>
> Well, the SAM9263 has timer/counters that could be used to measure PWM period
> and duty cycle.  But they are a different peripheral entirely.  I haven't done
> an exhaustive survey, but I'm not aware of any PWM-generating hardware that is
> simultaneously PWM-measuring hardware as well.  Seems like they are either one
> or the other.

the Blackfin timers/pwm's can flip between input and ouput based on
the configuration register.  everything else (pin/etc...) is
unchanged.

> Are you proposing that the API accommodate both input and output devices?

i dont think we should preclude it from the outset.

>> all the utility config functions lack "set" in their name.  it's a
>> little confusing as to whether the function is a get or set at first
>> glance.  rather than expecting drivers to poke directly into the
>> structure, a set of "get" functions would work better (even if they're
>> simply #define's that poke into the structure) and line up better with
>> how the GPIO framework operates.
>
> Good point.
>
> Originally, I was thinking along the lines of a set-and-forget use case.  Do you
> use "get" functionality much when generating PWM signals in your applications?

not really, but i see the existing code you've posted could already
utilize some of the "get" functions ...
-mike

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Mike Frysinger wrote:
> 
> the Blackfin timers/pwm's can flip between input and ouput based on
> the configuration register.  everything else (pin/etc...) is
> unchanged.

Interesting!

>> Are you proposing that the API accommodate both input and output devices?
> 
> i dont think we should preclude it from the outset.

I don't think have a problem with that.  At some point, I would need someone
with input/output hardware to test the input-specific parts, however.  Hint,
hint.  ;)

> not really, but i see the existing code you've posted could already
> utilize some of the "get" functions ...

Which parts?  I don't really like keeping the period_ticks and duty_ticks values
around, but in my case I have no choice--- unless I were to read the CPRE, CPRD
and CDTY values from the hardware directly.  Which could be what your proposed
"get" methods would do.

But that still isn't PWM "input" the way you are describing, because my hardware
wouldn't be _reading_ an incoming PWM: it would be just reporting back the
values that it was using to produce its output signal.  Huge difference.

I dunno, I'm still not sure that measuring the characteristics of an incoming
PWM signal isn't a different kind of device from one that produces PWM signals,
at least conceptually.  Which, in a way, makes it out of scope for the proposed
API.  Not saying I can't go along with the idea, I'm just not sure what a user
would expect to happen if they called pwm_get_period_ns() on the SAM9263 PWMC
device.  They certainly aren't going to get a measured value in return!



b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Mike Frysinger]

On Wed, Oct 8, 2008 at 23:46, Bill Gatliff wrote:
> Mike Frysinger wrote:
>>> Are you proposing that the API accommodate both input and output devices?
>>
>> i dont think we should preclude it from the outset.
>
> I don't think have a problem with that.  At some point, I would need someone
> with input/output hardware to test the input-specific parts, however.  Hint,
> hint.  ;)

if you'd seriously play with a Blackfin board, i think we can arrange that

>> not really, but i see the existing code you've posted could already
>> utilize some of the "get" functions ...
>
> Which parts?  I don't really like keeping the period_ticks and duty_ticks values
> around, but in my case I have no choice--- unless I were to read the CPRE, CPRD
> and CDTY values from the hardware directly.  Which could be what your proposed
> "get" methods would do.

sorry, i misread the led driver.  too many structures! :)

> But that still isn't PWM "input" the way you are describing, because my hardware
> wouldn't be _reading_ an incoming PWM: it would be just reporting back the
> values that it was using to produce its output signal.  Huge difference.
>
> I dunno, I'm still not sure that measuring the characteristics of an incoming
> PWM signal isn't a different kind of device from one that produces PWM signals,
> at least conceptually.  Which, in a way, makes it out of scope for the proposed
> API.  Not saying I can't go along with the idea, I'm just not sure what a user
> would expect to happen if they called pwm_get_period_ns() on the SAM9263 PWMC
> device.  They certainly aren't going to get a measured value in return!

while true, hardware that can support PWM as both input/output would
suffer from two frameworks.  if there's ambiguity in behavior (using
"get" in an output mode), then we can just stick it in the
documentation and move on.  the GPIO framework already has this
behavior (set a pin to output and then try and read the data) and i
dont recall it ever being an issue there.
-mike

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Mike Frysinger wrote:
> if you'd seriously play with a Blackfin board, i think we can arrange that

I'd seriously *love* to play with one, but I'm pretty strapped for time for
another couple of months.  The only purpose it would serve near-term would be to
prove out the input capabilities of a device that I probably wouldn't have time
to write a driver for.  :(

> while true, hardware that can support PWM as both input/output would
> suffer from two frameworks.  if there's ambiguity in behavior (using
> "get" in an output mode), then we can just stick it in the
> documentation and move on.  the GPIO framework already has this
> behavior (set a pin to output and then try and read the data) and i
> dont recall it ever being an issue there.

Good point.  I think I'm sold on the idea now.

We'd need a PWM_CONFIG_<something> to tell the hardware to switch to
"measurement mode" if such a mode is supported (suggestions for <something>
welcomed).  The config function would return an error if the measurement mode
wasn't supported by the device.  PWM_CONFIG_INPUT and PWM_CONFIG_OUTPUT, perhaps?

In output mode, the pwm_get_*() methods would return the driven values if the
device didn't support a (simultaneous) measurement mode, or cached values if the
device's configuration registers were write-only.  In measurement mode, they'd
return the measured values.

I think this'll work.



b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Mike Frysinger]

On Thu, Oct 9, 2008 at 00:18, Bill Gatliff wrote:
> Mike Frysinger wrote:
>> if you'd seriously play with a Blackfin board, i think we can arrange that
>
> I'd seriously *love* to play with one, but I'm pretty strapped for time for
> another couple of months.  The only purpose it would serve near-term would be to
> prove out the input capabilities of a device that I probably wouldn't have time
> to write a driver for.  :(

i'll probably look at the Blackfin-pwm core soonish ... we already
implemented a generic framework for it (look for "gptimers"), so it
shouldnt take too much time to transition code for it.

>> while true, hardware that can support PWM as both input/output would
>> suffer from two frameworks.  if there's ambiguity in behavior (using
>> "get" in an output mode), then we can just stick it in the
>> documentation and move on.  the GPIO framework already has this
>> behavior (set a pin to output and then try and read the data) and i
>> dont recall it ever being an issue there.
>
> Good point.  I think I'm sold on the idea now.
>
> We'd need a PWM_CONFIG_<something> to tell the hardware to switch to
> "measurement mode" if such a mode is supported (suggestions for <something>
> welcomed).  The config function would return an error if the measurement mode
> wasn't supported by the device.  PWM_CONFIG_INPUT and PWM_CONFIG_OUTPUT, perhaps?

only part on Kconfig i think we'll need is each implementation
selecting "GENERIC_PWM_INPUT" and "GENERIC_PWM_OUTPUT".  are we
looking towards having multiple master implementations being usable
simultaneously ?

otherwise, i dont think defining things in terms of "generating" and
"measuring" is really needed ... "input" and "output" seems
straightforward enough.

> In output mode, the pwm_get_*() methods would return the driven values if the
> device didn't support a (simultaneous) measurement mode, or cached values if the
> device's configuration registers were write-only.  In measurement mode, they'd
> return the measured values.

the current generic core looks like it could handle this easily enough
since we're working with an array of function pointers.
-mike

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1410 bytes --]

On Thu, 9 Oct 2008, Bill Gatliff wrote:
> Benjamin Herrenschmidt wrote:
> > On Wed, 2008-10-08 at 11:43 -0500, Bill Gatliff wrote:
> >> This series proposes a "generic PWM" driver API.
> >>
> >> This proposed API is motivated by the author's need to support
> >> pluggable devices; a secondary objective is to consolidate the
> >> existing PWM implementations behind an agreeable, consistent,
> >> redundancy-reducing interface.
> > 
> >  .../...
> > 
> > You should send your patches to the main linux kernel list !
> 
> Perhaps.  But it seemed more relevant to this crowd, and the linux-embedded
> crowd, and the linux-arm-kernel crowd.

Were did you actually sent them to?  Apparently you sent them to each mailing
list (at least linux-embedded and linuxppc-dev) _separately_ (or using bcc).

Hence different people may give the same comments without knowing about each
other, and you may have to explain everything multiple times.

I would go for lkml and linux-embedded, _together_.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Paul Mundt]

On Fri, Oct 10, 2008 at 11:00:09AM +0200, Geert Uytterhoeven wrote:
> On Thu, 9 Oct 2008, Bill Gatliff wrote:
> > Benjamin Herrenschmidt wrote:
> > > On Wed, 2008-10-08 at 11:43 -0500, Bill Gatliff wrote:
> > >> This series proposes a "generic PWM" driver API.
> > >>
> > >> This proposed API is motivated by the author's need to support
> > >> pluggable devices; a secondary objective is to consolidate the
> > >> existing PWM implementations behind an agreeable, consistent,
> > >> redundancy-reducing interface.
> > > 
> > >  .../...
> > > 
> > > You should send your patches to the main linux kernel list !
> > 
> > Perhaps.  But it seemed more relevant to this crowd, and the linux-embedded
> > crowd, and the linux-arm-kernel crowd.
> 
> Were did you actually sent them to?  Apparently you sent them to each mailing
> list (at least linux-embedded and linuxppc-dev) _separately_ (or using bcc).
> 
> Hence different people may give the same comments without knowing about each
> other, and you may have to explain everything multiple times.
> 
> I would go for lkml and linux-embedded, _together_.
> 
This is likely because some of those lists are subscribers only, so cross
posting is poor form. It makes sense to keep the discussion in one place,
and to send notification messages with a pointer to the list archives to
the other lists so folks can jump in if they really care. Splitting it
out doesn't help matters in the least, but unfortunately this is what
seems to happen the most when subscribers only lists are involved.

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[David Woodhouse]

On Fri, 2008-10-10 at 18:36 +0900, Paul Mundt wrote:
> This is likely because some of those lists are subscribers only, so cross
> posting is poor form. It makes sense to keep the discussion in one place,
> and to send notification messages with a pointer to the list archives to
> the other lists so folks can jump in if they really care. Splitting it
> out doesn't help matters in the least, but unfortunately this is what
> seems to happen the most when subscribers only lists are involved.

Subscriber-only lists are broken. Just don't use them.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

David Woodhouse wrote:
> Subscriber-only lists are broken. Just don't use them.

You owe me a new keyboard!  :)


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Paul Mundt wrote:
> This is likely because some of those lists are subscribers only, so cross
> posting is poor form. It makes sense to keep the discussion in one place,
> and to send notification messages with a pointer to the list archives to
> the other lists so folks can jump in if they really care. Splitting it
> out doesn't help matters in the least, but unfortunately this is what
> seems to happen the most when subscribers only lists are involved.

Alright, then maybe I can do this when I post the "final" changeset for review:
cross-post to lkml and linux-embedded, and then post one short note on l-a-k,
linuxppc-dev and elsewhere that refers those interested to the actual content.
I can live with that.


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Haavard Skinnemoen]

Bill Gatliff <bgat@billgatliff.com> wrote:
> Paul Mundt wrote:
> > This is likely because some of those lists are subscribers only, so cross
> > posting is poor form. It makes sense to keep the discussion in one place,
> > and to send notification messages with a pointer to the list archives to
> > the other lists so folks can jump in if they really care. Splitting it
> > out doesn't help matters in the least, but unfortunately this is what
> > seems to happen the most when subscribers only lists are involved.  
> 
> Alright, then maybe I can do this when I post the "final" changeset for review:
> cross-post to lkml and linux-embedded, and then post one short note on l-a-k,
> linuxppc-dev and elsewhere that refers those interested to the actual content.
> I can live with that.

Feel free to cross-post to kernel@avr32linux.org. It's open for
non-subscribers, and there may be people interested in PWM there
(especially since you include a driver for the PWM hardware on AVR32
devices.)

Haavard

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Paul Mundt]

On Fri, Oct 10, 2008 at 09:03:34AM -0500, Bill Gatliff wrote:
> Paul Mundt wrote:
> > This is likely because some of those lists are subscribers only, so cross
> > posting is poor form. It makes sense to keep the discussion in one place,
> > and to send notification messages with a pointer to the list archives to
> > the other lists so folks can jump in if they really care. Splitting it
> > out doesn't help matters in the least, but unfortunately this is what
> > seems to happen the most when subscribers only lists are involved.
> 
> Alright, then maybe I can do this when I post the "final" changeset for review:
> cross-post to lkml and linux-embedded, and then post one short note on l-a-k,
> linuxppc-dev and elsewhere that refers those interested to the actual content.
> I can live with that.
> 
linux-arm-kernel is the only one that is subscribers only out of that
list, according to MAINTAINERS. If rmk wants to mandate a broken policy,
that's perfectly fine, just don't expect the rest of the kernel community
to tolerate it.

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Paul Mundt wrote:
> On Fri, Oct 10, 2008 at 09:03:34AM -0500, Bill Gatliff wrote:
>> Paul Mundt wrote:
>>> This is likely because some of those lists are subscribers only, so cross
>>> posting is poor form. It makes sense to keep the discussion in one place,
>>> and to send notification messages with a pointer to the list archives to
>>> the other lists so folks can jump in if they really care. Splitting it
>>> out doesn't help matters in the least, but unfortunately this is what
>>> seems to happen the most when subscribers only lists are involved.
>> Alright, then maybe I can do this when I post the "final" changeset for review:
>> cross-post to lkml and linux-embedded, and then post one short note on l-a-k,
>> linuxppc-dev and elsewhere that refers those interested to the actual content.
>> I can live with that.
>>
> linux-arm-kernel is the only one that is subscribers only out of that
> list, according to MAINTAINERS. If rmk wants to mandate a broken policy,
> that's perfectly fine, just don't expect the rest of the kernel community
> to tolerate it.

Problem is, the rest of the kernel community is the one who takes it in the,
ahem, server when I cross-post.  And since my reference platform is currently
ARM, I can't leave l-a-k out.



b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Geert Uytterhoeven wrote:
> 
> Were did you actually sent them to?  Apparently you sent them to each mailing
> list (at least linux-embedded and linuxppc-dev) _separately_ (or using bcc).

I sent them separately to linux-embedded, linuxppc-dev, and linux-arm-kernel.
Those three groups seemed to have the developers who were most likely to provide
a motivated review and constructive response; unfortunately, some are
subscriber-only and so I couldn't just cross-post.  I was expecting some
criticism for this, but I'm not sure there's a good alternative.

I don't like the idea of posting in so many places, but PWM is a pretty
expansive topic: just about every SoC under the sun has some ability to do PWM,
and people use the signals for all sorts of things.  Both have to be taken into
consideration by the API, hence I need lots of review and feedback.

There isn't a lot of traffic on linux-embedded, and I'm not sure how many people
who read linux-arm-kernel also read linuxppc-dev.  Lkml's topic coverage is
huge, so I don't know how many hardcore embedded developers I would encounter
there.  I was hoping for a round of feedback at a lower level before pushing
anything upstream like that.

> Hence different people may give the same comments without knowing about each
> other, and you may have to explain everything multiple times.

Hasn't been a problem so far.  I posted the first version of the code on l-a-k,
and got some feedback on the pwm_device API and a lot of feedback on the way
users wanted to use the API to realize applications.  I incorporated all of it,
and in this "release" I broadened the exposure per recommendations received from
l-a-k.

> I would go for lkml and linux-embedded, _together_.

So, you're saying the same thing as me, basically.  But leaving out the lists
with very high ratios of device-specific domain knowledge, which is important
for the backend parts of what I'm proposing.  Blackfin's PWM-capable peripherals
work differently from those commonly found in ARM and PPC, for example.  I
haven't run anything by the MIPS or AVR guys, but I'm guessing they would have
something to add, too.

I'm beginning to appreciate what everyone must have had to deal with for GPIO.  :)


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Paul Mundt]

On Fri, Oct 10, 2008 at 08:59:08AM -0500, Bill Gatliff wrote:
> There isn't a lot of traffic on linux-embedded, and I'm not sure how many people
> who read linux-arm-kernel also read linuxppc-dev.  Lkml's topic coverage is
> huge, so I don't know how many hardcore embedded developers I would encounter
> there.  I was hoping for a round of feedback at a lower level before pushing
> anything upstream like that.
> 
This isn't your problem. If people are interested in general embedded
topics, they should be subscribed to the list. If they aren't, it's their
loss. Cross posting to every potentially relevant list is a complete
waste of time, and only helps to split out the discussion so nothing
actually gets accomplished in a centralized location.

> Hasn't been a problem so far.  I posted the first version of the code on l-a-k,
> and got some feedback on the pwm_device API and a lot of feedback on the way
> users wanted to use the API to realize applications.  I incorporated all of it,
> and in this "release" I broadened the exposure per recommendations received from
> l-a-k.
> 
This is precisely the problem. Stuff that gets "reviewed" on
linux-arm-kernel gets reviewed for ARM only. There has been way too much
crap that has been pushed into the kernel under the guise of being
generic and "reviewed" that has broken _every_ architecture _except_ ARM.
If you want to refute this, go look at the recent fiasco with musb, which
still hasn't been solved properly, primarily because the ARM people
couldn't be bothered using grep. This crap happens all the time, because
stuff is reviewed and merged in private, and the only time anyone else
notices is when their platform suddenly stops building.

Your first version should have been to linux-embedded and linux-kernel.
If you want to alert the linux-arm-kernel people to the fact that a
discussion is going on in this area, then feel free to post a
notification to the list with references to the relevant lists. There is
no reason why public lists should have to dig in to private archives to
try and play catch up.

> So, you're saying the same thing as me, basically.  But leaving out the lists
> with very high ratios of device-specific domain knowledge, which is important
> for the backend parts of what I'm proposing.  Blackfin's PWM-capable peripherals
> work differently from those commonly found in ARM and PPC, for example.  I
> haven't run anything by the MIPS or AVR guys, but I'm guessing they would have
> something to add, too.
> 
> I'm beginning to appreciate what everyone must have had to deal with for GPIO.  :)
> 
The GPIO mess was broken in different ways, which we're still trying to
fix today. The GPIO discussion did happen out on public lists though, so
all of the discussion on that was visible, even if the end result left
something to be desired.

If you're trying to pitch a generic API and doing your review on a
private list, you've already lost. If you're talking about things that
only effect arch/arm, feel free to do whatever you want. As soon as you
step outside of that structure, you have to follow common convention, or
you risk breaking things all over the place. I can't remember the last
time I saw a "generic" API reviewed on linux-arm-kernel that didn't end
up breaking every other architecture in existence. This is true for
drivers, also. Better yet, don't bother dropping the ARM depedency until
you've posted to a public list.

Some of us are pretty damn tired of cleaning up after the ARM people.

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Bill Gatliff]

Paul Mundt wrote:
>> Hasn't been a problem so far.  I posted the first version of the code on l-a-k,
>> and got some feedback on the pwm_device API and a lot of feedback on the way
>> users wanted to use the API to realize applications.  I incorporated all of it,
>> and in this "release" I broadened the exposure per recommendations received from
>> l-a-k.
>>
> This is precisely the problem. Stuff that gets "reviewed" on
> linux-arm-kernel gets reviewed for ARM only. There has been way too much
> crap that has been pushed into the kernel under the guise of being
> generic and "reviewed" that has broken _every_ architecture _except_ ARM.
> If you want to refute this, go look at the recent fiasco with musb, which
> still hasn't been solved properly, primarily because the ARM people
> couldn't be bothered using grep. This crap happens all the time, because
> stuff is reviewed and merged in private, and the only time anyone else
> notices is when their platform suddenly stops building.

I'll note for the record that I didn't post on linux-arm-kernel only.
Otherwise, we wouldn't be having this discussion.  :)

> Your first version should have been to linux-embedded and linux-kernel.
> If you want to alert the linux-arm-kernel people to the fact that a
> discussion is going on in this area, then feel free to post a
> notification to the list with references to the relevant lists. There is
> no reason why public lists should have to dig in to private archives to
> try and play catch up.

I'm not asking anyone to do that.  Just review the patches posted to the list of
your choice.  Or, don't review them.  Up to you.

My next update will be the one where I formally request a review with intent to
merge into mainline.  That one will go to linux-embedded only, with
notifications sent elsewhere as indicated per community request.  I don't have a
problem with that.  I can't change history, but I'm doing what you are asking of
me otherwise.

> If you're trying to pitch a generic API and doing your review on a
> private list, you've already lost. If you're talking about things that
> only effect arch/arm, feel free to do whatever you want. As soon as you
> step outside of that structure, you have to follow common convention, or
> you risk breaking things all over the place. I can't remember the last
> time I saw a "generic" API reviewed on linux-arm-kernel that didn't end
> up breaking every other architecture in existence. This is true for
> drivers, also. Better yet, don't bother dropping the ARM depedency until
> you've posted to a public list.

Again, we wouldn't be having this exchange if I was pitching a generic API on a
private list because I sense that you aren't an l-a-k subscriber.  :)

It's true that the early posts were on the ARM list, but you can see that I
didn't stop there.  I started there because the platform that supports the API
right now is ARM, and so I wanted that part to be right before moving upstream.
 That process worked: I received feedback on the ARM-specific bits which
improved the API as a whole.  The diversity of ARM machines plus Blackfin, PPC,
MIPS, X, Y, Z and PDQ machines was more than I could deal with until now.

Right, enough of that.  I really don't want to get distracted from the code.
I'll readily admit to not handing the mailing list submissions right, and I
resolve to do a better job effective immediately.  But I think that's the last
that I need to say on the subject.

If it makes you feel any better, I'll stop responding to your replies unless
they come to me via linux-embedded.  :)

> Some of us are pretty damn tired of cleaning up after the ARM people.

Sounds like the ARM people need you to drop by and help them do a better job.
Sounds like you could directly benefit from their doing a better job, too.  Win-win.




b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 0/6] Proposal for a Generic PWM Device API

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1411 bytes --]

On Fri, 10 Oct 2008, Bill Gatliff wrote:
> Paul Mundt wrote:
> > Your first version should have been to linux-embedded and linux-kernel.
> > If you want to alert the linux-arm-kernel people to the fact that a
> > discussion is going on in this area, then feel free to post a
> > notification to the list with references to the relevant lists. There is
> > no reason why public lists should have to dig in to private archives to
> > try and play catch up.
> 
> I'm not asking anyone to do that.  Just review the patches posted to the list of
> your choice.  Or, don't review them.  Up to you.
> 
> My next update will be the one where I formally request a review with intent to
> merge into mainline.  That one will go to linux-embedded only, with
> notifications sent elsewhere as indicated per community request.  I don't have a
> problem with that.  I can't change history, but I'm doing what you are asking of
> me otherwise.

For a formally request for review, you do want to CC lkml.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: [RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Hans-Christian Egtvedt]

On Wed,  8 Oct 2008 11:43:17 -0500
Bill Gatliff <bgat@billgatliff.com> wrote:

<snipp>

> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index a726f3b..cdea0bb 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -13,15 +13,6 @@ menuconfig MISC_DEVICES
>  
>  if MISC_DEVICES
>  
> -config ATMEL_PWM
> -	tristate "Atmel AT32/AT91 PWM support"
> -	depends on AVR32 || ARCH_AT91
> -	help
> -	  This option enables device driver support for the PWM
> channels
> -	  on certain Atmel prcoessors.  Pulse Width Modulation is
> used for
> -	  purposes including software controlled power-efficent
> backlights
> -	  on LCD displays, motor control, and waveform generation.
> -

You know that there are other users of ATMEL_PWM ?

There is a atmel-pwm-bl driver in drivers/video/backlight which could
probably be converted to pwm-bl using the new pwm framework.

>  config ATMEL_TCLIB
>  	bool "Atmel AT32/AT91 Timer/Counter Library"
>  	depends on (AVR32 || ARCH_AT91)

<snipp>

-- 
Best regards,
Hans-Christian Egtvedt

Re: [RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Haavard Skinnemoen]

Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:
> You know that there are other users of ATMEL_PWM ?
> 
> There is a atmel-pwm-bl driver in drivers/video/backlight which could
> probably be converted to pwm-bl using the new pwm framework.

There's already a pwm_bl driver there which I'm assuming is using the
generic pwm API. Hopefully, we can simply delete the atmel-pwm-bl
driver. Maybe some board code needs updating before that can happen
though.

Haavard

Re: [RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Hans-Christian Egtvedt]

On Thu, 9 Oct 2008 14:16:01 +0200
Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:

> Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:
> > You know that there are other users of ATMEL_PWM ?
> > 
> > There is a atmel-pwm-bl driver in drivers/video/backlight which
> > could probably be converted to pwm-bl using the new pwm framework.
> 
> There's already a pwm_bl driver there which I'm assuming is using the
> generic pwm API. Hopefully, we can simply delete the atmel-pwm-bl
> driver. Maybe some board code needs updating before that can happen
> though.
> 

That would be the Favr-32 board, since the atmel-pwm-bl driver was
developed for that board.

Weird how this stuff almost was inserted almost in parallel.

-- 
Best regards,
Hans-Christian Egtvedt

Re: [RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Bill Gatliff]

Hans-Christian Egtvedt wrote:
> On Thu, 9 Oct 2008 14:16:01 +0200
> Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:
> 
>> Hans-Christian Egtvedt <hans-christian.egtvedt@atmel.com> wrote:
>>> You know that there are other users of ATMEL_PWM ?
>>>
>>> There is a atmel-pwm-bl driver in drivers/video/backlight which
>>> could probably be converted to pwm-bl using the new pwm framework.
>> There's already a pwm_bl driver there which I'm assuming is using the
>> generic pwm API. Hopefully, we can simply delete the atmel-pwm-bl
>> driver. Maybe some board code needs updating before that can happen
>> though.

Yes.  I think atmel-pwm-bl goes away completely, and video/backlight/pwm_bl.c
gets modified (slightly) to use the new PWM API and becomes the general
backlight-via-PWM-peripheral solution.  After that, anyone who wants to write a
backend pwm_device driver suddenly can use their peripheral for backlights (and
other stuff) out-of-the-box.

I just glanced over drivers/video/backlight, it looks like there are a couple
other implementations that could move over to Generic PWM as well.

> That would be the Favr-32 board, since the atmel-pwm-bl driver was
> developed for that board.
> 
> Weird how this stuff almost was inserted almost in parallel.

Indeed.  But also a bit tragic--- some wasted effort.  :(

Not entirely weird, however, since I try to follow AVR32 as closely as I do ARM.
 I have an NGW100 and an STK1000, in addition to about five different ARM
machines including ones based on the AT91SAM9263.  These common frameworks help
me slide into insanity a little more slowly.  :)

(I have some PPC and MIPS boards, too.  Occupational hazard).



b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Bill Gatliff]

Hans-Christian Egtvedt wrote:

> You know that there are other users of ATMEL_PWM ?
> 
> There is a atmel-pwm-bl driver in drivers/video/backlight which could
> probably be converted to pwm-bl using the new pwm framework.

Yes, I'm aware of atmel-pwm-bl, and it's on my list--- along with adapting the
led-pwm stuff I posted to use the led-blink API.


b.g.
-- 
Bill Gatliff
bgat@billgatliff.com

Re: [RFC 5/6] [PWM] Install new Atmel PWMC driver in Kconfig, expunge old one

[Hans-Christian Egtvedt]

On Thu, 09 Oct 2008 08:40:47 -0500
Bill Gatliff <bgat@billgatliff.com> wrote:

> Hans-Christian Egtvedt wrote:
> 
> > You know that there are other users of ATMEL_PWM ?
> > 
> > There is a atmel-pwm-bl driver in drivers/video/backlight which
> > could probably be converted to pwm-bl using the new pwm framework.
> 
> Yes, I'm aware of atmel-pwm-bl, and it's on my list--- along with
> adapting the led-pwm stuff I posted to use the led-blink API.
> 

Great, let me know if you want me to test something, I have hardware
laying around here which I can test it on.

-- 
Best regards,
Hans-Christian Egtvedt

Re: [RFC 1/6] [PWM] Generic PWM API implementation

[Marc Pignat]

Cleanup implementation for __pwm_create_sysfs.

Signed-off-by: Marc Pignat <marc.pignat@hevs.ch>
---

Hi all!

Here is a missing piece in the generic pwm implementation.

Bill, the code is simpler that you was thinking ;)

In facts, attributes are deleted by device_unregister -> there is nothing to
worry about attributes (un-)successful creation.

Finding which device was created is also trivial (thanks to class_find_device).

Best regards

Marc

diff --git a/drivers/pwm/pwm.c b/drivers/pwm/pwm.c
index 2f28c20..391f820 100644
--- a/drivers/pwm/pwm.c
+++ b/drivers/pwm/pwm.c
@@ -626,15 +626,23 @@ static int __pwm_create_sysfs(struct pwm_device *pwm)
 			goto err_dev_create;
 		ret = sysfs_create_group(&dev->kobj, &pwm_device_attr_group);
 		if (ret)
-			goto err_dev_group_create;
+			goto err_dev_create;
 	}
 
 	return ret;
 
-err_dev_group_create:
 err_dev_create:
-	/* TODO: undo all the successful device_create calls */
-	return -ENODEV;
+	for (wchan = 0; wchan < pwm->nchan; wchan++) {
+		dev = class_find_device(&pwm_class, NULL,
+					&pwm->channels[wchan],
+					__match_device);
+		if (dev) {
+			put_device(dev);
+			device_unregister(dev);
+		}
+	}
+
+	return ret;
 }