Hi Tomi,
On Mon, Jun 07, 2021 at 10:44:17AM +0300, Tomi Valkeinen wrote:
> On 04/06/2021 16:47, Laurent Pinchart wrote:
> > On Mon, May 24, 2021 at 02:08:52PM +0300, Tomi Valkeinen wrote:
> >> cal_async_notifier_complete() doesn't handle errors returned from
> >> cal_ctx_v4l2_register(). Add the error handling.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> >> ---
> >> drivers/media/platform/ti-vpe/cal.c | 8 ++++++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> >> index ba8821a3b262..9e051c2e84a9 100644
> >> --- a/drivers/media/platform/ti-vpe/cal.c
> >> +++ b/drivers/media/platform/ti-vpe/cal.c
> >> @@ -743,8 +743,12 @@ static int cal_async_notifier_complete(struct v4l2_async_notifier *notifier)
> >> int ret = 0;
> >>
> >> for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
> >> - if (cal->ctx[i])
> >> - cal_ctx_v4l2_register(cal->ctx[i]);
> >> + if (!cal->ctx[i])
> >> + continue;
> >> +
> >> + ret = cal_ctx_v4l2_register(cal->ctx[i]);
> >> + if (ret)
> >> + return ret;
> >
> > This part looks good, so
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >
> > Don't we need to call cal_ctx_v4l2_unregister() in the error path of
> > cal_async_notifier_register() though ?
>
> Hmm, can you elaborate? I don't understand where and why we need to call
> the unregister.
cal_async_notifier_complete() can be called synchronously by
v4l2_async_notifier_register() if all async subdevs are present. If
cal_ctx_v4l2_register() fails for one contexts, the failure will be
propagated to cal_async_notifier_register(), then to
cal_media_register(), and cal_probe(). Unless I'm mistaken, the contexts
for which cal_ctx_v4l2_register() succeeded will not be cleaned
properly.
--
Regards,
Laurent Pinchart
