diff mbox series

[v7,1/3] gpio: mvebu: add pwm support for Armada 8K/7K

Message ID ba8d5d482a98690140e02c3a35506490e0c6ecb4.1610364681.git.baruch@tkos.co.il
State Accepted
Commit 85b7d8abfec70ae820ddfea493f93b0af7e50b51
Headers show
Series gpio: mvebu: Armada 8K/7K PWM support | expand

Commit Message

Baruch Siach Jan. 11, 2021, 11:46 a.m. UTC
Use the marvell,pwm-offset DT property to store the location of PWM
signal duration registers.

Since we have more than two GPIO chips per system, we can't use the
alias id to differentiate between them. Use the offset value for that.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/gpio/gpio-mvebu.c | 101 +++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 33 deletions(-)

Comments

Bartosz Golaszewski Jan. 22, 2021, 12:58 p.m. UTC | #1
On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
>

> Use the marvell,pwm-offset DT property to store the location of PWM

> signal duration registers.

>

> Since we have more than two GPIO chips per system, we can't use the

> alias id to differentiate between them. Use the offset value for that.

>

> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

> ---

>  drivers/gpio/gpio-mvebu.c | 101 +++++++++++++++++++++++++-------------

>  1 file changed, 68 insertions(+), 33 deletions(-)

>

> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c

> index 4261e3b22b4e..6bd45c59056a 100644

> --- a/drivers/gpio/gpio-mvebu.c

> +++ b/drivers/gpio/gpio-mvebu.c

> @@ -70,7 +70,12 @@

>   */

>  #define PWM_BLINK_ON_DURATION_OFF      0x0

>  #define PWM_BLINK_OFF_DURATION_OFF     0x4

> +#define PWM_BLINK_COUNTER_B_OFF                0x8

>

> +/* Armada 8k variant gpios register offsets */

> +#define AP80X_GPIO0_OFF_A8K            0x1040

> +#define CP11X_GPIO0_OFF_A8K            0x100

> +#define CP11X_GPIO1_OFF_A8K            0x140

>

>  /* The MV78200 has per-CPU registers for edge mask and level mask */

>  #define GPIO_EDGE_MASK_MV78200_OFF(cpu)          ((cpu) ? 0x30 : 0x18)

> @@ -93,6 +98,7 @@

>

>  struct mvebu_pwm {

>         struct regmap           *regs;

> +       u32                      offset;

>         unsigned long            clk_rate;

>         struct gpio_desc        *gpiod;

>         struct pwm_chip          chip;

> @@ -283,12 +289,12 @@ mvebu_gpio_write_level_mask(struct mvebu_gpio_chip *mvchip, u32 val)

>   */

>  static unsigned int mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *mvpwm)

>  {

> -       return PWM_BLINK_ON_DURATION_OFF;

> +       return mvpwm->offset + PWM_BLINK_ON_DURATION_OFF;

>  }

>

>  static unsigned int mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)

>  {

> -       return PWM_BLINK_OFF_DURATION_OFF;

> +       return mvpwm->offset + PWM_BLINK_OFF_DURATION_OFF;

>  }

>

>  /*

> @@ -775,51 +781,80 @@ static int mvebu_pwm_probe(struct platform_device *pdev,

>         struct device *dev = &pdev->dev;

>         struct mvebu_pwm *mvpwm;

>         void __iomem *base;

> +       u32 offset;

>         u32 set;

>

> -       if (!of_device_is_compatible(mvchip->chip.of_node,

> -                                    "marvell,armada-370-gpio"))

> -               return 0;

> -

> -       /*

> -        * There are only two sets of PWM configuration registers for

> -        * all the GPIO lines on those SoCs which this driver reserves

> -        * for the first two GPIO chips. So if the resource is missing

> -        * we can't treat it as an error.

> -        */

> -       if (!platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm"))

> +       if (of_device_is_compatible(mvchip->chip.of_node,

> +                                   "marvell,armada-370-gpio")) {

> +               /*

> +                * There are only two sets of PWM configuration registers for

> +                * all the GPIO lines on those SoCs which this driver reserves

> +                * for the first two GPIO chips. So if the resource is missing

> +                * we can't treat it as an error.

> +                */

> +               if (!platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm"))

> +                       return 0;

> +               offset = 0;

> +       } else if (mvchip->soc_variant == MVEBU_GPIO_SOC_VARIANT_A8K) {

> +               int ret = of_property_read_u32(dev->of_node,

> +                                              "marvell,pwm-offset", &offset);

> +               if (ret < 0)

> +                       return 0;

> +       } else {

>                 return 0;

> +       }

>

>         if (IS_ERR(mvchip->clk))

>                 return PTR_ERR(mvchip->clk);

>

> -       /*

> -        * Use set A for lines of GPIO chip with id 0, B for GPIO chip

> -        * with id 1. Don't allow further GPIO chips to be used for PWM.

> -        */

> -       if (id == 0)

> -               set = 0;

> -       else if (id == 1)

> -               set = U32_MAX;

> -       else

> -               return -EINVAL;

> -       regmap_write(mvchip->regs,

> -                    GPIO_BLINK_CNT_SELECT_OFF + mvchip->offset, set);

> -

>         mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);

>         if (!mvpwm)

>                 return -ENOMEM;

>         mvchip->mvpwm = mvpwm;

>         mvpwm->mvchip = mvchip;

> +       mvpwm->offset = offset;

> +

> +       if (mvchip->soc_variant == MVEBU_GPIO_SOC_VARIANT_A8K) {

> +               mvpwm->regs = mvchip->regs;

> +

> +               switch (mvchip->offset) {

> +               case AP80X_GPIO0_OFF_A8K:

> +               case CP11X_GPIO0_OFF_A8K:

> +                       /* Blink counter A */

> +                       set = 0;

> +                       break;

> +               case CP11X_GPIO1_OFF_A8K:

> +                       /* Blink counter B */

> +                       set = U32_MAX;

> +                       mvpwm->offset += PWM_BLINK_COUNTER_B_OFF;

> +                       break;

> +               default:

> +                       return -EINVAL;

> +               }

> +       } else {

> +               base = devm_platform_ioremap_resource_byname(pdev, "pwm");

> +               if (IS_ERR(base))

> +                       return PTR_ERR(base);

>

> -       base = devm_platform_ioremap_resource_byname(pdev, "pwm");

> -       if (IS_ERR(base))

> -               return PTR_ERR(base);

> +               mvpwm->regs = devm_regmap_init_mmio(&pdev->dev, base,

> +                                                   &mvebu_gpio_regmap_config);

> +               if (IS_ERR(mvpwm->regs))

> +                       return PTR_ERR(mvpwm->regs);

>

> -       mvpwm->regs = devm_regmap_init_mmio(&pdev->dev, base,

> -                                           &mvebu_gpio_regmap_config);

> -       if (IS_ERR(mvpwm->regs))

> -               return PTR_ERR(mvpwm->regs);

> +               /*

> +                * Use set A for lines of GPIO chip with id 0, B for GPIO chip

> +                * with id 1. Don't allow further GPIO chips to be used for PWM.

> +                */

> +               if (id == 0)

> +                       set = 0;

> +               else if (id == 1)

> +                       set = U32_MAX;

> +               else

> +                       return -EINVAL;

> +       }

> +

> +       regmap_write(mvchip->regs,

> +                    GPIO_BLINK_CNT_SELECT_OFF + mvchip->offset, set);


Hi Baruch!

Can you confirm that this line is on purpose and that it should be
executed even for chips that use a separate regmap for PWM?

Bartosz

>

>         mvpwm->clk_rate = clk_get_rate(mvchip->clk);

>         if (!mvpwm->clk_rate) {

> --

> 2.29.2

>
Baruch Siach Jan. 24, 2021, 6:17 a.m. UTC | #2
Hi Bartosz,

Thanks for you review.

On Fri, Jan 22 2021, Bartosz Golaszewski wrote:
> On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:

>> Use the marvell,pwm-offset DT property to store the location of PWM

>> signal duration registers.

>>

>> Since we have more than two GPIO chips per system, we can't use the

>> alias id to differentiate between them. Use the offset value for that.

>>

>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>


[...]

>> +       regmap_write(mvchip->regs,

>> +                    GPIO_BLINK_CNT_SELECT_OFF + mvchip->offset, set);

>

> Can you confirm that this line is on purpose and that it should be

> executed even for chips that use a separate regmap for PWM?


Yes. The blink counter selection register is at the same offset is all
chips that support the GPIO blink feature. Only the on/off registers
offset is different.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 4261e3b22b4e..6bd45c59056a 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -70,7 +70,12 @@ 
  */
 #define PWM_BLINK_ON_DURATION_OFF	0x0
 #define PWM_BLINK_OFF_DURATION_OFF	0x4
+#define PWM_BLINK_COUNTER_B_OFF		0x8
 
+/* Armada 8k variant gpios register offsets */
+#define AP80X_GPIO0_OFF_A8K		0x1040
+#define CP11X_GPIO0_OFF_A8K		0x100
+#define CP11X_GPIO1_OFF_A8K		0x140
 
 /* The MV78200 has per-CPU registers for edge mask and level mask */
 #define GPIO_EDGE_MASK_MV78200_OFF(cpu)	  ((cpu) ? 0x30 : 0x18)
@@ -93,6 +98,7 @@ 
 
 struct mvebu_pwm {
 	struct regmap		*regs;
+	u32			 offset;
 	unsigned long		 clk_rate;
 	struct gpio_desc	*gpiod;
 	struct pwm_chip		 chip;
@@ -283,12 +289,12 @@  mvebu_gpio_write_level_mask(struct mvebu_gpio_chip *mvchip, u32 val)
  */
 static unsigned int mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *mvpwm)
 {
-	return PWM_BLINK_ON_DURATION_OFF;
+	return mvpwm->offset + PWM_BLINK_ON_DURATION_OFF;
 }
 
 static unsigned int mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)
 {
-	return PWM_BLINK_OFF_DURATION_OFF;
+	return mvpwm->offset + PWM_BLINK_OFF_DURATION_OFF;
 }
 
 /*
@@ -775,51 +781,80 @@  static int mvebu_pwm_probe(struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	struct mvebu_pwm *mvpwm;
 	void __iomem *base;
+	u32 offset;
 	u32 set;
 
-	if (!of_device_is_compatible(mvchip->chip.of_node,
-				     "marvell,armada-370-gpio"))
-		return 0;
-
-	/*
-	 * There are only two sets of PWM configuration registers for
-	 * all the GPIO lines on those SoCs which this driver reserves
-	 * for the first two GPIO chips. So if the resource is missing
-	 * we can't treat it as an error.
-	 */
-	if (!platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm"))
+	if (of_device_is_compatible(mvchip->chip.of_node,
+				    "marvell,armada-370-gpio")) {
+		/*
+		 * There are only two sets of PWM configuration registers for
+		 * all the GPIO lines on those SoCs which this driver reserves
+		 * for the first two GPIO chips. So if the resource is missing
+		 * we can't treat it as an error.
+		 */
+		if (!platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm"))
+			return 0;
+		offset = 0;
+	} else if (mvchip->soc_variant == MVEBU_GPIO_SOC_VARIANT_A8K) {
+		int ret = of_property_read_u32(dev->of_node,
+					       "marvell,pwm-offset", &offset);
+		if (ret < 0)
+			return 0;
+	} else {
 		return 0;
+	}
 
 	if (IS_ERR(mvchip->clk))
 		return PTR_ERR(mvchip->clk);
 
-	/*
-	 * Use set A for lines of GPIO chip with id 0, B for GPIO chip
-	 * with id 1. Don't allow further GPIO chips to be used for PWM.
-	 */
-	if (id == 0)
-		set = 0;
-	else if (id == 1)
-		set = U32_MAX;
-	else
-		return -EINVAL;
-	regmap_write(mvchip->regs,
-		     GPIO_BLINK_CNT_SELECT_OFF + mvchip->offset, set);
-
 	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
 	if (!mvpwm)
 		return -ENOMEM;
 	mvchip->mvpwm = mvpwm;
 	mvpwm->mvchip = mvchip;
+	mvpwm->offset = offset;
+
+	if (mvchip->soc_variant == MVEBU_GPIO_SOC_VARIANT_A8K) {
+		mvpwm->regs = mvchip->regs;
+
+		switch (mvchip->offset) {
+		case AP80X_GPIO0_OFF_A8K:
+		case CP11X_GPIO0_OFF_A8K:
+			/* Blink counter A */
+			set = 0;
+			break;
+		case CP11X_GPIO1_OFF_A8K:
+			/* Blink counter B */
+			set = U32_MAX;
+			mvpwm->offset += PWM_BLINK_COUNTER_B_OFF;
+			break;
+		default:
+			return -EINVAL;
+		}
+	} else {
+		base = devm_platform_ioremap_resource_byname(pdev, "pwm");
+		if (IS_ERR(base))
+			return PTR_ERR(base);
 
-	base = devm_platform_ioremap_resource_byname(pdev, "pwm");
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+		mvpwm->regs = devm_regmap_init_mmio(&pdev->dev, base,
+						    &mvebu_gpio_regmap_config);
+		if (IS_ERR(mvpwm->regs))
+			return PTR_ERR(mvpwm->regs);
 
-	mvpwm->regs = devm_regmap_init_mmio(&pdev->dev, base,
-					    &mvebu_gpio_regmap_config);
-	if (IS_ERR(mvpwm->regs))
-		return PTR_ERR(mvpwm->regs);
+		/*
+		 * Use set A for lines of GPIO chip with id 0, B for GPIO chip
+		 * with id 1. Don't allow further GPIO chips to be used for PWM.
+		 */
+		if (id == 0)
+			set = 0;
+		else if (id == 1)
+			set = U32_MAX;
+		else
+			return -EINVAL;
+	}
+
+	regmap_write(mvchip->regs,
+		     GPIO_BLINK_CNT_SELECT_OFF + mvchip->offset, set);
 
 	mvpwm->clk_rate = clk_get_rate(mvchip->clk);
 	if (!mvpwm->clk_rate) {