Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-18 Thread Boris Brezillon
Hi Archit,

On Thu, 17 Dec 2015 15:18:46 +0530
Archit Taneja  wrote:

> 
> >
> > Note that you could put the oobfree area at the end of the OOB area,
> > since this 10-10-10-16-10 representation is already a virtual
> > representation of the OOB area (ECC bytes are actually interleaved with
> > in-band data on the flash).
> 
> But, when I read from the controller's internal buffer via DMA, I first
> get the oobfree area, and only then the last step's ECC bytes. So, the 
> content in chip->oob_poi is in the same order as mentioned
> above (10-10-10-16-10).
> 
> If the upper layers uses MTD_OPS_AUTO_OOB, and if I have a different
> layout as what it is in the oob_poi buffer, then it'll end up
> reading/writing the wrong bytes in nand_transfer_oob/nand_fill_oob.
> 
> Are you suggesting that I modify the contents of oob_poi by hand such
> that it has a cleaner 10-10-10-10-16 representation?

Hm, I thought you could just place the free oob bytes wherever you want
since there's only one oobfree region. AFAICS, it's just a matter of
passing ->oob_poi + 40 instead of passing ->oob_poi + 30 when preparing
the DMA descriptor (30 and 40 are just numbers for this specific use
case).
Anyway, I won't complain if you address all comments but this one.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-18 Thread Boris Brezillon
Hi Archit,

On Thu, 17 Dec 2015 15:18:46 +0530
Archit Taneja  wrote:

> 
> >
> > Note that you could put the oobfree area at the end of the OOB area,
> > since this 10-10-10-16-10 representation is already a virtual
> > representation of the OOB area (ECC bytes are actually interleaved with
> > in-band data on the flash).
> 
> But, when I read from the controller's internal buffer via DMA, I first
> get the oobfree area, and only then the last step's ECC bytes. So, the 
> content in chip->oob_poi is in the same order as mentioned
> above (10-10-10-16-10).
> 
> If the upper layers uses MTD_OPS_AUTO_OOB, and if I have a different
> layout as what it is in the oob_poi buffer, then it'll end up
> reading/writing the wrong bytes in nand_transfer_oob/nand_fill_oob.
> 
> Are you suggesting that I modify the contents of oob_poi by hand such
> that it has a cleaner 10-10-10-10-16 representation?

Hm, I thought you could just place the free oob bytes wherever you want
since there's only one oobfree region. AFAICS, it's just a matter of
passing ->oob_poi + 40 instead of passing ->oob_poi + 30 when preparing
the DMA descriptor (30 and 40 are just numbers for this specific use
case).
Anyway, I won't complain if you address all comments but this one.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-17 Thread Archit Taneja



On 12/16/2015 07:48 PM, Boris Brezillon wrote:

On Wed, 16 Dec 2015 17:27:48 +0530
Archit Taneja  wrote:


+/*
+ * NAND controller page layout info
+ *
+ * |---| |-|
+ * |   xx...xx|  | *xx...xx|
+ * |   DATAxx..ECC..xx|  | DATA**SPARE**xx..ECC..xx|
+ * |   (516)   xx...xx|  |  (516-n*4)  **(n*4)**xx...xx|
+ * |   xx...xx|  | *xx...xx|
+ * |---| |-|
+ * codeword 1,2..n-1   codeword n
+ *  <---(528/532 Bytes)><---(528/532 Bytes)-->
+ *
+ * n = number of codewords in the page
+ * . = ECC bytes
+ * * = spare bytes
+ * x = unused/reserved bytes
+ *
+ * 2K page: n = 4, spare = 16 bytes
+ * 4K page: n = 8, spare = 32 bytes
+ * 8K page: n = 16, spare = 64 bytes


Is there a reason not to use the following layout?

(
n x (
data = 512 bytes
protected OOB data = 4 bytes
ECC bytes = 12 or 16
)

+

remaining unprotected OOB bytes
)

This way the ECC layout definition would be easier to define, and
you'll have something that is closer to what a NAND chip expect (ECC
block/step size of 512 or 1024).

I know this is also dependent on the bootloader, hence my question.


I tried to figure this out looking at documentation and the downstream
drivers. What I understood was that all the OOB was intentionally kept
in the last step, so that things are faster when we only want to access
OOB. In that case, the controller will need to write to only one
step/codeword.


Well, I don't think sending a column change command is that costly
compared to a page retrieval command or ECC calculation.


Yeah, that's probably true. Although, for this controller, we have
a fixed behavior: if you read a step within a page, the controller
reads the entire 516 bytes into its internal buffer. In other words, the 
controller would need to read the entire page for just 16 bytes

of free OOB if we have the standard ECC layout.







The bootloaders also use the same layout.


Ok, then I guess we'll have to live with that (but it complicates a lot
the driver logic :-/)


Yeah, it does :/

The controller does support a mode (where free OOB isn't protected by 
ECC). In that case, the flash layout is similar to the one you

mentioned. But again, if the bootloaders use the weird layout, then
we don't have much choice.








+ *
+ * the qcom nand controller operates at a sub page/codeword level. each
+ * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes respectively.
+ * the number of ECC bytes vary based on the ECC strength and the bus width.
+ *
+ * the first n - 1 codewords contains 516 bytes of user data, the remaining
+ * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
+ * both user data and spare(oobavail) bytes that sum up to 516 bytes.
+ *
+ * the layout described above is used by the controller when the ECC block is
+ * enabled. When we read a page with ECC enabled, the unused/reserved bytes are
+ * skipped and not copied to our internal buffer. therefore, the nand_ecclayout
+ * layouts defined below doesn't consider the positions occupied by the 
reserved
+ * bytes


You could just read this portion with the ECC engine disabled when
you're asked for OOB data.


Yes, but there are ecc ops (like ecc->read_page/ecc->write_page) that
have an argument called 'oob_required'. We need to have ECC enabled when
running these ops.

In order to read this additional portion, I'll need to read/write each
step again with ECC disabled, which would really slow things down.


Nowadays, MTD FS are not using the OOB area, and oob_required is only
passed if the MTD user asked for OOB data, so that can be an acceptable
penalty. Anyway, that's not really important if we loose a few OOB
bytes.


Ah, okay.








+ *
+ * when the ECC block is disabled, one unused byte (or two for 16 bit bus 
width)
+ * in the last codeword is the position of bad block marker. the bad block
+ * marker cannot be accessed when ECC is enabled.


So, you're switching the BBM with the data at the BBM position
(possibly some in-band data), right?


Yes. When ECC isn't enabled, the BBM byte lies within the in-band data
of the last step. In fact, there are dummy BBM bytes in the previous
steps at the same offset.

With ECC enabled, the controller just skips that position (and the
dummy BBM bytes in previous steps) altogether.


Okay.






+ *
+ */
+
+/*
+ * Layouts for different page sizes and ecc modes. We skip the eccpos field
+ * since it isn't needed for this driver
+ */


If you know where they are stored, please specify them, even if they
are not used by the upper layers (this helps analyzing raw nand dumps).


+
+/* 2K page, 4 bit ECC */
+static struct nand_ecclayout layout_oob_64 = {
+   .eccbytes   = 40,
+   .oobfree= {
+  

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-17 Thread Archit Taneja



On 12/16/2015 07:48 PM, Boris Brezillon wrote:

On Wed, 16 Dec 2015 17:27:48 +0530
Archit Taneja  wrote:


+/*
+ * NAND controller page layout info
+ *
+ * |---| |-|
+ * |   xx...xx|  | *xx...xx|
+ * |   DATAxx..ECC..xx|  | DATA**SPARE**xx..ECC..xx|
+ * |   (516)   xx...xx|  |  (516-n*4)  **(n*4)**xx...xx|
+ * |   xx...xx|  | *xx...xx|
+ * |---| |-|
+ * codeword 1,2..n-1   codeword n
+ *  <---(528/532 Bytes)><---(528/532 Bytes)-->
+ *
+ * n = number of codewords in the page
+ * . = ECC bytes
+ * * = spare bytes
+ * x = unused/reserved bytes
+ *
+ * 2K page: n = 4, spare = 16 bytes
+ * 4K page: n = 8, spare = 32 bytes
+ * 8K page: n = 16, spare = 64 bytes


Is there a reason not to use the following layout?

(
n x (
data = 512 bytes
protected OOB data = 4 bytes
ECC bytes = 12 or 16
)

+

remaining unprotected OOB bytes
)

This way the ECC layout definition would be easier to define, and
you'll have something that is closer to what a NAND chip expect (ECC
block/step size of 512 or 1024).

I know this is also dependent on the bootloader, hence my question.


I tried to figure this out looking at documentation and the downstream
drivers. What I understood was that all the OOB was intentionally kept
in the last step, so that things are faster when we only want to access
OOB. In that case, the controller will need to write to only one
step/codeword.


Well, I don't think sending a column change command is that costly
compared to a page retrieval command or ECC calculation.


Yeah, that's probably true. Although, for this controller, we have
a fixed behavior: if you read a step within a page, the controller
reads the entire 516 bytes into its internal buffer. In other words, the 
controller would need to read the entire page for just 16 bytes

of free OOB if we have the standard ECC layout.







The bootloaders also use the same layout.


Ok, then I guess we'll have to live with that (but it complicates a lot
the driver logic :-/)


Yeah, it does :/

The controller does support a mode (where free OOB isn't protected by 
ECC). In that case, the flash layout is similar to the one you

mentioned. But again, if the bootloaders use the weird layout, then
we don't have much choice.








+ *
+ * the qcom nand controller operates at a sub page/codeword level. each
+ * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes respectively.
+ * the number of ECC bytes vary based on the ECC strength and the bus width.
+ *
+ * the first n - 1 codewords contains 516 bytes of user data, the remaining
+ * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
+ * both user data and spare(oobavail) bytes that sum up to 516 bytes.
+ *
+ * the layout described above is used by the controller when the ECC block is
+ * enabled. When we read a page with ECC enabled, the unused/reserved bytes are
+ * skipped and not copied to our internal buffer. therefore, the nand_ecclayout
+ * layouts defined below doesn't consider the positions occupied by the 
reserved
+ * bytes


You could just read this portion with the ECC engine disabled when
you're asked for OOB data.


Yes, but there are ecc ops (like ecc->read_page/ecc->write_page) that
have an argument called 'oob_required'. We need to have ECC enabled when
running these ops.

In order to read this additional portion, I'll need to read/write each
step again with ECC disabled, which would really slow things down.


Nowadays, MTD FS are not using the OOB area, and oob_required is only
passed if the MTD user asked for OOB data, so that can be an acceptable
penalty. Anyway, that's not really important if we loose a few OOB
bytes.


Ah, okay.








+ *
+ * when the ECC block is disabled, one unused byte (or two for 16 bit bus 
width)
+ * in the last codeword is the position of bad block marker. the bad block
+ * marker cannot be accessed when ECC is enabled.


So, you're switching the BBM with the data at the BBM position
(possibly some in-band data), right?


Yes. When ECC isn't enabled, the BBM byte lies within the in-band data
of the last step. In fact, there are dummy BBM bytes in the previous
steps at the same offset.

With ECC enabled, the controller just skips that position (and the
dummy BBM bytes in previous steps) altogether.


Okay.






+ *
+ */
+
+/*
+ * Layouts for different page sizes and ecc modes. We skip the eccpos field
+ * since it isn't needed for this driver
+ */


If you know where they are stored, please specify them, even if they
are not used by the upper layers (this helps analyzing raw nand dumps).


+
+/* 2K page, 4 bit ECC */
+static struct nand_ecclayout layout_oob_64 = {
+   .eccbytes   = 40,
+   .oobfree   

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Brian Norris
On Wed, Dec 16, 2015 at 05:27:48PM +0530, Archit Taneja wrote:
> On 12/16/2015 02:45 PM, Boris Brezillon wrote:
> >On Wed, 19 Aug 2015 10:19:03 +0530
> >Archit Taneja  wrote:

> >>+   return mtd_device_parse_register(mtd, NULL, , NULL, 0);
> >
> > return mtd_device_parse_register(mtd, NULL, NULL, NULL 0);
> 
> Will fix.

Or just:

return mtd_device_register(mtd, NULL, 0);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Boris Brezillon
On Wed, 16 Dec 2015 17:27:48 +0530
Archit Taneja  wrote:

> >> +/*
> >> + * NAND controller page layout info
> >> + *
> >> + * |---||-|
> >> + * |  xx...xx|  | *xx...xx|
> >> + * |  DATAxx..ECC..xx|  | DATA**SPARE**xx..ECC..xx|
> >> + * |   (516)  xx...xx|  |  (516-n*4)  **(n*4)**xx...xx|
> >> + * |  xx...xx|  | *xx...xx|
> >> + * |---||-|
> >> + * codeword 1,2..n-1  codeword n
> >> + *  <---(528/532 Bytes)> <---(528/532 Bytes)-->
> >> + *
> >> + * n = number of codewords in the page
> >> + * . = ECC bytes
> >> + * * = spare bytes
> >> + * x = unused/reserved bytes
> >> + *
> >> + * 2K page: n = 4, spare = 16 bytes
> >> + * 4K page: n = 8, spare = 32 bytes
> >> + * 8K page: n = 16, spare = 64 bytes
> >
> > Is there a reason not to use the following layout?
> >
> > (
> > n x (
> > data = 512 bytes
> > protected OOB data = 4 bytes
> > ECC bytes = 12 or 16
> > )
> >
> > +
> >
> > remaining unprotected OOB bytes
> > )
> >
> > This way the ECC layout definition would be easier to define, and
> > you'll have something that is closer to what a NAND chip expect (ECC
> > block/step size of 512 or 1024).
> >
> > I know this is also dependent on the bootloader, hence my question.
> 
> I tried to figure this out looking at documentation and the downstream
> drivers. What I understood was that all the OOB was intentionally kept
> in the last step, so that things are faster when we only want to access
> OOB. In that case, the controller will need to write to only one
> step/codeword.

Well, I don't think sending a column change command is that costly
compared to a page retrieval command or ECC calculation.

> 
> The bootloaders also use the same layout.

Ok, then I guess we'll have to live with that (but it complicates a lot
the driver logic :-/)

> 
> >
> >> + *
> >> + * the qcom nand controller operates at a sub page/codeword level. each
> >> + * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes 
> >> respectively.
> >> + * the number of ECC bytes vary based on the ECC strength and the bus 
> >> width.
> >> + *
> >> + * the first n - 1 codewords contains 516 bytes of user data, the 
> >> remaining
> >> + * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
> >> + * both user data and spare(oobavail) bytes that sum up to 516 bytes.
> >> + *
> >> + * the layout described above is used by the controller when the ECC 
> >> block is
> >> + * enabled. When we read a page with ECC enabled, the unused/reserved 
> >> bytes are
> >> + * skipped and not copied to our internal buffer. therefore, the 
> >> nand_ecclayout
> >> + * layouts defined below doesn't consider the positions occupied by the 
> >> reserved
> >> + * bytes
> >
> > You could just read this portion with the ECC engine disabled when
> > you're asked for OOB data.
> 
> Yes, but there are ecc ops (like ecc->read_page/ecc->write_page) that
> have an argument called 'oob_required'. We need to have ECC enabled when
> running these ops.
> 
> In order to read this additional portion, I'll need to read/write each 
> step again with ECC disabled, which would really slow things down.

Nowadays, MTD FS are not using the OOB area, and oob_required is only
passed if the MTD user asked for OOB data, so that can be an acceptable
penalty. Anyway, that's not really important if we loose a few OOB
bytes.

> 
> >
> >> + *
> >> + * when the ECC block is disabled, one unused byte (or two for 16 bit bus 
> >> width)
> >> + * in the last codeword is the position of bad block marker. the bad block
> >> + * marker cannot be accessed when ECC is enabled.
> >
> > So, you're switching the BBM with the data at the BBM position
> > (possibly some in-band data), right?
> 
> Yes. When ECC isn't enabled, the BBM byte lies within the in-band data 
> of the last step. In fact, there are dummy BBM bytes in the previous 
> steps at the same offset.
> 
> With ECC enabled, the controller just skips that position (and the
> dummy BBM bytes in previous steps) altogether.

Okay.

> 
> >
> >> + *
> >> + */
> >> +
> >> +/*
> >> + * Layouts for different page sizes and ecc modes. We skip the eccpos 
> >> field
> >> + * since it isn't needed for this driver
> >> + */
> >
> > If you know where they are stored, please specify them, even if they
> > are not used by the upper layers (this helps analyzing raw nand dumps).
> >
> >> +
> >> +/* 2K page, 4 bit ECC */
> >> +static struct nand_ecclayout layout_oob_64 = {
> >> +  .eccbytes   = 40,
> >> +  .oobfree= {
> >> +  { 30, 16 },
> >> +},
> >> +};
> >
> > According to your description it should either be eccbytes = 48 (if
> > you're considering reserved bytes as ECC bytes) or 32 (if 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Archit Taneja

Hi Boris,

On 12/16/2015 02:45 PM, Boris Brezillon wrote:

Hi Archit,

Again, sorry for the late review. It's probably not exhaustive but
points a few things that should be fixed.


Thanks for the thorough review! Some comments below.




On Wed, 19 Aug 2015 10:19:03 +0530
Archit Taneja  wrote:


The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 
---
  drivers/mtd/nand/Kconfig  |7 +
  drivers/mtd/nand/Makefile |1 +
  drivers/mtd/nand/qcom_nandc.c | 1910 +
  3 files changed, 1918 insertions(+)
  create mode 100644 drivers/mtd/nand/qcom_nandc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..6085b8a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
  Enables support for NAND controller on Hisilicon SoC Hip04.

+config MTD_NAND_QCOM
+   tristate "Support for NAND on QCOM SoCs"
+   depends on ARCH_QCOM
+   help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
  endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1f897ec..87b6a1d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)  += bcm47xxnflash/
  obj-$(CONFIG_MTD_NAND_SUNXI)  += sunxi_nand.o
  obj-$(CONFIG_MTD_NAND_HISI504)+= hisi504_nand.o
  obj-$(CONFIG_MTD_NAND_BRCMNAND)   += brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o

  nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..2337731
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,1910 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Boris Brezillon
Hi Archit,

Again, sorry for the late review. It's probably not exhaustive but
points a few things that should be fixed.


On Wed, 19 Aug 2015 10:19:03 +0530
Archit Taneja  wrote:

> The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> MDM9x15 series.
> 
> It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> broader interface for external slow peripheral devices such as LCD and
> NAND/NOR flash memory or SRAM like interfaces.
> 
> We add support for the NAND controller found within EBI2. For the SoCs
> of our interest, we only use the NAND controller within EBI2. Therefore,
> it's safe for us to assume that the NAND controller is a standalone block
> within the SoC.
> 
> The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> and spare data. The controller contains an internal 512 byte page buffer
> to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> for register read/write and data transfers. The controller performs page
> reads and writes at a codeword/step level of 512 bytes. It can support up
> to 2 external chips of different configurations.
> 
> The driver prepares register read and write configuration descriptors for
> each codeword, followed by data descriptors to read or write data from the
> controller's internal buffer. It uses a single ADM DMA channel that we get
> via dmaengine API. The controller requires 2 ADM CRCIs for command and
> data flow control. These are passed via DT.
> 
> The ecc layout used by the controller is syndrome like, but we can't use
> the standard syndrome ecc ops because of several reasons. First, the amount
> of data bytes covered by ecc isn't same in each step. Second, writing to
> free oob space requires us writing to the entire step in which the oob
> lies. This forces us to create our own ecc ops.
> 
> One more difference is how the controller accesses the bad block marker.
> The controller ignores reading the marker when ECC is enabled. ECC needs
> to be explicity disabled to read or write to the bad block marker. For
> this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> read the factory provided bad block markers.
> 
> v4:
> - Shrink submit_descs
> - add desc list node at the end of dma_prep_desc
> - Endianness and warning fixes
> 
> Signed-off-by: Stephen Boyd 
> 
> v3:
> - Refactor dma functions for maximum reuse
> - Use dma_slave_confing on stack
> - optimize and clean upempty_page_fixup using memchr_inv
> - ensure portability with dma register reads using le32_* funcs
> - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> - fix handling of return values of dmaengine funcs
> - constify wherever possible
> - Remove dependency on ADM DMA in Kconfig
> - Misc fixes and clean ups
> 
> v2:
> - Use new BBT flag that allows us to read BBM in raw mode
> - reduce memcpy-s in the driver
> - some refactor and clean ups because of above changes
> 
> Reviewed-by: Andy Gross 
> Signed-off-by: Archit Taneja 
> ---
>  drivers/mtd/nand/Kconfig  |7 +
>  drivers/mtd/nand/Makefile |1 +
>  drivers/mtd/nand/qcom_nandc.c | 1910 
> +
>  3 files changed, 1918 insertions(+)
>  create mode 100644 drivers/mtd/nand/qcom_nandc.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b2806a..6085b8a 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -538,4 +538,11 @@ config MTD_NAND_HISI504
>   help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>  
> +config MTD_NAND_QCOM
> + tristate "Support for NAND on QCOM SoCs"
> + depends on ARCH_QCOM
> + help
> +   Enables support for NAND flash chips on SoCs containing the EBI2 NAND
> +   controller. This controller is found on IPQ806x SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 1f897ec..87b6a1d 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)+= 
> bcm47xxnflash/
>  obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
>  obj-$(CONFIG_MTD_NAND_HISI504)   += hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += brcmnand/
> +obj-$(CONFIG_MTD_NAND_QCOM)  += qcom_nandc.o
>  
>  nand-objs := nand_base.o nand_bbt.o nand_timings.o
> diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
> new file mode 100644
> index 000..2337731
> --- /dev/null
> +++ b/drivers/mtd/nand/qcom_nandc.c
> @@ -0,0 +1,1910 @@
> +/*
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Brian Norris
On Wed, Dec 16, 2015 at 05:27:48PM +0530, Archit Taneja wrote:
> On 12/16/2015 02:45 PM, Boris Brezillon wrote:
> >On Wed, 19 Aug 2015 10:19:03 +0530
> >Archit Taneja  wrote:

> >>+   return mtd_device_parse_register(mtd, NULL, , NULL, 0);
> >
> > return mtd_device_parse_register(mtd, NULL, NULL, NULL 0);
> 
> Will fix.

Or just:

return mtd_device_register(mtd, NULL, 0);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Boris Brezillon
On Wed, 16 Dec 2015 17:27:48 +0530
Archit Taneja  wrote:

> >> +/*
> >> + * NAND controller page layout info
> >> + *
> >> + * |---||-|
> >> + * |  xx...xx|  | *xx...xx|
> >> + * |  DATAxx..ECC..xx|  | DATA**SPARE**xx..ECC..xx|
> >> + * |   (516)  xx...xx|  |  (516-n*4)  **(n*4)**xx...xx|
> >> + * |  xx...xx|  | *xx...xx|
> >> + * |---||-|
> >> + * codeword 1,2..n-1  codeword n
> >> + *  <---(528/532 Bytes)> <---(528/532 Bytes)-->
> >> + *
> >> + * n = number of codewords in the page
> >> + * . = ECC bytes
> >> + * * = spare bytes
> >> + * x = unused/reserved bytes
> >> + *
> >> + * 2K page: n = 4, spare = 16 bytes
> >> + * 4K page: n = 8, spare = 32 bytes
> >> + * 8K page: n = 16, spare = 64 bytes
> >
> > Is there a reason not to use the following layout?
> >
> > (
> > n x (
> > data = 512 bytes
> > protected OOB data = 4 bytes
> > ECC bytes = 12 or 16
> > )
> >
> > +
> >
> > remaining unprotected OOB bytes
> > )
> >
> > This way the ECC layout definition would be easier to define, and
> > you'll have something that is closer to what a NAND chip expect (ECC
> > block/step size of 512 or 1024).
> >
> > I know this is also dependent on the bootloader, hence my question.
> 
> I tried to figure this out looking at documentation and the downstream
> drivers. What I understood was that all the OOB was intentionally kept
> in the last step, so that things are faster when we only want to access
> OOB. In that case, the controller will need to write to only one
> step/codeword.

Well, I don't think sending a column change command is that costly
compared to a page retrieval command or ECC calculation.

> 
> The bootloaders also use the same layout.

Ok, then I guess we'll have to live with that (but it complicates a lot
the driver logic :-/)

> 
> >
> >> + *
> >> + * the qcom nand controller operates at a sub page/codeword level. each
> >> + * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes 
> >> respectively.
> >> + * the number of ECC bytes vary based on the ECC strength and the bus 
> >> width.
> >> + *
> >> + * the first n - 1 codewords contains 516 bytes of user data, the 
> >> remaining
> >> + * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
> >> + * both user data and spare(oobavail) bytes that sum up to 516 bytes.
> >> + *
> >> + * the layout described above is used by the controller when the ECC 
> >> block is
> >> + * enabled. When we read a page with ECC enabled, the unused/reserved 
> >> bytes are
> >> + * skipped and not copied to our internal buffer. therefore, the 
> >> nand_ecclayout
> >> + * layouts defined below doesn't consider the positions occupied by the 
> >> reserved
> >> + * bytes
> >
> > You could just read this portion with the ECC engine disabled when
> > you're asked for OOB data.
> 
> Yes, but there are ecc ops (like ecc->read_page/ecc->write_page) that
> have an argument called 'oob_required'. We need to have ECC enabled when
> running these ops.
> 
> In order to read this additional portion, I'll need to read/write each 
> step again with ECC disabled, which would really slow things down.

Nowadays, MTD FS are not using the OOB area, and oob_required is only
passed if the MTD user asked for OOB data, so that can be an acceptable
penalty. Anyway, that's not really important if we loose a few OOB
bytes.

> 
> >
> >> + *
> >> + * when the ECC block is disabled, one unused byte (or two for 16 bit bus 
> >> width)
> >> + * in the last codeword is the position of bad block marker. the bad block
> >> + * marker cannot be accessed when ECC is enabled.
> >
> > So, you're switching the BBM with the data at the BBM position
> > (possibly some in-band data), right?
> 
> Yes. When ECC isn't enabled, the BBM byte lies within the in-band data 
> of the last step. In fact, there are dummy BBM bytes in the previous 
> steps at the same offset.
> 
> With ECC enabled, the controller just skips that position (and the
> dummy BBM bytes in previous steps) altogether.

Okay.

> 
> >
> >> + *
> >> + */
> >> +
> >> +/*
> >> + * Layouts for different page sizes and ecc modes. We skip the eccpos 
> >> field
> >> + * since it isn't needed for this driver
> >> + */
> >
> > If you know where they are stored, please specify them, even if they
> > are not used by the upper layers (this helps analyzing raw nand dumps).
> >
> >> +
> >> +/* 2K page, 4 bit ECC */
> >> +static struct nand_ecclayout layout_oob_64 = {
> >> +  .eccbytes   = 40,
> >> +  .oobfree= {
> >> +  { 30, 16 },
> >> +},
> >> +};
> >
> > According to your description it should either be eccbytes = 48 (if
> > you're considering reserved bytes 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Archit Taneja

Hi Boris,

On 12/16/2015 02:45 PM, Boris Brezillon wrote:

Hi Archit,

Again, sorry for the late review. It's probably not exhaustive but
points a few things that should be fixed.


Thanks for the thorough review! Some comments below.




On Wed, 19 Aug 2015 10:19:03 +0530
Archit Taneja  wrote:


The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 
---
  drivers/mtd/nand/Kconfig  |7 +
  drivers/mtd/nand/Makefile |1 +
  drivers/mtd/nand/qcom_nandc.c | 1910 +
  3 files changed, 1918 insertions(+)
  create mode 100644 drivers/mtd/nand/qcom_nandc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..6085b8a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
  Enables support for NAND controller on Hisilicon SoC Hip04.

+config MTD_NAND_QCOM
+   tristate "Support for NAND on QCOM SoCs"
+   depends on ARCH_QCOM
+   help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
  endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1f897ec..87b6a1d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)  += bcm47xxnflash/
  obj-$(CONFIG_MTD_NAND_SUNXI)  += sunxi_nand.o
  obj-$(CONFIG_MTD_NAND_HISI504)+= hisi504_nand.o
  obj-$(CONFIG_MTD_NAND_BRCMNAND)   += brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o

  nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..2337731
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,1910 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-12-16 Thread Boris Brezillon
Hi Archit,

Again, sorry for the late review. It's probably not exhaustive but
points a few things that should be fixed.


On Wed, 19 Aug 2015 10:19:03 +0530
Archit Taneja  wrote:

> The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> MDM9x15 series.
> 
> It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> broader interface for external slow peripheral devices such as LCD and
> NAND/NOR flash memory or SRAM like interfaces.
> 
> We add support for the NAND controller found within EBI2. For the SoCs
> of our interest, we only use the NAND controller within EBI2. Therefore,
> it's safe for us to assume that the NAND controller is a standalone block
> within the SoC.
> 
> The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> and spare data. The controller contains an internal 512 byte page buffer
> to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> for register read/write and data transfers. The controller performs page
> reads and writes at a codeword/step level of 512 bytes. It can support up
> to 2 external chips of different configurations.
> 
> The driver prepares register read and write configuration descriptors for
> each codeword, followed by data descriptors to read or write data from the
> controller's internal buffer. It uses a single ADM DMA channel that we get
> via dmaengine API. The controller requires 2 ADM CRCIs for command and
> data flow control. These are passed via DT.
> 
> The ecc layout used by the controller is syndrome like, but we can't use
> the standard syndrome ecc ops because of several reasons. First, the amount
> of data bytes covered by ecc isn't same in each step. Second, writing to
> free oob space requires us writing to the entire step in which the oob
> lies. This forces us to create our own ecc ops.
> 
> One more difference is how the controller accesses the bad block marker.
> The controller ignores reading the marker when ECC is enabled. ECC needs
> to be explicity disabled to read or write to the bad block marker. For
> this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> read the factory provided bad block markers.
> 
> v4:
> - Shrink submit_descs
> - add desc list node at the end of dma_prep_desc
> - Endianness and warning fixes
> 
> Signed-off-by: Stephen Boyd 
> 
> v3:
> - Refactor dma functions for maximum reuse
> - Use dma_slave_confing on stack
> - optimize and clean upempty_page_fixup using memchr_inv
> - ensure portability with dma register reads using le32_* funcs
> - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> - fix handling of return values of dmaengine funcs
> - constify wherever possible
> - Remove dependency on ADM DMA in Kconfig
> - Misc fixes and clean ups
> 
> v2:
> - Use new BBT flag that allows us to read BBM in raw mode
> - reduce memcpy-s in the driver
> - some refactor and clean ups because of above changes
> 
> Reviewed-by: Andy Gross 
> Signed-off-by: Archit Taneja 
> ---
>  drivers/mtd/nand/Kconfig  |7 +
>  drivers/mtd/nand/Makefile |1 +
>  drivers/mtd/nand/qcom_nandc.c | 1910 
> +
>  3 files changed, 1918 insertions(+)
>  create mode 100644 drivers/mtd/nand/qcom_nandc.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b2806a..6085b8a 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -538,4 +538,11 @@ config MTD_NAND_HISI504
>   help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>  
> +config MTD_NAND_QCOM
> + tristate "Support for NAND on QCOM SoCs"
> + depends on ARCH_QCOM
> + help
> +   Enables support for NAND flash chips on SoCs containing the EBI2 NAND
> +   controller. This controller is found on IPQ806x SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 1f897ec..87b6a1d 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)+= 
> bcm47xxnflash/
>  obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
>  obj-$(CONFIG_MTD_NAND_HISI504)   += hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += brcmnand/
> +obj-$(CONFIG_MTD_NAND_QCOM)  += qcom_nandc.o
>  
>  nand-objs := nand_base.o nand_bbt.o nand_timings.o
> diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
> new file mode 100644
> index 000..2337731
> --- /dev/null
> +++ b/drivers/mtd/nand/qcom_nandc.c
> @@ -0,0 +1,1910 @@
> +/*
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-06 Thread Archit Taneja

Hi,

On 10/06/2015 02:47 PM, Brian Norris wrote:

Hi Archit,

On Mon, Oct 05, 2015 at 12:21:54PM +0530, Archit Taneja wrote:

On 10/02/2015 08:35 AM, Brian Norris wrote:

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 


Where does this sign-off come into play? It's not grouped with yours.
Did Stephen have something to do with v4 only? Also, we typically trim
the change log from the commit message (and place it below the '---' to
do this automatically). Or did you intend for these changelogs to stay
in the git history? I suppose it's not really harmful to keep it in if
you'd like...


He'd corrected a piece of the code by sharing a patch with with me. You
can place his sign-off once you and Stephen accept the final patch
revision.


OK, thanks for the clarification.


I don't have a problem with discarding the changelogs for the git
history. I can incorporate some of the major changes in the main
commit message above.


Whatever works for you. I'd like to incorporate any useful info in the
commit message, but changelog is sometimes noise.


v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 


Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
particularly interested in oobtest, since you attempted to handle both
ECC and raw OOB.


Yes. All the tests passed. Although, I couldn't figure out from the
oobtest console output if it tested both the ECC and RAW oob.


No, it doesn't actually use raw OOB (which is possibly a flaw; we should
improve the test sometime). It just uses the auto-place mode, which is
more useful for something like JFFS2. I just wanted to make sure the
test passed, since it looks like you did put a little effort on OOB
support.


---

[...]

+/*
+ * @cmd_crci:  ADM DMA CRCI for command flow control
+ * @data_crci: ADM DMA CRCI for data flow control
+ * @list:  DMA descriptor list (list of desc_infos)
+ * @data_buffer:   our local DMA buffer for page read/writes,
+ * used when we can't use the buffer provided
+ * by upper layers 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-06 Thread Brian Norris
Hi Archit,

On Mon, Oct 05, 2015 at 12:21:54PM +0530, Archit Taneja wrote:
> On 10/02/2015 08:35 AM, Brian Norris wrote:
> >On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:
> >>The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> >>MDM9x15 series.
> >>
> >>It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> >>and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> >>broader interface for external slow peripheral devices such as LCD and
> >>NAND/NOR flash memory or SRAM like interfaces.
> >>
> >>We add support for the NAND controller found within EBI2. For the SoCs
> >>of our interest, we only use the NAND controller within EBI2. Therefore,
> >>it's safe for us to assume that the NAND controller is a standalone block
> >>within the SoC.
> >>
> >>The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> >>flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> >>16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> >>and spare data. The controller contains an internal 512 byte page buffer
> >>to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> >>for register read/write and data transfers. The controller performs page
> >>reads and writes at a codeword/step level of 512 bytes. It can support up
> >>to 2 external chips of different configurations.
> >>
> >>The driver prepares register read and write configuration descriptors for
> >>each codeword, followed by data descriptors to read or write data from the
> >>controller's internal buffer. It uses a single ADM DMA channel that we get
> >>via dmaengine API. The controller requires 2 ADM CRCIs for command and
> >>data flow control. These are passed via DT.
> >>
> >>The ecc layout used by the controller is syndrome like, but we can't use
> >>the standard syndrome ecc ops because of several reasons. First, the amount
> >>of data bytes covered by ecc isn't same in each step. Second, writing to
> >>free oob space requires us writing to the entire step in which the oob
> >>lies. This forces us to create our own ecc ops.
> >>
> >>One more difference is how the controller accesses the bad block marker.
> >>The controller ignores reading the marker when ECC is enabled. ECC needs
> >>to be explicity disabled to read or write to the bad block marker. For
> >>this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> >>read the factory provided bad block markers.
> >>
> >>v4:
> >>- Shrink submit_descs
> >>- add desc list node at the end of dma_prep_desc
> >>- Endianness and warning fixes
> >>
> >>Signed-off-by: Stephen Boyd 
> >
> >Where does this sign-off come into play? It's not grouped with yours.
> >Did Stephen have something to do with v4 only? Also, we typically trim
> >the change log from the commit message (and place it below the '---' to
> >do this automatically). Or did you intend for these changelogs to stay
> >in the git history? I suppose it's not really harmful to keep it in if
> >you'd like...
> 
> He'd corrected a piece of the code by sharing a patch with with me. You
> can place his sign-off once you and Stephen accept the final patch
> revision.

OK, thanks for the clarification.

> I don't have a problem with discarding the changelogs for the git
> history. I can incorporate some of the major changes in the main
> commit message above.

Whatever works for you. I'd like to incorporate any useful info in the
commit message, but changelog is sometimes noise.

> >>v3:
> >>- Refactor dma functions for maximum reuse
> >>- Use dma_slave_confing on stack
> >>- optimize and clean upempty_page_fixup using memchr_inv
> >>- ensure portability with dma register reads using le32_* funcs
> >>- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> >>- fix handling of return values of dmaengine funcs
> >>- constify wherever possible
> >>- Remove dependency on ADM DMA in Kconfig
> >>- Misc fixes and clean ups
> >>
> >>v2:
> >>- Use new BBT flag that allows us to read BBM in raw mode
> >>- reduce memcpy-s in the driver
> >>- some refactor and clean ups because of above changes
> >>
> >>Reviewed-by: Andy Gross 
> >>Signed-off-by: Archit Taneja 
> >
> >Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
> >particularly interested in oobtest, since you attempted to handle both
> >ECC and raw OOB.
> 
> Yes. All the tests passed. Although, I couldn't figure out from the
> oobtest console output if it tested both the ECC and RAW oob.

No, it doesn't actually use raw OOB (which is possibly a flaw; we should
improve the test sometime). It just uses the auto-place mode, which is
more useful for something like JFFS2. I just wanted to make sure the
test passed, since it looks like you did put a little effort on OOB
support.

> >>---
[...]
> >>+/*
> >>+ * @cmd_crci:  ADM DMA CRCI for command flow control
> >>+ * @data_crci: ADM DMA CRCI for data flow 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-06 Thread Archit Taneja

Hi,

On 10/06/2015 02:47 PM, Brian Norris wrote:

Hi Archit,

On Mon, Oct 05, 2015 at 12:21:54PM +0530, Archit Taneja wrote:

On 10/02/2015 08:35 AM, Brian Norris wrote:

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 


Where does this sign-off come into play? It's not grouped with yours.
Did Stephen have something to do with v4 only? Also, we typically trim
the change log from the commit message (and place it below the '---' to
do this automatically). Or did you intend for these changelogs to stay
in the git history? I suppose it's not really harmful to keep it in if
you'd like...


He'd corrected a piece of the code by sharing a patch with with me. You
can place his sign-off once you and Stephen accept the final patch
revision.


OK, thanks for the clarification.


I don't have a problem with discarding the changelogs for the git
history. I can incorporate some of the major changes in the main
commit message above.


Whatever works for you. I'd like to incorporate any useful info in the
commit message, but changelog is sometimes noise.


v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 


Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
particularly interested in oobtest, since you attempted to handle both
ECC and raw OOB.


Yes. All the tests passed. Although, I couldn't figure out from the
oobtest console output if it tested both the ECC and RAW oob.


No, it doesn't actually use raw OOB (which is possibly a flaw; we should
improve the test sometime). It just uses the auto-place mode, which is
more useful for something like JFFS2. I just wanted to make sure the
test passed, since it looks like you did put a little effort on OOB
support.


---

[...]

+/*
+ * @cmd_crci:  ADM DMA CRCI for command flow control
+ * @data_crci: ADM DMA CRCI for data flow control
+ * @list:  DMA descriptor list (list of desc_infos)
+ * @data_buffer:   our local DMA buffer for page read/writes,
+ * used when we can't 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-06 Thread Brian Norris
Hi Archit,

On Mon, Oct 05, 2015 at 12:21:54PM +0530, Archit Taneja wrote:
> On 10/02/2015 08:35 AM, Brian Norris wrote:
> >On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:
> >>The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> >>MDM9x15 series.
> >>
> >>It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> >>and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> >>broader interface for external slow peripheral devices such as LCD and
> >>NAND/NOR flash memory or SRAM like interfaces.
> >>
> >>We add support for the NAND controller found within EBI2. For the SoCs
> >>of our interest, we only use the NAND controller within EBI2. Therefore,
> >>it's safe for us to assume that the NAND controller is a standalone block
> >>within the SoC.
> >>
> >>The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> >>flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> >>16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> >>and spare data. The controller contains an internal 512 byte page buffer
> >>to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> >>for register read/write and data transfers. The controller performs page
> >>reads and writes at a codeword/step level of 512 bytes. It can support up
> >>to 2 external chips of different configurations.
> >>
> >>The driver prepares register read and write configuration descriptors for
> >>each codeword, followed by data descriptors to read or write data from the
> >>controller's internal buffer. It uses a single ADM DMA channel that we get
> >>via dmaengine API. The controller requires 2 ADM CRCIs for command and
> >>data flow control. These are passed via DT.
> >>
> >>The ecc layout used by the controller is syndrome like, but we can't use
> >>the standard syndrome ecc ops because of several reasons. First, the amount
> >>of data bytes covered by ecc isn't same in each step. Second, writing to
> >>free oob space requires us writing to the entire step in which the oob
> >>lies. This forces us to create our own ecc ops.
> >>
> >>One more difference is how the controller accesses the bad block marker.
> >>The controller ignores reading the marker when ECC is enabled. ECC needs
> >>to be explicity disabled to read or write to the bad block marker. For
> >>this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> >>read the factory provided bad block markers.
> >>
> >>v4:
> >>- Shrink submit_descs
> >>- add desc list node at the end of dma_prep_desc
> >>- Endianness and warning fixes
> >>
> >>Signed-off-by: Stephen Boyd 
> >
> >Where does this sign-off come into play? It's not grouped with yours.
> >Did Stephen have something to do with v4 only? Also, we typically trim
> >the change log from the commit message (and place it below the '---' to
> >do this automatically). Or did you intend for these changelogs to stay
> >in the git history? I suppose it's not really harmful to keep it in if
> >you'd like...
> 
> He'd corrected a piece of the code by sharing a patch with with me. You
> can place his sign-off once you and Stephen accept the final patch
> revision.

OK, thanks for the clarification.

> I don't have a problem with discarding the changelogs for the git
> history. I can incorporate some of the major changes in the main
> commit message above.

Whatever works for you. I'd like to incorporate any useful info in the
commit message, but changelog is sometimes noise.

> >>v3:
> >>- Refactor dma functions for maximum reuse
> >>- Use dma_slave_confing on stack
> >>- optimize and clean upempty_page_fixup using memchr_inv
> >>- ensure portability with dma register reads using le32_* funcs
> >>- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> >>- fix handling of return values of dmaengine funcs
> >>- constify wherever possible
> >>- Remove dependency on ADM DMA in Kconfig
> >>- Misc fixes and clean ups
> >>
> >>v2:
> >>- Use new BBT flag that allows us to read BBM in raw mode
> >>- reduce memcpy-s in the driver
> >>- some refactor and clean ups because of above changes
> >>
> >>Reviewed-by: Andy Gross 
> >>Signed-off-by: Archit Taneja 
> >
> >Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
> >particularly interested in oobtest, since you attempted to handle both
> >ECC and raw OOB.
> 
> Yes. All the tests passed. Although, I couldn't figure out from the
> oobtest console output if it tested both the ECC and RAW oob.

No, it doesn't actually use raw OOB (which is possibly a flaw; we should
improve the test sometime). It just uses the auto-place mode, which is
more useful for something like JFFS2. I just wanted to make sure the
test passed, since it looks like you did put a little effort on OOB
support.

> >>---
[...]
> >>+/*
> >>+ * @cmd_crci:  ADM DMA CRCI for command flow control
> 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-05 Thread Archit Taneja

Hi Brian,

Thanks for the review.

On 10/02/2015 08:35 AM, Brian Norris wrote:

Hi Archit,

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 


Where does this sign-off come into play? It's not grouped with yours.
Did Stephen have something to do with v4 only? Also, we typically trim
the change log from the commit message (and place it below the '---' to
do this automatically). Or did you intend for these changelogs to stay
in the git history? I suppose it's not really harmful to keep it in if
you'd like...


He'd corrected a piece of the code by sharing a patch with with me. You
can place his sign-off once you and Stephen accept the final patch
revision.

I don't have a problem with discarding the changelogs for the git
history. I can incorporate some of the major changes in the main
commit message above.





v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 


Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
particularly interested in oobtest, since you attempted to handle both
ECC and raw OOB.


Yes. All the tests passed. Although, I couldn't figure out from the
oobtest console output if it tested both the ECC and RAW oob.




---
  drivers/mtd/nand/Kconfig  |7 +
  drivers/mtd/nand/Makefile |1 +
  drivers/mtd/nand/qcom_nandc.c | 1910 +
  3 files changed, 1918 insertions(+)
  create mode 100644 drivers/mtd/nand/qcom_nandc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..6085b8a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
  Enables support for NAND controller on Hisilicon SoC Hip04.

+config MTD_NAND_QCOM
+   tristate "Support for NAND on QCOM SoCs"
+   depends on ARCH_QCOM
+   help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
  endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1f897ec..87b6a1d 100644
--- a/drivers/mtd/nand/Makefile

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-05 Thread Archit Taneja

Hi Brian,

Thanks for the review.

On 10/02/2015 08:35 AM, Brian Norris wrote:

Hi Archit,

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 


Where does this sign-off come into play? It's not grouped with yours.
Did Stephen have something to do with v4 only? Also, we typically trim
the change log from the commit message (and place it below the '---' to
do this automatically). Or did you intend for these changelogs to stay
in the git history? I suppose it's not really harmful to keep it in if
you'd like...


He'd corrected a piece of the code by sharing a patch with with me. You
can place his sign-off once you and Stephen accept the final patch
revision.

I don't have a problem with discarding the changelogs for the git
history. I can incorporate some of the major changes in the main
commit message above.





v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 


Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
particularly interested in oobtest, since you attempted to handle both
ECC and raw OOB.


Yes. All the tests passed. Although, I couldn't figure out from the
oobtest console output if it tested both the ECC and RAW oob.




---
  drivers/mtd/nand/Kconfig  |7 +
  drivers/mtd/nand/Makefile |1 +
  drivers/mtd/nand/qcom_nandc.c | 1910 +
  3 files changed, 1918 insertions(+)
  create mode 100644 drivers/mtd/nand/qcom_nandc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..6085b8a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
  Enables support for NAND controller on Hisilicon SoC Hip04.

+config MTD_NAND_QCOM
+   tristate "Support for NAND on QCOM SoCs"
+   depends on ARCH_QCOM
+   help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
  endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-02 Thread Brian Norris
One more nit noticed by my build tests:

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:

[...]

> +static int qcom_nandc_ecc_init(struct qcom_nandc_data *this)
> +{
> + struct mtd_info *mtd = >mtd;
> + struct nand_chip *chip = >chip;
> + struct nand_ecc_ctrl *ecc = >ecc;
> + int cwperpage;

drivers/mtd/nand/qcom_nandc.c: In function ‘qcom_nandc_ecc_init’:
drivers/mtd/nand/qcom_nandc.c:1517:6: warning: variable ‘cwperpage’ set but not 
used [-Wunused-but-set-variable]

> + bool wide_bus;
> +
> + /* the nand controller fetches codewords/chunks of 512 bytes */
> + cwperpage = mtd->writesize >> 9;
> +
> + ecc->strength = this->ecc_strength;
> +
> + wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
> +
> + if (ecc->strength >= 8) {
> + /* 8 bit ECC defaults to BCH ECC on all platforms */
> + ecc->bytes = wide_bus ? 14 : 13;
> + } else {
> + /*
> +  * if the controller supports BCH for 4 bit ECC, the controller
> +  * uses lesser bytes for ECC. If RS is used, the ECC bytes is
> +  * always 10 bytes
> +  */
> + if (this->ecc_modes & ECC_BCH_4BIT)
> + ecc->bytes = wide_bus ? 8 : 7;
> + else
> + ecc->bytes = 10;
> + }
> +
> + /* each step consists of 512 bytes of data */
> + ecc->size = NANDC_STEP_SIZE;
> +
> + ecc->read_page  = qcom_nandc_read_page;
> + ecc->read_oob   = qcom_nandc_read_oob;
> + ecc->write_page = qcom_nandc_write_page;
> + ecc->write_oob  = qcom_nandc_write_oob;
> +
> + /*
> +  * the bad block marker is readable only when we read the page with ECC
> +  * disabled. all the ops above run with ECC enabled. We need raw read
> +  * and write function for oob in order to access bad block marker.
> +  */
> + ecc->read_oob_raw   = qcom_nandc_read_oob_raw;
> + ecc->write_oob_raw  = qcom_nandc_write_oob_raw;
> +
> + switch (mtd->oobsize) {
> + case 64:
> + ecc->layout = _oob_64;
> + break;
> + case 128:
> + ecc->layout = _oob_128;
> + break;
> + case 224:
> + if (wide_bus)
> + ecc->layout = _oob_224_x16;
> + else
> + ecc->layout = _oob_224_x8;
> + break;
> + case 256:
> + ecc->layout = _oob_256;
> + break;
> + default:
> + dev_err(this->dev, "unsupported NAND device, oobsize %d\n",
> + mtd->oobsize);
> + return -ENODEV;
> + }
> +
> + ecc->mode = NAND_ECC_HW;
> +
> + /* enable ecc by default */
> + this->use_ecc = true;
> +
> + return 0;
> +}

[...]

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-02 Thread Brian Norris
One more nit noticed by my build tests:

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:

[...]

> +static int qcom_nandc_ecc_init(struct qcom_nandc_data *this)
> +{
> + struct mtd_info *mtd = >mtd;
> + struct nand_chip *chip = >chip;
> + struct nand_ecc_ctrl *ecc = >ecc;
> + int cwperpage;

drivers/mtd/nand/qcom_nandc.c: In function ‘qcom_nandc_ecc_init’:
drivers/mtd/nand/qcom_nandc.c:1517:6: warning: variable ‘cwperpage’ set but not 
used [-Wunused-but-set-variable]

> + bool wide_bus;
> +
> + /* the nand controller fetches codewords/chunks of 512 bytes */
> + cwperpage = mtd->writesize >> 9;
> +
> + ecc->strength = this->ecc_strength;
> +
> + wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
> +
> + if (ecc->strength >= 8) {
> + /* 8 bit ECC defaults to BCH ECC on all platforms */
> + ecc->bytes = wide_bus ? 14 : 13;
> + } else {
> + /*
> +  * if the controller supports BCH for 4 bit ECC, the controller
> +  * uses lesser bytes for ECC. If RS is used, the ECC bytes is
> +  * always 10 bytes
> +  */
> + if (this->ecc_modes & ECC_BCH_4BIT)
> + ecc->bytes = wide_bus ? 8 : 7;
> + else
> + ecc->bytes = 10;
> + }
> +
> + /* each step consists of 512 bytes of data */
> + ecc->size = NANDC_STEP_SIZE;
> +
> + ecc->read_page  = qcom_nandc_read_page;
> + ecc->read_oob   = qcom_nandc_read_oob;
> + ecc->write_page = qcom_nandc_write_page;
> + ecc->write_oob  = qcom_nandc_write_oob;
> +
> + /*
> +  * the bad block marker is readable only when we read the page with ECC
> +  * disabled. all the ops above run with ECC enabled. We need raw read
> +  * and write function for oob in order to access bad block marker.
> +  */
> + ecc->read_oob_raw   = qcom_nandc_read_oob_raw;
> + ecc->write_oob_raw  = qcom_nandc_write_oob_raw;
> +
> + switch (mtd->oobsize) {
> + case 64:
> + ecc->layout = _oob_64;
> + break;
> + case 128:
> + ecc->layout = _oob_128;
> + break;
> + case 224:
> + if (wide_bus)
> + ecc->layout = _oob_224_x16;
> + else
> + ecc->layout = _oob_224_x8;
> + break;
> + case 256:
> + ecc->layout = _oob_256;
> + break;
> + default:
> + dev_err(this->dev, "unsupported NAND device, oobsize %d\n",
> + mtd->oobsize);
> + return -ENODEV;
> + }
> +
> + ecc->mode = NAND_ECC_HW;
> +
> + /* enable ecc by default */
> + this->use_ecc = true;
> +
> + return 0;
> +}

