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

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 <
> [email protected]> 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 <[email protected]>
> > > 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 <[email protected]>
> > >     Signed-off-by: Sascha Hauer <[email protected]>
> > >
> > > 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.

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 |