Re: [PATCH v5 16/28] mtd: rawnand: timings: Use default values for tPROG_max and tBERS_max

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Boris,

Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote on Tue, 26 May
2020 23:25:30 +0200:

> On Tue, 26 May 2020 21:17:13 +0200
> Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> 
> > The ONFI parameter page of a chip might define more fine grained
> > tPROG_max and tBERS_max. When we do not have this information, we
> > default to the highest possible values (they are maxima anyway).
> > 
> > There is no point setting these fields at runtime, so explicitly move
> > these defaults to the main ONFI SDR timings structure.  
> 
> Okay, now I see why you're doing that: you want to return the mode 0
> entry directly, and it makes sense, but it should be explained in the
> commit message.

Okay I will mention it in the other commit message, and perhaps move
the comment about them as well so that it does not get lost.

I can also assign these timings manually for mode 0 in the helper you
requested.

What do you prefer?

> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > ---
> >  drivers/mtd/nand/raw/nand_timings.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> > index c2286a75d134..52ee83e75646 100644
> > --- a/drivers/mtd/nand/raw/nand_timings.c
> > +++ b/drivers/mtd/nand/raw/nand_timings.c
> > @@ -20,6 +20,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> >  		.timings.sdr = {
> >  			.tCCS_min = 500000,
> >  			.tR_max = 200000000,
> > +			.tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> > +			.tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> >  			.tADL_min = 400000,
> >  			.tALH_min = 20000,
> >  			.tALS_min = 50000,
> > @@ -63,6 +65,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> >  		.timings.sdr = {
> >  			.tCCS_min = 500000,
> >  			.tR_max = 200000000,
> > +			.tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> > +			.tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> >  			.tADL_min = 400000,
> >  			.tALH_min = 10000,
> >  			.tALS_min = 25000,
> > @@ -106,6 +110,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> >  		.timings.sdr = {
> >  			.tCCS_min = 500000,
> >  			.tR_max = 200000000,
> > +			.tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> > +			.tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> >  			.tADL_min = 400000,
> >  			.tALH_min = 10000,
> >  			.tALS_min = 15000,
> > @@ -149,6 +155,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> >  		.timings.sdr = {
> >  			.tCCS_min = 500000,
> >  			.tR_max = 200000000,
> > +			.tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> > +			.tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> >  			.tADL_min = 400000,
> >  			.tALH_min = 5000,
> >  			.tALS_min = 10000,
> > @@ -192,6 +200,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> >  		.timings.sdr = {
> >  			.tCCS_min = 500000,
> >  			.tR_max = 200000000,
> > +			.tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> > +			.tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> >  			.tADL_min = 400000,
> >  			.tALH_min = 5000,
> >  			.tALS_min = 10000,
> > @@ -235,6 +245,8 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> >  		.timings.sdr = {
> >  			.tCCS_min = 500000,
> >  			.tR_max = 200000000,
> > +			.tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> > +			.tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX,
> >  			.tADL_min = 400000,
> >  			.tALH_min = 5000,
> >  			.tALS_min = 10000,
> > @@ -359,18 +371,6 @@ int onfi_fill_data_interface(struct nand_chip *chip,
> >  
> >  		/* nanoseconds -> picoseconds */
> >  		timings->tCCS_min = 1000UL * onfi->tCCS;
> > -	} else {
> > -		struct nand_sdr_timings *timings = &iface->timings.sdr;
> > -		/*
> > -		 * For non-ONFI chips we use the highest possible value for
> > -		 * tPROG and tBERS. tR and tCCS will take the default values
> > -		 * precised in the ONFI specification for timing mode 0,
> > -		 * respectively 200us and 500ns.
> > -		 */
> > -
> > -		/* microseconds -> picoseconds */
> > -		timings->tPROG_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
> > -		timings->tBERS_max = 1000000ULL * ONFI_DYN_TIMING_MAX;
> >  	}
> >  
> >  	return 0;  
> 

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