* [PATCH 0/2] mtd: nand: fsl-ifc: fix SRAM initialization for newer controller @ 2018-08-06 9:21 Kurt Kanzenbach 2018-08-06 9:21 ` [PATCH 1/2] mtd: nand: fsl-ifc: check result of SRAM initialization Kurt Kanzenbach 2018-08-06 9:21 ` [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions Kurt Kanzenbach 0 siblings, 2 replies; 7+ messages in thread From: Kurt Kanzenbach @ 2018-08-06 9:21 UTC (permalink / raw) To: Boris Brezillon Cc: Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Kurt Kanzenbach, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel Hi, the current way of initializing the internal SRAM of the IFC controller only works for older controller versions. Newer versions require a different method. So, adding support for it. Thereby, the result of the SRAM initialization should be checked. If it's not successful, further commands such as read won't work. Tested on hardware. Thanks, Kurt Kurt Kanzenbach (2): mtd: nand: fsl-ifc: check result of SRAM initialization mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions drivers/mtd/nand/raw/fsl_ifc_nand.c | 35 +++++++++++++++++++++++++++++++---- include/linux/fsl_ifc.h | 2 ++ 2 files changed, 33 insertions(+), 4 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] mtd: nand: fsl-ifc: check result of SRAM initialization 2018-08-06 9:21 [PATCH 0/2] mtd: nand: fsl-ifc: fix SRAM initialization for newer controller Kurt Kanzenbach @ 2018-08-06 9:21 ` Kurt Kanzenbach 2018-08-06 9:21 ` [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions Kurt Kanzenbach 1 sibling, 0 replies; 7+ messages in thread From: Kurt Kanzenbach @ 2018-08-06 9:21 UTC (permalink / raw) To: Boris Brezillon Cc: Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Kurt Kanzenbach, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel The SRAM initialization might fail. If that happens further NAND operations won't be successful. Therefore, the chip init routine should fail if the SRAM initialization didn't work. Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> --- drivers/mtd/nand/raw/fsl_ifc_nand.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c index 24f59d0066af..e4f5792dc589 100644 --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c @@ -761,7 +761,7 @@ static const struct nand_controller_ops fsl_ifc_controller_ops = { .attach_chip = fsl_ifc_attach_chip, }; -static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) +static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) { struct fsl_ifc_ctrl *ctrl = priv->ctrl; struct fsl_ifc_runtime __iomem *ifc_runtime = ctrl->rregs; @@ -805,12 +805,16 @@ static void fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) wait_event_timeout(ctrl->nand_wait, ctrl->nand_stat, msecs_to_jiffies(IFC_TIMEOUT_MSECS)); - if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) + if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC) { pr_err("fsl-ifc: Failed to Initialise SRAM\n"); + return -ETIMEDOUT; + } /* Restore CSOR and CSOR_ext */ ifc_out32(csor, &ifc_global->csor_cs[cs].csor); ifc_out32(csor_ext, &ifc_global->csor_cs[cs].csor_ext); + + return 0; } static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv) @@ -914,8 +918,13 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv) chip->ecc.algo = NAND_ECC_HAMMING; } - if (ctrl->version >= FSL_IFC_VERSION_1_1_0) - fsl_ifc_sram_init(priv); + if (ctrl->version >= FSL_IFC_VERSION_1_1_0) { + int ret; + + ret = fsl_ifc_sram_init(priv); + if (ret) + return ret; + } /* * As IFC version 2.0.0 has 16KB of internal SRAM as compared to older -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions 2018-08-06 9:21 [PATCH 0/2] mtd: nand: fsl-ifc: fix SRAM initialization for newer controller Kurt Kanzenbach 2018-08-06 9:21 ` [PATCH 1/2] mtd: nand: fsl-ifc: check result of SRAM initialization Kurt Kanzenbach @ 2018-08-06 9:21 ` Kurt Kanzenbach 2018-08-08 9:48 ` Miquel Raynal 1 sibling, 1 reply; 7+ messages in thread From: Kurt Kanzenbach @ 2018-08-06 9:21 UTC (permalink / raw) To: Boris Brezillon Cc: Miquel Raynal, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Kurt Kanzenbach, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel Newer versions of the IFC controller use a different method of initializing the internal SRAM: Instead of reading from flash, a bit in the NAND configuration register has to be set in order to trigger the self-initializing process. Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> --- drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++++++++++++++++++ include/linux/fsl_ifc.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c index e4f5792dc589..384d5e12b05c 100644 --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c @@ -30,6 +30,7 @@ #include <linux/mtd/partitions.h> #include <linux/mtd/nand_ecc.h> #include <linux/fsl_ifc.h> +#include <linux/iopoll.h> #define ERR_BYTE 0xFF /* Value returned for read bytes when read failed */ @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) uint32_t csor = 0, csor_8k = 0, csor_ext = 0; uint32_t cs = priv->bank; + if (ctrl->version > FSL_IFC_VERSION_1_1_0) { + u32 ncfgr, status; + int ret; + + /* Trigger auto initialization */ + ncfgr = ifc_in32(&ifc_runtime->ifc_nand.ncfgr); + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, &ifc_runtime->ifc_nand.ncfgr); + + /* Wait until done */ + ret = readx_poll_timeout(ifc_in32, &ifc_runtime->ifc_nand.ncfgr, + status, !(status & IFC_NAND_NCFGR_SRAM_INIT_EN), + 10, 1000); + if (ret) + dev_err(priv->dev, "Failed to initialize SRAM!\n"); + return ret; + } + /* Save CSOR and CSOR_ext */ csor = ifc_in32(&ifc_global->csor_cs[cs].csor); csor_ext = ifc_in32(&ifc_global->csor_cs[cs].csor_ext); diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h index 3fdfede2f0f3..5f343b796ad9 100644 --- a/include/linux/fsl_ifc.h +++ b/include/linux/fsl_ifc.h @@ -274,6 +274,8 @@ */ /* Auto Boot Mode */ #define IFC_NAND_NCFGR_BOOT 0x80000000 +/* SRAM Initialization */ +#define IFC_NAND_NCFGR_SRAM_INIT_EN 0x20000000 /* Addressing Mode-ROW0+n/COL0 */ #define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x00000000 /* Addressing Mode-ROW0+n/COL0+n */ -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions 2018-08-06 9:21 ` [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions Kurt Kanzenbach @ 2018-08-08 9:48 ` Miquel Raynal 2018-08-08 10:09 ` Kurt Kanzenbach 0 siblings, 1 reply; 7+ messages in thread From: Miquel Raynal @ 2018-08-08 9:48 UTC (permalink / raw) To: Kurt Kanzenbach Cc: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel Hi Kurt, Subject prefix should be "mtd: rawnand: fsl_ifc:". Kurt Kanzenbach <kurt@linutronix.de> wrote on Mon, 6 Aug 2018 11:21:37 +0200: > Newer versions of the IFC controller use a different method of initializing the > internal SRAM: Instead of reading from flash, a bit in the NAND configuration > register has to be set in order to trigger the self-initializing process. > > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > --- > drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++++++++++++++++++ > include/linux/fsl_ifc.h | 2 ++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c > index e4f5792dc589..384d5e12b05c 100644 > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c > @@ -30,6 +30,7 @@ > #include <linux/mtd/partitions.h> > #include <linux/mtd/nand_ecc.h> > #include <linux/fsl_ifc.h> > +#include <linux/iopoll.h> > > #define ERR_BYTE 0xFF /* Value returned for read > bytes when read failed */ > @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) > uint32_t csor = 0, csor_8k = 0, csor_ext = 0; > uint32_t cs = priv->bank; > > + if (ctrl->version > FSL_IFC_VERSION_1_1_0) { This is redundant and fsl_ifc_sram_init() is called only if "ctrl->version > FSL_FC_VERSION_1_1_0". So this means this function has never worked? If this is the case, there should be at least a Fixes: tag. Maybe it would be cleaner to always call fsl_ifc_sram_init() from the probe(), and just exit with a "return 0" here if the version is old? (I'll let you choose the way you prefer). > + u32 ncfgr, status; > + int ret; > + > + /* Trigger auto initialization */ > + ncfgr = ifc_in32(&ifc_runtime->ifc_nand.ncfgr); > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, &ifc_runtime->ifc_nand.ncfgr); > + > + /* Wait until done */ > + ret = readx_poll_timeout(ifc_in32, &ifc_runtime->ifc_nand.ncfgr, > + status, !(status & IFC_NAND_NCFGR_SRAM_INIT_EN), > + 10, 1000); Nit: I always prefer when delays/timeouts are defined (and may be reused). > + if (ret) > + dev_err(priv->dev, "Failed to initialize SRAM!\n"); Space > + return ret; > + } > + > /* Save CSOR and CSOR_ext */ > csor = ifc_in32(&ifc_global->csor_cs[cs].csor); > csor_ext = ifc_in32(&ifc_global->csor_cs[cs].csor_ext); > diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h > index 3fdfede2f0f3..5f343b796ad9 100644 > --- a/include/linux/fsl_ifc.h > +++ b/include/linux/fsl_ifc.h > @@ -274,6 +274,8 @@ > */ > /* Auto Boot Mode */ > #define IFC_NAND_NCFGR_BOOT 0x80000000 > +/* SRAM Initialization */ > +#define IFC_NAND_NCFGR_SRAM_INIT_EN 0x20000000 > /* Addressing Mode-ROW0+n/COL0 */ > #define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x00000000 > /* Addressing Mode-ROW0+n/COL0+n */ Thanks, Miquèl ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions 2018-08-08 9:48 ` Miquel Raynal @ 2018-08-08 10:09 ` Kurt Kanzenbach 2018-08-08 12:33 ` Miquel Raynal 0 siblings, 1 reply; 7+ messages in thread From: Kurt Kanzenbach @ 2018-08-08 10:09 UTC (permalink / raw) To: Miquel Raynal Cc: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel Hi Miquel, On Wed, Aug 08, 2018 at 11:48:32AM +0200, Miquel Raynal wrote: > Hi Kurt, > > Subject prefix should be "mtd: rawnand: fsl_ifc:". okay, noted. > > Kurt Kanzenbach <kurt@linutronix.de> wrote on Mon, 6 Aug 2018 11:21:37 > +0200: > > > Newer versions of the IFC controller use a different method of initializing the > > internal SRAM: Instead of reading from flash, a bit in the NAND configuration > > register has to be set in order to trigger the self-initializing process. > > > > Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de> > > --- > > drivers/mtd/nand/raw/fsl_ifc_nand.c | 18 ++++++++++++++++++ > > include/linux/fsl_ifc.h | 2 ++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/drivers/mtd/nand/raw/fsl_ifc_nand.c b/drivers/mtd/nand/raw/fsl_ifc_nand.c > > index e4f5792dc589..384d5e12b05c 100644 > > --- a/drivers/mtd/nand/raw/fsl_ifc_nand.c > > +++ b/drivers/mtd/nand/raw/fsl_ifc_nand.c > > @@ -30,6 +30,7 @@ > > #include <linux/mtd/partitions.h> > > #include <linux/mtd/nand_ecc.h> > > #include <linux/fsl_ifc.h> > > +#include <linux/iopoll.h> > > > > #define ERR_BYTE 0xFF /* Value returned for read > > bytes when read failed */ > > @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) > > uint32_t csor = 0, csor_8k = 0, csor_ext = 0; > > uint32_t cs = priv->bank; > > > > + if (ctrl->version > FSL_IFC_VERSION_1_1_0) { > > This is redundant and fsl_ifc_sram_init() is called only if > "ctrl->version > FSL_FC_VERSION_1_1_0". No, it's not. It's called when ctrl->version >= FSL_IFC_VERSION_1_1_0. Therefore, this check is needed. > > So this means this function has never worked? It did work for e.g. IFC controller in version 1.1.0. However, it worked for the newer versions by accident, because U-Boot already initialized the SRAM correctly. If you boot without NAND initialization in U-Boot, then you'll hit the issue. > > If this is the case, there should be at least a Fixes: tag. > > Maybe it would be cleaner to always call fsl_ifc_sram_init() from the > probe(), and just exit with a "return 0" here if the version is old? > (I'll let you choose the way you prefer). Sounds like a good idea. Otherwise we have to check the version twice. > > > + u32 ncfgr, status; > > + int ret; > > + > > + /* Trigger auto initialization */ > > + ncfgr = ifc_in32(&ifc_runtime->ifc_nand.ncfgr); > > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, &ifc_runtime->ifc_nand.ncfgr); > > + > > + /* Wait until done */ > > + ret = readx_poll_timeout(ifc_in32, &ifc_runtime->ifc_nand.ncfgr, > > + status, !(status & IFC_NAND_NCFGR_SRAM_INIT_EN), > > + 10, 1000); > > Nit: I always prefer when delays/timeouts are defined (and may be > reused). Me too. I've missed that there is already a timeout constant IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one. > > > + if (ret) > > + dev_err(priv->dev, "Failed to initialize SRAM!\n"); > > Space okay. Thanks, Kurt > > > + return ret; > > + } > > + > > /* Save CSOR and CSOR_ext */ > > csor = ifc_in32(&ifc_global->csor_cs[cs].csor); > > csor_ext = ifc_in32(&ifc_global->csor_cs[cs].csor_ext); > > diff --git a/include/linux/fsl_ifc.h b/include/linux/fsl_ifc.h > > index 3fdfede2f0f3..5f343b796ad9 100644 > > --- a/include/linux/fsl_ifc.h > > +++ b/include/linux/fsl_ifc.h > > @@ -274,6 +274,8 @@ > > */ > > /* Auto Boot Mode */ > > #define IFC_NAND_NCFGR_BOOT 0x80000000 > > +/* SRAM Initialization */ > > +#define IFC_NAND_NCFGR_SRAM_INIT_EN 0x20000000 > > /* Addressing Mode-ROW0+n/COL0 */ > > #define IFC_NAND_NCFGR_ADDR_MODE_RC0 0x00000000 > > /* Addressing Mode-ROW0+n/COL0+n */ > > > Thanks, > Miquèl ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions 2018-08-08 10:09 ` Kurt Kanzenbach @ 2018-08-08 12:33 ` Miquel Raynal 2018-08-08 13:12 ` Kurt Kanzenbach 0 siblings, 1 reply; 7+ messages in thread From: Miquel Raynal @ 2018-08-08 12:33 UTC (permalink / raw) To: Kurt Kanzenbach Cc: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel Hi Kurt, > > > @@ -769,6 +770,23 @@ static int fsl_ifc_sram_init(struct fsl_ifc_mtd *priv) > > > uint32_t csor = 0, csor_8k = 0, csor_ext = 0; > > > uint32_t cs = priv->bank; > > > > > > + if (ctrl->version > FSL_IFC_VERSION_1_1_0) { > > > > This is redundant and fsl_ifc_sram_init() is called only if > > "ctrl->version > FSL_FC_VERSION_1_1_0". > > No, it's not. It's called when ctrl->version >= > FSL_IFC_VERSION_1_1_0. Therefore, this check is needed. Oh right, I missed the "=". > > > > > So this means this function has never worked? > > It did work for e.g. IFC controller in version 1.1.0. > > However, it worked for the newer versions by accident, because U-Boot > already initialized the SRAM correctly. If you boot without NAND > initialization in U-Boot, then you'll hit the issue. > > > > > If this is the case, there should be at least a Fixes: tag. > > > > Maybe it would be cleaner to always call fsl_ifc_sram_init() from the > > probe(), and just exit with a "return 0" here if the version is old? > > (I'll let you choose the way you prefer). > > Sounds like a good idea. Otherwise we have to check the version twice. > > > > > > + u32 ncfgr, status; > > > + int ret; > > > + > > > + /* Trigger auto initialization */ > > > + ncfgr = ifc_in32(&ifc_runtime->ifc_nand.ncfgr); > > > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, &ifc_runtime->ifc_nand.ncfgr); > > > + > > > + /* Wait until done */ > > > + ret = readx_poll_timeout(ifc_in32, &ifc_runtime->ifc_nand.ncfgr, > > > + status, !(status & IFC_NAND_NCFGR_SRAM_INIT_EN), > > > + 10, 1000); > > > > Nit: I always prefer when delays/timeouts are defined (and may be > > reused). > > Me too. I've missed that there is already a timeout constant > IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one. Well, I'm not bothered with huge timeouts, it's in the error path so we don't really care. Thanks, Miquèl ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions 2018-08-08 12:33 ` Miquel Raynal @ 2018-08-08 13:12 ` Kurt Kanzenbach 0 siblings, 0 replies; 7+ messages in thread From: Kurt Kanzenbach @ 2018-08-08 13:12 UTC (permalink / raw) To: Miquel Raynal Cc: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Masahiro Yamada, Gregory CLEMENT, Jane Wan, Jagdish Gediya, linux-mtd, linux-kernel Hi Miquel, On Wed, Aug 08, 2018 at 02:33:52PM +0200, Miquel Raynal wrote: > Hi Kurt, > > > > > > + u32 ncfgr, status; > > > > + int ret; > > > > + > > > > + /* Trigger auto initialization */ > > > > + ncfgr = ifc_in32(&ifc_runtime->ifc_nand.ncfgr); > > > > + ifc_out32(ncfgr | IFC_NAND_NCFGR_SRAM_INIT_EN, &ifc_runtime->ifc_nand.ncfgr); > > > > + > > > > + /* Wait until done */ > > > > + ret = readx_poll_timeout(ifc_in32, &ifc_runtime->ifc_nand.ncfgr, > > > > + status, !(status & IFC_NAND_NCFGR_SRAM_INIT_EN), > > > > + 10, 1000); > > > > > > Nit: I always prefer when delays/timeouts are defined (and may be > > > reused). > > > > Me too. I've missed that there is already a timeout constant > > IFC_TIMEOUT_MSECS (500). As it's huge, I'll add a second one. > > Well, I'm not bothered with huge timeouts, it's in the error path so we > don't really care. okay. I'll send a v2 next week addressing your comments. Thanks, Kurt ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-08-08 13:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-06 9:21 [PATCH 0/2] mtd: nand: fsl-ifc: fix SRAM initialization for newer controller Kurt Kanzenbach 2018-08-06 9:21 ` [PATCH 1/2] mtd: nand: fsl-ifc: check result of SRAM initialization Kurt Kanzenbach 2018-08-06 9:21 ` [PATCH 2/2] mtd: nand: fsl-ifc: fixup SRAM init for newer ctrl versions Kurt Kanzenbach 2018-08-08 9:48 ` Miquel Raynal 2018-08-08 10:09 ` Kurt Kanzenbach 2018-08-08 12:33 ` Miquel Raynal 2018-08-08 13:12 ` Kurt Kanzenbach
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox