Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <721ccb76-c162-30ee-68cc-3316a2d62554@linaro.org>
Date:   Fri, 12 Aug 2022 10:42:28 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH 3/4] dt-bindings: PCI: microchip,pcie-host: fix incorrect
 child node name
Content-Language: en-US
To:     Conor Dooley <mail@conchuod.ie>,
        Daire McNamara <daire.mcnamara@microchip.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Greentime Hu <greentime.hu@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Conor Dooley <conor.dooley@microchip.com>
Cc:     linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org
References: <20220811203306.179744-1-mail@conchuod.ie>
 <20220811203306.179744-4-mail@conchuod.ie>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20220811203306.179744-4-mail@conchuod.ie>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/08/2022 23:33, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.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@2000000000: 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@microchip.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