1. Patchew
  2. linux
  3. View series

[PATCH] media: nxp: imx8-isi: remove kzalloc_objs

Rosen Penev posted 1 patch 1 month ago
There is a newer version of this series
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
2 files changed, 6 insertions(+), 8 deletions(-)
[PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Rosen Penev 1 month ago 
Use a flexible arraay member to combine allocations.

It looks like pipes never gets freed anywhere. Meaning this effectively
fixes a memory leak.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
 drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 16392420903a..657ffecc4d7e 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -459,25 +459,21 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
 
 static int mxc_isi_probe(struct platform_device *pdev)
 {
+	const struct mxc_isi_plat_data *pdata;
 	struct device *dev = &pdev->dev;
 	struct mxc_isi_dev *isi;
 	unsigned int dma_size;
 	unsigned int i;
 	int ret = 0;
 
-	isi = devm_kzalloc(dev, sizeof(*isi), GFP_KERNEL);
+	pdata = of_device_get_match_data(dev);
+	isi = devm_kzalloc(dev, struct_size(isi, pipes, pdata->num_channels), GFP_KERNEL);
 	if (!isi)
 		return -ENOMEM;
 
 	isi->dev = dev;
 	platform_set_drvdata(pdev, isi);
 
-	isi->pdata = of_device_get_match_data(dev);
-
-	isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);
-	if (!isi->pipes)
-		return -ENOMEM;
-
 	isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
 	if (isi->num_clks < 0)
 		return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
@@ -487,6 +483,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(isi->regs),
 				     "Failed to get ISI register map\n");
 
