linux-kernel - Re: [PATCH 0/3] Add flash/strobe support for ov9282

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]

Message-ID: <CAPY8ntB8i4OyUWAL8k899yUd5QsRifJXiOfWXKceGQ7TNZ4OUw@mail.gmail.com>
Date: Tue, 4 Mar 2025 17:46:34 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Richard Leitner <richard.leitner@...ux.dev>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] Add flash/strobe support for ov9282

Hi Richard

Thanks for the series.

On Mon, 3 Mar 2025 at 22:59, Richard Leitner <richard.leitner@...ux.dev> wrote:
>
> This series adds basic flash/strobe support for ov9282 sensors using
> their "hardware strobe output".
>
> Apart from en-/disabling the flash/strobe output, setting a timeout
> (duration of activated strobe per frame) is implemented. The calculation
> of this timeout is only interpolated from various measurements, as no
> documentation was found.

The bigger picture question is whether using these flash controls is
appropriate for controlling the strobe output on a sensor. That's a
question for others (mainly Sakari and Laurent).

V4L2_CID_FLASH_TIMEOUT feels wrong for setting the duration of the strobe pulse.
Whilst the description in the docs [1] is a little brief, you then
have the description of V4L2_FLASH_FAULT_TIMEOUT for
V4L2_CID_FLASH_FAULT
"The flash strobe was still on when the timeout set by the user ---
V4L2_CID_FLASH_TIMEOUT control --- has expired. Not all flash
controllers may set this in all such conditions."
which implies it is the hardware watchdog timeout to ensure the flash
LEDs don't burn out, not configuring the duration of the flash pulse.
Then again adp1653 adopts it as the flash duration.

Is there an expectation that V4L2_CID_FLASH_STROBE_SOURCE should also
be implemented, even if it is fixed to
V4L2_FLASH_STROBE_SOURCE_EXTERNAL?

The one saving grace with this sensor is that it has a global shutter,
so the strobe does correspond to the exposure period. With rolling
shutter sensors, the flash duration is typically two frames to cover
the exposure duration of all lines as the shutter rolls down.

  Dave

[1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-flash.html

> Further flash/strobe-related controls as well as a migration to v4l2-cci
> helpers will likely be implemented in future series.
>
> All register addresses/values are based on the OV9281 datasheet v1.53
> (january 2019). This series was tested using an ov9281 VisionComponents
> camera module.
>
> Signed-off-by: Richard Leitner <richard.leitner@...ux.dev>
> ---
> Richard Leitner (3):
>       media: i2c: ov9282: add output enable register definitions
>       media: i2c: ov9282: add led_mode v4l2 control
>       media: i2c: ov9282: add strobe_timeout v4l2 control
>
>  drivers/media/i2c/ov9282.c | 89 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 86 insertions(+), 3 deletions(-)
> ---
> base-commit: f41427b3bdee7d9845b13a80c0d03882212f4b20
> change-id: 20250303-ov9282-flash-strobe-ac6bd00c9de6
> prerequisite-change-id: 20250225-b4-ov9282-gain-ef1cdaba5bfd:v1
> prerequisite-patch-id: 86f2582378ff7095ab65ce4bb25a143eb639e840
> prerequisite-patch-id: b06eb6ec697aaf0b3155b4b2370f171d0d304ae2
> prerequisite-patch-id: b123047d71bfb9b93f743bbdd6893d5a98495801
>
> Best regards,
> --
> Richard Leitner <richard.leitner@...ux.dev>
>
>