On Thu, Aug 08, 2024 at 08:11:04AM +0200, Uwe Kleine-König wrote:
> Hello Greg,
>
> On Wed, Aug 07, 2024 at 04:59:03PM +0200, Greg Kroah-Hartman wrote:
> > 6.6-stable review patch. If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> >
> > [ Upstream commit f29ecd3748a28d0b52512afc81b3c13fd4a00c9b ]
> >
> > The .remove() callback for a platform driver returns an int which makes
> > many driver authors wrongly assume it's possible to do error handling by
> > returning an error code. However the value returned is ignored (apart
> > from emitting a warning) and this typically results in resource leaks.
> >
> > To improve here there is a quest to make the remove callback return
> > void. In the first step of this quest all drivers are converted to
> > .remove_new(), which already returns void. Eventually after all drivers
> > are converted, .remove_new() will be renamed to .remove().
> >
> > Trivially convert this driver from always returning zero in the remove
> > callback to the void returning variant.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > Acked-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > Stable-dep-of: e90c369cc2ff ("thermal/drivers/broadcom: Fix race between removal and clock disable")
> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> > ---
> > drivers/thermal/broadcom/bcm2835_thermal.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c
> > index 3acc9288b3105..5c1cebe075801 100644
> > --- a/drivers/thermal/broadcom/bcm2835_thermal.c
> > +++ b/drivers/thermal/broadcom/bcm2835_thermal.c
> > @@ -282,19 +282,17 @@ static int bcm2835_thermal_probe(struct platform_device *pdev)
> > return err;
> > }
> >
> > -static int bcm2835_thermal_remove(struct platform_device *pdev)
> > +static void bcm2835_thermal_remove(struct platform_device *pdev)
> > {
> > struct bcm2835_thermal_data *data = platform_get_drvdata(pdev);
> >
> > debugfs_remove_recursive(data->debugfsdir);
> > clk_disable_unprepare(data->clk);
> > -
> > - return 0;
> > }
> >
> > static struct platform_driver bcm2835_thermal_driver = {
> > .probe = bcm2835_thermal_probe,
> > - .remove = bcm2835_thermal_remove,
> > + .remove_new = bcm2835_thermal_remove,
> > .driver = {
> > .name = "bcm2835_thermal",
> > .of_match_table = bcm2835_thermal_of_match_table,
>
> While I'm confident this patch not to break anything (there are so many
> of this type in mainline now and none regressed), it should be also easy
> to port the follow up patches onto 6.6.x without this change.
>
> No strong opinion and maybe being able to cleanly cherry pick later
> changes is important enough to keep this conversion? Otherwise I'd tend
> to drop this patch.
Thanks for the review, I'll leave it in as it made it simpler for the
next commit.
thanks,
greg k-h
