linux-kernel - Re: [PATCH 1/2] iio: light: veml6030: extend regmap to support regfields and caching

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D705L20050OB.3UOW9R2QA4QA5@gmail.com>
Date: Sun, 12 Jan 2025 15:10:14 +0100
From: "Javier Carrasco" <javier.carrasco.cruz@...il.com>
To: "Jonathan Cameron" <jic23@...nel.org>
Cc: "Lars-Peter Clausen" <lars@...afoo.de>, "Rishi Gupta"
 <gupt21@...il.com>, <linux-iio@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, "Jonathan Cameron"
 <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH 1/2] iio: light: veml6030: extend regmap to support
 regfields and caching

On Sun Jan 12, 2025 at 2:18 PM CET, Jonathan Cameron wrote:
> On Tue, 07 Jan 2025 21:50:21 +0100
> Javier Carrasco <javier.carrasco.cruz@...il.com> wrote:
>
> > The configuration registers are not volatile and are not affected
> > by read operations (i.e. not precious), making them suitable to be
> > cached in order to reduce the number of accesses to the device.
> >
> > Add support for regfields as well to simplify register operations,
> > taking into account the different fields for the veml6030/veml7700 and
> > veml6035.
> >
> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
> > ---
> >  drivers/iio/light/veml6030.c | 141 +++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 116 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c
> > index 9b71825eea9bee2146be17ed2f30f5a8f7ad37e3..a6385c6d3fba59a6b22845a3c5e252b619faed65 100644
> > --- a/drivers/iio/light/veml6030.c
> > +++ b/drivers/iio/light/veml6030.c
> > @@ -65,6 +65,11 @@ enum veml6030_scan {
> >  	VEML6030_SCAN_TIMESTAMP,
> >  };
> >
> > +struct veml6030_rf {
> > +	struct regmap_field *it;
> > +	struct regmap_field *gain;
> > +};
> > +
> >  struct veml603x_chip {
> >  	const char *name;
> >  	const int(*scale_vals)[][2];
> > @@ -75,6 +80,7 @@ struct veml603x_chip {
> >  	int (*set_info)(struct iio_dev *indio_dev);
> >  	int (*set_als_gain)(struct iio_dev *indio_dev, int val, int val2);
> >  	int (*get_als_gain)(struct iio_dev *indio_dev, int *val, int *val2);
> > +	int (*regfield_init)(struct iio_dev *indio_dev);
>
> With only two fields, why use a callback rather than just adding the two
> const struct reg_field into this structure directly?

The rationale was that extending the driver for more devices with
additional fields would not require extra elements in the struct that
would only apply to some devices. All members of this struct are rather
basic and all devices will require them, and although integration time
and gain regfields will be required too, if a new regfield for a
specific device is added, it will be added to the rest as empty element.

But that's probably too much "if" and "would", so I am fine with your
suggestion.

>
> I'd also be tempted to do the caching and regfield changes as separate patches.
>

Then I will split the patch for v2.

> Jonathan

Thank you for your feedback and best regards,
Javier Carrasco