| Message ID | 20220816182547.3454843-2-mail@conchuod.ie (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix RISC-V/PCI dt-schema issues with dt-schema v2022.08 | expand |
On 16 Aug 2022, at 19:25, Conor Dooley <mail@conchuod.ie> wrote: > > From: Conor Dooley <conor.dooley@microchip.com> > > 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 <conor.dooley@microchip.com> > --- > 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 > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 16/08/2022 22:05, Jessica Clarke wrote: > On 16 Aug 2022, at 19:25, Conor Dooley <mail@conchuod.ie> wrote: >> >> From: Conor Dooley <conor.dooley@microchip.com> >> >> 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 <conor.dooley@microchip.com> >> --- >> 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 >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv >
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