Re: [PATCH] media: platform: s3c-camif: fix function prototype

[Sylwester Nawrocki <sylvester.nawrocki@gmail.com>]

On 05/22/2017 11:02 AM, Hans Verkuil wrote:
>> --- a/drivers/media/platform/s3c-camif/camif-regs.c
>> +++ b/drivers/media/platform/s3c-camif/camif-regs.c
>> @@ -58,7 +58,7 @@ void camif_hw_set_test_pattern(struct camif_dev *camif, unsigned int pattern)
>>   }
>>   
>>   void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
>> -			unsigned int cr, unsigned int cb)
>> +			unsigned int cb, unsigned int cr)
>>   {
>>   	static const struct v4l2_control colorfx[] = {
>>   		{ V4L2_COLORFX_NONE,		CIIMGEFF_FIN_BYPASS },
>
> This will also affect this line:
> 
> cfg |= cr | (cb << 13);
> 
> cr and cb are now swapped so this will result in a different color.
> 
> Sylwester, who is wrong here: the prototype or how this function is called?
> 
> I suspect that Gustavo is right and that the prototype is wrong. But in that
> case this patch should also change the cfg assignment.

The function is currently called in a wrong way, it's clear from looking
at the prototype. CR should end up in bits 0:7 and CR in bits 20:13 of
the register. So yes, colour will change after applying the patch - to the
expected one, matching the user API documentation.

Unfortunately I can't test it because I have only the s3c2440 SoC based 
evaluation board where the image effect is not supported.

Probably a more straightforward fix would be to amend the callers (apologies
Gustavo for misleading suggestions).  But I'm inclined to apply the $subject 
patch as is to just close this bug report case.

--
Regards,
Sylwester