Re: [PATCH/RFC] ARM: dts: marzen: Add FLASH node

[Geert Uytterhoeven <geert@linux-m68k.org>]

On Mon, Apr 10, 2023 at 11:25 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Apr 7, 2023 at 9:16 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> > On 4/3/23 17:29, Geert Uytterhoeven wrote:
> > > On Tue, Mar 21, 2023 at 4:01 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >> On Mon, Mar 20, 2023 at 7:57 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >>> On Mon, Mar 20, 2023 at 6:04 PM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> > >>>> Vignesh used to review CFI code, maybe he can intervene. I've never
> > >>>> worked with CFI, but I can try to help. I'll need more debug data though.
> > >>>>
> > >>>> On 3/20/23 16:43, Geert Uytterhoeven wrote:
> > >>>>> Add a device node for the Spansion S29GL512N NOR FLASH on the Marzen
> > >>>>> development board.
> > >>>>>
> > >>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >>>>> ---
> > >>>>> Although the S29GL512N is a CFI FLASH, using "cfi-flash" instead of
> > >>>>> "mtd-rom" does not work:
> > >>>>>   1. Probing fails with "physmap-flash 0.flash: map_probe failed",
> > >>>>
> > >>>> I would first try to understand why the probe fails.
> > >
> > >> I suddenly remembered I have a different board (APE6-EVM), where
> > >> the CFI-FLASH stopped working after adding support for secondary
> > >> CPUs. I always thought that was a hardware quirk...
> > >>
> > >> Turns out the CFI-FLASH on Marzen (quad Cortex-A9) is detected when
> > >> booting with "nosmp":
> > >
> > >> My first guess was that the probing process is migrated to a different
> > >> CPU core during probing, but printing smp_processor_id() didn't
> > >> confirm that; it's just running on a different CPU core than CPU0.
> > >> Wrapping the body of cfi_qry_mode_on() inside a get_cpu()/put_cpu()
> > >> pair to prevent migration also didn't fix it.
> > >>
> > >> Is CFI-FLASH known-broken on SMP?
> > >
> > > After actively looking for more boards with CFI FLASHes, and finding one
> > > more board where FLASH probing fails on SMP, I dug deeper.
> > > Turns out they all have in common that (a) the CFI FLASH is located at
> > > physical address zero, and (b) the secondary CPU bringup code relies
> > > on mapping (by special hardware) the region at address zero to the
> > > CPU boot code...
> > >
> > > Hence fixing this involves making sure that accessing FLASH and bringing
> > > CPU cores online do not happen concurrently...
> >
> > Would deferring probe for CFI work?
>
> Probe deferral of CFI FLASH would not help, as the FLASH is already
> probed after secondary CPU startup.
>
> Sequence of operations is:
>   1. Map first page(s) of physical address space to RAM containing
>      CPU startup code (not using the MMU, but using a special
>      register in the SoC),
>   2. Boot secondary CPU cores,
>   3. Probe CFI-FLASH: fails, as accesses to the first page(s) of
>      physical address space do not address the FLASH, but the CPU
>      startup code in RAM.
>   4. After boot, CPU cores can be offlined and onlined using CPU
>       hotplug through sysfs.
>
> When using "mtd-rom" instead of "cfi-flash", the system boots fine,
> but the first page of /dev/mtd0 does not contain the FLASH contents,
> but the CPU startup code, which I hadn't noticed originally...
>
> Disabling the special mapping for the CPU startup code after all cores
> have been brought up (in between steps 2 and 3) fixes the CFI-FLASH.
> However, if a CPU core is offlined and onlined at run-time, the special
> mapping has to be reinstantiated temporarily again, thus any FLASH
> accesses (by other CPUs) must be prevented temporarily, too.
>
> This issue is present on all Renesas SH/R-Mobile and R-Car SoCs
> that support SMP. I am wondering if there are any other SMP SoCS that
> suffer from similar issues, and that have solved them? I couldn't find
> any in-tree DTS for a board with CFI-FLASH or mtd-rom at address zero
> using an SMP ARM SoC...
>
> I guess the simplest "solution" is to disable in DT any device at address zero
> when SMP is used (when step 1 is executed)...

Or reserve the boot area using request_mem_region().
I have posted an RFC patch series doing that:
"[PATCH/RFC 0/4] ARM: shmobile: Reserve boot area when SMP is enabled"
https://lore.kernel.org/all/cover.1693409184.git.geert+renesas@glider.be

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds