Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Christophe,
Sorry for the delay, here are some answers from my previous comments.
Maybe you already addressed them, in this case please ignore them.
Also, please run and correct 'checkpatch.pl --strict' issues (mostly
uses of uint8_t instead of u8 but also a warning about the compatible).
Overall the driver is in a pretty good shape and should enter the next
release. I'll apply the patches after -rc1 once I'll have your v3+ with
everything corrected.
[...]
> >> index c7efc31..863662c 100644
> >> --- a/drivers/mtd/nand/raw/Kconfig
> >> +++ b/drivers/mtd/nand/raw/Kconfig
> >> @@ -541,4 +541,13 @@ config MTD_NAND_TEGRA
> >> is supported. Extra OOB bytes when using HW ECC are currently
> >> not supported.
> >> >> +config MTD_NAND_STM32_FMC2
> >> + tristate "Support for NAND controller on STM32MP SoCs"
> >> + depends on MACH_STM32MP157 || COMPILE_TEST
> >> + help
> >> +Enables support for NAND Flash chips on SoCs containing the FMC2
> >> +NAND controller. This controller is found on STM32MP SoCs.
> >> +The driver supports a maximum 8k page size. HW ECC is enabled and
> >> +supports a maximum 8-bit correction error per sector of 512 bytes.
> > > HW ECC should not be enabled by default. 8-bit/512B of correctability
> > is good but not that high and people might want to use software ECC in
> > conjunction with raw accesses.
>
> Yes, I agree. The driver only supports NAND_ECC_HW mode. NAND_ECC_SOFT mode
> was not requested by customers and was not implemented. The driver could be
> improved later to support mode like NAND_ECC_SOFT or NAND_ECC_ON_DIE. Should
> I remove "HW ECC is enabled" from Kconfig description?
Yes, please.
[...]
> >> +/* Select function */
> >> +static void stm32_fmc2_select_chip(struct nand_chip *chip, int chipnr)
> >> +{
> >> + struct stm32_fmc2 *fmc2 = nand_get_controller_data(chip);
> >> + struct dma_slave_config dma_cfg;
> >> +
> >> + if (chipnr < 0 || chipnr >= fmc2->ncs)
> >> + return;
> >> +
> >> + if (fmc2->cs_used[chipnr] == fmc2->cs_sel)
> >> + return;
> >> +
> >> + fmc2->cs_sel = fmc2->cs_used[chipnr];
> >> +
> >> + if (fmc2->dma_tx_ch && fmc2->dma_rx_ch) {
> >> + memset(&dma_cfg, 0, sizeof(dma_cfg));
> >> + dma_cfg.src_addr = fmc2->data_phys_addr[fmc2->cs_sel];
> >> + dma_cfg.dst_addr = fmc2->data_phys_addr[fmc2->cs_sel];
> >> + dma_cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> >> + dma_cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
> >> + dma_cfg.src_maxburst = 32;
> >> + dma_cfg.dst_maxburst = 32;
> >> +
> >> + if (dmaengine_slave_config(fmc2->dma_tx_ch, &dma_cfg))
> >> + dev_warn(fmc2->dev, "tx DMA engine slave config
> >> failed\n");
> >> +
> >> + if (dmaengine_slave_config(fmc2->dma_rx_ch, &dma_cfg))
> >> + dev_warn(fmc2->dev, "rx DMA engine slave config
> >> failed\n");
> >> + }
> > > What if there are two NAND chips using different timing modes? You
> > should probably reconfigure the timings registers, unless there are
> > already a set of timing registers per CS?
>
> Yes, it's true. In case of 2 NAND chips, timings and pcr registers should
> have been reconfigured. But, the driver only supports one NAND chip connected
> to 1 or 2 CS. There was no requirement on our side to support 2 different
> NAND chips. I do not have a board to test such configuration, so i do not
> want to deliver this support without being able to test it on my side. The
> driver will be improved later to support 2 different NAND chips, in case this
> configuration is requested by customers.
Sure, I'm not requesting you to support 2 NAND chips, I'm just
requesting to write this driver in a manner so that adding support for a
2nd NAND chip would be easy thanks to a better software design. That's
actually something that is done in the marvell_nand.c driver if you
need inspiration.
[...]
> >> +
> >> +void stm32_fmc2_read_data(struct nand_chip *chip, void *buf,
> >> +unsigned int len, bool force_8bit)
> >> +{
> >> + struct stm32_fmc2 *fmc2 = nand_get_controller_data(chip);
> >> + void __iomem *io_addr_r = fmc2->data_base[fmc2->cs_sel];
> >> + u8 *p = buf;
> >> + unsigned int i;
> >> +
> >> + if (force_8bit)
> >> + goto read_8bit;
> >> +
> >> + if (IS_ALIGNED((u32)buf, sizeof(u32)) && IS_ALIGNED(len, sizeof(u32))) {
> > > If you selected BOUNCE_BUFFER in the options, buf is supposedly
> > aligned, or am I missing something?
>
> 2 FMC2 internal modes can be used:
> - Sequencer mode (Patch 2/3): dmas are used and NAND_USE_BOUNCE_BUFFER
> option is selected.
> - Manual mode (Patch 3/3): no dma channel is used and
> NAND_USE_BOUNCE_BUFFER is not selected.
> Should i select NAND_USE_BOUNCE_BUFFER for sequencer and manual mode, and
> remove IS_ALIGNED test on buf?
If it's only useful in manual mode after patch 3/3, then the logic for
it should be in pa
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Boris,
On 09/24/2018 07:23 PM, Boris Brezillon wrote:
Hi Christophe,
On Mon, 17 Sep 2018 17:47:39 +0200
wrote:
+struct stm32_fmc2 {
+ struct nand_chip chip;
+ struct device *dev;
+ void __iomem *io_base;
+ void __iomem *data_base[FMC2_MAX_CE];
+ void __iomem *cmd_base[FMC2_MAX_CE];
+ void __iomem *addr_base[FMC2_MAX_CE];
+ phys_addr_t io_phys_addr;
+ phys_addr_t data_phys_addr[FMC2_MAX_CE];
+ struct clk *clk;
+
+ struct dma_chan *dma_tx_ch;
+ struct dma_chan *dma_rx_ch;
+ struct dma_chan *dma_ecc_ch;
+ struct sg_table dma_data_sg;
+ struct sg_table dma_ecc_sg;
+ u8 *ecc_buf;
+ int dma_ecc_len;
+
+ struct completion complete;
+ struct completion dma_data_complete;
+ struct completion dma_ecc_complete;
+
+ struct stm32_fmc2_timings timings;
+ u8 cs_assigned;
+ int cs_sel;
+ int ncs;
+ int cs_used[FMC2_MAX_CE];
+};
Can we have a clear separation between the NAND controller and NAND
chip structures. I know you only support a single chip per-controller
right now, but I prefer to have things clearly separated from the
beginning.
Yes, I can create 2 structures: one for the controller (stm32_fmc2) and
one for the NAND chip (stm32_fmc2_nand_chip).
Regards,
Christophe Kerello.
Regards,
Boris
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
On Mon, 24 Sep 2018 18:36:36 +0200
Christophe Kerello wrote:
> >> +static int stm32_fmc2_resume(struct device *dev)
> >> +{
> >> + struct stm32_fmc2 *fmc2 = dev_get_drvdata(dev);
> >> + int i, ret;
> >> +
> >> + pinctrl_pm_select_default_state(dev);
> >> +
> >> + ret = clk_prepare_enable(fmc2->clk);
> >> + if (ret) {
> >> + dev_err(dev, "can not enable the clock\n");
> >> + return ret;
> >> + }
> >> +
> >> + stm32_fmc2_init(fmc2);
> >> + stm32_fmc2_timings_init(fmc2);
> >> + stm32_fmc2_setup(fmc2);
> >> +
> >> + for (i = 0; i < fmc2->ncs; i++)
> >> + nand_reset(&fmc2->chip, i);
> >
> > This means you have one different NAND chip wired on each CS.
> >
> > We could have two CS wired to the same NAND chip. Calling nand_reset
> > twice would be harmless but a lost of time.
Actually, you have to call nand_reset() for each CS, otherwise not all
dies are reset.
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Christophe,
On Mon, 17 Sep 2018 17:47:39 +0200
wrote:
> +struct stm32_fmc2 {
> + struct nand_chip chip;
> + struct device *dev;
> + void __iomem *io_base;
> + void __iomem *data_base[FMC2_MAX_CE];
> + void __iomem *cmd_base[FMC2_MAX_CE];
> + void __iomem *addr_base[FMC2_MAX_CE];
> + phys_addr_t io_phys_addr;
> + phys_addr_t data_phys_addr[FMC2_MAX_CE];
> + struct clk *clk;
> +
> + struct dma_chan *dma_tx_ch;
> + struct dma_chan *dma_rx_ch;
> + struct dma_chan *dma_ecc_ch;
> + struct sg_table dma_data_sg;
> + struct sg_table dma_ecc_sg;
> + u8 *ecc_buf;
> + int dma_ecc_len;
> +
> + struct completion complete;
> + struct completion dma_data_complete;
> + struct completion dma_ecc_complete;
> +
> + struct stm32_fmc2_timings timings;
> + u8 cs_assigned;
> + int cs_sel;
> + int ncs;
> + int cs_used[FMC2_MAX_CE];
> +};
Can we have a clear separation between the NAND controller and NAND
chip structures. I know you only support a single chip per-controller
right now, but I prefer to have things clearly separated from the
beginning.
Regards,
Boris
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Miquèl, On 09/22/2018 03:48 PM, Miquel Raynal wrote: Hi Christophe, I suppose you received the kbuildrobot issues already, please have a look at them. Yes, kbuidroot issues will be solved in the v2 patchset. The driver looks well, some comments below. wrote on Mon, 17 Sep 2018 17:47:39 +0200: From: Christophe Kerello The driver adds the support for the STMicroelectronics FMC2 NAND Controller found on STM32MP SOCs. This patch is based on FMC2 command sequencer. The purpose of the command sequencer is to facilitate the programming and the reading of NAND flash pages with the ECC and to free the CPU of sequencing tasks. It requires one DMA channel for write and two DMA channels for read operations. Only NAND_ECC_HW mode is actually supported. The driver supports a maximum 8k page size. The following ECC strength and step size are currently supported: - nand-ecc-strength = <8>, nand-ecc-step-size = <512> (BCH8) - nand-ecc-strength = <4>, nand-ecc-step-size = <512> (BCH4) - nand-ecc-strength = <1>, nand-ecc-step-size = <512> (Extended ecc based on HAMMING) This patch has been tested on Micron MT29F8G08ABACAH4 and MT29F8G16ABACAH4 Signed-off-by: Christophe Kerello --- drivers/mtd/nand/raw/Kconfig |9 + drivers/mtd/nand/raw/Makefile |1 + drivers/mtd/nand/raw/stm32_fmc2_nand.c | 1729 3 files changed, 1739 insertions(+) create mode 100644 drivers/mtd/nand/raw/stm32_fmc2_nand.c diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index c7efc31..863662c 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -541,4 +541,13 @@ config MTD_NAND_TEGRA is supported. Extra OOB bytes when using HW ECC are currently not supported. +config MTD_NAND_STM32_FMC2 + tristate "Support for NAND controller on STM32MP SoCs" + depends on MACH_STM32MP157 || COMPILE_TEST + help + Enables support for NAND Flash chips on SoCs containing the FMC2 + NAND controller. This controller is found on STM32MP SoCs. + The driver supports a maximum 8k page size. HW ECC is enabled and + supports a maximum 8-bit correction error per sector of 512 bytes. HW ECC should not be enabled by default. 8-bit/512B of correctability is good but not that high and people might want to use software ECC in conjunction with raw accesses. Yes, I agree. The driver only supports NAND_ECC_HW mode. NAND_ECC_SOFT mode was not requested by customers and was not implemented. The driver could be improved later to support mode like NAND_ECC_SOFT or NAND_ECC_ON_DIE. Should I remove "HW ECC is enabled" from Kconfig description? + endif # MTD_NAND diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile index a6ef067..8c437e3 100644 --- a/drivers/mtd/nand/raw/Makefile +++ b/drivers/mtd/nand/raw/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/ obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o obj-$(CONFIG_MTD_NAND_MTK)+= mtk_ecc.o mtk_nand.o obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o +obj-$(CONFIG_MTD_NAND_STM32_FMC2) += stm32_fmc2_nand.o nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o nand-objs += nand_amd.o diff --git a/drivers/mtd/nand/raw/stm32_fmc2_nand.c b/drivers/mtd/nand/raw/stm32_fmc2_nand.c new file mode 100644 index 000..dd5762a --- /dev/null +++ b/drivers/mtd/nand/raw/stm32_fmc2_nand.c @@ -0,0 +1,1729 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved + * Author: Christophe Kerello for STMicroelectronics I'm not sure the "All rights reserved" has a meaning here. But you definitely not have to add "for STMicroelectronics" because it is already in the copyright. Ok. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Bad block marker length */ +#define FMC2_BBM_LEN 2 + +/* ECC step size */ +#define FMC2_ECC_STEP_SIZE 512 + +/* BCHDSRx registers length */ +#define FMC2_BCHDSRS_LEN 20 + +/* HECCR length */ +#define FMC2_HECCR_LEN 4 + +/* Max requests done for a 8k nand page size */ +#define FMC2_MAX_SG_COUNT 16 + +/* Max chip enable */ +#define FMC2_MAX_CE2 + +/* Timings */ +#define FMC2_THIZ 1 +#define FMC2_TIO 8000 +#define FMC2_TSYNC 3000 +#define FMC2_PCR_TIMING_MASK 0xf +#define FMC2_PMEM_PATT_TIMING_MASK 0xff + +/* FMC2 Controller Registers */ +#define FMC2_BCR1 0x0 +#define FMC2_PCR 0x80 +#define FMC2_SR0x84 +#define FMC2_PMEM 0x88 +#define FMC2_PATT 0x8c +#define FMC2_HECCR
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Miquèl,
On 09/23/2018 01:34 PM, Miquel Raynal wrote:
Hi Christophe,
wrote on Mon, 17 Sep 2018 17:47:39 +0200:
From: Christophe Kerello
The driver adds the support for the STMicroelectronics FMC2 NAND
Controller found on STM32MP SOCs.
This patch is based on FMC2 command sequencer.
The purpose of the command sequencer is to facilitate the programming
and the reading of NAND flash pages with the ECC and to free the CPU
of sequencing tasks.
It requires one DMA channel for write and two DMA channels for read
operations.
Only NAND_ECC_HW mode is actually supported.
The driver supports a maximum 8k page size.
The following ECC strength and step size are currently supported:
- nand-ecc-strength = <8>, nand-ecc-step-size = <512> (BCH8)
- nand-ecc-strength = <4>, nand-ecc-step-size = <512> (BCH4)
- nand-ecc-strength = <1>, nand-ecc-step-size = <512> (Extended ecc
based on HAMMING)
This patch has been tested on Micron MT29F8G08ABACAH4 and
MT29F8G16ABACAH4
Signed-off-by: Christophe Kerello
---
[...]
+/* NAND callbacks setup */
+static void stm32_fmc2_nand_callbacks_setup(struct stm32_fmc2 *fmc2)
+{
+ struct nand_chip *chip = &fmc2->chip;
+
+ /* Specific callbacks to read/write a page */
+ chip->ecc.correct = stm32_fmc2_ham_correct;
+ chip->ecc.write_page = stm32_fmc2_sequencer_write_page;
+ chip->ecc.read_page = stm32_fmc2_sequencer_read_page;
+ chip->ecc.write_page_raw = stm32_fmc2_sequencer_write_page_raw;
+ chip->ecc.read_page_raw = stm32_fmc2_sequencer_read_page_raw;
Are you sure all the tests in mtd-utils are successful?
I am not sure to understand the question, so i have ran some of them:
mtd_nandbiterrs.ko, mtd_speedtest.ko and mtd_oobtest.ko on a 2 MBytes
partition.
# insmod mtd_nandbiterrs.ko dev=1
[ 235.062876]
[ 235.062904] ==
[ 235.068747] mtd_nandbiterrs: MTD device: 1
[ 235.072904] mtd_nandbiterrs: MTD device size 2097152,
eraseblock=262144, page=4096, oob=224
[ 235.081177] mtd_nandbiterrs: Device uses 1 subpages of 4096 bytes
[ 235.087284] mtd_nandbiterrs: Using page=0, offset=0, eraseblock=0
[ 235.094109] mtd_nandbiterrs: incremental biterrors test
[ 235.098778] mtd_nandbiterrs: write_page
[ 235.103126] mtd_nandbiterrs: rewrite page
[ 235.106793] mtd_nandbiterrs: read_page
[ 235.110501] mtd_nandbiterrs: verify_page
[ 235.114406] mtd_nandbiterrs: Successfully corrected 0 bit errors per
subpage
[ 235.121124] mtd_nandbiterrs: Inserted biterror @ 0/5
[ 235.126117] mtd_nandbiterrs: rewrite page
[ 235.130490] mtd_nandbiterrs: read_page
[ 235.134247] mtd_nandbiterrs: Read reported 1 corrected bit errors
[ 235.139923] mtd_nandbiterrs: verify_page
[ 235.144124] mtd_nandbiterrs: Successfully corrected 1 bit errors per
subpage
[ 235.150891] mtd_nandbiterrs: Inserted biterror @ 0/2
[ 235.155888] mtd_nandbiterrs: rewrite page
[ 235.160244] mtd_nandbiterrs: read_page
[ 235.164001] mtd_nandbiterrs: Read reported 2 corrected bit errors
[ 235.169687] mtd_nandbiterrs: verify_page
[ 235.173881] mtd_nandbiterrs: Successfully corrected 2 bit errors per
subpage
[ 235.180653] mtd_nandbiterrs: Inserted biterror @ 0/0
[ 235.185631] mtd_nandbiterrs: rewrite page
[ 235.190052] mtd_nandbiterrs: read_page
[ 235.193743] mtd_nandbiterrs: Read reported 3 corrected bit errors
[ 235.199449] mtd_nandbiterrs: verify_page
[ 235.203631] mtd_nandbiterrs: Successfully corrected 3 bit errors per
subpage
[ 235.210415] mtd_nandbiterrs: Inserted biterror @ 1/7
[ 235.215391] mtd_nandbiterrs: rewrite page
[ 235.219760] mtd_nandbiterrs: read_page
[ 235.223507] mtd_nandbiterrs: Read reported 4 corrected bit errors
[ 235.229210] mtd_nandbiterrs: verify_page
[ 235.233392] mtd_nandbiterrs: Successfully corrected 4 bit errors per
subpage
[ 235.240175] mtd_nandbiterrs: Inserted biterror @ 1/5
[ 235.245149] mtd_nandbiterrs: rewrite page
[ 235.249516] mtd_nandbiterrs: read_page
[ 235.253272] mtd_nandbiterrs: Read reported 5 corrected bit errors
[ 235.258968] mtd_nandbiterrs: verify_page
[ 235.263151] mtd_nandbiterrs: Successfully corrected 5 bit errors per
subpage
[ 235.269933] mtd_nandbiterrs: Inserted biterror @ 1/2
[ 235.274923] mtd_nandbiterrs: rewrite page
[ 235.279272] mtd_nandbiterrs: read_page
[ 235.283021] mtd_nandbiterrs: Read reported 6 corrected bit errors
[ 235.288727] mtd_nandbiterrs: verify_page
[ 235.292910] mtd_nandbiterrs: Successfully corrected 6 bit errors per
subpage
[ 235.299694] mtd_nandbiterrs: Inserted biterror @ 1/0
[ 235.304671] mtd_nandbiterrs: rewrite page
[ 235.309028] mtd_nandbiterrs: read_page
[ 235.312780] mtd_nandbiterrs: Read reported 7 corrected bit errors
[ 235.318489] mtd_nandbiterrs: verify_page
[ 235.322673] mtd_nandbiterrs: Successfully corrected 7 bit errors per
subpage
[ 235.329457] mtd_nandbiterrs: Inserted biterror @ 2/6
[ 235.334432] mtd_nandbiterrs: rewrite page
[ 235.338796] mtd_nandbiterrs: read_page
[ 235
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Christophe,
wrote on Mon, 17 Sep 2018 17:47:39 +0200:
> From: Christophe Kerello
>
> The driver adds the support for the STMicroelectronics FMC2 NAND
> Controller found on STM32MP SOCs.
>
> This patch is based on FMC2 command sequencer.
> The purpose of the command sequencer is to facilitate the programming
> and the reading of NAND flash pages with the ECC and to free the CPU
> of sequencing tasks.
> It requires one DMA channel for write and two DMA channels for read
> operations.
>
> Only NAND_ECC_HW mode is actually supported.
> The driver supports a maximum 8k page size.
> The following ECC strength and step size are currently supported:
> - nand-ecc-strength = <8>, nand-ecc-step-size = <512> (BCH8)
> - nand-ecc-strength = <4>, nand-ecc-step-size = <512> (BCH4)
> - nand-ecc-strength = <1>, nand-ecc-step-size = <512> (Extended ecc
>based on HAMMING)
>
> This patch has been tested on Micron MT29F8G08ABACAH4 and
> MT29F8G16ABACAH4
>
> Signed-off-by: Christophe Kerello
> ---
[...]
> +/* NAND callbacks setup */
> +static void stm32_fmc2_nand_callbacks_setup(struct stm32_fmc2 *fmc2)
> +{
> + struct nand_chip *chip = &fmc2->chip;
> +
> + /* Specific callbacks to read/write a page */
> + chip->ecc.correct = stm32_fmc2_ham_correct;
> + chip->ecc.write_page = stm32_fmc2_sequencer_write_page;
> + chip->ecc.read_page = stm32_fmc2_sequencer_read_page;
> + chip->ecc.write_page_raw = stm32_fmc2_sequencer_write_page_raw;
> + chip->ecc.read_page_raw = stm32_fmc2_sequencer_read_page_raw;
Are you sure all the tests in mtd-utils are successful?
Thanks,
Miquèl
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Christophe, I suppose you received the kbuildrobot issues already, please have a look at them. The driver looks well, some comments below. wrote on Mon, 17 Sep 2018 17:47:39 +0200: > From: Christophe Kerello > > The driver adds the support for the STMicroelectronics FMC2 NAND > Controller found on STM32MP SOCs. > > This patch is based on FMC2 command sequencer. > The purpose of the command sequencer is to facilitate the programming > and the reading of NAND flash pages with the ECC and to free the CPU > of sequencing tasks. > It requires one DMA channel for write and two DMA channels for read > operations. > > Only NAND_ECC_HW mode is actually supported. > The driver supports a maximum 8k page size. > The following ECC strength and step size are currently supported: > - nand-ecc-strength = <8>, nand-ecc-step-size = <512> (BCH8) > - nand-ecc-strength = <4>, nand-ecc-step-size = <512> (BCH4) > - nand-ecc-strength = <1>, nand-ecc-step-size = <512> (Extended ecc >based on HAMMING) > > This patch has been tested on Micron MT29F8G08ABACAH4 and > MT29F8G16ABACAH4 > > Signed-off-by: Christophe Kerello > --- > drivers/mtd/nand/raw/Kconfig |9 + > drivers/mtd/nand/raw/Makefile |1 + > drivers/mtd/nand/raw/stm32_fmc2_nand.c | 1729 > > 3 files changed, 1739 insertions(+) > create mode 100644 drivers/mtd/nand/raw/stm32_fmc2_nand.c > > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index c7efc31..863662c 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -541,4 +541,13 @@ config MTD_NAND_TEGRA > is supported. Extra OOB bytes when using HW ECC are currently > not supported. > > +config MTD_NAND_STM32_FMC2 > + tristate "Support for NAND controller on STM32MP SoCs" > + depends on MACH_STM32MP157 || COMPILE_TEST > + help > + Enables support for NAND Flash chips on SoCs containing the FMC2 > + NAND controller. This controller is found on STM32MP SoCs. > + The driver supports a maximum 8k page size. HW ECC is enabled and > + supports a maximum 8-bit correction error per sector of 512 bytes. HW ECC should not be enabled by default. 8-bit/512B of correctability is good but not that high and people might want to use software ECC in conjunction with raw accesses. > + > endif # MTD_NAND > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile > index a6ef067..8c437e3 100644 > --- a/drivers/mtd/nand/raw/Makefile > +++ b/drivers/mtd/nand/raw/Makefile > @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/ > obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o > obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o > obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o > +obj-$(CONFIG_MTD_NAND_STM32_FMC2)+= stm32_fmc2_nand.o > > nand-objs := nand_base.o nand_bbt.o nand_timings.o nand_ids.o > nand-objs += nand_amd.o > diff --git a/drivers/mtd/nand/raw/stm32_fmc2_nand.c > b/drivers/mtd/nand/raw/stm32_fmc2_nand.c > new file mode 100644 > index 000..dd5762a > --- /dev/null > +++ b/drivers/mtd/nand/raw/stm32_fmc2_nand.c > @@ -0,0 +1,1729 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved > + * Author: Christophe Kerello for > STMicroelectronics I'm not sure the "All rights reserved" has a meaning here. But you definitely not have to add "for STMicroelectronics" because it is already in the copyright. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Bad block marker length */ > +#define FMC2_BBM_LEN 2 > + > +/* ECC step size */ > +#define FMC2_ECC_STEP_SIZE 512 > + > +/* BCHDSRx registers length */ > +#define FMC2_BCHDSRS_LEN 20 > + > +/* HECCR length */ > +#define FMC2_HECCR_LEN 4 > + > +/* Max requests done for a 8k nand page size */ > +#define FMC2_MAX_SG_COUNT16 > + > +/* Max chip enable */ > +#define FMC2_MAX_CE 2 > + > +/* Timings */ > +#define FMC2_THIZ1 > +#define FMC2_TIO 8000 > +#define FMC2_TSYNC 3000 > +#define FMC2_PCR_TIMING_MASK 0xf > +#define FMC2_PMEM_PATT_TIMING_MASK 0xff > + > +/* FMC2 Controller Registers */ > +#define FMC2_BCR10x0 > +#define FMC2_PCR 0x80 > +#define FMC2_SR 0x84 > +#define FMC2_PMEM0x88 > +#define FMC2_PATT0x8c > +#define FMC2_HECCR 0x94 > +#define FMC2_CSQCR 0x200 > +#define FMC2_CSQCFGR10x204 > +#define FMC2_CSQCFGR20x208 > +#define FMC2_CSQCFGR30x20c > +#define FMC2_CSQAR1
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Christophe,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mtd/nand/next]
[also build test WARNING on v4.19-rc4 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/christophe-kerello-st-com/mtd-rawnand-add-STM32-FMC2-NAND-flash-controller-driver/20180918-16
base: git://git.infradead.org/linux-mtd.git nand/next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=ia64
All warnings (new ones prefixed by >>):
In file included from include/linux/clk.h:16,
from drivers/mtd/nand/raw/stm32_fmc2_nand.c:7:
drivers/mtd/nand/raw/stm32_fmc2_nand.c: In function 'stm32_fmc2_read_data':
>> drivers/mtd/nand/raw/stm32_fmc2_nand.c:838:17: warning: cast from pointer to
>> integer of different size [-Wpointer-to-int-cast]
if (IS_ALIGNED((u32)buf, sizeof(u32)) && IS_ALIGNED(len, sizeof(u32))) {
^
include/linux/kernel.h:62:30: note: in definition of macro 'IS_ALIGNED'
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
^
>> drivers/mtd/nand/raw/stm32_fmc2_nand.c:838:17: warning: cast from pointer to
>> integer of different size [-Wpointer-to-int-cast]
if (IS_ALIGNED((u32)buf, sizeof(u32)) && IS_ALIGNED(len, sizeof(u32))) {
^
include/linux/kernel.h:62:44: note: in definition of macro 'IS_ALIGNED'
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
^
drivers/mtd/nand/raw/stm32_fmc2_nand.c: In function 'stm32_fmc2_write_data':
drivers/mtd/nand/raw/stm32_fmc2_nand.c:872:17: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
if (IS_ALIGNED((u32)buf, sizeof(u32)) && IS_ALIGNED(len, sizeof(u32))) {
^
include/linux/kernel.h:62:30: note: in definition of macro 'IS_ALIGNED'
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
^
drivers/mtd/nand/raw/stm32_fmc2_nand.c:872:17: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
if (IS_ALIGNED((u32)buf, sizeof(u32)) && IS_ALIGNED(len, sizeof(u32))) {
^
include/linux/kernel.h:62:44: note: in definition of macro 'IS_ALIGNED'
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
^
In file included from include/linux/module.h:18,
from drivers/mtd/nand/raw/stm32_fmc2_nand.c:13:
drivers/mtd/nand/raw/stm32_fmc2_nand.c: At top level:
drivers/mtd/nand/raw/stm32_fmc2_nand.c:1726:26: error: expected ',' or ';'
before 'DRIVER_NAME'
MODULE_ALIAS("platform:" DRIVER_NAME);
^~~
include/linux/moduleparam.h:24:26: note: in definition of macro
'__MODULE_INFO'
= __stringify(tag) "=" info
^~~~
include/linux/module.h:164:30: note: in expansion of macro 'MODULE_INFO'
#define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias)
^~~
drivers/mtd/nand/raw/stm32_fmc2_nand.c:1726:1: note: in expansion of macro
'MODULE_ALIAS'
MODULE_ALIAS("platform:" DRIVER_NAME);
^~~~
vim +838 drivers/mtd/nand/raw/stm32_fmc2_nand.c
826
827 void stm32_fmc2_read_data(struct nand_chip *chip, void *buf,
828unsigned int len, bool force_8bit)
829 {
830 struct stm32_fmc2 *fmc2 = nand_get_controller_data(chip);
831 void __iomem *io_addr_r = fmc2->data_base[fmc2->cs_sel];
832 u8 *p = buf;
833 unsigned int i;
834
835 if (force_8bit)
836 goto read_8bit;
837
> 838 if (IS_ALIGNED((u32)buf, sizeof(u32)) && IS_ALIGNED(len,
> sizeof(u32))) {
839 u32 *p = buf;
840
841 len >>= 2;
842 for (i = 0; i < len; i++)
843 p[i] = readl_relaxed(io_addr_r);
844 return;
845 }
846
847 if (chip->options & NAND_BUSWIDTH_16) {
848 u16 *p = buf;
849
850 len >>= 1;
851 for (i = 0; i < len; i++)
852 p[i] = readw_relaxed(io_addr_r);
853 return;
854 }
855
856 read_8bit:
857 for (i = 0; i < len; i++)
858 p[i] = readb_relaxed(io_addr_r);
859 }
860
---
0-DAY kernel test infrastructureO
Re: [PATCH 2/3] mtd: rawnand: stm32_fmc2: add STM32 FMC2 NAND flash controller driver
Hi Christophe,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mtd/nand/next]
[also build test ERROR on v4.19-rc4 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/christophe-kerello-st-com/mtd-rawnand-add-STM32-FMC2-NAND-flash-controller-driver/20180918-16
base: git://git.infradead.org/linux-mtd.git nand/next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the
linux-review/christophe-kerello-st-com/mtd-rawnand-add-STM32-FMC2-NAND-flash-controller-driver/20180918-16
HEAD 0dcec50ff1bb14c8b5e37b2bbd8c5f9744199293 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
In file included from include/linux/module.h:18:0,
from drivers/mtd/nand/raw/stm32_fmc2_nand.c:13:
>> drivers/mtd/nand/raw/stm32_fmc2_nand.c:1726:26: error: expected ',' or ';'
>> before 'DRIVER_NAME'
MODULE_ALIAS("platform:" DRIVER_NAME);
^
include/linux/moduleparam.h:24:26: note: in definition of macro
'__MODULE_INFO'
= __stringify(tag) "=" info
^~~~
include/linux/module.h:164:30: note: in expansion of macro 'MODULE_INFO'
#define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias)
^~~
>> drivers/mtd/nand/raw/stm32_fmc2_nand.c:1726:1: note: in expansion of macro
>> 'MODULE_ALIAS'
MODULE_ALIAS("platform:" DRIVER_NAME);
^~~~
vim +1726 drivers/mtd/nand/raw/stm32_fmc2_nand.c
1725
> 1726 MODULE_ALIAS("platform:" DRIVER_NAME);
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
