Current dts files with 'pwm' nodes are manually verified.
In order to automate this process pwm-rockchip.txt
has to be converted to yaml.
Signed-off-by: Johan Jonker <[email protected]>
---
Changed V2:
changed schema for clocks and clock-names
---
.../devicetree/bindings/pwm/pwm-rockchip.txt | 27 -------
.../devicetree/bindings/pwm/pwm-rockchip.yaml | 91 ++++++++++++++++++++++
2 files changed, 91 insertions(+), 27 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
deleted file mode 100644
index f70956dea..000000000
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-Rockchip PWM controller
-
-Required properties:
- - compatible: should be "rockchip,<name>-pwm"
- "rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
- "rockchip,rk3288-pwm": found on RK3288 SOC
- "rockchip,rv1108-pwm", "rockchip,rk3288-pwm": found on RV1108 SoC
- "rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
- - reg: physical base address and length of the controller's registers
- - clocks: See ../clock/clock-bindings.txt
- - For older hardware (rk2928, rk3066, rk3188, rk3228, rk3288, rk3399):
- - There is one clock that's used both to derive the functional clock
- for the device and as the bus clock.
- - For newer hardware (rk3328 and future socs): specified by name
- - "pwm": This is used to derive the functional clock.
- - "pclk": This is the APB bus clock.
- - #pwm-cells: must be 2 (rk2928) or 3 (rk3288). See pwm.yaml in this directory
- for a description of the cell format.
-
-Example:
-
- pwm0: pwm@20030000 {
- compatible = "rockchip,rk2928-pwm";
- reg = <0x20030000 0x10>;
- clocks = <&cru PCLK_PWM01>;
- #pwm-cells = <2>;
- };
diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
new file mode 100644
index 000000000..142ce85ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/pwm-rockchip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip PWM controller
+
+maintainers:
+ - Heiko Stuebner <[email protected]>
+
+properties:
+ compatible:
+ oneOf:
+ - const: rockchip,rk2928-pwm
+ - const: rockchip,rk3288-pwm
+ - const: rockchip,vop-pwm
+ - items:
+ - const: rockchip,rk3036-pwm
+ - const: rockchip,rk2928-pwm
+ - items:
+ - enum:
+ - rockchip,rv1108-pwm
+ - const: rockchip,rk3288-pwm
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ clock-names:
+ maxItems: 2
+
+ "#pwm-cells":
+ enum: [2, 3]
+ description:
+ Must be 2 (rk2928) or 3 (rk3288 and later).
+ See pwm.yaml for a description of the cell format.
+
+required:
+ - compatible
+ - reg
+ - "#pwm-cells"
+
+if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - rockchip,rv1108-pwm
+
+then:
+ properties:
+ clocks:
+ items:
+ - description: Used to derive the functional clock for the device.
+ - description: Used as the APB bus clock.
+
+ clock-names:
+ items:
+ - const: pwm
+ - const: pclk
+
+ required:
+ - clocks
+ - clock-names
+
+else:
+ properties:
+ clocks:
+ maxItems: 1
+ description:
+ Used both to derive the functional clock
+ for the device and as the bus clock.
+
+ required:
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/rk3188-cru-common.h>
+ pwm0: pwm@20030000 {
+ compatible = "rockchip,rk2928-pwm";
+ reg = <0x20030000 0x10>;
+ clocks = <&cru PCLK_PWM01>;
+ #pwm-cells = <2>;
+ };
--
2.11.0
A test with the command below gives this error:
/arch/arm/boot/dts/rv1108-evb.dt.yaml:
pwm@10280000: 'interrupts' does not match any of the regexes:
'pinctrl-[0-9]+'
"interrupts" is an undocumented property, so remove them
from pwm nodes in rv1108.dtsi.
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
Signed-off-by: Johan Jonker <[email protected]>
---
arch/arm/boot/dts/rv1108.dtsi | 8 --------
1 file changed, 8 deletions(-)
diff --git a/arch/arm/boot/dts/rv1108.dtsi b/arch/arm/boot/dts/rv1108.dtsi
index 68e2282f7..af033d4c9 100644
--- a/arch/arm/boot/dts/rv1108.dtsi
+++ b/arch/arm/boot/dts/rv1108.dtsi
@@ -217,7 +217,6 @@
pwm4: pwm@10280000 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x10280000 0x10>;
- interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -229,7 +228,6 @@
pwm5: pwm@10280010 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x10280010 0x10>;
- interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -241,7 +239,6 @@
pwm6: pwm@10280020 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x10280020 0x10>;
- interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -253,7 +250,6 @@
pwm7: pwm@10280030 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x10280030 0x10>;
- interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -391,7 +387,6 @@
pwm0: pwm@20040000 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x20040000 0x10>;
- interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM0_PMU>, <&cru PCLK_PWM0_PMU>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -403,7 +398,6 @@
pwm1: pwm@20040010 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x20040010 0x10>;
- interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM0_PMU>, <&cru PCLK_PWM0_PMU>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -415,7 +409,6 @@
pwm2: pwm@20040020 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x20040020 0x10>;
- interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM0_PMU>, <&cru PCLK_PWM0_PMU>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
@@ -427,7 +420,6 @@
pwm3: pwm@20040030 {
compatible = "rockchip,rv1108-pwm", "rockchip,rk3288-pwm";
reg = <0x20040030 0x10>;
- interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM0_PMU>, <&cru PCLK_PWM0_PMU>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
--
2.11.0
A test with the command below gives this error:
/arch/arm/boot/dts/rk3036-evb.dt.yaml:
pwm@20050030: clock-names: ['pwm'] is too short
Devices with only one pwm clock use it to both
to derive the functional clock for the device
and as the bus clock. The driver does not need
"clock-names" to get a handle, so remove them all.
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
Signed-off-by: Johan Jonker <[email protected]>
---
arch/arm/boot/dts/rk3036.dtsi | 4 ----
arch/arm/boot/dts/rk3288.dtsi | 4 ----
2 files changed, 8 deletions(-)
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 47a787a12..e24230d50 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -355,7 +355,6 @@
reg = <0x20050000 0x10>;
#pwm-cells = <3>;
clocks = <&cru PCLK_PWM>;
- clock-names = "pwm";
pinctrl-names = "default";
pinctrl-0 = <&pwm0_pin>;
status = "disabled";
@@ -366,7 +365,6 @@
reg = <0x20050010 0x10>;
#pwm-cells = <3>;
clocks = <&cru PCLK_PWM>;
- clock-names = "pwm";
pinctrl-names = "default";
pinctrl-0 = <&pwm1_pin>;
status = "disabled";
@@ -377,7 +375,6 @@
reg = <0x20050020 0x10>;
#pwm-cells = <3>;
clocks = <&cru PCLK_PWM>;
- clock-names = "pwm";
pinctrl-names = "default";
pinctrl-0 = <&pwm2_pin>;
status = "disabled";
@@ -388,7 +385,6 @@
reg = <0x20050030 0x10>;
#pwm-cells = <2>;
clocks = <&cru PCLK_PWM>;
- clock-names = "pwm";
pinctrl-names = "default";
pinctrl-0 = <&pwm3_pin>;
status = "disabled";
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index ea7416c31..05557ad02 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -679,7 +679,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm0_pin>;
clocks = <&cru PCLK_RKPWM>;
- clock-names = "pwm";
status = "disabled";
};
@@ -690,7 +689,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm1_pin>;
clocks = <&cru PCLK_RKPWM>;
- clock-names = "pwm";
status = "disabled";
};
@@ -701,7 +699,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm2_pin>;
clocks = <&cru PCLK_RKPWM>;
- clock-names = "pwm";
status = "disabled";
};
@@ -712,7 +709,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm3_pin>;
clocks = <&cru PCLK_RKPWM>;
- clock-names = "pwm";
status = "disabled";
};
--
2.11.0
A test with the command below gives this error:
/arch/arm64/boot/dts/rockchip/rk3368-evb-act8846.dt.yaml:
pwm@ff680030: clock-names: ['pwm'] is too short
Devices with only one pwm clock use it to both
to derive the functional clock for the device
and as the bus clock. The driver does not need
"clock-names" to get a handle, so remove them all.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
Signed-off-by: Johan Jonker <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3368.dtsi | 4 ----
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 ----
2 files changed, 8 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3368.dtsi b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
index 61b0a2a90..7832e26a3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3368.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3368.dtsi
@@ -561,7 +561,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm0_pin>;
clocks = <&cru PCLK_PWM1>;
- clock-names = "pwm";
status = "disabled";
};
@@ -572,7 +571,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm1_pin>;
clocks = <&cru PCLK_PWM1>;
- clock-names = "pwm";
status = "disabled";
};
@@ -581,7 +579,6 @@
reg = <0x0 0xff680020 0x0 0x10>;
#pwm-cells = <3>;
clocks = <&cru PCLK_PWM1>;
- clock-names = "pwm";
status = "disabled";
};
@@ -592,7 +589,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm3_pin>;
clocks = <&cru PCLK_PWM1>;
- clock-names = "pwm";
status = "disabled";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index e93a5f320..6221b027e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1185,7 +1185,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm0_pin>;
clocks = <&pmucru PCLK_RKPWM_PMU>;
- clock-names = "pwm";
status = "disabled";
};
@@ -1196,7 +1195,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm1_pin>;
clocks = <&pmucru PCLK_RKPWM_PMU>;
- clock-names = "pwm";
status = "disabled";
};
@@ -1207,7 +1205,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm2_pin>;
clocks = <&pmucru PCLK_RKPWM_PMU>;
- clock-names = "pwm";
status = "disabled";
};
@@ -1218,7 +1215,6 @@
pinctrl-names = "default";
pinctrl-0 = <&pwm3a_pin>;
clocks = <&pmucru PCLK_RKPWM_PMU>;
- clock-names = "pwm";
status = "disabled";
};
--
2.11.0
The compatible strings below are already in use in the Rockchip
dtsi files, but were somehow never added to a document, so add
"rockchip,rk3328-pwm"
"rockchip,rk3036-pwm", "rockchip,rk2928-pwm"
"rockchip,rk3368-pwm", "rockchip,rk3288-pwm"
"rockchip,rk3399-pwm", "rockchip,rk3288-pwm"
"rockchip,px30-pwm", "rockchip,rk3328-pwm"
"rockchip,rk3308-pwm", "rockchip,rk3328-pwm"
for pwm nodes to pwm-rockchip.yaml.
Signed-off-by: Johan Jonker <[email protected]>
---
Changed V2:
changed schema for clocks and clock-names
---
Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
index 142ce85ce..5596bee70 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
@@ -14,14 +14,22 @@ properties:
oneOf:
- const: rockchip,rk2928-pwm
- const: rockchip,rk3288-pwm
+ - const: rockchip,rk3328-pwm
- const: rockchip,vop-pwm
- items:
- const: rockchip,rk3036-pwm
- const: rockchip,rk2928-pwm
- items:
- enum:
+ - rockchip,rk3368-pwm
+ - rockchip,rk3399-pwm
- rockchip,rv1108-pwm
- const: rockchip,rk3288-pwm
+ - items:
+ - enum:
+ - rockchip,px30-pwm
+ - rockchip,rk3308-pwm
+ - const: rockchip,rk3328-pwm
reg:
maxItems: 1
@@ -49,6 +57,7 @@ if:
compatible:
contains:
enum:
+ - rockchip,rk3328-pwm
- rockchip,rv1108-pwm
then:
--
2.11.0
A test with the command below gives this error:
/arch/arm64/boot/dts/rockchip/rk3328-a1.dt.yaml: pwm@ff1b0030:
'interrupts' does not match any of the regexes: 'pinctrl-[0-9]+'
"interrupts" is an undocumented property, so remove them
from pwm nodes in rk3328.dtsi.
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
Signed-off-by: Johan Jonker <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index c2ca358c7..5c968b3cd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -480,7 +480,6 @@
pwm3: pwm@ff1b0030 {
compatible = "rockchip,rk3328-pwm";
reg = <0x0 0xff1b0030 0x0 0x10>;
- interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_PWM>, <&cru PCLK_PWM>;
clock-names = "pwm", "pclk";
pinctrl-names = "default";
--
2.11.0
On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <[email protected]> wrote:
>
> A test with the command below gives this error:
>
> /arch/arm/boot/dts/rv1108-evb.dt.yaml:
> pwm@10280000: 'interrupts' does not match any of the regexes:
> 'pinctrl-[0-9]+'
>
> "interrupts" is an undocumented property, so remove them
> from pwm nodes in rv1108.dtsi.
>
> make ARCH=arm dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
>
> Signed-off-by: Johan Jonker <[email protected]>
Given that the interrupts were specified, meaning they are wired up in hardware,
shouldn't the solution be to add the interrupts property to the binding instead?
After all, the device tree describes the actual hardware, not just what the
implementations need.
ChenYu
On 4/12/21 5:15 AM, Chen-Yu Tsai wrote:
> On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <[email protected]> wrote:
>>
>> A test with the command below gives this error:
>>
>> /arch/arm/boot/dts/rv1108-evb.dt.yaml:
>> pwm@10280000: 'interrupts' does not match any of the regexes:
>> 'pinctrl-[0-9]+'
>>
>> "interrupts" is an undocumented property, so remove them
>> from pwm nodes in rv1108.dtsi.
>>
>> make ARCH=arm dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
>>
>> Signed-off-by: Johan Jonker <[email protected]>
>
> Given that the interrupts were specified, meaning they are wired up in hardware,
> shouldn't the solution be to add the interrupts property to the binding instead?
>
> After all, the device tree describes the actual hardware, not just what the
> implementations need.
>
> ChenYu
>
Hi,
The question of what to do with it was asked in version 1, but no answer
was given, so I made a proposal.
The device tree description should be complete, but also as lean as
possible. If someone manages to sneak in undocumented properties without
reason then the ultimate consequence should be removal I think.
Not sure about the (missing?) rv1108 TRM, but for rk3328 the interrupt
is used for:
PWM_INTSTS 0x0040 W 0x00000000 Interrupt Status Register
Channel Interrupt Polarity Flag
This bit is used in capture mode in order to identify the
transition of the input waveform when interrupt is generated.
Channel Interrupt Status
Interrupt generated
PWM_INT_EN 0x0044 W 0x00000000 Interrupt Enable Register
Channel Interrupt Enable
Is there any current realistic use/setup for it to convince rob+dt this
should be added to pwm-rockchip.yaml?
The rk3328 interrupt rkpwm_int seems shared between channels, but only
included to pwm3. What is the proper way for that?
Johan
On Sun, 11 Apr 2021 15:10:03 +0200, Johan Jonker wrote:
> The compatible strings below are already in use in the Rockchip
> dtsi files, but were somehow never added to a document, so add
>
> "rockchip,rk3328-pwm"
>
> "rockchip,rk3036-pwm", "rockchip,rk2928-pwm"
>
> "rockchip,rk3368-pwm", "rockchip,rk3288-pwm"
> "rockchip,rk3399-pwm", "rockchip,rk3288-pwm"
>
> "rockchip,px30-pwm", "rockchip,rk3328-pwm"
> "rockchip,rk3308-pwm", "rockchip,rk3328-pwm"
>
> for pwm nodes to pwm-rockchip.yaml.
>
> Signed-off-by: Johan Jonker <[email protected]>
> ---
> Changed V2:
> changed schema for clocks and clock-names
> ---
> Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
If a tag was not added on purpose, please state why and what changed.
Hi,
Sorry, made a little mistake in version 2 with "rockchip,rk3036-pwm",
"rockchip,rk2928-pwm".
Please trash. Will send version 3.
By the change of schema for clocks and clock-names I add
"rockchip,rk3328-pwm" to the "if:", so strictly speaking v1 and (v2) v3
will not be the same.
Johan
On 4/12/21 5:05 PM, Rob Herring wrote:
> On Sun, 11 Apr 2021 15:10:03 +0200, Johan Jonker wrote:
>> The compatible strings below are already in use in the Rockchip
>> dtsi files, but were somehow never added to a document, so add
>>
>> "rockchip,rk3328-pwm"
>>
>> "rockchip,rk3036-pwm", "rockchip,rk2928-pwm"
>>
>> "rockchip,rk3368-pwm", "rockchip,rk3288-pwm"
>> "rockchip,rk3399-pwm", "rockchip,rk3288-pwm"
>>
>> "rockchip,px30-pwm", "rockchip,rk3328-pwm"
>> "rockchip,rk3308-pwm", "rockchip,rk3328-pwm"
>>
>> for pwm nodes to pwm-rockchip.yaml.
>>
>> Signed-off-by: Johan Jonker <[email protected]>
>> ---
>> Changed V2:
>> changed schema for clocks and clock-names
>> ---
>> Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> If a tag was not added on purpose, please state why and what changed.
>
On Mon, Apr 12, 2021 at 6:03 PM Johan Jonker <[email protected]> wrote:
>
> On 4/12/21 5:15 AM, Chen-Yu Tsai wrote:
> > On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <[email protected]> wrote:
> >>
> >> A test with the command below gives this error:
> >>
> >> /arch/arm/boot/dts/rv1108-evb.dt.yaml:
> >> pwm@10280000: 'interrupts' does not match any of the regexes:
> >> 'pinctrl-[0-9]+'
> >>
> >> "interrupts" is an undocumented property, so remove them
> >> from pwm nodes in rv1108.dtsi.
> >>
> >> make ARCH=arm dtbs_check
> >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
> >>
> >> Signed-off-by: Johan Jonker <[email protected]>
> >
> > Given that the interrupts were specified, meaning they are wired up in hardware,
> > shouldn't the solution be to add the interrupts property to the binding instead?
> >
> > After all, the device tree describes the actual hardware, not just what the
> > implementations need.
> >
> > ChenYu
> >
>
> Hi,
>
> The question of what to do with it was asked in version 1, but no answer
> was given, so I made a proposal.
> The device tree description should be complete, but also as lean as
> possible. If someone manages to sneak in undocumented properties without
> reason then the ultimate consequence should be removal I think.
>
> Not sure about the (missing?) rv1108 TRM, but for rk3328 the interrupt
> is used for:
>
> PWM_INTSTS 0x0040 W 0x00000000 Interrupt Status Register
> Channel Interrupt Polarity Flag
> This bit is used in capture mode in order to identify the
> transition of the input waveform when interrupt is generated.
> Channel Interrupt Status
> Interrupt generated
>
> PWM_INT_EN 0x0044 W 0x00000000 Interrupt Enable Register
> Channel Interrupt Enable
>
> Is there any current realistic use/setup for it to convince rob+dt this
> should be added to pwm-rockchip.yaml?
Well, the PWM core has capture support, and pwm-sti implements it with
interrupt support, so I guess there's at least a legitimate case for
adding that to the binding. Whether someone has an actual use case for
it and adds code to implement it is another story.
> The rk3328 interrupt rkpwm_int seems shared between channels, but only
> included to pwm3. What is the proper way for that?
I guess the bigger question is why was the PWM controller split into
four device nodes, instead of just one encompassing the whole block.
Now we'd have to introduce a new binding to support capture mode and
interrupts.
In that case I agree with dropping the interrupts for now, as it just
won't fit. But I would add this additional information to the commit
message.
Regards
ChenYu
On 4/12/21 12:33 PM, Chen-Yu Tsai wrote:
> On Mon, Apr 12, 2021 at 6:03 PM Johan Jonker <[email protected]> wrote:
>>
>> On 4/12/21 5:15 AM, Chen-Yu Tsai wrote:
>>> On Sun, Apr 11, 2021 at 9:11 PM Johan Jonker <[email protected]> wrote:
>>>>
>>>> A test with the command below gives this error:
>>>>
>>>> /arch/arm/boot/dts/rv1108-evb.dt.yaml:
>>>> pwm@10280000: 'interrupts' does not match any of the regexes:
>>>> 'pinctrl-[0-9]+'
>>>>
>>>> "interrupts" is an undocumented property, so remove them
>>>> from pwm nodes in rv1108.dtsi.
>>>>
>>>> make ARCH=arm dtbs_check
>>>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
>>>>
>>>> Signed-off-by: Johan Jonker <[email protected]>
>>>
>>> Given that the interrupts were specified, meaning they are wired up in hardware,
>>> shouldn't the solution be to add the interrupts property to the binding instead?
>>>
>>> After all, the device tree describes the actual hardware, not just what the
>>> implementations need.
>>>
>>> ChenYu
>>>
>>
>> Hi,
>>
>> The question of what to do with it was asked in version 1, but no answer
>> was given, so I made a proposal.
>> The device tree description should be complete, but also as lean as
>> possible. If someone manages to sneak in undocumented properties without
>> reason then the ultimate consequence should be removal I think.
>>
>> Not sure about the (missing?) rv1108 TRM, but for rk3328 the interrupt
>> is used for:
>>
>> PWM_INTSTS 0x0040 W 0x00000000 Interrupt Status Register
>> Channel Interrupt Polarity Flag
>> This bit is used in capture mode in order to identify the
>> transition of the input waveform when interrupt is generated.
>> Channel Interrupt Status
>> Interrupt generated
>>
>> PWM_INT_EN 0x0044 W 0x00000000 Interrupt Enable Register
>> Channel Interrupt Enable
>>
>> Is there any current realistic use/setup for it to convince rob+dt this
>> should be added to pwm-rockchip.yaml?
Found:
pwm3 combined with ir uses a irq. Keep that as it is for now.
https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/input/remotectl/rockchip_pwm_remotectl.c
>
> Well, the PWM core has capture support, and pwm-sti implements it with
> interrupt support, so I guess there's at least a legitimate case for
> adding that to the binding. Whether someone has an actual use case for
> it and adds code to implement it is another story.
>
>> The rk3328 interrupt rkpwm_int seems shared between channels, but only
>> included to pwm3. What is the proper way for that?
>
> I guess the bigger question is why was the PWM controller split into
> four device nodes, instead of just one encompassing the whole block.
> Now we'd have to introduce a new binding to support capture mode and
> interrupts.
>
> In that case I agree with dropping the interrupts for now, as it just
> won't fit. But I would add this additional information to the commit
> message.
Will wait with adding "interrupts" to pwm-rockchip.yaml till someone
makes a solution for the whole block. Convert only current
document/binding to reduce notifications.
For Heiko: patch 3 + 5 can go in the garbage bin:
[PATCH v2 3/6] ARM: dts: rockchip: remove interrupts properties from pwm
nodes rv1108.dtsi
[PATCH v2 5/6] arm64: dts: rockchip: remove interrupts properties from
pwm nodes rk3328.dtsi
Johan
>
>
> Regards
> ChenYu
>
On Sun, 11 Apr 2021 15:10:02 +0200, Johan Jonker wrote:
> Current dts files with 'pwm' nodes are manually verified.
> In order to automate this process pwm-rockchip.txt
> has to be converted to yaml.
>
> Signed-off-by: Johan Jonker <[email protected]>
> ---
> Changed V2:
> changed schema for clocks and clock-names
> ---
> .../devicetree/bindings/pwm/pwm-rockchip.txt | 27 -------
> .../devicetree/bindings/pwm/pwm-rockchip.yaml | 91 ++++++++++++++++++++++
> 2 files changed, 91 insertions(+), 27 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
>
Reviewed-by: Rob Herring <[email protected]>
On 4/12/21 10:57 PM, Rob Herring wrote:
> On Sun, 11 Apr 2021 15:10:02 +0200, Johan Jonker wrote:
>> Current dts files with 'pwm' nodes are manually verified.
>> In order to automate this process pwm-rockchip.txt
>> has to be converted to yaml.
>>
>> Signed-off-by: Johan Jonker <[email protected]>
>> ---
>> Changed V2:
>> changed schema for clocks and clock-names
>> ---
>> .../devicetree/bindings/pwm/pwm-rockchip.txt | 27 -------
>> .../devicetree/bindings/pwm/pwm-rockchip.yaml | 91 ++++++++++++++++++++++
>> 2 files changed, 91 insertions(+), 27 deletions(-)
>> delete mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
>> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
>>
>
> Reviewed-by: Rob Herring <[email protected]>
>
Hi
This tags version 2 with a little mistake instead of version 3?
Is that correct?
Johan