[PATCH V3 1/5] thermal: qoriq: Add clock operations

[PATCH V3 1/5] 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>
Reviewed-by: Guido Günther <agx@sigxcpu.org>
---
Changes since V2:
	- move this patch as first patch in the series;
	- add clock disable handling in error path of probe.
---
 drivers/thermal/qoriq_thermal.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 7b36493..2893947 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];
 };
@@ -209,6 +211,16 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 		goto err_iomap;
 	}
 
+	data->clk = devm_clk_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(data->clk))
+		return PTR_ERR(data->clk);
+
+	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 */
@@ -225,6 +237,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 	return 0;
 
 err_tmu:
+	clk_disable_unprepare(data->clk);
 	iounmap(data->regs);
 
 err_iomap:
@@ -241,6 +254,9 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
 	tmu_write(data, TMR_DISABLE, &data->regs->tmr);
 
 	iounmap(data->regs);
+
+	clk_disable_unprepare(data->clk);
+
 	platform_set_drvdata(pdev, NULL);
 
 	return 0;
@@ -257,14 +273,21 @@ static int 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 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 V3 2/5] thermal: qoriq: Fix error path of calling qoriq_tmu_register_tmu_zone fail

[Anson.Huang]

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

When registering tmu zone failed, the error path should be err_tmu
instead of err_iomap, as iounmap() needs to be called.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
New patch.
---
 drivers/thermal/qoriq_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c
index 2893947..5755a11 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -231,7 +231,7 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to register sensors\n");
 		ret = -ENODEV;
-		goto err_iomap;
+		goto err_tmu;
 	}
 
 	return 0;
-- 
2.7.4

[PATCH V3 3/5] 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>
---
No change, just adjust the patch sequence and handle the conflicts.
---
 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 5755a11..8d19601 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -204,11 +204,10 @@ 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);
 	}
 
 	data->clk = devm_clk_get_optional(&pdev->dev, NULL);
@@ -225,22 +224,19 @@ static int qoriq_tmu_probe(struct platform_device *pdev)
 
 	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_tmu;
+		goto err;
 	}
 
 	return 0;
 
-err_tmu:
+err:
 	clk_disable_unprepare(data->clk);
-	iounmap(data->regs);
-
-err_iomap:
 	platform_set_drvdata(pdev, NULL);
 
 	return ret;
@@ -253,8 +249,6 @@ static int qoriq_tmu_remove(struct platform_device *pdev)
 	/* Disable monitoring */
 	tmu_write(data, TMR_DISABLE, &data->regs->tmr);
 
-	iounmap(data->regs);
-
 	clk_disable_unprepare(data->clk);
 
 	platform_set_drvdata(pdev, NULL);
-- 
2.7.4

[PATCH V3 4/5] 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>
---
No changes.
---
 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 8d19601..39542c6 100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -256,8 +256,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);
@@ -272,7 +271,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;
 	int ret;
@@ -289,7 +288,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 V3 5/5] dt-bindings: thermal: qoriq: Add optional clocks property

[Anson.Huang]

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

Some platforms like i.MX8M series SoCs have clock control for TMU,
add optional clocks property to the binding doc.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No changes, noted the i.MX8M series SoCs need this clock in commit log.
---
 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

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Aisheng Dong]

> 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>
> Reviewed-by: Guido Günther <agx@sigxcpu.org>

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

Regards
Aisheng

RE: [PATCH V3 2/5] thermal: qoriq: Fix error path of calling qoriq_tmu_register_tmu_zone fail

[Aisheng Dong]

> From: Anson.Huang@nxp.com <Anson.Huang@nxp.com>
> Sent: Tuesday, July 30, 2019 10:21 AM
> 
> When registering tmu zone failed, the error path should be err_tmu instead of
> err_iomap, as iounmap() needs to be called.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

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

Regards
Aisheng

RE: [PATCH V3 3/5] thermal: qoriq: Use devm_platform_ioremap_resource() instead of of_iomap()

[Aisheng Dong]

> From: Anson.Huang@nxp.com <Anson.Huang@nxp.com>
> Sent: Tuesday, July 30, 2019 10:21 AM
> 
> 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>

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

Regards
Aisheng

RE: [PATCH V3 4/5] thermal: qoriq: Use __maybe_unused instead of #if CONFIG_PM_SLEEP

[Aisheng Dong]

> From: Anson.Huang@nxp.com <Anson.Huang@nxp.com>
> Sent: Tuesday, July 30, 2019 10:21 AM
> 
> 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>

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

Regards
Aisheng

RE: [PATCH V3 5/5] dt-bindings: thermal: qoriq: Add optional clocks property

[Aisheng Dong]

> From: Anson.Huang@nxp.com <Anson.Huang@nxp.com>
> Sent: Tuesday, July 30, 2019 10:21 AM
> 
> Some platforms like i.MX8M series SoCs have clock control for TMU, add
> optional clocks property to the binding doc.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

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

Regards
Aisheng

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Leonard Crestez]

On 7/30/2019 5:31 AM, 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>
> Reviewed-by: Guido Günther <agx@sigxcpu.org>

This series looks good, do you think it can be merged in time for v5.4? 
Today was v5.3-rc6.

In an earlier series the CLK_IS_CRITICAL flags was removed from the TMU 
clock so if the thermal driver doesn't explicitly enable it the system 
will hang on probe. This is what happens in linux-next right now!

Unless this patches is merged soon we'll end up with a 5.4-rc1 that 
doesn't boot on imx8mq. An easy fix would be to drop/revert commit 
951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for 
IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.

Merging patches out-of-order when they have hard (boot-breaking) 
dependencies also breaks bisect.

--
Regards,
Leonard

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

> On 7/30/2019 5:31 AM, 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>
> > Reviewed-by: Guido Günther <agx@sigxcpu.org>
> 
> This series looks good, do you think it can be merged in time for v5.4?
> Today was v5.3-rc6.

If the question is for me, then I am NOT sure, the thermal patches are pending
there for almost half year and I did NOT receive any response, looks like no one
is maintaining the thermal sub-system?

> 
> In an earlier series the CLK_IS_CRITICAL flags was removed from the TMU
> clock so if the thermal driver doesn't explicitly enable it the system will hang
> on probe. This is what happens in linux-next right now!

The thermal driver should be built with module, so default kernel should can boot
up, do you modify the thermal driver as built-in?

> 
> Unless this patches is merged soon we'll end up with a 5.4-rc1 that doesn't
> boot on imx8mq. An easy fix would be to drop/revert commit
> 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.

If the thermal driver is built as module, I think no need to revert the commit, but
if by default thermal driver is built-in or mod probed, then yes, it should NOT break
kernel boot up.

Anson.

> 
> Merging patches out-of-order when they have hard (boot-breaking)
> dependencies also breaks bisect.
> 
> --
> Regards,
> Leonard

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Zhang Rui]

On Tue, 2019-08-27 at 01:51 +0000, Anson Huang wrote:
> > On 7/30/2019 5:31 AM, 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>
> > > Reviewed-by: Guido Günther <agx@sigxcpu.org>
> > 
> > This series looks good, do you think it can be merged in time for
> > v5.4?
> > Today was v5.3-rc6.
> 
> If the question is for me, then I am NOT sure, the thermal patches
> are pending
> there for almost half year and I did NOT receive any response,

which patch series you're referring to?

>  looks like no one
> is maintaining the thermal sub-system?
> 

Eduardo is maintaining all the thermal-soc driver changes. Thus I
usually filtered out the soc driver patches in my mailbox.

The last email from Eduardo is that he is offline during this July and
will be back and taking patches in August.

I will double check with Eduardo anyway.

thanks,
rui


> > 
> > In an earlier series the CLK_IS_CRITICAL flags was removed from the
> > TMU
> > clock so if the thermal driver doesn't explicitly enable it the
> > system will hang
> > on probe. This is what happens in linux-next right now!
> 
> The thermal driver should be built with module, so default kernel
> should can boot
> up, do you modify the thermal driver as built-in?
> 
> > 
> > Unless this patches is merged soon we'll end up with a 5.4-rc1 that
> > doesn't
> > boot on imx8mq. An easy fix would be to drop/revert commit
> > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> 
> If the thermal driver is built as module, I think no need to revert
> the commit, but
> if by default thermal driver is built-in or mod probed, then yes, it
> should NOT break
> kernel boot up.
> 
> Anson.
> 
> > 
> > Merging patches out-of-order when they have hard (boot-breaking)
> > dependencies also breaks bisect.
> > 
> > --
> > Regards,
> > Leonard

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Rui

> On Tue, 2019-08-27 at 01:51 +0000, Anson Huang wrote:
> > > On 7/30/2019 5:31 AM, 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>
> > > > Reviewed-by: Guido Günther <agx@sigxcpu.org>
> > >
> > > This series looks good, do you think it can be merged in time for
> > > v5.4?
> > > Today was v5.3-rc6.
> >
> > If the question is for me, then I am NOT sure, the thermal patches are
> > pending there for almost half year and I did NOT receive any response,
> 
> which patch series you're referring to?

Below i.MX8QXP patch series, I meant I started it from last year, the latest resend
version is 2 months ago: 

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


> 
> >  looks like no one
> > is maintaining the thermal sub-system?

Below two patch series are also pending there for some time:

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

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

> >
> 
> Eduardo is maintaining all the thermal-soc driver changes. Thus I usually
> filtered out the soc driver patches in my mailbox.
> 
> The last email from Eduardo is that he is offline during this July and will be
> back and taking patches in August.
> 
> I will double check with Eduardo anyway.

Thanks, especially our i.MX8QXP thermal driver patch, I started it from last year and
due to some different opinion from different reviewers, this patch series version is
up to v15, but I did NOT receive response since June, appreciated if you guys can take
a look at this, I ping it many times but no feedback. Thanks!

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

Anson.

> 
> thanks,
> rui
> 
> 
> > >
> > > In an earlier series the CLK_IS_CRITICAL flags was removed from the
> > > TMU clock so if the thermal driver doesn't explicitly enable it the
> > > system will hang on probe. This is what happens in linux-next right
> > > now!
> >
> > The thermal driver should be built with module, so default kernel
> > should can boot up, do you modify the thermal driver as built-in?
> >
> > >
> > > Unless this patches is merged soon we'll end up with a 5.4-rc1 that
> > > doesn't boot on imx8mq. An easy fix would be to drop/revert commit
> > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> >
> > If the thermal driver is built as module, I think no need to revert
> > the commit, but if by default thermal driver is built-in or mod
> > probed, then yes, it should NOT break kernel boot up.
> >
> > Anson.
> >
> > >
> > > Merging patches out-of-order when they have hard (boot-breaking)
> > > dependencies also breaks bisect.
> > >
> > > --
> > > Regards,
> > > Leonard

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Leonard Crestez]

On 27.08.2019 04:51, Anson Huang wrote:
>> In an earlier series the CLK_IS_CRITICAL flags was removed from the TMU
>> clock so if the thermal driver doesn't explicitly enable it the system will hang
>> on probe. This is what happens in linux-next right now!
> 
> The thermal driver should be built with module, so default kernel should can boot
> up, do you modify the thermal driver as built-in?
> 
>> Unless this patches is merged soon we'll end up with a 5.4-rc1 that doesn't
>> boot on imx8mq. An easy fix would be to drop/revert commit
>> 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
>> IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> 
> If the thermal driver is built as module, I think no need to revert the commit, but
> if by default thermal driver is built-in or mod probed, then yes, it should NOT break
> kernel boot up.

The qoriq_thermal driver is built as a module in defconfig and when 
modules are properly installed in rootfs they will be automatically be 
probed on boot and cause a hang.

I usually run nfsroot with modules:

     make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root

--
Regards,
Leonard

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Zhang Rui]

On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> On 27.08.2019 04:51, Anson Huang wrote:
> > > In an earlier series the CLK_IS_CRITICAL flags was removed from
> > > the TMU
> > > clock so if the thermal driver doesn't explicitly enable it the
> > > system will hang
> > > on probe. This is what happens in linux-next right now!
> > 
> > The thermal driver should be built with module, so default kernel
> > should can boot
> > up, do you modify the thermal driver as built-in?
> > 
> > > Unless this patches is merged soon we'll end up with a 5.4-rc1
> > > that doesn't
> > > boot on imx8mq. An easy fix would be to drop/revert commit
> > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> > 
> > If the thermal driver is built as module, I think no need to revert
> > the commit, but
> > if by default thermal driver is built-in or mod probed, then yes,
> > it should NOT break
> > kernel boot up.
> 
> The qoriq_thermal driver is built as a module in defconfig and when 
> modules are properly installed in rootfs they will be automatically
> be 
> probed on boot and cause a hang.
> 
> I usually run nfsroot with modules:
> 
>      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root

so we need this patch shipped in the beginning of the merge window,
right?
if there is hard dependency between patches, it's better to send them
in one series, and get shipped via either tree.

BTW, who is maintaining qoriq driver from NXP? If Anson is maintaining
and developing this driver, it's better to update this in the driver or
the MAINTAINER file, I will take the driver specific patches as long as
we have ACK/Reviewed-By from the driver maintainer.

thanks,
rui

> 
> --
> Regards,
> Leonard

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Zhang Rui]

On Wed, 2019-08-28 at 16:32 +0800, Zhang Rui wrote:
> On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > On 27.08.2019 04:51, Anson Huang wrote:
> > > > In an earlier series the CLK_IS_CRITICAL flags was removed from
> > > > the TMU
> > > > clock so if the thermal driver doesn't explicitly enable it the
> > > > system will hang
> > > > on probe. This is what happens in linux-next right now!
> > > 
> > > The thermal driver should be built with module, so default kernel
> > > should can boot
> > > up, do you modify the thermal driver as built-in?
> > > 
> > > > Unless this patches is merged soon we'll end up with a 5.4-rc1
> > > > that doesn't
> > > > boot on imx8mq. An easy fix would be to drop/revert commit
> > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> > > 
> > > If the thermal driver is built as module, I think no need to
> > > revert
> > > the commit, but
> > > if by default thermal driver is built-in or mod probed, then yes,
> > > it should NOT break
> > > kernel boot up.
> > 
> > The qoriq_thermal driver is built as a module in defconfig and
> > when 
> > modules are properly installed in rootfs they will be automatically
> > be 
> > probed on boot and cause a hang.
> > 
> > I usually run nfsroot with modules:
> > 
> >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> 
> so we need this patch shipped in the beginning of the merge window,
> right?
> if there is hard dependency between patches, it's better to send them
> in one series, and get shipped via either tree.
> 
> BTW, who is maintaining qoriq driver from NXP? If Anson is
> maintaining
> and developing this driver, it's better to update this in the driver
> or
> the MAINTAINER file, I will take the driver specific patches as long
> as
> we have ACK/Reviewed-By from the driver maintainer.

And also, can you provide your feedback for this one?
https://patchwork.kernel.org/patch/10974147/

thanks,
rui
> 
> thanks,
> rui
> 
> > 
> > --
> > Regards,
> > Leonard

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Rui

> On Wed, 2019-08-28 at 16:32 +0800, Zhang Rui wrote:
> > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > In an earlier series the CLK_IS_CRITICAL flags was removed from
> > > > > the TMU clock so if the thermal driver doesn't explicitly enable
> > > > > it the system will hang on probe. This is what happens in
> > > > > linux-next right now!
> > > >
> > > > The thermal driver should be built with module, so default kernel
> > > > should can boot up, do you modify the thermal driver as built-in?
> > > >
> > > > > Unless this patches is merged soon we'll end up with a 5.4-rc1
> > > > > that doesn't boot on imx8mq. An easy fix would be to drop/revert
> > > > > commit
> > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> > > >
> > > > If the thermal driver is built as module, I think no need to
> > > > revert the commit, but if by default thermal driver is built-in or
> > > > mod probed, then yes, it should NOT break kernel boot up.
> > >
> > > The qoriq_thermal driver is built as a module in defconfig and when
> > > modules are properly installed in rootfs they will be automatically
> > > be probed on boot and cause a hang.
> > >
> > > I usually run nfsroot with modules:
> > >
> > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> >
> > so we need this patch shipped in the beginning of the merge window,
> > right?
> > if there is hard dependency between patches, it's better to send them
> > in one series, and get shipped via either tree.
> >
> > BTW, who is maintaining qoriq driver from NXP? If Anson is maintaining
> > and developing this driver, it's better to update this in the driver
> > or the MAINTAINER file, I will take the driver specific patches as
> > long as we have ACK/Reviewed-By from the driver maintainer.

I am NOT sure who is the qoriq driver from NXP, some of our i.MX SoCs use
qoriq thermal IP, so I have to add support for them.  The first maintainer for
this driver is hongtao.jia@nxp.com, but I can NOT find it from NXP's mail system,
NOT sure if he is still in NXP. The MAINTAINER file does NOT have info for this
Driver's maintainer, so how to update it? Just change the name in driver? Or leave
it as what it is?

 MODULE_AUTHOR("Jia Hongtao <hongtao.jia@nxp.com>");
 MODULE_DESCRIPTION("QorIQ Thermal Monitoring Unit driver");
 MODULE_LICENSE("GPL v2");

> 
> And also, can you provide your feedback for this one?
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fpatch%2F10974147%2F&amp;data=02%7C01%7Canson.h
> uang%40nxp.com%7C887e7c90f7c943ff0d9b08d72b92aea1%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637025781325203384&amp;sdata=Xg
> tX6mPdA50Nbb%2FmnS2om2bJNepTd1th6HmfwGuU9Hw%3D&amp;reserve
> d=0

I can take a look at it later.

Thanks,
Anson

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Rui

> On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > On 27.08.2019 04:51, Anson Huang wrote:
> > > > In an earlier series the CLK_IS_CRITICAL flags was removed from
> > > > the TMU clock so if the thermal driver doesn't explicitly enable
> > > > it the system will hang on probe. This is what happens in
> > > > linux-next right now!
> > >
> > > The thermal driver should be built with module, so default kernel
> > > should can boot up, do you modify the thermal driver as built-in?
> > >
> > > > Unless this patches is merged soon we'll end up with a 5.4-rc1
> > > > that doesn't boot on imx8mq. An easy fix would be to drop/revert
> > > > commit
> > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> > >
> > > If the thermal driver is built as module, I think no need to revert
> > > the commit, but if by default thermal driver is built-in or mod
> > > probed, then yes, it should NOT break kernel boot up.
> >
> > The qoriq_thermal driver is built as a module in defconfig and when
> > modules are properly installed in rootfs they will be automatically be
> > probed on boot and cause a hang.
> >
> > I usually run nfsroot with modules:
> >
> >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> 
> so we need this patch shipped in the beginning of the merge window, right?
> if there is hard dependency between patches, it's better to send them in one
> series, and get shipped via either tree.

There is no hard dependency in this patch series. Previous for the TMU clock disabled
patch, since thermal driver is built as module so I did NOT found the issue. The patch
series is the correct fix.

Thanks,
Anson 

> 
> BTW, who is maintaining qoriq driver from NXP? If Anson is maintaining and
> developing this driver, it's better to update this in the driver or the
> MAINTAINER file, I will take the driver specific patches as long as we have
> ACK/Reviewed-By from the driver maintainer.
> 
> thanks,
> rui
> 
> >
> > --
> > Regards,
> > Leonard

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Zhang Rui]

On Wed, 2019-08-28 at 08:49 +0000, Anson Huang wrote:
> Hi, Rui
> 
> > On Wed, 2019-08-28 at 16:32 +0800, Zhang Rui wrote:
> > > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > > In an earlier series the CLK_IS_CRITICAL flags was removed
> > > > > > from
> > > > > > the TMU clock so if the thermal driver doesn't explicitly
> > > > > > enable
> > > > > > it the system will hang on probe. This is what happens in
> > > > > > linux-next right now!
> > > > > 
> > > > > The thermal driver should be built with module, so default
> > > > > kernel
> > > > > should can boot up, do you modify the thermal driver as
> > > > > built-in?
> > > > > 
> > > > > > Unless this patches is merged soon we'll end up with a 5.4-
> > > > > > rc1
> > > > > > that doesn't boot on imx8mq. An easy fix would be to
> > > > > > drop/revert
> > > > > > commit
> > > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are
> > > > > > accepted.
> > > > > 
> > > > > If the thermal driver is built as module, I think no need to
> > > > > revert the commit, but if by default thermal driver is built-
> > > > > in or
> > > > > mod probed, then yes, it should NOT break kernel boot up.
> > > > 
> > > > The qoriq_thermal driver is built as a module in defconfig and
> > > > when
> > > > modules are properly installed in rootfs they will be
> > > > automatically
> > > > be probed on boot and cause a hang.
> > > > 
> > > > I usually run nfsroot with modules:
> > > > 
> > > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> > > 
> > > so we need this patch shipped in the beginning of the merge
> > > window,
> > > right?
> > > if there is hard dependency between patches, it's better to send
> > > them
> > > in one series, and get shipped via either tree.
> > > 
> > > BTW, who is maintaining qoriq driver from NXP? If Anson is
> > > maintaining
> > > and developing this driver, it's better to update this in the
> > > driver
> > > or the MAINTAINER file, I will take the driver specific patches
> > > as
> > > long as we have ACK/Reviewed-By from the driver maintainer.
> 
> I am NOT sure who is the qoriq driver from NXP, some of our i.MX SoCs
> use
> qoriq thermal IP, so I have to add support for them.  The first
> maintainer for
> this driver is hongtao.jia@nxp.com, but I can NOT find it from NXP's
> mail system,
> NOT sure if he is still in NXP. The MAINTAINER file does NOT have
> info for this
> Driver's maintainer, so how to update it? Just change the name in
> driver? Or leave
> it as what it is?
> 
>  MODULE_AUTHOR("Jia Hongtao <hongtao.jia@nxp.com>");
>  MODULE_DESCRIPTION("QorIQ Thermal Monitoring Unit driver");
>  MODULE_LICENSE("GPL v2");
> 
I see several people are actively working on this driver from NXP.
If you're okay, I'd like to get your comments on all the patches for
this driver before I take them, and I can update the MAINTAINER file
later so that you're CCed for all the qoriq thermal driver changes. 

> > 
> > And also, can you provide your feedback for this one?
> > 
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> > work.kernel.org%2Fpatch%2F10974147%2F&amp;data=02%7C01%7Canson.h
> > uang%40nxp.com%7C887e7c90f7c943ff0d9b08d72b92aea1%7C686ea1d3bc2
> > b4c6fa92cd99c5c301635%7C0%7C0%7C637025781325203384&amp;sdata=Xg
> > tX6mPdA50Nbb%2FmnS2om2bJNepTd1th6HmfwGuU9Hw%3D&amp;reserve
> > d=0
> 
> I can take a look at it later.
> 
thanks!

-rui
> Thanks,
> Anson

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Zhang Rui]

On Wed, 2019-08-28 at 08:51 +0000, Anson Huang wrote:
> Hi, Rui
> 
> > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > In an earlier series the CLK_IS_CRITICAL flags was removed
> > > > > from
> > > > > the TMU clock so if the thermal driver doesn't explicitly
> > > > > enable
> > > > > it the system will hang on probe. This is what happens in
> > > > > linux-next right now!
> > > > 
> > > > The thermal driver should be built with module, so default
> > > > kernel
> > > > should can boot up, do you modify the thermal driver as built-
> > > > in?
> > > > 
> > > > > Unless this patches is merged soon we'll end up with a 5.4-
> > > > > rc1
> > > > > that doesn't boot on imx8mq. An easy fix would be to
> > > > > drop/revert
> > > > > commit
> > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> > > > 
> > > > If the thermal driver is built as module, I think no need to
> > > > revert
> > > > the commit, but if by default thermal driver is built-in or mod
> > > > probed, then yes, it should NOT break kernel boot up.
> > > 
> > > The qoriq_thermal driver is built as a module in defconfig and
> > > when
> > > modules are properly installed in rootfs they will be
> > > automatically be
> > > probed on boot and cause a hang.
> > > 
> > > I usually run nfsroot with modules:
> > > 
> > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> > 
> > so we need this patch shipped in the beginning of the merge window,
> > right?
> > if there is hard dependency between patches, it's better to send
> > them in one
> > series, and get shipped via either tree.
> 
> There is no hard dependency in this patch series. Previous for the
> TMU clock disabled
> patch, since thermal driver is built as module so I did NOT found the
> issue. The patch
> series is the correct fix.
> 
Got it.
the clock patch is also queued for 5.4-rc1, right?
I will apply this series and try to push it as early as possible during
the merge window.

thanks,
rui
> Thanks,
> Anson 
> 
> > 
> > BTW, who is maintaining qoriq driver from NXP? If Anson is
> > maintaining and
> > developing this driver, it's better to update this in the driver or
> > the
> > MAINTAINER file, I will take the driver specific patches as long as
> > we have
> > ACK/Reviewed-By from the driver maintainer.
> > 
> > thanks,
> > rui
> > 
> > > 
> > > --
> > > Regards,
> > > Leonard
> 
> 

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Rui

> On Wed, 2019-08-28 at 08:51 +0000, Anson Huang wrote:
> > Hi, Rui
> >
> > > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > > In an earlier series the CLK_IS_CRITICAL flags was removed
> > > > > > from the TMU clock so if the thermal driver doesn't explicitly
> > > > > > enable it the system will hang on probe. This is what happens
> > > > > > in linux-next right now!
> > > > >
> > > > > The thermal driver should be built with module, so default
> > > > > kernel should can boot up, do you modify the thermal driver as
> > > > > built- in?
> > > > >
> > > > > > Unless this patches is merged soon we'll end up with a 5.4-
> > > > > > rc1
> > > > > > that doesn't boot on imx8mq. An easy fix would be to
> > > > > > drop/revert commit
> > > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are accepted.
> > > > >
> > > > > If the thermal driver is built as module, I think no need to
> > > > > revert the commit, but if by default thermal driver is built-in
> > > > > or mod probed, then yes, it should NOT break kernel boot up.
> > > >
> > > > The qoriq_thermal driver is built as a module in defconfig and
> > > > when modules are properly installed in rootfs they will be
> > > > automatically be probed on boot and cause a hang.
> > > >
> > > > I usually run nfsroot with modules:
> > > >
> > > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> > >
> > > so we need this patch shipped in the beginning of the merge window,
> > > right?
> > > if there is hard dependency between patches, it's better to send
> > > them in one series, and get shipped via either tree.
> >
> > There is no hard dependency in this patch series. Previous for the TMU
> > clock disabled patch, since thermal driver is built as module so I did
> > NOT found the issue. The patch series is the correct fix.
> >
> Got it.
> the clock patch is also queued for 5.4-rc1, right?
> I will apply this series and try to push it as early as possible during the merge
> window.

The clock patch is as below in Linux-next tree, while I did NOT see it in v5.3-rc6,
so it should be queued for 5.4-rc1, right?
Thanks for taking the patch series!


commit 951c1aef9691491ddf4dd5aab76f2665d56bd5d3
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.

    Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
    Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
    Acked-by: Stephen Boyd <sboyd@kernel.org>
    Signed-off-by: Shawn Guo <shawnguo@kernel.org>

drivers/clk/imx/clk-imx8mq.c


Thanks,
Anson

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Rui

> On Wed, 2019-08-28 at 08:49 +0000, Anson Huang wrote:
> > Hi, Rui
> >
> > > On Wed, 2019-08-28 at 16:32 +0800, Zhang Rui wrote:
> > > > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > > > In an earlier series the CLK_IS_CRITICAL flags was removed
> > > > > > > from the TMU clock so if the thermal driver doesn't
> > > > > > > explicitly enable it the system will hang on probe. This is
> > > > > > > what happens in linux-next right now!
> > > > > >
> > > > > > The thermal driver should be built with module, so default
> > > > > > kernel should can boot up, do you modify the thermal driver as
> > > > > > built-in?
> > > > > >
> > > > > > > Unless this patches is merged soon we'll end up with a 5.4-
> > > > > > > rc1
> > > > > > > that doesn't boot on imx8mq. An easy fix would be to
> > > > > > > drop/revert commit
> > > > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are
> > > > > > > accepted.
> > > > > >
> > > > > > If the thermal driver is built as module, I think no need to
> > > > > > revert the commit, but if by default thermal driver is built-
> > > > > > in or mod probed, then yes, it should NOT break kernel boot
> > > > > > up.
> > > > >
> > > > > The qoriq_thermal driver is built as a module in defconfig and
> > > > > when modules are properly installed in rootfs they will be
> > > > > automatically be probed on boot and cause a hang.
> > > > >
> > > > > I usually run nfsroot with modules:
> > > > >
> > > > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> > > >
> > > > so we need this patch shipped in the beginning of the merge
> > > > window, right?
> > > > if there is hard dependency between patches, it's better to send
> > > > them in one series, and get shipped via either tree.
> > > >
> > > > BTW, who is maintaining qoriq driver from NXP? If Anson is
> > > > maintaining and developing this driver, it's better to update this
> > > > in the driver or the MAINTAINER file, I will take the driver
> > > > specific patches as long as we have ACK/Reviewed-By from the
> > > > driver maintainer.
> >
> > I am NOT sure who is the qoriq driver from NXP, some of our i.MX SoCs
> > use qoriq thermal IP, so I have to add support for them.  The first
> > maintainer for this driver is hongtao.jia@nxp.com, but I can NOT find
> > it from NXP's mail system, NOT sure if he is still in NXP. The
> > MAINTAINER file does NOT have info for this Driver's maintainer, so
> > how to update it? Just change the name in driver? Or leave it as what
> > it is?
> >
> >  MODULE_AUTHOR("Jia Hongtao <hongtao.jia@nxp.com>");
> > MODULE_DESCRIPTION("QorIQ Thermal Monitoring Unit driver");
> > MODULE_LICENSE("GPL v2");
> >
> I see several people are actively working on this driver from NXP.
> If you're okay, I'd like to get your comments on all the patches for this driver
> before I take them, and I can update the MAINTAINER file later so that you're
> CCed for all the qoriq thermal driver changes.

I am OK, but it may take me some days to do it, I will make it ASAP, thanks.

Anson

> 
> > >
> > > And also, can you provide your feedback for this one?
> > >
> https://patch
> > >
> work.kernel.org%2Fpatch%2F10974147%2F&amp;data=02%7C01%7Canson.h
> > >
> uang%40nxp.com%7C887e7c90f7c943ff0d9b08d72b92aea1%7C686ea1d3bc2
> > >
> b4c6fa92cd99c5c301635%7C0%7C0%7C637025781325203384&amp;sdata=Xg
> > >
> tX6mPdA50Nbb%2FmnS2om2bJNepTd1th6HmfwGuU9Hw%3D&amp;reserve
> > > d=0
> >
> > I can take a look at it later.
> >
> thanks!
> 
> -rui
> > Thanks,
> > Anson

RE: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Anson Huang]

Hi, Rui

> > On Wed, 2019-08-28 at 08:51 +0000, Anson Huang wrote:
> > > Hi, Rui
> > >
> > > > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > > > In an earlier series the CLK_IS_CRITICAL flags was removed
> > > > > > > from the TMU clock so if the thermal driver doesn't
> > > > > > > explicitly enable it the system will hang on probe. This is
> > > > > > > what happens in linux-next right now!
> > > > > >
> > > > > > The thermal driver should be built with module, so default
> > > > > > kernel should can boot up, do you modify the thermal driver as
> > > > > > built- in?
> > > > > >
> > > > > > > Unless this patches is merged soon we'll end up with a 5.4-
> > > > > > > rc1
> > > > > > > that doesn't boot on imx8mq. An easy fix would be to
> > > > > > > drop/revert commit
> > > > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are
> accepted.
> > > > > >
> > > > > > If the thermal driver is built as module, I think no need to
> > > > > > revert the commit, but if by default thermal driver is
> > > > > > built-in or mod probed, then yes, it should NOT break kernel boot
> up.
> > > > >
> > > > > The qoriq_thermal driver is built as a module in defconfig and
> > > > > when modules are properly installed in rootfs they will be
> > > > > automatically be probed on boot and cause a hang.
> > > > >
> > > > > I usually run nfsroot with modules:
> > > > >
> > > > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-root
> > > >
> > > > so we need this patch shipped in the beginning of the merge
> > > > window, right?
> > > > if there is hard dependency between patches, it's better to send
> > > > them in one series, and get shipped via either tree.
> > >
> > > There is no hard dependency in this patch series. Previous for the
> > > TMU clock disabled patch, since thermal driver is built as module so
> > > I did NOT found the issue. The patch series is the correct fix.
> > >
> > Got it.
> > the clock patch is also queued for 5.4-rc1, right?
> > I will apply this series and try to push it as early as possible
> > during the merge window.
> 
> The clock patch is as below in Linux-next tree, while I did NOT see it in v5.3-
> rc6, so it should be queued for 5.4-rc1, right?
> Thanks for taking the patch series!

Sorry for pushing, so you will apply this patch series to avoid the i.MX8MQ kernel boot up hang
caused by insmod qoriq thermal driver, right? Then we no need to revert that TMU clock patch
951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT").

Thanks,
Anson

> 
> 
> commit 951c1aef9691491ddf4dd5aab76f2665d56bd5d3
> 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.
> 
>     Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>     Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
>     Acked-by: Stephen Boyd <sboyd@kernel.org>
>     Signed-off-by: Shawn Guo <shawnguo@kernel.org>
> 
> drivers/clk/imx/clk-imx8mq.c
> 
> 
> Thanks,
> Anson

Re: [PATCH V3 1/5] thermal: qoriq: Add clock operations

[Zhang Rui]

On Thu, 2019-08-29 at 02:49 +0000, Anson Huang wrote:
> Hi, Rui
> 
> > > On Wed, 2019-08-28 at 08:51 +0000, Anson Huang wrote:
> > > > Hi, Rui
> > > > 
> > > > > On Tue, 2019-08-27 at 12:41 +0000, Leonard Crestez wrote:
> > > > > > On 27.08.2019 04:51, Anson Huang wrote:
> > > > > > > > In an earlier series the CLK_IS_CRITICAL flags was
> > > > > > > > removed
> > > > > > > > from the TMU clock so if the thermal driver doesn't
> > > > > > > > explicitly enable it the system will hang on probe.
> > > > > > > > This is
> > > > > > > > what happens in linux-next right now!
> > > > > > > 
> > > > > > > The thermal driver should be built with module, so
> > > > > > > default
> > > > > > > kernel should can boot up, do you modify the thermal
> > > > > > > driver as
> > > > > > > built- in?
> > > > > > > 
> > > > > > > > Unless this patches is merged soon we'll end up with a
> > > > > > > > 5.4-
> > > > > > > > rc1
> > > > > > > > that doesn't boot on imx8mq. An easy fix would be to
> > > > > > > > drop/revert commit
> > > > > > > > 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag
> > > > > > > > for
> > > > > > > > IMX8MQ_CLK_TMU_ROOT") until the thermal patches are
> > 
> > accepted.
> > > > > > > 
> > > > > > > If the thermal driver is built as module, I think no need
> > > > > > > to
> > > > > > > revert the commit, but if by default thermal driver is
> > > > > > > built-in or mod probed, then yes, it should NOT break
> > > > > > > kernel boot
> > 
> > up.
> > > > > > 
> > > > > > The qoriq_thermal driver is built as a module in defconfig
> > > > > > and
> > > > > > when modules are properly installed in rootfs they will be
> > > > > > automatically be probed on boot and cause a hang.
> > > > > > 
> > > > > > I usually run nfsroot with modules:
> > > > > > 
> > > > > >      make modules_install INSTALL_MOD_PATH=/srv/nfs/imx8-
> > > > > > root
> > > > > 
> > > > > so we need this patch shipped in the beginning of the merge
> > > > > window, right?
> > > > > if there is hard dependency between patches, it's better to
> > > > > send
> > > > > them in one series, and get shipped via either tree.
> > > > 
> > > > There is no hard dependency in this patch series. Previous for
> > > > the
> > > > TMU clock disabled patch, since thermal driver is built as
> > > > module so
> > > > I did NOT found the issue. The patch series is the correct fix.
> > > > 
> > > 
> > > Got it.
> > > the clock patch is also queued for 5.4-rc1, right?
> > > I will apply this series and try to push it as early as possible
> > > during the merge window.
> > 
> > The clock patch is as below in Linux-next tree, while I did NOT see
> > it in v5.3-
> > rc6, so it should be queued for 5.4-rc1, right?
> > Thanks for taking the patch series!
> 
> Sorry for pushing, so you will apply this patch series to avoid the
> i.MX8MQ kernel boot up hang
> caused by insmod qoriq thermal driver, right? Then we no need to
> revert that TMU clock patch
> 951c1aef9691 ("clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> IMX8MQ_CLK_TMU_ROOT").
> 
right. I will queue it for 5.4-rc1.

thanks,
rui

> Thanks,
> Anson
> 
> > 
> > 
> > commit 951c1aef9691491ddf4dd5aab76f2665d56bd5d3
> > 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.
> > 
> >     Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >     Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
> >     Acked-by: Stephen Boyd <sboyd@kernel.org>
> >     Signed-off-by: Shawn Guo <shawnguo@kernel.org>
> > 
> > drivers/clk/imx/clk-imx8mq.c
> > 
> > 
> > Thanks,
> > Anson