Skip to contentSkip to conversation

gpio: mvebu: Armada 8K/7K PWM support

PATCH Superseded Baruch Siach 5 participants 11 messages Dec 10, 2020 – Jan 4, 2021

Subsystems (4)

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v4 cover gpio: mvebu: Armada 8K/7K PWM support

This series makes two changes to v3:

* Remove patches that are in LinusW linux-gpio for-next and fixes

* Rename the 'pwm-offset' property to 'marvell,pwm-offset' as suggested by Rob Herring

The original cover letter follows (with DT property name updated).

The gpio-mvebu driver supports the PWM functionality of the GPIO block for earlier Armada variants like XP, 370 and 38x. This series extends support to newer Armada variants that use CP11x and AP80x, like Armada 8K and 7K.

This series adds adds the 'marvell,pwm-offset' property to DT binding. 'marvell,pwm-offset' points to the base of A/B counter registers that determine the PWM period and duty cycle.

The existing PWM DT binding reflects an arbitrary decision to allocate the A counter to the first GPIO block, and B counter to the other one. In attempt to provide better future flexibility, the new 'marvell,pwm-offset' property always points to the base address of both A/B counters. The driver code still allocates the counters in the same way, but this might change in the future with no change to the DT.

Tested AP806 and CP110 (both) on Armada 8040 based system.

Baruch Siach (3):
gpio: mvebu: add pwm support for Armada 8K/7K
arm64: dts: armada: add pwm offsets for ap/cp gpios
dt-bindings: ap806: document marvell,gpio pwm-offset property

 .../arm/marvell/ap80x-system-controller.txt   |   8 ++
 arch/arm64/boot/dts/marvell/armada-ap80x.dtsi |   3 +
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi |  10 ++
 drivers/gpio/gpio-mvebu.c                     | 101 ++++++++++++------
 4 files changed, 89 insertions(+), 33 deletions(-)
Signature · 1 line
2.29.2
#1 lore raw

PATCH v4 1/3 gpio: mvebu: add pwm support for Armada 8K/7K

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.

---
v3: Split mvebu_pwm_probe() move out of this patch into a separate fix
(Andrew Lunn)

---
 drivers/gpio/gpio-mvebu.c | 101 +++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 33 deletions(-)
