[PATCH v3 0/4] Add the Allwinner A31/A31s PWM driver

[PATCH v3 0/4] Add the Allwinner A31/A31s PWM driver

[lis8215-Re5JQEeQqe8AvxtiuMwx3w]

From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Main goal is to add support of the sun6i PWM variant.

This is the third version of the sun6i PWM patchset.
Difference between v2 and v3:
 - regmap API used for access to PWM registers.

Thanks to Maxime Ripard for pointing me to the regmap API.

Siarhei Volkau (4):
  pwm: sunxi: Code switched to regmap API instead of iomem.
  pwm: sunxi: Selectable prescaler table for support sun6i.
  pwm: sunxi: Add support the Allwinner A31 PWM.
  ARM: dts: sun6i: Add the PWM block to the A31/A31s.

 .../devicetree/bindings/pwm/pwm-sun4i.txt          |   3 +-
 arch/arm/boot/dts/sun6i-a31.dtsi                   |   8 +
 drivers/pwm/Kconfig                                |   2 +-
 drivers/pwm/pwm-sun4i.c                            | 359 ++++++++++++++++-----
 4 files changed, 294 insertions(+), 78 deletions(-)

--
2.4.11

[PATCH v3 1/4] pwm: sunxi: Code switched to regmap API instead of iomem.

[lis8215-Re5JQEeQqe8AvxtiuMwx3w]

From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

sun6i PWM has different register map in comparison to sun4i compatible
SoCs. But bit map of the registers and behavior are very similar.

This patch introduces a uniform way to access PWM registers.

Signed-off-by: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/pwm/Kconfig     |   2 +-
 drivers/pwm/pwm-sun4i.c | 263 ++++++++++++++++++++++++++++++++++--------------
 2 files changed, 191 insertions(+), 74 deletions(-)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 2d0cfaa..6b4dc1a 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -416,7 +416,7 @@ config PWM_STMPE
 config PWM_SUN4I
 	tristate "Allwinner PWM support"
 	depends on ARCH_SUNXI || COMPILE_TEST
-	depends on HAS_IOMEM && COMMON_CLK
+	depends on REGMAP_MMIO && COMMON_CLK
 	help
 	  Generic PWM framework driver for Allwinner SoCs.

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index b0803f6..7291000 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -9,7 +9,7 @@
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/err.h>
-#include <linux/io.h>
+#include <linux/regmap.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -26,25 +26,56 @@
 #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_PRESCAL_LSB		0
+#define PWM_PRESCAL_MSB		3
+#define PWM_PRESCAL_MASK	GENMASK(PWM_PRESCAL_MSB - PWM_PRESCAL_LSB, 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_RDY_BIT(ch)		(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))
+#define BIT_CH(bit, chan)	((bit) + ((chan) * PWMCH_OFFSET))
+
+#define FIELD_PRESCALER		0
+#define FIELD_POLARITY		1
+#define FIELD_CLK_GATING	2
+#define FIELD_READY		3
+#define NUM_FIELDS		4
+
+#define MAX_CHANNELS		2
+
+#define SUN4I_REGMAP_FIELDS(chan) {\
+	[FIELD_PRESCALER] = \
+		REG_FIELD(PWM_CTRL_REG, \
+			  BIT_CH(PWM_PRESCAL_LSB, chan), \
+			  BIT_CH(PWM_PRESCAL_MSB, chan)), \
+	[FIELD_POLARITY] = \
+		REG_FIELD(PWM_CTRL_REG, \
+			  BIT_CH(PWM_ACT_STATE_BIT, chan), \
+			  BIT_CH(PWM_ACT_STATE_BIT, chan)), \
+	[FIELD_CLK_GATING] = \
+		REG_FIELD(PWM_CTRL_REG, \
+			  BIT_CH(PWM_CLK_GATING_BIT, chan), \
+			  BIT_CH(PWM_CLK_GATING_BIT, chan)), \
+	[FIELD_READY] = \
+		REG_FIELD(PWM_CTRL_REG, \
+			  PWM_RDY_BIT(chan), \
+			  PWM_RDY_BIT(chan)), \
+}
+

 static const u32 prescaler_table[] = {
 	120,
@@ -65,17 +96,26 @@ static const u32 prescaler_table[] = {
 	0, /* Actually 1 but tested separately */
 };

+struct sunxi_pwmch_reg_info {
+	struct reg_field fields[NUM_FIELDS];
+	unsigned int control_reg;
+	unsigned int period_reg;
+	u32 enable_bitmask;
+};
+
 struct sun4i_pwm_data {
 	bool has_prescaler_bypass;
 	bool has_rdy;
 	unsigned int npwm;
+	const struct sunxi_pwmch_reg_info *chan_info;
 };

 struct sun4i_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
-	void __iomem *base;
 	spinlock_t ctrl_lock;
+	struct regmap *regmap;
+	struct regmap_field *fields[MAX_CHANNELS][NUM_FIELDS];
 	const struct sun4i_pwm_data *data;
 };

@@ -84,23 +124,12 @@ static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip)
 	return container_of(chip, struct sun4i_pwm_chip, chip);
 }

-static inline u32 sun4i_pwm_readl(struct sun4i_pwm_chip *chip,
-				  unsigned long offset)
-{
-	return readl(chip->base + offset);
-}
-
-static inline void sun4i_pwm_writel(struct sun4i_pwm_chip *chip,
-				    u32 val, unsigned long offset)
-{
-	writel(val, chip->base + offset);
-}
-
 static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 			    int duty_ns, int period_ns)
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 prd, dty, val, clk_gate;
+	struct regmap_field **chan_fields = sun4i_pwm->fields[pwm->hwpwm];
+	u32 prd, dty, busy, clk_gate;
 	u64 clk_rate, div = 0;
 	unsigned int prescaler = 0;
 	int err;
@@ -152,45 +181,63 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	}

 	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);

-	if (sun4i_pwm->data->has_rdy && (val & PWM_RDY(pwm->hwpwm))) {
-		spin_unlock(&sun4i_pwm->ctrl_lock);
-		clk_disable_unprepare(sun4i_pwm->clk);
-		return -EBUSY;
+	if (sun4i_pwm->data->has_rdy) {
+		err = regmap_field_read(chan_fields[FIELD_READY], &busy);
+		if (err) {
+			dev_err(chip->dev, "failed to get ready bit\n");
+			goto err_cleanup;
+		}
+		if (busy) {
+			err = -EBUSY;
+			goto err_cleanup;
+		}
 	}

-	clk_gate = val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
+	err = regmap_field_read(chan_fields[FIELD_CLK_GATING], &clk_gate);
+	if (err) {
+		dev_err(chip->dev, "failed to get clock_gate bit\n");
+		goto err_cleanup;
+	}
 	if (clk_gate) {
-		val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-		sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
+		err = regmap_field_write(chan_fields[FIELD_CLK_GATING], 0);
+		if (err) {
+			dev_err(chip->dev, "failed to set clock_gate bit\n");
+			goto err_cleanup;
+		}
 	}

-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-	val &= ~BIT_CH(PWM_PRESCAL_MASK, pwm->hwpwm);
-	val |= BIT_CH(prescaler, pwm->hwpwm);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-
-	val = (dty & PWM_DTY_MASK) | PWM_PRD(prd);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm));
-
+	err = regmap_field_write(chan_fields[FIELD_PRESCALER], prescaler);
+	if (err) {
+		dev_err(chip->dev, "failed to set prescaler\n");
+		goto err_cleanup;
+	}
+	err = regmap_write(sun4i_pwm->regmap,
+			   sun4i_pwm->data->chan_info[pwm->hwpwm].period_reg,
+			   (dty & PWM_DTY_MASK) | PWM_PRD(prd));
+	if (err) {
+		dev_err(chip->dev, "failed to set period and duty cycle\n");
+		goto err_cleanup;
+	}
 	if (clk_gate) {
-		val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-		val |= clk_gate;
-		sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
+		err = regmap_field_write(chan_fields[FIELD_CLK_GATING], 1);
+		if (err) {
+			dev_err(chip->dev, "failed to set clock_gate bit\n");
+			goto err_cleanup;
+		}
 	}

+err_cleanup:
 	spin_unlock(&sun4i_pwm->ctrl_lock);
 	clk_disable_unprepare(sun4i_pwm->clk);

-	return 0;
+	return err;
 }

 static int sun4i_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 				  enum pwm_polarity polarity)
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 val;
 	int ret;

 	ret = clk_prepare_enable(sun4i_pwm->clk);
@@ -200,25 +247,21 @@ static int sun4i_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
 	}

 	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_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);
-
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
-
+	ret = regmap_field_write(sun4i_pwm->fields[pwm->hwpwm][FIELD_POLARITY],
+				 polarity == PWM_POLARITY_NORMAL);
+	if (ret)
+		dev_err(chip->dev, "failed to set polarity bit\n");
 	spin_unlock(&sun4i_pwm->ctrl_lock);
 	clk_disable_unprepare(sun4i_pwm->clk);

-	return 0;
+	return ret;
 }

 static int sun4i_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 val;
+	unsigned int ctl_reg;
+	u32 bit_mask;
 	int ret;

 	ret = clk_prepare_enable(sun4i_pwm->clk);
@@ -228,30 +271,51 @@ static int sun4i_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	}

 	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-	val |= BIT_CH(PWM_EN, pwm->hwpwm);
-	val |= BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
+	ctl_reg = sun4i_pwm->data->chan_info[pwm->hwpwm].control_reg;
+	bit_mask = sun4i_pwm->data->chan_info[pwm->hwpwm].enable_bitmask;
+	ret = regmap_update_bits(sun4i_pwm->regmap, ctl_reg,
+				 bit_mask, bit_mask);
+	if (ret)
+		dev_err(chip->dev, "failed to set clock_gate and enable bit\n");
 	spin_unlock(&sun4i_pwm->ctrl_lock);

-	return 0;
+	return ret;
 }

 static void sun4i_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
-	u32 val;
+	unsigned int ctl_reg;
+	u32 bit_mask;
+	int ret;

 	spin_lock(&sun4i_pwm->ctrl_lock);
-	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
-	val &= ~BIT_CH(PWM_EN, pwm->hwpwm);
-	val &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
-	sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
+	ctl_reg = sun4i_pwm->data->chan_info[pwm->hwpwm].control_reg;
+	bit_mask = sun4i_pwm->data->chan_info[pwm->hwpwm].enable_bitmask;
+	ret = regmap_update_bits(sun4i_pwm->regmap, ctl_reg, bit_mask, 0);
+	if (ret)
+		dev_err(chip->dev, "failed to set clock_gate and enable bit\n");
 	spin_unlock(&sun4i_pwm->ctrl_lock);

 	clk_disable_unprepare(sun4i_pwm->clk);
 }

+static const struct sunxi_pwmch_reg_info sun4i_field_info[2] = {
+	{
+		.fields = SUN4I_REGMAP_FIELDS(0),
+		.control_reg =    PWM_CTRL_REG,
+		.period_reg  =    PWM_CH_PRD(0),
+		.enable_bitmask = BIT(PWM_EN_BIT) | BIT(PWM_CLK_GATING_BIT),
+	},
+	{
+		.fields = SUN4I_REGMAP_FIELDS(1),
+		.control_reg =    PWM_CTRL_REG,
+		.period_reg  =    PWM_CH_PRD(1),
+		.enable_bitmask = BIT(PWM_EN_BIT + PWMCH_OFFSET) |
+				  BIT(PWM_CLK_GATING_BIT + PWMCH_OFFSET),
+	},
+};
+
 static const struct pwm_ops sun4i_pwm_ops = {
 	.config = sun4i_pwm_config,
 	.set_polarity = sun4i_pwm_set_polarity,
@@ -264,30 +328,35 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
 	.has_prescaler_bypass = false,
 	.has_rdy = false,
 	.npwm = 2,
+	.chan_info = sun4i_field_info,
 };

 static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 2,
+	.chan_info = sun4i_field_info,
 };

 static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 1,
+	.chan_info = sun4i_field_info,
 };

 static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 2,
+	.chan_info = sun4i_field_info,
 };

 static const struct sun4i_pwm_data sun4i_pwm_data_h3 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 1,
+	.chan_info = sun4i_field_info,
 };

 static const struct of_device_id sun4i_pwm_dt_ids[] = {
@@ -312,10 +381,37 @@ static const struct of_device_id sun4i_pwm_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, sun4i_pwm_dt_ids);

+static const struct regmap_config sunxi_mmio_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int sunxi_alloc_chan_fields(struct device *dev,
+				   struct sun4i_pwm_chip *pwm, int chan)
+{
+	struct regmap_field **fields = pwm->fields[chan];
+	const struct reg_field *info = pwm->data->chan_info[chan].fields;
+	int i;
+
+	for (i = 0; i < NUM_FIELDS; i++) {
+		fields[i] = devm_regmap_field_alloc(dev, pwm->regmap, info[i]);
+		if (IS_ERR(fields[i])) {
+			int err = PTR_ERR(fields[i]);
+
+			fields[i] = NULL;
+			return err;
+		}
+	}
+
+	return 0;
+}
+
 static int sun4i_pwm_probe(struct platform_device *pdev)
 {
 	struct sun4i_pwm_chip *pwm;
 	struct resource *res;
+	void __iomem *base;
 	u32 val;
 	int i, ret;
 	const struct of_device_id *match;
@@ -327,9 +423,16 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 		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);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	pwm->regmap = devm_regmap_init_mmio(&pdev->dev, base,
+					    &sunxi_mmio_regmap_config);
+	if (IS_ERR(pwm->regmap)) {
+		dev_err(&pdev->dev, "failed to initialise regmap\n");
+		return PTR_ERR(pwm->regmap);
+	}

 	pwm->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pwm->clk))
@@ -346,6 +449,14 @@ static int sun4i_pwm_probe(struct platform_device *pdev)

 	spin_lock_init(&pwm->ctrl_lock);

+	for (i = 0; i < pwm->chip.npwm; i++) {
+		ret = sunxi_alloc_chan_fields(&pdev->dev, pwm, i);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to alloc regmap fields\n");
+			return ret;
+		}
+	}
+
 	ret = pwmchip_add(&pwm->chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
@@ -360,11 +471,17 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 		goto clk_error;
 	}

-	val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
-	for (i = 0; i < pwm->chip.npwm; i++)
-		if (!(val & BIT_CH(PWM_ACT_STATE, i)))
+	for (i = 0; i < pwm->chip.npwm; i++) {
+		ret = regmap_field_read(pwm->fields[i][FIELD_POLARITY], &val);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to get polarity bit\n");
+			goto clk_error;
+		}
+		if (!val)
 			pwm_set_polarity(&pwm->chip.pwms[i],
 					 PWM_POLARITY_INVERSED);
+	}
+
 	clk_disable_unprepare(pwm->clk);

 	return 0;
--
2.4.11

Re: [PATCH v3 1/4] pwm: sunxi: Code switched to regmap API instead of iomem.

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3970 bytes --]

Hi, 

On Wed, Feb 15, 2017 at 11:06:04PM +0300, lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> sun6i PWM has different register map in comparison to sun4i compatible
> SoCs. But bit map of the registers and behavior are very similar.
> 
> This patch introduces a uniform way to access PWM registers.
> 
> Signed-off-by: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

In order to be easier to review (and bisect if needed), could you
split that patch into one to convert to the regmap API, without any
change but to replace the sun4i_pwm_readl/sun4i_pwm_writel calls by
their regmap equivalent, and then convert to the regmap fields the
registers that need to?

> ---
>  drivers/pwm/Kconfig     |   2 +-
>  drivers/pwm/pwm-sun4i.c | 263 ++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 191 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 2d0cfaa..6b4dc1a 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -416,7 +416,7 @@ config PWM_STMPE
>  config PWM_SUN4I
>  	tristate "Allwinner PWM support"
>  	depends on ARCH_SUNXI || COMPILE_TEST
> -	depends on HAS_IOMEM && COMMON_CLK
> +	depends on REGMAP_MMIO && COMMON_CLK
>  	help
>  	  Generic PWM framework driver for Allwinner SoCs.
> 
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index b0803f6..7291000 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -9,7 +9,7 @@
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
> -#include <linux/io.h>
> +#include <linux/regmap.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> @@ -26,25 +26,56 @@
>  #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_PRESCAL_LSB		0
> +#define PWM_PRESCAL_MSB		3
> +#define PWM_PRESCAL_MASK	GENMASK(PWM_PRESCAL_MSB - PWM_PRESCAL_LSB, 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_RDY_BIT(ch)		(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))
> +#define BIT_CH(bit, chan)	((bit) + ((chan) * PWMCH_OFFSET))
> +
> +#define FIELD_PRESCALER		0
> +#define FIELD_POLARITY		1
> +#define FIELD_CLK_GATING	2
> +#define FIELD_READY		3
> +#define NUM_FIELDS		4
> +
> +#define MAX_CHANNELS		2
> +
> +#define SUN4I_REGMAP_FIELDS(chan) {\
> +	[FIELD_PRESCALER] = \
> +		REG_FIELD(PWM_CTRL_REG, \
> +			  BIT_CH(PWM_PRESCAL_LSB, chan), \
> +			  BIT_CH(PWM_PRESCAL_MSB, chan)), \
> +	[FIELD_POLARITY] = \
> +		REG_FIELD(PWM_CTRL_REG, \
> +			  BIT_CH(PWM_ACT_STATE_BIT, chan), \
> +			  BIT_CH(PWM_ACT_STATE_BIT, chan)), \
> +	[FIELD_CLK_GATING] = \
> +		REG_FIELD(PWM_CTRL_REG, \
> +			  BIT_CH(PWM_CLK_GATING_BIT, chan), \
> +			  BIT_CH(PWM_CLK_GATING_BIT, chan)), \
> +	[FIELD_READY] = \
> +		REG_FIELD(PWM_CTRL_REG, \
> +			  PWM_RDY_BIT(chan), \
> +			  PWM_RDY_BIT(chan)), \
> +}
> +

This is not correct, unfortunately. If someone calls that macro with
chan++ or chan--, it will be modified four times instead of one as the
caller would expect.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 1/4] pwm: sunxi: Code switched to regmap API instead of iomem.

[Siarhei Volkau]

Hello,

> In order to be easier to review (and bisect if needed), could you
> split that patch into one to convert to the regmap API, without any
> change but to replace the sun4i_pwm_readl/sun4i_pwm_writel calls by
> their regmap equivalent, and then convert to the regmap fields the
> registers that need to?

Looks reasonable, will be done shortly.

>> +#define SUN4I_REGMAP_FIELDS(chan) {\
>> +     [FIELD_PRESCALER] = \
>> +             REG_FIELD(PWM_CTRL_REG, \
>> +                       BIT_CH(PWM_PRESCAL_LSB, chan), \
>> +                       BIT_CH(PWM_PRESCAL_MSB, chan)), \
>> +     [FIELD_POLARITY] = \
>> +             REG_FIELD(PWM_CTRL_REG, \
>> +                       BIT_CH(PWM_ACT_STATE_BIT, chan), \
>> +                       BIT_CH(PWM_ACT_STATE_BIT, chan)), \
>> +     [FIELD_CLK_GATING] = \
>> +             REG_FIELD(PWM_CTRL_REG, \
>> +                       BIT_CH(PWM_CLK_GATING_BIT, chan), \
>> +                       BIT_CH(PWM_CLK_GATING_BIT, chan)), \
>> +     [FIELD_READY] = \
>> +             REG_FIELD(PWM_CTRL_REG, \
>> +                       PWM_RDY_BIT(chan), \
>> +                       PWM_RDY_BIT(chan)), \
>> +}
>> +
>
> This is not correct, unfortunately. If someone calls that macro with
> chan++ or chan--, it will be modified four times instead of one as the
> caller would expect.

Ok, i will refactor that.
I think you should know about existence of unsafe macros (like MIN
MAX or ABS) into existing linux kernel tree, this macros can be easily
refactored into the inline functions. Please tell me your point of view
on that things.

Thanks,
Siarhei.

Re: [PATCH v3 1/4] pwm: sunxi: Code switched to regmap API instead of iomem.

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

Hi,

On Tue, Feb 21, 2017 at 09:47:43AM +0300, Siarhei Volkau wrote:
> >> +#define SUN4I_REGMAP_FIELDS(chan) {\
> >> +     [FIELD_PRESCALER] = \
> >> +             REG_FIELD(PWM_CTRL_REG, \
> >> +                       BIT_CH(PWM_PRESCAL_LSB, chan), \
> >> +                       BIT_CH(PWM_PRESCAL_MSB, chan)), \
> >> +     [FIELD_POLARITY] = \
> >> +             REG_FIELD(PWM_CTRL_REG, \
> >> +                       BIT_CH(PWM_ACT_STATE_BIT, chan), \
> >> +                       BIT_CH(PWM_ACT_STATE_BIT, chan)), \
> >> +     [FIELD_CLK_GATING] = \
> >> +             REG_FIELD(PWM_CTRL_REG, \
> >> +                       BIT_CH(PWM_CLK_GATING_BIT, chan), \
> >> +                       BIT_CH(PWM_CLK_GATING_BIT, chan)), \
> >> +     [FIELD_READY] = \
> >> +             REG_FIELD(PWM_CTRL_REG, \
> >> +                       PWM_RDY_BIT(chan), \
> >> +                       PWM_RDY_BIT(chan)), \
> >> +}
> >> +
> >
> > This is not correct, unfortunately. If someone calls that macro with
> > chan++ or chan--, it will be modified four times instead of one as the
> > caller would expect.
> 
> Ok, i will refactor that.
> I think you should know about existence of unsafe macros (like MIN
> MAX or ABS) into existing linux kernel tree, this macros can be easily
> refactored into the inline functions. Please tell me your point of view
> on that things.

This might be bad, but we also have functions for those, which should
behave properly. Anyway, I don't really expect you to fix the whole
kernel before taking that patch in :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v3 2/4] pwm: sunxi: Selectable prescaler table for support sun6i.

[lis8215-Re5JQEeQqe8AvxtiuMwx3w]

From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch not introduce new features, just prepare code for adding
sun6i PWM driver in next commit.

A31 SoC have a different set of prescalers than sun4i
compatible ASoCs, but its position and count in the control
register are the same.

This patch make the table of prescalers customizable.

Signed-off-by: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/pwm/pwm-sun4i.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 7291000..b4706af 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -77,7 +77,7 @@
 }


-static const u32 prescaler_table[] = {
+static const u32 sun4i_prescaler_table[] = {
 	120,
 	180,
 	240,
@@ -107,6 +107,7 @@ struct sun4i_pwm_data {
 	bool has_prescaler_bypass;
 	bool has_rdy;
 	unsigned int npwm;
+	const u32 *prescaler_table;
 	const struct sunxi_pwmch_reg_info *chan_info;
 };

@@ -129,6 +130,7 @@ static int sun4i_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
 	struct regmap_field **chan_fields = sun4i_pwm->fields[pwm->hwpwm];
+	const u32 *prescaler_table = sun4i_pwm->data->prescaler_table;
 	u32 prd, dty, busy, clk_gate;
 	u64 clk_rate, div = 0;
 	unsigned int prescaler = 0;
@@ -328,6 +330,7 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a10 = {
 	.has_prescaler_bypass = false,
 	.has_rdy = false,
 	.npwm = 2,
+	.prescaler_table = sun4i_prescaler_table,
 	.chan_info = sun4i_field_info,
 };

@@ -335,6 +338,7 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a10s = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 2,
+	.prescaler_table = sun4i_prescaler_table,
 	.chan_info = sun4i_field_info,
 };

@@ -342,6 +346,7 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 1,
+	.prescaler_table = sun4i_prescaler_table,
 	.chan_info = sun4i_field_info,
 };

@@ -349,6 +354,7 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 2,
+	.prescaler_table = sun4i_prescaler_table,
 	.chan_info = sun4i_field_info,
 };

@@ -356,6 +362,7 @@ static const struct sun4i_pwm_data sun4i_pwm_data_h3 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
 	.npwm = 1,
+	.prescaler_table = sun4i_prescaler_table,
 	.chan_info = sun4i_field_info,
 };

--
2.4.11

Re: [PATCH v3 2/4] pwm: sunxi: Selectable prescaler table for support sun6i.

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Wed, Feb 15, 2017 at 11:06:05PM +0300, lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This patch not introduce new features, just prepare code for adding
> sun6i PWM driver in next commit.
> 
> A31 SoC have a different set of prescalers than sun4i
> compatible ASoCs, but its position and count in the control
> register are the same.
> 
> This patch make the table of prescalers customizable.
> 
> Signed-off-by: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v3 3/4] pwm: sunxi: Add support the Allwinner A31 PWM.

[lis8215-Re5JQEeQqe8AvxtiuMwx3w]

From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch introduce the sun6i PWM driver itself:
 - sun6i channels configuration,
 - sun6i prescaler table,
 - DT bindings for A31 SoC,
 - documentation update,
 - module description and author update.

Signed-off-by: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/pwm/pwm-sun4i.txt          |  3 +-
 drivers/pwm/pwm-sun4i.c                            | 89 +++++++++++++++++++++-
 2 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
index f1cbeef..109b997 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-sun4i.txt
@@ -1,10 +1,11 @@
-Allwinner sun4i and sun7i SoC PWM controller
+Allwinner sunxi SoC PWM controller

 Required properties:
   - compatible: should be one of:
     - "allwinner,sun4i-a10-pwm"
     - "allwinner,sun5i-a10s-pwm"
     - "allwinner,sun5i-a13-pwm"
+    - "allwinner,sun6i-a31-pwm"
     - "allwinner,sun7i-a20-pwm"
     - "allwinner,sun8i-h3-pwm"
   - reg: physical base address and length of the controller's registers
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index b4706af..8c96251 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -1,7 +1,8 @@
 /*
- * Driver for Allwinner sun4i Pulse Width Modulation Controller
+ * Driver for Allwinner sunxi Pulse Width Modulation Controller
  *
  * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ * Copyright (C) 2017 Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  *
  * Licensed under GPLv2.
  */
@@ -27,6 +28,12 @@

 #define PWMCH_OFFSET		15

+#define SUN6I_PWM_CTL_OFFSET	0x0
+#define SUN6I_PWM_PRD_OFFSET	0x4
+#define SUN6I_PWMCH_OFFSET(ch)	(0x10 * (ch))
+#define SUN6I_PWM_CTL_REG(ch)	(SUN6I_PWMCH_OFFSET(ch) + SUN6I_PWM_CTL_OFFSET)
+#define SUN6I_PWM_PRD_REG(ch)	(SUN6I_PWMCH_OFFSET(ch) + SUN6I_PWM_PRD_OFFSET)
+
 #define PWM_PRESCAL_LSB		0
 #define PWM_PRESCAL_MSB		3
 #define PWM_PRESCAL_MASK	GENMASK(PWM_PRESCAL_MSB - PWM_PRESCAL_LSB, 0)
@@ -55,7 +62,7 @@
 #define FIELD_READY		3
 #define NUM_FIELDS		4

-#define MAX_CHANNELS		2
+#define MAX_CHANNELS		4

 #define SUN4I_REGMAP_FIELDS(chan) {\
 	[FIELD_PRESCALER] = \
@@ -76,6 +83,24 @@
 			  PWM_RDY_BIT(chan)), \
 }

