[PATCH 0/4] Facilitate new atmel raw nand driver for SAMA5D2
Hei hei,
for some downstream boards with SAMA5D27 SiP SoCs with raw NAND flash I added the necessary nodes to sama5d2.dtsi with just one little guesswork: the clocks. Motivation was to fix reading from raw nand flash which failed on some of those boards, while others worked fine. So I suppose the timings for the external interface were set to too slow values. (IIRC I took them from the sama5d2_ptc_ek_nandflash board code back in 2019.)
The solution was basically to switch from the old non-DM driver with hardcoded timings for the external memory interface to the new DM based driver introduced earlier this year, which sets the timings based on ONFI parameters read from the flash chip.
Works for me, but I have no boards with that SoC _and_ a raw NAND flash at hand, which are supported by upstream U-Boot. The only matching upstream config is sama5d2_ptc_ek_nandflash_defconfig but because we don't have that board, I did not touch it.
(When that driver was added with 6a8dfd57220d ("nand: atmel: Add DM based NAND driver") the author claims in the commit message it was tested on SAMA5D3, but none of the sama5d3 configs actually uses it.)
Some more changes to the board dts are necessary, but porting from Linux is straightforward. The necessary changes to the defconfig should look like this:
# CONFIG_I2C is not set CONFIG_LED=y CONFIG_LED_GPIO=y +CONFIG_ATMEL_EBI=y +CONFIG_MFD_ATMEL_SMC=y # CONFIG_MMC is not set CONFIG_MTD=y -CONFIG_NAND_ATMEL=y -CONFIG_ATMEL_NAND_HW_PMECC=y -CONFIG_PMECC_CAP=8 +CONFIG_DM_MTD=y +CONFIG_DM_NAND_ATMEL=y CONFIG_SYS_NAND_ONFI_DETECTION=y CONFIG_PHY_SMSC=y CONFIG_MACB=y
Hope the changes are acceptable nevertheless. The last patch has a trivial fix for the new atmel raw nand driver which I came up with while working on this.
Greets Alex
Cc: Eugen Hristev eugen.hristev@collabora.com Cc: Dario Binacchi dario.binacchi@amarulasolutions.com Cc: Michael Trimarchi michael@amarulasolutions.com Cc: Wenyou Yang wenyou.yang@microchip.com Cc: Balamanikandan Gunasundar balamanikandan.gunasundar@microchip.com
Alexander Dahl (4): ARM: dts: at91: sama5d2: Move sfr node ARM: dts: at91: sama5d2: Port ebi/nand nodes from linux ARM: dts: at91: sama5d2: Align more node names with Linux mtd: nand: raw: atmel: Remove duplicate definitions
arch/arm/dts/sama5d2.dtsi | 74 ++++++++++++++++++++++++++---- drivers/mtd/nand/raw/atmel/pmecc.c | 3 -- 2 files changed, 65 insertions(+), 12 deletions(-)
base-commit: 2f0282922b2c458eea7f85c500a948a587437b63
Nodes are ordered by register offset.
Fixes: 56246d1e8705 ("ARM: dts: at91: sama5: Add the sfr node") Signed-off-by: Alexander Dahl ada@thorsis.com --- arch/arm/dts/sama5d2.dtsi | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi index 819564fdd5b..c28a544da6f 100644 --- a/arch/arm/dts/sama5d2.dtsi +++ b/arch/arm/dts/sama5d2.dtsi @@ -698,6 +698,11 @@ status = "disabled"; };
+ sfr: sfr@f8030000 { + compatible = "atmel,sama5d2-sfr", "syscon"; + reg = <0xf8030000 0x98>; + }; + rstc@f8048000 { compatible = "atmel,sama5d3-rstc"; reg = <0xf8048000 0x10>; @@ -726,11 +731,6 @@ status = "disabled"; };
- sfr: sfr@f8030000 { - compatible = "atmel,sama5d2-sfr", "syscon"; - reg = <0xf8030000 0x98>; - }; - sckc@f8048050 { compatible = "atmel,at91sam9x5-sckc"; reg = <0xf8048050 0x4>;
Required for using the new DM based atmel nand driver. Ported from Linux v6.7-rc4.
Signed-off-by: Alexander Dahl ada@thorsis.com --- arch/arm/dts/sama5d2.dtsi | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi index c28a544da6f..0dbacade3b6 100644 --- a/arch/arm/dts/sama5d2.dtsi +++ b/arch/arm/dts/sama5d2.dtsi @@ -34,6 +34,15 @@ #size-cells = <1>; bootph-all;
+ nfc_sram: sram@100000 { + compatible = "mmio-sram"; + no-memory-wc; + reg = <0x00100000 0x2400>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x00100000 0x2400>; + }; + usb1: ohci@400000 { compatible = "atmel,at91rm9200-ohci", "usb-ohci"; reg = <0x00400000 0x100000>; @@ -50,6 +59,32 @@ status = "disabled"; };
+ ebi: ebi@10000000 { + compatible = "atmel,sama5d3-ebi"; + #address-cells = <2>; + #size-cells = <1>; + atmel,smc = <&hsmc>; + reg = <0x10000000 0x10000000 + 0x60000000 0x30000000>; + ranges = <0x0 0x0 0x10000000 0x10000000 + 0x1 0x0 0x60000000 0x10000000 + 0x2 0x0 0x70000000 0x10000000 + 0x3 0x0 0x80000000 0x10000000>; + clocks = <&h32ck>; + status = "disabled"; + + nand_controller: nand-controller { + compatible = "atmel,sama5d3-nand-controller"; + atmel,nfc-sram = <&nfc_sram>; + atmel,nfc-io = <&nfc_io>; + ecc-engine = <&pmecc>; + #address-cells = <2>; + #size-cells = <1>; + ranges; + status = "disabled"; + }; + }; + sdmmc0: sdio-host@a0000000 { compatible = "atmel,sama5d2-sdhci"; reg = <0xa0000000 0x300>; @@ -66,6 +101,11 @@ status = "disabled"; };
+ nfc_io: nfc-io@c0000000 { + compatible = "atmel,sama5d3-nfc-io", "syscon"; + reg = <0xc0000000 0x8000000>; + }; + apb { compatible = "simple-bus"; #address-cells = <1>; @@ -657,6 +697,22 @@ }; };
+ hsmc: hsmc@f8014000 { + compatible = "atmel,sama5d2-smc", "syscon", "simple-mfd"; + reg = <0xf8014000 0x1000>; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>; + clocks = <&hsmc_clk>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + pmecc: ecc-engine@f8014070 { + compatible = "atmel,sama5d2-pmecc"; + reg = <0xf8014070 0x490>, + <0xf8014500 0x200>; + }; + }; + uart0: serial@f801c000 { compatible = "atmel,at91sam9260-usart"; reg = <0xf801c000 0x100>;
Port from Linux v6.7-rc4. Should not hurt U-Boot but makes diffing easier and allows referencing node names in board dts.
Signed-off-by: Alexander Dahl ada@thorsis.com --- arch/arm/dts/sama5d2.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi index 0dbacade3b6..7b62fffb4ff 100644 --- a/arch/arm/dts/sama5d2.dtsi +++ b/arch/arm/dts/sama5d2.dtsi @@ -119,7 +119,7 @@ status = "disabled"; };
- pmc: pmc@f0014000 { + pmc: clock-controller@f0014000 { compatible = "atmel,sama5d2-pmc", "syscon"; reg = <0xf0014000 0x160>; #address-cells = <1>; @@ -759,13 +759,13 @@ reg = <0xf8030000 0x98>; };
- rstc@f8048000 { + reset_controller: reset-controller@f8048000 { compatible = "atmel,sama5d3-rstc"; reg = <0xf8048000 0x10>; clocks = <&clk32k>; };
- shdwc@f8048010 { + shutdown_controller: poweroff@f8048010 { compatible = "atmel,sama5d2-shdwc"; reg = <0xf8048010 0x10>; clocks = <&clk32k>; @@ -780,7 +780,7 @@ clocks = <&h32ck>; };
- watchdog@f8048040 { + watchdog: watchdog@f8048040 { compatible = "atmel,sama5d4-wdt"; reg = <0xf8048040 0x10>; clocks = <&clk32k>;
These removed definitions were specific to some sam9 SoCs, but not generic over all at91 SoCs. The correct SoC specific definitions for ATMEL_BASE_PMECC are spread over different header files in arch/arm/mach-at91/include/mach directory.
Fixes a build error on a custon board based on SAMA5D2:
Building current source for 73 boards (16 threads, 1 job per thread) arm: + vera2 +drivers/mtd/nand/raw/atmel/pmecc.c:819: warning: "ATMEL_BASE_PMECC" redefined + 819 | #define ATMEL_BASE_PMECC 0xffffe000 + | +In file included from include/configs/vera2.h:11, + from include/config.h:3, + from include/linux/mtd/rawnand.h:16, + from drivers/mtd/nand/raw/atmel/pmecc.c:44: +include/asm/arch/sama5d2.h:171: note: this is the location of the previous definition + 171 | #define ATMEL_BASE_PMECC (ATMEL_BASE_HSMC + 0x70) +drivers/mtd/nand/raw/atmel/pmecc.c:820: warning: "ATMEL_BASE_PMERRLOC" redefined + 820 | #define ATMEL_BASE_PMERRLOC 0xffffe600 +include/asm/arch/sama5d2.h:172: note: this is the location of the previous definition + 172 | #define ATMEL_BASE_PMERRLOC (ATMEL_BASE_HSMC + 0x500)
Fixes: a490e1b7c017 ("nand: atmel: Add pmecc driver") Signed-off-by: Alexander Dahl ada@thorsis.com --- drivers/mtd/nand/raw/atmel/pmecc.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c index e2e3f1ee6b5..51f6bd2e65b 100644 --- a/drivers/mtd/nand/raw/atmel/pmecc.c +++ b/drivers/mtd/nand/raw/atmel/pmecc.c @@ -816,9 +816,6 @@ int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user) } EXPORT_SYMBOL_GPL(atmel_pmecc_wait_rdy);
-#define ATMEL_BASE_PMECC 0xffffe000 -#define ATMEL_BASE_PMERRLOC 0xffffe600 - static struct atmel_pmecc * atmel_pmecc_create(struct udevice *dev, const struct atmel_pmecc_caps *caps,
On 12/12/23 18:04, Alexander Dahl wrote:
These removed definitions were specific to some sam9 SoCs, but not generic over all at91 SoCs. The correct SoC specific definitions for ATMEL_BASE_PMECC are spread over different header files in arch/arm/mach-at91/include/mach directory.
Fixes a build error on a custon board based on SAMA5D2:
Building current source for 73 boards (16 threads, 1 job per thread) arm: + vera2 +drivers/mtd/nand/raw/atmel/pmecc.c:819: warning: "ATMEL_BASE_PMECC" redefined + 819 | #define ATMEL_BASE_PMECC 0xffffe000 + | +In file included from include/configs/vera2.h:11, + from include/config.h:3, + from include/linux/mtd/rawnand.h:16, + from drivers/mtd/nand/raw/atmel/pmecc.c:44: +include/asm/arch/sama5d2.h:171: note: this is the location of the previous definition + 171 | #define ATMEL_BASE_PMECC (ATMEL_BASE_HSMC + 0x70) +drivers/mtd/nand/raw/atmel/pmecc.c:820: warning: "ATMEL_BASE_PMERRLOC" redefined + 820 | #define ATMEL_BASE_PMERRLOC 0xffffe600 +include/asm/arch/sama5d2.h:172: note: this is the location of the previous definition + 172 | #define ATMEL_BASE_PMERRLOC (ATMEL_BASE_HSMC + 0x500)Fixes: a490e1b7c017 ("nand: atmel: Add pmecc driver") Signed-off-by: Alexander Dahl ada@thorsis.com
drivers/mtd/nand/raw/atmel/pmecc.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c index e2e3f1ee6b5..51f6bd2e65b 100644 --- a/drivers/mtd/nand/raw/atmel/pmecc.c +++ b/drivers/mtd/nand/raw/atmel/pmecc.c @@ -816,9 +816,6 @@ int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user) } EXPORT_SYMBOL_GPL(atmel_pmecc_wait_rdy);
-#define ATMEL_BASE_PMECC 0xffffe000 -#define ATMEL_BASE_PMERRLOC 0xffffe600
static struct atmel_pmecc * atmel_pmecc_create(struct udevice *dev, const struct atmel_pmecc_caps *caps,
Hi Alexander,
What happens if we try to select and build this driver without sama5d2/sama5d3/sama5d4/sam9x5/sam9x60 ? Is it even possible ? Because it appears these defines are done for those SoCs in their mach header, but the driver uses them in any situation. And currently, these warnings are being ignored if the driver is built with sama5d2/sama5d3/sama5d4/sam9x5/sam9x60 ? or the driver actually isn't even built with these platforms at all ?
So, I guess it would be safer to do #ifndef ATMEL_BASE_PMECC, #define ...
Or, we could fix the driver to get these base addresses from the platform/DT ?
Eugen
Hello Eugen,
Am Tue, Dec 19, 2023 at 04:32:07PM +0200 schrieb Eugen Hristev:
On 12/12/23 18:04, Alexander Dahl wrote:
These removed definitions were specific to some sam9 SoCs, but not generic over all at91 SoCs. The correct SoC specific definitions for ATMEL_BASE_PMECC are spread over different header files in arch/arm/mach-at91/include/mach directory.
Fixes a build error on a custon board based on SAMA5D2:
Building current source for 73 boards (16 threads, 1 job per thread) arm: + vera2 +drivers/mtd/nand/raw/atmel/pmecc.c:819: warning: "ATMEL_BASE_PMECC" redefined + 819 | #define ATMEL_BASE_PMECC 0xffffe000 + | +In file included from include/configs/vera2.h:11, + from include/config.h:3, + from include/linux/mtd/rawnand.h:16, + from drivers/mtd/nand/raw/atmel/pmecc.c:44: +include/asm/arch/sama5d2.h:171: note: this is the location of the previous definition + 171 | #define ATMEL_BASE_PMECC (ATMEL_BASE_HSMC + 0x70) +drivers/mtd/nand/raw/atmel/pmecc.c:820: warning: "ATMEL_BASE_PMERRLOC" redefined + 820 | #define ATMEL_BASE_PMERRLOC 0xffffe600 +include/asm/arch/sama5d2.h:172: note: this is the location of the previous definition + 172 | #define ATMEL_BASE_PMERRLOC (ATMEL_BASE_HSMC + 0x500)Fixes: a490e1b7c017 ("nand: atmel: Add pmecc driver") Signed-off-by: Alexander Dahl ada@thorsis.com
drivers/mtd/nand/raw/atmel/pmecc.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c index e2e3f1ee6b5..51f6bd2e65b 100644 --- a/drivers/mtd/nand/raw/atmel/pmecc.c +++ b/drivers/mtd/nand/raw/atmel/pmecc.c @@ -816,9 +816,6 @@ int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user) } EXPORT_SYMBOL_GPL(atmel_pmecc_wait_rdy);
-#define ATMEL_BASE_PMECC 0xffffe000 -#define ATMEL_BASE_PMERRLOC 0xffffe600
static struct atmel_pmecc * atmel_pmecc_create(struct udevice *dev, const struct atmel_pmecc_caps *caps,
Hi Alexander,
What happens if we try to select and build this driver without sama5d2/sama5d3/sama5d4/sam9x5/sam9x60 ? Is it even possible ?
It is not possible on non-at91 boards because <mach/at91_sfr.h> is included so you need to select arch at91. But it is possible on older sam9 boards like sam9260, sam9g20 and the like. Tried that by using at91sam9g20ek_nandflash_defconfig as a base, then disabling CONFIG_NAND_ATMEL, then enabling CONFIG_DM_NAND_ATMEL (which activates build of drivers/mtd/nand/raw/atmel/pmecc.c), then enabling some more missing dependencies (CONFIG_MFD_ATMEL_SMC, CONFIG_REGMAP, CONFIG_SYSCON) and then it builds just fine.
(btw: should those be selected or implied by CONFIG_DM_NAND_ATMEL in 'drivers/mtd/nand/raw/Kconfig' then in another patch or is this stuff rather loose coupled?)
Because it appears these defines are done for those SoCs in their mach header, but the driver uses them in any situation.
I don't think so. There are two drivers, the old one is 'drivers/mtd/nand/raw/atmel_nand.c' selected by CONFIG_NAND_ATMEL while the new one is 'drivers/mtd/nand/raw/atmel/nand-controller.c' selected by CONFIG_DM_NAND_ATMEL (note the extra sub-directory). Only the new one leads to building the file in question. The symbols removed from pmecc.c are not used in pmecc.c but only in the old driver. The scope of those removed symbols would have been in pmecc.c only however, they are pointless at the place where they are currently defined. (Unless someone would #include that .c file, but that seems rather unusual.)
And the part where the symbols of the same name are used is conditionally built only if CONFIG_ATMEL_NAND_HW_PMECC is set. From a quick glance I would say that one is set only for newer boards, those families you mentioned below. But as said, all this is for the old driver.
And currently, these warnings are being ignored if the driver is built with sama5d2/sama5d3/sama5d4/sam9x5/sam9x60 ?
The warning only popped up for a custom board build, where I include <asm/arch/sama5d2.h> in file 'include/configs/myboard.h' and keep some old definitions for the old driver and then switching from the old to the new driver. I did not see it for any mainline at91 defconfig. Nevertheless I consider those defines wrong in that place.
or the driver actually isn't even built with these platforms at all ?
It depends on the (def)config. Currently in mainline U-Boot to my knowledge the new driver is only used by sam9x60 and sam7g*something, but it was ported from Linux and it should support all at91 SoCs. I think it is just a matter of who migrates all those boards to the new driver?
So, I guess it would be safer to do #ifndef ATMEL_BASE_PMECC, #define ...
Or, we could fix the driver to get these base addresses from the platform/DT ?
As said above. The new driver does not these definitions at all, but already gets these addresses from DT. See sam9x60 curiosity for example.
Greets Alex
On 12/19/23 18:39, Alexander Dahl wrote:
Hello Eugen,
Am Tue, Dec 19, 2023 at 04:32:07PM +0200 schrieb Eugen Hristev:
On 12/12/23 18:04, Alexander Dahl wrote:
These removed definitions were specific to some sam9 SoCs, but not generic over all at91 SoCs. The correct SoC specific definitions for ATMEL_BASE_PMECC are spread over different header files in arch/arm/mach-at91/include/mach directory.
Fixes a build error on a custon board based on SAMA5D2:
Building current source for 73 boards (16 threads, 1 job per thread) arm: + vera2 +drivers/mtd/nand/raw/atmel/pmecc.c:819: warning: "ATMEL_BASE_PMECC" redefined + 819 | #define ATMEL_BASE_PMECC 0xffffe000 + | +In file included from include/configs/vera2.h:11, + from include/config.h:3, + from include/linux/mtd/rawnand.h:16, + from drivers/mtd/nand/raw/atmel/pmecc.c:44: +include/asm/arch/sama5d2.h:171: note: this is the location of the previous definition + 171 | #define ATMEL_BASE_PMECC (ATMEL_BASE_HSMC + 0x70) +drivers/mtd/nand/raw/atmel/pmecc.c:820: warning: "ATMEL_BASE_PMERRLOC" redefined + 820 | #define ATMEL_BASE_PMERRLOC 0xffffe600 +include/asm/arch/sama5d2.h:172: note: this is the location of the previous definition + 172 | #define ATMEL_BASE_PMERRLOC (ATMEL_BASE_HSMC + 0x500)Fixes: a490e1b7c017 ("nand: atmel: Add pmecc driver") Signed-off-by: Alexander Dahl ada@thorsis.com
drivers/mtd/nand/raw/atmel/pmecc.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c b/drivers/mtd/nand/raw/atmel/pmecc.c index e2e3f1ee6b5..51f6bd2e65b 100644 --- a/drivers/mtd/nand/raw/atmel/pmecc.c +++ b/drivers/mtd/nand/raw/atmel/pmecc.c @@ -816,9 +816,6 @@ int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user) } EXPORT_SYMBOL_GPL(atmel_pmecc_wait_rdy);
-#define ATMEL_BASE_PMECC 0xffffe000 -#define ATMEL_BASE_PMERRLOC 0xffffe600
static struct atmel_pmecc * atmel_pmecc_create(struct udevice *dev, const struct atmel_pmecc_caps *caps,
Hi Alexander,
What happens if we try to select and build this driver without sama5d2/sama5d3/sama5d4/sam9x5/sam9x60 ? Is it even possible ?
It is not possible on non-at91 boards because <mach/at91_sfr.h> is included so you need to select arch at91. But it is possible on older sam9 boards like sam9260, sam9g20 and the like. Tried that by using at91sam9g20ek_nandflash_defconfig as a base, then disabling CONFIG_NAND_ATMEL, then enabling CONFIG_DM_NAND_ATMEL (which activates build of drivers/mtd/nand/raw/atmel/pmecc.c), then enabling some more missing dependencies (CONFIG_MFD_ATMEL_SMC, CONFIG_REGMAP, CONFIG_SYSCON) and then it builds just fine.
(btw: should those be selected or implied by CONFIG_DM_NAND_ATMEL in 'drivers/mtd/nand/raw/Kconfig' then in another patch or is this stuff rather loose coupled?)
Hi Alexander,
I tried to look around the drivers and the symbols, but it appears enabling any sort of driver makes things stop building, as the dependencies are not at all stated correctly. Thus, I cannot really answer that. About this modification in particular, I ran the CI loop, it appears to be fine, and I agree with the duplication, I was a bit reluctant to not break old platforms, but since the CI says it's fine... I applied your set to u-boot-at91/master .
Eugen
Because it appears these defines are done for those SoCs in their mach header, but the driver uses them in any situation.
I don't think so. There are two drivers, the old one is 'drivers/mtd/nand/raw/atmel_nand.c' selected by CONFIG_NAND_ATMEL while the new one is 'drivers/mtd/nand/raw/atmel/nand-controller.c' selected by CONFIG_DM_NAND_ATMEL (note the extra sub-directory). Only the new one leads to building the file in question. The symbols removed from pmecc.c are not used in pmecc.c but only in the old driver. The scope of those removed symbols would have been in pmecc.c only however, they are pointless at the place where they are currently defined. (Unless someone would #include that .c file, but that seems rather unusual.)
And the part where the symbols of the same name are used is conditionally built only if CONFIG_ATMEL_NAND_HW_PMECC is set. From a quick glance I would say that one is set only for newer boards, those families you mentioned below. But as said, all this is for the old driver.
And currently, these warnings are being ignored if the driver is built with sama5d2/sama5d3/sama5d4/sam9x5/sam9x60 ?
The warning only popped up for a custom board build, where I include <asm/arch/sama5d2.h> in file 'include/configs/myboard.h' and keep some old definitions for the old driver and then switching from the old to the new driver. I did not see it for any mainline at91 defconfig. Nevertheless I consider those defines wrong in that place.
or the driver actually isn't even built with these platforms at all ?
It depends on the (def)config. Currently in mainline U-Boot to my knowledge the new driver is only used by sam9x60 and sam7g*something, but it was ported from Linux and it should support all at91 SoCs. I think it is just a matter of who migrates all those boards to the new driver?
So, I guess it would be safer to do #ifndef ATMEL_BASE_PMECC, #define ...
Or, we could fix the driver to get these base addresses from the platform/DT ?
As said above. The new driver does not these definitions at all, but already gets these addresses from DT. See sam9x60 curiosity for example.
Greets Alex
On 12/12/23 18:04, Alexander Dahl wrote:
Hei hei,
for some downstream boards with SAMA5D27 SiP SoCs with raw NAND flash I added the necessary nodes to sama5d2.dtsi with just one little guesswork: the clocks. Motivation was to fix reading from raw nand flash which failed on some of those boards, while others worked fine. So I suppose the timings for the external interface were set to too slow values. (IIRC I took them from the sama5d2_ptc_ek_nandflash board code back in 2019.)
The solution was basically to switch from the old non-DM driver with hardcoded timings for the external memory interface to the new DM based driver introduced earlier this year, which sets the timings based on ONFI parameters read from the flash chip.
Works for me, but I have no boards with that SoC _and_ a raw NAND flash at hand, which are supported by upstream U-Boot. The only matching upstream config is sama5d2_ptc_ek_nandflash_defconfig but because we don't have that board, I did not touch it.
(When that driver was added with 6a8dfd57220d ("nand: atmel: Add DM based NAND driver") the author claims in the commit message it was tested on SAMA5D3, but none of the sama5d3 configs actually uses it.)
Some more changes to the board dts are necessary, but porting from Linux is straightforward. The necessary changes to the defconfig should look like this:
# CONFIG_I2C is not set CONFIG_LED=y CONFIG_LED_GPIO=y +CONFIG_ATMEL_EBI=y +CONFIG_MFD_ATMEL_SMC=y # CONFIG_MMC is not set CONFIG_MTD=y -CONFIG_NAND_ATMEL=y -CONFIG_ATMEL_NAND_HW_PMECC=y -CONFIG_PMECC_CAP=8 +CONFIG_DM_MTD=y +CONFIG_DM_NAND_ATMEL=y CONFIG_SYS_NAND_ONFI_DETECTION=y CONFIG_PHY_SMSC=y CONFIG_MACB=y
Hope the changes are acceptable nevertheless. The last patch has a trivial fix for the new atmel raw nand driver which I came up with while working on this.
Greets Alex
Cc: Eugen Hristev eugen.hristev@collabora.com Cc: Dario Binacchi dario.binacchi@amarulasolutions.com Cc: Michael Trimarchi michael@amarulasolutions.com Cc: Wenyou Yang wenyou.yang@microchip.com Cc: Balamanikandan Gunasundar balamanikandan.gunasundar@microchip.com
Alexander Dahl (4): ARM: dts: at91: sama5d2: Move sfr node ARM: dts: at91: sama5d2: Port ebi/nand nodes from linux ARM: dts: at91: sama5d2: Align more node names with Linux mtd: nand: raw: atmel: Remove duplicate definitions
arch/arm/dts/sama5d2.dtsi | 74 ++++++++++++++++++++++++++---- drivers/mtd/nand/raw/atmel/pmecc.c | 3 -- 2 files changed, 65 insertions(+), 12 deletions(-)
base-commit: 2f0282922b2c458eea7f85c500a948a587437b63
Applied series to u-boot-at91/master, thanks !
participants (2)
-
Alexander Dahl -
Eugen Hristev