linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] clk: imx7d: fix USDHC NAND clock
@ 2017-03-30  0:50 Stefan Agner
  2017-03-30  0:50 ` [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances Stefan Agner
  2017-04-01  3:00 ` [PATCH 1/2] clk: imx7d: fix USDHC NAND clock Dong Aisheng
  0 siblings, 2 replies; 10+ messages in thread
From: Stefan Agner @ 2017-03-30  0:50 UTC (permalink / raw)
  To: shawnguo, kernel, sboyd
  Cc: aisheng.dong, fabio.estevam, robh+dt, mark.rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel,
	Stefan Agner

The USDHC NAND root clock is not gated by any CCM clock gate. Remove
the bogus gate definition.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
The IMX7D_NAND_USDHC_BUS_ROOT_CLK clock is also in clks_init_on.
In a quick test I removed IMX7D_NAND_USDHC_BUS_ROOT_CLK from
clks_init_on, and the system including SD-cards seemed to work
fine... So I guess we could remove the clock from clks_init_on
after the two both changes got applied, any thoughts?

The gate 0x4130 was actually the DRAM gate, hence disabling that
clock lead to disabling this gate, and hence a crash before this
fixes... Maybe that was the reason it landed in clks_init_on...?

--
Stefan

 drivers/clk/imx/clk-imx7d.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index ae1d31be906e..4466acaacb71 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -724,7 +724,7 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_MAIN_AXI_ROOT_DIV] = imx_clk_divider2("axi_post_div", "axi_pre_div", base + 0x8800, 0, 6);
 	clks[IMX7D_DISP_AXI_ROOT_DIV] = imx_clk_divider2("disp_axi_post_div", "disp_axi_pre_div", base + 0x8880, 0, 6);
 	clks[IMX7D_ENET_AXI_ROOT_DIV] = imx_clk_divider2("enet_axi_post_div", "enet_axi_pre_div", base + 0x8900, 0, 6);
-	clks[IMX7D_NAND_USDHC_BUS_ROOT_DIV] = imx_clk_divider2("nand_usdhc_post_div", "nand_usdhc_pre_div", base + 0x8980, 0, 6);
+	clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = imx_clk_divider2("nand_usdhc_root_clk", "nand_usdhc_pre_div", base + 0x8980, 0, 6);
 	clks[IMX7D_AHB_CHANNEL_ROOT_DIV] = imx_clk_divider2("ahb_post_div", "ahb_pre_div", base + 0x9000, 0, 6);
 	clks[IMX7D_DRAM_ROOT_DIV] = imx_clk_divider2("dram_post_div", "dram_cg", base + 0x9880, 0, 3);
 	clks[IMX7D_DRAM_PHYM_ALT_ROOT_DIV] = imx_clk_divider2("dram_phym_alt_post_div", "dram_phym_alt_pre_div", base + 0xa000, 0, 3);
@@ -797,7 +797,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_ENET_AXI_ROOT_CLK] = imx_clk_gate4("enet_axi_root_clk", "enet_axi_post_div", base + 0x4060, 0);
 	clks[IMX7D_OCRAM_CLK] = imx_clk_gate4("ocram_clk", "axi_post_div", base + 0x4110, 0);
 	clks[IMX7D_OCRAM_S_CLK] = imx_clk_gate4("ocram_s_clk", "ahb_post_div", base + 0x4120, 0);
-	clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = imx_clk_gate4("nand_usdhc_root_clk", "nand_usdhc_post_div", base + 0x4130, 0);
 	clks[IMX7D_AHB_CHANNEL_ROOT_CLK] = imx_clk_gate4("ahb_root_clk", "ahb_post_div", base + 0x4200, 0);
 	clks[IMX7D_DRAM_ROOT_CLK] = imx_clk_gate4("dram_root_clk", "dram_post_div", base + 0x4130, 0);
 	clks[IMX7D_DRAM_PHYM_ROOT_CLK] = imx_clk_gate4("dram_phym_root_clk", "dram_phym_cg", base + 0x4130, 0);
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-03-30  0:50 [PATCH 1/2] clk: imx7d: fix USDHC NAND clock Stefan Agner
@ 2017-03-30  0:50 ` Stefan Agner
  2017-04-01  3:03   ` Dong Aisheng
  2017-04-01  3:00 ` [PATCH 1/2] clk: imx7d: fix USDHC NAND clock Dong Aisheng
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2017-03-30  0:50 UTC (permalink / raw)
  To: shawnguo, kernel, sboyd
  Cc: aisheng.dong, fabio.estevam, robh+dt, mark.rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel,
	Stefan Agner

The USDHC instances need the USDHC NAND clock in order to operate.
Add the clock as ahb bus clock.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/imx7s.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 5d3a43b8de20..5794febb19a4 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -936,7 +936,7 @@
 				reg = <0x30b40000 0x10000>;
 				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_CLK_DUMMY>,
-					<&clks IMX7D_CLK_DUMMY>,
+					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
 					<&clks IMX7D_USDHC1_ROOT_CLK>;
 				clock-names = "ipg", "ahb", "per";
 				bus-width = <4>;
@@ -948,7 +948,7 @@
 				reg = <0x30b50000 0x10000>;
 				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_CLK_DUMMY>,
-					<&clks IMX7D_CLK_DUMMY>,
+					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
 					<&clks IMX7D_USDHC2_ROOT_CLK>;
 				clock-names = "ipg", "ahb", "per";
 				bus-width = <4>;
@@ -960,7 +960,7 @@
 				reg = <0x30b60000 0x10000>;
 				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_CLK_DUMMY>,
-					<&clks IMX7D_CLK_DUMMY>,
+					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
 					<&clks IMX7D_USDHC3_ROOT_CLK>;
 				clock-names = "ipg", "ahb", "per";
 				bus-width = <4>;
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] clk: imx7d: fix USDHC NAND clock
  2017-03-30  0:50 [PATCH 1/2] clk: imx7d: fix USDHC NAND clock Stefan Agner
  2017-03-30  0:50 ` [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances Stefan Agner
@ 2017-04-01  3:00 ` Dong Aisheng
  1 sibling, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2017-04-01  3:00 UTC (permalink / raw)
  To: Stefan Agner
  Cc: shawnguo, kernel, sboyd, aisheng.dong, fabio.estevam, robh+dt,
	mark.rutland, linux-arm-kernel, devicetree, linux-clk,
	linux-kernel

On Wed, Mar 29, 2017 at 05:50:28PM -0700, Stefan Agner wrote:
> The USDHC NAND root clock is not gated by any CCM clock gate. Remove
> the bogus gate definition.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> The IMX7D_NAND_USDHC_BUS_ROOT_CLK clock is also in clks_init_on.
> In a quick test I removed IMX7D_NAND_USDHC_BUS_ROOT_CLK from
> clks_init_on, and the system including SD-cards seemed to work
> fine... So I guess we could remove the clock from clks_init_on
> after the two both changes got applied, any thoughts?
> 

Yes, it can be removed after apply.

> The gate 0x4130 was actually the DRAM gate, hence disabling that
> clock lead to disabling this gate, and hence a crash before this
> fixes... Maybe that was the reason it landed in clks_init_on...?
> 

Probably a history reason or mistake. :-)

Acked-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng

> --
> Stefan
> 
>  drivers/clk/imx/clk-imx7d.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> index ae1d31be906e..4466acaacb71 100644
> --- a/drivers/clk/imx/clk-imx7d.c
> +++ b/drivers/clk/imx/clk-imx7d.c
> @@ -724,7 +724,7 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  	clks[IMX7D_MAIN_AXI_ROOT_DIV] = imx_clk_divider2("axi_post_div", "axi_pre_div", base + 0x8800, 0, 6);
>  	clks[IMX7D_DISP_AXI_ROOT_DIV] = imx_clk_divider2("disp_axi_post_div", "disp_axi_pre_div", base + 0x8880, 0, 6);
>  	clks[IMX7D_ENET_AXI_ROOT_DIV] = imx_clk_divider2("enet_axi_post_div", "enet_axi_pre_div", base + 0x8900, 0, 6);
> -	clks[IMX7D_NAND_USDHC_BUS_ROOT_DIV] = imx_clk_divider2("nand_usdhc_post_div", "nand_usdhc_pre_div", base + 0x8980, 0, 6);
> +	clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = imx_clk_divider2("nand_usdhc_root_clk", "nand_usdhc_pre_div", base + 0x8980, 0, 6);
>  	clks[IMX7D_AHB_CHANNEL_ROOT_DIV] = imx_clk_divider2("ahb_post_div", "ahb_pre_div", base + 0x9000, 0, 6);
>  	clks[IMX7D_DRAM_ROOT_DIV] = imx_clk_divider2("dram_post_div", "dram_cg", base + 0x9880, 0, 3);
>  	clks[IMX7D_DRAM_PHYM_ALT_ROOT_DIV] = imx_clk_divider2("dram_phym_alt_post_div", "dram_phym_alt_pre_div", base + 0xa000, 0, 3);
> @@ -797,7 +797,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  	clks[IMX7D_ENET_AXI_ROOT_CLK] = imx_clk_gate4("enet_axi_root_clk", "enet_axi_post_div", base + 0x4060, 0);
>  	clks[IMX7D_OCRAM_CLK] = imx_clk_gate4("ocram_clk", "axi_post_div", base + 0x4110, 0);
>  	clks[IMX7D_OCRAM_S_CLK] = imx_clk_gate4("ocram_s_clk", "ahb_post_div", base + 0x4120, 0);
> -	clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = imx_clk_gate4("nand_usdhc_root_clk", "nand_usdhc_post_div", base + 0x4130, 0);
>  	clks[IMX7D_AHB_CHANNEL_ROOT_CLK] = imx_clk_gate4("ahb_root_clk", "ahb_post_div", base + 0x4200, 0);
>  	clks[IMX7D_DRAM_ROOT_CLK] = imx_clk_gate4("dram_root_clk", "dram_post_div", base + 0x4130, 0);
>  	clks[IMX7D_DRAM_PHYM_ROOT_CLK] = imx_clk_gate4("dram_phym_root_clk", "dram_phym_cg", base + 0x4130, 0);
> -- 
> 2.12.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-03-30  0:50 ` [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances Stefan Agner
@ 2017-04-01  3:03   ` Dong Aisheng
  2017-04-01  4:15     ` Stefan Agner
  0 siblings, 1 reply; 10+ messages in thread
From: Dong Aisheng @ 2017-04-01  3:03 UTC (permalink / raw)
  To: Stefan Agner
  Cc: shawnguo, kernel, sboyd, aisheng.dong, fabio.estevam, robh+dt,
	mark.rutland, linux-arm-kernel, devicetree, linux-clk,
	linux-kernel

On Wed, Mar 29, 2017 at 05:50:29PM -0700, Stefan Agner wrote:
> The USDHC instances need the USDHC NAND clock in order to operate.
> Add the clock as ahb bus clock.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  arch/arm/boot/dts/imx7s.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 5d3a43b8de20..5794febb19a4 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -936,7 +936,7 @@
>  				reg = <0x30b40000 0x10000>;
>  				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clks IMX7D_CLK_DUMMY>,

Would you please change the left ipg dummy to IMX7D_IPG_ROOT_CLK as well?

Otherwise,

Acked-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng

> -					<&clks IMX7D_CLK_DUMMY>,
> +					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>  					<&clks IMX7D_USDHC1_ROOT_CLK>;
>  				clock-names = "ipg", "ahb", "per";
>  				bus-width = <4>;
> @@ -948,7 +948,7 @@
>  				reg = <0x30b50000 0x10000>;
>  				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clks IMX7D_CLK_DUMMY>,
> -					<&clks IMX7D_CLK_DUMMY>,
> +					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>  					<&clks IMX7D_USDHC2_ROOT_CLK>;
>  				clock-names = "ipg", "ahb", "per";
>  				bus-width = <4>;
> @@ -960,7 +960,7 @@
>  				reg = <0x30b60000 0x10000>;
>  				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>  				clocks = <&clks IMX7D_CLK_DUMMY>,
> -					<&clks IMX7D_CLK_DUMMY>,
> +					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>  					<&clks IMX7D_USDHC3_ROOT_CLK>;
>  				clock-names = "ipg", "ahb", "per";
>  				bus-width = <4>;
> -- 
> 2.12.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-04-01  3:03   ` Dong Aisheng
@ 2017-04-01  4:15     ` Stefan Agner
  2017-04-02 17:02       ` Fabio Estevam
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2017-04-01  4:15 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: shawnguo, kernel, sboyd, aisheng.dong, fabio.estevam, robh+dt,
	mark.rutland, linux-arm-kernel, devicetree, linux-clk,
	linux-kernel

On 2017-03-31 20:03, Dong Aisheng wrote:
> On Wed, Mar 29, 2017 at 05:50:29PM -0700, Stefan Agner wrote:
>> The USDHC instances need the USDHC NAND clock in order to operate.
>> Add the clock as ahb bus clock.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  arch/arm/boot/dts/imx7s.dtsi | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 5d3a43b8de20..5794febb19a4 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -936,7 +936,7 @@
>>  				reg = <0x30b40000 0x10000>;
>>  				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
>>  				clocks = <&clks IMX7D_CLK_DUMMY>,
> 
> Would you please change the left ipg dummy to IMX7D_IPG_ROOT_CLK as well?

IMX7D_IPG_ROOT_CLK is currently not a valid clock in upstream... So we
would have to add it to the clock driver first.

I guess we could/should add it anyway at one point? But probably also as
init on, just to make sure Linux does not disable it since it is
currently used by several IPs implicitly.

--
Stefan

> 
> Otherwise,
> 
> Acked-by: Dong Aisheng <aisheng.dong@nxp.com>
> 
> Regards
> Dong Aisheng
> 
>> -					<&clks IMX7D_CLK_DUMMY>,
>> +					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>>  					<&clks IMX7D_USDHC1_ROOT_CLK>;
>>  				clock-names = "ipg", "ahb", "per";
>>  				bus-width = <4>;
>> @@ -948,7 +948,7 @@
>>  				reg = <0x30b50000 0x10000>;
>>  				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
>>  				clocks = <&clks IMX7D_CLK_DUMMY>,
>> -					<&clks IMX7D_CLK_DUMMY>,
>> +					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>>  					<&clks IMX7D_USDHC2_ROOT_CLK>;
>>  				clock-names = "ipg", "ahb", "per";
>>  				bus-width = <4>;
>> @@ -960,7 +960,7 @@
>>  				reg = <0x30b60000 0x10000>;
>>  				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>>  				clocks = <&clks IMX7D_CLK_DUMMY>,
>> -					<&clks IMX7D_CLK_DUMMY>,
>> +					<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>>  					<&clks IMX7D_USDHC3_ROOT_CLK>;
>>  				clock-names = "ipg", "ahb", "per";
>>  				bus-width = <4>;
>> --
>> 2.12.1
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-04-01  4:15     ` Stefan Agner
@ 2017-04-02 17:02       ` Fabio Estevam
  2017-04-05  2:15         ` Fabio Estevam
  0 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2017-04-02 17:02 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Dong Aisheng, Shawn Guo, Sascha Hauer, Stephen Boyd,
	Dong Aisheng, Fabio Estevam, robh+dt, Mark Rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel

On Sat, Apr 1, 2017 at 1:15 AM, Stefan Agner <stefan@agner.ch> wrote:

> IMX7D_IPG_ROOT_CLK is currently not a valid clock in upstream... So we
> would have to add it to the clock driver first.
>
> I guess we could/should add it anyway at one point? But probably also as
> init on, just to make sure Linux does not disable it since it is
> currently used by several IPs implicitly.

Yes, I made a previous attempt do add  IMX7D_IPG_ROOT_CLK and it did
not work as I did not put it in the init_on clock list.

Will submit a new patch adding it to init_on, thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-04-02 17:02       ` Fabio Estevam
@ 2017-04-05  2:15         ` Fabio Estevam
  2017-04-05  2:36           ` Stefan Agner
  0 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2017-04-05  2:15 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Dong Aisheng, Shawn Guo, Sascha Hauer, Stephen Boyd,
	Dong Aisheng, Fabio Estevam, robh+dt, Mark Rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel

On Sun, Apr 2, 2017 at 2:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sat, Apr 1, 2017 at 1:15 AM, Stefan Agner <stefan@agner.ch> wrote:
>
>> IMX7D_IPG_ROOT_CLK is currently not a valid clock in upstream... So we
>> would have to add it to the clock driver first.
>>
>> I guess we could/should add it anyway at one point? But probably also as
>> init on, just to make sure Linux does not disable it since it is
>> currently used by several IPs implicitly.
>
> Yes, I made a previous attempt do add  IMX7D_IPG_ROOT_CLK and it did
> not work as I did not put it in the init_on clock list.
>
> Will submit a new patch adding it to init_on, thanks.

I thought that adding IMX7D_IPG_ROOT_CLK would do the trick, but the
patch below also causes the kernel to not boot:

--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -386,7 +386,7 @@ static int const clks_init_on[] __initconst = {
        IMX7D_PLL_SYS_MAIN_480M_CLK, IMX7D_NAND_USDHC_BUS_ROOT_CLK,
        IMX7D_DRAM_PHYM_ROOT_CLK, IMX7D_DRAM_ROOT_CLK,
        IMX7D_DRAM_PHYM_ALT_ROOT_CLK, IMX7D_DRAM_ALT_ROOT_CLK,
-       IMX7D_AHB_CHANNEL_ROOT_CLK,
+       IMX7D_AHB_CHANNEL_ROOT_CLK, IMX7D_IPG_ROOT_CLK,
 };

 static struct clk_onecell_data clk_data;
@@ -788,7 +788,7 @@ static void __init imx7d_clocks_init(struct
device_node *ccm_node)
        clks[IMX7D_WRCLK_ROOT_DIV] =
imx_clk_divider2("wrclk_post_div", "wrclk_pre_div", base + 0xbd00, 0,
6);
        clks[IMX7D_CLKO1_ROOT_DIV] =
imx_clk_divider2("clko1_post_div", "clko1_pre_div", base + 0xbd80, 0,
6);
        clks[IMX7D_CLKO2_ROOT_DIV] =
imx_clk_divider2("clko2_post_div", "clko2_pre_div", base + 0xbe00, 0,
6);
-
+       clks[IMX7D_IPG_ROOT_CLK] = imx_clk_divider2("ipg_root_clk",
"ahb_root_clk", base + 0x9080, 0, 2);
        clks[IMX7D_ARM_A7_ROOT_CLK] = imx_clk_gate4("arm_a7_root_clk",
"arm_a7_div", base + 0x4000, 0);
        clks[IMX7D_ARM_M4_ROOT_CLK] = imx_clk_gate4("arm_m4_root_clk",
"arm_m4_div", base + 0x4010, 0);
        clks[IMX7D_ARM_M0_ROOT_CLK] = imx_clk_gate4("arm_m0_root_clk",
"arm_m0_div", base + 0x4020, 0);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-04-05  2:15         ` Fabio Estevam
@ 2017-04-05  2:36           ` Stefan Agner
  2017-04-11  2:59             ` Dong Aisheng
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2017-04-05  2:36 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Dong Aisheng, Shawn Guo, Sascha Hauer, Stephen Boyd,
	Dong Aisheng, Fabio Estevam, robh+dt, Mark Rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel

On 2017-04-04 19:15, Fabio Estevam wrote:
> On Sun, Apr 2, 2017 at 2:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Sat, Apr 1, 2017 at 1:15 AM, Stefan Agner <stefan@agner.ch> wrote:
>>
>>> IMX7D_IPG_ROOT_CLK is currently not a valid clock in upstream... So we
>>> would have to add it to the clock driver first.
>>>
>>> I guess we could/should add it anyway at one point? But probably also as
>>> init on, just to make sure Linux does not disable it since it is
>>> currently used by several IPs implicitly.
>>
>> Yes, I made a previous attempt do add  IMX7D_IPG_ROOT_CLK and it did
>> not work as I did not put it in the init_on clock list.
>>
>> Will submit a new patch adding it to init_on, thanks.
> 
> I thought that adding IMX7D_IPG_ROOT_CLK would do the trick, but the
> patch below also causes the kernel to not boot:
> 
> --- a/drivers/clk/imx/clk-imx7d.c
> +++ b/drivers/clk/imx/clk-imx7d.c
> @@ -386,7 +386,7 @@ static int const clks_init_on[] __initconst = {
>         IMX7D_PLL_SYS_MAIN_480M_CLK, IMX7D_NAND_USDHC_BUS_ROOT_CLK,
>         IMX7D_DRAM_PHYM_ROOT_CLK, IMX7D_DRAM_ROOT_CLK,
>         IMX7D_DRAM_PHYM_ALT_ROOT_CLK, IMX7D_DRAM_ALT_ROOT_CLK,
> -       IMX7D_AHB_CHANNEL_ROOT_CLK,
> +       IMX7D_AHB_CHANNEL_ROOT_CLK, IMX7D_IPG_ROOT_CLK,
>  };
> 
>  static struct clk_onecell_data clk_data;
> @@ -788,7 +788,7 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>         clks[IMX7D_WRCLK_ROOT_DIV] =
> imx_clk_divider2("wrclk_post_div", "wrclk_pre_div", base + 0xbd00, 0,
> 6);
>         clks[IMX7D_CLKO1_ROOT_DIV] =
> imx_clk_divider2("clko1_post_div", "clko1_pre_div", base + 0xbd80, 0,
> 6);
>         clks[IMX7D_CLKO2_ROOT_DIV] =
> imx_clk_divider2("clko2_post_div", "clko2_pre_div", base + 0xbe00, 0,
> 6);
> -
> +       clks[IMX7D_IPG_ROOT_CLK] = imx_clk_divider2("ipg_root_clk",
> "ahb_root_clk", base + 0x9080, 0, 2);
>         clks[IMX7D_ARM_A7_ROOT_CLK] = imx_clk_gate4("arm_a7_root_clk",
> "arm_a7_div", base + 0x4000, 0);
>         clks[IMX7D_ARM_M4_ROOT_CLK] = imx_clk_gate4("arm_m4_root_clk",
> "arm_m4_div", base + 0x4010, 0);
>         clks[IMX7D_ARM_M0_ROOT_CLK] = imx_clk_gate4("arm_m0_root_clk",
> "arm_m0_div", base + 0x4020, 0);

Hm, imx_clk_divider2 sets CLK_SET_RATE_PARENT, maybe that influences the
parent?

I guess we actually don't want the clock framework to change that clock
rate, not sure whether we can freeze it or similar.

--
Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-04-05  2:36           ` Stefan Agner
@ 2017-04-11  2:59             ` Dong Aisheng
  2017-04-11 23:23               ` Fabio Estevam
  0 siblings, 1 reply; 10+ messages in thread
From: Dong Aisheng @ 2017-04-11  2:59 UTC (permalink / raw)
  To: Stefan Agner
  Cc: Fabio Estevam, Shawn Guo, Sascha Hauer, Stephen Boyd,
	Dong Aisheng, Fabio Estevam, robh+dt, Mark Rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel

On Tue, Apr 04, 2017 at 07:36:01PM -0700, Stefan Agner wrote:
> On 2017-04-04 19:15, Fabio Estevam wrote:
> > On Sun, Apr 2, 2017 at 2:02 PM, Fabio Estevam <festevam@gmail.com> wrote:
> >> On Sat, Apr 1, 2017 at 1:15 AM, Stefan Agner <stefan@agner.ch> wrote:
> >>
> >>> IMX7D_IPG_ROOT_CLK is currently not a valid clock in upstream... So we
> >>> would have to add it to the clock driver first.
> >>>
> >>> I guess we could/should add it anyway at one point? But probably also as
> >>> init on, just to make sure Linux does not disable it since it is
> >>> currently used by several IPs implicitly.
> >>
> >> Yes, I made a previous attempt do add  IMX7D_IPG_ROOT_CLK and it did
> >> not work as I did not put it in the init_on clock list.
> >>
> >> Will submit a new patch adding it to init_on, thanks.
> > 
> > I thought that adding IMX7D_IPG_ROOT_CLK would do the trick, but the
> > patch below also causes the kernel to not boot:
> > 
> > --- a/drivers/clk/imx/clk-imx7d.c
> > +++ b/drivers/clk/imx/clk-imx7d.c
> > @@ -386,7 +386,7 @@ static int const clks_init_on[] __initconst = {
> >         IMX7D_PLL_SYS_MAIN_480M_CLK, IMX7D_NAND_USDHC_BUS_ROOT_CLK,
> >         IMX7D_DRAM_PHYM_ROOT_CLK, IMX7D_DRAM_ROOT_CLK,
> >         IMX7D_DRAM_PHYM_ALT_ROOT_CLK, IMX7D_DRAM_ALT_ROOT_CLK,
> > -       IMX7D_AHB_CHANNEL_ROOT_CLK,
> > +       IMX7D_AHB_CHANNEL_ROOT_CLK, IMX7D_IPG_ROOT_CLK,
> >  };
> > 
> >  static struct clk_onecell_data clk_data;
> > @@ -788,7 +788,7 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >         clks[IMX7D_WRCLK_ROOT_DIV] =
> > imx_clk_divider2("wrclk_post_div", "wrclk_pre_div", base + 0xbd00, 0,
> > 6);
> >         clks[IMX7D_CLKO1_ROOT_DIV] =
> > imx_clk_divider2("clko1_post_div", "clko1_pre_div", base + 0xbd80, 0,
> > 6);
> >         clks[IMX7D_CLKO2_ROOT_DIV] =
> > imx_clk_divider2("clko2_post_div", "clko2_pre_div", base + 0xbe00, 0,
> > 6);
> > -
> > +       clks[IMX7D_IPG_ROOT_CLK] = imx_clk_divider2("ipg_root_clk",
> > "ahb_root_clk", base + 0x9080, 0, 2);
> >         clks[IMX7D_ARM_A7_ROOT_CLK] = imx_clk_gate4("arm_a7_root_clk",
> > "arm_a7_div", base + 0x4000, 0);
> >         clks[IMX7D_ARM_M4_ROOT_CLK] = imx_clk_gate4("arm_m4_root_clk",
> > "arm_m4_div", base + 0x4010, 0);
> >         clks[IMX7D_ARM_M0_ROOT_CLK] = imx_clk_gate4("arm_m0_root_clk",
> > "arm_m0_div", base + 0x4020, 0);
> 
> Hm, imx_clk_divider2 sets CLK_SET_RATE_PARENT, maybe that influences the
> parent?
> 
> I guess we actually don't want the clock framework to change that clock
> rate, not sure whether we can freeze it or similar.
> 

This is caused by ahb_root_clk gets disabled accidently and system hangs.

Because this patch defines ipg_root_clk earlier before its parent
(ahb_root_clk) got registered, then it will be marked as a orphan clk
temporarily. Until the parent ahb_root_clk got registered, the clk core
will reparent it to the newly found parent. (see __clk_core_init() function).

Due to CLK_SET_RATE_PARENT flag, the ahb clk will be enabled during
set_parent operation and then disabled after that.
Then system hang cause we still get no chance to run init_on clks.

I just send out a proper fix patch with correct register sequence.

Probably we can switch all imx clk driver to CLK_IS_CRITICAL for critical
clocks in the future, but that's another thing to do later.

Stefan,
I think you can just resend your series based on my patches.

Regards
Dong Aisheng

> --
> Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
  2017-04-11  2:59             ` Dong Aisheng
@ 2017-04-11 23:23               ` Fabio Estevam
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio Estevam @ 2017-04-11 23:23 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Stefan Agner, Shawn Guo, Sascha Hauer, Stephen Boyd,
	Dong Aisheng, Fabio Estevam, robh+dt, Mark Rutland,
	linux-arm-kernel, devicetree, linux-clk, linux-kernel

On Mon, Apr 10, 2017 at 11:59 PM, Dong Aisheng <dongas86@gmail.com> wrote:

> This is caused by ahb_root_clk gets disabled accidently and system hangs.
>
> Because this patch defines ipg_root_clk earlier before its parent
> (ahb_root_clk) got registered, then it will be marked as a orphan clk
> temporarily. Until the parent ahb_root_clk got registered, the clk core
> will reparent it to the newly found parent. (see __clk_core_init() function).
>
> Due to CLK_SET_RATE_PARENT flag, the ahb clk will be enabled during
> set_parent operation and then disabled after that.
> Then system hang cause we still get no chance to run init_on clks.
>
> I just send out a proper fix patch with correct register sequence.

Excellent, thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-04-11 23:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-30  0:50 [PATCH 1/2] clk: imx7d: fix USDHC NAND clock Stefan Agner
2017-03-30  0:50 ` [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances Stefan Agner
2017-04-01  3:03   ` Dong Aisheng
2017-04-01  4:15     ` Stefan Agner
2017-04-02 17:02       ` Fabio Estevam
2017-04-05  2:15         ` Fabio Estevam
2017-04-05  2:36           ` Stefan Agner
2017-04-11  2:59             ` Dong Aisheng
2017-04-11 23:23               ` Fabio Estevam
2017-04-01  3:00 ` [PATCH 1/2] clk: imx7d: fix USDHC NAND clock Dong Aisheng

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).