| Message ID | c65f8a6f2c1ccadc9682b0e87b1fd9c47b912af7.1666599595.git.cmo@melexis.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: temperature: mlx90632 Style alignment for the driver | expand |
On Mon, Oct 24, 2022 at 11:24 AM Crt Mori <cmo@melexis.com> wrote: > > Changing and aligning the overall style of the driver with the recent > reviews. There is no functional change, only type generalization and > moving to the reverse Christmas tree for variable declarations. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Crt Mori <cmo@melexis.com> > --- > drivers/iio/temperature/mlx90632.c | 65 +++++++++++++++++------------- > 1 file changed, 36 insertions(+), 29 deletions(-) > > diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c > index 224db7513baa..a1b9d7fb2bb0 100644 > --- a/drivers/iio/temperature/mlx90632.c > +++ b/drivers/iio/temperature/mlx90632.c > @@ -219,11 +219,11 @@ static const struct regmap_config mlx90632_regmap = { > .cache_type = REGCACHE_RBTREE, > }; > > -static s32 mlx90632_pwr_set_sleep_step(struct regmap *regmap) > +static int mlx90632_pwr_set_sleep_step(struct regmap *regmap) > { > struct mlx90632_data *data = > iio_priv(dev_get_drvdata(regmap_get_device(regmap))); > - s32 ret; > + int ret; > > if (data->powerstatus == MLX90632_PWR_STATUS_SLEEP_STEP) > return 0; > @@ -234,14 +234,14 @@ static s32 mlx90632_pwr_set_sleep_step(struct regmap *regmap) > return ret; > > data->powerstatus = MLX90632_PWR_STATUS_SLEEP_STEP; > - return ret; > + return 0; > } > > -static s32 mlx90632_pwr_continuous(struct regmap *regmap) > +static int mlx90632_pwr_continuous(struct regmap *regmap) > { > struct mlx90632_data *data = > iio_priv(dev_get_drvdata(regmap_get_device(regmap))); > - s32 ret; > + int ret; > > if (data->powerstatus == MLX90632_PWR_STATUS_CONTINUOUS) > return 0; > @@ -252,7 +252,7 @@ static s32 mlx90632_pwr_continuous(struct regmap *regmap) > return ret; > > data->powerstatus = MLX90632_PWR_STATUS_CONTINUOUS; > - return ret; > + return 0; > } > > /** > @@ -444,8 +444,8 @@ static int mlx90632_channel_new_select(int perform_ret, uint8_t *channel_new, > static int mlx90632_read_ambient_raw(struct regmap *regmap, > s16 *ambient_new_raw, s16 *ambient_old_raw) > { > - int ret; > unsigned int read_tmp; > + int ret; > > ret = regmap_read(regmap, MLX90632_RAM_3(1), &read_tmp); > if (ret < 0) > @@ -464,11 +464,11 @@ static int mlx90632_read_object_raw(struct regmap *regmap, > int perform_measurement_ret, > s16 *object_new_raw, s16 *object_old_raw) > { > - int ret; > unsigned int read_tmp; > - s16 read; > - u8 channel = 0; > u8 channel_old = 0; > + u8 channel = 0; > + s16 read; > + int ret; > > ret = mlx90632_channel_new_select(perform_measurement_ret, &channel, > &channel_old); > @@ -503,7 +503,8 @@ static int mlx90632_read_all_channel(struct mlx90632_data *data, > s16 *ambient_new_raw, s16 *ambient_old_raw, > s16 *object_new_raw, s16 *object_old_raw) > { > - s32 ret, measurement; > + s32 measurement; > + int ret; > > mutex_lock(&data->lock); > ret = mlx90632_set_meas_type(data, MLX90632_MTYP_MEDICAL); > @@ -512,24 +513,24 @@ static int mlx90632_read_all_channel(struct mlx90632_data *data, > > switch (data->powerstatus) { > case MLX90632_PWR_STATUS_CONTINUOUS: > - measurement = mlx90632_perform_measurement(data); > - if (measurement < 0) { > - ret = measurement; > + ret = mlx90632_perform_measurement(data); > + if (ret < 0) > goto read_unlock; > - } > + > break; > case MLX90632_PWR_STATUS_SLEEP_STEP: > - measurement = mlx90632_perform_measurement_burst(data); > - if (measurement < 0) { > - ret = measurement; > + ret = mlx90632_perform_measurement_burst(data); > + if (ret < 0) > goto read_unlock; > - } > + > break; > default: > ret = -EOPNOTSUPP; > goto read_unlock; > } > > + measurement = ret; /* If we came here ret holds the measurement position */ > + > ret = mlx90632_read_ambient_raw(data->regmap, ambient_new_raw, > ambient_old_raw); > if (ret < 0) > @@ -615,15 +616,21 @@ static int mlx90632_read_all_channel_extended(struct mlx90632_data *data, s16 *o > if (ret < 0) > goto read_unlock; > > - if (data->powerstatus == MLX90632_PWR_STATUS_CONTINUOUS) { > + switch (data->powerstatus) { > + case MLX90632_PWR_STATUS_CONTINUOUS: > ret = read_poll_timeout(mlx90632_perform_measurement, meas, meas == 19, > 50000, 800000, false, data); > if (ret) > goto read_unlock; > - } else if (data->powerstatus == MLX90632_PWR_STATUS_SLEEP_STEP) { > + break; > + case MLX90632_PWR_STATUS_SLEEP_STEP: > ret = mlx90632_perform_measurement_burst(data); > if (ret < 0) > goto read_unlock; > + break; > + default: > + ret = -EOPNOTSUPP; > + goto read_unlock; > } > > ret = mlx90632_read_object_raw_extended(data->regmap, object_new_raw); > @@ -640,9 +647,9 @@ static int mlx90632_read_all_channel_extended(struct mlx90632_data *data, s16 *o > static int mlx90632_read_ee_register(struct regmap *regmap, u16 reg_lsb, > s32 *reg_value) > { > - s32 ret; > unsigned int read; > u32 value; > + s32 ret; > > ret = regmap_read(regmap, reg_lsb, &read); > if (ret < 0) > @@ -806,12 +813,12 @@ static s32 mlx90632_calc_temp_object_extended(s64 object, s64 ambient, s64 refle > > static int mlx90632_calc_object_dsp105(struct mlx90632_data *data, int *val) > { > - s32 ret; > + s16 ambient_new_raw, ambient_old_raw, object_new_raw, object_old_raw; > s32 Ea, Eb, Fa, Fb, Ga; > unsigned int read_tmp; > - s16 Ha, Hb, Gb, Ka; > - s16 ambient_new_raw, ambient_old_raw, object_new_raw, object_old_raw; > s64 object, ambient; > + s16 Ha, Hb, Gb, Ka; > + s32 ret; > > ret = mlx90632_read_ee_register(data->regmap, MLX90632_EE_Ea, &Ea); > if (ret < 0) > @@ -885,11 +892,11 @@ static int mlx90632_calc_object_dsp105(struct mlx90632_data *data, int *val) > > static int mlx90632_calc_ambient_dsp105(struct mlx90632_data *data, int *val) > { > - s32 ret; > + s16 ambient_new_raw, ambient_old_raw; > unsigned int read_tmp; > s32 PT, PR, PG, PO; > + s32 ret; > s16 Gb; > - s16 ambient_new_raw, ambient_old_raw; > > ret = mlx90632_read_ee_register(data->regmap, MLX90632_EE_P_R, &PR); > if (ret < 0) > @@ -1164,11 +1171,11 @@ static int mlx90632_enable_regulator(struct mlx90632_data *data) > static int mlx90632_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > - struct iio_dev *indio_dev; > struct mlx90632_data *mlx90632; > + struct iio_dev *indio_dev; > struct regmap *regmap; > - int ret; > unsigned int read; > + int ret; > > indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*mlx90632)); > if (!indio_dev) { > -- > 2.34.1 >
Le 24/10/2022 à 10:23, Crt Mori a écrit : > Changing and aligning the overall style of the driver with the recent > reviews. There is no functional change, only type generalization and > moving to the reverse Christmas tree for variable declarations. > Hi, i don't have the patch on which it is based, but I think that a few s/s32/int/ are missing. See below. CJ > Signed-off-by: Crt Mori <cmo@melexis.com> > --- > drivers/iio/temperature/mlx90632.c | 65 +++++++++++++++++------------- > 1 file changed, 36 insertions(+), 29 deletions(-) > > diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c > index 224db7513baa..a1b9d7fb2bb0 100644 > --- a/drivers/iio/temperature/mlx90632.c > +++ b/drivers/iio/temperature/mlx90632.c > @@ -219,11 +219,11 @@ static const struct regmap_config mlx90632_regmap = { > .cache_type = REGCACHE_RBTREE, > }; > > -static s32 mlx90632_pwr_set_sleep_step(struct regmap *regmap) > +static int mlx90632_pwr_set_sleep_step(struct regmap *regmap) > { > struct mlx90632_data *data = > iio_priv(dev_get_drvdata(regmap_get_device(regmap))); > - s32 ret; > + int ret; > > if (data->powerstatus == MLX90632_PWR_STATUS_SLEEP_STEP) > return 0; > @@ -234,14 +234,14 @@ static s32 mlx90632_pwr_set_sleep_step(struct regmap *regmap) > return ret; > > data->powerstatus = MLX90632_PWR_STATUS_SLEEP_STEP; > - return ret; > + return 0; > } > > -static s32 mlx90632_pwr_continuous(struct regmap *regmap) > +static int mlx90632_pwr_continuous(struct regmap *regmap) > { > struct mlx90632_data *data = > iio_priv(dev_get_drvdata(regmap_get_device(regmap))); > - s32 ret; > + int ret; > > if (data->powerstatus == MLX90632_PWR_STATUS_CONTINUOUS) > return 0; > @@ -252,7 +252,7 @@ static s32 mlx90632_pwr_continuous(struct regmap *regmap) > return ret; > > data->powerstatus = MLX90632_PWR_STATUS_CONTINUOUS; > - return ret; > + return 0; > } > > /** > @@ -444,8 +444,8 @@ static int mlx90632_channel_new_select(int perform_ret, uint8_t *channel_new, > static int mlx90632_read_ambient_raw(struct regmap *regmap, > s16 *ambient_new_raw, s16 *ambient_old_raw) > { > - int ret; > unsigned int read_tmp; > + int ret; > > ret = regmap_read(regmap, MLX90632_RAM_3(1), &read_tmp); > if (ret < 0) > @@ -464,11 +464,11 @@ static int mlx90632_read_object_raw(struct regmap *regmap, > int perform_measurement_ret, > s16 *object_new_raw, s16 *object_old_raw) > { > - int ret; > unsigned int read_tmp; > - s16 read; > - u8 channel = 0; > u8 channel_old = 0; > + u8 channel = 0; > + s16 read; > + int ret; > > ret = mlx90632_channel_new_select(perform_measurement_ret, &channel, > &channel_old); > @@ -503,7 +503,8 @@ static int mlx90632_read_all_channel(struct mlx90632_data *data, > s16 *ambient_new_raw, s16 *ambient_old_raw, > s16 *object_new_raw, s16 *object_old_raw) > { > - s32 ret, measurement; > + s32 measurement; > + int ret; > > mutex_lock(&data->lock); > ret = mlx90632_set_meas_type(data, MLX90632_MTYP_MEDICAL); > @@ -512,24 +513,24 @@ static int mlx90632_read_all_channel(struct mlx90632_data *data, > > switch (data->powerstatus) { > case MLX90632_PWR_STATUS_CONTINUOUS: > - measurement = mlx90632_perform_measurement(data); > - if (measurement < 0) { > - ret = measurement; > + ret = mlx90632_perform_measurement(data); > + if (ret < 0) > goto read_unlock; > - } > + > break; > case MLX90632_PWR_STATUS_SLEEP_STEP: > - measurement = mlx90632_perform_measurement_burst(data); > - if (measurement < 0) { > - ret = measurement; > + ret = mlx90632_perform_measurement_burst(data); > + if (ret < 0) > goto read_unlock; > - } > + > break; > default: > ret = -EOPNOTSUPP; > goto read_unlock; > } > > + measurement = ret; /* If we came here ret holds the measurement position */ > + > ret = mlx90632_read_ambient_raw(data->regmap, ambient_new_raw, > ambient_old_raw); > if (ret < 0) > @@ -615,15 +616,21 @@ static int mlx90632_read_all_channel_extended(struct mlx90632_data *data, s16 *o > if (ret < 0) > goto read_unlock; > > - if (data->powerstatus == MLX90632_PWR_STATUS_CONTINUOUS) { > + switch (data->powerstatus) { > + case MLX90632_PWR_STATUS_CONTINUOUS: > ret = read_poll_timeout(mlx90632_perform_measurement, meas, meas == 19, > 50000, 800000, false, data); > if (ret) > goto read_unlock; > - } else if (data->powerstatus == MLX90632_PWR_STATUS_SLEEP_STEP) { > + break; > + case MLX90632_PWR_STATUS_SLEEP_STEP: > ret = mlx90632_perform_measurement_burst(data); > if (ret < 0) > goto read_unlock; > + break; > + default: > + ret = -EOPNOTSUPP; > + goto read_unlock; > } > > ret = mlx90632_read_object_raw_extended(data->regmap, object_new_raw); > @@ -640,9 +647,9 @@ static int mlx90632_read_all_channel_extended(struct mlx90632_data *data, s16 *o > static int mlx90632_read_ee_register(struct regmap *regmap, u16 reg_lsb, > s32 *reg_value) > { > - s32 ret; > unsigned int read; > u32 value; > + s32 ret; int? > > ret = regmap_read(regmap, reg_lsb, &read); > if (ret < 0) > @@ -806,12 +813,12 @@ static s32 mlx90632_calc_temp_object_extended(s64 object, s64 ambient, s64 refle > > static int mlx90632_calc_object_dsp105(struct mlx90632_data *data, int *val) > { > - s32 ret; > + s16 ambient_new_raw, ambient_old_raw, object_new_raw, object_old_raw; > s32 Ea, Eb, Fa, Fb, Ga; > unsigned int read_tmp; > - s16 Ha, Hb, Gb, Ka; > - s16 ambient_new_raw, ambient_old_raw, object_new_raw, object_old_raw; > s64 object, ambient; > + s16 Ha, Hb, Gb, Ka; > + s32 ret; int? > ret = mlx90632_read_ee_register(data->regmap, MLX90632_EE_Ea, &Ea); > if (ret < 0) > @@ -885,11 +892,11 @@ static int mlx90632_calc_object_dsp105(struct mlx90632_data *data, int *val) > > static int mlx90632_calc_ambient_dsp105(struct mlx90632_data *data, int *val) > { > - s32 ret; > + s16 ambient_new_raw, ambient_old_raw; > unsigned int read_tmp; > s32 PT, PR, PG, PO; > + s32 ret; int? > s16 Gb; > - s16 ambient_new_raw, ambient_old_raw; > > ret = mlx90632_read_ee_register(data->regmap, MLX90632_EE_P_R, &PR); > if (ret < 0) > @@ -1164,11 +1171,11 @@ static int mlx90632_enable_regulator(struct mlx90632_data *data) > static int mlx90632_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > - struct iio_dev *indio_dev; > struct mlx90632_data *mlx90632; > + struct iio_dev *indio_dev; > struct regmap *regmap; > - int ret; > unsigned int read; > + int ret; > > indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*mlx90632)); > if (!indio_dev) {
diff --git a/drivers/iio/temperature/mlx90632.c b/drivers/iio/temperature/mlx90632.c index 224db7513baa..a1b9d7fb2bb0 100644 --- a/drivers/iio/temperature/mlx90632.c +++ b/drivers/iio/temperature/mlx90632.c @@ -219,11 +219,11 @@ static const struct regmap_config mlx90632_regmap = { .cache_type = REGCACHE_RBTREE, }; -static s32 mlx90632_pwr_set_sleep_step(struct regmap *regmap) +static int mlx90632_pwr_set_sleep_step(struct regmap *regmap) { struct mlx90632_data *data = iio_priv(dev_get_drvdata(regmap_get_device(regmap))); - s32 ret; + int ret; if (data->powerstatus == MLX90632_PWR_STATUS_SLEEP_STEP) return 0; @@ -234,14 +234,14 @@ static s32 mlx90632_pwr_set_sleep_step(struct regmap *regmap) return ret; data->powerstatus = MLX90632_PWR_STATUS_SLEEP_STEP; - return ret; + return 0; } -static s32 mlx90632_pwr_continuous(struct regmap *regmap) +static int mlx90632_pwr_continuous(struct regmap *regmap) { struct mlx90632_data *data = iio_priv(dev_get_drvdata(regmap_get_device(regmap))); - s32 ret; + int ret; if (data->powerstatus == MLX90632_PWR_STATUS_CONTINUOUS) return 0; @@ -252,7 +252,7 @@ static s32 mlx90632_pwr_continuous(struct regmap *regmap) return ret; data->powerstatus = MLX90632_PWR_STATUS_CONTINUOUS; - return ret; + return 0; } /** @@ -444,8 +444,8 @@ static int mlx90632_channel_new_select(int perform_ret, uint8_t *channel_new, static int mlx90632_read_ambient_raw(struct regmap *regmap, s16 *ambient_new_raw, s16 *ambient_old_raw) { - int ret; unsigned int read_tmp; + int ret; ret = regmap_read(regmap, MLX90632_RAM_3(1), &read_tmp); if (ret < 0) @@ -464,11 +464,11 @@ static int mlx90632_read_object_raw(struct regmap *regmap, int perform_measurement_ret, s16 *object_new_raw, s16 *object_old_raw) { - int ret; unsigned int read_tmp; - s16 read; - u8 channel = 0; u8 channel_old = 0; + u8 channel = 0; + s16 read; + int ret; ret = mlx90632_channel_new_select(perform_measurement_ret, &channel, &channel_old); @@ -503,7 +503,8 @@ static int mlx90632_read_all_channel(struct mlx90632_data *data, s16 *ambient_new_raw, s16 *ambient_old_raw, s16 *object_new_raw, s16 *object_old_raw) { - s32 ret, measurement; + s32 measurement; + int ret; mutex_lock(&data->lock); ret = mlx90632_set_meas_type(data, MLX90632_MTYP_MEDICAL); @@ -512,24 +513,24 @@ static int mlx90632_read_all_channel(struct mlx90632_data *data, switch (data->powerstatus) { case MLX90632_PWR_STATUS_CONTINUOUS: - measurement = mlx90632_perform_measurement(data); - if (measurement < 0) { - ret = measurement; + ret = mlx90632_perform_measurement(data); + if (ret < 0) goto read_unlock; - } + break; case MLX90632_PWR_STATUS_SLEEP_STEP: - measurement = mlx90632_perform_measurement_burst(data); - if (measurement < 0) { - ret = measurement; + ret = mlx90632_perform_measurement_burst(data); + if (ret < 0) goto read_unlock; - } + break; default: ret = -EOPNOTSUPP; goto read_unlock; } + measurement = ret; /* If we came here ret holds the measurement position */ + ret = mlx90632_read_ambient_raw(data->regmap, ambient_new_raw, ambient_old_raw); if (ret < 0) @@ -615,15 +616,21 @@ static int mlx90632_read_all_channel_extended(struct mlx90632_data *data, s16 *o if (ret < 0) goto read_unlock; - if (data->powerstatus == MLX90632_PWR_STATUS_CONTINUOUS) { + switch (data->powerstatus) { + case MLX90632_PWR_STATUS_CONTINUOUS: ret = read_poll_timeout(mlx90632_perform_measurement, meas, meas == 19, 50000, 800000, false, data); if (ret) goto read_unlock; - } else if (data->powerstatus == MLX90632_PWR_STATUS_SLEEP_STEP) { + break; + case MLX90632_PWR_STATUS_SLEEP_STEP: ret = mlx90632_perform_measurement_burst(data); if (ret < 0) goto read_unlock; + break; + default: + ret = -EOPNOTSUPP; + goto read_unlock; } ret = mlx90632_read_object_raw_extended(data->regmap, object_new_raw); @@ -640,9 +647,9 @@ static int mlx90632_read_all_channel_extended(struct mlx90632_data *data, s16 *o static int mlx90632_read_ee_register(struct regmap *regmap, u16 reg_lsb, s32 *reg_value) { - s32 ret; unsigned int read; u32 value; + s32 ret; ret = regmap_read(regmap, reg_lsb, &read); if (ret < 0) @@ -806,12 +813,12 @@ static s32 mlx90632_calc_temp_object_extended(s64 object, s64 ambient, s64 refle static int mlx90632_calc_object_dsp105(struct mlx90632_data *data, int *val) { - s32 ret; + s16 ambient_new_raw, ambient_old_raw, object_new_raw, object_old_raw; s32 Ea, Eb, Fa, Fb, Ga; unsigned int read_tmp; - s16 Ha, Hb, Gb, Ka; - s16 ambient_new_raw, ambient_old_raw, object_new_raw, object_old_raw; s64 object, ambient; + s16 Ha, Hb, Gb, Ka; + s32 ret; ret = mlx90632_read_ee_register(data->regmap, MLX90632_EE_Ea, &Ea); if (ret < 0) @@ -885,11 +892,11 @@ static int mlx90632_calc_object_dsp105(struct mlx90632_data *data, int *val) static int mlx90632_calc_ambient_dsp105(struct mlx90632_data *data, int *val) { - s32 ret; + s16 ambient_new_raw, ambient_old_raw; unsigned int read_tmp; s32 PT, PR, PG, PO; + s32 ret; s16 Gb; - s16 ambient_new_raw, ambient_old_raw; ret = mlx90632_read_ee_register(data->regmap, MLX90632_EE_P_R, &PR); if (ret < 0) @@ -1164,11 +1171,11 @@ static int mlx90632_enable_regulator(struct mlx90632_data *data) static int mlx90632_probe(struct i2c_client *client, const struct i2c_device_id *id) { - struct iio_dev *indio_dev; struct mlx90632_data *mlx90632; + struct iio_dev *indio_dev; struct regmap *regmap; - int ret; unsigned int read; + int ret; indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*mlx90632)); if (!indio_dev) {
Changing and aligning the overall style of the driver with the recent reviews. There is no functional change, only type generalization and moving to the reverse Christmas tree for variable declarations. Signed-off-by: Crt Mori <cmo@melexis.com> --- drivers/iio/temperature/mlx90632.c | 65 +++++++++++++++++------------- 1 file changed, 36 insertions(+), 29 deletions(-)