[PATCH 0/2] clk: sunxi-ng: d1: Fix halved MMC frequency

[PATCH 0/2] clk: sunxi-ng: d1: Fix halved MMC frequency

[Andre Przywara]

Hi,

as Kuba reported, on an Allwinner T113-s3 based board the SD card
performance is only half of what we expect: ~11 MB/s instead of the
usual 23 MB/s.
Looking at our MMC mod clock definition, this seems to be due to a
missing fixed (and hidden) divider of 2, a "feature" of every other
Allwinner SoC from the last 10 years.

Patch 2/2 changes the MMC mod clock definition to carry this post_div
of 2, in line with the A64, H6, A100, and H616 SoCs.
Since the D1 clock driver describes clock parents using an array of
pointers, we need to use a new macro that allows both parent_data and
post_div to be specified. This is also needed by the new A523 clocks,
so I lifted the patch from there[1]. Chen-Yu reviewed that one already,
but I made some changes to make the macro more flexible, so I dropped
his tag.

I couldn't test this on a D1 board, so I'd be grateful if someone could
confirm that the SD card performance is now as expected. I simply used
"hdparm -t" to get a ballpark number of read performance, but any other
benchmark or even a timed file copy should do.

Thanks to Kuba for the report!

Cheers,
Andre

[1]
https://lore.kernel.org/linux-sunxi/20250214125359.5204-1-andre.przywara@arm.com/T/#m9adaa1fcea09185c561f3fd01ba895fa67af456c

Andre Przywara (2):
  clk: sunxi-ng: mp: provide wrapper for setting feature flags
  clk: sunxi-ng: d1: Add missing divider for MMC mod clocks

 drivers/clk/sunxi-ng/ccu-sun20i-d1.c | 43 ++++++++++++++++------------
 drivers/clk/sunxi-ng/ccu_mp.h        | 19 ++++++++++--
 2 files changed, 42 insertions(+), 20 deletions(-)

-- 
2.25.1

[PATCH 1/2] clk: sunxi-ng: mp: provide wrapper for setting feature flags

[Andre Przywara]

So far our sunxi clock instantiation macros set the required flags
depending on the clock type, but the new "dual divider MP clock"
requires us to pass that piece of information in by the macro user.

Add a new wrapper macro that allows to specify a "features" field, to
allow marking those dual-divider clocks accordingly.
Since the MMC clocks will be a prominent user, combine this with the
postdiv field required there. Users who just want the feature, can
pass in a postdiv of 1, users of just the postdiv can pass in a feature
mask of 0.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/clk/sunxi-ng/ccu_mp.h | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu_mp.h b/drivers/clk/sunxi-ng/ccu_mp.h
index 6e50f3728fb5f..1da1dcaf62ea9 100644
--- a/drivers/clk/sunxi-ng/ccu_mp.h
+++ b/drivers/clk/sunxi-ng/ccu_mp.h
@@ -82,18 +82,22 @@ struct ccu_mp {
 				   _muxshift, _muxwidth,		\
 				   0, _flags)
 
-#define SUNXI_CCU_MP_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
+#define SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(_struct, _name, _parents, _reg, \
 					_mshift, _mwidth,		\
 					_pshift, _pwidth,		\
 					_muxshift, _muxwidth,		\
-					_gate, _flags)			\
+					_gate, _postdiv,		\
+					_flags, _features)		\
 	struct ccu_mp _struct = {					\
 		.enable	= _gate,					\
 		.m	= _SUNXI_CCU_DIV(_mshift, _mwidth),		\
 		.p	= _SUNXI_CCU_DIV(_pshift, _pwidth),		\
 		.mux	= _SUNXI_CCU_MUX(_muxshift, _muxwidth),		\
+		.fixed_post_div = _postdiv,				\
 		.common	= {						\
 			.reg		= _reg,				\
+			.features	= CCU_FEATURE_FIXED_POSTDIV |	\
+						_features,		\
 			.hw.init	= CLK_HW_INIT_PARENTS_DATA(_name, \
 								   _parents, \
 								   &ccu_mp_ops, \
@@ -101,6 +105,17 @@ struct ccu_mp {
 		}							\
 	}
 
+#define SUNXI_CCU_MP_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
+					_mshift, _mwidth,		\
+					_pshift, _pwidth,		\
+					_muxshift, _muxwidth,		\
+					_gate, _flags)			\
+	SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(_struct, _name, _parents,	\
+					     _reg, _mshift, _mwidth,	\
+					     _pshift, _pwidth,		\
+					     _muxshift, _muxwidth,	\
+					     _gate, 1, _flags, 0)
+
 #define SUNXI_CCU_MP_DATA_WITH_MUX(_struct, _name, _parents, _reg,	\
 				   _mshift, _mwidth,			\
 				   _pshift, _pwidth,			\
-- 
2.25.1

Re: [PATCH 1/2] clk: sunxi-ng: mp: provide wrapper for setting feature flags

[Jernej Škrabec]

Dne sreda, 26. februar 2025 ob 11:37:33 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> So far our sunxi clock instantiation macros set the required flags
> depending on the clock type, but the new "dual divider MP clock"
> requires us to pass that piece of information in by the macro user.
> 
> Add a new wrapper macro that allows to specify a "features" field, to
> allow marking those dual-divider clocks accordingly.
> Since the MMC clocks will be a prominent user, combine this with the
> postdiv field required there. Users who just want the feature, can
> pass in a postdiv of 1, users of just the postdiv can pass in a feature
> mask of 0.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

I'm fine with the new macro, but we don't need to chain them, especially
since this forces all of them to have post divider.

What about making special, standalone macro for that?

Best regards,
Jernej

> ---
>  drivers/clk/sunxi-ng/ccu_mp.h | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_mp.h b/drivers/clk/sunxi-ng/ccu_mp.h
> index 6e50f3728fb5f..1da1dcaf62ea9 100644
> --- a/drivers/clk/sunxi-ng/ccu_mp.h
> +++ b/drivers/clk/sunxi-ng/ccu_mp.h
> @@ -82,18 +82,22 @@ struct ccu_mp {
>  				   _muxshift, _muxwidth,		\
>  				   0, _flags)
>  
> -#define SUNXI_CCU_MP_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
> +#define SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(_struct, _name, _parents, _reg, \
>  					_mshift, _mwidth,		\
>  					_pshift, _pwidth,		\
>  					_muxshift, _muxwidth,		\
> -					_gate, _flags)			\
> +					_gate, _postdiv,		\
> +					_flags, _features)		\
>  	struct ccu_mp _struct = {					\
>  		.enable	= _gate,					\
>  		.m	= _SUNXI_CCU_DIV(_mshift, _mwidth),		\
>  		.p	= _SUNXI_CCU_DIV(_pshift, _pwidth),		\
>  		.mux	= _SUNXI_CCU_MUX(_muxshift, _muxwidth),		\
> +		.fixed_post_div = _postdiv,				\
>  		.common	= {						\
>  			.reg		= _reg,				\
> +			.features	= CCU_FEATURE_FIXED_POSTDIV |	\
> +						_features,		\
>  			.hw.init	= CLK_HW_INIT_PARENTS_DATA(_name, \
>  								   _parents, \
>  								   &ccu_mp_ops, \
> @@ -101,6 +105,17 @@ struct ccu_mp {
>  		}							\
>  	}
>  
> +#define SUNXI_CCU_MP_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
> +					_mshift, _mwidth,		\
> +					_pshift, _pwidth,		\
> +					_muxshift, _muxwidth,		\
> +					_gate, _flags)			\
> +	SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(_struct, _name, _parents,	\
> +					     _reg, _mshift, _mwidth,	\
> +					     _pshift, _pwidth,		\
> +					     _muxshift, _muxwidth,	\
> +					     _gate, 1, _flags, 0)
> +
>  #define SUNXI_CCU_MP_DATA_WITH_MUX(_struct, _name, _parents, _reg,	\
>  				   _mshift, _mwidth,			\
>  				   _pshift, _pwidth,			\
> 

[PATCH 2/2] clk: sunxi-ng: d1: Add missing divider for MMC mod clocks

[Andre Przywara]

The D1/R528/T113 SoCs have a hidden divider of 2 in the MMC mod clocks,
just as other recent SoCs. So far we did not describe that, which led
to the resulting MMC clock rate to be only half of its intended value.

Use a macro that allows to describe a fixed post-divider, to compensate
for that divisor.

This brings the MMC performance on those SoCs to its expected level,
so about 23 MB/s for SD cards, instead of the 11 MB/s measured so far.

Fixes: 35b97bb94111 ("clk: sunxi-ng: Add support for the D1 SoC clocks")
Reported-by: Kuba Szczodrzyński <kuba@szczodrzynski.pl>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/clk/sunxi-ng/ccu-sun20i-d1.c | 43 ++++++++++++++++------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun20i-d1.c b/drivers/clk/sunxi-ng/ccu-sun20i-d1.c
index bb66c906ebbb6..d52a0ef43ea6c 100644
--- a/drivers/clk/sunxi-ng/ccu-sun20i-d1.c
+++ b/drivers/clk/sunxi-ng/ccu-sun20i-d1.c
@@ -412,19 +412,24 @@ static const struct clk_parent_data mmc0_mmc1_parents[] = {
 	{ .hw = &pll_periph0_2x_clk.common.hw },
 	{ .hw = &pll_audio1_div2_clk.common.hw },
 };
-static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(mmc0_clk, "mmc0", mmc0_mmc1_parents, 0x830,
-				       0, 4,	/* M */
-				       8, 2,	/* P */
-				       24, 3,	/* mux */
-				       BIT(31),	/* gate */
-				       0);
 
-static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(mmc1_clk, "mmc1", mmc0_mmc1_parents, 0x834,
-				       0, 4,	/* M */
-				       8, 2,	/* P */
-				       24, 3,	/* mux */
-				       BIT(31),	/* gate */
-				       0);
+static SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(mmc0_clk, "mmc0", mmc0_mmc1_parents,
+					  0x830,
+					  0, 4,		/* M */
+					  8, 2,		/* P */
+					  24, 3,	/* mux */
+					  BIT(31),	/* gate */
+					  2,            /* post-div */
+					  0, 0);
+
+static SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(mmc1_clk, "mmc1", mmc0_mmc1_parents,
+					  0x834,
+					  0, 4,		/* M */
+					  8, 2,		/* P */
+					  24, 3,	/* mux */
+					  BIT(31),	/* gate */
+					  2,            /* post-div */
+					  0, 0);
 
 static const struct clk_parent_data mmc2_parents[] = {
 	{ .fw_name = "hosc" },
@@ -433,12 +438,14 @@ static const struct clk_parent_data mmc2_parents[] = {
 	{ .hw = &pll_periph0_800M_clk.common.hw },
 	{ .hw = &pll_audio1_div2_clk.common.hw },
 };
-static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(mmc2_clk, "mmc2", mmc2_parents, 0x838,
-				       0, 4,	/* M */
-				       8, 2,	/* P */
-				       24, 3,	/* mux */
-				       BIT(31),	/* gate */
-				       0);
+static SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(mmc2_clk, "mmc2", mmc2_parents,
+					  0x838,
+					  0, 4,	/* M */
+					  8, 2,	/* P */
+					  24, 3,	/* mux */
+					  BIT(31),	/* gate */
+					  2,            /* post-div */
+					  0, 0);
 
 static SUNXI_CCU_GATE_HWS(bus_mmc0_clk, "bus-mmc0", psi_ahb_hws,
 			  0x84c, BIT(0), 0);
-- 
2.25.1

Re: [PATCH 2/2] clk: sunxi-ng: d1: Add missing divider for MMC mod clocks

[Jernej Škrabec]

Dne sreda, 26. februar 2025 ob 11:37:34 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> The D1/R528/T113 SoCs have a hidden divider of 2 in the MMC mod clocks,
> just as other recent SoCs. So far we did not describe that, which led
> to the resulting MMC clock rate to be only half of its intended value.
> 
> Use a macro that allows to describe a fixed post-divider, to compensate
> for that divisor.
> 
> This brings the MMC performance on those SoCs to its expected level,
> so about 23 MB/s for SD cards, instead of the 11 MB/s measured so far.
> 
> Fixes: 35b97bb94111 ("clk: sunxi-ng: Add support for the D1 SoC clocks")
> Reported-by: Kuba Szczodrzyński <kuba@szczodrzynski.pl>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej