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

[Linus Walleij]

Hi Rudraksha,

thanks for your patch!

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

> From: Rudraksha Gupta <guptarud@xxxxxxxxx>
>
> 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@xxxxxxxxx>

(...)

> +       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