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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pavel,

On 9/11/20 9:05 AM, Pavel Machek wrote:
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.

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

commit 7aea8389a77abf9fde254aca2434a605c7704f58
Author: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>
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 <j.anaszewski@xxxxxxxxxxx>
    Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
    Cc: Richard Purdie <rpurdie@xxxxxxxxx>
    Acked-by: Pavel Machek <pavel@xxxxxx>
    Signed-off-by: Bryan Wu <cooloney@xxxxxxxxx>


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

+MODULE_AUTHOR("Gene Chen <gene_chen@xxxxxxxxxxx>");
+MODULE_DESCRIPTION("MT6360 Led Driver");

Led -> LED.

									Pavel


--
Best regards,
Jacek Anaszewski



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux