2019-07-05 05:06:19

by Anson Huang

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

From: Anson Huang <[email protected]>

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

Signed-off-by: Anson Huang <[email protected]>
---
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


2019-07-05 05:06:21

by Anson Huang

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

From: Anson Huang <[email protected]>

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

Signed-off-by: Anson Huang <[email protected]>
---
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

2019-07-05 05:06:27

by Anson Huang

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

From: Anson Huang <[email protected]>

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

Signed-off-by: Anson Huang <[email protected]>
---
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

2019-07-05 05:06:29

by Anson Huang

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

From: Anson Huang <[email protected]>

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 <[email protected]>
---
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

2019-07-05 05:07:51

by Anson Huang

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

From: Anson Huang <[email protected]>

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 <[email protected]>
---
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

2019-07-05 05:08:53

by Anson Huang

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

From: Anson Huang <[email protected]>

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

Signed-off-by: Anson Huang <[email protected]>
---
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

2019-07-05 07:44:44

by Abel Vesa

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

On 19-07-05 12:56:11, [email protected] wrote:
> From: Anson Huang <[email protected]>
>
> 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 <[email protected]>

Reviewed-by: Abel Vesa <[email protected]>

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

2019-07-23 03:21:07

by Stephen Boyd

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

Quoting [email protected] (2019-07-04 21:56:11)
> From: Anson Huang <[email protected]>
>
> 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 <[email protected]>
> ---

Acked-by: Stephen Boyd <[email protected]>

2019-07-23 09:18:10

by Shawn Guo

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

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

Applied, thanks.

2019-07-23 10:54:55

by Rob Herring (Arm)

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

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

Reviewed-by: Rob Herring <[email protected]>

2019-07-23 12:16:22

by Shawn Guo

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

On Fri, Jul 05, 2019 at 12:56:11PM +0800, [email protected] wrote:
> From: Anson Huang <[email protected]>
>
> 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 <[email protected]>

Applied, thanks.

2019-07-26 23:19:05

by Daniel Baluta

[permalink] [raw]
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 <[email protected]>
Date: Fri Jul 26 15:18:02 2019 +1000

Add linux-next specific files for 20190726

Signed-off-by: Stephen Rothwell <[email protected]>


I know this is crazy but reverting commit:

commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
Author: Anson Huang <[email protected]>
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 <[email protected]> wrote:
>
> From: Anson Huang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>

2019-07-27 06:53:54

by Anson Huang

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

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 <[email protected]>
> Date: Fri Jul 26 15:18:02 2019 +1000
>
> Add linux-next specific files for 20190726
>
> Signed-off-by: Stephen Rothwell <[email protected]>
>
>
> I know this is crazy but reverting commit:
>
> commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> Author: Anson Huang <[email protected]>
> 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

2019-07-27 06:56:52

by Daniel Baluta

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

On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <[email protected]> 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 <[email protected]>
> > Date: Fri Jul 26 15:18:02 2019 +1000
> >
> > Add linux-next specific files for 20190726
> >
> > Signed-off-by: Stephen Rothwell <[email protected]>
> >
> >
> > I know this is crazy but reverting commit:
> >
> > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> > Author: Anson Huang <[email protected]>
> > 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 <[email protected]>
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.

2019-07-27 16:37:00

by Abel Vesa

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

On 19-07-27 09:33:10, Daniel Baluta wrote:
> On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <[email protected]> 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 <[email protected]>
> > > Date: Fri Jul 26 15:18:02 2019 +1000
> > >
> > > Add linux-next specific files for 20190726
> > >
> > > Signed-off-by: Stephen Rothwell <[email protected]>
> > >
> > >
> > > I know this is crazy but reverting commit:
> > >
> > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> > > Author: Anson Huang <[email protected]>
> > > 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 <[email protected]>
> 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.

2019-07-27 18:29:21

by Guido Günther

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

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 <[email protected]>
> Date: Fri Jul 26 15:18:02 2019 +1000
>
> Add linux-next specific files for 20190726
>
> Signed-off-by: Stephen Rothwell <[email protected]>
>
>
> I know this is crazy but reverting commit:
>
> commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
> Author: Anson Huang <[email protected]>
> 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

2019-07-27 20:47:35

by Fabio Estevam

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

Hi Guido,

On Sat, Jul 27, 2019 at 3:26 PM Guido Günther <[email protected]> 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

2019-07-28 08:00:27

by Guido Günther

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

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 <[email protected]> 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

2019-07-28 15:26:52

by Fabio Estevam

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

Hi Guido,

On Sun, Jul 28, 2019 at 4:59 AM Guido Günther <[email protected]> wrote:

> Yes, this fixes it for me, thanks!

Thanks for testing it.

I will send a formal patch tomorrow.

Regards,

Fabio Estevam

2019-07-29 07:16:46

by Anson Huang

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

Hi, Daniel

> On Mon, Jul 29, 2019 at 4:29 AM Anson Huang <[email protected]>
> 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 <[email protected]>
> > > 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 <[email protected]>
> > > > > > Date: Fri Jul 26 15:18:02 2019 +1000
> > > > > >
> > > > > > Add linux-next specific files for 20190726
> > > > > >
> > > > > > Signed-off-by: Stephen Rothwell <[email protected]>
> > > > > >
> > > > > >
> > > > > > I know this is crazy but reverting commit:
> > > > > >
> > > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef
> > > (refs/bisect/bad)
> > > > > > Author: Anson Huang <[email protected]>
> > > > > > 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 <[email protected]>
> > > > 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

2019-07-29 07:22:42

by Daniel Baluta

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

<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:

2019-07-29 07:23:52

by Daniel Baluta

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

On Mon, Jul 29, 2019 at 10:20 AM Daniel Baluta <[email protected]> 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!

2019-07-29 07:43:28

by Daniel Baluta

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

On Mon, Jul 29, 2019 at 4:29 AM Anson Huang <[email protected]> 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 <[email protected]>
> > 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 <[email protected]>
> > > > > Date: Fri Jul 26 15:18:02 2019 +1000
> > > > >
> > > > > Add linux-next specific files for 20190726
> > > > >
> > > > > Signed-off-by: Stephen Rothwell <[email protected]>
> > > > >
> > > > >
> > > > > I know this is crazy but reverting commit:
> > > > >
> > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef
> > (refs/bisect/bad)
> > > > > Author: Anson Huang <[email protected]>
> > > > > 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 <[email protected]>
> > > 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

2019-07-29 08:22:32

by Guido Günther

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

Hi Anson,
On Fri, Jul 05, 2019 at 12:56:10PM +0800, [email protected] wrote:
> From: Anson Huang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>

Cheers,
-- Guido

> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

2019-07-29 08:50:25

by Anson Huang

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

Hi, Daniel

> On Mon, Jul 29, 2019 at 10:20 AM Daniel Baluta <[email protected]>
> 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

2019-07-29 08:53:26

by Daniel Baluta

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

On Mon, Jul 29, 2019 at 10:49 AM Anson Huang <[email protected]> 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.

2019-07-29 09:03:22

by Anson Huang

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

Hi, Guido

> On Fri, Jul 05, 2019 at 12:56:10PM +0800, [email protected] wrote:
> > From: Anson Huang <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>

Thanks,
Anson

>
> Cheers,
> -- Guido
>
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > 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
> >