Skip to contentSkip to conversation

Generic PWM API implementation

PATCH In review Bill Gatliff 6 participants 41 messages Oct 15, 2008 – Nov 28, 2009

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

cover Generic PWM API implementation

This series implements a Generic PWM Device API, including reference implementations for the Atmel PWMC device, an LED device, and an LED trigger. It is based on linux-2.6.27.

This 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.

The code included in this patch series draws heavily from the existing PWM infrastructure and driver for the AT91SAM9263 PWMC. The author is grateful to Russell King, Eric Miao, David Brownell and many others for providing such tall "shoulders" to stand upon. Modifications to their code as described in this changeset should not be interpreted as attempts to address shortcomings, but rather to extend functionality in ways that were not originally required.

The implementation of the Generic PWM Device API is structurally similar to the generic GPIO API, except that the PWM code uses platform bus_id strings instead of integers to identify target deviices. A configuration structure is also provided, both to facilitate atomic hardware state changes and so that the API can be extended in a source-code-compatible way to accomodate devices with features not anticipated by the current code.

Pulse width modulated signals are used in an astounding number and range of applications, and there is no "one true way" of either realizing them or employing them to accomplish real work. The review process revealed many use cases to the author, but countless others undoubtedly remain. The current API attempts to provide a useful feature set for the most basic users, packaged in such a way as to allow the API to be extended in a backwards-compatible way as new needs are identified.

The proposed code has been run-tested on a Cogent CSB737 (AT91SAM9263), mated to a custom circuit that drives multiple DC motors and sensors.

Feedback is welcome!

b.g.
--
Bill Gatliff
<[email protected]>

---

Bill Gatliff (6):
[PWM] Generic PWM API implementation
[PWM] Changes to existing include/linux/pwm.h to adapt to generic PWM
API
[PWM] Documentation
[PWM] Driver for Atmel PWMC peripheral
[PWM] Install new Atmel PWMC driver in Kconfig, expunge old one [PWM] New LED driver and trigger that use PWM API

 Documentation/pwm.txt      |  258 +++++++++++++++++
 arch/arm/Kconfig           |    2 +
 drivers/Makefile           |    2 +
 drivers/leds/Kconfig       |   24 ++-
 drivers/leds/Makefile      |    2 +
 drivers/leds/leds-pwm.c    |  167 +++++++++++
 drivers/leds/ledtrig-dim.c |   95 ++++++
 drivers/misc/Kconfig       |    9 -
 drivers/misc/Makefile      |    1 -
 drivers/misc/atmel_pwm.c   |  409 --------------------------
 drivers/pwm/Kconfig        |   24 ++
 drivers/pwm/Makefile       |    6 +
 drivers/pwm/atmel-pwm.c    |  633 ++++++++++++++++++++++++++++++++++++++++
 drivers/pwm/pwm.c          |  681 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pwm-led.h    |   34 +++
 include/linux/pwm.h        |  172 ++++++++++--
 16 files changed, 2074 insertions(+), 445 deletions(-)
 create mode 100644 Documentation/pwm.txt
 create mode 100644 drivers/leds/leds-pwm.c
 create mode 100644 drivers/leds/ledtrig-dim.c
 delete mode 100644 drivers/misc/atmel_pwm.c
 create mode 100644 drivers/pwm/Kconfig
 create mode 100644 drivers/pwm/Makefile
 create mode 100644 drivers/pwm/atmel-pwm.c
 create mode 100644 drivers/pwm/pwm.c
 create mode 100644 include/linux/pwm-led.h
#1 lore raw

PATCH 1/6 Generic PWM API implementation

An API that consolidates PWM-capable devices behind a common interface.

This code defines a pwm_device structure, which identifies a PWM-capable platform device having one or more output channels of type pwm_channel. The pwm_register() function maintains a list of pwm_devices; the pwm_request() and pwm_free() functions allocate and un-allocate PWM channels on user request.

The pwm_config_nosleep() and pwm_config() functions are the main entry points into the PWM device driver itself. The former may be called from contexts that are not allowed to sleep, i.e. interrupt handlers, and will return an error if the requested configuration change cannot be performed by the hardware without sleeping; the latter may be called everywhere else.

PWM drivers might sleep because some hardware makes it difficult to update the period, duty cycle or polarity of an output while the peripheral is running. The Atmel PWMC peripheral is one example: the duty cycle and period control registers share a single buffer register, but some updates require changes to both registers and/or registers that aren't buffered. When that is required, you must stop the peripheral at the end of a PWM period in order to make the state change. The Atmel PWMC driver implements the stop by sleeping until the end of the period.

An alternative implementation is to simply brute-force stop the PWM, update the peripheral control registers, and then restart the hardware. That approach is fine if you don't care about glitches in the PWM output. The Atmel PWMC driver tries to be more elegant, at the expense of some complexity and occasional conflict with sleep-prohibited contexts.

If you don't need to sleep or you don't care about glitch-free operation, then put everything into pwm_config_nosleep() and redirect pwm_config() there.

There are various helper functions that build the pwm_channel_config structure for simple updates, like just modifying the duty cycle percentage. These should probably be inlined, but aren't.

PWM channels can be "synchronized". As defined by this API, synchronized PWM channels all start, stop and restart together when any channel in the group is started, stopped, or restarted. The Atmel PWMC driver emulates this behavior; other hardware might support it more explicitly. There is currently no implementation to synchronize channels across PWM devices, but the API will allow for that if someone wants to write the code.
Hint: there will be a measurable delay between starting and stopping the various channels in a cross-device synchronization scenario, because you'll have to start and stop each channel individually.

Finally, the pwm_set_handler() function registers a callback function that the API will invoke in a worker thread at the end of the PWM period.

---
 drivers/pwm/pwm.c |  681 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 681 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/pwm.c
drivers/pwm/pwm.c new file +681 -0
new file mode 100644
index 0000000..5009de9
@@ -0,0 +1,681 @@
+/*
+ * 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_set_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_set_period_ns);
+
+
+unsigned long pwm_get_period_ns(struct pwm_channel *p)
+{
+	return pwm_ticks_to_ns(p, p->period_ticks);
+}
+EXPORT_SYMBOL(pwm_get_period_ns);
+
+
+int pwm_set_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_set_duty_ns);
+
+
+unsigned long pwm_get_duty_ns(struct pwm_channel *p)
+{
+	return pwm_ticks_to_ns(p, p->duty_ticks);
+}
+EXPORT_SYMBOL(pwm_get_duty_ns);
+
+
+int pwm_set_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_set_duty_percent);
+
+
+int pwm_set_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_set_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_get_duty_ns(p));
+}
+
+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_set_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_get_period_ns(p));
+}
+
+
+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_set_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_set_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);
Signature · 1 line
1.5.6.5
#2 lore raw

drivers/pwm/pwm.c line 44

Quoted 5 lines · Bill Gatliff
drivers/pwm/pwm.c new file +5 -0
+int pwm_register(struct pwm_device *pwm)
+{
+       struct pwm_channel *p;
+       int wchan;
+       int ret = 0;

the initialization to 0 here isnt needed
-mike

#3 lore raw
Quoted 8 lines · Mike Frysinger

drivers/pwm/pwm.c line 44

Quoted 5 lines · Bill Gatliff
drivers/pwm/pwm.c new file +5 -0
+int pwm_register(struct pwm_device *pwm)
+{
+       struct pwm_channel *p;
+       int wchan;
+       int ret = 0;

the initialization to 0 here isnt needed

This is literally the only feedback I have received. I take that to mean that the code basically works as advertised, and nobody else has any problems with it (it continues to work fine for me here).

So, what's the next step? How do I advocate for getting this API into mainline?

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#4 lore raw
Quoted 10 lines · Bill Gatliff
Quoted 8 lines · Mike Frysinger

drivers/pwm/pwm.c line 44

Quoted 5 lines · Bill Gatliff
drivers/pwm/pwm.c new file +5 -0
+int pwm_register(struct pwm_device *pwm)
+{
+       struct pwm_channel *p;
+       int wchan;
+       int ret = 0;

the initialization to 0 here isnt needed

Quoted 5 lines

This is literally the only feedback I have received. I take that to mean that the code basically works as advertised, and nobody else has any problems with it (it continues to work fine for me here).

So, what's the next step? How do I advocate for getting this API into mainline?

you looking to be maintainer ? if so, i imagine you start a git tree (on kernel.org?) for people to pull from, post patches to mainline now and indicate you'd ask for a 2.6.29 merge ...
-mike

#5 lore raw
In reply to Bill Gatliff
Quoted 1 line · Bill Gatliff

This is literally the only feedback I have received.

I was waiting for followup too. I finally get onto this list (for some reason the list management software dropped several previous "please add me" requests) and ... nothing!!

Patches #1 and #2 seem to be in the wrong order ... and #2 shouldn't give the header's full pathname. I'd have merged the two, myself.

Patch #6 seems to remove LEDS_ATMEL_PWM from Kconfig but leave the driver around ... and restore the now-deleted LEDS_CORGI support (leds-gpio suffices). If you're going to remove LEDS_ATMEL_PWM you should update the code in mach-at91/leds.c which sets up for that driver...

Quoted 2 lines

So, what's the next step?  How do I advocate for getting this API into mainline?

The number of backing implementations seemed a bit low ... just one. And that's switching Atmel's dedicated PWM, but not the other PWM driver in the tree (for PXA); so it's not terrifically accessible.

And even for Atmel's SOC's it seemed a bit light: the TC blocks will do PWM quite happily, and are found on many more chips than the dedicated PWM hardware.

My rule of thumb is that it's not worth considering a framework as being general enough until it's got three fairly different backing implementations. Two is enough to consider merging, especially with complicated drivers.

Plus the number of framework users is an issue. The PWM led driver is a good demo for two basic features, and it's useful: brightness (duty cycle beyond visual perception) and blink rate (duty cycle easily seen with the eye). But it's still only really one API customer, even given that new LED trigger...

So I'd suggest fleshing it out a bit more, so it works on some non-Atmel hardware as well as more Atmel SOCs. And fix the goofs I noted above. :)

- Dave

#6 lore raw
Quoted 9 lines · David Brownell
Quoted 2 lines · Bill Gatliff

So, what's the next step? How do I advocate for getting this API into mainline?

The number of backing implementations seemed a bit low ... just one. And that's switching Atmel's dedicated PWM, but not the other PWM driver in the tree (for PXA); so it's not terrifically accessible.

Quoted 4 lines

My rule of thumb is that it's not worth considering a framework as being general enough until it's got three fairly different backing implementations. Two is enough to consider merging, especially with complicated drivers.

every Blackfin processor so far has had dedicated PWM hardware in it, so a backend driver for that arch would show up ...

Quoted 6 lines

Plus the number of framework users is an issue. The PWM led driver is a good demo for two basic features, and it's useful: brightness (duty cycle beyond visual perception) and blink rate (duty cycle easily seen with the eye). But it's still only really one API customer, even given that new LED trigger...

there is an lcd driver or two in the tree (probably Blackfin specific) that uses the PWM hardware as an output to drive the hsync/vsync signals.

and while LIRC is still out-of-tree for mainline, i wrote a driver using the PWM hardware as an input ... but there wasnt anything else Blackfin specific in said driver other than the PWM stuff ...

i'm guessing you'd prefer to see those implemented before moving to mainline though ?
-mike

#7 lore raw
Quoted 2 lines · Mike Frysinger

every Blackfin processor so far has had dedicated PWM hardware in it, so a backend driver for that arch would show up ...

From who? I don't have that hardware. I'm happy to hear that someone is working on it, though, and I would be happy to help review.

Quoted 2 lines

i'm guessing you'd prefer to see those implemented before moving to mainline though ?

Sounds like if we can get a git tree on kernel.org, then the first stop would be my pulling the code into the tree, and then asking Linus to do a pull.

To which is response will probably be: "Bill who?" :)

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#8 lore raw
In reply to Mike Frysinger
Quoted 10 lines · Mike Frysinger
Quoted 9 lines · David Brownell
Quoted 2 lines · Bill Gatliff

So, what's the next step? How do I advocate for getting this API into mainline?

The number of backing implementations seemed a bit low ... just one. And that's switching Atmel's dedicated PWM, but not the other PWM driver in the tree (for PXA); so it's not terrifically accessible.

Quoted 7 lines
Quoted 4 lines

My rule of thumb is that it's not worth considering a framework as being general enough until it's got three fairly different backing implementations. Two is enough to consider merging, especially with complicated drivers.

every Blackfin processor so far has had dedicated PWM hardware in it, so a backend driver for that arch would show up ...

I'm not questioning the notion that more generic PWM support would be useful, note! Just pointing out my rule of thumb, which has proven to be a good one.

A variety of implementations shakes out portability issues, and highlights (what *WE* already know!) that this stuff is actually generally useful. And when there's no test harness, having most of a 3-providers vs 3-users matrix known to work is a good stand-in.

Quoted 5 lines
Quoted 1 line

Plus the number of framework users is an issue. ...

there is an lcd driver or two in the tree (probably Blackfin specific) that uses the PWM hardware as an output to drive the hsync/vsync signals.

... Right, but not through *this* framework. The proof that the framework is sufficient for that application too would be the code. And hsync/vsync seems like a usefully "different" app context to me. Certainly not one that came quickly to my mind!

But as we know, PWM gets used in all kinds of ways. :)

Quoted 6 lines

and while LIRC is still out-of-tree for mainline, i wrote a driver using the PWM hardware as an input ... but there wasnt anything else Blackfin specific in said driver other than the PWM stuff ...

i'm guessing you'd prefer to see those implemented before moving to mainline though ?

Well, more than just Bill's single implementation and client, yes. Maybe the Blackfin PWM provider, and one more API client ... though more of each would be fine too.

I'd imagine Bill would be glad to have more active hands on the code too ... a few issues always seem to turn up with the first users, and it's better to sort them out with "early adopters" before the floodgates open.

Plus: more users provides some evidence to folk like Andrew and Linus that the code is useful enough to merge. Embedded Linux isn't as visible to them as x86.

- Dave

#9 lore raw
In reply to David Brownell
Quoted 4 lines · David Brownell

Patches #1 and #2 seem to be in the wrong order ... and #2 shouldn't give the header's full pathname. I'd have merged the two, myself.

Ok, that's good feedback.

Quoted 5 lines

Patch #6 seems to remove LEDS_ATMEL_PWM from Kconfig but leave the driver around ... and restore the now-deleted LEDS_CORGI support (leds-gpio suffices). If you're going to remove LEDS_ATMEL_PWM you should update the code in mach-at91/leds.c which sets up for that driver...

I'll look into this before my next update.

Quoted 8 lines

The number of backing implementations seemed a bit low ... just one. And that's switching Atmel's dedicated PWM, but not the other PWM driver in the tree (for PXA); so it's not terrifically accessible.

And even for Atmel's SOC's it seemed a bit light: the TC blocks will do PWM quite happily, and are found on many more chips than the dedicated PWM hardware.

Alright. The SAM9263 has both PWMC and TC, so it's pretty easy to add TC's to the list of backing implementations because that hardware is already plugged in.
:) And I have PXA hardware here somewhere too, so I'll bring that one over as
well. Finally, I suddenly have the need to do bit-bang PWM over GPIO, so if nobody beats me to it (please, someone beat me to it!) then I'll implement that one as well.

Quoted 6 lines

Plus the number of framework users is an issue. The PWM led driver is a good demo for two basic features, and it's useful: brightness (duty cycle beyond visual perception) and blink rate (duty cycle easily seen with the eye). But it's still only really one API customer, even given that new LED trigger...

That's a bit of a catch-22. The whole point of the API is to provide a framework for users to adopt, where previously there was none. Adopters are more likely if the framework is mainlined.

Granted, having a git tree on kernel.org is pretty darned close. I'll look into that per Mike Frysinger.

Quoted 3 lines

So I'd suggest fleshing it out a bit more, so it works on some non-Atmel hardware as well as more Atmel SOCs. And fix the goofs I noted above. :)

Roger that. And then I expect an Acked-by from you. :)

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#10 lore raw

PATCH 2/6 Changes to existing include/linux/pwm.h to adapt to generic PWM API

In reply to Bill Gatliff

Modifications to the existing include/linux/pwm.h file for the generic PWM API.

---
 include/linux/pwm.h |  172 ++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 151 insertions(+), 21 deletions(-)
include/linux/pwm.h +151 -21
index 3945f80..1a4308e 100644
@@ -1,31 +1,161 @@
 #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_set_period_ns(struct pwm_channel *pwm,
+		      unsigned long period_ns);
+
+unsigned long int pwm_get_period_ns(struct pwm_channel *pwm);
+
+int pwm_set_duty_ns(struct pwm_channel *pwm,
+		    unsigned long duty_ns);
+
+int pwm_set_duty_percent(struct pwm_channel *pwm,
+			 int percent);
+
+unsigned long pwm_get_duty_ns(struct pwm_channel *pwm);
+
+int pwm_set_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 */
Signature · 1 line
1.5.6.5
#11 lore raw

