[PATCH 0/2] Arm based platforms dtc fixes for pl022

[PATCH 0/2] Arm based platforms dtc fixes for pl022

[Kuldeep Singh]

This patchset is an attempt to fix device trees and resolve bindings
warning for pl022. This requires updation in clock name to keep alignment
with other platforms and spi node properties for integrator platform.

This patch is based on
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git,
master
Since, these patches are based on same tree, so i added them in series
for seamless application.

Kuldeep Singh (2):
  arm: dts: realview/versatile/ste: Update spi clock name
  arm: dts: integrator: Update spi node properties

 arch/arm/boot/dts/arm-realview-eb.dtsi    |  2 +-
 arch/arm/boot/dts/arm-realview-pb1176.dts |  2 +-
 arch/arm/boot/dts/arm-realview-pb11mp.dts |  2 +-
 arch/arm/boot/dts/arm-realview-pbx.dtsi   |  2 +-
 arch/arm/boot/dts/integratorap-im-pd1.dts |  4 ++--
 arch/arm/boot/dts/ste-dbx5x0.dtsi         | 12 ++++++------
 arch/arm/boot/dts/versatile-ab.dts        |  2 +-
 7 files changed, 13 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH 1/2] arm: dts: realview/versatile/ste: Update spi clock name

[Kuldeep Singh]

SPI clock for pl022 is "sspclk" and meanwhile few platforms such as
realview, versatile etc. specify "SSPCLK" as clock name. Even though
bindings checks don't differentiate between the two names still keep
same convention throughout i.e sspclk to align with other platforms.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 arch/arm/boot/dts/arm-realview-eb.dtsi    |  2 +-
 arch/arm/boot/dts/arm-realview-pb1176.dts |  2 +-
 arch/arm/boot/dts/arm-realview-pb11mp.dts |  2 +-
 arch/arm/boot/dts/arm-realview-pbx.dtsi   |  2 +-
 arch/arm/boot/dts/ste-dbx5x0.dtsi         | 12 ++++++------
 arch/arm/boot/dts/versatile-ab.dts        |  2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/arm-realview-eb.dtsi b/arch/arm/boot/dts/arm-realview-eb.dtsi
index 2dfb32bf9d48..fbb2258b451f 100644
--- a/arch/arm/boot/dts/arm-realview-eb.dtsi
+++ b/arch/arm/boot/dts/arm-realview-eb.dtsi
@@ -399,7 +399,7 @@ ssp: spi@1000d000 {
 			compatible = "arm,pl022", "arm,primecell";
 			reg = <0x1000d000 0x1000>;
 			clocks = <&sspclk>, <&pclk>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 		};
 
 		wdog: watchdog@10010000 {
diff --git a/arch/arm/boot/dts/arm-realview-pb1176.dts b/arch/arm/boot/dts/arm-realview-pb1176.dts
index 06b8723b09eb..efed325af88d 100644
--- a/arch/arm/boot/dts/arm-realview-pb1176.dts
+++ b/arch/arm/boot/dts/arm-realview-pb1176.dts
@@ -410,7 +410,7 @@ pb1176_ssp: spi@1010b000 {
 			interrupt-parent = <&intc_dc1176>;
 			interrupts = <0 17 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&sspclk>, <&pclk>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 		};
 
 		pb1176_serial0: serial@1010c000 {
diff --git a/arch/arm/boot/dts/arm-realview-pb11mp.dts b/arch/arm/boot/dts/arm-realview-pb11mp.dts
index 295aef448123..89103d54ecc1 100644
--- a/arch/arm/boot/dts/arm-realview-pb11mp.dts
+++ b/arch/arm/boot/dts/arm-realview-pb11mp.dts
@@ -555,7 +555,7 @@ spi@1000d000 {
 			interrupt-parent = <&intc_pb11mp>;
 			interrupts = <0 11 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&sspclk>, <&pclk>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 		};
 
 		watchdog@1000f000 {
diff --git a/arch/arm/boot/dts/arm-realview-pbx.dtsi b/arch/arm/boot/dts/arm-realview-pbx.dtsi
index 6f61f968d689..ec1507c5147c 100644
--- a/arch/arm/boot/dts/arm-realview-pbx.dtsi
+++ b/arch/arm/boot/dts/arm-realview-pbx.dtsi
@@ -390,7 +390,7 @@ ssp: spi@1000d000 {
 			compatible = "arm,pl022", "arm,primecell";
 			reg = <0x1000d000 0x1000>;
 			clocks = <&sspclk>, <&pclk>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 		};
 
 		wdog0: watchdog@1000f000 {
diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
index dc0bcc7020f1..c28b32640254 100644
--- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
+++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
@@ -755,7 +755,7 @@ ssp0: spi@80002000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			clocks = <&prcc_kclk 3 1>, <&prcc_pclk 3 1>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 			dmas = <&dma 8 0 0x2>, /* Logical - DevToMem */
 			       <&dma 8 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
@@ -772,7 +772,7 @@ ssp1: spi@80003000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			clocks = <&prcc_kclk 3 2>, <&prcc_pclk 3 2>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 			dmas = <&dma 9 0 0x2>, /* Logical - DevToMem */
 			       <&dma 9 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
@@ -790,7 +790,7 @@ spi0: spi@8011a000 {
 			#size-cells = <0>;
 			/* Same clock wired to kernel and pclk */
 			clocks = <&prcc_pclk 2 8>, <&prcc_pclk 2 8>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 			dmas = <&dma 0 0 0x2>, /* Logical - DevToMem */
 			       <&dma 0 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
@@ -807,7 +807,7 @@ spi1: spi@80112000 {
 			#size-cells = <0>;
 			/* Same clock wired to kernel and pclk */
 			clocks = <&prcc_pclk 2 2>, <&prcc_pclk 2 2>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 			dmas = <&dma 35 0 0x2>, /* Logical - DevToMem */
 			       <&dma 35 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
@@ -824,7 +824,7 @@ spi2: spi@80111000 {
 			#size-cells = <0>;
 			/* Same clock wired to kernel and pclk */
 			clocks = <&prcc_pclk 2 1>, <&prcc_pclk 2 1>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 			dmas = <&dma 33 0 0x2>, /* Logical - DevToMem */
 			       <&dma 33 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
@@ -841,7 +841,7 @@ spi3: spi@80129000 {
 			#size-cells = <0>;
 			/* Same clock wired to kernel and pclk */
 			clocks = <&prcc_pclk 1 7>, <&prcc_pclk 1 7>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 			dmas = <&dma 40 0 0x2>, /* Logical - DevToMem */
 			       <&dma 40 0 0x0>; /* Logical - MemToDev */
 			dma-names = "rx", "tx";
diff --git a/arch/arm/boot/dts/versatile-ab.dts b/arch/arm/boot/dts/versatile-ab.dts
index 79f7cc241282..a520615f4d8d 100644
--- a/arch/arm/boot/dts/versatile-ab.dts
+++ b/arch/arm/boot/dts/versatile-ab.dts
@@ -391,7 +391,7 @@ spi@101f4000 {
 			reg = <0x101f4000 0x1000>;
 			interrupts = <11>;
 			clocks = <&xtal24mhz>, <&pclk>;
-			clock-names = "SSPCLK", "apb_pclk";
+			clock-names = "sspclk", "apb_pclk";
 		};
 
 		fpga {
-- 
2.25.1

Re: [PATCH 1/2] arm: dts: realview/versatile/ste: Update spi clock name

[Rob Herring]

On Mon, Mar 7, 2022 at 2:54 PM Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
>
> SPI clock for pl022 is "sspclk" and meanwhile few platforms such as
> realview, versatile etc. specify "SSPCLK" as clock name. Even though
> bindings checks don't differentiate between the two names still keep
> same convention throughout i.e sspclk to align with other platforms.

I don't see the point in worrying about this. The binding already
allows both. Why? Because I wrote the schema and checked it against
all the in tree users. The resulting warnings are what should be fixed
in the dts files IMO. What's not warning doesn't need to be fixed.

Rob

Re: [PATCH 1/2] arm: dts: realview/versatile/ste: Update spi clock name

[Kuldeep Singh]

On Mon, Mar 07, 2022 at 07:14:09PM -0600, Rob Herring wrote:
> On Mon, Mar 7, 2022 at 2:54 PM Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> >
> > SPI clock for pl022 is "sspclk" and meanwhile few platforms such as
> > realview, versatile etc. specify "SSPCLK" as clock name. Even though
> > bindings checks don't differentiate between the two names still keep
> > same convention throughout i.e sspclk to align with other platforms.
> 
> I don't see the point in worrying about this. The binding already
> allows both. Why? Because I wrote the schema and checked it against
> all the in tree users. The resulting warnings are what should be fixed
> in the dts files IMO. What's not warning doesn't need to be fixed.

Hi Rob,

I personally share a different opinion though. Kindly bear with me for
my explanations.

As there were no checks on DTs before, so DTs have been defining names
which may not align with bindings. In case of pl022, doc says sspclk is
connected clock and whereas binding specify SSPCLK and sspclk both. Why
keep both versions in bindings? just because DTs have it. I don't think
it's justifiable enough and that's what prompted me to keep single
entry. Please let me know in case my understanding is incorrect.

I kept "sspclk" as it is aligned with "apb_pclk" and I have not seen
many DT defining names in caps.

Moreover, I ran out few checks and came up with some observations:
	1. Keep SSPCLK in binding, 'make dtbs_check' doesn't complain. An
	obvious behavior.
	2. Remove SSPCLK from binding, 'make dtbs_check' again doesn't
	complain.

For 2), though binding doesn't have SSPCLK, still DTs with clock-name
SSPCLK do not complain which further leads me to conclusion that casing
is not considered while comparing. So, why should we keep both and can't
simply discard other one.

This may not be a concern as per warnings point of view, but let's say
in future comparison checks get even more stricter, then this will roll
out as a warning and needs to be looked again. Why shall we leave out a
chance which may need to be revisited later?

Regards
Kuldeep

[PATCH 2/2] arm: dts: integrator: Update spi node properties

[Kuldeep Singh]

SPI clock name for pl022 is "sspclk" and not "spiclk".
Fix below dtc warning:
clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']

Also, update node name to spi instead of ssp to enable spi bindings
check.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 arch/arm/boot/dts/integratorap-im-pd1.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/integratorap-im-pd1.dts b/arch/arm/boot/dts/integratorap-im-pd1.dts
index d47bfb66d069..ebe938556645 100644
--- a/arch/arm/boot/dts/integratorap-im-pd1.dts
+++ b/arch/arm/boot/dts/integratorap-im-pd1.dts
@@ -178,12 +178,12 @@ uart@200000 {
 		clock-names = "uartclk", "apb_pclk";
 	};
 
-	ssp@300000 {
+	ssp0: spi@300000 {
 		compatible = "arm,pl022", "arm,primecell";
 		reg = <0x00300000 0x1000>;
 		interrupts-extended = <&impd1_vic 3>;
 		clocks = <&impd1_sspclk>, <&sysclk>;
-		clock-names = "spiclk", "apb_pclk";
+		clock-names = "sspclk", "apb_pclk";
 	};
 
 	impd1_gpio0: gpio@400000 {
-- 
2.25.1

Re: [PATCH 2/2] arm: dts: integrator: Update spi node properties

[Krzysztof Kozlowski]

On 07/03/2022 21:53, Kuldeep Singh wrote:
> SPI clock name for pl022 is "sspclk" and not "spiclk".
> Fix below dtc warning:
> clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
> 
> Also, update node name to spi instead of ssp to enable spi bindings
> check.
> 
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
>  arch/arm/boot/dts/integratorap-im-pd1.dts | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/integratorap-im-pd1.dts b/arch/arm/boot/dts/integratorap-im-pd1.dts
> index d47bfb66d069..ebe938556645 100644
> --- a/arch/arm/boot/dts/integratorap-im-pd1.dts
> +++ b/arch/arm/boot/dts/integratorap-im-pd1.dts
> @@ -178,12 +178,12 @@ uart@200000 {
>  		clock-names = "uartclk", "apb_pclk";
>  	};
>  
> -	ssp@300000 {
> +	ssp0: spi@300000 {

Is the alias used anywhere? Why adding it?



Best regards,
Krzysztof

Re: [PATCH 2/2] arm: dts: integrator: Update spi node properties

[Kuldeep Singh]

On Mon, Mar 07, 2022 at 10:57:40PM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2022 21:53, Kuldeep Singh wrote:
> > SPI clock name for pl022 is "sspclk" and not "spiclk".
> > Fix below dtc warning:
> > clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
> > 
> > Also, update node name to spi instead of ssp to enable spi bindings
> > check.
> > 
> > Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> > ---
> >  arch/arm/boot/dts/integratorap-im-pd1.dts | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/integratorap-im-pd1.dts b/arch/arm/boot/dts/integratorap-im-pd1.dts
> > index d47bfb66d069..ebe938556645 100644
> > --- a/arch/arm/boot/dts/integratorap-im-pd1.dts
> > +++ b/arch/arm/boot/dts/integratorap-im-pd1.dts
> > @@ -178,12 +178,12 @@ uart@200000 {
> >  		clock-names = "uartclk", "apb_pclk";
> >  	};
> >  
> > -	ssp@300000 {
> > +	ssp0: spi@300000 {
> 
> Is the alias used anywhere? Why adding it?

I added an alias so as not to lose originality. At other places, ssp is
used more specifically for spi controller node and since here I need to
enable bindings check, so I mentioned both.

There are other platforms too which are using the same notation and
personally, I couldn't find any harm in this.

Regards
Kuldeep