Re: [PATCH v6 13/14] mtd: onenand: omap2: Configure driver from DT

[Ladislav Michl <ladis@linux-mips.org>]

Hi Aaro,

On Sat, Dec 22, 2018 at 03:02:46AM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Fri, Jan 12, 2018 at 02:17:25PM +0100, Ladislav Michl wrote:
> > Move away from platform data configuration and use pure DT approach.
> 
> Unfortunately this patch (now commit a758f50f10cf) has broken onenand
> probe on N9/N950:
> 
> [    3.129364] OneNAND Manufacturer: Unknown (0xa0)
> 
> The init order was changed so that onenand_scan() is called before the
> timings are configured:

That is intentional, as stated in cover letter:
"
Please note that unlike previous driver version, which basically ignored
DT specified timings, this one relies on it, so it is important to get
it right in your DT (dumping it from previous kernel version).

In case synchronous timings is requested, it is okay to specify timings
for the slowest chip ever used for you board as it is evetually optimized
after chip probe.
"

> >  	if ((r = onenand_scan(&c->mtd, 1)) < 0)
> >  		goto err_release_dma;
> >  
> > +	freq = omap2_onenand_get_freq(c->onenand.version_id);
> > +	if (freq > 0) {
> > +		switch (freq) {
> > +		case 104:
> > +			latency = 7;
> > +			break;
> > +		case 83:
> > +			latency = 6;
> > +			break;
> > +		case 66:
> > +			latency = 5;
> > +			break;
> > +		case 56:
> > +			latency = 4;
> > +			break;
> > +		default:	/* 40 MHz or lower */
> > +			latency = 3;
> > +			break;
> > +		}
> > +
> > +		r = gpmc_omap_onenand_set_timings(dev, c->gpmc_cs,
> > +						  freq, latency, &info);
> > +		if (r)
> > +			goto err_release_onenand;
> > +
> > +		r = omap2_onenand_set_cfg(c, info.sync_read, info.sync_write,
> > +					  latency, info.burst_len);
> > +		if (r)
> > +			goto err_release_onenand;
> > +
> > +		if (info.sync_read || info.sync_write)
> > +			dev_info(dev, "optimized timings for %d MHz\n", freq);
> > +	}
> 
> But looks like on N9/N950 we need to do the configuration for the
> onenand_scan() to succeed. I guess the order has to be this because we
> have no other way to know "version_id".
> 
> I tested by changing this other way round (and hardcoding the freq manually)
> and it seems to work.

Well, DT timings for onenand node seems to be copied over without actually
verifying correctness. I do not know which chip comes with N950, so please
verify timings in you DT according chip used. Idea is to use timings for
slowest chip used and driver will ask for faster one if possible.

Best regards,
	ladis