+	isi->pdata = pdata;
 	if (isi->pdata->gasket_ops) {
 		isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
 							      "fsl,blk-ctrl");
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
index 3cbd35305af0..99532efa4e41 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -286,7 +286,6 @@ struct mxc_isi_dev {
 	struct regmap			*gasket;
 
 	struct mxc_isi_crossbar		crossbar;
-	struct mxc_isi_pipe		*pipes;
 	struct mxc_isi_m2m		m2m;
 
 	struct media_device		media_dev;
@@ -294,6 +293,8 @@ struct mxc_isi_dev {
 	struct v4l2_async_notifier	notifier;
 
 	struct dentry			*debugfs_root;
+
+	struct mxc_isi_pipe		pipes[];
 };
 
 extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
-- 
2.53.0
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Laurent Pinchart 2 weeks, 6 days ago 
On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:
> Use a flexible arraay member to combine allocations.
> 
> It looks like pipes never gets freed anywhere. Meaning this effectively
> fixes a memory leak.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
>  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index 16392420903a..657ffecc4d7e 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -459,25 +459,21 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
>  
>  static int mxc_isi_probe(struct platform_device *pdev)
>  {
> +	const struct mxc_isi_plat_data *pdata;
>  	struct device *dev = &pdev->dev;
>  	struct mxc_isi_dev *isi;
>  	unsigned int dma_size;
>  	unsigned int i;
>  	int ret = 0;
>  
> -	isi = devm_kzalloc(dev, sizeof(*isi), GFP_KERNEL);
> +	pdata = of_device_get_match_data(dev);
> +	isi = devm_kzalloc(dev, struct_size(isi, pipes, pdata->num_channels), GFP_KERNEL);
>  	if (!isi)
>  		return -ENOMEM;
>  
>  	isi->dev = dev;
>  	platform_set_drvdata(pdev, isi);
>  
> -	isi->pdata = of_device_get_match_data(dev);

I'd keep

	isi->pdata = pdata;

here and not below.

> -
> -	isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);

Wouldn't it be simpler to just replace this with devm_kcalloc() ?

	isi->pipes = devm_kcalloc(dev, isi->pdata->num_channels,
				  sizeof(isi->pipes[0]), GFP_KERNEL);

No other change would be needed.

> -	if (!isi->pipes)
> -		return -ENOMEM;
> -
>  	isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
>  	if (isi->num_clks < 0)
>  		return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> @@ -487,6 +483,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(isi->regs),
>  				     "Failed to get ISI register map\n");
>  
> +	isi->pdata = pdata;
>  	if (isi->pdata->gasket_ops) {
>  		isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
>  							      "fsl,blk-ctrl");
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> index 3cbd35305af0..99532efa4e41 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> @@ -286,7 +286,6 @@ struct mxc_isi_dev {
>  	struct regmap			*gasket;
>  
>  	struct mxc_isi_crossbar		crossbar;
> -	struct mxc_isi_pipe		*pipes;
>  	struct mxc_isi_m2m		m2m;
>  
>  	struct media_device		media_dev;
> @@ -294,6 +293,8 @@ struct mxc_isi_dev {
>  	struct v4l2_async_notifier	notifier;
>  
>  	struct dentry			*debugfs_root;
> +
> +	struct mxc_isi_pipe		pipes[];
>  };
>  
>  extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;

-- 
Regards,

Laurent Pinchart
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Rosen Penev 2 weeks, 6 days ago 
On Thu, Mar 19, 2026 at 3:09 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:
> > Use a flexible arraay member to combine allocations.
> >
> > It looks like pipes never gets freed anywhere. Meaning this effectively
> > fixes a memory leak.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
> >  2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > index 16392420903a..657ffecc4d7e 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -459,25 +459,21 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> >
> >  static int mxc_isi_probe(struct platform_device *pdev)
> >  {
> > +     const struct mxc_isi_plat_data *pdata;
> >       struct device *dev = &pdev->dev;
> >       struct mxc_isi_dev *isi;
> >       unsigned int dma_size;
> >       unsigned int i;
> >       int ret = 0;
> >
> > -     isi = devm_kzalloc(dev, sizeof(*isi), GFP_KERNEL);
> > +     pdata = of_device_get_match_data(dev);
> > +     isi = devm_kzalloc(dev, struct_size(isi, pipes, pdata->num_channels), GFP_KERNEL);
> >       if (!isi)
> >               return -ENOMEM;
> >
> >       isi->dev = dev;
> >       platform_set_drvdata(pdev, isi);
> >
> > -     isi->pdata = of_device_get_match_data(dev);
>
> I'd keep
>
>         isi->pdata = pdata;
>
> here and not below.
Sure
>
> > -
> > -     isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);
>
> Wouldn't it be simpler to just replace this with devm_kcalloc() ?
That would only make sense if pipes remained a pointer. A flex array
member allows combining allocations since it doesn't take up extra
space.

Technically kzalloc_flex is simpler but I assume a devm version of
that will be handled in another treewide commit.
>
>         isi->pipes = devm_kcalloc(dev, isi->pdata->num_channels,
>                                   sizeof(isi->pipes[0]), GFP_KERNEL);
>
> No other change would be needed.
With a flex array member, that's a compile time error, which I use as
a hint to complete the conversion.
>
> > -     if (!isi->pipes)
> > -             return -ENOMEM;
> > -
> >       isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> >       if (isi->num_clks < 0)
> >               return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > @@ -487,6 +483,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
> >               return dev_err_probe(dev, PTR_ERR(isi->regs),
> >                                    "Failed to get ISI register map\n");
> >
> > +     isi->pdata = pdata;
> >       if (isi->pdata->gasket_ops) {
> >               isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
> >                                                             "fsl,blk-ctrl");
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > index 3cbd35305af0..99532efa4e41 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > @@ -286,7 +286,6 @@ struct mxc_isi_dev {
> >       struct regmap                   *gasket;
> >
> >       struct mxc_isi_crossbar         crossbar;
> > -     struct mxc_isi_pipe             *pipes;
> >       struct mxc_isi_m2m              m2m;
> >
> >       struct media_device             media_dev;
> > @@ -294,6 +293,8 @@ struct mxc_isi_dev {
> >       struct v4l2_async_notifier      notifier;
> >
> >       struct dentry                   *debugfs_root;
> > +
> > +     struct mxc_isi_pipe             pipes[];
> >  };
> >
> >  extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
>
> --
> Regards,
>
> Laurent Pinchart
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Kees Cook 2 weeks, 5 days ago 
On Thu, Mar 19, 2026 at 05:11:22PM -0700, Rosen Penev wrote:
> Technically kzalloc_flex is simpler but I assume a devm version of
> that will be handled in another treewide commit.

Yup, that's coming. I wanted to get the "first pass" of the base kmalloc
family converted first. There are a few classes of "kmalloc wrappers"
like devm_* that I'm hoping to convert to type-based allocations. devm
is next. :)

-- 
Kees Cook
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Rosen Penev 2 weeks ago 
On Fri, Mar 20, 2026 at 11:15 AM Kees Cook <kees@kernel.org> wrote:
>
> On Thu, Mar 19, 2026 at 05:11:22PM -0700, Rosen Penev wrote:
> > Technically kzalloc_flex is simpler but I assume a devm version of
> > that will be handled in another treewide commit.
>
> Yup, that's coming. I wanted to get the "first pass" of the base kmalloc
> family converted first. There are a few classes of "kmalloc wrappers"
> like devm_* that I'm hoping to convert to type-based allocations. devm
> is next. :)
As an aside, I recently noticed a pattern of kzalloc + kmemdup_array.
As kzalloc_flex is basically, kzalloc + kcalloc, Would a kmemdup_flex
make sense?
>
> --
> Kees Cook
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Laurent Pinchart 2 weeks, 6 days ago 
On Thu, Mar 19, 2026 at 05:11:22PM -0700, Rosen Penev wrote:
> On Thu, Mar 19, 2026 at 3:09 PM Laurent Pinchart wrote:
> > On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:
> > > Use a flexible arraay member to combine allocations.
> > >
> > > It looks like pipes never gets freed anywhere. Meaning this effectively
> > > fixes a memory leak.
> > >
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
> > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
> > >  2 files changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > index 16392420903a..657ffecc4d7e 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > @@ -459,25 +459,21 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > >
> > >  static int mxc_isi_probe(struct platform_device *pdev)
> > >  {
> > > +     const struct mxc_isi_plat_data *pdata;
> > >       struct device *dev = &pdev->dev;
> > >       struct mxc_isi_dev *isi;
> > >       unsigned int dma_size;
> > >       unsigned int i;
> > >       int ret = 0;
> > >
> > > -     isi = devm_kzalloc(dev, sizeof(*isi), GFP_KERNEL);
> > > +     pdata = of_device_get_match_data(dev);
> > > +     isi = devm_kzalloc(dev, struct_size(isi, pipes, pdata->num_channels), GFP_KERNEL);
> > >       if (!isi)
> > >               return -ENOMEM;
> > >
> > >       isi->dev = dev;
> > >       platform_set_drvdata(pdev, isi);
> > >
> > > -     isi->pdata = of_device_get_match_data(dev);
> >
> > I'd keep
> >
> >         isi->pdata = pdata;
> >
> > here and not below.
> 
> Sure
> 
> > > -
> > > -     isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);
> >
> > Wouldn't it be simpler to just replace this with devm_kcalloc() ?
> 
> That would only make sense if pipes remained a pointer. A flex array
> member allows combining allocations since it doesn't take up extra
> space.