[...]

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-01 Thread Brian Norris
Hi Archit,

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:
> The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> MDM9x15 series.
> 
> It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> broader interface for external slow peripheral devices such as LCD and
> NAND/NOR flash memory or SRAM like interfaces.
> 
> We add support for the NAND controller found within EBI2. For the SoCs
> of our interest, we only use the NAND controller within EBI2. Therefore,
> it's safe for us to assume that the NAND controller is a standalone block
> within the SoC.
> 
> The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> and spare data. The controller contains an internal 512 byte page buffer
> to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> for register read/write and data transfers. The controller performs page
> reads and writes at a codeword/step level of 512 bytes. It can support up
> to 2 external chips of different configurations.
> 
> The driver prepares register read and write configuration descriptors for
> each codeword, followed by data descriptors to read or write data from the
> controller's internal buffer. It uses a single ADM DMA channel that we get
> via dmaengine API. The controller requires 2 ADM CRCIs for command and
> data flow control. These are passed via DT.
> 
> The ecc layout used by the controller is syndrome like, but we can't use
> the standard syndrome ecc ops because of several reasons. First, the amount
> of data bytes covered by ecc isn't same in each step. Second, writing to
> free oob space requires us writing to the entire step in which the oob
> lies. This forces us to create our own ecc ops.
> 
> One more difference is how the controller accesses the bad block marker.
> The controller ignores reading the marker when ECC is enabled. ECC needs
> to be explicity disabled to read or write to the bad block marker. For
> this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> read the factory provided bad block markers.
> 
> v4:
> - Shrink submit_descs
> - add desc list node at the end of dma_prep_desc
> - Endianness and warning fixes
> 
> Signed-off-by: Stephen Boyd 

