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

[Marek Vasut <marex@xxxxxxx>]

On 2/5/20 11:08 AM, Marek Vasut wrote:
> On 2/5/20 10:50 AM, Miquel Raynal wrote:
>> Hi Marek,
>>
>> Marek Vasut <marex@xxxxxxx> wrote on Wed, 5 Feb 2020 10:41:05 +0100:
>>
>>> On 2/5/20 10:12 AM, Miquel Raynal wrote:
>>>> Hi Marek,
>>>>
>>>> Marek Vasut <marex@xxxxxxx> wrote on Wed,  5 Feb 2020 08:08:34 +0100:
>>>>   
>>>>> This reverts commit d311e0c27b8fcc27f707f8cac48cd8bdc4155224, which
>>>>> completely breaks NAND access on Altera SoCFPGA (detected on ArriaV
>>>>> SoC).
>>>>>
>>>>> On SoCFPGA, denali->clk_rate = 31.25 MHz and denali->clk_x_rate = 125 MHz,
>>>>> hence the driver sets NAND_KEEP_TIMINGS flag. This did not happen before
>>>>> and is actually incorrect, as on SoCFPGA we do not want to retain timings
>>>>> from previous stage (the timings might be incorrect or outright invalid).
>>>>>
>>>>> Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
>>>>> Cc: Dinh Nguyen <dinguyen@xxxxxxxxxx>
>>>>> Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx>
>>>>> Cc: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
>>>>> Cc: Tim Sander <tim@xxxxxxxxxxxxxxx>
>>>>> To: linux-mtd <linux-mtd@xxxxxxxxxxxxxxxxxxx>
>>>>> ---
>>>>>  drivers/mtd/nand/raw/denali.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
>>>>> index b6c463d02167..5fe3c62a756e 100644
>>>>> --- a/drivers/mtd/nand/raw/denali.c
>>>>> +++ b/drivers/mtd/nand/raw/denali.c
>>>>> @@ -1209,7 +1209,7 @@ int denali_chip_init(struct denali_controller *denali,
>>>>>  	}
>>>>>  
>>>>>  	/* clk rate info is needed for setup_data_interface */
>>>>> -	if (!denali->clk_rate || !denali->clk_x_rate)  
>>>>
>>>> I don't get it, if both clk_rate and clk_x_rate are set, the if
>>>> condition will not be entered, right?  
>>>
>>> Err, then it's the other way around and I need to keep the timings on
>>> socfpga ?
>>
>> Ok.
>>
>> Do you have a different compatible? Or a register to check? How do you
>> discriminate the different platforms by software? The quick and dirty
>> solution is to add a special case for your platform and specifically
>> use the NAND_KEEP_TIMINGS horror.
> 
> Sure, there's a socfpga compatible and at least two for uniphier.
> 
>> But I think using ->software_data_interface is the right solution. So
>> I would highly recommend fixing the implementation of this hook
>> for your platform and in this case the commit reverted is not the
>> culprit, the one introducing setup_data_interface is (for the Fixes:
>> tag).
> 
> I'll leave the details to Yamada-san.

Just got a confirmation that this fixes NAND behavior on SoCFPGA, so
this patch should go in in some form.

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