Yes, I understand that. My point was that with devm_kzalloc() here you
won't need to change the mxc_isi_dev structure. The leak fix will be a
one-liner patch.

> Technically kzalloc_flex is simpler but I assume a devm version of
> that will be handled in another treewide commit.
> >
> >         isi->pipes = devm_kcalloc(dev, isi->pdata->num_channels,
> >                                   sizeof(isi->pipes[0]), GFP_KERNEL);
> >
> > No other change would be needed.
> 
> With a flex array member, that's a compile time error, which I use as
> a hint to complete the conversion.
> 
> > > -     if (!isi->pipes)
> > > -             return -ENOMEM;
> > > -
> > >       isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > >       if (isi->num_clks < 0)
> > >               return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > > @@ -487,6 +483,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > >               return dev_err_probe(dev, PTR_ERR(isi->regs),
> > >                                    "Failed to get ISI register map\n");
> > >
> > > +     isi->pdata = pdata;
> > >       if (isi->pdata->gasket_ops) {
> > >               isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
> > >                                                             "fsl,blk-ctrl");
> > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > index 3cbd35305af0..99532efa4e41 100644
> > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > @@ -286,7 +286,6 @@ struct mxc_isi_dev {
> > >       struct regmap                   *gasket;
> > >
> > >       struct mxc_isi_crossbar         crossbar;
> > > -     struct mxc_isi_pipe             *pipes;
> > >       struct mxc_isi_m2m              m2m;
> > >
> > >       struct media_device             media_dev;
> > > @@ -294,6 +293,8 @@ struct mxc_isi_dev {
> > >       struct v4l2_async_notifier      notifier;
> > >
> > >       struct dentry                   *debugfs_root;
> > > +
> > > +     struct mxc_isi_pipe             pipes[];
> > >  };
> > >
> > >  extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;

-- 
Regards,

Laurent Pinchart
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Rosen Penev 2 weeks, 6 days ago 
On Thu, Mar 19, 2026 at 5:57 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Thu, Mar 19, 2026 at 05:11:22PM -0700, Rosen Penev wrote:
> > On Thu, Mar 19, 2026 at 3:09 PM Laurent Pinchart wrote:
> > > On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:
> > > > Use a flexible arraay member to combine allocations.
> > > >
> > > > It looks like pipes never gets freed anywhere. Meaning this effectively
> > > > fixes a memory leak.
> > > >
> > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > > ---
> > > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
> > > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
> > > >  2 files changed, 6 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > index 16392420903a..657ffecc4d7e 100644
> > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > @@ -459,25 +459,21 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > >
> > > >  static int mxc_isi_probe(struct platform_device *pdev)
> > > >  {
> > > > +     const struct mxc_isi_plat_data *pdata;
> > > >       struct device *dev = &pdev->dev;
> > > >       struct mxc_isi_dev *isi;
> > > >       unsigned int dma_size;
> > > >       unsigned int i;
> > > >       int ret = 0;
> > > >
> > > > -     isi = devm_kzalloc(dev, sizeof(*isi), GFP_KERNEL);
> > > > +     pdata = of_device_get_match_data(dev);
> > > > +     isi = devm_kzalloc(dev, struct_size(isi, pipes, pdata->num_channels), GFP_KERNEL);
> > > >       if (!isi)
> > > >               return -ENOMEM;
> > > >
> > > >       isi->dev = dev;
> > > >       platform_set_drvdata(pdev, isi);
> > > >
> > > > -     isi->pdata = of_device_get_match_data(dev);
> > >
> > > I'd keep
> > >
> > >         isi->pdata = pdata;
> > >
> > > here and not below.
> >
> > Sure
> >
> > > > -
> > > > -     isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);
> > >
> > > Wouldn't it be simpler to just replace this with devm_kcalloc() ?
> >
> > That would only make sense if pipes remained a pointer. A flex array
> > member allows combining allocations since it doesn't take up extra
> > space.
>
> Yes, I understand that. My point was that with devm_kzalloc() here you
> won't need to change the mxc_isi_dev structure. The leak fix will be a
> one-liner patch.
Leak fix is not the point of the patch to be fair.
>
> > Technically kzalloc_flex is simpler but I assume a devm version of
> > that will be handled in another treewide commit.
> > >
> > >         isi->pipes = devm_kcalloc(dev, isi->pdata->num_channels,
> > >                                   sizeof(isi->pipes[0]), GFP_KERNEL);
> > >
> > > No other change would be needed.
> >
> > With a flex array member, that's a compile time error, which I use as
> > a hint to complete the conversion.
> >
> > > > -     if (!isi->pipes)
> > > > -             return -ENOMEM;
> > > > -
> > > >       isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > > >       if (isi->num_clks < 0)
> > > >               return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > > > @@ -487,6 +483,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > >               return dev_err_probe(dev, PTR_ERR(isi->regs),
> > > >                                    "Failed to get ISI register map\n");
> > > >
> > > > +     isi->pdata = pdata;
> > > >       if (isi->pdata->gasket_ops) {
> > > >               isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
> > > >                                                             "fsl,blk-ctrl");
> > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > index 3cbd35305af0..99532efa4e41 100644
> > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > @@ -286,7 +286,6 @@ struct mxc_isi_dev {
> > > >       struct regmap                   *gasket;
> > > >
> > > >       struct mxc_isi_crossbar         crossbar;
> > > > -     struct mxc_isi_pipe             *pipes;
> > > >       struct mxc_isi_m2m              m2m;
> > > >
> > > >       struct media_device             media_dev;
> > > > @@ -294,6 +293,8 @@ struct mxc_isi_dev {
> > > >       struct v4l2_async_notifier      notifier;
> > > >
> > > >       struct dentry                   *debugfs_root;
> > > > +
> > > > +     struct mxc_isi_pipe             pipes[];
> > > >  };
> > > >
> > > >  extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;
>
> --
> Regards,
>
> Laurent Pinchart
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Laurent Pinchart 2 weeks, 6 days ago 
On Thu, Mar 19, 2026 at 06:06:07PM -0700, Rosen Penev wrote:
> On Thu, Mar 19, 2026 at 5:57 PM Laurent Pinchart wrote:
> > On Thu, Mar 19, 2026 at 05:11:22PM -0700, Rosen Penev wrote:
> > > On Thu, Mar 19, 2026 at 3:09 PM Laurent Pinchart wrote:
> > > > On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:
> > > > > Use a flexible arraay member to combine allocations.
> > > > >
> > > > > It looks like pipes never gets freed anywhere. Meaning this effectively
> > > > > fixes a memory leak.
> > > > >
> > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > > > ---
> > > > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c | 11 ++++-------
> > > > >  drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h |  3 ++-
> > > > >  2 files changed, 6 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > index 16392420903a..657ffecc4d7e 100644
> > > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > > > > @@ -459,25 +459,21 @@ static const struct dev_pm_ops mxc_isi_pm_ops = {
> > > > >
> > > > >  static int mxc_isi_probe(struct platform_device *pdev)
> > > > >  {
> > > > > +     const struct mxc_isi_plat_data *pdata;
> > > > >       struct device *dev = &pdev->dev;
> > > > >       struct mxc_isi_dev *isi;
> > > > >       unsigned int dma_size;
> > > > >       unsigned int i;
> > > > >       int ret = 0;
> > > > >
> > > > > -     isi = devm_kzalloc(dev, sizeof(*isi), GFP_KERNEL);
> > > > > +     pdata = of_device_get_match_data(dev);
> > > > > +     isi = devm_kzalloc(dev, struct_size(isi, pipes, pdata->num_channels), GFP_KERNEL);
> > > > >       if (!isi)
> > > > >               return -ENOMEM;
> > > > >
> > > > >       isi->dev = dev;
> > > > >       platform_set_drvdata(pdev, isi);
> > > > >
> > > > > -     isi->pdata = of_device_get_match_data(dev);
> > > >
> > > > I'd keep
> > > >
> > > >         isi->pdata = pdata;
> > > >
> > > > here and not below.
> > >
> > > Sure
> > >
> > > > > -
> > > > > -     isi->pipes = kzalloc_objs(isi->pipes[0], isi->pdata->num_channels);
> > > >
> > > > Wouldn't it be simpler to just replace this with devm_kcalloc() ?
> > >
> > > That would only make sense if pipes remained a pointer. A flex array
> > > member allows combining allocations since it doesn't take up extra
> > > space.
> >
> > Yes, I understand that. My point was that with devm_kzalloc() here you
> > won't need to change the mxc_isi_dev structure. The leak fix will be a
> > one-liner patch.
> 
> Leak fix is not the point of the patch to be fair.

If I understand correctly, your main point is to combine two
allocations.

I don't liek this much in this specific case. mxc_isi_dev is the
top-level driver structure. As part of all the data it stores, there
is a pointer to a dynamically-allocated array, and the current
implementation happens to not have any other dynamically allocated
member. That is however not an intrinsic property of the mxc_isi_dev
structure. Treating pipes as a flex arrat within mxc_isi_dev won't scale
to a second similar member, and there's no reason to treat pipes in a
special way. I prefer fixing the leak and avoiding the flex array.

> > > Technically kzalloc_flex is simpler but I assume a devm version of
> > > that will be handled in another treewide commit.
> > > >
> > > >         isi->pipes = devm_kcalloc(dev, isi->pdata->num_channels,
> > > >                                   sizeof(isi->pipes[0]), GFP_KERNEL);
> > > >
> > > > No other change would be needed.
> > >
> > > With a flex array member, that's a compile time error, which I use as
> > > a hint to complete the conversion.
> > >
> > > > > -     if (!isi->pipes)
> > > > > -             return -ENOMEM;
> > > > > -
> > > > >       isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
> > > > >       if (isi->num_clks < 0)
> > > > >               return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
> > > > > @@ -487,6 +483,7 @@ static int mxc_isi_probe(struct platform_device *pdev)
> > > > >               return dev_err_probe(dev, PTR_ERR(isi->regs),
> > > > >                                    "Failed to get ISI register map\n");
> > > > >
> > > > > +     isi->pdata = pdata;
> > > > >       if (isi->pdata->gasket_ops) {
> > > > >               isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
> > > > >                                                             "fsl,blk-ctrl");
> > > > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > > index 3cbd35305af0..99532efa4e41 100644
> > > > > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
> > > > > @@ -286,7 +286,6 @@ struct mxc_isi_dev {
> > > > >       struct regmap                   *gasket;
> > > > >
> > > > >       struct mxc_isi_crossbar         crossbar;
> > > > > -     struct mxc_isi_pipe             *pipes;
> > > > >       struct mxc_isi_m2m              m2m;
> > > > >
> > > > >       struct media_device             media_dev;
> > > > > @@ -294,6 +293,8 @@ struct mxc_isi_dev {
> > > > >       struct v4l2_async_notifier      notifier;
> > > > >
> > > > >       struct dentry                   *debugfs_root;
> > > > > +
> > > > > +     struct mxc_isi_pipe             pipes[];
> > > > >  };
> > > > >
> > > > >  extern const struct mxc_gasket_ops mxc_imx8_gasket_ops;

-- 
Regards,

Laurent Pinchart
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Marc Kleine-Budde 1 month ago 
On 05.03.2026 20:45:36, Rosen Penev wrote:
> Use a flexible arraay member to combine allocations.
>
> It looks like pipes never gets freed anywhere. Meaning this effectively
> fixes a memory leak.

It's a devm_kzalloc(), which means it's automatically free()ed after the
remove callback.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Rosen Penev 1 month ago 
On Thu, Mar 5, 2026 at 10:56 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 05.03.2026 20:45:36, Rosen Penev wrote:
> > Use a flexible arraay member to combine allocations.
> >
> > It looks like pipes never gets freed anywhere. Meaning this effectively
> > fixes a memory leak.
>
> It's a devm_kzalloc(), which means it's automatically free()ed after the
> remove callback.
Yes but pipes does not use devm.
>
> Marc
>
> --
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Marc Kleine-Budde 1 month ago 
On 05.03.2026 22:59:13, Rosen Penev wrote:
> On Thu, Mar 5, 2026 at 10:56 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> >
> > On 05.03.2026 20:45:36, Rosen Penev wrote:
> > > Use a flexible arraay member to combine allocations.
> > >
> > > It looks like pipes never gets freed anywhere. Meaning this effectively
> > > fixes a memory leak.
> >
> > It's a devm_kzalloc(), which means it's automatically free()ed after the
> > remove callback.
> Yes but pipes does not use devm.

Doh! Sorry, I should not review patches before first tee in the morning.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Frank Li 1 month ago 
On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:

Subjest: use flexible array to fix memory leak

> Use a flexible arraay member to combine allocations.

typo arraay
>
> It looks like pipes never gets freed anywhere. Meaning this effectively
> fixes a memory leak.

Remove "It looks like". The pipes never ...

Because fix memory leak, need fixes tag here.

Frank
>
Re: [PATCH] media: nxp: imx8-isi: remove kzalloc_objs
Posted by Rosen Penev 1 month ago 
On Thu, Mar 5, 2026 at 9:11 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Thu, Mar 05, 2026 at 08:45:36PM -0800, Rosen Penev wrote:
>
> Subjest: use flexible array to fix memory leak
>
> > Use a flexible arraay member to combine allocations.
>
> typo array
Will fix.
> >
> > It looks like pipes never gets freed anywhere. Meaning this effectively
> > fixes a memory leak.
>
> Remove "It looks like". The pipes never ...
>
> Because fix memory leak, need fixes tag here.
Since its inception. Lovely.

That had no struct_size AFAIK.
>
> Frank
> >

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.