[LINUX,v8,1/2] Documentation: nand: pl353: Add documentation for controller and driver

Message ID 1521024494-30632-1-git-send-email-nagasureshkumarrelli@gmail.com
State Superseded
Delegated to: Boris Brezillon
Headers show
Series
  • Add arm pl353 smc nand driver for xilinx zynq soc
Related show

Commit Message

nagasureshkumarrelli@gmail.com March 14, 2018, 10:48 a.m.
From: Naga Sureshkumar Relli <nagasure@xilinx.com>

Added notes about the controller and driver

Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
---
Changes in v8
 - None
Changes in v7:
- None
Changes in v6:
- None
Changes in v5:
- Fixed the review comments
Changes in v4:
- None
---
 Documentation/mtd/nand/pl353-nand.txt | 92 +++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/mtd/nand/pl353-nand.txt

Comments

Randy Dunlap March 14, 2018, 11:57 p.m. | #1
On 03/14/2018 03:48 AM, nagasureshkumarrelli@gmail.com wrote:
> From: Naga Sureshkumar Relli <nagasure@xilinx.com>
> 
> Added notes about the controller and driver
> 
> Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
> ---

Hi,

> ---
>  Documentation/mtd/nand/pl353-nand.txt | 92 +++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 Documentation/mtd/nand/pl353-nand.txt
> 
> diff --git a/Documentation/mtd/nand/pl353-nand.txt b/Documentation/mtd/nand/pl353-nand.txt
> new file mode 100644
> index 0000000..ac6fbd5
> --- /dev/null
> +++ b/Documentation/mtd/nand/pl353-nand.txt
> @@ -0,0 +1,92 @@
> +This documents provides some notes about the ARM pl353 smc controller used in

s/smc/SMC/

> +Zynq SOC and confined to NAND specific details.
> +
> +Overview of the controller
> +==========================
> +	The SMC (PL353) supports two memory interfaces:
> +	Interface 0 type SRAM.
> +	Interface 1 type NAND.
> +	This configuration supports the following configurable options:
> +	   . 32-bit or 64-bit AXI data width
> +	   . 8-bit, 16-bit, or 32-bit memory data width for interface 0
> +	   . 8-bit, or 16-bit memory data width for interface 1
> +	   . 1-4 chip selects on each interface
> +	   . SLC ECC block for interface 1
> +
> +For more information, refer the below link for TRM
> +http://infocenter.arm.com/help/topic/com.arm.doc.ddi0380g/
> +DDI0380G_smc_pl350_series_r2p1_trm.pdf
> +
> +NAND memory accesses
> +====================
> +	. Two phase NAND accesses
> +	. NAND command phase transfers
> +	. NAND data phase transfers
> +
> +Two phase NAND accesses
> +	The SMC defines two phases of commands when transferring data to or from
> +NAND flash.
> +
> +Command phase
> +	Commands and optional address information are written to the NAND flash.
> +The command and address can be associated with either a data phase operation to
> +write to or read from the array, or a status/ID register transfer.
> +
> +Data phase
> + Data is either written to or read from the NAND flash. This data can be either
> +data transferred to or from the array, or status/ID register information.
> +
> +NAND AXI address setup
> +       AXI address      Command phase      Data phase
> +	[31:24]         Chip address       Chip address
> +	[23]            NoOfAddCycles_2    Reserved
> +	[22]            NoOfAddCycles_1    Reserved
> +	[21]            NoOfAddCycles_0    ClearCS
> +	[20]            End command valid  End command valid
> +	[19]            0                  1
> +	[18:11]         End command        End command
> +	[10:3]          Start command      [10] ECC Last
> +					   [9:3] Reserved
> +	[2:0]           Reserved           Reserved
> +
> +ECC
> +===
> +    It operates on a number of 512 byte blocks of NAND memory and can be
> +programmed to store the ECC codes after the data in memory. For writes,
> +the ECC is written to the spare area of the page. For reads, the result of
> +a block ECC check are made available to the device driver.
> +
> +------------------------------------------------------------------------
> +|               n * 512 blocks                  | extra  | ecc    |     |
> +|                                               | block  | codes  |     |
> +------------------------------------------------------------------------
> +
> +The ECC calculation uses a simple Hamming code, using 1-bit correction 2-bit
> +detection. It starts when a valid read or write command with a 512 byte aligned
> +address is detected on the memory interface.
> +
> +Driver details
> +==============
> +	The NAND driver has dependency with the pl353_smc memory controller
> +driver for intializing the nand timing parameters, bus width, ECC modes,

              initializing the NAND

> +control and status information.
> +
> +Since the controller expects that the chipselect bit should be cleared for the
> +last data transfer i.e last 4 data bytes, the existing nandbase page
> +read/write routines for soft ecc and ecc none modes will not work. So, inorder

                                                                          in order

> +to make this driver work, it always updates the ecc mode as HW ECC and

                                                   ECC mode

> +implemented the page read/write functions for supporting the SW ECC.

   implements the

> +
> +HW ECC mode:
> +	Upto 2K page size is supported and beyond that it retuns

	Up to                                             returns

> +-ENOSUPPORT error. If the flsh has ONDIE ecc controller then the

                             flash (?) has on-die ECC

> +priority has given to the ONDIE ecc controller. Also the current

            is               on-die ECC

> +implementation has support for upto 64 byte oob area

                                  up to            area.

> +
> +SW ECC mode:
> +	It supports all the pgae sizes. But since, zynq soc bootrom uses

	                                           Zynq SOC

> +HW ECC for the devices that have pgae size <=2K so, to avoid any ecc related

                                    page      <= 2K, to avoid any ECC-related

> +issues during boot, prefer HW ECC over SW ECC.
> +
> +For devicetree binding information please refer the below dt binding file

                                      please refer to the below

> +Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt.
>
Miquel Raynal March 19, 2018, 9:08 p.m. | #2
Hi naga,

On Wed, 14 Mar 2018 16:18:14 +0530, <nagasureshkumarrelli@gmail.com>
wrote:

> From: Naga Sureshkumar Relli <nagasure@xilinx.com>
> 
> Added notes about the controller and driver
> 
> Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
> ---
> Changes in v8
>  - None
> Changes in v7:
> - None
> Changes in v6:
> - None
> Changes in v5:
> - Fixed the review comments
> Changes in v4:
> - None
> ---
>  Documentation/mtd/nand/pl353-nand.txt | 92 +++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 Documentation/mtd/nand/pl353-nand.txt
> 
> diff --git a/Documentation/mtd/nand/pl353-nand.txt b/Documentation/mtd/nand/pl353-nand.txt
> new file mode 100644
> index 0000000..ac6fbd5
> --- /dev/null
> +++ b/Documentation/mtd/nand/pl353-nand.txt
> @@ -0,0 +1,92 @@
> +This documents provides some notes about the ARM pl353 smc controller used in
> +Zynq SOC and confined to NAND specific details.
> +
> +Overview of the controller
> +==========================
> +	The SMC (PL353) supports two memory interfaces:
> +	Interface 0 type SRAM.
> +	Interface 1 type NAND.
> +	This configuration supports the following configurable options:
> +	   . 32-bit or 64-bit AXI data width
> +	   . 8-bit, 16-bit, or 32-bit memory data width for interface 0
> +	   . 8-bit, or 16-bit memory data width for interface 1
> +	   . 1-4 chip selects on each interface
> +	   . SLC ECC block for interface 1
> +
> +For more information, refer the below link for TRM
> +http://infocenter.arm.com/help/topic/com.arm.doc.ddi0380g/
> +DDI0380G_smc_pl350_series_r2p1_trm.pdf

I think it is better to do not break the links?

> +
> +NAND memory accesses
> +====================
> +	. Two phase NAND accesses
> +	. NAND command phase transfers
> +	. NAND data phase transfers
> +
> +Two phase NAND accesses
> +	The SMC defines two phases of commands when transferring data to or from
> +NAND flash.
> +
> +Command phase
> +	Commands and optional address information are written to the NAND flash.
> +The command and address can be associated with either a data phase operation to
> +write to or read from the array, or a status/ID register transfer.
> +
> +Data phase
> + Data is either written to or read from the NAND flash. This data can be either
> +data transferred to or from the array, or status/ID register information.
> +
> +NAND AXI address setup
> +       AXI address      Command phase      Data phase
> +	[31:24]         Chip address       Chip address
> +	[23]            NoOfAddCycles_2    Reserved
> +	[22]            NoOfAddCycles_1    Reserved
> +	[21]            NoOfAddCycles_0    ClearCS
> +	[20]            End command valid  End command valid
> +	[19]            0                  1
> +	[18:11]         End command        End command
> +	[10:3]          Start command      [10] ECC Last
> +					   [9:3] Reserved
> +	[2:0]           Reserved           Reserved
> +
> +ECC
> +===
> +    It operates on a number of 512 byte blocks of NAND memory and can be
> +programmed to store the ECC codes after the data in memory. For writes,
> +the ECC is written to the spare area of the page. For reads, the result of
> +a block ECC check are made available to the device driver.
> +
> +------------------------------------------------------------------------
> +|               n * 512 blocks                  | extra  | ecc    |     |
> +|                                               | block  | codes  |     |
> +------------------------------------------------------------------------
> +
> +The ECC calculation uses a simple Hamming code, using 1-bit correction 2-bit
> +detection. It starts when a valid read or write command with a 512 byte aligned
> +address is detected on the memory interface.
> +
> +Driver details
> +==============
> +	The NAND driver has dependency with the pl353_smc memory controller
> +driver for intializing the nand timing parameters, bus width, ECC modes,

                              ^NAND

> +control and status information.
> +
> +Since the controller expects that the chipselect bit should be cleared for the

                                         ^chip select   ^ would? is?

> +last data transfer i.e last 4 data bytes, the existing nandbase page

What is nandbase?

> +read/write routines for soft ecc and ecc none modes will not work. So, inorder

s/ecc/ECC/                                                   in order^

> +to make this driver work, it always updates the ecc mode as HW ECC and can

s/ecc/ECC/

> +implemented the page read/write functions for supporting the SW ECC.

s/can implemented/implements/?

I don't understand this paragraph, can you explain it please? I am not
sure to understand the limitation nor how you address it.

> +
> +HW ECC mode:
> +	Upto 2K page size is supported and beyond that it retuns
> +-ENOSUPPORT error. If the flsh has ONDIE ecc controller then the

    ^ -ENOTSUPP              ^flash   ^on-die ECC

> +priority has given to the ONDIE ecc controller. Also the current

            ^ is given?      ^on-die ECC

> +implementation has support for upto 64 byte oob area

                                  ^up to 64 bytes of OOB data.

> +
> +SW ECC mode:
> +	It supports all the pgae sizes. But since, zynq soc bootrom uses

                            ^ page                 ^Zync SOC

> +HW ECC for the devices that have pgae size <=2K so, to avoid any ecc related

                                    ^ page <= 2K,               ECC^

> +issues during boot, prefer HW ECC over SW ECC.

I suppose this means that if no ECC mode is given ie. no nand-ecc-mode
in the DT, the driver will use HW ECC by default, right?

> +
> +For devicetree binding information please refer the below dt binding file
> +Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt.

This file does not exist in my tree.


Thanks for contributing this driver,
Miquèl
Naga Sureshkumar Relli March 22, 2018, 4:27 a.m. | #3
Hi Randy,

Thanks for reviewing the patch.

 I will address below mentioned comments in next version of patch.

Thanks,
Naga Sureshkumar Relli.

> -----Original Message-----
> From: Randy Dunlap [mailto:rdunlap@infradead.org]
> Sent: Thursday, March 15, 2018 5:27 AM
> To: nagasureshkumarrelli@gmail.com; boris.brezillon@bootlin.com;
> richard@nod.at; dwmw2@infradead.org; computersforpeace@gmail.com;
> marek.vasut@gmail.com; cyrille.pitchen@wedev4u.fr;
> miquel.raynal@bootlin.com
> Cc: linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; Michal Simek
> <michals@xilinx.com>; Punnaiah Choudary Kalluri <punnaia@xilinx.com>; Naga
> Sureshkumar Relli <nagasure@xilinx.com>
> Subject: Re: [LINUX PATCH v8 1/2] Documentation: nand: pl353: Add
> documentation for controller and driver
> 
> On 03/14/2018 03:48 AM, nagasureshkumarrelli@gmail.com wrote:
> > From: Naga Sureshkumar Relli <nagasure@xilinx.com>
> >
> > Added notes about the controller and driver
> >
> > Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
> > ---
> 
> Hi,
> 
> > ---
> >  Documentation/mtd/nand/pl353-nand.txt | 92
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 92 insertions(+)
> >  create mode 100644 Documentation/mtd/nand/pl353-nand.txt
> >
> > diff --git a/Documentation/mtd/nand/pl353-nand.txt
> > b/Documentation/mtd/nand/pl353-nand.txt
> > new file mode 100644
> > index 0000000..ac6fbd5
> > --- /dev/null
> > +++ b/Documentation/mtd/nand/pl353-nand.txt
> > @@ -0,0 +1,92 @@
> > +This documents provides some notes about the ARM pl353 smc controller
> > +used in
> 
> s/smc/SMC/
> 
> > +Zynq SOC and confined to NAND specific details.
> > +
> > +Overview of the controller
> > +==========================
> > +	The SMC (PL353) supports two memory interfaces:
> > +	Interface 0 type SRAM.
> > +	Interface 1 type NAND.
> > +	This configuration supports the following configurable options:
> > +	   . 32-bit or 64-bit AXI data width
> > +	   . 8-bit, 16-bit, or 32-bit memory data width for interface 0
> > +	   . 8-bit, or 16-bit memory data width for interface 1
> > +	   . 1-4 chip selects on each interface
> > +	   . SLC ECC block for interface 1
> > +
> > +For more information, refer the below link for TRM
> > +http://infocenter.arm.com/help/topic/com.arm.doc.ddi0380g/
> > +DDI0380G_smc_pl350_series_r2p1_trm.pdf
> > +
> > +NAND memory accesses
> > +====================
> > +	. Two phase NAND accesses
> > +	. NAND command phase transfers
> > +	. NAND data phase transfers
> > +
> > +Two phase NAND accesses
> > +	The SMC defines two phases of commands when transferring data to or
> > +from NAND flash.
> > +
> > +Command phase
> > +	Commands and optional address information are written to the NAND
> flash.
> > +The command and address can be associated with either a data phase
> > +operation to write to or read from the array, or a status/ID register transfer.
> > +
> > +Data phase
> > + Data is either written to or read from the NAND flash. This data can
> > +be either data transferred to or from the array, or status/ID register
> information.
> > +
> > +NAND AXI address setup
> > +       AXI address      Command phase      Data phase
> > +	[31:24]         Chip address       Chip address
> > +	[23]            NoOfAddCycles_2    Reserved
> > +	[22]            NoOfAddCycles_1    Reserved
> > +	[21]            NoOfAddCycles_0    ClearCS
> > +	[20]            End command valid  End command valid
> > +	[19]            0                  1
> > +	[18:11]         End command        End command
> > +	[10:3]          Start command      [10] ECC Last
> > +					   [9:3] Reserved
> > +	[2:0]           Reserved           Reserved
> > +
> > +ECC
> > +===
> > +    It operates on a number of 512 byte blocks of NAND memory and can
> > +be programmed to store the ECC codes after the data in memory. For
> > +writes, the ECC is written to the spare area of the page. For reads,
> > +the result of a block ECC check are made available to the device driver.
> > +
> > +---------------------------------------------------------------------
> > +---
> > +|               n * 512 blocks                  | extra  | ecc    |     |
> > +|                                               | block  | codes  |     |
> > +---------------------------------------------------------------------
> > +---
> > +
> > +The ECC calculation uses a simple Hamming code, using 1-bit
> > +correction 2-bit detection. It starts when a valid read or write
> > +command with a 512 byte aligned address is detected on the memory
> interface.
> > +
> > +Driver details
> > +==============
> > +	The NAND driver has dependency with the pl353_smc memory
> controller
> > +driver for intializing the nand timing parameters, bus width, ECC
> > +modes,
> 
>               initializing the NAND
> 
> > +control and status information.
> > +
> > +Since the controller expects that the chipselect bit should be
> > +cleared for the last data transfer i.e last 4 data bytes, the
> > +existing nandbase page read/write routines for soft ecc and ecc none
> > +modes will not work. So, inorder
> 
>                                                                           in order
> 
> > +to make this driver work, it always updates the ecc mode as HW ECC
> > +and
> 
>                                                    ECC mode
> 
> > +implemented the page read/write functions for supporting the SW ECC.
> 
>    implements the
> 
> > +
> > +HW ECC mode:
> > +	Upto 2K page size is supported and beyond that it retuns
> 
> 	Up to                                             returns
> 
> > +-ENOSUPPORT error. If the flsh has ONDIE ecc controller then the
> 
>                              flash (?) has on-die ECC
> 
> > +priority has given to the ONDIE ecc controller. Also the current
> 
>             is               on-die ECC
> 
> > +implementation has support for upto 64 byte oob area
> 
>                                   up to            area.
> 
> > +
> > +SW ECC mode:
> > +	It supports all the pgae sizes. But since, zynq soc bootrom uses
> 
> 	                                           Zynq SOC
> 
> > +HW ECC for the devices that have pgae size <=2K so, to avoid any ecc
> > +related
> 
>                                     page      <= 2K, to avoid any ECC-related
> 
> > +issues during boot, prefer HW ECC over SW ECC.
> > +
> > +For devicetree binding information please refer the below dt binding
> > +file
> 
>                                       please refer to the below
> 
> > +Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt.
> >
> 
> 
> --
> ~Randy
Naga Sureshkumar Relli March 22, 2018, 5:36 a.m. | #4
Hi Miquel,

Thanks for reviewing the patch series.
Please see my comments below.

> -----Original Message-----
> From: Miquel Raynal [mailto:miquel.raynal@bootlin.com]
> Sent: Tuesday, March 20, 2018 2:38 AM
> To: nagasureshkumarrelli@gmail.com
> Cc: boris.brezillon@bootlin.com; richard@nod.at; dwmw2@infradead.org;
> computersforpeace@gmail.com; marek.vasut@gmail.com;
> cyrille.pitchen@wedev4u.fr; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; Michal Simek <michals@xilinx.com>; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>; Naga Sureshkumar Relli
> <nagasure@xilinx.com>
> Subject: Re: [LINUX PATCH v8 1/2] Documentation: nand: pl353: Add
> documentation for controller and driver
> 
> Hi naga,
> 
> On Wed, 14 Mar 2018 16:18:14 +0530, <nagasureshkumarrelli@gmail.com>
> wrote:
> 
> > From: Naga Sureshkumar Relli <nagasure@xilinx.com>
> >
> > Added notes about the controller and driver
> >
> > Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
> > ---
> > Changes in v8
> >  - None
> > Changes in v7:
> > - None
> > Changes in v6:
> > - None
> > Changes in v5:
> > - Fixed the review comments
> > Changes in v4:
> > - None
> > ---
> >  Documentation/mtd/nand/pl353-nand.txt | 92
> > +++++++++++++++++++++++++++++++++++
> >  1 file changed, 92 insertions(+)
> >  create mode 100644 Documentation/mtd/nand/pl353-nand.txt
> >
> > diff --git a/Documentation/mtd/nand/pl353-nand.txt
> > b/Documentation/mtd/nand/pl353-nand.txt
> > new file mode 100644
> > index 0000000..ac6fbd5
> > --- /dev/null
> > +++ b/Documentation/mtd/nand/pl353-nand.txt
> > @@ -0,0 +1,92 @@
> > +This documents provides some notes about the ARM pl353 smc controller
> > +used in Zynq SOC and confined to NAND specific details.
> > +
> > +Overview of the controller
> > +==========================
> > +	The SMC (PL353) supports two memory interfaces:
> > +	Interface 0 type SRAM.
> > +	Interface 1 type NAND.
> > +	This configuration supports the following configurable options:
> > +	   . 32-bit or 64-bit AXI data width
> > +	   . 8-bit, 16-bit, or 32-bit memory data width for interface 0
> > +	   . 8-bit, or 16-bit memory data width for interface 1
> > +	   . 1-4 chip selects on each interface
> > +	   . SLC ECC block for interface 1
> > +
> > +For more information, refer the below link for TRM
> > +http://infocenter.arm.com/help/topic/com.arm.doc.ddi0380g/
> > +DDI0380G_smc_pl350_series_r2p1_trm.pdf
> 
> I think it is better to do not break the links?
I will correct it in next patch.
> 
> > +
> > +NAND memory accesses
> > +====================
> > +	. Two phase NAND accesses
> > +	. NAND command phase transfers
> > +	. NAND data phase transfers
> > +
> > +Two phase NAND accesses
> > +	The SMC defines two phases of commands when transferring data to or
> > +from NAND flash.
> > +
> > +Command phase
> > +	Commands and optional address information are written to the NAND
> flash.
> > +The command and address can be associated with either a data phase
> > +operation to write to or read from the array, or a status/ID register transfer.
> > +
> > +Data phase
> > + Data is either written to or read from the NAND flash. This data can
> > +be either data transferred to or from the array, or status/ID register
> information.
> > +
> > +NAND AXI address setup
> > +       AXI address      Command phase      Data phase
> > +	[31:24]         Chip address       Chip address
> > +	[23]            NoOfAddCycles_2    Reserved
> > +	[22]            NoOfAddCycles_1    Reserved
> > +	[21]            NoOfAddCycles_0    ClearCS
> > +	[20]            End command valid  End command valid
> > +	[19]            0                  1
> > +	[18:11]         End command        End command
> > +	[10:3]          Start command      [10] ECC Last
> > +					   [9:3] Reserved
> > +	[2:0]           Reserved           Reserved
> > +
> > +ECC
> > +===
> > +    It operates on a number of 512 byte blocks of NAND memory and can
> > +be programmed to store the ECC codes after the data in memory. For
> > +writes, the ECC is written to the spare area of the page. For reads,
> > +the result of a block ECC check are made available to the device driver.
> > +
> > +---------------------------------------------------------------------
> > +---
> > +|               n * 512 blocks                  | extra  | ecc    |     |
> > +|                                               | block  | codes  |     |
> > +---------------------------------------------------------------------
> > +---
> > +
> > +The ECC calculation uses a simple Hamming code, using 1-bit
> > +correction 2-bit detection. It starts when a valid read or write
> > +command with a 512 byte aligned address is detected on the memory
> interface.
> > +
> > +Driver details
> > +==============
> > +	The NAND driver has dependency with the pl353_smc memory
> controller
> > +driver for intializing the nand timing parameters, bus width, ECC
> > +modes,
> 
>                               ^NAND
> 
> > +control and status information.
> > +
> > +Since the controller expects that the chipselect bit should be
> > +cleared for the
> 
>                                          ^chip select   ^ would? is?
Will correct in next patch.
> 
> > +last data transfer i.e last 4 data bytes, the existing nandbase page
> 
> What is nandbase?
It is just nand page read/write, I will correct it,
> 
> > +read/write routines for soft ecc and ecc none modes will not work.
> > +So, inorder
> 
> s/ecc/ECC/                                                   in order^
> 
> > +to make this driver work, it always updates the ecc mode as HW ECC
> > +and can
> 
> s/ecc/ECC/
I will update.
> 
> > +implemented the page read/write functions for supporting the SW ECC.
> 
> s/can implemented/implements/?
Will correct in next patch.

> 
> I don't understand this paragraph, can you explain it please? I am not sure to
> understand the limitation nor how you address it.
There is a limitation in SMC, that we must set ECC LAST on last data phase access, 
to tell ECC block not to expect any data further.
Ex:  When number of ECC STEP are 4, then till 3 we will write to flash using SMC with HW ECC enabled.
And for the last ECC STEP, we will subtract 4bytes from page size, and will initiate a transfer.
And the remaining 4 as one more transfer with ECC_LAST bit set in NAND Data phase register to notify
ECC block not to expect any more data. The last block should be align with end of 512 byte block.
Because of this limitation, we are not using core routines.
> 
> > +
> > +HW ECC mode:
> > +	Upto 2K page size is supported and beyond that it retuns -ENOSUPPORT
> > +error. If the flsh has ONDIE ecc controller then the
> 
>     ^ -ENOTSUPP              ^flash   ^on-die ECC
> 
Will correct in next patch.
> > +priority has given to the ONDIE ecc controller. Also the current
> 
>             ^ is given?      ^on-die ECC
> 
Will correct in next patch.
> > +implementation has support for upto 64 byte oob area
> 
>                                   ^up to 64 bytes of OOB data.
> 
Will correct in next patch.
> > +
> > +SW ECC mode:
> > +	It supports all the pgae sizes. But since, zynq soc bootrom uses
> 
>                             ^ page                 ^Zync SOC
> 
Will correct in next patch.
> > +HW ECC for the devices that have pgae size <=2K so, to avoid any ecc
> > +related
> 
>                                     ^ page <= 2K,               ECC^
> 
Will correct in next patch.
> > +issues during boot, prefer HW ECC over SW ECC.
> 
> I suppose this means that if no ECC mode is given ie. no nand-ecc-mode in the
> DT, the driver will use HW ECC by default, right?
Yes.
> 
> > +
> > +For devicetree binding information please refer the below dt binding
> > +file Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt.
> 
> This file does not exist in my tree.
Actually this driver has dependency with SMC driver.
Recently I sent one more patch series, there we will find all these.
This is for drivers/memory/
Since SMC controller has two interfaces, Nand and Nor.
So we have two drivers, one main driver is SMC and the second is NAND driver.
https://www.spinics.net/lists/kernel/msg2748832.html
https://www.spinics.net/lists/kernel/msg2748834.html
https://www.spinics.net/lists/kernel/msg2748840.html

> 
> 
> Thanks for contributing this driver,
> Miquèl
> 
> --
> Miquel Raynal, Bootlin (formerly Free Electrons) Embedded Linux and Kernel
> engineering https://bootlin.com

Thanks,
Naga Sureshkumar Relli.
Miquel Raynal March 22, 2018, 7:31 a.m. | #5
Hi Naga,

I removed linux-kernel from the cc: list (linux-mtd is enough).

> > > +Since the controller expects that the chipselect bit should be
> > > +cleared for the  
> > 
> >                                          ^chip select   ^ would? is?  
> Will correct in next patch.
> >   
> > > +last data transfer i.e last 4 data bytes, the existing nandbase page  
> > 
> > What is nandbase?  
> It is just nand page read/write, I will correct it,
> >   
> > > +read/write routines for soft ecc and ecc none modes will not work.
> > > +So, inorder  
> > 
> > s/ecc/ECC/                                                   in order^
> >   
> > > +to make this driver work, it always updates the ecc mode as HW ECC
> > > +and can  
> > 
> > s/ecc/ECC/  
> I will update.
> >   
> > > +implemented the page read/write functions for supporting the SW ECC.  
> > 
> > s/can implemented/implements/?  
> Will correct in next patch.
> 
> > 
> > I don't understand this paragraph, can you explain it please? I am not sure to
> > understand the limitation nor how you address it.  
> There is a limitation in SMC, that we must set ECC LAST on last data phase access, 
> to tell ECC block not to expect any data further.
> Ex:  When number of ECC STEP are 4, then till 3 we will write to flash using SMC with HW ECC enabled.
> And for the last ECC STEP, we will subtract 4bytes from page size, and will initiate a transfer.
> And the remaining 4 as one more transfer with ECC_LAST bit set in NAND Data phase register to notify
> ECC block not to expect any more data. The last block should be align with end of 512 byte block.
> Because of this limitation, we are not using core routines.

This is way more understandable, thanks. Can you adapt this
explanation into the documentation?

Otherwise I am still not convinced that you need special handlers for
SW ECC nor raw page accessors as they don't toggle the ECC/ECC_LAST bit
set, so please try to remove them from both the documentation and the
code.

> >   
> > > +
> > > +HW ECC mode:
> > > +	Upto 2K page size is supported and beyond that it retuns -ENOSUPPORT
> > > +error. If the flsh has ONDIE ecc controller then the  
> > 
> >     ^ -ENOTSUPP              ^flash   ^on-die ECC
> >   
> Will correct in next patch.
> > > +priority has given to the ONDIE ecc controller. Also the current  
> > 
> >             ^ is given?      ^on-die ECC
> >   
> Will correct in next patch.
> > > +implementation has support for upto 64 byte oob area  
> > 
> >                                   ^up to 64 bytes of OOB data.
> >   
> Will correct in next patch.
> > > +
> > > +SW ECC mode:
> > > +	It supports all the pgae sizes. But since, zynq soc bootrom uses  
> > 
> >                             ^ page                 ^Zync SOC
> >   
> Will correct in next patch.
> > > +HW ECC for the devices that have pgae size <=2K so, to avoid any ecc
> > > +related  
> > 
> >                                     ^ page <= 2K,               ECC^
> >   
> Will correct in next patch.
> > > +issues during boot, prefer HW ECC over SW ECC.  
> > 
> > I suppose this means that if no ECC mode is given ie. no nand-ecc-mode in the
> > DT, the driver will use HW ECC by default, right?  
> Yes.

Then can you reword to make it clearer? I think you can drop the
"issues during boot" argument and stick to something simple like "When
the kernel is not aware of the ECC mode, use HW ECC by default.

