| 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
| ||
|
Message-ID: <aJQXKN_ccpWVJ5oZ@ketchup>
Date: Thu, 7 Aug 2025 03:02:00 +0000
From: Haylen Chu <heylenay@....org>
To: Troy Mitchell <troy.mitchell@...ux.spacemit.com>,
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>, Yixun Lan <dlan@...too.org>,
Alex Elder <elder@...cstar.com>,
Inochi Amaoto <inochiama@...look.com>
Cc: linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org,
Jinmei Wei <weijinmei@...ux.spacemit.com>
Subject: Re: [PATCH 2/2] clk: spacemit: introduce i2s pre-clock and fix i2s
clock
On Thu, Aug 07, 2025 at 09:30:11AM +0800, Troy Mitchell wrote:
> Defining i2s_bclk and i2s_sysclk as fixed-rate clocks is insufficient
> for real I2S use cases.
This is a little misleading: they're modeled as gates with fixed-factor
for now whose rate is calculated from their parents instead of defined
statically. You could avoid possible confusion by replacing "fixed-rate"
with "fixed-factor".
> Moreover, the current I2S clock configuration does not work as expected
> due to missing parent clocks.
>
> This patch adds the missing parent clocks, defines i2s_sysclk as
> a DDN clock, and i2s_bclk as a DIV clock.
>
> The i2s_sysclk behaves as an M/N fractional divider in hardware,
> with an additional gate control.
>
> To properly model this, CCU_DDN_GATE_DEFINE is introduced.
Could it be represented as a DDN clock taking a gate as parent? Just
like what is described in the published clock diagram. Generally I'd
like to avoid introducing more clock types, there're already a lot.
> The original DDN operations applied an implicit divide-by-2, which should
> not be a default behavior.
>
> This patch removes that assumption, letting each clock define its
> actual behavior explicitly.
>
> The i2s_bclk is a non-linear, discrete divider clock.
> The previous macro only supported linear dividers,
> so CCU_DIV_TABLE_GATE_DEFINE is introduced to support
> the hardware accurately.
The divider IS linear, from the perspective of functionality, it just
implies a 1/2 factor. Could it be represented as an usual divider and a
1/2 fixed factor?
> The I2S-related clock registers can be found here [1].
So this patch actually does four separate things,
- Introduce a gate-capable variant of DDN clocks
- Make the pre-divider of DDN clocks flexible
- Support looking up mappings between register values and divisors
through a table when calculating rates of dividers
- Fix the definition of i2s_bclk and i2s_sysclk
IMHO it's better to split them into separate patches for clearness.
> Link:
> https://developer.spacemit.com/documentation?token=LCrKwWDasiJuROkVNusc2pWTnEb
> [1]
>
> Fixes: 1b72c59db0add ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
> Co-developer: Jinmei Wei <weijinmei@...ux.spacemit.com>
> Signed-off-by: Jinmei Wei <weijinmei@...ux.spacemit.com>
> Signed-off-by: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
> ---
> drivers/clk/spacemit/ccu-k1.c | 34 ++++++++++++++++++++++++----
> drivers/clk/spacemit/ccu_common.h | 13 +++++++++++
> drivers/clk/spacemit/ccu_ddn.c | 47 ++++++++++++++++++++++++++++++++++-----
> drivers/clk/spacemit/ccu_ddn.h | 25 +++++++++++++++++++--
> drivers/clk/spacemit/ccu_mix.c | 47 +++++++++++++++++++++++++++++----------
> drivers/clk/spacemit/ccu_mix.h | 26 +++++++++++++---------
> include/soc/spacemit/k1-syscon.h | 7 +++---
> 7 files changed, 161 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
> index 65e6de030717afa60eefab7bda88f9a13b857650..a6773d4c2ff32d270e1f09b0d93cfff727ea98fa 100644
> --- a/drivers/clk/spacemit/ccu-k1.c
> +++ b/drivers/clk/spacemit/ccu-k1.c
> @@ -136,13 +136,36 @@ CCU_GATE_DEFINE(pll1_d3_819p2, CCU_PARENT_HW(pll1_d3), MPMU_ACGR, BIT(14), 0);
...
> +static const struct clk_div_table i2s_bclk_div_table[] = {
> + { .val = 0, .div = 2 },
> + { .val = 1, .div = 4 },
> + { .val = 2, .div = 6 },
> + { .val = 3, .div = 8 },
> + { /* sentinel */ },
> +};
This is just a normal 0-based divider if you divide the divisor by 2.
> +CCU_DIV_TABLE_GATE_DEFINE(i2s_bclk, CCU_PARENT_HW(i2s_sysclk), MPMU_ISCCR,
> + 27, 2, i2s_bclk_div_table, BIT(29), 0);
>
> static const struct clk_parent_data apb_parents[] = {
> CCU_PARENT_HW(pll1_d96_25p6),
> @@ -756,6 +779,9 @@ static struct clk_hw *k1_ccu_mpmu_hws[] = {
> [CLK_I2S_BCLK] = &i2s_bclk.common.hw,
> [CLK_APB] = &apb_clk.common.hw,
> [CLK_WDT_BUS] = &wdt_bus_clk.common.hw,
> + [CLK_I2S_153P6] = &i2s_153p6.common.hw,
> + [CLK_I2S_153P6_BASE] = &i2s_153p6_base.common.hw,
> + [CLK_I2S_SYSCLK_SRC] = &i2s_sysclk_src.common.hw,
> };
>
> static const struct spacemit_ccu_data k1_ccu_mpmu_data = {
...
> diff --git a/include/soc/spacemit/k1-syscon.h b/include/soc/spacemit/k1-syscon.h
> index c59bd7a38e5b4219121341b9c0d9ffda13a9c3e2..253db8a602fe43a1109e2ba248af11109c7baa22 100644
> --- a/include/soc/spacemit/k1-syscon.h
> +++ b/include/soc/spacemit/k1-syscon.h
> @@ -29,10 +29,11 @@ to_spacemit_ccu_adev(struct auxiliary_device *adev)
> #define APBS_PLL3_SWCR3 0x12c
>
> /* MPMU register offset */
> +#define MPMU_FCCR 0x0008
> #define MPMU_POSR 0x0010
> -#define POSR_PLL1_LOCK BIT(27)
> -#define POSR_PLL2_LOCK BIT(28)
> -#define POSR_PLL3_LOCK BIT(29)
> +#define POSR_PLL1_LOCK BIT(27)
> +#define POSR_PLL2_LOCK BIT(28)
> +#define POSR_PLL3_LOCK BIT(29)
This reformatting doesn't seem related to the patch.
> #define MPMU_SUCCR 0x0014
> #define MPMU_ISCCR 0x0044
> #define MPMU_WDTPCR 0x0200
>
> --
> 2.50.1
>
Best regards,
Haylen Chu
Powered by blists - more mailing lists