Hi, On 1/23/23 19:18, Andy Shevchenko wrote: > On Mon, Jan 23, 2023 at 06:46:44PM +0100, Hans de Goede wrote: >> On 1/23/23 18:10, Andy Shevchenko wrote: >>> acpi_dev_get_first_match_dev() gets ACPI device with the bumped >>> refcount. The caller must drop it when it's done. >>> >>> Fix ACPI device refcounting in apple_gmux_backlight_present(). > > ... > >> Thank you for your work on this, much appreciated and I like >> the new acpi_get_first_match_physical_node(). >> >> But I don't think this patch is a good idea. There is a >> regression related to apple_gmux_backlight_present() >> with a patch-set fixing it pending. >> >> And that patch-set actually removes this function. Adding >> a fix for this real, but not really important leak now, >> will just make backporting the actual fix harder. >> >> So I would prefer for this patch to not go in and to >> go for (a to be submitted v2) of the patch-set fixing >> the regression right away instead. > > Maybe I missed something, but I noticed that you actually moved (not killed) > the code which is currently in this function. If it's the case, I prefer my > fix to be imported first. The code is not really moved, patch 2/3 of my patch-set factors out the detection code from drivers/platform/x86/apple-gmux.c's probe function. The new factored out code uses a similar construct as the apple_gmux_backlight_present() code (including the same leak). Then patch 3/3 drops apple_gmux_backlight_present() and calls the new factored out probe code. I'll fix the leak in v2 and then add the 3 patches to pdx86/fixes for the next pull-req to Linus (thus also fixing the leak). Regards, Hans
