diff mbox series

[v2,1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci

Message ID 20240712142922.3292722-1-Frank.Li@nxp.com
State New
Headers show
Series [v2,1/1] dt-bindings: ata: ahci-fsl-qoriq: add fsl,ls1046a-ahci and fsl,ls1012a-ahci | expand

Commit Message

Frank Li July 12, 2024, 2:29 p.m. UTC
Add missing documented compatible strings 'fsl,ls1046a-ahci' and
'fsl,ls1012a-ahci'. Allow 'fsl,ls1012a-ahci' to fallback to
'fsl,ls1043a-ahci'.

Fix below CHECK_DTB warnings
arch/arm64/boot/dts/freescale/fsl-ls1012a-frwy.dtb: /soc/sata@3200000: failed to match any schema with compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci']

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v1 to v2
- rework commit message to show fix CHECK_DTB warning.
---
 .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Niklas Cassel July 12, 2024, 3:19 p.m. UTC | #1
On Fri, Jul 12, 2024 at 10:29:22AM -0400, Frank Li wrote:
> Add missing documented compatible strings 'fsl,ls1046a-ahci' and
> 'fsl,ls1012a-ahci'. Allow 'fsl,ls1012a-ahci' to fallback to
> 'fsl,ls1043a-ahci'.
> 
> Fix below CHECK_DTB warnings
> arch/arm64/boot/dts/freescale/fsl-ls1012a-frwy.dtb: /soc/sata@3200000: failed to match any schema with compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci']
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - rework commit message to show fix CHECK_DTB warning.
> ---
>  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> index 162b3bb5427ed..a244bc603549d 100644
> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> @@ -11,13 +11,18 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - fsl,ls1021a-ahci
> -      - fsl,ls1043a-ahci
> -      - fsl,ls1028a-ahci
> -      - fsl,ls1088a-ahci
> -      - fsl,ls2080a-ahci
> -      - fsl,lx2160a-ahci
> +    oneOf:
> +      - items:
> +          - const: fsl,ls1012a-ahci
> +          - const: fsl,ls1043a-ahci
> +      - enum:
> +          - fsl,ls1021a-ahci
> +          - fsl,ls1043a-ahci
> +          - fsl,ls1046a-ahci
> +          - fsl,ls1028a-ahci
> +          - fsl,ls1088a-ahci
> +          - fsl,ls2080a-ahci
> +          - fsl,lx2160a-ahci

I think that you should add the following Fixes-tag:
Fixes: e58e12c5c34c ("dt-bindings: ata: ahci-fsl-qoriq: convert to yaml format")

Considering that the commit that your are fixing is only in libata for-6.11,
and has thus never been in a released kernel version, perhaps the following
patch would be better (if it also solves the warnings):

diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
index 162b3bb5427e..8953b1847305 100644
--- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
+++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
@@ -12,8 +12,10 @@ maintainers:
 properties:
   compatible:
     enum:
+      - fsl,ls1012a-ahci
       - fsl,ls1021a-ahci
       - fsl,ls1043a-ahci
+      - fsl,ls1046a-ahci
       - fsl,ls1028a-ahci
       - fsl,ls1088a-ahci
       - fsl,ls2080a-ahci


This assumes that we can get the patch included before 6.11 final is released,
but considering that the merge window hasn't even opened yet, that should be
doable.


Thoughts from DT maintainers?



Kind regards,
Niklas
Frank Li July 12, 2024, 4:04 p.m. UTC | #2
On Fri, Jul 12, 2024 at 05:19:34PM +0200, Niklas Cassel wrote:
> On Fri, Jul 12, 2024 at 10:29:22AM -0400, Frank Li wrote:
> > Add missing documented compatible strings 'fsl,ls1046a-ahci' and
> > 'fsl,ls1012a-ahci'. Allow 'fsl,ls1012a-ahci' to fallback to
> > 'fsl,ls1043a-ahci'.
> > 
> > Fix below CHECK_DTB warnings
> > arch/arm64/boot/dts/freescale/fsl-ls1012a-frwy.dtb: /soc/sata@3200000: failed to match any schema with compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci']
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Change from v1 to v2
> > - rework commit message to show fix CHECK_DTB warning.
> > ---
> >  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > index 162b3bb5427ed..a244bc603549d 100644
> > --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > @@ -11,13 +11,18 @@ maintainers:
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - fsl,ls1021a-ahci
> > -      - fsl,ls1043a-ahci
> > -      - fsl,ls1028a-ahci
> > -      - fsl,ls1088a-ahci
> > -      - fsl,ls2080a-ahci
> > -      - fsl,lx2160a-ahci
> > +    oneOf:
> > +      - items:
> > +          - const: fsl,ls1012a-ahci
> > +          - const: fsl,ls1043a-ahci
> > +      - enum:
> > +          - fsl,ls1021a-ahci
> > +          - fsl,ls1043a-ahci
> > +          - fsl,ls1046a-ahci
> > +          - fsl,ls1028a-ahci
> > +          - fsl,ls1088a-ahci
> > +          - fsl,ls2080a-ahci
> > +          - fsl,lx2160a-ahci
> 
> I think that you should add the following Fixes-tag:
> Fixes: e58e12c5c34c ("dt-bindings: ata: ahci-fsl-qoriq: convert to yaml format")

I am not sure if need it because e58e12c5c34c still not release yet.
Needn't backport. You may squash into e58e12c5c34c if you like.

> 
> Considering that the commit that your are fixing is only in libata for-6.11,
> and has thus never been in a released kernel version, perhaps the following
> patch would be better (if it also solves the warnings):
> 
> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> index 162b3bb5427e..8953b1847305 100644
> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> @@ -12,8 +12,10 @@ maintainers:
>  properties:
>    compatible:
>      enum:
> +      - fsl,ls1012a-ahci
>        - fsl,ls1021a-ahci
>        - fsl,ls1043a-ahci
> +      - fsl,ls1046a-ahci
>        - fsl,ls1028a-ahci
>        - fsl,ls1088a-ahci
>        - fsl,ls2080a-ahci

driver have not support "fsl,ls1012a-ahci", which have to fall back to
"fsl,ls1043a-ahci". and DTS already use 
     compatible = "fsl,ls1012a-ahci", "fsl,ls1046a-ahci".

It can't fix 1012's warning. Only fix 1046's warning.
DT team also don't want to remove fsl,ls1012a-ahci in dts file.

> 
> 
> This assumes that we can get the patch included before 6.11 final is released,
> but considering that the merge window hasn't even opened yet, that should be
> doable.
> 
> 
> Thoughts from DT maintainers?

Supposed through ata tree.

> 
> 
> 
> Kind regards,
> Niklas
Rob Herring (Arm) July 12, 2024, 5:16 p.m. UTC | #3
On Fri, Jul 12, 2024 at 8:29 AM Frank Li <Frank.Li@nxp.com> wrote:
>
> Add missing documented compatible strings 'fsl,ls1046a-ahci' and
> 'fsl,ls1012a-ahci'. Allow 'fsl,ls1012a-ahci' to fallback to
> 'fsl,ls1043a-ahci'.
>
> Fix below CHECK_DTB warnings
> arch/arm64/boot/dts/freescale/fsl-ls1012a-frwy.dtb: /soc/sata@3200000: failed to match any schema with compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci']
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v1 to v2
> - rework commit message to show fix CHECK_DTB warning.
> ---
>  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> index 162b3bb5427ed..a244bc603549d 100644
> --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> @@ -11,13 +11,18 @@ maintainers:
>
>  properties:
>    compatible:
> -    enum:
> -      - fsl,ls1021a-ahci
> -      - fsl,ls1043a-ahci
> -      - fsl,ls1028a-ahci
> -      - fsl,ls1088a-ahci
> -      - fsl,ls2080a-ahci
> -      - fsl,lx2160a-ahci
> +    oneOf:
> +      - items:
> +          - const: fsl,ls1012a-ahci
> +          - const: fsl,ls1043a-ahci
> +      - enum:
> +          - fsl,ls1021a-ahci
> +          - fsl,ls1043a-ahci
> +          - fsl,ls1046a-ahci
> +          - fsl,ls1028a-ahci

Fix the alpha-numeric order while you're changing it.

> +          - fsl,ls1088a-ahci
> +          - fsl,ls2080a-ahci
> +          - fsl,lx2160a-ahci

Reviewed-by: Rob Herring <robh@kernel.org>
Niklas Cassel July 12, 2024, 5:16 p.m. UTC | #4
On Fri, Jul 12, 2024 at 12:04:02PM -0400, Frank Li wrote:
> On Fri, Jul 12, 2024 at 05:19:34PM +0200, Niklas Cassel wrote:
> > On Fri, Jul 12, 2024 at 10:29:22AM -0400, Frank Li wrote:
> > > Add missing documented compatible strings 'fsl,ls1046a-ahci' and
> > > 'fsl,ls1012a-ahci'. Allow 'fsl,ls1012a-ahci' to fallback to
> > > 'fsl,ls1043a-ahci'.
> > > 
> > > Fix below CHECK_DTB warnings
> > > arch/arm64/boot/dts/freescale/fsl-ls1012a-frwy.dtb: /soc/sata@3200000: failed to match any schema with compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci']
> > > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > Change from v1 to v2
> > > - rework commit message to show fix CHECK_DTB warning.
> > > ---
> > >  .../devicetree/bindings/ata/fsl,ahci.yaml     | 19 ++++++++++++-------
> > >  1 file changed, 12 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > > index 162b3bb5427ed..a244bc603549d 100644
> > > --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > > +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > > @@ -11,13 +11,18 @@ maintainers:
> > >  
> > >  properties:
> > >    compatible:
> > > -    enum:
> > > -      - fsl,ls1021a-ahci
> > > -      - fsl,ls1043a-ahci
> > > -      - fsl,ls1028a-ahci
> > > -      - fsl,ls1088a-ahci
> > > -      - fsl,ls2080a-ahci
> > > -      - fsl,lx2160a-ahci
> > > +    oneOf:
> > > +      - items:
> > > +          - const: fsl,ls1012a-ahci
> > > +          - const: fsl,ls1043a-ahci
> > > +      - enum:
> > > +          - fsl,ls1021a-ahci
> > > +          - fsl,ls1043a-ahci
> > > +          - fsl,ls1046a-ahci
> > > +          - fsl,ls1028a-ahci
> > > +          - fsl,ls1088a-ahci
> > > +          - fsl,ls2080a-ahci
> > > +          - fsl,lx2160a-ahci
> > 
> > I think that you should add the following Fixes-tag:
> > Fixes: e58e12c5c34c ("dt-bindings: ata: ahci-fsl-qoriq: convert to yaml format")
> 
> I am not sure if need it because e58e12c5c34c still not release yet.
> Needn't backport. You may squash into e58e12c5c34c if you like.

Fixes tags are used to indicate that the commit Fixes another commit that
introduced a bug. e58e12c5c34c introduced a bug that wasn't there before,
namely a CHECK_DTB warning.

Cc: stable@vger.kernel.org
is used to indicate that the patch should be backported.

In your case, I would use Fixes:, but without Cc: stable.

Anyway, I could add the Fixes tag if we get a R-b tag from a DT maintainer.


> 
> > 
> > Considering that the commit that your are fixing is only in libata for-6.11,
> > and has thus never been in a released kernel version, perhaps the following
> > patch would be better (if it also solves the warnings):
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > index 162b3bb5427e..8953b1847305 100644
> > --- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > +++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
> > @@ -12,8 +12,10 @@ maintainers:
> >  properties:
> >    compatible:
> >      enum:
> > +      - fsl,ls1012a-ahci
> >        - fsl,ls1021a-ahci
> >        - fsl,ls1043a-ahci
> > +      - fsl,ls1046a-ahci
> >        - fsl,ls1028a-ahci
> >        - fsl,ls1088a-ahci
> >        - fsl,ls2080a-ahci
> 
> driver have not support "fsl,ls1012a-ahci", which have to fall back to
> "fsl,ls1043a-ahci". and DTS already use 
>      compatible = "fsl,ls1012a-ahci", "fsl,ls1046a-ahci".
> 
> It can't fix 1012's warning. Only fix 1046's warning.
> DT team also don't want to remove fsl,ls1012a-ahci in dts file.

That is not fully true.

Before my proposed patch:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=y freescale/fsl-ls1012a-qds.dtb
arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dtb: sata@3200000: compatible:0: 'fsl,ls1012a-ahci' is not one of ['fsl,ls1021a-ahci', 'fsl,ls1043a-ahci', 'fsl,ls1028a-ahci', 'fsl,ls1088a-ahci', 'fsl,ls2080a-ahci', 'fsl,lx2160a-ahci']
        from schema $id: http://devicetree.org/schemas/ata/fsl,ahci.yaml#
arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dtb: sata@3200000: compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci'] is too long
        from schema $id: http://devicetree.org/schemas/ata/fsl,ahci.yaml#


After my proposed patch:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CHECK_DTBS=y freescale/fsl-ls1012a-qds.dtb
arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dtb: sata@3200000: compatible: ['fsl,ls1012a-ahci', 'fsl,ls1043a-ahci'] is too long
        from schema $id: http://devicetree.org/schemas/ata/fsl,ahci.yaml#


So it would only fix one of the two warnings for fsl-ls1012a platforms. I see.
Perhaps you should add this additional warning to your commit message.

So the patch has to look like your suggested patch.

You could possible add '- description: PCIe controller in ls1012a'
in front of:
+      - items:
+          - const: fsl,ls1012a-ahci
+          - const: fsl,ls1043a-ahci

Too make it even clearer that this ugly item-list is only needed for ls1012a,
see e.g.:
https://github.com/torvalds/linux/blob/v6.10-rc7/Documentation/devicetree/bindings/pci/ti%2Cj721e-pci-host.yaml#L18-L21


Kind regards,
Niklas
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
index 162b3bb5427ed..a244bc603549d 100644
--- a/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
+++ b/Documentation/devicetree/bindings/ata/fsl,ahci.yaml
@@ -11,13 +11,18 @@  maintainers:
 
 properties:
   compatible:
-    enum:
-      - fsl,ls1021a-ahci
-      - fsl,ls1043a-ahci
-      - fsl,ls1028a-ahci
-      - fsl,ls1088a-ahci
-      - fsl,ls2080a-ahci
-      - fsl,lx2160a-ahci
+    oneOf:
+      - items:
+          - const: fsl,ls1012a-ahci
+          - const: fsl,ls1043a-ahci
+      - enum:
+          - fsl,ls1021a-ahci
+          - fsl,ls1043a-ahci
+          - fsl,ls1046a-ahci
+          - fsl,ls1028a-ahci
+          - fsl,ls1088a-ahci
+          - fsl,ls2080a-ahci
+          - fsl,lx2160a-ahci
 
   reg:
     minItems: 1