| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6bd4a0f2-90a9-4e99-8cd6-9fefd04f2323@baylibre.com>
Date: Thu, 12 Jun 2025 13:24:53 -0500
From: David Lechner <dlechner@...libre.com>
To: surajsonawane0215@...il.com, Jonathan Cameron <jic23@...nel.org>
Cc: Nuno Sá <nuno.sa@...log.com>,
Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] iio: chemical: Add driver for Sharp GP2Y1010AU0F
On 6/12/25 5:07 AM, surajsonawane0215@...il.com wrote:
> From: Suraj Sonawane <surajsonawane0215@...il.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@...il.com>
> ---
...
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index b22afa1f6..35c126836 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -129,6 +129,16 @@ config PMS7003
>
> To compile this driver as a module, choose M here: the module will
> be called pms7003.
> +
> +config GP2Y1010AU0F
> + tristate "Sharp GP2Y1010AU0F optical dust sensor"
> + depends on IIO
> + help
> + Say Y here to build support for Sharp GP2Y1010AU0F optical dust sensor
> + that measures particulate matter concentration in air.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called gp2y1010au0f.
The object file name is just gp2y1010.
>
> config SCD30_CORE
> tristate "SCD30 carbon dioxide sensor driver"
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> index 2287a00a6..f1d932ab0 100644
> --- a/drivers/iio/chemical/Makefile
> +++ b/drivers/iio/chemical/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_CCS811) += ccs811.o
> obj-$(CONFIG_ENS160) += ens160_core.o
> obj-$(CONFIG_ENS160_I2C) += ens160_i2c.o
> obj-$(CONFIG_ENS160_SPI) += ens160_spi.o
> +obj-$(CONFIG_GP2Y1010AU0F) += gp2y1010.o
> obj-$(CONFIG_IAQCORE) += ams-iaq-core.o
> obj-$(CONFIG_MHZ19B) += mhz19b.o
> 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@...il.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) */
If we aren't using the values from the devicetree for this
I would leave those out of the devicetree bindings until we
are really sure we need them.
> +
> +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)
> +{
> + 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);
As I mentioned in the reply to the devicetree bindings. I have some
doubts that we could get accurate enough timing to have the ADC read
exactly at the peak output +/-20 microseconds in Linux like this.
Are you using an oscilloscope to verify that the timing is correct?
> +
> + /* 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;
Since we can't read a raw value directly from the sensor,
might as well convert this to the processes value. This
would also allow to handle the non-linear parts at the
extremes of the graph shown in the datasheet.
> + 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),
> + },
> +};
There is only one channel, so we don't need an array.
> +
> +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;
This isn't used anywhere.
> +
> + 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);
Datasheet says this delay is "less than 1 sec". So this seems rather short.
> +
> + 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");
This check looks wrong. Aren't we expecting a voltage channel?
> +
> + indio_dev->name = dev_name(dev);
> + 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);
> +}
> +
Powered by blists - more mailing lists