Re: [PATCH] Revert "mtd: rawnand: denali: get ->setup_data_interface() working again"

[Marek Vasut <marex@xxxxxxx>]

On 3/11/20 3:39 PM, Miquel Raynal wrote:
> Hi Marek,

Hello Miquel,

[...]

>>> I checked the denali driver and indeed u-boot should not be much clever
>>> than Linux. Are the differences significant? The code is so close, you
>>> can probably check why you have differences. Also verify that the same
>>> ONFI mode is used.  
>>
>> It might've made sense to check those driver differences before making
>> such an statement ;-)
>> That said, I don't think either U-Boot or Linux uses the ONFI
>> information for this NAND, but I might be wrong.
> 
> I don't know what is the exact device but most of the time, even for
> non ONFI-compliant chips, the core starts talking at the lowest ONFI
> speed (mode 0) and then negotiate with the NAND chip the actual timings
> to use. This works if get/set_features is supported, otherwise you
> might have a default mode somewhere. Is it the same in both cases? Does
> the core tries to apply the same timings? Is the calculation the same?
> 
> These are pointers but I am sure you can figure all that out.

The calculation is obviously not the same anymore, due to the recent
changes in the Linux driver, which seems to have broken it (in Linux).

>>>>> and
>>>>> may optimize better the timings depending on the selected mode ([0-5])
>>>>> (hence the different calls to ->setup_data_interface().    
>>>>
>>>> I would expect those two should produce identical timing parameters,
>>>> period, otherwise one or the other is wrong. Thus far, it was Linux that
>>>> produced non-working results.  
>>>
>>> Again, we define minimum and maximum delays. If the right thing is to
>>> not wait more than 5us and you wait up to 6, it does not mean you
>>> wrote "bad timings". 4us would be a bad timing though. It depends on
>>> what you are looking at.  
>>
>> I am look at for example
>>
>>  denali->reg + TCWAW_AND_ADDR_2_DATA = 0x0000143f -> 0x00001432
>>
>> Register was 0x143f before, now is 0x1432 , which is less.
>> I guess that would be the "bad timing" then ?
> 
> Well, is it a minimum or a maximum ? How do you know U-Boot value is
> straight on the edge? If you want to know if timings are valid, open
> the part datasheet, do the math with a paper and compare. This is the
> scientific way to declare timings valid or invalid.

If the value were straight at the edge, I would expect this would
trigger errors over the years, when those values were used, or maybe it
would trigger an error in the thermal chamber tests ? If neither of that
happens, then the values are probably not at the edge enough to matter.

That said, timing calculations do not factor in only the datasheet
values, but also signal propagation delays and other details of the data
path on the PCB and in the SOC, so it's not as simple as you claim it
is. Moreover, while the NAND datasheet is available in public, the
Denali IP datasheet is not, so "do the math with a paper and compare" is
inapplicable here either way, sorry.

>>>>> Run a stress test, if it passes, you should be good :)    
>>>>
>>>> Thank you for the hint, I think the stress test thus far could be
>>>> considered sufficient. I guess we can agree on that ?  
>>>
>>> Oh yeah absolutely :)  
> 
> Just to be sure, we are talking about the new timings derived with
> Masahiro's patch in Linux here, right?

The timings which went through extensive testing are the original ones.

The ones coming out of Masahiro's patch at least do not trigger those
massive UBI errors, however they were tested only very lightly. And I
feel like adding +1/-1 somewhere into the patch is rather iffy, so I
would hope the Denali datasheet has something about this and why this is
needed.

> Because "perfect timings" => "work in the oven" but let be clear on
> the fact that "work in the oven" does not imply "perfect timings".

Let's be clear that I still prefer "practically working and possibly
imperfect" over "theoretically perfect and practically not working".

Also, correction, thermal chamber is not an owen, it does testing over
the entire temperature range of the device.

-- 
Best regards,
Marek Vasut

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