PATCH 3/6 Documentation

In reply to Bill Gatliff
---
 Documentation/pwm.txt |  258 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 258 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/pwm.txt
Documentation/pwm.txt new file +258 -0
new file mode 100644
index 0000000..b8932dd
@@ -0,0 +1,258 @@
+                       Generic PWM Device API
+
+                          October 8, 2008
+                           Bill Gatliff
+                       <[email protected]>
+
+
+
+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.  ;)
+
Signature · 1 line
1.5.6.5
#12 lore raw

PATCH 4/6 Driver for Atmel PWMC peripheral

In reply to Bill Gatliff

This driver binds the Atmel PWMC platform device to the Generic PWM Device API.

The config_nosleep() function changes only the peripheral states that do not require a sleep in order to avoid a spurious output; other state changes are in config(), which calls stop_sync() to wait for the end of the current PWM period. Note that the PWMC hardware has limited
double-buffering: within some constraints, you can change either the period OR the duty cycle with the hardware active, and the hardware will commit the register update at the end of the current PWM period.

---
 drivers/pwm/atmel-pwm.c |  633 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 633 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/atmel-pwm.c
drivers/pwm/atmel-pwm.c new file +633 -0
new file mode 100644
index 0000000..c7ec676
@@ -0,0 +1,633 @@
+/*
+ * 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;
+
+	if (p->pwm->config_nosleep) {
+		if (!p->pwm->config_nosleep(p, c))
+			return 0;
+	}
+
+	might_sleep();
+
+	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, "pwm_clk");
+	if (IS_ERR(ap->clk)) {
+		pr_info("%s: clk_get error %ld\n",
+			ap->pwm.bus_id, PTR_ERR(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 <[email protected]>");
+MODULE_DESCRIPTION("Driver for Atmel PWMC peripheral");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atmel_pwmc");
Signature · 1 line
1.5.6.5
#13 lore raw

PATCH 5/6 Install new Atmel PWMC driver in Kconfig, expunge old one

In reply to Bill Gatliff
---
 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
arch/arm/Kconfig +2 -0
index 4853f9d..80c09fa 100644
@@ -1228,6 +1228,8 @@ source "drivers/spi/Kconfig"
 
 source "drivers/gpio/Kconfig"
 
+source "drivers/pwm/Kconfig"
+
 source "drivers/w1/Kconfig"
 
 source "drivers/power/Kconfig"
drivers/Makefile +2 -0
index 2735bde..f242fc6 100644
@@ -6,6 +6,8 @@
 #
 
 obj-y				+= gpio/
+obj-$(CONFIG_GENERIC_PWM)	+= pwm/
+
 obj-$(CONFIG_PCI)		+= pci/
 obj-$(CONFIG_PARISC)		+= parisc/
 obj-$(CONFIG_RAPIDIO)		+= rapidio/
drivers/misc/Kconfig +0 -9
index a726f3b..cdea0bb 100644
@@ -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)
drivers/misc/Makefile +0 -1
index c6c13f6..9e67012 100644
@@ -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
drivers/misc/atmel_pwm.c deleted +0 -409

409 lines not on this page · raw message

drivers/pwm/Kconfig new file +24 -0
new file mode 100644
index 0000000..933bb2c
@@ -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
drivers/pwm/Makefile new file +6 -0
new file mode 100644
index 0000000..21634f6
@@ -0,0 +1,6 @@
+#
+# Makefile for pwm devices
+#
+obj-y := pwm.o
+
+obj-$(CONFIG_ATMEL_PWM)		+= atmel-pwm.o
Signature · 1 line
1.5.6.5
#14 lore raw

PATCH 6/6 New LED driver and trigger that use PWM API

In reply to Bill Gatliff

The leds-pwm driver maps LED API calls into PWM API calls. Both brighness_get/set and blink_set are supported.

Some LED API entry points are not sleep-safe, which will cause problems with PWM implementations that require sleeping during hardware state updates. The Atmel PWMC driver avoids these cases; other driver implementations must be aware.

The ledtrig-dim trigger maps current system load to an LED brighness of 0-100% for system loads in the range 0-1 as reported by avenrun[]. If the LED associated with the trigger is a suitable PWM output, then the user will see the brightness of the LED change as system load changes.

---
 drivers/leds/Kconfig       |   24 +++++--
 drivers/leds/Makefile      |    2 +
 drivers/leds/leds-pwm.c    |  167 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/leds/ledtrig-dim.c |   95 +++++++++++++++++++++++++
 include/linux/pwm-led.h    |   34 +++++++++
 5 files changed, 317 insertions(+), 5 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
drivers/leds/Kconfig +19 -5
index e3e4042..3339a4c 100644
@@ -17,12 +17,12 @@ config LEDS_CLASS
 
 comment "LED drivers"
 
-config LEDS_ATMEL_PWM
-	tristate "LED Support using Atmel PWM outputs"
-	depends on LEDS_CLASS && ATMEL_PWM
+config LEDS_CORGI
+	tristate "LED Support for the Sharp SL-C7x0 series"
+	depends on LEDS_CLASS && PXA_SHARP_C7xx
 	help
-	  This option enables support for LEDs driven using outputs
-	  of the dedicated PWM controller found on newer Atmel SOCs.
+	  This option enables support for the LEDs on Sharp Zaurus
+	  SL-C7x0 series (C700, C750, C760, C860).
 
 config LEDS_LOCOMO
 	tristate "LED Support for Locomo device"
@@ -113,6 +113,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
@@ -184,6 +190,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
drivers/leds/Makefile +2 -0
index eb186c3..3a16e37 100644
@@ -17,6 +17,7 @@ obj-$(CONFIG_LEDS_COBALT_RAQ)		+= leds-cobalt-raq.o
 obj-$(CONFIG_LEDS_SUNFIRE)		+= leds-sunfire.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
@@ -26,5 +27,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
drivers/leds/leds-pwm.c new file +167 -0

167 lines not on this page · raw message

drivers/leds/ledtrig-dim.c new file +95 -0

95 lines not on this page · raw message

include/linux/pwm-led.h new file +34 -0
new file mode 100644
index 0000000..92363c8
@@ -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 */
Signature · 1 line
1.5.6.5
#15 lore raw
In reply to Bill Gatliff

