Re: [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/7/24 3:49 PM, Jonathan Cameron wrote:
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?

I'll add a comment, thanks.




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux