On Sun, Feb 23, 2025 at 07:34:30PM +0100, Ricardo Ribalda wrote: > On Fri, 21 Feb 2025 at 10:18, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > > > On 21/02/2025 10:04, Hans Verkuil wrote: > > > Hi Ricardo, > > > > > > On 21/01/2025 22:14, Ricardo Ribalda wrote: > > >> When we obtain a reference to of a platform_device, we need to release > > >> it via put_device. > > >> > > >> Found by cocci: > > >> ./platform/nuvoton/npcm-video.c:1677:3-9: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function. > > >> ./platform/nuvoton/npcm-video.c:1684:3-9: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function. > > >> ./platform/nuvoton/npcm-video.c:1690:3-9: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function. > > >> ./platform/nuvoton/npcm-video.c:1694:1-7: ERROR: missing put_device; call of_find_device_by_node on line 1667, but without a corresponding object release within this function. > > > > > > This driver uses this construct: > > > > > > struct device *ece_dev __free(put_device) = &ece_pdev->dev; > > > > > > to automatically call put_device. So this patch would 'put' the device twice. > > > > > > Does cocci understand constructs like this? If I hadn't looked closely at the > > > code first, I would just have merged it. > > > > Oh wait, now that I am reading the following patches I see that it was those later > > patches that add the __free code. > > > > This is far too confusing. Please post a v2 that just combines the 'fix references' > > and 'use cleanup.h macros' in a single patch. It makes no sense to have this two-phase > > approach. > > I believe this is discouraged. > > cleanup.h macros does not exist in old kernel versions, so makes it > impossible to backport the fix to them. That's not a problem, fix things properly in the main tree and let the stable/lts kernels work it out on their own. > This is an example of other series following this policy: > https://lore.kernel.org/lkml/173608125422.1253657.3732758016133408588.stgit@devnote2/ > > They also mention the same here: > https://hackerbikepacker.com/kernel-auto-cleanup-1 .... I am pretty > sure that I read the policy in a more official location... but I > cannot find it right now :) No, it is NOT official policy at all. Otherwise you would be saying that no one could use these new functions for 6 years just because of really old kernels still living around somewhere. That's not how kernel development works, thankfully. thanks, greg k-h