Hi Bill

Quoted 3 lines · Bill Gatliff

This series implements a Generic PWM Device API, including reference implementations for the Atmel PWMC device, an LED device, and an LED trigger.  It is based on linux-2.6.27.

[...]

Quoted 7 lines

The implementation of the Generic PWM Device API is structurally similar to the generic GPIO API, except that the PWM code uses platform bus_id strings instead of integers to identify target deviices.  A configuration structure is also provided, both to facilitate atomic hardware state changes and so that the API can be extended in a source-code-compatible way to accomodate devices with features not anticipated by the current code.

Hey Bill,

I'm concerned about the approach taken here. As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines. The difference being that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate). Actually, many GPIOs have these properties too. I've got a part with output-only gpios, and gpio devices that also have a PWM.

What is the reason for bringing in an entirely new framework instead of extending the GPIO API or gpiolib? I'm not too excited about having two entirely different frameworks for what basically boils down to "numbered signal pins".

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#16 lore raw
Quoted 21 lines · Grant Likely
Quoted 3 lines · Bill Gatliff

This series implements a Generic PWM Device API, including reference implementations for the Atmel PWMC device, an LED device, and an LED trigger.  It is based on linux-2.6.27.

[...]

Quoted 7 lines

The implementation of the Generic PWM Device API is structurally similar to the generic GPIO API, except that the PWM code uses platform bus_id strings instead of integers to identify target deviices.  A configuration structure is also provided, both to facilitate atomic hardware state changes and so that the API can be extended in a source-code-compatible way to accomodate devices with features not anticipated by the current code.

I'm concerned about the approach taken here.  As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines.  The difference being that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate).  Actually, many GPIOs have these properties too.  I've got a part with output-only gpios, and gpio devices that also have a PWM.

Quoted 4 lines

What is the reason for bringing in an entirely new framework instead of extending the GPIO API or gpiolib?  I'm not too excited about having two entirely different frameworks for what basically boils down to "numbered signal pins".

unifying resource management obviously makes sense so as to avoid conflicts, but i dont think the fact that one pin can be multi purpose means it should be entirely forced into the GPIO framework, nor do i see any real gain for doing so.
-mike

#17 lore raw
Quoted 27 lines · Mike Frysinger
Quoted 21 lines · Grant Likely
Quoted 3 lines · Bill Gatliff

This series implements a Generic PWM Device API, including reference implementations for the Atmel PWMC device, an LED device, and an LED trigger.  It is based on linux-2.6.27.

[...]

Quoted 7 lines

The implementation of the Generic PWM Device API is structurally similar to the generic GPIO API, except that the PWM code uses platform bus_id strings instead of integers to identify target deviices.  A configuration structure is also provided, both to facilitate atomic hardware state changes and so that the API can be extended in a source-code-compatible way to accomodate devices with features not anticipated by the current code.

I'm concerned about the approach taken here.  As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines.  The difference being that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate).  Actually, many GPIOs have these properties too.  I've got a part with output-only gpios, and gpio devices that also have a PWM.

Quoted 4 lines

What is the reason for bringing in an entirely new framework instead of extending the GPIO API or gpiolib?  I'm not too excited about having two entirely different frameworks for what basically boils down to "numbered signal pins".

Quoted 4 lines

unifying resource management obviously makes sense so as to avoid conflicts, but i dont think the fact that one pin can be multi purpose means it should be entirely forced into the GPIO framework, nor do i see any real gain for doing so.

Common code is a big gain in and of itself. It means less to develop, less to maintain, and fewer APIs in the kernel. Right now, I don't see a fundamental difference is between GPIO and PWM pin management. It is essentially the same problem, and in many cases PWM pins can also be used as GPIOs. I think the question should be flipped around; rather than asking for a compelling reason for them to be merged; I want to know the compelling reason to keep them separate. What is the fundamental difference that keeps them apart?

However, since it was mentioned, I do see some real gains for using the same infrastructure:
- Devices using GPIO pins can easily be extended to take advantage of PWM modes. i could see GPIO LEDs taking advantage of this for example
- (as I already mentioned) PWM pins that can also behave as GPIOs don't need to register 2 interfaces.
- All the existing support code for hooking up GPIO pins to other devices can be reused as is.
- Individual platforms have the option of implementing the GPIO+PWM API directly (fast, but static), or they can hook in via GPIOLIB (dynamic; slower but pluggable)
- All the OF device tree bindings for GPIOs also work with PWMs.

What I would like to see is the PWM functions added to the GPIO API. GPIO drivers can then either implement them or not. If a GPIO driver supports the PWM function, great. If not, then it returns -EINVAL. Heck, I'll even got a driver right now that I'd use it with. I'm more than happy to help code it up even.

Cheers,
g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#18 lore raw
Quoted 4 lines · Grant Likely

Right now, I don't
see a fundamental difference is between GPIO and PWM pin management. It is essentially the same problem, and in many cases PWM pins can also be used as GPIOs.

Pin management for a given SoC is going to be relevant to setting every signal, no matter what peripheral it's associated with. The same argument applies to an MDIO bus, I2C, 1-wire, and more.

And I don't buy it in those cases either.

Quoted 4 lines

I think the question should be flipped around;
rather than asking for a compelling reason for them to be merged; I want to know the compelling reason to keep them separate. What is the fundamental difference that keeps them apart?

PWM is about periodic signal generation without CPU intervention.

GPIO is about explicit CPU management/interrogation of single signals.

Quoted 1 line

What I would like to see is the PWM functions added to the GPIO API.

No. If you want a pin mux interface, come up with one of them.

It shouldn't be a PWM interface, a GPIO interface, an I2C interface, a SPI interface, an MDIO interface, a 1-wire interface ... or any of dozens of other things. It'd be purely for pinmux.

#19 lore raw
Quoted 3 lines · David Brownell

It'd be purely for pinmux.

Ugh. That would be a tough interface to design. It makes me think of an old-time telephone switchboard, with an undefined number of wires and an equally-undefined number of plugs to insert them into.

"Good morning, Mabel! Give me SCL1 on AA25 please! No, I don't know who SCL1 is nor where AA25 is. I'll be waiting."

:)

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#20 lore raw
Quoted 6 lines · Bill Gatliff
Quoted 3 lines · David Brownell

It'd be purely for pinmux.

Ugh. That would be a tough interface to design.

True. That's part of why I object to wanting to combine it with GPIOs ... or, combine everything else with GPIOs too (like PWMs).

But Grant talks about wanting such things, and if he can deliver it, more power to him.

Quoted 6 lines

It makes me think of
an old-time telephone switchboard, with an undefined number of wires and an equally-undefined number of plugs to insert them into.

"Good morning, Mabel! Give me SCL1 on AA25 please! No, I don't know who SCL1 is nor where AA25 is. I'll be waiting."

"What? You're saying that I've got to take three other connections at the same time? And cut these other two calls? No, I only wanted to make that one call ..."

Quoted 6 lines

:)

#21 lore raw
Quoted 6 lines · David Brownell
Quoted 5 lines · Bill Gatliff
Quoted 3 lines · David Brownell

It'd be purely for pinmux.

Quoted 8 lines
Quoted 1 line

Ugh.  That would be a tough interface to design.

True.  That's part of why I object to wanting to combine it with GPIOs ... or, combine everything else with GPIOs too (like PWMs).

But Grant talks about wanting such things, and if he can deliver it, more power to him.

Just to be clear, I'm *not* talking about pin mux. I fully agree that is a different problem.

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#22 lore raw
In reply to David Brownell
Quoted 18 lines · David Brownell
Quoted 4 lines · Grant Likely

                Right now, I don't
see a fundamental difference is between GPIO and PWM pin management. It is essentially the same problem, and in many cases PWM pins can also be used as GPIOs.

