| Message ID | 1396597089-1081-1-git-send-email-abrodkin@synopsys.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Apr 04, Alexey Brodkin wrote: > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > Maybe it would be nice adding some driver description here, so the commit log actually says something useful about the commit. [..] > Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > +/** > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > + * @host: Pointer to private data structure. > + * @flag: Bit/flag offset in INT_STATUS register > + */ > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > +{ > + unsigned int i; > + > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > + unsigned int status = reg_get(host, INT_STATUS); > + > + if (status & (1 << flag)) { > + reg_set(host, INT_CLR_STATUS, 1 << flag); > + return; > + } > + > + udelay(10); > + } > + > + /* > + * Since we cannot report this problem any further than > + * axs_nand_{write|read}_buf() letting user know there's a problem. > + */ > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > + AXS_FLAG_WAIT_DELAY, flag); > +} Hm... I'm not sure the above is really true. The NAND core uses the replaceable chip->waitfunc callback to check the status of issued commands. See for instance: static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page) { int status = 0; const uint8_t *buf = chip->oob_poi; int length = mtd->oobsize; chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page); chip->write_buf(mtd, buf, length); /* Send command to program the OOB data */ chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); status = chip->waitfunc(mtd, chip); return status & NAND_STATUS_FAIL ? -EIO : 0; } On the other side, if you are clearing the flags in axs_flag_wait_and_reset() it might be a bit hard for you to get this right. IOW, I'm not saying you *must* do this, but instead suggesting that you take a look at waitfunc() and see if it helps report a proper error in the read/write path.
Hi Ezequiel, On Fri, 2014-04-04 at 11:09 -0300, Ezequiel Garcia wrote: > On Apr 04, Alexey Brodkin wrote: > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > Maybe it would be nice adding some driver description here, so the commit > log actually says something useful about the commit. Well, I thought about it but frankly I'm not sure if there's anything else to add to commit title "driver for NAND controller used on Synopsys AXS dev boards". Do you think more info may make sense? > > +/** > > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > > + * @host: Pointer to private data structure. > > + * @flag: Bit/flag offset in INT_STATUS register > > + */ > > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > > + unsigned int status = reg_get(host, INT_STATUS); > > + > > + if (status & (1 << flag)) { > > + reg_set(host, INT_CLR_STATUS, 1 << flag); > > + return; > > + } > > + > > + udelay(10); > > + } > > + > > + /* > > + * Since we cannot report this problem any further than > > + * axs_nand_{write|read}_buf() letting user know there's a problem. > > + */ > > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > > + AXS_FLAG_WAIT_DELAY, flag); > > +} > > Hm... I'm not sure the above is really true. > > The NAND core uses the replaceable chip->waitfunc callback to check the > status of issued commands. See for instance: > > static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, > int page) > { > int status = 0; > const uint8_t *buf = chip->oob_poi; > int length = mtd->oobsize; > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page); > chip->write_buf(mtd, buf, length); > /* Send command to program the OOB data */ > chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); > > status = chip->waitfunc(mtd, chip); > > return status & NAND_STATUS_FAIL ? -EIO : 0; > } > > On the other side, if you are clearing the flags in axs_flag_wait_and_reset() > it might be a bit hard for you to get this right. > > IOW, I'm not saying you *must* do this, but instead suggesting that you take > a look at waitfunc() and see if it helps report a proper error in the > read/write path. As I may understand from generic implementation of "waitfunc" in "nand_base.c" it checks status of NAND chip itself - which IMHO makes sense. And this is done via NAND_CMD_STATUS command sent to chip through NAND controller. In AXS NAND driver you may see 2 types of wait checks: 1. NAND_ISR_CMDDONE - this flag means that NAND controller has just executed provided command (i.e. set proper signal on control lines of NAND chip etc), but it doesn't mean NAND chip itself has completed command execution. That's why "waitfunc" is not direct replacement here. 2. NAND_ISR_{RX|TX}DMACOMPLETE - this flag indicates completion of DMA transfer to/from NAND chip to RAM. "waitfunc" won't work here as well. I hope this explanation makes sense. Regards, Alexey
Hi Ezequiel, On Mon, 2014-04-07 at 10:13 +0400, Alexey Brodkin wrote: > Hi Ezequiel, > > On Fri, 2014-04-04 at 11:09 -0300, Ezequiel Garcia wrote: > > On Apr 04, Alexey Brodkin wrote: > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > > > > Maybe it would be nice adding some driver description here, so the commit > > log actually says something useful about the commit. > > Well, I thought about it but frankly I'm not sure if there's anything > else to add to commit title "driver for NAND controller used on Synopsys > AXS dev boards". > > Do you think more info may make sense? > > > > +/** > > > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > > > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > > > + * @host: Pointer to private data structure. > > > + * @flag: Bit/flag offset in INT_STATUS register > > > + */ > > > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > > > +{ > > > + unsigned int i; > > > + > > > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > > > + unsigned int status = reg_get(host, INT_STATUS); > > > + > > > + if (status & (1 << flag)) { > > > + reg_set(host, INT_CLR_STATUS, 1 << flag); > > > + return; > > > + } > > > + > > > + udelay(10); > > > + } > > > + > > > + /* > > > + * Since we cannot report this problem any further than > > > + * axs_nand_{write|read}_buf() letting user know there's a problem. > > > + */ > > > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > > > + AXS_FLAG_WAIT_DELAY, flag); > > > +} > > > > Hm... I'm not sure the above is really true. > > > > The NAND core uses the replaceable chip->waitfunc callback to check the > > status of issued commands. See for instance: > > > > static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, > > int page) > > { > > int status = 0; > > const uint8_t *buf = chip->oob_poi; > > int length = mtd->oobsize; > > > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page); > > chip->write_buf(mtd, buf, length); > > /* Send command to program the OOB data */ > > chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); > > > > status = chip->waitfunc(mtd, chip); > > > > return status & NAND_STATUS_FAIL ? -EIO : 0; > > } > > > > On the other side, if you are clearing the flags in axs_flag_wait_and_reset() > > it might be a bit hard for you to get this right. > > > > IOW, I'm not saying you *must* do this, but instead suggesting that you take > > a look at waitfunc() and see if it helps report a proper error in the > > read/write path. > > As I may understand from generic implementation of "waitfunc" in > "nand_base.c" it checks status of NAND chip itself - which IMHO makes > sense. And this is done via NAND_CMD_STATUS command sent to chip through > NAND controller. > > In AXS NAND driver you may see 2 types of wait checks: > 1. NAND_ISR_CMDDONE - this flag means that NAND controller has just > executed provided command (i.e. set proper signal on control lines of > NAND chip etc), but it doesn't mean NAND chip itself has completed > command execution. That's why "waitfunc" is not direct replacement here. > > 2. NAND_ISR_{RX|TX}DMACOMPLETE - this flag indicates completion of DMA > transfer to/from NAND chip to RAM. "waitfunc" won't work here as well. > > I hope this explanation makes sense. Please treat this message as a gentle reminder. I'm wondering if my explanation in the previous email makes any sense or I still need to fix stuff. Regards, Alexey
On Apr 11, Alexey Brodkin wrote: > Hi Ezequiel, > > On Mon, 2014-04-07 at 10:13 +0400, Alexey Brodkin wrote: > > Hi Ezequiel, > > > > On Fri, 2014-04-04 at 11:09 -0300, Ezequiel Garcia wrote: > > > On Apr 04, Alexey Brodkin wrote: > > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > > > > > > > Maybe it would be nice adding some driver description here, so the commit > > > log actually says something useful about the commit. > > > > Well, I thought about it but frankly I'm not sure if there's anything > > else to add to commit title "driver for NAND controller used on Synopsys > > AXS dev boards". > > > > Do you think more info may make sense? > > > > > > +/** > > > > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > > > > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > > > > + * @host: Pointer to private data structure. > > > > + * @flag: Bit/flag offset in INT_STATUS register > > > > + */ > > > > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > > > > +{ > > > > + unsigned int i; > > > > + > > > > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > > > > + unsigned int status = reg_get(host, INT_STATUS); > > > > + > > > > + if (status & (1 << flag)) { > > > > + reg_set(host, INT_CLR_STATUS, 1 << flag); > > > > + return; > > > > + } > > > > + > > > > + udelay(10); > > > > + } > > > > + > > > > + /* > > > > + * Since we cannot report this problem any further than > > > > + * axs_nand_{write|read}_buf() letting user know there's a problem. > > > > + */ > > > > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > > > > + AXS_FLAG_WAIT_DELAY, flag); > > > > +} > > > > > > Hm... I'm not sure the above is really true. > > > > > > The NAND core uses the replaceable chip->waitfunc callback to check the > > > status of issued commands. See for instance: > > > > > > static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, > > > int page) > > > { > > > int status = 0; > > > const uint8_t *buf = chip->oob_poi; > > > int length = mtd->oobsize; > > > > > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page); > > > chip->write_buf(mtd, buf, length); > > > /* Send command to program the OOB data */ > > > chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); > > > > > > status = chip->waitfunc(mtd, chip); > > > > > > return status & NAND_STATUS_FAIL ? -EIO : 0; > > > } > > > > > > On the other side, if you are clearing the flags in axs_flag_wait_and_reset() > > > it might be a bit hard for you to get this right. > > > > > > IOW, I'm not saying you *must* do this, but instead suggesting that you take > > > a look at waitfunc() and see if it helps report a proper error in the > > > read/write path. > > > > As I may understand from generic implementation of "waitfunc" in > > "nand_base.c" it checks status of NAND chip itself - which IMHO makes > > sense. And this is done via NAND_CMD_STATUS command sent to chip through > > NAND controller. > > > > In AXS NAND driver you may see 2 types of wait checks: > > 1. NAND_ISR_CMDDONE - this flag means that NAND controller has just > > executed provided command (i.e. set proper signal on control lines of > > NAND chip etc), but it doesn't mean NAND chip itself has completed > > command execution. That's why "waitfunc" is not direct replacement here. > > > > 2. NAND_ISR_{RX|TX}DMACOMPLETE - this flag indicates completion of DMA > > transfer to/from NAND chip to RAM. "waitfunc" won't work here as well. > > > > I hope this explanation makes sense. > > Please treat this message as a gentle reminder. > I'm wondering if my explanation in the previous email makes any sense or > I still need to fix stuff. Well, I was merely suggesting to look into using waitfunc(), so you definitely don't need to fix anything (at least from my side). Just as a comment, regarding your explanation above, I think you can override the waitfunc() and check the NAND_CMD_STATUS, together with the check for your ISR flags. However, you know more about your hardware than me, so this is *just* a suggestion. The driver looks fine broadly speaking. I guess it's up to Brian now to provide further feedback.
Dear Brian, On Fri, 2014-04-11 at 12:07 -0300, ezequiel.garcia@free-electrons.com wrote: > On Apr 11, Alexey Brodkin wrote: > > Hi Ezequiel, > > > > On Mon, 2014-04-07 at 10:13 +0400, Alexey Brodkin wrote: > > > Hi Ezequiel, > > > > > > On Fri, 2014-04-04 at 11:09 -0300, Ezequiel Garcia wrote: > > > > On Apr 04, Alexey Brodkin wrote: > > > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > > > > > > > > > > Maybe it would be nice adding some driver description here, so the commit > > > > log actually says something useful about the commit. > > > > > > Well, I thought about it but frankly I'm not sure if there's anything > > > else to add to commit title "driver for NAND controller used on Synopsys > > > AXS dev boards". > > > > > > Do you think more info may make sense? > > > > > > > > +/** > > > > > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > > > > > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > > > > > + * @host: Pointer to private data structure. > > > > > + * @flag: Bit/flag offset in INT_STATUS register > > > > > + */ > > > > > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > > > > > +{ > > > > > + unsigned int i; > > > > > + > > > > > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > > > > > + unsigned int status = reg_get(host, INT_STATUS); > > > > > + > > > > > + if (status & (1 << flag)) { > > > > > + reg_set(host, INT_CLR_STATUS, 1 << flag); > > > > > + return; > > > > > + } > > > > > + > > > > > + udelay(10); > > > > > + } > > > > > + > > > > > + /* > > > > > + * Since we cannot report this problem any further than > > > > > + * axs_nand_{write|read}_buf() letting user know there's a problem. > > > > > + */ > > > > > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > > > > > + AXS_FLAG_WAIT_DELAY, flag); > > > > > +} > > > > > > > > Hm... I'm not sure the above is really true. > > > > > > > > The NAND core uses the replaceable chip->waitfunc callback to check the > > > > status of issued commands. See for instance: > > > > > > > > static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, > > > > int page) > > > > { > > > > int status = 0; > > > > const uint8_t *buf = chip->oob_poi; > > > > int length = mtd->oobsize; > > > > > > > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page); > > > > chip->write_buf(mtd, buf, length); > > > > /* Send command to program the OOB data */ > > > > chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); > > > > > > > > status = chip->waitfunc(mtd, chip); > > > > > > > > return status & NAND_STATUS_FAIL ? -EIO : 0; > > > > } > > > > > > > > On the other side, if you are clearing the flags in axs_flag_wait_and_reset() > > > > it might be a bit hard for you to get this right. > > > > > > > > IOW, I'm not saying you *must* do this, but instead suggesting that you take > > > > a look at waitfunc() and see if it helps report a proper error in the > > > > read/write path. > > > > > > As I may understand from generic implementation of "waitfunc" in > > > "nand_base.c" it checks status of NAND chip itself - which IMHO makes > > > sense. And this is done via NAND_CMD_STATUS command sent to chip through > > > NAND controller. > > > > > > In AXS NAND driver you may see 2 types of wait checks: > > > 1. NAND_ISR_CMDDONE - this flag means that NAND controller has just > > > executed provided command (i.e. set proper signal on control lines of > > > NAND chip etc), but it doesn't mean NAND chip itself has completed > > > command execution. That's why "waitfunc" is not direct replacement here. > > > > > > 2. NAND_ISR_{RX|TX}DMACOMPLETE - this flag indicates completion of DMA > > > transfer to/from NAND chip to RAM. "waitfunc" won't work here as well. > > > > > > I hope this explanation makes sense. > > > > Please treat this message as a gentle reminder. > > I'm wondering if my explanation in the previous email makes any sense or > > I still need to fix stuff. > > Well, I was merely suggesting to look into using waitfunc(), so you definitely > don't need to fix anything (at least from my side). > > Just as a comment, regarding your explanation above, I think you can override > the waitfunc() and check the NAND_CMD_STATUS, together with the check for your > ISR flags. However, you know more about your hardware than me, so this is > *just* a suggestion. > > The driver looks fine broadly speaking. I guess it's up to Brian now to provide > further feedback. I'm wondering if there's a chance for you to look at this patch anytime soon? Regards, Alexey
Dear Brian, On Fri, 2014-04-18 at 02:12 +0400, Alexey Brodkin wrote: > Dear Brian, > > On Fri, 2014-04-11 at 12:07 -0300, ezequiel.garcia@free-electrons.com > wrote: > > On Apr 11, Alexey Brodkin wrote: > > > Hi Ezequiel, > > > > > > On Mon, 2014-04-07 at 10:13 +0400, Alexey Brodkin wrote: > > > > Hi Ezequiel, > > > > > > > > On Fri, 2014-04-04 at 11:09 -0300, Ezequiel Garcia wrote: > > > > > On Apr 04, Alexey Brodkin wrote: > > > > > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > > > > > > > > > > > > > Maybe it would be nice adding some driver description here, so the commit > > > > > log actually says something useful about the commit. > > > > > > > > Well, I thought about it but frankly I'm not sure if there's anything > > > > else to add to commit title "driver for NAND controller used on Synopsys > > > > AXS dev boards". > > > > > > > > Do you think more info may make sense? > > > > > > > > > > +/** > > > > > > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > > > > > > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > > > > > > + * @host: Pointer to private data structure. > > > > > > + * @flag: Bit/flag offset in INT_STATUS register > > > > > > + */ > > > > > > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > > > > > > +{ > > > > > > + unsigned int i; > > > > > > + > > > > > > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > > > > > > + unsigned int status = reg_get(host, INT_STATUS); > > > > > > + > > > > > > + if (status & (1 << flag)) { > > > > > > + reg_set(host, INT_CLR_STATUS, 1 << flag); > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + udelay(10); > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * Since we cannot report this problem any further than > > > > > > + * axs_nand_{write|read}_buf() letting user know there's a problem. > > > > > > + */ > > > > > > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > > > > > > + AXS_FLAG_WAIT_DELAY, flag); > > > > > > +} > > > > > > > > > > Hm... I'm not sure the above is really true. > > > > > > > > > > The NAND core uses the replaceable chip->waitfunc callback to check the > > > > > status of issued commands. See for instance: > > > > > > > > > > static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, > > > > > int page) > > > > > { > > > > > int status = 0; > > > > > const uint8_t *buf = chip->oob_poi; > > > > > int length = mtd->oobsize; > > > > > > > > > > chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page); > > > > > chip->write_buf(mtd, buf, length); > > > > > /* Send command to program the OOB data */ > > > > > chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); > > > > > > > > > > status = chip->waitfunc(mtd, chip); > > > > > > > > > > return status & NAND_STATUS_FAIL ? -EIO : 0; > > > > > } > > > > > > > > > > On the other side, if you are clearing the flags in axs_flag_wait_and_reset() > > > > > it might be a bit hard for you to get this right. > > > > > > > > > > IOW, I'm not saying you *must* do this, but instead suggesting that you take > > > > > a look at waitfunc() and see if it helps report a proper error in the > > > > > read/write path. > > > > > > > > As I may understand from generic implementation of "waitfunc" in > > > > "nand_base.c" it checks status of NAND chip itself - which IMHO makes > > > > sense. And this is done via NAND_CMD_STATUS command sent to chip through > > > > NAND controller. > > > > > > > > In AXS NAND driver you may see 2 types of wait checks: > > > > 1. NAND_ISR_CMDDONE - this flag means that NAND controller has just > > > > executed provided command (i.e. set proper signal on control lines of > > > > NAND chip etc), but it doesn't mean NAND chip itself has completed > > > > command execution. That's why "waitfunc" is not direct replacement here. > > > > > > > > 2. NAND_ISR_{RX|TX}DMACOMPLETE - this flag indicates completion of DMA > > > > transfer to/from NAND chip to RAM. "waitfunc" won't work here as well. > > > > > > > > I hope this explanation makes sense. > > > > > > Please treat this message as a gentle reminder. > > > I'm wondering if my explanation in the previous email makes any sense or > > > I still need to fix stuff. > > > > Well, I was merely suggesting to look into using waitfunc(), so you definitely > > don't need to fix anything (at least from my side). > > > > Just as a comment, regarding your explanation above, I think you can override > > the waitfunc() and check the NAND_CMD_STATUS, together with the check for your > > ISR flags. However, you know more about your hardware than me, so this is > > *just* a suggestion. > > > > The driver looks fine broadly speaking. I guess it's up to Brian now to provide > > further feedback. > > I'm wondering if there's a chance for you to look at this patch anytime > soon? Please treat this as another gentle reminder to review this patch and if there're no objections please consider to apply this one. Regards, Alexey
Hi Alexey, On Fri, Apr 04, 2014 at 11:38:09AM +0400, Alexey Brodkin wrote: > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > Reviewed-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > Cc: Vineet Gupta <vgupta@synopsys.com> > cc: Brian Norris <computersforpeace@gmail.com> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: Francois Bedard <fbedard@synopsys.com> > Cc: linux-kernel@vger.kernel.org > Cc: devicetree@vger.kernel.org > --- > > In this re-spin I addressed Ezequiel comments. > > Changes compared to v1: > * Minor code clean-up > * Fixed typos > * axs_flag_is_set() replaced with axs_flag_wait_and_reset() > * axs_flag_wait_and_reset() has built-in check for timeout instead of endless > while() loop initially used > > Documentation/devicetree/bindings/mtd/axs-nand.txt | 17 + > drivers/mtd/nand/Kconfig | 6 + > drivers/mtd/nand/Makefile | 1 + > drivers/mtd/nand/axs_nand.c | 411 +++++++++++++++++++++ > 4 files changed, 435 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/axs-nand.txt > create mode 100644 drivers/mtd/nand/axs_nand.c > > diff --git a/Documentation/devicetree/bindings/mtd/axs-nand.txt b/Documentation/devicetree/bindings/mtd/axs-nand.txt > new file mode 100644 > index 0000000..c522b7f > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/axs-nand.txt > @@ -0,0 +1,17 @@ > +* Synopsys AXS NAND controller > + > +Required properties: > + - compatible: must be "snps,axs-nand" > + - reg: physical base address and size of the registers map > + > +The device tree may optionally contain sub-nodes describing partitions of the > +address space. See partition.txt for more detail. > + > +Examples: > + > +flash@0x16000 { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "snps,axs-nand"; > + reg = < 0x16000 0x200 >; > +}; > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index 93ae6a6..3661822 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -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_AXS > + tristate "Support for NAND on Synopsys AXS development board" > + help > + Enables support for NAND Flash chips on Synopsys AXS development > + boards. > + > endif # MTD_NAND > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index 542b568..635a918 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -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_AXS) += axs_nand.o > > nand-objs := nand_base.o nand_bbt.o > diff --git a/drivers/mtd/nand/axs_nand.c b/drivers/mtd/nand/axs_nand.c > new file mode 100644 > index 0000000..9ee21b6 > --- /dev/null > +++ b/drivers/mtd/nand/axs_nand.c > @@ -0,0 +1,411 @@ > +/* > + * Copyright (C) 2014 Synopsys, Inc. (www.synopsys.com) > + * > + * Driver for NAND controller on Synopsys AXS development board. > + * > + * This file is subject to the terms and conditions of the GNU General Public > + * License v2. See the file COPYING in the main directory of this archive for > + * more details. > + */ > + > +#include <linux/delay.h> > +#include <linux/dma-mapping.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/mtd/mtd.h> > +#include <linux/mtd/nand.h> > +#include <linux/mtd/partitions.h> > +#include <linux/of_address.h> > +#include <linux/of_platform.h> > +#include <linux/io.h> > + > +/* > + * There's an issue with DMA'd data if data buffer is cached. > + * So to make NAND storage available for now we'll map data buffer in > + * uncached memory. What sort of issue? A hardware bug? I'd prefer to have a single implementation here if possible. > + * > + * As soon as issue with cached buffer is resolved following define to be > + * removed as well as sources it enables. > + */ > +#define DATA_BUFFER_UNCACHED > + > +#define BUS_WIDTH 8 /* AXI data bus width in bytes */ > + > +/* DMA buffer descriptor masks */ > +#define BD_STAT_OWN (1 << 31) > +#define BD_STAT_BD_FIRST (1 << 3) > +#define BD_STAT_BD_LAST (1 << 2) > +#define BD_SIZES_BUFFER1_MASK 0xfff > + > +#define BD_STAT_BD_COMPLETE (BD_STAT_BD_FIRST | BD_STAT_BD_LAST) > + > +/* Controller command types */ > +#define B_CT_ADDRESS (0x0 << 16) /* Address operation */ > +#define B_CT_COMMAND (0x1 << 16) /* Command operation */ > +#define B_CT_WRITE (0x2 << 16) /* Write operation */ > +#define B_CT_READ (0x3 << 16) /* Read operation */ > + > +/* Controller command options */ > +#define B_WFR (1 << 19) /* 1b - Wait for ready */ > +#define B_LC (1 << 18) /* 1b - Last cycle */ > +#define B_IWC (1 << 13) /* 1b - Interrupt when complete */ > + > +enum { > + NAND_ISR_DATAREQUIRED = 0, > + NAND_ISR_TXUNDERFLOW, > + NAND_ISR_TXOVERFLOW, > + NAND_ISR_DATAAVAILABLE, > + NAND_ISR_RXUNDERFLOW, > + NAND_ISR_RXOVERFLOW, > + NAND_ISR_TXDMACOMPLETE, > + NAND_ISR_RXDMACOMPLETE, > + NAND_ISR_DESCRIPTORUNAVAILABLE, > + NAND_ISR_CMDDONE, > + NAND_ISR_CMDAVAILABLE, > + NAND_ISR_CMDERROR, > + NAND_ISR_DATATRANSFEROVER, > + NAND_ISR_NONE > +}; > + > +enum { > + AC_FIFO = 0, /* Address and command fifo */ > + IDMAC_BDADDR = 0x18, /* IDMAC descriptor list base address */ > + INT_STATUS = 0x118, /* Interrupt status register */ > + INT_CLR_STATUS = 0x120 /* Interrupt clear status register */ > +}; > + > +#define AXS_BUF_SIZE (PAGE_ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE)) NAND_MAX_PAGESIZE and NAND_MAX_OOBSIZE have been removed. Please rebase on l2-mtd.git for testing. > + > +struct asx_nand_bd { > + __le32 status; /* DES0 */ > + __le32 sizes; /* DES1 */ > + dma_addr_t buffer_ptr0; /* DES2 */ > + dma_addr_t buffer_ptr1; /* DES3 */ > +}; > + > +struct axs_nand_host { > + struct nand_chip nand_chip; > + struct mtd_info mtd; > + void __iomem *io_base; > + struct device *dev; > + struct asx_nand_bd *bd; /* Buffer descriptor */ > + dma_addr_t bd_dma; /* DMA handle for buffer descriptor */ > + uint8_t *db; /* Data buffer */ > + dma_addr_t db_dma; /* DMA handle for data buffer */ > +}; > + > +/** > + * reg_set - Sets register with provided value. > + * @host: Pointer to private data structure. > + * @reg: Register offset from base address. > + * @value: Value to set in register. > + */ > +static inline void reg_set(struct axs_nand_host *host, int reg, int value) > +{ > + iowrite32(value, host->io_base + reg); > +} > + > +/** > + * reg_get - Gets value of specified register. > + * @host: Pointer to private data structure. > + * @reg: Register offset from base address. > + * > + * returns: Value of requested register. > + */ > +static inline unsigned int reg_get(struct axs_nand_host *host, int reg) > +{ > + return ioread32(host->io_base + reg); > +} > + > +/* Maximum number of milliseconds we wait for flag to appear */ > +#define AXS_FLAG_WAIT_DELAY 1000 > + > +/** > + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register > + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. > + * @host: Pointer to private data structure. > + * @flag: Bit/flag offset in INT_STATUS register > + */ > +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) > +{ > + unsigned int i; > + > + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { > + unsigned int status = reg_get(host, INT_STATUS); > + > + if (status & (1 << flag)) { > + reg_set(host, INT_CLR_STATUS, 1 << flag); > + return; > + } > + > + udelay(10); > + } > + > + /* > + * Since we cannot report this problem any further than > + * axs_nand_{write|read}_buf() letting user know there's a problem. > + */ > + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", > + AXS_FLAG_WAIT_DELAY, flag); > +} I see Ezequiel commented about using this vs. waitfunc. I think your usage is OK here. But do you really need a polling loop here? Does your hardware have any kind of interrupt mechanism for this sort of thing? It is not really good practice to do I/O this way, as it keeps your CPU busy doing pointless work. At a minimum, maybe you can use cond_resched()? > + > +/** > + * axs_nand_write_buf - write buffer to chip > + * @mtd: MTD device structure > + * @buf: Data buffer > + * @len: Number of bytes to write > + */ > +static void axs_nand_write_buf(struct mtd_info *mtd, > + const uint8_t *buf, int len) > +{ > + struct nand_chip *this = mtd->priv; > + struct axs_nand_host *host = this->priv; > + > + memcpy(host->db, buf, len); > +#ifndef DATA_BUFFER_UNCACHED > + dma_sync_single_for_device(host->dev, host->db_dma, len, DMA_TO_DEVICE); > +#endif > + > + /* Setup buffer descriptor */ > + host->bd->status = BD_STAT_OWN | BD_STAT_BD_COMPLETE; > + host->bd->sizes = cpu_to_le32(ALIGN(len, BUS_WIDTH) & > + BD_SIZES_BUFFER1_MASK); > + host->bd->buffer_ptr0 = cpu_to_le32(host->db_dma); > + host->bd->buffer_ptr1 = 0; > + > + /* Issue "write" command */ > + reg_set(host, AC_FIFO, B_CT_WRITE | B_WFR | B_IWC | B_LC | (len - 1)); > + > + /* Wait for NAND command and DMA to complete */ > + axs_flag_wait_and_reset(host, NAND_ISR_CMDDONE); > + axs_flag_wait_and_reset(host, NAND_ISR_TXDMACOMPLETE); > +} > + > +/** > + * axs_nand_read_buf - read chip data into buffer > + * @mtd: MTD device structure > + * @buf: Buffer to store data > + * @len: Number of bytes to read > + */ > +static void axs_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) > +{ > + struct nand_chip *this = mtd->priv; > + struct axs_nand_host *host = this->priv; > + > + /* Setup buffer descriptor */ > + host->bd->status = BD_STAT_OWN | BD_STAT_BD_COMPLETE; > + host->bd->sizes = cpu_to_le32(ALIGN(len, BUS_WIDTH) & > + BD_SIZES_BUFFER1_MASK); > + host->bd->buffer_ptr0 = cpu_to_le32(host->db_dma); > + host->bd->buffer_ptr1 = 0; > + > + /* Issue "read" command */ > + reg_set(host, AC_FIFO, B_CT_READ | B_WFR | B_IWC | B_LC | (len - 1)); > + > + /* Wait for NAND command and DMA to complete */ > + axs_flag_wait_and_reset(host, NAND_ISR_CMDDONE); > + axs_flag_wait_and_reset(host, NAND_ISR_RXDMACOMPLETE); > + > +#ifndef DATA_BUFFER_UNCACHED > + dma_sync_single_for_cpu(host->dev, host->db_dma, len, DMA_FROM_DEVICE); > +#endif > + if (buf != host->db) > + memcpy(buf, host->db, len); > +} > + > +/** > + * axs_nand_read_byte - read one byte from the chip > + * @mtd: MTD device structure > + * > + * returns: read data byte > + */ > +static uint8_t axs_nand_read_byte(struct mtd_info *mtd) > +{ > + struct nand_chip *this = mtd->priv; > + struct axs_nand_host *host = this->priv; > + > + axs_nand_read_buf(mtd, host->db, sizeof(uint8_t)); > + return (uint8_t)host->db[0]; > +} > + > +/** > + * axs_nand_read_word - read one word from the chip > + * @mtd: MTD device structure > + * > + * returns: read data word > + */ > +static uint16_t axs_nand_read_word(struct mtd_info *mtd) > +{ > + struct nand_chip *this = mtd->priv; > + struct axs_nand_host *host = this->priv; > + > + axs_nand_read_buf(mtd, host->db, sizeof(uint16_t)); > + return (uint16_t)host->db[0]; This doesn't look right. You don't get a "word" access simply by casting your uint8_t into uint16_t. It's possible you're looking for something more like this: return ((uint16_t *)host->db)[0]; But then, you'd need to worry about endianness, and how your controller handles x16 buswidth devices. Have you tested anything with word access, or is this just an untested implementation? > +} > + > +/** > + * axs_nand_cmd_ctrl - hardware specific access to control-lines > + * @mtd: MTD device structure > + * @cmd: NAND command > + * @ctrl: NAND control options > + */ > +static void axs_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, > + unsigned int ctrl) > +{ > + struct nand_chip *nand_chip = mtd->priv; > + struct axs_nand_host *host = nand_chip->priv; > + > + if (cmd == NAND_CMD_NONE) > + return; > + > + cmd = cmd & 0xff; > + > + switch (ctrl & (NAND_ALE | NAND_CLE)) { > + /* Address */ > + case NAND_ALE: > + cmd |= B_CT_ADDRESS; > + break; > + > + /* Command */ > + case NAND_CLE: > + cmd |= B_CT_COMMAND | B_WFR; > + > + break; > + > + default: > + dev_err(host->dev, "Unknown ctrl %#x\n", ctrl); > + return; > + } > + > + reg_set(host, AC_FIFO, cmd | B_LC); > + axs_flag_wait_and_reset(host, NAND_ISR_CMDDONE); > +} > + > +static int axs_nand_probe(struct platform_device *pdev) > +{ > + struct mtd_part_parser_data ppdata; > + struct nand_chip *nand_chip; > + struct axs_nand_host *host; > + struct resource res_regs; > + struct mtd_info *mtd; > + int err; > + > + if (!pdev->dev.of_node) > + return -ENODEV; If you follow my suggestions below, then this driver won't technically be dependent on device tree (you could potentially instantiate a platform_device through some other means), so maybe the above check will no longer be useful? I'm not saying you would *want* to be writing board files, but at least I don't think this driver really needs the check, if it's only handling generic device driver tasks. > + > + /* Get registers base address from device tree */ > + err = of_address_to_resource(pdev->dev.of_node, 0, &res_regs); Can you just use platform_get_resource()? I believe the OF core automagically converts register resources from device tree into platform resources for you. > + if (err) { > + dev_err(&pdev->dev, "Failed to retrieve registers base from device tree\n"); You won't need this error handling if you keep it close to devm_ioremap_resource(), which does error checks on the resource for you. See the comments above devm_ioremap_resource(). > + return -ENODEV; > + } > + > + /* Allocate memory for the device structure (and zero it) */ > + host = kzalloc(sizeof(struct axs_nand_host), GFP_KERNEL); Try devm_kzalloc()? That will save you some error handling. Also, you might try the 'sizeof' style from Chapter 14 of Documentation/CodingStyle: host = dev_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL); The same 'sizeof' pattern can be used elsewhere. > + if (!host) > + return -ENOMEM; > + > + host->io_base = devm_ioremap_resource(&pdev->dev, &res_regs); > + if (IS_ERR(host->io_base)) { > + err = PTR_ERR(host->io_base); > + goto out; > + } > + dev_dbg(&pdev->dev, "Registers base address is 0x%p\n", host->io_base); I don't think you need the '0x' prefix to %p, as it's done automatically for you (did you test the print?). You could also look at using the custom %pR or %pr format for struct resource. See Documentation/printk-formats.txt. > + > + host->bd = dmam_alloc_coherent(&pdev->dev, sizeof(struct asx_nand_bd), > + &host->bd_dma, GFP_KERNEL); Ditto about sizeof. > + if (!host->bd) { > + dev_err(&pdev->dev, "Failed to allocate buffer descriptor\n"); > + err = -ENOMEM; > + goto out; > + } > + > + memset(host->bd, 0, sizeof(struct asx_nand_bd)); Ditto about sizeof. > + > +#ifdef DATA_BUFFER_UNCACHED > + host->db = dmam_alloc_coherent(&pdev->dev, AXS_BUF_SIZE, > +#else > + host->db = dmam_alloc_noncoherent(&pdev->dev, AXS_BUF_SIZE, > +#endif > + &host->db_dma, GFP_KERNEL); > + if (!host->db) { > + dev_err(&pdev->dev, "Failed to allocate data buffer\n"); > + err = -ENOMEM; > + goto out; > + } > + dev_dbg(&pdev->dev, "Data buffer mapped @ %p, DMA @ %x\n", host->db, > + host->db_dma); For the 'dma_addr_t', you can try %pad. Again, see Documentation/printk-formats.txt > + > + mtd = &host->mtd; > + nand_chip = &host->nand_chip; > + host->dev = &pdev->dev; > + > + nand_chip->priv = host; > + mtd->priv = nand_chip; > + mtd->name = "axs_nand"; Are you sure there will only be a single 'axs_nand' on a single system? Or do you want to consider a unique naming system? Some drivers use: mtd->name = dev_name(&pdev->dev); But this could be less friendly if you have long device names based on Device Tree addresses. > + mtd->owner = THIS_MODULE; > + mtd->dev.parent = &pdev->dev; > + ppdata.of_node = pdev->dev.of_node; > + > + nand_chip->cmd_ctrl = axs_nand_cmd_ctrl; > + nand_chip->read_byte = axs_nand_read_byte; > + nand_chip->read_word = axs_nand_read_word; > + nand_chip->write_buf = axs_nand_write_buf; > + nand_chip->read_buf = axs_nand_read_buf; > + nand_chip->ecc.mode = NAND_ECC_SOFT; > + > + dev_set_drvdata(&pdev->dev, host); > + > + reg_set(host, IDMAC_BDADDR, host->bd_dma); > + > + err = nand_scan(mtd, 1); > + if (err) > + goto out1; > + > + err = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0); > + if (!err) > + return err; > + > + nand_release(mtd); > + > +out1: > + dev_set_drvdata(&pdev->dev, NULL); ^^ I believe this step is unnecessary now, as it's done by the driver core. > +out: > + kfree(host); > + return err; > +} > + > +static int axs_nand_remove(struct platform_device *ofdev) > +{ > + struct axs_nand_host *host = dev_get_drvdata(&ofdev->dev); > + struct mtd_info *mtd = &host->mtd; > + > + nand_release(mtd); > + dev_set_drvdata(&ofdev->dev, NULL); ^^ Same here > + kfree(host); > + > + return 0; > +} > + > +static const struct of_device_id axs_nand_dt_ids[] = { > + { .compatible = "snps,axs-nand", }, > + { /* Sentinel */ } > +}; > + > +MODULE_DEVICE_TABLE(of, axs_nand_match); > + > +static struct platform_driver axs_nand_driver = { > + .probe = axs_nand_probe, > + .remove = axs_nand_remove, > + .driver = { > + .name = "asx-nand", > + .owner = THIS_MODULE, > + .of_match_table = axs_nand_dt_ids, > + }, > +}; > + > +module_platform_driver(axs_nand_driver); > + > +MODULE_AUTHOR("Alexey Brodkin <abrodkin@synopsys.com>"); > +MODULE_DESCRIPTION("NAND driver for Synopsys AXS development board"); > +MODULE_LICENSE("GPL"); Brian
diff --git a/Documentation/devicetree/bindings/mtd/axs-nand.txt b/Documentation/devicetree/bindings/mtd/axs-nand.txt new file mode 100644 index 0000000..c522b7f --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/axs-nand.txt @@ -0,0 +1,17 @@ +* Synopsys AXS NAND controller + +Required properties: + - compatible: must be "snps,axs-nand" + - reg: physical base address and size of the registers map + +The device tree may optionally contain sub-nodes describing partitions of the +address space. See partition.txt for more detail. + +Examples: + +flash@0x16000 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "snps,axs-nand"; + reg = < 0x16000 0x200 >; +}; diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index 93ae6a6..3661822 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -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_AXS + tristate "Support for NAND on Synopsys AXS development board" + help + Enables support for NAND Flash chips on Synopsys AXS development + boards. + endif # MTD_NAND diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 542b568..635a918 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -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_AXS) += axs_nand.o nand-objs := nand_base.o nand_bbt.o diff --git a/drivers/mtd/nand/axs_nand.c b/drivers/mtd/nand/axs_nand.c new file mode 100644 index 0000000..9ee21b6 --- /dev/null +++ b/drivers/mtd/nand/axs_nand.c @@ -0,0 +1,411 @@ +/* + * Copyright (C) 2014 Synopsys, Inc. (www.synopsys.com) + * + * Driver for NAND controller on Synopsys AXS development board. + * + * This file is subject to the terms and conditions of the GNU General Public + * License v2. See the file COPYING in the main directory of this archive for + * more details. + */ + +#include <linux/delay.h> +#include <linux/dma-mapping.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/partitions.h> +#include <linux/of_address.h> +#include <linux/of_platform.h> +#include <linux/io.h> + +/* + * There's an issue with DMA'd data if data buffer is cached. + * So to make NAND storage available for now we'll map data buffer in + * uncached memory. + * + * As soon as issue with cached buffer is resolved following define to be + * removed as well as sources it enables. + */ +#define DATA_BUFFER_UNCACHED + +#define BUS_WIDTH 8 /* AXI data bus width in bytes */ + +/* DMA buffer descriptor masks */ +#define BD_STAT_OWN (1 << 31) +#define BD_STAT_BD_FIRST (1 << 3) +#define BD_STAT_BD_LAST (1 << 2) +#define BD_SIZES_BUFFER1_MASK 0xfff + +#define BD_STAT_BD_COMPLETE (BD_STAT_BD_FIRST | BD_STAT_BD_LAST) + +/* Controller command types */ +#define B_CT_ADDRESS (0x0 << 16) /* Address operation */ +#define B_CT_COMMAND (0x1 << 16) /* Command operation */ +#define B_CT_WRITE (0x2 << 16) /* Write operation */ +#define B_CT_READ (0x3 << 16) /* Read operation */ + +/* Controller command options */ +#define B_WFR (1 << 19) /* 1b - Wait for ready */ +#define B_LC (1 << 18) /* 1b - Last cycle */ +#define B_IWC (1 << 13) /* 1b - Interrupt when complete */ + +enum { + NAND_ISR_DATAREQUIRED = 0, + NAND_ISR_TXUNDERFLOW, + NAND_ISR_TXOVERFLOW, + NAND_ISR_DATAAVAILABLE, + NAND_ISR_RXUNDERFLOW, + NAND_ISR_RXOVERFLOW, + NAND_ISR_TXDMACOMPLETE, + NAND_ISR_RXDMACOMPLETE, + NAND_ISR_DESCRIPTORUNAVAILABLE, + NAND_ISR_CMDDONE, + NAND_ISR_CMDAVAILABLE, + NAND_ISR_CMDERROR, + NAND_ISR_DATATRANSFEROVER, + NAND_ISR_NONE +}; + +enum { + AC_FIFO = 0, /* Address and command fifo */ + IDMAC_BDADDR = 0x18, /* IDMAC descriptor list base address */ + INT_STATUS = 0x118, /* Interrupt status register */ + INT_CLR_STATUS = 0x120 /* Interrupt clear status register */ +}; + +#define AXS_BUF_SIZE (PAGE_ALIGN(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE)) + +struct asx_nand_bd { + __le32 status; /* DES0 */ + __le32 sizes; /* DES1 */ + dma_addr_t buffer_ptr0; /* DES2 */ + dma_addr_t buffer_ptr1; /* DES3 */ +}; + +struct axs_nand_host { + struct nand_chip nand_chip; + struct mtd_info mtd; + void __iomem *io_base; + struct device *dev; + struct asx_nand_bd *bd; /* Buffer descriptor */ + dma_addr_t bd_dma; /* DMA handle for buffer descriptor */ + uint8_t *db; /* Data buffer */ + dma_addr_t db_dma; /* DMA handle for data buffer */ +}; + +/** + * reg_set - Sets register with provided value. + * @host: Pointer to private data structure. + * @reg: Register offset from base address. + * @value: Value to set in register. + */ +static inline void reg_set(struct axs_nand_host *host, int reg, int value) +{ + iowrite32(value, host->io_base + reg); +} + +/** + * reg_get - Gets value of specified register. + * @host: Pointer to private data structure. + * @reg: Register offset from base address. + * + * returns: Value of requested register. + */ +static inline unsigned int reg_get(struct axs_nand_host *host, int reg) +{ + return ioread32(host->io_base + reg); +} + +/* Maximum number of milliseconds we wait for flag to appear */ +#define AXS_FLAG_WAIT_DELAY 1000 + +/** + * axs_flag_wait_and_reset - Waits until requested flag in INT_STATUS register + * is set by HW and resets it by writing "1" in INT_CLR_STATUS. + * @host: Pointer to private data structure. + * @flag: Bit/flag offset in INT_STATUS register + */ +static void axs_flag_wait_and_reset(struct axs_nand_host *host, int flag) +{ + unsigned int i; + + for (i = 0; i < AXS_FLAG_WAIT_DELAY * 100; i++) { + unsigned int status = reg_get(host, INT_STATUS); + + if (status & (1 << flag)) { + reg_set(host, INT_CLR_STATUS, 1 << flag); + return; + } + + udelay(10); + } + + /* + * Since we cannot report this problem any further than + * axs_nand_{write|read}_buf() letting user know there's a problem. + */ + dev_err(host->dev, "Waited too long (%d s.) for flag/bit %d\n", + AXS_FLAG_WAIT_DELAY, flag); +} + +/** + * axs_nand_write_buf - write buffer to chip + * @mtd: MTD device structure + * @buf: Data buffer + * @len: Number of bytes to write + */ +static void axs_nand_write_buf(struct mtd_info *mtd, + const uint8_t *buf, int len) +{ + struct nand_chip *this = mtd->priv; + struct axs_nand_host *host = this->priv; + + memcpy(host->db, buf, len); +#ifndef DATA_BUFFER_UNCACHED + dma_sync_single_for_device(host->dev, host->db_dma, len, DMA_TO_DEVICE); +#endif + + /* Setup buffer descriptor */ + host->bd->status = BD_STAT_OWN | BD_STAT_BD_COMPLETE; + host->bd->sizes = cpu_to_le32(ALIGN(len, BUS_WIDTH) & + BD_SIZES_BUFFER1_MASK); + host->bd->buffer_ptr0 = cpu_to_le32(host->db_dma); + host->bd->buffer_ptr1 = 0; + + /* Issue "write" command */ + reg_set(host, AC_FIFO, B_CT_WRITE | B_WFR | B_IWC | B_LC | (len - 1)); + + /* Wait for NAND command and DMA to complete */ + axs_flag_wait_and_reset(host, NAND_ISR_CMDDONE); + axs_flag_wait_and_reset(host, NAND_ISR_TXDMACOMPLETE); +} + +/** + * axs_nand_read_buf - read chip data into buffer + * @mtd: MTD device structure + * @buf: Buffer to store data + * @len: Number of bytes to read + */ +static void axs_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) +{ + struct nand_chip *this = mtd->priv; + struct axs_nand_host *host = this->priv; + + /* Setup buffer descriptor */ + host->bd->status = BD_STAT_OWN | BD_STAT_BD_COMPLETE; + host->bd->sizes = cpu_to_le32(ALIGN(len, BUS_WIDTH) & + BD_SIZES_BUFFER1_MASK); + host->bd->buffer_ptr0 = cpu_to_le32(host->db_dma); + host->bd->buffer_ptr1 = 0; + + /* Issue "read" command */ + reg_set(host, AC_FIFO, B_CT_READ | B_WFR | B_IWC | B_LC | (len - 1)); + + /* Wait for NAND command and DMA to complete */ + axs_flag_wait_and_reset(host, NAND_ISR_CMDDONE); + axs_flag_wait_and_reset(host, NAND_ISR_RXDMACOMPLETE); + +#ifndef DATA_BUFFER_UNCACHED + dma_sync_single_for_cpu(host->dev, host->db_dma, len, DMA_FROM_DEVICE); +#endif + if (buf != host->db) + memcpy(buf, host->db, len); +} + +/** + * axs_nand_read_byte - read one byte from the chip + * @mtd: MTD device structure + * + * returns: read data byte + */ +static uint8_t axs_nand_read_byte(struct mtd_info *mtd) +{ + struct nand_chip *this = mtd->priv; + struct axs_nand_host *host = this->priv; + + axs_nand_read_buf(mtd, host->db, sizeof(uint8_t)); + return (uint8_t)host->db[0]; +} + +/** + * axs_nand_read_word - read one word from the chip + * @mtd: MTD device structure + * + * returns: read data word + */ +static uint16_t axs_nand_read_word(struct mtd_info *mtd) +{ + struct nand_chip *this = mtd->priv; + struct axs_nand_host *host = this->priv; + + axs_nand_read_buf(mtd, host->db, sizeof(uint16_t)); + return (uint16_t)host->db[0]; +} + +/** + * axs_nand_cmd_ctrl - hardware specific access to control-lines + * @mtd: MTD device structure + * @cmd: NAND command + * @ctrl: NAND control options + */ +static void axs_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, + unsigned int ctrl) +{ + struct nand_chip *nand_chip = mtd->priv; + struct axs_nand_host *host = nand_chip->priv; + + if (cmd == NAND_CMD_NONE) + return; + + cmd = cmd & 0xff; + + switch (ctrl & (NAND_ALE | NAND_CLE)) { + /* Address */ + case NAND_ALE: + cmd |= B_CT_ADDRESS; + break; + + /* Command */ + case NAND_CLE: + cmd |= B_CT_COMMAND | B_WFR; + + break; + + default: + dev_err(host->dev, "Unknown ctrl %#x\n", ctrl); + return; + } + + reg_set(host, AC_FIFO, cmd | B_LC); + axs_flag_wait_and_reset(host, NAND_ISR_CMDDONE); +} + +static int axs_nand_probe(struct platform_device *pdev) +{ + struct mtd_part_parser_data ppdata; + struct nand_chip *nand_chip; + struct axs_nand_host *host; + struct resource res_regs; + struct mtd_info *mtd; + int err; + + if (!pdev->dev.of_node) + return -ENODEV; + + /* Get registers base address from device tree */ + err = of_address_to_resource(pdev->dev.of_node, 0, &res_regs); + if (err) { + dev_err(&pdev->dev, "Failed to retrieve registers base from device tree\n"); + return -ENODEV; + } + + /* Allocate memory for the device structure (and zero it) */ + host = kzalloc(sizeof(struct axs_nand_host), GFP_KERNEL); + if (!host) + return -ENOMEM; + + host->io_base = devm_ioremap_resource(&pdev->dev, &res_regs); + if (IS_ERR(host->io_base)) { + err = PTR_ERR(host->io_base); + goto out; + } + dev_dbg(&pdev->dev, "Registers base address is 0x%p\n", host->io_base); + + host->bd = dmam_alloc_coherent(&pdev->dev, sizeof(struct asx_nand_bd), + &host->bd_dma, GFP_KERNEL); + if (!host->bd) { + dev_err(&pdev->dev, "Failed to allocate buffer descriptor\n"); + err = -ENOMEM; + goto out; + } + + memset(host->bd, 0, sizeof(struct asx_nand_bd)); + +#ifdef DATA_BUFFER_UNCACHED + host->db = dmam_alloc_coherent(&pdev->dev, AXS_BUF_SIZE, +#else + host->db = dmam_alloc_noncoherent(&pdev->dev, AXS_BUF_SIZE, +#endif + &host->db_dma, GFP_KERNEL); + if (!host->db) { + dev_err(&pdev->dev, "Failed to allocate data buffer\n"); + err = -ENOMEM; + goto out; + } + dev_dbg(&pdev->dev, "Data buffer mapped @ %p, DMA @ %x\n", host->db, + host->db_dma); + + mtd = &host->mtd; + nand_chip = &host->nand_chip; + host->dev = &pdev->dev; + + nand_chip->priv = host; + mtd->priv = nand_chip; + mtd->name = "axs_nand"; + mtd->owner = THIS_MODULE; + mtd->dev.parent = &pdev->dev; + ppdata.of_node = pdev->dev.of_node; + + nand_chip->cmd_ctrl = axs_nand_cmd_ctrl; + nand_chip->read_byte = axs_nand_read_byte; + nand_chip->read_word = axs_nand_read_word; + nand_chip->write_buf = axs_nand_write_buf; + nand_chip->read_buf = axs_nand_read_buf; + nand_chip->ecc.mode = NAND_ECC_SOFT; + + dev_set_drvdata(&pdev->dev, host); + + reg_set(host, IDMAC_BDADDR, host->bd_dma); + + err = nand_scan(mtd, 1); + if (err) + goto out1; + + err = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0); + if (!err) + return err; + + nand_release(mtd); + +out1: + dev_set_drvdata(&pdev->dev, NULL); +out: + kfree(host); + return err; +} + +static int axs_nand_remove(struct platform_device *ofdev) +{ + struct axs_nand_host *host = dev_get_drvdata(&ofdev->dev); + struct mtd_info *mtd = &host->mtd; + + nand_release(mtd); + dev_set_drvdata(&ofdev->dev, NULL); + kfree(host); + + return 0; +} + +static const struct of_device_id axs_nand_dt_ids[] = { + { .compatible = "snps,axs-nand", }, + { /* Sentinel */ } +}; + +MODULE_DEVICE_TABLE(of, axs_nand_match); + +static struct platform_driver axs_nand_driver = { + .probe = axs_nand_probe, + .remove = axs_nand_remove, + .driver = { + .name = "asx-nand", + .owner = THIS_MODULE, + .of_match_table = axs_nand_dt_ids, + }, +}; + +module_platform_driver(axs_nand_driver); + +MODULE_AUTHOR("Alexey Brodkin <abrodkin@synopsys.com>"); +MODULE_DESCRIPTION("NAND driver for Synopsys AXS development board"); +MODULE_LICENSE("GPL");