[v2] implement a generic PWM framework

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

changes since last version:

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

The following changes since commit b0af8dfdd67699e25083478c63eedef2e72ebd85:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

	Arnd

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

Hi Sascha,

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

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

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

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

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