drivers/gpio/gpio-mvebu.c +68 -33
index 672681a976f5..7019ad79ae37 100644
@@ -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;
 }
 
 /*
@@ -781,51 +787,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) {
Signature · 1 line
2.29.2
#2 lore raw

Re: [PATCH v4 1/3] gpio: mvebu: add pwm support for Armada 8K/7K

Quoted 24 lines · Baruch Siach
drivers/gpio/gpio-mvebu.c +7 -11
@@ -781,51 +787,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

drivers/gpio/gpio-mvebu.c static int mvebu_pwm_probe(struct platform_device *pdev, · line 808

Quoted 10 lines
drivers/gpio/gpio-mvebu.c +10 -0
+		 * 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;

The reason my patches were rejected was because I was trying to keep compatibility with the existing DTs w.r.t the clock - and Uwe didn't like that.

I notice that you keep compatibility by detecting the presence or absence of the marvell,pwm-offset property which achieves the same goal.

Also, you are missing fixing a bug in the PWM register calculations for get_state().

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
#3 lore raw

PATCH v4 2/3 arm64: dts: armada: add pwm offsets for ap/cp gpios

In reply to Baruch Siach

The 'marvell,pwm-offset' property of both GPIO blocks (per CP component) point to the same counter registers offset. The driver will decide how to use counters A/B.

This is different from the convention of pwm on earlier Armada series (370/38x). On those systems the assignment of A/B counters to GPIO blocks is coded in both DT and the driver. The actual behaviour of the current driver on Armada 8K/7K is the same as earlier systems.

Add also clock properties for base pwm frequency reference.

---
 arch/arm64/boot/dts/marvell/armada-ap80x.dtsi |  3 +++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 10 ++++++++++
 2 files changed, 13 insertions(+)
arch/arm64/boot/dts/marvell/armada-ap80x.dtsi +3 -0
index 12e477f1aeb9..6614472100c2 100644
@@ -281,6 +281,9 @@ ap_gpio: gpio@1040 {
 					gpio-controller;
 					#gpio-cells = <2>;
 					gpio-ranges = <&ap_pinctrl 0 0 20>;
+					marvell,pwm-offset = <0x10c0>;
+					#pwm-cells = <2>;
+					clocks = <&ap_clk 3>;
 				};
 			};
 
arch/arm64/boot/dts/marvell/armada-cp11x.dtsi +10 -0
index 9dcf16beabf5..9a3776534daf 100644
@@ -234,12 +234,17 @@ CP11X_LABEL(gpio1): gpio@100 {
 				gpio-controller;
 				#gpio-cells = <2>;
 				gpio-ranges = <&CP11X_LABEL(pinctrl) 0 0 32>;
+				marvell,pwm-offset = <0x1f0>;
+				#pwm-cells = <2>;
 				interrupt-controller;
 				interrupts = <86 IRQ_TYPE_LEVEL_HIGH>,
 					<85 IRQ_TYPE_LEVEL_HIGH>,
 					<84 IRQ_TYPE_LEVEL_HIGH>,
 					<83 IRQ_TYPE_LEVEL_HIGH>;
 				#interrupt-cells = <2>;
+				clock-names = "core", "axi";
+				clocks = <&CP11X_LABEL(clk) 1 21>,
+					 <&CP11X_LABEL(clk) 1 17>;
 				status = "disabled";
 			};
 
@@ -250,12 +255,17 @@ CP11X_LABEL(gpio2): gpio@140 {
 				gpio-controller;
 				#gpio-cells = <2>;
 				gpio-ranges = <&CP11X_LABEL(pinctrl) 0 32 31>;
+				marvell,pwm-offset = <0x1f0>;
+				#pwm-cells = <2>;
 				interrupt-controller;
 				interrupts = <82 IRQ_TYPE_LEVEL_HIGH>,
 					<81 IRQ_TYPE_LEVEL_HIGH>,
 					<80 IRQ_TYPE_LEVEL_HIGH>,
 					<79 IRQ_TYPE_LEVEL_HIGH>;
 				#interrupt-cells = <2>;
+				clock-names = "core", "axi";
+				clocks = <&CP11X_LABEL(clk) 1 21>,
+					 <&CP11X_LABEL(clk) 1 17>;
 				status = "disabled";
 			};
 		};
Signature · 1 line
2.29.2
#4 lore raw

PATCH v4 3/3 dt-bindings: ap806: document marvell, gpio pwm-offset property

In reply to Baruch Siach

Update the example as well. Add the '#pwm-cells' and 'clocks' properties for a complete working example.

---
 .../bindings/arm/marvell/ap80x-system-controller.txt      | 8 ++++++++
 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt +8 -0
index e31511255d8e..052a967c1f28 100644
@@ -80,6 +80,11 @@ Required properties:
 
 - offset: offset address inside the syscon block
 
+Optional properties:
+
+- marvell,pwm-offset: offset address of PWM duration control registers inside
+  the syscon block
+
 Example:
 ap_syscon: system-controller@6f4000 {
 	compatible = "syscon", "simple-mfd";
@@ -101,6 +106,9 @@ ap_syscon: system-controller@6f4000 {
 		gpio-controller;
 		#gpio-cells = <2>;
 		gpio-ranges = <&ap_pinctrl 0 0 19>;
+		marvell,pwm-offset = <0x10c0>;
+		#pwm-cells = <2>;
+		clocks = <&ap_clk 3>;
 	};
 };
 
Signature · 1 line
2.29.2
maintainer Reviewed-by on v4 3/3
#5 lore raw

Re: [PATCH v4 3/3] dt-bindings: ap806: document marvell, gpio pwm-offset property

Quoted 8 lines · Baruch Siach

Update the example as well. Add the '#pwm-cells' and 'clocks' properties for a complete working example.

---
 .../bindings/arm/marvell/ap80x-system-controller.txt      | 8 ++++++++
 1 file changed, 8 insertions(+)

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#6 lore raw
In reply to Baruch Siach

Hi Baruch,

this series seem to have missed the previous merge window due to remaining comments, will you rebase on v5.11-rc1 and resend them?

Yours,
Linus Walleij

#7 lore raw

Hi Linus,

Quoted 2 lines · Linus Walleij

this series seem to have missed the previous merge window due to remaining comments, will you rebase on v5.11-rc1 and resend them?

This series applies cleanly on v5.11-rc2. I have just tested.

As far as I understand there are no remaining comments. Rob acked the DT binding change.

Would you like me to post the series again anyway?

Thanks,
baruch

Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - [email protected] - tel: +972.52.368.4656, http://www.tkos.co.il -
#8 lore raw
Quoted 9 lines · Baruch Siach

Hi Linus,

Quoted 2 lines · Linus Walleij

this series seem to have missed the previous merge window due to remaining comments, will you rebase on v5.11-rc1 and resend them?

This series applies cleanly on v5.11-rc2. I have just tested.

Quoted 8 lines

As far as I understand there are no remaining comments. Rob acked the DT binding change.

Would you like me to post the series again anyway?

Thanks,
baruch

Yes, please resend it with RESEND PATCH in the tag.

Bartosz

#9 lore raw
In reply to Baruch Siach
Quoted 17 lines · Baruch Siach

This series makes two changes to v3:

* Remove patches that are in LinusW linux-gpio for-next and fixes

* Rename the 'pwm-offset' property to 'marvell,pwm-offset' as suggested by Rob Herring

The original cover letter follows (with DT property name updated).

The gpio-mvebu driver supports the PWM functionality of the GPIO block for earlier Armada variants like XP, 370 and 38x. This series extends support to newer Armada variants that use CP11x and AP80x, like Armada 8K and 7K.

This series adds adds the 'marvell,pwm-offset' property to DT binding. 'marvell,pwm-offset' points to the base of A/B counter registers that determine the PWM period and duty cycle.

Quoted 8 lines

The existing PWM DT binding reflects an arbitrary decision to allocate the A counter to the first GPIO block, and B counter to the other one. In attempt to provide better future flexibility, the new 'marvell,pwm-offset' property always points to the base address of both A/B counters. The driver code still allocates the counters in the same way, but this might change in the future with no change to the DT.

Tested AP806 and CP110 (both) on Armada 8040 based system.

Did you see the patches I sent during the last year doing this and adding support for the fan on the GT-8k?

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
#10 lore raw

Hi Russell,

Quoted 25 lines · Russell King - ARM Linux admin
Quoted 24 lines · Baruch Siach

This series makes two changes to v3:

* Remove patches that are in LinusW linux-gpio for-next and fixes

* Rename the 'pwm-offset' property to 'marvell,pwm-offset' as suggested by Rob Herring

The original cover letter follows (with DT property name updated).

The gpio-mvebu driver supports the PWM functionality of the GPIO block for earlier Armada variants like XP, 370 and 38x. This series extends support to newer Armada variants that use CP11x and AP80x, like Armada 8K and 7K.

This series adds adds the 'marvell,pwm-offset' property to DT binding. 'marvell,pwm-offset' points to the base of A/B counter registers that determine the PWM period and duty cycle.

The existing PWM DT binding reflects an arbitrary decision to allocate the A counter to the first GPIO block, and B counter to the other one. In attempt to provide better future flexibility, the new 'marvell,pwm-offset' property always points to the base address of both A/B counters. The driver code still allocates the counters in the same way, but this might change in the future with no change to the DT.

Quoted 4 lines
Quoted 1 line

Tested AP806 and CP110 (both) on Armada 8040 based system.

Did you see the patches I sent during the last year doing this and adding support for the fan on the GT-8k?

You refer to the series linked below, right?

https://lore.kernel.org/linux-pwm/[email protected]/

(For some reason the LAKM archive is missing two years, including this time frame)

I now remember that series. I even archived it locally. But then I forgot about it, so I ended up recreating Armada 8K PWM support from scratch. Sorry about that.

Any comment on this series?

baruch

Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - [email protected] - tel: +972.52.368.4656, http://www.tkos.co.il -