| Message ID | 20220524201729.39503-2-eajames@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: pressure: dps310: Reset chip if MEAS_CFG is corrupt | expand |
On Tue, 24 May 2022 at 20:17, Eddie James <eajames@linux.ibm.com> wrote: > > Move the startup procedure into a function, and correct a missing > check on the return code for writing the PRS_CFG register. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > drivers/iio/pressure/dps310.c | 192 ++++++++++++++++++---------------- > 1 file changed, 103 insertions(+), 89 deletions(-) > > diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c > index 36fb7ae0d0a9..f79b274bb38d 100644 > --- a/drivers/iio/pressure/dps310.c > +++ b/drivers/iio/pressure/dps310.c > @@ -159,6 +159,106 @@ static int dps310_get_coefs(struct dps310_data *data) > return 0; > } > > +/* > + * Some verions of chip will read temperatures in the ~60C range when > + * its actually ~20C. This is the manufacturer recommended workaround > + * to correct the issue. The registers used below are undocumented. > + */ > +static int dps310_temp_workaround(struct dps310_data *data) > +{ > + int rc; > + int reg; > + > + rc = regmap_read(data->regmap, 0x32, ®); > + if (rc < 0) > + return rc; > + > + /* > + * If bit 1 is set then the device is okay, and the workaround does not > + * need to be applied > + */ > + if (reg & BIT(1)) > + return 0; > + > + rc = regmap_write(data->regmap, 0x0e, 0xA5); > + if (rc < 0) > + return rc; > + > + rc = regmap_write(data->regmap, 0x0f, 0x96); > + if (rc < 0) > + return rc; > + > + rc = regmap_write(data->regmap, 0x62, 0x02); > + if (rc < 0) > + return rc; > + > + rc = regmap_write(data->regmap, 0x0e, 0x00); > + if (rc < 0) > + return rc; > + > + return regmap_write(data->regmap, 0x0f, 0x00); > +} > + > +static int dps310_startup(struct dps310_data *data) > +{ > + int rc; > + int ready; > + > + /* > + * Set up pressure sensor in single sample, one measurement per second > + * mode > + */ > + rc = regmap_write(data->regmap, DPS310_PRS_CFG, 0); > + if (rc < 0) > + return rc; > + > + /* > + * Set up external (MEMS) temperature sensor in single sample, one > + * measurement per second mode > + */ > + rc = regmap_write(data->regmap, DPS310_TMP_CFG, DPS310_TMP_EXT); > + if (rc < 0) > + return rc; > + > + /* Temp and pressure shifts are disabled when PRC <= 8 */ > + rc = regmap_write_bits(data->regmap, DPS310_CFG_REG, > + DPS310_PRS_SHIFT_EN | DPS310_TMP_SHIFT_EN, 0); > + if (rc < 0) > + return rc; > + > + /* MEAS_CFG doesn't update correctly unless first written with 0 */ > + rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, > + DPS310_MEAS_CTRL_BITS, 0); > + if (rc < 0) > + return rc; > + > + /* Turn on temperature and pressure measurement in the background */ > + rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, > + DPS310_MEAS_CTRL_BITS, DPS310_PRS_EN | > + DPS310_TEMP_EN | DPS310_BACKGROUND); > + if (rc < 0) > + return rc; > + > + /* > + * Calibration coefficients required for reporting temperature. > + * They are available 40ms after the device has started > + */ > + rc = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready, > + ready & DPS310_COEF_RDY, 10000, 40000); > + if (rc < 0) > + return rc; > + > + rc = dps310_get_coefs(data); > + if (rc < 0) > + return rc; > + > + rc = dps310_temp_workaround(data); > + if (rc < 0) > + return rc; > + > + return 0; > +} > + > static int dps310_get_pres_precision(struct dps310_data *data) > { > int rc; > @@ -677,52 +777,12 @@ static const struct iio_info dps310_info = { > .write_raw = dps310_write_raw, > }; > > -/* > - * Some verions of chip will read temperatures in the ~60C range when > - * its actually ~20C. This is the manufacturer recommended workaround > - * to correct the issue. The registers used below are undocumented. > - */ > -static int dps310_temp_workaround(struct dps310_data *data) > -{ > - int rc; > - int reg; > - > - rc = regmap_read(data->regmap, 0x32, ®); > - if (rc < 0) > - return rc; > - > - /* > - * If bit 1 is set then the device is okay, and the workaround does not > - * need to be applied > - */ > - if (reg & BIT(1)) > - return 0; > - > - rc = regmap_write(data->regmap, 0x0e, 0xA5); > - if (rc < 0) > - return rc; > - > - rc = regmap_write(data->regmap, 0x0f, 0x96); > - if (rc < 0) > - return rc; > - > - rc = regmap_write(data->regmap, 0x62, 0x02); > - if (rc < 0) > - return rc; > - > - rc = regmap_write(data->regmap, 0x0e, 0x00); > - if (rc < 0) > - return rc; > - > - return regmap_write(data->regmap, 0x0f, 0x00); > -} > - > static int dps310_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct dps310_data *data; > struct iio_dev *iio; > - int rc, ready; > + int rc; > > iio = devm_iio_device_alloc(&client->dev, sizeof(*data)); > if (!iio) > @@ -747,54 +807,8 @@ static int dps310_probe(struct i2c_client *client, > if (rc) > return rc; > > - /* > - * Set up pressure sensor in single sample, one measurement per second > - * mode > - */ > - rc = regmap_write(data->regmap, DPS310_PRS_CFG, 0); > - > - /* > - * Set up external (MEMS) temperature sensor in single sample, one > - * measurement per second mode > - */ > - rc = regmap_write(data->regmap, DPS310_TMP_CFG, DPS310_TMP_EXT); > - if (rc < 0) > - return rc; > - > - /* Temp and pressure shifts are disabled when PRC <= 8 */ > - rc = regmap_write_bits(data->regmap, DPS310_CFG_REG, > - DPS310_PRS_SHIFT_EN | DPS310_TMP_SHIFT_EN, 0); > - if (rc < 0) > - return rc; > - > - /* MEAS_CFG doesn't update correctly unless first written with 0 */ > - rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, > - DPS310_MEAS_CTRL_BITS, 0); > - if (rc < 0) > - return rc; > - > - /* Turn on temperature and pressure measurement in the background */ > - rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, > - DPS310_MEAS_CTRL_BITS, DPS310_PRS_EN | > - DPS310_TEMP_EN | DPS310_BACKGROUND); > - if (rc < 0) > - return rc; > - > - /* > - * Calibration coefficients required for reporting temperature. > - * They are available 40ms after the device has started > - */ > - rc = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready, > - ready & DPS310_COEF_RDY, 10000, 40000); > - if (rc < 0) > - return rc; > - > - rc = dps310_get_coefs(data); > - if (rc < 0) > - return rc; > - > - rc = dps310_temp_workaround(data); > - if (rc < 0) > + rc = dps310_startup(data); > + if (rc) > return rc; > > rc = devm_iio_device_register(&client->dev, iio); > -- > 2.27.0 >
On Tue, May 24, 2022 at 10:35 PM Eddie James <eajames@linux.ibm.com> wrote: > > Move the startup procedure into a function, and correct a missing > check on the return code for writing the PRS_CFG register. ... > +/* > + * Some verions of chip will read temperatures in the ~60C range when versions of the chip(s) > + * its actually ~20C. This is the manufacturer recommended workaround it's > + * to correct the issue. The registers used below are undocumented. > + */ Yes, they are in the original code, but you may just fix while at it. ... > + rc = dps310_temp_workaround(data); > + if (rc < 0) These ' < 0' parts are cargo cult, I believe. That's why this makes code longer and confusing by "what is the meaning of the positive values here?" > + return rc; > + > + return 0; That said, return dps310_temp_workaround(data); should suffice.
diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c index 36fb7ae0d0a9..f79b274bb38d 100644 --- a/drivers/iio/pressure/dps310.c +++ b/drivers/iio/pressure/dps310.c @@ -159,6 +159,106 @@ static int dps310_get_coefs(struct dps310_data *data) return 0; } +/* + * Some verions of chip will read temperatures in the ~60C range when + * its actually ~20C. This is the manufacturer recommended workaround + * to correct the issue. The registers used below are undocumented. + */ +static int dps310_temp_workaround(struct dps310_data *data) +{ + int rc; + int reg; + + rc = regmap_read(data->regmap, 0x32, ®); + if (rc < 0) + return rc; + + /* + * If bit 1 is set then the device is okay, and the workaround does not + * need to be applied + */ + if (reg & BIT(1)) + return 0; + + rc = regmap_write(data->regmap, 0x0e, 0xA5); + if (rc < 0) + return rc; + + rc = regmap_write(data->regmap, 0x0f, 0x96); + if (rc < 0) + return rc; + + rc = regmap_write(data->regmap, 0x62, 0x02); + if (rc < 0) + return rc; + + rc = regmap_write(data->regmap, 0x0e, 0x00); + if (rc < 0) + return rc; + + return regmap_write(data->regmap, 0x0f, 0x00); +} + +static int dps310_startup(struct dps310_data *data) +{ + int rc; + int ready; + + /* + * Set up pressure sensor in single sample, one measurement per second + * mode + */ + rc = regmap_write(data->regmap, DPS310_PRS_CFG, 0); + if (rc < 0) + return rc; + + /* + * Set up external (MEMS) temperature sensor in single sample, one + * measurement per second mode + */ + rc = regmap_write(data->regmap, DPS310_TMP_CFG, DPS310_TMP_EXT); + if (rc < 0) + return rc; + + /* Temp and pressure shifts are disabled when PRC <= 8 */ + rc = regmap_write_bits(data->regmap, DPS310_CFG_REG, + DPS310_PRS_SHIFT_EN | DPS310_TMP_SHIFT_EN, 0); + if (rc < 0) + return rc; + + /* MEAS_CFG doesn't update correctly unless first written with 0 */ + rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, + DPS310_MEAS_CTRL_BITS, 0); + if (rc < 0) + return rc; + + /* Turn on temperature and pressure measurement in the background */ + rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, + DPS310_MEAS_CTRL_BITS, DPS310_PRS_EN | + DPS310_TEMP_EN | DPS310_BACKGROUND); + if (rc < 0) + return rc; + + /* + * Calibration coefficients required for reporting temperature. + * They are available 40ms after the device has started + */ + rc = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready, + ready & DPS310_COEF_RDY, 10000, 40000); + if (rc < 0) + return rc; + + rc = dps310_get_coefs(data); + if (rc < 0) + return rc; + + rc = dps310_temp_workaround(data); + if (rc < 0) + return rc; + + return 0; +} + static int dps310_get_pres_precision(struct dps310_data *data) { int rc; @@ -677,52 +777,12 @@ static const struct iio_info dps310_info = { .write_raw = dps310_write_raw, }; -/* - * Some verions of chip will read temperatures in the ~60C range when - * its actually ~20C. This is the manufacturer recommended workaround - * to correct the issue. The registers used below are undocumented. - */ -static int dps310_temp_workaround(struct dps310_data *data) -{ - int rc; - int reg; - - rc = regmap_read(data->regmap, 0x32, ®); - if (rc < 0) - return rc; - - /* - * If bit 1 is set then the device is okay, and the workaround does not - * need to be applied - */ - if (reg & BIT(1)) - return 0; - - rc = regmap_write(data->regmap, 0x0e, 0xA5); - if (rc < 0) - return rc; - - rc = regmap_write(data->regmap, 0x0f, 0x96); - if (rc < 0) - return rc; - - rc = regmap_write(data->regmap, 0x62, 0x02); - if (rc < 0) - return rc; - - rc = regmap_write(data->regmap, 0x0e, 0x00); - if (rc < 0) - return rc; - - return regmap_write(data->regmap, 0x0f, 0x00); -} - static int dps310_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct dps310_data *data; struct iio_dev *iio; - int rc, ready; + int rc; iio = devm_iio_device_alloc(&client->dev, sizeof(*data)); if (!iio) @@ -747,54 +807,8 @@ static int dps310_probe(struct i2c_client *client, if (rc) return rc; - /* - * Set up pressure sensor in single sample, one measurement per second - * mode - */ - rc = regmap_write(data->regmap, DPS310_PRS_CFG, 0); - - /* - * Set up external (MEMS) temperature sensor in single sample, one - * measurement per second mode - */ - rc = regmap_write(data->regmap, DPS310_TMP_CFG, DPS310_TMP_EXT); - if (rc < 0) - return rc; - - /* Temp and pressure shifts are disabled when PRC <= 8 */ - rc = regmap_write_bits(data->regmap, DPS310_CFG_REG, - DPS310_PRS_SHIFT_EN | DPS310_TMP_SHIFT_EN, 0); - if (rc < 0) - return rc; - - /* MEAS_CFG doesn't update correctly unless first written with 0 */ - rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, - DPS310_MEAS_CTRL_BITS, 0); - if (rc < 0) - return rc; - - /* Turn on temperature and pressure measurement in the background */ - rc = regmap_write_bits(data->regmap, DPS310_MEAS_CFG, - DPS310_MEAS_CTRL_BITS, DPS310_PRS_EN | - DPS310_TEMP_EN | DPS310_BACKGROUND); - if (rc < 0) - return rc; - - /* - * Calibration coefficients required for reporting temperature. - * They are available 40ms after the device has started - */ - rc = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready, - ready & DPS310_COEF_RDY, 10000, 40000); - if (rc < 0) - return rc; - - rc = dps310_get_coefs(data); - if (rc < 0) - return rc; - - rc = dps310_temp_workaround(data); - if (rc < 0) + rc = dps310_startup(data); + if (rc) return rc; rc = devm_iio_device_register(&client->dev, iio);
Move the startup procedure into a function, and correct a missing check on the return code for writing the PRS_CFG register. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/iio/pressure/dps310.c | 192 ++++++++++++++++++---------------- 1 file changed, 103 insertions(+), 89 deletions(-)