Re: [PATCH 2/2] rcar-vin: group: use correct of_node

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Niklas,

On 25/04/17 15:30, Niklas Söderlund wrote:
> Hi Kieran,
> 
> Thanks for your patch.
> 
> On 2017-04-24 19:14:26 +0100, Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>>
>> 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+renesas@xxxxxxxxxxxxxxxx>
>> ---
>>  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?

Sure, no problem.


>>  
>>  		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?

Yes, squashing this in seems appropriate.

Regards

Kieran


>> -- 
>> 2.7.4
>>
> 



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux