Re: [PATCH 3/3] ARM: dts: qcom: msm8960: expressatt: Add camera flash

Linus Walleij
2026-03-08T23:37:15Z

Hi Rudraksha,

thanks for your patch!

On Sat, Mar 7, 2026 at 1:58 AM Rudraksha Gupta via B4 Relay
<devnull+guptarud.gmail.com@kernel.org> wrote:

> From: Rudraksha Gupta <guptarud@gmail.com>
>
> Add camera flash support for the Samsung Galaxy Express (expressatt).
>
> The flash IC uses a one-wire pulse-count protocol on GPIO 3, gated by
> PMIC MPP 4 which must be driven high to unlock the flash circuit.
>
> Downstream references:
> Link: https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-12.0-YNG4N/drivers/leds/Makefile#L51
> Link: https://github.com/LineageOS/android_kernel_samsung_d2/blob/stable/cm-12.0-YNG4N/arch/arm/mach-msm/board-apexq-camera.c#L591
>
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>

(...)

> + camera_flash: led-controller {
> + compatible = "richtek,rt8515";
> + enf-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;

I think you should ideally define richtek,rfs-ohms, if it's impossible
to find this information then use the default,
richtek,rfs-ohms = <16000>;

> + unlock-gpios = <&pm8921_mpps 4 GPIO_ACTIVE_HIGH>;

As mentioned I don't think this is right. The chip has no "unlock"
signal. I think this is a simple regulator (such as a switch).

I would do:

vin-supply = <&flash_gpio_reg>;

Then something like (better if you reserarch it a bit):

flash_gpio_reg: regulator-gpio-ldo-3v3 {
compatible = "regulator-fixed";
/* Supplied in turn by VBAT? I guess so. It is between 2.8 and 5V */
regulator-name = "FLASH_3V3"; // Or whatever the rail is best called?
regulator-min-microvolt = <3300000>; // If you have better guesses, use them
regulator-max-microvolt = <3300000>; // If you know VBAT then use
that voltage
gpio = <&pm8921_mpps 4 GPIO_ACTIVE_HIGH>;
startup-delay-us = <5000>; // FIXME
enable-active-high;
pinctrl-names = "default";
pinctrl-0 = <&flash_led_unlock>;
};

Notice:

+&pm8921_mpps {
+ flash_led_unlock: flash-led-unlock-state {
+ pins = "mpp4";
+ function = "digital";
+ output-low;
+ power-source = <PM8921_GPIO_S4>;
+ };

This seems completely unused in the current patch, but my addition
above uses it.

Yours,
Linus Walleij