lkml.org 
[lkml]   [2022]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V6 3/5] iio: accel: sca3300: modified to support multi chips
> >> +static int sca3300_set_frequency(struct sca3300_data *data, int val)
> >> +{
> >> + const struct sca3300_chip_info *chip = data->chip;
> >> + unsigned int index;
> >> + unsigned int i;
> >> +
> >> + if (sca3300_get_op_mode(data, &index))
> >> + return -EINVAL;
> >> +
> >> + for (i = 0; i < chip->num_avail_modes; i++) {
> >> + if ((val == chip->freq_table[chip->freq_map[i]]) &&
> >
> > The conditions being checked here are far from obvious, so I think this would benefit
> > from an explanatory comment.
> >
> > Something along the lines of,
> > "Find a mode in which the requested sampling frequency is available
> > and the scaling currently set is retained".
>
>
> In addition to a comment, how about small restructure of loop and giving
> local variables that tell the purpose, something like
>
>
> ...
>
> unsigned int opmode_scale, new_scale;
>
> opmode_scale = chip->accel_scale[chip->accel_scale_map[index]];
>
> /*
> * Find a mode in which the requested sampling frequency is available
> * and the scaling currently set is retained
> */
> for (i = 0; i < chip->num_avail_modes; i++) {
> if (val == chip->freq_table[chip->freq_map[i]]) {
> new_scale = chip->accel_scale[chip->accel_scale_map[i]]);
> if (opmode_scale == new_scale)
> break;
> }
> }
>
>
> That way it's IMHO more clear what we are comparing.
LGTM.

Thanks,

Jonathan

>
> Thanks,
> Tomas

\
 
 \ /
  Last update: 2022-05-22 12:36    [W:0.186 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site