| Message ID | 1385566771-366-1-git-send-email-denis@eukrea.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. ... > Signed-off-by: Denis Carikli <denis@eukrea.com> > --- > .../devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt | 19 ++ > drivers/mfd/mc13xxx-core.c | 11 ++ > drivers/pwm/Kconfig | 6 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-mc13xxx.c | 205 ++++++++++++++++++++ > include/linux/mfd/mc13783.h | 2 + > 6 files changed, 244 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt > create mode 100644 drivers/pwm/pwm-mc13xxx.c > > diff --git a/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt b/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt > new file mode 100644 > index 0000000..a1394d0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt > @@ -0,0 +1,19 @@ > +Freescale mc13xxx series PWM drivers. > + > +Supported PWMs: > +mc13892 series > +mc34708 series MC13892 does not have a dedicated PWM. You can probably use the existing driver mc13xxx-led driver. ---
Hi > +struct mc13xxx *get_mc13xxx(void) > +{ > + return mc13xxx_data; > +} > +EXPORT_SYMBOL_GPL(get_mc13xxx); > + > int mc13xxx_common_init(struct mc13xxx *mc13xxx, > struct mc13xxx_platform_data *pdata, int irq) > { > @@ -706,6 +714,9 @@ err_revision: > mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton"); > } > > + /* Linux will not have to handle more than one mc13xxx pmic. */ > + mc13xxx_data = mc13xxx; > + Why using a such hack instead of an MFD subdevice ? Regards, Philippe
On Wed, Nov 27, 2013 at 04:58:33PM +0100, Philippe Rétornaz wrote: > Hi > > >+struct mc13xxx *get_mc13xxx(void) > >+{ > >+ return mc13xxx_data; > >+} > >+EXPORT_SYMBOL_GPL(get_mc13xxx); > >+ > > int mc13xxx_common_init(struct mc13xxx *mc13xxx, > > struct mc13xxx_platform_data *pdata, int irq) > > { > >@@ -706,6 +714,9 @@ err_revision: > > mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton"); > > } > > > >+ /* Linux will not have to handle more than one mc13xxx pmic. */ > >+ mc13xxx_data = mc13xxx; > >+ > > Why using a such hack instead of an MFD subdevice ? I agree, let's not do this please. Thierry
On Wed, Nov 27, 2013 at 04:39:29PM +0100, Denis Carikli wrote: > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: devicetree@vger.kernel.org > Cc: linux-pwm@vger.kernel.org > Cc: Samuel Ortiz <sameo@linux.intel.com> > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Denis Carikli <denis@eukrea.com> > --- > .../devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt | 19 ++ > drivers/mfd/mc13xxx-core.c | 11 ++ > drivers/pwm/Kconfig | 6 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-mc13xxx.c | 205 ++++++++++++++++++++ > include/linux/mfd/mc13783.h | 2 + > 6 files changed, 244 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt > create mode 100644 drivers/pwm/pwm-mc13xxx.c I'll say the same thing I always say: please use "PWM" in prose instead of "pwm". It's an abbreviation, so it should be uppercased. > diff --git a/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt b/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt > new file mode 100644 > index 0000000..a1394d0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt > @@ -0,0 +1,19 @@ > +Freescale mc13xxx series PWM drivers. The binding doesn't describe drivers, so this should be something like: Freescale mc13xxx series PWM controllers > +Supported PWMs: Similarily this should be: "Supported PWM controllers:" > +mc13892 series > +mc34708 series And since this is a list it would make sense to prefix each item with a dash. > + > +Required properties: > +- compatible: "fsl,mc13892-pwm" or "fsl,mc34708-pwm" > +- mfd: phandle to mc13xxx mfd node. What's that doing here? If this is a function of the mc13xxx device, then it should be a child node of the mc13xxx node. > diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c > index dbbf8ee..e250f16 100644 > --- a/drivers/mfd/mc13xxx-core.c > +++ b/drivers/mfd/mc13xxx-core.c > @@ -133,6 +133,8 @@ > > #define MC13XXX_ADC2 45 > > +struct mc13xxx *mc13xxx_data; > + > void mc13xxx_lock(struct mc13xxx *mc13xxx) > { > if (!mutex_trylock(&mc13xxx->lock)) { > @@ -639,6 +641,12 @@ static inline int mc13xxx_probe_flags_dt(struct mc13xxx *mc13xxx) > } > #endif > > +struct mc13xxx *get_mc13xxx(void) > +{ > + return mc13xxx_data; > +} > +EXPORT_SYMBOL_GPL(get_mc13xxx); As mentioned elsewhere, this is horrible. > int mc13xxx_common_init(struct mc13xxx *mc13xxx, > struct mc13xxx_platform_data *pdata, int irq) > { > @@ -706,6 +714,9 @@ err_revision: > mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton"); > } > > + /* Linux will not have to handle more than one mc13xxx pmic. */ > + mc13xxx_data = mc13xxx; Who says that Linux will never have to handle more than one? Even if you could guarantee that, it still sets a bad example. > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index eece329..a959ecd 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -100,6 +100,12 @@ config PWM_LPC32XX > To compile this driver as a module, choose M here: the module > will be called pwm-lpc32xx. > > +config PWM_MC13XXX > + tristate "MC13XXX PWM support" > + depends on MFD_MC13XXX > + help > + Generic PWM framework driver for Freescale MC13XXX pmic. PMIC is an abbreviation too, so should be all uppercase. > diff --git a/drivers/pwm/pwm-mc13xxx.c b/drivers/pwm/pwm-mc13xxx.c [...] > +#include <linux/err.h> > +#include <linux/mfd/mc13783.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/of_platform.h> > +#include <linux/pwm.h> > +#include <linux/slab.h> Can this please be sorted alphabetically? > + > +#define MHz(x) (1000*1000*x) Please drop this. You use it exactly once, ... > +#define MC13XXX_REG_PWM_CTL 55 > +#define MC13XXX_BASE_CLK_FREQ (MHz(2) / 32) So you can just as well write (2000000 / 32) here. > +#define MC13XXX_PWM1CLKDIV_SHIFT 6 > +#define MC13XXX_PWM2DUTY_SHIFT 12 > +#define MC13XXX_PWMDUTYDIVISOR 32 > +#define MC13XXX_PWMCLKDIVISOR 64 This list is totally confusing. I can't tell which of these is a register and which isn't. I would at least expect registers to have hexadecimal offsets. Better yet, a comment should explain what the registers look like. > +#define MC13XXX_PWM_REG_SIZE 0x3f This doesn't seem to be a size at all. It's used as a mask, so why not call it MC13XXX_PWM_REG_MASK? > +static int pwm_mc13xxx_config(struct pwm_chip *chip, struct pwm_device *pwm, > + int duty_ns, int period_ns) > +{ > + struct mc13xxx_pwm_chip *mc13xxx_chip = to_mc13xxx_chip(chip); > + struct mc13xxx *mcdev = mc13xxx_chip->mcdev; > + struct device *dev = mc13xxx_chip->pwm_chip.dev; > + u32 offset; > + u32 mask = MC13XXX_PWM_REG_SIZE; > + u32 val; > + int ret; > + int duty_cycle; > + int min_duty_cycle; > + int period_cycle; > + unsigned long period = period_ns; Many of these can go on a single line. Also it looks like you can reuse some of them and thereby reduce the number of local variables. Having too many of them makes it very difficult to follow what the code is doing. > + > + dev_dbg(dev, "requested duty_ns=%d and period_ns=%d\n", > + duty_ns, period_ns); This can go away. > + > + /* period */ > + if (period < MC13XXX_MIN_PERIOD_NS) { > + dev_warn(dev, "period was under the range.\n"); > + period = MC13XXX_MIN_PERIOD_NS; > + } > + > + if (period > MC13XXX_MAX_PERIOD_NS) { > + dev_warn(dev, "period was over the range.\n"); > + period = MC13XXX_MAX_PERIOD_NS; > + } Shouldn't both of these be an error instead? > + > + period_cycle = DIV_ROUND_UP(NSEC_PER_SEC, period); > + period_cycle = DIV_ROUND_UP(MC13XXX_BASE_CLK_FREQ, period_cycle); > + period_cycle--; You can save one line here by turning this one into a - 1 appended to the previous line. > + > + offset = MC13XXX_REG_PWM_CTL + pwm->hwpwm * MC13XXX_PWM2DUTY_SHIFT + > + MC13XXX_PWM1CLKDIV_SHIFT; _SHIFT is usually for bitfields, not for register offsets. Perhaps it would also be a good idea to move this computation into a macro or static inline function, something like: #define MC13XXX_PWM_BASE(pwm) (0x37 + ((pwm) * 0xc)) And then use offsets to the individual registers: #define MC13XXX_PWM_CTL 0x00 #define MC13XXX_PWM_CLKDIV 0x06 Then it becomes much easier to use these: unsigned int base = MC13XXX_PWM_BASE(pwm->hwpwm); ret = mc13xxx_reg_read(mcdev, base + MC13XXX_PWM_CTL, &val); > + > + mc13xxx_lock(mcdev); > + ret = mc13xxx_reg_read(mcdev, offset, &val); > + val &= ~mask; Why aren't you using the #define here directly? > + val += (period_cycle & mask); > + ret = mc13xxx_reg_write(mcdev, offset, val); > + mc13xxx_unlock(mcdev); > + > + /* duty cycle */ > + min_duty_cycle = DIV_ROUND_UP(period, 32); > + > + if (duty_ns < min_duty_cycle) { > + dev_warn(dev, "duty cycle is under the range.\n"); > + duty_cycle = 0; Again, I think this should be an error and the function should fail when this happens. > + } else if (duty_ns > period) { The core code already checks for this. > + dev_warn(dev, "duty cycle is over the range.\n"); > + duty_cycle = 32; > + } else { > + duty_cycle = 32 * period; > + duty_cycle = DIV_ROUND_UP(period, duty_ns); Are you sure this is doing what it's supposed to? The second line overwrites the first one. Also Do you have a link to the manual for this controller? I'm very confused by all these restrictions and the computations. > + duty_cycle--; > + duty_cycle = 32 - duty_cycle; > + } > + > + offset = MC13XXX_REG_PWM_CTL + pwm->hwpwm * MC13XXX_PWM2DUTY_SHIFT; > + > + mc13xxx_lock(mcdev); > + ret = mc13xxx_reg_read(mcdev, offset, &val); > + val &= ~mask; > + val += (duty_cycle & mask); That's odd. The mask here is 0x3f (63), but the maximum value for the duty-cycle is 32 by the above computations, so masking is completely unnecessary. Also, you don't need any parentheses around (duty_cycle & mask). And since you're modifying a bitfield, the idiomatic way to write it is with a | instead of a +: val &= ~mask; val |= duty_cycle & mask; > + ret = mc13xxx_reg_write(mcdev, offset, val); > + mc13xxx_unlock(mcdev); Shouldn't the lock protect both register writes as a whole? Otherwise there's still the potential that two such accesses will be interleaved and cause undefined behaviour. > + return 0; This returns success no matter what mc13xxx_reg_write() returned. > +} > + > +static int pwm_mc13xxx_enable(struct pwm_chip *chip, struct pwm_device *pwm) > +{ > + /* The hardware doesn't need an enable */ > + > + return 0; > +} That's not really compatible with the semantics of the PWM subsystem. It must be possible to configure the PWM without enabling it. If the hardware doesn't have a separate bit to do that, then you'll need to put most of the .config() code into .enable(). You could for example compute the register values in advance, in .config(), but only write the registers in .enable(). > +static void pwm_mc13xxx_disable(struct pwm_chip *chip, struct pwm_device *pwm) > +{ > + /* The hardware doesn't need a disable */ > +} That's unfortunate. So the only way to "disable" the PWM is by setting the duty-cycle to 0? In that case it might be good to do that here. > + > +static const struct pwm_ops pwm_mc13xxx_ops = { > + .enable = pwm_mc13xxx_enable, > + .disable = pwm_mc13xxx_disable, > + .config = pwm_mc13xxx_config, > + .owner = THIS_MODULE, > +}; No tabs for aligning please, just use a single space on either side of the assignment operator. > +static int pwm_mc13xxx_probe(struct platform_device *pdev) > +{ > + struct mc13xxx_pwm_chip *chip; > + struct mc13xxx *mcdev; > + int err; > + > + mcdev = get_mc13xxx(); I've mentioned this before, this needs to go away. The usual way to do that is to make the PWM device a child of the MFD device and refer to the MFD device via pdev->dev.parent. > + if (!mcdev) { > + dev_err(&pdev->dev, "failed to find mc13xxx pmic.\n"); > + return -EINVAL; > + } > + > + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL); > + if (chip == NULL) { > + dev_err(&pdev->dev, "failed to allocate memory\n"); > + return -ENOMEM; > + } Don't use error messages for allocation failures. And while at it, can you change "chip == NULL" to "!chip", please? > + > + chip->pwm_chip.dev = &pdev->dev; > + chip->pwm_chip.ops = &pwm_mc13xxx_ops; > + chip->pwm_chip.base = pdev->id; No. This should always be -1 for new drivers. > +static int pwm_mc13xxx_remove(struct platform_device *pdev) > +{ > + struct mc13xxx_pwm_chip *chip = platform_get_drvdata(pdev); > + int ret; > + > + ret = pwmchip_remove(&chip->pwm_chip); > + if (ret < 0) > + return ret; > + > + return 0; This can be just: return pwmchip_remove(&chip->pwm_chip); By the way, that pwm_ prefix on the PWM chip field is somewhat redundant since the driver implements only one chip. I suggest you drop it. > +#ifdef CONFIG_OF > +static const struct of_device_id pwm_mc13xxx_of_match[] = { > + { .compatible = "fsl,mc13892-pwm" }, > + { .compatible = "fsl,mc34708-pwm" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, pwm_mc13xxx_of_match); > +#endif > + > +static struct platform_driver pwm_mc13xxx_driver = { > + .driver = { > + .name = "mc13xxx-pwm", > + .of_match_table = of_match_ptr(pwm_mc13xxx_of_match), > + .owner = THIS_MODULE, .owner is assigned automatically by the core, so this can be dropped. > + }, > + .probe = pwm_mc13xxx_probe, > + .remove = pwm_mc13xxx_remove, Please don't use tabs for alignment here. A single space on each side of the = will do just fine. For the whole structure, not just these two fields. > +}; > + > +module_platform_driver(pwm_mc13xxx_driver); No blank line between the above two lines. Thierry
diff --git a/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt b/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt new file mode 100644 index 0000000..a1394d0 --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt @@ -0,0 +1,19 @@ +Freescale mc13xxx series PWM drivers. + +Supported PWMs: +mc13892 series +mc34708 series + +Required properties: +- compatible: "fsl,mc13892-pwm" or "fsl,mc34708-pwm" +- mfd: phandle to mc13xxx mfd node. +- #pwm-cells: should be 2. See pwm.txt in this directory for a description of + the cells format. + +Example: + +mc13xxx_pwm: pwm { + compatible = "fsl,mc34708-pwm"; + mfd = <&pmic>; + #pwm-cells = <2>; +}; diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c index dbbf8ee..e250f16 100644 --- a/drivers/mfd/mc13xxx-core.c +++ b/drivers/mfd/mc13xxx-core.c @@ -133,6 +133,8 @@ #define MC13XXX_ADC2 45 +struct mc13xxx *mc13xxx_data; + void mc13xxx_lock(struct mc13xxx *mc13xxx) { if (!mutex_trylock(&mc13xxx->lock)) { @@ -639,6 +641,12 @@ static inline int mc13xxx_probe_flags_dt(struct mc13xxx *mc13xxx) } #endif +struct mc13xxx *get_mc13xxx(void) +{ + return mc13xxx_data; +} +EXPORT_SYMBOL_GPL(get_mc13xxx); + int mc13xxx_common_init(struct mc13xxx *mc13xxx, struct mc13xxx_platform_data *pdata, int irq) { @@ -706,6 +714,9 @@ err_revision: mc13xxx_add_subdevice(mc13xxx, "%s-pwrbutton"); } + /* Linux will not have to handle more than one mc13xxx pmic. */ + mc13xxx_data = mc13xxx; + return 0; } EXPORT_SYMBOL_GPL(mc13xxx_common_init); diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index eece329..a959ecd 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -100,6 +100,12 @@ config PWM_LPC32XX To compile this driver as a module, choose M here: the module will be called pwm-lpc32xx. +config PWM_MC13XXX + tristate "MC13XXX PWM support" + depends on MFD_MC13XXX + help + Generic PWM framework driver for Freescale MC13XXX pmic. + config PWM_MXS tristate "Freescale MXS PWM support" depends on ARCH_MXS && OF diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 8b754e4..19f67d7 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o obj-$(CONFIG_PWM_IMX) += pwm-imx.o obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o obj-$(CONFIG_PWM_LPC32XX) += pwm-lpc32xx.o +obj-$(CONFIG_PWM_MC13XXX) += pwm-mc13xxx.o obj-$(CONFIG_PWM_MXS) += pwm-mxs.o obj-$(CONFIG_PWM_PCA9685) += pwm-pca9685.o obj-$(CONFIG_PWM_PUV3) += pwm-puv3.o diff --git a/drivers/pwm/pwm-mc13xxx.c b/drivers/pwm/pwm-mc13xxx.c new file mode 100644 index 0000000..e60e4d8 --- /dev/null +++ b/drivers/pwm/pwm-mc13xxx.c @@ -0,0 +1,205 @@ +/* + * Copyright 2013 Eukréa Electromatique <denis@eukrea.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + + +#include <linux/err.h> +#include <linux/mfd/mc13783.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/of_platform.h> +#include <linux/pwm.h> +#include <linux/slab.h> + +#define MHz(x) (1000*1000*x) + +#define MC13XXX_REG_PWM_CTL 55 +#define MC13XXX_BASE_CLK_FREQ (MHz(2) / 32) +#define MC13XXX_PWM1CLKDIV_SHIFT 6 +#define MC13XXX_PWM2DUTY_SHIFT 12 +#define MC13XXX_PWMDUTYDIVISOR 32 +#define MC13XXX_PWMCLKDIVISOR 64 +#define MC13XXX_PWM_REG_SIZE 0x3f +#define MC13XXX_MIN_PERIOD_NS (NSEC_PER_SEC / MC13XXX_BASE_CLK_FREQ) +#define MC13XXX_MAX_PERIOD_NS (MC13XXX_PWMCLKDIVISOR * MC13XXX_MIN_PERIOD_NS) + +struct mc13xxx_pwm_chip { + struct pwm_chip pwm_chip; + struct mc13xxx *mcdev; +}; + +static inline +struct mc13xxx_pwm_chip *to_mc13xxx_chip(struct pwm_chip *chip) +{ + return container_of(chip, struct mc13xxx_pwm_chip, pwm_chip); +} + +static int pwm_mc13xxx_config(struct pwm_chip *chip, struct pwm_device *pwm, + int duty_ns, int period_ns) +{ + struct mc13xxx_pwm_chip *mc13xxx_chip = to_mc13xxx_chip(chip); + struct mc13xxx *mcdev = mc13xxx_chip->mcdev; + struct device *dev = mc13xxx_chip->pwm_chip.dev; + u32 offset; + u32 mask = MC13XXX_PWM_REG_SIZE; + u32 val; + int ret; + int duty_cycle; + int min_duty_cycle; + int period_cycle; + unsigned long period = period_ns; + + dev_dbg(dev, "requested duty_ns=%d and period_ns=%d\n", + duty_ns, period_ns); + + /* period */ + if (period < MC13XXX_MIN_PERIOD_NS) { + dev_warn(dev, "period was under the range.\n"); + period = MC13XXX_MIN_PERIOD_NS; + } + + if (period > MC13XXX_MAX_PERIOD_NS) { + dev_warn(dev, "period was over the range.\n"); + period = MC13XXX_MAX_PERIOD_NS; + } + + period_cycle = DIV_ROUND_UP(NSEC_PER_SEC, period); + period_cycle = DIV_ROUND_UP(MC13XXX_BASE_CLK_FREQ, period_cycle); + period_cycle--; + + offset = MC13XXX_REG_PWM_CTL + pwm->hwpwm * MC13XXX_PWM2DUTY_SHIFT + + MC13XXX_PWM1CLKDIV_SHIFT; + + mc13xxx_lock(mcdev); + ret = mc13xxx_reg_read(mcdev, offset, &val); + val &= ~mask; + val += (period_cycle & mask); + ret = mc13xxx_reg_write(mcdev, offset, val); + mc13xxx_unlock(mcdev); + + /* duty cycle */ + min_duty_cycle = DIV_ROUND_UP(period, 32); + + if (duty_ns < min_duty_cycle) { + dev_warn(dev, "duty cycle is under the range.\n"); + duty_cycle = 0; + } else if (duty_ns > period) { + dev_warn(dev, "duty cycle is over the range.\n"); + duty_cycle = 32; + } else { + duty_cycle = 32 * period; + duty_cycle = DIV_ROUND_UP(period, duty_ns); + duty_cycle--; + duty_cycle = 32 - duty_cycle; + } + + offset = MC13XXX_REG_PWM_CTL + pwm->hwpwm * MC13XXX_PWM2DUTY_SHIFT; + + mc13xxx_lock(mcdev); + ret = mc13xxx_reg_read(mcdev, offset, &val); + val &= ~mask; + val += (duty_cycle & mask); + ret = mc13xxx_reg_write(mcdev, offset, val); + mc13xxx_unlock(mcdev); + + return 0; +} + +static int pwm_mc13xxx_enable(struct pwm_chip *chip, struct pwm_device *pwm) +{ + /* The hardware doesn't need an enable */ + + return 0; +} + +static void pwm_mc13xxx_disable(struct pwm_chip *chip, struct pwm_device *pwm) +{ + /* The hardware doesn't need a disable */ +} + +static const struct pwm_ops pwm_mc13xxx_ops = { + .enable = pwm_mc13xxx_enable, + .disable = pwm_mc13xxx_disable, + .config = pwm_mc13xxx_config, + .owner = THIS_MODULE, +}; + +static int pwm_mc13xxx_probe(struct platform_device *pdev) +{ + struct mc13xxx_pwm_chip *chip; + struct mc13xxx *mcdev; + int err; + + mcdev = get_mc13xxx(); + if (!mcdev) { + dev_err(&pdev->dev, "failed to find mc13xxx pmic.\n"); + return -EINVAL; + } + + chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL); + if (chip == NULL) { + dev_err(&pdev->dev, "failed to allocate memory\n"); + return -ENOMEM; + } + + chip->pwm_chip.dev = &pdev->dev; + chip->pwm_chip.ops = &pwm_mc13xxx_ops; + chip->pwm_chip.base = pdev->id; + chip->pwm_chip.npwm = 2; + chip->mcdev = mcdev; + + err = pwmchip_add(&chip->pwm_chip); + if (err < 0) + return err; + + platform_set_drvdata(pdev, chip); + + return 0; +} + +static int pwm_mc13xxx_remove(struct platform_device *pdev) +{ + struct mc13xxx_pwm_chip *chip = platform_get_drvdata(pdev); + int ret; + + ret = pwmchip_remove(&chip->pwm_chip); + if (ret < 0) + return ret; + + return 0; +} + +#ifdef CONFIG_OF +static const struct of_device_id pwm_mc13xxx_of_match[] = { + { .compatible = "fsl,mc13892-pwm" }, + { .compatible = "fsl,mc34708-pwm" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, pwm_mc13xxx_of_match); +#endif + +static struct platform_driver pwm_mc13xxx_driver = { + .driver = { + .name = "mc13xxx-pwm", + .of_match_table = of_match_ptr(pwm_mc13xxx_of_match), + .owner = THIS_MODULE, + }, + .probe = pwm_mc13xxx_probe, + .remove = pwm_mc13xxx_remove, +}; + +module_platform_driver(pwm_mc13xxx_driver); + +MODULE_AUTHOR("Denis Carikli <denis@eukrea.com>"); +MODULE_DESCRIPTION("mc13xxx Pulse Width Modulation Driver"); +MODULE_ALIAS("platform:mc13xxx-pwm"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/mfd/mc13783.h b/include/linux/mfd/mc13783.h index a8eeda7..ec934e8 100644 --- a/include/linux/mfd/mc13783.h +++ b/include/linux/mfd/mc13783.h @@ -88,4 +88,6 @@ #define MC13783_IRQ_AHSSHORT 45 #define MC13783_NUM_IRQ MC13XXX_NUM_IRQ +struct mc13xxx *get_mc13xxx(void); + #endif /* ifndef __LINUX_MFD_MC13783_H */
Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Grant Likely <grant.likely@linaro.org> Cc: Rob Herring <rob.herring@calxeda.com> Cc: devicetree@vger.kernel.org Cc: linux-pwm@vger.kernel.org Cc: Samuel Ortiz <sameo@linux.intel.com> Cc: Lee Jones <lee.jones@linaro.org> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Denis Carikli <denis@eukrea.com> --- .../devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt | 19 ++ drivers/mfd/mc13xxx-core.c | 11 ++ drivers/pwm/Kconfig | 6 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-mc13xxx.c | 205 ++++++++++++++++++++ include/linux/mfd/mc13783.h | 2 + 6 files changed, 244 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/fsl,mc13xxx-pwm.txt create mode 100644 drivers/pwm/pwm-mc13xxx.c