Where does this sign-off come into play? It's not grouped with yours.
Did Stephen have something to do with v4 only? Also, we typically trim
the change log from the commit message (and place it below the '---' to
do this automatically). Or did you intend for these changelogs to stay
in the git history? I suppose it's not really harmful to keep it in if
you'd like...

> 
> v3:
> - Refactor dma functions for maximum reuse
> - Use dma_slave_confing on stack
> - optimize and clean upempty_page_fixup using memchr_inv
> - ensure portability with dma register reads using le32_* funcs
> - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> - fix handling of return values of dmaengine funcs
> - constify wherever possible
> - Remove dependency on ADM DMA in Kconfig
> - Misc fixes and clean ups
> 
> v2:
> - Use new BBT flag that allows us to read BBM in raw mode
> - reduce memcpy-s in the driver
> - some refactor and clean ups because of above changes
> 
> Reviewed-by: Andy Gross 
> Signed-off-by: Archit Taneja 

Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
particularly interested in oobtest, since you attempted to handle both
ECC and raw OOB.

> ---
>  drivers/mtd/nand/Kconfig  |7 +
>  drivers/mtd/nand/Makefile |1 +
>  drivers/mtd/nand/qcom_nandc.c | 1910 
> +
>  3 files changed, 1918 insertions(+)
>  create mode 100644 drivers/mtd/nand/qcom_nandc.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b2806a..6085b8a 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -538,4 +538,11 @@ config MTD_NAND_HISI504
>   help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>  
> +config MTD_NAND_QCOM
> + tristate "Support for NAND on QCOM SoCs"
> + depends on ARCH_QCOM
> + help
> +   Enables support for NAND flash chips on SoCs containing the EBI2 NAND
> +   controller. This controller is found on IPQ806x SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 1f897ec..87b6a1d 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)+= 
> bcm47xxnflash/
>  obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
>  obj-$(CONFIG_MTD_NAND_HISI504)   += hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += brcmnand/
> +obj-$(CONFIG_MTD_NAND_QCOM)  += qcom_nandc.o
>  

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-10-01 Thread Brian Norris
Hi Archit,

On Wed, Aug 19, 2015 at 10:19:03AM +0530, Archit Taneja wrote:
> The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> MDM9x15 series.
> 
> It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> broader interface for external slow peripheral devices such as LCD and
> NAND/NOR flash memory or SRAM like interfaces.
> 
> We add support for the NAND controller found within EBI2. For the SoCs
> of our interest, we only use the NAND controller within EBI2. Therefore,
> it's safe for us to assume that the NAND controller is a standalone block
> within the SoC.
> 
> The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> and spare data. The controller contains an internal 512 byte page buffer
> to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> for register read/write and data transfers. The controller performs page
> reads and writes at a codeword/step level of 512 bytes. It can support up
> to 2 external chips of different configurations.
> 
> The driver prepares register read and write configuration descriptors for
> each codeword, followed by data descriptors to read or write data from the
> controller's internal buffer. It uses a single ADM DMA channel that we get
> via dmaengine API. The controller requires 2 ADM CRCIs for command and
> data flow control. These are passed via DT.
> 
> The ecc layout used by the controller is syndrome like, but we can't use
> the standard syndrome ecc ops because of several reasons. First, the amount
> of data bytes covered by ecc isn't same in each step. Second, writing to
> free oob space requires us writing to the entire step in which the oob
> lies. This forces us to create our own ecc ops.
> 
> One more difference is how the controller accesses the bad block marker.
> The controller ignores reading the marker when ECC is enabled. ECC needs
> to be explicity disabled to read or write to the bad block marker. For
> this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> read the factory provided bad block markers.
> 
> v4:
> - Shrink submit_descs
> - add desc list node at the end of dma_prep_desc
> - Endianness and warning fixes
> 
> Signed-off-by: Stephen Boyd 

Where does this sign-off come into play? It's not grouped with yours.
Did Stephen have something to do with v4 only? Also, we typically trim
the change log from the commit message (and place it below the '---' to
do this automatically). Or did you intend for these changelogs to stay
in the git history? I suppose it's not really harmful to keep it in if
you'd like...

> 
> v3:
> - Refactor dma functions for maximum reuse
> - Use dma_slave_confing on stack
> - optimize and clean upempty_page_fixup using memchr_inv
> - ensure portability with dma register reads using le32_* funcs
> - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> - fix handling of return values of dmaengine funcs
> - constify wherever possible
> - Remove dependency on ADM DMA in Kconfig
> - Misc fixes and clean ups
> 
> v2:
> - Use new BBT flag that allows us to read BBM in raw mode
> - reduce memcpy-s in the driver
> - some refactor and clean ups because of above changes
> 
> Reviewed-by: Andy Gross 
> Signed-off-by: Archit Taneja 

Has this driver been tested with drivers/mtd/tests/? Which ones? I'm
particularly interested in oobtest, since you attempted to handle both
ECC and raw OOB.

> ---
>  drivers/mtd/nand/Kconfig  |7 +
>  drivers/mtd/nand/Makefile |1 +
>  drivers/mtd/nand/qcom_nandc.c | 1910 
> +
>  3 files changed, 1918 insertions(+)
>  create mode 100644 drivers/mtd/nand/qcom_nandc.c
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b2806a..6085b8a 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -538,4 +538,11 @@ config MTD_NAND_HISI504
>   help
> Enables support for NAND controller on Hisilicon SoC Hip04.
>  
> +config MTD_NAND_QCOM
> + tristate "Support for NAND on QCOM SoCs"
> + depends on ARCH_QCOM
> + help
> +   Enables support for NAND flash chips on SoCs containing the EBI2 NAND
> +   controller. This controller is found on IPQ806x SoC.
> +
>  endif # MTD_NAND
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 1f897ec..87b6a1d 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)+= 
> bcm47xxnflash/
>  obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
>  obj-$(CONFIG_MTD_NAND_HISI504)   += hisi504_nand.o
>  obj-$(CONFIG_MTD_NAND_BRCMNAND)  += 

Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-09-13 Thread Archit Taneja


On 8/27/2015 5:07 AM, Stephen Boyd wrote:

On 08/19, Archit Taneja wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 
---


Reviewed-by: Stephen Boyd 



Can someone from the the linux-mtd community review this?

Thanks,
Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-09-13 Thread Archit Taneja


On 8/27/2015 5:07 AM, Stephen Boyd wrote:

On 08/19, Archit Taneja wrote:

The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 
---


Reviewed-by: Stephen Boyd 



Can someone from the the linux-mtd community review this?

Thanks,
Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-08-26 Thread Stephen Boyd
On 08/19, Archit Taneja wrote:
> The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
> MDM9x15 series.
> 
> It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
> and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
> broader interface for external slow peripheral devices such as LCD and
> NAND/NOR flash memory or SRAM like interfaces.
> 
> We add support for the NAND controller found within EBI2. For the SoCs
> of our interest, we only use the NAND controller within EBI2. Therefore,
> it's safe for us to assume that the NAND controller is a standalone block
> within the SoC.
> 
> The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
> flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
> 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
> and spare data. The controller contains an internal 512 byte page buffer
> to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
> for register read/write and data transfers. The controller performs page
> reads and writes at a codeword/step level of 512 bytes. It can support up
> to 2 external chips of different configurations.
> 
> The driver prepares register read and write configuration descriptors for
> each codeword, followed by data descriptors to read or write data from the
> controller's internal buffer. It uses a single ADM DMA channel that we get
> via dmaengine API. The controller requires 2 ADM CRCIs for command and
> data flow control. These are passed via DT.
> 
> The ecc layout used by the controller is syndrome like, but we can't use
> the standard syndrome ecc ops because of several reasons. First, the amount
> of data bytes covered by ecc isn't same in each step. Second, writing to
> free oob space requires us writing to the entire step in which the oob
> lies. This forces us to create our own ecc ops.
> 
> One more difference is how the controller accesses the bad block marker.
> The controller ignores reading the marker when ECC is enabled. ECC needs
> to be explicity disabled to read or write to the bad block marker. For
> this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
> read the factory provided bad block markers.
> 
> v4:
> - Shrink submit_descs
> - add desc list node at the end of dma_prep_desc
> - Endianness and warning fixes
> 
> Signed-off-by: Stephen Boyd 
> 
> v3:
> - Refactor dma functions for maximum reuse
> - Use dma_slave_confing on stack
> - optimize and clean upempty_page_fixup using memchr_inv
> - ensure portability with dma register reads using le32_* funcs
> - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
> - fix handling of return values of dmaengine funcs
> - constify wherever possible
> - Remove dependency on ADM DMA in Kconfig
> - Misc fixes and clean ups
> 
> v2:
> - Use new BBT flag that allows us to read BBM in raw mode
> - reduce memcpy-s in the driver
> - some refactor and clean ups because of above changes
> 
> Reviewed-by: Andy Gross 
> Signed-off-by: Archit Taneja 
> ---

Reviewed-by: Stephen Boyd 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-08-26 Thread Stephen Boyd
On 08/19, Archit Taneja wrote:
 The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
 MDM9x15 series.
 
 It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
 and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
 broader interface for external slow peripheral devices such as LCD and
 NAND/NOR flash memory or SRAM like interfaces.
 
 We add support for the NAND controller found within EBI2. For the SoCs
 of our interest, we only use the NAND controller within EBI2. Therefore,
 it's safe for us to assume that the NAND controller is a standalone block
 within the SoC.
 
 The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
 flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
 16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
 and spare data. The controller contains an internal 512 byte page buffer
 to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
 for register read/write and data transfers. The controller performs page
 reads and writes at a codeword/step level of 512 bytes. It can support up
 to 2 external chips of different configurations.
 
 The driver prepares register read and write configuration descriptors for
 each codeword, followed by data descriptors to read or write data from the
 controller's internal buffer. It uses a single ADM DMA channel that we get
 via dmaengine API. The controller requires 2 ADM CRCIs for command and
 data flow control. These are passed via DT.
 
 The ecc layout used by the controller is syndrome like, but we can't use
 the standard syndrome ecc ops because of several reasons. First, the amount
 of data bytes covered by ecc isn't same in each step. Second, writing to
 free oob space requires us writing to the entire step in which the oob
 lies. This forces us to create our own ecc ops.
 
 One more difference is how the controller accesses the bad block marker.
 The controller ignores reading the marker when ECC is enabled. ECC needs
 to be explicity disabled to read or write to the bad block marker. For
 this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
 read the factory provided bad block markers.
 
 v4:
 - Shrink submit_descs
 - add desc list node at the end of dma_prep_desc
 - Endianness and warning fixes
 
 Signed-off-by: Stephen Boyd sb...@codeaurora.org
 
 v3:
 - Refactor dma functions for maximum reuse
 - Use dma_slave_confing on stack
 - optimize and clean upempty_page_fixup using memchr_inv
 - ensure portability with dma register reads using le32_* funcs
 - use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
 - fix handling of return values of dmaengine funcs
 - constify wherever possible
 - Remove dependency on ADM DMA in Kconfig
 - Misc fixes and clean ups
 
 v2:
 - Use new BBT flag that allows us to read BBM in raw mode
 - reduce memcpy-s in the driver
 - some refactor and clean ups because of above changes
 
 Reviewed-by: Andy Gross agr...@codeaurora.org
 Signed-off-by: Archit Taneja arch...@codeaurora.org
 ---

Reviewed-by: Stephen Boyd sb...@codeaurora.org

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-08-18 Thread Archit Taneja
The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd 

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross 
Signed-off-by: Archit Taneja 
---
 drivers/mtd/nand/Kconfig  |7 +
 drivers/mtd/nand/Makefile |1 +
 drivers/mtd/nand/qcom_nandc.c | 1910 +
 3 files changed, 1918 insertions(+)
 create mode 100644 drivers/mtd/nand/qcom_nandc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..6085b8a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
  Enables support for NAND controller on Hisilicon SoC Hip04.
 
+config MTD_NAND_QCOM
+   tristate "Support for NAND on QCOM SoCs"
+   depends on ARCH_QCOM
+   help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1f897ec..87b6a1d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)  += bcm47xxnflash/
 obj-$(CONFIG_MTD_NAND_SUNXI)   += sunxi_nand.o
 obj-$(CONFIG_MTD_NAND_HISI504) += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..2337731
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,1910 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 

[PATCH v4 2/5] mtd: nand: Qualcomm NAND controller driver

2015-08-18 Thread Archit Taneja
The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.

It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.

We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.

The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.

The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.

The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.

One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. For
this reason, we use the newly created flag NAND_BBT_ACCESS_BBM_RAW to
read the factory provided bad block markers.

v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes

Signed-off-by: Stephen Boyd sb...@codeaurora.org

v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups

v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes

Reviewed-by: Andy Gross agr...@codeaurora.org
Signed-off-by: Archit Taneja arch...@codeaurora.org
---
 drivers/mtd/nand/Kconfig  |7 +
 drivers/mtd/nand/Makefile |1 +
 drivers/mtd/nand/qcom_nandc.c | 1910 +
 3 files changed, 1918 insertions(+)
 create mode 100644 drivers/mtd/nand/qcom_nandc.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b2806a..6085b8a 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -538,4 +538,11 @@ config MTD_NAND_HISI504
help
  Enables support for NAND controller on Hisilicon SoC Hip04.
 
+config MTD_NAND_QCOM
+   tristate Support for NAND on QCOM SoCs
+   depends on ARCH_QCOM
+   help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 1f897ec..87b6a1d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -53,5 +53,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH)  += bcm47xxnflash/
 obj-$(CONFIG_MTD_NAND_SUNXI)   += sunxi_nand.o
 obj-$(CONFIG_MTD_NAND_HISI504) += hisi504_nand.o
 obj-$(CONFIG_MTD_NAND_BRCMNAND)+= brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o
 
 nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 000..2337731
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -0,0 +1,1910 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU