[PATCH 1/6] thermal: qoriq: Use devm_platform_ioremap_resource() instead of of_iomap()

[PATCH 1/6] thermal: qoriq: Use devm_platform_ioremap_resource() instead of of_iomap()

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Use devm_platform_ioremap_resource() instead of of_iomap() to
save the iounmap() call in error handle path;

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/thermal/qoriq_thermal.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 7b36493..c7c7de2 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -202,32 +202,27 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
 	data->little_endian = of_property_read_bool(np, "little-endian");
 
-	data->regs = of_iomap(np, 0);
-	if (!data->regs) {
+	data->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(data->regs)) {
 		dev_err(&pdev->dev, "Failed to get memory region\n");
-		ret = -ENODEV;
-		goto err_iomap;
+		return PTR_ERR(data->regs);
 	}
 
 	qoriq_tmu_init_device(data);	/* TMU initialization */
 
 	ret = qoriq_tmu_calibration(pdev);	/* TMU calibration */
 	if (ret < 0)
-		goto err_tmu;
+		goto err;
 
 	ret = qoriq_tmu_register_tmu_zone(pdev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to register sensors\n");
-		ret = -ENODEV;
-		goto err_iomap;
+		goto err;
 	}
 
 	return 0;
 
-err_tmu:
-	iounmap(data->regs);
-
-err_iomap:
+err:
 	platform_set_drvdata(pdev, NULL);
 
 	return ret;
@@ -240,7 +235,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
 	/* Disable monitoring */
 	tmu_write(data, TMR_DISABLE, &data->regs->tmr);
 
-	iounmap(data->regs);
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
-- 
2.7.4

[PATCH 2/6] thermal: qoriq: Use __maybe_unused instead of #if CONFIG_PM_SLEEP

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Use __maybe_unused for power management related functions
instead of #if CONFIG_PM_SLEEP to simply the code.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/thermal/qoriq_thermal.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index c7c7de2..2b2f79b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -240,8 +240,7 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int qoriq_tmu_suspend(struct device *dev)
+static int __maybe_unused qoriq_tmu_suspend(struct device *dev)
 {
 	u32 tmr;
 	struct qoriq_tmu_data *data = dev_get_drvdata(dev);
@@ -254,7 +253,7 @@ static int qoriq_tmu_suspend(struct device *dev)
 	return 0;
 }
 
-static int qoriq_tmu_resume(struct device *dev)
+static int __maybe_unused qoriq_tmu_resume(struct device *dev)
 {
 	u32 tmr;
 	struct qoriq_tmu_data *data = dev_get_drvdata(dev);
@@ -266,7 +265,6 @@ static int qoriq_tmu_resume(struct device *dev)
 
 	return 0;
 }
-#endif
 
 static SIMPLE_DEV_PM_OPS(qoriq_tmu_pm_ops,
 			 qoriq_tmu_suspend, qoriq_tmu_resume);
-- 
2.7.4

[PATCH 3/6] dt-bindings: thermal: qoriq: Add optional clocks property

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Some platforms have clock control for TMU, add optional
clocks property to the binding doc.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 Documentation/devicetree/bindings/thermal/qoriq-thermal.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt b/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
index 04cbb90..28f2cba 100644
--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.txt
@@ -23,6 +23,7 @@ Required properties:
 Optional property:
 - little-endian : If present, the TMU registers are little endian. If absent,
 	the default is big endian.
+- clocks : the clock for clocking the TMU silicon.
 
 Example:
 
-- 
2.7.4

[PATCH 4/6] thermal: qoriq: Add clock operations

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Some platforms like i.MX8MQ has clock control for this module,
need to add clock operations to make sure the driver is working
properly.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/thermal/qoriq_thermal.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 2b2f79b..0813c1b 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -2,6 +2,7 @@
 //
 // Copyright 2016 Freescale Semiconductor, Inc.
 
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
@@ -72,6 +73,7 @@ struct qoriq_sensor {
 
 struct qoriq_tmu_data {
 	struct qoriq_tmu_regs __iomem *regs;
+	struct clk *clk;
 	bool little_endian;
 	struct qoriq_sensor	*sensor[SITES_MAX];
 };
@@ -208,6 +210,19 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 		return PTR_ERR(data->regs);
 	}
 
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		if (PTR_ERR(data->clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		data->clk = NULL;
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable clock\n");
+		return ret;
+	}
+
 	qoriq_tmu_init_device(data);	/* TMU initialization */
 
 	ret = qoriq_tmu_calibration(pdev);	/* TMU calibration */
@@ -235,6 +250,8 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
 	/* Disable monitoring */
 	tmu_write(data, TMR_DISABLE, &data->regs->tmr);
 
+	clk_disable_unprepare(data->clk);
+
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
@@ -250,14 +267,21 @@ static int __maybe_unused qoriq_tmu_suspend(struct device *dev)
 	tmr &= ~TMR_ME;
 	tmu_write(data, tmr, &data->regs->tmr);
 
+	clk_disable_unprepare(data->clk);
+
 	return 0;
 }
 
 static int __maybe_unused qoriq_tmu_resume(struct device *dev)
 {
 	u32 tmr;
+	int ret;
 	struct qoriq_tmu_data *data = dev_get_drvdata(dev);
 
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
 	/* Enable monitoring */
 	tmr = tmu_read(data, &data->regs->tmr);
 	tmr |= TMR_ME;
-- 
2.7.4

[PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
should manage this clock, so no need to have CLK_IS_CRITICAL flag
set.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/clk/imx/clk-imx8mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index d407a07..91de69a 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	clks[IMX8MQ_CLK_DISP_AXI_ROOT]  = imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, &share_count_dcss);
 	clks[IMX8MQ_CLK_DISP_APB_ROOT]  = imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, &share_count_dcss);
 	clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, &share_count_dcss);
-	clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", "ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL);
+	clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", base + 0x4620, 0);
 	clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", "vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
 	clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", "csi1_core", base + 0x4650, 0);
 	clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", "csi2_core", base + 0x4660, 0);
-- 
2.7.4

[PATCH 6/6] arm64: dts: imx8mq: Add clock for TMU node

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

i.MX8MQ has clock gate for TMU module, add clock info to TMU
node for clock management.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index c61e968..edfc1aa 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -348,6 +348,7 @@
 				compatible = "fsl,imx8mq-tmu";
 				reg = <0x30260000 0x10000>;
 				interrupt = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clk IMX8MQ_CLK_TMU_ROOT>;
 				little-endian;
 				fsl,tmu-range = <0xb0000 0xa0026 0x80048 0x70061>;
 				fsl,tmu-calibration = <0x00000000 0x00000023
-- 
2.7.4

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Abel Vesa]

On 19-07-05 12:56:11, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> should manage this clock, so no need to have CLK_IS_CRITICAL flag
> set.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>  drivers/clk/imx/clk-imx8mq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index d407a07..91de69a 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
>  	clks[IMX8MQ_CLK_DISP_AXI_ROOT]  = imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, &share_count_dcss);
>  	clks[IMX8MQ_CLK_DISP_APB_ROOT]  = imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, &share_count_dcss);
>  	clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, &share_count_dcss);
> -	clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", "ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL);
> +	clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", base + 0x4620, 0);
>  	clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", "vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
>  	clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", "csi1_core", base + 0x4650, 0);
>  	clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", "csi2_core", base + 0x4660, 0);
> -- 
> 2.7.4
> 

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Stephen Boyd]

Quoting Anson.Huang@nxp.com (2019-07-04 21:56:11)
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> should manage this clock, so no need to have CLK_IS_CRITICAL flag
> set.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH 3/6] dt-bindings: thermal: qoriq: Add optional clocks property

[Rob Herring]

On Fri,  5 Jul 2019 12:56:09 +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> Some platforms have clock control for TMU, add optional
> clocks property to the binding doc.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  Documentation/devicetree/bindings/thermal/qoriq-thermal.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Shawn Guo]

On Fri, Jul 05, 2019 at 12:56:11PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> should manage this clock, so no need to have CLK_IS_CRITICAL flag
> set.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Applied, thanks.

Re: [PATCH 6/6] arm64: dts: imx8mq: Add clock for TMU node

[Shawn Guo]

On Fri, Jul 05, 2019 at 12:56:12PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> i.MX8MQ has clock gate for TMU module, add clock info to TMU
> node for clock management.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Applied, thanks.

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Daniel Baluta]

Hi all,

latest linux-next hangs at boot.

commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag:
next-20190726, origin/master, origin/HEAD)
Author: Stephen Rothwell <sfr@canb.auug.org.au>
Date:   Fri Jul 26 15:18:02 2019 +1000

    Add linux-next specific files for 20190726

    Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>


I know this is crazy but reverting commit:

commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
Author: Anson Huang <Anson.Huang@nxp.com>
Date:   Fri Jul 5 12:56:11 2019 +0800

    clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

    IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
    should manage this clock, so no need to have CLK_IS_CRITICAL flag
    set.



makes the boot work again.

Any idea?

On Fri, Jul 5, 2019 at 8:07 AM <Anson.Huang@nxp.com> wrote:
>
> From: Anson Huang <Anson.Huang@nxp.com>
>
> IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> should manage this clock, so no need to have CLK_IS_CRITICAL flag
> set.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/clk/imx/clk-imx8mq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index d407a07..91de69a 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
>         clks[IMX8MQ_CLK_DISP_AXI_ROOT]  = imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, &share_count_dcss);
>         clks[IMX8MQ_CLK_DISP_APB_ROOT]  = imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, &share_count_dcss);
>         clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, &share_count_dcss);
> -       clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", "ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL);
> +       clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", base + 0x4620, 0);
>         clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", "vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
>         clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", "csi1_core", base + 0x4650, 0);
>         clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", "csi2_core", base + 0x4660, 0);
> --
> 2.7.4
>

RE: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> IMX8MQ_CLK_TMU_ROOT
> 
> Hi all,
> 
> latest linux-next hangs at boot.
> 
> commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag:
> next-20190726, origin/master, origin/HEAD)
> Author: Stephen Rothwell <sfr@canb.auug.org.au>
> Date:   Fri Jul 26 15:18:02 2019 +1000
> 
>     Add linux-next specific files for 20190726
> 
>     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> 
> I know this is crazy but reverting commit:
> 
> commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> Author: Anson Huang <Anson.Huang@nxp.com>
> Date:   Fri Jul 5 12:56:11 2019 +0800
> 
>     clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
> 
>     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
>     should manage this clock, so no need to have CLK_IS_CRITICAL flag
>     set.
> 
> 
> 
> makes the boot work again.
> 
> Any idea?

I just found if disabling SDMA1, then kernel can boot up, it does NOT make sense
TMU clock is related to SDMA1, I will check with design and get back to you soon.

Anson

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Daniel Baluta]

On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> wrote:
>
> Hi, Daniel
>
> > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > IMX8MQ_CLK_TMU_ROOT
> >
> > Hi all,
> >
> > latest linux-next hangs at boot.
> >
> > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag:
> > next-20190726, origin/master, origin/HEAD)
> > Author: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date:   Fri Jul 26 15:18:02 2019 +1000
> >
> >     Add linux-next specific files for 20190726
> >
> >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> >
> >
> > I know this is crazy but reverting commit:
> >
> > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> > Author: Anson Huang <Anson.Huang@nxp.com>
> > Date:   Fri Jul 5 12:56:11 2019 +0800
> >
> >     clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
> >
> >     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> >     should manage this clock, so no need to have CLK_IS_CRITICAL flag
> >     set.
> >
> >
> >
> > makes the boot work again.
> >
> > Any idea?
>
> I just found if disabling SDMA1, then kernel can boot up, it does NOT make sense
> TMU clock is related to SDMA1, I will check with design and get back to you soon.
>

Hi Anson,

Applying Abel's patch:

commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master)
Author: Abel Vesa <abel.vesa@nxp.com>
Date:   Tue Jun 25 12:01:56 2019 +0300

    clk: imx8mq: Mark AHB clock as critical

    Keep the AHB clock always on since there is no driver to control it and
    all the other clocks that use it as parent rely on it being always enabled.



The kernel boots up again.

It make some sense. I don't understand though why having
IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel.

thanks,
Daniel.

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Abel Vesa]

On 19-07-27 09:33:10, Daniel Baluta wrote:
> On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> wrote:
> >
> > Hi, Daniel
> >
> > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > IMX8MQ_CLK_TMU_ROOT
> > >
> > > Hi all,
> > >
> > > latest linux-next hangs at boot.
> > >
> > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag:
> > > next-20190726, origin/master, origin/HEAD)
> > > Author: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Date:   Fri Jul 26 15:18:02 2019 +1000
> > >
> > >     Add linux-next specific files for 20190726
> > >
> > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > >
> > >
> > > I know this is crazy but reverting commit:
> > >
> > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> > > Author: Anson Huang <Anson.Huang@nxp.com>
> > > Date:   Fri Jul 5 12:56:11 2019 +0800
> > >
> > >     clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
> > >
> > >     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> > >     should manage this clock, so no need to have CLK_IS_CRITICAL flag
> > >     set.
> > >
> > >
> > >
> > > makes the boot work again.
> > >
> > > Any idea?
> >
> > I just found if disabling SDMA1, then kernel can boot up, it does NOT make sense
> > TMU clock is related to SDMA1, I will check with design and get back to you soon.
> >
> 
> Hi Anson,
> 
> Applying Abel's patch:
> 
> commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master)
> Author: Abel Vesa <abel.vesa@nxp.com>
> Date:   Tue Jun 25 12:01:56 2019 +0300
> 
>     clk: imx8mq: Mark AHB clock as critical
> 
>     Keep the AHB clock always on since there is no driver to control it and
>     all the other clocks that use it as parent rely on it being always enabled.
> 
> 
> 
> The kernel boots up again.
> 
> It make some sense. I don't understand though why having
> IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel.
> 

OK, so this is how it works.

By removing the critical flag from TMU, the AHB doesn't stay always on.
With my patch the AHB is marked as critical and therefore stays on.

The sdma1_clk has as parent the ipg_root which in turn has as parent the
ahb clock. And I think what happens is some read from the sdma registers hangs
because, for whatever reason, enabling the sdma1_clk doesn't propagate to enable
the ahb clock. I might be wrong though.

> thanks,
> Daniel.

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Guido Günther]

Hi Daniel,
On Sat, Jul 27, 2019 at 01:26:50AM +0300, Daniel Baluta wrote:
> Hi all,
> 
> latest linux-next hangs at boot.
> 
> commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag:
> next-20190726, origin/master, origin/HEAD)
> Author: Stephen Rothwell <sfr@canb.auug.org.au>
> Date:   Fri Jul 26 15:18:02 2019 +1000
> 
>     Add linux-next specific files for 20190726
> 
>     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> 
> I know this is crazy but reverting commit:
> 
> commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> Author: Anson Huang <Anson.Huang@nxp.com>
> Date:   Fri Jul 5 12:56:11 2019 +0800
> 
>     clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
> 
>     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
>     should manage this clock, so no need to have CLK_IS_CRITICAL flag
>     set.
> 
> 
> 
> makes the boot work again.

I noticed a boot hang yesterday on next-20190726 when loading the
qoriq_thermal which I worked around by blacklisting it. The
fsl,imx8mq-tmu node specifies a clock (IMX8MQ_CLK_TMU_ROOT) but does not
seem to enable, shouldn't it do so?

Cheers,
 -- Guido

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Fabio Estevam]

Hi Guido,

On Sat, Jul 27, 2019 at 3:26 PM Guido Günther <agx@sigxcpu.org> wrote:

> I noticed a boot hang yesterday on next-20190726 when loading the
> qoriq_thermal which I worked around by blacklisting it. The
> fsl,imx8mq-tmu node specifies a clock (IMX8MQ_CLK_TMU_ROOT) but does not
> seem to enable, shouldn't it do so?

Yes, I think you are right.

I don't have access to a imx8mq board at the moment, but something
like below would probably help:
http://code.bulix.org/pd88jp-812381

If it helps, I can send it as a formal patch.

Regards,

Fabio Estevam

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Guido Günther]

Hi Fabio,
On Sat, Jul 27, 2019 at 05:17:50PM -0300, Fabio Estevam wrote:
> Hi Guido,
> 
> On Sat, Jul 27, 2019 at 3:26 PM Guido Günther <agx@sigxcpu.org> wrote:
> 
> > I noticed a boot hang yesterday on next-20190726 when loading the
> > qoriq_thermal which I worked around by blacklisting it. The
> > fsl,imx8mq-tmu node specifies a clock (IMX8MQ_CLK_TMU_ROOT) but does not
> > seem to enable, shouldn't it do so?
> 
> Yes, I think you are right.
> 
> I don't have access to a imx8mq board at the moment, but something
> like below would probably help:
> http://code.bulix.org/pd88jp-812381
> 
> If it helps, I can send it as a formal patch.

Yes, this fixes it for me, thanks!
 -- Guido

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Fabio Estevam]

Hi Guido,

On Sun, Jul 28, 2019 at 4:59 AM Guido Günther <agx@sigxcpu.org> wrote:

> Yes, this fixes it for me, thanks!

Thanks for testing it.

I will send a formal patch tomorrow.

Regards,

Fabio Estevam

RE: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Anson Huang]

Hi, Abel/Daniel

> On 19-07-27 09:33:10, Daniel Baluta wrote:
> > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com>
> wrote:
> > >
> > > Hi, Daniel
> > >
> > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag
> > > > for IMX8MQ_CLK_TMU_ROOT
> > > >
> > > > Hi all,
> > > >
> > > > latest linux-next hangs at boot.
> > > >
> > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD ->
> master, tag:
> > > > next-20190726, origin/master, origin/HEAD)
> > > > Author: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > Date:   Fri Jul 26 15:18:02 2019 +1000
> > > >
> > > >     Add linux-next specific files for 20190726
> > > >
> > > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > >
> > > >
> > > > I know this is crazy but reverting commit:
> > > >
> > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef
> (refs/bisect/bad)
> > > > Author: Anson Huang <Anson.Huang@nxp.com>
> > > > Date:   Fri Jul 5 12:56:11 2019 +0800
> > > >
> > > >     clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > IMX8MQ_CLK_TMU_ROOT
> > > >
> > > >     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the
> driver
> > > >     should manage this clock, so no need to have CLK_IS_CRITICAL flag
> > > >     set.
> > > >
> > > >
> > > >
> > > > makes the boot work again.
> > > >
> > > > Any idea?
> > >
> > > I just found if disabling SDMA1, then kernel can boot up, it does
> > > NOT make sense TMU clock is related to SDMA1, I will check with design
> and get back to you soon.
> > >
> >
> > Hi Anson,
> >
> > Applying Abel's patch:
> >
> > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master)
> > Author: Abel Vesa <abel.vesa@nxp.com>
> > Date:   Tue Jun 25 12:01:56 2019 +0300
> >
> >     clk: imx8mq: Mark AHB clock as critical
> >
> >     Keep the AHB clock always on since there is no driver to control it and
> >     all the other clocks that use it as parent rely on it being always enabled.
> >
> >
> >
> > The kernel boots up again.
> >
> > It make some sense. I don't understand though why having
> > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel.
> >
> 
> OK, so this is how it works.
> 
> By removing the critical flag from TMU, the AHB doesn't stay always on.
> With my patch the AHB is marked as critical and therefore stays on.
> 
> The sdma1_clk has as parent the ipg_root which in turn has as parent the
> ahb clock. And I think what happens is some read from the sdma registers
> hangs because, for whatever reason, enabling the sdma1_clk doesn't
> propagate to enable the ahb clock. I might be wrong though.
> 

I can explain why Abel's patch can fix this issue, the AHB clock is a MUST
always ON for system bus, while it does NOT have CLK_IS_CRITICAL flag set for now,
when SDMA1 is probed, it will enable its clock, and the clk path is sdma1_clk->ipg_root->ahb,
after SDMA1 probed done, it will disable its clock since no one use it, and it will trigger
the ahb clock to be OFF, as its usecount is added by 1 when probe and decreased by 1 after
SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF.

So I think the best solution should be applying Abel's patch as you mentioned upper, the TMU
clock patch is NOT the root cause, it just triggers this issue accidently☹

But I saw Abel's AHB patch is still under discussion, so I think we need to speed it up and make 
kernel boot up work for development. AHB/IPG are always critical for i.MX SoCs.

Thanks,
Anson.

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Daniel Baluta]

On Mon, Jul 29, 2019 at 4:29 AM Anson Huang <anson.huang@nxp.com> wrote:
>
> Hi, Abel/Daniel
>
> > On 19-07-27 09:33:10, Daniel Baluta wrote:
> > > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com>
> > wrote:
> > > >
> > > > Hi, Daniel
> > > >
> > > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag
> > > > > for IMX8MQ_CLK_TMU_ROOT
> > > > >
> > > > > Hi all,
> > > > >
> > > > > latest linux-next hangs at boot.
> > > > >
> > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD ->
> > master, tag:
> > > > > next-20190726, origin/master, origin/HEAD)
> > > > > Author: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > > Date:   Fri Jul 26 15:18:02 2019 +1000
> > > > >
> > > > >     Add linux-next specific files for 20190726
> > > > >
> > > > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > >
> > > > >
> > > > > I know this is crazy but reverting commit:
> > > > >
> > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef
> > (refs/bisect/bad)
> > > > > Author: Anson Huang <Anson.Huang@nxp.com>
> > > > > Date:   Fri Jul 5 12:56:11 2019 +0800
> > > > >
> > > > >     clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > IMX8MQ_CLK_TMU_ROOT
> > > > >
> > > > >     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the
> > driver
> > > > >     should manage this clock, so no need to have CLK_IS_CRITICAL flag
> > > > >     set.
> > > > >
> > > > >
> > > > >
> > > > > makes the boot work again.
> > > > >
> > > > > Any idea?
> > > >
> > > > I just found if disabling SDMA1, then kernel can boot up, it does
> > > > NOT make sense TMU clock is related to SDMA1, I will check with design
> > and get back to you soon.
> > > >
> > >
> > > Hi Anson,
> > >
> > > Applying Abel's patch:
> > >
> > > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master)
> > > Author: Abel Vesa <abel.vesa@nxp.com>
> > > Date:   Tue Jun 25 12:01:56 2019 +0300
> > >
> > >     clk: imx8mq: Mark AHB clock as critical
> > >
> > >     Keep the AHB clock always on since there is no driver to control it and
> > >     all the other clocks that use it as parent rely on it being always enabled.
> > >
> > >
> > >
> > > The kernel boots up again.
> > >
> > > It make some sense. I don't understand though why having
> > > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel.
> > >
> >
> > OK, so this is how it works.
> >
> > By removing the critical flag from TMU, the AHB doesn't stay always on.
> > With my patch the AHB is marked as critical and therefore stays on.
> >
> > The sdma1_clk has as parent the ipg_root which in turn has as parent the
> > ahb clock. And I think what happens is some read from the sdma registers
> > hangs because, for whatever reason, enabling the sdma1_clk doesn't
> > propagate to enable the ahb clock. I might be wrong though.
> >
>
> I can explain why Abel's patch can fix this issue, the AHB clock is a MUST
> always ON for system bus, while it does NOT have CLK_IS_CRITICAL flag set for now,
> when SDMA1 is probed, it will enable its clock, and the clk path is sdma1_clk->ipg_root->ahb,
> after SDMA1 probed done, it will disable its clock since no one use it, and it will trigger
> the ahb clock to be OFF, as its usecount is added by 1 when probe and decreased by 1 after
> SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF.
>
> So I think the best solution should be applying Abel's patch as you mentioned upper, the TMU
> clock patch is NOT the root cause, it just triggers this issue accidently☹
>
> But I saw Abel's AHB patch is still under discussion, so I think we need to speed it up and make
> kernel boot up work for development. AHB/IPG are always critical for i.MX SoCs.

Thanks Anson,

Your explanation makes a lot of sense. We will take care today of Abel's patch.
What do you think about Fabio's patch? I also think this is a valid patch:

http://code.bulix.org/pd88jp-812381

RE: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Anson Huang]

Hi, Daniel

> On Mon, Jul 29, 2019 at 4:29 AM Anson Huang <anson.huang@nxp.com>
> wrote:
> >
> > Hi, Abel/Daniel
> >
> > > On 19-07-27 09:33:10, Daniel Baluta wrote:
> > > > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com>
> > > wrote:
> > > > >
> > > > > Hi, Daniel
> > > > >
> > > > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL
> > > > > > flag for IMX8MQ_CLK_TMU_ROOT
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > latest linux-next hangs at boot.
> > > > > >
> > > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD ->
> > > master, tag:
> > > > > > next-20190726, origin/master, origin/HEAD)
> > > > > > Author: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > > > Date:   Fri Jul 26 15:18:02 2019 +1000
> > > > > >
> > > > > >     Add linux-next specific files for 20190726
> > > > > >
> > > > > >     Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > > >
> > > > > >
> > > > > > I know this is crazy but reverting commit:
> > > > > >
> > > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef
> > > (refs/bisect/bad)
> > > > > > Author: Anson Huang <Anson.Huang@nxp.com>
> > > > > > Date:   Fri Jul 5 12:56:11 2019 +0800
> > > > > >
> > > > > >     clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > > IMX8MQ_CLK_TMU_ROOT
> > > > > >
> > > > > >     IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the
> > > driver
> > > > > >     should manage this clock, so no need to have CLK_IS_CRITICAL
> flag
> > > > > >     set.
> > > > > >
> > > > > >
> > > > > >
> > > > > > makes the boot work again.
> > > > > >
> > > > > > Any idea?
> > > > >
> > > > > I just found if disabling SDMA1, then kernel can boot up, it
> > > > > does NOT make sense TMU clock is related to SDMA1, I will check
> > > > > with design
> > > and get back to you soon.
> > > > >
> > > >
> > > > Hi Anson,
> > > >
> > > > Applying Abel's patch:
> > > >
> > > > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master)
> > > > Author: Abel Vesa <abel.vesa@nxp.com>
> > > > Date:   Tue Jun 25 12:01:56 2019 +0300
> > > >
> > > >     clk: imx8mq: Mark AHB clock as critical
> > > >
> > > >     Keep the AHB clock always on since there is no driver to control it and
> > > >     all the other clocks that use it as parent rely on it being always
> enabled.
> > > >
> > > >
> > > >
> > > > The kernel boots up again.
> > > >
> > > > It make some sense. I don't understand though why having
> > > > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel.
> > > >
> > >
> > > OK, so this is how it works.
> > >
> > > By removing the critical flag from TMU, the AHB doesn't stay always on.
> > > With my patch the AHB is marked as critical and therefore stays on.
> > >
> > > The sdma1_clk has as parent the ipg_root which in turn has as parent
> > > the ahb clock. And I think what happens is some read from the sdma
> > > registers hangs because, for whatever reason, enabling the sdma1_clk
> > > doesn't propagate to enable the ahb clock. I might be wrong though.
> > >
> >
> > I can explain why Abel's patch can fix this issue, the AHB clock is a
> > MUST always ON for system bus, while it does NOT have CLK_IS_CRITICAL
> > flag set for now, when SDMA1 is probed, it will enable its clock, and
> > the clk path is sdma1_clk->ipg_root->ahb, after SDMA1 probed done, it
> > will disable its clock since no one use it, and it will trigger the
> > ahb clock to be OFF, as its usecount is added by 1 when probe and
> > decreased by 1 after
> > SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF.
> >
> > So I think the best solution should be applying Abel's patch as you
> > mentioned upper, the TMU clock patch is NOT the root cause, it just
> > triggers this issue accidently☹
> >
> > But I saw Abel's AHB patch is still under discussion, so I think we
> > need to speed it up and make kernel boot up work for development.
> AHB/IPG are always critical for i.MX SoCs.
> 
> Thanks Anson,
> 
> Your explanation makes a lot of sense. We will take care today of Abel's
> patch.
> What do you think about Fabio's patch? I also think this is a valid patch:
> 
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcode.b
> ulix.org%2Fpd88jp-
> 812381&amp;data=02%7C01%7Canson.huang%40nxp.com%7C23b4c21e3cbc
> 4fcf2a3c08d713f131a8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636999798949622961&amp;sdata=Jx9B40rJKpQvakOCfx%2B3v80NTxPqUw
> D4pdGojQxVIoY%3D&amp;reserved=0

Hmm, when did Fabio sent out this patch? I can NOT find it...
I also have a patch in this series (#4/6) doing same thing on July 5th...

https://patchwork.kernel.org/patch/11032783/

Thanks.
Anson

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Daniel Baluta]

<snip>
> > Your explanation makes a lot of sense. We will take care today of Abel's
> > patch.
> > What do you think about Fabio's patch? I also think this is a valid patch:
> >
<snip>

>
> Hmm, when did Fabio sent out this patch? I can NOT find it...
> I also have a patch in this series (#4/6) doing same thing on July 5th...
>
> https://patchwork.kernel.org/patch/11032783/

He didn't send the patch yet. It was just a request for test here:

http://code.bulix.org/pd88jp-812381

But, now I see is exactly like your patch here:

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Daniel Baluta]

On Mon, Jul 29, 2019 at 10:20 AM Daniel Baluta <daniel.baluta@gmail.com> wrote:
>
> <snip>
> > > Your explanation makes a lot of sense. We will take care today of Abel's
> > > patch.
> > > What do you think about Fabio's patch? I also think this is a valid patch:
> > >
> <snip>
>
> >
> > Hmm, when did Fabio sent out this patch? I can NOT find it...
> > I also have a patch in this series (#4/6) doing same thing on July 5th...
> >
> > https://patchwork.kernel.org/patch/11032783/
>
> He didn't send the patch yet. It was just a request for test here:
>
> http://code.bulix.org/pd88jp-812381
>
> But, now I see is exactly like your patch here:

... pressed send to early.

https://lkml.org/lkml/2019/7/5/19

We are all set then. Thanks Anson for clarifications!

RE: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Anson Huang]

