* [PATCH 0/3] DTC fixes for Arm pl022 bindings
@ 2022-02-28 12:43 Kuldeep Singh
2022-02-28 12:43 ` [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Kuldeep Singh @ 2022-02-28 12:43 UTC (permalink / raw)
To: Mark Brown, linux-arm-kernel, linux-spi, devicetree, linux-kernel
Cc: Rob Herring, Linus Walleij
This patchset helps in resolving dtc warnings for arm pl022 bindings.
Fixing requires updation to clock and clock-names properties.
One more dts fix is required to eliminate all warnings which will be
sent from other tree.
Please note, this patch is based on top of
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git, for-next
Kuldeep Singh (3):
dt-bindings: spi: Update clocks property for ARM pl022
dt-bindings: spi: Update clock-names property for ARM pl022
dt-bindings: spi: Add spiclk to clock-names property in pl022
.../devicetree/bindings/spi/spi-pl022.yaml | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022
2022-02-28 12:43 [PATCH 0/3] DTC fixes for Arm pl022 bindings Kuldeep Singh
@ 2022-02-28 12:43 ` Kuldeep Singh
2022-02-28 14:26 ` Robin Murphy
2022-02-28 12:43 ` [PATCH 2/3] dt-bindings: spi: Update clock-names " Kuldeep Singh
2022-02-28 12:43 ` [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022 Kuldeep Singh
2 siblings, 1 reply; 11+ messages in thread
From: Kuldeep Singh @ 2022-02-28 12:43 UTC (permalink / raw)
To: Mark Brown, linux-arm-kernel, linux-spi, devicetree, linux-kernel
Cc: Rob Herring, Linus Walleij
Add missing minItems property to clocks in ARM pl022 bindings.
This also helps in resolving below dtc warnings:
arch/arm64/boot/dts/amd/amd-overdrive.dt.yaml: spi@e1020000: clocks: [[4]] is too short
From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
arch/arm64/boot/dts/amd/amd-overdrive.dt.yaml: spi@e1020000: clock-names: ['apb_pclk'] is too short
From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] dt-bindings: spi: Update clock-names property for ARM pl022
2022-02-28 12:43 [PATCH 0/3] DTC fixes for Arm pl022 bindings Kuldeep Singh
2022-02-28 12:43 ` [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
@ 2022-02-28 12:43 ` Kuldeep Singh
2022-02-28 12:43 ` [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022 Kuldeep Singh
2 siblings, 0 replies; 11+ messages in thread
From: Kuldeep Singh @ 2022-02-28 12:43 UTC (permalink / raw)
To: Mark Brown, linux-arm-kernel, linux-spi, devicetree, linux-kernel
Cc: Rob Herring, Linus Walleij
Below are the possibilities of pl022 clock-names property:
- apb_pclk
- [sspclk, SSPCLK, spiclk], apb_pclk
The current schema refers to second case and does not consider first
one. Hence, resolve the below dtc warnings by updating clock properties:
arch/arm64/boot/dts/lg/lg1313-ref.dt.yaml: spi@fe900000: clock-names:0:
'apb_pclk' is not one of ['SSPCLK', 'sspclk']
From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
index 7d36e15db5b3..5f6926a58b15 100644
--- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
@@ -38,11 +38,14 @@ properties:
maxItems: 2
clock-names:
- items:
+ oneOf:
- enum:
- - SSPCLK
- - sspclk
- - const: apb_pclk
+ - apb_pclk
+ - items:
+ - enum:
+ - sspclk
+ - SSPCLK
+ - const: apb_pclk
pl022,autosuspend-delay:
description: delay in ms following transfer completion before the
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022
2022-02-28 12:43 [PATCH 0/3] DTC fixes for Arm pl022 bindings Kuldeep Singh
2022-02-28 12:43 ` [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
2022-02-28 12:43 ` [PATCH 2/3] dt-bindings: spi: Update clock-names " Kuldeep Singh
@ 2022-02-28 12:43 ` Kuldeep Singh
2022-02-28 14:36 ` Robin Murphy
2 siblings, 1 reply; 11+ messages in thread
From: Kuldeep Singh @ 2022-02-28 12:43 UTC (permalink / raw)
To: Mark Brown, linux-arm-kernel, linux-spi, devicetree, linux-kernel
Cc: Rob Herring, Linus Walleij
Fix below dtc warning by making necessary addition of "spiclk" in
clock-names property.
arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dt.yaml: spi@190000:
clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
arch/arm64/boot/dts/broadcom/northstar2/ns2-svk.dt.yaml: spi@66190000:
clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
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 5f6926a58b15..fb3075a0c7fd 100644
--- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
@@ -45,6 +45,7 @@ properties:
- enum:
- sspclk
- SSPCLK
+ - spiclk
- const: apb_pclk
pl022,autosuspend-delay:
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022
2022-02-28 12:43 ` [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
@ 2022-02-28 14:26 ` Robin Murphy
2022-02-28 15:11 ` Mark Brown
0 siblings, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2022-02-28 14:26 UTC (permalink / raw)
To: Kuldeep Singh, Mark Brown, linux-arm-kernel, linux-spi,
devicetree, linux-kernel
Cc: Rob Herring, Linus Walleij
On 2022-02-28 12:43, Kuldeep Singh wrote:
> Add missing minItems property to clocks in ARM pl022 bindings.
>
> This also helps in resolving below dtc warnings:
> arch/arm64/boot/dts/amd/amd-overdrive.dt.yaml: spi@e1020000: clocks: [[4]] is too short
> From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
> arch/arm64/boot/dts/amd/amd-overdrive.dt.yaml: spi@e1020000: clock-names: ['apb_pclk'] is too short
> From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
Who says that minItems is missing? Looking at the PL022 TRM[1] it seems
clear that SSPCLK is pretty fundamental to useful operation. If that DT
ever worked, it must be that the same clock is wired to both inputs, and
the fact that that's how the neighbouring PL011 is described is strongly
suggestive.
If the point of schema is to find errors in DTs, doesn't it make more
sense to fix the DTs than to weaken the schema just to shut it up?
Of course in this particular case there's also the question of whether
the most humane way to "fix" the Seattle DTs is to simply delete them,
but that's orthogonal.
Robin.
[1] https://developer.arm.com/documentation/ddi0194/h/?lang=en
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
> 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:
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022
2022-02-28 12:43 ` [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022 Kuldeep Singh
@ 2022-02-28 14:36 ` Robin Murphy
2022-03-02 19:05 ` Kuldeep Singh
0 siblings, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2022-02-28 14:36 UTC (permalink / raw)
To: Kuldeep Singh, Mark Brown, linux-arm-kernel, linux-spi,
devicetree, linux-kernel
Cc: Rob Herring, Linus Walleij
On 2022-02-28 12:43, Kuldeep Singh wrote:
> Fix below dtc warning by making necessary addition of "spiclk" in
> clock-names property.
>
> arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dt.yaml: spi@190000:
> clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
> From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
> arch/arm64/boot/dts/broadcom/northstar2/ns2-svk.dt.yaml: spi@66190000:
> clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
> From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
As before, what makes the binding at fault rather than that DT? The
PL022's actual input is named SSPCLK, not SPICLK, so why should a driver
which wants to look up that clock by name expect to look for "spiclk"?
Robin.
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
> 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 5f6926a58b15..fb3075a0c7fd 100644
> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> @@ -45,6 +45,7 @@ properties:
> - enum:
> - sspclk
> - SSPCLK
> + - spiclk
> - const: apb_pclk
>
> pl022,autosuspend-delay:
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022
2022-02-28 14:26 ` Robin Murphy
@ 2022-02-28 15:11 ` Mark Brown
2022-02-28 15:27 ` Robin Murphy
0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2022-02-28 15:11 UTC (permalink / raw)
To: Robin Murphy
Cc: Kuldeep Singh, linux-arm-kernel, linux-spi, devicetree,
linux-kernel, Rob Herring, Linus Walleij
[-- Attachment #1: Type: text/plain, Size: 920 bytes --]
On Mon, Feb 28, 2022 at 02:26:12PM +0000, Robin Murphy wrote:
> Who says that minItems is missing? Looking at the PL022 TRM[1] it seems
> clear that SSPCLK is pretty fundamental to useful operation. If that DT ever
> worked, it must be that the same clock is wired to both inputs, and the fact
> that that's how the neighbouring PL011 is described is strongly suggestive.
Well, it could also be that the clock is wired to some other clock which
is always on (which I guess is why the driver allows this in the first
place, there's a lot of sloppy code around stuff like that in the tree).
> If the point of schema is to find errors in DTs, doesn't it make more sense
> to fix the DTs than to weaken the schema just to shut it up?
I do agree on this point though, given that the clock is actually
required for useful operation what's happening here is that the schema
is detecting an error in the use of the binding.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022
2022-02-28 15:11 ` Mark Brown
@ 2022-02-28 15:27 ` Robin Murphy
2022-02-28 15:46 ` Mark Brown
0 siblings, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2022-02-28 15:27 UTC (permalink / raw)
To: Mark Brown
Cc: Kuldeep Singh, linux-arm-kernel, linux-spi, devicetree,
linux-kernel, Rob Herring, Linus Walleij
On 2022-02-28 15:11, Mark Brown wrote:
> On Mon, Feb 28, 2022 at 02:26:12PM +0000, Robin Murphy wrote:
>
>> Who says that minItems is missing? Looking at the PL022 TRM[1] it seems
>> clear that SSPCLK is pretty fundamental to useful operation. If that DT ever
>> worked, it must be that the same clock is wired to both inputs, and the fact
>> that that's how the neighbouring PL011 is described is strongly suggestive.
>
> Well, it could also be that the clock is wired to some other clock which
> is always on (which I guess is why the driver allows this in the first
> place, there's a lot of sloppy code around stuff like that in the tree).
I wouldn't say the driver "allows" it, so much as it just blindly grabs
the first clock assuming it's SSPCLK per the binding, and thus it will
happen to work out if the underlying physical clock is the same as, or
equivalent to, the APB PCLK. Otherwise, it's already into some degree of
not working properly, by virtue of reading the wrong clock rate.
Robin.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022
2022-02-28 15:27 ` Robin Murphy
@ 2022-02-28 15:46 ` Mark Brown
2022-02-28 16:01 ` Robin Murphy
0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2022-02-28 15:46 UTC (permalink / raw)
To: Robin Murphy
Cc: Kuldeep Singh, linux-arm-kernel, linux-spi, devicetree,
linux-kernel, Rob Herring, Linus Walleij
[-- Attachment #1: Type: text/plain, Size: 836 bytes --]
On Mon, Feb 28, 2022 at 03:27:08PM +0000, Robin Murphy wrote:
> On 2022-02-28 15:11, Mark Brown wrote:
> > Well, it could also be that the clock is wired to some other clock which
> > is always on (which I guess is why the driver allows this in the first
> > place, there's a lot of sloppy code around stuff like that in the tree).
> I wouldn't say the driver "allows" it, so much as it just blindly grabs the
> first clock assuming it's SSPCLK per the binding, and thus it will happen to
> work out if the underlying physical clock is the same as, or equivalent to,
> the APB PCLK. Otherwise, it's already into some degree of not working
> properly, by virtue of reading the wrong clock rate.
Ah, the APB clock requirement is inherited from the AMBA implementation
isn't it? We really ought to be extending an AMBA binding here...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022
2022-02-28 15:46 ` Mark Brown
@ 2022-02-28 16:01 ` Robin Murphy
0 siblings, 0 replies; 11+ messages in thread
From: Robin Murphy @ 2022-02-28 16:01 UTC (permalink / raw)
To: Mark Brown
Cc: Kuldeep Singh, linux-arm-kernel, linux-spi, devicetree,
linux-kernel, Rob Herring, Linus Walleij
On 2022-02-28 15:46, Mark Brown wrote:
> On Mon, Feb 28, 2022 at 03:27:08PM +0000, Robin Murphy wrote:
>> On 2022-02-28 15:11, Mark Brown wrote:
>
>>> Well, it could also be that the clock is wired to some other clock which
>>> is always on (which I guess is why the driver allows this in the first
>>> place, there's a lot of sloppy code around stuff like that in the tree).
>
>> I wouldn't say the driver "allows" it, so much as it just blindly grabs the
>> first clock assuming it's SSPCLK per the binding, and thus it will happen to
>> work out if the underlying physical clock is the same as, or equivalent to,
>> the APB PCLK. Otherwise, it's already into some degree of not working
>> properly, by virtue of reading the wrong clock rate.
>
> Ah, the APB clock requirement is inherited from the AMBA implementation
> isn't it? We really ought to be extending an AMBA binding here...
Yup, both the "apb_pclk" clock specifier and the "arm,primecell"
compatible technically belong to the common AMBA binding, but I'm not
sure whether schema has the ability to compose at such fine-grained a
level :/
Robin.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022
2022-02-28 14:36 ` Robin Murphy
@ 2022-03-02 19:05 ` Kuldeep Singh
0 siblings, 0 replies; 11+ messages in thread
From: Kuldeep Singh @ 2022-03-02 19:05 UTC (permalink / raw)
To: Robin Murphy
Cc: Mark Brown, linux-arm-kernel, linux-spi, devicetree,
linux-kernel, Rob Herring, Linus Walleij
On Mon, Feb 28, 2022 at 02:36:23PM +0000, Robin Murphy wrote:
> On 2022-02-28 12:43, Kuldeep Singh wrote:
> > Fix below dtc warning by making necessary addition of "spiclk" in
> > clock-names property.
> >
> > arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dt.yaml: spi@190000:
> > clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
> > From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
> > arch/arm64/boot/dts/broadcom/northstar2/ns2-svk.dt.yaml: spi@66190000:
> > clock-names:0: 'spiclk' is not one of ['SSPCLK', 'sspclk']
> > From schema: Documentation/devicetree/bindings/spi/spi-pl022.yaml
>
> As before, what makes the binding at fault rather than that DT? The PL022's
> actual input is named SSPCLK, not SPICLK, so why should a driver which wants
> to look up that clock by name expect to look for "spiclk"?
That's right. It's the DT which is at the fault of defining spiclk
instead of sspclk and need to be fixed in DT itself. I didn't take a
look at pl022 doc and acted on the basis of DT info.
Moreover, DT also uses sspclk and SSPCLK names interchangeably which are
anyway same. This also require updation to follow single convention.
Appreciate your comments and valuable inputs.
--
Best Regards
Kuldeep
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-02 19:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 12:43 [PATCH 0/3] DTC fixes for Arm pl022 bindings Kuldeep Singh
2022-02-28 12:43 ` [PATCH 1/3] dt-bindings: spi: Update clocks property for ARM pl022 Kuldeep Singh
2022-02-28 14:26 ` Robin Murphy
2022-02-28 15:11 ` Mark Brown
2022-02-28 15:27 ` Robin Murphy
2022-02-28 15:46 ` Mark Brown
2022-02-28 16:01 ` Robin Murphy
2022-02-28 12:43 ` [PATCH 2/3] dt-bindings: spi: Update clock-names " Kuldeep Singh
2022-02-28 12:43 ` [PATCH 3/3] dt-bindings: spi: Add spiclk to clock-names property in pl022 Kuldeep Singh
2022-02-28 14:36 ` Robin Murphy
2022-03-02 19:05 ` 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).