Re: [PATCH 1/6] omap3isp: Don't rely on devm for memory resource management

[Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>]

Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Monday 20 Feb 2017 17:22:17 Sakari Ailus wrote:
...
> > @@ -516,9 +516,12 @@ int omap3isp_hist_init(struct isp_device *isp)
> >  	hist->event_type = V4L2_EVENT_OMAP3ISP_HIST;
> >
> >  	ret = omap3isp_stat_init(hist, "histogram", &hist_subdev_ops);
> > +
> > +err:
> >  	if (ret) {
> > -		if (hist->dma_ch)
> > +		if (!IS_ERR(hist->dma_ch))
>
> I think this change is wrong. dma_ch is initialize to NULL by kzalloc(). You
> will end up calling dma_release_channel() on a NULL channel if
> omap3isp_stat_init() fails and HIST_CONFIG_DMA is false. The check should be
>
> 	if (!IS_ERR_OR_NULL(hist->dma_ch))

Good catch! I'll fix that.

> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

Thanks!


--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx