Hi Kieran,

Thanks for your patch.

On 2017-04-24 19:14:26 +0100, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
> 
> The unbind function dereferences the subdev->dev node to obtain the
> of_node. In error paths, the subdev->dev can be set to NULL, whilst the
> correct reference to the of_node is available as subdev->of_node.
> 
> Correct the dereferencing, and move the variable outside of the loop as
> it is constant against the subdev, and not initialised per CSI, for both
> the bind and unbind functions
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 48557628e76d..a530dc388b95 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -469,7 +469,7 @@ static int rvin_digital_notify_bound(struct 
> v4l2_async_notifier *notifier,
>  
>       v4l2_set_subdev_hostdata(subdev, vin);
>  
> -     if (vin->digital.asd.match.of.node == subdev->dev->of_node) {
> +     if (vin->digital.asd.match.of.node == subdev->of_node) {
>               /* Find surce and sink pad of remote subdevice */

This code is already present in upstream. Could you break this out in a 
separate patch and resubmit it?

>  
>               ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
> @@ -738,12 +738,11 @@ static void rvin_group_notify_unbind(struct 
> v4l2_async_notifier *notifier,
>                                    struct v4l2_async_subdev *asd)
>  {
>       struct rvin_dev *vin = notifier_to_vin(notifier);
> +     struct device_node *del = subdev->of_node;
>       unsigned int i;
>  
>       mutex_lock(&vin->group->lock);
>       for (i = 0; i < RVIN_CSI_MAX; i++) {
> -             struct device_node *del = subdev->dev->of_node;
> -
>               if (vin->group->bridge[i].asd.match.of.node == del) {
>                       vin_dbg(vin, "Unbind bridge %s\n", subdev->name);
>                       vin->group->bridge[i].subdev = NULL;
> @@ -768,13 +767,13 @@ static int rvin_group_notify_bound(struct 
> v4l2_async_notifier *notifier,
>                                  struct v4l2_async_subdev *asd)
>  {
>       struct rvin_dev *vin = notifier_to_vin(notifier);
> +     struct device_node *new = subdev->of_node;
>       unsigned int i;
>  
>       v4l2_set_subdev_hostdata(subdev, vin);
>  
>       mutex_lock(&vin->group->lock);
>       for (i = 0; i < RVIN_CSI_MAX; i++) {
> -             struct device_node *new = subdev->dev->of_node;
>  
>               if (vin->group->bridge[i].asd.match.of.node == new) {
>                       vin_dbg(vin, "Bound bridge %s\n", subdev->name);

And I will squash these fixes in to the next version of my 'Gen3 with 
media controller support' series since that is not yet picked up. Is 
that OK with you?

> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Reply via email to