Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Mixing goto-based error handling and scope-based cleanup (__free) in the same function. --
commit 6b5f2b27db1f7295943bada498301e27ac1e7653 Author: Bartosz Golaszewski <[email protected]> net: bcmgenet: use platform_device_set_of_node() This commit encapsulates the assignment of the OF node for dynamically allocated platform devices with the provided helper. This is done ahead of reworking the reference counting logic for platform devices. > diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c > b/drivers/net/ethernet/broadcom/genet/bcmmii.c > index a4e0d5a682687..451b23a039ceb 100644 > --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c > +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c [ ... ] > @@ -490,10 +490,14 @@ static int bcmgenet_mii_register(struct bcmgenet_priv > *priv) > /* Retain this platform_device pointer for later cleanup */ > priv->mii_pdev = ppdev; > ppdev->dev.parent = &pdev->dev; > - if (dn) > - ppdev->dev.of_node = bcmgenet_mii_of_find_mdio(priv); > - else > + > + if (dn) { > + struct device_node *np __free(device_node) = > bcmgenet_mii_of_find_mdio(priv); [Severity: Low] Does introducing a __free() variable in a function that already relies on goto-based error handling conflict with the kernel's cleanup design guidelines? Looking at bcmgenet_mii_register(), the function uses goto out for its error paths. According to include/linux/cleanup.h, the expectation is that usage of goto and cleanup helpers is never mixed in the same function. It is generally recommended to either convert all resources that need a goto cleanup to scope-based cleanup, or convert none of them, to avoid creating confusing ownership semantics. > + > + platform_device_set_of_node(ppdev, np); > + } else { > ppd.phy_mask = ~0; > + } > > ret = platform_device_add_resources(ppdev, &res, 1); > if (ret) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
