[PATCH v7 0/3] Samsung Expressatt: Camera Flash

[PATCH v7 0/3] Samsung Expressatt: Camera Flash

[Rudraksha Gupta via B4 Relay]

This small series adds camera flash to an existing similar mainline
driver and adds it to the Samsung Expressatt's DTS

// Tests

// # Navigate to LED
sudo su
cd /sys/class/leds/white:flash

// # Should stay at dim brightness
echo 1 > brightness
echo 1 > brightness
echo 1 > brightness
echo 1 > brightness
echo 0 > brightness           # LED_OFF

// # Max Brightness
echo 50 > brightness
echo 0 > brightness           # LED_OFF
echo 99 > brightness
echo 0 > brightness           # LED_OFF
echo 1000 > brightness
echo 0 > brightness           # LED_OFF
echo 100 > brightness
echo 0 > brightness           # LED_OFF

// # Should increase in brightness
for i in $(seq 1 16); do echo $i > brightness; sleep 1; done
echo 0 > brightness           # LED_OFF

// # Test flash strobe (rt8515_led_flash_strobe_set)
cat max_flash_timeout          # check max
echo 200000 > flash_timeout    # 200ms
echo 1 > flash_strobe          # strobe ON → brightness_commit + timer
cat flash_strobe               # should read 1, then 0 after timeout
sleep 1
cat flash_strobe               # should be 0 (timer fired)

// # Test manual strobe cancel
echo 1 > flash_strobe ; echo 0 > flash_strobe          # immediate off

// # Check regulator error handling
dmesg | tail -20                     # look for any "failed to turn off LED" msgs

// # Multiple strobes shouldn't cause errors
echo 200000 > flash_timeout
echo 1 > flash_strobe
echo 1 > flash_strobe
sleep 1
cat flash_strobe
dmesg | tail -20

// # Multiple strobes shouldn't cause errors (extreme test)
for i in $(seq 1 500); do echo 1 > flash_strobe; echo 0 > flash_strobe; done
dmesg | tail -20

// # Test minimal timeout (turns on briefly)
echo 1 > flash_timeout
echo 1 > flash_strobe
sleep 0.1
cat flash_strobe
dmesg | tail -20

// # Flash strobe overrides brightness and flashes at full brightness
echo 200000 > flash_timeout
echo 1 > brightness
echo 1 > flash_strobe
sleep 1
cat flash_strobe
cat brightness
dmesg | tail -20

Downstream reference:
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>
---
Changes in v7:
- address Lee's comments
- PM8921_MPP_S4 instead of PM8921_GPIO_S4 for the MPP4 power-source
- Link to v6: https://lore.kernel.org/r/20260527-expressatt_camera_flash-v6-0-de0f150024e4@gmail.com

Changes in v6:
- Address Lee Jones's comments:
    - formatting
    - use cancel_delayed_work_sync()
- Link to v5: https://lore.kernel.org/r/20260503-expressatt_camera_flash-v5-0-95524506a799@gmail.com

Changes in v5:
- Address Lee Jones's comments:
    - formatting
    - use devm_regulator_get_optional()
    - use a workqueue instead of a timer
    - previously we were validating ent-gpios xor vin-supply at probe,
        but was removed. update the commit msg to reflect this
- Link to v4: https://lore.kernel.org/r/20260331-expressatt_camera_flash-v4-0-f1e99f474513@gmail.com

Changes in v4:
- Driver:
    - revert function renames
    - add comment to use flash instead if torch pin not available
- Link to v3: https://lore.kernel.org/r/20260326-expressatt_camera_flash-v3-0-e75e5d58990f@gmail.com

Changes in v3:
- DTS:
    - Renamed and reordered nodes
- Driver:
    - Use regulator_is_enabled() instead of reg_enabled
    - remove ent xor vin check
    - remove rt->reg == -ENODEV check
    - rename functions to reflect what they do and added ret's
    - Fixed: LED was increasing in brightness when setting the same
        brightness multiple times
- Link to v2: https://lore.kernel.org/r/20260318-expressatt_camera_flash-v2-0-5c2b9a623dcb@gmail.com

Changes in v2:
- dt-bindings: Explain the hardware and not the driver
- **/*: Use vin-supply instead of unlock-gpio
- expressatt DTS: Reorder pinctrl-*
- expressatt DTS: Define rfs-ohms to a default (couldn't find
                  information about this)
- Link to v1: https://lore.kernel.org/r/20260306-expressatt_camera_flash-v1-0-b1996f7cdfdd@gmail.com

To: Lee Jones <lee@kernel.org>
To: Pavel Machek <pavel@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Linus Walleij <linusw@kernel.org>
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
Cc: linux-leds@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org

---
Rudraksha Gupta (3):
      dt-bindings: leds: rt8515: Support single-GPIO flash ICs with vin supply
      leds: flash: rt8515: Support single-GPIO flash ICs with vin supply
      ARM: dts: qcom: msm8960: expressatt: Add camera flash

 .../devicetree/bindings/leds/richtek,rt8515.yaml   |  34 ++++-
 .../dts/qcom/qcom-msm8960-samsung-expressatt.dts   |  44 ++++++
 drivers/leds/flash/leds-rt8515.c                   | 147 +++++++++++++++------
 3 files changed, 184 insertions(+), 41 deletions(-)
---
base-commit: 237a1c39e8dfd3e1c6f1f023eea37a48ec04cc63
change-id: 20260306-expressatt_camera_flash-13c15a7427aa
prerequisite-message-id: <20260527-expressatt-touchkey-v2-1-049dca41fc3a@gmail.com>
prerequisite-patch-id: 2b24e9ba47500ef05d38e37b40d8ca2dceb4716a

Best regards,
--  
Rudraksha Gupta <guptarud@gmail.com>

[PATCH v7 1/3] dt-bindings: leds: rt8515: Support single-GPIO flash ICs with vin supply

[Rudraksha Gupta via B4 Relay]

From: Rudraksha Gupta <guptarud@gmail.com>

Some flash ICs use the same one-wire pulse-count protocol as the RT8515
but have only a single enable line for both flash and torch modes, plus
an optional input voltage supply (e.g. a GPIO-controlled fixed
regulator) that gates power to the chip.

Make ent-gpios optional and add a vin-supply property to support these
variants. Add a oneOf constraint requiring exactly one of ent-gpios or
vin-supply. Add a binding example showing the single-GPIO configuration
with an input supply.

Assisted-by: Claude:claude-opus-4.6
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 .../devicetree/bindings/leds/richtek,rt8515.yaml   | 34 +++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml b/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml
index 0356371a6b01..ab3c5139132c 100644
--- a/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml
+++ b/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml
@@ -15,6 +15,10 @@ description: |
   current for each mode is defined in hardware using two resistors
   RFS and RTS.
 
+  Some flash ICs use the same one-wire pulse-count protocol but have
+  only a single enable line for both flash and torch modes. For these
+  single-channel variants, only enf-gpios is needed.
+
 properties:
   compatible:
     const: richtek,rt8515
@@ -26,6 +30,11 @@ properties:
   ent-gpios:
     maxItems: 1
     description: A connection to the 'ENT' (enable torch) pin.
+      Not present on single-channel flash ICs that use only one enable
+      line for both flash and torch modes.
+
+  vin-supply:
+    description: Optional input supply for the flash IC.
 
   richtek,rfs-ohms:
     minimum: 7680
@@ -81,10 +90,15 @@ properties:
 
 required:
   - compatible
-  - ent-gpios
   - enf-gpios
   - led
 
+oneOf:
+  - required:
+      - ent-gpios
+  - required:
+      - vin-supply
+
 additionalProperties: false
 
 examples:
@@ -108,4 +122,22 @@ examples:
         };
     };
 
+  - |
+    /* Single-channel flash IC with input supply */
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
+
+    led-controller {
+        compatible = "richtek,rt8515";
+        enf-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
+        vin-supply = <&flash_reg>;
+        richtek,rfs-ohms = <16000>;
+
+        led {
+            function = LED_FUNCTION_FLASH;
+            color = <LED_COLOR_ID_WHITE>;
+            flash-max-timeout-us = <250000>;
+        };
+    };
+
 ...

-- 
2.55.0

[PATCH v7 2/3] leds: flash: rt8515: Support single-GPIO flash ICs with vin supply

[Rudraksha Gupta via B4 Relay]

From: Rudraksha Gupta <guptarud@gmail.com>

Extend the RT8515 driver to support flash ICs that use only a single
GPIO for both flash and torch modes (no separate ENT pin), with an
optional vin regulator that gates power to the flash IC.

Assisted-by: Claude:claude-opus-4.6
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 drivers/leds/flash/leds-rt8515.c | 147 ++++++++++++++++++++++++++++-----------
 1 file changed, 107 insertions(+), 40 deletions(-)

diff --git a/drivers/leds/flash/leds-rt8515.c b/drivers/leds/flash/leds-rt8515.c
index 00904cc90ed6..541f61e13f01 100644
--- a/drivers/leds/flash/leds-rt8515.c
+++ b/drivers/leds/flash/leds-rt8515.c
@@ -30,6 +30,7 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
+#include <linux/workqueue.h>
 
 #include <media/v4l2-flash-led-class.h>
 
@@ -43,6 +44,9 @@
 #define RT8515_TIMEOUT_US	250000U
 #define RT8515_MAX_TIMEOUT_US	300000U
 
+#define RT8515_OFF		0
+#define RT8515_ON		1
+
 struct rt8515 {
 	struct led_classdev_flash fled;
 	struct device *dev;
@@ -51,7 +55,7 @@ struct rt8515 {
 	struct regulator *reg;
 	struct gpio_desc *enable_torch;
 	struct gpio_desc *enable_flash;
-	struct timer_list powerdown_timer;
+	struct delayed_work powerdown_work;
 	u32 max_timeout; /* Flash max timeout */
 	int flash_max_intensity;
 	int torch_max_intensity;
@@ -62,27 +66,50 @@ static struct rt8515 *to_rt8515(struct led_classdev_flash *fled)
 	return container_of(fled, struct rt8515, fled);
 }
 
-static void rt8515_gpio_led_off(struct rt8515 *rt)
+static int rt8515_gpio_led_off(struct rt8515 *rt)
 {
-	gpiod_set_value(rt->enable_flash, 0);
-	gpiod_set_value(rt->enable_torch, 0);
+	gpiod_set_value(rt->enable_flash, RT8515_OFF);
+	gpiod_set_value(rt->enable_torch, RT8515_OFF);
+
+	if (!rt->reg)
+		return 0;
+
+	return regulator_is_enabled(rt->reg) > 0 ? regulator_disable(rt->reg) : 0;
 }
 
-static void rt8515_gpio_brightness_commit(struct gpio_desc *gpiod,
-					  int brightness)
+static int rt8515_gpio_brightness_commit(struct rt8515 *rt, struct gpio_desc *gpiod, int brightness)
 {
-	int i;
+	int ret;
+
+	/*
+	 * Reset the IC to start brightness from zero,
+	 * then re-enable and pulse to the desired level.
+	 */
+	ret = rt8515_gpio_led_off(rt);
+	if (ret)
+		return ret;
+
+	/* IC needs time to reset its brightness counter */
+	usleep_range(100, 200);
+
+	if (rt->reg) {
+		ret = regulator_enable(rt->reg);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * Toggling a GPIO line with a small delay increases the
 	 * brightness one step at a time.
 	 */
-	for (i = 0; i < brightness; i++) {
-		gpiod_set_value(gpiod, 0);
+	for (int i = 0; i < brightness; i++) {
+		gpiod_set_value(gpiod, RT8515_OFF);
 		udelay(1);
-		gpiod_set_value(gpiod, 1);
+		gpiod_set_value(gpiod, RT8515_ON);
 		udelay(1);
 	}
+
+	return 0;
 }
 
 /* This is setting the torch light level */
@@ -91,23 +118,37 @@ static int rt8515_led_brightness_set(struct led_classdev *led,
 {
 	struct led_classdev_flash *fled = lcdev_to_flcdev(led);
 	struct rt8515 *rt = to_rt8515(fled);
+	int ret = 0;
 
 	mutex_lock(&rt->lock);
 
 	if (brightness == LED_OFF) {
-		/* Off */
-		rt8515_gpio_led_off(rt);
+		ret = rt8515_gpio_led_off(rt);
+		if (ret)
+			goto out;
 	} else if (brightness < RT8515_TORCH_MAX) {
-		/* Step it up to movie mode brightness using the flash pin */
-		rt8515_gpio_brightness_commit(rt->enable_torch, brightness);
+		/*
+		 * Step it up to movie mode brightness.
+		 * If there is no separate torch pin, use the flash pin
+		 * for torch as well.
+		 */
+		ret = rt8515_gpio_brightness_commit(rt,
+				rt->enable_torch ?: rt->enable_flash, brightness);
+		if (ret)
+			goto out;
 	} else {
-		/* Max torch brightness requested */
-		gpiod_set_value(rt->enable_torch, 1);
+		/*
+		 * Max torch brightness requested.
+		 * If there is no separate torch pin, use the flash pin
+		 * for torch as well.
+		 */
+		gpiod_set_value(rt->enable_torch ?: rt->enable_flash, RT8515_ON);
 	}
 
+out:
 	mutex_unlock(&rt->lock);
 
-	return 0;
+	return ret;
 }
 
 static int rt8515_led_flash_strobe_set(struct led_classdev_flash *fled,
@@ -116,27 +157,35 @@ static int rt8515_led_flash_strobe_set(struct led_classdev_flash *fled,
 	struct rt8515 *rt = to_rt8515(fled);
 	struct led_flash_setting *timeout = &fled->timeout;
 	int brightness = rt->flash_max_intensity;
+	int ret;
+
+	if (!state) {
+		cancel_delayed_work_sync(&rt->powerdown_work);
+		mutex_lock(&rt->lock);
+		ret = rt8515_gpio_led_off(rt);
+		if (!ret)
+			fled->led_cdev.brightness = LED_OFF;
+		mutex_unlock(&rt->lock);
+		return ret;
+	}
 
 	mutex_lock(&rt->lock);
 
-	if (state) {
-		/* Enable LED flash mode and set brightness */
-		rt8515_gpio_brightness_commit(rt->enable_flash, brightness);
-		/* Set timeout */
-		mod_timer(&rt->powerdown_timer,
-			  jiffies + usecs_to_jiffies(timeout->val));
-	} else {
-		timer_delete_sync(&rt->powerdown_timer);
-		/* Turn the LED off */
-		rt8515_gpio_led_off(rt);
-	}
+	/* Enable LED flash mode and set brightness */
+	ret = rt8515_gpio_brightness_commit(rt, rt->enable_flash, brightness);
+	if (ret)
+		goto out;
+
+	/* Set timeout */
+	schedule_delayed_work(&rt->powerdown_work, usecs_to_jiffies(timeout->val));
 
 	fled->led_cdev.brightness = LED_OFF;
 	/* After this the torch LED will be disabled */
 
+out:
 	mutex_unlock(&rt->lock);
 
-	return 0;
+	return ret;
 }
 
 static int rt8515_led_flash_strobe_get(struct led_classdev_flash *fled,
@@ -144,7 +193,7 @@ static int rt8515_led_flash_strobe_get(struct led_classdev_flash *fled,
 {
 	struct rt8515 *rt = to_rt8515(fled);
 
-	*state = timer_pending(&rt->powerdown_timer);
+	*state = delayed_work_pending(&rt->powerdown_work);
 
 	return 0;
 }
@@ -162,12 +211,17 @@ static const struct led_flash_ops rt8515_flash_ops = {
 	.timeout_set = rt8515_led_flash_timeout_set,
 };
 
-static void rt8515_powerdown_timer(struct timer_list *t)
+static void rt8515_powerdown_work(struct work_struct *work)
 {
-	struct rt8515 *rt = timer_container_of(rt, t, powerdown_timer);
+	struct rt8515 *rt = container_of(work, struct rt8515, powerdown_work.work);
+	int ret;
+
+	mutex_lock(&rt->lock);
+	ret = rt8515_gpio_led_off(rt);
+	mutex_unlock(&rt->lock);
 
-	/* Turn the LED off */
-	rt8515_gpio_led_off(rt);
+	if (ret)
+		dev_err(rt->dev, "failed to turn off LED (%d)\n", ret);
 }
 
 static void rt8515_init_flash_timeout(struct rt8515 *rt)
@@ -297,11 +351,19 @@ static int rt8515_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(rt->enable_flash),
 				     "cannot get ENF (enable flash) GPIO\n");
 
-	/* ENT - Enable Torch line */
-	rt->enable_torch = devm_gpiod_get(dev, "ent", GPIOD_OUT_LOW);
+	/* ENT - Enable Torch line (optional for single-GPIO flash ICs) */
+	rt->enable_torch = devm_gpiod_get_optional(dev, "ent", GPIOD_OUT_LOW);
 	if (IS_ERR(rt->enable_torch))
 		return dev_err_probe(dev, PTR_ERR(rt->enable_torch),
-				     "cannot get ENT (enable torch) GPIO\n");
+				     "Failed to obtain the Enable Torch GPIO\n");
+
+	rt->reg = devm_regulator_get_optional(dev, "vin");
+	if (IS_ERR(rt->reg)) {
+		ret = PTR_ERR(rt->reg);
+		if (ret != -ENODEV)
+			return dev_err_probe(dev, ret, "Failed to obtain the input supply\n");
+		rt->reg = NULL;
+	}
 
 	child = device_get_next_child_node(dev, NULL);
 	if (!child) {
@@ -327,12 +389,17 @@ static int rt8515_probe(struct platform_device *pdev)
 		dev_warn(dev,
 			 "flash-max-timeout-us property missing\n");
 	}
-	timer_setup(&rt->powerdown_timer, rt8515_powerdown_timer, 0);
+	INIT_DELAYED_WORK(&rt->powerdown_work, rt8515_powerdown_work);
 	rt8515_init_flash_timeout(rt);
 
 	fled->ops = &rt8515_flash_ops;
 
-	led->max_brightness = rt->torch_max_intensity;
+	/*
+	 * If there is no separate torch pin, use flash max intensity
+	 * as max brightness instead.
+	 */
+	led->max_brightness = rt->enable_torch ?
+		rt->torch_max_intensity : rt->flash_max_intensity;
 	led->brightness_set_blocking = rt8515_led_brightness_set;
 	led->flags |= LED_CORE_SUSPENDRESUME | LED_DEV_CAP_FLASH;
 
@@ -371,7 +438,7 @@ static void rt8515_remove(struct platform_device *pdev)
 	struct rt8515 *rt = platform_get_drvdata(pdev);
 
 	rt8515_v4l2_flash_release(rt);
-	timer_delete_sync(&rt->powerdown_timer);
+	cancel_delayed_work_sync(&rt->powerdown_work);
 	mutex_destroy(&rt->lock);
 }
 

-- 
2.55.0

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

[Rudraksha Gupta via B4 Relay]

From: Rudraksha Gupta <guptarud@gmail.com>

Add camera flash support for the Samsung Galaxy Express (expressatt).

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

Assisted-by: Claude:claude-opus-4.6
Reviewed-by: David Heidelberg <david@ixit.cz>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 .../dts/qcom/qcom-msm8960-samsung-expressatt.dts   | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
index 5a3d88e31cf5..b7218ee1349f 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8960-samsung-expressatt.dts
@@ -1,11 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/reset/qcom,gcc-msm8960.h>
 
 #include "qcom-msm8960.dtsi"
 #include "pm8921.dtsi"
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/pinctrl/qcom,pmic-mpp.h>
 #include <dt-bindings/input/gpio-keys.h>
 
 / {
@@ -61,6 +63,32 @@ touchkey_enable: touchkey-enable {
 		regulator-boot-on;
 	};
 
+	vreg_flash: regulator-flash {
+		compatible = "regulator-fixed";
+		regulator-name = "VREG_FLASH_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pm8921_mpps 4 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		pinctrl-0 = <&flash_led_unlock>;
+		pinctrl-names = "default";
+	};
+
+	led-controller {
+		compatible = "richtek,rt8515";
+		enf-gpios = <&tlmm 3 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&vreg_flash>;
+		richtek,rfs-ohms = <16000>;
+		pinctrl-0 = <&cam_flash_en>;
+		pinctrl-names = "default";
+
+		led {
+			function = LED_FUNCTION_FLASH;
+			color = <LED_COLOR_ID_WHITE>;
+			flash-max-timeout-us = <250000>;
+		};
+	};
+
 	i2c-gpio-touchkey {
 		compatible = "i2c-gpio";
 		#address-cells = <1>;
@@ -171,6 +199,13 @@ touchscreen@4a {
 };
 
 &tlmm {
+	cam_flash_en: cam-flash-en-state {
+		pins = "gpio3";
+		function = "gpio";
+		drive-strength = <16>;
+		bias-pull-down;
+	};
+
 	spi1_default: spi1-default-state {
 		mosi-pins {
 			pins = "gpio6";
@@ -571,3 +606,12 @@ magnetometer@2e {
 		/* TODO: Figure out Mount Matrix */
 	};
 };
+
+&pm8921_mpps {
+	flash_led_unlock: flash-led-unlock-state {
+		pins = "mpp4";
+		function = "digital";
+		output-low;
+		power-source = <PM8921_MPP_S4>;
+	};
+};

-- 
2.55.0

Re: [PATCH v7 1/3] dt-bindings: leds: rt8515: Support single-GPIO flash ICs with vin supply

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `oneOf` constraint incorrectly makes `ent-gpios` and `vin-supply` mutually exclusive and strictly requires exactly one to be present, contradicting the claim that `vin-supply` is optional and artificially restricting valid hardware configurations.
--

commit 72463777ee1c18e7913b350d5d2ad857810ece89
Author: Rudraksha Gupta <guptarud@gmail.com>
Subject: dt-bindings: leds: rt8515: Support single-GPIO flash ICs with vin supply

This commit adds support for single-channel variants of the RT8515 flash
ICs. It makes ent-gpios optional and introduces a vin-supply property,
adding a oneOf constraint to manage their presence.

> diff --git a/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml b/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml
> index 0356371a6b014..ab3c5139132cc 100644
> --- a/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml
> +++ b/Documentation/devicetree/bindings/leds/richtek,rt8515.yaml

[ ... ]

> @@ -81,10 +90,15 @@ properties:
>  
>  required:
>    - compatible
> -  - ent-gpios
>    - enf-gpios
>    - led
>  
> +oneOf:
> +  - required:
> +      - ent-gpios
> +  - required:
> +      - vin-supply
> +

[Severity: Medium]
Does this oneOf constraint incorrectly restrict valid hardware configurations?

The oneOf constraint strictly requires exactly one of its subschemas to match.
If a single-channel IC (which naturally lacks ent-gpios) does not use the
explicitly optional vin-supply regulator, zero subschemas will match, causing
schema validation to fail.

Conversely, if a standard dual-channel IC (which requires ent-gpios) is also
powered by a vin-supply regulator, two subschemas will match. This will also
fail validation due to the strict exclusivity of oneOf.

Should this be modeled differently to allow vin-supply to be truly optional
while safely permitting single-channel configurations?

>  additionalProperties: false
>  
>  examples:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-expressatt_camera_flash-v7-0-e1fd2edc8d84@gmail.com?part=1

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

[sashiko-bot]

> From: Rudraksha Gupta <guptarud@gmail.com>
> 
> Add camera flash support for the Samsung Galaxy Express (expressatt).
> 
> 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
> 
> Assisted-by: Claude:claude-opus-4.6
> Reviewed-by: David Heidelberg <david@ixit.cz>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-expressatt_camera_flash-v7-0-e1fd2edc8d84@gmail.com?part=3

Re: [PATCH v7 2/3] leds: flash: rt8515: Support single-GPIO flash ICs with vin supply

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Regulator is not enabled when requesting maximum torch brightness, preventing the LED from turning on.
- [High] Replacing mod_timer with schedule_delayed_work silently drops updates to the strobe timeout and causes premature LED power down.
- [Medium] Unbalanced regulator disable due to reliance on the global/physical regulator state instead of the driver's consumer enable count.

Pre-existing issues:
- [Critical] Premature destruction of mutex and cancellation of workqueue during device removal while the LED class device is still registered, leading to a Use-After-Free.
--

commit 6a964f82a818f57f5a1a31366f614609bc9c5008
Author: Rudraksha Gupta <guptarud@gmail.com>

leds: flash: rt8515: Support single-GPIO flash ICs with vin supply

This commit extends the RT8515 driver to support flash ICs that use
only a single GPIO for both flash and torch modes, alongside an
optional vin regulator that gates power to the flash IC.

> diff --git a/drivers/leds/flash/leds-rt8515.c b/drivers/leds/flash/leds-rt8515.c
> index 00904cc90ed6e..541f61e13f018 100644
> --- a/drivers/leds/flash/leds-rt8515.c
> +++ b/drivers/leds/flash/leds-rt8515.c
[ ... ]
> @@ -62,27 +66,50 @@ static struct rt8515 *to_rt8515(struct led_classdev_flash *fled)
>  	return container_of(fled, struct rt8515, fled);
>  }
>  
> -static void rt8515_gpio_led_off(struct rt8515 *rt)
> +static int rt8515_gpio_led_off(struct rt8515 *rt)
>  {
> -	gpiod_set_value(rt->enable_flash, 0);
> -	gpiod_set_value(rt->enable_torch, 0);
> +	gpiod_set_value(rt->enable_flash, RT8515_OFF);
> +	gpiod_set_value(rt->enable_torch, RT8515_OFF);
> +
> +	if (!rt->reg)
> +		return 0;
> +
> +	return regulator_is_enabled(rt->reg) > 0 ? regulator_disable(rt->reg) : 0;

[Severity: Medium]
Since regulator_is_enabled() returns the global physical state rather than
the per-consumer enable count, could this lead to an unbalanced disable if
the regulator is marked always-on or shared with another consumer?

Because rt8515_gpio_led_off() is reachable from user-controllable sysfs
attributes via rt8515_led_brightness_set(), repeated calls when the driver
has not enabled the regulator might cause the regulator core to throw a
warning and return -EIO. Should the driver maintain its own boolean state
to track whether it has enabled the regulator?

>  }
[ ... ]
> @@ -91,23 +118,37 @@ static int rt8515_led_brightness_set(struct led_classdev *led,
>  {
>  	struct led_classdev_flash *fled = lcdev_to_flcdev(led);
>  	struct rt8515 *rt = to_rt8515(fled);
> +	int ret = 0;
>  
>  	mutex_lock(&rt->lock);
>  
>  	if (brightness == LED_OFF) {
> -		/* Off */
> -		rt8515_gpio_led_off(rt);
> +		ret = rt8515_gpio_led_off(rt);
> +		if (ret)
> +			goto out;
>  	} else if (brightness < RT8515_TORCH_MAX) {
> -		/* Step it up to movie mode brightness using the flash pin */
> -		rt8515_gpio_brightness_commit(rt->enable_torch, brightness);
> +		/*
> +		 * Step it up to movie mode brightness.
> +		 * If there is no separate torch pin, use the flash pin
> +		 * for torch as well.
> +		 */
> +		ret = rt8515_gpio_brightness_commit(rt,
> +				rt->enable_torch ?: rt->enable_flash, brightness);
> +		if (ret)
> +			goto out;
>  	} else {
> -		/* Max torch brightness requested */
> -		gpiod_set_value(rt->enable_torch, 1);
> +		/*
> +		 * Max torch brightness requested.
> +		 * If there is no separate torch pin, use the flash pin
> +		 * for torch as well.
> +		 */
> +		gpiod_set_value(rt->enable_torch ?: rt->enable_flash, RT8515_ON);

[Severity: High]
If a transition is made from LED_OFF directly to max torch brightness, the
optional vin regulator remains disabled because
rt8515_gpio_brightness_commit() is completely bypassed here. Does this mean
the max torch state might silently fail to power the IC?

>  	}
>  
> +out:
>  	mutex_unlock(&rt->lock);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int rt8515_led_flash_strobe_set(struct led_classdev_flash *fled,
> @@ -116,27 +157,35 @@ static int rt8515_led_flash_strobe_set(struct led_classdev_flash *fled,
[ ... ]
> -	if (state) {
> -		/* Enable LED flash mode and set brightness */
> -		rt8515_gpio_brightness_commit(rt->enable_flash, brightness);
> -		/* Set timeout */
> -		mod_timer(&rt->powerdown_timer,
> -			  jiffies + usecs_to_jiffies(timeout->val));
> -	} else {
> -		timer_delete_sync(&rt->powerdown_timer);
> -		/* Turn the LED off */
> -		rt8515_gpio_led_off(rt);
> -	}
> +	/* Enable LED flash mode and set brightness */
> +	ret = rt8515_gpio_brightness_commit(rt, rt->enable_flash, brightness);
> +	if (ret)
> +		goto out;
> +
> +	/* Set timeout */
> +	schedule_delayed_work(&rt->powerdown_work, usecs_to_jiffies(timeout->val));

[Severity: High]
Does schedule_delayed_work() correctly handle consecutive flash strobe
commands? Unlike mod_timer(), schedule_delayed_work() silently ignores
updates if the work is already pending. This could cause consecutive V4L2
flash strobes to fail to extend the timeout, shutting down the flash
prematurely.

Additionally, if the delayed work is actively executing and blocked waiting
on rt->lock, its pending bit is cleared. In that scenario, could
schedule_delayed_work() queue a duplicate instance of the work that
immediately shuts down the LED as soon as the lock is released? Would using
mod_delayed_work() resolve these state management issues?

>  
>  	fled->led_cdev.brightness = LED_OFF;
>  	/* After this the torch LED will be disabled */
>  
> +out:
>  	mutex_unlock(&rt->lock);
>  
> -	return 0;
> +	return ret;
>  }
[ ... ]
> @@ -327,12 +389,17 @@ static int rt8515_probe(struct platform_device *pdev)
[ ... ]
>  	platform_set_drvdata(pdev, rt);
>  
>  	ret = devm_led_classdev_flash_register_ext(dev, fled, &init_data);

[Severity: Critical]
This is a pre-existing issue, but registering the LED class device via the
devm API schedules its sysfs interfaces to be unregistered after
rt8515_remove() returns.

[ ... ]
> @@ -371,7 +438,7 @@ static void rt8515_remove(struct platform_device *pdev)
>  	struct rt8515 *rt = platform_get_drvdata(pdev);
>  
>  	rt8515_v4l2_flash_release(rt);
> -	timer_delete_sync(&rt->powerdown_timer);
> +	cancel_delayed_work_sync(&rt->powerdown_work);
>  	mutex_destroy(&rt->lock);

Because devres destroys the sysfs interfaces after this function returns,
could a concurrent user-space write to sysfs (like triggering a flash
strobe) occur during this window? This would cause the sysfs handler to
attempt to lock the destroyed mutex and queue work on the freed device
structure, leading to a use-after-free. Should devm_add_action_or_reset()
be used here instead to ensure the work and mutex are only destroyed after
the devm-managed LED registration is cleanly torn down?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-expressatt_camera_flash-v7-0-e1fd2edc8d84@gmail.com?part=2