[U-Boot] [PATCH] mtd: zynq: nand: Move board_nand_init() function to board.c
Putting board_nand_init() function inside NAND driver was not appropriate due to it doesn't allow board vendor to customise their NAND initialization code such as adding NAND lock/unlock code.
This commit was to move the board_nand_init() function from NAND driver to board.c file. This allow customization of board_nand_init() function.
Signed-off-by: Wilson Lee wilson.lee@ni.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Keng Soon Cheah keng.soon.cheah@ni.com Cc: Chen Yee Chew chen.yee.chew@ni.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Michal Simek michal.simek@xilinx.com Cc: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com Cc: Scott Wood oss@buserror.net --- arch/arm/mach-zynq/include/mach/nand.h | 9 +++++++++ board/xilinx/zynq/board.c | 10 ++++++++++ drivers/mtd/nand/zynq_nand.c | 12 +----------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 arch/arm/mach-zynq/include/mach/nand.h
diff --git a/arch/arm/mach-zynq/include/mach/nand.h b/arch/arm/mach-zynq/include/mach/nand.h new file mode 100644 index 0000000..61ef45f --- /dev/null +++ b/arch/arm/mach-zynq/include/mach/nand.h @@ -0,0 +1,9 @@ +/* + * Copyright (C) 2017 National Instruments Corp. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <nand.h> + +void zynq_nand_init(void); diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 90ef542..8bcb830 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -9,6 +9,7 @@ #include <fpga.h> #include <mmc.h> #include <zynqpl.h> +#include <asm/arch/nand.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h>
@@ -156,3 +157,12 @@ int dram_init(void) return 0; } #endif + +static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE]; +void board_nand_init(void) +{ + struct nand_chip *nand = &nand_chip[0]; + + if (zynq_nand_init(nand, 0)) + puts("ZYNQ NAND init failed\n"); +} diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index dec2c41..9be6856 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct mtd_info *mtd) return 0; }
-static int zynq_nand_init(struct nand_chip *nand_chip, int devnum) +int zynq_nand_init(struct nand_chip *nand_chip, int devnum) { struct zynq_nand_info *xnand; struct mtd_info *mtd; @@ -1191,13 +1191,3 @@ fail: free(xnand); return err; } - -static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE]; - -void board_nand_init(void) -{ - struct nand_chip *nand = &nand_chip[0]; - - if (zynq_nand_init(nand, 0)) - puts("ZYNQ NAND init failed\n"); -}
On 8.11.2017 03:44, Wilson Lee wrote:
Putting board_nand_init() function inside NAND driver was not appropriate due to it doesn't allow board vendor to customise their NAND initialization code such as adding NAND lock/unlock code.
This commit was to move the board_nand_init() function from NAND driver to board.c file. This allow customization of board_nand_init() function.
Signed-off-by: Wilson Lee wilson.lee@ni.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Keng Soon Cheah keng.soon.cheah@ni.com Cc: Chen Yee Chew chen.yee.chew@ni.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Michal Simek michal.simek@xilinx.com Cc: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com Cc: Scott Wood oss@buserror.net
arch/arm/mach-zynq/include/mach/nand.h | 9 +++++++++ board/xilinx/zynq/board.c | 10 ++++++++++ drivers/mtd/nand/zynq_nand.c | 12 +----------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 arch/arm/mach-zynq/include/mach/nand.h
diff --git a/arch/arm/mach-zynq/include/mach/nand.h b/arch/arm/mach-zynq/include/mach/nand.h new file mode 100644 index 0000000..61ef45f --- /dev/null +++ b/arch/arm/mach-zynq/include/mach/nand.h @@ -0,0 +1,9 @@ +/*
- Copyright (C) 2017 National Instruments Corp.
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <nand.h>
+void zynq_nand_init(void); diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 90ef542..8bcb830 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -9,6 +9,7 @@ #include <fpga.h> #include <mmc.h> #include <zynqpl.h> +#include <asm/arch/nand.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h>
@@ -156,3 +157,12 @@ int dram_init(void) return 0; } #endif
+static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE]; +void board_nand_init(void) +{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");+}
This is probably not enough for platforms which don't enable NAND.
diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index dec2c41..9be6856 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct mtd_info *mtd) return 0; }
-static int zynq_nand_init(struct nand_chip *nand_chip, int devnum) +int zynq_nand_init(struct nand_chip *nand_chip, int devnum) { struct zynq_nand_info *xnand; struct mtd_info *mtd; @@ -1191,13 +1191,3 @@ fail: free(xnand); return err; }
-static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
-void board_nand_init(void) -{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");-}
What's the sequence which you want to add? Isn't it better just to include this there or create a hook here?
Definitely some platforms moved this to board file and not a problem with moving it because it shouldn't be in nand driver.
Thanks, Michal
Hi Michal,
On Wed, 2017-11-08 at 08:54 +0100, Michal Simek wrote:
On 8.11.2017 03:44, Wilson Lee wrote:
Putting board_nand_init() function inside NAND driver was not appropriate due to it doesn't allow board vendor to customise their NAND initialization code such as adding NAND lock/unlock code.
This commit was to move the board_nand_init() function from NAND driver to board.c file. This allow customization of board_nand_init() function.
Signed-off-by: Wilson Lee wilson.lee@ni.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Keng Soon Cheah keng.soon.cheah@ni.com Cc: Chen Yee Chew chen.yee.chew@ni.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Michal Simek michal.simek@xilinx.com Cc: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com Cc: Scott Wood oss@buserror.net
arch/arm/mach-zynq/include/mach/nand.h | 9 +++++++++ board/xilinx/zynq/board.c | 10 ++++++++++ drivers/mtd/nand/zynq_nand.c | 12 +----------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 arch/arm/mach-zynq/include/mach/nand.h
diff --git a/arch/arm/mach-zynq/include/mach/nand.h b/arch/arm/mach-zynq/include/mach/nand.h new file mode 100644 index 0000000..61ef45f --- /dev/null +++ b/arch/arm/mach-zynq/include/mach/nand.h @@ -0,0 +1,9 @@ +/*
- Copyright (C) 2017 National Instruments Corp.
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <nand.h>
+void zynq_nand_init(void); diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 90ef542..8bcb830 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -9,6 +9,7 @@ #include <fpga.h> #include <mmc.h> #include <zynqpl.h> +#include <asm/arch/nand.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> @@ -156,3 +157,12 @@ int dram_init(void) return 0; } #endif
+static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE]; +void board_nand_init(void) +{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");+}
This is probably not enough for platforms which don't enable NAND.
diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index dec2c41..9be6856 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct mtd_info *mtd) return 0; } -static int zynq_nand_init(struct nand_chip *nand_chip, int devnum) +int zynq_nand_init(struct nand_chip *nand_chip, int devnum) { struct zynq_nand_info *xnand; struct mtd_info *mtd; @@ -1191,13 +1191,3 @@ fail: free(xnand); return err; }
-static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
-void board_nand_init(void) -{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");-}
What's the sequence which you want to add?
Actually we wish to perform a NAND unlock right after the NAND was begin initialized. So that, we can do some write operation (ubifs table R/W) when we mount it as ubifs.
Isn't it better just to include this there or create a hook here?
I was thinking for adding a hook here before. But, it will be more make sense to me if we correct the NAND driver by moving board_nand to board.c file.
Definitely some platforms moved this to board file and not a problem with moving it because it shouldn't be in nand driver.
Thanks, Michal
Thanks, Michal.
Best Regards, Wilson Lee
On 8.11.2017 10:24, Wilson Lee wrote:
Hi Michal,
On Wed, 2017-11-08 at 08:54 +0100, Michal Simek wrote:
On 8.11.2017 03:44, Wilson Lee wrote:
Putting board_nand_init() function inside NAND driver was not appropriate due to it doesn't allow board vendor to customise their NAND initialization code such as adding NAND lock/unlock code.
This commit was to move the board_nand_init() function from NAND driver to board.c file. This allow customization of board_nand_init() function.
Signed-off-by: Wilson Lee wilson.lee@ni.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Keng Soon Cheah keng.soon.cheah@ni.com Cc: Chen Yee Chew chen.yee.chew@ni.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Michal Simek michal.simek@xilinx.com Cc: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com Cc: Scott Wood oss@buserror.net
arch/arm/mach-zynq/include/mach/nand.h | 9 +++++++++ board/xilinx/zynq/board.c | 10 ++++++++++ drivers/mtd/nand/zynq_nand.c | 12 +----------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 arch/arm/mach-zynq/include/mach/nand.h
diff --git a/arch/arm/mach-zynq/include/mach/nand.h b/arch/arm/mach-zynq/include/mach/nand.h new file mode 100644 index 0000000..61ef45f --- /dev/null +++ b/arch/arm/mach-zynq/include/mach/nand.h @@ -0,0 +1,9 @@ +/*
- Copyright (C) 2017 National Instruments Corp.
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <nand.h>
+void zynq_nand_init(void); diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 90ef542..8bcb830 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -9,6 +9,7 @@ #include <fpga.h> #include <mmc.h> #include <zynqpl.h> +#include <asm/arch/nand.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> @@ -156,3 +157,12 @@ int dram_init(void) return 0; } #endif
+static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE]; +void board_nand_init(void) +{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");+}
This is probably not enough for platforms which don't enable NAND.
diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index dec2c41..9be6856 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct mtd_info *mtd) return 0; } -static int zynq_nand_init(struct nand_chip *nand_chip, int devnum) +int zynq_nand_init(struct nand_chip *nand_chip, int devnum) { struct zynq_nand_info *xnand; struct mtd_info *mtd; @@ -1191,13 +1191,3 @@ fail: free(xnand); return err; }
-static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
-void board_nand_init(void) -{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");-}
What's the sequence which you want to add?
Actually we wish to perform a NAND unlock right after the NAND was begin initialized. So that, we can do some write operation (ubifs table R/W) when we mount it as ubifs.
Isn't it better just to include this there or create a hook here?
I was thinking for adding a hook here before. But, it will be more make sense to me if we correct the NAND driver by moving board_nand to board.c file.
ok. It means please put there some ifdefs around and can you also please mark board_nand_init as weak in board.c that boards which include full board.c file can overwrite it by own function? At least I hope that we reduce number of duplications.
Thanks, Michal
Hi Wilson,
On 8 November 2017 at 06:24, Wilson Lee wilson.lee@ni.com wrote:
Hi Michal,
On Wed, 2017-11-08 at 08:54 +0100, Michal Simek wrote:
On 8.11.2017 03:44, Wilson Lee wrote:
Putting board_nand_init() function inside NAND driver was not appropriate due to it doesn't allow board vendor to customise their NAND initialization code such as adding NAND lock/unlock code.
This commit was to move the board_nand_init() function from NAND driver to board.c file. This allow customization of board_nand_init() function.
Signed-off-by: Wilson Lee wilson.lee@ni.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Keng Soon Cheah keng.soon.cheah@ni.com Cc: Chen Yee Chew chen.yee.chew@ni.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Michal Simek michal.simek@xilinx.com Cc: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com Cc: Scott Wood oss@buserror.net
arch/arm/mach-zynq/include/mach/nand.h | 9 +++++++++ board/xilinx/zynq/board.c | 10 ++++++++++ drivers/mtd/nand/zynq_nand.c | 12 +----------- 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 arch/arm/mach-zynq/include/mach/nand.h
diff --git a/arch/arm/mach-zynq/include/mach/nand.h b/arch/arm/mach-zynq/include/mach/nand.h new file mode 100644 index 0000000..61ef45f --- /dev/null +++ b/arch/arm/mach-zynq/include/mach/nand.h @@ -0,0 +1,9 @@ +/*
- Copyright (C) 2017 National Instruments Corp.
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <nand.h>
+void zynq_nand_init(void); diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 90ef542..8bcb830 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -9,6 +9,7 @@ #include <fpga.h> #include <mmc.h> #include <zynqpl.h> +#include <asm/arch/nand.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h>
@@ -156,3 +157,12 @@ int dram_init(void) return 0; } #endif
+static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE]; +void board_nand_init(void) +{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");+}
This is probably not enough for platforms which don't enable NAND.
diff --git a/drivers/mtd/nand/zynq_nand.c b/drivers/mtd/nand/zynq_nand.c index dec2c41..9be6856 100644 --- a/drivers/mtd/nand/zynq_nand.c +++ b/drivers/mtd/nand/zynq_nand.c @@ -1006,7 +1006,7 @@ static int zynq_nand_device_ready(struct mtd_info *mtd) return 0; }
-static int zynq_nand_init(struct nand_chip *nand_chip, int devnum) +int zynq_nand_init(struct nand_chip *nand_chip, int devnum) { struct zynq_nand_info *xnand; struct mtd_info *mtd; @@ -1191,13 +1191,3 @@ fail: free(xnand); return err; }
-static struct nand_chip nand_chip[CONFIG_SYS_MAX_NAND_DEVICE];
-void board_nand_init(void) -{
- struct nand_chip *nand = &nand_chip[0];
- if (zynq_nand_init(nand, 0))
puts("ZYNQ NAND init failed\n");-}
What's the sequence which you want to add?
Actually we wish to perform a NAND unlock right after the NAND was begin initialized. So that, we can do some write operation (ubifs table R/W) when we mount it as ubifs.
Why do you need to export board_nand_init in order to add do a NAND unlock after NAND initialization?
Perhaps you could point me at your U-Boot repo to see what you need exactly.
I'm seeing at least two ways of doing this:
1) Using a script that calls nand unlock command and then does the UBIFS stuff.
2) Using board_late_init like this:
static void unlock_nand(void) { int dev = nand_curr_device; struct mtd_info *mtd;
mtd = get_nand_dev_by_index(dev); nand_unlock(mtd, 0, mtd->size, 0); }
int board_late_init(void) { /* some other stuff .... */
unlock_nand(); }
To be honest, I dislike this patch quite a bit, because it's code meant for out-of-tree ports.
We tend to avoid keeping infra for out-of-tree code, because it makes the code hard to improve or cleanup without breaking the out-of-tree port doing so.
Isn't it better just to include this there or create a hook here?
I was thinking for adding a hook here before. But, it will be more make sense to me if we correct the NAND driver by moving board_nand to board.c file.
Definitely some platforms moved this to board file and not a problem with moving it because it shouldn't be in nand driver.
Thanks, Michal
Thanks, Michal.
Best Regards, Wilson Lee _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (3)
-
Ezequiel Garcia -
Michal Simek -
Wilson Lee