On Tue, Feb 04, 2025 at 09:07:05AM +0000, Biju Das wrote:
> Hi Geert,
>
> Thanks for the feedback.
>
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Geert Uytterhoeven
> > Sent: 03 February 2025 11:06
> > Subject: Re: [PATCH] drm/tegra: rgb: Simplify tegra_dc_rgb_probe()
> >
> > Hi Biju,
> >
> > Thanks for your patch!
> >
> > On Sat, 1 Feb 2025 at 11:57, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > Simplify tegra_dc_rgb_probe() by using of_get_available_child_by_name().
> >
> > That's not the only thing this patch does...
> >
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > > --- a/drivers/gpu/drm/tegra/rgb.c
> > > +++ b/drivers/gpu/drm/tegra/rgb.c
> > > @@ -202,12 +202,12 @@ static const struct drm_encoder_helper_funcs
> > > tegra_rgb_encoder_helper_funcs = {
> > >
> > > int tegra_dc_rgb_probe(struct tegra_dc *dc) {
> > > - struct device_node *np;
> > > + struct device_node *np _free(device_node) =
> >
> > Adding the _free()...
>
> Yes it fixes a memory leak aswell.
>
> >
> > > + of_get_available_child_by_name(dc->dev->of_node,
> > > + "rgb");
> > > struct tegra_rgb *rgb;
> > > int err;
> > >
> > > - np = of_get_child_by_name(dc->dev->of_node, "rgb");
> > > - if (!np || !of_device_is_available(np))
> > > + if (!np)
> > > return -ENODEV;
> >
> > ... fixes the reference count in case of an unavailable node...
> >
> > >
> > > rgb = devm_kzalloc(dc->dev, sizeof(*rgb), GFP_KERNEL);
> >
> > ... but as np is stored below, it must not be freed when it goes out of context?
>
> OK, But it is used in tegra_output_probe() and never freed.
> Maybe remove should free it??
It's not quite as simple as that. tegra_output_probe() can also store
output->dev->of_node in output->of_node, so we'd also need to track a
flag of some sort to denote that this needs to be freed.
Ultimately I'm not sure if it's really worth it. Do we really expect
these nodes to ever be freed (in which case this might leak memory)?
These are built-in devices and there's no code anywhere to remove any
such nodes.
Thierry
Attachment:
signature.asc
Description: PGP signature
