Hi Sylwester,

On Thursday 25 October 2012 11:06:56 Sylwester Nawrocki wrote:
> Make sure there is no v4l2_device_unregister_subdev() call
> on a subdev which wasn't registered.

I'm not implying that this fix is bad, but doesn't the V4L2 core already 
handle this ? v4l2_device_unregister_subdev() returns immediately without 
doing anything if the subdev hasn't been registered.

> Signed-off-by: Sylwester Nawrocki <s.nawro...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> ---
>  drivers/media/platform/s5p-fimc/fimc-mdevice.c |   25  ++++++++++----------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.c
> b/drivers/media/platform/s5p-fimc/fimc-mdevice.c index 715b258..a69f053
> 100644
> --- a/drivers/media/platform/s5p-fimc/fimc-mdevice.c
> +++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.c
> @@ -345,24 +345,23 @@ static int fimc_register_callback(struct device *dev,
> void *p) struct fimc_dev *fimc = dev_get_drvdata(dev);
>       struct v4l2_subdev *sd = &fimc->vid_cap.subdev;
>       struct fimc_md *fmd = p;
> -     int ret = 0;
> +     int ret;
> 
> -     if (!fimc || !fimc->pdev)
> +     if (fimc == NULL)
>               return 0;
> 
> -     if (fimc->pdev->id < 0 || fimc->pdev->id >= FIMC_MAX_DEVS)
> +     if (fimc->id >= FIMC_MAX_DEVS)
>               return 0;
> 
>       fimc->pipeline_ops = &fimc_pipeline_ops;
> -     fmd->fimc[fimc->pdev->id] = fimc;
>       sd->grp_id = FIMC_GROUP_ID;
> 
>       ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
> -     if (ret) {
> +     if (!ret)
> +             fmd->fimc[fimc->id] = fimc;
> +     else
>               v4l2_err(&fmd->v4l2_dev, "Failed to register FIMC.%d (%d)\n",
>                        fimc->id, ret);
> -     }
> -
>       return ret;
>  }
> 
> @@ -380,15 +379,15 @@ static int fimc_lite_register_callback(struct device
> *dev, void *p) return 0;
> 
>       fimc->pipeline_ops = &fimc_pipeline_ops;
> -     fmd->fimc_lite[fimc->index] = fimc;
>       sd->grp_id = FLITE_GROUP_ID;
> 
>       ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
> -     if (ret) {
> +     if (!ret)
> +             fmd->fimc_lite[fimc->index] = fimc;
> +     else
>               v4l2_err(&fmd->v4l2_dev,
>                        "Failed to register FIMC-LITE.%d (%d)\n",
>                        fimc->index, ret);
> -     }
>       return ret;
>  }
> 
> @@ -407,10 +406,12 @@ static int csis_register_callback(struct device *dev,
> void *p) v4l2_info(sd, "csis%d sd: %s\n", pdev->id, sd->name);
> 
>       id = pdev->id < 0 ? 0 : pdev->id;
> -     fmd->csis[id].sd = sd;
> +
>       sd->grp_id = CSIS_GROUP_ID;
>       ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
> -     if (ret)
> +     if (!ret)
> +             fmd->csis[id].sd = sd;
> +     else
>               v4l2_err(&fmd->v4l2_dev,
>                        "Failed to register CSIS subdevice: %d\n", ret);
>       return ret;
-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to