[PATCHv7 0/2] Add Allwinner SoCs PWM support

[PATCHv7 0/2] Add Allwinner SoCs PWM support

[Alexandre Belloni]

Hi,

This patch series adds support for the PWM controller found on the Allwinner
SoCs.

The first patch adds the driver itself.
The second patch adds the DT binding documentation

Changes in v7:
 - rebased on linux-pwm/for-next

Changes in v6:
 - Took into account comments from Thierry
 - Removed the prescaler bypass from sun4i (reported by Jon Smirl)
 - Rounding period half up when not using the prescaler (reported by Jon Smirl)
 - Corrected issues reported by Simon (PWM_PRD_MASK, channel 1 and off-by-one
   error)

Alexandre Belloni (2):
  pwm: Add Allwinner SoC support
  pwm: sunxi: document OF bindings

 .../devicetree/bindings/pwm/pwm-sunxi.txt          |  20 ++
 drivers/pwm/Kconfig                                |   9 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-sunxi.c                            | 371 +++++++++++++++++++++
 4 files changed, 401 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sunxi.txt
 create mode 100644 drivers/pwm/pwm-sunxi.c

-- 
1.9.1

[PATCHv7 1/2] pwm: Add Allwinner SoC support

[Alexandre Belloni]

This adds a generic PWM framework driver for the PWM controller
found on Allwinner SoCs.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/pwm/Kconfig     |   9 ++
 drivers/pwm/Makefile    |   1 +
 drivers/pwm/pwm-sunxi.c | 371 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 381 insertions(+)
 create mode 100644 drivers/pwm/pwm-sunxi.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 3865dfb9ed08..80e03df5744f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -262,6 +262,15 @@ config PWM_STI
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-sti.
 
+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 c458606c3755..a9386bd61956 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
 obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
 obj-$(CONFIG_PWM_SPEAR)		+= pwm-spear.o
 obj-$(CONFIG_PWM_STI)		+= pwm-sti.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..643f84ea013e
