Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751292AbdFAVhS (ORCPT ); Thu, 1 Jun 2017 17:37:18 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:35110 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751050AbdFAVhQ (ORCPT ); Thu, 1 Jun 2017 17:37:16 -0400 Subject: Re: [PATCH] media: platform: s3c-camif: fix function prototype To: Hans Verkuil , "Gustavo A. R. Silva" Cc: Mauro Carvalho Chehab , linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Sylwester Nawrocki References: <20170504214200.GA22855@embeddedgus> From: Sylwester Nawrocki Message-ID: Date: Thu, 1 Jun 2017 23:37:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1525 Lines: 39 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