Re: [PATCH v2 3/3] iio: chemical: Add driver for Sharp GP2Y1010AU0F

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 12 Jun 2025 15:37:46 +0530
surajsonawane0215@gmail.com wrote:

> From: Suraj Sonawane <surajsonawane0215@gmail.com>
> 
> Implement support for the Sharp GP2Y1010AU0F optical dust sensor which
> measures particulate matter concentration using infrared scattering.
> The sensor requires precise 320μs LED pulses with ADC sampling at 280μs
> after LED activation (as specified in datasheet section 6-1).
> 
> The driver provides:
> - Raw density readings via IIO_DENSITY channel type
> - Hardware-agnostic operation via GPIO and IIO ADC interfaces
> - Power management through regulator framework
> - Device Tree binding support
> 
> Datasheet: https://global.sharp/products/device/lineup/data/pdf/datasheet/gp2y1010au_appl_e.pdf
> 
> Signed-off-by: Suraj Sonawane <surajsonawane0215@gmail.com>
A few additional comments from me.

Jonathan

>  obj-$(CONFIG_PMS7003) += pms7003.o
> diff --git a/drivers/iio/chemical/gp2y1010.c b/drivers/iio/chemical/gp2y1010.c
> new file mode 100644
> index 000000000..3a8657035
> --- /dev/null
> +++ b/drivers/iio/chemical/gp2y1010.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Suraj Sonawane <surajsonawane0215@gmail.com>
> + * Sharp GP2Y1010AU0F Dust Sensor Driver
> + * Datasheet: https://global.sharp/products/device/lineup/data/pdf/datasheet/gp2y1010au_appl_e.pdf
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +/* Timings based on GP2Y1010AU0F datasheet Section 6-1 */
> +#define GP2Y1010_LED_PULSE_US     320  /* Total LED ON time (0.32 ms) */
> +#define GP2Y1010_SAMPLE_DELAY_US  280  /* ADC sampling after LED ON (0.28 ms) */
> +
> +struct gp2y1010_data {
> +	struct gpio_desc *led_gpio;
> +	struct iio_channel *adc_chan;
> +	int v_clean;  /* Calibration: voltage in clean air (mV) */
> +};
> +
> +static int gp2y1010_read_raw(struct iio_dev *indio_dev,
> +							 struct iio_chan_spec const *chan,
> +							 int *val, int *val2, long mask)

Odd alignment. Aim for under the s of struct. That is immediately after the bracket.

> +{
> +	struct gp2y1010_data *data = iio_priv(indio_dev);
> +	int ret, voltage_mv;
> +
> +	if (mask != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	gpiod_set_value(data->led_gpio, 1);
> +	udelay(GP2Y1010_SAMPLE_DELAY_US);
> +
> +	ret = iio_read_channel_processed(data->adc_chan, &voltage_mv);
> +
> +	/* Wait remaining time to complete 320 µs total LED pulse width */
> +	udelay(GP2Y1010_LED_PULSE_US - GP2Y1010_SAMPLE_DELAY_US);
> +	gpiod_set_value(data->led_gpio, 0);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = voltage_mv;
> +	return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info gp2y1010_info = {
> +	.read_raw = gp2y1010_read_raw,
> +};
> +
> +static const struct iio_chan_spec gp2y1010_channels[] = {
> +	{
> +		.type = IIO_DENSITY,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +	},

Where there is only one channel, no need to bother with an array.
There are lots of drivers that do this, but I'm trying to discourage it
in new drivers.
indio_dev->channels = &gp271010_channel;
indio_dev->num_channels = 1;

is obvious enough.

> +};
> +
> +static int gp2y1010_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct gp2y1010_data *data;
> +	enum iio_chan_type ch_type;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	data->v_clean = 900;
> +
> +	data->led_gpio = devm_gpiod_get(dev, "led", GPIOD_OUT_LOW);
> +	if (IS_ERR(data->led_gpio))
> +		return dev_err_probe(dev, PTR_ERR(data->led_gpio), "Failed to get LED GPIO\n");
> +
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return ret;
> +	udelay(100);
> +
> +	data->adc_chan = devm_iio_channel_get(dev, "dust");
> +	if (IS_ERR(data->adc_chan))
> +		return dev_err_probe(dev, PTR_ERR(data->adc_chan), "Failed to get ADC channel\n");
> +
> +	ret = iio_get_channel_type(data->adc_chan, &ch_type);
> +	if (ret < 0)
> +		return ret;
> +	if (ch_type != IIO_DENSITY)
> +		return dev_err_probe(dev, -EINVAL, "ADC channel is not density type\n");

I'm confused.  The ADC channel type providing us a measurement service (as here we
are the consumer) is going to be voltage (or maybe current) because ADCs don't measure dust.
This driver has to do the conversion from voltage to the value needed to report a density channel. 

> +
> +	indio_dev->name = dev_name(dev);

Hard code the part number here rather than getting it from elsewhere.  That makes
it much easier to see that this is what we expect.

> +	indio_dev->info = &gp2y1010_info;
> +	indio_dev->channels = gp2y1010_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(gp2y1010_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}