Message-ID: <30a33c28-1c80-8d2d-29d1-1e5aa5476af9@microchip.com>
Date:   Tue, 16 Aug 2022 21:32:01 +0000
From:   <Conor.Dooley@...rochip.com>
To:     <jrtc27@...c27.com>
CC:     <Daire.McNamara@...rochip.com>, <bhelgaas@...gle.com>,
        <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
        <paul.walmsley@...ive.com>, <greentime.hu@...ive.com>,
        <palmer@...belt.com>, <aou@...s.berkeley.edu>,
        <lpieralisi@...nel.org>, <Conor.Dooley@...rochip.com>,
        <linux-pci@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-riscv@...ts.infradead.org>
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 <mail@...chuod.ie> wrote:
>>
>> From: Conor Dooley <conor.dooley@...rochip.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@...000000: 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@...rochip.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@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

Powered by blists - more mailing lists