2018-06-22 20:42 GMT+09:00 Boris Brezillon boris.brezillon@bootlin.com:
Hi Masahiro,
On Tue, 19 Jun 2018 13:28:11 +0200 Boris Brezillon boris.brezillon@bootlin.com wrote:
Hi Masahiro,
On Fri, 15 Jun 2018 10:18:50 +0900 Masahiro Yamada yamada.masahiro@socionext.com wrote:
According to the Denali User's Guide, this IP needs three clocks:
clk: controller core clock
clk_x: bus interface clock
ecc_clk: clock at which ECC circuitry is run
Currently, denali_dt.c requires a single anonymous clock and its frequency. However, the driver needs to get the frequency of "clk_x" not "clk". This is confusing because people tend to assume the anonymous clock means the core clock. In fact, I got a report of SOCFPGA breakage because the timing parameters are calculated based on a wrong frequency.
Instead of the cheesy implementation, the clocks in the real hardware should be represented in the driver and the DT-binding.
However, adding new clocks would break the existing platforms. For the backward compatibility, the driver still accepts a single clock just as before. If clk_x is missing, clk_x_rate is set to a hardcoded value. This is fine for existing DT of Socionext UniPhier, and also fixes the issue of Altera (Intel) SOCFPGA because both platforms use 200 MHz for the bus interface clock.
Fixes: 1bb88666775e ("mtd: nand: denali: handle timing parameters by setup_data_interface()")
Sorry for changing my mind, but I think this patch is doing more than just fixing a bug. The bugfix should IMO be limited to unconditionally setting ->clk_x_rate to 200000000 since this is what was done before commit 1bb88666775e. And the remaining changes should go in nand/next.
Do you want me to write this patch? Note that I can't really take this patch since Rob asked for a new version with DT bindings changes placed in a separate patch.
Sorry for the delay. I was traveling.
Now I am back home, and started to flush my TODO queue.
V4 will be soon.