Hi Laurent,
Thanks again for the comments!
On Sun, Mar 22, 2015 at 10:26:39PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch. This looks good to me, except that there's one last
> bug I've spotted. Please see below.
>
> On Saturday 21 March 2015 00:05:04 Sakari Ailus wrote:
> > Add the ISP device to omap3 DT include file and add support to the driver to
> > use it.
> >
> > Also obtain information on the external entities and the ISP configuration
> > related to them through the Device Tree in addition to the platform data.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
> > ---
> > since v1:
> >
> > - Print endpoint name in debug message when parsing an endpoint.
> >
> > - Rename lane-polarity property as lane-polarities.
> >
> > - Print endpoint name with the interface if the interface is invalid.
> >
> > - Remove assignment to two variables at the same time.
> >
> > - Fix multiple sub-device support in isp_of_parse_nodes().
> >
> > - Put of_node properly in isp_of_parse_nodes() (requires Philipp Zabel's
> > patch "of: Decrement refcount of previous endpoint in
> > of_graph_get_next_endpoint".
> >
> > - Rename return value variable rval as ret to be consistent with the rest of
> > the driver.
> >
> > - Read the register offset from the syscom property's first argument.
> >
> > drivers/media/platform/omap3isp/isp.c | 218 ++++++++++++++++++++++--
> > drivers/media/platform/omap3isp/isp.h | 11 ++
> > drivers/media/platform/omap3isp/ispcsiphy.c | 7 +
> > 3 files changed, 224 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 992e74c..92a859e 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
>
> [snip]
>
> > +static int isp_of_parse_nodes(struct device *dev,
> > + struct v4l2_async_notifier *notifier)
> > +{
> > + struct device_node *node;
> > +
> > + notifier->subdevs = devm_kcalloc(
> > + dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
> > + if (!notifier->subdevs)
> > + return -ENOMEM;
> > +
> > + while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
> > + notifier->num_subdevs < ISP_MAX_SUBDEVS) {
>
> If the first condition evaluates to true and the second one to false, the loop
> will be exited without releasing the reference to the DT node. You could just
> switch the two conditions to fix this.
Oh, I missed this. I'll resend the set.
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
