LinuxLists.cc - [PATCH V3 1/5] thermal: qoriq: Add clock operations

[permalink] [raw]

Subject: [PATCH V3 4/5] 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]>
---
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

2019-07-30 04:54:29

[permalink] [raw]

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

From: Anson Huang <[email protected]>

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 <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
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

2019-07-30 04:54:36

[permalink] [raw]

Subject: [PATCH V3 3/5] 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]>
---
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

2019-07-30 04:54:37

[permalink] [raw]

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

From: Anson Huang <[email protected]>

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

2019-08-05 07:01:33

[permalink] [raw]

Subject: RE: [PATCH V3 1/5] 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]>
> Reviewed-by: Guido Günther <[email protected]>

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

2019-08-05 07:02:29

[permalink] [raw]

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

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

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

2019-08-05 07:03:14

[permalink] [raw]

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

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

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

2019-08-05 07:03:21

[permalink] [raw]

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

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

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

2019-08-05 07:05:06

[permalink] [raw]

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

> From: [email protected] <[email protected]>
> 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 <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>

Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

2019-08-26 15:37:57

by Leonard Crestez

[permalink] [raw]

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

On 7/30/2019 5:31 AM, [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]>
> Reviewed-by: Guido G?nther <[email protected]>

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

2019-08-27 01:52:45

[permalink] [raw]

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

> On 7/30/2019 5:31 AM, [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]>
> > Reviewed-by: Guido Günther <[email protected]>
>
> 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

2019-08-27 03:10:42

[permalink] [raw]

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

On Tue, 2019-08-27 at 01:51 +0000, Anson Huang wrote:
> > On 7/30/2019 5:31 AM, [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]>
> > > Reviewed-by: Guido Günther <[email protected]>
> >
> > 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

2019-08-27 03:25:43

[permalink] [raw]

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

Hi, Rui

> On Tue, 2019-08-27 at 01:51 +0000, Anson Huang wrote:
> > > On 7/30/2019 5:31 AM, [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]>
> > > > Reviewed-by: Guido Günther <[email protected]>
> > >
> > > 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

2019-08-27 12:43:07

by Leonard Crestez

[permalink] [raw]

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

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

2019-08-28 08:33:01

[permalink] [raw]

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

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

2019-08-28 08:36:41

[permalink] [raw]

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

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

2019-08-28 08:52:15

[permalink] [raw]

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

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 [email protected], 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 <[email protected]>");
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&data=02%7C01%7Canson.h
> uang%40nxp.com%7C887e7c90f7c943ff0d9b08d72b92aea1%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637025781325203384&sdata=Xg
> tX6mPdA50Nbb%2FmnS2om2bJNepTd1th6HmfwGuU9Hw%3D&reserve
> d=0

I can take a look at it later.

Thanks,
Anson

2019-08-28 08:53:20

[permalink] [raw]

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

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

2019-08-28 09:03:57

[permalink] [raw]

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

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 [email protected], 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 <[email protected]>");
> 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&data=02%7C01%7Canson.h
> > uang%40nxp.com%7C887e7c90f7c943ff0d9b08d72b92aea1%7C686ea1d3bc2
> > b4c6fa92cd99c5c301635%7C0%7C0%7C637025781325203384&sdata=Xg
> > tX6mPdA50Nbb%2FmnS2om2bJNepTd1th6HmfwGuU9Hw%3D&reserve
> > d=0
>
> I can take a look at it later.
>
thanks!

-rui
> Thanks,
> Anson

2019-08-28 09:05:32

[permalink] [raw]

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

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

2019-08-28 09:12:00

[permalink] [raw]

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

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

Signed-off-by: Anson Huang <[email protected]>
Reviewed-by: Abel Vesa <[email protected]>
Acked-by: Stephen Boyd <[email protected]>
Signed-off-by: Shawn Guo <[email protected]>

drivers/clk/imx/clk-imx8mq.c

Thanks,
Anson

2019-08-28 09:14:21

[permalink] [raw]

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

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 [email protected], 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 <[email protected]>");
> > 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&data=02%7C01%7Canson.h
> > >
> uang%40nxp.com%7C887e7c90f7c943ff0d9b08d72b92aea1%7C686ea1d3bc2
> > >
> b4c6fa92cd99c5c301635%7C0%7C0%7C637025781325203384&sdata=Xg
> > >
> tX6mPdA50Nbb%2FmnS2om2bJNepTd1th6HmfwGuU9Hw%3D&reserve
> > > d=0
> >
> > I can take a look at it later.
> >
> thanks!
>
> -rui
> > Thanks,
> > Anson

2019-08-29 02:51:59

[permalink] [raw]

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

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 <[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.
>
> Signed-off-by: Anson Huang <[email protected]>
> Reviewed-by: Abel Vesa <[email protected]>
> Acked-by: Stephen Boyd <[email protected]>
> Signed-off-by: Shawn Guo <[email protected]>
>
> drivers/clk/imx/clk-imx8mq.c
>
>
> Thanks,
> Anson

2019-08-29 03:02:33