* [PATCH v3 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller @ 2015-02-16 12:51 Maxime Ripard 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard 2015-02-16 12:51 ` [PATCH v3 2/2] ARM: mvebu: a385-db-ap: Enable the NAND Maxime Ripard 0 siblings, 2 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-16 12:51 UTC (permalink / raw) To: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris Cc: linux-mtd, Boris Brezillon, Thomas Petazzoni, linux-arm-kernel, linux-kernel, Tawfik Bayouk, Nadav Haklai, Lior Amsalem, Sudhakar Gundubogula, Seif Mazareeb, Maxime Ripard Hi, This patch serie enable the NAND support on the Armada 385 Access Point DB. In the process, some timeouts were found when we were accessing a freshly erased NAND page, which turned out to be an issue when draining the read FIFO where we were not following the datasheet. This has been fixed with the first patch, with stable CC'd. The second patch just enables the NAND controller in the DT. Thanks, Maxime Changes from v2: - Read the status bits only every 32 bytes read, and not 32 bits like was done before. - Changed the timeout routine code not use the jiffies that won't change in an interrupt context. Changes from v1: - Added a timeout to the busy waiting loop for RDDREQ Maxime Ripard (2): mtd: nand: pxa3xx: Fix PIO FIFO draining ARM: mvebu: a385-db-ap: Enable the NAND arch/arm/boot/dts/armada-385-db-ap.dts | 13 ++++++++++ drivers/mtd/nand/pxa3xx_nand.c | 47 +++++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 6 deletions(-) -- 2.3.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 12:51 [PATCH v3 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller Maxime Ripard @ 2015-02-16 12:51 ` Maxime Ripard 2015-02-16 13:34 ` Boris Brezillon ` (3 more replies) 2015-02-16 12:51 ` [PATCH v3 2/2] ARM: mvebu: a385-db-ap: Enable the NAND Maxime Ripard 1 sibling, 4 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-16 12:51 UTC (permalink / raw) To: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris Cc: linux-mtd, Boris Brezillon, Thomas Petazzoni, linux-arm-kernel, linux-kernel, Tawfik Bayouk, Nadav Haklai, Lior Amsalem, Sudhakar Gundubogula, Seif Mazareeb, Maxime Ripard, stable The NDDB register holds the data that are needed by the read and write commands. However, during a read PIO access, the datasheet specifies that after each 32 bits read in that register, when BCH is enabled, we have to make sure that the RDDREQ bit is set in the NDSR register. This fixes an issue that was seen on the Armada 385, and presumably other mvebu SoCs, when a read on a newly erased page would end up in the driver reporting a timeout from the NAND. Cc: <stable@vger.kernel.org> # v3.14 Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- drivers/mtd/nand/pxa3xx_nand.c | 47 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 96b0b1d27df1..b2d8d6960765 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -480,6 +480,41 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask) nand_writel(info, NDCR, ndcr | int_mask); } +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len) +{ + if (info->ecc_bch) { + int index = 0; + + while (index < (len * 4)) { + u32 timeout; + + __raw_readsl(info->mmio_base + NDDB, data + index, 8); + + /* + * According to the datasheet, when reading + * from NDDB with BCH enabled, after each 32 + * bytes reads, we have to make sure that the + * NDSR.RDDREQ bit is set + */ + for (timeout = 0; + !(nand_readl(info, NDSR) & NDSR_RDDREQ); + timeout++) { + if (timeout >= 5) { + dev_err(&info->pdev->dev, + "Timeout on RDDREQ while draining the FIFO\n"); + return; + } + + mdelay(1); + } + + index += 32; + } + } else { + __raw_readsl(info->mmio_base + NDDB, data, len); + } +} + static void handle_data_pio(struct pxa3xx_nand_info *info) { unsigned int do_bytes = min(info->data_size, info->chunk_size); @@ -496,14 +531,14 @@ static void handle_data_pio(struct pxa3xx_nand_info *info) DIV_ROUND_UP(info->oob_size, 4)); break; case STATE_PIO_READING: - __raw_readsl(info->mmio_base + NDDB, - info->data_buff + info->data_buff_pos, - DIV_ROUND_UP(do_bytes, 4)); + drain_fifo(info, + info->data_buff + info->data_buff_pos, + DIV_ROUND_UP(do_bytes, 4)); if (info->oob_size > 0) - __raw_readsl(info->mmio_base + NDDB, - info->oob_buff + info->oob_buff_pos, - DIV_ROUND_UP(info->oob_size, 4)); + drain_fifo(info, + info->oob_buff + info->oob_buff_pos, + DIV_ROUND_UP(info->oob_size, 4)); break; default: dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__, -- 2.3.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard @ 2015-02-16 13:34 ` Boris Brezillon 2015-02-16 13:35 ` Ezequiel Garcia ` (2 subsequent siblings) 3 siblings, 0 replies; 19+ messages in thread From: Boris Brezillon @ 2015-02-16 13:34 UTC (permalink / raw) To: Maxime Ripard Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, linux-mtd, Boris Brezillon, Thomas Petazzoni, linux-arm-kernel, linux-kernel, Tawfik Bayouk, Nadav Haklai, Lior Amsalem, Sudhakar Gundubogula, Seif Mazareeb, stable Hi Maxime, On Mon, 16 Feb 2015 13:51:11 +0100 Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > The NDDB register holds the data that are needed by the read and write > commands. > > However, during a read PIO access, the datasheet specifies that after each 32 > bits read in that register, when BCH is enabled, we have to make sure that the > RDDREQ bit is set in the NDSR register. > > This fixes an issue that was seen on the Armada 385, and presumably other mvebu > SoCs, when a read on a newly erased page would end up in the driver reporting a > timeout from the NAND. > > Cc: <stable@vger.kernel.org> # v3.14 > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > drivers/mtd/nand/pxa3xx_nand.c | 47 ++++++++++++++++++++++++++++++++++++------ > 1 file changed, 41 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index 96b0b1d27df1..b2d8d6960765 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -480,6 +480,41 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask) > nand_writel(info, NDCR, ndcr | int_mask); > } > > +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len) > +{ > + if (info->ecc_bch) { > + int index = 0; > + > + while (index < (len * 4)) { > + u32 timeout; > + > + __raw_readsl(info->mmio_base + NDDB, data + index, 8); > + Shouldn't you break here if you've read all the data you were expecting ? As I said in my previous review, I don't know what's happening if you wait for RDDREQ when the FIFO has been fully drained. > + /* > + * According to the datasheet, when reading > + * from NDDB with BCH enabled, after each 32 > + * bytes reads, we have to make sure that the > + * NDSR.RDDREQ bit is set > + */ > + for (timeout = 0; > + !(nand_readl(info, NDSR) & NDSR_RDDREQ); > + timeout++) { > + if (timeout >= 5) { > + dev_err(&info->pdev->dev, > + "Timeout on RDDREQ while draining the FIFO\n"); > + return; > + } > + > + mdelay(1); > + } > + > + index += 32; > + } > + } else { > + __raw_readsl(info->mmio_base + NDDB, data, len); > + } > +} Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard 2015-02-16 13:34 ` Boris Brezillon @ 2015-02-16 13:35 ` Ezequiel Garcia 2015-02-16 16:49 ` Maxime Ripard 2015-02-16 16:27 ` Thomas Petazzoni 2015-02-16 20:11 ` Robert Jarzmik 3 siblings, 1 reply; 19+ messages in thread From: Ezequiel Garcia @ 2015-02-16 13:35 UTC (permalink / raw) To: Maxime Ripard, Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Brian Norris Cc: linux-mtd, Boris Brezillon, Thomas Petazzoni, linux-arm-kernel, linux-kernel, Tawfik Bayouk, Nadav Haklai, Lior Amsalem, Sudhakar Gundubogula, Seif Mazareeb, stable On 02/16/2015 09:51 AM, Maxime Ripard wrote: > The NDDB register holds the data that are needed by the read and write > commands. > > However, during a read PIO access, the datasheet specifies that after each 32 > bits read in that register, when BCH is enabled, we have to make sure that the > RDDREQ bit is set in the NDSR register. > Typo s/32 bits/32 bytes -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 13:35 ` Ezequiel Garcia @ 2015-02-16 16:49 ` Maxime Ripard 0 siblings, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-16 16:49 UTC (permalink / raw) To: Ezequiel Garcia Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Brian Norris, linux-mtd, Boris Brezillon, Thomas Petazzoni, linux-arm-kernel, linux-kernel, Tawfik Bayouk, Nadav Haklai, Lior Amsalem, Sudhakar Gundubogula, Seif Mazareeb, stable [-- Attachment #1: Type: text/plain, Size: 604 bytes --] On Mon, Feb 16, 2015 at 10:35:50AM -0300, Ezequiel Garcia wrote: > On 02/16/2015 09:51 AM, Maxime Ripard wrote: > > The NDDB register holds the data that are needed by the read and write > > commands. > > > > However, during a read PIO access, the datasheet specifies that after each 32 > > bits read in that register, when BCH is enabled, we have to make sure that the > > RDDREQ bit is set in the NDSR register. > > > > Typo s/32 bits/32 bytes Good catch, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard 2015-02-16 13:34 ` Boris Brezillon 2015-02-16 13:35 ` Ezequiel Garcia @ 2015-02-16 16:27 ` Thomas Petazzoni 2015-02-16 16:41 ` Maxime Ripard 2015-02-16 20:11 ` Robert Jarzmik 3 siblings, 1 reply; 19+ messages in thread From: Thomas Petazzoni @ 2015-02-16 16:27 UTC (permalink / raw) To: Maxime Ripard Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel Dear Maxime Ripard, On Mon, 16 Feb 2015 13:51:11 +0100, Maxime Ripard wrote: > + while (index < (len * 4)) { > + u32 timeout; > + > + __raw_readsl(info->mmio_base + NDDB, data + index, 8); Are you guaranteed that 'len' is a multiple of 32 bytes? Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 16:27 ` Thomas Petazzoni @ 2015-02-16 16:41 ` Maxime Ripard 2015-02-16 16:57 ` Ezequiel Garcia 0 siblings, 1 reply; 19+ messages in thread From: Maxime Ripard @ 2015-02-16 16:41 UTC (permalink / raw) To: Thomas Petazzoni Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 621 bytes --] On Mon, Feb 16, 2015 at 05:27:53PM +0100, Thomas Petazzoni wrote: > Dear Maxime Ripard, > > On Mon, 16 Feb 2015 13:51:11 +0100, Maxime Ripard wrote: > > > + while (index < (len * 4)) { > > + u32 timeout; > > + > > + __raw_readsl(info->mmio_base + NDDB, data + index, 8); > > Are you guaranteed that 'len' is a multiple of 32 bytes? I don't know if you're guaranteed of anything, but the controller supports only 512, 2k, 4k and 8k pages, which are all mutiples of 32 bytes. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 16:41 ` Maxime Ripard @ 2015-02-16 16:57 ` Ezequiel Garcia 2015-02-17 10:29 ` Maxime Ripard 0 siblings, 1 reply; 19+ messages in thread From: Ezequiel Garcia @ 2015-02-16 16:57 UTC (permalink / raw) To: Maxime Ripard, Thomas Petazzoni Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 02/16/2015 01:41 PM, Maxime Ripard wrote: > On Mon, Feb 16, 2015 at 05:27:53PM +0100, Thomas Petazzoni wrote: >> Dear Maxime Ripard, >> >> On Mon, 16 Feb 2015 13:51:11 +0100, Maxime Ripard wrote: >> >>> + while (index < (len * 4)) { >>> + u32 timeout; >>> + >>> + __raw_readsl(info->mmio_base + NDDB, data + index, 8); >> >> Are you guaranteed that 'len' is a multiple of 32 bytes? > > I don't know if you're guaranteed of anything, but the controller > supports only 512, 2k, 4k and 8k pages, which are all mutiples of 32 > bytes. > 'len' here comes from: do_bytes = min(info->data_size, info->chunk_size); and DIV_ROUND_UP(do_bytes, 4) Where chunk_size is the size we want to read/write in each command step (keep in mind that with extended commands we issue multiple commands, and read/write data in chunks for each page). And data_size is initialized at mtd->writesize (i.e. the size of a page). Given all the flash pages I'm aware of are multiples of 32-bytes, and given a chunk is either a quarter or half a page... I'd say it's guaranteed to be 32-byte multiple, but perhaps it's a good idea to enforce it. - -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJU4iFfAAoJEIOKbhOEIHKiOuMQAK0yiPyjBKRcqX8qrpG9Ljcq JVhJTjn7VdiWhoh0n9BOt5bV3K0wAvcbt3LZvpGwf1EOifBaZB+f2QskZNLDUXyC JXPaonbdUqabEU0n9frduc9xBgbPhrwL4X0RzbJ0xZ+A2FrPt/80qUe8lsDmykH9 dyl3FOL3EQQiQ83D1VefkYbeDjaunvhA7Lfi7CcdPSFRv1FE47NQUW/8OjvZVczx uPcvdNj4818aXtFyOJQbR9xWOhVh7nxPlU8flHZPHuJ5WVCGWBbt++/4vmK+LZkv aZQ8W6dGiKI3ayT+PQ7nsETmoXZcjWTihq+nW+Ie2vs5PZf1iME5RYarLSKsc0Ac 4GjLnd4+0H3jeInvJ0MLw0dhkYM4PLkzp4CPo4vrH8z5F3cLXxaRkZYuv7gChden C2VITr9C8p1OSQJ2mF8m9gWdExkEuuy7q6vURx74C4KaeQA2R4ARAROm85o6JtmN dhozZIFrJQGwGuB5+7MI3yJj4OpFsBkxoq6U1JNDTwYnu3SnMOdwvq9kwqGXgR2I yQlu6MO6DYHkMtmw//kkqX+vnyhGexrFoesOyG4d40mOgyGYqyk+oadV7pNh2g5Y nXGr21Li80N65Sk+RaOFlvmIPaQ45Xn6gS7ckHcVVCZI9HAu87n5n15HEtfaj4Dc r9FkTUgw9cqcio5EVfEs =nkDF -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 16:57 ` Ezequiel Garcia @ 2015-02-17 10:29 ` Maxime Ripard 0 siblings, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-17 10:29 UTC (permalink / raw) To: Ezequiel Garcia Cc: Thomas Petazzoni, Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 1568 bytes --] On Mon, Feb 16, 2015 at 01:57:12PM -0300, Ezequiel Garcia wrote: > On 02/16/2015 01:41 PM, Maxime Ripard wrote: > > On Mon, Feb 16, 2015 at 05:27:53PM +0100, Thomas Petazzoni wrote: > >> Dear Maxime Ripard, > >> > >> On Mon, 16 Feb 2015 13:51:11 +0100, Maxime Ripard wrote: > >> > >>> + while (index < (len * 4)) { > >>> + u32 timeout; > >>> + > >>> + __raw_readsl(info->mmio_base + NDDB, data + index, 8); > >> > >> Are you guaranteed that 'len' is a multiple of 32 bytes? > > > > I don't know if you're guaranteed of anything, but the controller > > supports only 512, 2k, 4k and 8k pages, which are all mutiples of 32 > > bytes. > > > > 'len' here comes from: > > do_bytes = min(info->data_size, info->chunk_size); > > and > > DIV_ROUND_UP(do_bytes, 4) > > Where chunk_size is the size we want to read/write in each command step > (keep in mind that with extended commands we issue multiple commands, and > read/write data in chunks for each page). > > And data_size is initialized at mtd->writesize (i.e. the size of a page). > > Given all the flash pages I'm aware of are multiples of 32-bytes, and > given a chunk is either a quarter or half a page... I'd say it's > guaranteed to be 32-byte multiple, but perhaps it's a good idea to enforce it. I've fixed the function to both support non-aligned reading, just in case, and to not poll on the last chunk, as Boris suggested. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard ` (2 preceding siblings ...) 2015-02-16 16:27 ` Thomas Petazzoni @ 2015-02-16 20:11 ` Robert Jarzmik 2015-02-16 20:58 ` Maxime Ripard 3 siblings, 1 reply; 19+ messages in thread From: Robert Jarzmik @ 2015-02-16 20:11 UTC (permalink / raw) To: Maxime Ripard Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel Maxime Ripard <maxime.ripard@free-electrons.com> writes: > drivers/mtd/nand/pxa3xx_nand.c | 47 ++++++++++++++++++++++++++++++++++++------ > 1 file changed, 41 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index 96b0b1d27df1..b2d8d6960765 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -480,6 +480,41 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask) > nand_writel(info, NDCR, ndcr | int_mask); > } > > +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len) > +{ > + if (info->ecc_bch) { > + int index = 0; > + > + while (index < (len * 4)) { > + u32 timeout; > + > + __raw_readsl(info->mmio_base + NDDB, data + index, 8); > + > + /* > + * According to the datasheet, when reading > + * from NDDB with BCH enabled, after each 32 > + * bytes reads, we have to make sure that the > + * NDSR.RDDREQ bit is set > + */ > + for (timeout = 0; > + !(nand_readl(info, NDSR) & NDSR_RDDREQ); > + timeout++) { > + if (timeout >= 5) { > + dev_err(&info->pdev->dev, > + "Timeout on RDDREQ while draining the FIFO\n"); > + return; > + } > + > + mdelay(1); So in worst case, we'll end up with 4 times mdelay(1) times len / 32. For a 2048 page, it is : 256ms where everything is stuck (mdelay and not msleep). I know you had no choice because this is called from interrupt handler (top half). But having a irq handler and a irq thread handler would solve that issue, and you'll end up with msleep(1) in this code. I don't think an mdelay(256) is acceptable. Cheers. -- Robert ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 20:11 ` Robert Jarzmik @ 2015-02-16 20:58 ` Maxime Ripard 2015-02-16 21:36 ` Robert Jarzmik 0 siblings, 1 reply; 19+ messages in thread From: Maxime Ripard @ 2015-02-16 20:58 UTC (permalink / raw) To: Robert Jarzmik Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 2358 bytes --] Hi Robert, On Mon, Feb 16, 2015 at 09:11:24PM +0100, Robert Jarzmik wrote: > Maxime Ripard <maxime.ripard@free-electrons.com> writes: > > > drivers/mtd/nand/pxa3xx_nand.c | 47 ++++++++++++++++++++++++++++++++++++------ > > 1 file changed, 41 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > > index 96b0b1d27df1..b2d8d6960765 100644 > > --- a/drivers/mtd/nand/pxa3xx_nand.c > > +++ b/drivers/mtd/nand/pxa3xx_nand.c > > @@ -480,6 +480,41 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask) > > nand_writel(info, NDCR, ndcr | int_mask); > > } > > > > +static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len) > > +{ > > + if (info->ecc_bch) { > > + int index = 0; > > + > > + while (index < (len * 4)) { > > + u32 timeout; > > + > > + __raw_readsl(info->mmio_base + NDDB, data + index, 8); > > + > > + /* > > + * According to the datasheet, when reading > > + * from NDDB with BCH enabled, after each 32 > > + * bytes reads, we have to make sure that the > > + * NDSR.RDDREQ bit is set > > + */ > > + for (timeout = 0; > > + !(nand_readl(info, NDSR) & NDSR_RDDREQ); > > + timeout++) { > > + if (timeout >= 5) { > > + dev_err(&info->pdev->dev, > > + "Timeout on RDDREQ while draining the FIFO\n"); > > + return; > > + } > > + > > + mdelay(1); > So in worst case, we'll end up with 4 times mdelay(1) times len / 32. > For a 2048 page, it is : 256ms where everything is stuck (mdelay and not > msleep). > > I know you had no choice because this is called from interrupt handler (top > half). But having a irq handler and a irq thread handler would solve that issue, > and you'll end up with msleep(1) in this code. > > I don't think an mdelay(256) is acceptable. That's very true that this driver would need some love, but valentine's day was last week. I'm sorry, but this is a patch targeted for stable. This is a pure bugfix. I won't rewrite the whole driver solely to make the driver better, especially since that would make such a patch (or more likely a whole serie) unsuitable for stable. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 20:58 ` Maxime Ripard @ 2015-02-16 21:36 ` Robert Jarzmik 2015-02-17 9:47 ` Maxime Ripard 2015-02-17 10:37 ` Maxime Ripard 0 siblings, 2 replies; 19+ messages in thread From: Robert Jarzmik @ 2015-02-16 21:36 UTC (permalink / raw) To: Maxime Ripard Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel Maxime Ripard <maxime.ripard@free-electrons.com> writes: >> I don't think an mdelay(256) is acceptable. > > That's very true that this driver would need some love, but > valentine's day was last week. That doesn't cope with the 256ms mdelay. And a potential big mdelay is not what I'd call a bug fix, see below. > I'm sorry, but this is a patch targeted for stable. This is a pure > bugfix. I won't rewrite the whole driver solely to make the driver > better, especially since that would make such a patch (or more likely > a whole serie) unsuitable for stable. This is the rewrite I was asking for (not tested), consider it against your "rewrite the whole driver" : Modified drivers/mtd/nand/pxa3xx_nand.c diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index e512902..6e569e9 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -576,11 +576,20 @@ static void start_data_dma(struct pxa3xx_nand_info *info) {} #endif +static irqreturn_t pxa3xx_nand_irq_thread(int irq, void *data) +{ + struct pxa3xx_nand_info *info = data; + + handle_data_pio(info); + return IRQ_HANDLED; +} + static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) { struct pxa3xx_nand_info *info = devid; unsigned int status, is_completed = 0, is_ready = 0; unsigned int ready, cmd_done; + irqreturn_t ret = IRQ_HANDLED; if (info->cs == 0) { ready = NDSR_FLASH_RDY; @@ -622,7 +631,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) } else { info->state = (status & NDSR_RDDREQ) ? STATE_PIO_READING : STATE_PIO_WRITING; - handle_data_pio(info); + ret = IRQ_WAKE_THREAD; } } if (status & cmd_done) { @@ -663,7 +672,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) if (is_ready) complete(&info->dev_ready); NORMAL_IRQ_EXIT: - return IRQ_HANDLED; + return ret; } static inline int is_buf_blank(uint8_t *buf, size_t len) @@ -1688,7 +1697,8 @@ static int alloc_nand_resource(struct platform_device *pdev) /* initialize all interrupts to be disabled */ disable_int(info, NDSR_MASK); - ret = request_irq(irq, pxa3xx_nand_irq, 0, pdev->name, info); + ret = request_threaded_irq(irq, pxa3xx_nand_irq, + pxa3xx_nand_irq_thread, 0, pdev->name, info); if (ret < 0) { dev_err(&pdev->dev, "failed to request IRQ\n"); goto fail_free_buf; -- Robert ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 21:36 ` Robert Jarzmik @ 2015-02-17 9:47 ` Maxime Ripard 2015-02-17 10:37 ` Maxime Ripard 1 sibling, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-17 9:47 UTC (permalink / raw) To: Robert Jarzmik Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 3880 bytes --] On Mon, Feb 16, 2015 at 10:36:02PM +0100, Robert Jarzmik wrote: > Maxime Ripard <maxime.ripard@free-electrons.com> writes: > > >> I don't think an mdelay(256) is acceptable. > > > > That's very true that this driver would need some love, but > > valentine's day was last week. > > That doesn't cope with the 256ms mdelay. And a potential big mdelay > is not what I'd call a bug fix, see below. I really don't care about the delay itself, really. Just splitting the readsl into several chunks seems to slow the FIFO draining enough so that we don't even need to poll the RDDREQ bit. I was asked (rightfully) for a timeout, and since there's no particular indication in the datasheet on this, I came up with a totally random and made-up number. If you want to suggest any other random number, I'd be happy to use it. Plus, saying that this is strictly equivalent to mdelay(256) is just bad faith. It is a (very very very unlikely) worst case scenario. During my tests, I've never experienced even a single delay being used, let alone every single reads needing to poll the bit for 5ms, always succeeding at the last attempt. If we really care about this, we might as well start caring about other theoretically possible situations, such as the NAND chip being ripped off the board by an asteroid. > > I'm sorry, but this is a patch targeted for stable. This is a pure > > bugfix. I won't rewrite the whole driver solely to make the driver > > better, especially since that would make such a patch (or more likely > > a whole serie) unsuitable for stable. > > This is the rewrite I was asking for (not tested), consider it against your > "rewrite the whole driver" : > > Modified drivers/mtd/nand/pxa3xx_nand.c > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index e512902..6e569e9 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -576,11 +576,20 @@ static void start_data_dma(struct pxa3xx_nand_info *info) > {} > #endif > > +static irqreturn_t pxa3xx_nand_irq_thread(int irq, void *data) > +{ > + struct pxa3xx_nand_info *info = data; > + > + handle_data_pio(info); > + return IRQ_HANDLED; > +} > + > static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) > { > struct pxa3xx_nand_info *info = devid; > unsigned int status, is_completed = 0, is_ready = 0; > unsigned int ready, cmd_done; > + irqreturn_t ret = IRQ_HANDLED; > > if (info->cs == 0) { > ready = NDSR_FLASH_RDY; > @@ -622,7 +631,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) > } else { > info->state = (status & NDSR_RDDREQ) ? > STATE_PIO_READING : STATE_PIO_WRITING; > - handle_data_pio(info); > + ret = IRQ_WAKE_THREAD; > } > } > if (status & cmd_done) { > @@ -663,7 +672,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) > if (is_ready) > complete(&info->dev_ready); > NORMAL_IRQ_EXIT: > - return IRQ_HANDLED; > + return ret; > } > > static inline int is_buf_blank(uint8_t *buf, size_t len) > @@ -1688,7 +1697,8 @@ static int alloc_nand_resource(struct platform_device *pdev) > /* initialize all interrupts to be disabled */ > disable_int(info, NDSR_MASK); > > - ret = request_irq(irq, pxa3xx_nand_irq, 0, pdev->name, info); > + ret = request_threaded_irq(irq, pxa3xx_nand_irq, > + pxa3xx_nand_irq_thread, 0, pdev->name, info); > if (ret < 0) { > dev_err(&pdev->dev, "failed to request IRQ\n"); > goto fail_free_buf; While I agree that this change would be needed, it is still not stable material, so both patches will have to go through separate ways (and one will live without the other). Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-16 21:36 ` Robert Jarzmik 2015-02-17 9:47 ` Maxime Ripard @ 2015-02-17 10:37 ` Maxime Ripard 2015-02-17 17:07 ` Robert Jarzmik 1 sibling, 1 reply; 19+ messages in thread From: Maxime Ripard @ 2015-02-17 10:37 UTC (permalink / raw) To: Robert Jarzmik Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 2189 bytes --] On Mon, Feb 16, 2015 at 10:36:02PM +0100, Robert Jarzmik wrote: > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index e512902..6e569e9 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -576,11 +576,20 @@ static void start_data_dma(struct pxa3xx_nand_info *info) > {} > #endif > > +static irqreturn_t pxa3xx_nand_irq_thread(int irq, void *data) > +{ > + struct pxa3xx_nand_info *info = data; > + > + handle_data_pio(info); > + return IRQ_HANDLED; > +} > + > static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) > { > struct pxa3xx_nand_info *info = devid; > unsigned int status, is_completed = 0, is_ready = 0; > unsigned int ready, cmd_done; > + irqreturn_t ret = IRQ_HANDLED; > > if (info->cs == 0) { > ready = NDSR_FLASH_RDY; > @@ -622,7 +631,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) > } else { > info->state = (status & NDSR_RDDREQ) ? > STATE_PIO_READING : STATE_PIO_WRITING; > - handle_data_pio(info); > + ret = IRQ_WAKE_THREAD; > } > } > if (status & cmd_done) { > @@ -663,7 +672,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) > if (is_ready) > complete(&info->dev_ready); > NORMAL_IRQ_EXIT: > - return IRQ_HANDLED; > + return ret; > } > > static inline int is_buf_blank(uint8_t *buf, size_t len) > @@ -1688,7 +1697,8 @@ static int alloc_nand_resource(struct platform_device *pdev) > /* initialize all interrupts to be disabled */ > disable_int(info, NDSR_MASK); > > - ret = request_irq(irq, pxa3xx_nand_irq, 0, pdev->name, info); > + ret = request_threaded_irq(irq, pxa3xx_nand_irq, > + pxa3xx_nand_irq_thread, 0, pdev->name, info); > if (ret < 0) { > dev_err(&pdev->dev, "failed to request IRQ\n"); > goto fail_free_buf; I just gave this patch a try, and it blows up quite badly: http://code.bulix.org/p96krc-87889?raw It looks like there's more work here, most likely in the waitqueues wake up. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-17 10:37 ` Maxime Ripard @ 2015-02-17 17:07 ` Robert Jarzmik 2015-02-17 17:16 ` Ezequiel Garcia 0 siblings, 1 reply; 19+ messages in thread From: Robert Jarzmik @ 2015-02-17 17:07 UTC (permalink / raw) To: Maxime Ripard Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel Maxime Ripard <maxime.ripard@free-electrons.com> writes: > On Mon, Feb 16, 2015 at 10:36:02PM +0100, Robert Jarzmik wrote: >> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c >> index e512902..6e569e9 100644 >> --- a/drivers/mtd/nand/pxa3xx_nand.c >> +++ b/drivers/mtd/nand/pxa3xx_nand.c >> @@ -576,11 +576,20 @@ static void start_data_dma(struct pxa3xx_nand_info *info) >> {} >> #endif >> >> +static irqreturn_t pxa3xx_nand_irq_thread(int irq, void *data) >> +{ >> + struct pxa3xx_nand_info *info = data; >> + >> + handle_data_pio(info); >> + return IRQ_HANDLED; >> +} >> + >> static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) >> { >> struct pxa3xx_nand_info *info = devid; >> unsigned int status, is_completed = 0, is_ready = 0; >> unsigned int ready, cmd_done; >> + irqreturn_t ret = IRQ_HANDLED; >> >> if (info->cs == 0) { >> ready = NDSR_FLASH_RDY; >> @@ -622,7 +631,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) >> } else { >> info->state = (status & NDSR_RDDREQ) ? >> STATE_PIO_READING : STATE_PIO_WRITING; >> - handle_data_pio(info); >> + ret = IRQ_WAKE_THREAD; >> } >> } >> if (status & cmd_done) { >> @@ -663,7 +672,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) >> if (is_ready) >> complete(&info->dev_ready); >> NORMAL_IRQ_EXIT: >> - return IRQ_HANDLED; >> + return ret; >> } >> >> static inline int is_buf_blank(uint8_t *buf, size_t len) >> @@ -1688,7 +1697,8 @@ static int alloc_nand_resource(struct platform_device *pdev) >> /* initialize all interrupts to be disabled */ >> disable_int(info, NDSR_MASK); >> >> - ret = request_irq(irq, pxa3xx_nand_irq, 0, pdev->name, info); >> + ret = request_threaded_irq(irq, pxa3xx_nand_irq, >> + pxa3xx_nand_irq_thread, 0, pdev->name, info); >> if (ret < 0) { >> dev_err(&pdev->dev, "failed to request IRQ\n"); >> goto fail_free_buf; > > I just gave this patch a try, and it blows up quite badly: > http://code.bulix.org/p96krc-87889?raw Confirmed. OTOH, replacing : >> + ret = IRQ_WAKE_THREAD; With: + ret = IRQ_WAKE_THREAD; + goto NORMAL_IRQ_EXIT; is fully working in my environment, now I got 10mn to test it. I also added for cosmetics in pxa3xx_nand_irq_thread() a : info->state = STATE_CMD_DONE; > It looks like there's more work here, most likely in the waitqueues > wake up. Not that much if I'm right, hein ? And it enables the msleep() instead of mdelay(). And the flow of the driver is not changed for pxa3xx, only for Armada (ie. non dma case) for which you need to introduce a delay anyway, and therefore change the flow. It will be Brian choice eventually, but if you say that you will submit that approach for next cycle, and yours for stable, and that for next you'll convert mdelay() to msleep(), I'll stop arguing. Cheers. -- Robert ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-17 17:07 ` Robert Jarzmik @ 2015-02-17 17:16 ` Ezequiel Garcia 2015-02-24 3:45 ` Brian Norris 0 siblings, 1 reply; 19+ messages in thread From: Ezequiel Garcia @ 2015-02-17 17:16 UTC (permalink / raw) To: Robert Jarzmik, Maxime Ripard Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Brian Norris, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel On 02/17/2015 02:07 PM, Robert Jarzmik wrote: > Maxime Ripard <maxime.ripard@free-electrons.com> writes: > >> On Mon, Feb 16, 2015 at 10:36:02PM +0100, Robert Jarzmik wrote: >>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c >>> index e512902..6e569e9 100644 >>> --- a/drivers/mtd/nand/pxa3xx_nand.c >>> +++ b/drivers/mtd/nand/pxa3xx_nand.c >>> @@ -576,11 +576,20 @@ static void start_data_dma(struct pxa3xx_nand_info *info) >>> {} >>> #endif >>> >>> +static irqreturn_t pxa3xx_nand_irq_thread(int irq, void *data) >>> +{ >>> + struct pxa3xx_nand_info *info = data; >>> + >>> + handle_data_pio(info); >>> + return IRQ_HANDLED; >>> +} >>> + >>> static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) >>> { >>> struct pxa3xx_nand_info *info = devid; >>> unsigned int status, is_completed = 0, is_ready = 0; >>> unsigned int ready, cmd_done; >>> + irqreturn_t ret = IRQ_HANDLED; >>> >>> if (info->cs == 0) { >>> ready = NDSR_FLASH_RDY; >>> @@ -622,7 +631,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) >>> } else { >>> info->state = (status & NDSR_RDDREQ) ? >>> STATE_PIO_READING : STATE_PIO_WRITING; >>> - handle_data_pio(info); >>> + ret = IRQ_WAKE_THREAD; >>> } >>> } >>> if (status & cmd_done) { >>> @@ -663,7 +672,7 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid) >>> if (is_ready) >>> complete(&info->dev_ready); >>> NORMAL_IRQ_EXIT: >>> - return IRQ_HANDLED; >>> + return ret; >>> } >>> >>> static inline int is_buf_blank(uint8_t *buf, size_t len) >>> @@ -1688,7 +1697,8 @@ static int alloc_nand_resource(struct platform_device *pdev) >>> /* initialize all interrupts to be disabled */ >>> disable_int(info, NDSR_MASK); >>> >>> - ret = request_irq(irq, pxa3xx_nand_irq, 0, pdev->name, info); >>> + ret = request_threaded_irq(irq, pxa3xx_nand_irq, >>> + pxa3xx_nand_irq_thread, 0, pdev->name, info); >>> if (ret < 0) { >>> dev_err(&pdev->dev, "failed to request IRQ\n"); >>> goto fail_free_buf; >> >> I just gave this patch a try, and it blows up quite badly: >> http://code.bulix.org/p96krc-87889?raw > Confirmed. > OTOH, replacing : >>> + ret = IRQ_WAKE_THREAD; > With: > + ret = IRQ_WAKE_THREAD; > + goto NORMAL_IRQ_EXIT; > > is fully working in my environment, now I got 10mn to test it. > I also added for cosmetics in pxa3xx_nand_irq_thread() a : > info->state = STATE_CMD_DONE; > >> It looks like there's more work here, most likely in the waitqueues >> wake up. > Not that much if I'm right, hein ? > And it enables the msleep() instead of mdelay(). > > And the flow of the driver is not changed for pxa3xx, only for Armada (ie. non > dma case) for which you need to introduce a delay anyway, and therefore change > the flow. > > It will be Brian choice eventually, but if you say that you will submit that > approach for next cycle, and yours for stable, and that for next you'll convert > mdelay() to msleep(), I'll stop arguing. > How about you push a proper patchset with this alternative (and a nice cover letter explaining the need for a threaded irq) so we can discuss properly this new turn? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-17 17:16 ` Ezequiel Garcia @ 2015-02-24 3:45 ` Brian Norris 2015-02-24 8:17 ` Maxime Ripard 0 siblings, 1 reply; 19+ messages in thread From: Brian Norris @ 2015-02-24 3:45 UTC (permalink / raw) To: Ezequiel Garcia Cc: Robert Jarzmik, Maxime Ripard, Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel On Tue, Feb 17, 2015 at 02:16:43PM -0300, Ezequiel Garcia wrote: > On 02/17/2015 02:07 PM, Robert Jarzmik wrote: > > It will be Brian choice eventually, but if you say that you will submit that > > approach for next cycle, and yours for stable, and that for next you'll convert > > mdelay() to msleep(), I'll stop arguing. > > How about you push a proper patchset with this alternative (and a nice > cover letter explaining the need for a threaded irq) so we can discuss > properly this new turn? I think both Maxime's change (polling a new HW bit) and Robert J's change (move to a threaded IRQ) are good. I'll take another look, but I expect I'll take Maxime's v4 for the 4.0 cycle, and Robert J's v2 for 4.1 (or will we just jump straight to 5.0? I never know). Will I see a patch to convert to msleep() and/or a jiffies timeout in the near future? Brian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining 2015-02-24 3:45 ` Brian Norris @ 2015-02-24 8:17 ` Maxime Ripard 0 siblings, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-24 8:17 UTC (permalink / raw) To: Brian Norris Cc: Ezequiel Garcia, Robert Jarzmik, Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Lior Amsalem, Tawfik Bayouk, Thomas Petazzoni, Seif Mazareeb, linux-kernel, stable, Sudhakar Gundubogula, Nadav Haklai, Boris Brezillon, linux-mtd, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 1176 bytes --] Hi Brian, On Mon, Feb 23, 2015 at 07:45:48PM -0800, Brian Norris wrote: > On Tue, Feb 17, 2015 at 02:16:43PM -0300, Ezequiel Garcia wrote: > > On 02/17/2015 02:07 PM, Robert Jarzmik wrote: > > > It will be Brian choice eventually, but if you say that you will submit that > > > approach for next cycle, and yours for stable, and that for next you'll convert > > > mdelay() to msleep(), I'll stop arguing. > > > > How about you push a proper patchset with this alternative (and a nice > > cover letter explaining the need for a threaded irq) so we can discuss > > properly this new turn? > > I think both Maxime's change (polling a new HW bit) and Robert J's > change (move to a threaded IRQ) are good. I'll take another look, but I > expect I'll take Maxime's v4 for the 4.0 cycle, and Robert J's v2 for > 4.1 (or will we just jump straight to 5.0? I never know). That's what I would expect too. > Will I see a patch to convert to msleep() and/or a jiffies timeout > in the near future? As soon as both patches are merged. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 2/2] ARM: mvebu: a385-db-ap: Enable the NAND 2015-02-16 12:51 [PATCH v3 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller Maxime Ripard 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard @ 2015-02-16 12:51 ` Maxime Ripard 1 sibling, 0 replies; 19+ messages in thread From: Maxime Ripard @ 2015-02-16 12:51 UTC (permalink / raw) To: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, Brian Norris Cc: linux-mtd, Boris Brezillon, Thomas Petazzoni, linux-arm-kernel, linux-kernel, Tawfik Bayouk, Nadav Haklai, Lior Amsalem, Sudhakar Gundubogula, Seif Mazareeb, Maxime Ripard The Armada 385 Access Point Development Board has a 1GB NAND SLC chip from Micron as its main storage. Enable it. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- arch/arm/boot/dts/armada-385-db-ap.dts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/arch/arm/boot/dts/armada-385-db-ap.dts b/arch/arm/boot/dts/armada-385-db-ap.dts index b891b4c897f5..ee648fb19075 100644 --- a/arch/arm/boot/dts/armada-385-db-ap.dts +++ b/arch/arm/boot/dts/armada-385-db-ap.dts @@ -130,6 +130,19 @@ phy-mode = "rgmii-id"; }; + nfc: flash@d0000 { + status = "okay"; + #address-cells = <1>; + #size-cells = <1>; + + num-cs = <1>; + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + marvell,nand-keep-config; + marvell,nand-enable-arbiter; + nand-on-flash-bbt; + }; + usb3@f0000 { status = "okay"; usb-phy = <&usb3_phy>; -- 2.3.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-02-24 8:20 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-02-16 12:51 [PATCH v3 0/2] ARM: mvebu: a385-db-ap: Enable the NAND controller Maxime Ripard 2015-02-16 12:51 ` [PATCH v3 1/2] mtd: nand: pxa3xx: Fix PIO FIFO draining Maxime Ripard 2015-02-16 13:34 ` Boris Brezillon 2015-02-16 13:35 ` Ezequiel Garcia 2015-02-16 16:49 ` Maxime Ripard 2015-02-16 16:27 ` Thomas Petazzoni 2015-02-16 16:41 ` Maxime Ripard 2015-02-16 16:57 ` Ezequiel Garcia 2015-02-17 10:29 ` Maxime Ripard 2015-02-16 20:11 ` Robert Jarzmik 2015-02-16 20:58 ` Maxime Ripard 2015-02-16 21:36 ` Robert Jarzmik 2015-02-17 9:47 ` Maxime Ripard 2015-02-17 10:37 ` Maxime Ripard 2015-02-17 17:07 ` Robert Jarzmik 2015-02-17 17:16 ` Ezequiel Garcia 2015-02-24 3:45 ` Brian Norris 2015-02-24 8:17 ` Maxime Ripard 2015-02-16 12:51 ` [PATCH v3 2/2] ARM: mvebu: a385-db-ap: Enable the NAND Maxime Ripard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).