| Message ID | 20220307205357.66322-2-singh.kuldeep87k@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Arm based platforms dtc fixes for pl022 | expand |
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
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
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 {
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(-)