[<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