Pin management for a given SoC is going to be relevant to setting every signal, no matter what peripheral it's associated with.  The same argument applies to an MDIO bus, I2C, 1-wire, and more.

And I don't buy it in those cases either.

Quoted 4 lines

I think the question should be flipped around;
rather than asking for a compelling reason for them to be merged; I want to know the compelling reason to keep them separate.  What is the fundamental difference that keeps them apart?

Quoted 4 lines

PWM is about periodic signal generation without CPU intervention.

GPIO is about explicit CPU management/interrogation of single signals.

Can also be viewed from the perspective: It is about putting a pin into a particular state until I explicitly tell you to change it. Whether that state be a GPIO input, a GPIO high, a GPIO low, or a PWM periodic.

Quoted 7 lines
Quoted 1 line

What I would like to see is the PWM functions added to the GPIO API.

No.  If you want a pin mux interface, come up with one of them.

It shouldn't be a PWM interface, a GPIO interface, an I2C interface, a SPI interface, an MDIO interface, a 1-wire interface ... or any of dozens of other things.  It'd be purely for pinmux.

I'm not talking about a pin mux interface. I'm talking about discrete controllable entities. I agree that pin muxing is an entirely different scope. I'm also not talking about layers on top of the GPIO. I'm talking about the management code to obtain a reference to the pin your interested in. There is a non-trivial amount of code associated with getting a reference to a pin and the behaviour required is largely identical between GPIO and PWM. I don't want to see a new subsystem that largely does the exact same job, but is different in subtle ways. I think it should either be a unified PWM/GPIO pin management subsystem, or a common library used by each.

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#23 lore raw
Quoted 1 line · Grant Likely

I'm talking about discrete controllable entities.

At the extreme, I see discrete, single-pin GPIO as being a degenerate case of PWM: only 0% or 100% duty cycle e.g. one-bit granularity, a single output, and no dependencies with any other channels or pins. But the perspectives of the two groups of users are completely different, so I don't see any advantage to having a combined user-facing API.

Quoted 4 lines

I'm talking about the management code to obtain a reference to the pin your interested in. There is a non-trivial amount of code associated with getting a reference to a pin and the behaviour required is largely identical between GPIO and PWM.

True, but anything involving pins has to be dealt with at the platform level. And the general approach for that so far is that the board's startup code sets the pins how it wants them, and the peripheral drivers more or less assume that things are as they should be when it comes time to activate the peripherals themselves. Anything more sophisticated than that and you prevent the kind of reuse that's taking place now between AVR32 and AT91, for example.

Maybe that's an opportunity for improvement: an API for pin reservation, that GPIO, PWM and other platform-oriented drivers could use to request the pins that they want. A kind of common version of the OMAP mux management code, if you will.

But I don't think such an API would be all that useful for GPIO- and PWM-related scenarios. Out of necessity, PWM and GPIO are very specific to the board hardware, and on-the-fly pin configuration changes aren't
possible: either the hardware needs that pin to be an output, or it doesn't. You can't make that decision at runtime because you'd probably have to swap out or add resistors, drivers, etc. The software to do *that* would be.... tricky. :)

Quoted 4 lines

I don't want to see a new subsystem that largely does the exact same job, but is different in subtle ways. I think it should either be a unified PWM/GPIO pin management subsystem, or a common library used by each.

Where is there overlap? My PWM code is totally pin-agnostic, at least for the drivers I've worked with so far. And I'm not aware of any GPIO chip drivers that deal with pin multiplexing, either. Not saying there aren't any, only that I haven't seen them.

It's a nice idea in the abstract, but I sure don't see how to make it work well enough in practice to be worth the effort. I'm not saying it can't be done, just that it isn't my focus. Yours? :)

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#24 lore raw
Quoted 6 lines · Bill Gatliff
Quoted 1 line · Grant Likely

I'm talking about discrete controllable entities.

At the extreme, I see discrete, single-pin GPIO as being a degenerate case of PWM: only 0% or 100% duty cycle e.g. one-bit granularity, a single output, and no dependencies with any other channels or pins.

yes

Quoted 3 lines

But the perspectives of the two groups of users are completely different, so I don't see any advantage to having a combined user-facing API.

... for the control API; yes. ie, the functions that set the GPIO pin state or the PWM duty cycle. There is no overlap here (except for the degenerate case of using a PWM as an output only GPIO... which is useful. Board designers do crazy things).

Quoted 7 lines
Quoted 4 lines

I'm talking about the management code to obtain a reference to the pin your interested in.  There is a non-trivial amount of code associated with getting a reference to a pin and the behaviour required is largely identical between GPIO and PWM.

True, but anything involving pins has to be dealt with at the platform level.

Not exactly true. GPIO pins do not have to be dealt with at the platform level when GPIO lib is used. I've got several platforms where the routing of GPIOs is specified in the device tree data, and platform code never touches them. When the driver probes a device implemented via GPIOs, it reads the data and requests the GPIO numbers that it needs.

*however* I do agree that it is the responsibility of platform code to set up chip-internal pin muxing and routing. Actually, further than that, I think it is actually firmware's responsibility to set up chip internal pin muxing because it leads to more common platform code in the kernel (less board specific fixups), but the kernel can fix it up in a pinch. But I'm not arguing about the pin (hardware) setup code.

Quoted 11 lines

 And the general approach for that so far is that the board's startup code sets the pins how it wants them, and the peripheral drivers more or less assume that things are as they should be when it comes time to activate the peripherals themselves.  Anything more sophisticated than that and you prevent the kind of reuse that's taking place now between AVR32 and AT91, for example.

Maybe that's an opportunity for improvement: an API for pin reservation, that GPIO, PWM and other platform-oriented drivers could use to request the pins that they want.  A kind of common version of the OMAP mux management code, if you will.

Perhaps. But that's not what I'm taking issue with.

Quoted 9 lines

But I don't think such an API would be all that useful for GPIO- and PWM-related scenarios.  Out of necessity, PWM and GPIO are very specific to the board hardware, and on-the-fly pin configuration changes aren't
possible: either the hardware needs that pin to be an output, or it doesn't.  You can't make that decision at runtime because you'd probably have to swap out or add resistors, drivers, etc.  The software to do *that* would be.... tricky.  :)

Quoted 8 lines
Quoted 3 lines

I don't want to see a new subsystem that largely does the exact same job, but is different in subtle ways.  I think it should either be a unified PWM/GPIO pin management subsystem, or a common library used by each.

Where is there overlap?  My PWM code is totally pin-agnostic, at least for the drivers I've worked with so far.  And I'm not aware of any GPIO chip drivers that deal with pin multiplexing, either.  Not saying there aren't any, only that I haven't seen them.

The overlap is in the management of registered pins. I do not take issue with the API. In fact, I don't take issue with the code that you've written either. It appears to be well written. I take issue with all the common code behind the API to make it work and to allow GPIOs or PWMs to be registered at runtime. The overlap is the code and behaviour used to register pins and to obtain a reference to a pin.

On the PWM side; it's the code backing pwm_register(). pwm_unregister(), pwm_request(), pwm_free(), and the sysfs hooks.

For GPIO; it's gpiochip_add(), gpiochip_remove(), gpio_request(), gpio_free(), the sysfs hooks, and the device tree bindings.

They perform exactly the same task. The difference is that PWM devices implement the PWM API and GPIO devices implement the GPIO API; but the behaviour needed to manage them is identical. I don't like the fact that it will be implemented twice with subtle differences between them.

The argument has been made that the split is appropriate because the use case between GPIO and PWM is considerably different, and to a point I agree. The PWM use case definitely requires a different API. However, the argument also makes the assumption that what is a GPIO and what is a PWM can easily be pinned down. For example, I've got output only GPIOs, input only GPIOs, and a device that implements both PWM and GPIO behaviour (not pin muxing). And, as you say, the degenerate of PWM behaviour is analogous to GPIO output. The range of behaviour supported by a PWM or GPIO device is entirely dependent on what the hardware designer chooses to implement, and there isn't a canonical definition of either GPIO or PWM. If a hard distinction is made now to manage GPIO and PWM pins separately, what then do we do with other similar-yet-different pin controllers? What about a multi-voltage output GPIO pin? Or a Frequency generator pin? Is it appropriate to create a new subsystem for managing each one? And then have devices that implement more than one common-case api to register against each subsystem?

What an API can do is capture the common use cases, but it can never capture the full range of behaviour implemented by a particular device. What I see as important to driver writers (PWM/GPIO users) is the ability to obtain and use a reference to a controllable pin. Then it can either use a common-case API, or if it needs to, call device specific extra hooks to do something funky, like hardware triggering.

For pin controller driver writers, the need is to export as much behaviour as possible via a common-case API so that the device can easily be reused by things the driver author hasn't even conceived of.
Some behaviors won't fit, and some devices don't support behaviour
exposed by the API. It is perfectly valid for a pin controller driver to only implement the ops supported by the device and use -EINVAL if an unsupported op is attempted.

I say that it is better to have a single pin-controller subsystem (and again, not talking about pin-mux, that is something different) for registering pins to, and leave it up to the drivers to choose which ops to implement. It can implement just the PWM ops, or just the GPIO ops, or both. A pin user will use the same mechanism to obtain a reference to the pin regardless of the pin type, and can use that reference to call the common case apis (gpio_get/set_value(), pwm_config(), etc), or can use the reference when calling into a driver-specific hook. When new capabilities are identified, the list of implementable ops can be extended without impact on existing drivers. Drivers can take advantage of the new behaviour by implementing the new ops.

Quoted 3 lines

It's a nice idea in the abstract, but I sure don't see how to make it work well enough in practice to be worth the effort.  I'm not saying it can't be done, just that it isn't my focus.  Yours?  :)

The problem that I have is that it shifts the maintenance burden from the time of development (ie. now) to code maintenance time. By duplicating the infrastructure it doubles the code to be maintained. It means there are differences between how a GPIO/PWM controllers register their pins, and differences in how other drivers obtain and use them. It also establishes a pattern of writing new subsystems to handle similar-yet-different pin controllers. To me that is a non-trivial cost.

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#25 lore raw

Am 23.11.09 18:39 schrieb(en) Grant Likely:

Quoted 1 line

There is no overlap here (except for the degenerate case of using a PWM as an output only GPIO... which is useful. Board designers do crazy things).

But there may be cases where a device pin is used as GPIO *and* as pwm output. Think of a relay output driver which may be used a state GPIO to indicate that the device is up, or as a pulsed heartbeat output (some safety schemes require that) for the same purpose, selected by a run-time (not device tree!) config option.

I actually do exactly this with one of the Freescale 5200's pins, and would therefore also appreciate a "unified" approach.

Cheers, Albrecht.

#26 lore raw
In reply to Grant Likely
Quoted 2 lines · Grant Likely

*however* I do agree that it is the responsibility of platform code to set up chip-internal pin muxing and routing.

Fo over 95% of systems, I'd agree -- given that "platform" code includes the arch/.../mach-X/board-Y.c files. It's not realistic to expect boot loaders to always handle that stuff. If for no other reason than the way they're produced: get something that will boot <OS> and call it done. There will be nuances that need to be corrected later.

Quoted 5 lines

Actually, further than
that, I think it is actually firmware's responsibility to set up chip internal pin muxing because it leads to more common platform code in the kernel (less board specific fixups), but the kernel can fix it up in a pinch.

That something-less-than-5% remaining includes a lot of developer boards, where there are multiple viable configurations. The OS needs to know which config it's going into at boot time.

And there's even a crazed subset of that 5% which wants to do runtime reconfiguration. Those folk do not accept static board configs, whether done by a bootloader or anything else.

Some of that subset isn't entirely crazed. I was reading a chip errata document not long ago, which pointed out an issue I've seen before: suspend/resume cycles needed to reconfigure things dynamically, to prevent leakage. That's board-specific and non-static.

Quoted 1 line

But I'm not arguing about the pin (hardware) setup code.

That's good, since I don't think there's a Grand Scheme that can be agreed to in that space, either for boot time setup or runtime reconfiguration. The hardware varies too much.

- Dave

#27 lore raw
In reply to Grant Likely
Quoted 9 lines · Grant Likely

I take issue
with all the common code behind the API to make it work and to allow GPIOs or PWMs to be registered at runtime.  The overlap is the code and behaviour used to register pins and to obtain a reference to a pin.

On the PWM side; it's the code backing pwm_register(). pwm_unregister(), pwm_request(), pwm_free(), and the sysfs hooks.

Quoted 8 lines

For GPIO; it's gpiochip_add(), gpiochip_remove(), gpio_request(), gpio_free(), the sysfs hooks, and the device tree bindings.

They perform exactly the same task.  The difference is that PWM devices implement the PWM API and GPIO devices implement the GPIO API; but the behaviour needed to manage them is identical.  I don't like the fact that it will be implemented twice with subtle differences between them.

Most of that stuff sits inside sysfs. What's actually sharable?

Quoted 7 lines

The argument has been made that the split is appropriate because the use case between GPIO and PWM is considerably different, and to a point I agree.  The PWM use case definitely requires a different API. However, the argument also makes the assumption that what is a GPIO and what is a PWM can easily be pinned down.  For example, I've got output only GPIOs, input only GPIOs, and a device that implements both PWM and GPIO behaviour (not pin muxing).

In object oriented programming there's this thing called an "interface", and its application here is: you write a driver implementing both the "gpio" and "pwm" interfaces. Someone wanting a given behavior goes through that interface.

Quoted 5 lines

If a hard distinction is
made now to manage GPIO and PWM pins separately, what then do we do with other similar-yet-different pin controllers?  What about a multi-voltage output GPIO pin?  Or a Frequency generator pin?  Is it appropriate to create a new subsystem for managing each one?

None of those are particularly common. The Linux approach is to come up with interfaces as needed ... then generalize once there's enough data, if indeed there is enough. Just because you *can* come up with hardware doesn't mean it's widespread enough to need a shared API.

Quoted 3 lines

What an API can do is capture the common use cases, but it can never capture the full range of behaviour implemented by a particular device.

Right, so you need to become accustomed to the notion that there can be platform-specific capabilities. Kitchen-sink APIs are bad.

#28 lore raw
In reply to Grant Likely
Quoted 2 lines · Grant Likely

Common code is a big gain in and of itself.

I completely agree! Which is why I used the GPIO API in my PWM pseudo-device, along with an hrtimer.

Quoted 6 lines

What I would like to see is the PWM functions added to the GPIO API. GPIO drivers can then either implement them or not. If a GPIO driver supports the PWM function, great. If not, then it returns -EINVAL. Heck, I'll even got a driver right now that I'd use it with. I'm more than happy to help code it up even.

I think the appropriate thing to do in such cases is to reimplement the GPIO driver at its lowest level to provide both GPIO API and PWM API interfaces. In my way of thinking, such hardware is a kind of multi-function device and it should be treated that way. No need to expose that multi-functionality to the user unless necessary.

The odd case to me is the concept of "timed GPIO", as in the Android kernels. I see the timing capability as a convenience feature that could be realized by adding timers to the GPIO interface (as they did), or by having a one-shot PWM channel. I'm not really comfortable with either as an interface concept, however, because it just doesn't... feel right. :)

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#29 lore raw
In reply to Grant Likely
Quoted 3 lines · Grant Likely

I'm concerned about the approach taken here.  As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines.

PWM is not GPIO, and doesn't fit into a GPIO framework.

