* [PATCH v2 1/6] dt-bindings: pwm: amlogic: fix s4 bindings
2023-11-17 12:59 [PATCH v2 0/6] pwm: meson: dt-bindings fixup Jerome Brunet
@ 2023-11-17 12:59 ` Jerome Brunet
2023-11-19 16:04 ` Rob Herring
2023-11-17 12:59 ` [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type Jerome Brunet
` (4 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-17 12:59 UTC (permalink / raw)
To: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Jerome Brunet, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
s4 has been added to the compatible list while converting the Amlogic PWM
binding documentation from txt to yaml.
However, on the s4, the clock bindings have different meaning compared to
the previous SoCs.
On the previous SoCs the clock bindings used to describe which input the
PWM channel multiplexer should pick among its possible parents.
This is very much tied to the driver implementation, instead of describing
the HW for what it is. When support for the Amlogic PWM was first added,
how to deal with clocks through DT was not as clear as it nowadays.
The Linux driver now ignores this DT setting, but still relies on the
hard-coded list of clock sources.
On the s4, the input multiplexer is gone. The clock bindings actually
describe the clock as it exists, not a setting. The property has a
different meaning, even if it is still 2 clocks and it would pass the check
when support is actually added.
Also the s4 cannot work if the clocks are not provided, so the property no
longer optional.
Finally, for once it makes sense to see the input as being numbered
somehow. No need to bother with clock-names on the s4 type of PWM.
Fixes: 43a1c4ff3977 ("dt-bindings: pwm: Convert Amlogic Meson PWM binding")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
.../devicetree/bindings/pwm/pwm-amlogic.yaml | 69 ++++++++++++++++---
1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
index 527864a4d855..387976ed36d5 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
@@ -9,9 +9,6 @@ title: Amlogic PWM
maintainers:
- Heiner Kallweit <hkallweit1@gmail.com>
-allOf:
- - $ref: pwm.yaml#
-
properties:
compatible:
oneOf:
@@ -43,12 +40,8 @@ properties:
maxItems: 2
clock-names:
- oneOf:
- - items:
- - enum: [clkin0, clkin1]
- - items:
- - const: clkin0
- - const: clkin1
+ minItems: 1
+ maxItems: 2
"#pwm-cells":
const: 3
@@ -57,6 +50,57 @@ required:
- compatible
- reg
+allOf:
+ - $ref: pwm.yaml#
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,meson8-pwm
+ - amlogic,meson8b-pwm
+ - amlogic,meson-gxbb-pwm
+ - amlogic,meson-gxbb-ao-pwm
+ - amlogic,meson-axg-ee-pwm
+ - amlogic,meson-axg-ao-pwm
+ - amlogic,meson-g12a-ee-pwm
+ - amlogic,meson-g12a-ao-pwm-ab
+ - amlogic,meson-g12a-ao-pwm-cd
+ - amlogic,meson-gx-pwm
+ - amlogic,meson-gx-ao-pwm
+ then:
+ # Historic bindings tied to the driver implementation
+ # The clocks provided here are meant to be matched with the input
+ # known (hard-coded) in the driver and used to select pwm clock
+ # source. Currently, the linux driver ignores this.
+ properties:
+ clock-names:
+ oneOf:
+ - items:
+ - enum: [clkin0, clkin1]
+ - items:
+ - const: clkin0
+ - const: clkin1
+
+ # Newer IP block take a single input per channel, instead of 4 inputs
+ # for both channels
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,meson-s4-pwm
+ then:
+ properties:
+ clocks:
+ items:
+ - description: input clock of PWM channel A
+ - description: input clock of PWM channel B
+ clock-names: false
+ required:
+ - clocks
+
additionalProperties: false
examples:
@@ -68,3 +112,10 @@ examples:
clock-names = "clkin0", "clkin1";
#pwm-cells = <3>;
};
+ - |
+ pwm@1000 {
+ compatible = "amlogic,meson-s4-pwm";
+ reg = <0x1000 0x10>;
+ clocks = <&pwm_src_a>, <&pwm_src_b>;
+ #pwm-cells = <3>;
+ };
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 1/6] dt-bindings: pwm: amlogic: fix s4 bindings
2023-11-17 12:59 ` [PATCH v2 1/6] dt-bindings: pwm: amlogic: fix s4 bindings Jerome Brunet
@ 2023-11-19 16:04 ` Rob Herring
0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2023-11-19 16:04 UTC (permalink / raw)
To: Jerome Brunet
Cc: Conor Dooley, Neil Armstrong, devicetree, linux-amlogic,
Krzysztof Kozlowski, linux-kernel, linux-pwm, Kevin Hilman,
Rob Herring, Thierry Reding, JunYi Zhao
On Fri, 17 Nov 2023 13:59:11 +0100, Jerome Brunet wrote:
> s4 has been added to the compatible list while converting the Amlogic PWM
> binding documentation from txt to yaml.
>
> However, on the s4, the clock bindings have different meaning compared to
> the previous SoCs.
>
> On the previous SoCs the clock bindings used to describe which input the
> PWM channel multiplexer should pick among its possible parents.
>
> This is very much tied to the driver implementation, instead of describing
> the HW for what it is. When support for the Amlogic PWM was first added,
> how to deal with clocks through DT was not as clear as it nowadays.
> The Linux driver now ignores this DT setting, but still relies on the
> hard-coded list of clock sources.
>
> On the s4, the input multiplexer is gone. The clock bindings actually
> describe the clock as it exists, not a setting. The property has a
> different meaning, even if it is still 2 clocks and it would pass the check
> when support is actually added.
>
> Also the s4 cannot work if the clocks are not provided, so the property no
> longer optional.
>
> Finally, for once it makes sense to see the input as being numbered
> somehow. No need to bother with clock-names on the s4 type of PWM.
>
> Fixes: 43a1c4ff3977 ("dt-bindings: pwm: Convert Amlogic Meson PWM binding")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 69 ++++++++++++++++---
> 1 file changed, 60 insertions(+), 9 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-17 12:59 [PATCH v2 0/6] pwm: meson: dt-bindings fixup Jerome Brunet
2023-11-17 12:59 ` [PATCH v2 1/6] dt-bindings: pwm: amlogic: fix s4 bindings Jerome Brunet
@ 2023-11-17 12:59 ` Jerome Brunet
2023-11-19 16:05 ` Rob Herring
2023-11-17 12:59 ` [PATCH v2 3/6] pwm: meson: prepare addition of new compatible types Jerome Brunet
` (3 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-17 12:59 UTC (permalink / raw)
To: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Jerome Brunet, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.
The previous clock bindings for these SoCs described the driver and not the
HW itself. The clock provided was used to set the parent of the input clock
mux among the possible parents hard-coded in the driver.
The new bindings allows to describe the actual clock inputs of the PWM in
DT, like most bindings do, instead of relying of hard-coded data.
The new bindings make the old one deprecated.
There is enough experience on this HW to know that the PWM is exactly the
same all the supported SoCs. There is no need for a per-SoC compatible.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
.../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
index 387976ed36d5..48b11b7d5df6 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
@@ -22,6 +22,7 @@ properties:
- amlogic,meson-g12a-ao-pwm-ab
- amlogic,meson-g12a-ao-pwm-cd
- amlogic,meson-s4-pwm
+ - amlogic,meson8-pwm-v2
- items:
- const: amlogic,meson-gx-pwm
- const: amlogic,meson-gxbb-pwm
@@ -37,7 +38,7 @@ properties:
clocks:
minItems: 1
- maxItems: 2
+ maxItems: 4
clock-names:
minItems: 1
@@ -70,11 +71,14 @@ allOf:
- amlogic,meson-gx-pwm
- amlogic,meson-gx-ao-pwm
then:
- # Historic bindings tied to the driver implementation
+ # Obsolete historic bindings tied to the driver implementation
# The clocks provided here are meant to be matched with the input
# known (hard-coded) in the driver and used to select pwm clock
# source. Currently, the linux driver ignores this.
+ deprecated: true
properties:
+ clocks:
+ maxItems: 2
clock-names:
oneOf:
- items:
@@ -83,6 +87,27 @@ allOf:
- const: clkin0
- const: clkin1
+ # Newer binding where clock describe the actual clock inputs of the pwm
+ # block. These are necessary but some inputs may be grounded.
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,meson8-pwm-v2
+ then:
+ properties:
+ clocks:
+ minItems: 1
+ items:
+ - description: input clock 0 of the pwm block
+ - description: input clock 1 of the pwm block
+ - description: input clock 2 of the pwm block
+ - description: input clock 3 of the pwm block
+ clock-names: false
+ required:
+ - clocks
+
# Newer IP block take a single input per channel, instead of 4 inputs
# for both channels
- if:
@@ -112,6 +137,13 @@ examples:
clock-names = "clkin0", "clkin1";
#pwm-cells = <3>;
};
+ - |
+ pwm@2000 {
+ compatible = "amlogic,meson8-pwm-v2";
+ reg = <0x1000 0x10>;
+ clocks = <&xtal>, <0>, <&fdiv4>, <&fdiv5>;
+ #pwm-cells = <3>;
+ };
- |
pwm@1000 {
compatible = "amlogic,meson-s4-pwm";
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-17 12:59 ` [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type Jerome Brunet
@ 2023-11-19 16:05 ` Rob Herring
2023-11-20 8:27 ` Neil Armstrong
0 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2023-11-19 16:05 UTC (permalink / raw)
To: Jerome Brunet
Cc: JunYi Zhao, devicetree, Neil Armstrong, Rob Herring, Conor Dooley,
Kevin Hilman, Thierry Reding, linux-kernel, linux-pwm,
linux-amlogic, Krzysztof Kozlowski
On Fri, 17 Nov 2023 13:59:12 +0100, Jerome Brunet wrote:
> Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.
>
> The previous clock bindings for these SoCs described the driver and not the
> HW itself. The clock provided was used to set the parent of the input clock
> mux among the possible parents hard-coded in the driver.
>
> The new bindings allows to describe the actual clock inputs of the PWM in
> DT, like most bindings do, instead of relying of hard-coded data.
>
> The new bindings make the old one deprecated.
>
> There is enough experience on this HW to know that the PWM is exactly the
> same all the supported SoCs. There is no need for a per-SoC compatible.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-19 16:05 ` Rob Herring
@ 2023-11-20 8:27 ` Neil Armstrong
2023-11-20 9:18 ` Jerome Brunet
0 siblings, 1 reply; 24+ messages in thread
From: Neil Armstrong @ 2023-11-20 8:27 UTC (permalink / raw)
To: Rob Herring, Jerome Brunet
Cc: JunYi Zhao, devicetree, Rob Herring, Conor Dooley, Kevin Hilman,
Thierry Reding, linux-kernel, linux-pwm, linux-amlogic,
Krzysztof Kozlowski
Hi Rob,
On 19/11/2023 17:05, Rob Herring wrote:
>
> On Fri, 17 Nov 2023 13:59:12 +0100, Jerome Brunet wrote:
>> Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.
>>
>> The previous clock bindings for these SoCs described the driver and not the
>> HW itself. The clock provided was used to set the parent of the input clock
>> mux among the possible parents hard-coded in the driver.
>>
>> The new bindings allows to describe the actual clock inputs of the PWM in
>> DT, like most bindings do, instead of relying of hard-coded data.
>>
>> The new bindings make the old one deprecated.
>>
>> There is enough experience on this HW to know that the PWM is exactly the
>> same all the supported SoCs. There is no need for a per-SoC compatible.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
I'm puzzled, isn't it recommended to have a per-soc compatible now ?
I thought something like:
- items:
- enum:
- amlogic,gxbb-pwm
- amlogic,axg-pwm
- amlogic,g12a-pwm
- const: amlogic,pwm-v1
should be preferred instead of a single amlogic,meson8-pwm-v2 ?
Neil
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-20 8:27 ` Neil Armstrong
@ 2023-11-20 9:18 ` Jerome Brunet
2023-11-20 9:55 ` neil.armstrong
0 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-20 9:18 UTC (permalink / raw)
To: neil.armstrong
Cc: Rob Herring, Jerome Brunet, JunYi Zhao, devicetree, Rob Herring,
Conor Dooley, Kevin Hilman, Thierry Reding, linux-kernel,
linux-pwm, linux-amlogic, Krzysztof Kozlowski
On Mon 20 Nov 2023 at 09:27, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> Hi Rob,
>
> On 19/11/2023 17:05, Rob Herring wrote:
>> On Fri, 17 Nov 2023 13:59:12 +0100, Jerome Brunet wrote:
>>> Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.
>>>
>>> The previous clock bindings for these SoCs described the driver and not the
>>> HW itself. The clock provided was used to set the parent of the input clock
>>> mux among the possible parents hard-coded in the driver.
>>>
>>> The new bindings allows to describe the actual clock inputs of the PWM in
>>> DT, like most bindings do, instead of relying of hard-coded data.
>>>
>>> The new bindings make the old one deprecated.
>>>
>>> There is enough experience on this HW to know that the PWM is exactly the
>>> same all the supported SoCs. There is no need for a per-SoC compatible.
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>>
>
> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
I have specifically addressed this matter in the description,
haven't I ? What good would it do in this case ?
Plus the definition of a SoC is very vague. One could argue that
the content of the list bellow are vaguely defined families. Should we
add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
This list gets huge for no reason.
We know all existing PWM of this type are the same. We have been using
them for years. It is not a new support we know nothing about.
>
> I thought something like:
> - items:
> - enum:
> - amlogic,gxbb-pwm
> - amlogic,axg-pwm
> - amlogic,g12a-pwm
> - const: amlogic,pwm-v1
I'm not sure I understand what you are suggesting here.
Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
has that and I'm working to remove this type, so I don't get the point.
>
> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
This is named after the first SoC supporting the type.
Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
Plus the doc specifically advise against this type of names.
>
> Neil
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-20 9:18 ` Jerome Brunet
@ 2023-11-20 9:55 ` neil.armstrong
2023-11-20 10:04 ` Jerome Brunet
0 siblings, 1 reply; 24+ messages in thread
From: neil.armstrong @ 2023-11-20 9:55 UTC (permalink / raw)
To: Jerome Brunet
Cc: Rob Herring, JunYi Zhao, devicetree, Rob Herring, Conor Dooley,
Kevin Hilman, Thierry Reding, linux-kernel, linux-pwm,
linux-amlogic, Krzysztof Kozlowski
Hi Jerome,
On 20/11/2023 10:18, Jerome Brunet wrote:
>
> On Mon 20 Nov 2023 at 09:27, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
>> Hi Rob,
>>
>> On 19/11/2023 17:05, Rob Herring wrote:
>>> On Fri, 17 Nov 2023 13:59:12 +0100, Jerome Brunet wrote:
>>>> Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.
>>>>
>>>> The previous clock bindings for these SoCs described the driver and not the
>>>> HW itself. The clock provided was used to set the parent of the input clock
>>>> mux among the possible parents hard-coded in the driver.
>>>>
>>>> The new bindings allows to describe the actual clock inputs of the PWM in
>>>> DT, like most bindings do, instead of relying of hard-coded data.
>>>>
>>>> The new bindings make the old one deprecated.
>>>>
>>>> There is enough experience on this HW to know that the PWM is exactly the
>>>> same all the supported SoCs. There is no need for a per-SoC compatible.
>>>>
>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>> ---
>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>
>>
>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
>
> I have specifically addressed this matter in the description,
> haven't I ? What good would it do in this case ?
Yes you did but I was asked for the last year+ that all new compatible
should be soc specific (while imprecise, in our care soc family should be ok),
with a possible semi-generic callback with an IP version or a first soc
implementing the IP.
>
> Plus the definition of a SoC is very vague. One could argue that
> the content of the list bellow are vaguely defined families. Should we
> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
> This list gets huge for no reason.
I think in our case soc family is reasonable since they share same silicon
design.
>
> We know all existing PWM of this type are the same. We have been using
> them for years. It is not a new support we know nothing about.
>
>>
>> I thought something like:
>> - items:
>> - enum:
>> - amlogic,gxbb-pwm
>> - amlogic,axg-pwm
>> - amlogic,g12a-pwm
>> - const: amlogic,pwm-v1
>
> I'm not sure I understand what you are suggesting here.
> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
> has that and I'm working to remove this type, so I don't get the point.
>
>>
>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>
> This is named after the first SoC supporting the type.
>
> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
> Plus the doc specifically advise against this type of names.
The -v2 refers to a pure software/dt implementation versioning and not
an HW version, so I'm puzzled and I requires DT maintainers advice here.
Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
reasonable and s4 and later pwm could use the "amlogic,pwm-v2" fallback.
Neil
>
>>
>> Neil
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-20 9:55 ` neil.armstrong
@ 2023-11-20 10:04 ` Jerome Brunet
2023-11-22 8:37 ` Krzysztof Kozlowski
0 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-20 10:04 UTC (permalink / raw)
To: neil.armstrong
Cc: Jerome Brunet, Rob Herring, JunYi Zhao, devicetree, Rob Herring,
Conor Dooley, Kevin Hilman, Thierry Reding, linux-kernel,
linux-pwm, linux-amlogic, Krzysztof Kozlowski
On Mon 20 Nov 2023 at 10:55, neil.armstrong@linaro.org wrote:
> Hi Jerome,
>
> On 20/11/2023 10:18, Jerome Brunet wrote:
>> On Mon 20 Nov 2023 at 09:27, Neil Armstrong <neil.armstrong@linaro.org>
>> wrote:
>>
>>> Hi Rob,
>>>
>>> On 19/11/2023 17:05, Rob Herring wrote:
>>>> On Fri, 17 Nov 2023 13:59:12 +0100, Jerome Brunet wrote:
>>>>> Add a new compatible for the pwm found in the meson8 to sm1 Amlogic SoCs.
>>>>>
>>>>> The previous clock bindings for these SoCs described the driver and not the
>>>>> HW itself. The clock provided was used to set the parent of the input clock
>>>>> mux among the possible parents hard-coded in the driver.
>>>>>
>>>>> The new bindings allows to describe the actual clock inputs of the PWM in
>>>>> DT, like most bindings do, instead of relying of hard-coded data.
>>>>>
>>>>> The new bindings make the old one deprecated.
>>>>>
>>>>> There is enough experience on this HW to know that the PWM is exactly the
>>>>> same all the supported SoCs. There is no need for a per-SoC compatible.
>>>>>
>>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>>> ---
>>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>>
>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>
>>>
>>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
>> I have specifically addressed this matter in the description,
>> haven't I ? What good would it do in this case ?
>
> Yes you did but I was asked for the last year+ that all new compatible
> should be soc specific (while imprecise, in our care soc family should be ok),
> with a possible semi-generic callback with an IP version or a first soc
> implementing the IP.
>
>> Plus the definition of a SoC is very vague. One could argue that
>> the content of the list bellow are vaguely defined families. Should we
>> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
>> This list gets huge for no reason.
>
> I think in our case soc family is reasonable since they share same silicon
> design.
>
>> We know all existing PWM of this type are the same. We have been using
>> them for years. It is not a new support we know nothing about.
>>
>>>
>>> I thought something like:
>>> - items:
>>> - enum:
>>> - amlogic,gxbb-pwm
>>> - amlogic,axg-pwm
>>> - amlogic,g12a-pwm
>>> - const: amlogic,pwm-v1
>> I'm not sure I understand what you are suggesting here.
>> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
>> has that and I'm working to remove this type, so I don't get the point.
>>
>>>
>>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>> This is named after the first SoC supporting the type.
>> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
>> Plus the doc specifically advise against this type of names.
>
> The -v2 refers to a pure software/dt implementation versioning and not
> an HW version, so I'm puzzled and I requires DT maintainers advice here.
>
> Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
This is not my point. I picked this name because I have to pick a
specific device based one. Not because it is actually the first or
not. I don't see a problem with meson6 being compatible with
meson8-pwm-v2, if that ever comes along.
I think the binding here satisfy the rule that it should be specific,
and the intent that goes with it:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n42
> the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
> reasonable and s4 and later pwm could use the "amlogic,pwm-v2"
> fallback.
That is not how understand this:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n82
>
> Neil
>>
>>>
>>> Neil
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-20 10:04 ` Jerome Brunet
@ 2023-11-22 8:37 ` Krzysztof Kozlowski
2023-11-22 14:34 ` Jerome Brunet
0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 8:37 UTC (permalink / raw)
To: Jerome Brunet, neil.armstrong
Cc: Rob Herring, JunYi Zhao, devicetree, Rob Herring, Conor Dooley,
Kevin Hilman, Thierry Reding, linux-kernel, linux-pwm,
linux-amlogic, Krzysztof Kozlowski
On 20/11/2023 11:04, Jerome Brunet wrote:
>>>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>>>
>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>
>>>>
>>>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
Yes, it is.
>>> I have specifically addressed this matter in the description,
>>> haven't I ? What good would it do in this case ?
There is nothing about compatible naming in commit msg.
>>
>> Yes you did but I was asked for the last year+ that all new compatible
>> should be soc specific (while imprecise, in our care soc family should be ok),
>> with a possible semi-generic callback with an IP version or a first soc
>> implementing the IP.
>>
>>> Plus the definition of a SoC is very vague. One could argue that
>>> the content of the list bellow are vaguely defined families. Should we
>>> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
>>> This list gets huge for no reason.
>>
>> I think in our case soc family is reasonable since they share same silicon
>> design.
>>
>>> We know all existing PWM of this type are the same. We have been using
>>> them for years. It is not a new support we know nothing about.
>>>
>>>>
>>>> I thought something like:
>>>> - items:
>>>> - enum:
>>>> - amlogic,gxbb-pwm
>>>> - amlogic,axg-pwm
>>>> - amlogic,g12a-pwm
>>>> - const: amlogic,pwm-v1
>>> I'm not sure I understand what you are suggesting here.
>>> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
>>> has that and I'm working to remove this type, so I don't get the point.
>>>
>>>>
>>>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>>> This is named after the first SoC supporting the type.
>>> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
>>> Plus the doc specifically advise against this type of names.
>>
>> The -v2 refers to a pure software/dt implementation versioning and not
>> an HW version, so I'm puzzled and I requires DT maintainers advice here.
>>
>> Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
Yes, this should be SoC-based compatible, unless you have clear
versioning scheme by SoC/IP block vendor. You named it not a HW version,
which kind of answers to the "unless" case - that's not hardware version.
>
> This is not my point. I picked this name because I have to pick a
> specific device based one. Not because it is actually the first or
> not. I don't see a problem with meson6 being compatible with
> meson8-pwm-v2, if that ever comes along.
No, the point is not to use "v2". Use SoC compatibles.
>
> I think the binding here satisfy the rule that it should be specific,
> and the intent that goes with it:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n42
>
>> the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
>> reasonable and s4 and later pwm could use the "amlogic,pwm-v2"
>> fallback.
>
> That is not how understand this:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n82
>
Again, where the "v2" is defined? Where is any document explaining the
mapping between version blocks and SoC parts? Why do you list here only
major version? Blocks almost always have also minor (e.g. v2.0).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-22 8:37 ` Krzysztof Kozlowski
@ 2023-11-22 14:34 ` Jerome Brunet
2023-11-22 15:04 ` Krzysztof Kozlowski
0 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-22 14:34 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jerome Brunet, neil.armstrong, Rob Herring, JunYi Zhao,
devicetree, Rob Herring, Conor Dooley, Kevin Hilman,
Thierry Reding, linux-kernel, linux-pwm, linux-amlogic,
Krzysztof Kozlowski
On Wed 22 Nov 2023 at 09:37, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 20/11/2023 11:04, Jerome Brunet wrote:
>>>>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>
>>>>>
>>>>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
>
> Yes, it is.
>
>>>> I have specifically addressed this matter in the description,
>>>> haven't I ? What good would it do in this case ?
>
> There is nothing about compatible naming in commit msg.
Krzysztof, the whole commit desciption is explanation about why a new
compatible is introduced. I don't understand this comment.
>
>>>
>>> Yes you did but I was asked for the last year+ that all new compatible
>>> should be soc specific (while imprecise, in our care soc family should be ok),
>>> with a possible semi-generic callback with an IP version or a first soc
>>> implementing the IP.
>>>
>>>> Plus the definition of a SoC is very vague. One could argue that
>>>> the content of the list bellow are vaguely defined families. Should we
>>>> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
>>>> This list gets huge for no reason.
>>>
>>> I think in our case soc family is reasonable since they share same silicon
>>> design.
>>>
>>>> We know all existing PWM of this type are the same. We have been using
>>>> them for years. It is not a new support we know nothing about.
>>>>
>>>>>
>>>>> I thought something like:
>>>>> - items:
>>>>> - enum:
>>>>> - amlogic,gxbb-pwm
>>>>> - amlogic,axg-pwm
>>>>> - amlogic,g12a-pwm
>>>>> - const: amlogic,pwm-v1
>>>> I'm not sure I understand what you are suggesting here.
>>>> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
>>>> has that and I'm working to remove this type, so I don't get the point.
>>>>
>>>>>
>>>>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>>>> This is named after the first SoC supporting the type.
>>>> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
>>>> Plus the doc specifically advise against this type of names.
>>>
>>> The -v2 refers to a pure software/dt implementation versioning and not
>>> an HW version, so I'm puzzled and I requires DT maintainers advice here.
>>>
>>> Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
>
> Yes, this should be SoC-based compatible, unless you have clear
> versioning scheme by SoC/IP block vendor. You named it not a HW version,
> which kind of answers to the "unless" case - that's not hardware version.
>
This is specifically the point of the comment in commit description.
We know all the PWMs compatible are the same HW (version) as one found
in the meson8b.
It is certain that adding more compatible, listing all the SoC, will be
useless. I can do it if you insist.
>>
>> This is not my point. I picked this name because I have to pick a
>> specific device based one. Not because it is actually the first or
>> not. I don't see a problem with meson6 being compatible with
>> meson8-pwm-v2, if that ever comes along.
>
> No, the point is not to use "v2". Use SoC compatibles.
It is a SoC compatible. The second one.
The first one, as explained in the description was describing the driver
more that the HW.
Changing the way clock are passed from DT to the driver would be break
user of the old compatible. So a new compatible is introduced. I believe
this is recommended way to introduce incompatible binding changes.
v2 here denote a new interface version, nothing to do with HW
versioning. I happy to pick something else to denote this.
>
>>
>> I think the binding here satisfy the rule that it should be specific,
>> and the intent that goes with it:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n42
>>
>>> the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
>>> reasonable and s4 and later pwm could use the "amlogic,pwm-v2"
>>> fallback.
>>
>> That is not how understand this:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n82
>>
>
> Again, where the "v2" is defined? Where is any document explaining the
> mapping between version blocks and SoC parts? Why do you list here only
> major version? Blocks almost always have also minor (e.g. v2.0).
Again, v2 does has nothing to do with the HW. Never wrote it was.
The HW remains the same.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-22 14:34 ` Jerome Brunet
@ 2023-11-22 15:04 ` Krzysztof Kozlowski
2023-11-22 15:23 ` Jerome Brunet
0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 15:04 UTC (permalink / raw)
To: Jerome Brunet
Cc: neil.armstrong, Rob Herring, JunYi Zhao, devicetree, Rob Herring,
Conor Dooley, Kevin Hilman, Thierry Reding, linux-kernel,
linux-pwm, linux-amlogic, Krzysztof Kozlowski
On 22/11/2023 15:34, Jerome Brunet wrote:
>
> On Wed 22 Nov 2023 at 09:37, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>
>> On 20/11/2023 11:04, Jerome Brunet wrote:
>>>>>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>>>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>>
>>>>>>
>>>>>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
>>
>> Yes, it is.
>>
>>>>> I have specifically addressed this matter in the description,
>>>>> haven't I ? What good would it do in this case ?
>>
>> There is nothing about compatible naming in commit msg.
>
> Krzysztof, the whole commit desciption is explanation about why a new
> compatible is introduced. I don't understand this comment.
>
>>
>>>>
>>>> Yes you did but I was asked for the last year+ that all new compatible
>>>> should be soc specific (while imprecise, in our care soc family should be ok),
>>>> with a possible semi-generic callback with an IP version or a first soc
>>>> implementing the IP.
>>>>
>>>>> Plus the definition of a SoC is very vague. One could argue that
>>>>> the content of the list bellow are vaguely defined families. Should we
>>>>> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
>>>>> This list gets huge for no reason.
>>>>
>>>> I think in our case soc family is reasonable since they share same silicon
>>>> design.
>>>>
>>>>> We know all existing PWM of this type are the same. We have been using
>>>>> them for years. It is not a new support we know nothing about.
>>>>>
>>>>>>
>>>>>> I thought something like:
>>>>>> - items:
>>>>>> - enum:
>>>>>> - amlogic,gxbb-pwm
>>>>>> - amlogic,axg-pwm
>>>>>> - amlogic,g12a-pwm
>>>>>> - const: amlogic,pwm-v1
>>>>> I'm not sure I understand what you are suggesting here.
>>>>> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
>>>>> has that and I'm working to remove this type, so I don't get the point.
>>>>>
>>>>>>
>>>>>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>>>>> This is named after the first SoC supporting the type.
>>>>> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
>>>>> Plus the doc specifically advise against this type of names.
>>>>
>>>> The -v2 refers to a pure software/dt implementation versioning and not
>>>> an HW version, so I'm puzzled and I requires DT maintainers advice here.
>>>>
>>>> Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
>>
>> Yes, this should be SoC-based compatible, unless you have clear
>> versioning scheme by SoC/IP block vendor. You named it not a HW version,
>> which kind of answers to the "unless" case - that's not hardware version.
>>
>
> This is specifically the point of the comment in commit description.
> We know all the PWMs compatible are the same HW (version) as one found
> in the meson8b.
>
> It is certain that adding more compatible, listing all the SoC, will be
> useless. I can do it if you insist.
The docs you references insist on that, so yeah, I insist as well.
>
>>>
>>> This is not my point. I picked this name because I have to pick a
>>> specific device based one. Not because it is actually the first or
>>> not. I don't see a problem with meson6 being compatible with
>>> meson8-pwm-v2, if that ever comes along.
>>
>> No, the point is not to use "v2". Use SoC compatibles.
>
> It is a SoC compatible. The second one.
"v2" is not the soc. I assume meson8 is one specific SoC, right? Because
elinux says it is a *family*:
https://elinux.org/Amlogic/SoCs
>
> The first one, as explained in the description was describing the driver
> more that the HW.
>
> Changing the way clock are passed from DT to the driver would be break
> user of the old compatible. So a new compatible is introduced. I believe
> this is recommended way to introduce incompatible binding changes.
The way is not to introduce incompatible changes. Your way is also not
good as it breaks other users of DTS.
>
> v2 here denote a new interface version, nothing to do with HW
> versioning. I happy to pick something else to denote this.
Sorry, then it is not a SoC-based compatible.
>
>>
>>>
>>> I think the binding here satisfy the rule that it should be specific,
>>> and the intent that goes with it:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n42
>>>
>>>> the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
>>>> reasonable and s4 and later pwm could use the "amlogic,pwm-v2"
>>>> fallback.
>>>
>>> That is not how understand this:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n82
>>>
>>
>> Again, where the "v2" is defined? Where is any document explaining the
>> mapping between version blocks and SoC parts? Why do you list here only
>> major version? Blocks almost always have also minor (e.g. v2.0).
>
> Again, v2 does has nothing to do with the HW. Never wrote it was.
> The HW remains the same.
Don't add compatibles which are not related to HW, but represent
software versioning. Software does not matter for the bindings.
That's a clear NAK.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-22 15:04 ` Krzysztof Kozlowski
@ 2023-11-22 15:23 ` Jerome Brunet
2023-11-22 15:46 ` Krzysztof Kozlowski
0 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-22 15:23 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jerome Brunet, neil.armstrong, Rob Herring, JunYi Zhao,
devicetree, Rob Herring, Conor Dooley, Kevin Hilman,
Thierry Reding, linux-kernel, linux-pwm, linux-amlogic,
Krzysztof Kozlowski
On Wed 22 Nov 2023 at 16:04, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 22/11/2023 15:34, Jerome Brunet wrote:
>>
>> On Wed 22 Nov 2023 at 09:37, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 20/11/2023 11:04, Jerome Brunet wrote:
>>>>>>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>>>>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>>>
>>>>>>>
>>>>>>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
>>>
>>> Yes, it is.
>>>
>>>>>> I have specifically addressed this matter in the description,
>>>>>> haven't I ? What good would it do in this case ?
>>>
>>> There is nothing about compatible naming in commit msg.
>>
>> Krzysztof, the whole commit desciption is explanation about why a new
>> compatible is introduced. I don't understand this comment.
>>
>>>
>>>>>
>>>>> Yes you did but I was asked for the last year+ that all new compatible
>>>>> should be soc specific (while imprecise, in our care soc family should be ok),
>>>>> with a possible semi-generic callback with an IP version or a first soc
>>>>> implementing the IP.
>>>>>
>>>>>> Plus the definition of a SoC is very vague. One could argue that
>>>>>> the content of the list bellow are vaguely defined families. Should we
>>>>>> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
>>>>>> This list gets huge for no reason.
>>>>>
>>>>> I think in our case soc family is reasonable since they share same silicon
>>>>> design.
>>>>>
>>>>>> We know all existing PWM of this type are the same. We have been using
>>>>>> them for years. It is not a new support we know nothing about.
>>>>>>
>>>>>>>
>>>>>>> I thought something like:
>>>>>>> - items:
>>>>>>> - enum:
>>>>>>> - amlogic,gxbb-pwm
>>>>>>> - amlogic,axg-pwm
>>>>>>> - amlogic,g12a-pwm
>>>>>>> - const: amlogic,pwm-v1
>>>>>> I'm not sure I understand what you are suggesting here.
>>>>>> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
>>>>>> has that and I'm working to remove this type, so I don't get the point.
>>>>>>
>>>>>>>
>>>>>>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>>>>>> This is named after the first SoC supporting the type.
>>>>>> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
>>>>>> Plus the doc specifically advise against this type of names.
>>>>>
>>>>> The -v2 refers to a pure software/dt implementation versioning and not
>>>>> an HW version, so I'm puzzled and I requires DT maintainers advice here.
>>>>>
>>>>> Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
>>>
>>> Yes, this should be SoC-based compatible, unless you have clear
>>> versioning scheme by SoC/IP block vendor. You named it not a HW version,
>>> which kind of answers to the "unless" case - that's not hardware version.
>>>
>>
>> This is specifically the point of the comment in commit description.
>> We know all the PWMs compatible are the same HW (version) as one found
>> in the meson8b.
>>
>> It is certain that adding more compatible, listing all the SoC, will be
>> useless. I can do it if you insist.
>
> The docs you references insist on that, so yeah, I insist as well.
>
>>
>>>>
>>>> This is not my point. I picked this name because I have to pick a
>>>> specific device based one. Not because it is actually the first or
>>>> not. I don't see a problem with meson6 being compatible with
>>>> meson8-pwm-v2, if that ever comes along.
>>>
>>> No, the point is not to use "v2". Use SoC compatibles.
>>
>> It is a SoC compatible. The second one.
>
> "v2" is not the soc. I assume meson8 is one specific SoC, right? Because
> elinux says it is a *family*:
> https://elinux.org/Amlogic/SoCs
>
It is a family. yes
>
>>
>> The first one, as explained in the description was describing the driver
>> more that the HW.
>>
>> Changing the way clock are passed from DT to the driver would be break
>> user of the old compatible. So a new compatible is introduced. I believe
>> this is recommended way to introduce incompatible binding changes.
>
> The way is not to introduce incompatible changes. Your way is also not
> good as it breaks other users of DTS.
>
>>
>> v2 here denote a new interface version, nothing to do with HW
>> versioning. I happy to pick something else to denote this.
>
> Sorry, then it is not a SoC-based compatible.
>
>>
>>>
>>>>
>>>> I think the binding here satisfy the rule that it should be specific,
>>>> and the intent that goes with it:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n42
>>>>
>>>>> the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
>>>>> reasonable and s4 and later pwm could use the "amlogic,pwm-v2"
>>>>> fallback.
>>>>
>>>> That is not how understand this:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n82
>>>>
>>>
>>> Again, where the "v2" is defined? Where is any document explaining the
>>> mapping between version blocks and SoC parts? Why do you list here only
>>> major version? Blocks almost always have also minor (e.g. v2.0).
>>
>> Again, v2 does has nothing to do with the HW. Never wrote it was.
>> The HW remains the same.
>
> Don't add compatibles which are not related to HW, but represent
> software versioning. Software does not matter for the bindings.
What I did I explicitly what is recommended in Grant's presentation from
2013. 10y old, but I assume slide 10 "Making an incompatible update" is
still valid.
https://elinux.org/images/1/1e/DT_Binding_Process_glikely_ksummit_2013_10_28.pdf
Breaking the ABI of the old compatible would break all boards which use
u-boot DT and pass it to the kernel, because the meaning of the clock
property would change.
Doing things has suggested in this slide, and this patch, allows every
device to continue to work properly, whether the DT given is the one
shipped with u-boot (using the old compatible for now) or the kernel.
>
> That's a clear NAK.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-22 15:23 ` Jerome Brunet
@ 2023-11-22 15:46 ` Krzysztof Kozlowski
2023-11-22 16:14 ` Jerome Brunet
0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 15:46 UTC (permalink / raw)
To: Jerome Brunet
Cc: neil.armstrong, Rob Herring, JunYi Zhao, devicetree, Rob Herring,
Conor Dooley, Kevin Hilman, Thierry Reding, linux-kernel,
linux-pwm, linux-amlogic, Krzysztof Kozlowski
On 22/11/2023 16:23, Jerome Brunet wrote:
>
> On Wed 22 Nov 2023 at 16:04, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>
>> On 22/11/2023 15:34, Jerome Brunet wrote:
>>>
>>> On Wed 22 Nov 2023 at 09:37, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>>> On 20/11/2023 11:04, Jerome Brunet wrote:
>>>>>>>>>> .../devicetree/bindings/pwm/pwm-amlogic.yaml | 36 +++++++++++++++++--
>>>>>>>>>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'm puzzled, isn't it recommended to have a per-soc compatible now ?
>>>>
>>>> Yes, it is.
>>>>
>>>>>>> I have specifically addressed this matter in the description,
>>>>>>> haven't I ? What good would it do in this case ?
>>>>
>>>> There is nothing about compatible naming in commit msg.
>>>
>>> Krzysztof, the whole commit desciption is explanation about why a new
>>> compatible is introduced. I don't understand this comment.
>>>
>>>>
>>>>>>
>>>>>> Yes you did but I was asked for the last year+ that all new compatible
>>>>>> should be soc specific (while imprecise, in our care soc family should be ok),
>>>>>> with a possible semi-generic callback with an IP version or a first soc
>>>>>> implementing the IP.
>>>>>>
>>>>>>> Plus the definition of a SoC is very vague. One could argue that
>>>>>>> the content of the list bellow are vaguely defined families. Should we
>>>>>>> add meson8b, gxl, gxm, sm1 ? ... or even the actual SoC reference ?
>>>>>>> This list gets huge for no reason.
>>>>>>
>>>>>> I think in our case soc family is reasonable since they share same silicon
>>>>>> design.
>>>>>>
>>>>>>> We know all existing PWM of this type are the same. We have been using
>>>>>>> them for years. It is not a new support we know nothing about.
>>>>>>>
>>>>>>>>
>>>>>>>> I thought something like:
>>>>>>>> - items:
>>>>>>>> - enum:
>>>>>>>> - amlogic,gxbb-pwm
>>>>>>>> - amlogic,axg-pwm
>>>>>>>> - amlogic,g12a-pwm
>>>>>>>> - const: amlogic,pwm-v1
>>>>>>> I'm not sure I understand what you are suggesting here.
>>>>>>> Adding a "amlogic,pwm-v1" for the obsolete compatible ? No amlogic DT
>>>>>>> has that and I'm working to remove this type, so I don't get the point.
>>>>>>>
>>>>>>>>
>>>>>>>> should be preferred instead of a single amlogic,meson8-pwm-v2 ?
>>>>>>> This is named after the first SoC supporting the type.
>>>>>>> Naming it amlogic,pwm-v2 would feel weird with the s4 coming after.
>>>>>>> Plus the doc specifically advise against this type of names.
>>>>>>
>>>>>> The -v2 refers to a pure software/dt implementation versioning and not
>>>>>> an HW version, so I'm puzzled and I requires DT maintainers advice here.
>>>>>>
>>>>>> Yes meson8b is the first "known" platform, even if I'm pretty sure meson6 has
>>>>
>>>> Yes, this should be SoC-based compatible, unless you have clear
>>>> versioning scheme by SoC/IP block vendor. You named it not a HW version,
>>>> which kind of answers to the "unless" case - that's not hardware version.
>>>>
>>>
>>> This is specifically the point of the comment in commit description.
>>> We know all the PWMs compatible are the same HW (version) as one found
>>> in the meson8b.
>>>
>>> It is certain that adding more compatible, listing all the SoC, will be
>>> useless. I can do it if you insist.
>>
>> The docs you references insist on that, so yeah, I insist as well.
>>
>>>
>>>>>
>>>>> This is not my point. I picked this name because I have to pick a
>>>>> specific device based one. Not because it is actually the first or
>>>>> not. I don't see a problem with meson6 being compatible with
>>>>> meson8-pwm-v2, if that ever comes along.
>>>>
>>>> No, the point is not to use "v2". Use SoC compatibles.
>>>
>>> It is a SoC compatible. The second one.
>>
>> "v2" is not the soc. I assume meson8 is one specific SoC, right? Because
>> elinux says it is a *family*:
>> https://elinux.org/Amlogic/SoCs
>>
>
> It is a family. yes
>
>>
>>>
>>> The first one, as explained in the description was describing the driver
>>> more that the HW.
>>>
>>> Changing the way clock are passed from DT to the driver would be break
>>> user of the old compatible. So a new compatible is introduced. I believe
>>> this is recommended way to introduce incompatible binding changes.
>>
>> The way is not to introduce incompatible changes. Your way is also not
>> good as it breaks other users of DTS.
>>
>>>
>>> v2 here denote a new interface version, nothing to do with HW
>>> versioning. I happy to pick something else to denote this.
>>
>> Sorry, then it is not a SoC-based compatible.
>>
>>>
>>>>
>>>>>
>>>>> I think the binding here satisfy the rule that it should be specific,
>>>>> and the intent that goes with it:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n42
>>>>>
>>>>>> the same pwm architecture, this is why "amlogic,pwm-v1" as fallback seems more
>>>>>> reasonable and s4 and later pwm could use the "amlogic,pwm-v2"
>>>>>> fallback.
>>>>>
>>>>> That is not how understand this:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.7-rc2#n82
>>>>>
>>>>
>>>> Again, where the "v2" is defined? Where is any document explaining the
>>>> mapping between version blocks and SoC parts? Why do you list here only
>>>> major version? Blocks almost always have also minor (e.g. v2.0).
>>>
>>> Again, v2 does has nothing to do with the HW. Never wrote it was.
>>> The HW remains the same.
>>
>> Don't add compatibles which are not related to HW, but represent
>> software versioning. Software does not matter for the bindings.
>
> What I did I explicitly what is recommended in Grant's presentation from
> 2013. 10y old, but I assume slide 10 "Making an incompatible update" is
> still valid.
>
> https://elinux.org/images/1/1e/DT_Binding_Process_glikely_ksummit_2013_10_28.pdf
>
> Breaking the ABI of the old compatible would break all boards which use
> u-boot DT and pass it to the kernel, because the meaning of the clock
> property would change.
You broke U-Boot now as well - it will get your new DTS from the kernel
and stop working.
>
> Doing things has suggested in this slide, and this patch, allows every
> device to continue to work properly, whether the DT given is the one
> shipped with u-boot (using the old compatible for now) or the kernel.
OK, that explains the reasons. I read your commit msg and nothing like
this was mentioned there. What's more, you did not deprecate the old
binding, thus the confusion - it looked like you add entirely new
hardware (although you put "deprecated" but in some unrelated place, not
next to the compatibles).
Anyway, the main point of Neil was that you started using generic
compatible for all SoCs, which is wrong as well. I guess this was the
original discussion.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-22 15:46 ` Krzysztof Kozlowski
@ 2023-11-22 16:14 ` Jerome Brunet
2023-11-22 18:09 ` Krzysztof Kozlowski
0 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-22 16:14 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jerome Brunet, neil.armstrong, Rob Herring, JunYi Zhao,
devicetree, Rob Herring, Conor Dooley, Kevin Hilman,
Thierry Reding, linux-kernel, linux-pwm, linux-amlogic,
Krzysztof Kozlowski
On Wed 22 Nov 2023 at 16:46, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>>>>
>>>>> Again, where the "v2" is defined? Where is any document explaining the
>>>>> mapping between version blocks and SoC parts? Why do you list here only
>>>>> major version? Blocks almost always have also minor (e.g. v2.0).
>>>>
>>>> Again, v2 does has nothing to do with the HW. Never wrote it was.
>>>> The HW remains the same.
>>>
>>> Don't add compatibles which are not related to HW, but represent
>>> software versioning. Software does not matter for the bindings.
>>
>> What I did I explicitly what is recommended in Grant's presentation from
>> 2013. 10y old, but I assume slide 10 "Making an incompatible update" is
>> still valid.
>>
>> https://elinux.org/images/1/1e/DT_Binding_Process_glikely_ksummit_2013_10_28.pdf
>>
>> Breaking the ABI of the old compatible would break all boards which use
>> u-boot DT and pass it to the kernel, because the meaning of the clock
>> property would change.
>
> You broke U-Boot now as well - it will get your new DTS from the kernel
> and stop working.
U-boot will continue to match the old compatible and work properly.
When the dts using the new compatible lands in u-boot, it won't
match until proper driver support is added. It is a lot better than
breaking the ABI, which would have silently broke u-boot.
I don't really see a way around that.
If you have better way to fix a bad interface, feel free to share it.
>
>>
>> Doing things has suggested in this slide, and this patch, allows every
>> device to continue to work properly, whether the DT given is the one
>> shipped with u-boot (using the old compatible for now) or the kernel.
>
> OK, that explains the reasons. I read your commit msg and nothing like
> this was mentioned there. What's more, you did not deprecate the old
> binding, thus the confusion - it looked like you add entirely new
> hardware (although you put "deprecated" but in some unrelated place, not
> next to the compatibles).
The old interface being obsoleted by the new one is mentionned in the
commit description, the comments in the bindings and the bindings itself.
Thanks a lot for pointing out the placement mistake. I'll fix it.
The commit description says:
* What the patch does
* Why it does it:
* Why the old bindings is bad/broken
* How the new ones fixes the problem
* Why a single compatible properly describes, IMO, all the related HW.
This describes the entirety of what the change does.
That seemed clear enough for Rob. If that is not enough for you and you
would like it reworded, could please provide a few suggestions ?
>
> Anyway, the main point of Neil was that you started using generic
> compatible for all SoCs, which is wrong as well. I guess this was the
> original discussion.
The whole reason for this change is to properly describe the HW, which
is the 100% same on all the SoCs, or SoC families, concerned. The only
reason there was a lot of old compatibles is because it was used to match
data in the driver (this is clearly wrong). This data would now be
passed through DT.
I have been clear about this in the change description.
So why is it wrong to have single compatible for a type of device that
is 100% the same HW ?
It is lot a easier to apply a rule correctly when the intent is clear.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type
2023-11-22 16:14 ` Jerome Brunet
@ 2023-11-22 18:09 ` Krzysztof Kozlowski
0 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 18:09 UTC (permalink / raw)
To: Jerome Brunet
Cc: neil.armstrong, Rob Herring, JunYi Zhao, devicetree, Rob Herring,
Conor Dooley, Kevin Hilman, Thierry Reding, linux-kernel,
linux-pwm, linux-amlogic, Krzysztof Kozlowski
On 22/11/2023 17:14, Jerome Brunet wrote:
>
> On Wed 22 Nov 2023 at 16:46, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>
>
>>>>>>
>>>>>> Again, where the "v2" is defined? Where is any document explaining the
>>>>>> mapping between version blocks and SoC parts? Why do you list here only
>>>>>> major version? Blocks almost always have also minor (e.g. v2.0).
>>>>>
>>>>> Again, v2 does has nothing to do with the HW. Never wrote it was.
>>>>> The HW remains the same.
>>>>
>>>> Don't add compatibles which are not related to HW, but represent
>>>> software versioning. Software does not matter for the bindings.
>>>
>>> What I did I explicitly what is recommended in Grant's presentation from
>>> 2013. 10y old, but I assume slide 10 "Making an incompatible update" is
>>> still valid.
>>>
>>> https://elinux.org/images/1/1e/DT_Binding_Process_glikely_ksummit_2013_10_28.pdf
>>>
>>> Breaking the ABI of the old compatible would break all boards which use
>>> u-boot DT and pass it to the kernel, because the meaning of the clock
>>> property would change.
>>
>> You broke U-Boot now as well - it will get your new DTS from the kernel
>> and stop working.
>
> U-boot will continue to match the old compatible and work properly.
> When the dts using the new compatible lands in u-boot, it won't
> match until proper driver support is added. It is a lot better than
> breaking the ABI, which would have silently broke u-boot.
>
> I don't really see a way around that.
>
> If you have better way to fix a bad interface, feel free to share it.
>
>>
>>>
>>> Doing things has suggested in this slide, and this patch, allows every
>>> device to continue to work properly, whether the DT given is the one
>>> shipped with u-boot (using the old compatible for now) or the kernel.
>>
>> OK, that explains the reasons. I read your commit msg and nothing like
>> this was mentioned there. What's more, you did not deprecate the old
>> binding, thus the confusion - it looked like you add entirely new
>> hardware (although you put "deprecated" but in some unrelated place, not
>> next to the compatibles).
>
> The old interface being obsoleted by the new one is mentionned in the
> commit description, the comments in the bindings and the bindings itself.
> Thanks a lot for pointing out the placement mistake. I'll fix it.
>
> The commit description says:
> * What the patch does
> * Why it does it:
> * Why the old bindings is bad/broken
> * How the new ones fixes the problem
> * Why a single compatible properly describes, IMO, all the related HW.
>
> This describes the entirety of what the change does.
> That seemed clear enough for Rob. If that is not enough for you and you
> would like it reworded, could please provide a few suggestions ?
You did not deprecate the compatibles, so this has to be fixed. You put
the compatible in some other place, not really relevant.
>
>>
>> Anyway, the main point of Neil was that you started using generic
>> compatible for all SoCs, which is wrong as well. I guess this was the
>> original discussion.
>
> The whole reason for this change is to properly describe the HW, which
> is the 100% same on all the SoCs, or SoC families, concerned. The only
You still need specific compatibles, because the hardware is not 100%
the same. Programming model can, but hardware differs. Many times
engineers thought that devices are 100% compatible and then turned out
they are not. I am bored to repeat all this again and again.
> reason there was a lot of old compatibles is because it was used to match
> data in the driver (this is clearly wrong). This data would now be
> passed through DT.
>
> I have been clear about this in the change description.
>
> So why is it wrong to have single compatible for a type of device that
> is 100% the same HW ?
Because it is generic, not specific (you match "foo" against "bar" SoC).
The chapter from writing-bindings you referenced earlier mentioned this.
You need ability to add quirks and customize for these minor hardware
differences, even if programming model is the same.
>
> It is lot a easier to apply a rule correctly when the intent is clear.
>
>>
>> Best regards,
>> Krzysztof
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 3/6] pwm: meson: prepare addition of new compatible types
2023-11-17 12:59 [PATCH v2 0/6] pwm: meson: dt-bindings fixup Jerome Brunet
2023-11-17 12:59 ` [PATCH v2 1/6] dt-bindings: pwm: amlogic: fix s4 bindings Jerome Brunet
2023-11-17 12:59 ` [PATCH v2 2/6] dt-bindings: pwm: amlogic: add new compatible for meson8 pwm type Jerome Brunet
@ 2023-11-17 12:59 ` Jerome Brunet
2023-11-17 12:59 ` [PATCH v2 4/6] pwm: meson: add generic compatible for meson8 to sm1 Jerome Brunet
` (2 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Jerome Brunet @ 2023-11-17 12:59 UTC (permalink / raw)
To: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Jerome Brunet, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
Clean the amlogic pwm driver to prepare the addition of new pwm compatibles
* Generalize 4 inputs clock per channel.
AO pwm may just get 2 extra NULL entries which actually better
describes the reality of the HW.
* Use driver data to carry the device data and remove pwm_chip from it
* Stop carrying the internal clock elements with the device data.
These are not needed past init.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/pwm/pwm-meson.c | 150 +++++++++++++++++++++++-----------------
1 file changed, 87 insertions(+), 63 deletions(-)
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 5bea53243ed2..5cbd65cae28a 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -60,7 +60,7 @@
#define MISC_A_EN BIT(0)
#define MESON_NUM_PWMS 2
-#define MESON_MAX_MUX_PARENTS 4
+#define MESON_NUM_MUX_PARENTS 4
static struct meson_pwm_channel_data {
u8 reg_offset;
@@ -90,19 +90,14 @@ struct meson_pwm_channel {
unsigned int hi;
unsigned int lo;
- struct clk_mux mux;
- struct clk_divider div;
- struct clk_gate gate;
struct clk *clk;
};
struct meson_pwm_data {
const char * const *parent_names;
- unsigned int num_parents;
};
struct meson_pwm {
- struct pwm_chip chip;
const struct meson_pwm_data *data;
struct meson_pwm_channel channels[MESON_NUM_PWMS];
void __iomem *base;
@@ -115,7 +110,7 @@ struct meson_pwm {
static inline struct meson_pwm *to_meson_pwm(struct pwm_chip *chip)
{
- return container_of(chip, struct meson_pwm, chip);
+ return dev_get_drvdata(chip->dev);
}
static int meson_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -147,6 +142,7 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
const struct pwm_state *state)
{
struct meson_pwm_channel *channel = &meson->channels[pwm->hwpwm];
+ struct device *dev = pwm->chip->dev;
unsigned int cnt, duty_cnt;
unsigned long fin_freq;
u64 duty, period, freq;
@@ -169,19 +165,19 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
fin_freq = clk_round_rate(channel->clk, freq);
if (fin_freq == 0) {
- dev_err(meson->chip.dev, "invalid source clock frequency\n");
+ dev_err(dev, "invalid source clock frequency\n");
return -EINVAL;
}
- dev_dbg(meson->chip.dev, "fin_freq: %lu Hz\n", fin_freq);
+ dev_dbg(dev, "fin_freq: %lu Hz\n", fin_freq);
cnt = div_u64(fin_freq * period, NSEC_PER_SEC);
if (cnt > 0xffff) {
- dev_err(meson->chip.dev, "unable to get period cnt\n");
+ dev_err(dev, "unable to get period cnt\n");
return -EINVAL;
}
- dev_dbg(meson->chip.dev, "period=%llu cnt=%u\n", period, cnt);
+ dev_dbg(dev, "period=%llu cnt=%u\n", period, cnt);
if (duty == period) {
channel->hi = cnt;
@@ -192,7 +188,7 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
} else {
duty_cnt = div_u64(fin_freq * duty, NSEC_PER_SEC);
- dev_dbg(meson->chip.dev, "duty=%llu duty_cnt=%u\n", duty, duty_cnt);
+ dev_dbg(dev, "duty=%llu duty_cnt=%u\n", duty, duty_cnt);
channel->hi = duty_cnt;
channel->lo = cnt - duty_cnt;
@@ -215,7 +211,7 @@ static void meson_pwm_enable(struct meson_pwm *meson, struct pwm_device *pwm)
err = clk_set_rate(channel->clk, channel->rate);
if (err)
- dev_err(meson->chip.dev, "setting clock rate failed\n");
+ dev_err(pwm->chip->dev, "setting clock rate failed\n");
spin_lock_irqsave(&meson->lock, flags);
@@ -343,7 +339,6 @@ static const char * const pwm_meson8b_parent_names[] = {
static const struct meson_pwm_data pwm_meson8b_data = {
.parent_names = pwm_meson8b_parent_names,
- .num_parents = ARRAY_SIZE(pwm_meson8b_parent_names),
};
/*
@@ -351,12 +346,11 @@ static const struct meson_pwm_data pwm_meson8b_data = {
* The last 2 are grounded
*/
static const char * const pwm_gxbb_ao_parent_names[] = {
- "xtal", "clk81"
+ "xtal", "clk81", NULL, NULL,
};
static const struct meson_pwm_data pwm_gxbb_ao_data = {
.parent_names = pwm_gxbb_ao_parent_names,
- .num_parents = ARRAY_SIZE(pwm_gxbb_ao_parent_names),
};
static const char * const pwm_axg_ee_parent_names[] = {
@@ -365,7 +359,6 @@ static const char * const pwm_axg_ee_parent_names[] = {
static const struct meson_pwm_data pwm_axg_ee_data = {
.parent_names = pwm_axg_ee_parent_names,
- .num_parents = ARRAY_SIZE(pwm_axg_ee_parent_names),
};
static const char * const pwm_axg_ao_parent_names[] = {
@@ -374,7 +367,6 @@ static const char * const pwm_axg_ao_parent_names[] = {
static const struct meson_pwm_data pwm_axg_ao_data = {
.parent_names = pwm_axg_ao_parent_names,
- .num_parents = ARRAY_SIZE(pwm_axg_ao_parent_names),
};
static const char * const pwm_g12a_ao_ab_parent_names[] = {
@@ -383,16 +375,14 @@ static const char * const pwm_g12a_ao_ab_parent_names[] = {
static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
.parent_names = pwm_g12a_ao_ab_parent_names,
- .num_parents = ARRAY_SIZE(pwm_g12a_ao_ab_parent_names),
};
static const char * const pwm_g12a_ao_cd_parent_names[] = {
- "xtal", "g12a_ao_clk81",
+ "xtal", "g12a_ao_clk81", NULL, NULL,
};
static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
.parent_names = pwm_g12a_ao_cd_parent_names,
- .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names),
};
static const struct of_device_id meson_pwm_matches[] = {
@@ -432,23 +422,25 @@ static const struct of_device_id meson_pwm_matches[] = {
};
MODULE_DEVICE_TABLE(of, meson_pwm_matches);
-static int meson_pwm_init_channels(struct meson_pwm *meson)
+static int meson_pwm_init_clocks_legacy(struct device *dev,
+ struct clk_parent_data *mux_parent_data)
{
- struct clk_parent_data mux_parent_data[MESON_MAX_MUX_PARENTS] = {};
- struct device *dev = meson->chip.dev;
+ struct meson_pwm *meson = dev_get_drvdata(dev);
unsigned int i;
char name[255];
int err;
- for (i = 0; i < meson->data->num_parents; i++) {
- mux_parent_data[i].index = -1;
- mux_parent_data[i].name = meson->data->parent_names[i];
- }
-
- for (i = 0; i < meson->chip.npwm; i++) {
+ for (i = 0; i < MESON_NUM_PWMS; i++) {
struct meson_pwm_channel *channel = &meson->channels[i];
struct clk_parent_data div_parent = {}, gate_parent = {};
struct clk_init_data init = {};
+ struct clk_divider *div;
+ struct clk_gate *gate;
+ struct clk_mux *mux;
+
+ mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+ if (!mux)
+ return -ENOMEM;
snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i);
@@ -456,69 +448,76 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
init.ops = &clk_mux_ops;
init.flags = 0;
init.parent_data = mux_parent_data;
- init.num_parents = meson->data->num_parents;
-
- channel->mux.reg = meson->base + REG_MISC_AB;
- channel->mux.shift =
- meson_pwm_per_channel_data[i].clk_sel_shift;
- channel->mux.mask = MISC_CLK_SEL_MASK;
- channel->mux.flags = 0;
- channel->mux.lock = &meson->lock;
- channel->mux.table = NULL;
- channel->mux.hw.init = &init;
-
- err = devm_clk_hw_register(dev, &channel->mux.hw);
+ init.num_parents = MESON_NUM_MUX_PARENTS;
+
+ mux->reg = meson->base + REG_MISC_AB;
+ mux->shift = meson_pwm_per_channel_data[i].clk_sel_shift;
+ mux->mask = MISC_CLK_SEL_MASK;
+ mux->flags = 0;
+ mux->lock = &meson->lock;
+ mux->table = NULL;
+ mux->hw.init = &init;
+
+ err = devm_clk_hw_register(dev, &mux->hw);
if (err) {
dev_err(dev, "failed to register %s: %d\n", name, err);
return err;
}
+ div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
+ if (!div)
+ return -ENOMEM;
+
snprintf(name, sizeof(name), "%s#div%u", dev_name(dev), i);
init.name = name;
init.ops = &clk_divider_ops;
init.flags = CLK_SET_RATE_PARENT;
div_parent.index = -1;
- div_parent.hw = &channel->mux.hw;
+ div_parent.hw = &mux->hw;
init.parent_data = &div_parent;
init.num_parents = 1;
- channel->div.reg = meson->base + REG_MISC_AB;
- channel->div.shift = meson_pwm_per_channel_data[i].clk_div_shift;
- channel->div.width = MISC_CLK_DIV_WIDTH;
- channel->div.hw.init = &init;
- channel->div.flags = 0;
- channel->div.lock = &meson->lock;
+ div->reg = meson->base + REG_MISC_AB;
+ div->shift = meson_pwm_per_channel_data[i].clk_div_shift;
+ div->width = MISC_CLK_DIV_WIDTH;
+ div->hw.init = &init;
+ div->flags = 0;
+ div->lock = &meson->lock;
- err = devm_clk_hw_register(dev, &channel->div.hw);
+ err = devm_clk_hw_register(dev, &div->hw);
if (err) {
dev_err(dev, "failed to register %s: %d\n", name, err);
return err;
}
+ gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
+ if (!gate)
+ return -ENOMEM;
+
snprintf(name, sizeof(name), "%s#gate%u", dev_name(dev), i);
init.name = name;
init.ops = &clk_gate_ops;
init.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED;
gate_parent.index = -1;
- gate_parent.hw = &channel->div.hw;
+ gate_parent.hw = &div->hw;
init.parent_data = &gate_parent;
init.num_parents = 1;
- channel->gate.reg = meson->base + REG_MISC_AB;
- channel->gate.bit_idx = meson_pwm_per_channel_data[i].clk_en_shift;
- channel->gate.hw.init = &init;
- channel->gate.flags = 0;
- channel->gate.lock = &meson->lock;
+ gate->reg = meson->base + REG_MISC_AB;
+ gate->bit_idx = meson_pwm_per_channel_data[i].clk_en_shift;
+ gate->hw.init = &init;
+ gate->flags = 0;
+ gate->lock = &meson->lock;
- err = devm_clk_hw_register(dev, &channel->gate.hw);
+ err = devm_clk_hw_register(dev, &gate->hw);
if (err) {
dev_err(dev, "failed to register %s: %d\n", name, err);
return err;
}
- channel->clk = devm_clk_hw_get_clk(dev, &channel->gate.hw, NULL);
+ channel->clk = devm_clk_hw_get_clk(dev, &gate->hw, NULL);
if (IS_ERR(channel->clk)) {
err = PTR_ERR(channel->clk);
dev_err(dev, "failed to register %s: %d\n", name, err);
@@ -529,31 +528,56 @@ static int meson_pwm_init_channels(struct meson_pwm *meson)
return 0;
}
+static int meson_pwm_init_channels(struct device *dev)
+{
+ struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {};
+ struct meson_pwm *meson = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < MESON_NUM_MUX_PARENTS; i++) {
+ mux_parent_data[i].index = -1;
+ mux_parent_data[i].name = meson->data->parent_names[i];
+ }
+
+ return meson_pwm_init_clocks_legacy(dev, mux_parent_data);
+}
+
static int meson_pwm_probe(struct platform_device *pdev)
{
struct meson_pwm *meson;
+ struct pwm_chip *chip;
int err;
+ chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
meson = devm_kzalloc(&pdev->dev, sizeof(*meson), GFP_KERNEL);
if (!meson)
return -ENOMEM;
+ platform_set_drvdata(pdev, meson);
+
meson->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(meson->base))
return PTR_ERR(meson->base);
spin_lock_init(&meson->lock);
- meson->chip.dev = &pdev->dev;
- meson->chip.ops = &meson_pwm_ops;
- meson->chip.npwm = MESON_NUM_PWMS;
+ chip->dev = &pdev->dev;
+ chip->ops = &meson_pwm_ops;
+ chip->npwm = MESON_NUM_PWMS;
meson->data = of_device_get_match_data(&pdev->dev);
+ if (!meson->data) {
+ dev_err(&pdev->dev, "failed to match device\n");
+ return -ENODEV;
+ }
- err = meson_pwm_init_channels(meson);
+ err = meson_pwm_init_channels(&pdev->dev);
if (err < 0)
return err;
- err = devm_pwmchip_add(&pdev->dev, &meson->chip);
+ err = devm_pwmchip_add(&pdev->dev, chip);
if (err < 0) {
dev_err(&pdev->dev, "failed to register PWM chip: %d\n", err);
return err;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 4/6] pwm: meson: add generic compatible for meson8 to sm1
2023-11-17 12:59 [PATCH v2 0/6] pwm: meson: dt-bindings fixup Jerome Brunet
` (2 preceding siblings ...)
2023-11-17 12:59 ` [PATCH v2 3/6] pwm: meson: prepare addition of new compatible types Jerome Brunet
@ 2023-11-17 12:59 ` Jerome Brunet
2023-11-17 12:59 ` [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding Jerome Brunet
2023-11-17 12:59 ` [PATCH v2 6/6] arm64: " Jerome Brunet
5 siblings, 0 replies; 24+ messages in thread
From: Jerome Brunet @ 2023-11-17 12:59 UTC (permalink / raw)
To: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Jerome Brunet, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
Introduce a new compatible support in the Amlogic PWM driver.
The PWM HW is actually the same for all SoCs supported so far.
A specific compatible is needed only because the clock sources
of the PWMs are hard-coded in the driver.
It is better to have the clock source described in DT but this
changes the bindings so a new compatible must be introduced.
When all supported platform have migrated to the new compatible,
support for the legacy ones may be removed from the driver.
Adding a callback to setup the clock will also make it easier
to add support for the new PWM HW found in a1, s4, c3 and t7 SoC
families
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/pwm/pwm-meson.c | 224 ++++++++++++++++++++++++----------------
1 file changed, 133 insertions(+), 91 deletions(-)
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 5cbd65cae28a..d5d745a651d3 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -95,6 +95,7 @@ struct meson_pwm_channel {
struct meson_pwm_data {
const char * const *parent_names;
+ int (*channels_init)(struct device *dev);
};
struct meson_pwm {
@@ -333,95 +334,6 @@ static const struct pwm_ops meson_pwm_ops = {
.get_state = meson_pwm_get_state,
};
-static const char * const pwm_meson8b_parent_names[] = {
- "xtal", NULL, "fclk_div4", "fclk_div3"
-};
-
-static const struct meson_pwm_data pwm_meson8b_data = {
- .parent_names = pwm_meson8b_parent_names,
-};
-
-/*
- * Only the 2 first inputs of the GXBB AO PWMs are valid
- * The last 2 are grounded
- */
-static const char * const pwm_gxbb_ao_parent_names[] = {
- "xtal", "clk81", NULL, NULL,
-};
-
-static const struct meson_pwm_data pwm_gxbb_ao_data = {
- .parent_names = pwm_gxbb_ao_parent_names,
-};
-
-static const char * const pwm_axg_ee_parent_names[] = {
- "xtal", "fclk_div5", "fclk_div4", "fclk_div3"
-};
-
-static const struct meson_pwm_data pwm_axg_ee_data = {
- .parent_names = pwm_axg_ee_parent_names,
-};
-
-static const char * const pwm_axg_ao_parent_names[] = {
- "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5"
-};
-
-static const struct meson_pwm_data pwm_axg_ao_data = {
- .parent_names = pwm_axg_ao_parent_names,
-};
-
-static const char * const pwm_g12a_ao_ab_parent_names[] = {
- "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5"
-};
-
-static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
- .parent_names = pwm_g12a_ao_ab_parent_names,
-};
-
-static const char * const pwm_g12a_ao_cd_parent_names[] = {
- "xtal", "g12a_ao_clk81", NULL, NULL,
-};
-
-static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
- .parent_names = pwm_g12a_ao_cd_parent_names,
-};
-
-static const struct of_device_id meson_pwm_matches[] = {
- {
- .compatible = "amlogic,meson8b-pwm",
- .data = &pwm_meson8b_data
- },
- {
- .compatible = "amlogic,meson-gxbb-pwm",
- .data = &pwm_meson8b_data
- },
- {
- .compatible = "amlogic,meson-gxbb-ao-pwm",
- .data = &pwm_gxbb_ao_data
- },
- {
- .compatible = "amlogic,meson-axg-ee-pwm",
- .data = &pwm_axg_ee_data
- },
- {
- .compatible = "amlogic,meson-axg-ao-pwm",
- .data = &pwm_axg_ao_data
- },
- {
- .compatible = "amlogic,meson-g12a-ee-pwm",
- .data = &pwm_meson8b_data
- },
- {
- .compatible = "amlogic,meson-g12a-ao-pwm-ab",
- .data = &pwm_g12a_ao_ab_data
- },
- {
- .compatible = "amlogic,meson-g12a-ao-pwm-cd",
- .data = &pwm_g12a_ao_cd_data
- },
- {},
-};
-MODULE_DEVICE_TABLE(of, meson_pwm_matches);
-
static int meson_pwm_init_clocks_legacy(struct device *dev,
struct clk_parent_data *mux_parent_data)
{
@@ -528,12 +440,15 @@ static int meson_pwm_init_clocks_legacy(struct device *dev,
return 0;
}
-static int meson_pwm_init_channels(struct device *dev)
+static int meson_pwm_init_channels_legacy(struct device *dev)
{
struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {};
struct meson_pwm *meson = dev_get_drvdata(dev);
int i;
+ dev_info(dev, "using obsolete compatible, please consider updating dt\n");
+
+
for (i = 0; i < MESON_NUM_MUX_PARENTS; i++) {
mux_parent_data[i].index = -1;
mux_parent_data[i].name = meson->data->parent_names[i];
@@ -542,6 +457,133 @@ static int meson_pwm_init_channels(struct device *dev)
return meson_pwm_init_clocks_legacy(dev, mux_parent_data);
}
+static int meson_pwm_init_channels_meson8b_v2(struct device *dev)
+{
+ struct clk_parent_data mux_parent_data[MESON_NUM_MUX_PARENTS] = {};
+ int i;
+
+ /*
+ * NOTE: Instead of relying on the hard coded names in the driver
+ * as the legacy version, this relies on DT to provide the list of
+ * clocks.
+ * For once, using input numbers actually makes more sense than names.
+ * Also DT requires clock-names to be explicitly ordered, so there is
+ * no point bothering with clock names in this case.
+ */
+ for (i = 0; i < MESON_NUM_MUX_PARENTS; i++)
+ mux_parent_data[i].index = i;
+
+ return meson_pwm_init_clocks_legacy(dev, mux_parent_data);
+}
+
+static const char * const pwm_meson8b_parent_names[] = {
+ "xtal", NULL, "fclk_div4", "fclk_div3"
+};
+
+static const struct meson_pwm_data pwm_meson8b_data = {
+ .parent_names = pwm_meson8b_parent_names,
+ .channels_init = meson_pwm_init_channels_legacy,
+};
+
+/*
+ * Only the 2 first inputs of the GXBB AO PWMs are valid
+ * The last 2 are grounded
+ */
+static const char * const pwm_gxbb_ao_parent_names[] = {
+ "xtal", "clk81", NULL, NULL,
+};
+
+static const struct meson_pwm_data pwm_gxbb_ao_data = {
+ .parent_names = pwm_gxbb_ao_parent_names,
+ .channels_init = meson_pwm_init_channels_legacy,
+};
+
+static const char * const pwm_axg_ee_parent_names[] = {
+ "xtal", "fclk_div5", "fclk_div4", "fclk_div3"
+};
+
+static const struct meson_pwm_data pwm_axg_ee_data = {
+ .parent_names = pwm_axg_ee_parent_names,
+ .channels_init = meson_pwm_init_channels_legacy,
+};
+
+static const char * const pwm_axg_ao_parent_names[] = {
+ "xtal", "axg_ao_clk81", "fclk_div4", "fclk_div5"
+};
+
+static const struct meson_pwm_data pwm_axg_ao_data = {
+ .parent_names = pwm_axg_ao_parent_names,
+ .channels_init = meson_pwm_init_channels_legacy,
+};
+
+static const char * const pwm_g12a_ao_ab_parent_names[] = {
+ "xtal", "g12a_ao_clk81", "fclk_div4", "fclk_div5"
+};
+
+static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
+ .parent_names = pwm_g12a_ao_ab_parent_names,
+ .channels_init = meson_pwm_init_channels_legacy,
+};
+
+static const char * const pwm_g12a_ao_cd_parent_names[] = {
+ "xtal", "g12a_ao_clk81", NULL, NULL,
+};
+
+static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
+ .parent_names = pwm_g12a_ao_cd_parent_names,
+ .channels_init = meson_pwm_init_channels_legacy,
+};
+
+static const struct meson_pwm_data pwm_meson8_v2_data = {
+ .channels_init = meson_pwm_init_channels_meson8b_v2,
+};
+
+static const struct of_device_id meson_pwm_matches[] = {
+ {
+ .compatible = "amlogic,meson8-pwm-v2",
+ .data = &pwm_meson8_v2_data
+ },
+ /*
+ * The following compatibles are obsolete.
+ * Support for these may be removed once the related
+ * platforms have been updated
+ */
+ {
+ .compatible = "amlogic,meson8b-pwm",
+ .data = &pwm_meson8b_data
+ },
+ {
+ .compatible = "amlogic,meson-gxbb-pwm",
+ .data = &pwm_meson8b_data
+ },
+ {
+ .compatible = "amlogic,meson-gxbb-ao-pwm",
+ .data = &pwm_gxbb_ao_data
+ },
+ {
+ .compatible = "amlogic,meson-axg-ee-pwm",
+ .data = &pwm_axg_ee_data
+ },
+ {
+ .compatible = "amlogic,meson-axg-ao-pwm",
+ .data = &pwm_axg_ao_data
+ },
+ {
+ .compatible = "amlogic,meson-g12a-ee-pwm",
+ .data = &pwm_meson8b_data
+ },
+ {
+ .compatible = "amlogic,meson-g12a-ao-pwm-ab",
+ .data = &pwm_g12a_ao_ab_data
+ },
+ {
+ .compatible = "amlogic,meson-g12a-ao-pwm-cd",
+ .data = &pwm_g12a_ao_cd_data
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, meson_pwm_matches);
+
static int meson_pwm_probe(struct platform_device *pdev)
{
struct meson_pwm *meson;
@@ -573,7 +615,7 @@ static int meson_pwm_probe(struct platform_device *pdev)
return -ENODEV;
}
- err = meson_pwm_init_channels(&pdev->dev);
+ err = meson->data->channels_init(&pdev->dev);
if (err < 0)
return err;
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding
2023-11-17 12:59 [PATCH v2 0/6] pwm: meson: dt-bindings fixup Jerome Brunet
` (3 preceding siblings ...)
2023-11-17 12:59 ` [PATCH v2 4/6] pwm: meson: add generic compatible for meson8 to sm1 Jerome Brunet
@ 2023-11-17 12:59 ` Jerome Brunet
2023-11-22 8:39 ` Krzysztof Kozlowski
2023-11-17 12:59 ` [PATCH v2 6/6] arm64: " Jerome Brunet
5 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-17 12:59 UTC (permalink / raw)
To: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Jerome Brunet, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
Update Amlogic based SoC PWMs to meson8-pwm-v2 compatible
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
arch/arm/boot/dts/amlogic/meson.dtsi | 4 ++--
arch/arm/boot/dts/amlogic/meson8.dtsi | 16 +++++++++++++---
arch/arm/boot/dts/amlogic/meson8b-ec100.dts | 2 --
arch/arm/boot/dts/amlogic/meson8b-mxq.dts | 2 --
arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts | 2 --
arch/arm/boot/dts/amlogic/meson8b.dtsi | 16 +++++++++++++---
6 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/amlogic/meson.dtsi b/arch/arm/boot/dts/amlogic/meson.dtsi
index 8e3860d5d916..80cc004ad5fe 100644
--- a/arch/arm/boot/dts/amlogic/meson.dtsi
+++ b/arch/arm/boot/dts/amlogic/meson.dtsi
@@ -83,14 +83,14 @@ i2c_A: i2c@8500 {
};
pwm_ab: pwm@8550 {
- compatible = "amlogic,meson-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x8550 0x10>;
#pwm-cells = <3>;
status = "disabled";
};
pwm_cd: pwm@8650 {
- compatible = "amlogic,meson-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x8650 0x10>;
#pwm-cells = <3>;
status = "disabled";
diff --git a/arch/arm/boot/dts/amlogic/meson8.dtsi b/arch/arm/boot/dts/amlogic/meson8.dtsi
index 59932fbfd5d5..153b8fe9c506 100644
--- a/arch/arm/boot/dts/amlogic/meson8.dtsi
+++ b/arch/arm/boot/dts/amlogic/meson8.dtsi
@@ -450,10 +450,14 @@ analog_top: analog-top@81a8 {
};
pwm_ef: pwm@86c0 {
- compatible = "amlogic,meson8-pwm", "amlogic,meson8b-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x86c0 0x10>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
clock-measure@8758 {
@@ -702,11 +706,17 @@ timer@600 {
};
&pwm_ab {
- compatible = "amlogic,meson8-pwm", "amlogic,meson8b-pwm";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
&pwm_cd {
- compatible = "amlogic,meson8-pwm", "amlogic,meson8b-pwm";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
&rtc {
diff --git a/arch/arm/boot/dts/amlogic/meson8b-ec100.dts b/arch/arm/boot/dts/amlogic/meson8b-ec100.dts
index 3da47349eaaf..cdd7d04db256 100644
--- a/arch/arm/boot/dts/amlogic/meson8b-ec100.dts
+++ b/arch/arm/boot/dts/amlogic/meson8b-ec100.dts
@@ -441,8 +441,6 @@ &pwm_cd {
status = "okay";
pinctrl-0 = <&pwm_c1_pins>, <&pwm_d_pins>;
pinctrl-names = "default";
- clocks = <&xtal>, <&xtal>;
- clock-names = "clkin0", "clkin1";
};
&rtc {
diff --git a/arch/arm/boot/dts/amlogic/meson8b-mxq.dts b/arch/arm/boot/dts/amlogic/meson8b-mxq.dts
index 7adedd3258c3..68f4f70f4f03 100644
--- a/arch/arm/boot/dts/amlogic/meson8b-mxq.dts
+++ b/arch/arm/boot/dts/amlogic/meson8b-mxq.dts
@@ -162,8 +162,6 @@ &pwm_cd {
status = "okay";
pinctrl-0 = <&pwm_c1_pins>, <&pwm_d_pins>;
pinctrl-names = "default";
- clocks = <&xtal>, <&xtal>;
- clock-names = "clkin0", "clkin1";
};
&uart_AO {
diff --git a/arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts b/arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts
index 941682844faf..ff955b960688 100644
--- a/arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts
@@ -347,8 +347,6 @@ &pwm_cd {
status = "okay";
pinctrl-0 = <&pwm_c1_pins>, <&pwm_d_pins>;
pinctrl-names = "default";
- clocks = <&xtal>, <&xtal>;
- clock-names = "clkin0", "clkin1";
};
&rtc {
diff --git a/arch/arm/boot/dts/amlogic/meson8b.dtsi b/arch/arm/boot/dts/amlogic/meson8b.dtsi
index 5198f5177c2c..6c91eda92e8b 100644
--- a/arch/arm/boot/dts/amlogic/meson8b.dtsi
+++ b/arch/arm/boot/dts/amlogic/meson8b.dtsi
@@ -404,10 +404,14 @@ analog_top: analog-top@81a8 {
};
pwm_ef: pwm@86c0 {
- compatible = "amlogic,meson8b-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x86c0 0x10>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
clock-measure@8758 {
@@ -677,11 +681,17 @@ timer@600 {
};
&pwm_ab {
- compatible = "amlogic,meson8b-pwm";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
&pwm_cd {
- compatible = "amlogic,meson8b-pwm";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
&rtc {
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding
2023-11-17 12:59 ` [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding Jerome Brunet
@ 2023-11-22 8:39 ` Krzysztof Kozlowski
2023-11-22 14:52 ` Jerome Brunet
0 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 8:39 UTC (permalink / raw)
To: Jerome Brunet, Thierry Reding, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kevin Hilman, devicetree, linux-kernel, linux-amlogic, linux-pwm,
JunYi Zhao
On 17/11/2023 13:59, Jerome Brunet wrote:
> Update Amlogic based SoC PWMs to meson8-pwm-v2 compatible
Why? Your commit msg must explain this. You break users of this DTS on
older kernels and also this makes it impossible to apply via different
branches in the same cycle. All this needs explanation and proper
justification. Your message tells here nothing, because "what" is quite
obvious.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> arch/arm/boot/dts/amlogic/meson.dtsi | 4 ++--
> arch/arm/boot/dts/amlogic/meson8.dtsi | 16 +++++++++++++---
> arch/arm/boot/dts/amlogic/meson8b-ec100.dts | 2 --
> arch/arm/boot/dts/amlogic/meson8b-mxq.dts | 2 --
> arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts | 2 --
> arch/arm/boot/dts/amlogic/meson8b.dtsi | 16 +++++++++++++---
> 6 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/boot/dts/amlogic/meson.dtsi b/arch/arm/boot/dts/amlogic/meson.dtsi
> index 8e3860d5d916..80cc004ad5fe 100644
> --- a/arch/arm/boot/dts/amlogic/meson.dtsi
> +++ b/arch/arm/boot/dts/amlogic/meson.dtsi
> @@ -83,14 +83,14 @@ i2c_A: i2c@8500 {
> };
>
> pwm_ab: pwm@8550 {
> - compatible = "amlogic,meson-pwm";
> + compatible = "amlogic,meson8-pwm-v2";
That's breaking users of this DTS (old kernel, out of tree, other
projects) for no real reasons without explanation.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding
2023-11-22 8:39 ` Krzysztof Kozlowski
@ 2023-11-22 14:52 ` Jerome Brunet
2023-11-22 15:10 ` Krzysztof Kozlowski
0 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-22 14:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jerome Brunet, Thierry Reding, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kevin Hilman, devicetree,
linux-kernel, linux-amlogic, linux-pwm, JunYi Zhao
On Wed 22 Nov 2023 at 09:39, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 17/11/2023 13:59, Jerome Brunet wrote:
>> Update Amlogic based SoC PWMs to meson8-pwm-v2 compatible
>
> Why? Your commit msg must explain this. You break users of this DTS on
> older kernels and also this makes it impossible to apply via different
> branches in the same cycle. All this needs explanation and proper
> justification. Your message tells here nothing, because "what" is quite
> obvious.
>
I provided all the explanation possible through the different commits of
this series. I can re-state here if it helps
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>> arch/arm/boot/dts/amlogic/meson.dtsi | 4 ++--
>> arch/arm/boot/dts/amlogic/meson8.dtsi | 16 +++++++++++++---
>> arch/arm/boot/dts/amlogic/meson8b-ec100.dts | 2 --
>> arch/arm/boot/dts/amlogic/meson8b-mxq.dts | 2 --
>> arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts | 2 --
>> arch/arm/boot/dts/amlogic/meson8b.dtsi | 16 +++++++++++++---
>> 6 files changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/amlogic/meson.dtsi b/arch/arm/boot/dts/amlogic/meson.dtsi
>> index 8e3860d5d916..80cc004ad5fe 100644
>> --- a/arch/arm/boot/dts/amlogic/meson.dtsi
>> +++ b/arch/arm/boot/dts/amlogic/meson.dtsi
>> @@ -83,14 +83,14 @@ i2c_A: i2c@8500 {
>> };
>>
>> pwm_ab: pwm@8550 {
>> - compatible = "amlogic,meson-pwm";
>> + compatible = "amlogic,meson8-pwm-v2";
>
> That's breaking users of this DTS (old kernel, out of tree, other
> projects) for no real reasons without explanation.
"amlogic,meson-pwm" will continue to match, meaning of bindings is unchanged
How do you propose to fix badly designed bindings then ?
if we cant even introduce a new compatible to fix things up. It is supposed to
stay and broken till the end of time ?
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding
2023-11-22 14:52 ` Jerome Brunet
@ 2023-11-22 15:10 ` Krzysztof Kozlowski
0 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 15:10 UTC (permalink / raw)
To: Jerome Brunet
Cc: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
On 22/11/2023 15:52, Jerome Brunet wrote:
>
> On Wed 22 Nov 2023 at 09:39, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>
>> On 17/11/2023 13:59, Jerome Brunet wrote:
>>> Update Amlogic based SoC PWMs to meson8-pwm-v2 compatible
>>
>> Why? Your commit msg must explain this. You break users of this DTS on
>> older kernels and also this makes it impossible to apply via different
>> branches in the same cycle. All this needs explanation and proper
>> justification. Your message tells here nothing, because "what" is quite
>> obvious.
>>
>
> I provided all the explanation possible through the different commits of
> this series. I can re-state here if it helps
DTS commits stand on their own and must not go via same branch as
driver, so how does driver commit msg help Git history?
>
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>> arch/arm/boot/dts/amlogic/meson.dtsi | 4 ++--
>>> arch/arm/boot/dts/amlogic/meson8.dtsi | 16 +++++++++++++---
>>> arch/arm/boot/dts/amlogic/meson8b-ec100.dts | 2 --
>>> arch/arm/boot/dts/amlogic/meson8b-mxq.dts | 2 --
>>> arch/arm/boot/dts/amlogic/meson8b-odroidc1.dts | 2 --
>>> arch/arm/boot/dts/amlogic/meson8b.dtsi | 16 +++++++++++++---
>>> 6 files changed, 28 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/amlogic/meson.dtsi b/arch/arm/boot/dts/amlogic/meson.dtsi
>>> index 8e3860d5d916..80cc004ad5fe 100644
>>> --- a/arch/arm/boot/dts/amlogic/meson.dtsi
>>> +++ b/arch/arm/boot/dts/amlogic/meson.dtsi
>>> @@ -83,14 +83,14 @@ i2c_A: i2c@8500 {
>>> };
>>>
>>> pwm_ab: pwm@8550 {
>>> - compatible = "amlogic,meson-pwm";
>>> + compatible = "amlogic,meson8-pwm-v2";
>>
>> That's breaking users of this DTS (old kernel, out of tree, other
>> projects) for no real reasons without explanation.
>
> "amlogic,meson-pwm" will continue to match, meaning of bindings is unchanged
No, because new DTS does not have amlogic,meson-pwm, thus all existing
users see breakage.
>
> How do you propose to fix badly designed bindings then ?
Justify and introduce incompatible changes, breaking the ABI. Anyway
this is a requirement, because, as I said in other reply, you cannot
have compatible for software model!
>
> if we cant even introduce a new compatible to fix things up. It is supposed to
> stay and broken till the end of time ?
No, you cannot introduce new compatible for new OS.
Fix the bindings instead with proper justification. We did it many
times, what's the problem here?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 6/6] arm64: dts: amlogic: migrate pwms to new meson8 v2 binding
2023-11-17 12:59 [PATCH v2 0/6] pwm: meson: dt-bindings fixup Jerome Brunet
` (4 preceding siblings ...)
2023-11-17 12:59 ` [PATCH v2 5/6] arm: dts: amlogic: migrate pwms to new meson8 v2 binding Jerome Brunet
@ 2023-11-17 12:59 ` Jerome Brunet
2023-11-22 8:41 ` Krzysztof Kozlowski
5 siblings, 1 reply; 24+ messages in thread
From: Jerome Brunet @ 2023-11-17 12:59 UTC (permalink / raw)
To: Thierry Reding, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Jerome Brunet, Kevin Hilman, devicetree, linux-kernel,
linux-amlogic, linux-pwm, JunYi Zhao
Update Amlogic based SoC PWMs to meson8-pwm-v2 compatible
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 24 +++++++++++++---
.../boot/dts/amlogic/meson-g12-common.dtsi | 28 +++++++++++++++----
.../dts/amlogic/meson-g12a-radxa-zero.dts | 4 ---
.../boot/dts/amlogic/meson-g12a-sei510.dts | 4 ---
.../boot/dts/amlogic/meson-g12a-u200.dts | 2 --
.../boot/dts/amlogic/meson-g12a-x96-max.dts | 4 ---
.../amlogic/meson-g12b-a311d-libretech-cc.dts | 2 --
.../dts/amlogic/meson-g12b-bananapi-cm4.dtsi | 7 -----
.../boot/dts/amlogic/meson-g12b-bananapi.dtsi | 4 ---
.../dts/amlogic/meson-g12b-khadas-vim3.dtsi | 4 ---
.../boot/dts/amlogic/meson-g12b-odroid.dtsi | 4 ---
.../dts/amlogic/meson-g12b-radxa-zero2.dts | 8 ------
.../boot/dts/amlogic/meson-g12b-w400.dtsi | 6 ----
.../dts/amlogic/meson-gx-libretech-pc.dtsi | 6 ----
.../boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 2 --
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 8 +++---
.../boot/dts/amlogic/meson-gxbb-nanopi-k2.dts | 2 --
.../dts/amlogic/meson-gxbb-nexbox-a95x.dts | 2 --
.../boot/dts/amlogic/meson-gxbb-p20x.dtsi | 2 --
.../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 2 --
.../boot/dts/amlogic/meson-gxbb-wetek.dtsi | 2 --
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 26 +++++++++++++++++
.../boot/dts/amlogic/meson-gxl-s805x-p241.dts | 2 --
.../meson-gxl-s905w-jethome-jethub-j80.dts | 2 --
.../meson-gxl-s905x-hwacom-amazetv.dts | 2 --
.../amlogic/meson-gxl-s905x-khadas-vim.dts | 2 --
.../amlogic/meson-gxl-s905x-nexbox-a95x.dts | 2 --
.../dts/amlogic/meson-gxl-s905x-p212.dtsi | 2 --
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 26 +++++++++++++++++
.../dts/amlogic/meson-gxm-khadas-vim2.dts | 4 ---
.../boot/dts/amlogic/meson-gxm-rbox-pro.dts | 2 --
.../amlogic/meson-libretech-cottonwood.dtsi | 6 ----
.../boot/dts/amlogic/meson-sm1-ac2xx.dtsi | 6 ----
.../dts/amlogic/meson-sm1-khadas-vim3l.dts | 2 --
.../boot/dts/amlogic/meson-sm1-odroid.dtsi | 2 --
.../boot/dts/amlogic/meson-sm1-sei610.dts | 6 ----
36 files changed, 99 insertions(+), 120 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index a49aa62e3f9f..2cb68f95bcf9 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -1664,10 +1664,14 @@ sec_AO: ao-secure@140 {
};
pwm_AO_cd: pwm@2000 {
- compatible = "amlogic,meson-axg-ao-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x02000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <&clkc_AO CLKID_AO_CLK81>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV5>;
};
uart_AO: serial@3000 {
@@ -1699,10 +1703,14 @@ i2c_AO: i2c@5000 {
};
pwm_AO_ab: pwm@7000 {
- compatible = "amlogic,meson-axg-ao-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x07000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <&clkc_AO CLKID_AO_CLK81>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV5>;
};
ir: ir@8000 {
@@ -1777,17 +1785,25 @@ watchdog@f0d0 {
};
pwm_ab: pwm@1b000 {
- compatible = "amlogic,meson-axg-ee-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x1b000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <&clkc CLKID_FCLK_DIV5>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
pwm_cd: pwm@1a000 {
- compatible = "amlogic,meson-axg-ee-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x1a000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <&clkc CLKID_FCLK_DIV5>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
spicc0: spi@13000 {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index ff68b911b729..7bd8c4ddfb3e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -2039,10 +2039,12 @@ cecb_AO: cec@280 {
};
pwm_AO_cd: pwm@2000 {
- compatible = "amlogic,meson-g12a-ao-pwm-cd";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x2000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <&clkc_AO CLKID_AO_CLK81>;
};
uart_AO: serial@3000 {
@@ -2078,10 +2080,14 @@ i2c_AO: i2c@5000 {
};
pwm_AO_ab: pwm@7000 {
- compatible = "amlogic,meson-g12a-ao-pwm-ab";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x7000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <&clkc_AO CLKID_AO_CLK81>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV5>;
};
ir: ir@8000 {
@@ -2229,24 +2235,36 @@ spifc: spi@14000 {
};
pwm_ef: pwm@19000 {
- compatible = "amlogic,meson-g12a-ee-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x19000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
pwm_cd: pwm@1a000 {
- compatible = "amlogic,meson-g12a-ee-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x1a000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
pwm_ab: pwm@1b000 {
- compatible = "amlogic,meson-g12a-ee-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x1b000 0x0 0x20>;
#pwm-cells = <3>;
status = "disabled";
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
};
i2c3: i2c@1c000 {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
index fcd7e1d8e16f..c83298c050c3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts
@@ -280,8 +280,6 @@ &ir {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
@@ -289,8 +287,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
index 0ad0c2b7dfef..c32365412572 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
@@ -386,8 +386,6 @@ &ir {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
@@ -395,8 +393,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&pdm {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
index 8355ddd7e9ae..161f00d371fa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-u200.dts
@@ -498,8 +498,6 @@ &i2c3 {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index 4969a76460fa..f2a730367d59 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -325,8 +325,6 @@ &ir {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
@@ -360,8 +358,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&uart_A {
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
index 65b963d794cd..adedc1340c78 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
@@ -116,6 +116,4 @@ &cpu103 {
&pwm_ab {
pinctrl-0 = <&pwm_a_e_pins>, <&pwm_b_x7_pins>;
- clocks = <&xtal>, <&xtal>;
- clock-names = "clkin0", "clkin1";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4.dtsi
index 995ce10d5c81..ef09a233ca04 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4.dtsi
@@ -257,25 +257,18 @@ &pcie {
&pwm_ab {
pinctrl-0 = <&pwm_a_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
-
status = "okay";
};
&pwm_ef {
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
-
status = "okay";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
-
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi
index 0a6a12808568..5aba1968cb93 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi.dtsi
@@ -370,8 +370,6 @@ &pwm_ab {
status = "okay";
pinctrl-0 = <&pwm_a_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&pwm_cd {
@@ -390,8 +388,6 @@ &pwm_ef {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi
index 16dd409051b4..48650bad230d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-khadas-vim3.dtsi
@@ -92,16 +92,12 @@ &cpu103 {
&pwm_ab {
pinctrl-0 = <&pwm_a_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid.dtsi
index 9e12a34b2840..6ed01744f937 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid.dtsi
@@ -327,16 +327,12 @@ hdmi_tx_tmds_out: endpoint {
&pwm_ab {
pinctrl-0 = <&pwm_a_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-radxa-zero2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-radxa-zero2.dts
index 890f5bfebb03..44d74f22bec5 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-radxa-zero2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-radxa-zero2.dts
@@ -351,32 +351,24 @@ &ir {
&pwm_ab {
pinctrl-0 = <&pwm_a_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_ef {
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_AO_ab {
pinctrl-0 = <&pwm_ao_a_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
index ac8b7178257e..fa9f510332f7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
@@ -304,24 +304,18 @@ &ir {
&pwm_ab {
pinctrl-0 = <&pwm_a_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
&pwm_ef {
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi
index 5e7b9273b062..2b79560ea386 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi
@@ -341,24 +341,18 @@ rtc: rtc@51 {
&pwm_AO_ab {
pinctrl-0 = <&pwm_ao_a_3_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_ab {
pinctrl-0 = <&pwm_b_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
status = "okay";
};
&pwm_ef {
pinctrl-0 = <&pwm_e_pins>, <&pwm_f_clk_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
index 18f7b730289e..690552e5a104 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
@@ -237,8 +237,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 2673f0dbafe7..bf00672b9009 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -329,14 +329,14 @@ i2c_A: i2c@8500 {
};
pwm_ab: pwm@8550 {
- compatible = "amlogic,meson-gx-pwm", "amlogic,meson-gxbb-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x08550 0x0 0x10>;
#pwm-cells = <3>;
status = "disabled";
};
pwm_cd: pwm@8650 {
- compatible = "amlogic,meson-gx-pwm", "amlogic,meson-gxbb-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x08650 0x0 0x10>;
#pwm-cells = <3>;
status = "disabled";
@@ -351,7 +351,7 @@ saradc: adc@8680 {
};
pwm_ef: pwm@86c0 {
- compatible = "amlogic,meson-gx-pwm", "amlogic,meson-gxbb-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x086c0 0x0 0x10>;
#pwm-cells = <3>;
status = "disabled";
@@ -498,7 +498,7 @@ i2c_AO: i2c@500 {
};
pwm_AO_ab: pwm@550 {
- compatible = "amlogic,meson-gx-ao-pwm", "amlogic,meson-gxbb-ao-pwm";
+ compatible = "amlogic,meson8-pwm-v2";
reg = <0x0 0x00550 0x0 0x10>;
#pwm-cells = <3>;
status = "disabled";
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
index 1fd2e56e6b08..373451691090 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
@@ -294,8 +294,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 4aab1ab705b4..55d0353d0997 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -237,8 +237,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
/* Wireless SDIO Module */
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index e803a466fe4e..730a5e6dfff7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -150,8 +150,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
/* Wireless SDIO Module */
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index e8303089bff6..dde812ed8a5c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -219,8 +219,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
index 94dafb955301..20a501586431 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
@@ -185,8 +185,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 12ef6e81c8bd..a00d2e29da43 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -733,6 +733,32 @@ mux {
};
};
+&pwm_ab {
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
+};
+
+&pwm_cd {
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
+};
+
+&pwm_ef {
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
+};
+
+&pwm_AO_ab {
+ clocks = <&xtal>,
+ <&clkc CLKID_CLK81>;
+};
+
&pwrc {
resets = <&reset RESET_VIU>,
<&reset RESET_VENC>,
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts
index c0d6eb55100a..62d58fef220e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s805x-p241.dts
@@ -276,8 +276,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
/* This is connected to the Bluetooth module: */
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-jethome-jethub-j80.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-jethome-jethub-j80.dts
index a18d6d241a5a..0afc5a3e8d24 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-jethome-jethub-j80.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905w-jethome-jethub-j80.dts
@@ -116,8 +116,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts
index c8d74e61dec1..3f46b5a7bba5 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-hwacom-amazetv.dts
@@ -115,8 +115,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
/* SD card */
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
index fea65f20523a..20eb509ba063 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
@@ -207,8 +207,6 @@ &pwm_AO_ab {
status = "okay";
pinctrl-0 = <&pwm_ao_a_3_pins>, <&pwm_ao_b_pins>;
pinctrl-names = "default";
- clocks = <&xtal> , <&xtal>;
- clock-names = "clkin0", "clkin1" ;
};
&pwm_ef {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts
index f1acca5c4434..0fe8dcbfd11b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-nexbox-a95x.dts
@@ -145,8 +145,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
/* Wireless SDIO Module */
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi
index a150cc0e18ff..e04863294b64 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dtsi
@@ -101,8 +101,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 17bcfa4702e1..f9a9b12abec8 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -803,6 +803,32 @@ internal_phy: ethernet-phy@8 {
};
};
+&pwm_ab {
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
+};
+
+&pwm_cd {
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
+};
+
+&pwm_ef {
+ clocks = <&xtal>,
+ <0>,
+ <&clkc CLKID_FCLK_DIV4>,
+ <&clkc CLKID_FCLK_DIV3>;
+};
+
+&pwm_AO_ab {
+ clocks = <&xtal>,
+ <&clkc CLKID_CLK81>;
+};
+
&pwrc {
resets = <&reset RESET_VIU>,
<&reset RESET_VENC>,
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
index 860f307494c5..ae7e2c8a06e4 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
@@ -285,16 +285,12 @@ &pwm_AO_ab {
status = "okay";
pinctrl-0 = <&pwm_ao_a_3_pins>, <&pwm_ao_b_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>, <&pwm_f_clk_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
&sd_emmc_a {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
index 50d49aec41bd..70d65379b719 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
@@ -189,8 +189,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&clkc CLKID_FCLK_DIV4>;
- clock-names = "clkin0";
};
/* Wireless SDIO Module */
diff --git a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
index 35e8f5bae990..9660a4abcc6a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
@@ -454,24 +454,18 @@ &pwm_AO_cd {
status = "okay";
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
};
&pwm_ab {
status = "okay";
pinctrl-0 = <&pwm_b_x7_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
};
&pwm_cd {
status = "okay";
pinctrl-0 = <&pwm_d_x3_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-ac2xx.dtsi b/arch/arm64/boot/dts/amlogic/meson-sm1-ac2xx.dtsi
index 46a34731f7e2..5fd657f4934e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-ac2xx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-ac2xx.dtsi
@@ -199,24 +199,18 @@ &pwm_AO_ab {
status = "okay";
pinctrl-0 = <&pwm_ao_a_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
clocks = <&xtal>;
- clock-names = "clkin1";
- status = "okay";
};
&pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&saradc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
index 9c0b544e2209..5d75ad3f3e46 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-khadas-vim3l.dts
@@ -78,8 +78,6 @@ &cpu3 {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-odroid.dtsi b/arch/arm64/boot/dts/amlogic/meson-sm1-odroid.dtsi
index 1db2327bbd13..b9c8a037fe63 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-odroid.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-odroid.dtsi
@@ -388,8 +388,6 @@ &ir {
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
index 095579c55f18..e24e2dab3c7c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
@@ -432,15 +432,11 @@ &pwm_AO_ab {
status = "okay";
pinctrl-0 = <&pwm_ao_a_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&pwm_AO_cd {
pinctrl-0 = <&pwm_ao_d_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin1";
status = "okay";
};
@@ -448,8 +444,6 @@ &pwm_ef {
status = "okay";
pinctrl-0 = <&pwm_e_pins>;
pinctrl-names = "default";
- clocks = <&xtal>;
- clock-names = "clkin0";
};
&saradc {
--
2.42.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 6/6] arm64: dts: amlogic: migrate pwms to new meson8 v2 binding
2023-11-17 12:59 ` [PATCH v2 6/6] arm64: " Jerome Brunet
@ 2023-11-22 8:41 ` Krzysztof Kozlowski
0 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-22 8:41 UTC (permalink / raw)
To: Jerome Brunet, Thierry Reding, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Kevin Hilman, devicetree, linux-kernel, linux-amlogic, linux-pwm,
JunYi Zhao
On 17/11/2023 13:59, Jerome Brunet wrote:
> Update Amlogic based SoC PWMs to meson8-pwm-v2 compatible
Please write proper commit msgs.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
...
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
> index 18f7b730289e..690552e5a104 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
> @@ -237,8 +237,6 @@ &pwm_ef {
> status = "okay";
> pinctrl-0 = <&pwm_e_pins>;
> pinctrl-names = "default";
> - clocks = <&clkc CLKID_FCLK_DIV4>;
> - clock-names = "clkin0";
> };
>
> &saradc {
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index 2673f0dbafe7..bf00672b9009 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -329,14 +329,14 @@ i2c_A: i2c@8500 {
> };
>
> pwm_ab: pwm@8550 {
> - compatible = "amlogic,meson-gx-pwm", "amlogic,meson-gxbb-pwm";
> + compatible = "amlogic,meson8-pwm-v2";
This is just wrong. NAK.
Replacing specific and correct (nothing in commit msg said these are
incorrect!) compatibles with generic, unspecific one is a no-go. It does
not make sense to replace correct code with incorrect...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 24+ messages in thread