On Fri, 5 Jul 2024 11:11:45 +0200 Marek Vasut <marex@xxxxxxx> wrote: > Add LiteOn LTR-308 support into LTR-F216A kernel driver. > > The two devices seem to have almost identical register map, except that > the LTR-308 does not have three CLEAR_DATA registers, which are unused > by this driver. Furthermore, LTR-308 and LTR-F216A use different lux > calculation constants, 0.6 and 0.45 respectively. Both differences are > handled using chip info data. > > https://optoelectronics.liteon.com/upload/download/DS86-2016-0027/LTR-308ALS_Final_%20DS_V1%201.pdf > https://optoelectronics.liteon.com/upload/download/DS86-2019-0016/LTR-F216A_Final_DS_V1.4.PDF > > Signed-off-by: Marek Vasut <marex@xxxxxxx> One additional question inline... > --- > drivers/iio/light/ltrf216a.c | 49 ++++++++++++++++++++++++++++-------- > 1 file changed, 39 insertions(+), 10 deletions(-) > > diff --git a/drivers/iio/light/ltrf216a.c b/drivers/iio/light/ltrf216a.c > index 68dc48420a886..375312db4ef58 100644 > --- a/drivers/iio/light/ltrf216a.c > +++ b/drivers/iio/light/ltrf216a.c > @@ -68,6 +68,13 @@ static const int ltrf216a_int_time_reg[][2] = { > { 25, 0x40 }, > @@ -382,15 +394,19 @@ static bool ltrf216a_writable_reg(struct device *dev, unsigned int reg) > > static bool ltrf216a_volatile_reg(struct device *dev, unsigned int reg) > { > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > + struct ltrf216a_data *data = iio_priv(indio_dev); > + > switch (reg) { > case LTRF216A_MAIN_STATUS: > - case LTRF216A_ALS_CLEAR_DATA_0: > - case LTRF216A_ALS_CLEAR_DATA_1: > - case LTRF216A_ALS_CLEAR_DATA_2: > case LTRF216A_ALS_DATA_0: > case LTRF216A_ALS_DATA_1: > case LTRF216A_ALS_DATA_2: > return true; > + case LTRF216A_ALS_CLEAR_DATA_0: > + case LTRF216A_ALS_CLEAR_DATA_1: > + case LTRF216A_ALS_CLEAR_DATA_2: Is there any point in this covering registers we have already stated above are not readable? I guess we could argue that having this change is acting as a form of documentation. Maybe just adding a comment that they don't exist would be clearer? > + return data->info->has_clear_data; > default: > return false; > } ;
