[PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Markus Hoffrogge]

The sunxi nand SPL loader was broken at least for SUN4I,
SUN5I and SUN7I SOCs since the implementation change
from DMA to PIO usage - commit 6ddbb1e.

Root cause for this issue is the NFC control flag NFC_CTL_RAM_METHOD
being set by method nand_apply_config.

This flag controls the bus being used for the NFCs internal RAM access.
It must be set for the DMA use case only.
See A33_Nand_Flash_Controller_Specification.pdf page 12.

This fix is tested by myself on a Cubietruck A20 board.
Others should test it on new generation SOCs as well.

Signed-off-by: Markus Hoffrogge <mhoffrogge@gmail.com>
---
 drivers/mtd/nand/raw/sunxi_nand_spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c
b/drivers/mtd/nand/raw/sunxi_nand_spl.c
index a29a76c58d..6de0b0a355 100644
--- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
+++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
@@ -208,7 +208,7 @@ static void nand_apply_config(const struct nfc_config
*conf)

  val = readl(SUNXI_NFC_BASE + NFC_CTL);
  val &= ~NFC_CTL_PAGE_SIZE_MASK;
- writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size),
+ writel(val | NFC_CTL_PAGE_SIZE(conf->page_size),
        SUNXI_NFC_BASE + NFC_CTL);
  writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
  writel(conf->page_size, SUNXI_NFC_BASE + NFC_SPARE_AREA);
--

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Miquel Raynal]

Hi Markus,

+ Maxime, for the record :)

mhoffrogge@gmail.com wrote on Thu, 30 Jun 2022 01:26:39 +0200:

> The sunxi nand SPL loader was broken at least for SUN4I,
> SUN5I and SUN7I SOCs since the implementation change
> from DMA to PIO usage - commit 6ddbb1e.
> 
> Root cause for this issue is the NFC control flag NFC_CTL_RAM_METHOD
> being set by method nand_apply_config.
> 
> This flag controls the bus being used for the NFCs internal RAM access.
> It must be set for the DMA use case only.
> See A33_Nand_Flash_Controller_Specification.pdf page 12.
> 
> This fix is tested by myself on a Cubietruck A20 board.
> Others should test it on new generation SOCs as well.

Good to know that someone tackled this, Maxime already reported some
time ago that it was broken for a number of boards but I never took the
time to investigate, apologies.

> Signed-off-by: Markus Hoffrogge <mhoffrogge@gmail.com>
> ---
>  drivers/mtd/nand/raw/sunxi_nand_spl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index a29a76c58d..6de0b0a355 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -208,7 +208,7 @@ static void nand_apply_config(const struct nfc_config
> *conf)
> 
>   val = readl(SUNXI_NFC_BASE + NFC_CTL);
>   val &= ~NFC_CTL_PAGE_SIZE_MASK;
> - writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size),
> + writel(val | NFC_CTL_PAGE_SIZE(conf->page_size),
>         SUNXI_NFC_BASE + NFC_CTL);
>   writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
>   writel(conf->page_size, SUNXI_NFC_BASE + NFC_SPARE_AREA);
> --

Thanks,
Miquèl

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On Thu, Jun 30, 2022 at 09:13:22AM +0200, Miquel Raynal wrote:
> Hi Markus,
> 
> + Maxime, for the record :)
> 
> mhoffrogge@gmail.com wrote on Thu, 30 Jun 2022 01:26:39 +0200:
> 
> > The sunxi nand SPL loader was broken at least for SUN4I,
> > SUN5I and SUN7I SOCs since the implementation change
> > from DMA to PIO usage - commit 6ddbb1e.
> > 
> > Root cause for this issue is the NFC control flag NFC_CTL_RAM_METHOD
> > being set by method nand_apply_config.
> > 
> > This flag controls the bus being used for the NFCs internal RAM access.
> > It must be set for the DMA use case only.
> > See A33_Nand_Flash_Controller_Specification.pdf page 12.
> > 
> > This fix is tested by myself on a Cubietruck A20 board.
> > Others should test it on new generation SOCs as well.
> 
> Good to know that someone tackled this, Maxime already reported some
> time ago that it was broken for a number of boards but I never took the
> time to investigate, apologies.

Oh, that's awesome, thanks for tackling this :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Markus Hoffrogge]

Hi Miquel, hi Maxime,

could you probably run a test on a newer Allwinner SOC - like e.g.
SUNI8, SUNI9, SUNI50 or newer, since I can test it on a SUNI7 (A20)
board only?

The only aspect that makes me wonder is, that it did work on your
hardware before at all.

Maybe this is, since that NFC_CTL_RAM_METHOD flag does not matter on
newer SOC, if the DMA channel is not enabled for the NFC.

Markus
Am Do., 30. Juni 2022 um 09:31 Uhr schrieb Maxime Ripard <maxime@cerno.tech>:
>
> On Thu, Jun 30, 2022 at 09:13:22AM +0200, Miquel Raynal wrote:
> > Hi Markus,
> >
> > + Maxime, for the record :)
> >
> > mhoffrogge@gmail.com wrote on Thu, 30 Jun 2022 01:26:39 +0200:
> >
> > > The sunxi nand SPL loader was broken at least for SUN4I,
> > > SUN5I and SUN7I SOCs since the implementation change
> > > from DMA to PIO usage - commit 6ddbb1e.
> > >
> > > Root cause for this issue is the NFC control flag NFC_CTL_RAM_METHOD
> > > being set by method nand_apply_config.
> > >
> > > This flag controls the bus being used for the NFCs internal RAM access.
> > > It must be set for the DMA use case only.
> > > See A33_Nand_Flash_Controller_Specification.pdf page 12.
> > >
> > > This fix is tested by myself on a Cubietruck A20 board.
> > > Others should test it on new generation SOCs as well.
> >
> > Good to know that someone tackled this, Maxime already reported some
> > time ago that it was broken for a number of boards but I never took the
> > time to investigate, apologies.
>
> Oh, that's awesome, thanks for tackling this :)
>
> Maxime

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On Thu, Jun 30, 2022 at 10:04:31AM +0200, Markus Hoffrogge wrote:
> Hi Miquel, hi Maxime,
> 
> could you probably run a test on a newer Allwinner SOC - like e.g.
> SUNI8, SUNI9, SUNI50 or newer, since I can test it on a SUNI7 (A20)
> board only?
> 
> The only aspect that makes me wonder is, that it did work on your
> hardware before at all.
> 
> Maybe this is, since that NFC_CTL_RAM_METHOD flag does not matter on
> newer SOC, if the DMA channel is not enabled for the NFC.

I don't have any of those boards anymore, but that sounds plausible

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[andrés ramírez]

Hi. Markus.

>>>>> "Markus" == Markus Hoffrogge <mhoffrogge@gmail.com> writes:

    Markus> Hi Miquel, hi Maxime, could you probably run a test on a newer Allwinner SOC - like e.g.
    Markus> SUNI8, SUNI9, SUNI50 or newer, since I can test it on a SUNI7 (A20) board only?

I have a topwise A721 tablet which is sun4i (A10). Should I test on it?.

How to test it?. Just download latest 2022-07-rc and see if archlinux
boots?

Best Regards

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Markus Hoffrogge]

Am Do., 30. Juni 2022 um 16:01 Uhr schrieb andrés ramírez
<rrandresf@hotmail.com>:
>
> Hi. Markus.
>
> >>>>> "Markus" == Markus Hoffrogge <mhoffrogge@gmail.com> writes:
>
>     Markus> Hi Miquel, hi Maxime, could you probably run a test on a newer Allwinner SOC - like e.g.
>     Markus> SUNI8, SUNI9, SUNI50 or newer, since I can test it on a SUNI7 (A20) board only?
>
> I have a topwise A721 tablet which is sun4i (A10). Should I test on it?.
>
> How to test it?. Just download latest 2022-07-rc and see if archlinux
> boots?

Hi Andrés,

you can try, but A10 to me is not that critical, since the NFC is
close to the one of A20.
Yes, you would have to replace the SPL U-Boot in the SPL partition of the NAND.

br
Markus

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Markus Hoffrogge]

Hi Andre,

as this patch has been delegated to you,
I would like to mention, that this patch is superseding
 the patch https://patchwork.ozlabs.org/project/uboot/patch/20220330204543.3790-1-macroalpha82@gmail.com/
raised by Chris Morgan.

It should solve the issue in general and does not require to
revert to the former DMA implementation.

Kind regards
Markus

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Andre Przywara]

On Thu, 30 Jun 2022 23:30:50 +0200
Markus Hoffrogge <mhoffrogge@gmail.com> wrote:

Hi Markus,

> as this patch has been delegated to you,

thanks for sending the patch, and for the heads up, looks
like we should add that file somewhere into MAINTAINERS.
(And please keep people on CC:, to not split up the thread on the
list).

> I would like to mention, that this patch is superseding
>  the patch https://patchwork.ozlabs.org/project/uboot/patch/20220330204543.3790-1-macroalpha82@gmail.com/
> raised by Chris Morgan.

So from a first look at it, I like this version better, mostly because
it's much shorter ;-)

That being said, I tried to stay away from raw NAND as much as possible in
the past, and thought that our support in U-Boot is very minimal. So can
someone please enlighten me about the status:
- AFAIK raw NAND is only supported on the C.H.I.P. Pro, because this is
one the few devices featuring SLC NAND. I guess the main feature of raw
NAND is that it's cheaper, so using SLC instead of MLC kind of defeats the
main purpose of it.
- Is there a separate SPL story? As in: loading U-Boot proper from NAND
when the BROM loaded the SPL from there? IIUC read disturbance makes even
that part non-trivial.
- Because of the absence of SLC in modern devices, and the prevalence of
eMMC, raw NAND was rarely used on modern devices (>= H3). So did we ever
support that? My understanding was that the NAND controller itself is
fairly similar (is it?), but the MLC problem would still persist.
- How do you actually write to the NAND? It seems like even reading is
non-trivial, so is this done from U-Boot, (BSP?) Linux, or using vendor
tools, probably via FEL?

I recently got an H6 TV box, which happens to use raw (MLC) NAND flash. I
also inherited a Cubietruck, which is MLC as well. So can I use that
storage, at least for the firmware?

If yes, this should be documented somewhere, I guess? Or is it already?

Cheers,
Andre

> It should solve the issue in general and does not require to
> revert to the former DMA implementation.
> 
> Kind regards
> Markus

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Markus Hoffrogge]

Am Fr., 1. Juli 2022 um 11:57 Uhr schrieb Andre Przywara <
andre.przywara@arm.com>:
Hi Andre,

> On Thu, 30 Jun 2022 23:30:50 +0200
> Markus Hoffrogge <mhoffrogge@gmail.com> wrote:
>
> Hi Markus,
>
> > as this patch has been delegated to you,
>
> thanks for sending the patch, and for the heads up, looks
> like we should add that file somewhere into MAINTAINERS.
> (And please keep people on CC:, to not split up the thread on the
> list).
>
> > I would like to mention, that this patch is superseding
> >  the patch
> https://patchwork.ozlabs.org/project/uboot/patch/20220330204543.3790-1-macroalpha82@gmail.com/
> > raised by Chris Morgan.
>
> So from a first look at it, I like this version better, mostly because
> it's much shorter ;-)
>
> That being said, I tried to stay away from raw NAND as much as possible in
> the past, and thought that our support in U-Boot is very minimal. So can
> someone please enlighten me about the status:
> - AFAIK raw NAND is only supported on the C.H.I.P. Pro, because this is
> one the few devices featuring SLC NAND. I guess the main feature of raw
> NAND is that it's cheaper, so using SLC instead of MLC kind of defeats the
> main purpose of it.
> - Is there a separate SPL story? As in: loading U-Boot proper from NAND
> when the BROM loaded the SPL from there? IIUC read disturbance makes even
> that part non-trivial.
> - Because of the absence of SLC in modern devices, and the prevalence of
> eMMC, raw NAND was rarely used on modern devices (>= H3). So did we ever
> support that? My understanding was that the NAND controller itself is
> fairly similar (is it?), but the MLC problem would still persist.
>

