Received-SPF: pass (google.com: domain of linux-kernel+bounces-20215-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <dcd80eba-e00e-4e29-b46f-9c972bf018f3@linaro.org>
Date: Tue, 9 Jan 2024 00:29:52 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 02/18] clk: qcom: reset: Commonize the de/assert functions
Content-Language: en-US
To: Konrad Dybcio <konrad.dybcio@linaro.org>,
 Bjorn Andersson <andersson@kernel.org>,
 Michael Turquette <mturquette@baylibre.com>, Stephen Boyd
 <sboyd@kernel.org>, Philipp Zabel <p.zabel@pengutronix.de>
Cc: Marijn Suijten <marijn.suijten@somainline.org>,
 linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
 linux-kernel@vger.kernel.org, Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
 Dikshita Agarwal <quic_dikshita@quicinc.com>,
 Vikash Garodia <quic_vgarodia@quicinc.com>,
 Manivannan Sadhasivam <mani@kernel.org>
References: <20240105-topic-venus_reset-v1-0-981c7a624855@linaro.org>
 <20240105-topic-venus_reset-v1-2-981c7a624855@linaro.org>
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
In-Reply-To: <20240105-topic-venus_reset-v1-2-981c7a624855@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 08/01/2024 12:32, Konrad Dybcio wrote:
> -static int
> -qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +static int qcom_reset_set_assert(struct reset_controller_dev *rcdev, unsigned long id,
> +			     bool assert)

Personally I'd not elongate the function declaration.

>   {
>   	struct qcom_reset_controller *rst;
>   	const struct qcom_reset_map *map;
> @@ -33,21 +33,17 @@ qcom_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
>   	map = &rst->reset_map[id];
>   	mask = map->bitmask ? map->bitmask : BIT(map->bit);
>   
> -	return regmap_update_bits(rst->regmap, map->reg, mask, mask);
> +	return regmap_update_bits(rst->regmap, map->reg, mask, assert ? mask : 0);

and I'd probably do

u32 bits = 0;

if (assert)
     bits = mask;

regmap_update_bits(rst->regmap, map->reg, mask, bits);

because I prefer for aesthetic reasons not to do ternary inputs like that.

However its up to you to change or not.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod