Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <cb5d7483-9b6f-51c7-a2b1-9ff83c6e7c8a@quicinc.com>
Date:   Mon, 27 Mar 2023 12:30:09 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v8 1/2] leds: flash: add driver to support flash LED
 module in QCOM PMICs
To:     Pavel Machek <pavel@ucw.cz>
CC:     <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <lee@kernel.org>, <krzysztof.kozlowski@linaro.org>,
        <linux-leds@vger.kernel.org>, <quic_collinsd@quicinc.com>,
        <quic_subbaram@quicinc.com>, Luca Weiss <luca.weiss@fairphone.com>
References: <20230303095023.538917-1-quic_fenglinw@quicinc.com>
 <20230303095023.538917-2-quic_fenglinw@quicinc.com>
 <20230325170957.GA2904@bug>
Content-Language: en-US
From:   Fenglin Wu <quic_fenglinw@quicinc.com>
In-Reply-To: <20230325170957.GA2904@bug>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/26/2023 1:09 AM, Pavel Machek wrote:
> Hi!
>> @@ -61,6 +61,21 @@ config LEDS_MT6360
>>   	  Independent current sources supply for each flash LED support torch
>>   	  and strobe mode.
>>   
>> +config LEDS_QCOM_FLASH
>> +	tristate "LED support for flash module inside Qualcomm Technologies, Inc. PMIC"
>> +	depends on MFD_SPMI_PMIC || COMPILE_TEST
>> +	depends on LEDS_CLASS && OF
>> +	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
>> +	select REGMAP
>> +	help
>> +	  This option enables support for the flash module found in Qualcomm
>> +	  Technologies, Inc. PMICs. The flash module can have 3 or 4 flash LED
>> +	  channels and each channel is programmable to support up to 1.5 A full
>> +	  scale current. It also supports connecting two channels' output together
>> +	  to supply one LED component to achieve current up to 2 A. In such case,
>> +	  the total LED current will be split symmetrically on each channel and
>> +	  they will be enabled/disabled at the same time.
> 
> "This can be built as a module, module will be called..."

I can update it in a new change considering the initial change has been 
applied.

> 
>> +static int qcom_flash_led_brightness_set(struct led_classdev *led_cdev,
>> +					enum led_brightness brightness)
>> +{
>> +	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
>> +	struct qcom_flash_led *led = flcdev_to_qcom_fled(fled_cdev);
>> +	u32 current_ma = brightness * led->max_torch_current_ma / LED_FULL;
>> +	bool enable = !!brightness;
>> +	int rc;
>> +
>> +	rc = set_flash_current(led, current_ma, TORCH_MODE);
>> +	if (rc)
>> +		return rc;
> 
> I'd not mind supporting more than one brightness level -- AFAICT hw can do that.

It does support multiple levels and the 'current_ma' is calculated based 
on different brightness level.

> 
> BR,
> 									Pavel