linux-kernel - Re: [PATCH 3/4] dt-bindings: PCI: microchip,pcie-host: fix incorrect child node name

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Aug 2022 10:42:28 +0300
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Conor Dooley <mail@...chuod.ie>,
        Daire McNamara <daire.mcnamara@...rochip.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Greentime Hu <greentime.hu@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Conor Dooley <conor.dooley@...rochip.com>
Cc:     linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH 3/4] dt-bindings: PCI: microchip,pcie-host: fix incorrect
 child node name

On 11/08/2022 23:33, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.com>
> 
> v2022.08 of dt-schema improved checking of unevaluatedProperties, and
> exposed a previously unseen warning for the PCIe controller's interrupt
> controller node name:
> 
> arch/riscv/boot/dts/microchip/mpfs-icicle-kit.dtb: pcie@...0000000: Unevaluated properties are not allowed ('clock-names', 'clocks', 'legacy-interrupt-controller', 'microchip,axi-m-atr0' were unexpected)
>         From schema: Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
> 
> Make the property in the binding match the node name actually used in
> the dts.
> 
> Fixes: dcd49679fb3a ("dt-bindings: PCI: Fix 'unevaluatedProperties' warnings")
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> ---
> This is another one Rob where I feel like I'm doing the wrong thing.
> The Linux driver gets the child node without using the name, but
> another OS etc could in theory (or reality), right?

Yes and we had such cases when renaming device nodes caused regression.
My interpretation is that node name is not part of ABI, so anyone
depending on it made a mistake and they need to fix their stuff. I think
actually that is really poor coding and poor solution to parse device
node names and expect specific name.

Other folks interpretation is that we never break the users of kernel,
regardless what is documented in the ABI... so it depends. :)

Here however it is not a device node name, but a property name (although
still a node). Bindings require these to be specific, thus such name is
a part of ABI.

For your case, I wonder why it was called "legacy-interrupt-controller"
in the first place? Node names - also for properties - should be
generic, so generic name is just "interrupt-controller".

> ---
>  .../devicetree/bindings/pci/microchip,pcie-host.yaml          | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
> index 2a2166f09e2c..9b123bcd034c 100644
> --- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
> +++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml
> @@ -71,7 +71,7 @@ properties:
>    msi-parent:
>      description: MSI controller the device is capable of using.
>  
> -  interrupt-controller:
> +  legacy-interrupt-controller:


Best regards,
Krzysztof