Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191011103902.3145-1-wuxy@bitland.com.cn>
In-Reply-To: <20191011103902.3145-1-wuxy@bitland.com.cn>
From:   Tomasz Figa <tfiga@google.com>
Date:   Thu, 17 Oct 2019 19:40:08 +0900
Message-ID: <CAAFQd5Btb22tdJD74bmwYqaCYukeoX=aRUdoobpBdXX1W67LdQ@mail.gmail.com>
Subject: Re: [PATCH] media: ov5695: enable vsync pin output
To:     wuxy@bitland.com.cn
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Media Mailing List <linux-media@vger.kernel.org>,
        dongchun.zhu@mediatek.corp-partner.google.com,
        Nicolas Boichat <drinkcat@google.com>,
        wuxy <wuxy@bitland.corp-partner.google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Xingyu,

On Fri, Oct 11, 2019 at 7:39 PM <wuxy@bitland.com.cn> wrote:
>
> From: wuxy <wuxy@bitland.corp-partner.google.com>
>

Thanks for the patch! Please see my comments inline.

> For Kukui project, the ov5695 vsync signal needs to
> be set to output,from ov5695 datasheet,the related
> register control methods as follows:
>
> 0x3002 Bit[7] FISIN/VSYNC output enable
> 0x3010 Bit[7] enable FISIN/VSYNC as GPIO controlled by register
> 0x3008 Bit[7] register control FISIN/VSYNC output
>
> TEST= boot to shell
>
> Signed-off-by: Xingyu Wu <wuxy@bitland.corp-partner.google.com>
> ---
>  drivers/media/i2c/ov5695.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
> index 34b7046d9702..71f0eae6037b 100644
> --- a/drivers/media/i2c/ov5695.c
> +++ b/drivers/media/i2c/ov5695.c
> @@ -300,6 +300,9 @@ static const struct regval ov5695_global_regs[] = {
>   * mipi_datarate per lane 840Mbps
>   */
>  static const struct regval ov5695_2592x1944_regs[] = {
> +       {0x3002, 0x80},

The original value of 0xa1 that was in ov5695_global_regs[], has the
0x80 bit set already.

> +       {0x3008, 0x80},
> +       {0x3010, 0x80},

Doesn't this configure the pin to an always-1 output GPIO? I believe
the correct settings for both bits should be 0 and 0 for the pin to be
driven by the hardware vsync generator.

Best regards,
Tomasz