devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Roger Quadros <rogerq@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tony Lindgren <tony@atomide.com>,
	devicetree@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: mtd: ti,gpmc-nand: Add missing 'rb-gpios'
Date: Thu, 9 Dec 2021 11:40:38 +0100	[thread overview]
Message-ID: <20211209114038.710139a8@xps13> (raw)
In-Reply-To: <bebef734-d0d3-e78e-e07a-9160ead1f673@kernel.org>

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

  reply	other threads:[~2021-12-09 10:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-06 17:42 [PATCH] dt-bindings: mtd: ti,gpmc-nand: Add missing 'rb-gpios' Rob Herring
2021-12-07  8:36 ` Thierry Reding
2021-12-07 12:47 ` Roger Quadros
2021-12-09  9:42 ` Miquel Raynal
2021-12-09 10:08   ` Roger Quadros
2021-12-09 10:40     ` Miquel Raynal [this message]
2021-12-09 12:11       ` Roger Quadros
2021-12-09 12:37         ` Miquel Raynal
2021-12-09 16:50 ` Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211209114038.710139a8@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    --cc=robh@kernel.org \
    --cc=rogerq@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=tony@atomide.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).