On 9/11/20 1:24 AM, Gene Chen wrote:
Pavel Machek <pavel@xxxxxx> 於 2020年9月11日 週五 下午3:05寫道：
Hi!
+{
+ struct mt6360_led *led = container_of(lcdev, struct mt6360_led, flash.led_cdev);
+ struct mt6360_priv *priv = led->priv;
+ u32 enable_mask = MT6360_TORCHEN_MASK | MT6360_FLCSEN_MASK(led->led_no);
+ u32 val = (level) ? MT6360_FLCSEN_MASK(led->led_no) : 0;
+ u32 prev = priv->fled_torch_used, curr;
+ int ret;
+
+ dev_dbg(lcdev->dev, "[%d] brightness %d\n", led->led_no, level);
+ if (priv->fled_strobe_used) {
+ dev_warn(lcdev->dev, "Please disable strobe first [%d]\n", priv->fled_strobe_used);
Doesn't hardware handle that? IOW, what happens when you have enabled
both torch and flash? If flash just overrides torch mode, than you
should not prevent enabling torch in this case.
Yep, this is strange/confusing... and was reason why I asked for not
supporting strobe from sysfs.
Could I get you to remove code you are not commenting at when
reviewing?
MT6360 FLED register define is STROBE_EN/TORCH_EN/CS1/CS2 (current
source) 4 bits.
The STROBE_EN/TORCH_EN is shared by FLED1 and FLED2.
If I want to enable FLED1 torch mode, I set TORCH_EN and CS1
If I want to enable FLED2 strobe mode, I set STROBE_EN and CS2
For example I set FLED1 torch, then I set FLED2 strobe.
When I set FLED2 strobe, I will see the strobe current is FLED2 add
FLED1 current which is not I want.
So I need disable FLED1 torch first.
Considering every circumstances is complicated when share same H/W
logic control.
And the other problem is torch mode switch to strobe mode needs ramp
time because strobe and torch mode can't be co-exist.
Thank you for the explanation. So we have to keep your guards
but I would return -EBUSY instead of -EINVAL.
This would be also consistent with what
drivers/media/v4l2-core/v4l2-flash-led-class.c
does in its v4l2_flash_s_ctrl(), case V4L2_CID_FLASH_STROBE - it returns
-EBUSY if __software_strobe_mode_inactive() returns false.
The advantage of V4L2 Flash interface is that it has LED_MODE that
can be set to torch or flash, but in LED subsystem we don't have
the counterpart.
--
Best regards,
Jacek Anaszewski
