RE: [LINUX PATCH v8 1/2] Documentation: nand: pl353: Add documentation for controller and driver
From: Naga Sureshkumar Relli
Date: Thu Mar 22 2018 - 01:37:28 EST
Hi Miquel,
Thanks for reviewing the patch series.
Please see my comments below.
> -----Original Message-----
> From: Miquel Raynal [mailto:miquel.raynal@xxxxxxxxxxx]
> Sent: Tuesday, March 20, 2018 2:38 AM
> To: nagasureshkumarrelli@xxxxxxxxx
> Cc: boris.brezillon@xxxxxxxxxxx; richard@xxxxxx; dwmw2@xxxxxxxxxxxxx;
> computersforpeace@xxxxxxxxx; marek.vasut@xxxxxxxxx;
> cyrille.pitchen@xxxxxxxxxx; linux-mtd@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Michal Simek <michals@xxxxxxxxxx>; Punnaiah
> Choudary Kalluri <punnaia@xxxxxxxxxx>; Naga Sureshkumar Relli
> <nagasure@xxxxxxxxxx>
> 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@xxxxxxxxx>
> wrote:
>
> > From: Naga Sureshkumar Relli <nagasure@xxxxxxxxxx>
> >
> > Added notes about the controller and driver
> >
> > Signed-off-by: Naga Sureshkumar Relli <nagasure@xxxxxxxxxx>
> > ---
> > 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.