| Message ID | 1400523003-27082-2-git-send-email-alexandre.belloni@free-electrons.com |
|---|---|
| State | Rejected |
| Headers | show |
On Mon, May 19, 2014 at 08:10:02PM +0200, Alexandre Belloni wrote: [...] > diff --git a/drivers/pwm/pwm-sunxi.c b/drivers/pwm/pwm-sunxi.c [...] > +#include <linux/bitops.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pwm.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/slab.h> These should be ordered alphabetically. > +#define PWM_CTRL_REG 0x0 > + > +#define PWM_CH_PRD_BASE 0x4 > +#define PWM_CH_PRD_OFF 0x4 Is this supposed to be an offset? If so, then maybe it should be named PWM_CH_PRD_OFFSET? > +#define PWM_CH_PRD(x) (PWM_CH_PRD_BASE + PWM_CH_PRD_OFF * (x)) x is the channel number? If so maybe call that parameter "ch"? > +#define PWMCH_OFFSET 15 > +#define PWM_PRESCAL_MASK GENMASK(3, 0) > +#define PWM_PRESCAL_OFF 0 > +#define PWM_EN BIT(4) > +#define PWM_ACT_STATE BIT(5) > +#define PWM_CLK_GATING BIT(6) > +#define PWM_MODE BIT(7) > +#define PWM_PULSE BIT(8) > +#define PWM_BYPASS BIT(9) > + > +#define PWM_RDY_BASE 28 > +#define PWM_RDY_OFF 1 > +#define PWM_RDY(x) BIT(PWM_RDY_BASE + PWM_RDY_OFF * (x)) Some comments as for PWM_CH_PRD above. > +#define PWM_PRD_ACT_MASK GENMASK(7, 0) This seems to be unused. > +#define PWM_PRD(x) ((x - 1) << 16) x should be enclosed in (). > +#define PWM_PRD_MASK GENMASK(7, 0) > + > +#define BIT_CH(bit, chan) (bit << (chan * PWMCH_OFFSET)) There should be a space instead of a tab between "define" and "BIT_CH(...)". > +u32 prescal_table[] = { 120, 180, 240, 360, 480, 0, 0, 0, > + 12000, 24000, 36000, 48000, 72000, > + 0, 0, 1 }; static const? Also this is oddly formatted. I'd prefer: static const u32 prescal_table[] = { ... ... }; Also for readability I'd go for "prescaler_table" or "prescale_table". > +struct sunxi_pwm_data { > + bool has_rdy; > +}; > + > +struct sunxi_pwm_chip { > + struct pwm_chip chip; > + struct clk *clk; > + void __iomem *base; > + struct mutex ctrl_lock; > + const struct sunxi_pwm_data *data; > +}; > + > +#define to_sunxi_pwm_chip(chip) container_of(chip, struct sunxi_pwm_chip, chip) This should be a static inline function. > +static inline u32 sunxi_pwm_readl(struct sunxi_pwm_chip *chip, > + unsigned long offset) > +{ > + return readl(chip->base + offset); > +} > + > +static inline void sunxi_pwm_writel(struct sunxi_pwm_chip *chip, > + unsigned long offset, unsigned long val) Make val u32 for consistency with sunxi_pwm_readl()? Also I'd prefer if this function preserved the same parameter order as writel(). > +static int sunxi_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > + int duty_ns, int period_ns) > +{ > + struct sunxi_pwm_chip *sunxi_pwm = to_sunxi_pwm_chip(chip); > + u32 clk_rate, prd, dty; > + u64 div; > + u32 val, clk_gate; These can both go onto the same line as the other u32s above. > + int i, ret; I think i should be unsigned. And maybe rename ret to err since it's not used as a return value at all. > + > + clk_rate = clk_get_rate(sunxi_pwm->clk); > + > + /* First, test without any divider */ > + i = PWM_PRESCAL_MASK; I don't see where this value of I is used. > + div = clk_rate * period_ns; > + do_div(div, 1000000000); > + if (div > PWM_PRD_MASK) { > + /* Then go up from the first divider */ > + for (i = 0; i < PWM_PRESCAL_MASK; i++) { > + if (!prescal_table[i]) > + continue; > + div = clk_rate / prescal_table[i]; > + div = div * period_ns; > + do_div(div, 1000000000); Maybe: div = clk_rate / prescal_table[i]; do_div(div * period_ns, 1000000000); ? Also 1000000000 == NSEC_PER_SEC, so maybe use that instead. > + if (div <= PWM_PRD_MASK) > + break; > + } > + } > + > + if (div > PWM_PRD_MASK) { > + dev_err(chip->dev, "prescaler exceeds the maximum value\n"); Nit: the prescaler doesn't exceed anything. Rather the period exceeds the maximum. > + return -EINVAL; > + } > + > + prd = div; > + div *= duty_ns; > + do_div(div, period_ns); > + dty = div; > + > + ret = clk_prepare_enable(sunxi_pwm->clk); > + if (ret) { > + dev_err(chip->dev, "failed to enable PWM clock\n"); > + return ret; > + } > + > + mutex_lock(&sunxi_pwm->ctrl_lock); > + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); > + > + if (sunxi_pwm->data->has_rdy && (val & PWM_RDY(pwm->hwpwm))) { > + mutex_unlock(&sunxi_pwm->ctrl_lock); > + clk_disable_unprepare(sunxi_pwm->clk); > + return -EBUSY; > + } > + > + clk_gate = val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm); > + if (clk_gate) { > + val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); > + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); > + } > + > + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); > + val &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm); > + val |= i; Ah, this is where the initial value is used. Perhaps rename i to prescaler to make it more obvious what it is. > + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); > + > + sunxi_pwm_writel(sunxi_pwm, PWM_CH_PRD(pwm->hwpwm), dty | PWM_PRD(prd)); Maybe split this into two lines for readability: val = dty | PWM_PRD(prd); sunxi_pwm_writel(...); Also does dty need to be range-checked so it doesn't spill over into the PRD field? > +static int sunxi_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, > + enum pwm_polarity polarity) > +{ > + struct sunxi_pwm_chip *sunxi_pwm = to_sunxi_pwm_chip(chip); > + u32 val; > + int ret; > + > + ret = clk_prepare_enable(sunxi_pwm->clk); > + if (ret) { > + dev_err(chip->dev, "failed to enable PWM clock\n"); > + return ret; > + } > + > + mutex_lock(&sunxi_pwm->ctrl_lock); > + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); > + > + if (polarity != PWM_POLARITY_NORMAL) > + val &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm); > + else > + val |= BIT_CH(PWM_ACT_STATE, pwm->hwpwm); > + > + There's an extra blank line here. > +static int sunxi_pwm_probe(struct platform_device *pdev) > +{ > + struct sunxi_pwm_chip *sunxi_pwm; Perhaps just "pwm" or "sunxi"? sunxi_pwm is kind of redundant. > + struct resource *res; > + int ret; > + > + const struct of_device_id *match; There should be no blank line above this one. > + > + match = of_match_device(sunxi_pwm_dt_ids, &pdev->dev); > + if (!match || !match->data) None of this can ever happen. match would only be NULL if the table doesn't contain a matching entry for this device. But in that case the driver's .probe() wouldn't have been called anyway. And none of the entries have a NULL .data field, so no need to check for that either. > + return -ENODEV; > + > + sunxi_pwm = devm_kzalloc(&pdev->dev, sizeof(*sunxi_pwm), GFP_KERNEL); > + if (!sunxi_pwm) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + sunxi_pwm->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(sunxi_pwm->base)) > + return PTR_ERR(sunxi_pwm->base); > + > + sunxi_pwm->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(sunxi_pwm->clk)) > + return PTR_ERR(sunxi_pwm->clk); > + > + sunxi_pwm->chip.dev = &pdev->dev; > + sunxi_pwm->chip.ops = &sunxi_pwm_ops; > + > + sunxi_pwm->chip.base = -1; Why the blank line between the above? > + sunxi_pwm->chip.npwm = 2; > + sunxi_pwm->chip.can_sleep = true; > + sunxi_pwm->chip.of_xlate = of_pwm_xlate_with_flags; > + sunxi_pwm->chip.of_pwm_n_cells = 3; > + sunxi_pwm->data = match->data; > + > + mutex_init(&sunxi_pwm->ctrl_lock); > + > + ret = clk_prepare_enable(sunxi_pwm->clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to enable PWM clock\n"); > + goto error; > + } > + > + /* By default, the polarity is inversed, set it to normal */ > + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, > + BIT_CH(PWM_ACT_STATE, 0) | > + BIT_CH(PWM_ACT_STATE, 1)); > + clk_disable_unprepare(sunxi_pwm->clk); Why do you need to do this here? Doesn't this potentially cause transients if a bootloader had this configured with inversed polarity? Thierry
On 18/06/2014 at 01:26:06 +0200, Thierry Reding wrote : > On Mon, May 19, 2014 at 08:10:02PM +0200, Alexandre Belloni wrote: > > + /* By default, the polarity is inversed, set it to normal */ > > + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, > > + BIT_CH(PWM_ACT_STATE, 0) | > > + BIT_CH(PWM_ACT_STATE, 1)); > > + clk_disable_unprepare(sunxi_pwm->clk); > > Why do you need to do this here? Doesn't this potentially cause > transients if a bootloader had this configured with inversed polarity? It was done a few months ago but what I remember is the following happens: The PWM subsystem assumes that the polarity is PWM_POLARITY_NORMAL because of the kzalloc pwmchip_add(). Would you prefer something like: val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); for (i = 0; i < sunxi_pwm->chip.npwm; i++) { if (!(val & BIT_CH(PWM_ACT_STATE, i))) sunxi_pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED; } Then, you would have a race where the PWM polarity is not correct in sysfs between pwmchip_add() and that code. Also, if you want to preserve the state set by the bootloader, you actually have an issue with getting back the other members of the pwm_device struct (duty, period) and more importantly the PWMF_ENABLED flag. It now assumed that the PWM channel is not enabled when registering the chip. If you now say that it may be enabled before linux is booting and you want to keep it running, then you have an inconsistency between the real state of the PWM (enabled, with a duty, period and polarity set) and what the PWM susbsytem actually knows about the PWM (not enabled, duty and period == 0 and polarity is normal). I would agree that the usual use case would be that another driver will take the PWM and set the duty, period and polarity anyway but the issue with the PWMF_ENABLED flag remains. How do you want to fix this? Would you add a new callback that would be called by pwmchip_add(), before pwmchip_sysfs_export()? I actually find it ugly to set the pwm_device members from the probe, especially the flags. I would prefer they stay hidden by the API.
I was unaware of this driver and reimplemented it. Patch is attached. Some differences I noticed... 1) I implemented bypass mode 2) I had to do math in picoseconds to avoid round off/truncation errors. 3) counter registers are different lengths on SUN4 vs rest. Can someone with a scope verify if the prescaler of 1 works at high frequencies? Like cycles/active of 2/1, 3/1, 4,1... On Mon, Jun 23, 2014 at 1:01 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote: > On 18/06/2014 at 01:26:06 +0200, Thierry Reding wrote : >> On Mon, May 19, 2014 at 08:10:02PM +0200, Alexandre Belloni wrote: >> > + /* By default, the polarity is inversed, set it to normal */ >> > + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, >> > + BIT_CH(PWM_ACT_STATE, 0) | >> > + BIT_CH(PWM_ACT_STATE, 1)); >> > + clk_disable_unprepare(sunxi_pwm->clk); >> >> Why do you need to do this here? Doesn't this potentially cause >> transients if a bootloader had this configured with inversed polarity? > > > It was done a few months ago but what I remember is the following > happens: > > The PWM subsystem assumes that the polarity is PWM_POLARITY_NORMAL > because of the kzalloc pwmchip_add(). Would you prefer something like: > > val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); > for (i = 0; i < sunxi_pwm->chip.npwm; i++) { > if (!(val & BIT_CH(PWM_ACT_STATE, i))) > sunxi_pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED; > } > > Then, you would have a race where the PWM polarity is not correct in > sysfs between pwmchip_add() and that code. > > Also, if you want to preserve the state set by the bootloader, you > actually have an issue with getting back the other members of the > pwm_device struct (duty, period) and more importantly the PWMF_ENABLED > flag. It now assumed that the PWM channel is not enabled when > registering the chip. If you now say that it may be enabled before linux > is booting and you want to keep it running, then you have an > inconsistency between the real state of the PWM (enabled, with a duty, > period and polarity set) and what the PWM susbsytem actually knows about > the PWM (not enabled, duty and period == 0 and polarity is normal). > > I would agree that the usual use case would be that another driver will > take the PWM and set the duty, period and polarity anyway but the issue > with the PWMF_ENABLED flag remains. > > How do you want to fix this? Would you add a new callback that would be > called by pwmchip_add(), before pwmchip_sysfs_export()? > > I actually find it ugly to set the pwm_device members from the probe, > especially the flags. I would prefer they stay hidden by the API. > > > -- > Alexandre Belloni, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
New version that adds better support for the A10 plus it fixes an issue in the previous version. Note that the A10 does not support the 24Mhz bypass mode.
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 5b34ff29ea38..178b017be827 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -217,6 +217,15 @@ config PWM_SPEAR To compile this driver as a module, choose M here: the module will be called pwm-spear. +config PWM_SUNXI + tristate "Allwinner PWM support" + depends on ARCH_SUNXI || COMPILE_TEST + help + Generic PWM framework driver for Allwinner SoCs. + + To compile this driver as a module, choose M here: the module + will be called pwm-sunxi. + config PWM_TEGRA tristate "NVIDIA Tegra PWM support" depends on ARCH_TEGRA diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index e57d2c38a794..39997ea2e276 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_PWM_PXA) += pwm-pxa.o obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o obj-$(CONFIG_PWM_SPEAR) += pwm-spear.o +obj-$(CONFIG_PWM_SUNXI) += pwm-sunxi.o obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o diff --git a/drivers/pwm/pwm-sunxi.c b/drivers/pwm/pwm-sunxi.c new file mode 100644 index 000000000000..e7c3ca1d3c42 --- /dev/null +++ b/drivers/pwm/pwm-sunxi.c @@ -0,0 +1,338 @@ +/* + * Driver for Allwinner Pulse Width Modulation Controller + * + * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@free-electrons.com> + * + * Licensed under GPLv2. + */ + +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/slab.h> + +#define PWM_CTRL_REG 0x0 + +#define PWM_CH_PRD_BASE 0x4 +#define PWM_CH_PRD_OFF 0x4 +#define PWM_CH_PRD(x) (PWM_CH_PRD_BASE + PWM_CH_PRD_OFF * (x)) + +#define PWMCH_OFFSET 15 +#define PWM_PRESCAL_MASK GENMASK(3, 0) +#define PWM_PRESCAL_OFF 0 +#define PWM_EN BIT(4) +#define PWM_ACT_STATE BIT(5) +#define PWM_CLK_GATING BIT(6) +#define PWM_MODE BIT(7) +#define PWM_PULSE BIT(8) +#define PWM_BYPASS BIT(9) + +#define PWM_RDY_BASE 28 +#define PWM_RDY_OFF 1 +#define PWM_RDY(x) BIT(PWM_RDY_BASE + PWM_RDY_OFF * (x)) + +#define PWM_PRD_ACT_MASK GENMASK(7, 0) +#define PWM_PRD(x) ((x - 1) << 16) +#define PWM_PRD_MASK GENMASK(7, 0) + +#define BIT_CH(bit, chan) (bit << (chan * PWMCH_OFFSET)) + +u32 prescal_table[] = { 120, 180, 240, 360, 480, 0, 0, 0, + 12000, 24000, 36000, 48000, 72000, + 0, 0, 1 }; + +struct sunxi_pwm_data { + bool has_rdy; +}; + +struct sunxi_pwm_chip { + struct pwm_chip chip; + struct clk *clk; + void __iomem *base; + struct mutex ctrl_lock; + const struct sunxi_pwm_data *data; +}; + +#define to_sunxi_pwm_chip(chip) container_of(chip, struct sunxi_pwm_chip, chip) + +static inline u32 sunxi_pwm_readl(struct sunxi_pwm_chip *chip, + unsigned long offset) +{ + return readl(chip->base + offset); +} + +static inline void sunxi_pwm_writel(struct sunxi_pwm_chip *chip, + unsigned long offset, unsigned long val) +{ + writel(val, chip->base + offset); +} + +static int sunxi_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, + int duty_ns, int period_ns) +{ + struct sunxi_pwm_chip *sunxi_pwm = to_sunxi_pwm_chip(chip); + u32 clk_rate, prd, dty; + u64 div; + u32 val, clk_gate; + int i, ret; + + clk_rate = clk_get_rate(sunxi_pwm->clk); + + /* First, test without any divider */ + i = PWM_PRESCAL_MASK; + div = clk_rate * period_ns; + do_div(div, 1000000000); + if (div > PWM_PRD_MASK) { + /* Then go up from the first divider */ + for (i = 0; i < PWM_PRESCAL_MASK; i++) { + if (!prescal_table[i]) + continue; + div = clk_rate / prescal_table[i]; + div = div * period_ns; + do_div(div, 1000000000); + if (div <= PWM_PRD_MASK) + break; + } + } + + if (div > PWM_PRD_MASK) { + dev_err(chip->dev, "prescaler exceeds the maximum value\n"); + return -EINVAL; + } + + prd = div; + div *= duty_ns; + do_div(div, period_ns); + dty = div; + + ret = clk_prepare_enable(sunxi_pwm->clk); + if (ret) { + dev_err(chip->dev, "failed to enable PWM clock\n"); + return ret; + } + + mutex_lock(&sunxi_pwm->ctrl_lock); + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); + + if (sunxi_pwm->data->has_rdy && (val & PWM_RDY(pwm->hwpwm))) { + mutex_unlock(&sunxi_pwm->ctrl_lock); + clk_disable_unprepare(sunxi_pwm->clk); + return -EBUSY; + } + + clk_gate = val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm); + if (clk_gate) { + val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); + } + + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); + val &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm); + val |= i; + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); + + sunxi_pwm_writel(sunxi_pwm, PWM_CH_PRD(pwm->hwpwm), dty | PWM_PRD(prd)); + + if (clk_gate) { + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); + val |= clk_gate; + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); + } + + mutex_unlock(&sunxi_pwm->ctrl_lock); + clk_disable_unprepare(sunxi_pwm->clk); + + return 0; +} + +static int sunxi_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm, + enum pwm_polarity polarity) +{ + struct sunxi_pwm_chip *sunxi_pwm = to_sunxi_pwm_chip(chip); + u32 val; + int ret; + + ret = clk_prepare_enable(sunxi_pwm->clk); + if (ret) { + dev_err(chip->dev, "failed to enable PWM clock\n"); + return ret; + } + + mutex_lock(&sunxi_pwm->ctrl_lock); + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); + + if (polarity != PWM_POLARITY_NORMAL) + val &= ~BIT_CH(PWM_ACT_STATE, pwm->hwpwm); + else + val |= BIT_CH(PWM_ACT_STATE, pwm->hwpwm); + + + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); + + mutex_unlock(&sunxi_pwm->ctrl_lock); + clk_disable_unprepare(sunxi_pwm->clk); + + return 0; +} + +static int sunxi_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) +{ + struct sunxi_pwm_chip *sunxi_pwm = to_sunxi_pwm_chip(chip); + u32 val; + int ret; + + ret = clk_prepare_enable(sunxi_pwm->clk); + if (ret) { + dev_err(chip->dev, "failed to enable PWM clock\n"); + return ret; + } + + mutex_lock(&sunxi_pwm->ctrl_lock); + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); + val |= BIT_CH(PWM_EN, pwm->hwpwm); + val |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm); + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); + mutex_unlock(&sunxi_pwm->ctrl_lock); + + return 0; +} + +static void sunxi_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm) +{ + struct sunxi_pwm_chip *sunxi_pwm = to_sunxi_pwm_chip(chip); + u32 val; + + mutex_lock(&sunxi_pwm->ctrl_lock); + val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG); + val &= ~BIT_CH(PWM_EN, pwm->hwpwm); + val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm); + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, val); + mutex_unlock(&sunxi_pwm->ctrl_lock); + + clk_disable_unprepare(sunxi_pwm->clk); +} + +static const struct pwm_ops sunxi_pwm_ops = { + .config = sunxi_pwm_config, + .set_polarity = sunxi_pwm_set_polarity, + .enable = sunxi_pwm_enable, + .disable = sunxi_pwm_disable, + .owner = THIS_MODULE, +}; + +static const struct sunxi_pwm_data sunxi_pwm_data_a10 = { + .has_rdy = false, +}; + +static const struct sunxi_pwm_data sunxi_pwm_data_a20 = { + .has_rdy = true, +}; + +static const struct of_device_id sunxi_pwm_dt_ids[] = { + { + .compatible = "allwinner,sun4i-a10-pwm", + .data = &sunxi_pwm_data_a10, + }, { + .compatible = "allwinner,sun7i-a20-pwm", + .data = &sunxi_pwm_data_a20, + }, { + /* sentinel */ + }, +}; +MODULE_DEVICE_TABLE(of, sunxi_pwm_dt_ids); + +static int sunxi_pwm_probe(struct platform_device *pdev) +{ + struct sunxi_pwm_chip *sunxi_pwm; + struct resource *res; + int ret; + + const struct of_device_id *match; + + match = of_match_device(sunxi_pwm_dt_ids, &pdev->dev); + if (!match || !match->data) + return -ENODEV; + + sunxi_pwm = devm_kzalloc(&pdev->dev, sizeof(*sunxi_pwm), GFP_KERNEL); + if (!sunxi_pwm) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + sunxi_pwm->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(sunxi_pwm->base)) + return PTR_ERR(sunxi_pwm->base); + + sunxi_pwm->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(sunxi_pwm->clk)) + return PTR_ERR(sunxi_pwm->clk); + + sunxi_pwm->chip.dev = &pdev->dev; + sunxi_pwm->chip.ops = &sunxi_pwm_ops; + + sunxi_pwm->chip.base = -1; + sunxi_pwm->chip.npwm = 2; + sunxi_pwm->chip.can_sleep = true; + sunxi_pwm->chip.of_xlate = of_pwm_xlate_with_flags; + sunxi_pwm->chip.of_pwm_n_cells = 3; + sunxi_pwm->data = match->data; + + mutex_init(&sunxi_pwm->ctrl_lock); + + ret = clk_prepare_enable(sunxi_pwm->clk); + if (ret) { + dev_err(&pdev->dev, "failed to enable PWM clock\n"); + goto error; + } + + /* By default, the polarity is inversed, set it to normal */ + sunxi_pwm_writel(sunxi_pwm, PWM_CTRL_REG, + BIT_CH(PWM_ACT_STATE, 0) | + BIT_CH(PWM_ACT_STATE, 1)); + clk_disable_unprepare(sunxi_pwm->clk); + + ret = pwmchip_add(&sunxi_pwm->chip); + if (ret < 0) { + dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret); + goto error; + } + + platform_set_drvdata(pdev, sunxi_pwm); + + return ret; + +error: + mutex_destroy(&sunxi_pwm->ctrl_lock); + clk_disable_unprepare(sunxi_pwm->clk); + return ret; +} + +static int sunxi_pwm_remove(struct platform_device *pdev) +{ + struct sunxi_pwm_chip *sunxi_pwm = platform_get_drvdata(pdev); + + mutex_destroy(&sunxi_pwm->ctrl_lock); + + return pwmchip_remove(&sunxi_pwm->chip); +} + +static struct platform_driver sunxi_pwm_driver = { + .driver = { + .name = "sunxi-pwm", + .of_match_table = sunxi_pwm_dt_ids, + }, + .probe = sunxi_pwm_probe, + .remove = sunxi_pwm_remove, +}; +module_platform_driver(sunxi_pwm_driver); + +MODULE_ALIAS("platform:sunxi-pwm"); +MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>"); +MODULE_DESCRIPTION("Allwinner PWM driver"); +MODULE_LICENSE("GPL v2");