+#define SUN6I_REGMAP_FIELDS(chan) {\
+	[FIELD_PRESCALER] = \
+		REG_FIELD(SUN6I_PWM_CTL_REG(chan), \
+			  PWM_PRESCAL_LSB, \
+			  PWM_PRESCAL_MSB), \
+	[FIELD_POLARITY] = \
+		REG_FIELD(SUN6I_PWM_CTL_REG(chan), \
+			  PWM_ACT_STATE_BIT, \
+			  PWM_ACT_STATE_BIT), \
+	[FIELD_CLK_GATING] = \
+		REG_FIELD(SUN6I_PWM_CTL_REG(chan), \
+			  PWM_CLK_GATING_BIT, \
+			  PWM_CLK_GATING_BIT), \
+	[FIELD_READY] = \
+		REG_FIELD(SUN6I_PWM_CTL_REG(chan), \
+			  PWM_RDY_BASE, \
+			  PWM_RDY_BASE), \
+}

 static const u32 sun4i_prescaler_table[] = {
 	120,
@@ -96,6 +121,25 @@ static const u32 sun4i_prescaler_table[] = {
 	0, /* Actually 1 but tested separately */
 };

+static const u32 sun6i_prescaler_table[] = {
+	1,
+	2,
+	4,
+	8,
+	16,
+	32,
+	64,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0, /* Actually 1 but tested separately */
+};
+
 struct sunxi_pwmch_reg_info {
 	struct reg_field fields[NUM_FIELDS];
 	unsigned int control_reg;
@@ -318,6 +362,33 @@ static const struct sunxi_pwmch_reg_info sun4i_field_info[2] = {
 	},
 };

+static const struct sunxi_pwmch_reg_info sun6i_field_info[4] = {
+	{
+		.fields = SUN6I_REGMAP_FIELDS(0),
+		.control_reg =    SUN6I_PWM_CTL_REG(0),
+		.period_reg  =    SUN6I_PWM_PRD_REG(0),
+		.enable_bitmask = BIT(PWM_EN_BIT) | BIT(PWM_CLK_GATING_BIT),
+	},
+	{
+		.fields = SUN6I_REGMAP_FIELDS(1),
+		.control_reg =    SUN6I_PWM_CTL_REG(1),
+		.period_reg  =    SUN6I_PWM_PRD_REG(1),
+		.enable_bitmask = BIT(PWM_EN_BIT) | BIT(PWM_CLK_GATING_BIT),
+	},
+	{
+		.fields = SUN6I_REGMAP_FIELDS(2),
+		.control_reg =    SUN6I_PWM_CTL_REG(2),
+		.period_reg  =    SUN6I_PWM_PRD_REG(2),
+		.enable_bitmask = BIT(PWM_EN_BIT) | BIT(PWM_CLK_GATING_BIT),
+	},
+	{
+		.fields = SUN6I_REGMAP_FIELDS(3),
+		.control_reg =    SUN6I_PWM_CTL_REG(3),
+		.period_reg  =    SUN6I_PWM_PRD_REG(3),
+		.enable_bitmask = BIT(PWM_EN_BIT) | BIT(PWM_CLK_GATING_BIT),
+	},
+};
+
 static const struct pwm_ops sun4i_pwm_ops = {
 	.config = sun4i_pwm_config,
 	.set_polarity = sun4i_pwm_set_polarity,
@@ -350,6 +421,14 @@ static const struct sun4i_pwm_data sun4i_pwm_data_a13 = {
 	.chan_info = sun4i_field_info,
 };

+static const struct sun4i_pwm_data sun6i_pwm_data_a31 = {
+	.has_prescaler_bypass = false,
+	.has_rdy = true,
+	.npwm = 4,
+	.prescaler_table = sun6i_prescaler_table,
+	.chan_info = sun6i_field_info,
+};
+
 static const struct sun4i_pwm_data sun4i_pwm_data_a20 = {
 	.has_prescaler_bypass = true,
 	.has_rdy = true,
@@ -377,6 +456,9 @@ static const struct of_device_id sun4i_pwm_dt_ids[] = {
 		.compatible = "allwinner,sun5i-a13-pwm",
 		.data = &sun4i_pwm_data_a13,
 	}, {
+		.compatible = "allwinner,sun6i-a31-pwm",
+		.data = &sun6i_pwm_data_a31,
+	}, {
 		.compatible = "allwinner,sun7i-a20-pwm",
 		.data = &sun4i_pwm_data_a20,
 	}, {
@@ -517,5 +599,6 @@ module_platform_driver(sun4i_pwm_driver);

 MODULE_ALIAS("platform:sun4i-pwm");
 MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
-MODULE_DESCRIPTION("Allwinner sun4i PWM driver");
+MODULE_AUTHOR("Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("Allwinner sunxi PWM driver");
 MODULE_LICENSE("GPL v2");
--
2.4.11

[PATCH v3 4/4] ARM: dts: sun6i: Add the PWM block to the A31/A31s.

[lis8215-Re5JQEeQqe8AvxtiuMwx3w]

From: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Signed-off-by: Siarhei Volkau <lis8215-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index ee1eb6d..fcba129 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -627,6 +627,14 @@
 			status = "disabled";
 		};
 
+		pwm: pwm@01c21400 {
+			compatible = "allwinner,sun6i-a31-pwm";
+			reg = <0x01c21400 0x400>;
+			clocks = <&osc24M>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		lradc: lradc@01c22800 {
 			compatible = "allwinner,sun4i-a10-lradc-keys";
 			reg = <0x01c22800 0x100>;
-- 
2.4.11