hello! On Wed, Nov 29, 2023 at 07:24:31PM +0200, Andy Shevchenko wrote: > On Wed, Nov 29, 2023 at 07:04:12PM +0200, Petre Rodan wrote: > > Adds driver for digital Honeywell TruStability HSC and SSC series > > pressure and temperature sensors. > > Communication is one way. The sensor only requires 4 bytes worth of > > clock pulses on both i2c and spi in order to push the data out. > > The i2c address is hardcoded and depends on the part number. > > There is no additional GPIO control. > > ... > > > v6: modifications based on Andy's review > > - use str_has_prefix(), match_string() instead of strncmp() > > And why not using the respective property API for that case where > match_string() is used? I'm lost again. 437: ret = device_property_read_string(dev, "honeywell,pressure-triplet", &triplet); [..] 455: ret = match_string(hsc_triplet_variants, HSC_VARIANTS_MAX, triplet); if (ret < 0) return dev_err_probe(dev, -EINVAL, "honeywell,pressure-triplet is invalid\n"); hsc->pmin = hsc_range_config[ret].pmin; hsc->pmax = hsc_range_config[ret].pmax; triplet is got via device_property_read_string(), is there some other property function I should be using? > I'm also a bit tired to repeat about: > - capitalization and punctuation in the multi-line comments; > - broken indentation is some cases. sorry about that. I removed all comments you complained about. just to simplify the process. > Otherwise it's a good stuff, I leave it now to Jonathan. > > ... > > > + tmp = div_s64(((s64)(hsc->pmax - hsc->pmin)) * MICRO, > > + hsc->outmax - hsc->outmin); > > + hsc->p_scale = div_s64_rem(tmp, NANO, &hsc->p_scale_dec); > > + tmp = div_s64(((s64)hsc->pmin * (s64)(hsc->outmax - hsc->outmin)) * > > + MICRO, hsc->pmax - hsc->pmin); > > Why not put MICRO on the previous line? oh well, from the review I understood you were asking for the replacement of NANO with MICRO on the previous instruction and it did not make much sense ( units are in pascal and we need a kilopascal output to userland) now I understood it's an indentation request. however moving MICRO will cross the 80 column rule. but if there will be yet another modification request I'll move it. cheers, peter > > -- > With Best Regards, > Andy Shevchenko > > -- petre rodan