Re: [PATCH v3] media: v4l2-flash: Enter LED off state after file handle closed

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi ChiYuan,

On 1/12/26 10:20, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> To make sure LED enter off state after file handle is closed, initiatively
> configure LED_MODE to NONE. This can guarantee whatever the previous state
> is torch or strobe mode, the final state will be off.
> 
> Cc: stable@vger.kernel.org
> Fixes: 42bd6f59ae90 ("media: Add registration helpers for V4L2 flash sub-devices")
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
> Still cannot pass patch integration check, send v3 patch to fix all.
> 
> v3
> - Remove 'Reported-by' tag
> - Fix identation check for patch integration
> 
> v2
> - Fix commit message redudant space cause patch robot parsing error
> 
> Hi,
>    We encounter an issue. When the upper layer camera process is crashed,
> if the new process did not reinit the LED,  it will keeps the previous
> state whatever it's in torch or strobe mode
> 
> OS will handle the resource management. So when the process is crashed
> or terminated, the 'close' API will be called to release resources.
> That's why we add the initiative action to trigger LED off in file
> handle close is called.
> ---
>   drivers/media/v4l2-core/v4l2-flash-led-class.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> index 355595a0fefa..46606f5cc192 100644
> --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> @@ -623,6 +623,12 @@ static int v4l2_flash_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>   		return 0;
>   
>   	if (led_cdev) {
> +		/* If file handle is released, make sure LED enter off state */
> +		ret = v4l2_ctrl_s_ctrl(v4l2_flash->ctrls[LED_MODE],
> +				       V4L2_FLASH_LED_MODE_NONE);
> +		if (ret)
> +			return ret;
> +
>   		mutex_lock(&led_cdev->led_access);
>   
>   		if (v4l2_flash->ctrls[STROBE_SOURCE])
> 
> base-commit: 8ac28a6642d1cc8bac0632222e66add800b027fa

The patch itself looks good, but while at it I started wondering
if we shouldn't move below STROBE_SOURCE access before the lock.
I don't see now, why we placed it there.

Adding Sakari.

-- 
Best regards,
Jacek Anaszewski