[PATCH v3 0/3] MSM8916: improve gp2ap002 proximity sensor v3

[PATCH v3 0/3] MSM8916: improve gp2ap002 proximity sensor v3

[Christoph Rudorff]

Samsung J5 is equipped with GP2AP002 proximity sensor.
Unfortunally the driver wasn't working, no irq received.

According to datasheet page 8 "Recommended external circuit",
it got tree pins to supply: vio, vdd and vled->leda.

It missed to power up the LED.

I found this looking in downstream kernel, where
several #ifdefs handled the case of 2 or 3 supplies.

Now it works. Tested on 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>
Assisted-by: gpio-hammer
---
Changes in v3:
- rename vled leda
- j5 dt: moved declarations from j5-common
- Link to v2: https://patch.msgid.link/20261017-iio-light-gp2ap002-v2-0-d6879767a446@rudorff.com

Changes in v2:
- reorganized patch serie
- gp2ap002: use regulator bulk framework
- Link to v1: https://patch.msgid.link/20261016-iio-light-gp2ap002-v1-0-7c93312fa8d5@rudorff.com

To: Linus Walleij <linusw@kernel.org>
To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org

---
Christoph Rudorff (3):
      dt-bindings: iio: light: gp2ap002: add leda power supply
      iio: light: gp2ap002: add led power supply
      arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor

 .../bindings/iio/light/sharp,gp2ap002.yaml         |  3 +
 arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts    | 52 ++++++++++++++
 drivers/iio/light/gp2ap002.c                       | 84 ++++++++++------------
 3 files changed, 93 insertions(+), 46 deletions(-)
---
base-commit: a2de78ee6ed8bf8cd99d177c74865a6e8c3cb6ea
change-id: 20261016-iio-light-gp2ap002-6bd553aecb4b

Best regards,
--  
Christoph Rudorff <chris@rudorff.com>

[PATCH v3 1/3] dt-bindings: iio: light: gp2ap002: add leda power supply

[Christoph Rudorff]

According to datasheet page 8 "Recommended external circuit",
it has tree pins that need power: vio, vdd and leda ("vled" !)

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..643021f1d820 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
 
+  leda-supply:
+    description: LED anode power supply a phandle to a regulator
+
   io-channels:
     maxItems: 1
     description: ALSOUT ADC channel to read the ambient light

-- 
2.39.5

Re: [PATCH v3 1/3] dt-bindings: iio: light: gp2ap002: add leda power supply

[Linus Walleij]

On Tue, Oct 20, 2026 at 1:36 PM Christoph Rudorff <chris@rudorff.com> wrote:

> According to datasheet page 8 "Recommended external circuit",
> it has tree pins that need power: vio, vdd and leda ("vled" !)
>
> Signed-off-by: Christoph Rudorff <chris@rudorff.com>

Looking good!
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH v3 1/3] dt-bindings: iio: light: gp2ap002: add leda power supply

[Krzysztof Kozlowski]

On Tue, Oct 20, 2026 at 01:35:38PM +0200, Christoph Rudorff wrote:
> According to datasheet page 8 "Recommended external circuit",
> it has tree pins that need power: vio, vdd and leda ("vled" !)
> 
> Signed-off-by: Christoph Rudorff <chris@rudorff.com>
> ---
>  Documentation/devicetree/bindings/iio/light/sharp,gp2ap002.yaml | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH v3 2/3] iio: light: gp2ap002: add led power supply

[Christoph Rudorff]

tested on samsung-j5

v2: Use regulator bulk api.
v3: pretty up, rename vled to leda

Signed-off-by: Christoph Rudorff <chris@rudorff.com>
---
 drivers/iio/light/gp2ap002.c | 84 ++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 46 deletions(-)

diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index de29081fd3b2..551792ab3972 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -123,12 +123,18 @@
 #define GP2AP002_CON_OCON_HIGH		(0x3 << GP2AP002_CON_OCON_SHIFT)
 #define GP2AP002_CON_OCON_MASK		(0x3 << GP2AP002_CON_OCON_SHIFT)
 
+enum {
+	SUPPLY_VIO,
+	SUPPLY_VDD,
+	SUPPLY_LEDA,
+	NUM_SUPPLIES
+};
+
 /**
  * struct gp2ap002 - GP2AP002 state
  * @map: regmap pointer for the i2c regmap
  * @dev: pointer to parent device
- * @vdd: regulator controlling VDD
- * @vio: regulator controlling VIO
+ * @supplies: bulk regulator data
  * @alsout: IIO ADC channel to convert the ALSOUT signal
  * @hys_far: hysteresis control from device tree
  * @hys_close: hysteresis control from device tree
@@ -140,8 +146,7 @@
 struct gp2ap002 {
 	struct regmap *map;
 	struct device *dev;
-	struct regulator *vdd;
-	struct regulator *vio;
+	struct regulator_bulk_data supplies[NUM_SUPPLIES];
 	struct iio_channel *alsout;
 	u8 hys_far;
 	u8 hys_close;
@@ -444,6 +449,11 @@ static int gp2ap002_probe(struct i2c_client *client)
 	struct regmap *regmap;
 	int num_chan;
 	const char *compat;
+	static const char *const gp2ap002_supply_names[NUM_SUPPLIES] = {
+		[SUPPLY_VIO] = "vio",
+		[SUPPLY_VDD] = "vdd",
+		[SUPPLY_LEDA] = "leda",
+	};
 	u8 val;
 	int ret;
 
@@ -521,45 +531,38 @@ static int gp2ap002_probe(struct i2c_client *client)
 		}
 	}
 
-	gp2ap002->vdd = devm_regulator_get(dev, "vdd");
-	if (IS_ERR(gp2ap002->vdd))
-		return dev_err_probe(dev, PTR_ERR(gp2ap002->vdd),
-				     "failed to get VDD regulator\n");
+	regulator_bulk_set_supply_names(gp2ap002->supplies,
+					gp2ap002_supply_names,
+					NUM_SUPPLIES);
 
-	gp2ap002->vio = devm_regulator_get(dev, "vio");
-	if (IS_ERR(gp2ap002->vio))
-		return dev_err_probe(dev, PTR_ERR(gp2ap002->vio),
-				     "failed to get VIO regulator\n");
+	ret = devm_regulator_bulk_get(dev, NUM_SUPPLIES, gp2ap002->supplies);
+
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
 
-	/* Operating voltage 2.4V .. 3.6V according to datasheet */
-	ret = regulator_set_voltage(gp2ap002->vdd, 2400000, 3600000);
+	/* vdd operating voltage 2.4V .. 3.6V according to datasheet */
+	ret = regulator_set_voltage(gp2ap002->supplies[SUPPLY_VDD].consumer, 2400000, 3600000);
 	if (ret) {
-		dev_err(dev, "failed to sett VDD voltage\n");
+		dev_err(dev, "failed to set VDD voltage\n");
 		return ret;
 	}
-
 	/* VIO should be between 1.65V and VDD */
-	ret = regulator_get_voltage(gp2ap002->vdd);
+	ret = regulator_get_voltage(gp2ap002->supplies[SUPPLY_VDD].consumer);
 	if (ret < 0) {
 		dev_err(dev, "failed to get VDD voltage\n");
 		return ret;
 	}
-	ret = regulator_set_voltage(gp2ap002->vio, 1650000, ret);
+	ret = regulator_set_voltage(gp2ap002->supplies[SUPPLY_VIO].consumer, 1650000, ret);
 	if (ret) {
 		dev_err(dev, "failed to set VIO voltage\n");
 		return ret;
 	}
 
-	ret = regulator_enable(gp2ap002->vdd);
+	ret = regulator_bulk_enable(NUM_SUPPLIES, gp2ap002->supplies);
 	if (ret) {
-		dev_err(dev, "failed to enable VDD regulator\n");
+		dev_err(dev, "failed to enable regulators\n");
 		return ret;
 	}
-	ret = regulator_enable(gp2ap002->vio);
-	if (ret) {
-		dev_err(dev, "failed to enable VIO regulator\n");
-		goto out_disable_vdd;
-	}
 
 	msleep(20);
 
@@ -570,7 +573,7 @@ static int gp2ap002_probe(struct i2c_client *client)
 	ret = gp2ap002_init(gp2ap002);
 	if (ret) {
 		dev_err(dev, "initialization failed\n");
-		goto out_disable_vio;
+		goto out_disable_power;
 	}
 	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
@@ -616,10 +619,8 @@ static int gp2ap002_probe(struct i2c_client *client)
 	pm_runtime_put_noidle(dev);
 out_disable_pm:
 	pm_runtime_disable(dev);
-out_disable_vio:
-	regulator_disable(gp2ap002->vio);
-out_disable_vdd:
-	regulator_disable(gp2ap002->vdd);
+out_disable_power:
+	regulator_bulk_disable(NUM_SUPPLIES, gp2ap002->supplies);
 	return ret;
 }
 
@@ -633,8 +634,7 @@ static void gp2ap002_remove(struct i2c_client *client)
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
 	iio_device_unregister(indio_dev);
-	regulator_disable(gp2ap002->vio);
-	regulator_disable(gp2ap002->vdd);
+	regulator_bulk_disable(NUM_SUPPLIES, gp2ap002->supplies);
 }
 
 static int gp2ap002_runtime_suspend(struct device *dev)
@@ -657,8 +657,7 @@ static int gp2ap002_runtime_suspend(struct device *dev)
 	 * As these regulators may be shared, at least we are now in
 	 * sleep even if the regulators aren't really turned off.
 	 */
-	regulator_disable(gp2ap002->vio);
-	regulator_disable(gp2ap002->vdd);
+	regulator_bulk_disable(NUM_SUPPLIES, gp2ap002->supplies);
 
 	return 0;
 }
@@ -669,23 +668,18 @@ static int gp2ap002_runtime_resume(struct device *dev)
 	struct gp2ap002 *gp2ap002 = iio_priv(indio_dev);
 	int ret;
 
-	ret = regulator_enable(gp2ap002->vdd);
+	ret = regulator_bulk_enable(NUM_SUPPLIES, gp2ap002->supplies);
 	if (ret) {
-		dev_err(dev, "failed to enable VDD regulator in resume path\n");
+		dev_err(dev, "failed to enable regulators in resume path\n");
 		return ret;
 	}
-	ret = regulator_enable(gp2ap002->vio);
-	if (ret) {
-		dev_err(dev, "failed to enable VIO regulator in resume path\n");
-		goto out_disable_vdd;
-	}
 
 	msleep(20);
 
 	ret = gp2ap002_init(gp2ap002);
 	if (ret) {
 		dev_err(dev, "re-initialization failed\n");
-		goto out_disable_vio;
+		goto out_disable_power;
 	}
 
 	/* Re-activate the IRQ */
@@ -693,10 +687,8 @@ static int gp2ap002_runtime_resume(struct device *dev)
 
 	return 0;
 
-out_disable_vio:
-	regulator_disable(gp2ap002->vio);
-out_disable_vdd:
-	regulator_disable(gp2ap002->vdd);
+out_disable_power:
+	regulator_bulk_disable(NUM_SUPPLIES, gp2ap002->supplies);
 	return ret;
 }
 

-- 
2.39.5

Re: [PATCH v3 2/3] iio: light: gp2ap002: add led power supply

[Linus Walleij]

On Tue, Oct 20, 2026 at 1:36 PM Christoph Rudorff <chris@rudorff.com> wrote:

> tested on samsung-j5
>
> v2: Use regulator bulk api.
> v3: pretty up, rename vled to leda
>
> Signed-off-by: Christoph Rudorff <chris@rudorff.com>

I think that commit message needs a bit of elaboration ;)

With that fixed:
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH v3 2/3] iio: light: gp2ap002: add led power supply

[Jonathan Cameron]

On Tue, 20 Oct 2026 19:15:53 +0200
Linus Walleij <linusw@kernel.org> wrote:

> On Tue, Oct 20, 2026 at 1:36 PM Christoph Rudorff <chris@rudorff.com> wrote:
> 
> > tested on samsung-j5
> >
> > v2: Use regulator bulk api.
> > v3: pretty up, rename vled to leda
Change log goes after ---

> >
> > Signed-off-by: Christoph Rudorff <chris@rudorff.com>  
> 
> I think that commit message needs a bit of elaboration ;)
> 
> With that fixed:
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> 
> Yours,
> Linus Walleij

Re: [PATCH v3 2/3] iio: light: gp2ap002: add led power supply

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 01:35:39PM +0200, Christoph Rudorff wrote:

> tested on samsung-j5
> 
> v2: Use regulator bulk api.
> v3: pretty up, rename vled to leda

The comment and changelog area is...

> Signed-off-by: Christoph Rudorff <chris@rudorff.com>
> ---

...here, but the commit message is effectively missing. We may not accept such
a change, so please provide a commit message and move unnecessary bits to the
comment and changelog area.

...

> +enum {
> +	SUPPLY_VIO,
> +	SUPPLY_VDD,
> +	SUPPLY_LEDA,
> +	NUM_SUPPLIES

Not critical, but the naming is too broad, better to have namespaced variants

	GP2AP002_SUPPLY_VIO,

et cetera.

> +};

>  	struct regmap *regmap;
>  	int num_chan;
>  	const char *compat;
> +	static const char *const gp2ap002_supply_names[NUM_SUPPLIES] = {
> +		[SUPPLY_VIO] = "vio",
> +		[SUPPLY_VDD] = "vdd",
> +		[SUPPLY_LEDA] = "leda",
> +	};

Can be moved out of the function to see the static data (that's effectively
global anyway).

>  	u8 val;
>  	int ret;

...

> +	ret = devm_regulator_bulk_get(dev, NUM_SUPPLIES, gp2ap002->supplies);

> +

Redundant blank line.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");

...

> +	ret = regulator_set_voltage(gp2ap002->supplies[SUPPLY_VDD].consumer, 2400000, 3600000);

> -	ret = regulator_get_voltage(gp2ap002->vdd);
> +	ret = regulator_get_voltage(gp2ap002->supplies[SUPPLY_VDD].consumer);

> -	ret = regulator_set_voltage(gp2ap002->vio, 1650000, ret);
> +	ret = regulator_set_voltage(gp2ap002->supplies[SUPPLY_VIO].consumer, 1650000, ret);

To make these lines shorter you can introduce a temporary variable (or reuse
"compat" if it's feasible, might require to rename the variable, though)

	rname = gp2ap002->supplies[SUPPLY_VDD].consumer;
	ret = regulator_set_voltage(rname, 2400000, 3600000);
	...
	rname = gp2ap002->supplies[SUPPLY_VIO].consumer;
	ret = regulator_set_voltage(rname, 1650000, ret);

(Side note: This ret = foo(ret) doesn't sound good, the rule of thumb to have a separate
 variable for the 'ret', so you will have ret = foo($VAR) in the code.)

...

>  	ret = gp2ap002_init(gp2ap002);
>  	if (ret) {
>  		dev_err(dev, "initialization failed\n");
> -		goto out_disable_vio;
> +		goto out_disable_power;
>  	}
>  	pm_runtime_get_noresume(dev);
>  	pm_runtime_set_active(dev);
> @@ -616,10 +619,8 @@ static int gp2ap002_probe(struct i2c_client *client)
>  	pm_runtime_put_noidle(dev);
>  out_disable_pm:
>  	pm_runtime_disable(dev);
> -out_disable_vio:
> -	regulator_disable(gp2ap002->vio);
> -out_disable_vdd:
> -	regulator_disable(gp2ap002->vdd);
> +out_disable_power:
> +	regulator_bulk_disable(NUM_SUPPLIES, gp2ap002->supplies);
>  	return ret;

Wondering if we can use managed resources to disable regulators.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor

[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>
---
 arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 52 +++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
index ba8650971d6a..efac29485df8 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
@@ -8,6 +8,17 @@ / {
 	model = "Samsung Galaxy J5 (2015)";
 	compatible = "samsung,j5", "qcom,msm8916";
 	chassis-type = "handset";
+
+	reg_leda_proximity: regulator-leda-proximity {
+		compatible = "regulator-fixed";
+		regulator-name = "proximity_leda";
+
+		gpio = <&tlmm 73 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-0 = <&proximity_ldo_en_default>;
+		pinctrl-names = "default";
+	};
 };
 
 &accelerometer {
@@ -19,6 +30,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>;
+		vio-supply = <&pm8916_l5>;
+		leda-supply = <&reg_leda_proximity>;
+
+		sharp,proximity-far-hysteresis = /bits/ 8 <0x2f>;
+		sharp,proximity-close-hysteresis = /bits/ 8 <0x0f>;
+
+		pinctrl-0 = <&proximity_int_default>;
+		pinctrl-names = "default";
+	};
+};
+
 &pm8916_codec {
 	qcom,micbias1-ext-cap;
 };
@@ -30,3 +59,26 @@ &touchscreen {
 &usb_hs_phy {
 	qcom,init-seq = /bits/ 8 <0x1 0x19 0x2 0x0b>;
 };
+
+&pm8916_rpm_regulators {
+	pm8916_l17: l17 {
+		regulator-min-microvolt = <2850000>;
+		regulator-max-microvolt = <3100000>;
+	};
+};
+
+&tlmm {
+	proximity_ldo_en_default: proximity-ldo-en-default-state {
+		pins = "gpio73";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	proximity_int_default: proximity-int-default-state {
+		pins = "gpio113";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+};

-- 
2.39.5

Re: [PATCH v3 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor

[Linus Walleij]

On Tue, Oct 20, 2026 at 1:36 PM Christoph Rudorff <chris@rudorff.com> wrote:

> J5 got a gp2ap002 sensor, add it and it's supplies to the device tree
>
> Signed-off-by: Christoph Rudorff <chris@rudorff.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH v3 3/3] arm64: dts: qcom: msm8916-samsung-j5: Add proximity sensor

[Konrad Dybcio]

On 10/20/26 1:35 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>
> ---

I'd like for you to comment on my past reply:

https://lore.kernel.org/linux-arm-msm/5531e454-0d84-48f2-af0d-1ab34c149760@oss.qualcomm.com/

Konrad