Hi

On Wed, Aug 31, 2022 at 3:31 PM <eugen.hris...@microchip.com> wrote:
>
> On 8/31/22 4:14 PM, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Mon, Aug 29, 2022 at 8:20 AM Balamanikandan Gunasundar
> > <balamanikandan.gunasun...@microchip.com> wrote:
> >>
> >> Enable the EBI and NAND flash controller. Define the pinctrl and
> >> partition table
> >>
> >> Signed-off-by: Balamanikandan Gunasundar 
> >> <balamanikandan.gunasun...@microchip.com>
> >> ---
> >>   arch/arm/dts/sam9x60ek.dts | 103 +++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 103 insertions(+)
> >>
> >> diff --git a/arch/arm/dts/sam9x60ek.dts b/arch/arm/dts/sam9x60ek.dts
> >> index 54c694bd78..6cb81dd90f 100644
> >> --- a/arch/arm/dts/sam9x60ek.dts
> >> +++ b/arch/arm/dts/sam9x60ek.dts
> >> @@ -80,6 +80,44 @@
> >>                          };
> >>
> >>                          pinctrl {
> >> +                                       nand {
> >
> > I can see two tabs here. You don't need it. The indentation go far on the 
> > right
> >
> >> +                                               pinctrl_nand_oe_we: 
> >> nand-oe-we-0 {
> >> +                                                       atmel,pins =
> >> +                                                               <AT91_PIOD 
> >> 0 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 1 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)>;
> >> +                                               };
> >> +
> >> +                                               pinctrl_nand_rb: nand-rb-0 
> >> {
> >> +                                                       atmel,pins =
> >> +                                                               <AT91_PIOD 
> >> 5 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
> >> +                                               };
> >> +
> >> +                                               pinctrl_nand_cs: nand-cs-0 
> >> {
> >> +                                                       atmel,pins =
> >> +                                                               <AT91_PIOD 
> >> 4 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP>;
> >> +                                               };
> >> +                                       };
> >> +
> >> +                                       ebi {
> >> +                                               pinctrl_ebi_data_0_7: 
> >> ebi-data-lsb-0 {
> >> +                                                       atmel,pins =
> >> +                                                               <AT91_PIOD 
> >> 6 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 7 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 8 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 9 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 10 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 11 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 12 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 13 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)>;
> >> +                                               };
> >> +
> >> +                                               pinctrl_ebi_addr_nand: 
> >> ebi-addr-0 {
> >> +                                                       atmel,pins =
> >> +                                                               <AT91_PIOD 
> >> 2 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)
> >> +                                                                AT91_PIOD 
> >> 3 AT91_PERIPH_A (AT91_PINCTRL_NONE | AT91_PINCTRL_SLEWRATE_DIS)>;
> >> +                                               };
> >> +                                       };
> >> +
> >
> > Please remove the ebi and nand block so you have one tab less. Then I
> > suggest to align to linux dts and refer to pinctrl instead to create
> > another level of indentation
>
> Hello Michael,
>
> In Linux I see the nand and ebi block. We have to keep the same DT as in
> Linux. Do you agree ?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/at91-sam9x60ek.dts#n378
>

I have seen it already but I don't think that dtsi is aligned with it.
So you don't have pinctrl: pinctrl@0ffff400 and you can refer
to this node as it is. Here the question is if we need to continue to
have a no-align dts component inside the pinctrl and add more
elements in it. If you look at that file pinctrl_qspi: qspi { is not
in any block and for the linux should be even before. It should
be coherent in what we have or in what linux have now. This does not
block driver review so I think that can be acked by microchip
maintainers in this part.

Michael


> Eugen
>
> >
> >>                                          pinctrl_qspi: qspi {
> >>                                                  atmel,pins =
> >>                                                          <AT91_PIOB 19 
> >> AT91_PERIPH_A AT91_PINCTRL_NONE
> > This part can be tab back
> >
> > Michael
> >
> >> @@ -106,6 +144,71 @@
> >>          };
> >>   };
> >>
> >> +&ebi {
> >> +       pinctrl-names = "default";
> >> +       pinctrl-0 = <&pinctrl_ebi_addr_nand &pinctrl_ebi_data_0_7>;
> >> +       status = "okay";
> >> +
> >> +       nand_controller: nand-controller {
> >> +               pinctrl-names = "default";
> >> +               pinctrl-0 = <&pinctrl_nand_oe_we &pinctrl_nand_cs 
> >> &pinctrl_nand_rb>;
> >> +               status = "okay";
> >> +
> >> +               nand@3 {
> >> +                       reg = <0x3 0x0 0x800000>;
> >> +                       rb-gpios = <&pioD 5 GPIO_ACTIVE_HIGH>;
> >> +                       cs-gpios = <&pioD 4 GPIO_ACTIVE_HIGH>;
> >> +                       nand-bus-width = <8>;
> >> +                       nand-ecc-mode = "hw";
> >> +                       nand-ecc-strength = <8>;
> >> +                       nand-ecc-step-size = <512>;
> >> +                       nand-on-flash-bbt;
> >> +                       label = "atmel_nand";
> >> +
> >> +                       partitions {
> >> +                               compatible = "fixed-partitions";
> >> +                               #address-cells = <1>;
> >> +                               #size-cells = <1>;
> >> +
> >> +                               at91bootstrap@0 {
> >> +                                       label = "at91bootstrap";
> >> +                                       reg = <0x0 0x40000>;
> >> +                               };
> >> +
> >> +                               uboot@40000 {
> >> +                                       label = "u-boot";
> >> +                                       reg = <0x40000 0xc0000>;
> >> +                               };
> >> +
> >> +                               ubootenvred@100000 {
> >> +                                       label = "U-Boot Env Redundant";
> >> +                                       reg = <0x100000 0x40000>;
> >> +                               };
> >> +
> >> +                               ubootenv@140000 {
> >> +                                       label = "U-Boot Env";
> >> +                                       reg = <0x140000 0x40000>;
> >> +                               };
> >> +
> >> +                               dtb@180000 {
> >> +                                       label = "device tree";
> >> +                                       reg = <0x180000 0x80000>;
> >> +                               };
> >> +
> >> +                               kernel@200000 {
> >> +                                       label = "kernel";
> >> +                                       reg = <0x200000 0x600000>;
> >> +                               };
> >> +
> >> +                               rootfs@800000 {
> >> +                                       label = "rootfs";
> >> +                                       reg = <0x800000 0x1f800000>;
> >> +                               };
> >> +                       };
> >> +               };
> >> +       };
> >> +};
> >> +
> >>   &macb0 {
> >>          phy-mode = "rmii";
> >>          status = "okay";
> >> --
> >> 2.34.1
> >>
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > mich...@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > i...@amarulasolutions.com
> > www.amarulasolutions.com
> >
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com

Reply via email to