Re: [PATCH v4 06/10] dt-bindings: mtd: update STM32 FMC2 NAND controller documentation

[Christophe Kerello]

Hi Rob,

On 5/14/20 7:55 PM, Rob Herring wrote:
> On Thu, May 14, 2020 at 11:35 AM Christophe Kerello
> <christophe.kerello@xxxxxx> wrote:
> > Hi Rob,
> >
> > On 5/14/20 5:00 PM, Rob Herring wrote:
> > > On Wed, May 06, 2020 at 11:11:15AM +0200, Christophe Kerello wrote:
> > > > These bindings can be used on SOCs where the FMC2 NAND controller is
> > > > in standalone. In case that the FMC2 embeds 2 controllers (an external
> > > > bus controller and a raw NAND controller), the register base and the
> > > > clock will be defined in the parent node. It is the reason why the
> > > > register base address and the clock are now optional.
> > > >
> > > > Signed-off-by: Christophe Kerello <christophe.kerello@xxxxxx>
> > > > ---
> > > >    .../devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml   | 19 ++++++++++---------
> > > >    1 file changed, 10 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml b/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml
> > > > index b059267..68fac1a 100644
> > > > --- a/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml
> > > > +++ b/Documentation/devicetree/bindings/mtd/st,stm32-fmc2-nand.yaml
> > > > @@ -18,13 +18,15 @@ properties:
> > > >
> > > >      reg:
> > > >        items:
> > > > -      - description: Registers
> > > > +      - description: Registers (optional)
> > >
> > > The only thing that can be optional are the last entries. You have to do
> > > a 'oneOf' with 6 entries and 7 entries.
> >
> > Ok, so the way to describe the reg property in my case should be:
> >          reg:
> >            oneOf:
> >              - description: FMC2 embeds the NFC controller in standalone.
> >                items:
> >                  - description: Registers
> >                  - description: Chip select 0 data
> >                  - description: Chip select 0 command
> >                  - description: Chip select 0 address space
> >                  - description: Chip select 1 data
> >                  - description: Chip select 1 command
> >                  - description: Chip select 1 address space
> >
> >              - description: FMC2 embeds the NFC controller and the EBI
> >                  controller.
> >                items:
> >                  - description: Chip select 0 data
> >                  - description: Chip select 0 command
> >                  - description: Chip select 0 address space
> >                  - description: Chip select 1 data
> >                  - description: Chip select 1 command
> >                  - description: Chip select 1 address space
> >
> > > And where's your new compatible string for this different h/w?
> >
> >   From NFC controller point of view, it is the same HW.
>
> That's what everyone says until they have some quirk or integration
> difference to handle.
>
> > In the case that we have 2 controllers embedded, the register base is
> > shared.
> > The NFC driver will check at probe time the compatible string of its
> > parent node.
> > In case that it is "st,stm32mp1-fmc2-ebi", then the driver will find the
> > register base in the parent node (EBI node), otherwise it will find it
> > in the NFC node.
> > Is it better to have 2 compatible strings (one for each reg description)
> > than checking the parent's compatible string and have only one
> > compatible string?
>
> Why not just put the register base into the child node too? While
> overlapping 'reg' regions for siblings is bad, it's fine for child
> nodes. I guess since there are chip selects for the child nodes that
> may not work here.
>
> It doesn't hurt to have another compatible. You can always make the
> old one a fallback. With different compatibles you can make sure reg
> has the right number of entries.
>
> Rob

I will add a new compatible string to handle the reg property.
It will be part of v5.

Regards,
Christophe Kerello.