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

[Marek Vasut <marex@xxxxxxx>]

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.

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