> >   
> > > +
> > > +For devicetree binding information please refer the below dt binding
> > > +file Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt.  
> > 
> > This file does not exist in my tree.  
> Actually this driver has dependency with SMC driver.
> Recently I sent one more patch series, there we will find all these.
> This is for drivers/memory/
> Since SMC controller has two interfaces, Nand and Nor.
> So we have two drivers, one main driver is SMC and the second is NAND driver.
> https://www.spinics.net/lists/kernel/msg2748832.html
> https://www.spinics.net/lists/kernel/msg2748834.html
> https://www.spinics.net/lists/kernel/msg2748840.html

Ok, thanks for pointing the links.

> 
> > 
> > 
> > Thanks for contributing this driver,
> > Miquèl
> > 
> > --
> > Miquel Raynal, Bootlin (formerly Free Electrons) Embedded Linux and Kernel
> > engineering https://bootlin.com  
> 
> Thanks,
> Naga Sureshkumar Relli.
Naga Sureshkumar Relli March 23, 2018, 1:43 p.m. | #6
Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal [mailto:miquel.raynal@bootlin.com]
> Sent: Thursday, March 22, 2018 1:01 PM
> To: Naga Sureshkumar Relli <nagasure@xilinx.com>
> Cc: nagasureshkumarrelli@gmail.com; boris.brezillon@bootlin.com;
> richard@nod.at; dwmw2@infradead.org; computersforpeace@gmail.com;
> marek.vasut@gmail.com; linux-mtd@lists.infradead.org; Michal Simek
> <michals@xilinx.com>; Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [LINUX PATCH v8 1/2] Documentation: nand: pl353: Add
> documentation for controller and driver
> 
> Hi Naga,
> 
> I removed linux-kernel from the cc: list (linux-mtd is enough).
> 
> > > > +Since the controller expects that the chipselect bit should be
> > > > +cleared for the
> > >
> > >                                          ^chip select   ^ would? is?
> > Will correct in next patch.
> > >
> > > > +last data transfer i.e last 4 data bytes, the existing nandbase
> > > > +page
> > >
> > > What is nandbase?
> > It is just nand page read/write, I will correct it,
> > >
> > > > +read/write routines for soft ecc and ecc none modes will not work.
> > > > +So, inorder
> > >
> > > s/ecc/ECC/                                                   in order^
> > >
> > > > +to make this driver work, it always updates the ecc mode as HW
> > > > +ECC and can
> > >
> > > s/ecc/ECC/
> > I will update.
> > >
> > > > +implemented the page read/write functions for supporting the SW ECC.
> > >
> > > s/can implemented/implements/?
> > Will correct in next patch.
> >
> > >
> > > I don't understand this paragraph, can you explain it please? I am
> > > not sure to understand the limitation nor how you address it.
> > There is a limitation in SMC, that we must set ECC LAST on last data
> > phase access, to tell ECC block not to expect any data further.
> > Ex:  When number of ECC STEP are 4, then till 3 we will write to flash using
> SMC with HW ECC enabled.
> > And for the last ECC STEP, we will subtract 4bytes from page size, and will
> initiate a transfer.
> > And the remaining 4 as one more transfer with ECC_LAST bit set in NAND
> > Data phase register to notify ECC block not to expect any more data. The last
> block should be align with end of 512 byte block.
> > Because of this limitation, we are not using core routines.
> 
> This is way more understandable, thanks. Can you adapt this explanation into
> the documentation?
Ok, I will add in next series.
> 
> Otherwise I am still not convinced that you need special handlers for SW ECC nor
> raw page accessors as they don't toggle the ECC/ECC_LAST bit set, so please try
> to remove them from both the documentation and the code.
Sure, I will update.
> 
> > >
> > > > +
> > > > +HW ECC mode:
> > > > +	Upto 2K page size is supported and beyond that it retuns
> > > > +-ENOSUPPORT error. If the flsh has ONDIE ecc controller then the
> > >
> > >     ^ -ENOTSUPP              ^flash   ^on-die ECC
> > >
> > Will correct in next patch.
> > > > +priority has given to the ONDIE ecc controller. Also the current
> > >
> > >             ^ is given?      ^on-die ECC
> > >
> > Will correct in next patch.
> > > > +implementation has support for upto 64 byte oob area
> > >
> > >                                   ^up to 64 bytes of OOB data.
> > >
> > Will correct in next patch.
> > > > +
> > > > +SW ECC mode:
> > > > +	It supports all the pgae sizes. But since, zynq soc bootrom uses
> > >
> > >                             ^ page                 ^Zync SOC
> > >
> > Will correct in next patch.
> > > > +HW ECC for the devices that have pgae size <=2K so, to avoid any
> > > > +ecc related
> > >
> > >                                     ^ page <= 2K,               ECC^
> > >
> > Will correct in next patch.
> > > > +issues during boot, prefer HW ECC over SW ECC.
> > >
> > > I suppose this means that if no ECC mode is given ie. no
> > > nand-ecc-mode in the DT, the driver will use HW ECC by default, right?
> > Yes.
> 
> Then can you reword to make it clearer? I think you can drop the
> "issues during boot" argument and stick to something simple like "When
> the kernel is not aware of the ECC mode, use HW ECC by default.
Ok, I will update

