linux-kernel - Re: [PATCH v8 2/3] clk: canaan: Add clock driver for Canaan K230

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] [day] [month] [year] [list]

Message-ID: <9a0eedb5-1d90-4bdb-9bc3-4b3ade29cc2f@zohomail.com>
Date: Wed, 10 Sep 2025 16:38:11 +0800
From: Xukai Wang <kingxukai@...omail.com>
To: Vivian Wang <wangruikang@...as.ac.cn>, Yao Zi <ziyao@...root.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Conor Dooley <conor@...nel.org>
Cc: linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 Samuel Holland <samuel.holland@...ive.com>,
 Troy Mitchell <TroyMitchell988@...il.com>
Subject: Re: [PATCH v8 2/3] clk: canaan: Add clock driver for Canaan K230


On 2025/9/9 15:02, Vivian Wang wrote:
> On 9/8/25 22:13, Xukai Wang wrote:
>>>> [...]
>>>>
>>>> +
>>>> +static int k230_clk_set_rate_mul_div(struct clk_hw *hw, unsigned long rate,
>>>> +				     unsigned long parent_rate)
>>>> +{
>>>> +	struct k230_clk_rate *clk = hw_to_k230_clk_rate(hw);
>>>> +	struct k230_clk_rate_self *rate_self = &clk->clk;
>>>> +	u32 div, mul, div_reg, mul_reg;
>>>> +
>>>> +	if (rate > parent_rate)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (rate_self->read_only)
>>>> +		return 0;
>>>> +
>>>> +	if (k230_clk_find_approximate_mul_div(rate_self->mul_min, rate_self->mul_max,
>>>> +					      rate_self->div_min, rate_self->div_max,
>>>> +					      rate, parent_rate, &div, &mul))
>>>> +		return -EINVAL;
>>>> +
>>>> +	guard(spinlock)(rate_self->lock);
>>>> +
>>>> +	div_reg = readl(rate_self->reg + clk->div_reg_off);
>>>> +	div_reg |= ((div - 1) & rate_self->div_mask) << (rate_self->div_shift);
>>>> +	div_reg |= BIT(rate_self->write_enable_bit);
>>>> +	writel(div_reg, rate_self->reg + clk->div_reg_off);
>>>> +
>>>> +	mul_reg = readl(rate_self->reg + clk->mul_reg_off);
>>>> +	mul_reg |= ((mul - 1) & rate_self->mul_mask) << (rate_self->mul_shift);
>>>> +	mul_reg |= BIT(rate_self->write_enable_bit);
>>>> +	writel(mul_reg, rate_self->reg + clk->mul_reg_off);
>>>> +
>>>> +	return 0;
>>>> +}
>>> There are three variants of rate clocks, mul-only, div-only and mul-div
>>> ones, which are similar to clk-multiplier, clk-divider,
>>> clk-fractional-divider.
>>>
>>> The only difference is to setup new parameters for K230's rate clocks,
>>> a register bit, described as k230_clk_rate_self.write_enable_bit, must
>>> be set first.
>> Actually, I think the differences are not limited to just the
>> write_enable_bit. There are also distinct mul_min, mul_max, div_min, and
>> div_max values, which are not typically just 1 and (1 << bit_width) as
>> in standard clock divider or multiplier structures.
> So the part I have been thinking about is, consider just checking the
> {mul,div}_{min,max} values to determine which kind it is? As is this is
> just redundant information, since you can infer whether there is a
> configurable multiplier by checking if mul_{min,max} are equal. Same for
> div_{min,max}.
>
> Vivian "dramforever" Wang
Thanks for pointing it out. I see your idea, but I don’t think it’s
necessary to determine the clock type from {mul,div}_{min,max}
dynamically since we already statically specify each mul, div, and
mul-div clock by different macros.