| Message ID | 20190730022126.17883-1-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [V3,1/5] thermal: qoriq: Add clock operations | expand |
> 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
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
> 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
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
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
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
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
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
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&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
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
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&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
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 > >
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
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&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
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
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
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;