* [PATCH 1/2] dt-bindings: iio: light: ltrf216a: Document LTR-308 support @ 2024-07-05 9:11 Marek Vasut 2024-07-05 9:11 ` [PATCH 2/2] iio: light: ltrf216a: Add " Marek Vasut 0 siblings, 1 reply; 6+ messages in thread From: Marek Vasut @ 2024-07-05 9:11 UTC (permalink / raw) To: linux-iio Cc: Marek Vasut, Conor Dooley, Jonathan Cameron, Krzysztof Kozlowski, Lars-Peter Clausen, Rob Herring, Shreeya Patel, devicetree Document LiteOn LTR-308 support in LTR-F216A bindings. 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. 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@denx.de> --- Cc: Conor Dooley <conor+dt@kernel.org> Cc: Jonathan Cameron <jic23@kernel.org> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org> Cc: Lars-Peter Clausen <lars@metafoo.de> Cc: Marek Vasut <marex@denx.de> Cc: Rob Herring <robh@kernel.org> Cc: Shreeya Patel <shreeya.patel@collabora.com> Cc: devicetree@vger.kernel.org Cc: linux-iio@vger.kernel.org --- .../devicetree/bindings/iio/light/liteon,ltrf216a.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml index 7de1b0e721ca2..877e955d4ebd1 100644 --- a/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml +++ b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml @@ -14,7 +14,9 @@ description: properties: compatible: - const: liteon,ltrf216a + enum: + - liteon,ltr308 + - liteon,ltrf216a reg: maxItems: 1 -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support 2024-07-05 9:11 [PATCH 1/2] dt-bindings: iio: light: ltrf216a: Document LTR-308 support Marek Vasut @ 2024-07-05 9:11 ` Marek Vasut 2024-07-05 9:40 ` Krzysztof Kozlowski 2024-07-07 13:49 ` Jonathan Cameron 0 siblings, 2 replies; 6+ messages in thread From: Marek Vasut @ 2024-07-05 9:11 UTC (permalink / raw) To: linux-iio Cc: Marek Vasut, Conor Dooley, Jonathan Cameron, Krzysztof Kozlowski, Lars-Peter Clausen, Rob Herring, Shreeya Patel, devicetree 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@denx.de> --- Cc: Conor Dooley <conor+dt@kernel.org> Cc: Jonathan Cameron <jic23@kernel.org> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org> Cc: Lars-Peter Clausen <lars@metafoo.de> Cc: Marek Vasut <marex@denx.de> Cc: Rob Herring <robh@kernel.org> Cc: Shreeya Patel <shreeya.patel@collabora.com> Cc: devicetree@vger.kernel.org Cc: linux-iio@vger.kernel.org --- 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 }, }; +struct ltr_chip_info { + /* Chip contains CLEAR_DATA_0/1/2 registers at offset 0xa..0xc */ + bool has_clear_data; + /* Lux calculation multiplier for ALS data */ + int lux_multiplier; +}; + /* * Window Factor is needed when the device is under Window glass * with coated tinted ink. This is to compensate for the light loss @@ -79,6 +86,7 @@ static const int ltrf216a_int_time_reg[][2] = { struct ltrf216a_data { struct regmap *regmap; struct i2c_client *client; + const struct ltr_chip_info *info; u32 int_time; u16 int_time_fac; u8 als_gain_fac; @@ -246,7 +254,7 @@ static int ltrf216a_get_lux(struct ltrf216a_data *data) ltrf216a_set_power_state(data, false); - lux = greendata * 45 * LTRF216A_WIN_FAC; + lux = greendata * data->info->lux_multiplier * LTRF216A_WIN_FAC; return lux; } @@ -334,15 +342,15 @@ static const struct iio_info ltrf216a_info = { static bool ltrf216a_readable_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_CTRL: case LTRF216A_ALS_MEAS_RES: case LTRF216A_ALS_GAIN: case LTRF216A_PART_ID: 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: @@ -355,6 +363,10 @@ static bool ltrf216a_readable_reg(struct device *dev, unsigned int reg) case LTRF216A_ALS_THRES_LOW_1: case LTRF216A_ALS_THRES_LOW_2: return true; + case LTRF216A_ALS_CLEAR_DATA_0: + case LTRF216A_ALS_CLEAR_DATA_1: + case LTRF216A_ALS_CLEAR_DATA_2: + return data->info->has_clear_data; default: return false; } @@ -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: + return data->info->has_clear_data; default: return false; } @@ -433,6 +449,7 @@ static int ltrf216a_probe(struct i2c_client *client) i2c_set_clientdata(client, indio_dev); data->client = client; + data->info = i2c_get_match_data(client); mutex_init(&data->lock); @@ -520,15 +537,27 @@ static int ltrf216a_runtime_resume(struct device *dev) static DEFINE_RUNTIME_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, ltrf216a_runtime_resume, NULL); +struct ltr_chip_info ltr308_chip_info = { + .has_clear_data = false, + .lux_multiplier = 60, +}; + +struct ltr_chip_info ltrf216a_chip_info = { + .has_clear_data = true, + .lux_multiplier = 45, +}; + static const struct i2c_device_id ltrf216a_id[] = { - { "ltrf216a" }, + { "ltr308", .driver_data = (kernel_ulong_t)<r308_chip_info }, + { "ltrf216a", .driver_data = (kernel_ulong_t)<rf216a_chip_info }, {} }; MODULE_DEVICE_TABLE(i2c, ltrf216a_id); static const struct of_device_id ltrf216a_of_match[] = { - { .compatible = "liteon,ltrf216a" }, - { .compatible = "ltr,ltrf216a" }, + { .compatible = "liteon,ltr308", .data = <r308_chip_info }, + { .compatible = "liteon,ltrf216a", .data = <rf216a_chip_info }, + { .compatible = "ltr,ltrf216a", .data = <rf216a_chip_info }, {} }; MODULE_DEVICE_TABLE(of, ltrf216a_of_match); -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support 2024-07-05 9:11 ` [PATCH 2/2] iio: light: ltrf216a: Add " Marek Vasut @ 2024-07-05 9:40 ` Krzysztof Kozlowski 2024-07-05 9:47 ` Marek Vasut 2024-07-07 13:49 ` Jonathan Cameron 1 sibling, 1 reply; 6+ messages in thread From: Krzysztof Kozlowski @ 2024-07-05 9:40 UTC (permalink / raw) To: Marek Vasut, linux-iio Cc: Conor Dooley, Jonathan Cameron, Krzysztof Kozlowski, Lars-Peter Clausen, Rob Herring, Shreeya Patel, devicetree On 05/07/2024 11:11, Marek Vasut 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@denx.de> > --- ... > mutex_init(&data->lock); > > @@ -520,15 +537,27 @@ static int ltrf216a_runtime_resume(struct device *dev) > static DEFINE_RUNTIME_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, > ltrf216a_runtime_resume, NULL); > > +struct ltr_chip_info ltr308_chip_info = { static const > + .has_clear_data = false, > + .lux_multiplier = 60, > +}; > + > +struct ltr_chip_info ltrf216a_chip_info = { static const > + .has_clear_data = true, > + .lux_multiplier = 45, > +}; > + > static const struct i2c_device_id ltrf216a_id[] = { > - { "ltrf216a" }, > + { "ltr308", .driver_data = (kernel_ulong_t)<r308_chip_info }, > + { "ltrf216a", .driver_data = (kernel_ulong_t)<rf216a_chip_info }, > {} > }; > MODULE_DEVICE_TABLE(i2c, ltrf216a_id); > > static const struct of_device_id ltrf216a_of_match[] = { > - { .compatible = "liteon,ltrf216a" }, > - { .compatible = "ltr,ltrf216a" }, > + { .compatible = "liteon,ltr308", .data = <r308_chip_info }, > + { .compatible = "liteon,ltrf216a", .data = <rf216a_chip_info }, > + { .compatible = "ltr,ltrf216a", .data = <rf216a_chip_info }, Drop this one. You cannot have undocumented compatibles - and checkpatch tells you this - and we do not want to accept stuff just because someone made something somewhere (e.g. ACPI, out of tree junk etc). There was similar effort in the past and we made it clear. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support 2024-07-05 9:40 ` Krzysztof Kozlowski @ 2024-07-05 9:47 ` Marek Vasut 0 siblings, 0 replies; 6+ messages in thread From: Marek Vasut @ 2024-07-05 9:47 UTC (permalink / raw) To: Krzysztof Kozlowski, linux-iio Cc: Conor Dooley, Jonathan Cameron, Krzysztof Kozlowski, Lars-Peter Clausen, Rob Herring, Shreeya Patel, devicetree On 7/5/24 11:40 AM, Krzysztof Kozlowski wrote: > On 05/07/2024 11:11, Marek Vasut 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@denx.de> >> --- > > ... > >> mutex_init(&data->lock); >> >> @@ -520,15 +537,27 @@ static int ltrf216a_runtime_resume(struct device *dev) >> static DEFINE_RUNTIME_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, >> ltrf216a_runtime_resume, NULL); >> >> +struct ltr_chip_info ltr308_chip_info = { > > static const > >> + .has_clear_data = false, >> + .lux_multiplier = 60, >> +}; >> + >> +struct ltr_chip_info ltrf216a_chip_info = { > > static const Both fixed, thanks. >> + .has_clear_data = true, >> + .lux_multiplier = 45, >> +}; >> + >> static const struct i2c_device_id ltrf216a_id[] = { >> - { "ltrf216a" }, >> + { "ltr308", .driver_data = (kernel_ulong_t)<r308_chip_info }, >> + { "ltrf216a", .driver_data = (kernel_ulong_t)<rf216a_chip_info }, >> {} >> }; >> MODULE_DEVICE_TABLE(i2c, ltrf216a_id); >> >> static const struct of_device_id ltrf216a_of_match[] = { >> - { .compatible = "liteon,ltrf216a" }, >> - { .compatible = "ltr,ltrf216a" }, >> + { .compatible = "liteon,ltr308", .data = <r308_chip_info }, >> + { .compatible = "liteon,ltrf216a", .data = <rf216a_chip_info }, >> + { .compatible = "ltr,ltrf216a", .data = <rf216a_chip_info }, > > Drop this one. You cannot have undocumented compatibles - and checkpatch > tells you this - and we do not want to accept stuff just because someone > made something somewhere (e.g. ACPI, out of tree junk etc). There was > similar effort in the past and we made it clear. The "ltr,ltrf216a" was already part of the driver, wouldn't removing it be an ABI break ? I will add a separate patch to remove it, so it can be reverted if someone complains. Thanks . ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support 2024-07-05 9:11 ` [PATCH 2/2] iio: light: ltrf216a: Add " Marek Vasut 2024-07-05 9:40 ` Krzysztof Kozlowski @ 2024-07-07 13:49 ` Jonathan Cameron 2024-07-08 11:41 ` Marek Vasut 1 sibling, 1 reply; 6+ messages in thread From: Jonathan Cameron @ 2024-07-07 13:49 UTC (permalink / raw) To: Marek Vasut Cc: linux-iio, Conor Dooley, Krzysztof Kozlowski, Lars-Peter Clausen, Rob Herring, Shreeya Patel, devicetree On Fri, 5 Jul 2024 11:11:45 +0200 Marek Vasut <marex@denx.de> 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@denx.de> 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; > } ; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support 2024-07-07 13:49 ` Jonathan Cameron @ 2024-07-08 11:41 ` Marek Vasut 0 siblings, 0 replies; 6+ messages in thread From: Marek Vasut @ 2024-07-08 11:41 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-iio, Conor Dooley, Krzysztof Kozlowski, Lars-Peter Clausen, Rob Herring, Shreeya Patel, devicetree On 7/7/24 3:49 PM, Jonathan Cameron wrote: > On Fri, 5 Jul 2024 11:11:45 +0200 > Marek Vasut <marex@denx.de> 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@denx.de> > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-08 11:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-05 9:11 [PATCH 1/2] dt-bindings: iio: light: ltrf216a: Document LTR-308 support Marek Vasut 2024-07-05 9:11 ` [PATCH 2/2] iio: light: ltrf216a: Add " Marek Vasut 2024-07-05 9:40 ` Krzysztof Kozlowski 2024-07-05 9:47 ` Marek Vasut 2024-07-07 13:49 ` Jonathan Cameron 2024-07-08 11:41 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).