* [PATCH] clk: spacemit: fix sspax_clk
@ 2025-07-18 1:54 Troy Mitchell
2025-07-18 3:16 ` Jesse T
2025-07-18 10:51 ` Yixun Lan
0 siblings, 2 replies; 6+ messages in thread
From: Troy Mitchell @ 2025-07-18 1:54 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Yixun Lan
Cc: linux-clk, linux-riscv, spacemit, linux-kernel, Troy Mitchell
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@linux.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);
+ }
+
+ 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, \
+ _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@linux.spacemit.com>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: spacemit: fix sspax_clk
2025-07-18 1:54 [PATCH] clk: spacemit: fix sspax_clk Troy Mitchell
@ 2025-07-18 3:16 ` Jesse T
2025-07-18 10:51 ` Yixun Lan
1 sibling, 0 replies; 6+ messages in thread
From: Jesse T @ 2025-07-18 3:16 UTC (permalink / raw)
To: Troy Mitchell
Cc: Michael Turquette, Stephen Boyd, Yixun Lan, linux-clk,
linux-riscv, spacemit, linux-kernel
On Thu, Jul 17, 2025 at 10:10 PM Troy Mitchell
<troy.mitchell@linux.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@linux.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@linux.spacemit.com>
Congrats on working at spacemit!!
Thanks,
Jesse Taube
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: spacemit: fix sspax_clk
2025-07-18 1:54 [PATCH] clk: spacemit: fix sspax_clk Troy Mitchell
2025-07-18 3:16 ` Jesse T
@ 2025-07-18 10:51 ` Yixun Lan
2025-07-18 13:27 ` Troy Mitchell
1 sibling, 1 reply; 6+ messages in thread
From: Yixun Lan @ 2025-07-18 10:51 UTC (permalink / raw)
To: Troy Mitchell
Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-riscv, spacemit,
linux-kernel
Hi Troy,
On 09:54 Fri 18 Jul , Troy Mitchell 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.
>
so, if I parse above correctly, it describe current public document?
which value 7 of FNCLKSEL and BIT3 is wrongly marked as reserved.
https://developer.spacemit.com/documentation?token=LCrKwWDasiJuROkVNusc2pWTnEb#10.2.3.2-resource-reset-schemes
10.2.4.3.16 SSPAX CLOCK RESET CONTROL REGISTER (APBC_SSPAX_CLK_RST)
> 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.
>
so what's the difference of value 7 from other 0-6? has additional ops to
select i2s_bclk as parent? otherwise just say they are not reserved
please have more explanation for BIT3, it's quite obscure and unclear
anyway, can you coordinate with SpacemiT internal to update the docs?
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.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);
> + }
it occur to me, BIT(3) is kind of a conditional BIT..
what's the behavior of reading/writing to BIT3 when index != 7?
write value will be ignored, read will return zero?
> +
> + 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, \
> + _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@linux.spacemit.com>
>
>
--
Yixun Lan (dlan)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: spacemit: fix sspax_clk
2025-07-18 10:51 ` Yixun Lan
@ 2025-07-18 13:27 ` Troy Mitchell
2025-07-18 13:49 ` Troy Mitchell
0 siblings, 1 reply; 6+ messages in thread
From: Troy Mitchell @ 2025-07-18 13:27 UTC (permalink / raw)
To: Yixun Lan, Troy Mitchell
Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-riscv, spacemit,
linux-kernel
On Fri, Jul 18, 2025 at 06:51:20PM +0800, Yixun Lan wrote:
> Hi Troy,
>
> On 09:54 Fri 18 Jul , Troy Mitchell 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.
> >
> so, if I parse above correctly, it describe current public document?
> which value 7 of FNCLKSEL and BIT3 is wrongly marked as reserved.
>
> https://developer.spacemit.com/documentation?token=LCrKwWDasiJuROkVNusc2pWTnEb#10.2.3.2-resource-reset-schemes
> 10.2.4.3.16 SSPAX CLOCK RESET CONTROL REGISTER (APBC_SSPAX_CLK_RST)
>
yes, correct summary
> > 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.
> >
> so what's the difference of value 7 from other 0-6? has additional ops to
> select i2s_bclk as parent? otherwise just say they are not reserved
>
> please have more explanation for BIT3, it's quite obscure and unclear
Bit 3 is only valid when FNCLKSEL is 7,
indicating whether i2s_bclk is selected as the clock source.
And when FNCLKSEL is 7, bit 3 must be 1, otherwise it will cause unknown errors.
When FNCLKSEL is other values, bit 3 has no effect
I'll explain more in the next version
>
> anyway, can you coordinate with SpacemiT internal to update the docs?
>
I have already told them. I think the document will be updated ASAP.
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.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);
> > + }
> it occur to me, BIT(3) is kind of a conditional BIT..
>
> what's the behavior of reading/writing to BIT3 when index != 7?
> write value will be ignored, read will return zero?
I don't know. I haven't tested it, and I don't think it's worth testing.
I've written a function to set the parent using BIT(3) and an index.
Therefore, when the caller invokes get_parent, BIT(3) is not considered; only the index is.
- Troy
>
> > +
> > + 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, \
> > + _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@linux.spacemit.com>
> >
> >
>
> --
> Yixun Lan (dlan)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: spacemit: fix sspax_clk
2025-07-18 13:27 ` Troy Mitchell
@ 2025-07-18 13:49 ` Troy Mitchell
2025-07-19 5:51 ` Yao Zi
0 siblings, 1 reply; 6+ messages in thread
From: Troy Mitchell @ 2025-07-18 13:49 UTC (permalink / raw)
To: Yixun Lan, Troy Mitchell
Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-riscv, spacemit,
linux-kernel
On Fri, Jul 18, 2025 at 09:27:06PM +0800, Troy Mitchell wrote:
> On Fri, Jul 18, 2025 at 06:51:20PM +0800, Yixun Lan wrote:
> > Hi Troy,
> >
> > On 09:54 Fri 18 Jul , Troy Mitchell 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.
> > >
> > so, if I parse above correctly, it describe current public document?
> > which value 7 of FNCLKSEL and BIT3 is wrongly marked as reserved.
> >
> > https://developer.spacemit.com/documentation?token=LCrKwWDasiJuROkVNusc2pWTnEb#10.2.3.2-resource-reset-schemes
> > 10.2.4.3.16 SSPAX CLOCK RESET CONTROL REGISTER (APBC_SSPAX_CLK_RST)
> >
> yes, correct summary
>
> > > 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.
> > >
> > so what's the difference of value 7 from other 0-6? has additional ops to
> > select i2s_bclk as parent? otherwise just say they are not reserved
> >
> > please have more explanation for BIT3, it's quite obscure and unclear
> Bit 3 is only valid when FNCLKSEL is 7,
> indicating whether i2s_bclk is selected as the clock source.
> And when FNCLKSEL is 7, bit 3 must be 1, otherwise it will cause unknown errors.
> When FNCLKSEL is other values, bit 3 has no effect
>
> I'll explain more in the next version
>
> >
> > anyway, can you coordinate with SpacemiT internal to update the docs?
> >
> I have already told them. I think the document will be updated ASAP.
>
> > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.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);
> > > + }
> > it occur to me, BIT(3) is kind of a conditional BIT..
> >
> > what's the behavior of reading/writing to BIT3 when index != 7?
> > write value will be ignored, read will return zero?
> I don't know. I haven't tested it, and I don't think it's worth testing.
> I've written a function to set the parent using BIT(3) and an index.
> Therefore, when the caller invokes get_parent, BIT(3) is not considered; only the index is.
>
Sry, Yixun, We really need to test it!
If BIT(3) has no effct when FNCLKSEL is set to other values,
then we can avoid introducing a new function.
>
> - Troy
>
> >
> > > +
> > > + 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, \
> > > + _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@linux.spacemit.com>
> > >
> > >
> >
> > --
> > Yixun Lan (dlan)
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clk: spacemit: fix sspax_clk
2025-07-18 13:49 ` Troy Mitchell
@ 2025-07-19 5:51 ` Yao Zi
0 siblings, 0 replies; 6+ messages in thread
From: Yao Zi @ 2025-07-19 5:51 UTC (permalink / raw)
To: Troy Mitchell, Yixun Lan
Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-riscv, spacemit,
linux-kernel
On Fri, Jul 18, 2025 at 09:49:40PM +0800, Troy Mitchell wrote:
> On Fri, Jul 18, 2025 at 09:27:06PM +0800, Troy Mitchell wrote:
> > On Fri, Jul 18, 2025 at 06:51:20PM +0800, Yixun Lan wrote:
> > > Hi Troy,
> > >
> > > On 09:54 Fri 18 Jul , Troy Mitchell 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.
> > > >
> > > so, if I parse above correctly, it describe current public document?
> > > which value 7 of FNCLKSEL and BIT3 is wrongly marked as reserved.
> > >
> > > https://developer.spacemit.com/documentation?token=LCrKwWDasiJuROkVNusc2pWTnEb#10.2.3.2-resource-reset-schemes
> > > 10.2.4.3.16 SSPAX CLOCK RESET CONTROL REGISTER (APBC_SSPAX_CLK_RST)
> > >
> > yes, correct summary
> >
> > > > 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.
> > > >
> > > so what's the difference of value 7 from other 0-6? has additional ops to
> > > select i2s_bclk as parent? otherwise just say they are not reserved
> > >
> > > please have more explanation for BIT3, it's quite obscure and unclear
> > Bit 3 is only valid when FNCLKSEL is 7,
> > indicating whether i2s_bclk is selected as the clock source.
> > And when FNCLKSEL is 7, bit 3 must be 1, otherwise it will cause unknown errors.
> > When FNCLKSEL is other values, bit 3 has no effect
Suggest introducing a dummy gate clock utilizing BIT(3) as the enable
bit. Instead directly taking i2s_bclk as parent, you could change the
eighth parent of sspaX_clk to the new dummy gate, which ensures BIT(3)
is set when the FNCLKSEL == 7 and requires no specific handling in code.
Regards,
Yao Zi
> > I'll explain more in the next version
> >
> > >
> > > anyway, can you coordinate with SpacemiT internal to update the docs?
> > >
> > I have already told them. I think the document will be updated ASAP.
> >
> > > > Signed-off-by: Troy Mitchell <troy.mitchell@linux.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);
> > > > + }
> > > it occur to me, BIT(3) is kind of a conditional BIT..
> > >
> > > what's the behavior of reading/writing to BIT3 when index != 7?
> > > write value will be ignored, read will return zero?
> > I don't know. I haven't tested it, and I don't think it's worth testing.
> > I've written a function to set the parent using BIT(3) and an index.
> > Therefore, when the caller invokes get_parent, BIT(3) is not considered; only the index is.
> >
> Sry, Yixun, We really need to test it!
> If BIT(3) has no effct when FNCLKSEL is set to other values,
> then we can avoid introducing a new function.
> >
> > - Troy
> >
> > >
> > > > +
> > > > + 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, \
> > > > + _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@linux.spacemit.com>
> > > >
> > > >
> > >
> > > --
> > > Yixun Lan (dlan)
> > >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-19 5:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-18 1:54 [PATCH] clk: spacemit: fix sspax_clk Troy Mitchell
2025-07-18 3:16 ` Jesse T
2025-07-18 10:51 ` Yixun Lan
2025-07-18 13:27 ` Troy Mitchell
2025-07-18 13:49 ` Troy Mitchell
2025-07-19 5:51 ` Yao Zi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).