On Tue, 21 Apr 2020 15:22:11 +0300
Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Tue, Apr 21, 2020 at 10:56 AM Mathieu Othacehe <m.othacehe@xxxxxxxxx> wrote:
> >
> > Add sampling frequency support for proximity data on VCNL4010 and VCNL4020
> > chips.
>
> Couple of nitpicks below.
>
> ...
>
> > +static const int vcnl4010_prox_sampling_frequency[][2] = {
> > + {1, 950000},
> > + {3, 906250},
> > + {7, 812500},
> > + {16, 625000},
> > + {31, 250000},
> > + {62, 500000},
> > + {125, 0},
>
> > + {250, 0}
>
> Leave comma here, potentially helpful if it will be extended.
Hi Andy,
Doesn't particularly matter either way, but given this is a list of the values
supported by the device, very unlikely it will be extended.
Games like trying to share the first part of a longer array between
multiple device types might occur, but those are usually really ugly.
Jonathan
>
> > +};
>
> ...
>
> > +static int vcnl4010_write_proxy_samp_freq(struct vcnl4000_data *data, int val,
> > + int val2)
> > +{
> > + int i;
> > + const int len = ARRAY_SIZE(vcnl4010_prox_sampling_frequency);
> > +
> > + for (i = 0; i < len; i++) {
> > + if (val <= vcnl4010_prox_sampling_frequency[i][0])
> > + break;
> > + }
> > +
> > + if (i == len)
> > + return -EINVAL;
>
> I would refactor this
>
> unsigned int i = ARRAY_SIZE(vcnl4010_prox_sampling_frequency);
>
> do {
> if (val > vcnl4010_prox_sampling_frequency[i][0])
> break;
> } while (--i);
>
> You won't need to go full array to return error in this case.
>
> > + return i2c_smbus_write_byte_data(data->client, VCNL4010_PROX_RATE, i);
> > +}
>
