diff mbox series

[v2,1/2] dt-bindings: spi: Update clocks property for ARM pl022

Message ID 20220308072125.38381-1-singh.kuldeep87k@gmail.com
State New
Headers show
Series [v2,1/2] dt-bindings: spi: Update clocks property for ARM pl022 | expand

Commit Message

Kuldeep Singh March 8, 2022, 7:21 a.m. UTC
Add missing minItems property to clocks in ARM pl022 bindings.

This helps in resolving below warnings:
clocks: [[4]] is too short
clock-names: ['apb_pclk'] is too short

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
v2:
- Keep actual warning and remove path to file
- Reword commit message a bit

 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Rob Herring March 10, 2022, 10:08 p.m. UTC | #1
On Tue, Mar 08, 2022 at 12:51:25PM +0530, Kuldeep Singh wrote:
> Pl022 clock-names can be one of following:
> ['apb_pclk'] or ['sspclk', 'apb_pclk']
> 
> The current schema refers to second case only. Make necessary changes to
> incorporate both the cases and resolve below dtc warning:
> clock-names: 'apb_pclk' is not one of ['sspclk']
> 
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
> v2:
> - Reword commit message
> - Drop SSPCLK
> 
>  Documentation/devicetree/bindings/spi/spi-pl022.yaml | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> index 7d36e15db5b3..6cfab948624e 100644
> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> @@ -38,11 +38,13 @@ properties:
>      maxItems: 2
>  
>    clock-names:
> -    items:
> +    oneOf:
>        - enum:
> -          - SSPCLK
> -          - sspclk
> -      - const: apb_pclk
> +          - apb_pclk

Are you going to make the driver work with no SPI clock? Because this 
change is saying that it is valid to have the APB bus clock and no SPI 
clock.

Rob

> +      - items:
> +          - enum:
> +              - sspclk
> +          - const: apb_pclk
>  
>    pl022,autosuspend-delay:
>      description: delay in ms following transfer completion before the
> -- 
> 2.25.1
> 
>
Kuldeep Singh March 11, 2022, 2:57 a.m. UTC | #2
On Thu, Mar 10, 2022 at 04:08:42PM -0600, Rob Herring wrote:
> On Tue, Mar 08, 2022 at 12:51:25PM +0530, Kuldeep Singh wrote:
> > Pl022 clock-names can be one of following:
> > ['apb_pclk'] or ['sspclk', 'apb_pclk']
> > 
> > The current schema refers to second case only. Make necessary changes to
> > incorporate both the cases and resolve below dtc warning:
> > clock-names: 'apb_pclk' is not one of ['sspclk']
> > 
> > Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> > ---
> > v2:
> > - Reword commit message
> > - Drop SSPCLK
> > 
> >  Documentation/devicetree/bindings/spi/spi-pl022.yaml | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> > index 7d36e15db5b3..6cfab948624e 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> > @@ -38,11 +38,13 @@ properties:
> >      maxItems: 2
> >  
> >    clock-names:
> > -    items:
> > +    oneOf:
> >        - enum:
> > -          - SSPCLK
> > -          - sspclk
> > -      - const: apb_pclk
> > +          - apb_pclk
> 
> Are you going to make the driver work with no SPI clock? Because this 
> change is saying that it is valid to have the APB bus clock and no SPI 
> clock.

Kindly take a loot at newer version to this series as this one is
deprecated.

https://lore.kernel.org/linux-spi/20220309171847.5345-1-singh.kuldeep87k@gmail.com/

- Kuldeep
> 
> Rob
> 
> > +      - items:
> > +          - enum:
> > +              - sspclk
> > +          - const: apb_pclk
> >  
> >    pl022,autosuspend-delay:
> >      description: delay in ms following transfer completion before the
> > -- 
> > 2.25.1
> > 
> >
Kuldeep Singh March 11, 2022, 3:31 a.m. UTC | #3
On Fri, Mar 11, 2022 at 08:25:02AM +0530, Kuldeep Singh wrote:
> On Thu, Mar 10, 2022 at 04:05:37PM -0600, Rob Herring wrote:
> > On Tue, Mar 08, 2022 at 12:51:24PM +0530, Kuldeep Singh wrote:
> > > Add missing minItems property to clocks in ARM pl022 bindings.
> > > 
> > > This helps in resolving below warnings:
> > > clocks: [[4]] is too short
> > > clock-names: ['apb_pclk'] is too short
> > 
> > Again, the error is in the dts files, not the schema.
> 
> Rob, kindly note this series number is deprecated and I have sent v3
> version some time back. Here's the link:
> 
> https://lore.kernel.org/linux-spi/20220309171847.5345-1-singh.kuldeep87k@gmail.com/T/#u
> 
> > 
> > 
> > There's 2 possible answers. First, both clock inputs use the same source 
> > clock. That's an easy fix. List the clock twice. Second, one clock is 
> > not described in DT or visible to s/w. It still has to be in the h/w and 
> > could be described as a 'fixed-clock'. A DT should either be all in with 
> > clocks or not use the clock binding IMO. Describing some clocks and not 
> > others is not a good solution.
> > 
> > For example, let's look at bcm-cygnus as one of the single clock 
> > examples. The first thing I notice is there is a apb_pclk already 
> > defined. The pl330 uses it. The watchdog (also Primecell) lists the 
> > source clock twice. So what should pl022 be? IDK, ask the Broadcom 
> > folks. If they don't know, then list the source clock twice. That's 
> > effectively no change from what we have now.

I just noticed not all platforms possessing single clock are raising
'dtbs_check' warning. For example, bcm-cygnus and lpc32xx passes check
even though their DT clock entry has just "apb_pclk".

Any reason why they pass successfully through checks?

> 
> Yes, I took motivation from sp805 watchdog(primecell) while resolving DT
> conflicts. I found LG and amd seattle platform with single clock in DT
> for which I have sent patches. Link is below:
> 
> https://lore.kernel.org/linux-devicetree/CAL_Jsq+k+ridWTkdy4xwTC7VxUTU8tu+Q2BA9kbQVA222PWvZw@mail.gmail.com/
> 
> Moreover, I observed that clocks and clock-names are not required
> properties for pl022. I am wondering reason behind the same when you
> first made changes. Any specific reason not adding them which I am not
> aware of or it just got missed?
> 
> - Kuldeep
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
index 6d633728fc2b..7d36e15db5b3 100644
--- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
@@ -34,6 +34,7 @@  properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     maxItems: 2
 
   clock-names: