[PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[Raymond Hackley]

The phones listed below have Richteck RT5033 LED, which has GPIO pin
configurations similar to SGM3140 Flash LED driver.
Add it to the device trees.

- Samsung Galaxy A3/A5/A7 2015
- Samsung Galaxy E5/E7
- Samsung Galaxy Grand Max

Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
 .../qcom/msm8916-samsung-a2015-common.dtsi    | 23 +++++++++++++++++++
 .../boot/dts/qcom/msm8939-samsung-a7.dts      | 23 +++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
index 019bf73178fa..48990716cafd 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
@@ -4,6 +4,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 
 / {
@@ -36,6 +37,21 @@ clk_pwm: pwm {
 		status = "disabled";
 	};
 
+	flash-led-controller {
+		compatible = "richtek,rt5033-led";
+		enable-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
+		flash-gpios = <&tlmm 36 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&camera_flash_default>;
+		pinctrl-names = "default";
+
+		flash_led: led {
+			function = LED_FUNCTION_FLASH;
+			color = <LED_COLOR_ID_WHITE>;
+			flash-max-timeout-us = <544000>;
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 
@@ -293,6 +309,13 @@ accel_int_default: accel-int-default-state {
 		bias-disable;
 	};
 
+	camera_flash_default: camera-flash-default-state {
+		pins = "gpio34", "gpio36";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
 	fg_alert_default: fg-alert-default-state {
 		pins = "gpio121";
 		function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
index ba652909d162..8d00a2328ca8 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
@@ -7,6 +7,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
 
 / {
 	model = "Samsung Galaxy A7 (2015)";
@@ -31,6 +32,21 @@ tz-apps@85500000 {
 		};
 	};
 
+	flash-led-controller {
+		compatible = "richtek,rt5033-led";
+		enable-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
+		flash-gpios = <&tlmm 36 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-0 = <&camera_flash_default>;
+		pinctrl-names = "default";
+
+		flash_led: led {
+			function = LED_FUNCTION_FLASH;
+			color = <LED_COLOR_ID_WHITE>;
+			flash-max-timeout-us = <544000>;
+		};
+	};
+
 	gpio-hall-sensor {
 		compatible = "gpio-keys";
 
@@ -360,6 +376,13 @@ accel_int_default: accel-int-default-state {
 		bias-disable;
 	};
 
+	camera_flash_default: camera-flash-default-state {
+		pins = "gpio34", "gpio36";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
 	fg_alert_default: fg-alert-default-state {
 		pins = "gpio121";
 		function = "gpio";
-- 
2.39.2

Re: [PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[Konrad Dybcio]

On 22.08.2023 16:07, Raymond Hackley wrote:
> The phones listed below have Richteck RT5033 LED, which has GPIO pin
> configurations similar to SGM3140 Flash LED driver.
> Add it to the device trees.
> 
> - Samsung Galaxy A3/A5/A7 2015
> - Samsung Galaxy E5/E7
> - Samsung Galaxy Grand Max
> 
> Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
> ---
What about the other subdevices registered with the RT5033 MFD?

Konrad

Re: [PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[Raymond Hackley]

Hi Konrad,

> What about the other subdevices registered with the RT5033 MFD?

Currently the MFD is not able to control the flash LED, due to missing
driver for it.
Alternatively we control it with GPIO pins. With driver leds-sgm3140, the
binding is not under the RT5033 MFD.

Regards,
Raymond

Re: [PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[Konrad Dybcio]

On 22.08.2023 17:01, Raymond Hackley wrote:
> Hi Konrad,
> 
>> What about the other subdevices registered with the RT5033 MFD?
> 
> Currently the MFD is not able to control the flash LED, due to missing
> driver for it.
> Alternatively we control it with GPIO pins. With driver leds-sgm3140, the
> binding is not under the RT5033 MFD.
Hm.. I have mixed feelings..

Does the led controlled onboard this chip have some super complex
functionality that is going to be exposed through a specific driver
and that is not exposed through the sgm3140 driver?

Konrad

Re: [PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[Henrik Grimler]

Hi Konrad,

On Sat, Aug 26, 2023 at 02:09:38PM +0200, Konrad Dybcio wrote:
> On 22.08.2023 17:01, Raymond Hackley wrote:
> > Hi Konrad,
> > 
> >> What about the other subdevices registered with the RT5033 MFD?
> > 
> > Currently the MFD is not able to control the flash LED, due to missing
> > driver for it.
> > Alternatively we control it with GPIO pins. With driver leds-sgm3140, the
> > binding is not under the RT5033 MFD.
> Hm.. I have mixed feelings..
> 
> Does the led controlled onboard this chip have some super complex
> functionality that is going to be exposed through a specific driver
> and that is not exposed through the sgm3140 driver?

The rt5033-led&sgm3140 situation was discussed in msm8916-mainline
matrix room before, here is a summary for list.

Using the full rt5033-led driver (old patchset here [1]) has some
benefits, like:
* led can have different intensity levels
* led can (supposedly) be used in strobe mode

Using sgm3140 driver mostly works fine, but there could potentially be
situations where led stops working and a reboot to stock android
kernel is required to get it working again.  So far that has only been
reproduced by first booting a kernel with full rt5033-led driver
(based on [1]), and then booting a kernel using sgm3140 driver
instead.

[1] https://lore.kernel.org/linux-leds/1448446948-13729-1-git-send-email-ingi2.kim@samsung.com/

Best regards,
Henrik Grimler

Re: [PATCH] arm64: dts: qcom: msm8916/39-samsung-a2015: Add flash LED

[Konrad Dybcio]

On 28.08.2023 21:43, Henrik Grimler wrote:
> Hi Konrad,
> 
> On Sat, Aug 26, 2023 at 02:09:38PM +0200, Konrad Dybcio wrote:
>> On 22.08.2023 17:01, Raymond Hackley wrote:
>>> Hi Konrad,
>>>
>>>> What about the other subdevices registered with the RT5033 MFD?
>>>
>>> Currently the MFD is not able to control the flash LED, due to missing
>>> driver for it.
>>> Alternatively we control it with GPIO pins. With driver leds-sgm3140, the
>>> binding is not under the RT5033 MFD.
>> Hm.. I have mixed feelings..
>>
>> Does the led controlled onboard this chip have some super complex
>> functionality that is going to be exposed through a specific driver
>> and that is not exposed through the sgm3140 driver?
> 
> The rt5033-led&sgm3140 situation was discussed in msm8916-mainline
> matrix room before, here is a summary for list.
> 
> Using the full rt5033-led driver (old patchset here [1]) has some
> benefits, like:
> * led can have different intensity levels
> * led can (supposedly) be used in strobe mode
> 
> Using sgm3140 driver mostly works fine, but there could potentially be
> situations where led stops working and a reboot to stock android
> kernel is required to get it working again.  So far that has only been
> reproduced by first booting a kernel with full rt5033-led driver
> (based on [1]), and then booting a kernel using sgm3140 driver
> instead.
> 
> [1] https://lore.kernel.org/linux-leds/1448446948-13729-1-git-send-email-ingi2.kim@samsung.com/
2015, whoops..

Looks like the reviewers were mostly happy (style comments for the
most part), so perhaps it'd be worth to bring it back?

As for this patch, I'll be happy to take it if you can confirm the
rt5033-leds driver won't require bindings changes (i.e. the node
you're adding today would work fine as-is if you removed sgm3140
and probed the separate driver), at least in the form that it was
posted in the link above..

Konrad