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: <baae0cce-979d-4e4d-aacc-73387ed7d8a8@zohomail.com>
Date: Fri, 18 Apr 2025 22:19:43 +0800
From: Xukai Wang <kingxukai@...omail.com>
To: Troy Mitchell <troymitchell988@...il.com>
Cc: 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>, 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>
Subject: Re: [PATCH v6 2/3] clk: canaan: Add clock driver for Canaan K230


On 2025/4/18 20:31, Troy Mitchell wrote:
> On Tue, Apr 15, 2025 at 10:25:12PM +0800, Xukai Wang wrote:
>> +	switch (rate_cfg->method) {
>> +	/*
>> +	 * K230_MUL: div_mask+1/div_max...
>> +	 * K230_DIV: mul_max/div_mask+1
>> +	 * K230_MUL_DIV: mul_mask/div_mask...
>> +	 */
>> +	case K230_MUL:
>> +		div = rate_cfg->rate_div_max;
>> +		mul = (readl(rate_cfg->rate_reg) >> rate_cfg->rate_div_shift)
>> +			& rate_cfg->rate_div_mask;
>> +		mul++;
>> +		break;
>> +	case K230_DIV:
>> +		mul = rate_cfg->rate_mul_max;
>> +		div = (readl(rate_cfg->rate_reg) >> rate_cfg->rate_div_shift)
>> +			& rate_cfg->rate_div_mask;
>> +		div++;
>> +		break;
>> +	case K230_MUL_DIV:
>> +		if (!rate_cfg_c) {
>> +			mul = (readl(rate_cfg->rate_reg) >> rate_cfg->rate_mul_shift)
>> +				& rate_cfg->rate_mul_mask;
>> +			div = (readl(rate_cfg->rate_reg) >> rate_cfg->rate_div_shift)
>> +				& rate_cfg->rate_div_mask;
>> +		} else {
>> +			mul = (readl(rate_cfg_c->rate_reg_c) >> rate_cfg_c->rate_mul_shift_c)
>> +				& rate_cfg_c->rate_mul_mask_c;
>> +			div = (readl(rate_cfg->rate_reg) >> rate_cfg->rate_div_shift)
>> +				& rate_cfg->rate_div_mask;
>> +		}
>> +		break;
> Should we report an error in other cases?

This is impossible. The compiler will warn if an enum case is missing.[1]

>> +	}
>> +
>> +	return mul_u64_u32_div(parent_rate, mul, div);
>> +}
>> +
>> +static int k230_clk_find_approximate(struct k230_clk *clk,
>> +				     u32 mul_min,
>> +				     u32 mul_max,
>> +				     u32 div_min,
>> +				     u32 div_max,
>> +				     enum k230_clk_div_type method,
>> +				     unsigned long rate,
>> +				     unsigned long parent_rate,
>> +				     u32 *div,
>> +				     u32 *mul)
>> +{
>> +	long abs_min;
>> +	long abs_current;
>> +	long perfect_divide;
>> +	struct k230_clk_cfg *cfg = k230_clk_cfgs[clk->id];
>> +	struct k230_clk_rate_cfg *rate_cfg = cfg->rate_cfg;
>> +
>> +	const u32 codec_clk[9] = {
>> +		2048000,
>> +		3072000,
>> +		4096000,
>> +		6144000,
>> +		8192000,
>> +		11289600,
>> +		12288000,
>> +		24576000,
>> +		49152000
>> +	};
>> +
>> +	const u32 codec_div[9][2] = {
>> +		{3125, 16},
>> +		{3125, 24},
>> +		{3125, 32},
>> +		{3125, 48},
>> +		{3125, 64},
>> +		{15625, 441},
>> +		{3125, 96},
>> +		{3125, 192},
>> +		{3125, 384}
>> +	};
>> +
>> +	const u32 pdm_clk[20] = {
>> +		128000,
>> +		192000,
>> +		256000,
>> +		384000,
>> +		512000,
>> +		768000,
>> +		1024000,
>> +		1411200,
>> +		1536000,
>> +		2048000,
>> +		2822400,
>> +		3072000,
>> +		4096000,
>> +		5644800,
>> +		6144000,
>> +		8192000,
>> +		11289600,
>> +		12288000,
>> +		24576000,
>> +		49152000
>> +	};
>> +
>> +	const u32 pdm_div[20][2] = {
>> +		{3125, 1},
>> +		{6250, 3},
>> +		{3125, 2},
>> +		{3125, 3},
>> +		{3125, 4},
>> +		{3125, 6},
>> +		{3125, 8},
>> +		{125000, 441},
>> +		{3125, 12},
>> +		{3125, 16},
>> +		{62500, 441},
>> +		{3125, 24},
>> +		{3125, 32},
>> +		{31250, 441},
>> +		{3125, 48},
>> +		{3125, 64},
>> +		{15625, 441},
>> +		{3125, 96},
>> +		{3125, 192},
>> +		{3125, 384}
>> +	};
>> +
>> +	switch (method) {
>> +	/* only mul can be changeable 1/12,2/12,3/12...*/
>> +	case K230_MUL:
>> +		perfect_divide = (long)((parent_rate * 1000) / rate);
>> +		abs_min = abs(perfect_divide -
>> +			     (long)(((long)div_max * 1000) / (long)mul_min));
>> +		*mul = mul_min;
>> +
>> +		for (u32 i = mul_min + 1; i <= mul_max; i++) {
>> +			abs_current = abs(perfect_divide -
>> +					(long)((long)((long)div_max * 1000) / (long)i));
>> +			if (abs_min > abs_current) {
>> +				abs_min = abs_current;
>> +				*mul = i;
>> +			}
>> +		}
>> +
>> +		*div = div_max;
>> +		break;
>> +	/* only div can be changeable, 1/1,1/2,1/3...*/
>> +	case K230_DIV:
>> +		perfect_divide = (long)((parent_rate * 1000) / rate);
>> +		abs_min = abs(perfect_divide -
>> +			     (long)(((long)div_min * 1000) / (long)mul_max));
>> +		*div = div_min;
>> +
>> +		for (u32 i = div_min + 1; i <= div_max; i++) {
>> +			abs_current = abs(perfect_divide -
>> +					 (long)((long)((long)i * 1000) / (long)mul_max));
>> +			if (abs_min > abs_current) {
>> +				abs_min = abs_current;
>> +				*div = i;
>> +			}
>> +		}
>> +
>> +		*mul = mul_max;
>> +		break;
>> +	/* mul and div can be changeable. */
>> +	case K230_MUL_DIV:
>> +		if (rate_cfg->rate_reg_off == K230_CLK_CODEC_ADC_MCLKDIV_OFFSET ||
>> +		    rate_cfg->rate_reg_off == K230_CLK_CODEC_DAC_MCLKDIV_OFFSET) {
>> +			for (u32 j = 0; j < 9; j++) {
>> +				if (0 == (rate - codec_clk[j])) {
>> +					*div = codec_div[j][0];
>> +					*mul = codec_div[j][1];
>> +				}
>> +			}
>> +		} else if (rate_cfg->rate_reg_off == K230_CLK_AUDIO_CLKDIV_OFFSET ||
>> +			   rate_cfg->rate_reg_off == K230_CLK_PDM_CLKDIV_OFFSET) {
>> +			for (u32 j = 0; j < 20; j++) {
>> +				if (0 == (rate - pdm_clk[j])) {
>>
>> +					*div = pdm_div[j][0];
>> +					*mul = pdm_div[j][1];
>> +				}
>> +			}
>> +		} else {
>> +			return -EINVAL;
>> +		}
>> +		break;
> Should we report an error when other case?

The default case is impossible.
The compiler will warn if an enum case is missing.
And Stephen Boyd suggested to remove the default case[1].
Link: https://lore.kernel.org/all/3fb73691f50e599c361dddaff08d3af5.sboyd@kernel.org/ [1]


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