All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/3] gpio: mvebu: Armada 8K/7K PWM support
@ 2021-01-11 11:46 ` Baruch Siach
  0 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Baruch Siach, Andrew Lunn, Gregory Clement, Russell King,
	Sebastian Hesselbarth, Thomas Petazzoni, Chris Packham,
	Sascha Hauer, Ralph Sennhauser, linux-pwm, linux-gpio,
	linux-arm-kernel, devicetree

This version is identical to v4 with the typo fix from v5.

This series has no dependency on the fixes series that I posted separately.

Tested on top of v5.11-rc2.

Changes in v7:

  * Split the get_state fix to a separate independent fixes series

Changes in v6:

  * Reduce rounding error in the get_state fix (RMK)

Changes in v5:

  * Add a fix for get_state

  * Fix typo in patch #4 subject line

  * Add Rob's review tag on the binding documentation patch

Changes in v4:

  * 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 gpio marvell,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(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v7 0/3] gpio: mvebu: Armada 8K/7K PWM support
@ 2021-01-11 11:46 ` Baruch Siach
  0 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Andrew Lunn, Baruch Siach, linux-pwm, Gregory Clement,
	Russell King, linux-gpio, devicetree, Chris Packham,
	Thomas Petazzoni, Ralph Sennhauser, Sascha Hauer,
	linux-arm-kernel, Sebastian Hesselbarth

This version is identical to v4 with the typo fix from v5.

This series has no dependency on the fixes series that I posted separately.

Tested on top of v5.11-rc2.

Changes in v7:

  * Split the get_state fix to a separate independent fixes series

Changes in v6:

  * Reduce rounding error in the get_state fix (RMK)

Changes in v5:

  * Add a fix for get_state

  * Fix typo in patch #4 subject line

  * Add Rob's review tag on the binding documentation patch

Changes in v4:

  * 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 gpio marvell,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(-)

-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K
  2021-01-11 11:46 ` Baruch Siach
@ 2021-01-11 11:46   ` Baruch Siach
  -1 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Baruch Siach, Andrew Lunn, Gregory Clement, Russell King,
	Sebastian Hesselbarth, Thomas Petazzoni, Chris Packham,
	Sascha Hauer, Ralph Sennhauser, linux-pwm, linux-gpio,
	linux-arm-kernel, devicetree

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);
 
 	mvpwm->clk_rate = clk_get_rate(mvchip->clk);
 	if (!mvpwm->clk_rate) {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K
@ 2021-01-11 11:46   ` Baruch Siach
  0 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Andrew Lunn, Baruch Siach, linux-pwm, Gregory Clement,
	Russell King, linux-gpio, devicetree, Chris Packham,
	Thomas Petazzoni, Ralph Sennhauser, Sascha Hauer,
	linux-arm-kernel, Sebastian Hesselbarth

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);
 
 	mvpwm->clk_rate = clk_get_rate(mvchip->clk);
 	if (!mvpwm->clk_rate) {
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
  2021-01-11 11:46 ` Baruch Siach
@ 2021-01-11 11:46   ` Baruch Siach
  -1 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Baruch Siach, Andrew Lunn, Gregory Clement, Russell King,
	Sebastian Hesselbarth, Thomas Petazzoni, Chris Packham,
	Sascha Hauer, Ralph Sennhauser, linux-pwm, linux-gpio,
	linux-arm-kernel, devicetree

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.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 arch/arm64/boot/dts/marvell/armada-ap80x.dtsi |  3 +++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
index 12e477f1aeb9..6614472100c2 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
@@ -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>;
 				};
 			};
 
diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 994a2fce449a..d774a39334d9 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -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";
 			};
 		};
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
@ 2021-01-11 11:46   ` Baruch Siach
  0 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Andrew Lunn, Baruch Siach, linux-pwm, Gregory Clement,
	Russell King, linux-gpio, devicetree, Chris Packham,
	Thomas Petazzoni, Ralph Sennhauser, Sascha Hauer,
	linux-arm-kernel, Sebastian Hesselbarth

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.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 arch/arm64/boot/dts/marvell/armada-ap80x.dtsi |  3 +++
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
index 12e477f1aeb9..6614472100c2 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
@@ -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>;
 				};
 			};
 
diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 994a2fce449a..d774a39334d9 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -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";
 			};
 		};
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
  2021-01-11 11:46 ` Baruch Siach
@ 2021-01-11 11:46   ` Baruch Siach
  -1 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Baruch Siach, Rob Herring, Andrew Lunn, Gregory Clement,
	Russell King, Sebastian Hesselbarth, Thomas Petazzoni,
	Chris Packham, Sascha Hauer, Ralph Sennhauser, linux-pwm,
	linux-gpio, linux-arm-kernel, devicetree

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

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 .../bindings/arm/marvell/ap80x-system-controller.txt      | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
index e31511255d8e..052a967c1f28 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
@@ -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>;
 	};
 };
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell, pwm-offset property
@ 2021-01-11 11:46   ` Baruch Siach
  0 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-11 11:46 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Bartosz Golaszewski, Rob Herring
  Cc: Baruch Siach, Andrew Lunn, Gregory Clement, Russell King,
	linux-pwm, linux-gpio, devicetree, Chris Packham,
	Thomas Petazzoni, Ralph Sennhauser, Sascha Hauer,
	linux-arm-kernel, Sebastian Hesselbarth

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

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 .../bindings/arm/marvell/ap80x-system-controller.txt      | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt b/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
index e31511255d8e..052a967c1f28 100644
--- a/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/ap80x-system-controller.txt
@@ -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>;
 	};
 };
 
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
  2021-01-11 11:46   ` [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell, pwm-offset property Baruch Siach
@ 2021-01-12  8:49     ` Linus Walleij
  -1 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2021-01-12  8:49 UTC (permalink / raw)
  To: Baruch Siach, Rob Herring
  Cc: Thierry Reding, Uwe Kleine-König, Lee Jones,
	Bartosz Golaszewski, Rob Herring, Andrew Lunn, Gregory Clement,
	Russell King, Sebastian Hesselbarth, Thomas Petazzoni,
	Chris Packham, Sascha Hauer, Ralph Sennhauser, linux-pwm,
	open list:GPIO SUBSYSTEM, Linux ARM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Baruch,

this caught my eye:

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

> Update the example as well. Add the '#pwm-cells' and 'clocks' properties
> for a complete working example.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

(...)
> +Optional properties:
> +
> +- marvell,pwm-offset: offset address of PWM duration control registers inside
> +  the syscon block
(...)
>  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>;

This seems to be one of those cases where we start to encode things related
to the hardware variant into the device tree.

Is this just documenting ABI that was introduced in the past and we can not
do anything about now? In that case it is OK I suppose.

For a new binding we would certainly require that the system controller
provide a specific tertiary compatible string for this, lest we disguise
the not-so-simple system controller as "simple-mfd" so:

compatible = "syscon", "simple-mfd", "my-silicon-id";

Then detect the PWM offset by using
if(of_device_is_compatibe(np, "my-silicon-id"))
in the code rather than parsing any marvell,pwm-offset property.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
@ 2021-01-12  8:49     ` Linus Walleij
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2021-01-12  8:49 UTC (permalink / raw)
  To: Baruch Siach, Rob Herring
  Cc: Andrew Lunn, linux-pwm, Gregory Clement, Chris Packham,
	Russell King, Rob Herring, Bartosz Golaszewski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thierry Reding, Linux ARM, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Lee Jones, Sascha Hauer,
	open list:GPIO SUBSYSTEM, Sebastian Hesselbarth

Hi Baruch,

this caught my eye:

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

> Update the example as well. Add the '#pwm-cells' and 'clocks' properties
> for a complete working example.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

(...)
> +Optional properties:
> +
> +- marvell,pwm-offset: offset address of PWM duration control registers inside
> +  the syscon block
(...)
>  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>;

This seems to be one of those cases where we start to encode things related
to the hardware variant into the device tree.

Is this just documenting ABI that was introduced in the past and we can not
do anything about now? In that case it is OK I suppose.

For a new binding we would certainly require that the system controller
provide a specific tertiary compatible string for this, lest we disguise
the not-so-simple system controller as "simple-mfd" so:

compatible = "syscon", "simple-mfd", "my-silicon-id";

Then detect the PWM offset by using
if(of_device_is_compatibe(np, "my-silicon-id"))
in the code rather than parsing any marvell,pwm-offset property.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
  2021-01-12  8:49     ` Linus Walleij
@ 2021-01-12 10:36       ` Russell King - ARM Linux admin
  -1 siblings, 0 replies; 25+ messages in thread
From: Russell King - ARM Linux admin @ 2021-01-12 10:36 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Baruch Siach, Rob Herring, Andrew Lunn, linux-pwm,
	Gregory Clement, Chris Packham, Rob Herring, Bartosz Golaszewski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thierry Reding, Linux ARM, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Lee Jones, Sascha Hauer,
	open list:GPIO SUBSYSTEM, Sebastian Hesselbarth

On Tue, Jan 12, 2021 at 09:49:16AM +0100, Linus Walleij wrote:
> Hi Baruch,
> 
> this caught my eye:
> 
> On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
> 
> > Update the example as well. Add the '#pwm-cells' and 'clocks' properties
> > for a complete working example.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> (...)
> > +Optional properties:
> > +
> > +- marvell,pwm-offset: offset address of PWM duration control registers inside
> > +  the syscon block
> (...)
> >  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>;
> 
> This seems to be one of those cases where we start to encode things related
> to the hardware variant into the device tree.
> 
> Is this just documenting ABI that was introduced in the past and we can not
> do anything about now? In that case it is OK I suppose.
> 
> For a new binding we would certainly require that the system controller
> provide a specific tertiary compatible string for this, lest we disguise
> the not-so-simple system controller as "simple-mfd" so:
> 
> compatible = "syscon", "simple-mfd", "my-silicon-id";
> 
> Then detect the PWM offset by using
> if(of_device_is_compatibe(np, "my-silicon-id"))
> in the code rather than parsing any marvell,pwm-offset property.

I think it would be a good idea to describe the hardware more fully.
For the CP110 and AP80x dies on Armada 8040:

CP110	AP80x
Offset	Offset
00/40	5040	Data Out
04/44	5044	Data Out Enable
08/48	5048	Blink Enable
0c/4c	504c	Data In polarity
10/50	5050	Data In
14/54	5054	IRQ Cause
18/58	5058	IRQ Mask
1c/5c	505c	IRQ Level mask
20/60	5060	Blink Counter Select
28/68	5068	Control Set
2c/6c	506c	Control Clear
30/70	5070	Data Out Set
34/74	5074	Data Out Clear
f0	50c0	Blink Counter A ON duration
f4	50c4	Blink Counter A OFF duration
f8	50c8	Blink Counter B ON duration
fc	50cc	Blink Counter B OFF duration

We identify both of these using a compatible of "marvell,armada-8k-gpio"
which really only describes the first 64 bytes of the register set:

			ap_gpio: gpio@1040 {
				compatible = "marvell,armada-8k-gpio";
				offset = <0x1040>;
				...
			};

			CP11X_LABEL(gpio1): gpio@100 {
				compatible = "marvell,armada-8k-gpio";
				offset = <0x100>;
				...
			};

			CP11X_LABEL(gpio2): gpio@140 {
				compatible = "marvell,armada-8k-gpio";
				offset = <0x140>;
				...
			};

Note that on the CP11x dies, there are two GPIO controllers sharing the
same set of blink counter registers - one at offset 0 the other at
offset 0x40.

However, the pwm-offset is the offset in the regmap of the parent node.

It is possible to use a more specific compatible that would describe
the PWM offset for the CP11x and AP806 (which would need two different
ones) but that starts getting messy when you consider that we already
describe an offset in regmap for the first 64 registers, and encoding
the blink register offset in a compatible would partially end up
encoding the "offset" we already have.

In any case, these offsets are a function of how it was originally
chosen to describe the hardware in DT, rather than anything about the
hardware itself. The choice to use a syscon/regmap is purely an
implementation decision rather than something from the hardware, so
this DT description is already based around describing what is required
for the Linux implementation, rather than purely being a hardware
description.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
@ 2021-01-12 10:36       ` Russell King - ARM Linux admin
  0 siblings, 0 replies; 25+ messages in thread
From: Russell King - ARM Linux admin @ 2021-01-12 10:36 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Baruch Siach, linux-pwm, Gregory Clement,
	open list:GPIO SUBSYSTEM, Rob Herring, Bartosz Golaszewski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Packham, Andrew Lunn, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Thierry Reding,
	Lee Jones, Sascha Hauer, Linux ARM, Sebastian Hesselbarth

On Tue, Jan 12, 2021 at 09:49:16AM +0100, Linus Walleij wrote:
> Hi Baruch,
> 
> this caught my eye:
> 
> On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
> 
> > Update the example as well. Add the '#pwm-cells' and 'clocks' properties
> > for a complete working example.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> (...)
> > +Optional properties:
> > +
> > +- marvell,pwm-offset: offset address of PWM duration control registers inside
> > +  the syscon block
> (...)
> >  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>;
> 
> This seems to be one of those cases where we start to encode things related
> to the hardware variant into the device tree.
> 
> Is this just documenting ABI that was introduced in the past and we can not
> do anything about now? In that case it is OK I suppose.
> 
> For a new binding we would certainly require that the system controller
> provide a specific tertiary compatible string for this, lest we disguise
> the not-so-simple system controller as "simple-mfd" so:
> 
> compatible = "syscon", "simple-mfd", "my-silicon-id";
> 
> Then detect the PWM offset by using
> if(of_device_is_compatibe(np, "my-silicon-id"))
> in the code rather than parsing any marvell,pwm-offset property.

I think it would be a good idea to describe the hardware more fully.
For the CP110 and AP80x dies on Armada 8040:

CP110	AP80x
Offset	Offset
00/40	5040	Data Out
04/44	5044	Data Out Enable
08/48	5048	Blink Enable
0c/4c	504c	Data In polarity
10/50	5050	Data In
14/54	5054	IRQ Cause
18/58	5058	IRQ Mask
1c/5c	505c	IRQ Level mask
20/60	5060	Blink Counter Select
28/68	5068	Control Set
2c/6c	506c	Control Clear
30/70	5070	Data Out Set
34/74	5074	Data Out Clear
f0	50c0	Blink Counter A ON duration
f4	50c4	Blink Counter A OFF duration
f8	50c8	Blink Counter B ON duration
fc	50cc	Blink Counter B OFF duration

We identify both of these using a compatible of "marvell,armada-8k-gpio"
which really only describes the first 64 bytes of the register set:

			ap_gpio: gpio@1040 {
				compatible = "marvell,armada-8k-gpio";
				offset = <0x1040>;
				...
			};

			CP11X_LABEL(gpio1): gpio@100 {
				compatible = "marvell,armada-8k-gpio";
				offset = <0x100>;
				...
			};

			CP11X_LABEL(gpio2): gpio@140 {
				compatible = "marvell,armada-8k-gpio";
				offset = <0x140>;
				...
			};

Note that on the CP11x dies, there are two GPIO controllers sharing the
same set of blink counter registers - one at offset 0 the other at
offset 0x40.

However, the pwm-offset is the offset in the regmap of the parent node.

It is possible to use a more specific compatible that would describe
the PWM offset for the CP11x and AP806 (which would need two different
ones) but that starts getting messy when you consider that we already
describe an offset in regmap for the first 64 registers, and encoding
the blink register offset in a compatible would partially end up
encoding the "offset" we already have.

In any case, these offsets are a function of how it was originally
chosen to describe the hardware in DT, rather than anything about the
hardware itself. The choice to use a syscon/regmap is purely an
implementation decision rather than something from the hardware, so
this DT description is already based around describing what is required
for the Linux implementation, rather than purely being a hardware
description.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
  2021-01-12 10:36       ` Russell King - ARM Linux admin
@ 2021-01-18 13:37         ` Linus Walleij
  -1 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2021-01-18 13:37 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Baruch Siach, Rob Herring, Andrew Lunn, linux-pwm,
	Gregory Clement, Chris Packham, Rob Herring, Bartosz Golaszewski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thierry Reding, Linux ARM, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Lee Jones, Sascha Hauer,
	open list:GPIO SUBSYSTEM, Sebastian Hesselbarth

On Tue, Jan 12, 2021 at 11:36 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:

> In any case, these offsets are a function of how it was originally
> chosen to describe the hardware in DT, rather than anything about the
> hardware itself. The choice to use a syscon/regmap is purely an
> implementation decision rather than something from the hardware, so
> this DT description is already based around describing what is required
> for the Linux implementation, rather than purely being a hardware
> description.

OK I will not complain about it then, this kind of thing happens
sometimes.

Thanks Russell!
Linus Walleij

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property
@ 2021-01-18 13:37         ` Linus Walleij
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2021-01-18 13:37 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Baruch Siach, linux-pwm, Gregory Clement,
	open list:GPIO SUBSYSTEM, Rob Herring, Bartosz Golaszewski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Chris Packham, Andrew Lunn, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Thierry Reding,
	Lee Jones, Sascha Hauer, Linux ARM, Sebastian Hesselbarth

On Tue, Jan 12, 2021 at 11:36 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:

> In any case, these offsets are a function of how it was originally
> chosen to describe the hardware in DT, rather than anything about the
> hardware itself. The choice to use a syscon/regmap is purely an
> implementation decision rather than something from the hardware, so
> this DT description is already based around describing what is required
> for the Linux implementation, rather than purely being a hardware
> description.

OK I will not complain about it then, this kind of thing happens
sometimes.

Thanks Russell!
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K
  2021-01-11 11:46   ` Baruch Siach
@ 2021-01-22 12:58     ` Bartosz Golaszewski
  -1 siblings, 0 replies; 25+ messages in thread
From: Bartosz Golaszewski @ 2021-01-22 12:58 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Rob Herring, Andrew Lunn, Gregory Clement, Russell King,
	Sebastian Hesselbarth, Thomas Petazzoni, Chris Packham,
	Sascha Hauer, Ralph Sennhauser, linux-pwm, linux-gpio, arm-soc,
	linux-devicetree

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
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K
@ 2021-01-22 12:58     ` Bartosz Golaszewski
  0 siblings, 0 replies; 25+ messages in thread
From: Bartosz Golaszewski @ 2021-01-22 12:58 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Andrew Lunn, Sascha Hauer, linux-pwm, Linus Walleij,
	Chris Packham, Russell King, Rob Herring, linux-gpio,
	linux-devicetree, Thierry Reding, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Lee Jones,
	Gregory Clement, arm-soc, Sebastian Hesselbarth

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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K
  2021-01-22 12:58     ` Bartosz Golaszewski
@ 2021-01-24  6:17       ` Baruch Siach
  -1 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-24  6:17 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Thierry Reding, Uwe Kleine-König, Lee Jones, Linus Walleij,
	Rob Herring, Andrew Lunn, Gregory Clement, Russell King,
	Sebastian Hesselbarth, Thomas Petazzoni, Chris Packham,
	Sascha Hauer, Ralph Sennhauser, linux-pwm, linux-gpio, arm-soc,
	linux-devicetree

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 -

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K
@ 2021-01-24  6:17       ` Baruch Siach
  0 siblings, 0 replies; 25+ messages in thread
From: Baruch Siach @ 2021-01-24  6:17 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andrew Lunn, Sascha Hauer, linux-pwm, Linus Walleij,
	Chris Packham, Russell King, Rob Herring, linux-gpio,
	linux-devicetree, Thierry Reding, Thomas Petazzoni,
	Uwe Kleine-König, Ralph Sennhauser, Lee Jones,
	Gregory Clement, arm-soc, Sebastian Hesselbarth

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 -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
  2021-01-11 11:46   ` Baruch Siach
@ 2021-01-25  9:50     ` Bartosz Golaszewski
  -1 siblings, 0 replies; 25+ messages in thread
From: Bartosz Golaszewski @ 2021-01-25  9:50 UTC (permalink / raw)
  To: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth
  Cc: Linus Walleij, Rob Herring, Russell King, Chris Packham,
	linux-pwm, linux-gpio, arm-soc, linux-devicetree, Baruch Siach

On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
>
> 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.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---

Andrew, Gregory, Sebastian,

Can we get your Acks on this patch? Are you fine with it going through
the GPIO tree?

Bartosz

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
@ 2021-01-25  9:50     ` Bartosz Golaszewski
  0 siblings, 0 replies; 25+ messages in thread
From: Bartosz Golaszewski @ 2021-01-25  9:50 UTC (permalink / raw)
  To: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth
  Cc: linux-pwm, Baruch Siach, linux-devicetree, Linus Walleij,
	Russell King, linux-gpio, Rob Herring, Chris Packham, arm-soc

On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
>
> 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.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---

Andrew, Gregory, Sebastian,

Can we get your Acks on this patch? Are you fine with it going through
the GPIO tree?

Bartosz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
  2021-01-11 11:46   ` Baruch Siach
@ 2021-01-29 15:55     ` Gregory CLEMENT
  -1 siblings, 0 replies; 25+ messages in thread
From: Gregory CLEMENT @ 2021-01-29 15:55 UTC (permalink / raw)
  To: Baruch Siach, Thierry Reding, Uwe Kleine-König, Lee Jones,
	Linus Walleij, Bartosz Golaszewski, Rob Herring
  Cc: Baruch Siach, Andrew Lunn, Russell King, Sebastian Hesselbarth,
	Thomas Petazzoni, Chris Packham, Sascha Hauer, Ralph Sennhauser,
	linux-pwm, linux-gpio, linux-arm-kernel, devicetree

Hi Baruch,

> 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.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Applied on mvebu/dt64

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/armada-ap80x.dtsi |  3 +++
>  arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 10 ++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> index 12e477f1aeb9..6614472100c2 100644
> --- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> @@ -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>;
>  				};
>  			};
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> index 994a2fce449a..d774a39334d9 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> @@ -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";
>  			};
>  		};
> -- 
> 2.29.2
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
@ 2021-01-29 15:55     ` Gregory CLEMENT
  0 siblings, 0 replies; 25+ messages in thread
From: Gregory CLEMENT @ 2021-01-29 15:55 UTC (permalink / raw)
  To: Baruch Siach, Thierry Reding, Uwe Kleine-König, Lee Jones,
	Linus Walleij, Bartosz Golaszewski, Rob Herring
  Cc: Andrew Lunn, Baruch Siach, linux-pwm, Sascha Hauer, Russell King,
	linux-gpio, devicetree, Chris Packham, Thomas Petazzoni,
	Ralph Sennhauser, linux-arm-kernel, Sebastian Hesselbarth

Hi Baruch,

> 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.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Applied on mvebu/dt64

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/armada-ap80x.dtsi |  3 +++
>  arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 10 ++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> index 12e477f1aeb9..6614472100c2 100644
> --- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> @@ -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>;
>  				};
>  			};
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> index 994a2fce449a..d774a39334d9 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
> @@ -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";
>  			};
>  		};
> -- 
> 2.29.2
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
  2021-01-25  9:50     ` Bartosz Golaszewski
  (?)
@ 2021-01-29 15:56     ` Gregory CLEMENT
  2021-02-02 11:27         ` Bartosz Golaszewski
  -1 siblings, 1 reply; 25+ messages in thread
From: Gregory CLEMENT @ 2021-01-29 15:56 UTC (permalink / raw)
  To: Bartosz Golaszewski, Andrew Lunn, Sebastian Hesselbarth
  Cc: linux-pwm, Baruch Siach, linux-devicetree, Linus Walleij,
	Russell King, linux-gpio, Rob Herring, Chris Packham, arm-soc

Hello Bartosz,

> On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
>>
>> 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.
>>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>
> Andrew, Gregory, Sebastian,
>
> Can we get your Acks on this patch? Are you fine with it going through
> the GPIO tree?

I prefer applying it on our tree to avoid merge conflicts.

Gregory

>
> Bartosz

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
  2021-01-29 15:56     ` Gregory CLEMENT
@ 2021-02-02 11:27         ` Bartosz Golaszewski
  0 siblings, 0 replies; 25+ messages in thread
From: Bartosz Golaszewski @ 2021-02-02 11:27 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Andrew Lunn, Sebastian Hesselbarth, Linus Walleij, Rob Herring,
	Russell King, Chris Packham, linux-pwm, linux-gpio, arm-soc,
	linux-devicetree, Baruch Siach

On Fri, Jan 29, 2021 at 4:56 PM Gregory CLEMENT
<gregory.clement@bootlin.com> wrote:
>
> Hello Bartosz,
>
> > On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
> >>
> >> 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.
> >>
> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> >> ---
> >
> > Andrew, Gregory, Sebastian,
> >
> > Can we get your Acks on this patch? Are you fine with it going through
> > the GPIO tree?
>
> I prefer applying it on our tree to avoid merge conflicts.
>
> Gregory

Ok, I applied the remaining patches from this series.

Bartosz

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios
@ 2021-02-02 11:27         ` Bartosz Golaszewski
  0 siblings, 0 replies; 25+ messages in thread
From: Bartosz Golaszewski @ 2021-02-02 11:27 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Andrew Lunn, Baruch Siach, linux-pwm, Linus Walleij,
	Russell King, linux-gpio, linux-devicetree, Rob Herring,
	Chris Packham, arm-soc, Sebastian Hesselbarth

On Fri, Jan 29, 2021 at 4:56 PM Gregory CLEMENT
<gregory.clement@bootlin.com> wrote:
>
> Hello Bartosz,
>
> > On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach <baruch@tkos.co.il> wrote:
> >>
> >> 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.
> >>
> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> >> ---
> >
> > Andrew, Gregory, Sebastian,
> >
> > Can we get your Acks on this patch? Are you fine with it going through
> > the GPIO tree?
>
> I prefer applying it on our tree to avoid merge conflicts.
>
> Gregory

Ok, I applied the remaining patches from this series.

Bartosz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2021-02-02 11:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11 11:46 [PATCH v7 0/3] gpio: mvebu: Armada 8K/7K PWM support Baruch Siach
2021-01-11 11:46 ` Baruch Siach
2021-01-11 11:46 ` [PATCH v7 1/3] gpio: mvebu: add pwm support for Armada 8K/7K Baruch Siach
2021-01-11 11:46   ` Baruch Siach
2021-01-22 12:58   ` Bartosz Golaszewski
2021-01-22 12:58     ` Bartosz Golaszewski
2021-01-24  6:17     ` Baruch Siach
2021-01-24  6:17       ` Baruch Siach
2021-01-11 11:46 ` [PATCH v7 2/3] arm64: dts: armada: add pwm offsets for ap/cp gpios Baruch Siach
2021-01-11 11:46   ` Baruch Siach
2021-01-25  9:50   ` Bartosz Golaszewski
2021-01-25  9:50     ` Bartosz Golaszewski
2021-01-29 15:56     ` Gregory CLEMENT
2021-02-02 11:27       ` Bartosz Golaszewski
2021-02-02 11:27         ` Bartosz Golaszewski
2021-01-29 15:55   ` Gregory CLEMENT
2021-01-29 15:55     ` Gregory CLEMENT
2021-01-11 11:46 ` [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property Baruch Siach
2021-01-11 11:46   ` [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell, pwm-offset property Baruch Siach
2021-01-12  8:49   ` [PATCH v7 3/3] dt-bindings: ap806: document gpio marvell,pwm-offset property Linus Walleij
2021-01-12  8:49     ` Linus Walleij
2021-01-12 10:36     ` Russell King - ARM Linux admin
2021-01-12 10:36       ` Russell King - ARM Linux admin
2021-01-18 13:37       ` Linus Walleij
2021-01-18 13:37         ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.