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

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

[cy_huang]

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
-- 
2.34.1

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

[Jacek Anaszewski]

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

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

[ChiYuan Huang]

On Sat, Feb 21, 2026 at 04:48:48PM +0100, Jacek Anaszewski wrote:
Hi, Jacek:

> Hi ChiYuan,
> 
...
> On 1/12/26 10:20, cy_huang@richtek.com wrote:
> >   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.
> 
My assumption is LED should already be called 'led_sysfs_enable(false)',
no other APIs except V4L2 singular handle. But your guess is right. If we
put the change after the lock, should be more safe.

Thanks for the reviewing, if the common is to put after the lock, then
I can send v4 patch to fix it.
> Adding Sakari.
> 
> -- 
> Best regards,
> Jacek Anaszewski
> 

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

[ChiYuan Huang]

On Mon, Feb 23, 2026 at 08:18:50AM +0800, ChiYuan Huang wrote:
> On Sat, Feb 21, 2026 at 04:48:48PM +0100, Jacek Anaszewski wrote:
> Hi, Jacek:
> 
> > Hi ChiYuan,
> > 
> ...
> > On 1/12/26 10:20, cy_huang@richtek.com wrote:
> > >   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.
> > 
> My assumption is LED should already be called 'led_sysfs_enable(false)',
> no other APIs except V4L2 singular handle. But your guess is right. If we
> put the change after the lock, should be more safe.
> 
> Thanks for the reviewing, if the common is to put after the lock, then
> I can send v4 patch to fix it.

Aha, I remember the another thing is the 'return'.

To put after the lock, the lock problem need to be handled.
Should rewrite the lock using 'scoped_guard' and move all into the scope.

> > Adding Sakari.
> > 
> > -- 
> > Best regards,
> > Jacek Anaszewski
> > 

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

[Sakari Ailus]

Hi Jacek,

On Sat, Feb 21, 2026 at 04:48:48PM +0100, Jacek Anaszewski wrote:
> 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.

Thanks for cc'ing me.

The behaviour this patch changes has been around for a decade or so. I
wonder what breaks if we change it now. It works the same way as the sysfs
LED API, too, and I think common behaviour between the two APIs was the
reasoning back then as well.

-- 
Kind regards,

Sakari Ailus

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

[Jacek Anaszewski]

Hi Sakari,

On 2/23/26 10:43, Sakari Ailus wrote:
> Hi Jacek,
> 
> On Sat, Feb 21, 2026 at 04:48:48PM +0100, Jacek Anaszewski wrote:
>> 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.
> 
> Thanks for cc'ing me.
> 
> The behaviour this patch changes has been around for a decade or so. I
> wonder what breaks if we change it now. It works the same way as the sysfs
> LED API, too, and I think common behaviour between the two APIs was the
> reasoning back then as well.

The thing is that v4l2_flash_open() disables LED sysfs interface via
led_sysfs_disable() and v4l2_flash_close() enables it via 
led_sysfs_enable(). led_sysfs_{enable|disable}() modify the state of
LED_SYSFS_DISABLE flag.

Therefore it is only the led_sysfs_{enable|disable}() API that needs to
be called under led_access lock, since the LED subsystem sysfs handlers
test the LED_SYSFS_DISABLE flag under the same lock, and return -EBUSY
in case it is set.

The call to v4l2_flash_close() is synchronized internally in V4L2 core
I believe.

Therefore I think that we can safely move the
"if (v4l2_flash->ctrls[STROBE_SOURCE])" condition before the lock.

Otherwise we would have to put this new v4l2_ctrl_s_ctrl() call,
added in this patch also under the lock because why not. It would spark
questions in the future asking how it is different from the above
"v4l2_ctrl_s_ctrl(v4l2_flash->ctrls[LED_MODE]" case.

-- 
Best regards,
Jacek Anaszewski

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

[Sakari Ailus]

Hi Jacek,

On Mon, Feb 23, 2026 at 07:32:16PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 2/23/26 10:43, Sakari Ailus wrote:
> > Hi Jacek,
> > 
> > On Sat, Feb 21, 2026 at 04:48:48PM +0100, Jacek Anaszewski wrote:
> > > 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.
> > 
> > Thanks for cc'ing me.
> > 
> > The behaviour this patch changes has been around for a decade or so. I
> > wonder what breaks if we change it now. It works the same way as the sysfs
> > LED API, too, and I think common behaviour between the two APIs was the
> > reasoning back then as well.
> 
> The thing is that v4l2_flash_open() disables LED sysfs interface via
> led_sysfs_disable() and v4l2_flash_close() enables it via
> led_sysfs_enable(). led_sysfs_{enable|disable}() modify the state of
> LED_SYSFS_DISABLE flag.
> 
> Therefore it is only the led_sysfs_{enable|disable}() API that needs to
> be called under led_access lock, since the LED subsystem sysfs handlers
> test the LED_SYSFS_DISABLE flag under the same lock, and return -EBUSY
> in case it is set.
> 
> The call to v4l2_flash_close() is synchronized internally in V4L2 core
> I believe.
> 
> Therefore I think that we can safely move the
> "if (v4l2_flash->ctrls[STROBE_SOURCE])" condition before the lock.
> 
> Otherwise we would have to put this new v4l2_ctrl_s_ctrl() call,
> added in this patch also under the lock because why not. It would spark
> questions in the future asking how it is different from the above
> "v4l2_ctrl_s_ctrl(v4l2_flash->ctrls[LED_MODE]" case.

My concern really is that this is not a bugfix but a change of an existing
UAPI that's been out there for a decade. Such a change is likely to cause
troubles to the users. Let's see what others think.

-- 
Kind regards,

Sakari Ailus