Re: [PATCH 6/7] media: au0828 change to use Managed Media Controller API

[Shuah Khan <shuahkh@osg.samsung.com>]

On 07/20/2015 01:01 PM, Dan Carpenter wrote:
> On Mon, Jul 20, 2015 at 09:55:50AM -0600, Shuah Khan wrote:
>>>>  #ifdef CONFIG_MEDIA_CONTROLLER
>>>> -	if (dev->media_dev) {
>>>> -		media_device_unregister(dev->media_dev);
>>>> -		kfree(dev->media_dev);
>>>> -		dev->media_dev = NULL;
>>>> +	if (dev->media_dev &&
>>>> +		media_devnode_is_registered(&dev->media_dev->devnode)) {
>>>> +			media_device_unregister_entity_notify(dev->media_dev,
>>>> +							&dev->entity_notify);
>>>> +			media_device_unregister(dev->media_dev);
>>>> +			dev->media_dev = NULL;
>>>
>>>
>>> The indenting is slightly off here.  It should be:
>>>
>>> 	if (dev->media_dev &&
>>> 	    media_devnode_is_registered(&dev->media_dev->devnode)) {
>>> 		media_device_unregister_entity_notify(dev->media_dev,
>>> 						      &dev->entity_notify);
>>> 		media_device_unregister(dev->media_dev);
>>> 		dev->media_dev = NULL;
>>> 	}
>>>
>>> Aligning if statements using spaces like that is nicer and checkpatch.pl
>>> won't complain.
>>>
>>
>> Yeah. I try to do that whenever. In this case, if I align, the line
>> becomes longer than 80 chars adding more things to worry about. In
>> such cases, I tend to not worry about aligning.
> 
> The main thing is that the body of the if statement is intended 16
> spaces instead of 8.  Otherwise I wouldn't have commented.
> 

Yes. I see what you are saying. I will fix it in v2 series.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978