Re: [PATCH] drm/vc4: remove all usages of of_node_put()

[Amer Al Shanawany <amer.shanawany@xxxxxxxxx>]

On 18/06/2024 13.54, Amer Al Shanawany wrote:
> On 5/22/24 16:48, Amer Al Shanawany wrote:
> > On 27/04/2024 15.40, Amer Al Shanawany wrote:
> > > Use the scope-based cleanup feature to clean up 'struct device_node *'
> > > when they go out of scope, and remove all instances of of_node_put()
> > > within the same scope, to simplify function exit and avoid potential
> > > memory leaks.
> > >
> > > Suggested-by: Julia Lawall <julia.lawall@xxxxxxxx>
> > > Signed-off-by: Amer Al Shanawany <amer.shanawany@xxxxxxxxx>
> > > ---
> > >    drivers/gpu/drm/vc4/vc4_drv.c  | 14 ++++++--------
> > >    drivers/gpu/drm/vc4/vc4_hdmi.c |  4 +---
> > >    drivers/gpu/drm/vc4/vc4_hvs.c  |  4 +---
> > >    3 files changed, 8 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> > > index c133e96b8aca..bb0bb052e595 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_drv.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> > > @@ -289,7 +289,6 @@ static int vc4_drm_bind(struct device *dev)
> > >        struct rpi_firmware *firmware = NULL;
> > >        struct drm_device *drm;
> > >        struct vc4_dev *vc4;
> > > -    struct device_node *node;
> > >        struct drm_crtc *crtc;
> > >        bool is_vc5;
> > >        int ret = 0;
> > > @@ -302,11 +301,10 @@ static int vc4_drm_bind(struct device *dev)
> > >        else
> > >            driver = &vc4_drm_driver;
> > >    -    node = of_find_matching_node_and_match(NULL, vc4_dma_range_matches,
> > > -                           NULL);
> > > +    struct device_node *node __free(device_node) =
> > > +        of_find_matching_node_and_match(NULL, vc4_dma_range_matches, NULL);
> > >        if (node) {
> > >            ret = of_dma_configure(dev, node, true);
> > > -        of_node_put(node);
> > >              if (ret)
> > >                return ret;
> > > @@ -341,10 +339,10 @@ static int vc4_drm_bind(struct device *dev)
> > >                goto err;
> > >        }
> > >    -    node = of_find_compatible_node(NULL, NULL, "raspberrypi,bcm2835-firmware");
> > > -    if (node) {
> > > -        firmware = rpi_firmware_get(node);
> > > -        of_node_put(node);
> > > +    struct device_node *np __free(device_node) =
> > > +        of_find_compatible_node(NULL, NULL, "raspberrypi,bcm2835-firmware");
> > > +    if (np) {
> > > +        firmware = rpi_firmware_get(np);
> > >              if (!firmware) {
> > >                ret = -EPROBE_DEFER;
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index d30f8e8e8967..915e8da3f41a 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -3661,7 +3661,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> > >        struct drm_device *drm = dev_get_drvdata(master);
> > >        struct vc4_hdmi *vc4_hdmi;
> > >        struct drm_encoder *encoder;
> > > -    struct device_node *ddc_node;
> > >        int ret;
> > >          vc4_hdmi = drmm_kzalloc(drm, sizeof(*vc4_hdmi), GFP_KERNEL);
> > > @@ -3699,14 +3698,13 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> > >        if (ret)
> > >            return ret;
> > >    -    ddc_node = of_parse_phandle(dev->of_node, "ddc", 0);
> > > +    struct device_node *ddc_node __free(device_node) = of_parse_phandle(dev->of_node, "ddc", 0);
> > >        if (!ddc_node) {
> > >            DRM_ERROR("Failed to find ddc node in device tree\n");
> > >            return -ENODEV;
> > >        }
> > >          vc4_hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
> > > -    of_node_put(ddc_node);
> > >        if (!vc4_hdmi->ddc) {
> > >            DRM_DEBUG("Failed to get ddc i2c adapter by node\n");
> > >            return -EPROBE_DEFER;
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> > > index 04af672caacb..6e3613e06e09 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> > > @@ -845,15 +845,13 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
> > >          if (vc4->is_vc5) {
> > >            struct rpi_firmware *firmware;
> > > -        struct device_node *node;
> > > +        struct device_node *node __free(device_node) = rpi_firmware_find_node();
> > >            unsigned int max_rate;
> > >    -        node = rpi_firmware_find_node();
> > >            if (!node)
> > >                return -EINVAL;
> > >              firmware = rpi_firmware_get(node);
> > > -        of_node_put(node);
> > >            if (!firmware)
> > >                return -EPROBE_DEFER;
> > >    
> > Hi,
> >
> > This patch is marked as new/archived on patchwork[1], however it didn't receive any feedback, and a similar patch has been already merged [2].
> >
> >
> > [1]: https://patchwork.kernel.org/project/dri-devel/patch/20240427134044.38910-1-amer.shanawany@xxxxxxxxx/
> >
> > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d65bfb9546eb627e3c578336355c5b81797f2255
> >
> > Thank you
> >
> > Amer
> Hello,
>
> Could anyone kindly provide some feedback on this patch?
>
> Thank you
>
> Amer
Hello all,

I have come to realize that unsubscribing from the mailing list shortly 
after sending this patch prevented me from receiving any feedback. My 
apologies for this oversight.

Thank you

Amer