Re: [PATCH] dt-bindings: mtd: ti,gpmc-nand: Add missing 'rb-gpios'

Roger Quadros
2021-12-09T12:13:08Z

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