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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1597453a-31fc-49eb-8b69-efeb8805c67a@ixit.cz>
Date: Sat, 15 Feb 2025 15:12:46 +0100
From: David Heidelberg <david@...t.cz>
To: Svyatoslav Ryhel <clamor95@...il.com>, Jonathan Cameron
 <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Thierry Reding
 <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>,
 Javier Carrasco <javier.carrasco.cruz@...il.com>,
 Matti Vaittinen <mazziesaccount@...il.com>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Emil Gedenryd <emil.gedenryd@...s.com>,
 Arthur Becker <arthur.becker@...tec.com>,
 Mudit Sharma <muditsharma.info@...il.com>,
 Per-Daniel Olsson <perdaniel.olsson@...s.com>,
 Subhajit Ghosh <subhajit.ghosh@...aklogic.com>,
 Ivan Orlov <ivan.orlov0322@...il.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v2 2/3] iio: light: Add support for AL3000a illuminance
 sensor



On 15/02/2025 11:31, Svyatoslav Ryhel wrote:
> AL3000a is a simple I2C-based ambient light sensor, which is
> closely related to AL3010 and AL3320a, but has significantly
> different way of processing data generated by the sensor.
> 
> Tested-by: Robert Eckelmann <longnoserob@...il.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
> ---
>   drivers/iio/light/Kconfig   |  10 ++
>   drivers/iio/light/Makefile  |   1 +
>   drivers/iio/light/al3000a.c | 221 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 232 insertions(+)
>   create mode 100644 drivers/iio/light/al3000a.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index e34e551eef3e..142f7f7ef0ec 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -43,6 +43,16 @@ config ADUX1020
>   	 To compile this driver as a module, choose M here: the
>   	 module will be called adux1020.
>   
> +config AL3000A
> +	tristate "AL3000a ambient light sensor"
> +	depends on I2C
> +	help
> +	  Say Y here if you want to build a driver for the Dyna Image AL3000a
> +	  ambient light sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called al3000a.
> +
>   config AL3010
>   	tristate "AL3010 ambient light sensor"
>   	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 11a4041b918a..17030a4cc340 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -7,6 +7,7 @@
>   obj-$(CONFIG_ACPI_ALS)		+= acpi-als.o
>   obj-$(CONFIG_ADJD_S311)		+= adjd_s311.o
>   obj-$(CONFIG_ADUX1020)		+= adux1020.o
> +obj-$(CONFIG_AL3000A)		+= al3000a.o
>   obj-$(CONFIG_AL3010)		+= al3010.o
>   obj-$(CONFIG_AL3320A)		+= al3320a.o
>   obj-$(CONFIG_APDS9300)		+= apds9300.o
> diff --git a/drivers/iio/light/al3000a.c b/drivers/iio/light/al3000a.c
> new file mode 100644
> index 000000000000..58d4336dd081
> --- /dev/null
> +++ b/drivers/iio/light/al3000a.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/types.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define AL3000A_DRV_NAME		"al3000a"
> +#define AL3000A_REG_SYSTEM		0x00
> +#define AL3000A_REG_DATA		0x05
> +
> +#define AL3000A_CONFIG_ENABLE		0x00
> +#define AL3000A_CONFIG_DISABLE		0x0b
> +#define AL3000A_CONFIG_RESET		0x0f
> +#define AL3000A_GAIN_MASK		GENMASK(5, 0)
> +
> +/*
> + * This are pre-calculated lux values based on possible output of sensor
> + * (range 0x00 - 0x3F)
> + */
> +static const u32 lux_table[] = {
> +	1, 1, 1, 2, 2, 2, 3, 4,					/* 0 - 7 */
> +	4, 5, 6, 7, 9, 11, 13, 16,				/* 8 - 15 */
> +	19, 22, 27, 32, 39, 46, 56, 67,				/* 16 - 23 */
> +	80, 96, 116, 139, 167, 200, 240, 289,			/* 24 - 31 */
> +	347, 416, 499, 600, 720, 864, 1037, 1245,		/* 32 - 39 */
> +	1495, 1795, 2155, 2587, 3105, 3728, 4475, 5373,		/* 40 - 47 */
> +	6450, 7743, 9296, 11160, 13397, 16084, 19309, 23180,	/* 48 - 55 */
> +	27828, 33408, 40107, 48148, 57803, 69393, 83306, 100000 /* 56 - 63 */
> +};
> +
> +static const struct regmap_config al3000a_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = AL3000A_REG_DATA,
> +};
> +
> +struct al3000a_data {
> +	struct regmap *regmap;
> +	struct regulator *vdd_supply;
> +};
> +
> +static const struct iio_chan_spec al3000a_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static int al3000a_set_pwr(struct al3000a_data *data, bool pwr)
> +{
> +	struct device *dev = regmap_get_device(data->regmap);
> +	u8 val = pwr ? AL3000A_CONFIG_ENABLE : AL3000A_CONFIG_DISABLE;
> +	int ret;
> +
> +	if (pwr) {
> +		ret = regulator_enable(data->vdd_supply);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to enable vdd power supply\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, val);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to write system register\n");
> +		return ret;
> +	}
> +
> +	if (!pwr) {
> +		ret = regulator_disable(data->vdd_supply);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to disable vdd power supply\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void al3000a_set_pwr_off(void *_data)
> +{
> +	struct al3000a_data *data = _data;
> +
> +	al3000a_set_pwr(data, false);
> +}
> +
> +static int al3000a_init(struct al3000a_data *data)
> +{
> +	int ret;
> +
> +	ret = al3000a_set_pwr(data, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_RESET);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_ENABLE);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int al3000a_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct al3000a_data *data = iio_priv(indio_dev);
> +	int ret, gain;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_read(data->regmap, AL3000A_REG_DATA, &gain);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = lux_table[gain & AL3000A_GAIN_MASK];

Why did you chosen to do post-processing in the RAW channel instead 
doing it in INFO_SCALE (same as al3010 does)?

Except this, LGTM.

Documentation and DT patch:

Reviewed-by: David Heidelberg <david@...t.cz>
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 1;
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info al3000a_info = {
> +	.read_raw	= al3000a_read_raw,
> +};
> +
> +static int al3000a_probe(struct i2c_client *client)
> +{
> +	struct al3000a_data *data;
> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	data->regmap = devm_regmap_init_i2c(client, &al3000a_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return dev_err_probe(dev, PTR_ERR(data->regmap),
> +				     "cannot allocate regmap\n");
> +
> +	data->vdd_supply = devm_regulator_get(dev, "vdd");
> +	if (IS_ERR(data->vdd_supply))
> +		return dev_err_probe(dev, PTR_ERR(data->vdd_supply),
> +				     "failed to get vdd regulator\n");
> +
> +	indio_dev->info = &al3000a_info;
> +	indio_dev->name = AL3000A_DRV_NAME;
> +	indio_dev->channels = al3000a_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(al3000a_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = al3000a_init(data);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to init ALS\n");
> +
> +	ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to add action\n");
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static int al3000a_suspend(struct device *dev)
> +{
> +	struct al3000a_data *data = iio_priv(dev_get_drvdata(dev));
> +
> +	return al3000a_set_pwr(data, false);
> +}
> +
> +static int al3000a_resume(struct device *dev)
> +{
> +	struct al3000a_data *data = iio_priv(dev_get_drvdata(dev));
> +
> +	return al3000a_set_pwr(data, true);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(al3000a_pm_ops, al3000a_suspend, al3000a_resume);
> +
> +static const struct of_device_id al3000a_of_match[] = {
> +	{ .compatible = "dynaimage,al3000a" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, al3000a_of_match);
> +
> +static struct i2c_driver al3000a_driver = {
> +	.driver = {
> +		.name = AL3000A_DRV_NAME,
> +		.of_match_table = al3000a_of_match,
> +		.pm = pm_sleep_ptr(&al3000a_pm_ops),
> +	},
> +	.probe = al3000a_probe,
> +};
> +module_i2c_driver(al3000a_driver);
> +
> +MODULE_AUTHOR("Svyatolsav Ryhel <clamor95@...il.com>");
> +MODULE_DESCRIPTION("al3000a Ambient Light Sensor driver");
> +MODULE_LICENSE("GPL");

-- 
David Heidelberg


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