2022-08-16 18:31:49

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 1/6] dt-bindings: PCI: fu740-pci: fix missing clock-names

From: Conor Dooley <[email protected]>

The commit b92225b034c0 ("dt-bindings: PCI: designware: Fix
'unevaluatedProperties' warnings") removed the clock-names property as
a requirement and from the example as it triggered unevaluatedProperty
warnings. dtbs_check was not able to pick up on this at the time, but
now can:

arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dtb: pcie@e00000000: Unevaluated properties are not allowed ('clock-names' was unexpected)
From schema: linux/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml

The property was already in use by the FU740 DTS and the clock must be
enabled. The Linux driver does not use this property, but outside of
the kernel this property may have users. Re-add the property and its
"clocks" dependency.

Fixes: b92225b034c0 ("dt-bindings: PCI: designware: Fix 'unevaluatedProperties' warnings")
Fixes: 43cea116be0b ("dt-bindings: PCI: Add SiFive FU740 PCIe host controller")
Signed-off-by: Conor Dooley <[email protected]>
---
v2022.08 of dt-schema is required.
---
.../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
index 195e6afeb169..c7a9a2dc0fa6 100644
--- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
@@ -51,6 +51,12 @@ properties:
description: A phandle to the PCIe power up reset line.
maxItems: 1

+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: pcie_aux
+
pwren-gpios:
description: Should specify the GPIO for controlling the PCI bus device power on.
maxItems: 1
--
2.37.1


2022-08-16 21:40:45

by Jessica Clarke

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] dt-bindings: PCI: fu740-pci: fix missing clock-names

On 16 Aug 2022, at 19:25, Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> The commit b92225b034c0 ("dt-bindings: PCI: designware: Fix
> 'unevaluatedProperties' warnings") removed the clock-names property as
> a requirement and from the example as it triggered unevaluatedProperty
> warnings. dtbs_check was not able to pick up on this at the time, but
> now can:
>
> arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dtb: pcie@e00000000: Unevaluated properties are not allowed ('clock-names' was unexpected)
> From schema: linux/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
>
> The property was already in use by the FU740 DTS and the clock must be
> enabled. The Linux driver does not use this property, but outside of
> the kernel this property may have users. Re-add the property and its
> "clocks" dependency.

Are you sure about this? I see a devm_clk_get("pcie_aux") that surely
won't without the property. FreeBSD’s similarly relies on the name,
though it also has a fallback to the U-Boot pcieaux name (because the
world is terrible and people can’t even agree on that) so it works
with the U-Boot-provided FDT (it would be nice if Linux had this as a
goal, and people worked with U-Boot devs to get everything needed for
newly-exposed devices merged back there so I don’t have to be the one
to notice and do it...).

Jess

> Fixes: b92225b034c0 ("dt-bindings: PCI: designware: Fix 'unevaluatedProperties' warnings")
> Fixes: 43cea116be0b ("dt-bindings: PCI: Add SiFive FU740 PCIe host controller")
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> v2022.08 of dt-schema is required.
> ---
> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> index 195e6afeb169..c7a9a2dc0fa6 100644
> --- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
> @@ -51,6 +51,12 @@ properties:
> description: A phandle to the PCIe power up reset line.
> maxItems: 1
>
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: pcie_aux
> +
> pwren-gpios:
> description: Should specify the GPIO for controlling the PCI bus device power on.
> maxItems: 1
> --
> 2.37.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-08-16 22:12:47

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] dt-bindings: PCI: fu740-pci: fix missing clock-names

On 16/08/2022 22:05, Jessica Clarke wrote:
> On 16 Aug 2022, at 19:25, Conor Dooley <[email protected]> wrote:
>>
>> From: Conor Dooley <[email protected]>
>>
>> The commit b92225b034c0 ("dt-bindings: PCI: designware: Fix
>> 'unevaluatedProperties' warnings") removed the clock-names property as
>> a requirement and from the example as it triggered unevaluatedProperty
>> warnings. dtbs_check was not able to pick up on this at the time, but
>> now can:
>>
>> arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dtb: pcie@e00000000: Unevaluated properties are not allowed ('clock-names' was unexpected)
>> From schema: linux/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
>>
>> The property was already in use by the FU740 DTS and the clock must be
>> enabled. The Linux driver does not use this property, but outside of
>> the kernel this property may have users. Re-add the property and its
>> "clocks" dependency.
>
> Are you sure about this? I see a devm_clk_get("pcie_aux") that surely
> won't without the property. FreeBSD’s similarly relies on the name,

I'm having a bit of a howler this week. I read that line of code and
for some reason came to the conclusion that it didn't match the one
in the dt. I even did it more than once given I re-wrote this commit
message. At least you're paying attention & my change is incomplete
rather than broken...

Since there's reliance on the property, it needs to become required.

> though it also has a fallback to the U-Boot pcieaux name (because the
> world is terrible and people can’t even agree on that) so it works
> with the U-Boot-provided FDT (it would be nice if Linux had this as a
> goal, and people worked with U-Boot devs to get everything needed for
> newly-exposed devices merged back there so I don’t have to be the one
> to notice and do it...).

For polarfire we are a little out of sync & plan on fixing that soonTM.

>
> Jess
>
>> Fixes: b92225b034c0 ("dt-bindings: PCI: designware: Fix 'unevaluatedProperties' warnings")
>> Fixes: 43cea116be0b ("dt-bindings: PCI: Add SiFive FU740 PCIe host controller")
>> Signed-off-by: Conor Dooley <[email protected]>
>> ---
>> v2022.08 of dt-schema is required.
>> ---
>> .../devicetree/bindings/pci/sifive,fu740-pcie.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
>> index 195e6afeb169..c7a9a2dc0fa6 100644
>> --- a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
>> @@ -51,6 +51,12 @@ properties:
>> description: A phandle to the PCIe power up reset line.
>> maxItems: 1
>>
>> + clocks:
>> + maxItems: 1
>> +
>> + clock-names:
>> + const: pcie_aux
>> +
>> pwren-gpios:
>> description: Should specify the GPIO for controlling the PCI bus device power on.
>> maxItems: 1
>> --
>> 2.37.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>