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: <CAPVz0n1T_jXXDhm6gF7gDDqZ=b6abR1Tqk=5kLo=Ws4FF2EVJw@mail.gmail.com>
Date: Sat, 15 Feb 2025 16:22:05 +0200
From: Svyatoslav Ryhel <clamor95@...il.com>
To: David Heidelberg <david@...t.cz>
Cc: 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>, 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

сб, 15 лют. 2025 р. о 16:12 David Heidelberg <david@...t.cz> пише:
>
>
>
> 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)?
>

>From my observation INFO_SCALE will just perform multiplication of RAW
to SCALE. In this case values which are read are not actual raw values
of illumination. Next is my assumption (since there is no datasheet),
but values obtained from register are similar to values from adc
thermal sensors, they need be converted via reference table to get
actual data.

> 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