RE: [LINUX, v8, 2/2] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

[Naga Sureshkumar Relli <nagasure@xilinx.com>]

Hi Miquel,


> -----Original Message-----
> From: Miquel Raynal [mailto:miquel.raynal@bootlin.com]
> Sent: Thursday, May 17, 2018 6:54 PM
> To: Naga Sureshkumar Relli <nagasure@xilinx.com>
> Cc: Helmut Grohne <h.grohne@intenta.de>; linux-mtd@lists.infradead.org; Boris Brezillon
> <Boris.Brezillon@bootlin.com>
> Subject: Re: [LINUX, v8, 2/2] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand
> interface
> 
> Hi Naga,
> 
> On Fri, 4 May 2018 06:46:50 +0000, Naga Sureshkumar Relli <nagasure@xilinx.com> wrote:
> 
> > Hi Miquel & Helmut,
> >
> > Thanks for updating the latest changes.
> >
> > > -----Original Message-----
> > > From: Miquel Raynal [mailto:miquel.raynal@bootlin.com]
> > > Sent: Thursday, May 3, 2018 7:56 PM
> > > To: Helmut Grohne <h.grohne@intenta.de>
> > > Cc: Naga Sureshkumar Relli <nagasure@xilinx.com>;
> > > linux-mtd@lists.infradead.org; Boris Brezillon
> > > <Boris.Brezillon@bootlin.com>
> > > Subject: Re: [LINUX, v8, 2/2] mtd: rawnand: pl353: Add basic driver
> > > for arm pl353 smc nand interface
> > >
> > > Hi Naga,
> > >
> > > On Thu, 3 May 2018 14:56:01 +0200, Helmut Grohne
> > > <h.grohne@intenta.de>
> > > wrote:
> > >
> > > > On Wed, Mar 14, 2018 at 04:18:25PM +0530, Naga Sureshkumar Relli wrote:
> > > > > +	xnand->row_addr_cycles = nand_chip->onfi_params.addr_cycles & 0xF;
> > > > > +	xnand->col_addr_cycles =
> > > > > +				(nand_chip->onfi_params.addr_cycles >> 4) & 0xF;
> > > >
> > > > Meanwhile onfi_params have been removed from the mainline and this
> > > > patch no longer builds against v4.17-rc3. Can you resend a version
> > > > that builds against with more recent kernels?
> > >
> > > Actually I'm not sure you need addr_cycles from the ONFI param page.
> > > What about non- ONFI chips? Anyway you won't find this entry anymore
> > > as the param page is now freed after probe.
> > >
> > > I don't get why you need this though?
> > > Maybe you should have a look at the NAND_ROW_ADDR_3 flag?
> >
> > The arm SMC pl353 controller operates in cmd phase and data phase.
> > In cmd phase it requires number of address cycles, hence we are using
> > this By reading from parameter page.
> > DOC:
> > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0380g/DDI0380G_smc
> > _pl350_series_r2p1_trm.pdf if you see table2-2 NAND AXI address setup,
> > it describes the usage of cmd and data phases.
> > If we remove this onfi_params, is there an alternative to write the
> > number of address cycles filed in Cmd phase register?
> > Or can I use max value?, that means as per spec it supports max of 7 address cycles(it is a
> 3bit value).
> 
> In ->exec_op():
> If your operation contains address cycles, you'll get the number with
> nand_subop_get_num_addr_cyc(subop, op_id). You already retrieve correctly that number
> from your ->exec_op() implementation. I don't think you need anything else.
> 
> In ->ecc.read/write*() helpers:
> The number of column cycles is 1 if mtd->writesize <= 512, otherwise it is 2. The number of
> row cycles is 3 if the NAND_ROW_ADDR_3 flag is present in chip->options, otherwise it is
> 2 (it won't change, just check it once at probe time).
> 
> Thanks,
> Miquèl

Thanks for giving more info on this address cycles implementation.
I am currently updating the driver by addressing all your comments that I got
On this v8 series including the latest comments.
I will send the next series.

Thanks,
Naga Sureshkumar Relli