On 07/18/2018 09:22 PM, Masahiro Yamada wrote:
> 2018-07-11 23:51 GMT+09:00 Dinh Nguyen <dinh.linux@xxxxxxxxx>:
>> Hi Masahiro,
>>
>> On Tue, Jul 10, 2018 at 9:45 PM Masahiro Yamada
>> <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>>>
>>> 2018-07-10 22:35 GMT+09:00 Dinh Nguyen <dinguyen@xxxxxxxxxx>:
>>>>
>>>>
>>>> On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
>>>>> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@xxxxxxxxxx>:
>>>>>> The NAND IP needs 2 clocks(nand_x_clk and nand_clk). This patch adds a
>>>>>> nand_x_clk, which is derived from the nand_clk, but has a fixed divider
>>>>>> of 4.
>>>>>>
>>>>>> Update the NAND node to use the additional clock.
>>>>>>
>>>>>> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxx>
>>>>>> ---
>>>>>> arch/arm/boot/dts/socfpga_arria10.dtsi | 13 +++++++++++--
>>>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
>>>>>> index a4dcb68..558b5ea 100644
>>>>>> --- a/arch/arm/boot/dts/socfpga_arria10.dtsi
>>>>>> +++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
>>>>>> @@ -377,13 +377,21 @@
>>>>>> clk-gate = <0xC8 11>;
>>>>>> };
>>>>>>
>>>>>> - nand_clk: nand_clk {
>>>>>> + nand_x_clk: nand_x_clk {
>>>>>> #clock-cells = <0>;
>>>>>> compatible = "altr,socfpga-a10-gate-clk";
>>>>>> clocks = <&l4_mp_clk>;
>>>>>> clk-gate = <0xC8 10>;
>>>>>> };
>>>>>>
>>>>>> + nand_clk: nand_clk {
>>>>>> + #clock-cells = <0>;
>>>>>> + compatible = "altr,socfpga-a10-gate-clk";
>>>>>> + clocks = <&nand_x_clk>;
>>>>>> + fixed-divider = <4>;
>>>>>> + clk-gate = <0xC8 10>;
>>>>>> + };
>>>>>> +
>>>>>> spi_m_clk: spi_m_clk {
>>>>>> #clock-cells = <0>;
>>>>>> compatible = "altr,socfpga-a10-gate-clk";
>>>>>> @@ -650,7 +658,8 @@
>>>>>> reg-names = "nand_data", "denali_reg";
>>>>>> interrupts = <0 99 4>;
>>>>>> dma-mask = <0xffffffff>;
>>>>>> - clocks = <&nand_clk>;
>>>>>> + clocks = <&nand_clk>, <&nand_x_clk>;
>>>>>> + clock-names = "nand", "nand_x";
>>>>>
>>>>>
>>>>> Doesn't SOCFPGA really need the ECC clock?
>>>>>
>>>>
>>>> I don't see any mentions of an ECC clock in any of the SoCFPGA
>>>> documentation for the NAND IP.
>>>
>>>
>>> I'd like you to check the IP documentation
>>> published by Denali (or Cadence),
>>> not published by Altera.
>>>
>>> Is it available for you?
>>>
>>>
>>>
>>> In my SoCs, the same clock signal (nand-clk-200m)
>>> is connected to clk_x and ecc_clk, like this:
>>>
>>> -------------
>>> -------------- | DENALI IP |
>>> | | | |
>>> | clk |---(nand-clk-50m)--------->| clk |
>>> | controller | | |
>>> | |---(nand-clk-200m)-------->| clk_x |
>>> -------------- | | |
>>> --->| ecc_clk |
>>> | |
>>> -------------
Here's a diagram of the clocking for the NAND IP on SOCFPGA.
-------------
-------------- | DENALI IP |
| | | |
| clk | | |-->clk(/4) |
| controller | | | |
| |---(nand_clk)-------------|-->clk_x |
------------- | | |
| |-->ecc_clk |
| |
---------------
>From the clock controller, it provides a single 200MHz clock to the NAND
IP. Inside the NAND IP, there is a /4 for the clk. The 200MHz clock is
used for the clk_x and ecc_clk.
The nand-clk is the only clock that can be gated.
>>>
>>>
>>> Some clock lines are often tied-up,
>>> but such low-level information does not appear in
>>> user documents.
>>> I talked to a hardware engineer to get this information.
>>>
>>>
>>>
>>> I guess the situation is similar for your case,
>>> but I do not have information enough to argue this.
>>>
>>
>> I don't disagree with this representation. But after asking around,
>> I'm getting the impression that the ECC clock is not exposed to the
>> clock manager. This implies that the clock is always turned on and
>> there doesn't seem to be any software control for it. I tested ECC
>> with this patch and it looks to be working correctly, without any ecc
>> clock representation.
>>
>> I'm not sure if I should expose a dummy ecc_clk to satisfy the binding?
>
>
> DT should represent the real HW.
> If the Denali IP in SOCFPGA needs ecc_clk input,
> DT should describe it.
>
>
> I do not have knowledge about the SOCFPGA internals.
> I do not know which is correct:
>
> - The Denali IP in SOCFPGA needs ecc_clk,
> but there is no software control for it.
>
> - The Denali IP in SOCFPGA does not need ecc_clk.
>
So should I be representing the clocks even though it doesn't come from
the clock controller?
Dinh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
