* [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
* 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 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
* [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
* 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 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 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: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
* [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 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
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.