[PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Paweł Chmiel]

This clock must be always enabled to allow access to any registers in
fsys1 CMU. Until proper solution based on runtime PM is applied
(similar to what was done for Exynos5433), fix this by calling
clk_prepare_enable() directly from clock provider driver.

It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
UFS module is probed before pmic used to power that device.
In this case defer probe was happening and that clock was disabled by
UFS driver, causing whole boot to hang on next CMU access.

Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
Changes from v2:
  - Avoid __clk_lookup() call when enabling clock
Changes from v1:
  - Instead of marking clock as critical, enable it manually in driver.
---
 drivers/clk/samsung/clk-exynos7.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
index c1ff715e960c..24d3fc5c5d0f 100644
--- a/drivers/clk/samsung/clk-exynos7.c
+++ b/drivers/clk/samsung/clk-exynos7.c
@@ -6,6 +6,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/of.h>
+#include <linux/clk.h>
 
 #include "clk.h"
 #include <dt-bindings/clock/exynos7-clk.h>
@@ -570,7 +571,18 @@ static const struct samsung_cmu_info top1_cmu_info __initconst = {
 
 static void __init exynos7_clk_top1_init(struct device_node *np)
 {
-	samsung_cmu_register_one(np, &top1_cmu_info);
+	struct samsung_clk_provider *ctx;
+	struct clk_hw **hws;
+
+	ctx = samsung_cmu_register_one(np, &top1_cmu_info);
+	if (!ctx)
+		return;
+	hws = ctx->clk_data.hws;
+
+	/*
+	 * Keep top FSYS1 aclk enabled permanently. It's required for CMU register access.
+	 */
+	clk_prepare_enable(hws[CLK_ACLK_FSYS1_200]->clk);
 }
 
 CLK_OF_DECLARE(exynos7_clk_top1, "samsung,exynos7-clock-top1",
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Chanwoo Choi]

On 2/1/21 2:04 AM, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
> 
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
> 
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> Changes from v2:
>   - Avoid __clk_lookup() call when enabling clock
> Changes from v1:
>   - Instead of marking clock as critical, enable it manually in driver.
> ---
>  drivers/clk/samsung/clk-exynos7.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
> index c1ff715e960c..24d3fc5c5d0f 100644
> --- a/drivers/clk/samsung/clk-exynos7.c
> +++ b/drivers/clk/samsung/clk-exynos7.c
> @@ -6,6 +6,7 @@
>  
>  #include <linux/clk-provider.h>
>  #include <linux/of.h>
> +#include <linux/clk.h>
>  
>  #include "clk.h"
>  #include <dt-bindings/clock/exynos7-clk.h>
> @@ -570,7 +571,18 @@ static const struct samsung_cmu_info top1_cmu_info __initconst = {
>  
>  static void __init exynos7_clk_top1_init(struct device_node *np)
>  {
> -	samsung_cmu_register_one(np, &top1_cmu_info);
> +	struct samsung_clk_provider *ctx;
> +	struct clk_hw **hws;
> +
> +	ctx = samsung_cmu_register_one(np, &top1_cmu_info);
> +	if (!ctx)
> +		return;
> +	hws = ctx->clk_data.hws;
> +
> +	/*
> +	 * Keep top FSYS1 aclk enabled permanently. It's required for CMU register access.
> +	 */
> +	clk_prepare_enable(hws[CLK_ACLK_FSYS1_200]->clk);
>  }
>  
>  CLK_OF_DECLARE(exynos7_clk_top1, "samsung,exynos7-clock-top1",
> 

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Chanwoo Choi]

On 2/1/21 2:04 AM, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
> 
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
> 
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> Changes from v2:
>   - Avoid __clk_lookup() call when enabling clock
> Changes from v1:
>   - Instead of marking clock as critical, enable it manually in driver.
> ---
>  drivers/clk/samsung/clk-exynos7.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos7.c b/drivers/clk/samsung/clk-exynos7.c
> index c1ff715e960c..24d3fc5c5d0f 100644
> --- a/drivers/clk/samsung/clk-exynos7.c
> +++ b/drivers/clk/samsung/clk-exynos7.c
> @@ -6,6 +6,7 @@
>  
>  #include <linux/clk-provider.h>
>  #include <linux/of.h>
> +#include <linux/clk.h>
>  
>  #include "clk.h"
>  #include <dt-bindings/clock/exynos7-clk.h>
> @@ -570,7 +571,18 @@ static const struct samsung_cmu_info top1_cmu_info __initconst = {
>  
>  static void __init exynos7_clk_top1_init(struct device_node *np)
>  {
> -	samsung_cmu_register_one(np, &top1_cmu_info);
> +	struct samsung_clk_provider *ctx;
> +	struct clk_hw **hws;
> +
> +	ctx = samsung_cmu_register_one(np, &top1_cmu_info);
> +	if (!ctx)
> +		return;
> +	hws = ctx->clk_data.hws;
> +
> +	/*
> +	 * Keep top FSYS1 aclk enabled permanently. It's required for CMU register access.
> +	 */
> +	clk_prepare_enable(hws[CLK_ACLK_FSYS1_200]->clk);
>  }
>  
>  CLK_OF_DECLARE(exynos7_clk_top1, "samsung,exynos7-clock-top1",
> 

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Sylwester Nawrocki]

On 1/31/21 18:04, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
> 
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
> 
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> Changes from v2:
>    - Avoid __clk_lookup() call when enabling clock
> Changes from v1:
>    - Instead of marking clock as critical, enable it manually in driver.

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Sylwester Nawrocki]

On 1/31/21 18:04, Paweł Chmiel wrote:
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
> 
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
> 
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> Changes from v2:
>    - Avoid __clk_lookup() call when enabling clock
> Changes from v1:
>    - Instead of marking clock as critical, enable it manually in driver.

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Stephen Boyd]

Quoting (2021-01-31 09:04:28)
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
> 
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
> 

Does this need a Fixes tag?

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Stephen Boyd]

