Re: [PATCH] rcar-vin: Use of_nodes as specified by the subdev

[Simon Horman <horms@verge.net.au>]

On Wed, Apr 26, 2017 at 08:48:25AM +0100, Kieran Bingham wrote:
> Hi Simon,
> 
> On 26/04/17 08:23, Simon Horman wrote:
> > Hi Kieran,
> > 
> > On Tue, Apr 25, 2017 at 03:55:00PM +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >>
> >> The rvin_digital_notify_bound() call dereferences the subdev->dev
> >> pointer to obtain the of_node. On some error paths, this dev node can be
> >> set as NULL. The of_node is mapped into the subdevice structure on
> >> initialisation, so this is a safer source to compare the nodes.
> >>
> >> Dereference the of_node from the subdev structure instead of the dev
> >> structure.
> >>
> >> Fixes: 83fba2c06f19 ("rcar-vin: rework how subdevice is found and
> >> 	bound")
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> ---
> >>  drivers/media/platform/rcar-vin/rcar-core.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> >> index 5861ab281150..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 */
> >>  
> >>  		ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
> > 
> > I see two different accesses to subdev->dev->of_node in the version of
> > rcar-core.c in linux-next. So I'm unsure if the following comment makes
> > sense in the context of the version you are working on. It is that
> > I wonder if all accesses to subdev->dev->of_node should be updated.
> 
> Yes, all uses in rcar-core should be updated, This patch is targeted directly at
> mainline, in which only one reference occurs.
> 
> I presume(?) the references in linux-next, relate to the previous version of
> this patch which I posted as a reply to Niklas' patch series:
>  "[PATCH v3 00/27] rcar-vin: Add Gen3 with media controller support"
> 
> The first version of this patch (which was titled differently) covered three
> uses, but two of them were not yet in mainline.
> 
> The 'fixes' for those references are going to be squashed in to Niklas' next
> version of his patchset.

Understood, sounds good to me.