Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 1/2] leds: mt6360: Add LED driver for MT6360
To:     Pavel Machek <pavel@ucw.cz>
Cc:     Gene Chen <gene.chen.richtek@gmail.com>, robh+dt@kernel.org,
        matthias.bgg@gmail.com, dmurphy@ti.com, linux-leds@vger.kernel.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
        gene_chen@richtek.com, Wilma.Wu@mediatek.com,
        shufan_lee@richtek.com, cy_huang@richtek.com,
        benjamin.chao@mediatek.com
References: <1599474459-20853-1-git-send-email-gene.chen.richtek@gmail.com>
 <1599474459-20853-2-git-send-email-gene.chen.richtek@gmail.com>
 <559a568e-3a2e-33c6-43aa-547a18f8e26b@gmail.com> <20200911070503.GA9818@amd>
From:   Jacek Anaszewski <jacek.anaszewski@gmail.com>
Message-ID: <d2721bb7-6f0e-19af-41b9-0f3ae95cf4f9@gmail.com>
Date:   Fri, 11 Sep 2020 22:56:10 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <20200911070503.GA9818@amd>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@samsung.com>
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@samsung.com>
     Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
     Cc: Richard Purdie <rpurdie@rpsys.net>
     Acked-by: Pavel Machek <pavel@ucw.cz>
     Signed-off-by: Bryan Wu <cooloney@gmail.com>


> Could I get you to remove code you are not commenting at when
> reviewing?
> 
>>> +MODULE_AUTHOR("Gene Chen <gene_chen@richtek.com>");
>>> +MODULE_DESCRIPTION("MT6360 Led Driver");
> 
> Led -> LED.
> 
> 									Pavel
> 

-- 
Best regards,
Jacek Anaszewski