* [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022
@ 2022-03-08 7:21 Kuldeep Singh
2022-03-08 7:21 ` [PATCH v2 2/2] dt-bindings: spi: Update clock-names " Kuldeep Singh
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Kuldeep Singh @ 2022-03-08 7:21 UTC (permalink / raw)
To: Mark Brown, linux-arm-kernel, linux-spi, devicetree
Cc: Rob Herring, Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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(+)
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:
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] dt-bindings: spi: Update clock-names property for ARM pl022
2022-03-08 7:21 [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
@ 2022-03-08 7:21 ` Kuldeep Singh
2022-03-10 22:08 ` Rob Herring
2022-03-08 8:27 ` [PATCH v2 1/2] dt-bindings: spi: Update clocks " Krzysztof Kozlowski
2022-03-10 22:05 ` Rob Herring
2 siblings, 1 reply; 9+ messages in thread
From: Kuldeep Singh @ 2022-03-08 7:21 UTC (permalink / raw)
To: Mark Brown, linux-arm-kernel, linux-spi, devicetree
Cc: Rob Herring, Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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
+ - items:
+ - enum:
+ - sspclk
+ - const: apb_pclk
pl022,autosuspend-delay:
description: delay in ms following transfer completion before the
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022
2022-03-08 7:21 [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
2022-03-08 7:21 ` [PATCH v2 2/2] dt-bindings: spi: Update clock-names " Kuldeep Singh
@ 2022-03-08 8:27 ` Krzysztof Kozlowski
2022-03-09 13:57 ` Kuldeep Singh
2022-03-10 22:05 ` Rob Herring
2 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-08 8:27 UTC (permalink / raw)
To: Kuldeep Singh, Mark Brown, linux-arm-kernel, linux-spi, devicetree
Cc: Rob Herring, Robin Murphy, Linus Walleij
On 08/03/2022 08:21, 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
>
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
> v2:
> - Keep actual warning and remove path to file
> - Reword commit message a bit
It still misses information whether it is actually correct from PL022
point of view to have just one clock.
If the DTS are wrong, do not change the bindings to match such wrong
DTS. If the DTS is correct, please explain why bindings are wrong.
>
> Documentation/devicetree/bindings/spi/spi-pl022.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> 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
This does not match clock-names which requires two clocks. It's not
correct now.
>
> clock-names:
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022
2022-03-08 8:27 ` [PATCH v2 1/2] dt-bindings: spi: Update clocks " Krzysztof Kozlowski
@ 2022-03-09 13:57 ` Kuldeep Singh
0 siblings, 0 replies; 9+ messages in thread
From: Kuldeep Singh @ 2022-03-09 13:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree, Rob Herring,
Robin Murphy, Linus Walleij
On Tue, Mar 08, 2022 at 09:27:48AM +0100, Krzysztof Kozlowski wrote:
> On 08/03/2022 08:21, 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
> >
> > Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> > ---
> > v2:
> > - Keep actual warning and remove path to file
> > - Reword commit message a bit
>
> It still misses information whether it is actually correct from PL022
> point of view to have just one clock.
>
> If the DTS are wrong, do not change the bindings to match such wrong
> DTS. If the DTS is correct, please explain why bindings are wrong.
Thanks Krzysztof for pointing it out.
So far I was wondering spiclk clock-name was the only culprit there and
had different perception w.r.t number of clocks.
Anyway, below reference is from pl022 point of view:
https://documentation-service.arm.com/static/5e8e3bc7fd977155116a936d?token=
https://developer.arm.com/documentation/ddi0194/h/functional-overview/primecell-ssp-operation/clock-ratios
The reference no where mention that single clock can exist.
So far Amd seattle and LG platforms don't comply with binding and define
single clock which require updations.
So now from bindings perspective, I am looking forward to keep only one
notation of sspclk and will respin a patch. Kindly provide comments on
the same.
Regards
Kuldeep
> >
> > Documentation/devicetree/bindings/spi/spi-pl022.yaml | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > 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
>
> This does not match clock-names which requires two clocks. It's not
> correct now.
>
> >
> > clock-names:
>
>
> Best regards,
> Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022
2022-03-08 7:21 [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
2022-03-08 7:21 ` [PATCH v2 2/2] dt-bindings: spi: Update clock-names " Kuldeep Singh
2022-03-08 8:27 ` [PATCH v2 1/2] dt-bindings: spi: Update clocks " Krzysztof Kozlowski
@ 2022-03-10 22:05 ` Rob Herring
2022-03-11 2:55 ` Kuldeep Singh
2 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2022-03-10 22:05 UTC (permalink / raw)
To: Kuldeep Singh
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree,
Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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.
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.
The other issue with allowing a single clock is then any new user can
just repeat this mistake.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: spi: Update clock-names property for ARM pl022
2022-03-08 7:21 ` [PATCH v2 2/2] dt-bindings: spi: Update clock-names " Kuldeep Singh
@ 2022-03-10 22:08 ` Rob Herring
2022-03-11 2:57 ` Kuldeep Singh
0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2022-03-10 22:08 UTC (permalink / raw)
To: Kuldeep Singh
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree,
Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022
2022-03-10 22:05 ` Rob Herring
@ 2022-03-11 2:55 ` Kuldeep Singh
2022-03-11 3:31 ` Kuldeep Singh
0 siblings, 1 reply; 9+ messages in thread
From: Kuldeep Singh @ 2022-03-11 2:55 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree,
Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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.
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: spi: Update clock-names property for ARM pl022
2022-03-10 22:08 ` Rob Herring
@ 2022-03-11 2:57 ` Kuldeep Singh
0 siblings, 0 replies; 9+ messages in thread
From: Kuldeep Singh @ 2022-03-11 2:57 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree,
Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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
> >
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022
2022-03-11 2:55 ` Kuldeep Singh
@ 2022-03-11 3:31 ` Kuldeep Singh
0 siblings, 0 replies; 9+ messages in thread
From: Kuldeep Singh @ 2022-03-11 3:31 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree,
Robin Murphy, Linus Walleij, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-11 3:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-08 7:21 [PATCH v2 1/2] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
2022-03-08 7:21 ` [PATCH v2 2/2] dt-bindings: spi: Update clock-names " Kuldeep Singh
2022-03-10 22:08 ` Rob Herring
2022-03-11 2:57 ` Kuldeep Singh
2022-03-08 8:27 ` [PATCH v2 1/2] dt-bindings: spi: Update clocks " Krzysztof Kozlowski
2022-03-09 13:57 ` Kuldeep Singh
2022-03-10 22:05 ` Rob Herring
2022-03-11 2:55 ` Kuldeep Singh
2022-03-11 3:31 ` Kuldeep Singh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).