Quoting (2021-01-31 09:04:28)
> This clock must be always enabled to allow access to any registers in
> fsys1 CMU. Until proper solution based on runtime PM is applied
> (similar to what was done for Exynos5433), fix this by calling
> clk_prepare_enable() directly from clock provider driver.
> 
> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
> UFS module is probed before pmic used to power that device.
> In this case defer probe was happening and that clock was disabled by
> UFS driver, causing whole boot to hang on next CMU access.
> 

Does this need a Fixes tag?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Sylwester Nawrocki]

On 09.02.2021 08:48, Stephen Boyd wrote:
> Quoting (2021-01-31 09:04:28)
>> This clock must be always enabled to allow access to any registers in
>> fsys1 CMU. Until proper solution based on runtime PM is applied
>> (similar to what was done for Exynos5433), fix this by calling
>> clk_prepare_enable() directly from clock provider driver.
>>
>> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
>> UFS module is probed before pmic used to power that device.
>> In this case defer probe was happening and that clock was disabled by
>> UFS driver, causing whole boot to hang on next CMU access.
>>
> 
> Does this need a Fixes tag?
 
That would be

Fixes: 753195a749a6 ("clk: samsung: exynos7: Correct CMU_FSYS1 clocks names")

i.e. commit that introduced definition of the clock. But the fix cannot be 
backported that far as build fails with an error:

drivers/clk/samsung/clk-exynos7.c: In function ‘exynos7_clk_top1_init’:
drivers/clk/samsung/clk-exynos7.c:554:21: error: ‘struct clk_onecell_data’ has no member named ‘hws’
  554 |  hws = ctx->clk_data.hws;

It could only by backported up to:
ecb1f1f7311f ("clk: samsung: Convert common drivers to the new clk_hw API")

We need a different patch to fix it properly in stable kernels.
And dts for board this bugfix patch was prepared is not upstream yet.
    

Re: [PATCH v3] clk: exynos7: Keep aclk_fsys1_200 enabled

[Sylwester Nawrocki]

On 09.02.2021 08:48, Stephen Boyd wrote:
> Quoting (2021-01-31 09:04:28)
>> This clock must be always enabled to allow access to any registers in
>> fsys1 CMU. Until proper solution based on runtime PM is applied
>> (similar to what was done for Exynos5433), fix this by calling
>> clk_prepare_enable() directly from clock provider driver.
>>
>> It was observed on Samsung Galaxy S6 device (based on Exynos7420), where
>> UFS module is probed before pmic used to power that device.
>> In this case defer probe was happening and that clock was disabled by
>> UFS driver, causing whole boot to hang on next CMU access.
>>
> 
> Does this need a Fixes tag?
 
That would be

Fixes: 753195a749a6 ("clk: samsung: exynos7: Correct CMU_FSYS1 clocks names")

i.e. commit that introduced definition of the clock. But the fix cannot be 
backported that far as build fails with an error:

drivers/clk/samsung/clk-exynos7.c: In function ‘exynos7_clk_top1_init’:
drivers/clk/samsung/clk-exynos7.c:554:21: error: ‘struct clk_onecell_data’ has no member named ‘hws’
  554 |  hws = ctx->clk_data.hws;

It could only by backported up to:
ecb1f1f7311f ("clk: samsung: Convert common drivers to the new clk_hw API")

We need a different patch to fix it properly in stable kernels.
And dts for board this bugfix patch was prepared is not upstream yet.
    


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel