On Thu, Jun 10, 2021 at 4:31 PM Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote: > On 10.06.2021 16:21:30, Andy Shevchenko wrote: > > On Thu, Jun 10, 2021 at 3:55 PM Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote: ... > > > + int ret; > > > > > > ret = ltr501_drdy(data, LTR501_STATUS_PS_RDY); > > > if (ret < 0) > > > return ret; > > > > > > - ret = regmap_bulk_read(data->regmap, LTR501_PS_DATA, > > > - &status, 2); > > > - if (ret < 0) > > > - return ret; > > > - > > > - return status; > > > + return regmap_bulk_read(data->regmap, LTR501_PS_DATA, > > > + buf, sizeof(__le16)); > > > > This is slightly weird since we pass a pointer to a buffer of one > > element (buffer can be longer, but here it's always one element is in > > use). The original code is better (initially). Also making caller to > > do endiannes conversion each time is not good. > > We decided to implement the same semantics as ltr501_read_als(), where > the caller does the endianness conversion. I'm fully in favour of consistency, but in this case I think it would gain from converting the old code to the above mentioned schema. > I'll change the code and send > a v2 (with a proper cover letter subject :)) Thank you! -- With Best Regards, Andy Shevchenko
