diff mbox series

dt-bindings: mtd: ti,gpmc-nand: Add missing 'rb-gpios'

Message ID 20211206174209.2297565-1-robh@kernel.org
State New
Headers show
Series dt-bindings: mtd: ti,gpmc-nand: Add missing 'rb-gpios' | expand

Commit Message

Rob Herring (Arm) Dec. 6, 2021, 5:42 p.m. UTC
With 'unevaluatedProperties' support implemented, the TI GPMC example
has a warning:

Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)

Add the missing definition for 'rb-gpios'.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Roger Quadros <rogerq@kernel.org>
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Thierry Reding Dec. 7, 2021, 8:36 a.m. UTC | #1
On Mon, Dec 06, 2021 at 11:42:09AM -0600, Rob Herring wrote:
> With 'unevaluatedProperties' support implemented, the TI GPMC example
> has a warning:
> 
> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
> 
> Add the missing definition for 'rb-gpios'.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Roger Quadros <rogerq@kernel.org>
> Cc: linux-mtd@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Thierry Reding <treding@nvidia.com>
Roger Quadros Dec. 7, 2021, 12:47 p.m. UTC | #2
On 06/12/2021 19:42, Rob Herring wrote:
> With 'unevaluatedProperties' support implemented, the TI GPMC example
> has a warning:
> 
> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
> 
> Add the missing definition for 'rb-gpios'.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Roger Quadros <rogerq@kernel.org>
> Cc: linux-mtd@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>

Reviewed-by: Roger Quadros <rogerq@kernel.org>

> ---
>  Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
>
Miquel Raynal Dec. 9, 2021, 9:42 a.m. UTC | #3
Hi Rob,

robh@kernel.org wrote on Mon,  6 Dec 2021 11:42:09 -0600:

> With 'unevaluatedProperties' support implemented, the TI GPMC example
> has a warning:
> 
> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
> 
> Add the missing definition for 'rb-gpios'.

rb-gpios is already defined in nand-controller.yaml. I seems like the
real problem is that this file does not refer to it. Can you update the
fix?

While at it you might also want to drop the rb-gpios property from
ingenic,nand.yaml, which also defines it a second time.

> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Roger Quadros <rogerq@kernel.org>
> Cc: linux-mtd@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> index beb26b9bcfb2..1c280f52baa0 100644
> --- a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> @@ -53,6 +53,11 @@ properties:
>      enum: [8, 16]
>      default: 8
>  
> +  rb-gpios:
> +    description:
> +      GPIO connection to R/B signal from NAND chip
> +    maxItems: 1
> +
>  patternProperties:
>    "@[0-9a-f]+$":
>      $ref: "/schemas/mtd/partitions/partition.yaml"

Thanks,
Miquèl
Roger Quadros Dec. 9, 2021, 10:08 a.m. UTC | #4
Hi Miquel,

On 09/12/2021 11:42, Miquel Raynal wrote:
> Hi Rob,
> 
> robh@kernel.org wrote on Mon,  6 Dec 2021 11:42:09 -0600:
> 
>> With 'unevaluatedProperties' support implemented, the TI GPMC example
>> has a warning:
>>
>> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
>>
>> Add the missing definition for 'rb-gpios'.
> 
> rb-gpios is already defined in nand-controller.yaml. I seems like the
> real problem is that this file does not refer to it. Can you update the
> fix?

I don't think we can refer to nand-controller.yaml right now as we are not
fully compatible with it yet. Please see examples below.

ti,gpmc-nand example:

      nand@0,0 {
        compatible = "ti,omap2-nand";
        reg = <0 0 4>;          /* device IO registers */
        interrupt-parent = <&gpmc>;
        interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
                     <1 IRQ_TYPE_NONE>; /* termcount */
        ti,nand-xfer-type = "prefetch-dma";
        ti,nand-ecc-opt = "bch16";
        ti,elm-id = <&elm>;
        #address-cells = <1>;
        #size-cells = <1>;

        /* NAND generic properties */
        nand-bus-width = <8>;
        rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */

        /* GPMC properties*/
        gpmc,device-width = <1>;

        partition@0 {
          label = "NAND.SPL";
          reg = <0x00000000 0x00040000>;
        };
        partition@1 {
          label = "NAND.SPL.backup1";
          reg = <0x00040000 0x00040000>;
        };
      };


nand-controller example:

    nand-controller {
      #address-cells = <1>;
      #size-cells = <0>;
      cs-gpios = <0>, <&gpioA 1>; /* A single native CS is available */

      /* controller specific properties */

      nand@0 {
        reg = <0>; /* Native CS */
        nand-use-soft-ecc-engine;
        nand-ecc-algo = "bch";

        /* controller specific properties */
      };

      nand@1 {
        reg = <1>; /* GPIO CS */
      };
    };


> 
> While at it you might also want to drop the rb-gpios property from
> ingenic,nand.yaml, which also defines it a second time.
> 
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Roger Quadros <rogerq@kernel.org>
>> Cc: linux-mtd@lists.infradead.org
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> index beb26b9bcfb2..1c280f52baa0 100644
>> --- a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>> @@ -53,6 +53,11 @@ properties:
>>      enum: [8, 16]
>>      default: 8
>>  
>> +  rb-gpios:
>> +    description:
>> +      GPIO connection to R/B signal from NAND chip
>> +    maxItems: 1
>> +
>>  patternProperties:
>>    "@[0-9a-f]+$":
>>      $ref: "/schemas/mtd/partitions/partition.yaml"
> 
> Thanks,
> Miquèl
> 

cheers,
-roger
Miquel Raynal Dec. 9, 2021, 10:40 a.m. UTC | #5
Hi Roger,

rogerq@kernel.org wrote on Thu, 9 Dec 2021 12:08:07 +0200:

> Hi Miquel,
> 
> On 09/12/2021 11:42, Miquel Raynal wrote:
> > Hi Rob,
> > 
> > robh@kernel.org wrote on Mon,  6 Dec 2021 11:42:09 -0600:
> >   
> >> With 'unevaluatedProperties' support implemented, the TI GPMC example
> >> has a warning:
> >>
> >> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
> >>
> >> Add the missing definition for 'rb-gpios'.  
> > 
> > rb-gpios is already defined in nand-controller.yaml. I seems like the
> > real problem is that this file does not refer to it. Can you update the
> > fix?  
> 
> I don't think we can refer to nand-controller.yaml right now as we are not
> fully compatible with it yet. Please see examples below.

This is a *very* wrong way of defining a NAND setup. I will take the
patch to silence the warning, but please convert this representation to
the 'new' one. I believe on the driver side it should not be too
complicated to support having a few of these properties moved to a NAND
chip subnode and still support the below binding. Just be very clear
that if the legacy bindings are used, only a single chip is supported.

> ti,gpmc-nand example:
> 
>       nand@0,0 {
>         compatible = "ti,omap2-nand";
>         reg = <0 0 4>;          /* device IO registers */
>         interrupt-parent = <&gpmc>;
>         interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
>                      <1 IRQ_TYPE_NONE>; /* termcount */
>         ti,nand-xfer-type = "prefetch-dma";
>         ti,nand-ecc-opt = "bch16";
>         ti,elm-id = <&elm>;
>         #address-cells = <1>;
>         #size-cells = <1>;
> 
>         /* NAND generic properties */
>         nand-bus-width = <8>;
>         rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */
> 
>         /* GPMC properties*/
>         gpmc,device-width = <1>;
> 
>         partition@0 {
>           label = "NAND.SPL";
>           reg = <0x00000000 0x00040000>;
>         };
>         partition@1 {
>           label = "NAND.SPL.backup1";
>           reg = <0x00040000 0x00040000>;
>         };
>       };
> 
> 
> nand-controller example:
> 
>     nand-controller {
>       #address-cells = <1>;
>       #size-cells = <0>;
>       cs-gpios = <0>, <&gpioA 1>; /* A single native CS is available */
> 
>       /* controller specific properties */
> 
>       nand@0 {
>         reg = <0>; /* Native CS */
>         nand-use-soft-ecc-engine;
>         nand-ecc-algo = "bch";
> 
>         /* controller specific properties */
>       };
> 
>       nand@1 {
>         reg = <1>; /* GPIO CS */
>       };
>     };
> 
> 
> > 
> > While at it you might also want to drop the rb-gpios property from
> > ingenic,nand.yaml, which also defines it a second time.
> >   
> >> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> >> Cc: Richard Weinberger <richard@nod.at>
> >> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> >> Cc: Tony Lindgren <tony@atomide.com>
> >> Cc: Roger Quadros <rogerq@kernel.org>
> >> Cc: linux-mtd@lists.infradead.org
> >> Signed-off-by: Rob Herring <robh@kernel.org>
> >> ---
> >>  Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >> index beb26b9bcfb2..1c280f52baa0 100644
> >> --- a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >> @@ -53,6 +53,11 @@ properties:
> >>      enum: [8, 16]
> >>      default: 8
> >>  
> >> +  rb-gpios:
> >> +    description:
> >> +      GPIO connection to R/B signal from NAND chip
> >> +    maxItems: 1
> >> +
> >>  patternProperties:
> >>    "@[0-9a-f]+$":
> >>      $ref: "/schemas/mtd/partitions/partition.yaml"  
> > 
> > Thanks,
> > Miquèl
> >   
> 
> cheers,
> -roger
> 

Thanks,
Miquèl
Roger Quadros Dec. 9, 2021, 12:11 p.m. UTC | #6
Miquel,

On 09/12/2021 12:40, Miquel Raynal wrote:
> Hi Roger,
> 
> rogerq@kernel.org wrote on Thu, 9 Dec 2021 12:08:07 +0200:
> 
>> Hi Miquel,
>>
>> On 09/12/2021 11:42, Miquel Raynal wrote:
>>> Hi Rob,
>>>
>>> robh@kernel.org wrote on Mon,  6 Dec 2021 11:42:09 -0600:
>>>   
>>>> With 'unevaluatedProperties' support implemented, the TI GPMC example
>>>> has a warning:
>>>>
>>>> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
>>>>
>>>> Add the missing definition for 'rb-gpios'.  
>>>
>>> rb-gpios is already defined in nand-controller.yaml. I seems like the
>>> real problem is that this file does not refer to it. Can you update the
>>> fix?  
>>
>> I don't think we can refer to nand-controller.yaml right now as we are not
>> fully compatible with it yet. Please see examples below.
> 
> This is a *very* wrong way of defining a NAND setup. I will take the
> patch to silence the warning, but please convert this representation to

Thanks!

> the 'new' one. I believe on the driver side it should not be too
> complicated to support having a few of these properties moved to a NAND
> chip subnode and still support the below binding. Just be very clear
> that if the legacy bindings are used, only a single chip is supported.

I agree with you. It has been that way since ages. I will look at cleaning
it up whenever possible for me. I think we will have to support the old
bindings as well as the new one going forward as many legacy platforms
are using it.

cheers,
-roger

> 
>> ti,gpmc-nand example:
>>
>>       nand@0,0 {
>>         compatible = "ti,omap2-nand";
>>         reg = <0 0 4>;          /* device IO registers */
>>         interrupt-parent = <&gpmc>;
>>         interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
>>                      <1 IRQ_TYPE_NONE>; /* termcount */
>>         ti,nand-xfer-type = "prefetch-dma";
>>         ti,nand-ecc-opt = "bch16";
>>         ti,elm-id = <&elm>;
>>         #address-cells = <1>;
>>         #size-cells = <1>;
>>
>>         /* NAND generic properties */
>>         nand-bus-width = <8>;
>>         rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>;  /* gpmc_wait0 */
>>
>>         /* GPMC properties*/
>>         gpmc,device-width = <1>;
>>
>>         partition@0 {
>>           label = "NAND.SPL";
>>           reg = <0x00000000 0x00040000>;
>>         };
>>         partition@1 {
>>           label = "NAND.SPL.backup1";
>>           reg = <0x00040000 0x00040000>;
>>         };
>>       };
>>
>>
>> nand-controller example:
>>
>>     nand-controller {
>>       #address-cells = <1>;
>>       #size-cells = <0>;
>>       cs-gpios = <0>, <&gpioA 1>; /* A single native CS is available */
>>
>>       /* controller specific properties */
>>
>>       nand@0 {
>>         reg = <0>; /* Native CS */
>>         nand-use-soft-ecc-engine;
>>         nand-ecc-algo = "bch";
>>
>>         /* controller specific properties */
>>       };
>>
>>       nand@1 {
>>         reg = <1>; /* GPIO CS */
>>       };
>>     };
>>
>>
>>>
>>> While at it you might also want to drop the rb-gpios property from
>>> ingenic,nand.yaml, which also defines it a second time.
>>>   
>>>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>>>> Cc: Richard Weinberger <richard@nod.at>
>>>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>>>> Cc: Tony Lindgren <tony@atomide.com>
>>>> Cc: Roger Quadros <rogerq@kernel.org>
>>>> Cc: linux-mtd@lists.infradead.org
>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>>>> index beb26b9bcfb2..1c280f52baa0 100644
>>>> --- a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>>>> +++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>>>> @@ -53,6 +53,11 @@ properties:
>>>>      enum: [8, 16]
>>>>      default: 8
>>>>  
>>>> +  rb-gpios:
>>>> +    description:
>>>> +      GPIO connection to R/B signal from NAND chip
>>>> +    maxItems: 1
>>>> +
>>>>  patternProperties:
>>>>    "@[0-9a-f]+$":
>>>>      $ref: "/schemas/mtd/partitions/partition.yaml"  
>>>
>>> Thanks,
>>> Miquèl
>>>   
>>
>> cheers,
>> -roger
>>
> 
> Thanks,
> Miquèl
>
Miquel Raynal Dec. 9, 2021, 12:37 p.m. UTC | #7
Hi Roger,

> >>>> With 'unevaluatedProperties' support implemented, the TI GPMC example
> >>>> has a warning:
> >>>>
> >>>> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
> >>>>
> >>>> Add the missing definition for 'rb-gpios'.    
> >>>
> >>> rb-gpios is already defined in nand-controller.yaml. I seems like the
> >>> real problem is that this file does not refer to it. Can you update the
> >>> fix?    
> >>
> >> I don't think we can refer to nand-controller.yaml right now as we are not
> >> fully compatible with it yet. Please see examples below.  
> > 
> > This is a *very* wrong way of defining a NAND setup. I will take the
> > patch to silence the warning, but please convert this representation to  
> 
> Thanks!
> 
> > the 'new' one. I believe on the driver side it should not be too
> > complicated to support having a few of these properties moved to a NAND
> > chip subnode and still support the below binding. Just be very clear
> > that if the legacy bindings are used, only a single chip is supported.  
> 
> I agree with you. It has been that way since ages. I will look at cleaning
> it up whenever possible for me. I think we will have to support the old
> bindings as well as the new one going forward as many legacy platforms
> are using it.

Yes of course, this is even a primary requirement.

Thanks,
Miquèl
Miquel Raynal Dec. 9, 2021, 4:50 p.m. UTC | #8
On Mon, 2021-12-06 at 17:42:09 UTC, Rob Herring wrote:
> With 'unevaluatedProperties' support implemented, the TI GPMC example
> has a warning:
> 
> Documentation/devicetree/bindings/memory-controllers/ti,gpmc.example.dt.yaml: nand@0,0: Unevaluated properties are not allowed ('rb-gpios' was unexpected)
> 
> Add the missing definition for 'rb-gpios'.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Roger Quadros <rogerq@kernel.org>
> Cc: linux-mtd@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> Reviewed-by: Roger Quadros <rogerq@kernel.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
index beb26b9bcfb2..1c280f52baa0 100644
--- a/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
@@ -53,6 +53,11 @@  properties:
     enum: [8, 16]
     default: 8
 
+  rb-gpios:
+    description:
+      GPIO connection to R/B signal from NAND chip
+    maxItems: 1
+
 patternProperties:
   "@[0-9a-f]+$":
     $ref: "/schemas/mtd/partitions/partition.yaml"