linux-kernel - Re: [PATCH 02/18] clk: qcom: reset: Commonize the de/assert functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dcd80eba-e00e-4e29-b46f-9c972bf018f3@linaro.org>
Date: Tue, 9 Jan 2024 00:29:52 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Konrad Dybcio <konrad.dybcio@...aro.org>,
 Bjorn Andersson <andersson@...nel.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>
Cc: Marijn Suijten <marijn.suijten@...ainline.org>,
 linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org, Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Dikshita Agarwal <quic_dikshita@...cinc.com>,
 Vikash Garodia <quic_vgarodia@...cinc.com>,
 Manivannan Sadhasivam <mani@...nel.org>
Subject: Re: [PATCH 02/18] clk: qcom: reset: Commonize the de/assert functions

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@...aro.org>

---
bod