From: Jonathan Cameron <jic23@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: linux-amlogic@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, gnstark@salutedevices.com,
neil.armstrong@linaro.org, lars@metafoo.de
Subject: Re: [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
Date: Sun, 24 Mar 2024 14:07:56 +0000 [thread overview]
Message-ID: <20240324140756.5d001387@jic23-huawei> (raw)
In-Reply-To: <20240323231309.415425-3-martin.blumenstingl@googlemail.com>
On Sun, 24 Mar 2024 00:13:08 +0100
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
> 1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
> #define MESON_SAR_ADC_REG11 0x2c
> #define MESON_SAR_ADC_REG11_BANDGAP_EN BIT(13)
> #define MESON_SAR_ADC_REG11_CMV_SEL BIT(6)
> - #define MESON_SAR_ADC_REG11_VREF_VOLTAGE BIT(5)
> - #define MESON_SAR_ADC_REG11_EOC BIT(1)
> - #define MESON_SAR_ADC_REG11_VREF_SEL BIT(0)
> + #define MESON_SAR_ADC_REG11_VREF_VOLTAGE BIT(5)
> + #define MESON_SAR_ADC_REG11_EOC BIT(1)
> + #define MESON_SAR_ADC_REG11_VREF_SEL BIT(0)
Looks like a spaces to tab conversion. Not related to rest of patch so
put them back as spaces. If you really want to tidy that up with tabs
later, separate patch please.
Otherwise LGTM
>
> #define MESON_SAR_ADC_REG13 0x34
> #define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
> VREF_VDDA = 1,
> };
>
> +enum meson_sar_adc_vref_voltage {
> + VREF_VOLTAGE_0V9 = 0,
> + VREF_VOLTAGE_1V8 = 1,
> +};
> +
> enum meson_sar_adc_avg_mode {
> NO_AVERAGING = 0x0,
> MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
> u8 temperature_trimming_bits;
> unsigned int temperature_multiplier;
> unsigned int temperature_divider;
> - u8 disable_ring_counter;
> + bool disable_ring_counter;
> bool has_reg11;
> bool has_vref_select;
> - u8 vref_select;
> - u8 cmv_select;
> - u8 adc_eoc;
> - enum meson_sar_adc_vref_sel vref_voltage;
> + bool cmv_select;
> + bool adc_eoc;
> + enum meson_sar_adc_vref_sel vref_select;
> + enum meson_sar_adc_vref_voltage vref_voltage;
> };
>
> struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
> MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
> }
>
> - regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> - priv->param->disable_ring_counter);
> + if (priv->param->disable_ring_counter)
> + regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> + else
> + regval = 0;
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
> MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> regval);
>
> if (priv->param->has_reg11) {
> - regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> + regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_EOC, regval);
>
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>
> - regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> - priv->param->cmv_select);
> + regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_CMV_SEL, regval);
> }
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
> .regmap_config = &meson_sar_adc_regmap_config_gxbb,
> .resolution = 10,
> .has_reg11 = true,
> - .vref_voltage = 1,
> - .cmv_select = 1,
> + .vref_voltage = VREF_VOLTAGE_1V8,
> + .cmv_select = true,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .vref_voltage = 1,
> - .cmv_select = 1,
> + .vref_voltage = VREF_VOLTAGE_1V8,
> + .cmv_select = true,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .vref_voltage = 1,
> + .vref_voltage = VREF_VOLTAGE_1V8,
> .has_vref_select = true,
> .vref_select = VREF_VDDA,
> - .cmv_select = 1,
> + .cmv_select = true,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .adc_eoc = 1,
> + .vref_voltage = VREF_VOLTAGE_0V9,
> + .adc_eoc = true,
> .has_vref_select = true,
> .vref_select = VREF_VDDA,
> };
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-03-24 14:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-23 23:13 [PATCH v1 0/3] iio: adc: meson: a few improvements Martin Blumenstingl
2024-03-23 23:13 ` [PATCH v1 1/3] iio: adc: meson: fix voltage reference selection field name typo Martin Blumenstingl
2024-03-25 23:45 ` George Stark
2024-03-31 21:30 ` Martin Blumenstingl
2024-03-23 23:13 ` [PATCH v1 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param Martin Blumenstingl
2024-03-24 14:07 ` Jonathan Cameron [this message]
2024-03-26 0:41 ` George Stark
2024-12-24 14:37 ` Martin Blumenstingl
2024-12-25 18:59 ` George Stark
2024-03-23 23:13 ` [PATCH v1 3/3] iio: adc: meson: simplify MESON_SAR_ADC_REG11 register access Martin Blumenstingl
2024-03-24 14:04 ` [PATCH v1 0/3] iio: adc: meson: a few improvements Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240324140756.5d001387@jic23-huawei \
--to=jic23@kernel.org \
--cc=gnstark@salutedevices.com \
--cc=lars@metafoo.de \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=neil.armstrong@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox