Received-SPF: pass (google.com: domain of linux-kernel+bounces-212943-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <3b7950db-bde8-407e-b568-51cacee93ad8@linux.alibaba.com>
Date: Thu, 13 Jun 2024 17:24:14 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RESEND][PATCH] iio: simplify with regmap_set_bits(),
 regmap_clear_bits()
To: Trevor Gamblin <tgamblin@baylibre.com>,
 Lucas Stankus <lucas.p.stankus@gmail.com>,
 Lars-Peter Clausen <lars@metafoo.de>,
 Michael Hennerich <Michael.Hennerich@analog.com>,
 Jonathan Cameron <jic23@kernel.org>,
 Dmitry Rokosov <ddrokosov@sberdevices.ru>,
 Cosmin Tanislav <cosmin.tanislav@analog.com>, Chen-Yu Tsai <wens@csie.org>,
 Hans de Goede <hdegoede@redhat.com>, Ray Jui <rjui@broadcom.com>,
 Scott Branden <sbranden@broadcom.com>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@broadcom.com>, Shawn Guo <shawnguo@kernel.org>,
 Sascha Hauer <s.hauer@pengutronix.de>,
 Pengutronix Kernel Team <kernel@pengutronix.de>,
 Fabio Estevam <festevam@gmail.com>,
 Neil Armstrong <neil.armstrong@linaro.org>,
 Kevin Hilman <khilman@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>,
 Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
 Saravanan Sekar <sravanhome@gmail.com>, Orson Zhai <orsonzhai@gmail.com>,
 Chunyan Zhang <zhang.lyra@gmail.com>,
 Maxime Coquelin <mcoquelin.stm32@gmail.com>,
 Alexandre Torgue <alexandre.torgue@foss.st.com>, =?UTF-8?Q?Nuno_S=C3=A1?=
 <nuno.sa@analog.com>, Linus Walleij <linus.walleij@linaro.org>,
 Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>, Crt Mori <cmo@melexis.com>
Cc: =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= <u.kleine-koenig@baylibre.com>,
 linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev,
 linux-amlogic@lists.infradead.org, linux-arm-msm@vger.kernel.org,
 linux-stm32@st-md-mailman.stormreply.com
References: <20240611165214.4091591-1-tgamblin@baylibre.com>
From: Baolin Wang <baolin.wang@linux.alibaba.com>
In-Reply-To: <20240611165214.4091591-1-tgamblin@baylibre.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 2024/6/12 00:52, Trevor Gamblin wrote:
> Simplify the way regmap is accessed in iio drivers.
> 
> Instead of using regmap_update_bits() and passing the mask twice, use
> regmap_set_bits().
> 
> Instead of using regmap_update_bits() and passing val = 0, use
> regmap_clear_bits().
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
> ---

[...]

> diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> index b4a2e057d80f..2535c2c3e60b 100644
> --- a/drivers/iio/adc/sc27xx_adc.c
> +++ b/drivers/iio/adc/sc27xx_adc.c
> @@ -508,13 +508,13 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>   		}
>   	}
>   
> -	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> -				 SC27XX_ADC_EN, SC27XX_ADC_EN);
> +	ret = regmap_set_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> +			      SC27XX_ADC_EN);
>   	if (ret)
>   		goto regulator_restore;
>   
> -	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
> -				 SC27XX_ADC_IRQ_CLR, SC27XX_ADC_IRQ_CLR);
> +	ret = regmap_set_bits(data->regmap, data->base + SC27XX_ADC_INT_CLR,
> +			      SC27XX_ADC_IRQ_CLR);
>   	if (ret)
>   		goto disable_adc;
>   
> @@ -537,8 +537,8 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>   	if (ret)
>   		goto disable_adc;
>   
> -	ret = regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> -				 SC27XX_ADC_CHN_RUN, SC27XX_ADC_CHN_RUN);
> +	ret = regmap_set_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> +			      SC27XX_ADC_CHN_RUN);
>   	if (ret)
>   		goto disable_adc;
>   
> @@ -559,8 +559,8 @@ static int sc27xx_adc_read(struct sc27xx_adc_data *data, int channel,
>   	value &= SC27XX_ADC_DATA_MASK;
>   
>   disable_adc:
> -	regmap_update_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> -			   SC27XX_ADC_EN, 0);
> +	regmap_clear_bits(data->regmap, data->base + SC27XX_ADC_CTL,
> +			  SC27XX_ADC_EN);
>   regulator_restore:
>   	if ((data->var_data->set_volref) && (channel == 30 || channel == 31)) {
>   		ret_volref = regulator_set_voltage(data->volref,
> @@ -765,15 +765,14 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>   {
>   	int ret;
>   
> -	ret = regmap_update_bits(data->regmap, data->var_data->module_en,
> -				 SC27XX_MODULE_ADC_EN, SC27XX_MODULE_ADC_EN);
> +	ret = regmap_set_bits(data->regmap, data->var_data->module_en,
> +			      SC27XX_MODULE_ADC_EN);
>   	if (ret)
>   		return ret;
>   
>   	/* Enable ADC work clock and controller clock */
> -	ret = regmap_update_bits(data->regmap, data->var_data->clk_en,
> -				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN,
> -				 SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
> +	ret = regmap_set_bits(data->regmap, data->var_data->clk_en,
> +			      SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
>   	if (ret)
>   		goto disable_adc;
>   
> @@ -789,11 +788,11 @@ static int sc27xx_adc_enable(struct sc27xx_adc_data *data)
>   	return 0;
>   
>   disable_clk:
> -	regmap_update_bits(data->regmap, data->var_data->clk_en,
> -			   SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN, 0);
> +	regmap_clear_bits(data->regmap, data->var_data->clk_en,
> +			  SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
>   disable_adc:
> -	regmap_update_bits(data->regmap, data->var_data->module_en,
> -			   SC27XX_MODULE_ADC_EN, 0);
> +	regmap_clear_bits(data->regmap, data->var_data->module_en,
> +			  SC27XX_MODULE_ADC_EN);
>   
>   	return ret;
>   }
> @@ -803,11 +802,11 @@ static void sc27xx_adc_disable(void *_data)
>   	struct sc27xx_adc_data *data = _data;
>   
>   	/* Disable ADC work clock and controller clock */
> -	regmap_update_bits(data->regmap, data->var_data->clk_en,
> -			   SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN, 0);
> +	regmap_clear_bits(data->regmap, data->var_data->clk_en,
> +			  SC27XX_CLK_ADC_EN | SC27XX_CLK_ADC_CLK_EN);
>   
> -	regmap_update_bits(data->regmap, data->var_data->module_en,
> -			   SC27XX_MODULE_ADC_EN, 0);
> +	regmap_clear_bits(data->regmap, data->var_data->module_en,
> +			  SC27XX_MODULE_ADC_EN);
>   }
>   
>   static const struct sc27xx_adc_variant_data sc2731_data = {

For SPRD sc27xx parts, looks good to me. Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>