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

[Marek Vasut <marex@xxxxxxx>]

On 2/17/20 9:33 AM, Masahiro Yamada wrote:
> Hi,

Hi,

[...]

> If possible, can you provide the dump of
> the attached debug code?

Without this revert:

Denali: clk_rate=31250000, clk_x_rate=125000000
Denali: tREA=40000
Denali: acc_clks=5
Denali: tRHW=200000
Denali: re_2_we=25
Denali: tRHZ=200000
Denali: re_2_re=25
Denali: tCCS=500000000
Denali: tWHR=120000
Denali: we_2_re=63
Denali: tADL=400000
Denali: addr_2_data=50
Denali: tREH=30000
Denali: tWH=30000
Denali: rdwr_en_hi=4
Denali: tRP=50000
Denali: tWP=50000
Denali: tRC=100000
Denali: tWC=100000
Denali: rdwr_en_lo_hi=13
Denali: rdwr_en_lo=9
Denali: tCS=70000
Denali: tCEA=100000
Denali: cs_setup=8
Denali: clk_rate=31250000, clk_x_rate=125000000
Denali: tREA=16000
Denali: acc_clks=2
Denali: tRHW=100000
Denali: re_2_we=13
Denali: tRHZ=100000
Denali: re_2_re=13
Denali: tCCS=100000
Denali: tWHR=80000
Denali: we_2_re=13
Denali: tADL=400000
Denali: addr_2_data=50
Denali: tREH=7000
Denali: tWH=7000
Denali: rdwr_en_hi=1
Denali: tRP=10000
Denali: tWP=10000
Denali: tRC=20000
Denali: tWC=20000
Denali: rdwr_en_lo_hi=4
Denali: rdwr_en_lo=3
Denali: tCS=15000
Denali: tCEA=25000
Denali: cs_setup=2

With this revert, setup_data_interface() is not called, so there is no log.

[...]

>>> When denali->clk_x_rate is zero,
>>> NAND_KEEP_TIMINGS would not be set with your patch.
>>> So, ->setup_data_interface() would be called.
>>>
>>> This would cause zero divion at line 781.
>>>         t_x = DIV_ROUND_DOWN_ULL(1000000000000ULL, denali->clk_x_rate);
>>
>> If the clock are non-zero, how would this be a division by zero ?
> 
> 
> You have a false assumption "If the clock are non-zero...".
> 
> clk_get_rate() may return zero if the clock driver
> is not ready to provide the frequency information.
> 
> 
> 
> The current code:
>   If clk_rate or clk_x_rate is zero,
>    do not call denali_setup_data_interface().
>   If neither clk_rate nor clk_x is zero,
>    call denali_setup_data_interface().
> 
> 
> With your patch:
>   If clk_rate or clk_x_rate is zero,
>    call denali_setup_data_interface(),
>    which causes zero division.
>   If neither clk_rate nor clk_x is zero,
>    do not call denali_setup_data_interface().

OK, so it's just a miscommunication. In my case, neither of the clock
are zero. On SoCFPGA, I think clk_rate = clk_x_rate / 4, but I'm not
sure if that's always the case.

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