On Sun, 15 Sep 2024 10:31:11 +0200 Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote: > On 14/09/2024 16:57, Jonathan Cameron wrote: > > On Fri, 13 Sep 2024 15:19:00 +0200 > > Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote: > > > >> The driver still uses the sensor resolution provided in the datasheet > >> until Rev. 1.6, 28-Apr-2022, which was updated with Rev 1.7, > >> 28-Nov-2023. The original ambient light resolution has been updated from > >> 0.0036 lx/ct to 0.0042 lx/ct, which is the value that can be found in > >> the current device datasheet. > >> > >> Update the default resolution for IT = 100 ms and GAIN = 1/8 from the > >> original 4608 mlux/cnt to the current value from the "Resolution and > >> maximum detection range" table (Application Note 84367, page 5), 5376 > >> mlux/cnt. > >> > >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> > > Interesting. So does the datasheet say this was fixing an error, or > > is there any chance there are different versions of the chip out there? > > > > Also, should we treat this as a fix? I think we probably should given > > we don't really want stable kernels to have wrong data being reported. > > If so, please reply with a fixes tag. > > > > Jonathan > > > > According to the Product Information Notification (link in the cover > letter): > > "Reason for Change: Adjusted resolution as this was wrongly stated in > the current datasheet." > > "If resolution is defined in the particular application by the customer, > no changes in the system should be made. In the case resolution was > taken from the datasheet or app note, this has to be adjusted accordingly." > > Which means that stable kernels are using the wrong resolution. I don't > know what IIO usually does in such cases, because a fix could > potentially make existing applications return "wrong data". If that is > alright, and applications are meant to be adjusted after the kernel > update, I have no problems to make this patch as a fix and add the > stable tag. It's unfortunate, but fixing a bug is a valid reason for ABI change (which this is - sort of) so existing applications will need to be fixed if anyone notices. So please send this as a fix with appropriate tags and that datasheet change log included in the patch description. Thanks, Jonathan > > Best regards, > Javier Carrasco > > >> --- > >> drivers/iio/light/veml6030.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c > >> index 5d4c2e35b987..d5add040d0b3 100644 > >> --- a/drivers/iio/light/veml6030.c > >> +++ b/drivers/iio/light/veml6030.c > >> @@ -779,7 +779,7 @@ static int veml6030_hw_init(struct iio_dev *indio_dev) > >> > >> /* Cache currently active measurement parameters */ > >> data->cur_gain = 3; > >> - data->cur_resolution = 4608; > >> + data->cur_resolution = 5376; > >> data->cur_integration_time = 3; > >> > >> return ret; > >> > > >
