mtd: nand: add sunxi NAND Flash Controller support
Subsystems (5)
v3 cover mtd: nand: add sunxi NAND Flash Controller support
This series adds support for the sunxi NAND Flash Controller (NFC).
I'll split this series for the next round:
1) NAND timings
2) sunxi NAND controller driver
But I'd like to have some feedback on the NAND timings stuff before doing this.
Any review on the sunxi driver is welcome (for the moment I only got feedback on the NAND timings code).
I also have patches adding support for HW randomizer and per ECC partition config that you can find here:
https://groups.google.com/forum/#!searchin/linux-sunxi/boris/linux-sunxi/s3lBb01I0Js/Bem2X1wKPa4J
But I'd like to get the sunxi NAND driver mainlined before adding more intrusive parts.
Best Regards,
Boris
Changes since v2:
- merge HW ECC implementation in base implementation patch
- fix timing config when interfacing with an ONFI compatible chip
Changes since v1:
- add HW ECC support
- rework NAND timings retrieval (use ONFI timing mode instead of raw timings)
- add nand-ecc-level property to specify NAND ECC requirements from DT
Boris BREZILLON (9):
mtd: nand: define struct nand_timings
mtd: nand: add ONFI timing mode to nand_timings converter
of: mtd: add NAND timing mode retrieval support
of: mtd: add documentation for the ONFI NAND timing mode property
mtd: nand: add sunxi NAND flash controller support
mtd: nand: add sunxi NFC dt bindings doc
ARM: dt/sunxi: add NFC node to Allwinner A20 SoC
ARM: dt/sunxi: add A20 NAND controller pin definitions
ARM: sunxi/dt: enable NAND on cubietruck board
Documentation/devicetree/bindings/mtd/nand.txt | 8 + .../devicetree/bindings/mtd/sunxi-nand.txt | 48 + arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 17 + arch/arm/boot/dts/sun7i-a20.dtsi | 91 ++ drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 3 +- drivers/mtd/nand/nand_timings.c | 248 ++++ drivers/mtd/nand/sunxi_nand.c | 1276 ++++++++++++++++++++ drivers/of/of_mtd.c | 19 + include/linux/mtd/nand.h | 53 + include/linux/of_mtd.h | 8 + 11 files changed, 1776 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt create mode 100644 drivers/mtd/nand/nand_timings.c create mode 100644 drivers/mtd/nand/sunxi_nand.c
Signature · 1 line
1.7.9.5
PATCH v3 1/9 mtd: nand: define struct nand_timings
Define a struct containing the standard NAND timings as described in NAND datasheets.
--- include/linux/mtd/nand.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)
include/linux/mtd/nand.h +49 -0
@@ -846,4 +846,53 @@ static inline bool nand_is_slc(struct nand_chip *chip) {return chip->bits_per_cell ==1 ; } + +/** +* struct nand_sdr_timings - SDR NAND chip timings +* +* This struct defines the timing requirements of a SDR NAND chip. +* These informations can be found in every NAND datasheets and the timings +* meaning are described in the ONFI specifications: +* www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf? (chapter 4.15 Timing +* Parameters) +* +*/ + +struct nand_sdr_timings { +u32 tALH_min; +u32 tADL_min; +u32 tALS_min; +u32 tAR_min; +u32 tCEA_max; +u32 tCEH_min; +u32 tCH_min; +u32 tCHZ_max; +u32 tCLH_min; +u32 tCLR_min; +u32 tCLS_min; +u32 tCOH_min; +u32 tCS_min; +u32 tDH_min; +u32 tDS_min; +u32 tFEAT_max; +u32 tIR_min; +u32 tITC_max; +u32 tRC_min; +u32 tREA_max; +u32 tREH_min; +u32 tRHOH_min; +u32 tRHW_min; +u32 tRHZ_max; +u32 tRLOH_min; +u32 tRP_min; +u32 tRR_min; +u64 tRST_max; +u32 tWB_max; +u32 tWC_min; +u32 tWH_min; +u32 tWHR_min; +u32 tWP_min; +u32 tWW_min; +}; +#endif /* __LINUX_MTD_NAND_H */
Signature · 1 line
1.7.9.5
Re: [PATCH v3 1/9] mtd: nand: define struct nand_timings
Hi Boris,
Quoted 14 lines · Boris BREZILLON
Define a struct containing the standard NAND timings as described in NAND datasheets.
Signed-off-by Boris BREZILLON <[email protected]>--- include/linux/mtd/nand.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)include/linux/mtd/nand.h +0
-0@@ -846,4 +846,53 @@ static inline bool nand_is_slc(struct nand_chip *chip) {
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 856
Quoted 10 lines
include/linux/mtd/nand.h +8
-0return chip->bits_per_cell ==1 ; } + +/** +* struct nand_sdr_timings - SDR NAND chip timings +* +* This struct defines the timing requirements of a SDR NAND chip. +* These informations can be found in every NAND datasheets and the timings +* meaning are described in the ONFI specifications: +* www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf? (chapter 4.15 Timing
Can you remove the unicode U+200E character?
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 857
Quoted 1 line
include/linux/mtd/nand.h +1
-0+* Parameters)
Please document the units for these fields here. It looks like you're using picoseconds.
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 860
Quoted 3 lines
include/linux/mtd/nand.h +3
-0+* +*/ +
Extra blank line.
Quoted 28 lines
include/linux/mtd/nand.h +28
-0+struct nand_sdr_timings { +u32 tALH_min; +u32 tADL_min; +u32 tALS_min; +u32 tAR_min; +u32 tCEA_max; +u32 tCEH_min; +u32 tCH_min; +u32 tCHZ_max; +u32 tCLH_min; +u32 tCLR_min; +u32 tCLS_min; +u32 tCOH_min; +u32 tCS_min; +u32 tDH_min; +u32 tDS_min; +u32 tFEAT_max; +u32 tIR_min; +u32 tITC_max; +u32 tRC_min; +u32 tREA_max; +u32 tREH_min; +u32 tRHOH_min; +u32 tRHW_min; +u32 tRHZ_max; +u32 tRLOH_min; +u32 tRP_min; +u32 tRR_min;
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 898
Quoted 10 lines
include/linux/mtd/nand.h +9
-0+u64 tRST_max; +u32 tWB_max; +u32 tWC_min; +u32 tWH_min; +u32 tWHR_min; +u32 tWP_min; + u32 tWW_min; +}; +#endif /* __LINUX_MTD_NAND_H */
Brian
Re: [PATCH v3 1/9] mtd: nand: define struct nand_timings
Hi Brian,
Quoted 12 lines · Brian Norris
Hi Boris,
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 856
Quoted 8 lines · Boris BREZILLON
include/linux/mtd/nand.h +8
-0+ +/** +* struct nand_sdr_timings - SDR NAND chip timings +* +* This struct defines the timing requirements of a SDR NAND chip. +* These informations can be found in every NAND datasheets and the timings +* meaning are described in the ONFI specifications: +* www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf? (chapter 4.15 Timing Can you remove the unicode U+200E character?
Sure
Quoted 4 lines
Quoted 2 lines
+ * Parameters)
Please document the units for these fields here. It looks like you're using picoseconds.
I'll add field units (which are indeed picoseconds) to this comment.
Best Regards,
Boris
Re: [PATCH v3 1/9] mtd: nand: define struct nand_timings
In reply to Brian NorrisQuoted 12 lines
Quoted 12 lines
Define a struct containing the standard NAND timings as described in NAND datasheets.
Signed-off-by Boris BREZILLON <[email protected]>--- include/linux/mtd/nand.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)include/linux/mtd/nand.h +0
-0
[...]
Quoted 4 lines
Quoted 1 line
+ * Parameters)
Please document the units for these fields here. It looks like you're using picoseconds.
Can't we leave this open to interpretation? For instance, it's more convenient for our driver to handle these as nano second values.
Quoted 6 lines
Quoted 3 lines
+ *
+ */
+Extra blank line.
Quoted 14 lines
Quoted 4 lines
include/linux/mtd/nand.h +4
-0+struct nand_sdr_timings { +u32 tALH_min; +u32 tADL_min; +u32 tALS_min;include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 874
Quoted 10 lines
include/linux/mtd/nand.h +10
-0+u32 tAR_min; +u32 tCEA_max; +u32 tCEH_min; +u32 tCH_min; +u32 tCHZ_max; +u32 tCLH_min; +u32 tCLR_min; +u32 tCLS_min; +u32 tCOH_min; +u32 tCS_min;
u32 tCSD_min;
Quoted 5 lines
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 879
Quoted 5 lines
include/linux/mtd/nand.h +5
-0+u32 tDH_min; +u32 tDS_min; +u32 tFEAT_max; +u32 tIR_min; +u32 tITC_max;
u32 tR_max;
Quoted 19 lines
Quoted 19 lines
include/linux/mtd/nand.h +18
-0+u32 tRC_min; +u32 tREA_max; +u32 tREH_min; +u32 tRHOH_min; +u32 tRHW_min; +u32 tRHZ_max; +u32 tRLOH_min; +u32 tRP_min; +u32 tRR_min; +u64 tRST_max; +u32 tWB_max; +u32 tWC_min; +u32 tWH_min; +u32 tWHR_min; +u32 tWP_min; + u32 tWW_min; +}; +#endif /* __LINUX_MTD_NAND_H */
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v3 1/9] mtd: nand: define struct nand_timings
Quoted 25 lines · Lee Jones
Quoted 12 lines
Quoted 12 lines
Define a struct containing the standard NAND timings as described in NAND datasheets.
Signed-off-by Boris BREZILLON <[email protected]>--- include/linux/mtd/nand.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)include/linux/mtd/nand.h +0
-0[...]
Quoted 3 lines
Quoted 1 line
+ * Parameters)
Please document the units for these fields here. It looks like you're using picoseconds.
Can't we leave this open to interpretation? For instance, it's more convenient for our driver to handle these as nano second values.
Quoted 5 lines
Quoted 3 lines
+ *
+ */
+Extra blank line.
Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
include/linux/mtd/nand.h +6
-0+struct nand_sdr_timings { +u32 tALH_min; +u32 tADL_min; +u32 tALS_min; +u32 tAR_min; +u32 tCEA_max;Quoted 8 lines
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 874
Quoted 8 lines
include/linux/mtd/nand.h +8
-0+u32 tCEH_min; +u32 tCH_min; +u32 tCHZ_max; +u32 tCLH_min; +u32 tCLR_min; +u32 tCLS_min; +u32 tCOH_min; +u32 tCS_min;u32 tCSD_min;
This is related to ddr2 timings, and this structure only define sdr related timings.
Quoted 6 lines
Quoted 5 lines
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 879
Quoted 5 lines
include/linux/mtd/nand.h +5
-0+u32 tDH_min; +u32 tDS_min; +u32 tFEAT_max; +u32 tIR_min; +u32 tITC_max;u32 tR_max;
Actually this one cannot be retrieved from the ONFI timing mode, you'll have to read bytes 137 and 138 in the parameter page.
Hence I got rid of it in the first place. Then I considered adding all those missing timings in another structure (see [1]).
I noticed you posted something similar on the MTD mailing list (and Lucas was waiting for the feature too).
Could we work together to propose something that fulfills all our needs ?
Here's what I need:
- get detailled timings to be able to configure the sunxi NAND controller appropriately
- a way to get these timings on non-ONFi NANDs (the proposed solution is exposing the closest supported ONFI timing mode in the NAND chip DT node, but I'm open to any new proposal).
Best Regards,
Boris
[1] http://lists.infradead.org/pipermail/linux-mtd/2014-March/052525.html
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH v3 1/9] mtd: nand: define struct nand_timings
In reply to Lee JonesQuoted 6 lines · Lee Jones
Quoted 3 lines
Please document the units for these fields here. It looks like you're using picoseconds.
Can't we leave this open to interpretation? For instance, it's more convenient for our driver to handle these as nano second values.
No, their values will be determined by the nand_base core, and we must have something consistent for drivers to rely on. However, I don't really have a hard preference on nanoseconds versus picoseconds. If we see that many of the values reach low-digit nanosecons, or fractional nanoseconds, it probably makes sense to have the higher resolution.
Quoted 1 line
Quoted 1 line
Quoted 1 line
+struct
nand_sdr_timings{
...
Quoted 3 lines
Quoted 1 line
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 874
Quoted 1 line
include/linux/mtd/nand.h +1
-0+u32 tCS_min;u32 tCSD_min;
Is this a suggested addition, Lee? I agree with Boris that this looks like a DDR mode, which should not be covered here.
Quoted 7 lines
Quoted 5 lines
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 879
Quoted 5 lines
include/linux/mtd/nand.h +5
-0+u32 tDH_min; +u32 tDS_min; +u32 tFEAT_max; +u32 tIR_min; +u32 tITC_max;u32 tR_max;
Same here, is this a suggested new field? If you need it, then we can follow up like Boris suggested, with a different method, since tR is not part of the electrical parameters of the timing mode.
...
Quoted 3 lines
Quoted 3 lines
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 898
Quoted 3 lines
include/linux/mtd/nand.h +2
-0+}; +#endif /* __LINUX_MTD_NAND_H */
Brian
PATCH v3 2/9 mtd: nand: add ONFI timing mode to nand_timings converter
In reply to Boris BREZILLONAdd a converter to retrieve NAND timings from an ONFI NAND timing mode. This only support SDR NAND timings for now.
--- drivers/mtd/nand/Makefile | 2 +- drivers/mtd/nand/nand_timings.c | 248 +++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 4 + 3 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/nand_timings.c
drivers/mtd/nand/Makefile +1 -1
@@ -2,7 +2,7 @@ # linux/drivers/nand/Makefile # -obj-$(CONFIG_MTD_NAND) += nand.o +obj-$(CONFIG_MTD_NAND) += nand.o nand_timings.o obj-$(CONFIG_MTD_NAND_ECC) += nand_ecc.o obj-$(CONFIG_MTD_NAND_BCH) += nand_bch.o obj-$(CONFIG_MTD_NAND_IDS) += nand_ids.o nand_hynix.o
drivers/mtd/nand/nand_timings.c new file +248 -0
248 lines not on this page · raw message
include/linux/mtd/nand.h +4 -0
@@ -847,6 +847,7 @@ static inline bool nand_is_slc(struct nand_chip *chip)return chip->bits_per_cell ==1 ; } +/** * struct nand_sdr_timings - SDR NAND chip timings *
@@ -895,4 +896,7 @@ struct nand_sdr_timings { u32 tWW_min; }; +/* convert an ONFI timing mode to its timing characteristics. */ +const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings (int mode); +#endif /* __LINUX_MTD_NAND_H */
Signature · 1 line
1.7.9.5
Re: [PATCH v3 2/9] mtd: nand: add ONFI timing mode to nand_timings converter
Hi,
Quoted 16 lines · Boris BREZILLON
Add a converter to retrieve NAND timings from an ONFI NAND timing mode. This only support SDR NAND timings for now.
Signed-off-by Boris BREZILLON <[email protected]>--- drivers/mtd/nand/Makefile | 2 +- drivers/mtd/nand/nand_timings.c | 248 +++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 4 + 3 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 drivers/mtd/nand/nand_timings.cdrivers/mtd/nand/Makefile +0
-0
drivers/mtd/nand/Makefile line 8
Quoted 8 lines
drivers/mtd/nand/Makefile +1
-1@@ -2,7 +2,7 @@ # linux/drivers/nand/Makefile # -obj-$(CONFIG_MTD_NAND) += nand.o +obj-$(CONFIG_MTD_NAND) += nand.o nand_timings.o obj-$(CONFIG_MTD_NAND_ECC) += nand_ecc.o obj-$(CONFIG_MTD_NAND_BCH) += nand_bch.o obj-$(CONFIG_MTD_NAND_IDS) += nand_ids.o nand_hynix.o
This patch doesn't apply to l2-mtd.git. Are you basing this on your Hynix patches? I thought some (all?) of them were determined unnecessary. Anyway, please rebase on l2-mtd.git, since there are a couple of other bits that don't apply properly.
Quoted 36 lines
drivers/mtd/nand/nand_timings.c new file +30
-0@@ -0,0 +1,248 @@ +/* +* Copyright (C) 2014 Boris BREZILLON <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +*/ +#include <linux/mtd/nand.h> + +static const struct nand_sdr_timings onfi_sdr_timings[] = { +/* Mode 0 */ + { + .tADL_min =200000 , + .tALH_min =20000 , + .tALS_min =50000 , + .tAR_min =25000 , + .tCEA_max =100000 , + .tCEH_min =20000 , + .tCH_min =20000 , + .tCHZ_max =100000 , + .tCLH_min =20000 , + .tCLR_min =20000 , + .tCLS_min =50000 , + .tCOH_min =0 , + .tCS_min =70000 , + .tDH_min =20000 , + .tDS_min =40000 , + .tFEAT_max =1000000 , + .tIR_min =10000 ,
drivers/mtd/nand/nand_timings.c line 40
Quoted 10 lines
drivers/mtd/nand/nand_timings.c new file +10
-0+ .tITC_max = 1000000, + .tRC_min = 100000, + .tREA_max = 40000, + .tREH_min = 30000, + .tRHOH_min = 0, + .tRHW_min = 200000, + .tRHZ_max = 200000, + .tRLOH_min = 0, + .tRP_min = 50000, + .tRST_max = 250000000000,
I was initially wary of potential overflow here, but apparently the C standard (section 6.4.4.1) ensures that literal constants like this will be promoted to a sufficiently-large type.
Quoted 193 lines
drivers/mtd/nand/nand_timings.c new file +193
-0+ .tWB_max = 200000, + .tRR_min = 40000, + .tWC_min = 100000, + .tWH_min = 30000, + .tWHR_min = 120000, + .tWP_min = 50000, + .tWW_min = 100000, + }, +/* Mode 1 */ + { + .tADL_min = 100000, + .tALH_min = 10000, + .tALS_min = 25000, + .tAR_min = 10000, + .tCEA_max = 45000, + .tCEH_min = 20000, + .tCH_min = 10000, + .tCHZ_max = 50000, + .tCLH_min = 10000, + .tCLR_min = 10000, + .tCLS_min = 25000, + .tCOH_min = 15000, + .tCS_min = 35000, + .tDH_min = 10000, + .tDS_min = 20000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 50000, + .tREA_max = 30000, + .tREH_min = 15000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 0, + .tRP_min = 25000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 45000, + .tWH_min = 15000, + .tWHR_min = 80000, + .tWP_min = 25000, + .tWW_min = 100000, + }, + /* Mode 2 */ + { + .tADL_min = 100000, + .tALH_min = 10000, + .tALS_min = 15000, + .tAR_min = 10000, + .tCEA_max = 30000, + .tCEH_min = 20000, + .tCH_min = 10000, + .tCHZ_max = 50000, + .tCLH_min = 10000, + .tCLR_min = 10000, + .tCLS_min = 15000, + .tCOH_min = 15000, + .tCS_min = 25000, + .tDH_min = 5000, + .tDS_min = 15000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 35000, + .tREA_max = 25000, + .tREH_min = 15000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 0, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tRP_min = 17000, + .tWC_min = 35000, + .tWH_min = 15000, + .tWHR_min = 80000, + .tWP_min = 17000, + .tWW_min = 100000, + }, + /* Mode 3 */ + { + .tADL_min = 100000, + .tALH_min = 5000, + .tALS_min = 10000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 50000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 10000, + .tCOH_min = 15000, + .tCS_min = 25000, + .tDH_min = 5000, + .tDS_min = 10000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 30000, + .tREA_max = 20000, + .tREH_min = 10000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 0, + .tRP_min = 15000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 30000, + .tWH_min = 10000, + .tWHR_min = 80000, + .tWP_min = 15000, + .tWW_min = 100000, + }, + /* Mode 4 */ + { + .tADL_min = 70000, + .tALH_min = 5000, + .tALS_min = 10000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 30000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 10000, + .tCOH_min = 15000, + .tCS_min = 20000, + .tDH_min = 5000, + .tDS_min = 10000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 25000, + .tREA_max = 20000, + .tREH_min = 10000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 5000, + .tRP_min = 12000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 25000, + .tWH_min = 10000, + .tWHR_min = 80000, + .tWP_min = 12000, + .tWW_min = 100000, + }, + /* Mode 5 */ + { + .tADL_min = 70000, + .tALH_min = 5000, + .tALS_min = 10000, + .tAR_min = 10000, + .tCEA_max = 25000, + .tCEH_min = 20000, + .tCH_min = 5000, + .tCHZ_max = 30000, + .tCLH_min = 5000, + .tCLR_min = 10000, + .tCLS_min = 10000, + .tCOH_min = 15000, + .tCS_min = 15000, + .tDH_min = 5000, + .tDS_min = 7000, + .tFEAT_max = 1000000, + .tIR_min = 0, + .tITC_max = 1000000, + .tRC_min = 20000, + .tREA_max = 16000, + .tREH_min = 7000, + .tRHOH_min = 15000, + .tRHW_min = 100000, + .tRHZ_max = 100000, + .tRLOH_min = 5000, + .tRP_min = 10000, + .tRR_min = 20000, + .tRST_max = 500000000, + .tWB_max = 100000, + .tWC_min = 20000, + .tWH_min = 7000, + .tWHR_min = 80000, + .tWP_min = 10000, + .tWW_min = 100000, + },
drivers/mtd/nand/nand_timings.c line 243
Quoted 10 lines
drivers/mtd/nand/nand_timings.c new file +10
-0+}; + +/** +* onfi_async_timing_mode_to_sdr_timings - [NAND Interface] Retrieve NAND +* timings according to the given ONFI timing mode +* @mode: ONFI timing mode +*/ +const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings (int mode) +{ +if (mode <0 || mode >ARRAY_SIZE (onfi_sdr_timings))
Should be ">=".
if (mode < 0 || mode >= ARRAY_SIZE(onfi_sdr_timings))
Quoted 10 lines
drivers/mtd/nand/nand_timings.c new file +5
-0+return ERR_PTR (-EINVAL ); + +return &onfi_sdr_timings[mode]; +} +EXPORT_SYMBOL (onfi_async_timing_mode_to_sdr_timings);include/linux/mtd/nand.h +0
-0
include/linux/mtd/nand.h static inline bool nand_is_slc(struct nand_chip *chip) · line 850
Quoted 4 lines
include/linux/mtd/nand.h +1
-0@@ -847,6 +847,7 @@ static inline bool nand_is_slc(struct nand_chip *chip)return chip->bits_per_cell ==1 ; } +
Unnecessary whitespace change.
include/linux/mtd/nand.h struct nand_sdr_timings { · line 902
Quoted 11 lines
include/linux/mtd/nand.h +3
-0/** * struct nand_sdr_timings - SDR NAND chip timings * @@ -895,4 +896,7 @@ struct nand_sdr_timings { u32 tWW_min; }; +/* convert an ONFI timing mode to its timing characteristics. */ +const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings (int mode); +#endif /* __LINUX_MTD_NAND_H */
Brian
Re: [PATCH v3 2/9] mtd: nand: add ONFI timing mode to nand_timings converter
In reply to Boris BREZILLONHi Boris,
Since I think you were planning on revisiting this soon, I have one more comment:
drivers/mtd/nand/Makefile line 5
Quoted 10 lines · Boris BREZILLON
drivers/mtd/nand/Makefile +1
-1@@ -2,7 +2,7 @@ # linux/drivers/nand/Makefile # -obj-$(CONFIG_MTD_NAND) += nand.o +obj-$(CONFIG_MTD_NAND) += nand.o nand_timings.o
This is not the right place, as it will generate a new module 'nand_timings' (nand_timings.ko, if CONFIG_MTD_NAND=m). You probably want to just add nand_timings.o to the 'nand-objs' list at the bottom of the Makefile, like this:
nand-objs := nand_base.o nand_bbt.o nand_timings.o
drivers/mtd/nand/Makefile line 8
Quoted 3 lines
drivers/mtd/nand/Makefile +0
-0obj-$(CONFIG_MTD_NAND_ECC) += nand_ecc.o obj-$(CONFIG_MTD_NAND_BCH) += nand_bch.o obj-$(CONFIG_MTD_NAND_IDS) += nand_ids.o nand_hynix.o
Thanks,
Brian
PATCH v3 3/9 of: mtd: add NAND timing mode retrieval support
In reply to Boris BREZILLONAdd a function to retrieve NAND timing mode (ONFI timing mode) from a given DT node.
--- drivers/of/of_mtd.c | 19 +++++++++++++++++++ include/linux/of_mtd.h | 8 ++++++++ 2 files changed, 27 insertions(+)
drivers/of/of_mtd.c +19 -0
@@ -117,3 +117,22 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)return of_property_read_bool (np,"nand-on-flash-bbt" ); }EXPORT_SYMBOL_GPL (of_get_nand_on_flash_bbt); + +/** +* of_get_nand_timings - Get nand timings for the given device_node +* @np: Pointer to the given device_node +* +* return 0 on success errno other wise +*/ +int of_get_nand_onfi_timing_mode (struct device_node *np) +{ +int err; +u32 mode; + + err =of_property_read_u32 (np,"onfi,nand-timing-mode" , &mode); +if (err) +return err; + +return mode; +} +EXPORT_SYMBOL_GPL (of_get_nand_onfi_timing_mode);
include/linux/of_mtd.h +8 -0
@@ -9,6 +9,8 @@#ifndef __LINUX_OF_MTD_H#define __LINUX_OF_MTD_H +#include <linux/mtd/nand.h> +#ifdef CONFIG_OF_MTD #include <linux/of.h>
@@ -17,6 +19,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);int of_get_nand_ecc_strength (struct device_node *np);int of_get_nand_bus_width (struct device_node *np);bool of_get_nand_on_flash_bbt (struct device_node *np); +int of_get_nand_onfi_timing_mode (struct device_node *np);#else /* CONFIG_OF_MTD */
@@ -45,6 +48,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)return false; } +static inline int of_get_nand_onfi_timing_mode (struct device_node *np) +{ +return -ENOSYS ; +} +#endif /* CONFIG_OF_MTD */ #endif /* __LINUX_OF_MTD_H */
Signature · 1 line
1.7.9.5
Re: [PATCH v3 3/9] of: mtd: add NAND timing mode retrieval support
Quoted 24 lines · Boris BREZILLON
Add a function to retrieve NAND timing mode (ONFI timing mode) from a given DT node.
Signed-off-by Boris BREZILLON <[email protected]>--- drivers/of/of_mtd.c | 19 +++++++++++++++++++ include/linux/of_mtd.h | 8 ++++++++ 2 files changed, 27 insertions(+)drivers/of/of_mtd.c +7
-0@@ -117,3 +117,22 @@ bool of_get_nand_on_flash_bbt(struct device_node *np)return of_property_read_bool (np,"nand-on-flash-bbt" ); }EXPORT_SYMBOL_GPL (of_get_nand_on_flash_bbt); + +/** +* of_get_nand_timings - Get nand timings for the given device_node +* @np: Pointer to the given device_node +* +* return 0 on success errno other wise +*/
drivers/of/of_mtd.c bool of_get_nand_on_flash_bbt(struct device_node *np) · line 136
Quoted 10 lines
drivers/of/of_mtd.c +10
-0+int of_get_nand_onfi_timing_mode (struct device_node *np) +{ +int err; +u32 mode; + + err =of_property_read_u32 (np,"onfi,nand-timing-mode" , &mode); +if (err) +return err; + +return mode;
To fit the style of the rest of this file (and to save some lines) this might as well be:
return ret ? ret : mode;
include/linux/of_mtd.h line 13
Quoted 12 lines
drivers/of/of_mtd.c +2
-0+} +EXPORT_SYMBOL_GPL (of_get_nand_onfi_timing_mode);include/linux/of_mtd.h +2
-0@@ -9,6 +9,8 @@#ifndef __LINUX_OF_MTD_H#define __LINUX_OF_MTD_H +#include <linux/mtd/nand.h> +
What's this header used for here? I think you can drop it.
Quoted 13 lines
include/linux/of_mtd.h +1
-0#ifdef CONFIG_OF_MTD #include <linux/of.h> @@ -17,6 +19,7 @@ int of_get_nand_ecc_step_size(struct device_node *np);int of_get_nand_ecc_strength (struct device_node *np);int of_get_nand_bus_width (struct device_node *np);bool of_get_nand_on_flash_bbt (struct device_node *np); +int of_get_nand_onfi_timing_mode (struct device_node *np);#else /* CONFIG_OF_MTD */ @@ -45,6 +48,11 @@ static inline bool of_get_nand_on_flash_bbt(struct device_node *np)return false;
include/linux/of_mtd.h static inline bool of_get_nand_on_flash_bbt(struct device_node *np) · line 58
Quoted 10 lines
include/linux/of_mtd.h +5
-0} +static inline int of_get_nand_onfi_timing_mode (struct device_node *np) +{ +return -ENOSYS ; +} +#endif /* CONFIG_OF_MTD */ #endif /* __LINUX_OF_MTD_H */
Brian
PATCH v3 4/9 of: mtd: add documentation for the ONFI NAND timing mode property
In reply to Boris BREZILLONAdd documentation for the ONFI NAND timing mode property.
--- Documentation/devicetree/bindings/mtd/nand.txt | 8 ++++++++ 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/mtd/nand.txt +8 -0
@@ -19,3 +19,11 @@ errors per {size} bytes". The interpretation of these parameters is implementation-defined, so not all implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support. + +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document: + www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
Signature · 1 line
1.7.9.5
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Quoted 1 line · Boris BREZILLON
Add documentation for the ONFI NAND timing mode property.
I don?t see a Toggle/JEDEC mode timing property. Will that be defined for Toshiba, Samsung and San Disk flash? Or will this be limited to Micron, Intel and Hynix (the only ones supporting ONFI)?
Warner
Quoted 28 lines
Signed-off-by Boris BREZILLON <[email protected]>---
Documentation/devicetree/bindings/mtd/nand.txt| 8 ++++++++1 file changed, 8 insertions(+)Documentation/devicetree/bindings/mtd/nand.txt +0
-0@@ -19,3 +19,11 @@ errors per {size} bytes".The interpretation of these parameters is implementation-defined, so not all implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support.
Documentation/devicetree/bindings/mtd/nand.txt +8
-0+ +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document: + www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdfSignature · 6 lines
1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Le 12/03/2014 19:27, Warner Losh a ?crit :
Quoted 6 lines · Boris BREZILLON
Quoted 2 lines
Add documentation for the ONFI NAND timing mode property.
I don?t see a Toggle/JEDEC mode timing property. Will that be defined for Toshiba, Samsung and San Disk flash? Or will this be limited to Micron, Intel and Hynix (the only ones supporting ONFI)?
There is currently no Toggle/JEDEC timing mode support, and I don't know what these
timing modes describe.
But I guess they can be converted to standard timings described in nand_sdr_timings
struct (or in your case in the future nand_ddr_timings struct).
Could you check that the timings described by these modes use the same naming convention
as those defined in the ONFI specification (www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
chapter 4.15.4) ?
If this is the case, we can define a new DT property and new converters.
Best Regards,
Boris
Quoted 31 lines
Warner
Quoted 28 lines
Signed-off-by Boris BREZILLON <[email protected]>---
Documentation/devicetree/bindings/mtd/nand.txt| 8 ++++++++1 file changed, 8 insertions(+)Documentation/devicetree/bindings/mtd/nand.txt +0
-0@@ -19,3 +19,11 @@ errors per {size} bytes".The interpretation of these parameters is implementation-defined, so not all implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support.
Documentation/devicetree/bindings/mtd/nand.txt +8
-0+ +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document: + www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdfSignature · 6 lines
1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
In reply to Boris BREZILLONHi Boris,
Quoted 13 lines · Boris BREZILLON
Add documentation for the ONFI NAND timing mode property.
Signed-off-by Boris BREZILLON <[email protected]>--- Documentation/devicetree/bindings/mtd/nand.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/mtd/nand.txt +0
-0@@ -19,3 +19,11 @@ errors per {size} bytes". The interpretation of these parameters is implementation-defined, so not all
Documentation/devicetree/bindings/mtd/nand.txt errors per {size} bytes". · line 29
Quoted 10 lines
Documentation/devicetree/bindings/mtd/nand.txt +8
-0implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support. + +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document: + www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
I'm not 100% convinced this property should go in the device tree. With most other flash properties (device size, page size, and even minimum ECC requirements), we try to auto-detect these parameters to some extent. ONFI makes it easy for some class of chips, but for others, we typically rely on an in-kernel device ID table or ID decoding heuristic -- we don't require a DT description of every property of the flash. So what makes this property different?
I realize that we may not include device ID entries for every flash that you need in the ID table (although we still are able to detect the important properties accurately, like page and block size). But would it suffice to default these flash to a lowest common timing mode, like mode 0?
If no other option works well, then I am still open to describing the supported timing modes in the DT.
BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash typically support multiple timing modes? And if so, how are we supposed to *change* modes? AFAIK, ONFI provides the only standard for configuring the flash's timing mode. So maybe you're really only wanting a "default timing mode" property that is a single integer, not a bitfield.
Regards,
Brian
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Hi Brian,
Quoted 25 lines · Brian Norris
Hi Boris,
Quoted 22 lines · Boris BREZILLON
Add documentation for the ONFI NAND timing mode property.
Signed-off-by Boris BREZILLON <[email protected]>--- Documentation/devicetree/bindings/mtd/nand.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/mtd/nand.txt +7
-0@@ -19,3 +19,11 @@ errors per {size} bytes". The interpretation of these parameters is implementation-defined, so not all implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support. + +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document:
Quoted 8 lines
Documentation/devicetree/bindings/mtd/nand.txt errors per {size} bytes". · line 29
Quoted 1 line
Documentation/devicetree/bindings/mtd/nand.txt +1
-0+ www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdfI'm not 100% convinced this property should go in the device tree. With most other flash properties (device size, page size, and even minimum ECC requirements), we try to auto-detect these parameters to some extent. ONFI makes it easy for some class of chips, but for others, we typically rely on an in-kernel device ID table or ID decoding heuristic -- we don't require a DT description of every property of the flash. So what makes this property different?
AFAICT nothing, but the same goes for the ECC requirements, and we've recently added DT bindings to define these requirements.
I'm not telling we should drop these ECC requirements bindings (actually I'm using them :-)), but what's different with the timings requirements ?
Moreover, we will end up with a lot of new entries in the device ID table if we decide to put these informations in this table.
Quoted 6 lines
I realize that we may not include device ID entries for every flash that you need in the ID table (although we still are able to detect the important properties accurately, like page and block size). But would it suffice to default these flash to a lowest common timing mode, like mode 0?
IMHO this is not a good solution: you'll end up with lower perfomances on most of the supported NAND chips and I'm not sure this is what we want.
Quoted 10 lines
If no other option works well, then I am still open to describing the supported timing modes in the DT.
BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash typically support multiple timing modes? And if so, how are we supposed to *change* modes? AFAIK, ONFI provides the only standard for configuring the flash's timing mode. So maybe you're really only wanting a "default timing mode" property that is a single integer, not a bitfield.
Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell me if I'm wrong), it should work because most of the timings are min requirements.
This means, even if you provide slower signals transitions, the NAND will work as expected.
But I can modify the bindings to just encode the maximum supported timing mode.
Thanks for your feedback.
Best Regards,
Boris
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Quoted 4 lines · Boris BREZILLON
AFAICT nothing, but the same goes for the ECC requirements, and we've recently added DT bindings to define these requirements.
I'm not telling we should drop these ECC requirements bindings (actually I'm using them :-)), but what's different with the timings requirements ?
ECC requirements are almost always something that has to be matched to the bootloader (since the bootloader typicaly reads the NAND to boot), so it is sensible to put that in the DT
The timings are a property of the chip, and if they can be detected they should be. IMHO, the main purpose of a DT property would be to lower the speed if, for some reason, the board cannot support the device's full speed.
Quoted 4 lines
Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell me if I'm wrong), it should work because most of the timings are min requirements. This means, even if you provide slower signals transitions, the NAND will work as expected.
IIRC for ONFI a device must always work in the mode 0 timings, without requiring a command?
Jason
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Quoted 9 lines · Jason Gunthorpe
Quoted 4 lines · Boris BREZILLON
AFAICT nothing, but the same goes for the ECC requirements, and we've recently added DT bindings to define these requirements.
I'm not telling we should drop these ECC requirements bindings (actually I'm using them :-)), but what's different with the timings requirements ?ECC requirements are almost always something that has to be matched to the bootloader (since the bootloader typicaly reads the NAND to boot), so it is sensible to put that in the DT
+1 You beat me to this :)
Quoted 4 lines
The timings are a property of the chip, and if they can be detected they should be. IMHO, the main purpose of a DT property would be to lower the speed if, for some reason, the board cannot support the device's full speed.
Agreed.
Now, we still have the open question of whether we can autodetect timing modes easily for non-ONFI chips.
Quoted 7 lines
Quoted 4 lines
Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell me if I'm wrong), it should work because most of the timings are min requirements. This means, even if you provide slower signals transitions, the NAND will work as expected.
IIRC for ONFI a device must always work in the mode 0 timings, without requiring a command?
I believe so.
FYI, despite the name of the binding, we are mostly interested in non-ONFI NAND here.
Brian
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
In reply to Boris BREZILLONQuoted 37 lines · Boris BREZILLON
Hi Brian,
Quoted 25 lines · Brian Norris
Hi Boris,
Quoted 22 lines · Boris BREZILLON
Add documentation for the ONFI NAND timing mode property.
Signed-off-by Boris BREZILLON <[email protected]>--- Documentation/devicetree/bindings/mtd/nand.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/mtd/nand.txt +7
-0@@ -19,3 +19,11 @@ errors per {size} bytes". The interpretation of these parameters is implementation-defined, so not all implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support. + +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document:Quoted 8 lines
Documentation/devicetree/bindings/mtd/nand.txt errors per {size} bytes". · line 29
Quoted 1 line
Documentation/devicetree/bindings/mtd/nand.txt +1
-0+ www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdfI'm not 100% convinced this property should go in the device tree. With most other flash properties (device size, page size, and even minimum ECC requirements), we try to auto-detect these parameters to some extent. ONFI makes it easy for some class of chips, but for others, we typically rely on an in-kernel device ID table or ID decoding heuristic -- we don't require a DT description of every property of the flash. So what makes this property different?
Quoted 4 lines
AFAICT nothing, but the same goes for the ECC requirements, and we've recently added DT bindings to define these requirements.
I'm not telling we should drop these ECC requirements bindings (actually I'm using them :-)), but what's different with the timings requirements ?
ECC selection is not quite as scientific; with ECC, there are external factors that influence the ECC mode that you should use, since any data read/written from Linux has to be compatible with any data read/written with another entity (e.g., bootloader). Note that the ECC bindings do not represent a property of the flash chip itself (i.e., they don't hold the "minimum required ECC strength"), but of the entire flash system (i.e., "what ECC must I use to play nicely with the rest of the world").
With timing modes, this is purely a property of the flash chip, and we do not have to synchronize it with the bootloader. We don't exactly care if a bootloader and Linux use slightly different timing modes.
Quoted 2 lines
Moreover, we will end up with a lot of new entries in the device ID table if we decide to put these informations in this table.
Yes, that could be a problem.
What sort of non-ONFI flash chips do you have that need this property? And what timing mode(s) do they use? Is there, for instance, a pattern such that all Hynix MLC of a certain generation use a particular timing mode?
Quoted 8 lines
Quoted 5 lines
I realize that we may not include device ID entries for every flash that you need in the ID table (although we still are able to detect the important properties accurately, like page and block size). But would it suffice to default these flash to a lowest common timing mode, like mode 0?
IMHO this is not a good solution: you'll end up with lower perfomances on most of the supported NAND chips and I'm not sure this is what we want.
No, we wouldn't want to always use mode 0. But it's possible we can get good enough heuristics for most flash, if we can integrate timing modes into the current extended ID decoding. Not sure.
I'm also concerned here that this kind of binding will be difficult to use properly. A user/developer/board-designer would have to read the datasheet and compare all its values to the ONFI spec to find the closest match, and they would have to do this for each new flash they use. If we can help them by autodetecting this, that would be great.
Quoted 10 lines
Quoted 9 lines
If no other option works well, then I am still open to describing the supported timing modes in the DT.
BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash typically support multiple timing modes? And if so, how are we supposed to *change* modes? AFAIK, ONFI provides the only standard for configuring the flash's timing mode. So maybe you're really only wanting a "default timing mode" property that is a single integer, not a bitfield.
Quoted 5 lines
Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell me if I'm wrong), it should work because most of the timings are min requirements.
This means, even if you provide slower signals transitions, the NAND will work as expected.
So you're saying that even though the chip actually specifies a single set of timings, you would describe this as a bitmask of several supported ONFI timing modes, up to the "max performance"?
Is there ever a case where (for instance) a non-ONFI flash supports the equivalent of timing mode 3, but it does not support mode 2 or 1?
Quoted 2 lines
But I can modify the bindings to just encode the maximum supported timing mode.
AIUI, the non-ONFI datasheets really only specify a single timing mode, so I think we should only specify the "max." And as a bonus, this actually makes the binding easier to use. A driver does not care about how many different modes are supported; it only needs to know what the max is.
Brian
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Quoted 41 lines · Brian Norris
Quoted 36 lines · Boris BREZILLON
Hi Brian,
Quoted 25 lines · Brian Norris
Hi Boris,
Quoted 22 lines · Boris BREZILLON
Add documentation for the ONFI NAND timing mode property.
Signed-off-by Boris BREZILLON <[email protected]>--- Documentation/devicetree/bindings/mtd/nand.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/mtd/nand.txt +7
-0@@ -19,3 +19,11 @@ errors per {size} bytes". The interpretation of these parameters is implementation-defined, so not all implementations must support all possible combinations. However, implementations are encouraged to further specify the value(s) they support. + +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of + the NAND chip. Each supported mode is represented as a bit position (i.e. : + mode 0 and 1 => (1 << 0) | (1 << 1) = 0x3). + This is only used when the chip does not support the ONFI standard. + The last bit set represent the closest mode fulfilling the NAND chip timings. + For a full description of the different timing modes see this document:Quoted 8 lines
Documentation/devicetree/bindings/mtd/nand.txt errors per {size} bytes". · line 29
Quoted 1 line
Documentation/devicetree/bindings/mtd/nand.txt +1
-0+ www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdfI'm not 100% convinced this property should go in the device tree. With most other flash properties (device size, page size, and even minimum ECC requirements), we try to auto-detect these parameters to some extent. ONFI makes it easy for some class of chips, but for others, we typically rely on an in-kernel device ID table or ID decoding heuristic -- we don't require a DT description of every property of the flash. So what makes this property different?
Quoted 4 lines
AFAICT nothing, but the same goes for the ECC requirements, and we've recently added DT bindings to define these requirements.
I'm not telling we should drop these ECC requirements bindings (actually I'm using them :-)), but what's different with the timings requirements ?
Quoted 7 lines
ECC selection is not quite as scientific; with ECC, there are external factors that influence the ECC mode that you should use, since any data read/written from Linux has to be compatible with any data read/written with another entity (e.g., bootloader). Note that the ECC bindings do not represent a property of the flash chip itself (i.e., they don't hold the "minimum required ECC strength"), but of the entire flash system (i.e., "what ECC must I use to play nicely with the rest of the world").
If the ECC bindings don't encode the "minimum required ECC strength" but rather the "ECC config on a specific board" then I guess "minimum required ECC strength" for non-ONFI chips should be defined somewhere else (stored in the device ID table ?).
Actually, in the sunxi NAND controller driver, I'm using the DT defined ECC config when the NAND does not support ONFI timings retrieval.
Quoted 4 lines
With timing modes, this is purely a property of the flash chip, and we do not have to synchronize it with the bootloader. We don't exactly care if a bootloader and Linux use slightly different timing modes.
Agreed.
Quoted 6 lines
Quoted 3 lines
Moreover, we will end up with a lot of new entries in the device ID table if we decide to put these informations in this table.
Yes, that could be a problem.
What sort of non-ONFI flash chips do you have that need this property?
I only have the Hynix one defined in my patch series. Other people tested my driver on different boards but I don't recall exactly which NAND they had (a samsung one IIRC).
Quoted 3 lines
And what timing mode(s) do they use? Is there, for instance, a pattern such that all Hynix MLC of a certain generation use a particular timing mode?
I'll take a look.
Quoted 34 lines
Quoted 8 lines
Quoted 6 lines
I realize that we may not include device ID entries for every flash that you need in the ID table (although we still are able to detect the important properties accurately, like page and block size). But would it suffice to default these flash to a lowest common timing mode, like mode 0?
IMHO this is not a good solution: you'll end up with lower perfomances on most of the supported NAND chips and I'm not sure this is what we want.
No, we wouldn't want to always use mode 0. But it's possible we can get good enough heuristics for most flash, if we can integrate timing modes into the current extended ID decoding. Not sure.
I'm also concerned here that this kind of binding will be difficult to use properly. A user/developer/board-designer would have to read the datasheet and compare all its values to the ONFI spec to find the closest match, and they would have to do this for each new flash they use. If we can help them by autodetecting this, that would be great.
Quoted 9 lines
Quoted 9 lines
If no other option works well, then I am still open to describing the supported timing modes in the DT.
BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash typically support multiple timing modes? And if so, how are we supposed to *change* modes? AFAIK, ONFI provides the only standard for configuring the flash's timing mode. So maybe you're really only wanting a "default timing mode" property that is a single integer, not a bitfield.
Quoted 5 lines
Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell me if I'm wrong), it should work because most of the timings are min requirements.
This means, even if you provide slower signals transitions, the NAND will work as expected.
Quoted 6 lines
So you're saying that even though the chip actually specifies a single set of timings, you would describe this as a bitmask of several supported ONFI timing modes, up to the "max performance"?
Is there ever a case where (for instance) a non-ONFI flash supports the equivalent of timing mode 3, but it does not support mode 2 or 1?
I don't think so.
Quoted 7 lines
Quoted 2 lines
But I can modify the bindings to just encode the maximum supported timing mode.
AIUI, the non-ONFI datasheets really only specify a single timing mode, so I think we should only specify the "max." And as a bonus, this actually makes the binding easier to use. A driver does not care about how many different modes are supported; it only needs to know what the max is.
Agreed, actually my first binding was defining it this way.
Quoted 2 lines
Brian
Signature · 3 lines
Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property
Hi Boris,
Looking back at this thread, there's at least one or two things I forgot to answer. Sorry.
Quoted 1 line · Boris BREZILLON
On 20/05/2014 21:52, Brian Norris wrote:
[...]
Quoted 4 lines
If the ECC bindings don't encode the "minimum required ECC strength" but rather the "ECC config on a specific board" then I guess "minimum required ECC strength" for non-ONFI chips should be defined somewhere else (stored in the device ID table ?).
They are. See nand_flash_dev::ecc, which holds fields for ecc_strength_ds and step_ds. If we have to, we can add a "timing mode" field to this struct.
Quoted 8 lines
Quoted 6 lines
So you're saying that even though the chip actually specifies a single set of timings, you would describe this as a bitmask of several supported ONFI timing modes, up to the "max performance"?
Is there ever a case where (for instance) a non-ONFI flash supports the equivalent of timing mode 3, but it does not support mode 2 or 1?
I don't think so.
OK, then I don't think the mask approach is necessary, if we do ever settle on using a DT binding here. (I hope we can avoid this.)
Quoted 9 lines
Quoted 7 lines
Quoted 2 lines
But I can modify the bindings to just encode the maximum supported timing mode.
AIUI, the non-ONFI datasheets really only specify a single timing mode, so I think we should only specify the "max." And as a bonus, this actually makes the binding easier to use. A driver does not care about how many different modes are supported; it only needs to know what the max is.
Agreed, actually my first binding was defining it this way.
Was there a good reason for changing it?
Thanks,
Brian
PATCH v3 5/9 mtd: nand: add sunxi NAND flash controller support
In reply to Boris BREZILLONAdd support for the sunxi NAND Flash Controller (NFC).
--- drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/sunxi_nand.c | 1276 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1283 insertions(+) create mode 100644 drivers/mtd/nand/sunxi_nand.c
drivers/mtd/nand/Kconfig +6 -0
@@ -510,4 +510,10 @@ config MTD_NAND_XWAY Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached to the External Bus Unit (EBU). +config MTD_NAND_SUNXI + tristate "Support for NAND on Allwinner SoCs" + depends on ARCH_SUNXI + help + Enables support for NAND Flash chips on Allwinner SoCs. + endif # MTD_NAND
drivers/mtd/nand/Makefile +1 -0
@@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ +obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o nand-objs := nand_base.o nand_bbt.o
drivers/mtd/nand/sunxi_nand.c new file +1276 -0
1,276 lines not on this page · raw message
Signature · 1 line
1.7.9.5
Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support
Hello Boris,
Sorry for the review delay.
Emilio, if you have hardware to test this, it would be nice to give Boris some Tested-by?
Quoted 101 lines · Boris BREZILLON
Add support for the sunxi NAND Flash Controller (NFC).
Signed-off-by Boris BREZILLON <[email protected]>--- drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/sunxi_nand.c | 1276 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1283 insertions(+) create mode 100644 drivers/mtd/nand/sunxi_nand.cdrivers/mtd/nand/Kconfig +6
-0@@ -510,4 +510,10 @@ config MTD_NAND_XWAY Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached to the External Bus Unit (EBU). +config MTD_NAND_SUNXI + tristate "Support for NAND on Allwinner SoCs" + depends on ARCH_SUNXI + help + Enables support for NAND Flash chips on Allwinner SoCs. + endif # MTD_NANDdrivers/mtd/nand/Makefile +1
-0@@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ +obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o nand-objs := nand_base.o nand_bbt.odrivers/mtd/nand/sunxi_nand.c new file +59
-0@@ -0,0 +1,1276 @@ +/* +* Copyright (C) 2013 Boris BREZILLON <[email protected]> +* +* Derived from: +* https://github.com/yuq/sunxi-nfc-mtd +* Copyright (C) 2013 Qiang Yu <[email protected]> +* +* https://github.com/hno/Allwinner-Info +* Copyright (C) 2013 Henrik Nordstr?m <Henrik Nordstr?m> +* +* Copyright (C) 2013 Dmitriy B. <[email protected]> +* Copyright (C) 2013 Sergey Lapin <[email protected]> +* +* 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. +*/ + +#include <linux/dma-mapping.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> +#include <linux/of_mtd.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/partitions.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/dmaengine.h> +#include <linux/gpio.h> +#include <linux/interrupt.h> +#include <linux/io.h> + +#define NFC_REG_CTL 0x0000 +#define NFC_REG_ST 0x0004 +#define NFC_REG_INT 0x0008 +#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_CNT 0x0020 +#define NFC_REG_CMD 0x0024 +#define NFC_REG_RCMD_SET 0x0028 +#define NFC_REG_WCMD_SET 0x002C +#define NFC_REG_IO_DATA 0x0030 +#define NFC_REG_ECC_CTL 0x0034 +#define NFC_REG_ECC_ST 0x0038 +#define NFC_REG_DEBUG 0x003C
drivers/mtd/nand/sunxi_nand.c line 69
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+#define NFC_REG_ECC_CNT0 0x0040 +#define NFC_REG_ECC_CNT1 0x0044 +#define NFC_REG_ECC_CNT2 0x0048 +#define NFC_REG_ECC_CNT3 0x004c +#define NFC_REG_USER_DATA_BASE 0x0050 +#define NFC_REG_SPARE_AREA 0x00A0 +#define NFC_RAM0_BASE 0x0400 +#define NFC_RAM1_BASE 0x0800 + +/*define bit use in NFC_CTL*/
nit: Use BIT() for these?
Quoted 64 lines
drivers/mtd/nand/sunxi_nand.c new file +64
-0+#define NFC_EN (1 << 0) +#define NFC_RESET (1 << 1) +#define NFC_BUS_WIDYH (1 << 2) +#define NFC_RB_SEL (1 << 3) +#define NFC_CE_SEL (7 << 24) +#define NFC_CE_CTL (1 << 6) +#define NFC_CE_CTL1 (1 << 7) +#define NFC_PAGE_SIZE (0xf << 8) +#define NFC_SAM (1 << 12) +#define NFC_RAM_METHOD (1 << 14) +#define NFC_DEBUG_CTL (1 << 31) + +/*define bit use in NFC_ST*/ +#define NFC_RB_B2R (1 << 0) +#define NFC_CMD_INT_FLAG (1 << 1) +#define NFC_DMA_INT_FLAG (1 << 2) +#define NFC_CMD_FIFO_STATUS (1 << 3) +#define NFC_STA (1 << 4) +#define NFC_NATCH_INT_FLAG (1 << 5) +#define NFC_RB_STATE0 (1 << 8) +#define NFC_RB_STATE1 (1 << 9) +#define NFC_RB_STATE2 (1 << 10) +#define NFC_RB_STATE3 (1 << 11) + +/*define bit use in NFC_INT*/ +#define NFC_B2R_INT_ENABLE (1 << 0) +#define NFC_CMD_INT_ENABLE (1 << 1) +#define NFC_DMA_INT_ENABLE (1 << 2) +#define NFC_INT_MASK (NFC_B2R_INT_ENABLE | \ + NFC_CMD_INT_ENABLE | \ + NFC_DMA_INT_ENABLE) + + +/*define bit use in NFC_CMD*/ +#define NFC_CMD_LOW_BYTE (0xff << 0) +#define NFC_CMD_HIGH_BYTE (0xff << 8) +#define NFC_ADR_NUM (0x7 << 16) +#define NFC_SEND_ADR (1 << 19) +#define NFC_ACCESS_DIR (1 << 20) +#define NFC_DATA_TRANS (1 << 21) +#define NFC_SEND_CMD1 (1 << 22) +#define NFC_WAIT_FLAG (1 << 23) +#define NFC_SEND_CMD2 (1 << 24) +#define NFC_SEQ (1 << 25) +#define NFC_DATA_SWAP_METHOD (1 << 26) +#define NFC_ROW_AUTO_INC (1 << 27) +#define NFC_SEND_CMD3 (1 << 28) +#define NFC_SEND_CMD4 (1 << 29) +#define NFC_CMD_TYPE (3 << 30) + +/* define bit use in NFC_RCMD_SET*/ +#define NFC_READ_CMD (0xff << 0) +#define NFC_RANDOM_READ_CMD0 (0xff << 8) +#define NFC_RANDOM_READ_CMD1 (0xff << 16) + +/*define bit use in NFC_WCMD_SET*/ +#define NFC_PROGRAM_CMD (0xff << 0) +#define NFC_RANDOM_WRITE_CMD (0xff << 8) +#define NFC_READ_CMD0 (0xff << 16) +#define NFC_READ_CMD1 (0xff << 24) + +/*define bit use in NFC_ECC_CTL*/ +#define NFC_ECC_EN (1 << 0) +#define NFC_ECC_PIPELINE (1 << 3)
drivers/mtd/nand/sunxi_nand.c line 143
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+#define NFC_ECC_EXCEPTION (1 << 4) +#define NFC_ECC_BLOCK_SIZE (1 << 5) +#define NFC_RANDOM_EN (1 << 9) +#define NFC_RANDOM_DIRECTION (1 << 10) +#define NFC_ECC_MODE_SHIFT 12 +#define NFC_ECC_MODE (0xf << NFC_ECC_MODE_SHIFT) +#define NFC_RANDOM_SEED (0x7fff << 16) + + +
Kill and avoid these extra empty lines.
drivers/mtd/nand/sunxi_nand.c line 149
Quoted 6 lines
drivers/mtd/nand/sunxi_nand.c new file +6
-0+enum sunxi_nand_rb_type { +RB_NONE , +RB_NATIVE , +RB_GPIO , +}; +
Can you add some documentation about this read-back stuff? I know documenting is not a super-fun activity, but it's a bit tough to review otherwise.
Quoted 20 lines
drivers/mtd/nand/sunxi_nand.c new file +20
-0+struct sunxi_nand_rb { +enum sunxi_nand_rb_type type; +union { +int gpio; +int nativeid; + } info; +}; + +struct sunxi_nand_chip_sel { +u8 cs; +struct sunxi_nand_rb rb; +}; + +#define DEFAULT_NAME_FORMAT "nand@%d" +#define MAX_NAME_SIZE (sizeof("nand@") + 2) + +struct sunxi_nand_hw_ecc { +int mode; +struct nand_ecclayout layout; +};
drivers/mtd/nand/sunxi_nand.c line 179
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ +struct sunxi_nand_chip { +struct list_head node; +struct nand_chip nand; +struct mtd_info mtd; +char default_name[MAX_NAME_SIZE ]; +unsigned long clk_rate; +int selected; +int nsels; +struct sunxi_nand_chip_sel sels[0 ];
Hm... you prepare the whole multiple chip support but you really support a single chip? I'd say drop entirely and support just a single chip, it'll make the driver much cleaner. We can always add the support later, after proper testing.
drivers/mtd/nand/sunxi_nand.c line 190
Quoted 11 lines
drivers/mtd/nand/sunxi_nand.c new file +11
-0+}; + +static inline struct sunxi_nand_chip *to_sunxi_nand (struct nand_chip *nand) +{ + return container_of(nand, struct sunxi_nand_chip, nand); +} + +struct sunxi_nfc { +struct nand_hw_control controller; +void __iomem *regs; +int irq;
You don't seem to need the irq stored anywhere, as you're using devm_request_irq.
Quoted 29 lines
drivers/mtd/nand/sunxi_nand.c new file +29
-0+struct clk *ahb_clk; +struct clk *sclk; +unsigned long assigned_cs; +unsigned long clk_rate; +struct list_head chips; +struct completion complete; +}; + +static inline struct sunxi_nfc *to_sunxi_nfc (struct nand_hw_control *ctrl) +{ + return container_of(ctrl, struct sunxi_nfc, controller); +} + +static irqreturn_t sunxi_nfc_interrupt (int irq,void *dev_id) +{ +struct sunxi_nfc *nfc = dev_id; +u32 st =readl (nfc->regs +NFC_REG_ST ); +u32 ien =readl (nfc->regs +NFC_REG_INT ); + +if (!(ien & st)) +return IRQ_NONE ; + +if ((ien & st) == ien) +complete (&nfc->complete); + +writel (st &NFC_INT_MASK , nfc->regs +NFC_REG_ST ); +writel (~st & ien &NFC_INT_MASK , nfc->regs +NFC_REG_INT ); + +return IRQ_HANDLED ;
drivers/mtd/nand/sunxi_nand.c line 229
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+} + +static int sunxi_nfc_wait_int (struct sunxi_nfc *nfc,u32 flags, +unsigned int timeout_ms) +{ +init_completion (&nfc->complete); + +writel (flags, nfc->regs +NFC_REG_INT ); +if (!timeout_ms) +wait_for_completion (&nfc->complete);
In the same vein as infinite loops (see below), I'd avoid this no-timeout wait. Or maybe you can *guarantee* it'll be always completed?
Quoted 29 lines
drivers/mtd/nand/sunxi_nand.c new file +29
-0+else if (!wait_for_completion_timeout (&nfc->complete, +msecs_to_jiffies (timeout_ms))) +return -ETIMEDOUT ; + +return 0 ; +} + +static int sunxi_nfc_dev_ready (struct mtd_info *mtd) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +struct sunxi_nand_rb *rb; +unsigned long timeo = (sunxi_nand->nand.state ==FL_ERASING ?400 :20 ); +int ret; + +if (sunxi_nand->selected <0 ) +return 0 ; + + rb = &sunxi_nand->sels[sunxi_nand->selected].rb; + +switch (rb->type) { +case RB_NATIVE : + ret = !!(readl (nfc->regs +NFC_REG_ST ) & + (NFC_RB_STATE0 << rb->info.nativeid)); +if (ret) +break ; + +sunxi_nfc_wait_int (nfc,NFC_RB_B2R , timeo);
drivers/mtd/nand/sunxi_nand.c line 268
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ ret = !!(readl (nfc->regs +NFC_REG_ST ) & + (NFC_RB_STATE0 << rb->info.nativeid)); +break ; +case RB_GPIO : + ret =gpio_get_value (rb->info.gpio); +break ; +case RB_NONE : +default : + ret =0 ; +pr_err ("cannot check R/B NAND status!" );
I'd suggest avoiding this kind of pr_err. The user won't know who's saying this. Try using dev_err or pr_fmt.
Quoted 55 lines
drivers/mtd/nand/sunxi_nand.c new file +55
-0+break ; + } + +return ret; +} + +static void sunxi_nfc_select_chip (struct mtd_info *mtd,int chip) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +struct sunxi_nand_chip_sel *sel; +u32 ctl; + +if (chip >0 && chip >= sunxi_nand->nsels) +return ; + +if (chip == sunxi_nand->selected) +return ; + + ctl =readl (nfc->regs +NFC_REG_CTL ) & + ~(NFC_CE_SEL |NFC_RB_SEL |NFC_EN ); + +if (chip >=0 ) { + sel = &sunxi_nand->sels[chip]; + + ctl |= (sel->cs <<24 ) |NFC_EN | + (((nand->page_shift -10 ) &0xf ) <<8 ); +if (sel->rb.type ==RB_NONE ) { + nand->dev_ready =NULL ; + }else { + nand->dev_ready = sunxi_nfc_dev_ready; +if (sel->rb.type ==RB_NATIVE ) + ctl |= (sel->rb.info.nativeid <<3 ); + } + +writel (mtd->writesize, nfc->regs +NFC_REG_SPARE_AREA ); + +if (nfc->clk_rate != sunxi_nand->clk_rate) { +clk_set_rate (nfc->sclk, sunxi_nand->clk_rate); + nfc->clk_rate = sunxi_nand->clk_rate; + } + } + +writel (ctl, nfc->regs +NFC_REG_CTL ); + + sunxi_nand->selected = chip; +} + +static void sunxi_nfc_read_buf (struct mtd_info *mtd,uint8_t *buf,int len) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +int cnt;
drivers/mtd/nand/sunxi_nand.c line 333
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+int offs =0 ; +u32 tmp; + +while (len > offs) { + cnt = len - offs; +if (cnt >1024 ) + cnt =1024 ; + +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ;
How about avoiding the infinite loop?
Quoted 18 lines
drivers/mtd/nand/sunxi_nand.c new file +18
-0+writel (cnt, nfc->regs +NFC_REG_CNT ); + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD ; +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +if (buf) +memcpy_fromio (buf + offs, nfc->regs +NFC_RAM0_BASE , + cnt); + offs += cnt; + } +} + +static void sunxi_nfc_write_buf (struct mtd_info *mtd,const uint8_t *buf, +int len) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +int cnt;
drivers/mtd/nand/sunxi_nand.c line 361
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+int offs =0 ; +u32 tmp; + +while (len > offs) { + cnt = len - offs; +if (cnt >1024 ) + cnt =1024 ; + +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ;
Ditto.
Quoted 20 lines
drivers/mtd/nand/sunxi_nand.c new file +20
-0+writel (cnt, nfc->regs +NFC_REG_CNT ); +memcpy_toio (nfc->regs +NFC_RAM0_BASE , buf + offs, cnt); + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD | +NFC_ACCESS_DIR ; +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); + offs += cnt; + } +} + +static uint8_t sunxi_nfc_read_byte (struct mtd_info *mtd) +{ +uint8_t ret; + +sunxi_nfc_read_buf (mtd, &ret,1 ); + +return ret; +} + +static void sunxi_nfc_cmd_ctrl(struct mtd_info *mtd, int dat,
drivers/mtd/nand/sunxi_nand.c line 391
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ unsigned int ctrl) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +u32 tmp; + +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ; +
Ditto.
Quoted 46 lines
drivers/mtd/nand/sunxi_nand.c new file +46
-0+if (ctrl &NAND_CTRL_CHANGE ) { + tmp =readl (nfc->regs +NFC_REG_CTL ); +if (ctrl &NAND_NCE ) + tmp |=NFC_CE_CTL ; +else + tmp &= ~NFC_CE_CTL ; +writel (tmp, nfc->regs +NFC_REG_CTL ); + } + +if (dat ==NAND_CMD_NONE ) +return ; + +if (ctrl &NAND_CLE ) { +writel (NFC_SEND_CMD1 | dat, nfc->regs +NFC_REG_CMD ); + }else { +writel (dat, nfc->regs +NFC_REG_ADDR_LOW ); +writel (NFC_SEND_ADR , nfc->regs +NFC_REG_CMD ); + } + +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +} + +static int sunxi_nfc_hw_ecc_read_page (struct mtd_info *mtd, +struct nand_chip *chip,uint8_t *buf, +int oob_required,int page) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct nand_ecclayout *layout = ecc->layout; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int steps = mtd->writesize / ecc->size; +unsigned int max_bitflips =0 ; +int offset; +u32 tmp; +int i; +int cnt; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < steps; i++) { +if (i)
drivers/mtd/nand/sunxi_nand.c line 447
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ chip->cmdfunc (mtd,NAND_CMD_RNDOUT , i * ecc->size,-1 ); + + offset = mtd->writesize + layout->eccpos[i * ecc->bytes] -4 ; + + chip->read_buf (mtd,NULL , ecc->size); + + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ; +
Ditto.
Quoted 9 lines
drivers/mtd/nand/sunxi_nand.c new file +9
-0+ tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD | (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +memcpy_fromio (buf + (i * ecc->size), + nfc->regs +NFC_RAM0_BASE , ecc->size); + +if (readl (nfc->regs +NFC_REG_ECC_ST ) &0x1 ) { + mtd->ecc_stats.failed++; + }else {
drivers/mtd/nand/sunxi_nand.c line 466
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ tmp =readl (nfc->regs +NFC_REG_ECC_CNT0 ) &0xff ; + mtd->ecc_stats.corrected += tmp; + max_bitflips = max_t(unsigned int, max_bitflips, tmp); + } + +if (oob_required) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); +while ((readl (nfc->regs +NFC_REG_ST ) & +NFC_CMD_FIFO_STATUS )) + ;
Ditto.
Quoted 57 lines
drivers/mtd/nand/sunxi_nand.c new file +57
-0+ offset -= mtd->writesize; + chip->read_buf (mtd, chip->oob_poi + offset, + ecc->bytes +4 ); + } + } + +if (oob_required) { + cnt = ecc->layout->oobfree[0 ].length -4 ; +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , mtd->writesize, +-1 ); + chip->read_buf (mtd, chip->oob_poi, cnt); + } + } + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~NFC_ECC_EN ; + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +return max_bitflips; +} + +static int sunxi_nfc_hw_ecc_write_page (struct mtd_info *mtd, +struct nand_chip *chip, +const uint8_t *buf,int oob_required) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct nand_ecclayout *layout = ecc->layout; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int offset; +u32 tmp; +int i; +int cnt; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < mtd->writesize / ecc->size; i++) { +if (i) + chip->cmdfunc (mtd,NAND_CMD_RNDIN , i * ecc->size,-1 ); + + chip->write_buf (mtd, buf + (i * ecc->size), ecc->size); + + offset = layout->eccpos[i * ecc->bytes] -4 + mtd->writesize; + +/* Fill OOB data in */ +if (oob_required) { + tmp =0xffffffff ; +memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , &tmp, +4 ); + }else {
drivers/mtd/nand/sunxi_nand.c line 533
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , + chip->oob_poi + offset - mtd->writesize, +4 ); + } + + chip->cmdfunc (mtd,NAND_CMD_RNDIN , offset,-1 ); +while ((readl (nfc->regs +NFC_REG_ST ) & +NFC_CMD_FIFO_STATUS )) + ; +
Ditto.
Quoted 302 lines
drivers/mtd/nand/sunxi_nand.c new file +302
-0+ tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD |NFC_ACCESS_DIR | + (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); + } + +if (oob_required) { + cnt = ecc->layout->oobfree[0 ].length -4 ; +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDIN , mtd->writesize,-1 ); + chip->write_buf (mtd, chip->oob_poi, cnt); + } + } + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_EN |NFC_ECC_PIPELINE ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +return 0 ; +} + +static int sunxi_nfc_hw_syndrome_ecc_read_page (struct mtd_info *mtd, +struct nand_chip *chip, +uint8_t *buf,int oob_required, +int page) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int steps = mtd->writesize / ecc->size; +unsigned int max_bitflips =0 ; +uint8_t *oob = chip->oob_poi; +int offset =0 ; +int cnt; +u32 tmp; +int i; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < steps; i++) { + chip->read_buf (mtd,NULL , ecc->size); + + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD | (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +memcpy_fromio (buf, nfc->regs +NFC_RAM0_BASE , ecc->size); + buf += ecc->size; + offset += ecc->size; + +if (readl (nfc->regs +NFC_REG_ECC_ST ) &0x1 ) { + mtd->ecc_stats.failed++; + }else { + tmp =readl (nfc->regs +NFC_REG_ECC_CNT0 ) &0xff ; + mtd->ecc_stats.corrected += tmp; + max_bitflips = max_t(unsigned int, max_bitflips, tmp); + } + +if (oob_required) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); + chip->read_buf (mtd, oob, ecc->bytes + ecc->prepad); + oob += ecc->bytes + ecc->prepad; + } + + offset += ecc->bytes + ecc->prepad; + } + +if (oob_required) { + cnt = mtd->oobsize - (oob - chip->oob_poi); +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); + chip->read_buf (mtd, oob, cnt); + } + } + +writel (readl (nfc->regs +NFC_REG_ECC_CTL ) & ~NFC_ECC_EN , + nfc->regs +NFC_REG_ECC_CTL ); + +return max_bitflips; +} + +static int sunxi_nfc_hw_syndrome_ecc_write_page (struct mtd_info *mtd, +struct nand_chip *chip, +const uint8_t *buf, +int oob_required) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int steps = mtd->writesize / ecc->size; +uint8_t *oob = chip->oob_poi; +int offset =0 ; +int cnt; +u32 tmp; +int i; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < steps; i++) { + chip->write_buf (mtd, buf + (i * ecc->size), ecc->size); + offset += ecc->size; + +/* Fill OOB data in */ +if (oob_required) { + tmp =0xffffffff ; +memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , &tmp, +4 ); + }else { +memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , oob , +4 ); + } + + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD |NFC_ACCESS_DIR | + (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); + + offset += ecc->bytes + ecc->prepad; + oob += ecc->bytes + ecc->prepad; + } + +if (oob_required) { + cnt = mtd->oobsize - (oob - chip->oob_poi); +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDIN , offset,-1 ); + chip->write_buf (mtd, oob, cnt); + } + } + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_EN |NFC_ECC_PIPELINE ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +return 0 ; +} + +static int sunxi_nand_chip_set_timings (struct sunxi_nand_chip *chip, +const struct nand_sdr_timings *timings) +{ +u32 min_clk_period =0 ; + +/* T1 <=> tCLS */ +if (timings->tCLS_min > min_clk_period) + min_clk_period = timings->tCLS_min; + +/* T2 <=> tCLH */ +if (timings->tCLH_min > min_clk_period) + min_clk_period = timings->tCLH_min; + +/* T3 <=> tCS */ +if (timings->tCS_min > min_clk_period) + min_clk_period = timings->tCS_min; + +/* T4 <=> tCH */ +if (timings->tCH_min > min_clk_period) + min_clk_period = timings->tCH_min; + +/* T5 <=> tWP */ +if (timings->tWP_min > min_clk_period) + min_clk_period = timings->tWP_min; + +/* T6 <=> tWH */ +if (timings->tWH_min > min_clk_period) + min_clk_period = timings->tWH_min; + +/* T7 <=> tALS */ +if (timings->tALS_min > min_clk_period) + min_clk_period = timings->tALS_min; + +/* T8 <=> tDS */ +if (timings->tDS_min > min_clk_period) + min_clk_period = timings->tDS_min; + +/* T9 <=> tDH */ +if (timings->tDH_min > min_clk_period) + min_clk_period = timings->tDH_min; + +/* T10 <=> tRR */ +if (timings->tRR_min > (min_clk_period *3 )) + min_clk_period = (timings->tRR_min +2 ) /3 ; + +/* T11 <=> tALH */ +if (timings->tALH_min > min_clk_period) + min_clk_period = timings->tALH_min; + +/* T12 <=> tRP */ +if (timings->tRP_min > min_clk_period) + min_clk_period = timings->tRP_min; + +/* T13 <=> tREH */ +if (timings->tREH_min > min_clk_period) + min_clk_period = timings->tREH_min; + +/* T14 <=> tRC */ +if (timings->tRC_min > (min_clk_period *2 )) + min_clk_period = (timings->tRC_min +1 ) /2 ; + +/* T15 <=> tWC */ +if (timings->tWC_min > (min_clk_period *2 )) + min_clk_period = (timings->tWC_min +1 ) /2 ; + + +/* min_clk_period = (NAND-clk-period * 2) */ +if (min_clk_period <1000 ) + min_clk_period =1000 ; + + min_clk_period /=1000 ; + chip->clk_rate = (2 *1000000000 ) / min_clk_period; + +/* TODO: configure T16-T19 */ + +return 0 ; +} + +static int sunxi_nand_chip_init_timings (struct sunxi_nand_chip *chip, +struct device_node *np) +{ +const struct nand_sdr_timings *timings; +int ret; +int mode; + + mode =onfi_get_async_timing_mode (&chip->nand); +if (mode ==ONFI_TIMING_MODE_UNKNOWN ) { + mode =of_get_nand_onfi_timing_mode (np); +if (mode <0 ) + mode =1 ; + + mode =fls (mode) -1 ; +if (mode <0 ) + mode =0 ; + }else { +uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN ] = {}; + mode =fls (mode) -1 ; +if (mode <0 ) + mode =0 ; + + feature[0 ] = mode; + ret = chip->nand.onfi_set_features (&chip->mtd, &chip->nand, +ONFI_FEATURE_ADDR_TIMING_MODE , + feature); +if (ret) +return ret; + } + + timings =onfi_async_timing_mode_to_sdr_timings (mode); +if (IS_ERR (timings)) +return PTR_ERR (timings); + +return sunxi_nand_chip_set_timings (chip, timings); +} + +static int sunxi_nand_hw_common_ecc_ctrl_init (struct mtd_info *mtd, +struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct sunxi_nand_hw_ecc *data; +struct nand_ecclayout *layout; +int nsectors; +int ret; + +if (!ecc->strength || !ecc->size) +return -EINVAL ; + + data =kzalloc (sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + +/* Add ECC info retrieval from DT */ +if (ecc->strength <=16 ) { + ecc->strength =16 ; + data->mode =0 ; + }else if (ecc->strength <=24 ) { + ecc->strength =24 ; + data->mode =1 ; + }else if (ecc->strength <=28 ) { + ecc->strength =28 ; + data->mode =2 ; + }else if (ecc->strength <=32 ) { + ecc->strength =32 ; + data->mode =3 ; + }else if (ecc->strength <=40 ) { + ecc->strength =40 ; + data->mode =4 ; + }else if (ecc->strength <=48 ) { + ecc->strength =48 ; + data->mode =5 ; + }else if (ecc->strength <=56 ) { + ecc->strength =56 ; + data->mode =6 ; + }else if (ecc->strength <=60 ) { + ecc->strength =60 ;
drivers/mtd/nand/sunxi_nand.c line 842
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+ data->mode =7 ; + }else if (ecc->strength <=64 ) { + ecc->strength =64 ; + data->mode =8 ; + }else { +pr_err ("unsupported strength\n" ); +return -ENOTSUPP ;
You're leaking the 'data' allocated above in here.
Quoted 130 lines
drivers/mtd/nand/sunxi_nand.c new file +130
-0+ } + +/* HW ECC always request ECC bytes for 1024 bytes blocks */ + ecc->bytes = ((ecc->strength *fls (8 *1024 )) +7 ) /8 ; + +/* HW ECC always work with even numbers of ECC bytes */ +if (ecc->bytes %2 ) + ecc->bytes++; + + layout = &data->layout; + nsectors = mtd->writesize / ecc->size; + +if (mtd->oobsize < ((ecc->bytes +4 ) * nsectors)) { + ret = -EINVAL ; + goto err; + } + + layout->eccbytes = (ecc->bytes * nsectors); + + ecc->layout = layout; + ecc->priv = data; + +return 0 ; + +err: +kfree (data); + +return ret; +} + +static void sunxi_nand_hw_common_ecc_ctrl_cleanup (struct nand_ecc_ctrl *ecc) +{ +kfree (ecc->priv); +} + +static int sunxi_nand_hw_ecc_ctrl_init (struct mtd_info *mtd, +struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_ecclayout *layout; +int nsectors; +int i, j; +int ret; + + ret =sunxi_nand_hw_common_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; + + ecc->read_page = sunxi_nfc_hw_ecc_read_page; + ecc->write_page = sunxi_nfc_hw_ecc_write_page; + layout = ecc->layout; + nsectors = mtd->writesize / ecc->size; +/* +* The first 2 bytes are used for BB markers. +* We merge the 4 user available bytes from HW ECC with this +* first section, hence why the + 2 operation (- 2 + 4). +*/ + layout->oobfree[0 ].length = mtd->oobsize +2 - + ((ecc->bytes +4 ) * nsectors); + layout->oobfree[0 ].offset =2 ; +for (i =0 ; i < nsectors; i++) { +/* +* The first 4 ECC block bytes are already counted in the first +* oobfree entry. +*/ +if (i) { + layout->oobfree[i].offset = + layout->oobfree[i -1 ].offset + + layout->oobfree[i -1 ].length + + ecc->bytes; + layout->oobfree[i].length =4 ; + } + +for (j =0 ; j < ecc->bytes; j++) + layout->eccpos[(ecc->bytes * i) + j] = + layout->oobfree[i].offset + + layout->oobfree[i].length + j; + } + +return 0 ; +} + +static int sunxi_nand_hw_syndrome_ecc_ctrl_init (struct mtd_info *mtd, +struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_ecclayout *layout; +int nsectors; +int i; +int ret; + + ret =sunxi_nand_hw_common_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; + + ecc->prepad =4 ; + ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page; + ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page; + + layout = ecc->layout; + nsectors = mtd->writesize / ecc->size; + +for (i =0 ; i < (ecc->bytes * nsectors); i++) + layout->eccpos[i] = i; + + layout->oobfree[0 ].length = mtd->oobsize - i; + layout->oobfree[0 ].offset = i; + +return 0 ; +} + +static void sunxi_nand_ecc_cleanup (struct nand_ecc_ctrl *ecc) +{ +switch (ecc->mode) { +case NAND_ECC_HW : +case NAND_ECC_HW_SYNDROME : +sunxi_nand_hw_common_ecc_ctrl_cleanup (ecc); +break ; +default : +break ; + } +} + +static int sunxi_nand_ecc_init (struct mtd_info *mtd,struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_chip *nand = mtd->priv; +int ecc_step_size, ecc_strength; +int ret; +
drivers/mtd/nand/sunxi_nand.c line 982
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ ecc_step_size =of_get_nand_ecc_step_size (np); + ecc_strength =of_get_nand_ecc_strength (np); +if (ecc_step_size >0 && ecc_strength >0 ) { + ecc->size = ecc_step_size; + ecc->strength = ecc_strength; + }else { + ecc->size = nand->ecc_step_ds; + ecc->strength = nand->ecc_strength_ds; + } +
Shouldn't you check the devicetree value is not weaker than the ONFI-obtained?
Quoted 132 lines
drivers/mtd/nand/sunxi_nand.c new file +132
-0+ ecc->mode =of_get_nand_ecc_mode (np); +switch (ecc->mode) { +case NAND_ECC_SOFT_BCH : +if (!ecc->size || !ecc->strength) +return -EINVAL ; + ecc->bytes = ((ecc->strength *fls (8 * ecc->size)) +7 ) /8 ; +break ; +case NAND_ECC_HW : + ret =sunxi_nand_hw_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; +break ; +case NAND_ECC_HW_SYNDROME : + ret =sunxi_nand_hw_syndrome_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; +break ; +case NAND_ECC_NONE : +case NAND_ECC_SOFT : +break ; +default : +return -EINVAL ; + } + +return 0 ; +} + +static int sunxi_nand_chip_init (struct device *dev,struct sunxi_nfc *nfc, +struct device_node *np) +{ +const struct nand_sdr_timings *timings; +struct sunxi_nand_chip *chip; +struct mtd_part_parser_data ppdata; +struct mtd_info *mtd; +struct nand_chip *nand; +int nsels; +int ret; +int i; +u32 tmp; + +if (!of_get_property (np,"reg" , &nsels)) +return -EINVAL ; + + nsels /=sizeof (u32); +if (!nsels) +return -EINVAL ; + + chip =devm_kzalloc (dev, +sizeof (*chip) + + (nsels *sizeof (struct sunxi_nand_chip_sel )), +GFP_KERNEL ); +if (!chip) +return -ENOMEM ; + + chip->nsels = nsels; + chip->selected =-1 ; + +for (i =0 ; i < nsels; i++) { + ret =of_property_read_u32_index (np,"reg" , i, &tmp); +if (ret) +return ret; + +if (tmp >7 ) +return -EINVAL ; + +if (test_and_set_bit (tmp, &nfc->assigned_cs)) +return -EINVAL ; + + chip->sels[i].cs = tmp; + +if (!of_property_read_u32_index (np,"allwinner,rb" , i, &tmp) && + tmp <2 ) { + chip->sels[i].rb.type =RB_NATIVE ; + chip->sels[i].rb.info.nativeid = tmp; + }else { + ret =of_get_named_gpio (np,"rb-gpios" , i); +if (ret >=0 ) { + tmp = ret; + chip->sels[i].rb.type =RB_GPIO ; + chip->sels[i].rb.info.gpio = tmp; + ret =devm_gpio_request (dev, tmp,"nand-rb" ); +if (ret) +return ret; + + ret =gpio_direction_input (tmp); +if (ret) +return ret; + }else { + chip->sels[i].rb.type =RB_NONE ; + } + } + } + + timings =onfi_async_timing_mode_to_sdr_timings (0 ); +if (IS_ERR (timings)) +return PTR_ERR (timings); + + ret =sunxi_nand_chip_set_timings (chip, timings); + + nand = &chip->nand; +/* Default tR value specified in the ONFI spec (chapter 4.15.1) */ + nand->chip_delay =200 ; + nand->controller = &nfc->controller; + nand->select_chip = sunxi_nfc_select_chip; + nand->cmd_ctrl = sunxi_nfc_cmd_ctrl; + nand->read_buf = sunxi_nfc_read_buf; + nand->write_buf = sunxi_nfc_write_buf; + nand->read_byte = sunxi_nfc_read_byte; + +if (of_get_nand_on_flash_bbt (np)) + nand->bbt_options |=NAND_BBT_USE_FLASH ; + + mtd = &chip->mtd; + mtd->dev.parent = dev; + mtd->priv = nand; + mtd->owner =THIS_MODULE ; + + ret =nand_scan_ident (mtd, nsels,NULL ); +if (ret) +return ret; + + ret =sunxi_nand_chip_init_timings (chip, np); +if (ret) +return ret; + + ret =sunxi_nand_ecc_init (mtd, &nand->ecc, np); +if (ret) +return ret; + + ret =nand_scan_tail (mtd); +if (ret) +return ret;
drivers/mtd/nand/sunxi_nand.c line 1124
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ +if (of_property_read_string (np,"nand-name" , &mtd->name)) { +snprintf (chip->default_name,MAX_NAME_SIZE , +DEFAULT_NAME_FORMAT , chip->sels[i].cs); + mtd->name = chip->default_name; + } + + ppdata.of_node = np; + ret =mtd_device_parse_register (mtd,NULL , &ppdata,NULL ,0 ); +if (!ret)
Aren't you leaking the BBT resources allocated in nand_scan_tail? I **think** you need to call nand_release if the mtd device register fails.
Quoted 38 lines
drivers/mtd/nand/sunxi_nand.c new file +38
-0+return ret; + +list_add_tail (&chip->node, &nfc->chips); + +return 0 ; +} + +static int sunxi_nand_chips_init (struct device *dev,struct sunxi_nfc *nfc) +{ +struct device_node *np = dev->of_node; +struct device_node *nand_np; +int nchips =of_get_child_count (np); +int ret; + +if (nchips >8 ) +return -EINVAL ; + +for_each_child_of_node (np, nand_np) { + ret =sunxi_nand_chip_init (dev, nfc, nand_np); +if (ret) +return ret; + } + +return 0 ; +} + +static void sunxi_nand_chips_cleanup (struct sunxi_nfc *nfc) +{ +struct sunxi_nand_chip *chip; + +while (!list_empty (&nfc->chips)) { + chip = list_first_entry(&nfc->chips, struct sunxi_nand_chip, + node); +nand_release (&chip->mtd); +sunxi_nand_ecc_cleanup (&chip->nand.ecc); + } +} +
drivers/mtd/nand/sunxi_nand.c line 1172
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+static int sunxi_nfc_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct resource *r; +struct sunxi_nfc *nfc; +int ret; + + nfc =devm_kzalloc (dev,sizeof (*nfc),GFP_KERNEL ); +if (!nfc) { +dev_err (dev,"failed to allocate NFC struct\n" );
OOM message not needed.
drivers/mtd/nand/sunxi_nand.c line 1183
Quoted 11 lines
drivers/mtd/nand/sunxi_nand.c new file +11
-0+return -ENOMEM ; + } + +spin_lock_init (&nfc->controller.lock); +init_waitqueue_head (&nfc->controller.wq); +INIT_LIST_HEAD (&nfc->chips); + + r =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + nfc->regs =devm_ioremap_resource (dev, r); +if (IS_ERR (nfc->regs)) { +dev_err (dev,"failed to remap iomem\n" );
Message not needed, see devm_ioremap_resource() documentation.
Quoted 25 lines
drivers/mtd/nand/sunxi_nand.c new file +25
-0+return PTR_ERR (nfc->regs); + } + + nfc->irq =platform_get_irq (pdev,0 ); +if (nfc->irq <0 ) { +dev_err (dev,"failed to retrieve irq\n" ); +return nfc->irq; + } + + nfc->ahb_clk =devm_clk_get (dev,"ahb_clk" ); +if (IS_ERR (nfc->ahb_clk)) { +dev_err (dev,"failed to retrieve ahb_clk\n" ); +return PTR_ERR (nfc->ahb_clk); + } + + ret =clk_prepare_enable (nfc->ahb_clk); +if (ret) +return ret; + + nfc->sclk =devm_clk_get (dev,"sclk" ); +if (IS_ERR (nfc->sclk)) { +dev_err (dev,"failed to retrieve nand_clk\n" ); + ret =PTR_ERR (nfc->sclk); + goto out_ahb_clk_unprepare; + }
drivers/mtd/nand/sunxi_nand.c line 1218
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ + ret =clk_prepare_enable (nfc->sclk); +if (ret) + goto out_ahb_clk_unprepare; + +/* Reset NFC */ +writel (readl (nfc->regs +NFC_REG_CTL ) |NFC_RESET , + nfc->regs +NFC_REG_CTL ); +while (readl (nfc->regs +NFC_REG_CTL ) &NFC_RESET ) + ;
Again: maybe you should avoid potentially infinite loop, no matter what.
drivers/mtd/nand/sunxi_nand.c line 1229
Quoted 11 lines
drivers/mtd/nand/sunxi_nand.c new file +11
-0+ +writel (0 , nfc->regs +NFC_REG_INT ); + ret =devm_request_irq (dev, nfc->irq, sunxi_nfc_interrupt, +0 ,"sunxi-nand" , nfc); +if (ret) + goto out_sclk_unprepare; + +platform_set_drvdata (pdev, nfc); + +writel (0x100 , nfc->regs +NFC_REG_TIMING_CTL ); +writel (0x7ff , nfc->regs +NFC_REG_TIMING_CFG );
Hm... can you remove the magic numbers?
Quoted 27 lines
drivers/mtd/nand/sunxi_nand.c new file +27
-0+ + ret =sunxi_nand_chips_init (dev, nfc); +if (ret) { +dev_err (dev,"failed to init nand chips\n" ); + goto out_sclk_unprepare; + } + +return 0 ; + +out_sclk_unprepare: +clk_disable_unprepare (nfc->sclk); +out_ahb_clk_unprepare: +clk_disable_unprepare (nfc->ahb_clk); + +return ret; +} + +static int sunxi_nfc_remove (struct platform_device *pdev) +{ +struct sunxi_nfc *nfc =platform_get_drvdata (pdev); + +sunxi_nand_chips_cleanup (nfc); + +return 0 ; +} + +static const struct of_device_id sunxi_nfc_ids[] = {
drivers/mtd/nand/sunxi_nand.c line 1266
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ { .compatible = "allwinner,sun4i-nand" }, + {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, sunxi_nfc_ids); + +static struct platform_driver sunxi_nfc_driver = { + .driver = { + .name ="sunxi_nand" , + .owner =THIS_MODULE , + .of_match_table =of_match_ptr (sunxi_nfc_ids),
Redundant of_match_ptr. See 5576bc7bef2919dd2b185bffb768bf9c0da76788.
I think that's all I can spot, without knowing the hardware details.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support
Quoted 112 lines · Ezequiel Garcia
Hello Boris,
Sorry for the review delay.
Emilio, if you have hardware to test this, it would be nice to give Boris some Tested-by?
Quoted 104 lines · Boris BREZILLON
Add support for the sunxi NAND Flash Controller (NFC).
Signed-off-by Boris BREZILLON <[email protected]>--- drivers/mtd/nand/Kconfig | 6 + drivers/mtd/nand/Makefile | 1 + drivers/mtd/nand/sunxi_nand.c | 1276 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1283 insertions(+) create mode 100644 drivers/mtd/nand/sunxi_nand.cdrivers/mtd/nand/Kconfig +6
-0@@ -510,4 +510,10 @@ config MTD_NAND_XWAY Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached to the External Bus Unit (EBU). +config MTD_NAND_SUNXI + tristate "Support for NAND on Allwinner SoCs" + depends on ARCH_SUNXI + help + Enables support for NAND Flash chips on Allwinner SoCs. + endif # MTD_NANDdrivers/mtd/nand/Makefile +1
-0@@ -49,5 +49,6 @@ obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/ obj-$(CONFIG_MTD_NAND_XWAY) += xway_nand.o obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/ +obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o nand-objs := nand_base.o nand_bbt.odrivers/mtd/nand/sunxi_nand.c new file +62
-0@@ -0,0 +1,1276 @@ +/* +* Copyright (C) 2013 Boris BREZILLON <[email protected]> +* +* Derived from: +* https://github.com/yuq/sunxi-nfc-mtd +* Copyright (C) 2013 Qiang Yu <[email protected]> +* +* https://github.com/hno/Allwinner-Info +* Copyright (C) 2013 Henrik Nordstr?m <Henrik Nordstr?m> +* +* Copyright (C) 2013 Dmitriy B. <[email protected]> +* Copyright (C) 2013 Sergey Lapin <[email protected]> +* +* 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. +*/ + +#include <linux/dma-mapping.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/platform_device.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> +#include <linux/of_mtd.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/partitions.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/dmaengine.h> +#include <linux/gpio.h> +#include <linux/interrupt.h> +#include <linux/io.h> + +#define NFC_REG_CTL 0x0000 +#define NFC_REG_ST 0x0004 +#define NFC_REG_INT 0x0008 +#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_CNT 0x0020 +#define NFC_REG_CMD 0x0024 +#define NFC_REG_RCMD_SET 0x0028 +#define NFC_REG_WCMD_SET 0x002C +#define NFC_REG_IO_DATA 0x0030 +#define NFC_REG_ECC_CTL 0x0034 +#define NFC_REG_ECC_ST 0x0038 +#define NFC_REG_DEBUG 0x003C +#define NFC_REG_ECC_CNT0 0x0040 +#define NFC_REG_ECC_CNT1 0x0044 +#define NFC_REG_ECC_CNT2 0x0048
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 69
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+#define NFC_REG_ECC_CNT3 0x004c +#define NFC_REG_USER_DATA_BASE 0x0050 +#define NFC_REG_SPARE_AREA 0x00A0 +#define NFC_RAM0_BASE 0x0400 +#define NFC_RAM1_BASE 0x0800 + +/*define bit use in NFC_CTL*/ nit: Use
BIT()for these?
Okay.
Quoted 68 lines
Quoted 68 lines
drivers/mtd/nand/sunxi_nand.c new file +67
-0+#define NFC_EN (1 << 0) +#define NFC_RESET (1 << 1) +#define NFC_BUS_WIDYH (1 << 2) +#define NFC_RB_SEL (1 << 3) +#define NFC_CE_SEL (7 << 24) +#define NFC_CE_CTL (1 << 6) +#define NFC_CE_CTL1 (1 << 7) +#define NFC_PAGE_SIZE (0xf << 8) +#define NFC_SAM (1 << 12) +#define NFC_RAM_METHOD (1 << 14) +#define NFC_DEBUG_CTL (1 << 31) + +/*define bit use in NFC_ST*/ +#define NFC_RB_B2R (1 << 0) +#define NFC_CMD_INT_FLAG (1 << 1) +#define NFC_DMA_INT_FLAG (1 << 2) +#define NFC_CMD_FIFO_STATUS (1 << 3) +#define NFC_STA (1 << 4) +#define NFC_NATCH_INT_FLAG (1 << 5) +#define NFC_RB_STATE0 (1 << 8) +#define NFC_RB_STATE1 (1 << 9) +#define NFC_RB_STATE2 (1 << 10) +#define NFC_RB_STATE3 (1 << 11) + +/*define bit use in NFC_INT*/ +#define NFC_B2R_INT_ENABLE (1 << 0) +#define NFC_CMD_INT_ENABLE (1 << 1) +#define NFC_DMA_INT_ENABLE (1 << 2) +#define NFC_INT_MASK (NFC_B2R_INT_ENABLE | \ + NFC_CMD_INT_ENABLE | \ + NFC_DMA_INT_ENABLE) + + +/*define bit use in NFC_CMD*/ +#define NFC_CMD_LOW_BYTE (0xff << 0) +#define NFC_CMD_HIGH_BYTE (0xff << 8) +#define NFC_ADR_NUM (0x7 << 16) +#define NFC_SEND_ADR (1 << 19) +#define NFC_ACCESS_DIR (1 << 20) +#define NFC_DATA_TRANS (1 << 21) +#define NFC_SEND_CMD1 (1 << 22) +#define NFC_WAIT_FLAG (1 << 23) +#define NFC_SEND_CMD2 (1 << 24) +#define NFC_SEQ (1 << 25) +#define NFC_DATA_SWAP_METHOD (1 << 26) +#define NFC_ROW_AUTO_INC (1 << 27) +#define NFC_SEND_CMD3 (1 << 28) +#define NFC_SEND_CMD4 (1 << 29) +#define NFC_CMD_TYPE (3 << 30) + +/* define bit use in NFC_RCMD_SET*/ +#define NFC_READ_CMD (0xff << 0) +#define NFC_RANDOM_READ_CMD0 (0xff << 8) +#define NFC_RANDOM_READ_CMD1 (0xff << 16) + +/*define bit use in NFC_WCMD_SET*/ +#define NFC_PROGRAM_CMD (0xff << 0) +#define NFC_RANDOM_WRITE_CMD (0xff << 8) +#define NFC_READ_CMD0 (0xff << 16) +#define NFC_READ_CMD1 (0xff << 24) + +/*define bit use in NFC_ECC_CTL*/ +#define NFC_ECC_EN (1 << 0) +#define NFC_ECC_PIPELINE (1 << 3) +#define NFC_ECC_EXCEPTION (1 << 4) +#define NFC_ECC_BLOCK_SIZE (1 << 5) +#define NFC_RANDOM_EN (1 << 9)
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 143
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+#define NFC_RANDOM_DIRECTION (1 << 10) +#define NFC_ECC_MODE_SHIFT 12 +#define NFC_ECC_MODE (0xf << NFC_ECC_MODE_SHIFT) +#define NFC_RANDOM_SEED (0x7fff << 16) + + +Kill and avoid these extra empty lines.
Sure.
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 149
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +6
-0+enum sunxi_nand_rb_type { +RB_NONE , +RB_NATIVE , +RB_GPIO , +}; +Can you add some documentation about this read-back stuff?
This has nothing to do with read-back, RB means ready/busy :-).
Quoted 2 lines
I know documenting is not a super-fun activity, but it's a bit tough to review otherwise.
Okay, I'll try to spend more time documenting non obvious stuff ;-)
Quoted 27 lines
Quoted 27 lines
drivers/mtd/nand/sunxi_nand.c new file +26
-0+struct sunxi_nand_rb { +enum sunxi_nand_rb_type type; +union { +int gpio; +int nativeid; + } info; +}; + +struct sunxi_nand_chip_sel { +u8 cs; +struct sunxi_nand_rb rb; +}; + +#define DEFAULT_NAME_FORMAT "nand@%d" +#define MAX_NAME_SIZE (sizeof("nand@") + 2) + +struct sunxi_nand_hw_ecc { +int mode; +struct nand_ecclayout layout; +}; + +struct sunxi_nand_chip { +struct list_head node; +struct nand_chip nand; +struct mtd_info mtd; +char default_name[MAX_NAME_SIZE ];
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 179
Quoted 4 lines
drivers/mtd/nand/sunxi_nand.c new file +4
-0+unsigned long clk_rate; +int selected; +int nsels; +struct sunxi_nand_chip_sel sels[0 ];Hm... you prepare the whole multiple chip support but you really support a single chip? I'd say drop entirely and support just a single chip, it'll make the driver much cleaner. We can always add the support later, after proper testing.
Really, I spent so much time supporting multiple chips!
No I'm kidding :-), this was pretty simple, hence I added multiple chips support right away.
Anyway, even if I drop this multiple chip support I want to keep the DT bindings as proposed (one node for the controller and the nand chip defined as child node of this controller node) to avoid breaking the DT ABI in future versions.
Quoted 5 lines
Quoted 5 lines
drivers/mtd/nand/sunxi_nand.c new file +5
-0+}; + +static inline struct sunxi_nand_chip *to_sunxi_nand (struct nand_chip *nand) +{ + return container_of(nand, struct sunxi_nand_chip, nand);
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 190
Quoted 6 lines
drivers/mtd/nand/sunxi_nand.c new file +6
-0+} + +struct sunxi_nfc { +struct nand_hw_control controller; +void __iomem *regs; +int irq;You don't seem to need the irq stored anywhere, as you're using
devm_request_irq.
Okay, I'll drop this field.
Quoted 34 lines
Quoted 34 lines
drivers/mtd/nand/sunxi_nand.c new file +33
-0+struct clk *ahb_clk; +struct clk *sclk; +unsigned long assigned_cs; +unsigned long clk_rate; +struct list_head chips; +struct completion complete; +}; + +static inline struct sunxi_nfc *to_sunxi_nfc (struct nand_hw_control *ctrl) +{ + return container_of(ctrl, struct sunxi_nfc, controller); +} + +static irqreturn_t sunxi_nfc_interrupt (int irq,void *dev_id) +{ +struct sunxi_nfc *nfc = dev_id; +u32 st =readl (nfc->regs +NFC_REG_ST ); +u32 ien =readl (nfc->regs +NFC_REG_INT ); + +if (!(ien & st)) +return IRQ_NONE ; + +if ((ien & st) == ien) +complete (&nfc->complete); + +writel (st &NFC_INT_MASK , nfc->regs +NFC_REG_ST ); +writel (~st & ien &NFC_INT_MASK , nfc->regs +NFC_REG_INT ); + +return IRQ_HANDLED ; +} + +static int sunxi_nfc_wait_int (struct sunxi_nfc *nfc,u32 flags, +unsigned int timeout_ms)
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 229
Quoted 6 lines
drivers/mtd/nand/sunxi_nand.c new file +6
-0+{ +init_completion (&nfc->complete); + +writel (flags, nfc->regs +NFC_REG_INT ); +if (!timeout_ms) +wait_for_completion (&nfc->complete);In the same vein as infinite loops (see below), I'd avoid this no-timeout wait. Or maybe you can *guarantee* it'll be always completed?
Sure, I'll just return an error is timeout_ms is set to 0.
Quoted 34 lines
Quoted 34 lines
drivers/mtd/nand/sunxi_nand.c new file +33
-0+else if (!wait_for_completion_timeout (&nfc->complete, +msecs_to_jiffies (timeout_ms))) +return -ETIMEDOUT ; + +return 0 ; +} + +static int sunxi_nfc_dev_ready (struct mtd_info *mtd) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +struct sunxi_nand_rb *rb; +unsigned long timeo = (sunxi_nand->nand.state ==FL_ERASING ?400 :20 ); +int ret; + +if (sunxi_nand->selected <0 ) +return 0 ; + + rb = &sunxi_nand->sels[sunxi_nand->selected].rb; + +switch (rb->type) { +case RB_NATIVE : + ret = !!(readl (nfc->regs +NFC_REG_ST ) & + (NFC_RB_STATE0 << rb->info.nativeid)); +if (ret) +break ; + +sunxi_nfc_wait_int (nfc,NFC_RB_B2R , timeo); + ret = !!(readl (nfc->regs +NFC_REG_ST ) & + (NFC_RB_STATE0 << rb->info.nativeid)); +break ; +case RB_GPIO :
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 268
Quoted 6 lines
drivers/mtd/nand/sunxi_nand.c new file +6
-0+ ret =gpio_get_value (rb->info.gpio); +break ; +case RB_NONE : +default : + ret =0 ; +pr_err ("cannot check R/B NAND status!" );I'd suggest avoiding this kind of
pr_err. The user won't know who's saying this. Try usingdev_errorpr_fmt.
I cannot use devm here, because this function might be called before mtd_info registration (called from nand_scan_ident).
I'll define the pr_fmt macro instead.
Quoted 59 lines
Quoted 59 lines
drivers/mtd/nand/sunxi_nand.c new file +58
-0+break ; + } + +return ret; +} + +static void sunxi_nfc_select_chip (struct mtd_info *mtd,int chip) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +struct sunxi_nand_chip_sel *sel; +u32 ctl; + +if (chip >0 && chip >= sunxi_nand->nsels) +return ; + +if (chip == sunxi_nand->selected) +return ; + + ctl =readl (nfc->regs +NFC_REG_CTL ) & + ~(NFC_CE_SEL |NFC_RB_SEL |NFC_EN ); + +if (chip >=0 ) { + sel = &sunxi_nand->sels[chip]; + + ctl |= (sel->cs <<24 ) |NFC_EN | + (((nand->page_shift -10 ) &0xf ) <<8 ); +if (sel->rb.type ==RB_NONE ) { + nand->dev_ready =NULL ; + }else { + nand->dev_ready = sunxi_nfc_dev_ready; +if (sel->rb.type ==RB_NATIVE ) + ctl |= (sel->rb.info.nativeid <<3 ); + } + +writel (mtd->writesize, nfc->regs +NFC_REG_SPARE_AREA ); + +if (nfc->clk_rate != sunxi_nand->clk_rate) { +clk_set_rate (nfc->sclk, sunxi_nand->clk_rate); + nfc->clk_rate = sunxi_nand->clk_rate; + } + } + +writel (ctl, nfc->regs +NFC_REG_CTL ); + + sunxi_nand->selected = chip; +} + +static void sunxi_nfc_read_buf (struct mtd_info *mtd,uint8_t *buf,int len) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +int cnt; +int offs =0 ; +u32 tmp; +
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 333
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+while (len > offs) { + cnt = len - offs; +if (cnt >1024 ) + cnt =1024 ; + +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ;How about avoiding the infinite loop?
Okay, just have to figure out what the appropriate timeout is.
Quoted 22 lines
Quoted 22 lines
drivers/mtd/nand/sunxi_nand.c new file +21
-0+writel (cnt, nfc->regs +NFC_REG_CNT ); + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD ; +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +if (buf) +memcpy_fromio (buf + offs, nfc->regs +NFC_RAM0_BASE , + cnt); + offs += cnt; + } +} + +static void sunxi_nfc_write_buf (struct mtd_info *mtd,const uint8_t *buf, +int len) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +int cnt; +int offs =0 ; +u32 tmp; +
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 333
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+while (len > offs) { + cnt = len - offs; +if (cnt >1024 ) + cnt =1024 ; + +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ;Ditto.
Same as above, I'll have to find an appropriate timeout value.
Quoted 491 lines
Quoted 21 lines
drivers/mtd/nand/sunxi_nand.c new file +20
-0+writel (cnt, nfc->regs +NFC_REG_CNT ); +memcpy_toio (nfc->regs +NFC_RAM0_BASE , buf + offs, cnt); + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD | +NFC_ACCESS_DIR ; +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); + offs += cnt; + } +} + +static uint8_t sunxi_nfc_read_byte (struct mtd_info *mtd) +{ +uint8_t ret; + +sunxi_nfc_read_buf (mtd, &ret,1 ); + +return ret; +} + +static void sunxi_nfc_cmd_ctrl(struct mtd_info *mtd, int dat,drivers/mtd/nand/sunxi_nand.c line 391
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ unsigned int ctrl) +{ +struct nand_chip *nand = mtd->priv; +struct sunxi_nand_chip *sunxi_nand =to_sunxi_nand (nand); +struct sunxi_nfc *nfc =to_sunxi_nfc (sunxi_nand->nand.controller); +u32 tmp; + +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ; +Ditto.
Quoted 46 lines
drivers/mtd/nand/sunxi_nand.c new file +46
-0+if (ctrl &NAND_CTRL_CHANGE ) { + tmp =readl (nfc->regs +NFC_REG_CTL ); +if (ctrl &NAND_NCE ) + tmp |=NFC_CE_CTL ; +else + tmp &= ~NFC_CE_CTL ; +writel (tmp, nfc->regs +NFC_REG_CTL ); + } + +if (dat ==NAND_CMD_NONE ) +return ; + +if (ctrl &NAND_CLE ) { +writel (NFC_SEND_CMD1 | dat, nfc->regs +NFC_REG_CMD ); + }else { +writel (dat, nfc->regs +NFC_REG_ADDR_LOW ); +writel (NFC_SEND_ADR , nfc->regs +NFC_REG_CMD ); + } + +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +} + +static int sunxi_nfc_hw_ecc_read_page (struct mtd_info *mtd, +struct nand_chip *chip,uint8_t *buf, +int oob_required,int page) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct nand_ecclayout *layout = ecc->layout; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int steps = mtd->writesize / ecc->size; +unsigned int max_bitflips =0 ; +int offset; +u32 tmp; +int i; +int cnt; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < steps; i++) { +if (i)drivers/mtd/nand/sunxi_nand.c line 447
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ chip->cmdfunc (mtd,NAND_CMD_RNDOUT , i * ecc->size,-1 ); + + offset = mtd->writesize + layout->eccpos[i * ecc->bytes] -4 ; + + chip->read_buf (mtd,NULL , ecc->size); + + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); +while ((readl (nfc->regs +NFC_REG_ST ) &NFC_CMD_FIFO_STATUS )) + ; +Ditto.
Quoted 9 lines
drivers/mtd/nand/sunxi_nand.c new file +9
-0+ tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD | (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +memcpy_fromio (buf + (i * ecc->size), + nfc->regs +NFC_RAM0_BASE , ecc->size); + +if (readl (nfc->regs +NFC_REG_ECC_ST ) &0x1 ) { + mtd->ecc_stats.failed++; + }else {drivers/mtd/nand/sunxi_nand.c line 466
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ tmp =readl (nfc->regs +NFC_REG_ECC_CNT0 ) &0xff ; + mtd->ecc_stats.corrected += tmp; + max_bitflips = max_t(unsigned int, max_bitflips, tmp); + } + +if (oob_required) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); +while ((readl (nfc->regs +NFC_REG_ST ) & +NFC_CMD_FIFO_STATUS )) + ;Ditto.
Quoted 57 lines
drivers/mtd/nand/sunxi_nand.c new file +57
-0+ offset -= mtd->writesize; + chip->read_buf (mtd, chip->oob_poi + offset, + ecc->bytes +4 ); + } + } + +if (oob_required) { + cnt = ecc->layout->oobfree[0 ].length -4 ; +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , mtd->writesize, +-1 ); + chip->read_buf (mtd, chip->oob_poi, cnt); + } + } + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~NFC_ECC_EN ; + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +return max_bitflips; +} + +static int sunxi_nfc_hw_ecc_write_page (struct mtd_info *mtd, +struct nand_chip *chip, +const uint8_t *buf,int oob_required) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct nand_ecclayout *layout = ecc->layout; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int offset; +u32 tmp; +int i; +int cnt; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < mtd->writesize / ecc->size; i++) { +if (i) + chip->cmdfunc (mtd,NAND_CMD_RNDIN , i * ecc->size,-1 ); + + chip->write_buf (mtd, buf + (i * ecc->size), ecc->size); + + offset = layout->eccpos[i * ecc->bytes] -4 + mtd->writesize; + +/* Fill OOB data in */ +if (oob_required) { + tmp =0xffffffff ; +memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , &tmp, +4 ); + }else {drivers/mtd/nand/sunxi_nand.c line 533
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , + chip->oob_poi + offset - mtd->writesize, +4 ); + } + + chip->cmdfunc (mtd,NAND_CMD_RNDIN , offset,-1 ); +while ((readl (nfc->regs +NFC_REG_ST ) & +NFC_CMD_FIFO_STATUS )) + ; +Ditto.
Quoted 308 lines
drivers/mtd/nand/sunxi_nand.c new file +308
-0+ tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD |NFC_ACCESS_DIR | + (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); + } + +if (oob_required) { + cnt = ecc->layout->oobfree[0 ].length -4 ; +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDIN , mtd->writesize,-1 ); + chip->write_buf (mtd, chip->oob_poi, cnt); + } + } + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_EN |NFC_ECC_PIPELINE ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +return 0 ; +} + +static int sunxi_nfc_hw_syndrome_ecc_read_page (struct mtd_info *mtd, +struct nand_chip *chip, +uint8_t *buf,int oob_required, +int page) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int steps = mtd->writesize / ecc->size; +unsigned int max_bitflips =0 ; +uint8_t *oob = chip->oob_poi; +int offset =0 ; +int cnt; +u32 tmp; +int i; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < steps; i++) { + chip->read_buf (mtd,NULL , ecc->size); + + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD | (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); +memcpy_fromio (buf, nfc->regs +NFC_RAM0_BASE , ecc->size); + buf += ecc->size; + offset += ecc->size; + +if (readl (nfc->regs +NFC_REG_ECC_ST ) &0x1 ) { + mtd->ecc_stats.failed++; + }else { + tmp =readl (nfc->regs +NFC_REG_ECC_CNT0 ) &0xff ; + mtd->ecc_stats.corrected += tmp; + max_bitflips = max_t(unsigned int, max_bitflips, tmp); + } + +if (oob_required) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); + chip->read_buf (mtd, oob, ecc->bytes + ecc->prepad); + oob += ecc->bytes + ecc->prepad; + } + + offset += ecc->bytes + ecc->prepad; + } + +if (oob_required) { + cnt = mtd->oobsize - (oob - chip->oob_poi); +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDOUT , offset,-1 ); + chip->read_buf (mtd, oob, cnt); + } + } + +writel (readl (nfc->regs +NFC_REG_ECC_CTL ) & ~NFC_ECC_EN , + nfc->regs +NFC_REG_ECC_CTL ); + +return max_bitflips; +} + +static int sunxi_nfc_hw_syndrome_ecc_write_page (struct mtd_info *mtd, +struct nand_chip *chip, +const uint8_t *buf, +int oob_required) +{ +struct sunxi_nfc *nfc =to_sunxi_nfc (chip->controller); +struct nand_ecc_ctrl *ecc = &chip->ecc; +struct sunxi_nand_hw_ecc *data = ecc->priv; +int steps = mtd->writesize / ecc->size; +uint8_t *oob = chip->oob_poi; +int offset =0 ; +int cnt; +u32 tmp; +int i; + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_MODE |NFC_ECC_PIPELINE |NFC_ECC_BLOCK_SIZE | +NFC_ECC_BLOCK_SIZE ); + tmp |=NFC_ECC_EN | (data->mode <<NFC_ECC_MODE_SHIFT ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +for (i =0 ; i < steps; i++) { + chip->write_buf (mtd, buf + (i * ecc->size), ecc->size); + offset += ecc->size; + +/* Fill OOB data in */ +if (oob_required) { + tmp =0xffffffff ; +memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , &tmp, +4 ); + }else { +memcpy_toio (nfc->regs +NFC_REG_USER_DATA_BASE , oob , +4 ); + } + + tmp =NFC_DATA_TRANS |NFC_DATA_SWAP_METHOD |NFC_ACCESS_DIR | + (1 <<30 ); +writel (tmp, nfc->regs +NFC_REG_CMD ); +sunxi_nfc_wait_int (nfc,NFC_CMD_INT_FLAG ,0 ); + + offset += ecc->bytes + ecc->prepad; + oob += ecc->bytes + ecc->prepad; + } + +if (oob_required) { + cnt = mtd->oobsize - (oob - chip->oob_poi); +if (cnt >0 ) { + chip->cmdfunc (mtd,NAND_CMD_RNDIN , offset,-1 ); + chip->write_buf (mtd, oob, cnt); + } + } + + tmp =readl (nfc->regs +NFC_REG_ECC_CTL ); + tmp &= ~(NFC_ECC_EN |NFC_ECC_PIPELINE ); + +writel (tmp, nfc->regs +NFC_REG_ECC_CTL ); + +return 0 ; +} + +static int sunxi_nand_chip_set_timings (struct sunxi_nand_chip *chip, +const struct nand_sdr_timings *timings) +{ +u32 min_clk_period =0 ; + +/* T1 <=> tCLS */ +if (timings->tCLS_min > min_clk_period) + min_clk_period = timings->tCLS_min; + +/* T2 <=> tCLH */ +if (timings->tCLH_min > min_clk_period) + min_clk_period = timings->tCLH_min; + +/* T3 <=> tCS */ +if (timings->tCS_min > min_clk_period) + min_clk_period = timings->tCS_min; + +/* T4 <=> tCH */ +if (timings->tCH_min > min_clk_period) + min_clk_period = timings->tCH_min; + +/* T5 <=> tWP */ +if (timings->tWP_min > min_clk_period) + min_clk_period = timings->tWP_min; + +/* T6 <=> tWH */ +if (timings->tWH_min > min_clk_period) + min_clk_period = timings->tWH_min; + +/* T7 <=> tALS */ +if (timings->tALS_min > min_clk_period) + min_clk_period = timings->tALS_min; + +/* T8 <=> tDS */ +if (timings->tDS_min > min_clk_period) + min_clk_period = timings->tDS_min; + +/* T9 <=> tDH */ +if (timings->tDH_min > min_clk_period) + min_clk_period = timings->tDH_min; + +/* T10 <=> tRR */ +if (timings->tRR_min > (min_clk_period *3 )) + min_clk_period = (timings->tRR_min +2 ) /3 ; + +/* T11 <=> tALH */ +if (timings->tALH_min > min_clk_period) + min_clk_period = timings->tALH_min; + +/* T12 <=> tRP */ +if (timings->tRP_min > min_clk_period) + min_clk_period = timings->tRP_min; + +/* T13 <=> tREH */ +if (timings->tREH_min > min_clk_period) + min_clk_period = timings->tREH_min; + +/* T14 <=> tRC */ +if (timings->tRC_min > (min_clk_period *2 )) + min_clk_period = (timings->tRC_min +1 ) /2 ; + +/* T15 <=> tWC */ +if (timings->tWC_min > (min_clk_period *2 )) + min_clk_period = (timings->tWC_min +1 ) /2 ; + + +/* min_clk_period = (NAND-clk-period * 2) */ +if (min_clk_period <1000 ) + min_clk_period =1000 ; + + min_clk_period /=1000 ; + chip->clk_rate = (2 *1000000000 ) / min_clk_period; + +/* TODO: configure T16-T19 */ + +return 0 ; +} + +static int sunxi_nand_chip_init_timings (struct sunxi_nand_chip *chip, +struct device_node *np) +{ +const struct nand_sdr_timings *timings; +int ret; +int mode; + + mode =onfi_get_async_timing_mode (&chip->nand); +if (mode ==ONFI_TIMING_MODE_UNKNOWN ) { + mode =of_get_nand_onfi_timing_mode (np); +if (mode <0 ) + mode =1 ; + + mode =fls (mode) -1 ; +if (mode <0 ) + mode =0 ; + }else { +uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN ] = {}; + mode =fls (mode) -1 ; +if (mode <0 ) + mode =0 ; + + feature[0 ] = mode; + ret = chip->nand.onfi_set_features (&chip->mtd, &chip->nand, +ONFI_FEATURE_ADDR_TIMING_MODE , + feature); +if (ret) +return ret; + } + + timings =onfi_async_timing_mode_to_sdr_timings (mode); +if (IS_ERR (timings)) +return PTR_ERR (timings); + +return sunxi_nand_chip_set_timings (chip, timings); +} + +static int sunxi_nand_hw_common_ecc_ctrl_init (struct mtd_info *mtd, +struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct sunxi_nand_hw_ecc *data; +struct nand_ecclayout *layout; +int nsectors; +int ret; + +if (!ecc->strength || !ecc->size) +return -EINVAL ; + + data =kzalloc (sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + +/* Add ECC info retrieval from DT */ +if (ecc->strength <=16 ) { + ecc->strength =16 ; + data->mode =0 ; + }else if (ecc->strength <=24 ) { + ecc->strength =24 ; + data->mode =1 ; + }else if (ecc->strength <=28 ) { + ecc->strength =28 ; + data->mode =2 ; + }else if (ecc->strength <=32 ) { + ecc->strength =32 ; + data->mode =3 ; + }else if (ecc->strength <=40 ) { + ecc->strength =40 ; + data->mode =4 ; + }else if (ecc->strength <=48 ) { + ecc->strength =48 ; + data->mode =5 ; + }else if (ecc->strength <=56 ) { + ecc->strength =56 ; + data->mode =6 ; + }else if (ecc->strength <=60 ) { + ecc->strength =60 ; + data->mode =7 ; + }else if (ecc->strength <=64 ) { + ecc->strength =64 ; + data->mode =8 ; + }else { +pr_err ("unsupported strength\n" );
Quoted 2 lines
drivers/mtd/nand/sunxi_nand.c line 842
Quoted 1 line
drivers/mtd/nand/sunxi_nand.c new file +1
-0+return -ENOTSUPP ;You're leaking the 'data' allocated above in here.
Oops, I'll fix that.
Quoted 134 lines
Quoted 134 lines
drivers/mtd/nand/sunxi_nand.c new file +133
-0+ } + +/* HW ECC always request ECC bytes for 1024 bytes blocks */ + ecc->bytes = ((ecc->strength *fls (8 *1024 )) +7 ) /8 ; + +/* HW ECC always work with even numbers of ECC bytes */ +if (ecc->bytes %2 ) + ecc->bytes++; + + layout = &data->layout; + nsectors = mtd->writesize / ecc->size; + +if (mtd->oobsize < ((ecc->bytes +4 ) * nsectors)) { + ret = -EINVAL ; + goto err; + } + + layout->eccbytes = (ecc->bytes * nsectors); + + ecc->layout = layout; + ecc->priv = data; + +return 0 ; + +err: +kfree (data); + +return ret; +} + +static void sunxi_nand_hw_common_ecc_ctrl_cleanup (struct nand_ecc_ctrl *ecc) +{ +kfree (ecc->priv); +} + +static int sunxi_nand_hw_ecc_ctrl_init (struct mtd_info *mtd, +struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_ecclayout *layout; +int nsectors; +int i, j; +int ret; + + ret =sunxi_nand_hw_common_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; + + ecc->read_page = sunxi_nfc_hw_ecc_read_page; + ecc->write_page = sunxi_nfc_hw_ecc_write_page; + layout = ecc->layout; + nsectors = mtd->writesize / ecc->size; +/* +* The first 2 bytes are used for BB markers. +* We merge the 4 user available bytes from HW ECC with this +* first section, hence why the + 2 operation (- 2 + 4). +*/ + layout->oobfree[0 ].length = mtd->oobsize +2 - + ((ecc->bytes +4 ) * nsectors); + layout->oobfree[0 ].offset =2 ; +for (i =0 ; i < nsectors; i++) { +/* +* The first 4 ECC block bytes are already counted in the first +* oobfree entry. +*/ +if (i) { + layout->oobfree[i].offset = + layout->oobfree[i -1 ].offset + + layout->oobfree[i -1 ].length + + ecc->bytes; + layout->oobfree[i].length =4 ; + } + +for (j =0 ; j < ecc->bytes; j++) + layout->eccpos[(ecc->bytes * i) + j] = + layout->oobfree[i].offset + + layout->oobfree[i].length + j; + } + +return 0 ; +} + +static int sunxi_nand_hw_syndrome_ecc_ctrl_init (struct mtd_info *mtd, +struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_ecclayout *layout; +int nsectors; +int i; +int ret; + + ret =sunxi_nand_hw_common_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; + + ecc->prepad =4 ; + ecc->read_page = sunxi_nfc_hw_syndrome_ecc_read_page; + ecc->write_page = sunxi_nfc_hw_syndrome_ecc_write_page; + + layout = ecc->layout; + nsectors = mtd->writesize / ecc->size; + +for (i =0 ; i < (ecc->bytes * nsectors); i++) + layout->eccpos[i] = i; + + layout->oobfree[0 ].length = mtd->oobsize - i; + layout->oobfree[0 ].offset = i; + +return 0 ; +} + +static void sunxi_nand_ecc_cleanup (struct nand_ecc_ctrl *ecc) +{ +switch (ecc->mode) { +case NAND_ECC_HW : +case NAND_ECC_HW_SYNDROME : +sunxi_nand_hw_common_ecc_ctrl_cleanup (ecc); +break ; +default : +break ; + } +} + +static int sunxi_nand_ecc_init (struct mtd_info *mtd,struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_chip *nand = mtd->priv; +int ecc_step_size, ecc_strength; +int ret; + + ecc_step_size =of_get_nand_ecc_step_size (np); + ecc_strength =of_get_nand_ecc_strength (np); +if (ecc_step_size >0 && ecc_strength >0 ) {
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 982
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+ ecc->size = ecc_step_size; + ecc->strength = ecc_strength; + }else { + ecc->size = nand->ecc_step_ds; + ecc->strength = nand->ecc_strength_ds; + } +Shouldn't you check the devicetree value is not weaker than the ONFI-obtained?
I can definitely do that.
Quoted 137 lines
Quoted 137 lines
drivers/mtd/nand/sunxi_nand.c new file +136
-0+ ecc->mode =of_get_nand_ecc_mode (np); +switch (ecc->mode) { +case NAND_ECC_SOFT_BCH : +if (!ecc->size || !ecc->strength) +return -EINVAL ; + ecc->bytes = ((ecc->strength *fls (8 * ecc->size)) +7 ) /8 ; +break ; +case NAND_ECC_HW : + ret =sunxi_nand_hw_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; +break ; +case NAND_ECC_HW_SYNDROME : + ret =sunxi_nand_hw_syndrome_ecc_ctrl_init (mtd, ecc, np); +if (ret) +return ret; +break ; +case NAND_ECC_NONE : +case NAND_ECC_SOFT : +break ; +default : +return -EINVAL ; + } + +return 0 ; +} + +static int sunxi_nand_chip_init (struct device *dev,struct sunxi_nfc *nfc, +struct device_node *np) +{ +const struct nand_sdr_timings *timings; +struct sunxi_nand_chip *chip; +struct mtd_part_parser_data ppdata; +struct mtd_info *mtd; +struct nand_chip *nand; +int nsels; +int ret; +int i; +u32 tmp; + +if (!of_get_property (np,"reg" , &nsels)) +return -EINVAL ; + + nsels /=sizeof (u32); +if (!nsels) +return -EINVAL ; + + chip =devm_kzalloc (dev, +sizeof (*chip) + + (nsels *sizeof (struct sunxi_nand_chip_sel )), +GFP_KERNEL ); +if (!chip) +return -ENOMEM ; + + chip->nsels = nsels; + chip->selected =-1 ; + +for (i =0 ; i < nsels; i++) { + ret =of_property_read_u32_index (np,"reg" , i, &tmp); +if (ret) +return ret; + +if (tmp >7 ) +return -EINVAL ; + +if (test_and_set_bit (tmp, &nfc->assigned_cs)) +return -EINVAL ; + + chip->sels[i].cs = tmp; + +if (!of_property_read_u32_index (np,"allwinner,rb" , i, &tmp) && + tmp <2 ) { + chip->sels[i].rb.type =RB_NATIVE ; + chip->sels[i].rb.info.nativeid = tmp; + }else { + ret =of_get_named_gpio (np,"rb-gpios" , i); +if (ret >=0 ) { + tmp = ret; + chip->sels[i].rb.type =RB_GPIO ; + chip->sels[i].rb.info.gpio = tmp; + ret =devm_gpio_request (dev, tmp,"nand-rb" ); +if (ret) +return ret; + + ret =gpio_direction_input (tmp); +if (ret) +return ret; + }else { + chip->sels[i].rb.type =RB_NONE ; + } + } + } + + timings =onfi_async_timing_mode_to_sdr_timings (0 ); +if (IS_ERR (timings)) +return PTR_ERR (timings); + + ret =sunxi_nand_chip_set_timings (chip, timings); + + nand = &chip->nand; +/* Default tR value specified in the ONFI spec (chapter 4.15.1) */ + nand->chip_delay =200 ; + nand->controller = &nfc->controller; + nand->select_chip = sunxi_nfc_select_chip; + nand->cmd_ctrl = sunxi_nfc_cmd_ctrl; + nand->read_buf = sunxi_nfc_read_buf; + nand->write_buf = sunxi_nfc_write_buf; + nand->read_byte = sunxi_nfc_read_byte; + +if (of_get_nand_on_flash_bbt (np)) + nand->bbt_options |=NAND_BBT_USE_FLASH ; + + mtd = &chip->mtd; + mtd->dev.parent = dev; + mtd->priv = nand; + mtd->owner =THIS_MODULE ; + + ret =nand_scan_ident (mtd, nsels,NULL ); +if (ret) +return ret; + + ret =sunxi_nand_chip_init_timings (chip, np); +if (ret) +return ret; + + ret =sunxi_nand_ecc_init (mtd, &nand->ecc, np); +if (ret) +return ret; + + ret =nand_scan_tail (mtd); +if (ret) +return ret; + +if (of_property_read_string (np,"nand-name" , &mtd->name)) { +snprintf (chip->default_name,MAX_NAME_SIZE , +DEFAULT_NAME_FORMAT , chip->sels[i].cs);
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 1124
Quoted 6 lines
drivers/mtd/nand/sunxi_nand.c new file +6
-0+ mtd->name = chip->default_name; + } + + ppdata.of_node = np; + ret =mtd_device_parse_register (mtd,NULL , &ppdata,NULL ,0 ); +if (!ret)Aren't you leaking the BBT resources allocated in
nand_scan_tail? I **think** you need to callnand_releaseif the mtd device register fails.
Absolutely, I'll call nand_release before returning the error code.
Quoted 42 lines
Quoted 42 lines
drivers/mtd/nand/sunxi_nand.c new file +41
-0+return ret; + +list_add_tail (&chip->node, &nfc->chips); + +return 0 ; +} + +static int sunxi_nand_chips_init (struct device *dev,struct sunxi_nfc *nfc) +{ +struct device_node *np = dev->of_node; +struct device_node *nand_np; +int nchips =of_get_child_count (np); +int ret; + +if (nchips >8 ) +return -EINVAL ; + +for_each_child_of_node (np, nand_np) { + ret =sunxi_nand_chip_init (dev, nfc, nand_np); +if (ret) +return ret; + } + +return 0 ; +} + +static void sunxi_nand_chips_cleanup (struct sunxi_nfc *nfc) +{ +struct sunxi_nand_chip *chip; + +while (!list_empty (&nfc->chips)) { + chip = list_first_entry(&nfc->chips, struct sunxi_nand_chip, + node); +nand_release (&chip->mtd); +sunxi_nand_ecc_cleanup (&chip->nand.ecc); + } +} + +static int sunxi_nfc_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev;
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 1172
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+struct resource *r; +struct sunxi_nfc *nfc; +int ret; + + nfc =devm_kzalloc (dev,sizeof (*nfc),GFP_KERNEL ); +if (!nfc) { +dev_err (dev,"failed to allocate NFC struct\n" );OOM message not needed.
I'll drop it.
Quoted 13 lines
drivers/mtd/nand/sunxi_nand.c line 1183
Quoted 12 lines
drivers/mtd/nand/sunxi_nand.c new file +11
-0+return -ENOMEM ; + } + +spin_lock_init (&nfc->controller.lock); +init_waitqueue_head (&nfc->controller.wq); +INIT_LIST_HEAD (&nfc->chips); + + r =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + nfc->regs =devm_ioremap_resource (dev, r); +if (IS_ERR (nfc->regs)) { +dev_err (dev,"failed to remap iomem\n" );Message not needed, see
devm_ioremap_resource()documentation.
ditto.
Quoted 38 lines
Quoted 26 lines
drivers/mtd/nand/sunxi_nand.c new file +25
-0+return PTR_ERR (nfc->regs); + } + + nfc->irq =platform_get_irq (pdev,0 ); +if (nfc->irq <0 ) { +dev_err (dev,"failed to retrieve irq\n" ); +return nfc->irq; + } + + nfc->ahb_clk =devm_clk_get (dev,"ahb_clk" ); +if (IS_ERR (nfc->ahb_clk)) { +dev_err (dev,"failed to retrieve ahb_clk\n" ); +return PTR_ERR (nfc->ahb_clk); + } + + ret =clk_prepare_enable (nfc->ahb_clk); +if (ret) +return ret; + + nfc->sclk =devm_clk_get (dev,"sclk" ); +if (IS_ERR (nfc->sclk)) { +dev_err (dev,"failed to retrieve nand_clk\n" ); + ret =PTR_ERR (nfc->sclk); + goto out_ahb_clk_unprepare; + }drivers/mtd/nand/sunxi_nand.c line 1218
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+ + ret =clk_prepare_enable (nfc->sclk); +if (ret) + goto out_ahb_clk_unprepare; + +/* Reset NFC */ +writel (readl (nfc->regs +NFC_REG_CTL ) |NFC_RESET , + nfc->regs +NFC_REG_CTL ); +while (readl (nfc->regs +NFC_REG_CTL ) &NFC_RESET ) + ;Again: maybe you should avoid potentially infinite loop, no matter what.
Quoted 12 lines
drivers/mtd/nand/sunxi_nand.c line 1229
Quoted 11 lines
drivers/mtd/nand/sunxi_nand.c new file +11
-0+ +writel (0 , nfc->regs +NFC_REG_INT ); + ret =devm_request_irq (dev, nfc->irq, sunxi_nfc_interrupt, +0 ,"sunxi-nand" , nfc); +if (ret) + goto out_sclk_unprepare; + +platform_set_drvdata (pdev, nfc); + +writel (0x100 , nfc->regs +NFC_REG_TIMING_CTL ); +writel (0x7ff , nfc->regs +NFC_REG_TIMING_CFG );Hm... can you remove the magic numbers?
I wish I could, but unfortunately I don't enough information from the datasheet and reverse engineering work to know what these registers are used for and these value encodes.
I'll add a TODO comment to state that we're missing informations here and that it should be reworked as soon as we get these informations.
Quoted 31 lines
Quoted 31 lines
drivers/mtd/nand/sunxi_nand.c new file +30
-0+ + ret =sunxi_nand_chips_init (dev, nfc); +if (ret) { +dev_err (dev,"failed to init nand chips\n" ); + goto out_sclk_unprepare; + } + +return 0 ; + +out_sclk_unprepare: +clk_disable_unprepare (nfc->sclk); +out_ahb_clk_unprepare: +clk_disable_unprepare (nfc->ahb_clk); + +return ret; +} + +static int sunxi_nfc_remove (struct platform_device *pdev) +{ +struct sunxi_nfc *nfc =platform_get_drvdata (pdev); + +sunxi_nand_chips_cleanup (nfc); + +return 0 ; +} + +static const struct of_device_id sunxi_nfc_ids[] = { + { .compatible ="allwinner,sun4i-nand" }, + {/* sentinel */ } +};
Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 1266
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+MODULE_DEVICE_TABLE (of, sunxi_nfc_ids); + +static struct platform_driver sunxi_nfc_driver = { + .driver = { + .name ="sunxi_nand" , + .owner =THIS_MODULE , + .of_match_table =of_match_ptr (sunxi_nfc_ids),Redundant
of_match_ptr. See5576bc7bef2919dd2b185bffb768bf9c0da76788.
I'll drop it.
Quoted 2 lines
I think that's all I can spot, without knowing the hardware details.
Thanks a lot for your review.
Best Regards,
Boris
Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support
Quoted 4 lines · Boris BREZILLON
Quoted 1 line
Can you add some documentation about this read-back stuff?
This has nothing to do with read-back, RB means ready/busy :-).
Hehe... right... I guess I was thinking in read/busy.
See? That's why you need to document it: so it's fool-proof.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support
In reply to Boris BREZILLONQuoted 5 lines · Boris BREZILLON
Quoted 4 lines · Ezequiel Garcia
Quoted 3 lines · Boris BREZILLON
drivers/mtd/nand/sunxi_nand.c new file +0
-0@@ -0,0 +1,1276 @@
...
Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+static int sunxi_nand_ecc_init (struct mtd_info *mtd,struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_chip *nand = mtd->priv; +int ecc_step_size, ecc_strength; +int ret; + + ecc_step_size =of_get_nand_ecc_step_size (np); + ecc_strength =of_get_nand_ecc_strength (np); +if (ecc_step_size >0 && ecc_strength >0 ) {Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 982
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+ ecc->size = ecc_step_size; + ecc->strength = ecc_strength; + }else { + ecc->size = nand->ecc_step_ds; + ecc->strength = nand->ecc_strength_ds; + } +Shouldn't you check the devicetree value is not weaker than the ONFI-obtained?
Quoted 1 line
I can definitely do that.
You can do that here, but take a look at the discussion Ezequiel and I had about this:
http://thread.gmane.org/gmane.linux.drivers.devicetree/67462
We probably don't want to be doing anything drastic like overriding the device tree configuration. Instead, we might want to stick a warning into the core nand_base code that does the proper comparison of the '*_ds' values with the actual values chosen in chip->ecc->{size,strength}. The comparison is kind of subtle, actually, so it'd be good to do it exactly once for everyone.
Brian
Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support
Quoted 31 lines · Brian Norris
Quoted 5 lines · Boris BREZILLON
Quoted 4 lines · Ezequiel Garcia
Quoted 3 lines · Boris BREZILLON
drivers/mtd/nand/sunxi_nand.c new file +0
-0@@ -0,0 +1,1276 @@...
Quoted 19 lines
Quoted 10 lines
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+static int sunxi_nand_ecc_init (struct mtd_info *mtd,struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_chip *nand = mtd->priv; +int ecc_step_size, ecc_strength; +int ret; + + ecc_step_size =of_get_nand_ecc_step_size (np); + ecc_strength =of_get_nand_ecc_strength (np); +if (ecc_step_size >0 && ecc_strength >0 ) {Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 982
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+ ecc->size = ecc_step_size; + ecc->strength = ecc_strength; + }else { + ecc->size = nand->ecc_step_ds; + ecc->strength = nand->ecc_strength_ds; + } +Shouldn't you check the devicetree value is not weaker than the ONFI-obtained?
Quoted 1 line
I can definitely do that.
You can do that here, but take a look at the discussion Ezequiel and I had about this:
Quoted 8 lines
http://thread.gmane.org/gmane.linux.drivers.devicetree/67462
We probably don't want to be doing anything drastic like overriding the device tree configuration. Instead, we might want to stick a warning into the core
nand_basecode that does the proper comparison of the '*_ds' values with the actual values chosen in chip->ecc->{size,strength}. The comparison is kind of subtle, actually, so it'd be good to do it exactly once for everyone.
I forgot, Ezequiel already submitted this. I'll look at it soon:
http://patchwork.ozlabs.org/patch/348901/
Brian
Re: [PATCH v3 5/9] mtd: nand: add sunxi NAND flash controller support
Quoted 30 lines · Brian Norris
Quoted 29 lines · Brian Norris
Quoted 5 lines · Boris BREZILLON
Quoted 4 lines · Ezequiel Garcia
Quoted 3 lines · Boris BREZILLON
drivers/mtd/nand/sunxi_nand.c new file +0
-0@@ -0,0 +1,1276 @@...
Quoted 18 lines
Quoted 10 lines
Quoted 10 lines
drivers/mtd/nand/sunxi_nand.c new file +10
-0+static int sunxi_nand_ecc_init (struct mtd_info *mtd,struct nand_ecc_ctrl *ecc, +struct device_node *np) +{ +struct nand_chip *nand = mtd->priv; +int ecc_step_size, ecc_strength; +int ret; + + ecc_step_size =of_get_nand_ecc_step_size (np); + ecc_strength =of_get_nand_ecc_strength (np); +if (ecc_step_size >0 && ecc_strength >0 ) {Quoted 8 lines
drivers/mtd/nand/sunxi_nand.c line 982
Quoted 7 lines
drivers/mtd/nand/sunxi_nand.c new file +7
-0+ ecc->size = ecc_step_size; + ecc->strength = ecc_strength; + }else { + ecc->size = nand->ecc_step_ds; + ecc->strength = nand->ecc_strength_ds; + } +Shouldn't you check the devicetree value is not weaker than the ONFI-obtained?
Quoted 1 line
I can definitely do that.
You can do that here, but take a look at the discussion Ezequiel and I had about this:
Quoted 8 lines
Quoted 8 lines
http://thread.gmane.org/gmane.linux.drivers.devicetree/67462
We probably don't want to be doing anything drastic like overriding the device tree configuration. Instead, we might want to stick a warning into the core
nand_basecode that does the proper comparison of the '*_ds' values with the actual values chosen in chip->ecc->{size,strength}. The comparison is kind of subtle, actually, so it'd be good to do it exactly once for everyone.
Fair enough, I'll follow your suggestions on this specific point ;)
Quoted 3 lines
I forgot, Ezequiel already submitted this. I'll look at it soon:
Thanks for pointing this out.
Quoted 2 lines
Brian
PATCH v3 6/9 mtd: nand: add sunxi NFC dt bindings doc
In reply to Boris BREZILLONAdd the sunxi NAND Flash Controller dt bindings documentation.
--- .../devicetree/bindings/mtd/sunxi-nand.txt | 48 ++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/sunxi-nand.txt
Documentation/devicetree/bindings/mtd/sunxi-nand.txt new file +48 -0
@@ -0,0 +1,48 @@ +Allwinner NAND Flash Controller (NFC) + +Required properties: +- compatible : "allwinner,sun4i-nand". +- reg : shall contain registers location and length for data and reg. +- interrupts : shall define the nand controller interrupt. +- #address-cells: shall be set to 1. Encode the nand CS. +- #size-cells : shall be set to 0. +- clocks : shall reference nand controller clocks. +- clock-names : nand controller internal clock names. Shall contain : + * "ahb_clk" : AHB gating clock + * "sclk" : nand controller clock + +Optional children nodes: +Children nodes represent the available nand chips. + +Optional properties: +- onfi,nand-timing-mode : mandatory if the chip does not support the ONFI + standard. +- allwinner,rb : shall contain the native Ready/Busy ids. + or +- rb-gpios : shall contain the gpios used as R/B pins. +- nand-ecc-mode : one of the supported ECC modes ("hw", "hw_syndrome", "soft", + "soft_bch" or "none") + +see Documentation/devicetree/mtd/nand.txt for generic bindings. + + +Examples: +nfc: nand at 01c03000 { + compatible = "allwinner,sun4i-nand"; + reg = <0x01c03000 0x1000>; + interrupts = <0 37 1>; + clocks = <&ahb_gates 13>, <&nand_clk>; + clock-names = "ahb_clk", "sclk"; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&nand_pins_a &nand_cs0_pins_a &nand_rb0_pins_a>; + status = "okay"; + + nand at 0 { + reg = <0>; + allwinner,rb = <0>; + nand-ecc-mode = "soft_bch"; + onfi,nand-timing-mode = <4>; + }; +};
Signature · 1 line
1.7.9.5
PATCH v3 7/9 ARM: dt/sunxi: add NFC node to Allwinner A20 SoC
In reply to Boris BREZILLONAdd NAND Flash controller node definition to the A20 SoC.
--- arch/arm/boot/dts/sun7i-a20.dtsi | 11 +++++++++++ 1 file changed, 11 insertions(+)
arch/arm/boot/dts/sun7i-a20.dtsi +11 -0
@@ -339,6 +339,17 @@ #size-cells = <1>; ranges; + nfc: nand at 01c03000 { + compatible = "allwinner,sun4i-nand"; + reg = <0x01c03000 0x1000>; + interrupts = <0 37 4>; + clocks = <&ahb_gates 13>, <&nand_clk>; + clock-names = "ahb_clk", "sclk"; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + }; + emac: ethernet at 01c0b000 { compatible = "allwinner,sun4i-a10-emac"; reg = <0x01c0b000 0x1000>;
Signature · 1 line
1.7.9.5
PATCH v3 8/9 ARM: dt/sunxi: add A20 NAND controller pin definitions
In reply to Boris BREZILLONDefine the NAND controller pin configs.
--- arch/arm/boot/dts/sun7i-a20.dtsi | 80 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+)
arch/arm/boot/dts/sun7i-a20.dtsi +80 -0
@@ -443,6 +443,86 @@ allwinner,drive = <0>; allwinner,pull = <0>; }; + + nand_pins_a: nand_base0 at 0 { + allwinner,pins = "PC0", "PC1", "PC2", + "PC5", "PC8", "PC9", "PC10", + "PC11", "PC12", "PC13", "PC14", + "PC15", "PC16"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs0_pins_a: nand_cs at 0 { + allwinner,pins = "PC4"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs1_pins_a: nand_cs at 1 { + allwinner,pins = "PC3"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs2_pins_a: nand_cs at 2 { + allwinner,pins = "PC17"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs3_pins_a: nand_cs at 3 { + allwinner,pins = "PC18"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs4_pins_a: nand_cs at 4 { + allwinner,pins = "PC19"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs5_pins_a: nand_cs at 5 { + allwinner,pins = "PC20"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs6_pins_a: nand_cs at 6 { + allwinner,pins = "PC21"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_cs7_pins_a: nand_cs at 7 { + allwinner,pins = "PC22"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_rb0_pins_a: nand_rb at 0 { + allwinner,pins = "PC6"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; + + nand_rb1_pins_a: nand_rb at 1 { + allwinner,pins = "PC7"; + allwinner,function = "nand0"; + allwinner,drive = <0>; + allwinner,pull = <0>; + }; }; timer at 01c20c00 {
Signature · 1 line
1.7.9.5
PATCH v3 9/9 ARM: sunxi/dt: enable NAND on cubietruck board
In reply to Boris BREZILLONEnable the NFC and describe the NAND flash connected to this controller.
--- arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
arch/arm/boot/dts/sun7i-a20-cubietruck.dts +17 -0
@@ -19,6 +19,23 @@ compatible = "cubietech,cubietruck", "allwinner,sun7i-a20"; soc at 01c00000 { + nfc: nand at 01c03000 { + pinctrl-names = "default"; + pinctrl-0 = <&nand_pins_a &nand_cs0_pins_a &nand_rb0_pins_a>; + status = "okay"; + + nand at 0 { + #address-cells = <2>; + #size-cells = <2>; + reg = <0>; + allwinner,rb = <0>; + nand-ecc-mode = "hw"; + onfi,nand-timing-mode = <0x1f>; + nand-ecc-strength = <40>; + nand-ecc-step-size = <1024>; + }; + }; + pinctrl at 01c20800 { led_pins_cubietruck: led_pins at 0 { allwinner,pins = "PH7", "PH11", "PH20", "PH21";
Signature · 1 line
1.7.9.5