On Mon, Mar 23, 2020 at 10:54:21PM +0100, Wolfram Sang wrote:
>
> > + struct i2c_timings i2c_t, *t = &i2c_t;
> >
> > /* Fall back to previously used values if not supplied */
> > - t->bus_freq_hz = t->bus_freq_hz ?: 100000;
> > - t->scl_fall_ns = t->scl_fall_ns ?: 35;
> > - t->scl_rise_ns = t->scl_rise_ns ?: 200;
> > - t->scl_int_delay_ns = t->scl_int_delay_ns ?: 50;
> > + t->bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
> > + t->scl_fall_ns = 35;
> > + t->scl_rise_ns = 200;
> > + t->scl_int_delay_ns = 50;
>
> Here, the initialization to 0 is missing, so some values are broken.
Yes, and this is fine. They are not being used. So, the idea is, whenever we
pass "false" as a parameter to the function we must take care of all fields we
are using.
> Why don't we just drop the pointer and init the array directly?
>
> struct i2c_timings t = {
> .bus_freq_hz = ...
> ...
> }
I can do it if you think it's better. I have no strong opinion here.
>From code prospective I guess it will be something similar anyway.
--
With Best Regards,
Andy Shevchenko
