Use devm_add_action_or_reset() to handle all cleanups of failure in
.probe and .remove, then .remove callback can be dropped.
Signed-off-by: Anson Huang <[email protected]>
---
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);
--
2.7.4
On Wed, Mar 11, 2020 at 10:44 AM Anson Huang <[email protected]> wrote:
>
> Use devm_add_action_or_reset() to handle all cleanups of failure in
> .probe and .remove, then .remove callback can be dropped.
Reviewed-by: Amit Kucheria <[email protected]>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> 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);
>
> --
> 2.7.4
>
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?
> Signed-off-by: Anson Huang <[email protected]>
> ---
> 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);
>
>
--
<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
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?
Thanks,
Anson
>
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > 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
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 <[email protected]>
>>> ---
>>> 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
Hi, Daniel
> Subject: Re: [PATCH 1/2] thermal: qoriq: Use devm_add_action_or_reset() to
> handle all cleanups
>
> 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.
I think so as per my memory, see similar patches as below:
commit 19ec11a2233d24a7811836fa735203aaccf95a23
Author: Bartosz Golaszewski <[email protected]>
Date: Thu Jul 11 10:29:35 2019 +0200
gpio: em: remove the gpiochip before removing the irq domain
In commit 8764c4ca5049 ("gpio: em: use the managed version of
gpiochip_add_data()") we implicitly altered the ordering of resource
freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
internally, we now can potentially use the irq_domain after it was
destroyed in the remove() callback (as devm resources are freed after
remove() has returned).
Use devm_add_action_or_reset() to keep the ordering right and entirely
kill the remove() callback in the driver.
commit d9aa5ca429ad30dde96e5966173d18004f16f312
Author: Alexandre Belloni <[email protected]>
Date: Fri Apr 19 10:25:01 2019 +0200
rtc: ds2404: simplify .probe and remove .remove
Use devm_add_action_or_reset to simplify .probe and remove .remove
Signed-off-by: Alexandre Belloni <[email protected]>
drivers/rtc/rtc-ds2404.c
Thanks,
Anson
Hi, Daniel
> Subject: RE: [PATCH 1/2] thermal: qoriq: Use devm_add_action_or_reset() to
> handle all cleanups
>
> Hi, Daniel
>
> > Subject: Re: [PATCH 1/2] thermal: qoriq: Use
> > devm_add_action_or_reset() to handle all cleanups
> >
> > 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.
>
> I think so as per my memory, see similar patches as below:
>
>
> commit 19ec11a2233d24a7811836fa735203aaccf95a23
> Author: Bartosz Golaszewski <[email protected]>
> Date: Thu Jul 11 10:29:35 2019 +0200
>
> gpio: em: remove the gpiochip before removing the irq domain
>
> In commit 8764c4ca5049 ("gpio: em: use the managed version of
> gpiochip_add_data()") we implicitly altered the ordering of resource
> freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
> internally, we now can potentially use the irq_domain after it was
> destroyed in the remove() callback (as devm resources are freed after
> remove() has returned).
>
> Use devm_add_action_or_reset() to keep the ordering right and entirely
> kill the remove() callback in the driver.
>
>
> commit d9aa5ca429ad30dde96e5966173d18004f16f312
> Author: Alexandre Belloni <[email protected]>
> Date: Fri Apr 19 10:25:01 2019 +0200
>
> rtc: ds2404: simplify .probe and remove .remove
>
> Use devm_add_action_or_reset to simplify .probe and remove .remove
>
> Signed-off-by: Alexandre Belloni <[email protected]>
>
> drivers/rtc/rtc-ds2404.c
Any further concern?
Thanks,
Anson
On 17/03/2020 02:14, Anson Huang wrote:
> Hi, Daniel
>
>> Subject: RE: [PATCH 1/2] thermal: qoriq: Use devm_add_action_or_reset() to
>> handle all cleanups
>>
[ ... ]
>>>>> 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.
>>
>> I think so as per my memory, see similar patches as below:
>>
>>
>> commit 19ec11a2233d24a7811836fa735203aaccf95a23
>> Author: Bartosz Golaszewski <[email protected]>
>> Date: Thu Jul 11 10:29:35 2019 +0200
>>
>> gpio: em: remove the gpiochip before removing the irq domain
>>
>> In commit 8764c4ca5049 ("gpio: em: use the managed version of
>> gpiochip_add_data()") we implicitly altered the ordering of resource
>> freeing: since gpiochip_remove() calls gpiochip_irqchip_remove()
>> internally, we now can potentially use the irq_domain after it was
>> destroyed in the remove() callback (as devm resources are freed after
>> remove() has returned).
>>
>> Use devm_add_action_or_reset() to keep the ordering right and entirely
>> kill the remove() callback in the driver.
>>
>>
>> commit d9aa5ca429ad30dde96e5966173d18004f16f312
>> Author: Alexandre Belloni <[email protected]>
>> Date: Fri Apr 19 10:25:01 2019 +0200
>>
>> rtc: ds2404: simplify .probe and remove .remove
>>
>> Use devm_add_action_or_reset to simplify .probe and remove .remove
>>
>> Signed-off-by: Alexandre Belloni <[email protected]>
>>
>> drivers/rtc/rtc-ds2404.c
>
> Any further concern?
No more concerns, I've applied the patches.
Thanks
-- Daniel
--
<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