* [PATCH 1/2] ARM: dts: arria10: update NAND clocking
@ 2018-07-09 22:41 Dinh Nguyen
2018-07-09 22:41 ` [PATCH 2/2] ARM: dts: socfpga: update NAND clocking for c5/a5 Dinh Nguyen
2018-07-10 1:31 ` [PATCH 1/2] ARM: dts: arria10: update NAND clocking Masahiro Yamada
0 siblings, 2 replies; 9+ messages in thread
From: Dinh Nguyen @ 2018-07-09 22:41 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, marex, yamada.masahiro, dinguyen, devicetree, robh+dt
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@kernel.org>
---
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";
status = "disabled";
};
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: dts: socfpga: update NAND clocking for c5/a5
2018-07-09 22:41 [PATCH 1/2] ARM: dts: arria10: update NAND clocking Dinh Nguyen
@ 2018-07-09 22:41 ` Dinh Nguyen
2018-07-10 1:31 ` [PATCH 1/2] ARM: dts: arria10: update NAND clocking Masahiro Yamada
1 sibling, 0 replies; 9+ messages in thread
From: Dinh Nguyen @ 2018-07-09 22:41 UTC (permalink / raw)
To: linux-arm-kernel
Cc: mark.rutland, marex, yamada.masahiro, dinguyen, devicetree, robh+dt
The nand_x_clk is derived from the nand_clk. Also, update the NAND dts property
with the correct clocks property.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
arch/arm/boot/dts/socfpga.dtsi | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index b38f8c2..ae432b9 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -486,7 +486,7 @@
nand_clk: nand_clk {
#clock-cells = <0>;
compatible = "altr,socfpga-gate-clk";
- clocks = <&f2s_periph_ref_clk>, <&main_nand_sdmmc_clk>, <&per_nand_mmc_clk>;
+ clocks = <&nand_x_clk>;
clk-gate = <0xa0 10>;
fixed-divider = <4>;
};
@@ -754,7 +754,8 @@
reg-names = "nand_data", "denali_reg";
interrupts = <0x0 0x90 0x4>;
dma-mask = <0xffffffff>;
- clocks = <&nand_x_clk>;
+ clocks = <&nand_clk>, <&nand_x_clk>;
+ clock-names = "nand", "nand_x";
status = "disabled";
};
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-09 22:41 [PATCH 1/2] ARM: dts: arria10: update NAND clocking Dinh Nguyen
2018-07-09 22:41 ` [PATCH 2/2] ARM: dts: socfpga: update NAND clocking for c5/a5 Dinh Nguyen
@ 2018-07-10 1:31 ` Masahiro Yamada
2018-07-10 13:35 ` Dinh Nguyen
1 sibling, 1 reply; 9+ messages in thread
From: Masahiro Yamada @ 2018-07-10 1:31 UTC (permalink / raw)
To: Dinh Nguyen
Cc: Mark Rutland, DTML, Marek Vasut, Richard Weinberger, Rob Herring,
linux-arm-kernel
2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
> 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@kernel.org>
> ---
> 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?
SOCFPGA has HW ECC engine.
I suggested to add ECC clock, and Richard agreed.
https://lkml.org/lkml/2018/6/22/573
> status = "disabled";
> };
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-10 1:31 ` [PATCH 1/2] ARM: dts: arria10: update NAND clocking Masahiro Yamada
@ 2018-07-10 13:35 ` Dinh Nguyen
2018-07-11 2:43 ` Masahiro Yamada
0 siblings, 1 reply; 9+ messages in thread
From: Dinh Nguyen @ 2018-07-10 13:35 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Mark Rutland, DTML, Marek Vasut, Richard Weinberger, Rob Herring,
linux-arm-kernel
On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>> 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@kernel.org>
>> ---
>> 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.
Dinh
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-10 13:35 ` Dinh Nguyen
@ 2018-07-11 2:43 ` Masahiro Yamada
2018-07-11 14:51 ` Dinh Nguyen
0 siblings, 1 reply; 9+ messages in thread
From: Masahiro Yamada @ 2018-07-11 2:43 UTC (permalink / raw)
To: Dinh Nguyen
Cc: Mark Rutland, DTML, Marek Vasut, Richard Weinberger, Rob Herring,
linux-arm-kernel
2018-07-10 22:35 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>
>
> On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
>> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>>> 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@kernel.org>
>>> ---
>>> 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 |
| |
-------------
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.
--
Best Regards
Masahiro Yamada
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-11 2:43 ` Masahiro Yamada
@ 2018-07-11 14:51 ` Dinh Nguyen
2018-07-19 2:22 ` Masahiro Yamada
0 siblings, 1 reply; 9+ messages in thread
From: Dinh Nguyen @ 2018-07-11 14:51 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Mark Rutland, Marek Vasut, devicetree, Richard Weinberger,
Dinh Nguyen, Rob Herring, linux-arm-kernel
Hi Masahiro,
On Tue, Jul 10, 2018 at 9:45 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> 2018-07-10 22:35 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
> >
> >
> > On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
> >> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
> >>> 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@kernel.org>
> >>> ---
> >>> 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 |
> | |
> -------------
>
>
> 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?
Dinh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-11 14:51 ` Dinh Nguyen
@ 2018-07-19 2:22 ` Masahiro Yamada
2018-07-20 23:16 ` Dinh Nguyen
0 siblings, 1 reply; 9+ messages in thread
From: Masahiro Yamada @ 2018-07-19 2:22 UTC (permalink / raw)
To: Dinh Nguyen
Cc: Mark Rutland, Marek Vasut, devicetree, Richard Weinberger,
Dinh Nguyen, Rob Herring, linux-arm-kernel
2018-07-11 23:51 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
> Hi Masahiro,
>
> On Tue, Jul 10, 2018 at 9:45 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>>
>> 2018-07-10 22:35 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>> >
>> >
>> > On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
>> >> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>> >>> 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@kernel.org>
>> >>> ---
>> >>> 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 |
>> | |
>> -------------
>>
>>
>> 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.
> Dinh
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-19 2:22 ` Masahiro Yamada
@ 2018-07-20 23:16 ` Dinh Nguyen
2018-07-23 1:48 ` Masahiro Yamada
0 siblings, 1 reply; 9+ messages in thread
From: Dinh Nguyen @ 2018-07-20 23:16 UTC (permalink / raw)
To: Masahiro Yamada, Dinh Nguyen
Cc: Mark Rutland, devicetree, Marek Vasut, Richard Weinberger,
Rob Herring, linux-arm-kernel
On 07/18/2018 09:22 PM, Masahiro Yamada wrote:
> 2018-07-11 23:51 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>> Hi Masahiro,
>>
>> On Tue, Jul 10, 2018 at 9:45 PM Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>>
>>> 2018-07-10 22:35 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>>>>
>>>>
>>>> On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
>>>>> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>>>>>> 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@kernel.org>
>>>>>> ---
>>>>>> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: dts: arria10: update NAND clocking
2018-07-20 23:16 ` Dinh Nguyen
@ 2018-07-23 1:48 ` Masahiro Yamada
0 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2018-07-23 1:48 UTC (permalink / raw)
To: Dinh Nguyen
Cc: Mark Rutland, devicetree, Marek Vasut, Dinh Nguyen,
Richard Weinberger, Rob Herring, linux-arm-kernel
2018-07-21 8:16 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>
>
> On 07/18/2018 09:22 PM, Masahiro Yamada wrote:
>> 2018-07-11 23:51 GMT+09:00 Dinh Nguyen <dinh.linux@gmail.com>:
>>> Hi Masahiro,
>>>
>>> On Tue, Jul 10, 2018 at 9:45 PM Masahiro Yamada
>>> <yamada.masahiro@socionext.com> wrote:
>>>>
>>>> 2018-07-10 22:35 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>>>>>
>>>>>
>>>>> On 07/09/2018 08:31 PM, Masahiro Yamada wrote:
>>>>>> 2018-07-10 7:41 GMT+09:00 Dinh Nguyen <dinguyen@kernel.org>:
>>>>>>> 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@kernel.org>
>>>>>>> ---
>>>>>>> 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.
The Denali IP does not contain the divisor in it.
My best guess is Altera implemented a simple
wrapper with the 1/4 divisor, like this?
----------------------------
| Wrapper |
| ------------- |
| | DENALI IP | |
| | | |
| |-(1/4)-->| clk | |
| | | | |
---(nand_clk)--|--|-------->| clk_x | |
| | | | |
| |-------->| ecc_clk | |
| | | |
| ------------- |
----------------------------
>>>>
>>>>
>>>> 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?
Anyway, you know the same clock is connected to nand_x to ecc_clk.
So, there is no problem for adding "ecc" as well, right?
--
Best Regards
Masahiro Yamada
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-07-23 1:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-09 22:41 [PATCH 1/2] ARM: dts: arria10: update NAND clocking Dinh Nguyen
2018-07-09 22:41 ` [PATCH 2/2] ARM: dts: socfpga: update NAND clocking for c5/a5 Dinh Nguyen
2018-07-10 1:31 ` [PATCH 1/2] ARM: dts: arria10: update NAND clocking Masahiro Yamada
2018-07-10 13:35 ` Dinh Nguyen
2018-07-11 2:43 ` Masahiro Yamada
2018-07-11 14:51 ` Dinh Nguyen
2018-07-19 2:22 ` Masahiro Yamada
2018-07-20 23:16 ` Dinh Nguyen
2018-07-23 1:48 ` Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).