Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
On Tue, 2016-06-07 at 07:41 +0200, Boris Brezillon wrote: > On Mon, 06 Jun 2016 18:54:03 -0500 > Scott Wood wrote: > > > Of course the driver model > > is probably the long-term solution. > > Definitely, and talking about things that need to be reworked, do you > know why u-boot is using its own MTD partition infrastructure instead > of relying on mtdpart.c? U-Boot's partition code predates the importation of the MTD code. > That's really a pain when one wants to add a new feature (like > definitions of partitions in the DT, or SLC mode on MLC NANDs) because > he has to do it twice. Defining partitions in the DT isn't such a great idea, at least on reference boards, as it's configuration that users are likely to want to change. > And that's not the only inconsistent part in the MTD/NAND layer IMO. > MTD is providing a generic abstraction for all flashes, but nand_util > is still directly accessing the NAND layer instead of going through the > MTD abstraction. As with partitions, that code predates the existence of the MTD abstraction in U-Boot. > By using the MTD abstraction everywhere (I mean for all flash devices), > we could provide generic utils (flash erase, flash write), even if > specific tools might be needed in a few cases. There are a lot of special NANDisms being handled in that code (bad block skipping, JFFS2 OOB cleanmarkers, etc), so I wonder what a generic version would look like. > Anyway, good to hear that you plan to switch to the driver model. I don't plan to do much of anything with the NAND code -- I'm still acting as custodian because nobody stepped up when I asked for volunteers to take it over a couple years back, but it's pretty low on my priority list regarding active development[1]. But if someone else wants to DM-ize a NAND driver I have no problem with that. :-) -Scott [1] Linux syncs are an exception, as they're easier to do than to review, especially since a patch only shows the end result rather than the process to produce it. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
On Mon, 06 Jun 2016 18:54:03 -0500 Scott Wood wrote: > On Mon, 2016-06-06 at 20:31 +0200, Boris Brezillon wrote: > > On Mon, 06 Jun 2016 12:56:48 -0500 > > Scott Wood wrote: > > > > > On Mon, 2016-06-06 at 18:22 +0200, Boris Brezillon wrote: > > > > On Mon, 6 Jun 2016 17:36:10 +0200 > > > > Hans de Goede wrote: > > > > > > > > > > +#ifndef CONFIG_SPL_BUILD > > > > > > +void sunxi_nand_init(void); > > > > > > +#endif > > > > > > + > > > > > > > > > > Can we have this in a header somewhere please, and without > > > > > the #ifdef around it, that is not necessary for prototypes. > > > > > > > > Hehe, I was expecting this one :-). Do you know where I should put this > > > > prototype definition? A board.h file in board/sunxi/? > > > > > > It's defined in drivers/mtd/nand and called from board/sunxi so the > > > prototype > > > needs to go somewhere under include/. It can go in include/configs/sunxi > > > -common.h with #ifndef __ASSEMBLY__ around it -- or as long as it's > > > limited to > > > one init func per driver, maybe we could just put it in include/nand.h. > > > > Hm, none of these solutions seem ideal. > > > > Maybe we could define a generic void nand_controller_init(void) > > prototype so that we don't need to add new xxx_nand_init() functions for > > platforms needing this 2 steps initialization (platform specific pinmux > > + clocks config before NAND controller initialization). > > > > Otherwise, I think I'll go for the 2nd solution (defining > > sunxi_nand_init() in include/nand.h). > > I'd prefer not having a generic nand_controller_init() because some platforms > may want to pass arguments, plus I don't want to rule out the possibility of > two different NAND controller types being supported at once. > > include/nand.h is fine with me, as long as any driver than wants more than an > initfunc moves its stuff into a dedicated header. Sure. > Of course the driver model > is probably the long-term solution. Definitely, and talking about things that need to be reworked, do you know why u-boot is using its own MTD partition infrastructure instead of relying on mtdpart.c? That's really a pain when one wants to add a new feature (like definitions of partitions in the DT, or SLC mode on MLC NANDs) because he has to do it twice. And that's not the only inconsistent part in the MTD/NAND layer IMO. MTD is providing a generic abstraction for all flashes, but nand_util is still directly accessing the NAND layer instead of going through the MTD abstraction. By using the MTD abstraction everywhere (I mean for all flash devices), we could provide generic utils (flash erase, flash write), even if specific tools might be needed in a few cases. Anyway, good to hear that you plan to switch to the driver model. Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
On Mon, 2016-06-06 at 20:31 +0200, Boris Brezillon wrote: > On Mon, 06 Jun 2016 12:56:48 -0500 > Scott Wood wrote: > > > On Mon, 2016-06-06 at 18:22 +0200, Boris Brezillon wrote: > > > On Mon, 6 Jun 2016 17:36:10 +0200 > > > Hans de Goede wrote: > > > > > > > > +#ifndef CONFIG_SPL_BUILD > > > > > +void sunxi_nand_init(void); > > > > > +#endif > > > > > + > > > > > > > > Can we have this in a header somewhere please, and without > > > > the #ifdef around it, that is not necessary for prototypes. > > > > > > Hehe, I was expecting this one :-). Do you know where I should put this > > > prototype definition? A board.h file in board/sunxi/? > > > > It's defined in drivers/mtd/nand and called from board/sunxi so the > > prototype > > needs to go somewhere under include/. It can go in include/configs/sunxi > > -common.h with #ifndef __ASSEMBLY__ around it -- or as long as it's > > limited to > > one init func per driver, maybe we could just put it in include/nand.h. > > Hm, none of these solutions seem ideal. > > Maybe we could define a generic void nand_controller_init(void) > prototype so that we don't need to add new xxx_nand_init() functions for > platforms needing this 2 steps initialization (platform specific pinmux > + clocks config before NAND controller initialization). > > Otherwise, I think I'll go for the 2nd solution (defining > sunxi_nand_init() in include/nand.h). I'd prefer not having a generic nand_controller_init() because some platforms may want to pass arguments, plus I don't want to rule out the possibility of two different NAND controller types being supported at once. include/nand.h is fine with me, as long as any driver than wants more than an initfunc moves its stuff into a dedicated header. Of course the driver model is probably the long-term solution. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
On Mon, 06 Jun 2016 12:56:48 -0500 Scott Wood wrote: > On Mon, 2016-06-06 at 18:22 +0200, Boris Brezillon wrote: > > On Mon, 6 Jun 2016 17:36:10 +0200 > > Hans de Goede wrote: > > > > > > +#ifndef CONFIG_SPL_BUILD > > > > +void sunxi_nand_init(void); > > > > +#endif > > > > + > > > > > > Can we have this in a header somewhere please, and without > > > the #ifdef around it, that is not necessary for prototypes. > > > > Hehe, I was expecting this one :-). Do you know where I should put this > > prototype definition? A board.h file in board/sunxi/? > > It's defined in drivers/mtd/nand and called from board/sunxi so the prototype > needs to go somewhere under include/. It can go in include/configs/sunxi > -common.h with #ifndef __ASSEMBLY__ around it -- or as long as it's limited to > one init func per driver, maybe we could just put it in include/nand.h. Hm, none of these solutions seem ideal. Maybe we could define a generic void nand_controller_init(void) prototype so that we don't need to add new xxx_nand_init() functions for platforms needing this 2 steps initialization (platform specific pinmux + clocks config before NAND controller initialization). Otherwise, I think I'll go for the 2nd solution (defining sunxi_nand_init() in include/nand.h). Let me know what you prefer. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
On Mon, 2016-06-06 at 18:22 +0200, Boris Brezillon wrote: > On Mon, 6 Jun 2016 17:36:10 +0200 > Hans de Goede wrote: > > > > +#ifndef CONFIG_SPL_BUILD > > > +void sunxi_nand_init(void); > > > +#endif > > > + > > > > Can we have this in a header somewhere please, and without > > the #ifdef around it, that is not necessary for prototypes. > > Hehe, I was expecting this one :-). Do you know where I should put this > prototype definition? A board.h file in board/sunxi/? It's defined in drivers/mtd/nand and called from board/sunxi so the prototype needs to go somewhere under include/. It can go in include/configs/sunxi -common.h with #ifndef __ASSEMBLY__ around it -- or as long as it's limited to one init func per driver, maybe we could just put it in include/nand.h. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
On Mon, 6 Jun 2016 17:36:10 +0200 Hans de Goede wrote: > > +#ifndef CONFIG_SPL_BUILD > > +void sunxi_nand_init(void); > > +#endif > > + > > Can we have this in a header somewhere please, and without > the #ifdef around it, that is not necessary for prototypes. Hehe, I was expecting this one :-). Do you know where I should put this prototype definition? A board.h file in board/sunxi/? -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
Hi, One small remark inline. On 06-06-16 17:21, Boris Brezillon wrote: We already have an SPL driver for the sunxi NAND controller, now add the normal/standard one. The source has been copied from Linux 4.6 with a few changes to make it work in u-boot. Signed-off-by: Boris Brezillon --- board/sunxi/board.c|9 +- drivers/mtd/nand/Kconfig |8 +- drivers/mtd/nand/Makefile |1 + drivers/mtd/nand/sunxi_nand.c | 1845 include/configs/sunxi-common.h |2 + include/fdtdec.h |1 + lib/fdtdec.c |1 + 7 files changed, 1863 insertions(+), 4 deletions(-) create mode 100644 drivers/mtd/nand/sunxi_nand.c diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 3cf3614..e0b3fb0 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -142,7 +142,7 @@ void dram_init_banksize(void) } #endif -#if defined(CONFIG_NAND_SUNXI) && defined(CONFIG_SPL_BUILD) +#if defined(CONFIG_NAND_SUNXI) static void nand_pinmux_setup(void) { unsigned int pin; @@ -175,10 +175,17 @@ static void nand_clock_setup(void) setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1); } +#ifndef CONFIG_SPL_BUILD +void sunxi_nand_init(void); +#endif + Can we have this in a header somewhere please, and without the #ifdef around it, that is not necessary for prototypes. Otherwise the entire series looks good to me. Regards, Hans void board_nand_init(void) { nand_pinmux_setup(); nand_clock_setup(); +#ifndef CONFIG_SPL_BUILD + sunxi_nand_init(); +#endif } #endif diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 8c46a2f..5ce7d6d 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -64,12 +64,14 @@ config NAND_PXA3XX PXA3xx processors (NFCv1) and also on Armada 370/XP (NFCv2). config NAND_SUNXI - bool "Support for NAND on Allwinner SoCs in SPL" + bool "Support for NAND on Allwinner SoCs" depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I select SYS_NAND_SELF_INIT ---help--- - Enable support for NAND. This option allows SPL to read from - sunxi NAND using DMA transfers. + Enable support for NAND. This option enables the standard and + SPL drivers. + The SPL driver only supports reading from the NAND using DMA + transfers. config NAND_ARASAN bool "Configure Arasan Nand" diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 837d397..1df9273 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o obj-$(CONFIG_NAND_PLAT) += nand_plat.o +obj-$(CONFIG_NAND_SUNXI) += sunxi_nand.o else # minimal SPL drivers diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c new file mode 100644 index 000..c4e2cd7 --- /dev/null +++ b/drivers/mtd/nand/sunxi_nand.c @@ -0,0 +1,1845 @@ +/* + * Copyright (C) 2013 Boris BREZILLON + * Copyright (C) 2015 Roy Spliet + * + * Derived from: + * https://github.com/yuq/sunxi-nfc-mtd + * Copyright (C) 2013 Qiang Yu + * + * https://github.com/hno/Allwinner-Info + * Copyright (C) 2013 Henrik Nordström + * + * Copyright (C) 2013 Dmitriy B. + * Copyright (C) 2013 Sergey Lapin + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * SPDX-License-Identifier:GPL-2.0+ + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +#define NFC_REG_CTL0x +#define NFC_REG_ST 0x0004 +#define NFC_REG_INT0x0008 +#define NFC_REG_TIMING_CTL 0x000C +#define NFC_REG_TIMING_CFG 0x0010 +#define NFC_REG_ADDR_LOW 0x0014 +#define NFC_REG_ADDR_HIGH 0x0018 +#define NFC_REG_SECTOR_NUM 0x001C +#define NFC_REG_CNT0x0020 +#define NFC_REG_CMD0x0024 +#define NFC_REG_RCMD_SET 0x0028 +#define NFC_REG_WCMD_SET 0x002C +#define NFC_REG_IO_DATA0x0030 +#define NFC_REG_ECC_CTL0x0034 +#define NFC_REG_ECC_ST 0x0038 +#define NFC_REG_DEBUG 0x003C +#define NFC_REG_ECC_ERR_CNT(x) ((0x0040 + (x)) & ~0x3) +#define NFC_REG_USER_DATA(x) (0x0050 + ((x) * 4)) +#define NFC_REG_SPARE_AREA 0x00A0 +#define NFC_REG_PAT_ID 0
[U-Boot] [PATCH 3/6] mtd: nand: Add the sunxi NAND controller driver
We already have an SPL driver for the sunxi NAND controller, now add the normal/standard one. The source has been copied from Linux 4.6 with a few changes to make it work in u-boot. Signed-off-by: Boris Brezillon --- board/sunxi/board.c|9 +- drivers/mtd/nand/Kconfig |8 +- drivers/mtd/nand/Makefile |1 + drivers/mtd/nand/sunxi_nand.c | 1845 include/configs/sunxi-common.h |2 + include/fdtdec.h |1 + lib/fdtdec.c |1 + 7 files changed, 1863 insertions(+), 4 deletions(-) create mode 100644 drivers/mtd/nand/sunxi_nand.c diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 3cf3614..e0b3fb0 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -142,7 +142,7 @@ void dram_init_banksize(void) } #endif -#if defined(CONFIG_NAND_SUNXI) && defined(CONFIG_SPL_BUILD) +#if defined(CONFIG_NAND_SUNXI) static void nand_pinmux_setup(void) { unsigned int pin; @@ -175,10 +175,17 @@ static void nand_clock_setup(void) setbits_le32(&ccm->nand0_clk_cfg, CCM_NAND_CTRL_ENABLE | AHB_DIV_1); } +#ifndef CONFIG_SPL_BUILD +void sunxi_nand_init(void); +#endif + void board_nand_init(void) { nand_pinmux_setup(); nand_clock_setup(); +#ifndef CONFIG_SPL_BUILD + sunxi_nand_init(); +#endif } #endif diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 8c46a2f..5ce7d6d 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -64,12 +64,14 @@ config NAND_PXA3XX PXA3xx processors (NFCv1) and also on Armada 370/XP (NFCv2). config NAND_SUNXI - bool "Support for NAND on Allwinner SoCs in SPL" + bool "Support for NAND on Allwinner SoCs" depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I select SYS_NAND_SELF_INIT ---help--- - Enable support for NAND. This option allows SPL to read from - sunxi NAND using DMA transfers. + Enable support for NAND. This option enables the standard and + SPL drivers. + The SPL driver only supports reading from the NAND using DMA + transfers. config NAND_ARASAN bool "Configure Arasan Nand" diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 837d397..1df9273 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o obj-$(CONFIG_NAND_PLAT) += nand_plat.o +obj-$(CONFIG_NAND_SUNXI) += sunxi_nand.o else # minimal SPL drivers diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c new file mode 100644 index 000..c4e2cd7 --- /dev/null +++ b/drivers/mtd/nand/sunxi_nand.c @@ -0,0 +1,1845 @@ +/* + * Copyright (C) 2013 Boris BREZILLON + * Copyright (C) 2015 Roy Spliet + * + * Derived from: + * https://github.com/yuq/sunxi-nfc-mtd + * Copyright (C) 2013 Qiang Yu + * + * https://github.com/hno/Allwinner-Info + * Copyright (C) 2013 Henrik Nordström + * + * Copyright (C) 2013 Dmitriy B. + * Copyright (C) 2013 Sergey Lapin + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * SPDX-License-Identifier:GPL-2.0+ + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +#define NFC_REG_CTL0x +#define NFC_REG_ST 0x0004 +#define NFC_REG_INT0x0008 +#define NFC_REG_TIMING_CTL 0x000C +#define NFC_REG_TIMING_CFG 0x0010 +#define NFC_REG_ADDR_LOW 0x0014 +#define NFC_REG_ADDR_HIGH 0x0018 +#define NFC_REG_SECTOR_NUM 0x001C +#define NFC_REG_CNT0x0020 +#define NFC_REG_CMD0x0024 +#define NFC_REG_RCMD_SET 0x0028 +#define NFC_REG_WCMD_SET 0x002C +#define NFC_REG_IO_DATA0x0030 +#define NFC_REG_ECC_CTL0x0034 +#define NFC_REG_ECC_ST 0x0038 +#define NFC_REG_DEBUG 0x003C +#define NFC_REG_ECC_ERR_CNT(x) ((0x0040 + (x)) & ~0x3) +#define NFC_REG_USER_DATA(x) (0x0050 + ((x) * 4)) +#define NFC_REG_SPARE_AREA 0x00A0 +#define NFC_REG_PAT_ID 0x00A4 +#define NFC_RAM0_BASE 0x0400 +#define NFC_RAM1_BASE 0x0800 + +/* define bit use in NFC_CTL */ +#define NFC_EN BIT(0) +#define NFC_RESET BIT(1) +#define NFC_BUS_WIDTH_MSK BIT(2) +#define NFC_BUS_W