This patch series contains the sixth attempt at implementation of
a generic PWM device interface framework. Think gpiolib, but for
devices and pseudo-devices that generate pulse-wave-modulated outputs.
Compared to the previous version, this patch series:
* Cleans up error handling in atmel-pwmc device registration and removal
* Removes excessive inlines from atmel-pwmc implementation
* Drops unnecessary IS_ERR_OR_NULL test from ioremap return value
* Removes Atmel PWMC's Kconfig patch from the gpio-pwm patch file
Functionally-speaking, this series has regressed somewhat from
previous versions because I am currently focusing my attention on the
API itself. I include only implementations for GPIO+hrtimer devices
and the Atmel PWMC peripheral as references in this series; I will
post patches for LED drivers, PXA, Samsung, etc. devices once I know
that the API itself is on its way to mainline. (I believe that the
two reference implementations sufficiently confirm the utility of the
API itself).
The code in this series is significantly clearer and more
straightforward than previous versions. Thanks to everyone who helped
me with this refactoring! I'm pretty convinced that the code you see
here is at last suitable for pulling into mainline.
Finally, the attached code CAN be used to control devices that drive
stepper motors and the like, but doing so is discouraged as I am
anticipating a request to develop an API specifically for such
situations.
Regards,
b.g.
Bill Gatliff (3):
PWM: Implement a generic PWM framework
PWM: GPIO+hrtimer device emulation
PWM: Atmel PWMC driver
Documentation/pwm.txt | 277 +++++++++++++++++++++
drivers/Kconfig | 2 +
drivers/Makefile | 2 +
drivers/pwm/Kconfig | 29 +++
drivers/pwm/Makefile | 7 +
drivers/pwm/atmel-pwmc.c | 494 +++++++++++++++++++++++++++++++++++++
drivers/pwm/gpio-pwm.c | 348 ++++++++++++++++++++++++++
drivers/pwm/pwm.c | 610 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pwm/pwm.h | 155 ++++++++++++
9 files changed, 1924 insertions(+), 0 deletions(-)
create mode 100644 Documentation/pwm.txt
create mode 100644 drivers/pwm/Kconfig
create mode 100644 drivers/pwm/Makefile
create mode 100644 drivers/pwm/atmel-pwmc.c
create mode 100644 drivers/pwm/gpio-pwm.c
create mode 100644 drivers/pwm/pwm.c
create mode 100644 include/linux/pwm/pwm.h
--
1.7.2.3
Updates the existing PWM-related functions to support multiple
and/or hotplugged PWM devices, and adds a sysfs interface.
Moves the code to drivers/pwm.
For now, this new code can exist alongside the current PWM
implementations; the existing implementations will be migrated
to this new framework as time permits. Eventually, the current
PWM implementation will be deprecated and then expunged.
Signed-off-by: Bill Gatliff <[email protected]>
---
Documentation/pwm.txt | 277 +++++++++++++++++++++
drivers/Kconfig | 2 +
drivers/Makefile | 2 +
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 4 +
drivers/pwm/pwm.c | 610 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pwm/pwm.h | 155 ++++++++++++
7 files changed, 1060 insertions(+), 0 deletions(-)
create mode 100644 Documentation/pwm.txt
create mode 100644 drivers/pwm/Kconfig
create mode 100644 drivers/pwm/Makefile
create mode 100644 drivers/pwm/pwm.c
create mode 100644 include/linux/pwm/pwm.h
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
new file mode 100644
index 0000000..2b15395
--- /dev/null
+++ b/Documentation/pwm.txt
@@ -0,0 +1,277 @@
+ Generic PWM Device API
+
+ February 7, 2011
+ Bill Gatliff
+ <[email protected]>
+
+
+
+The code in drivers/pwm and include/linux/pwm/ 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 suddenly 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 accommodate 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, sensors and solenoids through a
+custom-designed, optically-isolated H-bridge driver.
+
+
+
+Functional Overview
+
+The Generic PWM Device API framework is implemented in
+include/linux/pwm/pwm.h and drivers/pwm/pwm.c. The functions therein
+use information from pwm_device and pwm__config structures to invoke
+services in PWM peripheral device drivers. Consult
+drivers/pwm/atmel-pwmc.c for an example driver for the Atmel PWMC
+peripheral.
+
+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 Generic PWM API framework also provides a sysfs interface to PWM
+devices, which is adequate for basic application needs and testing.
+
+Driver authors fill out a pwm_device structure, which describes the
+capabilities of the PWM hardware being utilized. 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 as users begin to
+configure and utilize the hardware.
+
+Many PWM-capable peripherals provide two, three, or more channels of
+PWM output. The driver author completes and registers a pwm_device
+structure for each channel they wish to be supported by the Generic
+PWM API.
+
+Note that PWM signals can be produced by a variety of peripherals,
+beyond the true PWM peripherals 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 be accommodated by the Generic PWM Device API framework.
+
+
+
+Using the API to Generate PWM Signals -- Basic Functions for Users
+
+
+pwm_request() -- Returns a pwm_device 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_release() 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_release() -- 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_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 by the config structure.
+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_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, timer
+handlers, and other interrupt contexts, but must confine their
+configuration changes to only those that the driver can implement
+without sleeping. 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. By default, all PWM
+channels are unsynchronized.
+
+
+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 plain-text 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 Atmel PWMC
+platform 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 dev_name(pdev->dev) in your
+probe() method).
+
+
+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) Invoked 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.
+
+
+set_callback -- (optional) If the hardware supports an end-of-period
+interrupt, invoke the function provided in this callback during the
+device's 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-compatible 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.
+
+
+
+FAQs and Additional Notes
+
+The Atmel PWMC pwm_config() function tries to satisfy the user's
+configuration request by first invoking pwm_config_nosleep(). If that
+operation fails, then the PWM peripheral is brought to a synchronized
+stop, the configuration changes are made, and the device is restarted.
+
+The Atmel PWMC's use of pwm_config_nosleep() from pwm_config()
+minimizes redundant code between the two functions, and relieves the
+pwm_config() function of the need to explicitly test whether a
+requested configuration change can be carried out while the PWM device
+is in its current mode.
+
+PWM API driver authors are encouraged to adopt the Atmel PWMC's
+pwm_config()-vs.-pwm_config_nosleep() strategy in implementations for
+other devices as well.
+
+
+
+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. ;)
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9bfb71f..413e4f9 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -56,6 +56,8 @@ source "drivers/pps/Kconfig"
source "drivers/gpio/Kconfig"
+source "drivers/pwm/Kconfig"
+
source "drivers/w1/Kconfig"
source "drivers/power/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index b423bb1..4e37abf 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -6,6 +6,8 @@
#
obj-y += gpio/
+obj-$(CONFIG_GENERIC_PWM) += pwm/
+
obj-$(CONFIG_PCI) += pci/
obj-$(CONFIG_PARISC) += parisc/
obj-$(CONFIG_RAPIDIO) += rapidio/
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
new file mode 100644
index 0000000..bc550f7
--- /dev/null
+++ b/drivers/pwm/Kconfig
@@ -0,0 +1,10 @@
+#
+# PWM infrastructure and devices
+#
+
+menuconfig GENERIC_PWM
+ tristate "PWM Support"
+ help
+ Enables PWM device support implemented via a generic
+ framework. If unsure, say N.
+
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
new file mode 100644
index 0000000..7baa201
--- /dev/null
+++ b/drivers/pwm/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for pwm devices
+#
+obj-$(CONFIG_GENERIC_PWM) := pwm.o
diff --git a/drivers/pwm/pwm.c b/drivers/pwm/pwm.c
new file mode 100644
index 0000000..f958e4b
--- /dev/null
+++ b/drivers/pwm/pwm.c
@@ -0,0 +1,610 @@
+/*
+ * PWM API implementation
+ *
+ * Copyright (C) 2011 Bill Gatliff <[email protected]>
+ * Copyright (C) 2011 Arun Murthy <[email protected]>
+ *
+ * This program is free software; you may redistribute and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/completion.h>
+#include <linux/workqueue.h>
+#include <linux/list.h>
+#include <linux/sched.h>
+#include <linux/pwm/pwm.h>
+
+static const char *REQUEST_SYSFS = "sysfs";
+static LIST_HEAD(pwm_device_list);
+static DEFINE_MUTEX(device_list_mutex);
+static struct class pwm_class;
+static struct workqueue_struct *pwm_handler_workqueue;
+
+static int pwm_match_name(struct device *dev, void *name)
+{
+ return !strcmp(name, dev_name(dev));
+}
+
+static int __pwm_request(struct pwm_device *p, const char *label)
+{
+ int ret;
+
+ ret = test_and_set_bit(FLAG_REQUESTED, &p->flags);
+ if (ret) {
+ ret = -EBUSY;
+ goto done;
+ }
+
+ p->label = label;
+
+ if (p->ops->request) {
+ ret = p->ops->request(p);
+ if (ret) {
+ clear_bit(FLAG_REQUESTED, &p->flags);
+ goto done;
+ }
+ }
+
+done:
+ return ret;
+}
+
+static struct pwm_device *__pwm_request_byname(const char *name,
+ const char *label)
+{
+ struct device *d;
+ struct pwm_device *p;
+ int ret;
+
+ d = class_find_device(&pwm_class, NULL, (char*)name, pwm_match_name);
+ if (IS_ERR_OR_NULL(d))
+ return ERR_PTR(-EINVAL);
+
+ p = dev_get_drvdata(d);
+ ret = __pwm_request(p, label);
+
+ if (ret)
+ return ERR_PTR(ret);
+ return p;
+}
+
+struct pwm_device *pwm_request_byname(const char *name, const char *label)
+{
+ struct pwm_device *p;
+
+ mutex_lock(&device_list_mutex);
+ p = __pwm_request_byname(name, label);
+ mutex_unlock(&device_list_mutex);
+ return p;
+}
+EXPORT_SYMBOL(pwm_request_byname);
+
+struct pwm_device *pwm_request(const char *bus_id, int id, const char *label)
+{
+ char name[256];
+ int ret;
+
+ if (id == -1)
+ ret = scnprintf(name, sizeof name, "%s", bus_id);
+ else
+ ret = scnprintf(name, sizeof name, "%s:%d", bus_id, id);
+ if (ret <= 0 || ret >= sizeof name)
+ return ERR_PTR(-EINVAL);
+
+ return pwm_request_byname(name, label);
+}
+EXPORT_SYMBOL(pwm_request);
+
+void pwm_release(struct pwm_device *p)
+{
+ mutex_lock(&device_list_mutex);
+
+ if (!test_and_clear_bit(FLAG_REQUESTED, &p->flags)) {
+ WARN(1, "%s: releasing unrequested PWM device %s\n",
+ __func__, dev_name(p->dev));
+ goto done;
+ }
+
+ pwm_stop(p);
+ pwm_unsynchronize(p, NULL);
+ pwm_set_handler(p, NULL, NULL);
+
+ p->label = NULL;
+
+ if (p->ops->release)
+ p->ops->release(p);
+done:
+ mutex_unlock(&device_list_mutex);
+}
+EXPORT_SYMBOL(pwm_release);
+
+static unsigned long pwm_ns_to_ticks(struct pwm_device *p, unsigned long nsecs)
+{
+ unsigned long long ticks;
+
+ ticks = nsecs;
+ ticks *= p->tick_hz;
+ do_div(ticks, 1000000000);
+ return ticks;
+}
+
+static unsigned long pwm_ticks_to_ns(struct pwm_device *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;
+}
+
+int pwm_config_nosleep(struct pwm_device *p, struct pwm_config *c)
+{
+ if (!p->ops->config_nosleep)
+ return -EINVAL;
+
+ return p->ops->config_nosleep(p, c);
+}
+EXPORT_SYMBOL(pwm_config_nosleep);
+
+int pwm_config(struct pwm_device *p, struct pwm_config *c)
+{
+ int ret = 0;
+
+ switch (c->config_mask & (BIT(PWM_CONFIG_PERIOD_TICKS)
+ | BIT(PWM_CONFIG_DUTY_TICKS))) {
+ case BIT(PWM_CONFIG_PERIOD_TICKS):
+ if (p->duty_ticks > c->period_ticks) {
+ ret = -EINVAL;
+ goto err;
+ }
+ break;
+ case BIT(PWM_CONFIG_DUTY_TICKS):
+ if (p->period_ticks < c->duty_ticks) {
+ ret = -EINVAL;
+ goto err;
+ }
+ break;
+ case BIT(PWM_CONFIG_DUTY_TICKS) | BIT(PWM_CONFIG_PERIOD_TICKS):
+ if (c->duty_ticks > c->period_ticks) {
+ ret = -EINVAL;
+ goto err;
+ }
+ break;
+ default:
+ break;
+ }
+
+err:
+ dev_dbg(p->dev, "%s: config_mask %lu period_ticks %lu "
+ "duty_ticks %lu polarity %d\n",
+ __func__, c->config_mask, c->period_ticks,
+ c->duty_ticks, c->polarity);
+
+ if (ret)
+ return ret;
+ return p->ops->config(p, c);
+}
+EXPORT_SYMBOL(pwm_config);
+
+int pwm_set(struct pwm_device *p, unsigned long period_ns,
+ unsigned long duty_ns, int active_high)
+{
+ struct pwm_config c = {
+ .config_mask = (BIT(PWM_CONFIG_PERIOD_TICKS)
+ | BIT(PWM_CONFIG_DUTY_TICKS)
+ | BIT(PWM_CONFIG_POLARITY)),
+ .period_ticks = pwm_ns_to_ticks(p, period_ns),
+ .duty_ticks = pwm_ns_to_ticks(p, duty_ns),
+ .polarity = active_high
+ };
+
+ return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_set);
+
+int pwm_set_period_ns(struct pwm_device *p, unsigned long period_ns)
+{
+ struct pwm_config c = {
+ .config_mask = BIT(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_device *p)
+{
+ return pwm_ticks_to_ns(p, p->period_ticks);
+}
+EXPORT_SYMBOL(pwm_get_period_ns);
+
+int pwm_set_duty_ns(struct pwm_device *p, unsigned long duty_ns)
+{
+ struct pwm_config c = {
+ .config_mask = BIT(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_device *p)
+{
+ return pwm_ticks_to_ns(p, p->duty_ticks);
+}
+EXPORT_SYMBOL(pwm_get_duty_ns);
+
+int pwm_set_polarity(struct pwm_device *p, int active_high)
+{
+ struct pwm_config c = {
+ .config_mask = BIT(PWM_CONFIG_POLARITY),
+ .polarity = active_high,
+ };
+ return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_set_polarity);
+
+int pwm_start(struct pwm_device *p)
+{
+ struct pwm_config c = {
+ .config_mask = BIT(PWM_CONFIG_START),
+ };
+ return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_start);
+
+int pwm_stop(struct pwm_device *p)
+{
+ struct pwm_config c = {
+ .config_mask = BIT(PWM_CONFIG_STOP),
+ };
+ return pwm_config(p, &c);
+}
+EXPORT_SYMBOL(pwm_stop);
+
+int pwm_synchronize(struct pwm_device *p, struct pwm_device *to_p)
+{
+ if (!p->ops->synchronize)
+ return -EINVAL;
+
+ return p->ops->synchronize(p, to_p);
+}
+EXPORT_SYMBOL(pwm_synchronize);
+
+int pwm_unsynchronize(struct pwm_device *p, struct pwm_device *from_p)
+{
+ if (!p->ops->unsynchronize)
+ return -EINVAL;
+
+ return p->ops->unsynchronize(p, from_p);
+}
+EXPORT_SYMBOL(pwm_unsynchronize);
+
+static void pwm_handler(struct work_struct *w)
+{
+ struct pwm_device *p = container_of(w, struct pwm_device,
+ handler_work);
+ if (p->handler)
+ p->handler(p, p->handler_data);
+}
+
+void pwm_callback(struct pwm_device *p)
+{
+ queue_work(pwm_handler_workqueue, &p->handler_work);
+}
+EXPORT_SYMBOL(pwm_callback);
+
+int pwm_set_handler(struct pwm_device *p, pwm_handler_t handler, void *data)
+{
+ struct pwm_config c;
+ int ret;
+
+ if (handler)
+ c.config_mask = BIT(PWM_CONFIG_ENABLE_CALLBACK);
+ else
+ c.config_mask = BIT(PWM_CONFIG_DISABLE_CALLBACK);
+
+ ret = pwm_config(p, &c);
+
+ if (!ret && handler) {
+ p->handler_data = data;
+ p->handler = handler;
+ INIT_WORK(&p->handler_work, pwm_handler);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL(pwm_set_handler);
+
+static ssize_t pwm_run_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_device *p = dev_get_drvdata(dev);
+ return sprintf(buf, "%d\n", pwm_is_running(p));
+}
+
+static ssize_t pwm_run_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct pwm_device *p = dev_get_drvdata(dev);
+
+ if (!pwm_is_exported(p))
+ return -EINVAL;
+
+ if (sysfs_streq(buf, "1"))
+ pwm_start(p);
+ else if (sysfs_streq(buf, "0"))
+ pwm_stop(p);
+
+ return len;
+}
+static DEVICE_ATTR(run, S_IRUGO | S_IWUSR, pwm_run_show, pwm_run_store);
+
+static ssize_t pwm_tick_hz_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_device *p = dev_get_drvdata(dev);
+ return sprintf(buf, "%lu\n", p->tick_hz);
+}
+static DEVICE_ATTR(tick_hz, S_IRUGO, pwm_tick_hz_show, NULL);
+
+static ssize_t pwm_duty_ns_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_device *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_device *p = dev_get_drvdata(dev);
+
+ if (!pwm_is_exported(p))
+ return -EINVAL;
+
+ if (!strict_strtoul(buf, 10, &duty_ns))
+ pwm_set_duty_ns(p, duty_ns);
+ return len;
+}
+static DEVICE_ATTR(duty_ns, S_IRUGO | S_IWUSR, 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_device *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_device *p = dev_get_drvdata(dev);
+
+ if (!pwm_is_exported(p))
+ return -EINVAL;
+
+ if (!strict_strtoul(buf, 10, &period_ns))
+ pwm_set_period_ns(p, period_ns);
+ return len;
+}
+static DEVICE_ATTR(period_ns, S_IRUGO | S_IWUSR, 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_device *p = dev_get_drvdata(dev);
+ return sprintf(buf, "%d\n", p->active_high ? 1 : 0);
+}
+
+static ssize_t pwm_polarity_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ unsigned long polarity;
+ struct pwm_device *p = dev_get_drvdata(dev);
+
+ if (!pwm_is_exported(p))
+ return -EINVAL;
+
+ if (!strict_strtoul(buf, 10, &polarity))
+ pwm_set_polarity(p, polarity);
+ return len;
+}
+static DEVICE_ATTR(polarity, S_IRUGO | S_IWUSR, pwm_polarity_show, pwm_polarity_store);
+
+static ssize_t pwm_request_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pwm_device *p = dev_get_drvdata(dev);
+ int ret;
+
+ mutex_lock(&device_list_mutex);
+ ret = __pwm_request(p, REQUEST_SYSFS);
+ mutex_unlock(&device_list_mutex);
+
+ if (!ret)
+ set_bit(FLAG_EXPORTED, &p->flags);
+
+ return sprintf(buf, "%s\n", p->label);
+}
+
+static ssize_t pwm_request_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct pwm_device *p = dev_get_drvdata(dev);
+
+ pwm_release(p);
+ clear_bit(FLAG_EXPORTED, &p->flags);
+ return len;
+}
+static DEVICE_ATTR(request, S_IRUGO | S_IWUSR, pwm_request_show, pwm_request_store);
+
+static const struct attribute *pwm_attrs[] = {
+ &dev_attr_tick_hz.attr,
+ &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 struct class_attribute pwm_class_attrs[] = {
+ __ATTR_NULL,
+};
+
+static struct class pwm_class = {
+ .name = "pwm",
+ .owner = THIS_MODULE,
+
+ .class_attrs = pwm_class_attrs,
+};
+
+int pwm_register_byname(struct pwm_device *p, struct device *parent,
+ const char *name)
+{
+ struct device *d;
+ int ret;
+
+ if (!p->ops || !p->ops->config)
+ return -EINVAL;
+
+ mutex_lock(&device_list_mutex);
+
+ d = class_find_device(&pwm_class, NULL, (char*)name, pwm_match_name);
+ if (d) {
+ ret = -EEXIST;
+ goto err_found_device;
+ }
+
+ p->dev = device_create(&pwm_class, parent, MKDEV(0, 0), NULL, name);
+ if (IS_ERR(p->dev)) {
+ ret = PTR_ERR(p->dev);
+ goto err_device_create;
+ }
+
+ ret = sysfs_create_group(&p->dev->kobj, &pwm_device_attr_group);
+ if (ret)
+ goto err_create_group;
+
+ dev_set_drvdata(p->dev, p);
+ p->flags = BIT(FLAG_REGISTERED);
+
+ goto done;
+
+err_create_group:
+ device_unregister(p->dev);
+ p->flags = 0;
+
+err_device_create:
+err_found_device:
+done:
+ mutex_unlock(&device_list_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(pwm_register_byname);
+
+int pwm_register(struct pwm_device *p, struct device *parent, int id)
+{
+ int ret;
+ char name[256];
+
+ if (IS_ERR_OR_NULL(parent))
+ return -EINVAL;
+
+ if (id == -1)
+ ret = scnprintf(name, sizeof name, "%s", dev_name(parent));
+ else
+ ret = scnprintf(name, sizeof name, "%s:%d", dev_name(parent), id);
+ if (ret <= 0 || ret >= sizeof name)
+ return -EINVAL;
+
+ return pwm_register_byname(p, parent, name);
+}
+EXPORT_SYMBOL(pwm_register);
+
+int pwm_unregister(struct pwm_device *p)
+{
+ int ret = 0;
+
+ mutex_lock(&device_list_mutex);
+
+ if (pwm_is_running(p) || pwm_is_requested(p)) {
+ ret = -EBUSY;
+ goto done;
+ }
+
+ sysfs_remove_group(&p->dev->kobj, &pwm_device_attr_group);
+ device_unregister(p->dev);
+ p->flags = 0;
+
+done:
+ mutex_unlock(&device_list_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL(pwm_unregister);
+
+static int __init pwm_init(void)
+{
+ pwm_handler_workqueue = create_singlethread_workqueue("pwm");
+ if (IS_ERR_OR_NULL(pwm_handler_workqueue)) {
+ pr_err("%s: failed to create PWM workqueue; aborting\n",
+ __func__);
+ return -ENOMEM;
+ }
+
+ return class_register(&pwm_class);
+}
+
+static void __exit pwm_exit(void)
+{
+ destroy_workqueue(pwm_handler_workqueue);
+ class_unregister(&pwm_class);
+}
+
+postcore_initcall(pwm_init);
+module_exit(pwm_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bill Gatliff <[email protected]>");
+MODULE_DESCRIPTION("Generic PWM device API implementation");
diff --git a/include/linux/pwm/pwm.h b/include/linux/pwm/pwm.h
new file mode 100644
index 0000000..1447333
--- /dev/null
+++ b/include/linux/pwm/pwm.h
@@ -0,0 +1,155 @@
+/*
+ * Copyright (C) 2011 Bill Gatliff < [email protected]>
+ * Copyright (C) 2011 Arun Murthy <[email protected]>
+ *
+ * This program is free software; you may redistribute and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+#ifndef __LINUX_PWM_H
+#define __LINUX_PWM_H
+
+enum {
+ FLAG_REGISTERED = 0,
+ FLAG_REQUESTED = 1,
+ FLAG_STOP = 2,
+ FLAG_RUNNING = 3,
+ FLAG_EXPORTED = 4,
+};
+
+enum {
+ PWM_CONFIG_DUTY_TICKS = 0,
+ PWM_CONFIG_PERIOD_TICKS = 1,
+ PWM_CONFIG_POLARITY = 2,
+ PWM_CONFIG_START = 3,
+ PWM_CONFIG_STOP = 4,
+
+ PWM_CONFIG_ENABLE_CALLBACK = 9,
+ PWM_CONFIG_DISABLE_CALLBACK = 10,
+};
+
+struct pwm_config;
+struct pwm_device;
+
+typedef int (*pwm_handler_t)(struct pwm_device *p, void *data);
+typedef void (*pwm_callback_t)(struct pwm_device *p);
+
+struct pwm_device_ops {
+ int (*request) (struct pwm_device *p);
+ void (*release) (struct pwm_device *p);
+ int (*config) (struct pwm_device *p,
+ struct pwm_config *c);
+ int (*config_nosleep) (struct pwm_device *p,
+ struct pwm_config *c);
+ int (*synchronize) (struct pwm_device *p,
+ struct pwm_device *to_p);
+ int (*unsynchronize) (struct pwm_device *p,
+ struct pwm_device *from_p);
+};
+
+struct pwm_config {
+ unsigned long config_mask;
+
+ unsigned long duty_ticks;
+ unsigned long period_ticks;
+ int polarity;
+};
+
+struct pwm_device {
+ struct device *dev;
+ const struct pwm_device_ops *ops;
+
+ const void *data;
+
+ const char *label;
+
+ unsigned long flags;
+
+ unsigned long tick_hz;
+
+ struct work_struct handler_work;
+ pwm_handler_t handler;
+ void *handler_data;
+
+ int active_high;
+ unsigned long period_ticks;
+ unsigned long duty_ticks;
+};
+
+struct pwm_device *pwm_request_byname(const char *name, const char *label);
+struct pwm_device *pwm_request(const char *bus_id, int id, const char *label);
+void pwm_release(struct pwm_device *p);
+
+static inline int pwm_is_registered(const struct pwm_device *p)
+{
+ return test_bit(FLAG_REGISTERED, &p->flags);
+}
+
+static inline int pwm_is_requested(const struct pwm_device *p)
+{
+ return test_bit(FLAG_REQUESTED, &p->flags);
+}
+
+static inline int pwm_is_running(const struct pwm_device *p)
+{
+ return test_bit(FLAG_RUNNING, &p->flags);
+}
+
+static inline int pwm_is_exported(const struct pwm_device *p)
+{
+ return test_bit(FLAG_EXPORTED, &p->flags);
+}
+
+static inline void pwm_set_drvdata(struct pwm_device *p, const void *data)
+{
+ p->data = data;
+}
+
+static inline void *pwm_get_drvdata(const struct pwm_device *p)
+{
+ return (void*)p->data;
+}
+
+
+int pwm_register(struct pwm_device *p, struct device *parent, int id);
+int pwm_register_byname(struct pwm_device *p, struct device *parent,
+ const char *name);
+int pwm_unregister(struct pwm_device *p);
+
+int pwm_set(struct pwm_device *p, unsigned long period_ns,
+ unsigned long duty_ns, int active_high);
+
+int pwm_set_period_ns(struct pwm_device *p, unsigned long period_ns);
+unsigned long pwm_get_period_ns(struct pwm_device *p);
+
+int pwm_set_duty_ns(struct pwm_device *p, unsigned long duty_ns);
+unsigned long pwm_get_duty_ns(struct pwm_device *p);
+
+int pwm_set_polarity(struct pwm_device *p, int active_high);
+
+int pwm_start(struct pwm_device *p);
+int pwm_stop(struct pwm_device *p);
+
+int pwm_config_nosleep(struct pwm_device *p, struct pwm_config *c);
+int pwm_config(struct pwm_device *p, struct pwm_config *c);
+
+int pwm_synchronize(struct pwm_device *p, struct pwm_device *to_p);
+int pwm_unsynchronize(struct pwm_device *p, struct pwm_device *from_p);
+int pwm_set_handler(struct pwm_device *p, pwm_handler_t handler, void *data);
+
+void pwm_callback(struct pwm_device *p);
+
+struct pwm_device *gpio_pwm_create(int gpio);
+int gpio_pwm_destroy(struct pwm_device *p);
+
+#endif
--
1.7.2.3
Driver to allow the Atmel PWMC peripheral found on various
AT91 SoCs to be controlled using the Generic PWM framework.
Tested on the AT91SAM9263.
Signed-off-by: Bill Gatliff <[email protected]>
---
drivers/pwm/Kconfig | 8 +
drivers/pwm/Makefile | 1 +
drivers/pwm/atmel-pwmc.c | 494 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 503 insertions(+), 0 deletions(-)
create mode 100644 drivers/pwm/atmel-pwmc.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 476de67..560324a 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -19,3 +19,11 @@ config GPIO_PWM
To compile this feature as a module, chose M here; the module
will be called gpio-pwm. If unsure, say N.
+
+config ATMEL_PWMC
+ tristate "Atmel AT32/AT91 PWMC support"
+ depends on GENERIC_PWM && (AVR32 || ARCH_AT91SAM9263 || ARCH_AT91SAM9RL || ARCH_AT91CAP9)
+ help
+ This option enables support under the generic PWM
+ framework for PWMC peripheral channels found on
+ certain Atmel microcontrollers. If unsure, say N.
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index ecec3e4..d274fa0 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_GENERIC_PWM) := pwm.o
obj-$(CONFIG_GPIO_PWM) += gpio-pwm.o
+obj-$(CONFIG_ATMEL_PWMC) += atmel-pwmc.o
diff --git a/drivers/pwm/atmel-pwmc.c b/drivers/pwm/atmel-pwmc.c
new file mode 100644
index 0000000..491ec88
--- /dev/null
+++ b/drivers/pwm/atmel-pwmc.c
@@ -0,0 +1,494 @@
+/*
+ * Atmel PWMC peripheral driver
+ *
+ * Copyright (C) 2011 Bill Gatliff <[email protected]>
+ * Copyright (C) 2007 David Brownell
+ *
+ * This program is free software; you may redistribute and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, 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/slab.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/completion.h>
+#include <linux/pwm/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,
+};
+
+/* TODO: NCHAN==4 only for certain AT91-ish parts! */
+#define NCHAN 4
+struct atmel_pwmc {
+ struct pwm_device p[NCHAN];
+ struct pwm_device_ops ops;
+ spinlock_t lock;
+ struct completion complete;
+ void __iomem *iobase;
+ struct clk *clk;
+ int irq;
+ u32 ccnt_mask;
+};
+
+/* TODO: debugfs attributes for peripheral register values */
+
+static inline void pwmc_writel(const struct atmel_pwmc *p, unsigned offset, u32 val)
+{
+ __raw_writel(val, p->iobase + offset);
+}
+
+static inline u32 pwmc_readl(const struct atmel_pwmc *p, unsigned offset)
+{
+ return __raw_readl(p->iobase + offset);
+}
+
+static void pwmc_chan_writel(const struct pwm_device *p,
+ u32 offset, u32 val)
+{
+ const struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int chan = p - &ap->p[0];
+
+ 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
+ + (chan * PWMC_CHAN_STRIDE), val);
+}
+
+static u32 pwmc_chan_readl(const struct pwm_device *p, u32 offset)
+{
+ const struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int chan = p - &ap->p[0];
+
+ return pwmc_readl(ap, offset + PWMC_CHAN_BASE
+ + (chan * PWMC_CHAN_STRIDE));
+}
+
+static int __atmel_pwmc_is_on(struct pwm_device *p)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int chan = p - &ap->p[0];
+
+ return (pwmc_readl(ap, PWMC_SR) & BIT(chan)) ? 1 : 0;
+}
+
+static void __atmel_pwmc_stop(struct pwm_device *p)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int chan = p - &ap->p[0];
+
+ pwmc_writel(ap, PWMC_DIS, BIT(chan));
+}
+
+static void __atmel_pwmc_start(struct pwm_device *p)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int chan = p - &ap->p[0];
+
+ pwmc_writel(ap, PWMC_ENA, BIT(chan));
+}
+
+static void __atmel_pwmc_config_polarity(struct pwm_device *p,
+ struct pwm_config *c)
+{
+ unsigned long cmr = pwmc_chan_readl(p, PWMC_CMR);
+
+ if (c->polarity)
+ clear_bit(PWMC_CMR_CPOL, &cmr);
+ else
+ set_bit(PWMC_CMR_CPOL, &cmr);
+ pwmc_chan_writel(p, PWMC_CMR, cmr);
+ p->active_high = c->polarity ? 1 : 0;
+
+ dev_dbg(p->dev, "polarity %d\n", c->polarity);
+}
+
+static void __atmel_pwmc_config_duty_ticks(struct pwm_device *p,
+ struct pwm_config *c)
+{
+ unsigned long cmr, cprd, cpre, cdty;
+
+ cmr = pwmc_chan_readl(p, PWMC_CMR);
+ cprd = pwmc_chan_readl(p, PWMC_CPRD);
+
+ cpre = cmr & PWMC_CMR_CPRE_MASK;
+ clear_bit(PWMC_CMR_CPD, &cmr);
+
+ cdty = cprd - (c->duty_ticks >> cpre);
+
+ p->duty_ticks = c->duty_ticks;
+
+ if (__atmel_pwmc_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);
+
+ dev_dbg(p->dev, "duty_ticks = %lu cprd = %lx"
+ " cdty = %lx cpre = %lx\n", p->duty_ticks,
+ cprd, cdty, cpre);
+}
+
+static void __atmel_pwmc_config_period_ticks(struct pwm_device *p,
+ struct pwm_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;
+
+ dev_dbg(p->dev, "period_ticks = %lu cprd = %x cpre = %x\n",
+ p->period_ticks, cprd, cpre);
+}
+
+static int atmel_pwmc_config_nosleep(struct pwm_device *p, struct pwm_config *c)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int ret = 0;
+ unsigned long flags;
+ int chan = p - &ap->p[0];
+
+ spin_lock_irqsave(&ap->lock, flags);
+
+ switch (c->config_mask) {
+
+ case BIT(PWM_CONFIG_DUTY_TICKS):
+ __atmel_pwmc_config_duty_ticks(p, c);
+ break;
+
+ case BIT(PWM_CONFIG_STOP):
+ __atmel_pwmc_stop(p);
+ break;
+
+ case BIT(PWM_CONFIG_START):
+ __atmel_pwmc_start(p);
+ break;
+
+ case BIT(PWM_CONFIG_POLARITY):
+ __atmel_pwmc_config_polarity(p, c);
+ break;
+
+ case BIT(PWM_CONFIG_ENABLE_CALLBACK):
+ pwmc_writel(ap, PWMC_IER, BIT(chan));
+ break;
+
+ case BIT(PWM_CONFIG_DISABLE_CALLBACK):
+ pwmc_writel(ap, PWMC_IDR, BIT(chan));
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ spin_unlock_irqrestore(&ap->lock, flags);
+ return ret;
+}
+
+static int atmel_pwmc_stop_sync(struct pwm_device *p)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ int was_on = __atmel_pwmc_is_on(p);
+ int chan = p - &ap->p[0];
+ int ret;
+
+ if (!was_on)
+ return 0;
+
+ do {
+ init_completion(&ap->complete);
+ set_bit(FLAG_STOP, &p->flags);
+ pwmc_writel(ap, PWMC_IER, BIT(chan));
+
+ dev_dbg(p->dev, "waiting on stop_sync completion...\n");
+
+ ret = wait_for_completion_interruptible(&ap->complete);
+
+ dev_dbg(p->dev, "stop_sync complete (%d)\n", ret);
+
+ if (ret)
+ return ret;
+ } while (test_bit(FLAG_STOP, &p->flags));
+
+ return 1;
+}
+
+static int atmel_pwmc_config(struct pwm_device *p, struct pwm_config *c)
+{
+ int was_on = 0;
+
+ if (p->ops->config_nosleep) {
+ if (!p->ops->config_nosleep(p, c))
+ return 0;
+ }
+
+ might_sleep();
+
+ dev_dbg(p->dev, "config_mask %lx\n", c->config_mask);
+
+ was_on = atmel_pwmc_stop_sync(p);
+ if (was_on < 0)
+ return was_on;
+
+ if (test_bit(PWM_CONFIG_PERIOD_TICKS, &c->config_mask)) {
+ __atmel_pwmc_config_period_ticks(p, c);
+ if (!test_bit(PWM_CONFIG_DUTY_TICKS, &c->config_mask)) {
+ struct pwm_config d = {
+ .config_mask = PWM_CONFIG_DUTY_TICKS,
+ .duty_ticks = p->duty_ticks,
+ };
+ __atmel_pwmc_config_duty_ticks(p, &d);
+ }
+ }
+
+ if (test_bit(PWM_CONFIG_DUTY_TICKS, &c->config_mask))
+ __atmel_pwmc_config_duty_ticks(p, c);
+
+ if (test_bit(PWM_CONFIG_POLARITY, &c->config_mask))
+ __atmel_pwmc_config_polarity(p, c);
+
+ if (test_bit(PWM_CONFIG_START, &c->config_mask)
+ || (was_on && !test_bit(PWM_CONFIG_STOP, &c->config_mask)))
+ __atmel_pwmc_start(p);
+
+ return 0;
+}
+
+static int atmel_pwmc_request(struct pwm_device *p)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ unsigned long flags;
+
+ spin_lock_irqsave(&ap->lock, flags);
+ clk_enable(ap->clk);
+ p->tick_hz = clk_get_rate(ap->clk);
+ __atmel_pwmc_stop(p);
+ spin_unlock_irqrestore(&ap->lock, flags);
+
+ return 0;
+}
+
+static void atmel_pwmc_release(struct pwm_device *p)
+{
+ struct atmel_pwmc *ap = pwm_get_drvdata(p);
+ clk_disable(ap->clk);
+}
+
+static irqreturn_t atmel_pwmc_irq(int irq, void *data)
+{
+ struct atmel_pwmc *ap = data;
+ struct pwm_device *p;
+ u32 isr;
+ int chan;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ap->lock, flags);
+
+ isr = pwmc_readl(ap, PWMC_ISR);
+ for (chan = 0; isr; chan++, isr >>= 1) {
+ p = &ap->p[chan];
+ if (isr & 1) {
+ pwm_callback(p);
+ if (test_bit(FLAG_STOP, &p->flags)) {
+ __atmel_pwmc_stop(p);
+ clear_bit(FLAG_STOP, &p->flags);
+ }
+ complete_all(&ap->complete);
+ }
+ }
+
+ spin_unlock_irqrestore(&ap->lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+static int __devinit atmel_pwmc_probe(struct platform_device *pdev)
+{
+ struct atmel_pwmc *ap;
+ struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ int chan;
+ int ret = 0;
+
+ ap = kzalloc(sizeof *ap, GFP_KERNEL);
+ if (!ap) {
+ ret = -ENOMEM;
+ goto err_atmel_pwmc_alloc;
+ }
+
+ spin_lock_init(&ap->lock);
+ init_completion(&ap->complete);
+ platform_set_drvdata(pdev, ap);
+
+ /* TODO: the datasheets are unclear as to how large CCNT
+ * actually is across all adopters of the PWMC; sixteen bits
+ * seems a safe assumption for now */
+ ap->ccnt_mask = 0xffffUL;
+
+ ap->ops.request = atmel_pwmc_request;
+ ap->ops.release = atmel_pwmc_release;
+ ap->ops.config_nosleep = atmel_pwmc_config_nosleep;
+ ap->ops.config = atmel_pwmc_config;
+
+ ap->clk = clk_get(&pdev->dev, "pwm_clk");
+ if (IS_ERR(ap->clk)) {
+ ret = -ENODEV;
+ goto err_clk_get;
+ }
+
+ ap->iobase = ioremap_nocache(r->start, resource_size(r));
+ 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);
+
+ for (chan = 0; chan < NCHAN; chan++) {
+ ap->p[chan].ops = &ap->ops;
+ pwm_set_drvdata(&ap->p[chan], ap);
+ ret = pwm_register(&ap->p[chan], &pdev->dev, chan);
+ if (ret)
+ goto err_pwm_register;
+ }
+
+ ap->irq = platform_get_irq(pdev, 0);
+ if (ap->irq != -ENXIO) {
+ ret = request_irq(ap->irq, atmel_pwmc_irq, 0,
+ dev_name(&pdev->dev), ap);
+ if (ret)
+ goto err_request_irq;
+ }
+
+ return 0;
+
+err_request_irq:
+err_pwm_register:
+ while (--chan > 0)
+ pwm_unregister(&ap->p[chan]);
+
+ iounmap(ap->iobase);
+err_ioremap:
+ clk_put(ap->clk);
+err_clk_get:
+ platform_set_drvdata(pdev, NULL);
+ kfree(ap);
+err_atmel_pwmc_alloc:
+ dev_dbg(&pdev->dev, "%s: error, returning %d\n", __func__, ret);
+ return ret;
+}
+
+static int __devexit atmel_pwmc_remove(struct platform_device *pdev)
+{
+ struct atmel_pwmc *ap = platform_get_drvdata(pdev);
+ int chan;
+
+ for (chan = 0; chan < NCHAN; chan++)
+ pwm_unregister(&ap->p[chan]);
+
+ 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_pwmc_driver = {
+ .driver = {
+ /* note: this name has to match the one in at91*_devices.c */
+ .name = "atmel_pwmc",
+ .owner = THIS_MODULE,
+ },
+ .probe = atmel_pwmc_probe,
+ .remove = __devexit_p(atmel_pwmc_remove),
+};
+
+static int __init atmel_pwmc_init(void)
+{
+ return platform_driver_register(&atmel_pwmc_driver);
+}
+module_init(atmel_pwmc_init);
+
+static void __exit atmel_pwmc_exit(void)
+{
+ platform_driver_unregister(&atmel_pwmc_driver);
+}
+module_exit(atmel_pwmc_exit);
+
+MODULE_AUTHOR("Bill Gatliff <[email protected]>");
+MODULE_DESCRIPTION("Driver for Atmel PWMC peripheral");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atmel_pwmc");
--
1.7.2.3
Emulates a PWM device using a GPIO pin and an hrtimer. Subject
to CPU, scheduler and hardware limitations, can support many
PWM outputs, e.g. as many as you have GPIO pins available for.
On a 200 MHz ARM9 processor, a PWM frequency of 100 Hz can be attained
with this code so long as the duty cycle remains between about 20-80%.
At higher or lower duty cycles, the transition events may arrive too
close for the scheduler and CPU to reliably service.
This driver supports creation of new GPIO+hrtimer PWM devices via
configfs:
# mount config /config -t configfs
# mkdir /config/gpio-pwm/<gpio number>
The new PWM device will appear as /sys/class/pwm/gpio-pwm.<gpio number>.
Caveats:
* The GPIO pin number must be valid, not already in use
* The output state of the GPIO pin is configured when the PWM starts
running i.e. not immediately upon request, because the polarity of
the inactive state of the pin isn't known until the pwm device's
'polarity' attribute is configured
* After creating and binding the pwm device, you must then request
it by reading from /sys/class/pwm/gpio-pwm.<gpio number>/request
Unbind and destroy the pwm device by first stopping and unrequesting
the pwm device under sysfs as usual; then do:
# rm -rf /config/gpio-pwm/<gpio number>
Signed-off-by: Bill Gatliff <[email protected]>
---
drivers/pwm/Kconfig | 11 ++
drivers/pwm/Makefile | 2 +
drivers/pwm/gpio-pwm.c | 348 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 361 insertions(+), 0 deletions(-)
create mode 100644 drivers/pwm/gpio-pwm.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index bc550f7..476de67 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -8,3 +8,14 @@ menuconfig GENERIC_PWM
Enables PWM device support implemented via a generic
framework. If unsure, say N.
+config GPIO_PWM
+ tristate "GPIO+hrtimer PWM device emulation"
+ depends on GENERIC_PWM
+ help
+ When enabled, this feature emulates single-channel PWM
+ devices using high-resolution timers and GPIO pins. You may
+ create as many of these devices as desired, subject to CPU
+ throughput limitations and GPIO pin availability.
+
+ To compile this feature as a module, chose M here; the module
+ will be called gpio-pwm. If unsure, say N.
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 7baa201..ecec3e4 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -2,3 +2,5 @@
# Makefile for pwm devices
#
obj-$(CONFIG_GENERIC_PWM) := pwm.o
+
+obj-$(CONFIG_GPIO_PWM) += gpio-pwm.o
diff --git a/drivers/pwm/gpio-pwm.c b/drivers/pwm/gpio-pwm.c
new file mode 100644
index 0000000..39e2dc6
--- /dev/null
+++ b/drivers/pwm/gpio-pwm.c
@@ -0,0 +1,348 @@
+/*
+ * Emulates a PWM device using an hrtimer and GPIO pin
+ *
+ * Copyright (C) 2011 Bill Gatliff <[email protected]>
+ *
+ * This program is free software; you may redistribute and/or modify
+ * it under the terms of the GNU General Public License Version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, 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/hrtimer.h>
+#include <linux/err.h>
+#include <linux/workqueue.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/completion.h>
+#include <linux/configfs.h>
+#include <linux/pwm/pwm.h>
+
+#define DRIVER_NAME KBUILD_MODNAME
+
+struct gpio_pwm {
+ struct pwm_device pwm;
+ struct hrtimer t;
+ struct work_struct work;
+ spinlock_t lock;
+ struct completion complete;
+ int gpio;
+ int callback;
+ unsigned long polarity : 1;
+ unsigned long active : 1;
+ char name[16];
+};
+
+static inline struct gpio_pwm *to_gpio_pwm(const struct pwm_device *p)
+{
+ return container_of(p, struct gpio_pwm, pwm);
+}
+
+static void gpio_pwm_work(struct work_struct *work)
+{
+ struct gpio_pwm *gp = container_of(work, struct gpio_pwm, work);
+
+ gpio_direction_output(gp->gpio, !(!!gp->polarity ^ !!gp->active));
+}
+
+static enum hrtimer_restart gpio_pwm_timeout(struct hrtimer *t)
+{
+ struct gpio_pwm *gp = container_of(t, struct gpio_pwm, t);
+ struct pwm_device *p = &gp->pwm;
+
+ if (unlikely(p->duty_ticks == 0))
+ gp->active = 0;
+ else if (unlikely(p->duty_ticks == p->period_ticks))
+ gp->active = 1;
+ else
+ gp->active ^= 1;
+
+ if (gpio_cansleep(gp->gpio))
+ schedule_work(&gp->work);
+ else
+ gpio_pwm_work(&gp->work);
+
+ if (!gp->active && gp->callback)
+ pwm_callback(p);
+
+ if (unlikely(!gp->active && test_bit(FLAG_STOP, &p->flags))) {
+ clear_bit(FLAG_STOP, &p->flags);
+ complete_all(&gp->complete);
+ goto done;
+ }
+
+ if (gp->active)
+ hrtimer_forward_now(&gp->t, ktime_set(0, p->duty_ticks));
+ else
+ hrtimer_forward_now(&gp->t, ktime_set(0, p->period_ticks
+ - p->duty_ticks));
+
+done:
+ return HRTIMER_RESTART;
+}
+
+static void gpio_pwm_start(struct pwm_device *p)
+{
+ struct gpio_pwm *gp = to_gpio_pwm(p);
+
+ gp->active = 0;
+ hrtimer_start(&gp->t, ktime_set(0, p->period_ticks - p->duty_ticks),
+ HRTIMER_MODE_REL);
+ set_bit(FLAG_RUNNING, &p->flags);
+}
+
+static int gpio_pwm_config_nosleep(struct pwm_device *p, struct pwm_config *c)
+{
+ struct gpio_pwm *gp = to_gpio_pwm(p);
+ int ret = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gp->lock, flags);
+
+ switch (c->config_mask) {
+
+ case BIT(PWM_CONFIG_DUTY_TICKS):
+ p->duty_ticks = c->duty_ticks;
+ break;
+
+ case BIT(PWM_CONFIG_START):
+ if (!hrtimer_active(&gp->t)) {
+ gpio_pwm_start(p);
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ spin_unlock_irqrestore(&gp->lock, flags);
+ return ret;
+}
+
+static int gpio_pwm_stop_sync(struct pwm_device *p)
+{
+ struct gpio_pwm *gp = to_gpio_pwm(p);
+ int ret;
+ int was_on = hrtimer_active(&gp->t);
+
+ if (was_on) {
+ do {
+ init_completion(&gp->complete);
+ set_bit(FLAG_STOP, &p->flags);
+ ret = wait_for_completion_interruptible(&gp->complete);
+ if (ret)
+ return ret;
+ } while (test_bit(FLAG_STOP, &p->flags));
+ }
+
+ clear_bit(FLAG_RUNNING, &p->flags);
+
+ return was_on;
+}
+
+static int gpio_pwm_config(struct pwm_device *p, struct pwm_config *c)
+{
+ struct gpio_pwm *gp = to_gpio_pwm(p);
+ int was_on = 0;
+
+ if (p->ops->config_nosleep) {
+ if (!p->ops->config_nosleep(p, c))
+ return 0;
+ }
+
+ might_sleep();
+
+ was_on = gpio_pwm_stop_sync(p);
+ if (was_on < 0)
+ return was_on;
+
+ if (test_bit(PWM_CONFIG_ENABLE_CALLBACK, &c->config_mask))
+ gp->callback = 1;
+ if (test_bit(PWM_CONFIG_DISABLE_CALLBACK, &c->config_mask))
+ gp->callback = 0;
+
+ if (test_bit(PWM_CONFIG_PERIOD_TICKS, &c->config_mask))
+ p->period_ticks = c->period_ticks;
+ if (test_bit(PWM_CONFIG_DUTY_TICKS, &c->config_mask))
+ p->duty_ticks = c->duty_ticks;
+ if (test_bit(PWM_CONFIG_POLARITY, &c->config_mask))
+ gp->polarity = !!c->polarity;
+
+ if (test_bit(PWM_CONFIG_START, &c->config_mask)
+ || (was_on && !test_bit(PWM_CONFIG_STOP, &c->config_mask)))
+ gpio_pwm_start(p);
+
+ return 0;
+}
+
+static int gpio_pwm_request(struct pwm_device *p)
+{
+ p->tick_hz = 1000000000UL;
+ return 0;
+}
+
+static const struct pwm_device_ops gpio_pwm_device_ops = {
+ .config = gpio_pwm_config,
+ .config_nosleep = gpio_pwm_config_nosleep,
+ .request = gpio_pwm_request,
+};
+
+struct pwm_device *gpio_pwm_create(int gpio)
+{
+ struct gpio_pwm *gp;
+ int ret = 0;
+
+ if (!gpio_is_valid(gpio))
+ return ERR_PTR(-EINVAL);
+
+ if (gpio_request(gpio, DRIVER_NAME))
+ return ERR_PTR(-EBUSY);
+
+ gp = kzalloc(sizeof(*gp), GFP_KERNEL);
+ if (!gp)
+ goto err_alloc;
+
+ pwm_set_drvdata(&gp->pwm, gp);
+ gp->pwm.ops = &gpio_pwm_device_ops;
+ gp->gpio = gpio;
+
+ INIT_WORK(&gp->work, gpio_pwm_work);
+ init_completion(&gp->complete);
+ hrtimer_init(&gp->t, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ gp->t.function = gpio_pwm_timeout;
+
+ scnprintf(gp->name, sizeof(gp->name), "%s:%d", DRIVER_NAME, gpio);
+ ret = pwm_register_byname(&gp->pwm, NULL, gp->name);
+ if (ret)
+ goto err_pwm_register;
+
+ return &gp->pwm;
+
+err_pwm_register:
+ kfree(gp);
+err_alloc:
+ gpio_free(gpio);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(gpio_pwm_create);
+
+int gpio_pwm_destroy(struct pwm_device *p)
+{
+ struct gpio_pwm *gp = pwm_get_drvdata(p);
+
+ if (pwm_is_requested(&gp->pwm)) {
+ if (pwm_is_running(&gp->pwm))
+ pwm_stop(&gp->pwm);
+ pwm_release(&gp->pwm);
+ }
+ hrtimer_cancel(&gp->t);
+ cancel_work_sync(&gp->work);
+
+ pwm_unregister(&gp->pwm);
+ gpio_free(gp->gpio);
+ kfree(gp);
+
+ return 0;
+}
+EXPORT_SYMBOL(gpio_pwm_destroy);
+
+#ifdef CONFIG_CONFIGFS_FS
+struct gpio_pwm_target {
+ struct config_item item;
+ struct pwm_device *p;
+};
+
+static struct config_item_type gpio_pwm_item_type = {
+ .ct_owner = THIS_MODULE,
+};
+
+static struct config_item *make_gpio_pwm_target(struct config_group *group,
+ const char *name)
+{
+ struct gpio_pwm_target *t;
+ unsigned long gpio;
+ int ret;
+
+ t = kzalloc(sizeof(*t), GFP_KERNEL);
+ if (!t)
+ return ERR_PTR(-ENOMEM);
+
+ ret = strict_strtoul(name, 10, &gpio);
+ if (ret || !gpio_is_valid(gpio)) {
+ ret = -EINVAL;
+ goto err_invalid_gpio;
+ }
+
+ config_item_init_type_name(&t->item, name, &gpio_pwm_item_type);
+
+ t->p = gpio_pwm_create(gpio);
+ if (IS_ERR_OR_NULL(t->p))
+ goto err_gpio_pwm_create;
+
+ return &t->item;
+
+err_gpio_pwm_create:
+err_invalid_gpio:
+ kfree(t);
+ return ERR_PTR(ret);
+}
+
+static void drop_gpio_pwm_target(struct config_group *group,
+ struct config_item *item)
+{
+ struct gpio_pwm_target *t =
+ container_of(item, struct gpio_pwm_target, item);
+
+ gpio_pwm_destroy(t->p);
+ config_item_put(&t->item);
+ kfree(t);
+}
+
+static struct configfs_group_operations gpio_pwm_subsys_group_ops = {
+ .make_item = make_gpio_pwm_target,
+ .drop_item = drop_gpio_pwm_target,
+};
+
+static struct config_item_type gpio_pwm_subsys_type = {
+ .ct_group_ops = &gpio_pwm_subsys_group_ops,
+ .ct_owner = THIS_MODULE,
+};
+
+static struct configfs_subsystem gpio_pwm_subsys = {
+ .su_group = {
+ .cg_item = {
+ .ci_name = DRIVER_NAME,
+ .ci_type = &gpio_pwm_subsys_type,
+ },
+ },
+};
+
+static int __init gpio_pwm_init(void)
+{
+ config_group_init(&gpio_pwm_subsys.su_group);
+ mutex_init(&gpio_pwm_subsys.su_mutex);
+ return configfs_register_subsystem(&gpio_pwm_subsys);
+}
+module_init(gpio_pwm_init);
+
+static void __exit gpio_pwm_exit(void)
+{
+ configfs_unregister_subsystem(&gpio_pwm_subsys);
+}
+module_exit(gpio_pwm_exit);
+#endif
+
+MODULE_AUTHOR("Bill Gatliff <[email protected]>");
+MODULE_DESCRIPTION("PWM channel emulator using GPIO and a high-resolution timer");
+MODULE_LICENSE("GPL");
--
1.7.2.3
On 03/06/2011 07:16 AM, Lars-Peter Clausen wrote:
> On 03/06/2011 05:17 AM, Bill Gatliff wrote:
>> +
>> +
>> +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.
>> +
This looks like a poor man's IRQ handler implementation. In my opinion something
like pwm_to_irq would be better suited interface.
- Lars
On 03/06/2011 05:17 AM, Bill Gatliff wrote:
> Updates the existing PWM-related functions to support multiple
> and/or hotplugged PWM devices, and adds a sysfs interface.
> Moves the code to drivers/pwm.
>
> For now, this new code can exist alongside the current PWM
> implementations; the existing implementations will be migrated
> to this new framework as time permits. Eventually, the current
> PWM implementation will be deprecated and then expunged.
>
> Signed-off-by: Bill Gatliff <[email protected]>
> ---
> Documentation/pwm.txt | 277 +++++++++++++++++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 2 +
> drivers/pwm/Kconfig | 10 +
> drivers/pwm/Makefile | 4 +
> drivers/pwm/pwm.c | 610 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pwm/pwm.h | 155 ++++++++++++
> 7 files changed, 1060 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/pwm.txt
> create mode 100644 drivers/pwm/Kconfig
> create mode 100644 drivers/pwm/Makefile
> create mode 100644 drivers/pwm/pwm.c
> create mode 100644 include/linux/pwm/pwm.h
>
> diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
> new file mode 100644
> index 0000000..2b15395
> --- /dev/null
> +++ b/Documentation/pwm.txt
> @@ -0,0 +1,277 @@
> + Generic PWM Device API
> +
> + February 7, 2011
> + Bill Gatliff
> + <[email protected]>
> +
> +
> +
> +The code in drivers/pwm and include/linux/pwm/ 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 suddenly 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 accommodate 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, sensors and solenoids through a
> +custom-designed, optically-isolated H-bridge driver.
> +
> +
> +
> +Functional Overview
> +
> +The Generic PWM Device API framework is implemented in
> +include/linux/pwm/pwm.h and drivers/pwm/pwm.c. The functions therein
> +use information from pwm_device and pwm__config structures to invoke
> +services in PWM peripheral device drivers. Consult
> +drivers/pwm/atmel-pwmc.c for an example driver for the Atmel PWMC
> +peripheral.
> +
> +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 Generic PWM API framework also provides a sysfs interface to PWM
> +devices, which is adequate for basic application needs and testing.
> +
> +Driver authors fill out a pwm_device structure, which describes the
> +capabilities of the PWM hardware being utilized. 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 as users begin to
> +configure and utilize the hardware.
> +
> +Many PWM-capable peripherals provide two, three, or more channels of
> +PWM output. The driver author completes and registers a pwm_device
> +structure for each channel they wish to be supported by the Generic
> +PWM API.
> +
> +Note that PWM signals can be produced by a variety of peripherals,
> +beyond the true PWM peripherals 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 be accommodated by the Generic PWM Device API framework.
> +
> +
> +
> +Using the API to Generate PWM Signals -- Basic Functions for Users
> +
> +
> +pwm_request() -- Returns a pwm_device 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_release() 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_release() -- 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_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 by the config structure.
> +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_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, timer
> +handlers, and other interrupt contexts, but must confine their
> +configuration changes to only those that the driver can implement
> +without sleeping. 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. By default, all PWM
> +channels are unsynchronized.
> +
> +
> +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 plain-text 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 Atmel PWMC
> +platform 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 dev_name(pdev->dev) in your
> +probe() method).
> +
> +
> +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) Invoked 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.
> +
> +
> +set_callback -- (optional) If the hardware supports an end-of-period
> +interrupt, invoke the function provided in this callback during the
> +device's 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-compatible 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.
> +
> +
> +
> +FAQs and Additional Notes
> +
> +The Atmel PWMC pwm_config() function tries to satisfy the user's
> +configuration request by first invoking pwm_config_nosleep(). If that
> +operation fails, then the PWM peripheral is brought to a synchronized
> +stop, the configuration changes are made, and the device is restarted.
> +
> +The Atmel PWMC's use of pwm_config_nosleep() from pwm_config()
> +minimizes redundant code between the two functions, and relieves the
> +pwm_config() function of the need to explicitly test whether a
> +requested configuration change can be carried out while the PWM device
> +is in its current mode.
> +
> +PWM API driver authors are encouraged to adopt the Atmel PWMC's
> +pwm_config()-vs.-pwm_config_nosleep() strategy in implementations for
> +other devices as well.
> +
> +
> +
> +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. ;)
Two minor remarks on the documentation: You are not mentioning the functions
arguments and there is no documentation on the sysfs interface at all.
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 9bfb71f..413e4f9 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -56,6 +56,8 @@ source "drivers/pps/Kconfig"
>
> source "drivers/gpio/Kconfig"
>
> +source "drivers/pwm/Kconfig"
> +
> source "drivers/w1/Kconfig"
>
> source "drivers/power/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index b423bb1..4e37abf 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -6,6 +6,8 @@
> #
>
> obj-y += gpio/
> +obj-$(CONFIG_GENERIC_PWM) += pwm/
> +
> obj-$(CONFIG_PCI) += pci/
> obj-$(CONFIG_PARISC) += parisc/
> obj-$(CONFIG_RAPIDIO) += rapidio/
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> new file mode 100644
> index 0000000..bc550f7
> --- /dev/null
> +++ b/drivers/pwm/Kconfig
> @@ -0,0 +1,10 @@
> +#
> +# PWM infrastructure and devices
> +#
> +
> +menuconfig GENERIC_PWM
> + tristate "PWM Support"
> + help
> + Enables PWM device support implemented via a generic
> + framework. If unsure, say N.
> +
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> new file mode 100644
> index 0000000..7baa201
> --- /dev/null
> +++ b/drivers/pwm/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for pwm devices
> +#
> +obj-$(CONFIG_GENERIC_PWM) := pwm.o
> diff --git a/drivers/pwm/pwm.c b/drivers/pwm/pwm.c
> new file mode 100644
> index 0000000..f958e4b
> --- /dev/null
> +++ b/drivers/pwm/pwm.c
> @@ -0,0 +1,610 @@
> +/*
> + * PWM API implementation
> + *
> + * Copyright (C) 2011 Bill Gatliff <[email protected]>
> + * Copyright (C) 2011 Arun Murthy <[email protected]>
> + *
> + * This program is free software; you may redistribute and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> + * USA
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/completion.h>
> +#include <linux/workqueue.h>
> +#include <linux/list.h>
> +#include <linux/sched.h>
> +#include <linux/pwm/pwm.h>
> +
> +static const char *REQUEST_SYSFS = "sysfs";
> +static LIST_HEAD(pwm_device_list);
The list is not used.
> +static DEFINE_MUTEX(device_list_mutex);
> +static struct class pwm_class;
> +static struct workqueue_struct *pwm_handler_workqueue;
> +
> +static int pwm_match_name(struct device *dev, void *name)
> +{
> + return !strcmp(name, dev_name(dev));
> +}
> +
> +static int __pwm_request(struct pwm_device *p, const char *label)
> +{
> + int ret;
> +
> + ret = test_and_set_bit(FLAG_REQUESTED, &p->flags);
> + if (ret) {
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + p->label = label;
> +
> + if (p->ops->request) {
> + ret = p->ops->request(p);
> + if (ret) {
> + clear_bit(FLAG_REQUESTED, &p->flags);
> + goto done;
> + }
> + }
> +
> +done:
> + return ret;
> +}
> +
> +static struct pwm_device *__pwm_request_byname(const char *name,
> + const char *label)
> +{
> + struct device *d;
> + struct pwm_device *p;
> + int ret;
> +
> + d = class_find_device(&pwm_class, NULL, (char*)name, pwm_match_name);
> + if (IS_ERR_OR_NULL(d))
> + return ERR_PTR(-EINVAL);
> +
> + p = dev_get_drvdata(d);
> + ret = __pwm_request(p, label);
> +
> + if (ret)
> + return ERR_PTR(ret);
> + return p;
> +}
> +
> +struct pwm_device *pwm_request_byname(const char *name, const char *label)
> +{
> + struct pwm_device *p;
> +
> + mutex_lock(&device_list_mutex);
> + p = __pwm_request_byname(name, label);
> + mutex_unlock(&device_list_mutex);
> + return p;
> +}
> +EXPORT_SYMBOL(pwm_request_byname);
> +
> +struct pwm_device *pwm_request(const char *bus_id, int id, const char *label)
> +{
> + char name[256];
> + int ret;
> +
> + if (id == -1)
> + ret = scnprintf(name, sizeof name, "%s", bus_id);
sizeof(name)
> + else
> + ret = scnprintf(name, sizeof name, "%s:%d", bus_id, id);
> + if (ret <= 0 || ret >= sizeof name)
> + return ERR_PTR(-EINVAL);
> +
> + return pwm_request_byname(name, label);
> +}
> +EXPORT_SYMBOL(pwm_request);
> +
> +void pwm_release(struct pwm_device *p)
> +{
> + mutex_lock(&device_list_mutex);
> +
> + if (!test_and_clear_bit(FLAG_REQUESTED, &p->flags)) {
> + WARN(1, "%s: releasing unrequested PWM device %s\n",
> + __func__, dev_name(p->dev));
> + goto done;
> + }
> +
> + pwm_stop(p);
> + pwm_unsynchronize(p, NULL);
> + pwm_set_handler(p, NULL, NULL);
> +
> + p->label = NULL;
> +
> + if (p->ops->release)
> + p->ops->release(p);
> +done:
> + mutex_unlock(&device_list_mutex);
> +}
> +EXPORT_SYMBOL(pwm_release);
> +
> +static unsigned long pwm_ns_to_ticks(struct pwm_device *p, unsigned long nsecs)
> +{
> + unsigned long long ticks;
> +
> + ticks = nsecs;
> + ticks *= p->tick_hz;
> + do_div(ticks, 1000000000);
> + return ticks;
> +}
> +
> +static unsigned long pwm_ticks_to_ns(struct pwm_device *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;
> +}
> +
> +int pwm_config_nosleep(struct pwm_device *p, struct pwm_config *c)
> +{
> + if (!p->ops->config_nosleep)
> + return -EINVAL;
Would ENOSYS be more appropriate?
> +
> + return p->ops->config_nosleep(p, c);
> +}
> +EXPORT_SYMBOL(pwm_config_nosleep);
> +
> +int pwm_config(struct pwm_device *p, struct pwm_config *c)
> +{
> + int ret = 0;
> +
> + switch (c->config_mask & (BIT(PWM_CONFIG_PERIOD_TICKS)
> + | BIT(PWM_CONFIG_DUTY_TICKS))) {
> + case BIT(PWM_CONFIG_PERIOD_TICKS):
> + if (p->duty_ticks > c->period_ticks) {
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + case BIT(PWM_CONFIG_DUTY_TICKS):
> + if (p->period_ticks < c->duty_ticks) {
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + case BIT(PWM_CONFIG_DUTY_TICKS) | BIT(PWM_CONFIG_PERIOD_TICKS):
> + if (c->duty_ticks > c->period_ticks) {
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + default:
> + break;
> + }
> +
> +err:
> + dev_dbg(p->dev, "%s: config_mask %lu period_ticks %lu "
> + "duty_ticks %lu polarity %d\n",
> + __func__, c->config_mask, c->period_ticks,
> + c->duty_ticks, c->polarity);
> +
> + if (ret)
> + return ret;
This looks a bit confusing. Maybe it would be better to move the dev_dbg at the
top of the function and the 'err' label at the end of the function.
> + return p->ops->config(p, c);
> +}
> +EXPORT_SYMBOL(pwm_config);
> +
> +int pwm_set(struct pwm_device *p, unsigned long period_ns,
> + unsigned long duty_ns, int active_high)
> +{
> + struct pwm_config c = {
> + .config_mask = (BIT(PWM_CONFIG_PERIOD_TICKS)
> + | BIT(PWM_CONFIG_DUTY_TICKS)
> + | BIT(PWM_CONFIG_POLARITY)),
> + .period_ticks = pwm_ns_to_ticks(p, period_ns),
> + .duty_ticks = pwm_ns_to_ticks(p, duty_ns),
> + .polarity = active_high
> + };
> +
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_set);
> +
> +int pwm_set_period_ns(struct pwm_device *p, unsigned long period_ns)
> +{
> + struct pwm_config c = {
> + .config_mask = BIT(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_device *p)
> +{
> + return pwm_ticks_to_ns(p, p->period_ticks);
> +}
> +EXPORT_SYMBOL(pwm_get_period_ns);
> +
> +int pwm_set_duty_ns(struct pwm_device *p, unsigned long duty_ns)
> +{
> + struct pwm_config c = {
> + .config_mask = BIT(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_device *p)
> +{
> + return pwm_ticks_to_ns(p, p->duty_ticks);
> +}
> +EXPORT_SYMBOL(pwm_get_duty_ns);
> +
> +int pwm_set_polarity(struct pwm_device *p, int active_high)
> +{
> + struct pwm_config c = {
> + .config_mask = BIT(PWM_CONFIG_POLARITY),
> + .polarity = active_high,
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_set_polarity);
> +
> +int pwm_start(struct pwm_device *p)
> +{
> + struct pwm_config c = {
> + .config_mask = BIT(PWM_CONFIG_START),
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_start);
> +
> +int pwm_stop(struct pwm_device *p)
> +{
> + struct pwm_config c = {
> + .config_mask = BIT(PWM_CONFIG_STOP),
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_stop);
> +
> +int pwm_synchronize(struct pwm_device *p, struct pwm_device *to_p)
> +{
> + if (!p->ops->synchronize)
> + return -EINVAL;
ENOSYS here too
> +
> + return p->ops->synchronize(p, to_p);
> +}
> +EXPORT_SYMBOL(pwm_synchronize);
> +
> +int pwm_unsynchronize(struct pwm_device *p, struct pwm_device *from_p)
> +{
> + if (!p->ops->unsynchronize)
> + return -EINVAL;
and here.
> +
> + return p->ops->unsynchronize(p, from_p);
> +}
> +EXPORT_SYMBOL(pwm_unsynchronize);
> +
> +static void pwm_handler(struct work_struct *w)
> +{
> + struct pwm_device *p = container_of(w, struct pwm_device,
> + handler_work);
> + if (p->handler)
> + p->handler(p, p->handler_data);
> +}
> +
> +void pwm_callback(struct pwm_device *p)
> +{
> + queue_work(pwm_handler_workqueue, &p->handler_work);
> +}
> +EXPORT_SYMBOL(pwm_callback);
> +
> +int pwm_set_handler(struct pwm_device *p, pwm_handler_t handler, void *data)
> +{
> + struct pwm_config c;
> + int ret;
> +
> + if (handler)
> + c.config_mask = BIT(PWM_CONFIG_ENABLE_CALLBACK);
> + else
> + c.config_mask = BIT(PWM_CONFIG_DISABLE_CALLBACK);
> +
> + ret = pwm_config(p, &c);
There is a chance for a race here. The device could fire, for example due to an
irq, before the handler has been initalized.
> +
> + if (!ret && handler) {
> + p->handler_data = data;
> + p->handler = handler;
> + INIT_WORK(&p->handler_work, pwm_handler);
The INIT_WORK should probably into pwm_device_register.
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(pwm_set_handler);
> +
> +static ssize_t pwm_run_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_device *p = dev_get_drvdata(dev);
> + return sprintf(buf, "%d\n", pwm_is_running(p));
> +}
> +
> +static ssize_t pwm_run_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct pwm_device *p = dev_get_drvdata(dev);
> +
> + if (!pwm_is_exported(p))
> + return -EINVAL;
I'm not sure, but maybe -EPERM is better here.
> +
> + if (sysfs_streq(buf, "1"))
> + pwm_start(p);
> + else if (sysfs_streq(buf, "0"))
> + pwm_stop(p);
Maybe return -EINVAL if it is neither 0 or 1.
> +
> + return len;
> +}
> +static DEVICE_ATTR(run, S_IRUGO | S_IWUSR, pwm_run_show, pwm_run_store);
> +
> +static ssize_t pwm_tick_hz_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_device *p = dev_get_drvdata(dev);
> + return sprintf(buf, "%lu\n", p->tick_hz);
> +}
> +static DEVICE_ATTR(tick_hz, S_IRUGO, pwm_tick_hz_show, NULL);
> +
> +static ssize_t pwm_duty_ns_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_device *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_device *p = dev_get_drvdata(dev);
> +
> + if (!pwm_is_exported(p))
> + return -EINVAL;
> +
> + if (!strict_strtoul(buf, 10, &duty_ns))
> + pwm_set_duty_ns(p, duty_ns);
How about returning the error value of strict_strtou if it fails?
> + return len;
> +}
> +static DEVICE_ATTR(duty_ns, S_IRUGO | S_IWUSR, 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_device *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_device *p = dev_get_drvdata(dev);
> +
> + if (!pwm_is_exported(p))
> + return -EINVAL;
> +
> + if (!strict_strtoul(buf, 10, &period_ns))
> + pwm_set_period_ns(p, period_ns);
> + return len;
> +}
> +static DEVICE_ATTR(period_ns, S_IRUGO | S_IWUSR, 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_device *p = dev_get_drvdata(dev);
> + return sprintf(buf, "%d\n", p->active_high ? 1 : 0);
> +}
> +
> +static ssize_t pwm_polarity_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + unsigned long polarity;
> + struct pwm_device *p = dev_get_drvdata(dev);
> +
> + if (!pwm_is_exported(p))
> + return -EINVAL;
> +
> + if (!strict_strtoul(buf, 10, &polarity))
> + pwm_set_polarity(p, polarity);
> + return len;
> +}
> +static DEVICE_ATTR(polarity, S_IRUGO | S_IWUSR, pwm_polarity_show, pwm_polarity_store);
> +
> +static ssize_t pwm_request_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_device *p = dev_get_drvdata(dev);
> + int ret;
> +
> + mutex_lock(&device_list_mutex);
> + ret = __pwm_request(p, REQUEST_SYSFS);
> + mutex_unlock(&device_list_mutex);
> +
> + if (!ret)
> + set_bit(FLAG_EXPORTED, &p->flags);
> +
> + return sprintf(buf, "%s\n", p->label);
> +}
> +
> +static ssize_t pwm_request_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct pwm_device *p = dev_get_drvdata(dev);
> +
> + pwm_release(p);
> + clear_bit(FLAG_EXPORTED, &p->flags);
> + return len;
> +}
> +static DEVICE_ATTR(request, S_IRUGO | S_IWUSR, pwm_request_show, pwm_request_store);
So `cat request` requests the device and `echo > request` frees the device?
Thats a bit odd in my opinion.
> +
> +static const struct attribute *pwm_attrs[] = {
> + &dev_attr_tick_hz.attr,
> + &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 struct class_attribute pwm_class_attrs[] = {
> + __ATTR_NULL,
> +};
> +
> +static struct class pwm_class = {
> + .name = "pwm",
> + .owner = THIS_MODULE,
> +
> + .class_attrs = pwm_class_attrs,
Just leaving it NULL should work, if you don't want any class attributes.
> +};
> +
> +int pwm_register_byname(struct pwm_device *p, struct device *parent,
> + const char *name)
> +{
> + struct device *d;
> + int ret;
> +
> + if (!p->ops || !p->ops->config)
> + return -EINVAL;
> +
> + mutex_lock(&device_list_mutex);
> +
> + d = class_find_device(&pwm_class, NULL, (char*)name, pwm_match_name);
> + if (d) {
> + ret = -EEXIST;
> + goto err_found_device;
> + }
device_create should fail if a device with the same name already exits, so I'm
not sure if it makes sense to do the check here.
> +
> + p->dev = device_create(&pwm_class, parent, MKDEV(0, 0), NULL, name);
... NULL, "%s", name);
Or you could use device_create_vargs and get rid of that scnprintf in pwm_register.
> + if (IS_ERR(p->dev)) {
> + ret = PTR_ERR(p->dev);
> + goto err_device_create;
> + }
I think it would be better to embedd the device struct directly into the
pwm_device struct. You could also remove the data field of the pwm_device
struct and use dev_{get,set}_drvdata for pwm_{get,set}_drvdata.
> +
> + ret = sysfs_create_group(&p->dev->kobj, &pwm_device_attr_group);
> + if (ret)
> + goto err_create_group;
It should be possible to use the classes dev_attrs here instead.
> +
> + dev_set_drvdata(p->dev, p);
> + p->flags = BIT(FLAG_REGISTERED);
> +
> + goto done;
> +
> +err_create_group:
> + device_unregister(p->dev);
> + p->flags = 0;
> +
> +err_device_create:
> +err_found_device:
> +done:
> + mutex_unlock(&device_list_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(pwm_register_byname);
> +
> +int pwm_register(struct pwm_device *p, struct device *parent, int id)
> +{
> + int ret;
> + char name[256];
> +
> + if (IS_ERR_OR_NULL(parent))
> + return -EINVAL;
> +
> + if (id == -1)
> + ret = scnprintf(name, sizeof name, "%s", dev_name(parent));
> + else
> + ret = scnprintf(name, sizeof name, "%s:%d", dev_name(parent), id);
> + if (ret <= 0 || ret >= sizeof name)
> + return -EINVAL;
> +
> + return pwm_register_byname(p, parent, name);
> +}
> +EXPORT_SYMBOL(pwm_register);
> +
> +int pwm_unregister(struct pwm_device *p)
> +{
> + int ret = 0;
> +
> + mutex_lock(&device_list_mutex);
> +
> + if (pwm_is_running(p) || pwm_is_requested(p)) {
Is it possible that the pwm is running but not requested?
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + sysfs_remove_group(&p->dev->kobj, &pwm_device_attr_group);
> + device_unregister(p->dev);
> + p->flags = 0;
> +
> +done:
> + mutex_unlock(&device_list_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(pwm_unregister);
> +
> +static int __init pwm_init(void)
> +{
> + pwm_handler_workqueue = create_singlethread_workqueue("pwm");
> + if (IS_ERR_OR_NULL(pwm_handler_workqueue)) {
> + pr_err("%s: failed to create PWM workqueue; aborting\n",
> + __func__);
> + return -ENOMEM;
> + }
> +
> + return class_register(&pwm_class);
> +}
> +
> +static void __exit pwm_exit(void)
> +{
> + destroy_workqueue(pwm_handler_workqueue);
> + class_unregister(&pwm_class);
> +}
> +
> +postcore_initcall(pwm_init);
> +module_exit(pwm_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Bill Gatliff <[email protected]>");
> +MODULE_DESCRIPTION("Generic PWM device API implementation");
> diff --git a/include/linux/pwm/pwm.h b/include/linux/pwm/pwm.h
> new file mode 100644
> index 0000000..1447333
> --- /dev/null
> +++ b/include/linux/pwm/pwm.h
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright (C) 2011 Bill Gatliff < [email protected]>
> + * Copyright (C) 2011 Arun Murthy <[email protected]>
> + *
> + * This program is free software; you may redistribute and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> + * USA
> + */
> +#ifndef __LINUX_PWM_H
> +#define __LINUX_PWM_H
> +
> +enum {
> + FLAG_REGISTERED = 0,
Does the REGISTERED flag makes any sense, I mean are there any usecases for
pwm_is_registered? Non registered pwm_devices should not be visible to the
outside world, so if you got an pointer to an pwm_device outside of an
pwm_device driver it should also be registered. And inside a pwm_device driver
the driver should know whether it already registered the pwm_device or not.
> + FLAG_REQUESTED = 1,
> + FLAG_STOP = 2,
> + FLAG_RUNNING = 3,
> + FLAG_EXPORTED = 4,
> +};
> +
> +enum {
> + PWM_CONFIG_DUTY_TICKS = 0,
> + PWM_CONFIG_PERIOD_TICKS = 1,
> + PWM_CONFIG_POLARITY = 2,
> + PWM_CONFIG_START = 3,
> + PWM_CONFIG_STOP = 4,
> +
> + PWM_CONFIG_ENABLE_CALLBACK = 9,
> + PWM_CONFIG_DISABLE_CALLBACK = 10,
> +};
> +
> +struct pwm_config;
> +struct pwm_device;
> +
> +typedef int (*pwm_handler_t)(struct pwm_device *p, void *data);
> +typedef void (*pwm_callback_t)(struct pwm_device *p);
> +
> +struct pwm_device_ops {
> + int (*request) (struct pwm_device *p);
> + void (*release) (struct pwm_device *p);
> + int (*config) (struct pwm_device *p,
> + struct pwm_config *c);
> + int (*config_nosleep) (struct pwm_device *p,
> + struct pwm_config *c);
> + int (*synchronize) (struct pwm_device *p,
> + struct pwm_device *to_p);
> + int (*unsynchronize) (struct pwm_device *p,
> + struct pwm_device *from_p);
> +};
> +
> +struct pwm_config {
> + unsigned long config_mask;
> +
> + unsigned long duty_ticks;
> + unsigned long period_ticks;
> + int polarity;
> +};
> +
> +struct pwm_device {
> + struct device *dev;
> + const struct pwm_device_ops *ops;
> +
> + const void *data;
> +
> + const char *label;
> +
> + unsigned long flags;
> +
> + unsigned long tick_hz;
> +
> + struct work_struct handler_work;
> + pwm_handler_t handler;
> + void *handler_data;
> +
> + int active_high;
In the config you call it polarity, here you call it active_high. A consistent
naming would be better in my opinion.
> + unsigned long period_ticks;
> + unsigned long duty_ticks;
> +};
> +
> +struct pwm_device *pwm_request_byname(const char *name, const char *label);
> +struct pwm_device *pwm_request(const char *bus_id, int id, const char *label);
> +void pwm_release(struct pwm_device *p);
> +
> +static inline int pwm_is_registered(const struct pwm_device *p)
> +{
> + return test_bit(FLAG_REGISTERED, &p->flags);
> +}
> +
> +static inline int pwm_is_requested(const struct pwm_device *p)
> +{
> + return test_bit(FLAG_REQUESTED, &p->flags);
> +}
> +
> +static inline int pwm_is_running(const struct pwm_device *p)
> +{
> + return test_bit(FLAG_RUNNING, &p->flags);
> +}
> +
> +static inline int pwm_is_exported(const struct pwm_device *p)
> +{
> + return test_bit(FLAG_EXPORTED, &p->flags);
> +}
> +
> +static inline void pwm_set_drvdata(struct pwm_device *p, const void *data)
> +{
> + p->data = data;
> +}
> +
> +static inline void *pwm_get_drvdata(const struct pwm_device *p)
> +{
> + return (void*)p->data;
> +}
> +
> +
> +int pwm_register(struct pwm_device *p, struct device *parent, int id);
> +int pwm_register_byname(struct pwm_device *p, struct device *parent,
> + const char *name);
> +int pwm_unregister(struct pwm_device *p);
> +
> +int pwm_set(struct pwm_device *p, unsigned long period_ns,
> + unsigned long duty_ns, int active_high);
> +
> +int pwm_set_period_ns(struct pwm_device *p, unsigned long period_ns);
> +unsigned long pwm_get_period_ns(struct pwm_device *p);
> +
> +int pwm_set_duty_ns(struct pwm_device *p, unsigned long duty_ns);
> +unsigned long pwm_get_duty_ns(struct pwm_device *p);
> +
> +int pwm_set_polarity(struct pwm_device *p, int active_high);
> +
> +int pwm_start(struct pwm_device *p);
> +int pwm_stop(struct pwm_device *p);
> +
> +int pwm_config_nosleep(struct pwm_device *p, struct pwm_config *c);
> +int pwm_config(struct pwm_device *p, struct pwm_config *c);
> +
> +int pwm_synchronize(struct pwm_device *p, struct pwm_device *to_p);
> +int pwm_unsynchronize(struct pwm_device *p, struct pwm_device *from_p);
> +int pwm_set_handler(struct pwm_device *p, pwm_handler_t handler, void *data);
> +
> +void pwm_callback(struct pwm_device *p);
> +
> +struct pwm_device *gpio_pwm_create(int gpio);
> +int gpio_pwm_destroy(struct pwm_device *p);
These two definitions probably belong to the second patch.
> +
> +#endif
2011/3/6 Bill Gatliff <[email protected]>:
> Updates the existing PWM-related functions to support multiple
> and/or hotplugged PWM devices, and adds a sysfs interface.
> Moves the code to drivers/pwm.
Cool!
> For now, this new code can exist alongside the current PWM
> implementations; the existing implementations will be migrated
> to this new framework as time permits. ?Eventually, the current
> PWM implementation will be deprecated and then expunged.
Is it possible to get atleast one driver switched over to this
framework (i.e. moved over to drivers/pwm and the origin
deleted) as part of the patch series? Mainly so one knows
what to do to convert existing drivers.
Yours,
Linus Walleij
On 06/03/2011 04:17, Bill Gatliff wrote:
> +The Generic PWM Device API framework is implemented in
> +include/linux/pwm/pwm.h and drivers/pwm/pwm.c. The functions therein
> +use information from pwm_device and pwm__config structures to invoke
Could you fix that to be pwm_config please.
[snip]
> --- /dev/null
> +++ b/drivers/pwm/Kconfig
> @@ -0,0 +1,10 @@
> +#
> +# PWM infrastructure and devices
> +#
> +
> +menuconfig GENERIC_PWM
> + tristate "PWM Support"
> + help
> + Enables PWM device support implemented via a generic
> + framework. If unsure, say N.
> +
Does this need help text? Can't we just select GENERIC_PWM for the
drivers / users that need it?
> diff --git a/drivers/pwm/pwm.c b/drivers/pwm/pwm.c
[snip]
> +static int __pwm_request(struct pwm_device *p, const char *label)
> +{
> + int ret;
> +
> + ret = test_and_set_bit(FLAG_REQUESTED, &p->flags);
> + if (ret) {
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + p->label = label;
> +
> + if (p->ops->request) {
> + ret = p->ops->request(p);
> + if (ret) {
> + clear_bit(FLAG_REQUESTED, &p->flags);
> + goto done;
You don't need this goto here.
> + }
> + }
> +
> +done:
> + return ret;
> +}
> +
[snip]
> +struct pwm_device *pwm_request(const char *bus_id, int id, const char *label)
> +{
> + char name[256];
> + int ret;
> +
> + if (id == -1)
> + ret = scnprintf(name, sizeof name, "%s", bus_id);
Kernel doc comment for the function to describe the id == -1 case?
> + else
> + ret = scnprintf(name, sizeof name, "%s:%d", bus_id, id);
> + if (ret <= 0 || ret >= sizeof name)
> + return ERR_PTR(-EINVAL);
> +
> + return pwm_request_byname(name, label);
> +}
> +EXPORT_SYMBOL(pwm_request);
[snip]
> +int pwm_config(struct pwm_device *p, struct pwm_config *c)
> +{
> + int ret = 0;
might_sleep() ?
[snip]
> +static void pwm_handler(struct work_struct *w)
> +{
> + struct pwm_device *p = container_of(w, struct pwm_device,
> + handler_work);
You could just not queue the work if !p->handler
> + if (p->handler)
> + p->handler(p, p->handler_data);
> +}
> +
> +void pwm_callback(struct pwm_device *p)
> +{
> + queue_work(pwm_handler_workqueue, &p->handler_work);
> +}
> +EXPORT_SYMBOL(pwm_callback);
> +
[snip]
> +int pwm_register(struct pwm_device *p, struct device *parent, int id)
> +{
> + int ret;
> + char name[256];
> +
> + if (IS_ERR_OR_NULL(parent))
> + return -EINVAL;
> +
> + if (id == -1)
> + ret = scnprintf(name, sizeof name, "%s", dev_name(parent));
A kernel doc comment to explain id == -1 would be nice.
> + else
> + ret = scnprintf(name, sizeof name, "%s:%d", dev_name(parent), id);
> + if (ret <= 0 || ret >= sizeof name)
> + return -EINVAL;
> +
> + return pwm_register_byname(p, parent, name);
> +}
> +EXPORT_SYMBOL(pwm_register);
> +
Very nice framework!
Would be nice to have kernel doc comments for the functions but that can
follow on later.
Thanks,
Jack
Lars:
On Sun, Mar 6, 2011 at 12:16 AM, Lars-Peter Clausen <[email protected]> wrote:
>
> Two minor remarks on the documentation: You are not mentioning the functions
> arguments and there is no documentation on the sysfs interface at all.
Ok, will fix.
>> +static LIST_HEAD(pwm_device_list);
> The list is not used.
Fixed.
> sizeof(name)
Technically, sizeof is an operator and therefore the parentheses are
redundant. But since everyone hates it when I leave them off, I put
them back. :)
>> + ? ? if (!p->ops->config_nosleep)
>> + ? ? ? ? ? ? return -EINVAL;
>
> Would ENOSYS be more appropriate?
Yes. Fixed.
>> +err:
>> + ? ? dev_dbg(p->dev, "%s: config_mask %lu period_ticks %lu "
>> + ? ? ? ? ? ? "duty_ticks %lu polarity %d\n",
>> + ? ? ? ? ? ? __func__, c->config_mask, c->period_ticks,
>> + ? ? ? ? ? ? c->duty_ticks, c->polarity);
>> +
>> + ? ? if (ret)
>> + ? ? ? ? ? ? return ret;
>
> This looks a bit confusing. Maybe it would be better to move the dev_dbg at the
> top of the function and the 'err' label at the end of the function.
That code clearly needs refactoring. Fixed.
>> + ? ? if (!p->ops->synchronize)
>> + ? ? ? ? ? ? return -EINVAL;
> ENOSYS here too
Fixed.
>> + ? ? if (!p->ops->unsynchronize)
>> + ? ? ? ? ? ? return -EINVAL;
> and here.
Fixed.
>> +int pwm_set_handler(struct pwm_device *p, pwm_handler_t handler, void *data)
>> +{
>> + ? ? struct pwm_config c;
>> + ? ? int ret;
>> +
>> + ? ? if (handler)
>> + ? ? ? ? ? ? c.config_mask = BIT(PWM_CONFIG_ENABLE_CALLBACK);
>> + ? ? else
>> + ? ? ? ? ? ? c.config_mask = BIT(PWM_CONFIG_DISABLE_CALLBACK);
>> +
>> + ? ? ret = pwm_config(p, &c);
>
> There is a chance for a race here. The device could fire, for example due to an
> irq, before the handler has been initalized.
Good catch. Fixed.
>> + ? ? if (!ret && handler) {
>> + ? ? ? ? ? ? p->handler_data = data;
>> + ? ? ? ? ? ? p->handler = handler;
>> + ? ? ? ? ? ? INIT_WORK(&p->handler_work, pwm_handler);
>
> The INIT_WORK should probably into pwm_device_register.
It could, but it isn't needed unless a handler is used. So I prefer
to leave it where it is.
>> +static ssize_t pwm_run_store(struct device *dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?struct device_attribute *attr,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ?const char *buf, size_t len)
>> +{
>> + ? ? struct pwm_device *p = dev_get_drvdata(dev);
>> +
>> + ? ? if (!pwm_is_exported(p))
>> + ? ? ? ? ? ? return -EINVAL;
>
> I'm not sure, but maybe -EPERM is better here.
I think you are probably right. Fixed.
>> +
>> + ? ? if (sysfs_streq(buf, "1"))
>> + ? ? ? ? ? ? pwm_start(p);
>> + ? ? else if (sysfs_streq(buf, "0"))
>> + ? ? ? ? ? ? pwm_stop(p);
> Maybe return -EINVAL if it is neither 0 or 1.
Fixed.
>> + ? ? if (!strict_strtoul(buf, 10, &duty_ns))
>> + ? ? ? ? ? ? pwm_set_duty_ns(p, duty_ns);
>
> How about returning the error value of strict_strtou if it fails?
Good idea. Fixed.
> So `cat request` requests the device and `echo > request` frees the device?
> Thats a bit odd in my opinion.
Yes, and no.
For comparison, GPIO pins are exported to userspace like this:
# echo 160 > /sys/class/gpio/export
That's convenient when you know the number of the GPIO pin you wish to
export. Upon export, a directory /sys/class/gpio/gpio<pin> shows up.
Prior to the export request, if the directory doesn't exist then you
know that either nothing in userspace is using the pin. If the
directory doesn't exist after the export request, then you know your
request failed--- probably because the kernel is using the pin.
The names for PWM devices are more complex. To prevent needing to
know the name of the desired PWM channel in advance, the list of all
available PWM devices is always present in /sys/class/pwm. Also, it
isn't an error for an application to want to read the PWM device state
(to update an instrument panel, for example) even when a kernel driver
is actively controlling the device itself. So I can't do things
exactly how GPIO does them.
Once you identify the PWM device you are seeking, you read from its
.../request attribute to ask for permission to use it. The response
you get back is either "sysfs" to state that userspace now controls
the PWM device, or the label provided by the kernel requester to
indicate that the kernel is using the device.
As long as (a) I want PWM device names to always be visible under
/sys/class/pwm, and (b) I want applications to be able to read-only
monitor PWM state even when the kernel is controlling the device, I
don't know how to make things work differently than I have implemented
without making things unnecessarily complicated.
Suggestions welcome.
>> +static struct class pwm_class = {
>> + ? ? .name = "pwm",
>> + ? ? .owner = THIS_MODULE,
>> +
>> + ? ? .class_attrs = pwm_class_attrs,
>
> Just leaving it NULL should work, if you don't want any class attributes.
Today I don't want any attributes, but I might someday soon. I don't
have any immediate plans, though...
>> + ? ? d = class_find_device(&pwm_class, NULL, (char*)name, pwm_match_name);
>> + ? ? if (d) {
>> + ? ? ? ? ? ? ret = -EEXIST;
>> + ? ? ? ? ? ? goto err_found_device;
>> + ? ? }
> device_create should fail if a device with the same name already exits, so I'm
> not sure if it makes sense to do the check here.
I'm pretty sure you are right. Fixed.
>> + ? ? p->dev = device_create(&pwm_class, parent, MKDEV(0, 0), NULL, name);
> ... NULL, "%s", name);
I guess things would get ugly with my code if someone passed a name
that had a "%<something>" in it, no? :)
> Or you could use device_create_vargs and get rid of that scnprintf in pwm_register.
Ooh, that sounds interesting. I'll look at that.
>
>> + ? ? if (IS_ERR(p->dev)) {
>> + ? ? ? ? ? ? ret = PTR_ERR(p->dev);
>> + ? ? ? ? ? ? goto err_device_create;
>> + ? ? }
> I think it would be better to embedd the device struct directly into the
> pwm_device struct. You could also remove the data field of the pwm_device
> struct and use dev_{get,set}_drvdata for pwm_{get,set}_drvdata.
Will look at that and follow up.
>> + ? ? ret = sysfs_create_group(&p->dev->kobj, &pwm_device_attr_group);
>> + ? ? if (ret)
>> + ? ? ? ? ? ? goto err_create_group;
>
> It should be possible to use the classes dev_attrs here instead.
I tinkered with that early on, and ended up with the impression that
class attributes were applied to the class as a whole, and not each
member of the class. Maybe I just messed up somewhere. Will
investigate and follow up.
>> + ? ? if (pwm_is_running(p) || pwm_is_requested(p)) {
>
> Is it possible that the pwm is running but not requested?
Belt-and-suspenders. I put it there in case I screwed up the flags
somewhere. Will take it back out.
>> + ? ? FLAG_REGISTERED ? ? ? ? = 0,
> Does the REGISTERED flag makes any sense, I mean are there any usecases for
> pwm_is_registered? Non registered pwm_devices should not be visible to the
> outside world, so if you got an pointer to an pwm_device outside of an
> pwm_device driver it should also be registered. And inside a pwm_device driver
> the driver should know whether it already registered the pwm_device or not.
I have it so that device drivers don't themselves have to keep track
of which of their individual devices failed to register during
initialization. An example of that used to be in atmel-pwmc.c, but
then I cleaned up the code to make the flag unnecessary.
Will take it out.
>> +
>> + ? ? int active_high;
> In the config you call it polarity, here you call it active_high. A consistent
> naming would be better in my opinion.
Fixed.
>> +struct pwm_device *gpio_pwm_create(int gpio);
>> +int gpio_pwm_destroy(struct pwm_device *p);
>
> These two definitions probably belong to the second patch.
Ok.
Thanks for the review!
b.g.
--
Bill Gatliff
[email protected]
Lars:
On Sun, Mar 6, 2011 at 12:35 AM, Lars-Peter Clausen <[email protected]> wrote:
> On 03/06/2011 07:16 AM, Lars-Peter Clausen wrote:
>> On 03/06/2011 05:17 AM, Bill Gatliff wrote:
>>> +
>>> +
>>> +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.
>>> +
>
> This looks like a poor man's IRQ handler implementation. In my opinion something
> like pwm_to_irq would be better suited interface.
Not sure what you mean here. Can you elaborate?
b.g.
--
Bill Gatliff
[email protected]
Linus:
On Sun, Mar 6, 2011 at 6:16 AM, Linus Walleij <[email protected]> wrote:
>
> Is it possible to get atleast one driver switched over to this
> framework (i.e. moved over to drivers/pwm and the origin
> deleted) as part of the patch series? Mainly so one knows
> what to do to convert existing drivers.
Will try to get that when I post the next set of patches for review.
It's a tricky thing, however, because it sometimes isn't enough to
just fix the PWM device driver itself--- I also have to fix the users.
So the work involved can quickly increase, only to be done yet again
at the next review. :(
b.g.
--
Bill Gatliff
[email protected]
Jack:
On Sun, Mar 6, 2011 at 10:20 PM, Jack Stone <[email protected]> wrote:
> On 06/03/2011 04:17, Bill Gatliff wrote:
>> +The Generic PWM Device API framework is implemented in
>> +include/linux/pwm/pwm.h and drivers/pwm/pwm.c. ?The functions therein
>> +use information from pwm_device and pwm__config structures to invoke
>
> Could you fix that to be pwm_config please.
Fixed.
>> +menuconfig GENERIC_PWM
>> + ? ? tristate "PWM Support"
>> + ? ? help
>> + ? ? ? Enables PWM device support implemented via a generic
>> + ? ? ? framework. ?If unsure, say N.
>> +
>
> Does this need help text? Can't we just select GENERIC_PWM for the
> drivers / users that need it?
That would be my preference, actually. I'll see if I can do that at
the next review.
>> +
>> + ? ? if (p->ops->request) {
>> + ? ? ? ? ? ? ret = p->ops->request(p);
>> + ? ? ? ? ? ? if (ret) {
>> + ? ? ? ? ? ? ? ? ? ? clear_bit(FLAG_REQUESTED, &p->flags);
>> + ? ? ? ? ? ? ? ? ? ? goto done;
>
> You don't need this goto here.
Fixed.
>> +struct pwm_device *pwm_request(const char *bus_id, int id, const char *label)
>> +{
>> + ? ? char name[256];
>> + ? ? int ret;
>> +
>> + ? ? if (id == -1)
>> + ? ? ? ? ? ? ret = scnprintf(name, sizeof name, "%s", bus_id);
>
> Kernel doc comment for the function to describe the id == -1 case?
Fixed.
>> +static void pwm_handler(struct work_struct *w)
>> +{
>> + ? ? struct pwm_device *p = container_of(w, struct pwm_device,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? handler_work);
>
> ? ? ? ?You could just not queue the work if !p->handler
Good point. Fixed.
>> + ? ? if (id == -1)
>> + ? ? ? ? ? ? ret = scnprintf(name, sizeof name, "%s", dev_name(parent));
>
> A kernel doc comment to explain id == -1 would be nice.
>
Fixed.
> Very nice framework!
Thank you!!
>
> Would be nice to have kernel doc comments for the functions but that can
> follow on later.
I'll try to do some of it for the next review series.
b.g.
--
Bill Gatliff
[email protected]
Jack:
On Mon, Mar 7, 2011 at 10:35 AM, Bill Gatliff <[email protected]> wrote:
> On Sun, Mar 6, 2011 at 10:20 PM, Jack Stone <[email protected]> wrote:
>>> +menuconfig GENERIC_PWM
>>> + ? ? tristate "PWM Support"
>>> + ? ? help
>>> + ? ? ? Enables PWM device support implemented via a generic
>>> + ? ? ? framework. ?If unsure, say N.
>>> +
>>
>> Does this need help text? Can't we just select GENERIC_PWM for the
>> drivers / users that need it?
>
> That would be my preference, actually. ?I'll see if I can do that at
> the next review.
Actually, I take that back. Unlike gpiolib, the PWM API isn't
mandatory. So I think I'll leave this as a menuconfig.
b.g.
--
Bill Gatliff
[email protected]
On 03/07/2011 05:26 PM, Bill Gatliff wrote:
>
>> So `cat request` requests the device and `echo > request` frees the device?
>> Thats a bit odd in my opinion.
>
> Yes, and no.
>
> For comparison, GPIO pins are exported to userspace like this:
>
> # echo 160 > /sys/class/gpio/export
>
> That's convenient when you know the number of the GPIO pin you wish to
> export. Upon export, a directory /sys/class/gpio/gpio<pin> shows up.
> Prior to the export request, if the directory doesn't exist then you
> know that either nothing in userspace is using the pin. If the
> directory doesn't exist after the export request, then you know your
> request failed--- probably because the kernel is using the pin.
>
> The names for PWM devices are more complex. To prevent needing to
> know the name of the desired PWM channel in advance, the list of all
> available PWM devices is always present in /sys/class/pwm. Also, it
> isn't an error for an application to want to read the PWM device state
> (to update an instrument panel, for example) even when a kernel driver
> is actively controlling the device itself. So I can't do things
> exactly how GPIO does them.
>
> Once you identify the PWM device you are seeking, you read from its
> .../request attribute to ask for permission to use it. The response
> you get back is either "sysfs" to state that userspace now controls
> the PWM device, or the label provided by the kernel requester to
> indicate that the kernel is using the device.
>
> As long as (a) I want PWM device names to always be visible under
> /sys/class/pwm, and (b) I want applications to be able to read-only
> monitor PWM state even when the kernel is controlling the device, I
> don't know how to make things work differently than I have implemented
> without making things unnecessarily complicated.
>
> Suggestions welcome.
>
I don't see much of a problem with PWM devices being always visible or having
the attributes available read-only, but I see a problem with how a PWM device
is requested by userspace.
You basically turned a read-operation into a modify-operation and thus changed
its semantics. You'd normally not expect a object to change its external
visible state, if the state of the object is read.
Image some berserk running file indexer going through sysfs. You'd suddenly
find yourself with all PWM devices being exported.
I see two alternatives to your implementation:
1) The device is exported by writing a non-empty string to the 'request'
sysfs-attribute. The written string is then used as the label under which the
device is requested.
To release the device a empty string would be written to the 'request'
sysfs-attribute.
2) Add a another sysfs-attribute called 'export'. Writing a '1' will export the
device, writing a '0' will release it.
I'd prefer the later since it keeps things simple and I'm not sure if anything
is gained making it possible to supply the label when requesting the device
from userspace.
>>> +static struct class pwm_class = {
>>> + .name = "pwm",
>>> + .owner = THIS_MODULE,
>>> +
>>> + .class_attrs = pwm_class_attrs,
>>
>> Just leaving it NULL should work, if you don't want any class attributes.
>
> Today I don't want any attributes, but I might someday soon. I don't
> have any immediate plans, though...
You can add it back, if you want to add any class attributes. Just keeping a
list with only an end-of-list element around doesn't make much sense.
>>> + d = class_find_device(&pwm_class, NULL, (char*)name, pwm_match_name);
>>> + if (d) {
>>> + ret = -EEXIST;
>>> + goto err_found_device;
>>> + }
>> device_create should fail if a device with the same name already exits, so I'm
>> not sure if it makes sense to do the check here.
>
> I'm pretty sure you are right. Fixed.
>
>>> + p->dev = device_create(&pwm_class, parent, MKDEV(0, 0), NULL, name);
>> ... NULL, "%s", name);
>
> I guess things would get ugly with my code if someone passed a name
> that had a "%<something>" in it, no? :)
>
>> Or you could use device_create_vargs and get rid of that scnprintf in pwm_register.
>
> Ooh, that sounds interesting. I'll look at that.
I had a look at the atmel pwm driver in the mean time and since you are using
scnprintf there as well, it seems like a good idea in general to have a method
for registering a PWM device where you can pass a format string.
>
>>
>>> + if (IS_ERR(p->dev)) {
>>> + ret = PTR_ERR(p->dev);
>>> + goto err_device_create;
>>> + }
>> I think it would be better to embed the device struct directly into the
>> pwm_device struct. You could also remove the data field of the pwm_device
>> struct and use dev_{get,set}_drvdata for pwm_{get,set}_drvdata.
>
> Will look at that and follow up.
>
>>> + ret = sysfs_create_group(&p->dev->kobj, &pwm_device_attr_group);
>>> + if (ret)
>>> + goto err_create_group;
>>
>> It should be possible to use the classes dev_attrs here instead.
>
> I tinkered with that early on, and ended up with the impression that
> class attributes were applied to the class as a whole, and not each
> member of the class. Maybe I just messed up somewhere. Will
> investigate and follow up.
There is both. There is 'class_attrs' which are only created once for the class
and there is 'dev_attrs' which are created on a per device basis.
- Lars
On 03/07/2011 05:28 PM, Bill Gatliff wrote:
> Lars:
>
> On Sun, Mar 6, 2011 at 12:35 AM, Lars-Peter Clausen <[email protected]> wrote:
>> On 03/06/2011 07:16 AM, Lars-Peter Clausen wrote:
>>> On 03/06/2011 05:17 AM, Bill Gatliff wrote:
>>>> +
>>>> +
>>>> +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.
>>>> +
>>
>> This looks like a poor man's IRQ handler implementation. In my opinion something
>> like pwm_to_irq would be better suited interface.
>
> Not sure what you mean here. Can you elaborate?
>
>
> b.g.
I was thinking about an interface similar to gpio_to_irq. So you could map a
PWM device to a IRQ number and use the existing the existing genirq framework
to manage the callback handler.
The hardware will quite likely inform the software about when the period has
ended through an interrupt.
So what you basically will end up doing in almost every PWM device driver
implementation is to call pwm_handler() from its interrupt handler.
I for example have a SoC with an PWM unit where at least for some channels an
IRQ number from the main interrupt controller is directly mapped to PWM
channel. In such an case I could simply return the IRQ number directly instead
of implementing a IRQ handler which does nothing but calls pwm_handler().
For other case where the IRQ is muxed onto multiple PWM device you can use an
irq_chip.
I think there are still some problems with your current callback handler
implementation that could prove to be racy. Also each driver has to keep track
whether the handler should be invoked or not.
All these problems should go away with a genirq based implementation.
Maybe I'm on the wrong track here though. Could you explain what your expected
use case is for this part of the API? For example what would a driver using a
PWM device normally be doing in the handler it passed to the PWM device?
Oh and another thing, will it be mandatory for a PWM driver to provide callback
functionality and what if the underlying hardware does not support it?
- Lars
On 07/03/2011 17:08, Bill Gatliff wrote:
> Jack:
>
> On Mon, Mar 7, 2011 at 10:35 AM, Bill Gatliff <[email protected]> wrote:
>> On Sun, Mar 6, 2011 at 10:20 PM, Jack Stone <[email protected]> wrote:
>>>> +menuconfig GENERIC_PWM
>>>> + tristate "PWM Support"
>>>> + help
>>>> + Enables PWM device support implemented via a generic
>>>> + framework. If unsure, say N.
>>>> +
>>>
>>> Does this need help text? Can't we just select GENERIC_PWM for the
>>> drivers / users that need it?
>>
>> That would be my preference, actually. I'll see if I can do that at
>> the next review.
>
> Actually, I take that back. Unlike gpiolib, the PWM API isn't
> mandatory. So I think I'll leave this as a menuconfig.
>
>
> b.g.
It's up to you, of course, but I don't see why I would want to choose.
It makes life much easier for users if they only have to support one way
of talking to PWM devices and any device can be connected to any user,
with appropriate connections provided by platform data.
If you make it a depend rather than it being selected when needed then
either the driver supports multiple interfaces (which is harder to
maintain) or the person configuring the kernel has to know to build
GENERIC_PWM to get the drivers they want.
It seems much simpler to me to always use it in appropriate
drivers/users and therefore if the person configuring the kernel does
not have to know anything new and the support will only be included if
needed.
Just my ?0.02,
Jack
Lars-Peter:
On Mon, Mar 7, 2011 at 1:26 PM, Lars-Peter Clausen <[email protected]> wrote:
> I was thinking about an interface similar to gpio_to_irq. So you could map a
> PWM device to a IRQ number and use the existing the existing genirq framework
> to manage the callback handler.
I really like this idea, though at the moment I don't see clearly how
to implement it. I'm just a little rusty on those data structures,
that's all.
I'm going to pull all of the callback functionality out of the API,
and will take it as a task to make something like this possible in a
future update:
struct pwm_device *p;
request_irq(pwm_to_irq(p), my_pwm_callback, ...);
> Maybe I'm on the wrong track here though. Could you explain what your expected
> use case is for this part of the API? For example what would a driver using a
> PWM device normally be doing in the handler it passed to the PWM device?
Honestly, I'm not really sure how useful the callback functionality
is. In fact, it's probably a bad idea to offer it because it allows
someone to do pulse counting for things like stepper motors, and I'm
already thinking about an API for that. Abusing a PWM to do pulse
counting is pretty CPU-intensive.
The only time end-of-period interrupts come up is when someone wants
to do a coordinated stop i.e. doesn't want a short pulse. But that's
really a concern for the driver, not users.
So in the above where I said that I might put the callback-like
functionality back into the API, actually I might not. :)
b.g.
--
Bill Gatliff
[email protected]
Jack:
On Tue, Mar 8, 2011 at 12:12 PM, Jack Stone <[email protected]> wrote:
> It's up to you, of course, but I don't see why I would want to choose.
It isn't that I want users to be able to choose between GENERIC_PWM
and the platform-specific PWM; it's that I want to have the option of
doing PWM, or not. Once a machine/platform is converted over to
GENERIC_PWM, there won't be any going back.
> It makes life much easier for users if they only have to support one way
> of talking to PWM devices and any device can be connected to any user,
> with appropriate connections provided by platform data.
Right. That's the vision, in fact. But I don't have the manpower to
do a complete conversion to GENERIC_PWM all at once, so I'm trying to
take it in stages. Introduce the generic API first, and then switch
machines/platforms over to it one at a time.
> It seems much simpler to me to always use it in appropriate
> drivers/users and therefore if the person configuring the kernel does
> not have to know anything new and the support will only be included if
> needed.
Right.
It isn't that I want users to be able to decide between the "new PWM"
and "old PWM" APIs; it's that I want them to be able to turn PWM
support on or off at compile time.
b.g.
--
Bill Gatliff
[email protected]
On Mon, Mar 7, 2011 at 1:02 PM, Lars-Peter Clausen <[email protected]> wrote:
>
> There is both. There is 'class_attrs' which are only created once for the class
> and there is 'dev_attrs' which are created on a per device basis.
Got it. Fixed. :)
b.g.
--
Bill Gatliff
[email protected]
On Sun, Mar 6, 2011 at 12:16 AM, Lars-Peter Clausen <[email protected]> wrote:
>
>> + ? ? if (IS_ERR(p->dev)) {
>> + ? ? ? ? ? ? ret = PTR_ERR(p->dev);
>> + ? ? ? ? ? ? goto err_device_create;
>> + ? ? }
> I think it would be better to embedd the device struct directly into the
> pwm_device struct. You could also remove the data field of the pwm_device
> struct and use dev_{get,set}_drvdata for pwm_{get,set}_drvdata.
In theory I agree with you, but it would take away my ability to do
things like device_create_vargs() and thereby make my code a bit more
complicated and error prone. Do you think the advantages outweigh the
disadvantages?
b.g.
--
Bill Gatliff
[email protected]
On 03/09/2011 06:05 PM, Bill Gatliff wrote:
> On Sun, Mar 6, 2011 at 12:16 AM, Lars-Peter Clausen <[email protected]> wrote:
>
>>
>>> + if (IS_ERR(p->dev)) {
>>> + ret = PTR_ERR(p->dev);
>>> + goto err_device_create;
>>> + }
>> I think it would be better to embedd the device struct directly into the
>> pwm_device struct. You could also remove the data field of the pwm_device
>> struct and use dev_{get,set}_drvdata for pwm_{get,set}_drvdata.
>
> In theory I agree with you, but it would take away my ability to do
> things like device_create_vargs() and thereby make my code a bit more
> complicated and error prone. Do you think the advantages outweigh the
> disadvantages?
>
The advantages are less memory fragmentation, less cache misses and slightly
smaller generated code, but in practice this won't probably be noticeable since
there wont be vast amounts of PWM devices.
I prefer it because I think it is the nicer model.
But aside from that I've been looking into reference counting, locking and
possible races with the current framework.
So and as I see it there are two major problems:
The first problem is, that if there is no indirect refcounting on the module
which implements a device driver. For example if the parent is NULL or in a
different module, it is possible to unload the module while the driver is still
in use.
This could be fixed for example by adding a owner field to the pwm_device_ops
struct and get a reference to the owner when requesting a device.
Another option would simply be to declare it API misuse if there is not parent
or the parent has not the same owner.
The second problem is a bit serious. There is a chance of use after free.
There could still be opened sysfs files when pwm_device_unregister is called.
And thus it is possible that the opened file is read after
pwm_device_unregister has been called and the pwm_device struct might already
be freed thus causing access to already freed memory.
To fix this I suggest to move the allocation of the pwm_device to
pwm_device_register and pass in the ops instead of the device itself. Maybe the
struct could even be made opaque to the outside word in this process.
The struct would be freed from the devices 'release' callback, which is only
called after the last reference to the device has been dropped.
Related to this problem is what happens when pwm_device_unregister is called
while the device is requested. The function returns -EBUSY, but non of your
drivers currently check the return value and free the pwm_device struct anyway.
So there is going to be a use after free too.
Also the reference count of the pwm device is increased in pwm_request (by
class_find_device), but that reference is never released. A device_put should
be added to pwm_release().
- Lars