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

[Luca Lauro <famlauro93l@gmail.com>]

Il giorno ven 21 ago 2026 alle ore 12:07 Sascha Hauer
<s.hauer@pengutronix.de> ha scritto:
>
> Hi Luca,
>
> On 2026-08-20 19:59, Luca Lauro wrote:
> > Il giorno mer 19 ago 2026 alle ore 09:16 Sascha Hauer
> > <s.hauer@pengutronix.de> ha scritto:
> > >
> > > Hi Uwe,
> > >
> > > On 2026-08-18 10:44, Uwe Kleine-König wrote:
> > > > >
> > > > > 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
> > > >
> > > > Apart from the different value, the latter name is the better one B-)
> > > >
> > > > > 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?
> > > >
> > > > 0xffff0000 is in line with
> > > > https://datasheet.datasheetarchive.com/originals/crawler/marvell.com/002fa441a27967d992f905776d519926.pdf
> > > > (page 630). So I'd expect that 0xffff0000 is correct, but I don't care
> > > > much.
> > >
> > > Page 630 describes the register at offset 0x20000. I think the correct
> > > page to look at is 626 which describes 0x20184 aka ARMADA_370_XP_DDR_SIZE_CSn(0).
> > >
> > > That one has the window size in the upper 8 bit. Looks like we should
> > > just revert 7351b6b5c59c. @Luca, does the SDRAM size detection work for
> > > you with that patch reverted?
> >
> > Hi Sascha,
> >
> > I tested with kwboot on my RN102...
> > SDRAM detection:
> > With the generic lowlevel path (armada_370_xp_barebox_entry) the board
> > does not reach the banner at all; independently from
> > ARMADA_370_XP_DDR_SIZE_MASK value. With the previous
> > workaround-based lowlevel code restored, the board boots reliably and
> > reports the correct 512 MiB of DRAM. So at the moment I cannot verify
> > the generic SDRAM detection on RN102 because the board does not boot
> > with that path.
>
> I believe ARMADA_370_XP_DDR_SIZE_MASK 0xff000000 is correct and we
> should revert Uwes patch. This however doesn't seem to be your problem.
>
> Could you experiment with the placing of mvebu_remap_registers()? You
> may call this only once. First of all, when using
> armada_370_xp_barebox_entry(), have you removed the call from
> mvebu_remap_registers() from your board code? Otherwise it won't work.
>
> Then, could you remove the call to mvebu_remap_registers() from
> armada_370_xp_barebox_entry() and do it in your board code instead?
>
> There's something going on and the memory detection itself doesn't seem
> to make the difference.
>

Thanks for the detailed hints. I can confirm that ARMADA_370_XP_DDR_SIZE_MASK
= 0xff000000 matches the register layout on RN102/RN104, and I agree that
reverting 7351b6b5c59c is the correct direction.

Regarding the generic lowlevel path: yes, when testing
armada_370_xp_barebox_entry() I removed the mvebu_remap_registers() call
from my board code, so the function was only invoked once.

I will experiment with the placement of mvebu_remap_registers() as you
suggested: first by removing the call inside armada_370_xp_barebox_entry()
and invoking it only from the board code, and then by testing the opposite
placement. As you noted, the SDRAM mask itself does not seem to be the
root cause, so I will focus on the remapping sequence and its timing.

I will report back with results.

> >
> > GPIO / LED blink:
> > Even with deep-probe enabled and with explicit
> > of_device_ensure_probed_by_alias("gpio0"), "gpio1" and "gpio2", the
> > GPIO blink logic cannot access the controller registers:
> >
> >     WARNING: gpio0 regmap not available
> >
> > The mvebu GPIO controller is not a syscon device, so
> > syscon_node_to_regmap() always returns an error. As a result, the blink
> > logic cannot use regmap to access the GPIO registers, and the LEDs are
> > not initialized in time during early disk power-on.
> >
> > The previous implementation used direct MMIO access to the GPIO
> > registers, which worked correctly on RN102/RN104, but was removed during
> > the cleanup. Without either restoring that path or providing a regmap
> > backend for the mvebu GPIO controller, the LED blink feature cannot
> > function.
> >
> > Let me know which direction you prefer. I can restore the direct MMIO
> > access for the blink logic if that is acceptable.
>
> When I first looked at it I didn't realize that the whole GPIO code in the
> board code was to enable a hardware blinking feature that is not
> available through the GPIO API.
>
> Linux provides an extra PWM driver for this purpose and doing the same
> would be my preferred solution. You might consider this overkill and not
> worth the additional work, so I won't insist on it. My second best
> solution would be some marvell_orion_gpio_blink() function that takes
> the Controller's base address and some other parameters to let the GPIO
> blink. That would allow us to share it between boards.

Thanks for the clarification, that helps a lot.

I agree with your assessment: the hardware blink engine on the mvebu GPIO
controller is not exposed through the standard GPIO API, and the regmap
approach cannot work because the controller is not a syscon device. This
explains why the cleaned-up version cannot access the blink registers
in time during early disk power-on.

Given this, I will follow your preferred (and cleaner) solution and
try to implement a dedicated
PWM driver for the Marvell/Orion GPIO blink engine, based on the
Linux driver.

I will update the series accordingly.

>
> 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 |
>