Hi, Daniel

> On Mon, Jul 29, 2019 at 10:20 AM Daniel Baluta <daniel.baluta@gmail.com>
> wrote:
> >
> > <snip>
> > > > Your explanation makes a lot of sense. We will take care today of
> > > > Abel's patch.
> > > > What do you think about Fabio's patch? I also think this is a valid patch:
> > > >
> > <snip>
> >
> > >
> > > Hmm, when did Fabio sent out this patch? I can NOT find it...
> > > I also have a patch in this series (#4/6) doing same thing on July 5th...
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> > >
> tchwork.kernel.org%2Fpatch%2F11032783%2F&amp;data=02%7C01%7Canso
> n.hu
> > >
> ang%40nxp.com%7C048d8695b3dc4ceef6bd08d713f55e8a%7C686ea1d3bc2
> b4c6fa
> > >
> 92cd99c5c301635%7C0%7C0%7C636999816880118674&amp;sdata=1HIMQ0l
> iKpEFS
> > > 6P2WSG%2FH9evspxIdxAvFmaklH1woDk%3D&amp;reserved=0
> >
> > He didn't send the patch yet. It was just a request for test here:
> >
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcode.
> > bulix.org%2Fpd88jp-
> 812381&amp;data=02%7C01%7Canson.huang%40nxp.com%7C0
> >
> 48d8695b3dc4ceef6bd08d713f55e8a%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0
> > %7C0%7C636999816880118674&amp;sdata=p70mgCDucCgLJ8TTRMn3a%2
> Fk68FXGQeiR
> > FR0fVSV7Jlo%3D&amp;reserved=0
> >
> > But, now I see is exactly like your patch here:
> 
> ... pressed send to early.
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.o
> rg%2Flkml%2F2019%2F7%2F5%2F19&amp;data=02%7C01%7Canson.huang%
> 40nxp.com%7C048d8695b3dc4ceef6bd08d713f55e8a%7C686ea1d3bc2b4c6f
> a92cd99c5c301635%7C0%7C0%7C636999816880118674&amp;sdata=ciWj8y
> WPvkIZ5ni%2BohZuaqAXf9Pb2FCWhp9GQhpMwAY%3D&amp;reserved=0
> 
> We are all set then. Thanks Anson for clarifications!

Thanks, so we are all clear about this issue, need to wait thermal maintainer to review
the rest patch in this series, but I did NOT receive any response from thermal sub-system
maintainer for really long time, NOT sure when the thermal patches can be accepted.

Anson

Re: [PATCH 4/6] thermal: qoriq: Add clock operations

[Guido Günther]

Hi Anson,
On Fri, Jul 05, 2019 at 12:56:10PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> Some platforms like i.MX8MQ has clock control for this module,
> need to add clock operations to make sure the driver is working
> properly.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/thermal/qoriq_thermal.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
> index 2b2f79b..0813c1b 100644
> --- a/drivers/thermal/qoriq_thermal.c
> +++ b/drivers/thermal/qoriq_thermal.c
> @@ -2,6 +2,7 @@
>  //
>  // Copyright 2016 Freescale Semiconductor, Inc.
>  
> +#include <linux/clk.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/err.h>
> @@ -72,6 +73,7 @@ struct qoriq_sensor {
>  
>  struct qoriq_tmu_data {
>  	struct qoriq_tmu_regs __iomem *regs;
> +	struct clk *clk;
>  	bool little_endian;
>  	struct qoriq_sensor	*sensor[SITES_MAX];
>  };
> @@ -208,6 +210,19 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
>  		return PTR_ERR(data->regs);
>  	}
>  
> +	data->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(data->clk)) {
> +		if (PTR_ERR(data->clk) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		data->clk = NULL;
> +	}

Wouldn't devm_clk_get_optional make more sense?

> +
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to enable clock\n");
> +		return ret;
> +	}
> +
>  	qoriq_tmu_init_device(data);	/* TMU initialization */
>  
>  	ret = qoriq_tmu_calibration(pdev);	/* TMU calibration */
> @@ -235,6 +250,8 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
>  	/* Disable monitoring */
>  	tmu_write(data, TMR_DISABLE, &data->regs->tmr);
>  
> +	clk_disable_unprepare(data->clk);
> +
>  	platform_set_drvdata(pdev, NULL);
>  
>  	return 0;
> @@ -250,14 +267,21 @@ static int __maybe_unused qoriq_tmu_suspend(struct device *dev)
>  	tmr &= ~TMR_ME;
>  	tmu_write(data, tmr, &data->regs->tmr);
>  
> +	clk_disable_unprepare(data->clk);
> +
>  	return 0;
>  }
>  
>  static int __maybe_unused qoriq_tmu_resume(struct device *dev)
>  {
>  	u32 tmr;
> +	int ret;
>  	struct qoriq_tmu_data *data = dev_get_drvdata(dev);
>  
> +	ret = clk_prepare_enable(data->clk);
> +	if (ret)
> +		return ret;
> +
>  	/* Enable monitoring */
>  	tmr = tmu_read(data, &data->regs->tmr);
>  	tmr |= TMR_ME;

Apart from that it looks like what Fabio sent and what i tested so

Reviewed-by: Guido Günther <agx@sigxcpu.org>

Cheers,
 -- Guido

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

Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT

[Daniel Baluta]

On Mon, Jul 29, 2019 at 10:49 AM Anson Huang <anson.huang@nxp.com> wrote:

> > We are all set then. Thanks Anson for clarifications!
>
> Thanks, so we are all clear about this issue, need to wait thermal maintainer to review
> the rest patch in this series, but I did NOT receive any response from thermal sub-system
> maintainer for really long time, NOT sure when the thermal patches can be accepted.

This is really unfortunate. I think it is safe to do a RESEND of the
patches as it has
been at least 3 weeks since your first send them.

Pick any reviewed-by you got and do a resend.

RE: [PATCH 4/6] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Guido

> On Fri, Jul 05, 2019 at 12:56:10PM +0800, Anson.Huang@nxp.com wrote:
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > Some platforms like i.MX8MQ has clock control for this module, need to
> > add clock operations to make sure the driver is working properly.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/thermal/qoriq_thermal.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/thermal/qoriq_thermal.c
> > b/drivers/thermal/qoriq_thermal.c index 2b2f79b..0813c1b 100644
> > --- a/drivers/thermal/qoriq_thermal.c
> > +++ b/drivers/thermal/qoriq_thermal.c
> > @@ -2,6 +2,7 @@
> >  //
> >  // Copyright 2016 Freescale Semiconductor, Inc.
> >
> > +#include <linux/clk.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/err.h>
> > @@ -72,6 +73,7 @@ struct qoriq_sensor {
> >
> >  struct qoriq_tmu_data {
> >  	struct qoriq_tmu_regs __iomem *regs;
> > +	struct clk *clk;
> >  	bool little_endian;
> >  	struct qoriq_sensor	*sensor[SITES_MAX];
> >  };
> > @@ -208,6 +210,19 @@ static int qoriq_tmu_probe(struct platform_device
> *pdev)
> >  		return PTR_ERR(data->regs);
> >  	}
> >
> > +	data->clk = devm_clk_get(&pdev->dev, NULL);
> > +	if (IS_ERR(data->clk)) {
> > +		if (PTR_ERR(data->clk) == -EPROBE_DEFER)
> > +			return -EPROBE_DEFER;
> > +		data->clk = NULL;
> > +	}
> 
> Wouldn't devm_clk_get_optional make more sense?

Yes, looks like it is better, will fix it in V2.

> 
> > +
> > +	ret = clk_prepare_enable(data->clk);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to enable clock\n");
> > +		return ret;
> > +	}
> > +
> >  	qoriq_tmu_init_device(data);	/* TMU initialization */
> >
> >  	ret = qoriq_tmu_calibration(pdev);	/* TMU calibration */
> > @@ -235,6 +250,8 @@ static int qoriq_tmu_remove(struct
> platform_device *pdev)
> >  	/* Disable monitoring */
> >  	tmu_write(data, TMR_DISABLE, &data->regs->tmr);
> >
> > +	clk_disable_unprepare(data->clk);
> > +
> >  	platform_set_drvdata(pdev, NULL);
> >
> >  	return 0;
> > @@ -250,14 +267,21 @@ static int __maybe_unused
> qoriq_tmu_suspend(struct device *dev)
> >  	tmr &= ~TMR_ME;
> >  	tmu_write(data, tmr, &data->regs->tmr);
> >
> > +	clk_disable_unprepare(data->clk);
> > +
> >  	return 0;
> >  }
> >
> >  static int __maybe_unused qoriq_tmu_resume(struct device *dev)  {
> >  	u32 tmr;
> > +	int ret;
> >  	struct qoriq_tmu_data *data = dev_get_drvdata(dev);
> >
> > +	ret = clk_prepare_enable(data->clk);
> > +	if (ret)
> > +		return ret;
> > +
> >  	/* Enable monitoring */
> >  	tmr = tmu_read(data, &data->regs->tmr);
> >  	tmr |= TMR_ME;
> 
> Apart from that it looks like what Fabio sent and what i tested so
> 
> Reviewed-by: Guido Günther <agx@sigxcpu.org>

Thanks,
Anson

> 
> Cheers,
>  -- Guido
> 
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-
> kernel&amp;data=02%7C0
> >
> 1%7Canson.huang%40nxp.com%7C9263c240da82482af57908d713fc7d0b%7
> C686ea1d
> >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636999847472894624&amp;sdat
> a=0YAlK
> > V8ZS37vHFz311nOdBP8qBbqisvjFBtaSS1PV9k%3D&amp;reserved=0
> >