| 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: <CAJFTR8TfctXn=+o0SbjUgC0xHu6w7JPrAR7rERrL1u1EvLgKyw@mail.gmail.com>
Date: Thu, 17 Jul 2025 23:16:43 -0400
From: Jesse T <mr.bossman075@...il.com>
To: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
Yixun Lan <dlan@...too.org>, linux-clk@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: spacemit: fix sspax_clk
On Thu, Jul 17, 2025 at 10:10 PM Troy Mitchell
<troy.mitchell@...ux.spacemit.com> wrote:
>
> In the SpacemiT public document, when the FNCLKSEL field of
> the APBC_SSPAX_CLK_RST register is 7 (3'b111),
> which is a reserved value. And BIT3 of the same register is
> a reserved bit.
>
> But the document is wrong, the actual situation is:
> when FNCLKSEL is 7 (3'b111), and the purpose of bit 3 is
> if to select i2s_bclk as the parent clock.
>
> And only when FNCLKSEL is 7 (3'b111), The bit 3 is not a reserved bit.
>
> Signed-off-by: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
> ---
> drivers/clk/spacemit/ccu-k1.c | 4 ++--
> drivers/clk/spacemit/ccu_mix.c | 29 +++++++++++++++++++++++++++++
> drivers/clk/spacemit/ccu_mix.h | 14 ++++++++++++++
> 3 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
> index cdde37a0523537c2f436e481ae8d6ec5a581b87e..0e22f6fb2c45b68ab20a9b1563a1a6dec1a7e16c 100644
> --- a/drivers/clk/spacemit/ccu-k1.c
> +++ b/drivers/clk/spacemit/ccu-k1.c
> @@ -359,8 +359,8 @@ static const struct clk_parent_data sspa_parents[] = {
> CCU_PARENT_HW(pll1_d3072_0p8),
> CCU_PARENT_HW(i2s_bclk),
> };
> -CCU_MUX_GATE_DEFINE(sspa0_clk, sspa_parents, APBC_SSPA0_CLK_RST, 4, 3, BIT(1), 0);
> -CCU_MUX_GATE_DEFINE(sspa1_clk, sspa_parents, APBC_SSPA1_CLK_RST, 4, 3, BIT(1), 0);
> +CCU_SSPA_MUX_GATE_DEFINE(sspa0_clk, sspa_parents, APBC_SSPA0_CLK_RST, 4, 3, BIT(1), 0);
> +CCU_SSPA_MUX_GATE_DEFINE(sspa1_clk, sspa_parents, APBC_SSPA1_CLK_RST, 4, 3, BIT(1), 0);
> CCU_GATE_DEFINE(dro_clk, CCU_PARENT_HW(apb_clk), APBC_DRO_CLK_RST, BIT(1), 0);
> CCU_GATE_DEFINE(ir_clk, CCU_PARENT_HW(apb_clk), APBC_IR_CLK_RST, BIT(1), 0);
> CCU_GATE_DEFINE(tsen_clk, CCU_PARENT_HW(apb_clk), APBC_TSEN_CLK_RST, BIT(1), 0);
> diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
> index 9b852aa61f78aed5256bfe6fc3b01932d6db6256..bfc65fc00df67299523eb5d1d2ed479c61fc6141 100644
> --- a/drivers/clk/spacemit/ccu_mix.c
> +++ b/drivers/clk/spacemit/ccu_mix.c
> @@ -191,6 +191,25 @@ static int ccu_mux_set_parent(struct clk_hw *hw, u8 index)
> return ccu_mix_trigger_fc(hw);
> }
>
> +static int ccu_mux_set_sspa_parent(struct clk_hw *hw, u8 index)
> +{
> + struct ccu_mix *mix = hw_to_ccu_mix(hw);
> + struct ccu_mux_config *mux = &mix->mux;
> + u32 mask, val;
> +
> + mask = GENMASK(mux->width + mux->shift - 1, mux->shift);
> + val = index << mux->shift;
> +
> + if (index == 7) {
> + mask |= BIT(3);
> + val |= BIT(3);
Can the value 3 here be a macro like FNCLKSEL_SSPX_I2S_BCLK_PARENT.
A comment would be nice too.
> + }
> +
> + ccu_update(&mix->common, ctrl, mask, val);
> +
> + return ccu_mix_trigger_fc(hw);
> +}
> +
> const struct clk_ops spacemit_ccu_gate_ops = {
> .disable = ccu_gate_disable,
> .enable = ccu_gate_enable,
> @@ -235,6 +254,16 @@ const struct clk_ops spacemit_ccu_mux_gate_ops = {
> .set_parent = ccu_mux_set_parent,
> };
>
> +const struct clk_ops spacemit_ccu_sspa_mux_gate_ops = {
> + .disable = ccu_gate_disable,
> + .enable = ccu_gate_enable,
> + .is_enabled = ccu_gate_is_enabled,
> +
> + .determine_rate = ccu_mix_determine_rate,
> + .get_parent = ccu_mux_get_parent,
> + .set_parent = ccu_mux_set_sspa_parent,
> +};
> +
> const struct clk_ops spacemit_ccu_div_gate_ops = {
> .disable = ccu_gate_disable,
> .enable = ccu_gate_enable,
> diff --git a/drivers/clk/spacemit/ccu_mix.h b/drivers/clk/spacemit/ccu_mix.h
> index 51d19f5d6aacb7203d1eddf96047cf3174533601..7753446386353bf849787ed4ec7c85c298238ab5 100644
> --- a/drivers/clk/spacemit/ccu_mix.h
> +++ b/drivers/clk/spacemit/ccu_mix.h
> @@ -124,6 +124,19 @@ static struct ccu_mix _name = { \
> } \
> }
>
> +#define CCU_SSPA_MUX_GATE_DEFINE(_name, _parents, _reg_ctrl, \
I think it would be better to add a flags bit to struct ccu_mux_config
Then add CCU_MUX_GATE_DEFINE_FLAGS, something like how
drivers/clk/nxp/clk-lpc18xx-ccu.c has CCU_BRANCH_HAVE_DIV2 and CCU_BRANCH_IS_BUS
This would allow for multiple different variants of the same mux to be
added without having a bunch of
peripheral specific functions.
> + _shift, _width, _mask_gate, _flags) \
> +static struct ccu_mix _name = { \
> + .gate = CCU_GATE_INIT(_mask_gate), \
> + .mux = CCU_MUX_INIT(_shift, _width), \
> + .common = { \
> + .reg_ctrl = _reg_ctrl, \
> + CCU_MIX_INITHW_PARENTS(_name, _parents, \
> + spacemit_ccu_sspa_mux_gate_ops, \
> + _flags), \
> + } \
> +}
> +
> #define CCU_DIV_GATE_DEFINE(_name, _parent, _reg_ctrl, _shift, _width, \
> _mask_gate, _flags) \
> static struct ccu_mix _name = { \
> @@ -213,6 +226,7 @@ extern const struct clk_ops spacemit_ccu_div_ops;
> extern const struct clk_ops spacemit_ccu_factor_gate_ops;
> extern const struct clk_ops spacemit_ccu_div_gate_ops;
> extern const struct clk_ops spacemit_ccu_mux_gate_ops;
> +extern const struct clk_ops spacemit_ccu_sspa_mux_gate_ops;
> extern const struct clk_ops spacemit_ccu_mux_div_ops;
> extern const struct clk_ops spacemit_ccu_mux_div_gate_ops;
> #endif /* _CCU_DIV_H_ */
>
> ---
> base-commit: 733923397fd95405a48f165c9b1fbc8c4b0a4681
> change-id: 20250717-k1-clk-i2s-e4272f1f915b
>
> Best regards,
> --
> Troy Mitchell <troy.mitchell@...ux.spacemit.com>
Congrats on working at spacemit!!
Thanks,
Jesse Taube
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Powered by blists - more mailing lists