linux-kernel - Re: [PATCH v3 1/3] media: i2c: ov5640: Enable data pins on poweron for DVP mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 4 Sep 2020 12:44:14 +0200
From:   Jacopo Mondi <jacopo@...ndi.org>
To:     Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc:     Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Steve Longerbeam <slongerbeam@...il.com>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Paul <paul.kocialkowski@...tlin.com>,
        Hugues Fruchet <hugues.fruchet@...com>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org,
        Prabhakar <prabhakar.csengg@...il.com>
Subject: Re: [PATCH v3 1/3] media: i2c: ov5640: Enable data pins on poweron
 for DVP mode

Hi Prabhakar, Sakari,

On Wed, Sep 02, 2020 at 01:10:53AM +0300, Sakari Ailus wrote:
> Hi Prabhakar,
>
> My apologies for the late reply.
>
> On Thu, Aug 13, 2020 at 06:13:35PM +0100, Lad Prabhakar wrote:
> > During testing this sensor on iW-RainboW-G21D-Qseven platform in 8-bit DVP
> > mode with rcar-vin bridge noticed the capture worked fine for the first run
> > (with yavta), but for subsequent runs the bridge driver waited for the
> > frame to be captured. Debugging further noticed the data lines were
> > enabled/disabled in stream on/off callback and dumping the register
> > contents 0x3017/0x3018 in ov5640_set_stream_dvp() reported the correct
> > values, but yet frame capturing failed.
> >
> > To get around this issue the following actions are performed for
> > parallel mode (DVP):
> > 1: Keeps the sensor in software power down mode and is woken up only in
> >    ov5640_set_stream_dvp() callback.
>
> I'd suppose with s_power, the main driver would power the device off
> when it's not streaming.
>
> > 2: Enables data lines in s_power callback
> > 3: Configures HVP lines in s_power callback instead of configuring
> >    everytime in ov5640_set_stream_dvp().
> > 4: Disables MIPI interface.
>
> Could you split this into two (or even more) patches so that the first
> refactors the receiver setup and another one changes how it actually works?
> That way this would be quite a bit easier to review.
>
> While some of the above seem entirely reasonable, the changes are vast and
> testing should be done on different boards to make sure things won't break.
> That said, this depends on others who have the hardware.

I left it as a comment during review of v2, but now more formally:

For CSI-2 capture operations:
Tested-by: Jacopo Mondi <jacopo@...ndi.org>

Thanks
  j

>
> --
> Kind regards,
>
> Sakari Ailus