Since *everything* boils down to one or more signal lines, your argument leads directly to Linux having no native hardware interface except GPIOs. Not ... practical. ;)

Quoted 3 lines

The difference being
that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate)

If you want to combine PWM with something else ... timers would be a better target. They're both fundamentally about periodic phenomena. And quite a lot of timers support PWM output modes...

(A generic interface to hardware timers is lacking, too.)

Quoted 4 lines

What is the reason for bringing in an entirely new framework instead of extending the GPIO API or gpiolib?  I'm not too excited about having two entirely different frameworks for what basically boils down to "numbered signal pins".

You seem to mis-understand what PWM is all about, then. The whole point of a PWM is to set up a periodic activity that will run without CPU intervention.

GPIOs, on the other hand, are packaged for manual bit twiddling. While it's possible to create low-speed implementations of serial protocols using GPIOs (like 2-wire/I2C, one-wire, and various SPI variants), those are explicitly the high-overhead (and low performance) substitutes, to be used only when native hardware isn't available (or is broken etc).

For example you won't often get 40 Mbit/sec SPI if you are bitbanging over GPIOs ... and if you do, it won't leave much CPU horsepower for much of anything else.

- Dave

#30 lore raw
Quoted 7 lines · David Brownell
Quoted 4 lines · Grant Likely

I'm concerned about the approach taken here. As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines.

Quoted 6 lines

PWM is not GPIO, and doesn't fit into a GPIO framework.

Since *everything* boils down to one or more signal lines, your argument leads directly to Linux having no native hardware interface except GPIOs. Not ... practical. ;)

Wait. Isn't that what Ubicom's chips do? :)

Quoted 6 lines

If you want to combine PWM with something else ... timers would be a better target. They're both fundamentally about periodic phenomena. And quite a lot of timers support PWM output modes...

(A generic interface to hardware timers is lacking, too.)

True, and I have code (not yet published) to support a couple of timer/counter peripherals under the PWM interface. So for that functionality at least, I don't think a more generic or standalone API is necessary.

I don't know how to define a generic interface for the counter a.k.a. "input capture" behavior of such devices, though. Still an unsolved problem, but I don't think it will be a part of the PWM API. It's a different metaphor.

Quoted 8 lines

GPIOs, on the other hand, are packaged for manual bit twiddling. While it's possible to create low-speed implementations of serial protocols using GPIOs (like 2-wire/I2C, one-wire, and various SPI variants), those are explicitly the high-overhead (and low performance) substitutes, to be used only when native hardware isn't available (or is broken etc).

And when using GPIO to generate I2C or SPI signals, you don't do it through the GPIO interface--- you do it through the I2C or SPI interface, and a driver behind that API talks to the GPIO API.... or to a different driver if you change your mind. Same idea for the PWM API.

Signature · 2 lines
Bill Gatliff
[email protected]
#31 lore raw
In reply to David Brownell
Quoted 10 lines · David Brownell
Quoted 3 lines · Grant Likely

I'm concerned about the approach taken here.  As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines.

PWM is not GPIO, and doesn't fit into a GPIO framework.

Since *everything* boils down to one or more signal lines, your argument leads directly to Linux having no native hardware interface except GPIOs.  Not ... practical. ;)

I think you've missed my point and taken it to an illogical extreme to counter it. I agree that PWMs are not GPIOs and visa versa. However, *some* devices are both GPIOs and PWMs. Also what is needed to manage GPIO and PWM pins is pretty much identical.

Quoted 11 lines
Quoted 3 lines

The difference being
that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate)

If you want to combine PWM with something else ... timers would be a better target.  They're both fundamentally about periodic phenomena.  And quite a lot of timers support PWM output modes...

(A generic interface to hardware timers is lacking, too.)

Quoted 8 lines
Quoted 4 lines

What is the reason for bringing in an entirely new framework instead of extending the GPIO API or gpiolib?  I'm not too excited about having two entirely different frameworks for what basically boils down to "numbered signal pins".

You seem to mis-understand what PWM is all about, then. The whole point of a PWM is to set up a periodic activity that will run without CPU intervention.

I understand that.

Quoted 7 lines

GPIOs, on the other hand, are packaged for manual bit twiddling.  While it's possible to create low-speed implementations of serial protocols using GPIOs (like 2-wire/I2C, one-wire, and various SPI variants), those are explicitly the high-overhead (and low performance) substitutes, to be used only when native hardware isn't available (or is broken etc).

But that *isn't* the primary purpose of the GPIO subsystem. All that stuff is layered on top of the GPIO pin management code and doesn't really play into this debate.

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#32 lore raw
Quoted 5 lines · Grant Likely

But that *isn't* the primary purpose of the GPIO subsystem. All that stuff is layered on top of the GPIO pin management code and doesn't really play into this debate.

I don't understand you. You are saying that the majority of gpiochip implementations also deal with pin multiplexing? They must be terribly broken on the platforms I'm working on, then, because they don't seem to work at all! :)

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#33 lore raw
Quoted 11 lines · Bill Gatliff
Quoted 5 lines · Grant Likely

But that *isn't* the primary purpose of the GPIO subsystem.  All that stuff is layered on top of the GPIO pin management code and doesn't really play into this debate.

I don't understand you.  You are saying that the majority of gpiochip implementations also deal with pin multiplexing?  They must be terribly broken on the platforms I'm working on, then, because they don't seem to work at all!  :)

Nope. pin mux is a different problem. I'm talking about the code to register pin controller drivers and for users to obtain a reference to a pin.

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#34 lore raw
In reply to Grant Likely
Quoted 10 lines · Grant Likely
Quoted 3 lines · David Brownell

Since *everything* boils down to one or more signal lines, your argument leads directly to Linux having no native hardware interface except GPIOs. ?Not ... practical. ;)

I think you've missed my point and taken it to an illogical extreme to counter it. I agree that PWMs are not GPIOs and visa versa. However, *some* devices are both GPIOs and PWMs. Also what is needed to manage GPIO and PWM pins is pretty much identical.

On most of the ARM SoCs PWM and GPIO aren't particularly special here - most of the on-SoC functionality is multiplexed onto pins through the same hardware interface. A very large proportion of the pins of the SoC will have muxes to bring out the signals from the internal IP blocks, and pretty much all of those will have GPIO as one of those functions.

For many SoCs there will be multiple pin options for bringing out each of the internal signals which complicates matters further.

Quoted 3 lines

But that *isn't* the primary purpose of the GPIO subsystem. All that stuff is layered on top of the GPIO pin management code and doesn't really play into this debate.

The GPIO subsystem isn't doing pin management in that way for most systems, it's just controlling the GPIO functionality and relies on separate configuration to ensure that the relevant pins are in GPIO mode.

#35 lore raw
Quoted 12 lines · Mark Brown
Quoted 10 lines · Grant Likely
Quoted 3 lines · David Brownell

Since *everything* boils down to one or more signal lines, your argument leads directly to Linux having no native hardware interface except GPIOs. ?Not ... practical. ;)

I think you've missed my point and taken it to an illogical extreme to counter it.  I agree that PWMs are not GPIOs and visa versa.  However, *some* devices are both GPIOs and PWMs.  Also what is needed to manage GPIO and PWM pins is pretty much identical.

Quoted 5 lines

On most of the ARM SoCs PWM and GPIO aren't particularly special here - most of the on-SoC functionality is multiplexed onto pins through the same hardware interface.  A very large proportion of the pins of the SoC will have muxes to bring out the signals from the internal IP blocks, and pretty much all of those will have GPIO as one of those functions.

Right, pin-mux is a different problem. But there are also devices that implement both PWM and GPIO functionality in the same IP block. I think pin muxing, and pin controller drivers are different problem domains and should be handled separately.

Quoted 8 lines
Quoted 3 lines

But that *isn't* the primary purpose of the GPIO subsystem.  All that stuff is layered on top of the GPIO pin management code and doesn't really play into this debate.

The GPIO subsystem isn't doing pin management in that way for most systems, it's just controlling the GPIO functionality and relies on separate configuration to ensure that the relevant pins are in GPIO mode.

Sorry. when I said pin management I meant how Linux keeps track of pin controllers. Not pin mux. I should use different terminology perhaps to reduce confusion.

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#36 lore raw
Quoted 2 lines · Grant Likely

Right, pin-mux is a different problem. But there are also devices that implement both PWM and GPIO functionality in the same IP block.

That's not the general case, though - most of the SoCs seem to have PWM as a separate IP block. In the general case PWM and GPIO have nothing to do with each other.

Quoted 2 lines

I think pin muxing, and pin controller drivers are different problem domains and should be handled separately.

...

Quoted 3 lines

Sorry. when I said pin management I meant how Linux keeps track of pin controllers. Not pin mux. I should use different terminology perhaps to reduce confusion.

I have to confess I'm a bit lost as to what you mean by a "pin controller" as opposed to "pin mux" interface. For a substantial proportion of ARMs they're going to be one and the same.

Judging from some of the other messages in the thread I suspect you're thinking of a much closer mapping between PWM and GPIO pins - many SoCs do have distinct PWM controllers that aren't terribly tied to a GPIO pin. For them the whole concept of requesting a "pin" or having the PWM controller be tied to a particular pin is going to be at best confusing, you really do want to request the PWM controller itself and let the pin mux setup figure out where that emerges from the SoC.

#37 lore raw
Quoted 4 lines · Mark Brown

Judging from some of the other messages in the thread I suspect you're thinking of a much closer mapping between PWM and GPIO pins - many SoCs do have distinct PWM controllers that aren't terribly tied to a GPIO pin.

Sometimes they can be coupled to one of several pins ... possibly even outputting to several at the same time.

Quoted 4 lines

For them the whole concept of requesting a "pin" or having the PWM controller be tied to a particular pin is going to be at best confusing, you really do want to request the PWM controller itself and let the pin mux setup figure out where that emerges from the SoC.

Nicely put. At some level one ends with something like "pwm.2 channel 3" getting allocated, and does not care about pins except as more low-level artifacts to be ignored outside of board setup code.

- Dave

#38 lore raw
In reply to Grant Likely
Quoted 21 lines · Grant Likely

Hi Bill

Quoted 4 lines · Bill Gatliff

This series implements a Generic PWM Device API, including reference implementations for the Atmel PWMC device, an LED device, and an LED trigger. It is based on linux-2.6.27.

[...]

Quoted 8 lines

The implementation of the Generic PWM Device API is structurally similar to the generic GPIO API, except that the PWM code uses platform bus_id strings instead of integers to identify target deviices. A configuration structure is also provided, both to facilitate atomic hardware state changes and so that the API can be extended in a source-code-compatible way to accomodate devices with features not anticipated by the current code.

Hey Bill,

Quoted 8 lines

I'm concerned about the approach taken here. As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines. The difference being that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate). Actually, many GPIOs have these properties too. I've got a part with output-only gpios, and gpio devices that also have a PWM.

It's true that PWM signals can be emitted via GPIO pins, and in fact the API code I submitted uses the GPIO API combined with an hrtimer to generate a low-speed PWM output.

It's also true that things like LEDs might want to be driven by PWM signals, and indeed LEDs can also be driven by GPIO pins.

Finally, it's accurate to say that a lot of microcontrollers multiplex GPIO functions onto pins that also provide PWM functionality.

But in my opinion, none of the above means that the PWM API and GPIO API should be coupled at the interface abstraction. All it means is that in some cases, one implementation might be able to "stand in" as an embodiment of the other's interface. And we all agree on that for PWM and GPIO.

The reason I think that PWM merits its own API is because I want the users of the interface to be able to treat it as a PWM-only metaphor. Behind the scenes, we may in fact have a connection with the GPIO interface--- and in fact we already do with my code. But PWM generation can also come from timer/counter peripherals having a compare/match capability, from true PWM generation hardware (which are in most cases just three tightly-coupled timer/counters with compare/match hardware), as well as from standalone chips that are controlled over an I2C or SPI interface. The latter example is in fact the initial motivation for the code, even though at the moment I don't have any patches available to submit because the devices in question are 8-bit microcontrollers running custom firmware.

Given a PWM-capable chip that you control over an I2C or SPI interface, I don't think anyone would argue that such chips should be viewed as an extension of the SPI or I2C APIs just because the chips use those communications protocols. I think it's a similar argument for PWM and GPIO.

Overall, I think your question is a very good one and it's one that I asked myself early on. And it's a question that we should ask for EVERY new interface abstraction that's developed for kernel-facing or application-facing users.

Quoted 5 lines

What is the reason for bringing in an entirely new framework instead of extending the GPIO API or gpiolib? I'm not too excited about having two entirely different frameworks for what basically boils down to "numbered signal pins".

I hope my reasoning is clear. I might not be right, but at least you know what my thoughts are. Questions, comments and flames welcome!

b.g.

Signature · 2 lines
Bill Gatliff
[email protected]
#39 lore raw
Quoted 37 lines · Bill Gatliff
Quoted 21 lines · Grant Likely

Hi Bill

Quoted 4 lines · Bill Gatliff

This series implements a Generic PWM Device API, including reference implementations for the Atmel PWMC device, an LED device, and an LED trigger.  It is based on linux-2.6.27.

[...]

Quoted 8 lines

The implementation of the Generic PWM Device API is structurally similar to the generic GPIO API, except that the PWM code uses platform bus_id strings instead of integers to identify target deviices.  A configuration structure is also provided, both to facilitate atomic hardware state changes and so that the API can be extended in a source-code-compatible way to accomodate devices with features not anticipated by the current code.

Hey Bill,

Quoted 7 lines

I'm concerned about the approach taken here.  As I understand it, the PWM signals are very similar to GPIOs in that each PWM device controls an external signal line, just like GPIO lines.  The difference being that PWMs cannot do input, and has additional capabilities (can be programmed with a signal; not just on/off/tristate).  Actually, many GPIOs have these properties too.  I've got a part with output-only gpios, and gpio devices that also have a PWM.

It's true that PWM signals can be emitted via GPIO pins, and in fact the API code I submitted uses the GPIO API combined with an hrtimer to generate a low-speed PWM output.

It's also true that things like LEDs might want to be driven by PWM signals, and indeed LEDs can also be driven by GPIO pins.

Quoted 8 lines

Finally, it's accurate to say that a lot of microcontrollers multiplex GPIO functions onto pins that also provide PWM functionality.

But in my opinion, none of the above means that the PWM API and GPIO API should be coupled at the interface abstraction.  All it means is that in some cases, one implementation might be able to "stand in" as an embodiment of the other's interface.  And we all agree on that for PWM and GPIO.

I have no problem with the PWM API. I think it describes most of the PWM *usage* requirements well, and I agree that GPIO devices can never implement the PWM API.

Quoted 4 lines

Given a PWM-capable chip that you control over an I2C or SPI interface, I don't think anyone would argue that such chips should be viewed as an extension of the SPI or I2C APIs just because the chips use those communications protocols.

Nor would I.

Quoted 1 line

I think it's a similar argument for PWM and GPIO.

I don't think that's true, but I'm approaching it from the viewpoint of managing pins; not the usage API. ie. the difference between gpiolib (management) and the gpio api (usage).

g.

Signature · 2 lines
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
#40 lore raw
Quoted 3 lines · Grant Likely

I don't think that's true, but I'm approaching it from the viewpoint of managing pins; not the usage API.  ie. the difference between gpiolib (management) and the gpio api (usage).

Don't follow. The gpiolib code is unrelated to pin managment. It's purely an aid for implementations of the API.