| Message ID | 20190729083915.4855-4-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [V2,1/4] thermal: qoriq: Use devm_platform_ioremap_resource() instead of of_iomap() | expand |
Hi Anson, On Mon, Jul 29, 2019 at 6:04 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. I haven't seen this series earlier, and I have sent a similar patch for Guido to test. Since this patch solves a hang problem, I would suggest that this one becomes the first of the series. Also, the "clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT" should only be applied after this one in order to avoid the hang. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > Reviewed-by: Guido Günther <agx@sigxcpu.org> > --- > Changes since V1: > - use devm_clk_get_optional() instead of devm_clk_get(). > --- > drivers/thermal/qoriq_thermal.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c > index 2b2f79b..0ae45c0 100644 > --- a/drivers/thermal/qoriq_thermal.c > +++ b/drivers/thermal/qoriq_thermal.c > @@ -2,6 +2,7 @@ > // > // Copyright 2016 Freescale Semiconductor, Inc. > > +#include <linux/clk.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/err.h> > @@ -72,6 +73,7 @@ struct qoriq_sensor { > > struct qoriq_tmu_data { > struct qoriq_tmu_regs __iomem *regs; > + struct clk *clk; > bool little_endian; > struct qoriq_sensor *sensor[SITES_MAX]; > }; > @@ -208,6 +210,16 @@ static int qoriq_tmu_probe(struct platform_device *pdev) > return PTR_ERR(data->regs); > } > > + 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 */ In case of failure the TMU clock should be disabled in the error path. Thanks
Hi Anson, On Tue, Jul 30, 2019 at 12:00 AM Anson Huang <anson.huang@nxp.com> wrote: > Shawn already applied the patch, and Abel has the AHB clock patch to fix that, > so just wait for the AHB clock patch in instead of revert the TMU clock patch? Sorry, I don't understand Abel's patch as there is not a proper description of what it tries to fix. If I understand correctly Abel's patch is not the proper fix and the real fix for the kernel TMU hang in linux-next is to manage the TMU clock inside the TMU driver, like you did in this patch. To avoid a revert one possible solution would be to send only this one as a fix for 5.3. You can point that it Fixes the commit that adds i.MX8M support for the TMU driver.
diff --git a/drivers/thermal/qoriq_thermal.c b/drivers/thermal/qoriq_thermal.c index 2b2f79b..0ae45c0 100644 --- a/drivers/thermal/qoriq_thermal.c +++ b/drivers/thermal/qoriq_thermal.c @@ -2,6 +2,7 @@ // // Copyright 2016 Freescale Semiconductor, Inc. +#include <linux/clk.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/err.h> @@ -72,6 +73,7 @@ struct qoriq_sensor { struct qoriq_tmu_data { struct qoriq_tmu_regs __iomem *regs; + struct clk *clk; bool little_endian; struct qoriq_sensor *sensor[SITES_MAX]; }; @@ -208,6 +210,16 @@ static int qoriq_tmu_probe(struct platform_device *pdev) return PTR_ERR(data->regs); } + 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 */ @@ -235,6 +247,8 @@ static int qoriq_tmu_remove(struct platform_device *pdev) /* Disable monitoring */ tmu_write(data, TMR_DISABLE, &data->regs->tmr); + clk_disable_unprepare(data->clk); + platform_set_drvdata(pdev, NULL); return 0; @@ -250,14 +264,21 @@ static int __maybe_unused qoriq_tmu_suspend(struct device *dev) tmr &= ~TMR_ME; tmu_write(data, tmr, &data->regs->tmr); + clk_disable_unprepare(data->clk); + return 0; } static int __maybe_unused qoriq_tmu_resume(struct device *dev) { u32 tmr; + int ret; struct qoriq_tmu_data *data = dev_get_drvdata(dev); + ret = clk_prepare_enable(data->clk); + if (ret) + return ret; + /* Enable monitoring */ tmr = tmu_read(data, &data->regs->tmr); tmr |= TMR_ME;