From: Joshua Crofts <joshua.crofts1@gmail.com>
To: Kaustabh Chakraborty <kauschluss@disroot.org>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Peter Griffin" <peter.griffin@linaro.org>,
"Alim Akhtar" <alim.akhtar@samsung.com>,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org
Subject: Re: [PATCH 2/3] iio: proximity: add driver for Sharp GP2AP070S proximity sensor
Date: Thu, 23 Jul 2026 23:44:39 +0200 [thread overview]
Message-ID: <20260723234439.06411731@systembl0wer> (raw)
In-Reply-To: <20260723-gp2ap070s-v1-2-b8ca3a4c10dd@disroot.org>
On Thu, 23 Jul 2026 22:58:34 +0530
Kaustabh Chakraborty <kauschluss@disroot.org> wrote:
> The GP2AP070S is a proximity sensor designed and manufactured by Sharp
> Corporation. This sensor is used in mobile devices, including, but not
> limited to - the Samsung Galaxy J6.
>
> The driver has been adopted from Samsung's downstream kernel
> implementation [1]. Due to the lack of public documentation about the
> schematics of this device. The downstream driver acts as the secondary
> source of information. Driver clarity has also been improved with the
> help of the GP2AP* drivers in iio/light.
>
> Link: https://github.com/Exynos7870/android_kernel_samsung_universal7870/blob/lineage-16.0/drivers/sensors/gp2ap070s.c [1]
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
Some comments inline. Additionally, please check out Sashiko's
findings: https://sashiko.dev/#/patchset/20260723-gp2ap070s-v1-0-b8ca3a4c10dd%40disroot.org.
> drivers/iio/proximity/Kconfig | 11 +
> drivers/iio/proximity/Makefile | 1 +
> drivers/iio/proximity/gp2ap070s.c | 502 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 514 insertions(+)
>
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> index bb77fad2a1b3..3f30ebfc21bc 100644
> --- a/drivers/iio/proximity/Kconfig
> +++ b/drivers/iio/proximity/Kconfig
> @@ -41,6 +41,17 @@ config D3323AA
> To compile this driver as a module, choose M here: the module will be
> called d3323aa.
>
> +config GP2AP070S
> + tristate "Sharp GP2AP070S proximity sensor"
> + select REGMAP_I2C
> + depends on I2C
A very small nit (and probably a personal opinion), but "depends on" should
go before "select"
> + help
> + Say Y here to build a driver for the Sharp GP2AP070S proximity
> + sensor.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called gp2ap070s.
> +
> config HX9023S
> tristate "TYHX HX9023S SAR sensor"
> select IIO_BUFFER
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> index 4352833dd8a4..627ffb04acb2 100644
> --- a/drivers/iio/proximity/Makefile
> +++ b/drivers/iio/proximity/Makefile
> @@ -7,6 +7,7 @@
> obj-$(CONFIG_AS3935) += as3935.o
> obj-$(CONFIG_CROS_EC_MKBP_PROXIMITY) += cros_ec_mkbp_proximity.o
> obj-$(CONFIG_D3323AA) += d3323aa.o
> +obj-$(CONFIG_GP2AP070S) += gp2ap070s.o
> obj-$(CONFIG_HX9023S) += hx9023s.o
> obj-$(CONFIG_IRSD200) += irsd200.o
> obj-$(CONFIG_ISL29501) += isl29501.o
> diff --git a/drivers/iio/proximity/gp2ap070s.c b/drivers/iio/proximity/gp2ap070s.c
> new file mode 100644
> index 000000000000..9625f53d956e
> --- /dev/null
> +++ b/drivers/iio/proximity/gp2ap070s.c
> @@ -0,0 +1,502 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * IIO driver for Sharp GP2AP070S proximity sensor.
> + *
> + * Copyright (C) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
Please add iio/* includes after the generic linux/* headers. Ensure
that there is a blank line between the two groups.
Additionally, you're also missing <asm/byteorder.h>, array_size.h, err.h,
types.h and delay.h.
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
Don't include this, this is included in i2c.h (see Uwe Kleine-Konig's
work).
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define GP2AP070S_REG_COM1 0x80
> +#define GP2AP070S_REG_COM2 0x81
> +#define GP2AP070S_REG_COM3 0x82
> +#define GP2AP070S_REG_COM4 0x83
> +#define GP2AP070S_REG_PS1 0x85
> +#define GP2AP070S_REG_PS2 0x86
> +#define GP2AP070S_REG_PS3 0x87
> +#define GP2AP070S_REG_PS_THD_LO_LE16 0x88
> +#define GP2AP070S_REG_PS_THD_HI_LE16 0x8a
> +#define GP2AP070S_REG_OS_D0_LE16 0x8c
> +#define GP2AP070S_REG_D0_LE16 0x90
> +
> +/* GP2AP070S_REG_COM1 */
> +#define GP2AP070S_VAL_COM1_WKUP BIT(7)
> +#define GP2AP070S_VAL_COM1_EN BIT(5)
> +
> +/* GP2AP070S_REG_COM3 */
> +#define GP2AP070S_VAL_COM3_INT_PULSE BIT(1)
> +
> +/* GP2AP070S_REG_COM4 */
> +#define GP2AP070S_VAL_COM1_BLINK GENMASK(2, 0) /* LED Blink Interval */
You don't need a comment like this if you name your macro reasonably
(which you did IMO).
> +
> +#define GP2AP070S_VAL_COM1_BLINK_0ms 0
> +#define GP2AP070S_VAL_COM1_BLINK_2ms 1
> +#define GP2AP070S_VAL_COM1_BLINK_8ms 2
> +#define GP2AP070S_VAL_COM1_BLINK_33ms 3
> +#define GP2AP070S_VAL_COM1_BLINK_66ms 4
> +#define GP2AP070S_VAL_COM1_BLINK_131ms 5
> +#define GP2AP070S_VAL_COM1_BLINK_262ms 6
> +#define GP2AP070S_VAL_COM1_BLINK_524ms 7
> +
> +/* GP2AP070S_REG_PS1 */
> +#define GP2AP070S_VAL_PS1_RESOL GENMASK(5, 4) /* Resolution */
> +
> +#define GP2AP070S_VAL_PS1_RESOL_14ms 0
> +#define GP2AP070S_VAL_PS1_RESOL_12ms 1
> +#define GP2AP070S_VAL_PS1_RESOL_10ms 2
> +#define GP2AP070S_VAL_PS1_RESOL_8ms 3
> +
> +/* GP2AP070S_REG_PS2 */
> +#define GP2AP070S_VAL_PS2_IOUT GENMASK(6, 4) /* Current Output */
> +#define GP2AP070S_VAL_PS2_SUM32 BIT(2)
> +
> +#define GP2AP070S_VAL_PS2_IOUT_0mA 0
> +#define GP2AP070S_VAL_PS2_IOUT_24mA 1
> +#define GP2AP070S_VAL_PS2_IOUT_89mA 2
> +#define GP2AP070S_VAL_PS2_IOUT_130mA 3
> +#define GP2AP070S_VAL_PS2_IOUT_190mA 4
> +
> +/* GP2AP070S_REG_PS3 */
> +#define GP2AP070S_VAL_PS3_PRST GENMASK(6, 4) /* Repeating Measurements */
> +
> +struct gp2ap070s_drvdata {
> + struct device *dev;
> + struct regmap *regmap;
> + struct regulator_bulk_data *regulators;
> + u32 near_level;
In the _write_event_value function,
> +};
> +
> +static const struct regmap_config gp2ap070s_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static const char *const gp2ap070s_regulator_names[] = {
> + "vdd",
> + "vled",
> +};
> +
> +static ssize_t gp2ap070s_iio_read_near_level(struct iio_dev *indio_dev,
> + uintptr_t priv,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + struct gp2ap070s_drvdata *drvdata = iio_priv(indio_dev);
> +
> + return sprintf(buf, "%u\n", drvdata->near_level);
Use sysfs_emit() instead.
> +}
> +
> +static const struct iio_chan_spec_ext_info gp2ap070s_iio_chan_spec_ext_info[] = {
> + {
> + .name = "nearlevel",
> + .shared = IIO_SEPARATE,
> + .read = gp2ap070s_iio_read_near_level,
> + },
> + { /* sentinel */ }
Remove the comment.
> +};
> +
...
> +static int gp2ap070s_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct iio_dev *indio_dev;
> + struct gp2ap070s_drvdata *drvdata;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*drvdata));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + drvdata = iio_priv(indio_dev);
> + i2c_set_clientdata(client, drvdata);
> +
> + drvdata->dev = dev;
> + drvdata->regmap = devm_regmap_init_i2c(client, &gp2ap070s_regmap_config);
> + if (IS_ERR(drvdata->regmap))
> + return dev_err_probe(dev, PTR_ERR(drvdata->regmap), "Failed to create regmap\n");
> +
> + ret = devm_regulator_bulk_get_enable(dev,
> + ARRAY_SIZE(gp2ap070s_regulator_names),
> + gp2ap070s_regulator_names);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get and enable regulators");
> +
> + usleep_range(10000, 11000);
> +
> + indio_dev->name = "gp2ap070s";
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = gp2ap070s_iio_chan_spec;
> + indio_dev->num_channels = ARRAY_SIZE(gp2ap070s_iio_chan_spec);
> + indio_dev->info = &gp2ap070s_iio_info;
> +
> + device_property_read_u32(&client->dev, "proximity-near-level",
> + &drvdata->near_level);
> +
> + ret = gp2ap070s_probe_hw_register(drvdata);
> + if (ret)
> + return ret;
> +
> + ret = devm_request_threaded_irq(dev, client->irq, NULL,
> + gp2ap070s_irq_handler, IRQF_ONESHOT,
> + "gp2ap070s-irq", indio_dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to request IRQ");
Just return ret instead, dev_err_probe() is called automatically on failure.
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct of_device_id gp2ap070s_of_device_id[] = {
> + { .compatible = "sharp,gp2ap070s" },
> + { /* sentinel */ }
Remove the comment.
> +};
> +MODULE_DEVICE_TABLE(of, gp2ap070s_of_device_id);
> +
> +static const struct i2c_device_id gp2ap070s_i2c_device_id[] = {
> + { .name = "gp2ap070s" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, gp2ap070s_i2c_device_id);
> +
> +static struct i2c_driver gp2ap070s_i2c_driver = {
> + .driver = {
> + .name = "gp2ap070s",
> + .of_match_table = gp2ap070s_of_device_id,
> + },
> + .probe = gp2ap070s_probe,
> + .id_table = gp2ap070s_i2c_device_id,
> +};
> +module_i2c_driver(gp2ap070s_i2c_driver);
> +
> +MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
> +MODULE_DESCRIPTION("Sharp GP2AP070S Proximity Sensor");
> +MODULE_LICENSE("GPL");
>
--
Kind regards,
Joshua Crofts
next prev parent reply other threads:[~2026-07-23 21:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-23 17:28 [PATCH 0/3] Add Sharp GP2AP070S Proximity Driver and enable it in Galaxy J6 (j6lte) Kaustabh Chakraborty
2026-07-23 17:28 ` [PATCH 1/3] dt-bindings: iio: proximity: add Sharp GP2AP070S proximity sensor Kaustabh Chakraborty
2026-07-23 21:47 ` Joshua Crofts
2026-07-23 17:28 ` [PATCH 2/3] iio: proximity: add driver for " Kaustabh Chakraborty
2026-07-23 17:40 ` sashiko-bot
2026-07-23 21:44 ` Joshua Crofts [this message]
2026-07-23 17:28 ` [PATCH 3/3] arm64: dts: exynos7870-j6lte: add " Kaustabh Chakraborty
2026-07-23 17:42 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260723234439.06411731@systembl0wer \
--to=joshua.crofts1@gmail.com \
--cc=alim.akhtar@samsung.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=kauschluss@disroot.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=peter.griffin@linaro.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.