Re: [PATCH v4 01/14] ARM: mvebu: add Netgear RN102 support

[Luca Lauro <famlauro93l@gmail.com>]

Il giorno ven 21 ago 2026 alle ore 11:17 Sascha Hauer
<s.hauer@pengutronix.de> ha scritto:
>
> On 2026-08-20 16:27, Luca Lauro wrote:
> > Hi Sascha,
> >
> > about the NAND node: the upstream DTS does contain the NAND configuration
> > properties, but they are placed inside the `nand@0` child node. Barebox
> > pxa3xx-nand reads these properties from the controller node instead, so
> > the upstream layout is not sufficient for barebox to probe and configure
> > the NAND controller correctly.
> >
> > For this reason the overlay needs to replicate NAND configuration
> > properties in the controller node. Without them, barebox does not apply
> > settings and NAND doesn't work.
> >
> > The only part that is truly duplicated is `status = "okay"`, which I can
> > drop in v5.
> >
> > Thanks for the review.
> >
> > Il giorno lun 17 ago 2026 alle ore 09:51 Sascha Hauer <
> > s.hauer@pengutronix.de> ha scritto:
> >
> > > On 2026-08-13 17:26, Luca Lauro via B4 Relay wrote:
> > > > +                                      filetype_kwbimage_v1);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static const struct of_device_id rn102_of_match[] = {
> > > > +     { .compatible = "netgear,rn102" },
> > >
> > > How is the driver probed? The string "netgear,rn102" is in no dts.
> > > Unless I am missing something this should be "netgear,readynas-102".
> > >
> > > Same for the rn104 patch.
> > >
> > > > +/*
> > > > + * NOTE:
> > > > + * armada_370_xp_barebox_entry() cannot be used here because the
> > > > + * upstream SDRAM size detection for Armada 370-XP misinterprets
> > > > + * the DDR_SIZE_CSn registers on this board and reports an incorrect
> > > > + * memory size (256MB instead of 512MB on RN102).
> > > > + *
> > > > + * Until the generic detection code is fixed, we compute the SDRAM
> > > > + * size manually using the DDR_SIZE_CSn values.
> > > > + */
> > > > +static unsigned long armada_370_xp_memory_find(void)
> > > > +{
> > > > +     unsigned long mem_size = 0;
> > > > +
> > > > +     for (int cs = 0; cs < 4; cs++) {
> > > > +             u32 ctrl = readl(ARMADA_370_XP_SDRAM_BASE +
> > > DDR_SIZE_CSn(cs));
> > > > +
> > > > +             /* Skip non-enabled CS */
> > > > +             if ((ctrl & DDR_SIZE_ENABLED) != DDR_SIZE_ENABLED)
> > > > +                     continue;
> > > > +
> > > > +             mem_size += (ctrl | ~DDR_SIZE_MASK) + 1;
> > > > +     }
> > > > +
> > > > +     return mem_size;
> > > > +}
> > >
> > > The only difference I can spot here between this function and the
> > > existing variant in arch/arm/mach-mvebu/common.c is:
> > >
> > > #define  DDR_SIZE_MASK                       0xff000000
> > >
> > > whereas the common.c variant uses:
> > >
> > > #define ARMADA_370_XP_DDR_SIZE_MASK     0xffff0000
> > >
> > > The latter goes down to this:
> > >
> > > > commit 7351b6b5c59c7a280787998006f39a5cd3a2f18b
> > > > Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > Date:   Tue Jun 13 00:37:49 2017 +0200
> > > >
> > > >     ARM: mvebu: fix size mask for RAM window
> > > >
> > > >     The size field in the window control register occupies bits 31:16. So
> > > >     adapt ARMADA_370_XP_DDR_SIZE_MASK accordingly. This fixes detection
> > > of
> > > >     RAM chips smaller than 32 MiB and so probably doesn't affect any
> > > >     supported machine.
> > > >
> > > >     Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > >     Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > >
> > > > diff --git a/arch/arm/mach-mvebu/common.c b/arch/arm/mach-mvebu/common.c
> > > > index 06bfb72615..fa971da11e 100644
> > > > --- a/arch/arm/mach-mvebu/common.c
> > > > +++ b/arch/arm/mach-mvebu/common.c
> > > > @@ -47,7 +47,7 @@
> > > >  #define ARMADA_370_XP_SDRAM_BASE       (IOMEM(MVEBU_REMAP_INT_REG_BASE)
> > > + 0x20000)
> > > >  #define ARMADA_370_XP_DDR_SIZE_CSn(n)  (0x184 + ((n) * 0x8))
> > > >  #define ARMADA_370_XP_DDR_SIZE_ENABLED BIT(0)
> > > > -#define ARMADA_370_XP_DDR_SIZE_MASK    0xff000000
> > > > +#define ARMADA_370_XP_DDR_SIZE_MASK    0xffff0000
> > > >
> > > >  /*
> > > >   * Marvell MVEBU SoC id and revision can be read from any PCIe
> > >
> > > @Uwe, Where did you get that information from. Could it be that we
> > > should just revert this one given that it seems to be untested on your
> > > side?
> > >
> > > > +
> > > > +&nand_controller {
> > > > +     compatible = "marvell,armada370-nand", "marvell,pxa3xx-nand";
> > > > +     status = "okay";
> > >
> > > These two properties are already in the upstream dts files, please drop.
>
> Could you retry on current -next? It contains
>
> fac937411f mtd: nand: nand_mrvl_nfc: support the nand-controller bindings
>
> which seems to fix that issue. At least that fixed the binding on my
> pxa3xx board.

I tested the RN102 on current barebox-next, including commit
fac937411f (“mtd: nand: nand_mrvl_nfc: support the nand-controller
bindings”).

Results:

- The NAND controller on RN102 is now probed correctly without any
  workaround in the controller node. ECC strength and step size are
  taken from the upstream DTS, and the BBT is detected properly.

- The barebox DT overlay can now limit itself to redefining the
  partition layout under nand@0. The duplicated NAND configuration
  properties in the controller node are no longer required.

- Environment and barebox-state backends work correctly with the
  updated overlay.

So the upstream nand-controller binding support fixes the issue on
RN102 as well. I will drop the duplicated NAND properties in v5 and
keep only the partition definitions.

Thanks,
Luca

>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>