Re: [PATCH RFC v1 0/8] intel-nand-controller: Fixes, cleanups and questions

From: Miquel Raynal
Date: Tue Jun 28 2022 - 10:38:59 EST


Hi Martin,

martin.blumenstingl@xxxxxxxxxxxxxx wrote on Tue, 28 Jun 2022 13:27:23
+0200:

> Hello,
>
> I am trying to replace the xway_nand driver (which is still using the
> legacy NAND API) with the intel-nand-controller driver. The Intel LGM
> IP (for which intel-nand-controller was implemented) uses a newer
> version of the EBU NAND and HSNAND IP found in Lantiq XWAY SoCs. The
> most notable change is the addition of HSNAND Intel LGM SoCs (it's not
> clear to me if/which Lantiq SoCs also have this DMA engine).
>
> While testing my changes on a Lantiq xRX200 SoC I came across some
> issues with the intel-nand-controller driver. The problems I found are:
> 1) Mismatch between dt-bindings and driver implementation (compatible
> string, patch #1 and patch #4) and hardware capabilities (number of
> CS lines, patch #1).
> 2) The driver reads the CS (chip select) line from the NAND controller's
> reg property. In the dt-bindings example this is 0xe0f00000. I don't
> understand how this can even work on Intel SoCs. My understanding is
> that it must be read from the NAND chip (child node).

Yes

> 3) A few smaller code cleanups to make the driver easier to understand
> (patches #5 to #8)
> 4) I tried to understand the timing parameter calculation code but found
> that it probably doesn't work on the Intel LGM SoCs either. The
> dt-bindings example use clock ID 125 which is LGM_GCLK_EBU. So far
> this is fine because EBU is the actual IP block for the NAND
> interface. However, drivers/clk/x86/clk-lgm.c defines this clock as
> a gate without a parent, so it's rate (as read by Linux) is always 0.
> The intel-nand-controller driver then tries to calculate:
> rate = clk_get_rate(ctrl->clk) / HZ_PER_MHZ
> (rate will be 0 because clk_get_rate() returns 0) and then:
> DIV_ROUND_UP(USEC_PER_SEC, rate)
> (this then tries to divide by zero)
>
> Before this series is applied it would be great to have these questions
> answered:
> - My understanding is that the chip select line (reg property of the
> NAND controller's child node) refers to the chip select line of the
> controller. Let's say we have a controller with two CS lines. A NAND
> flash chip (which a single chip select line) is connected to the
> second CS line of the controller. Is my understanding correct that
> the NAND chip device-tree node should get reg = <1> in this case?

Yes

> - Who from Maxlinear (who took over Intel's AnyWAN division, which
> previously worked on the drivers for the Intel LGM SoCs) can send a
> patch to correct the LGM_GCLK_EBU clock rate in
> drivers/clk/x86/clk-lgm.c? Or is LGM dead and the various drivers
> should be removed instead?
> - Who from Maxlinear can provide insights into which clock is connected
> to the EBU NAND controller on Lantiq XWAY (Danube, xRX100, xRX200,
> xRX300) SoCs as well as newer GRX350/GRX550 SoCs so that I can make
> the intel-nand-controller work without hardcoded timing settings on
> the XWAY SoCs?
>
> Due to the severity of issues 2) and 4) above I am targeting linux-next
> with this series. In my opinion there's no point in backporting these
> fixes to a driver which has been broken since it was upstreamed.

The changes look good to me, please resend with the bindings file name
fixed and we should be good.

Thanks,
Miquèl