Messages in this thread | | | Subject | Re: [PATCH 1/2] thermal: qoriq: Use devm_add_action_or_reset() to handle all cleanups | From | Daniel Lezcano <> | Date | Thu, 12 Mar 2020 12:56:39 +0100 |
| |
On 12/03/2020 12:47, Anson Huang wrote: > Hi, Daniel > >> Subject: Re: [PATCH 1/2] thermal: qoriq: Use devm_add_action_or_reset() to >> handle all cleanups >> >> On 11/03/2020 06:07, Anson Huang wrote: >>> Use devm_add_action_or_reset() to handle all cleanups of failure in >>> .probe and .remove, then .remove callback can be dropped. >> >> Is this change compatible with the tristate? > > I think so, any concern need me to double confirm?
TBH, I discovered the function with your patch. My concern is if the callback is called when unloading the module.
>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com> >>> --- >>> drivers/thermal/qoriq_thermal.c | 35 >>> ++++++++++++++--------------------- >>> 1 file changed, 14 insertions(+), 21 deletions(-) >>> >>> diff --git a/drivers/thermal/qoriq_thermal.c >>> b/drivers/thermal/qoriq_thermal.c index 874bc46..67a8d84 100644 >>> --- a/drivers/thermal/qoriq_thermal.c >>> +++ b/drivers/thermal/qoriq_thermal.c >>> @@ -228,6 +228,14 @@ static const struct regmap_access_table >> qoriq_rd_table = { >>> .n_yes_ranges = ARRAY_SIZE(qoriq_yes_ranges), >>> }; >>> >>> +static void qoriq_tmu_action(void *p) { >>> + struct qoriq_tmu_data *data = p; >>> + >>> + regmap_write(data->regmap, REGS_TMR, TMR_DISABLE); >>> + clk_disable_unprepare(data->clk); >>> +} >>> + >>> static int qoriq_tmu_probe(struct platform_device *pdev) { >>> int ret; >>> @@ -278,6 +286,10 @@ static int qoriq_tmu_probe(struct platform_device >> *pdev) >>> return ret; >>> } >>> >>> + ret = devm_add_action_or_reset(dev, qoriq_tmu_action, data); >>> + if (ret) >>> + return ret; >>> + >>> /* version register offset at: 0xbf8 on both v1 and v2 */ >>> ret = regmap_read(data->regmap, REGS_IPBRR(0), &ver); >>> if (ret) { >>> @@ -290,35 +302,17 @@ static int qoriq_tmu_probe(struct >>> platform_device *pdev) >>> >>> ret = qoriq_tmu_calibration(dev, data); /* TMU calibration */ >>> if (ret < 0) >>> - goto err; >>> + return ret; >>> >>> ret = qoriq_tmu_register_tmu_zone(dev, data); >>> if (ret < 0) { >>> dev_err(dev, "Failed to register sensors\n"); >>> - ret = -ENODEV; >>> - goto err; >>> + return ret; >>> } >>> >>> platform_set_drvdata(pdev, data); >>> >>> return 0; >>> - >>> -err: >>> - clk_disable_unprepare(data->clk); >>> - >>> - return ret; >>> -} >>> - >>> -static int qoriq_tmu_remove(struct platform_device *pdev) -{ >>> - struct qoriq_tmu_data *data = platform_get_drvdata(pdev); >>> - >>> - /* Disable monitoring */ >>> - regmap_write(data->regmap, REGS_TMR, TMR_DISABLE); >>> - >>> - clk_disable_unprepare(data->clk); >>> - >>> - return 0; >>> } >>> >>> static int __maybe_unused qoriq_tmu_suspend(struct device *dev) @@ >>> -365,7 +359,6 @@ static struct platform_driver qoriq_tmu = { >>> .of_match_table = qoriq_tmu_match, >>> }, >>> .probe = qoriq_tmu_probe, >>> - .remove = qoriq_tmu_remove, >>> }; >>> module_platform_driver(qoriq_tmu); >>> >>> >> >> >> -- >> >> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. >> linaro.org%2F&data=02%7C01%7CAnson.Huang%40nxp.com%7C37ea3 >> 145642b47b4df7208d7c677e57d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7 >> C0%7C0%7C637196090558448813&sdata=%2FCfH8GPxd57CDlau1pMcq >> LH7GjGIh%2Fu%2Bfq7teGsS8KM%3D&reserved=0> Linaro.org │ Open >> source software for ARM SoCs >> >> Follow Linaro: >> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. >> facebook.com%2Fpages%2FLinaro&data=02%7C01%7CAnson.Huang%4 >> 0nxp.com%7C37ea3145642b47b4df7208d7c677e57d%7C686ea1d3bc2b4c6fa >> 92cd99c5c301635%7C0%7C0%7C637196090558448813&sdata=KBUWpT >> 4quqo08r8YhbMVk%2Fyf2jIT1CKgc5i3jI9gCwo%3D&reserved=0> >> Facebook | >> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte >> r.com%2F%23!%2Flinaroorg&data=02%7C01%7CAnson.Huang%40nxp.c >> om%7C37ea3145642b47b4df7208d7c677e57d%7C686ea1d3bc2b4c6fa92cd9 >> 9c5c301635%7C0%7C0%7C637196090558448813&sdata=%2FEgeRMWd >> qbbaOMl2Tfb%2BtLmfttqN1WcFbl9%2B5tXtOic%3D&reserved=0> >> Twitter | >> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. >> linaro.org%2Flinaro- >> blog%2F&data=02%7C01%7CAnson.Huang%40nxp.com%7C37ea314564 >> 2b47b4df7208d7c677e57d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 >> C0%7C637196090558448813&sdata=SXZow%2F0B%2BIgpfOfAUIG1NsR >> mcHp868ve%2Ffkejg3ZDvE%3D&reserved=0> Blog >
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |