Messages in this thread | | | Date | Mon, 20 Nov 2023 18:21:05 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v4 2/2] iio: adc: ad7173: add AD7173 driver |
| |
On Mon, Nov 20, 2023 at 05:55:12PM +0200, Ceclan Dumitru wrote: > On 11/20/23 15:00, Andy Shevchenko wrote: > > On Thu, Nov 16, 2023 at 03:46:55PM +0200, mitrutzceclan wrote:
...
> >> +struct ad7173_channel_config { > >> + bool live; > >> + u8 cfg_slot; > >> + /* Following fields are used to compare equality. Bipolar must be first */ > >> + bool bipolar; > >> + bool input_buf; > >> + u8 odr; > >> + u8 ref_sel; > > > > If you group better by types, it might save a few bytes on the architectures / > > compilers where bool != byte. > > > Grouping by type will result in not being able to use memcmp() for > comparing configs. But then there is the issue that I was under the > assumption that bool=byte. If that is not the case, the config equality > check might be comparing padding bytes.
It's most likely the case, BUT... it is not guaranteed by the C standard.
> In this case what do you suggest: > - using the packed attribute > - using only u8 > - drop memcmp, manually compare fields
Use struct_group() to show explicitly the group of members. If it's not an ABI in any sense, then memcmp() is fine.
...
> >> + cmp_size = sizeof(*cfg) - offset; > > > > sizeof_field() from the above mentioned header? > > This computes the size of multiple fields, following cfg_slot. Better to > group the fields that need to be compared into another struct then use > sizeof_field()?
See above.
...
> >> + return vref / (MICRO/MILLI); > > > > What does the denominator mean and why you can't simply use MILL? > > Original vref values are in micro, I considered that it was adequate to > represent the conversion from MICRO to MILLI as a fraction. > > >> + *val = st->info->sinc5_data_rates[reg] / (MICRO/MILLI); > >> + *val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO/MILLI); > > > > Same Q about denominator. > > > Here, a misunderstanding on my part of a suggestion from Jonathan in V2, > will be removed.
You need to clarify with him that.
-- With Best Regards, Andy Shevchenko
| |