| Message ID | 1395735604-26706-8-git-send-email-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Lee, >From: Lee Jones [mailto:lee.jones@linaro.org] missing commit log :-) Though $subject is self-explanatory, but you can add more description about assumption and hardware caveats about the controller, and its use. > >Signed-off-by: Lee Jones <lee.jones@linaro.org> >--- > drivers/mtd/nand/stm_nand_bch.c | 56 +++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/stm_nand.h | 20 +++++++++++++++ > 2 files changed, 76 insertions(+) > >diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c >index 76a0d02..1a93f8d 100644 >--- a/drivers/mtd/nand/stm_nand_bch.c >+++ b/drivers/mtd/nand/stm_nand_bch.c >@@ -14,6 +14,7 @@ > > #include <linux/kernel.h> > #include <linux/module.h> >+#include <linux/delay.h> > #include <linux/io.h> > #include <linux/interrupt.h> > #include <linux/device.h> >@@ -102,6 +103,56 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi, > writel(val, nandi->base + NANDBCH_INT_EN); > } > >+static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank) >+{ >+ dev_dbg(nandi->dev, "%s\n", __func__); >+ >+ /* Initialise BCH Controller */ >+ emiss_nandi_select(STM_NANDI_BCH); >+ >+ /* Reset and disable boot-mode controller */ >+ writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG); >+ udelay(1); >+ writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG); Why using 'udelay' ? Isn't there any status register which tells you that controller is reset / initialized ? Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status. >+ >+ /* Reset AFM controller */ >+ writel(CFG_RESET, nandi->base + NANDBCH_CONTROLLER_CFG); >+ udelay(1); >+ writel(0x00000000, nandi->base + NANDBCH_CONTROLLER_CFG); >+ >+ /* Set EMI Bank */ >+ writel(0x1 << emi_bank, nandi->base + NANDBCH_FLEX_MUXCTRL); >+ >+ /* Reset ECC stats */ >+ writel(0x7f0, nandi->base + NANDBCH_CONTROLLER_CFG); >+ udelay(1); >+ "0x7f0" ?? please use Macro instead. with regards, pekon
On Wed, 26 Mar 2014, Gupta, Pekon wrote: > Hi Lee, > > >From: Lee Jones [mailto:lee.jones@linaro.org] > missing commit log :-) > Though $subject is self-explanatory, but you can add more description about > assumption and hardware caveats about the controller, and its use. > > > > >Signed-off-by: Lee Jones <lee.jones@linaro.org> > >--- > > drivers/mtd/nand/stm_nand_bch.c | 56 +++++++++++++++++++++++++++++++++++++++++ > > include/linux/mtd/stm_nand.h | 20 +++++++++++++++ > > 2 files changed, 76 insertions(+) > > > >diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c > >index 76a0d02..1a93f8d 100644 > >--- a/drivers/mtd/nand/stm_nand_bch.c > >+++ b/drivers/mtd/nand/stm_nand_bch.c > >@@ -14,6 +14,7 @@ > > > > #include <linux/kernel.h> > > #include <linux/module.h> > >+#include <linux/delay.h> > > #include <linux/io.h> > > #include <linux/interrupt.h> > > #include <linux/device.h> > >@@ -102,6 +103,56 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi, > > writel(val, nandi->base + NANDBCH_INT_EN); > > } > > > >+static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank) > >+{ > >+ dev_dbg(nandi->dev, "%s\n", __func__); > >+ > >+ /* Initialise BCH Controller */ > >+ emiss_nandi_select(STM_NANDI_BCH); > >+ > >+ /* Reset and disable boot-mode controller */ > >+ writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG); > >+ udelay(1); > >+ writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG); > > Why using 'udelay' ? > Isn't there any status register which tells you that controller is reset / initialized ? > Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status. Documenation says: "The soft reset bit has to be reset to ‘0’ to de-assert the soft reset. The soft reset bit is expected to be asserted for at least one clock cycle for proper reset" > >+ > >+ /* Reset AFM controller */ > >+ writel(CFG_RESET, nandi->base + NANDBCH_CONTROLLER_CFG); > >+ udelay(1); > >+ writel(0x00000000, nandi->base + NANDBCH_CONTROLLER_CFG); > >+ > >+ /* Set EMI Bank */ > >+ writel(0x1 << emi_bank, nandi->base + NANDBCH_FLEX_MUXCTRL); > >+ > >+ /* Reset ECC stats */ > >+ writel(0x7f0, nandi->base + NANDBCH_CONTROLLER_CFG); > >+ udelay(1); > >+ > "0x7f0" ?? please use Macro instead. Sure.
>From: Lee Jones [mailto:lee.jones@linaro.org] >>On Wed, 26 Mar 2014, Gupta, Pekon wrote: [...] >> >+ /* Reset and disable boot-mode controller */ >> >+ writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG); >> >+ udelay(1); >> >+ writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG); >> >> Why using 'udelay' ? >> Isn't there any status register which tells you that controller is reset / initialized ? >> Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status. > >Documenation says: > > "The soft reset bit has to be reset to ‘0’ to de-assert the soft > reset. The soft reset bit is expected to be asserted for at least > one clock cycle for proper reset" > That’s the hardware way of saying that 'enable the clock before applying reset'. Clock is required to propagate reset-logic to flip-flops in pipeline, which do not get direct reset. However that apart. You may safely drop udelay(1) because this 'udelay' is at CPU side and won't guarantee anything about clocks at your controller side. But I leave it to you as this delay is pretty small. with regards, pekon
> >> >+ /* Reset and disable boot-mode controller */ > >> >+ writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG); > >> >+ udelay(1); > >> >+ writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG); > >> > >> Why using 'udelay' ? > >> Isn't there any status register which tells you that controller is reset / initialized ? > >> Or may be polling on NANDBCH_BOOTBANK_CFG may itself give you status. > > > >Documenation says: > > > > "The soft reset bit has to be reset to ‘0’ to de-assert the soft > > reset. The soft reset bit is expected to be asserted for at least > > one clock cycle for proper reset" > > > That’s the hardware way of saying that 'enable the clock before applying reset'. > Clock is required to propagate reset-logic to flip-flops in pipeline, which do not get direct reset. > > However that apart. You may safely drop udelay(1) because this 'udelay' is at > CPU side and won't guarantee anything about clocks at your controller side. > But I leave it to you as this delay is pretty small. I'd like to keep it in if it's all the same to you. The original author is pretty competent and I like to think that it's there for a reason - and as you rightly say, the delay is pretty small.
diff --git a/drivers/mtd/nand/stm_nand_bch.c b/drivers/mtd/nand/stm_nand_bch.c index 76a0d02..1a93f8d 100644 --- a/drivers/mtd/nand/stm_nand_bch.c +++ b/drivers/mtd/nand/stm_nand_bch.c @@ -14,6 +14,7 @@ #include <linux/kernel.h> #include <linux/module.h> +#include <linux/delay.h> #include <linux/io.h> #include <linux/interrupt.h> #include <linux/device.h> @@ -102,6 +103,56 @@ static void nandi_disable_interrupts(struct nandi_controller *nandi, writel(val, nandi->base + NANDBCH_INT_EN); } +static void nandi_init_bch(struct nandi_controller *nandi, int emi_bank) +{ + dev_dbg(nandi->dev, "%s\n", __func__); + + /* Initialise BCH Controller */ + emiss_nandi_select(STM_NANDI_BCH); + + /* Reset and disable boot-mode controller */ + writel(BOOT_CFG_RESET, nandi->base + NANDBCH_BOOTBANK_CFG); + udelay(1); + writel(0x00000000, nandi->base + NANDBCH_BOOTBANK_CFG); + + /* Reset AFM controller */ + writel(CFG_RESET, nandi->base + NANDBCH_CONTROLLER_CFG); + udelay(1); + writel(0x00000000, nandi->base + NANDBCH_CONTROLLER_CFG); + + /* Set EMI Bank */ + writel(0x1 << emi_bank, nandi->base + NANDBCH_FLEX_MUXCTRL); + + /* Reset ECC stats */ + writel(0x7f0, nandi->base + NANDBCH_CONTROLLER_CFG); + udelay(1); + + /* Enable AFM */ + writel(CFG_ENABLE_AFM, nandi->base + NANDBCH_CONTROLLER_CFG); + + /* Configure Read DMA Plugs (values supplied by Validation) */ + writel(0x00000005, nandi->dma + EMISS_NAND_RD_DMA_PAGE_SIZE); + writel(0x00000005, nandi->dma + EMISS_NAND_RD_DMA_MAX_OPCODE_SIZE); + writel(0x00000002, nandi->dma + EMISS_NAND_RD_DMA_MIN_OPCODE_SIZE); + writel(0x00000001, nandi->dma + EMISS_NAND_RD_DMA_MAX_CHUNK_SIZE); + writel(0x00000000, nandi->dma + EMISS_NAND_RD_DMA_MAX_MESSAGE_SIZE); + + /* Configure Write DMA Plugs (values supplied by Validation) */ + writel(0x00000005, nandi->dma + EMISS_NAND_WR_DMA_PAGE_SIZE); + writel(0x00000005, nandi->dma + EMISS_NAND_WR_DMA_MAX_OPCODE_SIZE); + writel(0x00000002, nandi->dma + EMISS_NAND_WR_DMA_MIN_OPCODE_SIZE); + writel(0x00000001, nandi->dma + EMISS_NAND_WR_DMA_MAX_CHUNK_SIZE); + writel(0x00000000, nandi->dma + EMISS_NAND_WR_DMA_MAX_MESSAGE_SIZE); + + nandi_enable_interrupts(nandi, NAND_INT_ENABLE); +} + +static void nandi_init_controller(struct nandi_controller *nandi, + int emi_bank) +{ + nandi_init_bch(nandi, emi_bank); +} + static int remap_named_resource(struct platform_device *pdev, char *name, void __iomem **io_ptr) @@ -174,6 +225,7 @@ nandi_init_resources(struct platform_device *pdev) static int stm_nand_bch_probe(struct platform_device *pdev) { struct stm_plat_nand_bch_data *pdata = pdev->dev.platform_data; + struct stm_nand_bank_data *bank; struct nandi_controller *nandi; if (!pdata) { @@ -190,6 +242,10 @@ static int stm_nand_bch_probe(struct platform_device *pdev) init_completion(&nandi->seq_completed); init_completion(&nandi->rbn_completed); + bank = pdata->bank; + if (bank) + nandi_init_controller(nandi, bank->csn); + return 0; } diff --git a/include/linux/mtd/stm_nand.h b/include/linux/mtd/stm_nand.h index da2006c..99a69ca 100644 --- a/include/linux/mtd/stm_nand.h +++ b/include/linux/mtd/stm_nand.h @@ -17,6 +17,26 @@ #include <linux/io.h> +/* + * Board-level specification relating to a 'bank' of NAND Flash + */ +struct stm_nand_bank_data { + int csn; + int nr_partitions; + struct mtd_partition *partitions; + unsigned int options; + unsigned int bbt_options; + + struct nand_timing_spec *timing_spec; + + /* + * No. of IP clk cycles by which to 'relax' the timing configuration. + * Required on some boards to to accommodate board-level limitations. + * Used in conjunction with 'nand_timing_spec' and ONFI configuration. + */ + int timing_relax; +}; + struct stm_plat_nand_bch_data { struct stm_nand_bank_data *bank; enum stm_nand_bch_ecc_config bch_ecc_cfg;
Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/mtd/nand/stm_nand_bch.c | 56 +++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/stm_nand.h | 20 +++++++++++++++ 2 files changed, 76 insertions(+)