Skip to contentSkip to conversation

dt-bindings: pwm: Document Tegra194 and Tegra264 controllers

PATCH In review Thierry Reding 3 participants 15 messages Mar 20 – Mar 25

Subsystems (3)
lore
Share & tools
lore

Thierry Reding

author maintainer
#0 lore raw

PATCH 1/2 dt-bindings: pwm: Document Tegra194 and Tegra264 controllers

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---
 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml +2 -0
index 41cea4979132..15706d2a808d 100644
@@ -16,6 +16,8 @@ properties:
       - enum:
           - nvidia,tegra20-pwm
           - nvidia,tegra186-pwm
+          - nvidia,tegra194-pwm
+          - nvidia,tegra264-pwm
 
       - items:
           - enum:
Signature · 1 line
2.52.0
author maintainer
#1 lore raw

PATCH 2/2 arm64: tegra: Add PWM controllers on Tegra264

From: Thierry Reding <[email protected]>

Tegra264 has a number of PWM controller which are similar to those found on earlier chips but with some added functionality.

---
 arch/arm64/boot/dts/nvidia/tegra264.dtsi | 72 ++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
arch/arm64/boot/dts/nvidia/tegra264.dtsi +72 -0
index efb4d21aa4e3..61c001fd2651 100644
@@ -3344,6 +3344,18 @@ i2c3: i2c@c610000 {
 			status = "disabled";
 		};
 
+		pwm4: pwm@c6a0000 {
+			compatible = "nvidia,tegra264-pwm";
+			reg = <0x0 0xc6a0000 0x0 0x10000>;
+			status = "disabled";
+
+			clocks = <&bpmp TEGRA264_CLK_PWM4>;
+			resets = <&bpmp TEGRA264_RESET_PWM4>;
+			reset-names = "pwm";
+
+			#pwm-cells = <2>;
+		};
+
 		pmc: pmc@c800000 {
 			compatible = "nvidia,tegra264-pmc";
 			reg = <0x0 0x0c800000 0x0 0x100000>,
@@ -3579,6 +3591,66 @@ i2c16: i2c@c430000 {
 			status = "disabled";
 		};
 
+		pwm2: pwm@c5e0000 {
+			compatible = "nvidia,tegra264-pwm";
+			reg = <0x0 0xc5e0000 0x0 0x10000>;
+			status = "disabled";
+
+			clocks = <&bpmp TEGRA264_CLK_PWM2>;
+			resets = <&bpmp TEGRA264_RESET_PWM2>;
+			reset-names = "pwm";
+
+			#pwm-cells = <2>;
+		};
+
+		pwm3: pwm@c5f0000 {
+			compatible = "nvidia,tegra264-pwm";
+			reg = <0x0 0xc5f0000 0x0 0x10000>;
+			status = "disabled";
+
+			clocks = <&bpmp TEGRA264_CLK_PWM3>;
+			resets = <&bpmp TEGRA264_RESET_PWM3>;
+			reset-names = "pwm";
+
+			#pwm-cells = <2>;
+		};
+
+		pwm5: pwm@c600000 {
+			compatible = "nvidia,tegra264-pwm";
+			reg = <0x0 0xc600000 0x0 0x10000>;
+			status = "disabled";
+
+			clocks = <&bpmp TEGRA264_CLK_PWM5>;
+			resets = <&bpmp TEGRA264_RESET_PWM5>;
+			reset-names = "pwm";
+
+			#pwm-cells = <2>;
+		};
+
+		pwm9: pwm@c610000 {
+			compatible = "nvidia,tegra264-pwm";
+			reg = <0x0 0xc610000 0x0 0x10000>;
+			status = "disabled";
+
+			clocks = <&bpmp TEGRA264_CLK_PWM9>;
+			resets = <&bpmp TEGRA264_RESET_PWM9>;
+			reset-names = "pwm";
+
+			#pwm-cells = <2>;
+		};
+
+		pwm10: pwm@c620000 {
+			compatible = "nvidia,tegra264-pwm";
+			reg = <0x0 0xc620000 0x0 0x10000>;
+			status = "disabled";
+
+			clocks = <&bpmp TEGRA264_CLK_PWM10>;
+			resets = <&bpmp TEGRA264_RESET_PWM10>;
+			reset-names = "pwm";
+
+			#pwm-cells = <2>;
+		};
+
 		i2c0: i2c@c630000 {
 			compatible = "nvidia,tegra264-i2c";
 			reg = <0x00 0x0c630000 0x0 0x10000>;
Signature · 1 line
2.52.0
#2 lore raw
In reply to Thierry Reding
Quoted 5 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

Extra features means devices are compatible.

You also always need a new compatible even without extra features, so last part is just confusing. Suggests like you could skip new compatible.

Quoted 11 lines
---
 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml +0 -0
index 41cea4979132..15706d2a808d 100644

Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml properties: · line 19

Quoted 4 lines
Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml +1 -0
@@ -16,6 +16,8 @@ properties:
       - enum:
           - nvidia,tegra20-pwm
           - nvidia,tegra186-pwm
+          - nvidia,tegra194-pwm

Not explained in commit msg and it's a duplicated compatible.

Best regards,
Krzysztof

author maintainer
#3 lore raw
Quoted 7 lines · Krzysztof Kozlowski
Quoted 5 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

Quoted 5 lines

Extra features means devices are compatible.

You also always need a new compatible even without extra features, so last part is just confusing. Suggests like you could skip new compatible.

Erm... if the hardware was exactly identical, then there'd be no need for a new compatible string. It's certainly good practice to add one anyway, but what's the point in having 10 different compatible strings for exactly the same hardware?

In this case the new compatible string is necessary so that we can take advantage of the new features that are not covered yet by an existing binding.

Thierry

#4 lore raw
Quoted 10 lines · Thierry Reding
Quoted 9 lines · Krzysztof Kozlowski
Quoted 5 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

Extra features means devices are compatible.

Quoted 8 lines
Quoted 3 lines

You also always need a new compatible even without extra features, so last part is just confusing. Suggests like you could skip new compatible.

Erm... if the hardware was exactly identical, then there'd be no need for a new compatible string. It's certainly good practice to add one anyway, but what's the point in having 10 different compatible strings for exactly the same hardware?

Because integration is different. Nothing new here, DT maintainers request it for years and is since long time documented in writing bindings.

Best regards,
Krzysztof

author maintainer
#5 lore raw
Quoted 15 lines · Krzysztof Kozlowski
Quoted 14 lines · Thierry Reding
Quoted 12 lines · Krzysztof Kozlowski
Quoted 5 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

Extra features means devices are compatible.

You also always need a new compatible even without extra features, so last part is just confusing. Suggests like you could skip new compatible.

Quoted 7 lines
Quoted 4 lines

Erm... if the hardware was exactly identical, then there'd be no need for a new compatible string. It's certainly good practice to add one anyway, but what's the point in having 10 different compatible strings for exactly the same hardware?

Because integration is different. Nothing new here, DT maintainers request it for years and is since long time documented in writing bindings.

It's more subtle than that. The reason why we want a specific compatible string is because the different integration might potentially introduce a bug that we might, potentially, have to work around at some point using a specific compatible string. If the hardware is backwards- compatible and the driver can match on an existing compatible string and work, there's no requirement for an extra, more specific compatible.

What you describe is good practice, and I agree it's a good idea. But if we had chosen to not implement the new features and not update the bindings and instead reuse any of the existing compatible strings, nobody would've care one bit.

Heck, if your position is that you *need* a new compatible for every new integration, there'd be no point in even having fallback compatible strings in the first place.

Thierry

#6 lore raw
In reply to Thierry Reding
Quoted 10 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---
 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

Best regards,
Krzysztof

#7 lore raw
Quoted 13 lines · Krzysztof Kozlowski
Quoted 11 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---

 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Quoted 6 lines

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

Best regards,
Krzysztof

Just posted: https://lore.kernel.org/linux-tegra/[email protected]/T/#mfb40392e07d7ac9cedbaf853442eed822da7671e

Mikko

#8 lore raw
Quoted 14 lines · Mikko Perttunen
Quoted 13 lines · Krzysztof Kozlowski
Quoted 11 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---

 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Quoted 8 lines
Quoted 6 lines

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

Best regards,
Krzysztof

Just posted: https://lore.kernel.org/linux-tegra/[email protected]/T/#mfb40392e07d7ac9cedbaf853442eed822da7671e

so this is completely misplaced. Please read submitting patches in DT dir and follow standard style of sending patches upstream. Just like every other contributor. You don't get any exceptions here.

Best regards,
Krzysztof

#9 lore raw
Quoted 22 lines · Krzysztof Kozlowski
Quoted 21 lines · Mikko Perttunen
Quoted 19 lines · Krzysztof Kozlowski
Quoted 11 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---

 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

Best regards,
Krzysztof

Quoted 7 lines
Quoted 3 lines

Just posted:
https://lore.kernel.org/linux-tegra/20260323-t264-pwm-v1-0-4c4ff743050f@n
vidia.com/T/#mfb40392e07d7ac9cedbaf853442eed822da7671e

so this is completely misplaced. Please read submitting patches in DT dir and follow standard style of sending patches upstream. Just like every other contributor. You don't get any exceptions here.

You asked to see patches, so I thought I'd be helpful and clean up my work in progress and respond. I'm not looking for any exceptions. Please do not make such accusations.

Quoted 2 lines

Best regards,
Krzysztof

#10 lore raw
Quoted 26 lines · Mikko Perttunen
Quoted 25 lines · Krzysztof Kozlowski
Quoted 24 lines · Mikko Perttunen
Quoted 13 lines · Krzysztof Kozlowski
Quoted 11 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---

 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Quoted 6 lines

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

Best regards,
Krzysztof

Just posted:
https://lore.kernel.org/linux-tegra/20260323-t264-pwm-v1-0-4c4ff743050f@n
vidia.com/T/#mfb40392e07d7ac9cedbaf853442eed822da7671e

Quoted 8 lines
Quoted 4 lines

so this is completely misplaced. Please read submitting patches in DT dir and follow standard style of sending patches upstream. Just like every other contributor. You don't get any exceptions here.

You asked to see patches, so I thought I'd be helpful and clean up my work in progress and respond. I'm not looking for any exceptions. Please do not make such accusations.

I asked where are the patches. The non-existing cover letter of this patchset explained nothing in this regard. After my question where are the patches, you sent them without explanation why, so don't assume I can read your intentions that you posted them just so I can see the patches and you do not intend to merge them.

Best regards,
Krzysztof

author maintainer
#11 lore raw
Quoted 36 lines · Krzysztof Kozlowski
Quoted 26 lines · Mikko Perttunen
Quoted 25 lines · Krzysztof Kozlowski
Quoted 24 lines · Mikko Perttunen
Quoted 13 lines · Krzysztof Kozlowski
Quoted 11 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---

 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Quoted 6 lines

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

Best regards,
Krzysztof

Just posted:
https://lore.kernel.org/linux-tegra/20260323-t264-pwm-v1-0-4c4ff743050f@n
vidia.com/T/#mfb40392e07d7ac9cedbaf853442eed822da7671e

Quoted 8 lines
Quoted 4 lines

so this is completely misplaced. Please read submitting patches in DT dir and follow standard style of sending patches upstream. Just like every other contributor. You don't get any exceptions here.

You asked to see patches, so I thought I'd be helpful and clean up my work in progress and respond. I'm not looking for any exceptions. Please do not make such accusations.

Quoted 5 lines

I asked where are the patches. The non-existing cover letter of this patchset explained nothing in this regard. After my question where are the patches, you sent them without explanation why, so don't assume I can read your intentions that you posted them just so I can see the patches and you do not intend to merge them.

Alright, alright. I'll take the blame for this. I should've coordinated better with Mikko and made my intentions clearer in the cover letter. Let's move forward by integrating both series.

Mikko, can you please integrate the DT bindings and DT patches into your driver series?

Thanks,
Thierry

#12 lore raw
Quoted 48 lines · Thierry Reding
Quoted 41 lines · Krzysztof Kozlowski
Quoted 32 lines · Mikko Perttunen
Quoted 31 lines · Krzysztof Kozlowski
Quoted 29 lines · Mikko Perttunen
Quoted 16 lines · Krzysztof Kozlowski
Quoted 8 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one
on prior generations, but it comes with some extra features, hence a new
compatible string is needed.

Quoted 6 lines

---

 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2
 ++
 1 file changed, 2 insertions(+)
Quoted 7 lines

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches
should be sent)?

Best regards,
Krzysztof

Just posted:
https://lore.kernel.org/linux-tegra/20260323-t264-pwm-v1-0-4c4ff743050
f@n vidia.com/T/#mfb40392e07d7ac9cedbaf853442eed822da7671e

Quoted 7 lines
Quoted 3 lines

so this is completely misplaced. Please read submitting patches in DT dir and follow standard style of sending patches upstream. Just like every other contributor. You don't get any exceptions here.

You asked to see patches, so I thought I'd be helpful and clean up my work in progress and respond. I'm not looking for any exceptions. Please do not make such accusations.

Quoted 5 lines

I asked where are the patches. The non-existing cover letter of this patchset explained nothing in this regard. After my question where are the patches, you sent them without explanation why, so don't assume I can read your intentions that you posted them just so I can see the patches and you do not intend to merge them.

Quoted 6 lines

Alright, alright. I'll take the blame for this. I should've coordinated better with Mikko and made my intentions clearer in the cover letter. Let's move forward by integrating both series.

Mikko, can you please integrate the DT bindings and DT patches into your driver series?

Will do.

Mikko

Quoted 3 lines

Thanks,
Thierry

author maintainer
#13 lore raw
In reply to Krzysztof Kozlowski
Quoted 7 lines · Krzysztof Kozlowski
Quoted 6 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

Quoted 8 lines
Quoted 4 lines
---
 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

I don't see anything in the documentation about a requirement for driver patches to be included. It does indicate what the ordering of patches should be if driver code is included, but doesn't specifically say that driver code must accompany binding updates.

Nor do I understand why it should be needed. I do understand why you would want to have driver code to go along with a completely new binding but in cases like this where we're really only adding a new compatible string to an otherwise unmodified binding, I don't see any advantage in having to put both together.

Thierry

#14 lore raw
Quoted 13 lines · Thierry Reding
Quoted 12 lines · Krzysztof Kozlowski
Quoted 10 lines · Thierry Reding

From: Thierry Reding <[email protected]>

The PWM controller found on Tegra264 is largely compatible with the one on prior generations, but it comes with some extra features, hence a new compatible string is needed.

---
 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 2 ++
 1 file changed, 2 insertions(+)
Quoted 7 lines
Quoted 3 lines

Where is the driver patch? Why this is not being part of driver submission (see also submitting bindings DT in description how patches should be sent)?

I don't see anything in the documentation about a requirement for driver patches to be included. It does indicate what the ordering of patches should be if driver code is included, but doesn't specifically say that

It does. Explicitly:
3) For a series going through multiple trees, the binding patch should be kept with the driver using the binding.

Quoted 1 line

driver code must accompany binding updates.

And you have entire cover letter to explain that...

Quoted 2 lines

Nor do I understand why it should be needed. I do understand why you

It's not needed. It is just odd not to have it here.

Quoted 4 lines

would want to have driver code to go along with a completely new binding but in cases like this where we're really only adding a new compatible string to an otherwise unmodified binding, I don't see any advantage in having to put both together.

And the problem appeared later - the driver was posted separately without the binding. That's a no go and really odd process.

For the other patches you have without drivers, it would be fine. And you would not get questions if it was explained.

Best regards,
Krzysztof