diff mbox series

[3/3] arm64: dts: qcom: sm7225-fairphone-fp4: configure flash LED

Message ID 20221209-fp4-pm6150l-flash-v1-3-531521eb2a72@fairphone.com
State Accepted
Commit 1c170714490e4d8c0886019145c9d90dfade14f9
Headers show
Series Add PM6150L flash LED to Fairphone 4 | expand

Commit Message

Luca Weiss Dec. 9, 2022, 1:54 p.m. UTC 
Configure the pm6150l flash node for the dual flash LEDs found on FP4.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Luca Weiss Dec. 12, 2022, 1:59 p.m. UTC | #1 
On Sat Dec 10, 2022 at 6:16 PM CET, Pavel Machek wrote:
> Hi!
>
> > Configure the pm6150l flash node for the dual flash LEDs found on FP4.
>
> > +&pm6150l_flash {
> > +	status = "okay";
> > +
> > +	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>;
> > +	};

Hi Pavel,

>
> I'm pretty sure the flash is not yellow.

The marketing term is Dual LED flash or Dual-tone flash, one LED is a
blue-ish white and one is a yellow-ish white, but from what I can tell,
in the original code it's always referred to as white and yellow so I
also followed that here.

Also the LEDs are right next to each other so in practise for torch just
both go on, and for camera flash I cannot really tell you but I guess
it's doing something there with the camera tuning.

See also this picture:
https://shop.fairphone.com/media/catalog/product/cache/b752d78484639b19641a8560800d919d/p/_/p_5b_main_camera_back.jpg

>
> Plus, how is the node in /sys/class/leds called? Can you make an entry
> in Documentation/leds/well-known-leds.txt and ensure the name stays
> consistent across devices?

/ # ls -al /sys/class/leds/white:flash/
total 0
drwxr-xr-x    3 0        0                0 Jan  1 00:00 .
drwxr-xr-x    4 0        0                0 Jan  1 00:00 ..
-rw-r--r--    1 0        0             4096 Jan  1 00:00 brightness
lrwxrwxrwx    1 0        0                0 Jan  1 00:00 device -> ../../../c440000.spmi:pmic@5:led-controller@d300
-rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_brightness
-r--r--r--    1 0        0             4096 Jan  1 00:00 flash_fault
-rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_strobe
-rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_timeout
-r--r--r--    1 0        0             4096 Jan  1 00:00 max_brightness
-r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_brightness
-r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_timeout
drwxr-xr-x    2 0        0                0 Jan  1 00:00 power
lrwxrwxrwx    1 0        0                0 Jan  1 00:00 subsystem -> ../../../../../../../../../class/leds
-rw-r--r--    1 0        0             4096 Jan  1 00:00 uevent
/ # ls -al /sys/class/leds/yellow:flash/
total 0
drwxr-xr-x    3 0        0                0 Jan  1 00:00 .
drwxr-xr-x    4 0        0                0 Jan  1 00:00 ..
-rw-r--r--    1 0        0             4096 Jan  1 00:00 brightness
lrwxrwxrwx    1 0        0                0 Jan  1 00:00 device -> ../../../c440000.spmi:pmic@5:led-controller@d300
-rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_brightness
-r--r--r--    1 0        0             4096 Jan  1 00:00 flash_fault
-rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_strobe
-rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_timeout
-r--r--r--    1 0        0             4096 Jan  1 00:00 max_brightness
-r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_brightness
-r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_timeout
drwxr-xr-x    2 0        0                0 Jan  1 00:00 power
lrwxrwxrwx    1 0        0                0 Jan  1 00:00 subsystem -> ../../../../../../../../../class/leds
-rw-r--r--    1 0        0             4096 Jan  1 00:00 uevent

There's also already flash LED on PinePhone and some MSM8916 devices,
but I think they also have white:flash based on the dt.

>
> Best regards,
> 								Pavel
> -- 
> People of Russia, stop Putin before his war on Ukraine escalates.
Fenglin Wu Jan. 5, 2023, 3:30 a.m. UTC | #2 
On 2022/12/12 21:59, Luca Weiss wrote:
> On Sat Dec 10, 2022 at 6:16 PM CET, Pavel Machek wrote:
>> Hi!
>>
>>> Configure the pm6150l flash node for the dual flash LEDs found on FP4.
>>
>>> +&pm6150l_flash {
>>> +	status = "okay";
>>> +
>>> +	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>;
>>> +	};
> 
> Hi Pavel,
> 
>>
>> I'm pretty sure the flash is not yellow.
> 
> The marketing term is Dual LED flash or Dual-tone flash, one LED is a
> blue-ish white and one is a yellow-ish white, but from what I can tell,
> in the original code it's always referred to as white and yellow so I
> also followed that here.
> 
> Also the LEDs are right next to each other so in practise for torch just
> both go on, and for camera flash I cannot really tell you but I guess
> it's doing something there with the camera tuning.
> 
> See also this picture:
> https://shop.fairphone.com/media/catalog/product/cache/b752d78484639b19641a8560800d919d/p/_/p_5b_main_camera_back.jpg
> 
Hi Pavel,

Luca is right. It is normally called dual CCT (Correlated Color 
Temperature) flash LED. It has 2 LEDs, one is with higher CCT (~6000K) 
so it looks like a white LED, another is with lower CCT (~2000K) and it 
looks like a yellow LED. I am not an expert of this but my understanding 
is the camera tuning process normally adjusts the brightness of the two 
LEDs and enables them to get different CCT for different snapshots.
I was thinking to use the "white" and "yellow" to name the flash LEDs 
which should be much better that just using indexes, it implicitly tell 
that the "white" one is having higher CCT and the "yellow" one is having 
lower CCT.

Fenglin
>>
>> Plus, how is the node in /sys/class/leds called? Can you make an entry
>> in Documentation/leds/well-known-leds.txt and ensure the name stays
>> consistent across devices?
> 
> / # ls -al /sys/class/leds/white:flash/
> total 0
> drwxr-xr-x    3 0        0                0 Jan  1 00:00 .
> drwxr-xr-x    4 0        0                0 Jan  1 00:00 ..
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 brightness
> lrwxrwxrwx    1 0        0                0 Jan  1 00:00 device -> ../../../c440000.spmi:pmic@5:led-controller@d300
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_brightness
> -r--r--r--    1 0        0             4096 Jan  1 00:00 flash_fault
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_strobe
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_timeout
> -r--r--r--    1 0        0             4096 Jan  1 00:00 max_brightness
> -r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_brightness
> -r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_timeout
> drwxr-xr-x    2 0        0                0 Jan  1 00:00 power
> lrwxrwxrwx    1 0        0                0 Jan  1 00:00 subsystem -> ../../../../../../../../../class/leds
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 uevent
> / # ls -al /sys/class/leds/yellow:flash/
> total 0
> drwxr-xr-x    3 0        0                0 Jan  1 00:00 .
> drwxr-xr-x    4 0        0                0 Jan  1 00:00 ..
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 brightness
> lrwxrwxrwx    1 0        0                0 Jan  1 00:00 device -> ../../../c440000.spmi:pmic@5:led-controller@d300
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_brightness
> -r--r--r--    1 0        0             4096 Jan  1 00:00 flash_fault
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_strobe
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 flash_timeout
> -r--r--r--    1 0        0             4096 Jan  1 00:00 max_brightness
> -r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_brightness
> -r--r--r--    1 0        0             4096 Jan  1 00:00 max_flash_timeout
> drwxr-xr-x    2 0        0                0 Jan  1 00:00 power
> lrwxrwxrwx    1 0        0                0 Jan  1 00:00 subsystem -> ../../../../../../../../../class/leds
> -rw-r--r--    1 0        0             4096 Jan  1 00:00 uevent
> 
> There's also already flash LED on PinePhone and some MSM8916 devices,
> but I think they also have white:flash based on the dt.
> 
>>
>> Best regards,
>> 								Pavel
>> -- 
>> People of Russia, stop Putin before his war on Ukraine escalates.
>
Pavel Machek March 23, 2023, 7:54 p.m. UTC | #3 
Hi!


> > > I'm pretty sure the flash is not yellow.
> > 
> > The marketing term is Dual LED flash or Dual-tone flash, one LED is a
> > blue-ish white and one is a yellow-ish white, but from what I can tell,
> > in the original code it's always referred to as white and yellow so I
> > also followed that here.
> > 
> > Also the LEDs are right next to each other so in practise for torch just
> > both go on, and for camera flash I cannot really tell you but I guess
> > it's doing something there with the camera tuning.
> > 
> > See also this picture:
> > https://shop.fairphone.com/media/catalog/product/cache/b752d78484639b19641a8560800d919d/p/_/p_5b_main_camera_back.jpg
> > 
> Hi Pavel,
> 
> Luca is right. It is normally called dual CCT (Correlated Color Temperature)
> flash LED. It has 2 LEDs, one is with higher CCT (~6000K) so it looks like a
> white LED, another is with lower CCT (~2000K) and it looks like a yellow
> LED. I am not an expert of this but my understanding is the camera tuning
> process normally adjusts the brightness of the two LEDs and enables them to
> get different CCT for different snapshots.

I believe this is normally called "warm white" and "cool white", no?
Yellow would be monochromatic light at cca 575nm, see
https://en.wikipedia.org/wiki/Shades_of_yellow .

If we need to add some defines for that, lets do that.

BR,
									Pavel
Luca Weiss March 31, 2023, 8:59 a.m. UTC | #4 
Hi Pavel,

On Thu Mar 23, 2023 at 8:54 PM CET, Pavel Machek wrote:
> Hi!
>
>
> > > > I'm pretty sure the flash is not yellow.
> > > 
> > > The marketing term is Dual LED flash or Dual-tone flash, one LED is a
> > > blue-ish white and one is a yellow-ish white, but from what I can tell,
> > > in the original code it's always referred to as white and yellow so I
> > > also followed that here.
> > > 
> > > Also the LEDs are right next to each other so in practise for torch just
> > > both go on, and for camera flash I cannot really tell you but I guess
> > > it's doing something there with the camera tuning.
> > > 
> > > See also this picture:
> > > https://shop.fairphone.com/media/catalog/product/cache/b752d78484639b19641a8560800d919d/p/_/p_5b_main_camera_back.jpg
> > > 
> > Hi Pavel,
> > 
> > Luca is right. It is normally called dual CCT (Correlated Color Temperature)
> > flash LED. It has 2 LEDs, one is with higher CCT (~6000K) so it looks like a
> > white LED, another is with lower CCT (~2000K) and it looks like a yellow
> > LED. I am not an expert of this but my understanding is the camera tuning
> > process normally adjusts the brightness of the two LEDs and enables them to
> > get different CCT for different snapshots.
>
> I believe this is normally called "warm white" and "cool white", no?
> Yellow would be monochromatic light at cca 575nm, see
> https://en.wikipedia.org/wiki/Shades_of_yellow .

I don't really have any more information I can provide right now. If you
feel it should be called warm white and cool white, feel free to send a
patch changing it.

I'm personally okay with it being called white & yellow since that seems
to be the term used in (downstream) software for these kinds of leds.

Regards
Luca

>
> If we need to add some defines for that, lets do that.
>
> BR,
> 									Pavel
> -- 
> People of Russia, stop Putin before his war on Ukraine escalates.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
index c456e9594ea5..fef7d1d02925 100644
--- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
+++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts
@@ -7,6 +7,7 @@ 
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sm7225.dtsi"
@@ -367,6 +368,28 @@  &mpss {
 	firmware-name = "qcom/sm7225/fairphone4/modem.mdt";
 };
 
+&pm6150l_flash {
+	status = "okay";
+
+	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>;
+	};
+
+	led-1 {
+		function = LED_FUNCTION_FLASH;
+		color = <LED_COLOR_ID_WHITE>;
+		led-sources = <2>;
+		led-max-microamp = <180000>;
+		flash-max-microamp = <1000000>;
+		flash-max-timeout-us = <1280000>;
+	};
+};
+
 &pm6150l_wled {
 	status = "okay";