--- /dev/null
+++ b/drivers/pwm/pwm-sunxi.c
@@ -0,0 +1,371 @@
+/*
+ * 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/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+#include <linux/time.h>
+
+#define PWM_CTRL_REG		0x0
+
+#define PWM_CH_PRD_BASE		0x4
+#define PWM_CH_PRD_OFFSET	0x4
+#define PWM_CH_PRD(ch)		(PWM_CH_PRD_BASE + PWM_CH_PRD_OFFSET * (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_OFFSET		1
+#define PWM_RDY(ch)		BIT(PWM_RDY_BASE + PWM_RDY_OFFSET * (ch))
+
+#define PWM_PRD(prd)		(((prd) - 1) << 16)
+#define PWM_PRD_MASK		GENMASK(15, 0)
+
+#define PWM_DTY_MASK		GENMASK(15, 0)
+
+#define BIT_CH(bit, chan)	(bit << (chan * PWMCH_OFFSET))
+
+static const u32 prescaler_table[] = {
+	120,
+	180,
+	240,
+	360,
+	480,
+	0,
+	0,
+	0,
+	12000,
+	24000,
+	36000,
+	48000,
+	72000,
+	0,
+	0,
+	0, /* Actually 1 but tested separately */
+};
+
+struct sunxi_pwm_data {
+	bool has_prescaler_bypass;
+	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;
+};
+
+static inline struct sunxi_pwm_chip *to_sunxi_pwm_chip(struct pwm_chip *chip)
+{
+	return 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,
+				    u32 val, unsigned long offset)
+{
+	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, val, clk_gate;
+	u64 div = 0;
+	unsigned int prescaler = 0;
+	int err;
+
+	clk_rate = clk_get_rate(sunxi_pwm->clk);
+
+	if (sunxi_pwm->data->has_prescaler_bypass) {
+		/* First, test without any prescaler when available */
+		prescaler = PWM_PRESCAL_MASK;
+		/*
+		 * When not using any prescaler, the clock period in nanoseconds
+		 * is not an integer so round it half up instead of
+		 * truncating to get less surprising values.
+		 */
+		div = clk_rate * (u64)period_ns + NSEC_PER_SEC/2;
+		do_div(div, NSEC_PER_SEC);
+		if (div - 1 > PWM_PRD_MASK)
+			prescaler = 0;
+	}
+
+	if (prescaler == 0) {
+		/* Go up from the first divider */
+		for (prescaler = 0; prescaler < PWM_PRESCAL_MASK; prescaler++) {
+			if (!prescaler_table[prescaler])
+				continue;
+			div = clk_rate / prescaler_table[prescaler];
+			div = div * (u64)period_ns;
+			do_div(div, NSEC_PER_SEC);
+			if (div - 1 <= PWM_PRD_MASK)
+				break;
+		}
+
+		if (div - 1 > PWM_PRD_MASK) {
+			dev_err(chip->dev, "period exceeds the maximum value\n");
+			return -EINVAL;
+		}
+	}
+
+	prd = div;
+	div *= duty_ns;
+	do_div(div, period_ns);
+	dty = div;
+
+	err = clk_prepare_enable(sunxi_pwm->clk);
+	if (err) {
+		dev_err(chip->dev, "failed to enable PWM clock\n");
+		return err;
+	}
+
+	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, val, PWM_CTRL_REG);
+	}
+
+	val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG);
+	val &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm);
+	val |= BIT_CH(prescaler, pwm->hwpwm);
+	sunxi_pwm_writel(sunxi_pwm, val, PWM_CTRL_REG);
+
+	val = (dty & PWM_DTY_MASK) | PWM_PRD(prd);
+	sunxi_pwm_writel(sunxi_pwm, val, PWM_CH_PRD(pwm->hwpwm));
+
+	if (clk_gate) {
+		val = sunxi_pwm_readl(sunxi_pwm, PWM_CTRL_REG);
+		val |= clk_gate;
+		sunxi_pwm_writel(sunxi_pwm, val, PWM_CTRL_REG);
+	}
+
+	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, val, PWM_CTRL_REG);
+
+	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, val, PWM_CTRL_REG);
+	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, val, PWM_CTRL_REG);
+	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_prescaler_bypass = false,
+	.has_rdy = false,
+};
+
+static const struct sunxi_pwm_data sunxi_pwm_data_a20 = {
+	.has_prescaler_bypass = true,
+	.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 *pwm;
+	struct resource *res;
+	u32 val;
+	int i, ret;
+	const struct of_device_id *match;
+
+	match = of_match_device(sunxi_pwm_dt_ids, &pdev->dev);
+
+	pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
+	if (!pwm)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pwm->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pwm->base))
+		return PTR_ERR(pwm->base);
+
+	pwm->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pwm->clk))
+		return PTR_ERR(pwm->clk);
+
+	pwm->chip.dev = &pdev->dev;
+	pwm->chip.ops = &sunxi_pwm_ops;
+	pwm->chip.base = -1;
+	pwm->chip.npwm = 2;
+	pwm->chip.can_sleep = true;
+	pwm->chip.of_xlate = of_pwm_xlate_with_flags;
+	pwm->chip.of_pwm_n_cells = 3;
+	pwm->data = match->data;
+
+	mutex_init(&pwm->ctrl_lock);
+
+	ret = pwmchip_add(&pwm->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret);
+		goto error;
+	}
+
+	ret = clk_prepare_enable(pwm->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable PWM clock\n");
+		goto error;
+	}
+
+	val = sunxi_pwm_readl(pwm, PWM_CTRL_REG);
+	for (i = 0; i < pwm->chip.npwm; i++) {
+		if (!(val & BIT_CH(PWM_ACT_STATE, i)))
+			pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED;
+	}
+
+	clk_disable_unprepare(pwm->clk);
+
+	platform_set_drvdata(pdev, pwm);
+
+	return ret;
+
+error:
+	mutex_destroy(&pwm->ctrl_lock);
+	clk_disable_unprepare(pwm->clk);
+	return ret;
+}
+
+static int sunxi_pwm_remove(struct platform_device *pdev)
+{
+	struct sunxi_pwm_chip *pwm = platform_get_drvdata(pdev);
+
+	mutex_destroy(&pwm->ctrl_lock);
+
+	return pwmchip_remove(&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");
-- 
1.9.1

[PATCHv7 1/2] pwm: Add Allwinner SoC support

[Thierry Reding]

On Wed, Sep 17, 2014 at 09:53:57PM +0200, Alexandre Belloni wrote:
[...]
> diff --git a/drivers/pwm/pwm-sunxi.c b/drivers/pwm/pwm-sunxi.c
> new file mode 100644
> index 000000000000..643f84ea013e
> --- /dev/null
> +++ b/drivers/pwm/pwm-sunxi.c
> @@ -0,0 +1,371 @@
> +/*
> + * Driver for Allwinner Pulse Width Modulation Controller
> + *
> + * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@free-electrons.com>

This is somewhat weird. So you are the copyright holder, not your
employer? The email address is somewhat misleading.

> +#define BIT_CH(bit, chan)	(bit << (chan * PWMCH_OFFSET))

bit and chan could use additional parentheses here for extra safety.

> +static const u32 prescaler_table[] = {
> +	120,
> +	180,
> +	240,
> +	360,
> +	480,
> +	0,
> +	0,
> +	0,
> +	12000,
> +	24000,
> +	36000,
> +	48000,
> +	72000,
> +	0,
> +	0,
> +	0, /* Actually 1 but tested separately */
> +};

Did I already mention that this was really weird?

> +static inline u32 sunxi_pwm_readl(struct sunxi_pwm_chip *chip,
> +				  unsigned long offset)
> +{
> +	return readl(chip->base + offset);
> +}
> +
> +

There's a gratuitous blank line here.

> +static int sunxi_pwm_probe(struct platform_device *pdev)
> +{
[...]
> +
> +	ret = pwmchip_add(&pwm->chip);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret);

"chip: %d" to make it clear that it's not some kind of chip ID.

> +		goto error;
> +	}
> +
> +	ret = clk_prepare_enable(pwm->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to enable PWM clock\n");
> +		goto error;

Don't you want to remove the stale PWM chip here? Why not do this at the
very end, after everything's been set up properly?

> +MODULE_ALIAS("platform:sunxi-pwm");
> +MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
> +MODULE_DESCRIPTION("Allwinner PWM driver");

Perhaps "Allwinner sunxi PWM driver"? Presumably Allwinner could at some
point release a different family of SoCs.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141006/086cee29/attachment-0001.sig>

[PATCHv7 1/2] pwm: Add Allwinner SoC support

[Alexandre Belloni]

On 06/10/2014 at 15:24:05 +0200, Thierry Reding wrote :
> On Wed, Sep 17, 2014 at 09:53:57PM +0200, Alexandre Belloni wrote:
> [...]
> > diff --git a/drivers/pwm/pwm-sunxi.c b/drivers/pwm/pwm-sunxi.c
> > new file mode 100644
> > index 000000000000..643f84ea013e
> > --- /dev/null
> > +++ b/drivers/pwm/pwm-sunxi.c
> > @@ -0,0 +1,371 @@
> > +/*
> > + * Driver for Allwinner Pulse Width Modulation Controller
> > + *
> > + * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@free-electrons.com>
> 
> This is somewhat weird. So you are the copyright holder, not your
> employer? The email address is somewhat misleading.
> 

It has been done on my free, unpaid time so yes...

Anyway, even my employer could not actually claim to hold copyright on
what I'm writing as this right is not transferable in France.

> > +#define BIT_CH(bit, chan)	(bit << (chan * PWMCH_OFFSET))
> 
> bit and chan could use additional parentheses here for extra safety.
> 

OK

> > +static const u32 prescaler_table[] = {
> > +	120,
> > +	180,
> > +	240,
> > +	360,
> > +	480,
> > +	0,
> > +	0,
> > +	0,
> > +	12000,
> > +	24000,
> > +	36000,
> > +	48000,
> > +	72000,
> > +	0,
> > +	0,
> > +	0, /* Actually 1 but tested separately */
> > +};
> 
> Did I already mention that this was really weird?
> 

I'm not sure what you want to do here then. This is simply coming from
the datasheet. The last one (0xF) being a bypass is actually only existing on 

> > +static inline u32 sunxi_pwm_readl(struct sunxi_pwm_chip *chip,
> > +				  unsigned long offset)
> > +{
> > +	return readl(chip->base + offset);
> > +}
> > +
> > +
> 
> There's a gratuitous blank line here.
> 
> > +static int sunxi_pwm_probe(struct platform_device *pdev)
> > +{
> [...]
> > +
> > +	ret = pwmchip_add(&pwm->chip);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret);
> 
> "chip: %d" to make it clear that it's not some kind of chip ID.
> 
> > +		goto error;
> > +	}
> > +
> > +	ret = clk_prepare_enable(pwm->clk);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to enable PWM clock\n");
> > +		goto error;
> 
> Don't you want to remove the stale PWM chip here? Why not do this at the
> very end, after everything's been set up properly?

Yeah, as this is not critical, I will simply exit without an error in
that case and I'll move that block after platform_set_drvdata().

> 
> > +MODULE_ALIAS("platform:sunxi-pwm");
> > +MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
> > +MODULE_DESCRIPTION("Allwinner PWM driver");
> 
> Perhaps "Allwinner sunxi PWM driver"? Presumably Allwinner could at some
> point release a different family of SoCs.
> 

Ok


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCHv7 1/2] pwm: Add Allwinner SoC support

[Alexandre Belloni]

Oops, missing line:

On 06/10/2014 at 16:30:08 +0200, Alexandre Belloni wrote :
> > > +static const u32 prescaler_table[] = {
> > > +	120,
> > > +	180,
> > > +	240,
> > > +	360,
> > > +	480,
> > > +	0,
> > > +	0,
> > > +	0,
> > > +	12000,
> > > +	24000,
> > > +	36000,
> > > +	48000,
> > > +	72000,
> > > +	0,
> > > +	0,
> > > +	0, /* Actually 1 but tested separately */
> > > +};
> > 
> > Did I already mention that this was really weird?
> > 
> 
> I'm not sure what you want to do here then. This is simply coming from
> the datasheet. The last one (0xF) being a bypass is actually only existing on 

A20 and later.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCHv7 1/2] pwm: Add Allwinner SoC support

[Thierry Reding]

On Mon, Oct 06, 2014 at 04:30:08PM +0200, Alexandre Belloni wrote:
> On 06/10/2014 at 15:24:05 +0200, Thierry Reding wrote :
> > On Wed, Sep 17, 2014 at 09:53:57PM +0200, Alexandre Belloni wrote:
> > [...]
> > > diff --git a/drivers/pwm/pwm-sunxi.c b/drivers/pwm/pwm-sunxi.c
> > > new file mode 100644
> > > index 000000000000..643f84ea013e
> > > --- /dev/null
> > > +++ b/drivers/pwm/pwm-sunxi.c
> > > @@ -0,0 +1,371 @@
> > > +/*
> > > + * Driver for Allwinner Pulse Width Modulation Controller
> > > + *
> > > + * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > 
> > This is somewhat weird. So you are the copyright holder, not your
> > employer? The email address is somewhat misleading.
> > 
> 
> It has been done on my free, unpaid time so yes...
> 
> Anyway, even my employer could not actually claim to hold copyright on
> what I'm writing as this right is not transferable in France.

Interesting. And yet there's a bunch of Copyright.*Free Electrons in the
kernel. There's also this little mention on Wikipedia:

	"The droit d?auteur or authors' rights, in France, Belgium or
	Germany, grant (subject to some exceptions) the benefice of the
	right to natural persons (the author or his heir(s)) and denies
	it to legal persons (except for collective works, and for
	software), ..."[0]

Which indicates that for software this isn't true. Anyway, what I meant
to say was that it's unusual (at least in my experience) to submit code
authored and copyrighted using a corporate email address. So typically
I've seen people that work on open source in their spare time use a
different email address to separate these "identities".

But I don't mind this all that much, it just jumped out at me.

[0]: http://en.wikipedia.org/wiki/Copyright_law_of_France#Difference_between_copyright_and_droit_d.27auteur

> > > +static const u32 prescaler_table[] = {
> > > +	120,
> > > +	180,
> > > +	240,
> > > +	360,
> > > +	480,
> > > +	0,
> > > +	0,
> > > +	0,
> > > +	12000,
> > > +	24000,
> > > +	36000,
> > > +	48000,
> > > +	72000,
> > > +	0,
> > > +	0,
> > > +	0, /* Actually 1 but tested separately */
> > > +};
> > 
> > Did I already mention that this was really weird?
> > 
> 
> I'm not sure what you want to do here then. This is simply coming from
> the datasheet. The last one (0xF) being a bypass is actually only existing on 

No need to do anything about it. I just think it's really, really weird.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141006/5d748bdd/attachment-0001.sig>

[PATCHv7 2/2] pwm: sunxi: document OF bindings

[Alexandre Belloni]

This is the documentation for the Allwinner SoCs PWM bindings.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/pwm/pwm-sunxi.txt | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-sunxi.txt

diff --git a/Documentation/devicetree/bindings/pwm/pwm-sunxi.txt b/Documentation/devicetree/bindings/pwm/pwm-sunxi.txt
new file mode 100644
index 000000000000..215d33d94870
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-sunxi.txt
@@ -0,0 +1,20 @@
+Allwinner SoC PWM controller
+
+Required properties:
+  - compatible: should be one of:
+    - "allwinner,sun4i-a10-pwm"
+    - "allwinner,sun7i-a20-pwm"
+  - reg: physical base address and length of the controller's registers
+  - #pwm-cells: should be 3. See pwm.txt in this directory for a description of
+    the cells format.
+  - clocks: From common clock binding, handle to the parent clock.
+
+Example:
+
+	pwm: pwm at 01c20e00 {
+		compatible = "allwinner,sun7i-a20-pwm";
+		reg = <0x01c20e00 0xc>;
+		clocks = <&osc24M>;
+		#pwm-cells = <3>;
+		status = "disabled";
+	};
-- 
1.9.1