Re: [PATCH v4 06/10] dt-bindings: mtd: update STM32 FMC2 NAND controller documentation
Hi Rob, On 5/14/20 7:55 PM, Rob Herring wrote: On Thu, May 14, 2020 at 11:35 AM Christophe Kerello 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 --- .../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.
Re: [PATCH v4 06/10] dt-bindings: mtd: update STM32 FMC2 NAND controller documentation
On Thu, May 14, 2020 at 11:35 AM Christophe Kerello 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 > >> --- > >> .../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
Re: [PATCH v4 06/10] dt-bindings: mtd: update STM32 FMC2 NAND controller documentation
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 --- .../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. 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? Regards, Christophe Kerello. - 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 +minItems: 6 +maxItems: 7 interrupts: maxItems: 1 @@ -61,7 +63,6 @@ required: - compatible - reg - interrupts - - clocks examples: - | @@ -77,13 +78,13 @@ examples: <0x8100 0x1000>, <0x8901 0x1000>, <0x8902 0x1000>; -interrupts = ; -dmas = <&mdma1 20 0x10 0x12000a02 0x0 0x0>, - <&mdma1 20 0x10 0x12000a08 0x0 0x0>, - <&mdma1 21 0x10 0x12000a0a 0x0 0x0>; -dma-names = "tx", "rx", "ecc"; -clocks = <&rcc FMC_K>; -resets = <&rcc FMC_R>; + interrupts = ; + dmas = <&mdma1 20 0x2 0x12000a02 0x0 0x0>, + <&mdma1 20 0x2 0x12000a08 0x0 0x0>, + <&mdma1 21 0x2 0x12000a0a 0x0 0x0>; + dma-names = "tx", "rx", "ecc"; + clocks = <&rcc FMC_K>; + resets = <&rcc FMC_R>; #address-cells = <1>; #size-cells = <0>; -- 1.9.1
Re: [PATCH v4 06/10] dt-bindings: mtd: update STM32 FMC2 NAND controller documentation
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 > --- > .../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. And where's your new compatible string for this different h/w? >- 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 > +minItems: 6 > +maxItems: 7 > >interrupts: > maxItems: 1 > @@ -61,7 +63,6 @@ required: >- compatible >- reg >- interrupts > - - clocks > > examples: >- | > @@ -77,13 +78,13 @@ examples: > <0x8100 0x1000>, > <0x8901 0x1000>, > <0x8902 0x1000>; > -interrupts = ; > -dmas = <&mdma1 20 0x10 0x12000a02 0x0 0x0>, > - <&mdma1 20 0x10 0x12000a08 0x0 0x0>, > - <&mdma1 21 0x10 0x12000a0a 0x0 0x0>; > -dma-names = "tx", "rx", "ecc"; > -clocks = <&rcc FMC_K>; > -resets = <&rcc FMC_R>; > + interrupts = ; > + dmas = <&mdma1 20 0x2 0x12000a02 0x0 0x0>, > + <&mdma1 20 0x2 0x12000a08 0x0 0x0>, > + <&mdma1 21 0x2 0x12000a0a 0x0 0x0>; > + dma-names = "tx", "rx", "ecc"; > + clocks = <&rcc FMC_K>; > + resets = <&rcc FMC_R>; >#address-cells = <1>; >#size-cells = <0>; > > -- > 1.9.1 >
