linux-kernel - Re: [PATCH v5 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu" for i.MX8M PCIe Endpoint

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]

Message-ID: <ZwaagtTx1ar1CW4V@ryzen.lan>
Date: Wed, 9 Oct 2024 17:00:18 +0200
From: Niklas Cassel <cassel@...nel.org>
To: Richard Zhu <hongxing.zhu@....com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	shawnguo@...nel.org, l.stach@...gutronix.de,
	devicetree@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kernel@...gutronix.de, imx@...ts.linux.dev
Subject: Re: [PATCH v5 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and
 "atu" for i.MX8M PCIe Endpoint

On Tue, Aug 13, 2024 at 03:42:20PM +0800, Richard Zhu wrote:
> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> 
> For i.MX8M PCIe EP, the dbi2 and atu addresses are pre-defined in the
> driver. This method is not good.
> 
> In commit b7d67c6130ee ("PCI: imx6: Add iMX95 Endpoint (EP) support"),
> Frank suggests to fetch the dbi2 and atu from DT directly. This commit is
> preparation to do that for i.MX8M PCIe EP.
> 
> These changes wouldn't break driver function. When "dbi2" and "atu"
> properties are present, i.MX PCIe driver would fetch the according base
> addresses from DT directly. If only two reg properties are provided, i.MX
> PCIe driver would fall back to the old method.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> Reviewed-by: Frank Li <Frank.Li@....com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml  | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> index a06f75df8458..84ca12e8b25b 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> @@ -65,12 +65,14 @@ allOf:
>      then:
>        properties:
>          reg:
> -          minItems: 2
> -          maxItems: 2
> +          minItems: 4
> +          maxItems: 4

Now it seems like this patch has already been picked up,
but how is this not breaking DT backwards compatibility?

You are here increasing minItems, which means that an older DT
should now fail to validate using the new schema?

I thought that it was only acceptable to add new optional properties
after the DT binding has been accepted.

What am I missing?


If the specific compatible isn't used by any DTS in a released kernel,
then I think that the commit log should have clearly stated so,
and explained that that is the reason why it is okay to break DT backwards
compatibility.


Kind regards,
Niklas

>          reg-names:
>            items:
>              - const: dbi
>              - const: addr_space
> +            - const: dbi2
> +            - const: atu
>  
>    - if:
>        properties:
> @@ -129,8 +131,11 @@ examples:
>  
>      pcie_ep: pcie-ep@...00000 {
>        compatible = "fsl,imx8mp-pcie-ep";
> -      reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
> -      reg-names = "dbi", "addr_space";
> +      reg = <0x33800000 0x100000>,
> +            <0x18000000 0x8000000>,
> +            <0x33900000 0x100000>,
> +            <0x33b00000 0x100000>;
> +      reg-names = "dbi", "addr_space", "dbi2", "atu";
>        clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
>                 <&clk IMX8MP_CLK_HSIO_AXI>,
>                 <&clk IMX8MP_CLK_PCIE_ROOT>;
> -- 
> 2.37.1
>