Re: [PATCH v3 1/2] leds: mt6360: Add LED driver for MT6360

Hi Pavel,

On 9/11/20 9:05 AM, Pavel Machek wrote:

Hi!

quoted

quoted

+{
+	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.

What you say now is even more confusing when we look at your ack
under this patch:

commit 7aea8389a77abf9fde254aca2434a605c7704f58
Author: Jacek Anaszewski [off-list ref]
Date:   Fri Jan 9 07:22:51 2015 -0800

     leds: Add LED Flash class extension to the LED subsystem

     Some LED devices support two operation modes - torch and flash.
     This patch provides support for flash LED devices in the LED subsystem
     by introducing new sysfs attributes and kernel internal interface.
     The attributes being introduced are: flash_brightness, flash_strobe,
     flash_timeout, max_flash_timeout, max_flash_brightness, flash_fault,
     flash_sync_strobe and available_sync_leds. All the flash related
     features are placed in a separate module.

     The modifications aim to be compatible with V4L2 framework requirements
     related to the flash devices management. The design assumes that V4L2
     sub-device can take of the LED class device control and communicate
     with it through the kernel internal interface. When V4L2 Flash 
sub-device
     file is opened, the LED class device sysfs interface is made
     unavailable.

     Signed-off-by: Jacek Anaszewski [off-list ref]
     Acked-by: Kyungmin Park [off-list ref]
     Cc: Richard Purdie [off-list ref]
     Acked-by: Pavel Machek [off-list ref]
     Signed-off-by: Bryan Wu [off-list ref]

Could I get you to remove code you are not commenting at when
reviewing?

quoted

quoted

+MODULE_AUTHOR("Gene Chen [off-list ref]");
+MODULE_DESCRIPTION("MT6360 Led Driver");

Led -> LED.

									Pavel

-- 
Best regards,
Jacek Anaszewski