Re: [PATCH v2 13/22] mtd: rawnand: Pass the CS line to be selected in struct nand_operation

[Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx>]

Hi Boris,

On Saturday, November 10, 2018 9:04:59 AM CET Boris Brezillon wrote:
> Hi Janusz,
> 
> On Sat, 10 Nov 2018 02:30:04 +0100
> Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx> wrote:
> 
> > Hi Boris,
> > 
> > On Tuesday, November 6, 2018 4:08:01 PM CET Boris Brezillon wrote:
> > > In order to deprecate the ->select_chip hook we need to pass the CS
> > > line a NAND operations are targeting. This is done through the
> > > addition of a cs field to the nand_operation struct.
> > > 
> > > We also need to keep track of the currently selected target to
> > > properly initialize op->cs, hence the ->cur_cs field addition to the
> > > nand_chip struct.
> > > 
> > > Note that op->cs is not assigned in nand_exec_op() because we might
> > > rework the way we execute NAND operations in the future (adopt a
> > > queuing mechanism instead of the serialization we have right now).
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> > > ---
> > > Changes in v2:
> > > - Initialize ->cur_cs
> > > ---
> > >  drivers/mtd/nand/raw/internals.h  |  3 +++
> > >  drivers/mtd/nand/raw/nand_base.c  | 39 +++++++++++++++++--------------
> > >  drivers/mtd/nand/raw/nand_hynix.c |  4 ++--
> > >  include/linux/mtd/rawnand.h       | 11 ++++++++-
> > >  4 files changed, 37 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> > > index 6e2f61fbc5f0..b62728d5884b 100644
> > > --- a/drivers/mtd/nand/raw/internals.h
> > > +++ b/drivers/mtd/nand/raw/internals.h
> > > @@ -101,6 +101,9 @@ static inline int nand_exec_op(struct nand_chip *chip,
> > >  	if (!chip->exec_op)
> > >  		return -ENOTSUPP;
> > >  
> > > +	if (WARN_ON(op->cs >= chip->numchips))
> > > +		return -EINVAL;  
> > 
> > This needs a fix to nand_scan_ident() like the following:
> > 
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -5034,10 +5034,11 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
> >  
> >  	/* Set the default functions */
> >  	nand_set_defaults(chip);
> >  
> >  	/* Read the flash type */
> > +	chip->numchips = 1;
> 
> Oh, you're right. Actually it should be
> 
> 	chip->numchips = maxchips;

Indeed, setting it to 1 would probably result in chips beyond the first one 
never detected, which is not my case.

> ... 
> > PS.
> > I'm ready to submit a series which converts ams-delta-nand to use GPIO 
> > API for data I/O.  My only concerns is which branch should I rebase it on.
> 
> I'd say nand/next, unless you have dependencies on new GPIO stuff that
> are not in 4.20-rc1.

All GPIO dependencies are already in 4..20-rc1.   My concern was rather 
related to a future merge with this series which will raise some conflicts 
against mine.

Thanks,
Janusz




______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/