* usb-skeleton.c - generates warning
From: Andrew Worsley @ 2010-10-16 11:00 UTC (permalink / raw)
To: linux-embedded
Sorry if this is the wrong mailing list for this - I am writing a USB
driver (for an embedded system) modeled on the
drivers/usb/usb-skeleton.c
version 2.6.30 but it gets the following warning.
It looks like it is a generic issue, perhaps the skeleton is not
maintained any more?
If so is there a reference explain the right way? I couldn't find
anything on google.
Looking at other drivers it appears this isn't done anymore? If so
perhaps this should be updated?
Suggestions appreciated.
Thanks
Andrew
...
usb 1-5: jtag_release()
usb 1-5: jtag_open()
usb 1-5: jtag_read(file=ded6e3c0, buf=092f8440, count=512,..)
usb 1-5: jtag_flush()
usb 1-5: jtag_release()
usb 1-5: jtag_open()
usb 1-5: jtag_write(file=df0ebe40, buf=093783f0, count=2506,..)
usb 1-5: jtag_flush()
usb 1-5: jtag_release()
------------[ cut here ]------------
WARNING: at /tmp/9789a690-d005-11df-8a73-00241dc62196/voyage-linux-2.6.30/arch/x86/include/asm/dma-mapping.h:294
hcd_buffer_free+0x85/0xb9 [usbcore]()
Hardware name: POULSBO
Modules linked in: ctam_ftdi ipv6 tun e1000e ata_piix usb_storage
ff_memless intel_agp agpgart loop hwmon_vid hostap_pci hostap lib80211
natsemi serio_raw p
cspkr asix pl2303 usbnet ftdi_sio usbserial usbhid i2c_isch hid evdev
ext3 jbd mbcache sd_mod pata_sch ata_generic libata uhci_hcd ehci_hcd
scsi_mod ide_pci
_generic r8169 mii usbcore amd74xx lm90 led_class ide_generic ide_core
[last unloaded: ctam_ftdi]
Pid: 2234, comm: loadFPGA Tainted: G W 2.6.30.10-robin-z5xx #1
Call Trace:
[<c0126a04>] ? warn_slowpath_common+0x5e/0x8a
[<c0126a3a>] ? warn_slowpath_null+0xa/0xc
[<dff512d1>] ? hcd_buffer_free+0x85/0xb9 [usbcore]
[<e043b328>] ? jtag_write_bulk_callback+0x55/0x61 [ctam_ftdi]
[<dff4c1e2>] ? usb_hcd_giveback_urb+0x60/0x8f [usbcore]
[<dfffde94>] ? ehci_urb_done+0x9d/0xa9 [ehci_hcd]
[<dffff42d>] ? qh_completions+0x359/0x3e9 [ehci_hcd]
[<dffff552>] ? ehci_work+0x95/0x782 [ehci_hcd]
[<e016f5c1>] ? serial_write+0x72/0x7d [usbserial]
[<c0175aa5>] ? handle_mm_fault+0x293/0x5f4
[<e00024f9>] ? ehci_irq+0x171/0x1c4 [ehci_hcd]
[<dff4beca>] ? usb_hcd_irq+0x2f/0x6c [usbcore]
[<c0156287>] ? handle_IRQ_event+0x4e/0x101
[<c0157510>] ? handle_fasteoi_irq+0x66/0x97
[<c0104def>] ? handle_irq+0x17/0x1b
[<c010485d>] ? do_IRQ+0x38/0x76
[<c0103569>] ? common_interrupt+0x29/0x30
---[ end trace 7b48bc9372419b94 ]---
usb 1-5: jtag_open()
usb 1-5: jtag_flush()
usb 1-5: jtag_read(file=de7748c0, buf=09a63000, count=512,..)
usb 1-5: jtag_flush()
usb 1-5: jtag_release()
^ permalink raw reply
* Re: [PWM 06/10] Incorporate PWM API code into KBuild
From: Grant Likely @ 2010-10-16 8:02 UTC (permalink / raw)
To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1285946271-17728-6-git-send-email-bgat@billgatliff.com>
On Fri, Oct 01, 2010 at 10:17:47AM -0500, Bill Gatliff wrote:
> Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
Ditto on description.
> ---
> drivers/Kconfig | 2 ++
> drivers/Makefile | 2 ++
> drivers/leds/Kconfig | 22 ++++++++++++++++------
> drivers/leds/Makefile | 2 ++
> drivers/pwm/Kconfig | 28 ++++++++++++++++++++++++++++
> drivers/pwm/Makefile | 6 ++++++
> 6 files changed, 56 insertions(+), 6 deletions(-)
> create mode 100644 drivers/pwm/Kconfig
> create mode 100644 drivers/pwm/Makefile
Hmmm... is this patch series bisectable?
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a2b902f..60390cb 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -54,6 +54,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 a2aea53..fa7ca1c 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/leds/Kconfig b/drivers/leds/Kconfig
> index 3af2cde..b434fa3 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -249,12 +249,6 @@ config LEDS_DAC124S085
> This option enables support for DAC124S085 SPI DAC from NatSemi,
> which can be used to control up to four LEDs.
>
> -config LEDS_PWM
> - tristate "PWM driven LED Support"
> - depends on HAVE_PWM
> - help
> - This option enables support for pwm driven LEDs
> -
> config LEDS_REGULATOR
> tristate "REGULATOR driven LED support"
> depends on REGULATOR
> @@ -354,6 +348,14 @@ config LEDS_TRIGGER_HEARTBEAT
> load average.
> If unsure, say Y.
>
> +config LEDS_TRIGGER_DIM
> + tristate "LED Dimmer Trigger"
> + depends on LEDS_TRIGGERS
> + help
> + Regulates the brightness of an LED based on the 1-minute CPU
> + load average. Ideal for PWM-driven LEDs.
> + If unsure, say Y.
> +
> config LEDS_TRIGGER_BACKLIGHT
> tristate "LED backlight Trigger"
> help
> @@ -374,6 +376,14 @@ config LEDS_TRIGGER_GPIO
>
> If unsure, say N.
>
> +config LEDS_TRIGGER_DIM
> + tristate "LED Dimmer Trigger"
> + depends on LEDS_TRIGGERS
> + help
> + Regulates the brightness of an LED based on the 1-minute CPU
> + load average. Ideal for PWM-driven LEDs.
> + If unsure, say Y.
> +
Inconsistent whitespace
> config LEDS_TRIGGER_DEFAULT_ON
> tristate "LED Default ON Trigger"
> help
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 7d6b958..a4ccea4 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_LEDS_COBALT_RAQ) += leds-cobalt-raq.o
> obj-$(CONFIG_LEDS_SUNFIRE) += leds-sunfire.o
> obj-$(CONFIG_LEDS_PCA9532) += leds-pca9532.o
> obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o
> +obj-$(CONFIG_LEDS_PWM) += leds-pwm.o
> obj-$(CONFIG_LEDS_LP3944) += leds-lp3944.o
> obj-$(CONFIG_LEDS_CLEVO_MAIL) += leds-clevo-mail.o
> obj-$(CONFIG_LEDS_HP6XX) += leds-hp6xx.o
> @@ -46,6 +47,7 @@ obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> obj-$(CONFIG_LEDS_TRIGGER_TIMER) += ledtrig-timer.o
> obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK) += ledtrig-ide-disk.o
> obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT) += ledtrig-heartbeat.o
> +obj-$(CONFIG_LEDS_TRIGGER_DIM) += ledtrig-dim.o
> obj-$(CONFIG_LEDS_TRIGGER_BACKLIGHT) += ledtrig-backlight.o
> obj-$(CONFIG_LEDS_TRIGGER_GPIO) += ledtrig-gpio.o
> obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> new file mode 100644
> index 0000000..0584c25
> --- /dev/null
> +++ b/drivers/pwm/Kconfig
> @@ -0,0 +1,28 @@
> +#
> +# PWM infrastructure and devices
> +#
> +
> +menuconfig GENERIC_PWM
> + tristate "PWM Support"
> + depends on SYSFS
> + help
> + This enables PWM support through the generic PWM API.
> + If unsure, say N.
> +
> +if GENERIC_PWM
> +
> +config ATMEL_PWM
> + tristate "Atmel AT32/AT91 PWM support"
> + depends on AVR32 || ARCH_AT91
> + help
> + This option enables device driver support for the PWMC
> + peripheral channels found on certain Atmel processors.
> + If unsure, say N.
> +
> +config GPIO_PWM
> + tristate "PWM emulation using GPIO"
> + help
> + This option enables a single-channel PWM device using
Ditto
> + a kernel interval timer and a GPIO pin. If unsure, say N.
> +
> +endif
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> new file mode 100644
> index 0000000..e8cacc5
> --- /dev/null
> +++ b/drivers/pwm/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for pwm devices
> +#
> +obj-y := pwm.o
> +obj-$(CONFIG_ATMEL_PWM) += atmel-pwm.o
> +obj-$(CONFIG_GPIO_PWM) += gpio.o
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PWM 05/10] LED "dim" trigger based on PWM control of the LED
From: Grant Likely @ 2010-10-16 8:00 UTC (permalink / raw)
To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1285946271-17728-5-git-send-email-bgat@billgatliff.com>
On Fri, Oct 01, 2010 at 10:17:46AM -0500, Bill Gatliff wrote:
> Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
Ditto on description
Otherwise pretty straight forward. Looks okay to me.
g.
> ---
> drivers/leds/ledtrig-dim.c | 96 ++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 96 insertions(+), 0 deletions(-)
> create mode 100644 drivers/leds/ledtrig-dim.c
>
> diff --git a/drivers/leds/ledtrig-dim.c b/drivers/leds/ledtrig-dim.c
> new file mode 100644
> index 0000000..8f93f44
> --- /dev/null
> +++ b/drivers/leds/ledtrig-dim.c
> @@ -0,0 +1,96 @@
> +/*
> + * LED Dim Trigger
> + *
> + * Copyright (C) 2010 Bill Gatliff <bgat@billgatliff.com>
> + *
> + * "Dims" an LED based on system load. Derived from Atsushi Nemoto's
> + * ledtrig-heartbeat.c.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/leds.h>
> +
> +#include "leds.h"
> +
> +struct dim_trig_data {
> + struct timer_list timer;
> +};
> +
> +
> +static void
> +led_dim_function(unsigned long data)
> +{
> + struct led_classdev *led_cdev = (struct led_classdev *)data;
> + struct dim_trig_data *dim_data = led_cdev->trigger_data;
> + unsigned int brightness;
> +
> + brightness = ((LED_FULL - LED_OFF) * avenrun[0]) / EXP_1;
> + if (brightness > LED_FULL)
> + brightness = LED_FULL;
> +
> + led_set_brightness(led_cdev, brightness);
> + mod_timer(&dim_data->timer, jiffies + msecs_to_jiffies(500));
> +}
> +
> +
> +static void
> +dim_trig_activate(struct led_classdev *led_cdev)
> +{
> + struct dim_trig_data *dim_data;
> +
> + dim_data = kzalloc(sizeof(*dim_data), GFP_KERNEL);
> + if (!dim_data)
> + return;
> +
> + led_cdev->trigger_data = dim_data;
> + setup_timer(&dim_data->timer,
> + led_dim_function, (unsigned long)led_cdev);
> + led_dim_function(dim_data->timer.data);
> +}
> +
> +
> +static void
> +dim_trig_deactivate(struct led_classdev *led_cdev)
> +{
> + struct dim_trig_data *dim_data = led_cdev->trigger_data;
> +
> + if (dim_data) {
> + del_timer_sync(&dim_data->timer);
> + kfree(dim_data);
> + }
> +}
> +
> +
> +static struct led_trigger dim_led_trigger = {
> + .name = "dim",
> + .activate = dim_trig_activate,
> + .deactivate = dim_trig_deactivate,
> +};
> +
> +
> +static int __init dim_trig_init(void)
> +{
> + return led_trigger_register(&dim_led_trigger);
> +}
> +module_init(dim_trig_init);
> +
> +
> +static void __exit dim_trig_exit(void)
> +{
> + led_trigger_unregister(&dim_led_trigger);
> +}
> +module_exit(dim_trig_exit);
> +
> +
> +MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
> +MODULE_DESCRIPTION("Dim LED trigger");
> +MODULE_LICENSE("GPL");
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PWM 04/10] Implements PWM-based LED control
From: Grant Likely @ 2010-10-16 7:58 UTC (permalink / raw)
To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1285946271-17728-4-git-send-email-bgat@billgatliff.com>
On Fri, Oct 01, 2010 at 10:17:45AM -0500, Bill Gatliff wrote:
> Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
Ditto on comment and patch title
> ---
> drivers/leds/Kconfig | 19 +++-
> drivers/leds/leds-pwm.c | 224 +++++++++++++++++++++++-------------------
> include/linux/pwm/pwm-led.h | 33 +++++++
> 3 files changed, 169 insertions(+), 107 deletions(-)
> create mode 100644 include/linux/pwm/pwm-led.h
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index e411262..3af2cde 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -26,12 +26,19 @@ config LEDS_88PM860X
> This option enables support for on-chip LED drivers found on Marvell
> Semiconductor 88PM8606 PMIC.
>
> -config LEDS_ATMEL_PWM
> - tristate "LED Support using Atmel PWM outputs"
> - depends on ATMEL_PWM
> - help
> - This option enables support for LEDs driven using outputs
> - of the dedicated PWM controller found on newer Atmel SOCs.
> +config LEDS_PWM
> + tristate "Support for LEDs connected to Generic PWM channels"
> + depends on LEDS_CLASS && GENERIC_PWM
> + help
> + Enables support for LEDs connected to PWM devices that
Inconsistent whitespace indentation
> + conform to the Generic PWM API. This API allows drivers
> + to adjust the brightness of the LED by varying the duty
> + cycle of the signal at the PWM channel output, using PWM API
> + commands, rather than merely turning them on and off.
> +
> + If your platform has devices with drivers that implement
> + the Generic PWM API, and those devices have outputs that
> + are connected to LEDs, then you probably want to say 'Y' here.
>
> config LEDS_LOCOMO
> tristate "LED Support for Locomo device"
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index da3fa8d..ab064ac 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -1,153 +1,175 @@
> /*
> - * linux/drivers/leds-pwm.c
> + * drivers/leds/leds-pwm.c
> *
> - * simple PWM based LED control
> + * Copyright (C) 2010 Bill Gatliff <bgat@billgatliff.com>
> + * Copyright (C) 2009 Loutao Fu, Pengutronix <l.fu@pengutronix.de>
> *
> - * Copyright 2009 Luotao Fu @ Pengutronix (l.fu@pengutronix.de)
> + * Based on leds-gpio.c by Raphael Assenat <raph@8d.com>
> *
> - * based on leds-gpio.c by Raphael Assenat <raph@8d.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> + * 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/platform_device.h>
> -#include <linux/fb.h>
> -#include <linux/leds.h>
> -#include <linux/err.h>
> -#include <linux/pwm.h>
> -#include <linux/leds_pwm.h>
> #include <linux/slab.h>
> -
> -struct led_pwm_data {
> - struct led_classdev cdev;
> - struct pwm_device *pwm;
> - unsigned int active_low;
> - unsigned int period;
> +#include <linux/leds.h>
> +#include <linux/io.h>
> +#include <linux/pwm/pwm.h>
> +#include <linux/pwm/pwm-led.h>
> +
> +struct led_pwm {
> + struct led_classdev led;
> + struct pwm_channel *pwm;
> + int percent;
> };
Looks to be gratuitous modification. Changing the name of the
structure (led_pwm_data --> led_pwm) and changing cdev --> led makes
this diff very large for no good reason. If you really want to change
the names, then do it in a separate cleanup patch without functional
changes.
>
> -static void led_pwm_set(struct led_classdev *led_cdev,
> - enum led_brightness brightness)
Ditto here. Reformatting and renaming the function name just makes
the patch hard to review. Please repost without the unnecessary
rework (or at least split into a separate patch).
> +static inline struct led_pwm *to_led_pwm(const struct led_classdev *c)
> +{
> + return container_of(c, struct led_pwm, led);
> +}
> +
> +static void
> +led_pwm_brightness_set(struct led_classdev *c,
> + enum led_brightness b)
> +{
> + struct led_pwm *led = to_led_pwm(c);
> + int percent;
> +
> + percent = (b * 100) / (LED_FULL - LED_OFF);
> + led->percent = percent;
> + pwm_set_duty_percent(led->pwm, percent);
> +}
> +
> +static enum led_brightness
> +led_pwm_brightness_get(struct led_classdev *c)
> {
> - struct led_pwm_data *led_dat =
> - container_of(led_cdev, struct led_pwm_data, cdev);
> - unsigned int max = led_dat->cdev.max_brightness;
> - unsigned int period = led_dat->period;
> -
> - if (brightness == 0) {
> - pwm_config(led_dat->pwm, 0, period);
> - pwm_disable(led_dat->pwm);
> - } else {
> - pwm_config(led_dat->pwm, brightness * period / max, period);
> - pwm_enable(led_dat->pwm);
> + struct led_pwm *led = to_led_pwm(c);
> + return led->percent;
> +}
> +
> +static int
> +led_pwm_blink_set(struct led_classdev *c,
> + unsigned long *on_ms,
> + unsigned long *off_ms)
> +{
> + struct led_pwm *led = to_led_pwm(c);
> + struct pwm_channel_config cfg;
> +
> + if (*on_ms == 0 && *off_ms == 0) {
> + *on_ms = 1000UL;
> + *off_ms = 1000UL;
> }
> +
> + cfg.config_mask = PWM_CONFIG_DUTY_NS
> + | PWM_CONFIG_PERIOD_NS;
> +
> + cfg.duty_ns = *on_ms * 1000000UL;
> + cfg.period_ns = (*on_ms + *off_ms) * 1000000UL;
> +
> + return pwm_config(led->pwm, &cfg);
> }
>
> -static int led_pwm_probe(struct platform_device *pdev)
> +static int __devinit
> +led_pwm_probe(struct platform_device *pdev)
> {
> - struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
> - struct led_pwm *cur_led;
> - struct led_pwm_data *leds_data, *led_dat;
> - int i, ret = 0;
> + struct pwm_led_platform_data *pdata = pdev->dev.platform_data;
> + struct led_pwm *led;
> + int ret;
>
> - if (!pdata)
> - return -EBUSY;
> + if (!pdata || !pdata->led_info)
> + return -EINVAL;
>
> - leds_data = kzalloc(sizeof(struct led_pwm_data) * pdata->num_leds,
> - GFP_KERNEL);
> - if (!leds_data)
> + led = kzalloc(sizeof(*led), GFP_KERNEL);
> + if (!led)
> return -ENOMEM;
>
> - for (i = 0; i < pdata->num_leds; i++) {
> - cur_led = &pdata->leds[i];
> - led_dat = &leds_data[i];
> -
> - led_dat->pwm = pwm_request(cur_led->pwm_id,
> - cur_led->name);
> - if (IS_ERR(led_dat->pwm)) {
> - dev_err(&pdev->dev, "unable to request PWM %d\n",
> - cur_led->pwm_id);
> - goto err;
> - }
> -
> - led_dat->cdev.name = cur_led->name;
> - led_dat->cdev.default_trigger = cur_led->default_trigger;
> - led_dat->active_low = cur_led->active_low;
> - led_dat->period = cur_led->pwm_period_ns;
> - led_dat->cdev.brightness_set = led_pwm_set;
> - led_dat->cdev.brightness = LED_OFF;
> - led_dat->cdev.max_brightness = cur_led->max_brightness;
> - led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
> -
> - ret = led_classdev_register(&pdev->dev, &led_dat->cdev);
> - if (ret < 0) {
> - pwm_free(led_dat->pwm);
> - goto err;
> - }
> + led->pwm = pwm_request(pdata->bus_id, pdata->chan,
> + pdata->led_info->name);
> + if (!led->pwm) {
> + ret = -EINVAL;
> + goto err_pwm_request;
> }
>
> - platform_set_drvdata(pdev, leds_data);
> + platform_set_drvdata(pdev, led);
>
> - return 0;
> + led->led.name = pdata->led_info->name;
> + led->led.default_trigger = pdata->led_info->default_trigger;
> + led->led.brightness_set = led_pwm_brightness_set;
> + led->led.brightness_get = led_pwm_brightness_get;
> + led->led.blink_set = led_pwm_blink_set;
> + led->led.brightness = LED_OFF;
>
> -err:
> - if (i > 0) {
> - for (i = i - 1; i >= 0; i--) {
> - led_classdev_unregister(&leds_data[i].cdev);
> - pwm_free(leds_data[i].pwm);
> - }
> - }
> + ret = pwm_config(led->pwm, pdata->config);
> + if (ret)
> + goto err_pwm_config;
> + pwm_start(led->pwm);
> +
> + ret = led_classdev_register(&pdev->dev, &led->led);
> + if (ret < 0)
> + goto err_classdev_register;
>
> - kfree(leds_data);
> + return 0;
> +
> +err_classdev_register:
> + pwm_stop(led->pwm);
> +err_pwm_config:
> + pwm_release(led->pwm);
> +err_pwm_request:
> + kfree(led);
>
> return ret;
> }
>
> -static int __devexit led_pwm_remove(struct platform_device *pdev)
> +static int __devexit
> +led_pwm_remove(struct platform_device *pdev)
> {
> - int i;
> - struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
> - struct led_pwm_data *leds_data;
> + struct led_pwm *led = platform_get_drvdata(pdev);
>
> - leds_data = platform_get_drvdata(pdev);
> + led_classdev_unregister(&led->led);
>
> - for (i = 0; i < pdata->num_leds; i++) {
> - led_classdev_unregister(&leds_data[i].cdev);
> - pwm_free(leds_data[i].pwm);
> + if (led->pwm) {
> + pwm_stop(led->pwm);
> + pwm_release(led->pwm);
> }
>
> - kfree(leds_data);
> + kfree(led);
>
> return 0;
> }
>
> static struct platform_driver led_pwm_driver = {
> - .probe = led_pwm_probe,
> - .remove = __devexit_p(led_pwm_remove),
> - .driver = {
> - .name = "leds_pwm",
> - .owner = THIS_MODULE,
> + .driver = {
> + .name = "leds-pwm",
> + .owner = THIS_MODULE,
> },
> + .probe = led_pwm_probe,
> + .remove = led_pwm_remove,
> };
>
> -static int __init led_pwm_init(void)
> +static int __init led_pwm_modinit(void)
> {
> return platform_driver_register(&led_pwm_driver);
> }
> +module_init(led_pwm_modinit);
>
> -static void __exit led_pwm_exit(void)
> +static void __exit led_pwm_modexit(void)
> {
> platform_driver_unregister(&led_pwm_driver);
> }
> +module_exit(led_pwm_modexit);
>
> -module_init(led_pwm_init);
> -module_exit(led_pwm_exit);
> -
> -MODULE_AUTHOR("Luotao Fu <l.fu@pengutronix.de>");
> -MODULE_DESCRIPTION("PWM LED driver for PXA");
> +MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
> +MODULE_DESCRIPTION("Driver for LEDs with PWM-controlled brightness");
> MODULE_LICENSE("GPL");
> MODULE_ALIAS("platform:leds-pwm");
> diff --git a/include/linux/pwm/pwm-led.h b/include/linux/pwm/pwm-led.h
> new file mode 100644
> index 0000000..8ffeecc
> --- /dev/null
> +++ b/include/linux/pwm/pwm-led.h
> @@ -0,0 +1,33 @@
> +/*
> + * include/linux/pwm-led.h
> + *
> + * Copyright (C) 2008 Bill Gatliff <bgat@billgatliff.com>
> + *
> + * 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_LED_H
> +#define __LINUX_PWM_LED_H
> +
> +struct led_info;
> +struct pwm_channel_config;
> +
> +struct pwm_led_platform_data {
> + const char *bus_id;
> + int chan;
> + struct pwm_channel_config *config;
> + struct led_info *led_info;
> +};
> +
> +#endif /* __LINUX_PWM_LED_H */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PWM 03/10] Expunge old Atmel PWMC driver, replacing it with one that conforms to the PWM API
From: Grant Likely @ 2010-10-16 7:50 UTC (permalink / raw)
To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1285946271-17728-3-git-send-email-bgat@billgatliff.com>
On Fri, Oct 01, 2010 at 10:17:44AM -0500, Bill Gatliff wrote:
> Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
This patch needs a better description about what is going on here. If
you're replacing an old driver, you must talk about what is changing
and why. You cannot assume that a future reader will have the context
of the full patch series.
Also, since this patch deletes existing code; does it break any users
of the current driver? Is it fully bisectable? What testing has been
done?
Finally, please shorten the patch title. It should be somewhere in
the neighbourhood of 70 characters or less so that it doesn't spill
out the end of reviewers inbox list.
> ---
> drivers/misc/Makefile | 1 -
> drivers/misc/atmel_pwm.c | 410 --------------------------------
> drivers/pwm/atmel-pwm.c | 592 ++++++++++++++++++++++++++++++++++++++++++++++
Since the old driver is being removed; shouldn't the atmel_pwm.h
header file be removed also?
> 3 files changed, 592 insertions(+), 411 deletions(-)
> delete mode 100644 drivers/misc/atmel_pwm.c
> create mode 100644 drivers/pwm/atmel-pwm.c
>
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 42eab95..b5962a2 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -6,7 +6,6 @@ obj-$(CONFIG_IBM_ASM) += ibmasm/
> obj-$(CONFIG_AD525X_DPOT) += ad525x_dpot.o
> obj-$(CONFIG_AD525X_DPOT_I2C) += ad525x_dpot-i2c.o
> obj-$(CONFIG_AD525X_DPOT_SPI) += ad525x_dpot-spi.o
> -obj-$(CONFIG_ATMEL_PWM) += atmel_pwm.o
> obj-$(CONFIG_ATMEL_SSC) += atmel-ssc.o
> obj-$(CONFIG_ATMEL_TCLIB) += atmel_tclib.o
> obj-$(CONFIG_BMP085) += bmp085.o
> diff --git a/drivers/misc/atmel_pwm.c b/drivers/misc/atmel_pwm.c
> deleted file mode 100644
> index 0f3fb4f..0000000
> --- a/drivers/misc/atmel_pwm.c
> +++ /dev/null
> @@ -1,410 +0,0 @@
> -#include <linux/module.h>
> -#include <linux/clk.h>
> -#include <linux/err.h>
> -#include <linux/slab.h>
> -#include <linux/io.h>
> -#include <linux/interrupt.h>
> -#include <linux/platform_device.h>
> -#include <linux/atmel_pwm.h>
> -
> -
> -/*
> - * This is a simple driver for the PWM controller found in various newer
> - * Atmel SOCs, including the AVR32 series and the AT91sam9263.
> - *
> - * Chips with current Linux ports have only 4 PWM channels, out of max 32.
> - * AT32UC3A and AT32UC3B chips have 7 channels (but currently no Linux).
> - * Docs are inconsistent about the width of the channel counter registers;
> - * it's at least 16 bits, but several places say 20 bits.
> - */
> -#define PWM_NCHAN 4 /* max 32 */
> -
> -struct pwm {
> - spinlock_t lock;
> - struct platform_device *pdev;
> - u32 mask;
> - int irq;
> - void __iomem *base;
> - struct clk *clk;
> - struct pwm_channel *channel[PWM_NCHAN];
> - void (*handler[PWM_NCHAN])(struct pwm_channel *);
> -};
> -
> -
> -/* global PWM controller registers */
> -#define PWM_MR 0x00
> -#define PWM_ENA 0x04
> -#define PWM_DIS 0x08
> -#define PWM_SR 0x0c
> -#define PWM_IER 0x10
> -#define PWM_IDR 0x14
> -#define PWM_IMR 0x18
> -#define PWM_ISR 0x1c
> -
> -static inline void pwm_writel(const struct pwm *p, unsigned offset, u32 val)
> -{
> - __raw_writel(val, p->base + offset);
> -}
> -
> -static inline u32 pwm_readl(const struct pwm *p, unsigned offset)
> -{
> - return __raw_readl(p->base + offset);
> -}
> -
> -static inline void __iomem *pwmc_regs(const struct pwm *p, int index)
> -{
> - return p->base + 0x200 + index * 0x20;
> -}
> -
> -static struct pwm *pwm;
> -
> -static void pwm_dumpregs(struct pwm_channel *ch, char *tag)
> -{
> - struct device *dev = &pwm->pdev->dev;
> -
> - dev_dbg(dev, "%s: mr %08x, sr %08x, imr %08x\n",
> - tag,
> - pwm_readl(pwm, PWM_MR),
> - pwm_readl(pwm, PWM_SR),
> - pwm_readl(pwm, PWM_IMR));
> - dev_dbg(dev,
> - "pwm ch%d - mr %08x, dty %u, prd %u, cnt %u\n",
> - ch->index,
> - pwm_channel_readl(ch, PWM_CMR),
> - pwm_channel_readl(ch, PWM_CDTY),
> - pwm_channel_readl(ch, PWM_CPRD),
> - pwm_channel_readl(ch, PWM_CCNT));
> -}
> -
> -
> -/**
> - * pwm_channel_alloc - allocate an unused PWM channel
> - * @index: identifies the channel
> - * @ch: structure to be initialized
> - *
> - * Drivers allocate PWM channels according to the board's wiring, and
> - * matching board-specific setup code. Returns zero or negative errno.
> - */
> -int pwm_channel_alloc(int index, struct pwm_channel *ch)
> -{
> - unsigned long flags;
> - int status = 0;
> -
> - /* insist on PWM init, with this signal pinned out */
> - if (!pwm || !(pwm->mask & 1 << index))
> - return -ENODEV;
> -
> - if (index < 0 || index >= PWM_NCHAN || !ch)
> - return -EINVAL;
> - memset(ch, 0, sizeof *ch);
> -
> - spin_lock_irqsave(&pwm->lock, flags);
> - if (pwm->channel[index])
> - status = -EBUSY;
> - else {
> - clk_enable(pwm->clk);
> -
> - ch->regs = pwmc_regs(pwm, index);
> - ch->index = index;
> -
> - /* REVISIT: ap7000 seems to go 2x as fast as we expect!! */
> - ch->mck = clk_get_rate(pwm->clk);
> -
> - pwm->channel[index] = ch;
> - pwm->handler[index] = NULL;
> -
> - /* channel and irq are always disabled when we return */
> - pwm_writel(pwm, PWM_DIS, 1 << index);
> - pwm_writel(pwm, PWM_IDR, 1 << index);
> - }
> - spin_unlock_irqrestore(&pwm->lock, flags);
> - return status;
> -}
> -EXPORT_SYMBOL(pwm_channel_alloc);
> -
> -static int pwmcheck(struct pwm_channel *ch)
> -{
> - int index;
> -
> - if (!pwm)
> - return -ENODEV;
> - if (!ch)
> - return -EINVAL;
> - index = ch->index;
> - if (index < 0 || index >= PWM_NCHAN || pwm->channel[index] != ch)
> - return -EINVAL;
> -
> - return index;
> -}
> -
> -/**
> - * pwm_channel_free - release a previously allocated channel
> - * @ch: the channel being released
> - *
> - * The channel is completely shut down (counter and IRQ disabled),
> - * and made available for re-use. Returns zero, or negative errno.
> - */
> -int pwm_channel_free(struct pwm_channel *ch)
> -{
> - unsigned long flags;
> - int t;
> -
> - spin_lock_irqsave(&pwm->lock, flags);
> - t = pwmcheck(ch);
> - if (t >= 0) {
> - pwm->channel[t] = NULL;
> - pwm->handler[t] = NULL;
> -
> - /* channel and irq are always disabled when we return */
> - pwm_writel(pwm, PWM_DIS, 1 << t);
> - pwm_writel(pwm, PWM_IDR, 1 << t);
> -
> - clk_disable(pwm->clk);
> - t = 0;
> - }
> - spin_unlock_irqrestore(&pwm->lock, flags);
> - return t;
> -}
> -EXPORT_SYMBOL(pwm_channel_free);
> -
> -int __pwm_channel_onoff(struct pwm_channel *ch, int enabled)
> -{
> - unsigned long flags;
> - int t;
> -
> - /* OMITTED FUNCTIONALITY: starting several channels in synch */
> -
> - spin_lock_irqsave(&pwm->lock, flags);
> - t = pwmcheck(ch);
> - if (t >= 0) {
> - pwm_writel(pwm, enabled ? PWM_ENA : PWM_DIS, 1 << t);
> - t = 0;
> - pwm_dumpregs(ch, enabled ? "enable" : "disable");
> - }
> - spin_unlock_irqrestore(&pwm->lock, flags);
> -
> - return t;
> -}
> -EXPORT_SYMBOL(__pwm_channel_onoff);
> -
> -/**
> - * pwm_clk_alloc - allocate and configure CLKA or CLKB
> - * @prescale: from 0..10, the power of two used to divide MCK
> - * @div: from 1..255, the linear divisor to use
> - *
> - * Returns PWM_CPR_CLKA, PWM_CPR_CLKB, or negative errno. The allocated
> - * clock will run with a period of (2^prescale * div) / MCK, or twice as
> - * long if center aligned PWM output is used. The clock must later be
> - * deconfigured using pwm_clk_free().
> - */
> -int pwm_clk_alloc(unsigned prescale, unsigned div)
> -{
> - unsigned long flags;
> - u32 mr;
> - u32 val = (prescale << 8) | div;
> - int ret = -EBUSY;
> -
> - if (prescale >= 10 || div == 0 || div > 255)
> - return -EINVAL;
> -
> - spin_lock_irqsave(&pwm->lock, flags);
> - mr = pwm_readl(pwm, PWM_MR);
> - if ((mr & 0xffff) == 0) {
> - mr |= val;
> - ret = PWM_CPR_CLKA;
> - } else if ((mr & (0xffff << 16)) == 0) {
> - mr |= val << 16;
> - ret = PWM_CPR_CLKB;
> - }
> - if (ret > 0)
> - pwm_writel(pwm, PWM_MR, mr);
> - spin_unlock_irqrestore(&pwm->lock, flags);
> - return ret;
> -}
> -EXPORT_SYMBOL(pwm_clk_alloc);
> -
> -/**
> - * pwm_clk_free - deconfigure and release CLKA or CLKB
> - *
> - * Reverses the effect of pwm_clk_alloc().
> - */
> -void pwm_clk_free(unsigned clk)
> -{
> - unsigned long flags;
> - u32 mr;
> -
> - spin_lock_irqsave(&pwm->lock, flags);
> - mr = pwm_readl(pwm, PWM_MR);
> - if (clk == PWM_CPR_CLKA)
> - pwm_writel(pwm, PWM_MR, mr & ~(0xffff << 0));
> - if (clk == PWM_CPR_CLKB)
> - pwm_writel(pwm, PWM_MR, mr & ~(0xffff << 16));
> - spin_unlock_irqrestore(&pwm->lock, flags);
> -}
> -EXPORT_SYMBOL(pwm_clk_free);
> -
> -/**
> - * pwm_channel_handler - manage channel's IRQ handler
> - * @ch: the channel
> - * @handler: the handler to use, possibly NULL
> - *
> - * If the handler is non-null, the handler will be called after every
> - * period of this PWM channel. If the handler is null, this channel
> - * won't generate an IRQ.
> - */
> -int pwm_channel_handler(struct pwm_channel *ch,
> - void (*handler)(struct pwm_channel *ch))
> -{
> - unsigned long flags;
> - int t;
> -
> - spin_lock_irqsave(&pwm->lock, flags);
> - t = pwmcheck(ch);
> - if (t >= 0) {
> - pwm->handler[t] = handler;
> - pwm_writel(pwm, handler ? PWM_IER : PWM_IDR, 1 << t);
> - t = 0;
> - }
> - spin_unlock_irqrestore(&pwm->lock, flags);
> -
> - return t;
> -}
> -EXPORT_SYMBOL(pwm_channel_handler);
> -
> -static irqreturn_t pwm_irq(int id, void *_pwm)
> -{
> - struct pwm *p = _pwm;
> - irqreturn_t handled = IRQ_NONE;
> - u32 irqstat;
> - int index;
> -
> - spin_lock(&p->lock);
> -
> - /* ack irqs, then handle them */
> - irqstat = pwm_readl(pwm, PWM_ISR);
> -
> - while (irqstat) {
> - struct pwm_channel *ch;
> - void (*handler)(struct pwm_channel *ch);
> -
> - index = ffs(irqstat) - 1;
> - irqstat &= ~(1 << index);
> - ch = pwm->channel[index];
> - handler = pwm->handler[index];
> - if (handler && ch) {
> - spin_unlock(&p->lock);
> - handler(ch);
> - spin_lock(&p->lock);
> - handled = IRQ_HANDLED;
> - }
> - }
> -
> - spin_unlock(&p->lock);
> - return handled;
> -}
> -
> -static int __init pwm_probe(struct platform_device *pdev)
> -{
> - struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - int irq = platform_get_irq(pdev, 0);
> - u32 *mp = pdev->dev.platform_data;
> - struct pwm *p;
> - int status = -EIO;
> -
> - if (pwm)
> - return -EBUSY;
> - if (!r || irq < 0 || !mp || !*mp)
> - return -ENODEV;
> - if (*mp & ~((1<<PWM_NCHAN)-1)) {
> - dev_warn(&pdev->dev, "mask 0x%x ... more than %d channels\n",
> - *mp, PWM_NCHAN);
> - return -EINVAL;
> - }
> -
> - p = kzalloc(sizeof(*p), GFP_KERNEL);
> - if (!p)
> - return -ENOMEM;
> -
> - spin_lock_init(&p->lock);
> - p->pdev = pdev;
> - p->mask = *mp;
> - p->irq = irq;
> - p->base = ioremap(r->start, r->end - r->start + 1);
> - if (!p->base)
> - goto fail;
> - p->clk = clk_get(&pdev->dev, "pwm_clk");
> - if (IS_ERR(p->clk)) {
> - status = PTR_ERR(p->clk);
> - p->clk = NULL;
> - goto fail;
> - }
> -
> - status = request_irq(irq, pwm_irq, 0, pdev->name, p);
> - if (status < 0)
> - goto fail;
> -
> - pwm = p;
> - platform_set_drvdata(pdev, p);
> -
> - return 0;
> -
> -fail:
> - if (p->clk)
> - clk_put(p->clk);
> - if (p->base)
> - iounmap(p->base);
> -
> - kfree(p);
> - return status;
> -}
> -
> -static int __exit pwm_remove(struct platform_device *pdev)
> -{
> - struct pwm *p = platform_get_drvdata(pdev);
> -
> - if (p != pwm)
> - return -EINVAL;
> -
> - clk_enable(pwm->clk);
> - pwm_writel(pwm, PWM_DIS, (1 << PWM_NCHAN) - 1);
> - pwm_writel(pwm, PWM_IDR, (1 << PWM_NCHAN) - 1);
> - clk_disable(pwm->clk);
> -
> - pwm = NULL;
> -
> - free_irq(p->irq, p);
> - clk_put(p->clk);
> - iounmap(p->base);
> - kfree(p);
> -
> - return 0;
> -}
> -
> -static struct platform_driver atmel_pwm_driver = {
> - .driver = {
> - .name = "atmel_pwm",
> - .owner = THIS_MODULE,
> - },
> - .remove = __exit_p(pwm_remove),
> -
> - /* NOTE: PWM can keep running in AVR32 "idle" and "frozen" states;
> - * and all AT91sam9263 states, albeit at reduced clock rate if
> - * MCK becomes the slow clock (i.e. what Linux labels STR).
> - */
> -};
> -
> -static int __init pwm_init(void)
> -{
> - return platform_driver_probe(&atmel_pwm_driver, pwm_probe);
> -}
> -module_init(pwm_init);
> -
> -static void __exit pwm_exit(void)
> -{
> - platform_driver_unregister(&atmel_pwm_driver);
> -}
> -module_exit(pwm_exit);
> -
> -MODULE_DESCRIPTION("Driver for AT32/AT91 PWM module");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:atmel_pwm");
> diff --git a/drivers/pwm/atmel-pwm.c b/drivers/pwm/atmel-pwm.c
> new file mode 100644
> index 0000000..8566866
> --- /dev/null
> +++ b/drivers/pwm/atmel-pwm.c
> @@ -0,0 +1,592 @@
> +/*
> + * drivers/pwm/atmel-pwm.c
> + *
> + * Copyright (C) 2010 Bill Gatliff <bgat@billgatliff.com>
> + * 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/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,
> +};
> +
> +struct atmel_pwm {
> + struct pwm_device pwm;
> + spinlock_t lock;
> + void __iomem *iobase;
> + struct clk *clk;
> + u32 *sync_mask;
> + int irq;
> + u32 ccnt_mask;
> +};
> +
> +static inline struct atmel_pwm *to_atmel_pwm(const struct pwm_channel *p)
> +{
> + return container_of(p->pwm, struct atmel_pwm, pwm);
> +}
> +
> +static inline void
> +pwmc_writel(const struct atmel_pwm *p,
> + unsigned offset, u32 val)
> +{
> + __raw_writel(val, p->iobase + offset);
> +}
> +
> +static inline u32
> +pwmc_readl(const struct atmel_pwm *p,
> + unsigned offset)
> +{
> + return __raw_readl(p->iobase + offset);
> +}
> +
> +static inline void
> +pwmc_chan_writel(const struct pwm_channel *p,
> + u32 offset, u32 val)
> +{
> + const struct atmel_pwm *ap = to_atmel_pwm(p);
> +
> + if (PWMC_CMR == offset)
> + val &= ((1 << PWMC_CMR_CPD)
> + | (1 << PWMC_CMR_CPOL)
> + | (1 << PWMC_CMR_CALG)
> + | (PWMC_CMR_CPRE_MASK));
> + else
> + val &= ap->ccnt_mask;
> +
> + pwmc_writel(ap, offset + PWMC_CHAN_BASE
> + + (p->chan * PWMC_CHAN_STRIDE), val);
> +}
> +
> +static inline u32
> +pwmc_chan_readl(const struct pwm_channel *p,
> + u32 offset)
> +{
> + const struct atmel_pwm *ap = to_atmel_pwm(p);
> +
> + return pwmc_readl(ap, offset + PWMC_CHAN_BASE
> + + (p->chan * PWMC_CHAN_STRIDE));
> +}
> +
> +static inline int
> +__atmel_pwm_is_on(struct pwm_channel *p)
> +{
> + struct atmel_pwm *ap = to_atmel_pwm(p);
> + return (pwmc_readl(ap, PWMC_SR) & (1 << p->chan)) ? 1 : 0;
> +}
> +
> +static inline void
> +__atmel_pwm_unsynchronize(struct pwm_channel *p,
> + struct pwm_channel *to_p)
> +{
> + const struct atmel_pwm *ap = to_atmel_pwm(p);
> + int wchan;
> +
> + if (to_p) {
> + ap->sync_mask[p->chan] &= ~(1 << to_p->chan);
> + ap->sync_mask[to_p->chan] &= ~(1 << p->chan);
> + goto done;
> + }
> +
> + ap->sync_mask[p->chan] = 0;
> + for (wchan = 0; wchan < ap->pwm.nchan; wchan++)
> + ap->sync_mask[wchan] &= ~(1 << p->chan);
> +done:
> + dev_dbg(p->pwm->dev, "sync_mask %x\n", ap->sync_mask[p->chan]);
> +}
> +
> +static inline void
> +__atmel_pwm_synchronize(struct pwm_channel *p,
> + struct pwm_channel *to_p)
> +{
> + const struct atmel_pwm *ap = to_atmel_pwm(p);
> +
> + if (!to_p)
> + return;
> +
> + ap->sync_mask[p->chan] |= (1 << to_p->chan);
> + ap->sync_mask[to_p->chan] |= (1 << p->chan);
> +
> + dev_dbg(p->pwm->dev, "sync_mask %x\n", ap->sync_mask[p->chan]);
> +}
> +
> +static inline void
> +__atmel_pwm_stop(struct pwm_channel *p)
> +{
> + struct atmel_pwm *ap = to_atmel_pwm(p);
> + u32 chid = 1 << p->chan;
> +
> + pwmc_writel(ap, PWMC_DIS, ap->sync_mask[p->chan] | chid);
> +}
> +
> +static inline void
> +__atmel_pwm_start(struct pwm_channel *p)
> +{
> + struct atmel_pwm *ap = to_atmel_pwm(p);
> + u32 chid = 1 << p->chan;
> +
> + pwmc_writel(ap, PWMC_ENA, ap->sync_mask[p->chan] | chid);
> +}
> +
> +static int
> +atmel_pwm_synchronize(struct pwm_channel *p,
> + struct pwm_channel *to_p)
> +{
> + unsigned long flags;
> + spin_lock_irqsave(&p->lock, flags);
> + __atmel_pwm_synchronize(p, to_p);
> + spin_unlock_irqrestore(&p->lock, flags);
> + return 0;
> +}
> +
> +static int
> +atmel_pwm_unsynchronize(struct pwm_channel *p,
> + struct pwm_channel *from_p)
> +{
> + unsigned long flags;
> + spin_lock_irqsave(&p->lock, flags);
> + __atmel_pwm_unsynchronize(p, from_p);
> + spin_unlock_irqrestore(&p->lock, flags);
> + return 0;
> +}
> +
> +static inline int
> +__atmel_pwm_config_polarity(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + u32 cmr = pwmc_chan_readl(p, PWMC_CMR);
> +
> + if (c->polarity)
> + cmr &= ~BIT(PWMC_CMR_CPOL);
> + else
> + cmr |= BIT(PWMC_CMR_CPOL);
> + pwmc_chan_writel(p, PWMC_CMR, cmr);
> + p->active_high = c->polarity ? 1 : 0;
> +
> + dev_dbg(p->pwm->dev, "polarity %d\n", c->polarity);
> + return 0;
> +}
> +
> +static inline int
> +__atmel_pwm_config_duty_ticks(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + u32 cmr, cprd, cpre, cdty;
> +
> + cmr = pwmc_chan_readl(p, PWMC_CMR);
> + cprd = pwmc_chan_readl(p, PWMC_CPRD);
> +
> + cpre = cmr & PWMC_CMR_CPRE_MASK;
> + cmr &= ~BIT(PWMC_CMR_CPD);
> +
> + cdty = cprd - (c->duty_ticks >> cpre);
> +
> + p->duty_ticks = c->duty_ticks;
> +
> + if (__atmel_pwm_is_on(p)) {
> + pwmc_chan_writel(p, PWMC_CMR, cmr);
> + pwmc_chan_writel(p, PWMC_CUPD, cdty);
> + } else
> + pwmc_chan_writel(p, PWMC_CDTY, cdty);
> +
> + dev_dbg(p->pwm->dev, "duty_ticks = %lu cprd = %x"
> + " cdty = %x cpre = %x\n", p->duty_ticks,
> + cprd, cdty, cpre);
> +
> + return 0;
> +}
> +
> +static inline int
> +__atmel_pwm_config_period_ticks(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + u32 cmr, cprd, cpre;
> +
> + cpre = fls(c->period_ticks);
> + if (cpre < 16)
> + cpre = 0;
> + else {
> + cpre -= 15;
> + if (cpre > 10)
> + return -EINVAL;
> + }
> +
> + cmr = pwmc_chan_readl(p, PWMC_CMR);
> + cmr &= ~PWMC_CMR_CPRE_MASK;
> + cmr |= cpre;
> +
> + cprd = c->period_ticks >> cpre;
> +
> + pwmc_chan_writel(p, PWMC_CMR, cmr);
> + pwmc_chan_writel(p, PWMC_CPRD, cprd);
> + p->period_ticks = c->period_ticks;
> +
> + dev_dbg(p->pwm->dev, "period_ticks = %lu cprd = %x cpre = %x\n",
> + p->period_ticks, cprd, cpre);
> +
> + return 0;
> +}
> +
> +static int
> +atmel_pwm_config_nosleep(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + int ret = 0;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&p->lock, flags);
> +
> + switch (c->config_mask) {
> +
> + case PWM_CONFIG_DUTY_TICKS:
> + __atmel_pwm_config_duty_ticks(p, c);
> + break;
> +
> + case PWM_CONFIG_STOP:
> + __atmel_pwm_stop(p);
> + break;
> +
> + case PWM_CONFIG_START:
> + __atmel_pwm_start(p);
> + break;
> +
> + case PWM_CONFIG_POLARITY:
> + __atmel_pwm_config_polarity(p, c);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + spin_unlock_irqrestore(&p->lock, flags);
> + return ret;
> +}
> +
> +static int
> +atmel_pwm_stop_sync(struct pwm_channel *p)
> +{
> + struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
> + int ret;
> + int was_on = __atmel_pwm_is_on(p);
> +
> + if (was_on) {
> + do {
> + init_completion(&p->complete);
> + set_bit(FLAG_STOP, &p->flags);
> + pwmc_writel(ap, PWMC_IER, 1 << p->chan);
> +
> + dev_dbg(p->pwm->dev, "waiting on stop_sync completion...\n");
> +
> + ret = wait_for_completion_interruptible(&p->complete);
> +
> + dev_dbg(p->pwm->dev, "stop_sync complete (%d)\n", ret);
> +
> + if (ret)
> + return ret;
> + } while (p->flags & BIT(FLAG_STOP));
> + }
> +
> + return was_on;
> +}
> +
> +static int
> +atmel_pwm_config(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + int was_on = 0;
> +
> + if (p->pwm->config_nosleep) {
> + if (!p->pwm->config_nosleep(p, c))
> + return 0;
> + }
> +
> + might_sleep();
> +
> + dev_dbg(p->pwm->dev, "config_mask %x\n", c->config_mask);
> +
> + was_on = atmel_pwm_stop_sync(p);
> + if (was_on < 0)
> + return was_on;
> +
> + if (c->config_mask & PWM_CONFIG_PERIOD_TICKS) {
> + __atmel_pwm_config_period_ticks(p, c);
> + if (!(c->config_mask & PWM_CONFIG_DUTY_TICKS)) {
> + struct pwm_channel_config d = {
> + .config_mask = PWM_CONFIG_DUTY_TICKS,
> + .duty_ticks = p->duty_ticks,
> + };
> + __atmel_pwm_config_duty_ticks(p, &d);
> + }
> + }
> +
> + if (c->config_mask & PWM_CONFIG_DUTY_TICKS)
> + __atmel_pwm_config_duty_ticks(p, c);
> +
> + if (c->config_mask & PWM_CONFIG_POLARITY)
> + __atmel_pwm_config_polarity(p, c);
> +
> + if ((c->config_mask & PWM_CONFIG_START)
> + || (was_on && !(c->config_mask & PWM_CONFIG_STOP)))
> + __atmel_pwm_start(p);
> +
> + return 0;
> +}
> +
> +static void
> +__atmel_pwm_set_callback(struct pwm_channel *p,
> + pwm_callback_t callback)
> +{
> + struct atmel_pwm *ap = container_of(p->pwm, struct atmel_pwm, pwm);
> +
> + p->callback = callback;
> + pwmc_writel(ap, p->callback ? PWMC_IER : PWMC_IDR, 1 << p->chan);
> +}
> +
> +static int
> +atmel_pwm_set_callback(struct pwm_channel *p,
> + pwm_callback_t callback)
> +{
> + struct atmel_pwm *ap = to_atmel_pwm(p);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ap->lock, flags);
> + __atmel_pwm_set_callback(p, callback);
> + spin_unlock_irqrestore(&ap->lock, flags);
> +
> + return 0;
> +}
> +
> +static int
> +atmel_pwm_request(struct pwm_channel *p)
> +{
> + struct atmel_pwm *ap = to_atmel_pwm(p);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&p->lock, flags);
> + clk_enable(ap->clk);
> + p->tick_hz = clk_get_rate(ap->clk);
> + __atmel_pwm_unsynchronize(p, NULL);
> + __atmel_pwm_stop(p);
> + spin_unlock_irqrestore(&p->lock, flags);
> +
> + return 0;
> +}
> +
> +static void
> +atmel_pwm_release(struct pwm_channel *p)
> +{
> + struct atmel_pwm *ap = to_atmel_pwm(p);
> + clk_disable(ap->clk);
> +}
> +
> +static irqreturn_t
> +atmel_pwmc_irq(int irq, void *data)
> +{
> + struct atmel_pwm *ap = data;
> + struct pwm_channel *p;
> + u32 isr;
> + int chid;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ap->lock, flags);
> +
> + isr = pwmc_readl(ap, PWMC_ISR);
> + for (chid = 0; isr; chid++, isr >>= 1) {
> + p = &ap->pwm.channels[chid];
> + if (isr & 1) {
> + if (p->callback)
> + p->callback(p);
> + if (p->flags & BIT(FLAG_STOP)) {
> + __atmel_pwm_stop(p);
> + clear_bit(FLAG_STOP, &p->flags);
> + }
> + complete_all(&p->complete);
> + }
> + }
> +
> + spin_unlock_irqrestore(&ap->lock, flags);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __devinit
> +atmel_pwmc_probe(struct platform_device *pdev)
> +{
> + struct atmel_pwm *ap;
> + struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + int ret = 0;
> +
> + ap = kzalloc(sizeof(*ap), GFP_KERNEL);
> + if (!ap) {
> + ret = -ENOMEM;
> + goto err_atmel_pwm_alloc;
> + }
> +
> + spin_lock_init(&ap->lock);
> + platform_set_drvdata(pdev, ap);
> +
> + ap->pwm.dev = &pdev->dev;
> + ap->pwm.bus_id = dev_name(&pdev->dev);
> +
> + ap->pwm.nchan = 4; /* TODO: true only for SAM9263 and AP7000 */
> + ap->ccnt_mask = 0xffffUL; /* TODO: true only for SAM9263 */
> +
> + ap->sync_mask = kzalloc(ap->pwm.nchan * sizeof(u32), GFP_KERNEL);
> + if (!ap->sync_mask) {
> + ret = -ENOMEM;
> + goto err_alloc_sync_masks;
> + }
> +
> + ap->pwm.owner = THIS_MODULE;
> + ap->pwm.request = atmel_pwm_request;
> + ap->pwm.release = atmel_pwm_release;
> + ap->pwm.config_nosleep = atmel_pwm_config_nosleep;
> + ap->pwm.config = atmel_pwm_config;
> + ap->pwm.synchronize = atmel_pwm_synchronize;
> + ap->pwm.unsynchronize = atmel_pwm_unsynchronize;
> + ap->pwm.set_callback = atmel_pwm_set_callback;
> +
> + ap->clk = clk_get(&pdev->dev, "pwm_clk");
> + if (PTR_ERR(ap->clk)) {
> + ret = -ENODEV;
> + goto err_clk_get;
> + }
> +
> + ap->iobase = ioremap_nocache(r->start, r->end - r->start + 1);
> + if (!ap->iobase) {
> + ret = -ENODEV;
> + goto err_ioremap;
> + }
> +
> + clk_enable(ap->clk);
> + pwmc_writel(ap, PWMC_DIS, -1);
> + pwmc_writel(ap, PWMC_IDR, -1);
> + clk_disable(ap->clk);
> +
> + ap->irq = platform_get_irq(pdev, 0);
> + if (ap->irq != -ENXIO) {
> + ret = request_irq(ap->irq, atmel_pwmc_irq, 0,
> + ap->pwm.bus_id, ap);
> + if (ret)
> + goto err_request_irq;
> + }
> +
> + ret = pwm_register(&ap->pwm);
> + if (ret)
> + goto err_pwm_register;
> +
> + return 0;
> +
> +err_pwm_register:
> + if (ap->irq != -ENXIO)
> + free_irq(ap->irq, ap);
> +err_request_irq:
> + iounmap(ap->iobase);
> +err_ioremap:
> + clk_put(ap->clk);
> +err_clk_get:
> + platform_set_drvdata(pdev, NULL);
> +err_alloc_sync_masks:
> + kfree(ap);
> +err_atmel_pwm_alloc:
> + return ret;
> +}
> +
> +static int __devexit
> +atmel_pwmc_remove(struct platform_device *pdev)
> +{
> + struct atmel_pwm *ap = platform_get_drvdata(pdev);
> + int ret;
> +
> + /* TODO: what can we do if this fails? */
> + ret = pwm_unregister(&ap->pwm);
> +
> + clk_enable(ap->clk);
> + pwmc_writel(ap, PWMC_IDR, -1);
> + pwmc_writel(ap, PWMC_DIS, -1);
> + clk_disable(ap->clk);
> +
> + if (ap->irq != -ENXIO)
> + free_irq(ap->irq, ap);
> +
> + clk_put(ap->clk);
> + iounmap(ap->iobase);
> +
> + kfree(ap);
> +
> + return 0;
> +}
> +
> +static struct platform_driver atmel_pwm_driver = {
> + .driver = {
> + .name = "atmel_pwmc",
> + .owner = THIS_MODULE,
> + },
> + .probe = atmel_pwmc_probe,
> + .remove = __devexit_p(atmel_pwmc_remove),
> +};
> +
> +static int __init atmel_pwm_init(void)
> +{
> + return platform_driver_register(&atmel_pwm_driver);
> +}
> +module_init(atmel_pwm_init);
> +
> +static void __exit atmel_pwm_exit(void)
> +{
> + platform_driver_unregister(&atmel_pwm_driver);
> +}
> +module_exit(atmel_pwm_exit);
> +
> +MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
> +MODULE_DESCRIPTION("Driver for Atmel PWMC peripheral");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:atmel_pwmc");
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PWM 01/10] API to consolidate PWM devices behind a common user and kernel interface
From: Grant Likely @ 2010-10-16 7:42 UTC (permalink / raw)
To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1285946271-17728-1-git-send-email-bgat@billgatliff.com>
On Fri, Oct 01, 2010 at 10:17:42AM -0500, Bill Gatliff wrote:
> Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
Hi Bill, comments below.
> ---
> Documentation/pwm.txt | 260 +++++++++++++++++++
> drivers/pwm/pwm.c | 635 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pwm.h | 31 ---
> include/linux/pwm/pwm.h | 128 ++++++++++
> 4 files changed, 1023 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/pwm.txt
> create mode 100644 drivers/pwm/pwm.c
> delete mode 100644 include/linux/pwm.h
> create mode 100644 include/linux/pwm/pwm.h
>
> diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
> new file mode 100644
> index 0000000..34b1e5a
> --- /dev/null
> +++ b/Documentation/pwm.txt
> @@ -0,0 +1,260 @@
> + Generic PWM Device API
> +
> + August 5, 2010
> + Bill Gatliff
> + <bgat@billgatliff.com>
> +
> +
> +
> +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, pwm_channel and pwm_channel_config
> +structures to invoke services in PWM peripheral device drivers.
> +Consult drivers/pwm/atmel-pwm.c for an example driver.
> +
> +There are two classes of adopters of the PWM framework:
> +
> + "Users" -- those wishing to employ the API merely to produce PWM
> + signals; once they have identified the appropriate physical output
> + on the platform in question, they don't care about the details of
> + the underlying hardware
> +
> + "Driver authors" -- those wishing to bind devices that can generate
> + PWM signals to the Generic PWM Device API, so that the services of
> + those devices become available to users. Assuming the hardware can
> + support the needs of a user, driver authors don't care about the
> + details of the user's application
> +
> +Generally speaking, users will first invoke pwm_request() to obtain a
> +handle to a PWM device. They will then pass that handle to functions
> +like pwm_duty_ns() and pwm_period_ns() to set the duty cycle and
> +period of the PWM signal, respectively. They will also invoke
> +pwm_start() and pwm_stop() to turn the signal on and off.
> +
> +The 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 constructed--- including the
> +number of distinct output "channels" the peripheral offers. They then
> +invoke pwm_register() (usually from within their device's probe()
> +handler) to make the PWM API aware of their device. The framework
> +will call back to the methods described in the pwm_device structure as
> +users begin to configure and utilize the hardware.
> +
> +Note that PWM signals can be produced by a variety of peripherals,
> +beyond the true "PWM hardware" offered by many system-on-chip devices.
> +Other possibilities include timer/counters with compare-match
> +capabilities, carefully-programmed synchronous serial ports
> +(e.g. SPI), and GPIO pins driven by kernel interval timers. With a
> +proper pwm_device structure, these devices and pseudo-devices can all
> +be accommodated by the Generic PWM Device API framework.
> +
> +
> +
> +Using the API to Generate PWM Signals -- Basic Functions for Users
> +
> +
> +pwm_request() -- Returns a pwm_channel pointer, which is subsequently
> +passed to the other user-related PWM functions. Once requested, a PWM
> +channel is marked as in-use and subsequent requests prior to
> +pwm_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.
I'm not hugely keen on the naming approach, and I'd rather see the
pwm core be responsible for the namespace. Something like "pwm-%i:%i" where the first number is a controller enumeration (dynamically assigned) and the second is the channel number. Matching a controller can be accomplished by looking
at the parent device. The name doesn't need to be encoded directly
into the pwm device for that to work.
However, it isn't a major issue that I have and I won't make a big
stink about it.
> +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_channel_config structure to the
> +associated device driver. This function is invoked by pwm_start(),
> +pwm_duty_ns(), etc. and is one of two main entry points to the PWM
> +driver for the hardware being used. The configuration change is
> +guaranteed atomic if multiple configuration changes are specified.
> +This function might sleep, depending on what the device driver has to
> +do to satisfy the request. All PWM device drivers must support this
> +entry point.
> +
> +
> +pwm_config_nosleep() -- Passes a pwm_channel_config structure to the
> +associated device driver. If the driver must sleep in order to
> +implement the requested configuration change, -EWOULDBLOCK is
> +returned. Users may call this function from interrupt handlers, for
> +example. This is the other main entry point into the PWM hardware
> +driver, but not all device drivers support this entry point.
> +
> +
> +pwm_synchronize(), pwm_unsynchronize() -- "Synchronizes" two or more
> +PWM channels, if the underlying hardware permits. (If it doesn't, the
> +framework facilitates emulating this capability but it is not yet
> +implemented). Synchronized channels will start and stop
> +simultaneously when any single channel in the group is started or
> +stopped. Use pwm_unsynchronize(..., NULL) to completely detach a
> +channel from any other synchronized channels. 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 device registered
> +thereby receives bus_id "atmel_pwmc.0", which is what you put in
> +pwm_device.bus_id. Channels are then named "atmel_pwmc.0:[0-3]".
> +(Hint: just use pdev->dev.bus_id in your probe() method).
> +
> +
> +nchan -- the number of distinct output channels provided by the device.
> +
> +
> +request -- (optional) Invoked each time a user requests a channel.
> +Use to turn on clocks, clean up register states, etc. The framework
> +takes care of device locking/unlocking; you will see only successful
> +requests.
> +
> +
> +free -- (optional) Callback for each time a user relinquishes a
> +channel. The framework will have already stopped, unsynchronized and
> +un-handled the channel. Use to turn off clocks, etc. as necessary.
> +
> +
> +synchronize, unsynchronize -- (optional) Callbacks to
> +synchronize/unsynchronize channels. Some devices provide this
> +capability in hardware; for others, it can be emulated (see
> +atmel_pwmc.c's sync_mask for an example).
> +
> +
> +set_callback -- (optional) Invoked when a user requests a handler. If
> +the hardware supports an end-of-period interrupt, invoke the function
> +indicated during your interrupt handler. The callback function itself
> +is always internal to the API, and does not map directly to the user's
> +callback function.
> +
> +
> +config -- Invoked to change the device configuration, always from a
> +sleep-capable context. All the changes indicated must be performed
> +atomically, ideally synchronized to an end-of-period event (so that
> +you avoid short or long output pulses). You may sleep, etc. as
> +necessary within this function.
> +
> +
> +config_nosleep -- (optional) Invoked to change device configuration
> +from within a context that is not allowed to sleep. If you cannot
> +perform the requested configuration changes without sleeping, return
> +-EWOULDBLOCK.
> +
> +
> +
> +Acknowledgements
> +
> +
> +The author expresses his gratitude to the countless developers who
> +have reviewed and submitted feedback on the various versions of the
> +Generic PWM Device API code, and those who have submitted drivers and
> +applications that use the framework. You know who you are. ;)
> +
> diff --git a/drivers/pwm/pwm.c b/drivers/pwm/pwm.c
> new file mode 100644
> index 0000000..2774116
> --- /dev/null
> +++ b/drivers/pwm/pwm.c
> @@ -0,0 +1,635 @@
> +/*
> + * drivers/pwm/pwm.c
Nitpick: putting the filename into the file itself I find tends to be
useless
> + *
> + * Copyright (C) 2010 Bill Gatliff <bgat@billgatliff.com>
> + *
> + * 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/spinlock.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 int __pwm_create_sysfs(struct pwm_device *pwm);
If you order the functions in this file correct, the forward
declaration should not be necessary.
> +
> +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;
> +
> +int pwm_register(struct pwm_device *pwm)
> +{
> + struct pwm_channel *p;
> + int wchan;
> + int ret;
> +
> + spin_lock_init(&pwm->list_lock);
> +
> + p = kcalloc(pwm->nchan, sizeof(*p), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + for (wchan = 0; wchan < pwm->nchan; wchan++) {
> + spin_lock_init(&p[wchan].lock);
> + init_completion(&p[wchan].complete);
> + p[wchan].chan = wchan;
> + p[wchan].pwm = pwm;
> + }
> +
> + pwm->channels = p;
> +
> + mutex_lock(&device_list_mutex);
> +
> + list_add_tail(&pwm->list, &pwm_device_list);
> + ret = __pwm_create_sysfs(pwm);
> + if (ret) {
> + mutex_unlock(&device_list_mutex);
> + goto err_create_sysfs;
> + }
> +
> + mutex_unlock(&device_list_mutex);
> +
> + dev_info(pwm->dev, "%d channel%s\n", pwm->nchan,
> + pwm->nchan > 1 ? "s" : "");
> + return 0;
> +
> +err_create_sysfs:
> + kfree(p);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(pwm_register);
> +
> +static int __match_device(struct device *dev, void *data)
> +{
> + return dev_get_drvdata(dev) == data;
> +}
> +
> +int pwm_unregister(struct pwm_device *pwm)
> +{
> + int wchan;
> + struct device *dev;
> +
> + mutex_lock(&device_list_mutex);
> +
> + for (wchan = 0; wchan < pwm->nchan; wchan++) {
> + if (pwm->channels[wchan].flags & BIT(FLAG_REQUESTED)) {
> + mutex_unlock(&device_list_mutex);
> + return -EBUSY;
> + }
> + }
> +
> + for (wchan = 0; wchan < pwm->nchan; wchan++) {
> + dev = class_find_device(&pwm_class, NULL,
> + &pwm->channels[wchan],
> + __match_device);
If the pwm_channel structure carried a pointer to its device
structure, then this lookup wouldn't be needed.
> + if (dev) {
> + put_device(dev);
> + device_unregister(dev);
> + }
> + }
> +
> + kfree(pwm->channels);
> + list_del(&pwm->list);
> + mutex_unlock(&device_list_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pwm_unregister);
> +
> +static struct pwm_device *
> +__pwm_find_device(const char *bus_id)
> +{
> + struct pwm_device *p;
> +
> + list_for_each_entry(p, &pwm_device_list, list) {
> + if (!strcmp(bus_id, p->bus_id))
> + return p;
> + }
> + return NULL;
> +}
> +
> +static int
> +__pwm_request_channel(struct pwm_channel *p,
> + const char *requester)
> +{
> + int ret;
> +
> + if (test_and_set_bit(FLAG_REQUESTED, &p->flags))
> + return -EBUSY;
> +
> + if (p->pwm->request) {
> + ret = p->pwm->request(p);
> + if (ret) {
> + clear_bit(FLAG_REQUESTED, &p->flags);
> + return ret;
> + }
> + }
> +
> + p->requester = requester;
> + if (!strcmp(requester, REQUEST_SYSFS))
> + p->pid = current->pid;
> +
> + return 0;
> +}
> +
> +struct pwm_channel *
> +pwm_request(const char *bus_id,
> + int chan,
> + const char *requester)
> +{
> + struct pwm_device *p;
> + int ret;
> +
> + mutex_lock(&device_list_mutex);
> +
> + p = __pwm_find_device(bus_id);
> + if (!p || chan >= p->nchan)
> + goto err_no_device;
> +
> + if (!try_module_get(p->owner))
> + goto err_module_get_failed;
> +
> + ret = __pwm_request_channel(&p->channels[chan], requester);
> + if (ret)
> + goto err_request_failed;
> +
> + mutex_unlock(&device_list_mutex);
> + return &p->channels[chan];
> +
> +err_request_failed:
> + module_put(p->owner);
> +err_module_get_failed:
> +err_no_device:
> + mutex_unlock(&device_list_mutex);
> + return NULL;
> +}
> +EXPORT_SYMBOL(pwm_request);
> +
> +void pwm_release(struct pwm_channel *p)
> +{
> + mutex_lock(&device_list_mutex);
> +
> + if (!test_and_clear_bit(FLAG_REQUESTED, &p->flags))
> + goto done;
> +
> + pwm_stop(p);
> + pwm_unsynchronize(p, NULL);
> + pwm_set_handler(p, NULL, NULL);
> +
> + if (p->pwm->release)
> + p->pwm->release(p);
> + module_put(p->pwm->owner);
> +done:
> + mutex_unlock(&device_list_mutex);
> +}
> +EXPORT_SYMBOL(pwm_release);
> +
> +unsigned long pwm_ns_to_ticks(struct pwm_channel *p,
> + unsigned long nsecs)
> +{
> + unsigned long long ticks;
> +
> + ticks = nsecs;
> + ticks *= p->tick_hz;
> + do_div(ticks, 1000000000);
> + return ticks;
> +}
> +EXPORT_SYMBOL(pwm_ns_to_ticks);
> +
> +unsigned long pwm_ticks_to_ns(struct pwm_channel *p,
> + unsigned long ticks)
> +{
> + unsigned long long ns;
> +
> + if (!p->tick_hz)
> + return 0;
> +
> + ns = ticks;
> + ns *= 1000000000UL;
> + do_div(ns, p->tick_hz);
> + return ns;
> +}
> +EXPORT_SYMBOL(pwm_ticks_to_ns);
> +
> +static void
> +pwm_config_ns_to_ticks(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + if (c->config_mask & PWM_CONFIG_PERIOD_NS) {
> + c->period_ticks = pwm_ns_to_ticks(p, c->period_ns);
> + c->config_mask &= ~PWM_CONFIG_PERIOD_NS;
> + c->config_mask |= PWM_CONFIG_PERIOD_TICKS;
> + }
> +
> + if (c->config_mask & PWM_CONFIG_DUTY_NS) {
> + c->duty_ticks = pwm_ns_to_ticks(p, c->duty_ns);
> + c->config_mask &= ~PWM_CONFIG_DUTY_NS;
> + c->config_mask |= PWM_CONFIG_DUTY_TICKS;
> + }
> +}
> +
> +static void
> +pwm_config_percent_to_ticks(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + if (c->config_mask & PWM_CONFIG_DUTY_PERCENT) {
> + if (c->config_mask & PWM_CONFIG_PERIOD_TICKS)
> + c->duty_ticks = c->period_ticks;
> + else
> + c->duty_ticks = p->period_ticks;
> +
> + c->duty_ticks *= c->duty_percent;
> + c->duty_ticks /= 100;
> + c->config_mask &= ~PWM_CONFIG_DUTY_PERCENT;
> + c->config_mask |= PWM_CONFIG_DUTY_TICKS;
> + }
> +}
> +
> +int pwm_config_nosleep(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + if (!p->pwm->config_nosleep)
> + return -EINVAL;
> +
> + pwm_config_ns_to_ticks(p, c);
> + pwm_config_percent_to_ticks(p, c);
> +
> + return p->pwm->config_nosleep(p, c);
> +}
> +EXPORT_SYMBOL(pwm_config_nosleep);
> +
> +int pwm_config(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + int ret = 0;
> +
> + if (unlikely(!p->pwm->config))
> + return -EINVAL;
> +
> + pwm_config_ns_to_ticks(p, c);
> + pwm_config_percent_to_ticks(p, c);
> +
> + switch (c->config_mask & (PWM_CONFIG_PERIOD_TICKS
> + | PWM_CONFIG_DUTY_TICKS)) {
> + case PWM_CONFIG_PERIOD_TICKS:
> + if (p->duty_ticks > c->period_ticks) {
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + case PWM_CONFIG_DUTY_TICKS:
> + if (p->period_ticks < c->duty_ticks) {
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + case PWM_CONFIG_DUTY_TICKS | PWM_CONFIG_PERIOD_TICKS:
> + if (c->duty_ticks > c->period_ticks) {
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + default:
> + break;
> + }
> +
> +err:
> + if (ret)
> + return ret;
> + return p->pwm->config(p, c);
> +}
> +EXPORT_SYMBOL(pwm_config);
> +
> +int pwm_set_period_ns(struct pwm_channel *p,
> + unsigned long period_ns)
> +{
> + struct pwm_channel_config c = {
> + .config_mask = PWM_CONFIG_PERIOD_TICKS,
> + .period_ticks = pwm_ns_to_ticks(p, period_ns),
> + };
> +
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_set_period_ns);
> +
> +unsigned long pwm_get_period_ns(struct pwm_channel *p)
> +{
> + return pwm_ticks_to_ns(p, p->period_ticks);
> +}
> +EXPORT_SYMBOL(pwm_get_period_ns);
> +
> +int pwm_set_duty_ns(struct pwm_channel *p,
> + unsigned long duty_ns)
> +{
> + struct pwm_channel_config c = {
> + .config_mask = PWM_CONFIG_DUTY_TICKS,
> + .duty_ticks = pwm_ns_to_ticks(p, duty_ns),
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_set_duty_ns);
> +
> +unsigned long pwm_get_duty_ns(struct pwm_channel *p)
> +{
> + return pwm_ticks_to_ns(p, p->duty_ticks);
> +}
> +EXPORT_SYMBOL(pwm_get_duty_ns);
> +
> +int pwm_set_duty_percent(struct pwm_channel *p,
> + int percent)
> +{
> + struct pwm_channel_config c = {
> + .config_mask = PWM_CONFIG_DUTY_PERCENT,
> + .duty_percent = percent,
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_set_duty_percent);
> +
> +int pwm_set_polarity(struct pwm_channel *p,
> + int active_high)
> +{
> + struct pwm_channel_config c = {
> + .config_mask = PWM_CONFIG_POLARITY,
> + .polarity = !!active_high,
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_set_polarity);
> +
> +int pwm_start(struct pwm_channel *p)
> +{
> + struct pwm_channel_config c = {
> + .config_mask = PWM_CONFIG_START,
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_start);
> +
> +int pwm_stop(struct pwm_channel *p)
> +{
> + struct pwm_channel_config c = {
> + .config_mask = PWM_CONFIG_STOP,
> + };
> + return pwm_config(p, &c);
> +}
> +EXPORT_SYMBOL(pwm_stop);
The above 8 functions are so tiny that they should probably be static
inlines in the header file.
> +
> +int pwm_synchronize(struct pwm_channel *p,
> + struct pwm_channel *to_p)
> +{
> + if (p->pwm != to_p->pwm) {
> + /* TODO: support cross-device synchronization */
> + return -EINVAL;
> + }
> +
> + if (!p->pwm->synchronize)
> + return -EINVAL;
> +
> + return p->pwm->synchronize(p, to_p);
> +}
> +EXPORT_SYMBOL(pwm_synchronize);
> +
> +int pwm_unsynchronize(struct pwm_channel *p,
> + struct pwm_channel *from_p)
> +{
> + if (from_p && (p->pwm != from_p->pwm)) {
> + /* TODO: support cross-device synchronization */
> + return -EINVAL;
> + }
> +
> + if (!p->pwm->unsynchronize)
> + return -EINVAL;
> +
> + return p->pwm->unsynchronize(p, from_p);
> +}
> +EXPORT_SYMBOL(pwm_unsynchronize);
> +
> +static void pwm_handler(struct work_struct *w)
> +{
> + struct pwm_channel *p = container_of(w, struct pwm_channel,
> + handler_work);
> + if (p->handler && p->handler(p, p->handler_data))
> + pwm_stop(p);
> +}
> +
> +static void __pwm_callback(struct pwm_channel *p)
> +{
> + queue_work(pwm_handler_workqueue, &p->handler_work);
> +}
> +
> +int pwm_set_handler(struct pwm_channel *p,
> + pwm_handler_t handler,
> + void *data)
> +{
> + if (p->pwm->set_callback) {
> + p->handler_data = data;
> + p->handler = handler;
> + INIT_WORK(&p->handler_work, pwm_handler);
> + return p->pwm->set_callback(p, handler ? __pwm_callback : NULL);
> + }
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL(pwm_set_handler);
> +
> +static ssize_t pwm_run_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + struct pwm_channel *p = dev_get_drvdata(dev);
> + if (sysfs_streq(buf, "1"))
> + pwm_start(p);
> + else if (sysfs_streq(buf, "0"))
> + pwm_stop(p);
> + return len;
> +}
> +static DEVICE_ATTR(run, 0200, NULL, pwm_run_store);
> +
> +static ssize_t pwm_duty_ns_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_channel *p = dev_get_drvdata(dev);
> + return sprintf(buf, "%lu\n", pwm_get_duty_ns(p));
> +}
> +
> +static ssize_t pwm_duty_ns_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + unsigned long duty_ns;
> + struct pwm_channel *p = dev_get_drvdata(dev);
> +
> + if (1 == sscanf(buf, "%lu", &duty_ns))
> + pwm_set_duty_ns(p, duty_ns);
> + return len;
> +}
> +static DEVICE_ATTR(duty_ns, 0644, pwm_duty_ns_show, pwm_duty_ns_store);
> +
> +static ssize_t pwm_period_ns_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_channel *p = dev_get_drvdata(dev);
> + return sprintf(buf, "%lu\n", pwm_get_period_ns(p));
> +}
> +
> +static ssize_t pwm_period_ns_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + unsigned long period_ns;
> + struct pwm_channel *p = dev_get_drvdata(dev);
> +
> + if (1 == sscanf(buf, "%lu", &period_ns))
> + pwm_set_period_ns(p, period_ns);
> + return len;
> +}
> +static DEVICE_ATTR(period_ns, 0644, pwm_period_ns_show, pwm_period_ns_store);
> +
> +static ssize_t pwm_polarity_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_channel *p = dev_get_drvdata(dev);
> + return sprintf(buf, "%d\n", !!p->active_high);
> +}
> +
> +static ssize_t pwm_polarity_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + int polarity;
> + struct pwm_channel *p = dev_get_drvdata(dev);
> +
> + if (1 == sscanf(buf, "%d", &polarity))
> + pwm_set_polarity(p, polarity);
> + return len;
> +}
> +static DEVICE_ATTR(polarity, 0644, pwm_polarity_show, pwm_polarity_store);
> +
> +static ssize_t pwm_request_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pwm_channel *p = dev_get_drvdata(dev);
> + mutex_lock(&device_list_mutex);
> + __pwm_request_channel(p, REQUEST_SYSFS);
> + mutex_unlock(&device_list_mutex);
> +
> + if (p->pid)
> + return sprintf(buf, "%s %d\n", p->requester, p->pid);
> + else
> + return sprintf(buf, "%s\n", p->requester);
> +}
> +
> +static ssize_t pwm_request_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + struct pwm_channel *p = dev_get_drvdata(dev);
> + pwm_release(p);
> + return len;
> +}
> +static DEVICE_ATTR(request, 0644, pwm_request_show, pwm_request_store);
> +
> +static const struct attribute *pwm_attrs[] =
> +{
> + &dev_attr_run.attr,
> + &dev_attr_polarity.attr,
> + &dev_attr_duty_ns.attr,
> + &dev_attr_period_ns.attr,
> + &dev_attr_request.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group pwm_device_attr_group = {
> + .attrs = (struct attribute **)pwm_attrs,
> +};
> +
> +static int __pwm_create_sysfs(struct pwm_device *pwm)
> +{
> + int ret = 0;
> + struct device *dev;
> + int wchan;
> +
> + for (wchan = 0; wchan < pwm->nchan; wchan++) {
> + dev = device_create(&pwm_class, pwm->dev, MKDEV(0, 0),
> + pwm->channels + wchan,
> + "%s:%d", pwm->bus_id, wchan);
> + if (!dev)
> + goto err_dev_create;
> + ret = sysfs_create_group(&dev->kobj, &pwm_device_attr_group);
> + if (ret)
> + goto err_dev_create;
> + }
Rather than open coding the registration of sysfs files, I believe the
right thing to do is to use class attributes so that the files get
automatically registered for you and are immediately advertised to
userspace (very important for correct interaction with udev).
Take a look at struct class->dev_attrs. It is just a null terminated
list of attributes that need to be registered for each device that is
a member of the class. It also takes care of unwinding correctly if
the registration fails.
Also, sysfs_create_group() should not be called directly when working
with devices.
> +
> + return ret;
> +
> +err_dev_create:
> + for (wchan = 0; wchan < pwm->nchan; wchan++) {
> + dev = class_find_device(&pwm_class, NULL,
> + &pwm->channels[wchan],
> + __match_device);
> + if (dev) {
> + put_device(dev);
> + device_unregister(dev);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static struct class_attribute pwm_class_attrs[] = {
> + __ATTR_NULL,
> +};
> +
> +static struct class pwm_class = {
> + .name = "pwm",
> + .owner = THIS_MODULE,
> +
> + .class_attrs = pwm_class_attrs,
> +};
> +
> +static int __init pwm_init(void)
> +{
> + int ret;
> +
> + /* TODO: how to deal with devices that register very early? */
Same thing we always do for bootstrapping. If it *must* be early,
then we do a simple direct access to get things running in platform
code, and then take over in the driver when it finally gets probed.
The driver model helps with many things, but really early stuff isn't
one of them.
> + ret = class_register(&pwm_class);
> + if (ret < 0)
> + return ret;
> +
> + pwm_handler_workqueue = create_workqueue("pwmd");
> +
> + return 0;
> +}
> +postcore_initcall(pwm_init);
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> deleted file mode 100644
> index 7c77575..0000000
> --- a/include/linux/pwm.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -#ifndef __LINUX_PWM_H
> -#define __LINUX_PWM_H
> -
> -struct pwm_device;
> -
> -/*
> - * pwm_request - request a PWM device
> - */
> -struct pwm_device *pwm_request(int pwm_id, const char *label);
> -
> -/*
> - * pwm_free - free a PWM device
> - */
> -void pwm_free(struct pwm_device *pwm);
> -
> -/*
> - * pwm_config - change a PWM device configuration
> - */
> -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
> -
> -/*
> - * pwm_enable - start a PWM output toggling
> - */
> -int pwm_enable(struct pwm_device *pwm);
> -
> -/*
> - * pwm_disable - stop a PWM output toggling
> - */
> -void pwm_disable(struct pwm_device *pwm);
> -
> -#endif /* __LINUX_PWM_H */
> diff --git a/include/linux/pwm/pwm.h b/include/linux/pwm/pwm.h
> new file mode 100644
> index 0000000..a10824c
> --- /dev/null
> +++ b/include/linux/pwm/pwm.h
> @@ -0,0 +1,128 @@
> +/*
> + * include/linux/pwm.h
> + *
> + * Copyright (C) 2010 Bill Gatliff < bgat@billgatliff.com>
> + *
> + * 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 {
> + PWM_CONFIG_DUTY_TICKS = BIT(0),
> + PWM_CONFIG_PERIOD_TICKS = BIT(1),
> + PWM_CONFIG_POLARITY = BIT(2),
> + PWM_CONFIG_START = BIT(3),
> + PWM_CONFIG_STOP = BIT(4),
> +
> + PWM_CONFIG_HANDLER = BIT(5),
> +
> + PWM_CONFIG_DUTY_NS = BIT(6),
> + PWM_CONFIG_DUTY_PERCENT = BIT(7),
> + PWM_CONFIG_PERIOD_NS = BIT(8),
> +};
> +
> +struct pwm_channel;
> +struct work_struct;
> +
> +typedef int (*pwm_handler_t)(struct pwm_channel *p, void *data);
> +typedef void (*pwm_callback_t)(struct pwm_channel *p);
> +
> +struct pwm_channel_config {
> + int config_mask;
> + unsigned long duty_ticks;
> + unsigned long period_ticks;
> + int polarity;
> +
> + pwm_handler_t handler;
> +
> + unsigned long duty_ns;
> + unsigned long period_ns;
> + int duty_percent;
> +};
> +
> +struct pwm_device {
> + struct list_head list;
> + spinlock_t list_lock;
> + struct device *dev;
> + struct module *owner;
> + struct pwm_channel *channels;
> +
> + const char *bus_id;
> + int nchan;
> +
> + int (*request) (struct pwm_channel *p);
> + void (*release) (struct pwm_channel *p);
> + int (*config) (struct pwm_channel *p, struct pwm_channel_config *c);
> + int (*config_nosleep)(struct pwm_channel *p, struct pwm_channel_config *c);
> + int (*synchronize) (struct pwm_channel *p, struct pwm_channel *to_p);
> + int (*unsynchronize)(struct pwm_channel *p, struct pwm_channel *from_p);
> + int (*set_callback) (struct pwm_channel *p, pwm_callback_t callback);
> +};
> +
> +int pwm_register(struct pwm_device *pwm);
> +int pwm_unregister(struct pwm_device *pwm);
> +
> +enum {
> + FLAG_REQUESTED = 0,
> + FLAG_STOP = 1,
> +};
> +
> +struct pwm_channel {
> + struct list_head list;
> + struct pwm_device *pwm;
> + const char *requester;
> + pid_t pid;
> + int chan;
> + unsigned long flags;
> + unsigned long tick_hz;
> +
> + spinlock_t lock;
> + struct completion complete;
> +
> + pwm_callback_t callback;
> +
> + struct work_struct handler_work;
> + pwm_handler_t handler;
> + void *handler_data;
> +
> + int active_high;
> + unsigned long period_ticks;
> + unsigned long duty_ticks;
> +};
> +
> +struct gpio_pwm_platform_data {
> + int gpio;
> +};
> +
> +struct pwm_channel *pwm_request(const char *bus_id, int chan, const char *requester);
> +void pwm_release(struct pwm_channel *pwm);
> +int pwm_config_nosleep(struct pwm_channel *pwm, struct pwm_channel_config *c);
> +int pwm_config(struct pwm_channel *pwm, struct pwm_channel_config *c);
> +unsigned long pwm_ns_to_ticks(struct pwm_channel *pwm, unsigned long nsecs);
> +unsigned long pwm_ticks_to_ns(struct pwm_channel *pwm, unsigned long ticks);
> +int pwm_set_period_ns(struct pwm_channel *pwm, unsigned long period_ns);
> +unsigned long pwm_get_period_ns(struct pwm_channel *pwm);
> +int pwm_set_duty_ns(struct pwm_channel *pwm, unsigned long duty_ns);
> +int pwm_set_duty_percent(struct pwm_channel *pwm, int percent);
> +unsigned long pwm_get_duty_ns(struct pwm_channel *pwm);
> +int pwm_set_polarity(struct pwm_channel *pwm, int active_high);
> +int pwm_start(struct pwm_channel *pwm);
> +int pwm_stop(struct pwm_channel *pwm);
> +int pwm_set_handler(struct pwm_channel *pwm, pwm_handler_t handler, void *data);
> +int pwm_synchronize(struct pwm_channel *p, struct pwm_channel *to_p);
> +int pwm_unsynchronize(struct pwm_channel *p, struct pwm_channel *from_p);
> +
> +#endif /* __LINUX_PWM_H */
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PWM 02/10] Emulates PWM hardware using a high-resolution timer and a GPIO pin
From: Grant Likely @ 2010-10-16 6:54 UTC (permalink / raw)
To: Bill Gatliff; +Cc: linux-embedded
In-Reply-To: <1285946271-17728-2-git-send-email-bgat@billgatliff.com>
On Fri, Oct 01, 2010 at 10:17:43AM -0500, Bill Gatliff wrote:
> Signed-off-by: Bill Gatliff <bgat@billgatliff.com>
> ---
Code looks pretty clean. Minor comments below, plus a rehash of an
old argument.
> drivers/pwm/gpio.c | 298 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 298 insertions(+), 0 deletions(-)
> create mode 100644 drivers/pwm/gpio.c
>
> diff --git a/drivers/pwm/gpio.c b/drivers/pwm/gpio.c
> new file mode 100644
> index 0000000..f4aab06
> --- /dev/null
> +++ b/drivers/pwm/gpio.c
> @@ -0,0 +1,298 @@
> +/*
> + * drivers/pwm/gpio.c
> + *
> + * Models a single-channel PWM device using a timer and a GPIO pin.
> + *
> + * Copyright (C) 2010 Bill Gatliff <bgat@billgatliff.com>
> + *
> + * 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/hrtimer.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/workqueue.h>
> +#include <linux/gpio.h>
> +#include <linux/pwm/pwm.h>
> +
> +struct gpio_pwm {
> + struct pwm_device pwm;
> + struct hrtimer timer;
> + struct work_struct work;
> + pwm_callback_t callback;
> + int gpio;
> + unsigned long polarity : 1;
> + unsigned long active : 1;
> +};
> +
> +static inline struct gpio_pwm *to_gpio_pwm(const struct pwm_channel *p)
> +{
> + return container_of(p->pwm, struct gpio_pwm, pwm);
> +}
> +
> +static void
> +gpio_pwm_work (struct work_struct *work)
By breaking the line here; 'grep gpio_pwm_work' is a lot less useful
because it doesn't tell you the return type (a very common thing to do
when figuring out how an API works). Only split lines if you really
need to (ie, longer than 80 chars), and if you do need a line break,
then break in the parameters list.
> +{
> + struct gpio_pwm *gp = container_of(work, struct gpio_pwm, work);
> +
> + if (gp->active)
> + gpio_direction_output(gp->gpio, gp->polarity ? 1 : 0);
> + else
> + gpio_direction_output(gp->gpio, gp->polarity ? 0 : 1);
> +}
Bitwise operations are probably faster than two conditionals.
How about:
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, timer);
> + ktime_t tnew;
> +
> + if (unlikely(gp->pwm.channels[0].duty_ticks == 0))
> + gp->active = 0;
This function references gp->pwm.channels[0] a lot. You might want to
load the pointer into a local variable in the interest of readability.
> + else if (unlikely(gp->pwm.channels[0].duty_ticks
> + == gp->pwm.channels[0].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->pwm.channels[0].callback)
> + gp->pwm.channels[0].callback(&gp->pwm.channels[0]);
> +
> + if (unlikely(!gp->active &&
> + (gp->pwm.channels[0].flags & BIT(FLAG_STOP)))) {
> + clear_bit(FLAG_STOP, &gp->pwm.channels[0].flags);
> + complete_all(&gp->pwm.channels[0].complete);
> + return HRTIMER_NORESTART;
> + }
> +
> + if (gp->active)
> + tnew = ktime_set(0, gp->pwm.channels[0].duty_ticks);
> + else
> + tnew = ktime_set(0, gp->pwm.channels[0].period_ticks
> + - gp->pwm.channels[0].duty_ticks);
> + hrtimer_start(&gp->timer, tnew, HRTIMER_MODE_REL);
> +
> + return HRTIMER_NORESTART;
> +}
> +
> +static void gpio_pwm_start(struct pwm_channel *p)
> +{
> + struct gpio_pwm *gp = to_gpio_pwm(p);
> +
> + gp->active = 0;
> + gpio_pwm_timeout(&gp->timer);
> +}
> +
> +static int
> +gpio_pwm_config_nosleep(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + struct gpio_pwm *gp = to_gpio_pwm(p);
> + int ret = 0;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&p->lock, flags);
> +
> + switch (c->config_mask) {
> +
> + case PWM_CONFIG_DUTY_TICKS:
> + p->duty_ticks = c->duty_ticks;
> + break;
> +
> + case PWM_CONFIG_START:
> + if (!hrtimer_active(&gp->timer)) {
> + gpio_pwm_start(p);
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + spin_unlock_irqrestore(&p->lock, flags);
> + return ret;
> +}
> +
> +static int
> +gpio_pwm_stop_sync(struct pwm_channel *p)
> +{
> + struct gpio_pwm *gp = to_gpio_pwm(p);
> + int ret;
> + int was_on = hrtimer_active(&gp->timer);
> +
> + if (was_on) {
> + do {
> + init_completion(&p->complete);
> + set_bit(FLAG_STOP, &p->flags);
> + ret = wait_for_completion_interruptible(&p->complete);
> + if (ret)
> + return ret;
> + } while (p->flags & BIT(FLAG_STOP));
> + }
> +
> + return was_on;
> +}
> +
> +static int
> +gpio_pwm_config(struct pwm_channel *p,
> + struct pwm_channel_config *c)
> +{
> + struct gpio_pwm *gp = to_gpio_pwm(p);
> + int was_on = 0;
> +
> + if (p->pwm->config_nosleep) {
> + if (!p->pwm->config_nosleep(p, c))
> + return 0;
> + }
> +
> + might_sleep();
> +
> + was_on = gpio_pwm_stop_sync(p);
> + if (was_on < 0)
> + return was_on;
> +
> + if (c->config_mask & PWM_CONFIG_PERIOD_TICKS)
> + p->period_ticks = c->period_ticks;
> +
> + if (c->config_mask & PWM_CONFIG_DUTY_TICKS)
> + p->duty_ticks = c->duty_ticks;
> +
> + if (c->config_mask & PWM_CONFIG_POLARITY) {
> + gp->polarity = c->polarity ? 1 : 0;
> + p->active_high = gp->polarity;
> + }
> +
> + if ((c->config_mask & PWM_CONFIG_START)
> + || (was_on && !(c->config_mask & PWM_CONFIG_STOP)))
> + gpio_pwm_start(p);
> +
> + return 0;
> +}
> +
> +static int
> +gpio_pwm_set_callback(struct pwm_channel *p,
> + pwm_callback_t callback)
> +{
> + struct gpio_pwm *gp = to_gpio_pwm(p);
> + gp->callback = callback;
> + return 0;
> +}
> +
> +static int
> +gpio_pwm_request(struct pwm_channel *p)
> +{
> + p->tick_hz = 1000000000UL;
> + return 0;
> +}
> +
> +static int __devinit
> +gpio_pwm_probe(struct platform_device *pdev)
> +{
> + struct gpio_pwm *gp;
> + struct gpio_pwm_platform_data *gpd = pdev->dev.platform_data;
> + int ret = 0;
> +
> + /* TODO: create configfs entries, so users can assign GPIOs to
> + * PWMs at runtime instead of creating a platform_device
> + * specification and rebuilding their kernel */
(digging up an old argument, I know)
I still think that the PWM api would do well to piggyback on the
namespace and support code of gpiolib. Some pins would support gpio
functions, some would support pwm, some would support both, but it
makes sense to share the overall pin management code. (I'm not
talking about pin muxing; that is a separate problem)
In that scenario, the code in this file becomes library available to
be used by regular gpios to implement pwm behaviour.
> +
> + if (!gpd || gpio_request(gpd->gpio, dev_name(&pdev->dev)))
> + return -EINVAL;
> +
> + gp = kzalloc(sizeof(*gp), GFP_KERNEL);
> + if (!gp) {
> + ret = -ENOMEM;
> + goto err_alloc;
> + }
> +
> + platform_set_drvdata(pdev, gp);
> +
> + gp->pwm.dev = &pdev->dev;
> + gp->pwm.bus_id = dev_name(&pdev->dev);
> + gp->pwm.nchan = 1;
> + gp->gpio = gpd->gpio;
> +
> + INIT_WORK(&gp->work, gpio_pwm_work);
> +
> + hrtimer_init(&gp->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + gp->timer.function = gpio_pwm_timeout;
> +
> + gp->pwm.owner = THIS_MODULE;
> + gp->pwm.config_nosleep = gpio_pwm_config_nosleep;
> + gp->pwm.config = gpio_pwm_config;
> + gp->pwm.request = gpio_pwm_request;
> + gp->pwm.set_callback = gpio_pwm_set_callback;
> +
> + ret = pwm_register(&gp->pwm);
> + if (ret)
> + goto err_pwm_register;
> +
> + return 0;
> +
> +err_pwm_register:
> + platform_set_drvdata(pdev, 0);
> + kfree(gp);
> +err_alloc:
> + return ret;
> +}
> +
> +static int __devexit
> +gpio_pwm_remove(struct platform_device *pdev)
> +{
> + struct gpio_pwm *gp = platform_get_drvdata(pdev);
> + int ret;
> +
> + ret = pwm_unregister(&gp->pwm);
> + hrtimer_cancel(&gp->timer);
> + cancel_work_sync(&gp->work);
> + platform_set_drvdata(pdev, 0);
> + kfree(gp);
> +
> + return 0;
> +}
> +
> +static struct platform_driver gpio_pwm_driver = {
> + .driver = {
> + .name = "gpio_pwm",
> + .owner = THIS_MODULE,
> + },
> + .probe = gpio_pwm_probe,
> + .remove = __devexit_p(gpio_pwm_remove),
> +};
> +
> +static int __init gpio_pwm_init(void)
> +{
> + return platform_driver_register(&gpio_pwm_driver);
> +}
> +module_init(gpio_pwm_init);
> +
> +static void __exit gpio_pwm_exit(void)
> +{
> + platform_driver_unregister(&gpio_pwm_driver);
> +}
> +module_exit(gpio_pwm_exit);
> +
> +MODULE_AUTHOR("Bill Gatliff <bgat@billgatliff.com>");
> +MODULE_DESCRIPTION("PWM output using GPIO and a high-resolution timer");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:gpio_pwm");
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PWM 01/10] API to consolidate PWM devices behind a common user and kernel interface
From: Grant Likely @ 2010-10-16 6:05 UTC (permalink / raw)
To: Bill Gatliff; +Cc: Hector Oron, linux-embedded
In-Reply-To: <AANLkTim2tbWNviJottUg3jAD2ybd5oUPJu3vdPvNa_fX@mail.gmail.com>
On Wed, Oct 6, 2010 at 12:48 PM, Bill Gatliff <bgat@billgatliff.com> wrote:
> Hector:
>
> On Sat, Oct 2, 2010 at 7:25 AM, Hector Oron <hector.oron@gmail.com> wrote:
>> Hello Bill,
>>
>> Thanks for trying (again) to get this in mainline.
>> Are you planning to request a slot in the GIT at kernel.org to hold
>> and maintain this nice framework?
>
> I did make such a request once, and it went nowhere. :(
Git trees don't have to be hosted on git.kernel.org. None of my trees
are on kernel.org, and Linus pulls from me all the time. You can host
your own git server, or use a git hosting provider.
But, if you're serious about getting it mainlined, then you need to
post the whole series to linux-kernel. Also, without a specific
maintainer to pick it up for you, it helps to cc: Andrew Morton. I
recommend doing so sooner rather than later. It probably won't make
2.6.37 since the merge window is opening in about a week, but it would
be a good candidate for .38. I'll go through all the patches right
now and make my comments.
g.
^ permalink raw reply
* Re: Linux Plumbers Embedded microconference
From: Grant Likely @ 2010-10-16 3:50 UTC (permalink / raw)
To: Kevin Hilman
Cc: Mark Brown, Jeremy Kerr, Tim Bird, Scott Wood, linux-embedded,
Liam Girdwood, Loïc Minier, John Linn, Becky Bruce,
Kumar Gala, Paul Walmsley
In-Reply-To: <1287159628.14514.177.camel@localhost>
On Fri, Oct 15, 2010 at 09:20:28AM -0700, Kevin Hilman wrote:
> Hi Grant,
>
> Do you have a draft agenda yet for the microconf? Specifically, I'd
> like to get a better feel for how much time we'll dedicate to each
> topic, and therefore be better prepared for the discussions.
Working on it. RealSoonNow.
> Also, some TI folks are asking me if I know whether the AMP topic is
> going to be discussed. TI is actively working on this and would like to
> participate in any discussions. Their 1st generation code for this is
> in staging (tidspbridge) while the 2nd generation is under active
> deveopment with some of the lead developers planning to be at LPC.
Yes, AMP is absolutely on the agenda. It seems to be a big topic for
a lot of folks.
g.
^ permalink raw reply
* Re: Linux Plumbers Embedded microconference
From: Kevin Hilman @ 2010-10-15 16:20 UTC (permalink / raw)
To: Grant Likely
Cc: Mark Brown, Jeremy Kerr, Tim Bird, Scott Wood, linux-embedded,
Liam Girdwood, Loïc Minier, John Linn, Becky Bruce,
Kumar Gala, Paul Walmsley
In-Reply-To: <AANLkTinqm-EsovG8c1mfX2zuAgHt5OoRCcD+owBbndNc@mail.gmail.com>
On Fri, 2010-09-17 at 17:32 -0600, Grant Likely wrote:
> I'm now getting around to drafting the agenda for the embedded
> microconference. From looking at the proposals and some of the recent
> mailing list discussions, there are some topics that bubble up to the
> surface for me:
>
> 1) Device model usage - Support for runtime PM has been a hot topic,
> but the way the device model is populated and used on embedded
> platforms also has impact on correct initialization ordering, and how
> to instantiate 'system' devices composed of multiple discrete devices
> across the system. ie. connecting a codec and a DAI in ASoC.
>
> 2) Device Tree, HWMOD, static pdata, SFI, and other methods for
> teaching the kernel about the machine.
>
> 3) Common infrastructure beyond the kernel. Android has fastboot and
> other tools. Many folks use u-boot for development, but something
> custom (smaller) for deployment. Are there any other tools/techniques
> from Android/MeeGo/Linaro/WebOS/etc. that would be useful to a wider
> audience. Tim Bird has offered to lead this discussion.
>
> 4) Asymmetric multiprocessing (AMP) intercommunication. Cores are
> cheap, hardware is built with lots of them but how do they
> communicate? This is an issues for DSPs and for embedded
> virtualization. Syslink has been proposed. Freescale PowerPC has
> multicore chips that can be carved up for AMP. Patches have been
> circulated to repurpose virtio for interprocessor communication.
>
> ...
>
> 1 & 2 are somewhat interrelated as they are both aspects of embedded
> requirements on the device model. I'm not sure, but I may end up
> merging these two topics to a degree. My impression is that the same
> problems are being wrestled with in different problem domains. I'd
> like to schedule 3 or 4 people to give a brief (10-15min) overview of
> how they need devices registered, and how it fits in with the driver
> model, followed by discussion. Hopefully it will identify areas where
> common solution can/should be implemented. Or in other words; take
> our own blinders off for a bit and see how other people are solving
> the same problems. :-)
>
> Here is my draft list:
>
> Kevin: How HWMOD is used to describe interconnections between internal
> SoC devices.
> Mark or Liam: ASoC - How what needs to be done to collect disparate
> devices (codecs, audio controllers, etc) into a single SoC device.
> Jeremy: Populating the device model with device tree external data (why and how)
> (I'm also open to other suggestions)
>
> Mark and Liam, I know you haven't made a proposal to do this, but if
> you'd be willing I think it would be valuable.
>
> ...
>
> The AMP IPC topic is interesting to me, but I'd like to get some
> feedback before I commit to it. I would schedule it the same way that
> the device model discussion is organized; 3-4 overviews 10-15 minutes
> each followed by discussion. Right now I've got a proposal from the
> TI folks to talk about Syslink. Freescalers, what say you? Could I
> convince one of you to talk about AMP IPC on your powerpc multicore
> SoCs? Multicore on Xilinx FPGAs would also be interesting, but I'm
> not up to date on if anybody is actively working on that. Are there
> any other AMP IPC mechanisms that should be discussed?
>
> ...
>
> Tim, Jeremy and Kevin; I've accepted your micro-conference topics. As
> the conference gets closer I'll write up a draft of the actual agenda
> and your proposals can be massaged appropriately to reflect exactly
> what issues will be discussed.
Hi Grant,
Do you have a draft agenda yet for the microconf? Specifically, I'd
like to get a better feel for how much time we'll dedicate to each
topic, and therefore be better prepared for the discussions.
Also, some TI folks are asking me if I know whether the AMP topic is
going to be discussed. TI is actively working on this and would like to
participate in any discussions. Their 1st generation code for this is
in staging (tidspbridge) while the 2nd generation is under active
deveopment with some of the lead developers planning to be at LPC.
Kevin
^ permalink raw reply
* Re: [PATCH 15/16] pramfs: test module
From: Christoph Hellwig @ 2010-10-13 11:23 UTC (permalink / raw)
To: Kieran Bingham
Cc: Marco Stornelli, Linux Kernel, Linux Embedded, Linux FS Devel,
Tim Bird
In-Reply-To: <4CB301E4.40405@gmail.com>
On Mon, Oct 11, 2010 at 01:24:04PM +0100, Kieran Bingham wrote:
> >@@ -0,0 +1,49 @@
> >+/*
> >+ * FILE NAME fs/pramfs/namei.c
> FILE NAME != namei.c
Yes, that's why you should never do such filename comments which are not
only utterly prone to be out of data, but also 100% useless.
^ permalink raw reply
* Re: [PATCH 14(16] pramfs: memory protection
From: Andi Kleen @ 2010-10-12 11:56 UTC (permalink / raw)
To: Marco Stornelli
Cc: Andi Kleen, Linux Kernel, Linux Embedded, Linux FS Devel,
Tim Bird, linux-mm
In-Reply-To: <AANLkTinpoL+AMU62PMvXs78Y6v0efDm3eq++NiVk8XUB@mail.gmail.com>
> per-arch?! Wow. Mmm...maybe I have to change something at fs level to
> avoid that. An alternative could be to use the follow_pte solution but
> avoid the protection via Kconfig if the fs is used on some archs (ia64
> or MIPS), with large pages and so on. An help of the kernel community
> to know all these particular cases is welcome.
It depends if the protection is a fundamental part of your design
(but if it is I would argue that's broken because it's really not very good
protection): If it's just an optional nice to have you can stub
it out on architectures that don't support it.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 14(16] pramfs: memory protection
From: Marco Stornelli @ 2010-10-12 10:47 UTC (permalink / raw)
To: Andi Kleen
Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird, linux-mm
In-Reply-To: <20101012074522.GA20436@basil.fritz.box>
2010/10/12 Andi Kleen <andi@firstfloor.org>:
> On Mon, Oct 11, 2010 at 07:32:10PM +0200, Marco Stornelli wrote:
>> Il 10/10/2010 18:46, Andi Kleen ha scritto:
>> > This won't work at all on x86 because you don't handle large
>> > pages.
>> >
>> > And it doesn't work on x86-64 because the first 2GB are double
>> > mapped (direct and kernel text mapping)
>> >
>> > Thirdly I expect it won't either on architectures that map
>> > the direct mapping with special registers (like IA64 or MIPS)
>>
>> Andi, what do you think to use the already implemented follow_pte
>> instead?
>
> Has all the same problems. Really you need an per architecture
> function. Perhaps some architectures could use a common helper,
> but certainly not all.
>
per-arch?! Wow. Mmm...maybe I have to change something at fs level to
avoid that. An alternative could be to use the follow_pte solution but
avoid the protection via Kconfig if the fs is used on some archs (ia64
or MIPS), with large pages and so on. An help of the kernel community
to know all these particular cases is welcome.
Regards,
Marco
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 14(16] pramfs: memory protection
From: Andi Kleen @ 2010-10-12 7:45 UTC (permalink / raw)
To: Marco Stornelli
Cc: Andi Kleen, Linux Kernel, Linux Embedded, Linux FS Devel,
Tim Bird, linux-mm
In-Reply-To: <4CB34A1A.3030003@gmail.com>
On Mon, Oct 11, 2010 at 07:32:10PM +0200, Marco Stornelli wrote:
> Il 10/10/2010 18:46, Andi Kleen ha scritto:
> > This won't work at all on x86 because you don't handle large
> > pages.
> >
> > And it doesn't work on x86-64 because the first 2GB are double
> > mapped (direct and kernel text mapping)
> >
> > Thirdly I expect it won't either on architectures that map
> > the direct mapping with special registers (like IA64 or MIPS)
>
> Andi, what do you think to use the already implemented follow_pte
> instead?
Has all the same problems. Really you need an per architecture
function. Perhaps some architectures could use a common helper,
but certainly not all.
x86 already has some infrastructure for this, but it currently
has serious problems too (like not merging mappings on unmap)
and is generally overdesigned ugly code.
-Andi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 14(16] pramfs: memory protection
From: Marco Stornelli @ 2010-10-11 17:32 UTC (permalink / raw)
To: Andi Kleen
Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird, linux-mm
In-Reply-To: <87aamm3si1.fsf@basil.nowhere.org>
Il 10/10/2010 18:46, Andi Kleen ha scritto:
> This won't work at all on x86 because you don't handle large
> pages.
>
> And it doesn't work on x86-64 because the first 2GB are double
> mapped (direct and kernel text mapping)
>
> Thirdly I expect it won't either on architectures that map
> the direct mapping with special registers (like IA64 or MIPS)
Andi, what do you think to use the already implemented follow_pte
instead?
int writeable_kernel_pte_range(unsigned long address, unsigned long size,
unsigned int rw)
{
unsigned long addr = address & PAGE_MASK;
unsigned long end = address + size;
unsigned long start = addr;
int ret = -EINVAL;
pte_t *ptep, pte;
spinlock_t *lock = &init_mm.page_table_lock;
do {
ret = follow_pte(&init_mm, addr, &ptep, &lock);
if (ret)
goto out;
pte = *ptep;
if (pte_present(pte)) {
pte = rw ? pte_mkwrite(pte) : pte_wrprotect(pte);
*ptep = pte;
}
pte_unmap_unlock(ptep, lock);
addr += PAGE_SIZE;
} while (addr && (addr < end));
ret = 0;
out:
flush_tlb_kernel_range(start, end);
return ret;
}
>
> I'm not sure this is very useful anyways. It doesn't protect
> against stray DMA and it doesn't protect against writes through
> broken user PTEs.
>
> -Andi
>
It's a way to have more protection against kernel bug, for a
in-memory fs can be important. However this option can be
enabled/disabled at fs level.
Marco
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 11/16] pramfs: ACL management
From: Marco Stornelli @ 2010-10-11 16:50 UTC (permalink / raw)
To: Kieran Bingham; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <4CB30266.1010706@gmail.com>
Il 11/10/2010 14:26, Kieran Bingham ha scritto:
> On 10/10/2010 17:33, Marco Stornelli wrote:
>> From: Marco Stornelli<marco.stornelli@gmail.com>
>>
>> ACL operations.
>>
>> Signed-off-by: Marco Stornelli<marco.stornelli@gmail.com>
>> ---
>> diff -Nurp linux-2.6.36-orig/fs/pramfs/acl.c linux-2.6.36/fs/pramfs/acl.c
>> --- linux-2.6.36-orig/fs/pramfs/acl.c 1970-01-01 01:00:00.000000000
>> +0100
>> +++ linux-2.6.36/fs/pramfs/acl.c 2010-09-14 18:49:52.000000000 +0200
>> @@ -0,0 +1,418 @@
>> +/*
>> + * FILE NAME fs/pramfs/acl.h
> Found another one :)
> FILE NAME != acl.h ...
> --
> KB
>
Oops, damn copy&paste ;) I'll fix it asap.
Thanks.
Marco
^ permalink raw reply
* Re: [PATCH 11/16] pramfs: ACL management
From: Kieran Bingham @ 2010-10-11 12:26 UTC (permalink / raw)
To: Marco Stornelli; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <4CB1EAD8.2030206@gmail.com>
On 10/10/2010 17:33, Marco Stornelli wrote:
> From: Marco Stornelli<marco.stornelli@gmail.com>
>
> ACL operations.
>
> Signed-off-by: Marco Stornelli<marco.stornelli@gmail.com>
> ---
> diff -Nurp linux-2.6.36-orig/fs/pramfs/acl.c linux-2.6.36/fs/pramfs/acl.c
> --- linux-2.6.36-orig/fs/pramfs/acl.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.36/fs/pramfs/acl.c 2010-09-14 18:49:52.000000000 +0200
> @@ -0,0 +1,418 @@
> +/*
> + * FILE NAME fs/pramfs/acl.h
Found another one :)
FILE NAME != acl.h ...
--
KB
^ permalink raw reply
* Re: [PATCH 15/16] pramfs: test module
From: Kieran Bingham @ 2010-10-11 12:24 UTC (permalink / raw)
To: Marco Stornelli; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <4CB1EBDD.8050509@gmail.com>
On 10/10/2010 17:37, Marco Stornelli wrote:
> From: Marco Stornelli<marco.stornelli@gmail.com>
>
> Test module.
>
> Signed-off-by: Marco Stornelli<marco.stornelli@gmail.com>
> ---
> diff -Nurp linux-2.6.36-orig/fs/pramfs/pramfs_test.c linux-2.6.36/fs/pramfs/pramfs_test.c
> --- linux-2.6.36-orig/fs/pramfs/pramfs_test.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.36/fs/pramfs/pramfs_test.c 2010-09-14 18:49:52.000000000 +0200
> @@ -0,0 +1,49 @@
> +/*
> + * FILE NAME fs/pramfs/namei.c
FILE NAME != namei.c
^ permalink raw reply
* Re: [PATCH 16/16] pramfs Makefile and Kconfig
From: Marco Stornelli @ 2010-10-11 7:09 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <20101010100211.3c747512.rdunlap@xenotime.net>
2010/10/10 Randy Dunlap <rdunlap@xenotime.net>:
> On Sun, 10 Oct 2010 18:39:11 +0200 Marco Stornelli wrote:
>
>> From: Marco Stornelli <marco.stornelli@gmail.com>
>>
>> Makefile and Kconfig.
>>
>> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
>> ---
>> diff -Nurp linux-2.6.36-orig/fs/pramfs/Kconfig linux-2.6.36/fs/pramfs/Kconfig
>> --- linux-2.6.36-orig/fs/pramfs/Kconfig 1970-01-01 01:00:00.000000000 +0100
>> +++ linux-2.6.36/fs/pramfs/Kconfig 2010-09-14 18:49:52.000000000 +0200
>> @@ -0,0 +1,72 @@
>> +config PRAMFS
>> + tristate "Persistent and Protected RAM file system support"
>> + depends on HAS_IOMEM && EXPERIMENTAL
>> + select CRC16
>> + help
>> + If your system has a block of fast (comparable in access speed to
>> + system memory) and non-volatile RAM and you wish to mount a
>> + light-weight, full-featured, and space-efficient filesystem over it,
>> + say Y here, and read <file:Documentation/filesystems/pramfs.txt>.
>> +
>> + To compile this as a module, choose M here: the module will be
>> + called pramfs.ko.
>
> called pramfs.
>
> (we don't add the .ko suffix; well, we try not to do that)
Ok.
>
>> +
>> +config PRAMFS_XIP
>> + bool "Enable Execute-in-place in PRAMFS"
>> + depends on PRAMFS && !PRAMFS_WRITE_PROTECT
>> + help
>> + Say Y here to enable xip feature of PRAMFS.
>
> XIP
Ok.
>
>> +
>> +config PRAMFS_WRITE_PROTECT
>> + bool "Enable PRAMFS write protection"
>> + depends on PRAMFS && MMU
>> + default y
>> + help
>> + Say Y here to enable the write protect feature of PRAMFS.
>> +
>> +config PRAMFS_XATTR
>> + bool "PRAMFS extended attributes"
>> + depends on PRAMFS
>> + help
>> + Extended attributes are name:value pairs associated with inodes by
>> + the kernel or by users (see the attr(5) manual page, or visit
>> + <http://acl.bestbits.at/> for details).
>> +
>> + If unsure, say N.
>> +
>> +config PRAMFS_POSIX_ACL
>> + bool "PRAMFS POSIX Access Control Lists"
>> + depends on PRAMFS_XATTR
>> + select FS_POSIX_ACL
>> + help
>> + Posix Access Control Lists (ACLs) support permissions for users and
>> + groups beyond the owner/group/world scheme.
>> +
>> + To learn more about Access Control Lists, visit the Posix ACLs for
>> + Linux website <http://acl.bestbits.at/>.
>> +
>> + If you don't know what Access Control Lists are, say N
>
> end sentence with period ('.').
Ok.
^ permalink raw reply
* Re: [PATCH 15/16] pramfs: test module
From: Marco Stornelli @ 2010-10-11 7:08 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <20101010095703.aeae2f15.rdunlap@xenotime.net>
2010/10/10 Randy Dunlap <rdunlap@xenotime.net>:
> On Sun, 10 Oct 2010 18:37:49 +0200 Marco Stornelli wrote:
>
> Above 2 lines need to indented more.
>
Ack.
>> + return 1;
>> + }
>> +
>> + /*
>> + * Attempt an unprotected clear of checksum information in the
>> + * superblock, this should cause a kernel page protection fault.
>> + */
>> + printk("%s: writing to kernel VA %p\n", __func__, psb);
>> + psb->s_sum = 0;
>> +
>> + return 0;
>> +}
>> +
>> +void test_pramfs_write_cleanup(void) {}
>> +
>> +/* Module information */
>> +MODULE_LICENSE("GPL");
>> +module_init(test_pramfs_write);
>> +module_exit(test_pramfs_write_cleanup);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 14(16] pramfs: memory protection
From: Marco Stornelli @ 2010-10-11 6:57 UTC (permalink / raw)
To: Andi Kleen
Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird, linux-mm
In-Reply-To: <87aamm3si1.fsf@basil.nowhere.org>
2010/10/10 Andi Kleen <andi@firstfloor.org>:
> Marco Stornelli <marco.stornelli@gmail.com> writes:
>> +
>> + do {
>> + pgd = pgd_offset(&init_mm, address);
>> + if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
>> + goto out;
>> +
>> + pud = pud_offset(pgd, address);
>> + if (pud_none(*pud) || unlikely(pud_bad(*pud)))
>> + goto out;
>> +
>> + pmd = pmd_offset(pud, address);
>> + if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
>> + goto out;
>> +
>> + ptep = pte_offset_kernel(pmd, addr);
>> + pte = *ptep;
>> + if (pte_present(pte)) {
>
> This won't work at all on x86 because you don't handle large
> pages.
On x86 works because I tested. Maybe there's a particular
configuration with large pages. Sincerly I'm only an "user", so if
you/Linus or others want to change it or rewrite it, for me it's ok.
The pte manipulation are a bit out of scope for a fs, so I let the
things to the mm experts.
^ permalink raw reply
* Re: logfs segfaults at umount time
From: Michael Opdenacker @ 2010-10-10 19:06 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: joern, logfs, linux-embedded mailing list, Tim Bird
In-Reply-To: <20101010171633.5E90C14F310@gemini.denx.de>
Hi Wolfgang,
On 10/10/2010 07:16 PM, Wolfgang Denk wrote:
> Dear Michael,
>
> In message <4CB1E5B3.1090300@free-electrons.com> you wrote:
>> root@calao:~# mount -t logfs /dev/mtdblock0 /mnt/flash/
>> root@calao:~# cp -rf /fs/8m/* /mnt/flash/
>> eth0: TX underrun, resetting buffers
>> eth0: TX underrun, resetting buffers
>>
> Seems you have some problems here already?
>
I don't think they are related. My rootfs in on NFS, and these happen
all the time on these at91 boards, causing no problem with the other
filesystems.
Cheers,
Michael.
--
Michael Opdenacker, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
+ 33 621 604 642
^ permalink raw reply
* Re: logfs segfaults at umount time
From: Wolfgang Denk @ 2010-10-10 17:16 UTC (permalink / raw)
To: Michael Opdenacker; +Cc: joern, logfs, linux-embedded mailing list, Tim Bird
In-Reply-To: <4CB1E5B3.1090300@free-electrons.com>
Dear Michael,
In message <4CB1E5B3.1090300@free-electrons.com> you wrote:
>
> I'm running some flash filesystem benchmarks for CELF, and I'm facing
> kernel segfaults when I try to umount my logfs filesystem.
...
> root@calao:~# mount -t logfs /dev/mtdblock0 /mnt/flash/
> root@calao:~# cp -rf /fs/8m/* /mnt/flash/
> eth0: TX underrun, resetting buffers
> eth0: TX underrun, resetting buffers
Seems you have some problems here already?
> root@calao:~# umount /mnt/flash/
> kernel BUG at fs/logfs/segment.c:858!
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> pgd = c14a4000
> [00000000] *pgd=23a3c031, *pte=00000000, *ppte=00000000
> Internal error: Oops: 817 [#1]
...
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
We have phasers, I vote we blast 'em!
-- Bailey, "The Corbomite Maneuver", stardate 1514.2
^ permalink raw reply
* Re: [PATCH 16/16] pramfs Makefile and Kconfig
From: Randy Dunlap @ 2010-10-10 17:02 UTC (permalink / raw)
To: Marco Stornelli; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <4CB1EC2F.7070807@gmail.com>
On Sun, 10 Oct 2010 18:39:11 +0200 Marco Stornelli wrote:
> From: Marco Stornelli <marco.stornelli@gmail.com>
>
> Makefile and Kconfig.
>
> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
> ---
> diff -Nurp linux-2.6.36-orig/fs/pramfs/Kconfig linux-2.6.36/fs/pramfs/Kconfig
> --- linux-2.6.36-orig/fs/pramfs/Kconfig 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.36/fs/pramfs/Kconfig 2010-09-14 18:49:52.000000000 +0200
> @@ -0,0 +1,72 @@
> +config PRAMFS
> + tristate "Persistent and Protected RAM file system support"
> + depends on HAS_IOMEM && EXPERIMENTAL
> + select CRC16
> + help
> + If your system has a block of fast (comparable in access speed to
> + system memory) and non-volatile RAM and you wish to mount a
> + light-weight, full-featured, and space-efficient filesystem over it,
> + say Y here, and read <file:Documentation/filesystems/pramfs.txt>.
> +
> + To compile this as a module, choose M here: the module will be
> + called pramfs.ko.
called pramfs.
(we don't add the .ko suffix; well, we try not to do that)
> +
> +config PRAMFS_XIP
> + bool "Enable Execute-in-place in PRAMFS"
> + depends on PRAMFS && !PRAMFS_WRITE_PROTECT
> + help
> + Say Y here to enable xip feature of PRAMFS.
XIP
> +
> +config PRAMFS_WRITE_PROTECT
> + bool "Enable PRAMFS write protection"
> + depends on PRAMFS && MMU
> + default y
> + help
> + Say Y here to enable the write protect feature of PRAMFS.
> +
> +config PRAMFS_XATTR
> + bool "PRAMFS extended attributes"
> + depends on PRAMFS
> + help
> + Extended attributes are name:value pairs associated with inodes by
> + the kernel or by users (see the attr(5) manual page, or visit
> + <http://acl.bestbits.at/> for details).
> +
> + If unsure, say N.
> +
> +config PRAMFS_POSIX_ACL
> + bool "PRAMFS POSIX Access Control Lists"
> + depends on PRAMFS_XATTR
> + select FS_POSIX_ACL
> + help
> + Posix Access Control Lists (ACLs) support permissions for users and
> + groups beyond the owner/group/world scheme.
> +
> + To learn more about Access Control Lists, visit the Posix ACLs for
> + Linux website <http://acl.bestbits.at/>.
> +
> + If you don't know what Access Control Lists are, say N
end sentence with period ('.').
> +
> +config PRAMFS_SECURITY
> + bool "PRAMFS Security Labels"
> + depends on PRAMFS_XATTR
> + help
> + Security labels support alternative access control models
> + implemented by security modules like SELinux. This option
> + enables an extended attribute handler for file security
> + labels in the pram filesystem.
> +
> + If you are not using a security module that requires using
> + extended attributes for file security labels, say N.
> +
> +config PRAMFS_TEST
> + boolean
> + depends on PRAMFS
> +
> +config TEST_MODULE
> + tristate "PRAMFS Test"
> + depends on PRAMFS && m
> + select PRAMFS_TEST
> + help
> + Say Y here to build a simple module to test the protection of
> + PRAMFS. The module will be called pramfs_test.ko.
called pramfs_test.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: [PATCH 15/16] pramfs: test module
From: Randy Dunlap @ 2010-10-10 16:57 UTC (permalink / raw)
To: Marco Stornelli; +Cc: Linux Kernel, Linux Embedded, Linux FS Devel, Tim Bird
In-Reply-To: <4CB1EBDD.8050509@gmail.com>
On Sun, 10 Oct 2010 18:37:49 +0200 Marco Stornelli wrote:
> From: Marco Stornelli <marco.stornelli@gmail.com>
>
> Test module.
>
> Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
> ---
> diff -Nurp linux-2.6.36-orig/fs/pramfs/pramfs_test.c linux-2.6.36/fs/pramfs/pramfs_test.c
> --- linux-2.6.36-orig/fs/pramfs/pramfs_test.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.36/fs/pramfs/pramfs_test.c 2010-09-14 18:49:52.000000000 +0200
> @@ -0,0 +1,49 @@
> +/*
> + * FILE NAME fs/pramfs/namei.c
> + *
> + * BRIEF DESCRIPTION
> + *
> + * Pramfs test module.
> + *
> + * Copyright 2009-2010 Marco Stornelli <marco.stornelli@gmail.com>
> + * Copyright 2003 Sony Corporation
> + * Copyright 2003 Matsushita Electric Industrial Co., Ltd.
> + * 2003-2004 (c) MontaVista Software, Inc. , Steve Longerbeam
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +#include <linux/module.h>
> +#include <linux/version.h>
> +#include <linux/init.h>
> +#include <linux/fs.h>
> +#include "pram.h"
> +
> +int __init test_pramfs_write(void)
> +{
> + struct pram_super_block *psb;
> +
> + psb = get_pram_super();
> + if (!psb) {
> + printk(KERN_ERR
> + "%s: PRAMFS super block not found (not mounted?)\n",
> + __func__);
Above 2 lines need to indented more.
> + return 1;
> + }
> +
> + /*
> + * Attempt an unprotected clear of checksum information in the
> + * superblock, this should cause a kernel page protection fault.
> + */
> + printk("%s: writing to kernel VA %p\n", __func__, psb);
> + psb->s_sum = 0;
> +
> + return 0;
> +}
> +
> +void test_pramfs_write_cleanup(void) {}
> +
> +/* Module information */
> +MODULE_LICENSE("GPL");
> +module_init(test_pramfs_write);
> +module_exit(test_pramfs_write_cleanup);
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox