mbox series

[v3,0/2] Add LED driver for flash module in QCOM PMICs

Message ID 20221018014024.948731-1-quic_fenglinw@quicinc.com (mailing list archive)
Headers show
Series Add LED driver for flash module in QCOM PMICs | expand

Message

Fenglin Wu Oct. 18, 2022, 1:40 a.m. UTC 
Initial driver and binding document changes for supporting flash LED
module in Qualcomm Technologies, Inc. PMICs.

Changes in V3:
  1. Updated the driver to use regmap_field for register access.
  2. Adressed the review comments in binding document change.

Changes in V2:
  1. Addressed review comments in binding change, thanks Krzysztof!
  2. Updated driver to address the compilation issue reported by
     kernel test robot.


Fenglin Wu (2):
  leds: flash: add driver to support flash LED module in QCOM PMICs
  dt-bindings: add bindings for QCOM flash LED

 .../bindings/leds/qcom,spmi-flash-led.yaml    | 116 +++
 drivers/leds/flash/Kconfig                    |  15 +
 drivers/leds/flash/Makefile                   |   1 +
 drivers/leds/flash/leds-qcom-flash.c          | 700 ++++++++++++++++++
 4 files changed, 832 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
 create mode 100644 drivers/leds/flash/leds-qcom-flash.c

Comments

Luca Weiss Oct. 19, 2022, 7:23 a.m. UTC | #1 
Hi Fenglin,

On Tue Oct 18, 2022 at 3:40 AM CEST, Fenglin Wu wrote:
> Initial driver and binding document changes for supporting flash LED
> module in Qualcomm Technologies, Inc. PMICs.
>

Thanks for these patches, it's really nice to see drivers like this
being sent upstream!

I've just tried these patches on pm6150l which also is compatible with
this driver (and used on sm7225-fairphone-fp4).

The two different flash LEDs on the device I could adjust as expected
using sysfs:

$ echo 255 > /sys/class/leds/yellow:flash-0/brightness
$ echo 255 > /sys/class/leds/white:flash-0/brightness

Also lower brightness values resulted in lower brightness on the LED, so
all is good here!

But for flash usage, I couldn't figure out how to use it, doing the
following resulted in no change on the LED.

$ cat /sys/class/leds/white:flash-0/max_flash_brightness
1000000
$ echo 1000000 > /sys/class/leds/white:flash-0/flash_brightness

Here's my LED definition:

  led-0 {
    function = LED_FUNCTION_FLASH;
    color = <LED_COLOR_ID_YELLOW>;
    led-sources = <1>;
    led-max-microamp = <180000>;
    flash-max-microamp = <1000000>;
    flash-max-timeout-us = <1280000>;
  };

From values are from msm-4.19 kernel:

  qcom,flash_0 {
    qcom,current-ma = <1000>; // => flash-max-microamp
    qcom,duration-ms = <1280>; // => flash-max-timeout-us
    qcom,id = <0>; // => led-sources?
  };

  qcom,torch_0 {
    qcom,current-ma = <180>; // => led-max-microamp
    qcom,id = <0>; // => led-sources?
  };

Could you please let me know how flash is supposed to work or if I
maybe have messed up some setting here?

Regards
Luca

> Changes in V3:
>   1. Updated the driver to use regmap_field for register access.
>   2. Adressed the review comments in binding document change.
>
> Changes in V2:
>   1. Addressed review comments in binding change, thanks Krzysztof!
>   2. Updated driver to address the compilation issue reported by
>      kernel test robot.
>
>
> Fenglin Wu (2):
>   leds: flash: add driver to support flash LED module in QCOM PMICs
>   dt-bindings: add bindings for QCOM flash LED
>
>  .../bindings/leds/qcom,spmi-flash-led.yaml    | 116 +++
>  drivers/leds/flash/Kconfig                    |  15 +
>  drivers/leds/flash/Makefile                   |   1 +
>  drivers/leds/flash/leds-qcom-flash.c          | 700 ++++++++++++++++++
>  4 files changed, 832 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
>  create mode 100644 drivers/leds/flash/leds-qcom-flash.c
>
> -- 
> 2.25.1
Fenglin Wu Oct. 19, 2022, 10:17 a.m. UTC | #2 
On 2022/10/19 15:23, Luca Weiss wrote:
> Hi Fenglin,
> 
> On Tue Oct 18, 2022 at 3:40 AM CEST, Fenglin Wu wrote:
>> Initial driver and binding document changes for supporting flash LED
>> module in Qualcomm Technologies, Inc. PMICs.
>>
> 
> Thanks for these patches, it's really nice to see drivers like this
> being sent upstream!
> 
> I've just tried these patches on pm6150l which also is compatible with
> this driver (and used on sm7225-fairphone-fp4).
> 
> The two different flash LEDs on the device I could adjust as expected
> using sysfs:
> 
> $ echo 255 > /sys/class/leds/yellow:flash-0/brightness
> $ echo 255 > /sys/class/leds/white:flash-0/brightness
> 
> Also lower brightness values resulted in lower brightness on the LED, so
> all is good here!
> 
> But for flash usage, I couldn't figure out how to use it, doing the
> following resulted in no change on the LED.
> 
> $ cat /sys/class/leds/white:flash-0/max_flash_brightness
> 1000000
> $ echo 1000000 > /sys/class/leds/white:flash-0/flash_brightness
> 
> Here's my LED definition:
> 
>    led-0 {
>      function = LED_FUNCTION_FLASH;
>      color = <LED_COLOR_ID_YELLOW>;
>      led-sources = <1>;
>      led-max-microamp = <180000>;
>      flash-max-microamp = <1000000>;
>      flash-max-timeout-us = <1280000>;
>    };
> 
>  From values are from msm-4.19 kernel:
> 
>    qcom,flash_0 {
>      qcom,current-ma = <1000>; // => flash-max-microamp
>      qcom,duration-ms = <1280>; // => flash-max-timeout-us
>      qcom,id = <0>; // => led-sources?
>    };
> 
>    qcom,torch_0 {
>      qcom,current-ma = <180>; // => led-max-microamp
>      qcom,id = <0>; // => led-sources?
>    };
> 
> Could you please let me know how flash is supposed to work or if I
> maybe have messed up some setting here?
> 
> Regards
> Luca

Hi Luca,

Thanks for testing the driver at your end.
The "brightness" node is for enabling/disable/adjusting brightness when 
the LED is working in torch mode, the nodes for enabling/adjusting the 
LED behavior in flash mode are "flash_brightness" "flash_timeout" 
"flash_strobe".
You can strobe the flash by "echo 1 > flash_strobe" directly and the 
default brightness/timeout value will be used, or you can update the 
settings with "echo xxx > flash_brightness; echo xxx > flash_timeout" 
then strobe the LED with "echo 1 > flash_strobe". Please remember you 
always need to "echo 0 > flash_strobe" 1st if you want to enable it again.
Thanks

Fenglin
> 
>> Changes in V3:
>>    1. Updated the driver to use regmap_field for register access.
>>    2. Adressed the review comments in binding document change.
>>
>> Changes in V2:
>>    1. Addressed review comments in binding change, thanks Krzysztof!
>>    2. Updated driver to address the compilation issue reported by
>>       kernel test robot.
>>
>>
>> Fenglin Wu (2):
>>    leds: flash: add driver to support flash LED module in QCOM PMICs
>>    dt-bindings: add bindings for QCOM flash LED
>>
>>   .../bindings/leds/qcom,spmi-flash-led.yaml    | 116 +++
>>   drivers/leds/flash/Kconfig                    |  15 +
>>   drivers/leds/flash/Makefile                   |   1 +
>>   drivers/leds/flash/leds-qcom-flash.c          | 700 ++++++++++++++++++
>>   4 files changed, 832 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
>>   create mode 100644 drivers/leds/flash/leds-qcom-flash.c
>>
>> -- 
>> 2.25.1
>
Fenglin Wu Oct. 19, 2022, 10:19 a.m. UTC | #3 
On 2022/10/19 15:23, Luca Weiss wrote:
> Hi Fenglin,
> 
> On Tue Oct 18, 2022 at 3:40 AM CEST, Fenglin Wu wrote:
>> Initial driver and binding document changes for supporting flash LED
>> module in Qualcomm Technologies, Inc. PMICs.
>>
> 
> Thanks for these patches, it's really nice to see drivers like this
> being sent upstream!
> 
> I've just tried these patches on pm6150l which also is compatible with
> this driver (and used on sm7225-fairphone-fp4).
> 
> The two different flash LEDs on the device I could adjust as expected
> using sysfs:
> 
> $ echo 255 > /sys/class/leds/yellow:flash-0/brightness
> $ echo 255 > /sys/class/leds/white:flash-0/brightness
> 
> Also lower brightness values resulted in lower brightness on the LED, so
> all is good here!
> 
> But for flash usage, I couldn't figure out how to use it, doing the
> following resulted in no change on the LED.
> 
> $ cat /sys/class/leds/white:flash-0/max_flash_brightness
> 1000000
> $ echo 1000000 > /sys/class/leds/white:flash-0/flash_brightness
> 
> Here's my LED definition:
> 
>    led-0 {
>      function = LED_FUNCTION_FLASH;
>      color = <LED_COLOR_ID_YELLOW>;
>      led-sources = <1>;
>      led-max-microamp = <180000>;
>      flash-max-microamp = <1000000>;
>      flash-max-timeout-us = <1280000>;
>    };
> 
>  From values are from msm-4.19 kernel:
> 
>    qcom,flash_0 {
>      qcom,current-ma = <1000>; // => flash-max-microamp
>      qcom,duration-ms = <1280>; // => flash-max-timeout-us
>      qcom,id = <0>; // => led-sources?
>    };
> 
>    qcom,torch_0 {
>      qcom,current-ma = <180>; // => led-max-microamp
>      qcom,id = <0>; // => led-sources?
>    };
> 
> Could you please let me know how flash is supposed to work or if I
> maybe have messed up some setting here?
> 
> Regards
> Luca

Hi Luca,

Thanks for testing the driver at your end.
The "brightness" node is for enabling/disable/adjusting brightness when 
the LED is working in torch mode, the nodes for enabling/adjusting the 
LED behavior in flash mode are "flash_brightness" "flash_timeout" 
"flash_strobe".
You can strobe the flash by "echo 1 > flash_strobe" directly and the 
default brightness/timeout value will be used, or you can update the 
settings with "echo xxx > flash_brightness; echo xxx > flash_timeout" 
then strobe the LED with "echo 1 > flash_strobe". Please remember you 
always need to "echo 0 > flash_strobe" 1st if you want to enable it again.
Thanks

Fenglin
> 
>> Changes in V3:
>>    1. Updated the driver to use regmap_field for register access.
>>    2. Adressed the review comments in binding document change.
>>
>> Changes in V2:
>>    1. Addressed review comments in binding change, thanks Krzysztof!
>>    2. Updated driver to address the compilation issue reported by
>>       kernel test robot.
>>
>>
>> Fenglin Wu (2):
>>    leds: flash: add driver to support flash LED module in QCOM PMICs
>>    dt-bindings: add bindings for QCOM flash LED
>>
>>   .../bindings/leds/qcom,spmi-flash-led.yaml    | 116 +++
>>   drivers/leds/flash/Kconfig                    |  15 +
>>   drivers/leds/flash/Makefile                   |   1 +
>>   drivers/leds/flash/leds-qcom-flash.c          | 700 ++++++++++++++++++
>>   4 files changed, 832 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
>>   create mode 100644 drivers/leds/flash/leds-qcom-flash.c
>>
>> -- 
>> 2.25.1
>
Luca Weiss Oct. 19, 2022, 11:09 a.m. UTC | #4 
Hi Fenglin,

On Wed Oct 19, 2022 at 12:17 PM CEST, Fenglin Wu wrote:
>
>
> On 2022/10/19 15:23, Luca Weiss wrote:
> > Hi Fenglin,
> > 
> > On Tue Oct 18, 2022 at 3:40 AM CEST, Fenglin Wu wrote:
> >> Initial driver and binding document changes for supporting flash LED
> >> module in Qualcomm Technologies, Inc. PMICs.
> >>
> > 
> > Thanks for these patches, it's really nice to see drivers like this
> > being sent upstream!
> > 
> > I've just tried these patches on pm6150l which also is compatible with
> > this driver (and used on sm7225-fairphone-fp4).
> > 
> > The two different flash LEDs on the device I could adjust as expected
> > using sysfs:
> > 
> > $ echo 255 > /sys/class/leds/yellow:flash-0/brightness
> > $ echo 255 > /sys/class/leds/white:flash-0/brightness
> > 
> > Also lower brightness values resulted in lower brightness on the LED, so
> > all is good here!
> > 
> > But for flash usage, I couldn't figure out how to use it, doing the
> > following resulted in no change on the LED.
> > 
> > $ cat /sys/class/leds/white:flash-0/max_flash_brightness
> > 1000000
> > $ echo 1000000 > /sys/class/leds/white:flash-0/flash_brightness
> > 
> > Here's my LED definition:
> > 
> >    led-0 {
> >      function = LED_FUNCTION_FLASH;
> >      color = <LED_COLOR_ID_YELLOW>;
> >      led-sources = <1>;
> >      led-max-microamp = <180000>;
> >      flash-max-microamp = <1000000>;
> >      flash-max-timeout-us = <1280000>;
> >    };
> > 
> >  From values are from msm-4.19 kernel:
> > 
> >    qcom,flash_0 {
> >      qcom,current-ma = <1000>; // => flash-max-microamp
> >      qcom,duration-ms = <1280>; // => flash-max-timeout-us
> >      qcom,id = <0>; // => led-sources?
> >    };
> > 
> >    qcom,torch_0 {
> >      qcom,current-ma = <180>; // => led-max-microamp
> >      qcom,id = <0>; // => led-sources?
> >    };
> > 
> > Could you please let me know how flash is supposed to work or if I
> > maybe have messed up some setting here?
> > 
> > Regards
> > Luca
>
> Hi Luca,
>
> Thanks for testing the driver at your end.
> The "brightness" node is for enabling/disable/adjusting brightness when 
> the LED is working in torch mode, the nodes for enabling/adjusting the 
> LED behavior in flash mode are "flash_brightness" "flash_timeout" 
> "flash_strobe".
> You can strobe the flash by "echo 1 > flash_strobe" directly and the 
> default brightness/timeout value will be used, or you can update the 
> settings with "echo xxx > flash_brightness; echo xxx > flash_timeout" 
> then strobe the LED with "echo 1 > flash_strobe". Please remember you 
> always need to "echo 0 > flash_strobe" 1st if you want to enable it again.
> Thanks

Indeed with flash_strobe it works as expected!

Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sm7225-fairphone-fp4 + pm6150l

Thanks again,
Luca

>
> Fenglin
> > 
> >> Changes in V3:
> >>    1. Updated the driver to use regmap_field for register access.
> >>    2. Adressed the review comments in binding document change.
> >>
> >> Changes in V2:
> >>    1. Addressed review comments in binding change, thanks Krzysztof!
> >>    2. Updated driver to address the compilation issue reported by
> >>       kernel test robot.
> >>
> >>
> >> Fenglin Wu (2):
> >>    leds: flash: add driver to support flash LED module in QCOM PMICs
> >>    dt-bindings: add bindings for QCOM flash LED
> >>
> >>   .../bindings/leds/qcom,spmi-flash-led.yaml    | 116 +++
> >>   drivers/leds/flash/Kconfig                    |  15 +
> >>   drivers/leds/flash/Makefile                   |   1 +
> >>   drivers/leds/flash/leds-qcom-flash.c          | 700 ++++++++++++++++++
> >>   4 files changed, 832 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/leds/qcom,spmi-flash-led.yaml
> >>   create mode 100644 drivers/leds/flash/leds-qcom-flash.c
> >>
> >> -- 
> >> 2.25.1
> >