* [PATCH 0/3] from my MSM8916-mainline:
@ 2026-10-16 14:43 Christoph Rudorff
2026-10-16 14:43 ` [PATCH 1/3] iio: light: gp2ap002: add led power supply Christoph Rudorff
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Christoph Rudorff @ 2026-10-16 14:43 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm,
Christoph Rudorff
Samsung J5 is equipped with GP2AP002 proximity sensor.
Unfortunally the driver wasn't working, no irq received.
We missed to power up the LED.
Now it works.
Signed-off-by: Christoph Rudorff <chris@rudorff.com>
---
Christoph Rudorff (3):
iio: light: gp2ap002: add led power supply
iio: light: Update DT bindings for GP2AP002
arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor
.../bindings/iio/light/sharp,gp2ap002.yaml | 3 ++
.../boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 19 ++++++++++
arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 40 ++++++++++++++++++++++
drivers/iio/light/gp2ap002.c | 28 +++++++++++++--
4 files changed, 87 insertions(+), 3 deletions(-)
---
base-commit: a2de78ee6ed8bf8cd99d177c74865a6e8c3cb6ea
change-id: 20261016-iio-light-gp2ap002-6bd553aecb4b
Best regards,
--
Christoph Rudorff <chris@rudorff.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] iio: light: gp2ap002: add led power supply
2026-10-16 14:43 [PATCH 0/3] from my MSM8916-mainline: Christoph Rudorff
@ 2026-10-16 14:43 ` Christoph Rudorff
2026-10-16 14:48 ` sashiko-bot
2026-10-17 0:05 ` Jonathan Cameron
2026-10-16 14:43 ` [PATCH 2/3] iio: light: Update DT bindings for GP2AP002 Christoph Rudorff
2026-10-16 14:43 ` [PATCH 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor Christoph Rudorff
2 siblings, 2 replies; 11+ messages in thread
From: Christoph Rudorff @ 2026-10-16 14:43 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm,
Christoph Rudorff
According to data sheet "Recommended external circuit"
this got up to three supplies, we missed VLED
Now it works (tested on msm8916-samsung-j5):
$ cd /sys/bus/iio/devices/iio\:device1
$ echo 1 > events/in_proximity_thresh_either_en
$ iio_event_monitor /dev/iio\:device1
Event: time: 1791460068846871297, type: proximity, channel: 0,
evtype: thresh, direction: rising
Event: time: 1791460068876669724, type: proximity, channel: 0,
evtype: thresh, direction: falling
...
Signed-off-by: Christoph Rudorff <chris@rudorff.com>
---
drivers/iio/light/gp2ap002.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index de29081fd3b2..c03eb0f1988b 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -129,6 +129,7 @@
* @dev: pointer to parent device
* @vdd: regulator controlling VDD
* @vio: regulator controlling VIO
+ * @vled: regulator controlling VLED
* @alsout: IIO ADC channel to convert the ALSOUT signal
* @hys_far: hysteresis control from device tree
* @hys_close: hysteresis control from device tree
@@ -142,6 +143,7 @@ struct gp2ap002 {
struct device *dev;
struct regulator *vdd;
struct regulator *vio;
+ struct regulator *vled;
struct iio_channel *alsout;
u8 hys_far;
u8 hys_close;
@@ -169,7 +171,7 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
if (val & GP2AP002_PROX_VO_DETECT) {
/* Close */
- dev_dbg(gp2ap002->dev, "close\n");
+ dev_dbg(gp2ap002->dev, "close %i\n", val);
ret = regmap_write(gp2ap002->map, GP2AP002_HYS,
gp2ap002->hys_far);
if (ret)
@@ -179,7 +181,7 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING);
} else {
/* Far */
- dev_dbg(gp2ap002->dev, "far\n");
+ dev_dbg(gp2ap002->dev, "far %i\n", val);
ret = regmap_write(gp2ap002->map, GP2AP002_HYS,
gp2ap002->hys_close);
if (ret)
@@ -530,14 +532,22 @@ static int gp2ap002_probe(struct i2c_client *client)
if (IS_ERR(gp2ap002->vio))
return dev_err_probe(dev, PTR_ERR(gp2ap002->vio),
"failed to get VIO regulator\n");
+ // returns a dummy if not declared:
+ gp2ap002->vled = devm_regulator_get(dev, "vled");
+ if (IS_ERR(gp2ap002->vled))
+ return dev_err_probe(dev, PTR_ERR(gp2ap002->vled),
+ "failed to get VLED regulator\n");
/* Operating voltage 2.4V .. 3.6V according to datasheet */
ret = regulator_set_voltage(gp2ap002->vdd, 2400000, 3600000);
if (ret) {
- dev_err(dev, "failed to sett VDD voltage\n");
+ dev_err(dev, "failed to set VDD voltage\n");
return ret;
}
+ /* LED voltage 2.4V .. 3.6V according to datasheet */
+ /* nothing to set: it's a simple "regulator-fixed" gpio pin */
+
/* VIO should be between 1.65V and VDD */
ret = regulator_get_voltage(gp2ap002->vdd);
if (ret < 0) {
@@ -560,6 +570,11 @@ static int gp2ap002_probe(struct i2c_client *client)
dev_err(dev, "failed to enable VIO regulator\n");
goto out_disable_vdd;
}
+ ret = regulator_enable(gp2ap002->vled);
+ if (ret) {
+ dev_err(dev, "failed to enable VLED regulator\n");
+ goto out_disable_vio;
+ }
msleep(20);
@@ -635,6 +650,7 @@ static void gp2ap002_remove(struct i2c_client *client)
iio_device_unregister(indio_dev);
regulator_disable(gp2ap002->vio);
regulator_disable(gp2ap002->vdd);
+ regulator_disable(gp2ap002->vled);
}
static int gp2ap002_runtime_suspend(struct device *dev)
@@ -659,6 +675,7 @@ static int gp2ap002_runtime_suspend(struct device *dev)
*/
regulator_disable(gp2ap002->vio);
regulator_disable(gp2ap002->vdd);
+ regulator_disable(gp2ap002->vled);
return 0;
}
@@ -679,6 +696,11 @@ static int gp2ap002_runtime_resume(struct device *dev)
dev_err(dev, "failed to enable VIO regulator in resume path\n");
goto out_disable_vdd;
}
+ ret = regulator_enable(gp2ap002->vled);
+ if (ret) {
+ dev_err(dev, "failed to enable VLED regulator in resume path\n");
+ goto out_disable_vio;
+ }
msleep(20);
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] iio: light: Update DT bindings for GP2AP002
2026-10-16 14:43 [PATCH 0/3] from my MSM8916-mainline: Christoph Rudorff
2026-10-16 14:43 ` [PATCH 1/3] iio: light: gp2ap002: add led power supply Christoph Rudorff
@ 2026-10-16 14:43 ` Christoph Rudorff
2026-10-16 14:45 ` Krzysztof Kozlowski
` (2 more replies)
2026-10-16 14:43 ` [PATCH 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor Christoph Rudorff
2 siblings, 3 replies; 11+ messages in thread
From: Christoph Rudorff @ 2026-10-16 14:43 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm,
Christoph Rudorff
add missing vled-supply
Signed-off-by: Christoph Rudorff <chris@rudorff.com>
---
Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml b/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml
index 99bddf31cbed..2b5025043168 100644
--- a/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml
+++ b/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml
@@ -33,6 +33,9 @@ properties:
vio-supply:
description: VIO power supply a phandle to a regulator
+ vled-supply:
+ description: LED power supply a phandle to a regulator
+
io-channels:
maxItems: 1
description: ALSOUT ADC channel to read the ambient light
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor
2026-10-16 14:43 [PATCH 0/3] from my MSM8916-mainline: Christoph Rudorff
2026-10-16 14:43 ` [PATCH 1/3] iio: light: gp2ap002: add led power supply Christoph Rudorff
2026-10-16 14:43 ` [PATCH 2/3] iio: light: Update DT bindings for GP2AP002 Christoph Rudorff
@ 2026-10-16 14:43 ` Christoph Rudorff
2026-10-16 15:11 ` Konrad Dybcio
2 siblings, 1 reply; 11+ messages in thread
From: Christoph Rudorff @ 2026-10-16 14:43 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm,
Christoph Rudorff
J5 got a gp2ap002 sensor, add it and it's supplies to the device tree
Signed-off-by: Christoph Rudorff <chris@rudorff.com>
---
.../boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 19 ++++++++++
arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 40 ++++++++++++++++++++++
2 files changed, 59 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
index 697f25d51d9d..83a5d2807f52 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
@@ -149,6 +149,18 @@ &pm8916_resin {
linux,code = <KEY_VOLUMEDOWN>;
};
+&pm8916_rpm_regulators {
+ pm8916_l16: l16 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ pm8916_l17: l17 {
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ };
+};
+
&sdhc_1 {
status = "okay";
};
@@ -248,6 +260,13 @@ sdc2_cd_default: sdc2-cd-default-state {
bias-disable;
};
+ proxy_int_default: proxy-int-default-state {
+ pins = "gpio113";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
sensors_i2c_default: sensors-i2c-default-state {
pins = "gpio31", "gpio32";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
index ba8650971d6a..136b948e46b4 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
@@ -8,6 +8,19 @@ / {
model = "Samsung Galaxy J5 (2015)";
compatible = "samsung,j5", "qcom,msm8916";
chassis-type = "handset";
+
+ reg_vdd_proxy: regulator-vdd-proximity {
+ compatible = "regulator-fixed";
+ regulator-name = "proximity_led";
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+
+ gpio = <&tlmm 73 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&proxy_ldo_en_default>;
+ pinctrl-names = "default";
+ };
};
&accelerometer {
@@ -19,6 +32,24 @@ &blsp_i2c5 {
status = "disabled";
};
+&i2c_sensors {
+ proximity-sensor@44 {
+ compatible = "sharp,gp2ap002s00f";
+ reg = <0x44>;
+ interrupt-parent = <&tlmm>;
+ interrupts = <113 IRQ_TYPE_EDGE_FALLING>;
+ vdd-supply = <&pm8916_l17>;
+ vled-supply = <®_vdd_proxy>;
+ vio-supply = <&pm8916_l5>;
+
+ sharp,proximity-far-hysteresis = /bits/ 8 <0x2f>;
+ sharp,proximity-close-hysteresis = /bits/ 8 <0x0f>;
+
+ pinctrl-0 = <&proxy_int_default>;
+ pinctrl-names = "default";
+ };
+};
+
&pm8916_codec {
qcom,micbias1-ext-cap;
};
@@ -30,3 +61,12 @@ &touchscreen {
&usb_hs_phy {
qcom,init-seq = /bits/ 8 <0x1 0x19 0x2 0x0b>;
};
+
+&tlmm {
+ proxy_ldo_en_default: proxy-ldo-en-default-state {
+ pins = "gpio73";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+};
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] iio: light: Update DT bindings for GP2AP002
2026-10-16 14:43 ` [PATCH 2/3] iio: light: Update DT bindings for GP2AP002 Christoph Rudorff
@ 2026-10-16 14:45 ` Krzysztof Kozlowski
2026-10-16 14:58 ` chr[]
2026-10-16 14:49 ` sashiko-bot
2026-10-16 21:01 ` Linus Walleij
2 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-16 14:45 UTC (permalink / raw)
To: Christoph Rudorff, Linus Walleij, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm
On 16/10/2026 16:43, Christoph Rudorff wrote:
> add missing vled-supply
Please use full sentences.
Why missing? Did you check datasheet?
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
Subject - everything is "update". Be specific. See git history for examples.
Please organize the patch documenting the bindings (DT bindings) before
the patch using that ABI.
See also:
https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] iio: light: gp2ap002: add led power supply
2026-10-16 14:43 ` [PATCH 1/3] iio: light: gp2ap002: add led power supply Christoph Rudorff
@ 2026-10-16 14:48 ` sashiko-bot
2026-10-17 0:05 ` Jonathan Cameron
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-10-16 14:48 UTC (permalink / raw)
To: Christoph Rudorff; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] iio: light: gp2ap002: memory/resource leak of vled regulator in error paths
- [Medium] iio: light: gp2ap002: incorrect regulator disable order violates hardware teardown sequencing
--
commit 377213d2096a5527d0fa1ddbd0a74bf96e0940f2
Author: Christoph Rudorff <chris@rudorff.com>
iio: light: gp2ap002: add led power supply
This commit adds a missing VLED power supply to the gp2ap002 driver.
It initializes the vled regulator and handles its enable and disable paths
in the probe, remove, suspend, and resume functions.
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index de29081fd3b20..c03eb0f1988bc 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
[ ... ]
> @@ -560,6 +570,11 @@ static int gp2ap002_probe(struct i2c_client *client)
> dev_err(dev, "failed to enable VIO regulator\n");
> goto out_disable_vdd;
> }
> + ret = regulator_enable(gp2ap002->vled);
> + if (ret) {
> + dev_err(dev, "failed to enable VLED regulator\n");
> + goto out_disable_vio;
> + }
>
> msleep(20);
[Severity: Medium]
When gp2ap002_init() or devm_request_threaded_irq() fail further down in
gp2ap002_probe(), the code jumps to out_disable_vio or out_put_pm. However,
the error handling block only disables the vio and vdd regulators.
For example, when initialization fails:
ret = gp2ap002_init(gp2ap002);
if (ret) {
dev_err(dev, "initialization failed\n");
goto out_disable_vio;
}
Does this regression leave the newly added vled regulator permanently enabled,
leaking its enable count? Should we add an out_disable_vled label that
disables the vled regulator?
[ ... ]
> @@ -635,6 +650,7 @@ static void gp2ap002_remove(struct i2c_client *client)
> iio_device_unregister(indio_dev);
> regulator_disable(gp2ap002->vio);
> regulator_disable(gp2ap002->vdd);
> + regulator_disable(gp2ap002->vled);
[Severity: Medium]
In gp2ap002_probe(), the regulators are enabled in the order: vdd -> vio ->
vled. To prevent powering peripherals while the core logic is unpowered
(which could lead to backpowering or leakage currents), should the teardown
occur in the exact reverse order?
Disabling them here as vio -> vdd -> vled seems to violate that sequence.
> }
>
> static int gp2ap002_runtime_suspend(struct device *dev)
> @@ -659,6 +675,7 @@ static int gp2ap002_runtime_suspend(struct device *dev)
> */
> regulator_disable(gp2ap002->vio);
> regulator_disable(gp2ap002->vdd);
> + regulator_disable(gp2ap002->vled);
[Severity: Medium]
As noted above in gp2ap002_remove(), does this disable sequence regression
also violate the hardware teardown ordering by disabling vled last instead
of first?
>
> return 0;
> }
> @@ -679,6 +696,11 @@ static int gp2ap002_runtime_resume(struct device *dev)
> dev_err(dev, "failed to enable VIO regulator in resume path\n");
> goto out_disable_vdd;
> }
> + ret = regulator_enable(gp2ap002->vled);
> + if (ret) {
> + dev_err(dev, "failed to enable VLED regulator in resume path\n");
> + goto out_disable_vio;
> + }
>
> msleep(20);
[Severity: Medium]
If gp2ap002_init() fails later in gp2ap002_runtime_resume(), the code jumps
to out_disable_vio:
ret = gp2ap002_init(gp2ap002);
if (ret) {
dev_err(dev, "re-initialization failed\n");
goto out_disable_vio;
}
Similar to the probe function, does this regression leak the enable count
for the vled regulator, since it is not disabled in the error path?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-iio-light-gp2ap002-v1-0-7c93312fa8d5@rudorff.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] iio: light: Update DT bindings for GP2AP002
2026-10-16 14:43 ` [PATCH 2/3] iio: light: Update DT bindings for GP2AP002 Christoph Rudorff
2026-10-16 14:45 ` Krzysztof Kozlowski
@ 2026-10-16 14:49 ` sashiko-bot
2026-10-16 21:01 ` Linus Walleij
2 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-10-16 14:49 UTC (permalink / raw)
To: Christoph Rudorff; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iio: gp2ap002: vled regulator leak in error paths
--
commit 999604192c18f52748ad72ac0d26233473334acf
Author: Christoph Rudorff <chris@rudorff.com>
iio: light: Update DT bindings for GP2AP002
This commit adds the missing vled-supply regulator to the device tree
bindings for the Sharp GP2AP002 sensor.
> diff --git a/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml b/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml
> index 99bddf31cbed2..2b5025043168a 100644
> --- a/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml
> @@ -33,6 +33,9 @@ properties:
> vio-supply:
> description: VIO power supply a phandle to a regulator
>
> + vled-supply:
> + description: LED power supply a phandle to a regulator
> +
> io-channels:
> maxItems: 1
> description: ALSOUT ADC channel to read the ambient light
While reviewing this binding addition, I noticed the corresponding driver
code might be missing cleanup for this regulator.
drivers/iio/light/gp2ap002.c:gp2ap002_probe() {
...
ret = regulator_enable(gp2ap002->vled);
if (ret) {
dev_err(dev, "failed to enable VLED regulator\n");
goto out_disable_vio;
}
msleep(20);
/*
* Initialize the device and signal to runtime PM that now we are
* definitely up and using power.
*/
ret = gp2ap002_init(gp2ap002);
if (ret) {
dev_err(dev, "initialization failed\n");
goto out_disable_vio;
}
...
out_disable_vio:
regulator_disable(gp2ap002->vio);
out_disable_vdd:
regulator_disable(gp2ap002->vdd);
return ret;
}
[Severity: Medium]
Does this code leak the vled regulator? If initialization fails, the error path
jumps to out_disable_vio, completely skipping the disablement of vled and
leaving it indefinitely enabled.
The same sequence appears during runtime resume:
drivers/iio/light/gp2ap002.c:gp2ap002_runtime_resume() {
...
ret = regulator_enable(gp2ap002->vled);
if (ret) {
dev_err(dev, "failed to enable VLED regulator in resume path\n");
goto out_disable_vio;
}
msleep(20);
ret = gp2ap002_init(gp2ap002);
if (ret) {
dev_err(dev, "re-initialization failed\n");
goto out_disable_vio;
}
...
out_disable_vio:
regulator_disable(gp2ap002->vio);
out_disable_vdd:
regulator_disable(gp2ap002->vdd);
return ret;
}
[Severity: Medium]
Does this code similarly leak the vled regulator during a resume failure?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-iio-light-gp2ap002-v1-0-7c93312fa8d5@rudorff.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] iio: light: Update DT bindings for GP2AP002
2026-10-16 14:45 ` Krzysztof Kozlowski
@ 2026-10-16 14:58 ` chr[]
0 siblings, 0 replies; 11+ messages in thread
From: chr[] @ 2026-10-16 14:58 UTC (permalink / raw)
To: Krzysztof Kozlowski, Linus Walleij, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm
Am 16.10.26 um 16:45 schrieb Krzysztof Kozlowski:
> On 16/10/2026 16:43, Christoph Rudorff wrote:
>> add missing vled-supply
>
> Please use full sentences.
>
> Why missing? Did you check datasheet?
Yes, I did. I mentioned that in the patch fixing the driver.
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
Well, I did ... ok, none of the three commit message subjects of that file
were well formatted.
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
> Subject - everything is "update". Be specific. See git history for examples.
>
> Please organize the patch documenting the bindings (DT bindings) before
> the patch using that ABI.
Will do!
regards,
chris
> See also:
> https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46
>
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor
2026-10-16 14:43 ` [PATCH 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor Christoph Rudorff
@ 2026-10-16 15:11 ` Konrad Dybcio
0 siblings, 0 replies; 11+ messages in thread
From: Konrad Dybcio @ 2026-10-16 15:11 UTC (permalink / raw)
To: Christoph Rudorff, Linus Walleij, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-iio, linux-kernel, devicetree, linux-arm-msm
On 10/16/26 4:43 PM, Christoph Rudorff wrote:
> J5 got a gp2ap002 sensor, add it and it's supplies to the device tree
>
> Signed-off-by: Christoph Rudorff <chris@rudorff.com>
> ---
> .../boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 19 ++++++++++
> arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 40 ++++++++++++++++++++++
It's odd for the regulator and pin setting to be common, but the
sensor to only be there for one of these devices
Konrad
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] iio: light: Update DT bindings for GP2AP002
2026-10-16 14:43 ` [PATCH 2/3] iio: light: Update DT bindings for GP2AP002 Christoph Rudorff
2026-10-16 14:45 ` Krzysztof Kozlowski
2026-10-16 14:49 ` sashiko-bot
@ 2026-10-16 21:01 ` Linus Walleij
2 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2026-10-16 21:01 UTC (permalink / raw)
To: Christoph Rudorff
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-iio, linux-kernel, devicetree, linux-arm-msm
Hi Christoph,
thanks for your patch!
On Fri, Oct 16, 2026 at 4:43 PM Christoph Rudorff <chris@rudorff.com> wrote:
> + vled-supply:
> + description: LED power supply a phandle to a regulator
What datasheet is this from?
My datasheet for the GP2AP002S00F clearly has only
VIO and VCC, enjoy:
https://dflund.se/~triad/krad/GP2AP002S00F.pdf
There is however two *outputs* from the GP2AP002S00F
called LEDA and LEDC.
These read "LED Anode" and "LED Cathode" and is where
the IR LED is supposed to be connected.
I'm just puzzled by this patch.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] iio: light: gp2ap002: add led power supply
2026-10-16 14:43 ` [PATCH 1/3] iio: light: gp2ap002: add led power supply Christoph Rudorff
2026-10-16 14:48 ` sashiko-bot
@ 2026-10-17 0:05 ` Jonathan Cameron
1 sibling, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-10-17 0:05 UTC (permalink / raw)
To: Christoph Rudorff
Cc: Linus Walleij, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-iio, linux-kernel, devicetree, linux-arm-msm
On Fri, 16 Oct 2026 16:43:06 +0200
Christoph Rudorff <chris@rudorff.com> wrote:
> According to data sheet "Recommended external circuit"
> this got up to three supplies, we missed VLED
>
> Now it works (tested on msm8916-samsung-j5):
>
> $ cd /sys/bus/iio/devices/iio\:device1
> $ echo 1 > events/in_proximity_thresh_either_en
>
> $ iio_event_monitor /dev/iio\:device1
>
> Event: time: 1791460068846871297, type: proximity, channel: 0,
> evtype: thresh, direction: rising
>
> Event: time: 1791460068876669724, type: proximity, channel: 0,
> evtype: thresh, direction: falling
>
> ...
>
> Signed-off-by: Christoph Rudorff <chris@rudorff.com>
A few comments inline if you resolve the question Linus raised.
> ---
> drivers/iio/light/gp2ap002.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index de29081fd3b2..c03eb0f1988b 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
> @@ -129,6 +129,7 @@
> * @dev: pointer to parent device
> * @vdd: regulator controlling VDD
> * @vio: regulator controlling VIO
> + * @vled: regulator controlling VLED
> * @alsout: IIO ADC channel to convert the ALSOUT signal
> * @hys_far: hysteresis control from device tree
> * @hys_close: hysteresis control from device tree
> @@ -142,6 +143,7 @@ struct gp2ap002 {
> struct device *dev;
> struct regulator *vdd;
> struct regulator *vio;
> + struct regulator *vled;
> struct iio_channel *alsout;
> u8 hys_far;
> u8 hys_close;
> @@ -169,7 +171,7 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
>
> if (val & GP2AP002_PROX_VO_DETECT) {
> /* Close */
> - dev_dbg(gp2ap002->dev, "close\n");
> + dev_dbg(gp2ap002->dev, "close %i\n", val);
Why?
> ret = regmap_write(gp2ap002->map, GP2AP002_HYS,
> gp2ap002->hys_far);
> if (ret)
> @@ -179,7 +181,7 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
> IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING);
> } else {
> /* Far */
> - dev_dbg(gp2ap002->dev, "far\n");
> + dev_dbg(gp2ap002->dev, "far %i\n", val);
Why?
> ret = regmap_write(gp2ap002->map, GP2AP002_HYS,
> gp2ap002->hys_close);
> if (ret)
> @@ -530,14 +532,22 @@ static int gp2ap002_probe(struct i2c_client *client)
> if (IS_ERR(gp2ap002->vio))
> return dev_err_probe(dev, PTR_ERR(gp2ap002->vio),
> "failed to get VIO regulator\n");
> + // returns a dummy if not declared:
See comment syntax elsewhere and match it.
> + gp2ap002->vled = devm_regulator_get(dev, "vled");
> + if (IS_ERR(gp2ap002->vled))
> + return dev_err_probe(dev, PTR_ERR(gp2ap002->vled),
> + "failed to get VLED regulator\n");
>
> /* Operating voltage 2.4V .. 3.6V according to datasheet */
> ret = regulator_set_voltage(gp2ap002->vdd, 2400000, 3600000);
> if (ret) {
> - dev_err(dev, "failed to sett VDD voltage\n");
> + dev_err(dev, "failed to set VDD voltage\n");
Good to fix, but unrelated so separate patch.
> return ret;
> }
>
> + /* LED voltage 2.4V .. 3.6V according to datasheet */
> + /* nothing to set: it's a simple "regulator-fixed" gpio pin */
Use multiline comment syntax.
> +
> /* VIO should be between 1.65V and VDD */
> ret = regulator_get_voltage(gp2ap002->vdd);
> if (ret < 0) {
> @@ -560,6 +570,11 @@ static int gp2ap002_probe(struct i2c_client *client)
> dev_err(dev, "failed to enable VIO regulator\n");
> goto out_disable_vdd;
> }
> + ret = regulator_enable(gp2ap002->vled);
> + if (ret) {
> + dev_err(dev, "failed to enable VLED regulator\n");
> + goto out_disable_vio;
> + }
>
> msleep(20);
>
> @@ -635,6 +650,7 @@ static void gp2ap002_remove(struct i2c_client *client)
> iio_device_unregister(indio_dev);
> regulator_disable(gp2ap002->vio);
> regulator_disable(gp2ap002->vdd);
> + regulator_disable(gp2ap002->vled);
There are enough of them that if this goes forward consider using
the bulk handling.
> }
>
> static int gp2ap002_runtime_suspend(struct device *dev)
> @@ -659,6 +675,7 @@ static int gp2ap002_runtime_suspend(struct device *dev)
> */
> regulator_disable(gp2ap002->vio);
> regulator_disable(gp2ap002->vdd);
> + regulator_disable(gp2ap002->vled);
>
> return 0;
> }
> @@ -679,6 +696,11 @@ static int gp2ap002_runtime_resume(struct device *dev)
> dev_err(dev, "failed to enable VIO regulator in resume path\n");
> goto out_disable_vdd;
> }
> + ret = regulator_enable(gp2ap002->vled);
> + if (ret) {
> + dev_err(dev, "failed to enable VLED regulator in resume path\n");
> + goto out_disable_vio;
> + }
>
> msleep(20);
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-10-17 0:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 14:43 [PATCH 0/3] from my MSM8916-mainline: Christoph Rudorff
2026-10-16 14:43 ` [PATCH 1/3] iio: light: gp2ap002: add led power supply Christoph Rudorff
2026-10-16 14:48 ` sashiko-bot
2026-10-17 0:05 ` Jonathan Cameron
2026-10-16 14:43 ` [PATCH 2/3] iio: light: Update DT bindings for GP2AP002 Christoph Rudorff
2026-10-16 14:45 ` Krzysztof Kozlowski
2026-10-16 14:58 ` chr[]
2026-10-16 14:49 ` sashiko-bot
2026-10-16 21:01 ` Linus Walleij
2026-10-16 14:43 ` [PATCH 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor Christoph Rudorff
2026-10-16 15:11 ` Konrad Dybcio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox