Hi Geert,
> > +{
> > + int ret;
> > +
> > + ret = device_property_read_u32(dev, prop_name, cur_val_p);
> > + if (ret && use_def)
> > + *cur_val_p = def_val;
>
> Alternatively, you could just preinitialize the value with the default value
> before calling this function, and ignoring ret.
> That would remove the need for both the def_val and use_def parameters.
I can't do that because if !use_def and ret, then the value must not be
changed.
> > + if (t->bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ)
> > + d = 1000;
> > + else if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ)
> > + d = 300;
> > + else
> > + d = 120;
> > + i2c_parse_timing(dev, "i2c-scl-rising-time-ns", &t->scl_rise_ns, d, u);
> >
> > - ret = device_property_read_u32(dev, "i2c-analog-filter-cutoff-frequency", &t->analog_filter_cutoff_freq_hz);
> > - if (ret && use_defaults)
> > - t->analog_filter_cutoff_freq_hz = 0;
> > + if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ)
> > + d = 300;
> > + else
> > + d = 120;
>
> Is the difference with above intentional, or an oversight?
If this is an oversight, then it is also in the I2C specs ;)
> if the former, I like the dreaded ternary operator (only) for cases like this:
>
> d = t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ ? 300 : 120
Yup, that would be an improvement!
Thanks,
Wolfram
Attachment:
signature.asc
Description: PGP signature