Thanks,
Naga Sureshkumar Relli.
> 
> > >
> > > > +
> > > > +For devicetree binding information please refer the below dt binding
> > > > +file Documentation/devicetree/bindings/memory-controllers/pl353-
> smc.txt.
> > >
> > > This file does not exist in my tree.
> > Actually this driver has dependency with SMC driver.
> > Recently I sent one more patch series, there we will find all these.
> > This is for drivers/memory/
> > Since SMC controller has two interfaces, Nand and Nor.
> > So we have two drivers, one main driver is SMC and the second is NAND driver.
> > https://www.spinics.net/lists/kernel/msg2748832.html
> > https://www.spinics.net/lists/kernel/msg2748834.html
> > https://www.spinics.net/lists/kernel/msg2748840.html
> 
> Ok, thanks for pointing the links.
> 
> >
> > >
> > >
> > > Thanks for contributing this driver,
> > > Miquèl
> > >
> > > --
> > > Miquel Raynal, Bootlin (formerly Free Electrons) Embedded Linux and Kernel
> > > engineering https://bootlin.com
> >
> > Thanks,
> > Naga Sureshkumar Relli.
> 
> 
> 
> --
> Miquel Raynal, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

Patch

diff --git a/Documentation/mtd/nand/pl353-nand.txt b/Documentation/mtd/nand/pl353-nand.txt
new file mode 100644
index 0000000..ac6fbd5
--- /dev/null
+++ b/Documentation/mtd/nand/pl353-nand.txt
@@ -0,0 +1,92 @@ 
+This documents provides some notes about the ARM pl353 smc controller used in
+Zynq SOC and confined to NAND specific details.
+
+Overview of the controller
+==========================
+	The SMC (PL353) supports two memory interfaces:
+	Interface 0 type SRAM.
+	Interface 1 type NAND.
+	This configuration supports the following configurable options:
+	   . 32-bit or 64-bit AXI data width
+	   . 8-bit, 16-bit, or 32-bit memory data width for interface 0
+	   . 8-bit, or 16-bit memory data width for interface 1
+	   . 1-4 chip selects on each interface
+	   . SLC ECC block for interface 1
+
+For more information, refer the below link for TRM
+http://infocenter.arm.com/help/topic/com.arm.doc.ddi0380g/
+DDI0380G_smc_pl350_series_r2p1_trm.pdf
+
+NAND memory accesses
+====================
+	. Two phase NAND accesses
+	. NAND command phase transfers
+	. NAND data phase transfers
+
+Two phase NAND accesses
+	The SMC defines two phases of commands when transferring data to or from
+NAND flash.
+
+Command phase
+	Commands and optional address information are written to the NAND flash.
+The command and address can be associated with either a data phase operation to
+write to or read from the array, or a status/ID register transfer.
+
+Data phase
+ Data is either written to or read from the NAND flash. This data can be either
+data transferred to or from the array, or status/ID register information.
+
+NAND AXI address setup
+       AXI address      Command phase      Data phase
+	[31:24]         Chip address       Chip address
+	[23]            NoOfAddCycles_2    Reserved
+	[22]            NoOfAddCycles_1    Reserved
+	[21]            NoOfAddCycles_0    ClearCS
+	[20]            End command valid  End command valid
+	[19]            0                  1
+	[18:11]         End command        End command
+	[10:3]          Start command      [10] ECC Last
+					   [9:3] Reserved
+	[2:0]           Reserved           Reserved
+
+ECC
+===
+    It operates on a number of 512 byte blocks of NAND memory and can be
+programmed to store the ECC codes after the data in memory. For writes,
+the ECC is written to the spare area of the page. For reads, the result of
+a block ECC check are made available to the device driver.
+
+------------------------------------------------------------------------
+|               n * 512 blocks                  | extra  | ecc    |     |
+|                                               | block  | codes  |     |
+------------------------------------------------------------------------
+
+The ECC calculation uses a simple Hamming code, using 1-bit correction 2-bit
+detection. It starts when a valid read or write command with a 512 byte aligned
+address is detected on the memory interface.
+
+Driver details
+==============
+	The NAND driver has dependency with the pl353_smc memory controller
+driver for intializing the nand timing parameters, bus width, ECC modes,
+control and status information.
+
+Since the controller expects that the chipselect bit should be cleared for the
+last data transfer i.e last 4 data bytes, the existing nandbase page
+read/write routines for soft ecc and ecc none modes will not work. So, inorder
+to make this driver work, it always updates the ecc mode as HW ECC and
+implemented the page read/write functions for supporting the SW ECC.
+
+HW ECC mode:
+	Upto 2K page size is supported and beyond that it retuns
+-ENOSUPPORT error. If the flsh has ONDIE ecc controller then the
+priority has given to the ONDIE ecc controller. Also the current
+implementation has support for upto 64 byte oob area
+
+SW ECC mode:
+	It supports all the pgae sizes. But since, zynq soc bootrom uses
+HW ECC for the devices that have pgae size <=2K so, to avoid any ecc related
+issues during boot, prefer HW ECC over SW ECC.
+
+For devicetree binding information please refer the below dt binding file
+Documentation/devicetree/bindings/memory-controllers/pl353-smc.txt.