Re: [PATCH v3 5/8] dt-bindings: mtd: ti,gpmc-nand: Convert to yaml

[Rob Herring]

On Tue, Sep 7, 2021 at 11:57 AM Roger Quadros <rogerq@xxxxxxxxxx> wrote:
>
> Hi Miquel,
>
> On 07/09/2021 19:35, Miquel Raynal wrote:
> > Hi Grygorii,
> >
> >>>
> >>>> +
> >>>> +  nand-bus-width:
> >>>> +    description:
> >>>> +      Bus width to the NAND chip
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>> +    enum: [8, 16]
> >>>> +    default: 8
> >>>
> >>> This is part of nand-controller.yaml binding and should not be there.
> >>>
> >>>> +
> >>>> +allOf:
> >>>> +  - $ref: "../memory-controllers/ti,gpmc-child.yaml"
> >>>
> >>> Maybe you need to reference the nand controller bindings as well
> >>>
> >>
> >> This will not work out of the box :( as nand-controller.yaml defines both
> >>   nand controller and nand memory. It potentially might work if it will be possible to split
> >> nand memory definition (or nand memory properties) out of and-controller.yaml, similarly to
> >> ti,gpmc-child.yaml from this series.
> >
> > What you think would be the issue?
>
> The issue is that dt_binding checks will fail if I reference nand-controller.yaml
> as we currently represent the controller as follows
>
> memory-controller { /* GPMC controller */
>         memory-controller-props;
>         nand-chip {
>                 /* @chip select 0 */
>                 nand-controller-props;
>                 memory-controller-timing-props;
>                 chip-props;
>         }
>         nand-chip {
>                 /* @chip select 1 */
>                 nand-controller-props;
>                 memory-controller-timing-props;
>                 chip-props;
>         }
>         nor-chip {
>                 /* @chip select 2 */
>                 memory-controller-timing-props;
>                 chip-props;
>         }
> }
>
> The NAND controller IO registers are at different addresses for different
> chip select regions. Also, this is one way we can specify GPMC settings/timings
> for different chip selects.
>
> >
> > I am not opposed to split nand-controller.yaml into
> > nand-controller.yaml and nand-chip.yaml if it simplifies the
> > description of controllers but I don't get why it would be needed. In
> > particular since we expect all drivers to support the
> >
> > nand-controller {
> >       controller-props;
> >       nand-chip {
> >               chip-props;
> >       }
> > }
>
> Changing to this format will cause a lot of churn in DT files, which I'm not sure
> if it gives enough benefit.
> TI platforms will never have 2 NAND chips in the same chip select region.

Probably best to just leave this alone. Unless this is getting used in
new chips? If so, I'd say it's a separate change.

> > organization which has been enforced since at least 2018. Having a
> > controller vs. chip representation is fundamentally right. But here I
> > see how "legacy" are these bindings with so much unneeded specific "ti,"
> > properties... On one side it would be good to verify that the driver
> > supports this representation (which I believe is true) and on the other
> > side maybe it's time to advertise "better" bindings as well.
>
> Yes, I'm OK to mark ti specific properties deprecated and use standard NAND chip
> bindings.

I don't think it's really worth it to go half way using common
properties but not the common structure.

Rob