I fully agree, but the MLC problem has been adressed already a while ago
by introducing the slc_mode on mainline Kernel for MLC
with the NAND page pairing approach by Boris Brezillon and Richard
Weinberger.
See
https://bootlin.com/pub/conferences/2016/elce/brezillon-ubi-mlc/ubi-mlc.pdf.

It will half the usable size of the NAND, but I guess it is perfect
and best approach for getting the MLC used in a most robust way.
So this at least should be sufficient for using it for some minimal
rootfs for boot configuration or initial startup OS for configuring and
e.g. attaching/initializing a SATA SSD.

Main motivation is to get the board NAND booting
not to rely on a SD card, since this is weak in rough or
industrial environments and the SDCard slot should be free
for other usage.
The OS will be on a SATA SSD in most usecases anyway.

- How do you actually write to the NAND? It seems like even reading is
> non-trivial, so is this done from U-Boot, (BSP?) Linux, or using vendor
> tools, probably via FEL?
>

Writing to MLC NAND I am doing as follows on a Cubietruck (CT):
  1. I create an Armbian image for SDCARD for the CT with a currently
    user patched CT UBoot and kernel config with the following:
    (I will provide an appropriate patch for this separately later)
    Note: This CT config extension does not break the SDCard boot.
    - enabling SUNXI NAND and SUNXI NAND SPL
      Extended config on UBoot configs/Cubietruck_defconfig:
      [...append to current CT config]
      CONFIG_NAND=y
      CONFIG_NAND_SUNXI=y
      CONFIG_MTD=y
      CONFIG_MTD_RAW_NAND=y
      #
      # SPL loader NAND offsets according to Kernel
      # .dts mtd partitions:
      #
      CONFIG_SYS_NAND_U_BOOT_OFFS=0x400000
      CONFIG_SYS_NAND_U_BOOT_OFFS_REDUND=0x600000
      #
      # CT specific Hynix MLC NAND 27UCG8T2ATR-BC config:
      #
      CONFIG_NAND_SUNXI_SPL_ECC_STRENGTH=64
      CONFIG_NAND_SUNXI_SPL_ECC_SIZE=1024
      CONFIG_NAND_SUNXI_SPL_USABLE_PAGE_SIZE=4096
      CONFIG_SYS_NAND_PAGE_SIZE=8192
      CONFIG_SYS_NAND_OOBSIZE=640
      CONFIG_SYS_NAND_BLOCK_SIZE=0x200000
      [EOF]
    - overlay .dts for the kernel to add nfc node with SPL and UBoot
      NAND partitions 2MB each on NAND blocksize boundaries providing:
      - /dev/mtd0 - SPL
      - /dev/mtd1 - SPL-backup
      - /dev/mtd2 - UBoot
      - /dev/mtd3 - UBoot-backup
      - /dev/mtd4 - UBI (UBI in slc_mode for rootfs - later use)
                    This rootfs yould more or less hold the kernel
                    image and other scripts relevant for booring
                    to be more flexible for updates on the kernel
                    and the boot environment scripts.
    - add mtd-utils package for the OS
    - I will provide a github link for my Armbian fork soon
      its not yet commited
  2. Plugin SDCard and boot CT with this image from SD card
     - copy UBoot SPL ./spl/sunxi-spl-with-ecc.bin to the CT
     - copy UBoot image u-boot-dtb.img to the CT
  3. Logon to CT and run on CT:
     - erase /dev/mtd0
       - sudo flash_erase /dev/mtd0 0 0
     - write the SUNXI UBoot SPL ./spl/sunxi-spl-with-ecc.bin to /dev/mtd0
       - sudo nandwrite -o -n /dev/mtd0 sunxi-spl-with-ecc.bin
     - erase /dev/mtd2
       - sudo flash_erase /dev/mtd2 0 0
     - write u-boot-dtb.img to /dev/mtd2
       - sudo nandwrite -p /dev/mtd2 u-boot-dtb.img
   4. Shutdown CT
   5. Unplug SDCard and reboot
      - Now CT boots from NAND and will find SATA OS, if SATA available

I recently got an H6 TV box, which happens to use raw (MLC) NAND flash. I
> also inherited a Cubietruck, which is MLC as well. So can I use that
> storage, at least for the firmware?
>
>
Yes, CT should work as described above.

If yes, this should be documented somewhere, I guess? Or is it already?
>

No, it is not yet documented.
I am currently working on an approrpiate pull request for Armbian build.
I would like to finish up with the UBI partition setup
to complete that portion.
For this U-Boot CT config I am supposed to provide another patch later.

Where would be the best place to document this on the U-Boot tree ?

br
Markus


>
> Cheers,
> Andre
>
> It should solve the issue in general and does not require to
> > revert to the former DMA implementation.
> >
> > Kind regards
> > Markus
>
>

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Michael Nazzareno Trimarchi]

Hi

On Thu, Jun 30, 2022 at 9:31 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Thu, Jun 30, 2022 at 09:13:22AM +0200, Miquel Raynal wrote:
> > Hi Markus,
> >
> > + Maxime, for the record :)
> >
> > mhoffrogge@gmail.com wrote on Thu, 30 Jun 2022 01:26:39 +0200:
> >
> > > The sunxi nand SPL loader was broken at least for SUN4I,
> > > SUN5I and SUN7I SOCs since the implementation change
> > > from DMA to PIO usage - commit 6ddbb1e.
> > >
> > > Root cause for this issue is the NFC control flag NFC_CTL_RAM_METHOD
> > > being set by method nand_apply_config.
> > >
> > > This flag controls the bus being used for the NFCs internal RAM access.
> > > It must be set for the DMA use case only.
> > > See A33_Nand_Flash_Controller_Specification.pdf page 12.
> > >
> > > This fix is tested by myself on a Cubietruck A20 board.
> > > Others should test it on new generation SOCs as well.
> >
> > Good to know that someone tackled this, Maxime already reported some
> > time ago that it was broken for a number of boards but I never took the
> > time to investigate, apologies.
>
> Oh, that's awesome, thanks for tackling this :)

I don't have boards too with Allwinner but maybe Jagan has

Michael

>
> Maxime

Re: [PATCH] sunxi-nand: fix the PIO instead of DMA implementation

[Andre Przywara]

On Thu, 30 Jun 2022 01:26:39 +0200
Markus Hoffrogge <mhoffrogge@gmail.com> wrote:

Hi,

> The sunxi nand SPL loader was broken at least for SUN4I,
> SUN5I and SUN7I SOCs since the implementation change
> from DMA to PIO usage - commit 6ddbb1e.
> 
> Root cause for this issue is the NFC control flag NFC_CTL_RAM_METHOD
> being set by method nand_apply_config.
> 
> This flag controls the bus being used for the NFCs internal RAM access.
> It must be set for the DMA use case only.
> See A33_Nand_Flash_Controller_Specification.pdf page 12.
> 
> This fix is tested by myself on a Cubietruck A20 board.
> Others should test it on new generation SOCs as well.

I can't really test this, but looking at the datasheet indeed bit 14 must
not be set when we intend to read the SRAM buffers directly via MMIO.

> Signed-off-by: Markus Hoffrogge <mhoffrogge@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Applied to sunxi/master.

Cheers,
Andre

> ---
>  drivers/mtd/nand/raw/sunxi_nand_spl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> index a29a76c58d..6de0b0a355 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand_spl.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand_spl.c
> @@ -208,7 +208,7 @@ static void nand_apply_config(const struct nfc_config
> *conf)
> 
>   val = readl(SUNXI_NFC_BASE + NFC_CTL);
>   val &= ~NFC_CTL_PAGE_SIZE_MASK;
> - writel(val | NFC_CTL_RAM_METHOD | NFC_CTL_PAGE_SIZE(conf->page_size),
> + writel(val | NFC_CTL_PAGE_SIZE(conf->page_size),
>         SUNXI_NFC_BASE + NFC_CTL);
>   writel(conf->ecc_size, SUNXI_NFC_BASE + NFC_CNT);
>   writel(conf->page_size, SUNXI_NFC_BASE + NFC_SPARE_AREA);
> --