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
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(-)
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(+)
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(+)
Hi Baruch,
this caught my eye:
On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach [off-list ref] 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
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
From: Russell King - ARM Linux admin <linux@armlinux.org.uk> Date: 2021-01-12 10:37:28
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 [off-list ref] wrote:
quoted
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>
(...)
quoted
+Optional properties:
+
+- marvell,pwm-offset: offset address of PWM duration control registers inside
+ the syscon block
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
On Tue, Jan 12, 2021 at 11:36 AM Russell King - ARM Linux admin
[off-list ref] 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
On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach [off-list ref] wrote:
quoted hunk
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(-)
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 [off-list ref] wrote:
quoted
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>
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
On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach [off-list ref] 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
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>
On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach [off-list ref] wrote:
quoted
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
On Fri, Jan 29, 2021 at 4:56 PM Gregory CLEMENT
[off-list ref] wrote:
Hello Bartosz,
quoted
On Mon, Jan 11, 2021 at 12:47 PM Baruch Siach [off-list ref] wrote:
quoted
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