[PATCH v3 0/3] Change tuning implementation

[PATCH v3 0/3] Change tuning implementation

[William Qiu]

Hi,

This series of patches changes the tuning implementation, from the
previous way of reading and writing system controller registers to
reading and writing UHS_REG_EXT register, thus optimizing the tuning
of obtaining delay-chain.

Changes v2->v3:
- Rebased to v6.6rc2.
- Dropped redundant criteria.
- Keeped "starfive,sysreg" in dts file.

Changes v1->v2:
- Rebased to v6.6rc1.
- Keeped "starfive,sysreg" in dt-bindings but removed from required.
- Changed the function interface name.
- Maked the code implementation more concise.

The patch series is based on v6.6rc2.

William Qiu (3):
  dt-bindings: mmc: Remove properties from required
  mmc: starfive: Change tuning implementation
  riscv: dts: starfive: add assigned-clock* to limit frquency

 .../bindings/mmc/starfive,jh7110-mmc.yaml     |   2 -
 .../jh7110-starfive-visionfive-2.dtsi         |   4 +
 drivers/mmc/host/dw_mmc-starfive.c            | 137 +++++-------------
 3 files changed, 44 insertions(+), 99 deletions(-)

--
2.34.1

[PATCH v3 1/3] dt-bindings: mmc: Remove properties from required

[William Qiu]

Due to the change of tuning implementation, it's no longer necessary to
use the "starfive,sysreg" property in dts, so remove it from required.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
index 51e1b04e799f..553a75195c2e 100644
--- a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
+++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
@@ -55,7 +55,6 @@ required:
   - clocks
   - clock-names
   - interrupts
-  - starfive,sysreg
 
 unevaluatedProperties: false
 
@@ -73,5 +72,4 @@ examples:
         fifo-depth = <32>;
         fifo-watermark-aligned;
         data-addr = <0>;
-        starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
     };
-- 
2.34.1

[PATCH v3 2/3] mmc: starfive: Change tuning implementation

[William Qiu]

Before, we used syscon to achieve tuning, but the actual measurement
showed little effect, so the tuning implementation was modified here,
and it was realized by reading and writing the UHS_REG_EXT register.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 drivers/mmc/host/dw_mmc-starfive.c | 137 +++++++++--------------------
 1 file changed, 40 insertions(+), 97 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-starfive.c b/drivers/mmc/host/dw_mmc-starfive.c
index fd05a648a8bb..b4d81ef0f3af 100644
--- a/drivers/mmc/host/dw_mmc-starfive.c
+++ b/drivers/mmc/host/dw_mmc-starfive.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2022 StarFive Technology Co., Ltd.
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/mfd/syscon.h>
@@ -20,13 +21,7 @@
 #define ALL_INT_CLR		0x1ffff
 #define MAX_DELAY_CHAIN		32
 
-struct starfive_priv {
-	struct device *dev;
-	struct regmap *reg_syscon;
-	u32 syscon_offset;
-	u32 syscon_shift;
-	u32 syscon_mask;
-};
+#define STARFIVE_SMPL_PHASE     GENMASK(20, 16)
 
 static void dw_mci_starfive_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 {
@@ -44,117 +39,65 @@ static void dw_mci_starfive_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 	}
 }
 
+static void dw_mci_starfive_set_sample_phase(struct dw_mci *host, u32 smpl_phase)
+{
+	/* change driver phase and sample phase */
+	u32 reg_value = mci_readl(host, UHS_REG_EXT);
+
+	/* In UHS_REG_EXT, only 5 bits valid in DRV_PHASE and SMPL_PHASE */
+	reg_value &= ~STARFIVE_SMPL_PHASE;
+	reg_value |= FIELD_PREP(STARFIVE_SMPL_PHASE, smpl_phase);
+	mci_writel(host, UHS_REG_EXT, reg_value);
+
+	/* We should delay 1ms wait for timing setting finished. */
+	mdelay(1);
+}
+
 static int dw_mci_starfive_execute_tuning(struct dw_mci_slot *slot,
 					     u32 opcode)
 {
 	static const int grade  = MAX_DELAY_CHAIN;
 	struct dw_mci *host = slot->host;
-	struct starfive_priv *priv = host->priv;
-	int rise_point = -1, fall_point = -1;
-	int err, prev_err = 0;
-	int i;
-	bool found = 0;
-	u32 regval;
-
-	/*
-	 * Use grade as the max delay chain, and use the rise_point and
-	 * fall_point to ensure the best sampling point of a data input
-	 * signals.
-	 */
-	for (i = 0; i < grade; i++) {
-		regval = i << priv->syscon_shift;
-		err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset,
-						priv->syscon_mask, regval);
-		if (err)
-			return err;
+	int smpl_phase, smpl_raise = -1, smpl_fall = -1;
+	int ret;
+
+	for (smpl_phase = 0; smpl_phase < grade; smpl_phase++) {
+		dw_mci_starfive_set_sample_phase(host, smpl_phase);
 		mci_writel(host, RINTSTS, ALL_INT_CLR);
 
-		err = mmc_send_tuning(slot->mmc, opcode, NULL);
-		if (!err)
-			found = 1;
+		ret = mmc_send_tuning(slot->mmc, opcode, NULL);
 
-		if (i > 0) {
-			if (err && !prev_err)
-				fall_point = i - 1;
-			if (!err && prev_err)
-				rise_point = i;
+		if (!ret && smpl_raise < 0) {
+			smpl_raise = smpl_phase;
+		} else if (ret && smpl_raise >= 0) {
+			smpl_fall = smpl_phase - 1;
+			break;
 		}
-
-		if (rise_point != -1 && fall_point != -1)
-			goto tuning_out;
-
-		prev_err = err;
-		err = 0;
 	}
 
-tuning_out:
-	if (found) {
-		if (rise_point == -1)
-			rise_point = 0;
-		if (fall_point == -1)
-			fall_point = grade - 1;
-		if (fall_point < rise_point) {
-			if ((rise_point + fall_point) >
-			    (grade - 1))
-				i = fall_point / 2;
-			else
-				i = (rise_point + grade - 1) / 2;
-		} else {
-			i = (rise_point + fall_point) / 2;
-		}
-
-		regval = i << priv->syscon_shift;
-		err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset,
-						priv->syscon_mask, regval);
-		if (err)
-			return err;
-		mci_writel(host, RINTSTS, ALL_INT_CLR);
+	if (smpl_phase >= grade)
+		smpl_fall = grade - 1;
 
-		dev_info(host->dev, "Found valid delay chain! use it [delay=%d]\n", i);
-	} else {
+	if (smpl_raise < 0) {
+		smpl_phase = 0;
 		dev_err(host->dev, "No valid delay chain! use default\n");
-		err = -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
-	mci_writel(host, RINTSTS, ALL_INT_CLR);
-	return err;
-}
-
-static int dw_mci_starfive_parse_dt(struct dw_mci *host)
-{
-	struct of_phandle_args args;
-	struct starfive_priv *priv;
-	int ret;
-
-	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	smpl_phase = (smpl_raise + smpl_fall) / 2;
+	dev_dbg(host->dev, "Found valid delay chain! use it [delay=%d]\n", smpl_phase);
+	ret = 0;
 
-	ret = of_parse_phandle_with_fixed_args(host->dev->of_node,
-						"starfive,sysreg", 3, 0, &args);
-	if (ret) {
-		dev_err(host->dev, "Failed to parse starfive,sysreg\n");
-		return -EINVAL;
-	}
-
-	priv->reg_syscon = syscon_node_to_regmap(args.np);
-	of_node_put(args.np);
-	if (IS_ERR(priv->reg_syscon))
-		return PTR_ERR(priv->reg_syscon);
-
-	priv->syscon_offset = args.args[0];
-	priv->syscon_shift  = args.args[1];
-	priv->syscon_mask   = args.args[2];
-
-	host->priv = priv;
-
-	return 0;
+out:
+	dw_mci_starfive_set_sample_phase(host, smpl_phase);
+	mci_writel(host, RINTSTS, ALL_INT_CLR);
+	return ret;
 }
 
 static const struct dw_mci_drv_data starfive_data = {
 	.common_caps		= MMC_CAP_CMD23,
 	.set_ios		= dw_mci_starfive_set_ios,
-	.parse_dt		= dw_mci_starfive_parse_dt,
 	.execute_tuning		= dw_mci_starfive_execute_tuning,
 };
 
-- 
2.34.1

[PATCH v3 3/3] riscv: dts: starfive: add assigned-clock* to limit frquency

[William Qiu]

In JH7110 SoC, we need to go by-pass mode, so we need add the
assigned-clock* properties to limit clock frquency.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index d79f94432b27..d1f2ec308bca 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -205,6 +205,8 @@ &i2c6 {
 
 &mmc0 {
 	max-frequency = <100000000>;
+	assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
+	assigned-clock-rates = <50000000>;
 	bus-width = <8>;
 	cap-mmc-highspeed;
 	mmc-ddr-1_8v;
@@ -221,6 +223,8 @@ &mmc0 {
 
 &mmc1 {
 	max-frequency = <100000000>;
+	assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
+	assigned-clock-rates = <50000000>;
 	bus-width = <4>;
 	no-sdio;
 	no-mmc;
-- 
2.34.1

Re: [PATCH v3 3/3] riscv: dts: starfive: add assigned-clock* to limit frquency

[Emil Renner Berthing]

William Qiu wrote:
> In JH7110 SoC, we need to go by-pass mode, so we need add the
> assigned-clock* properties to limit clock frquency.
>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>

Thanks!

Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> ---
>  .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index d79f94432b27..d1f2ec308bca 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -205,6 +205,8 @@ &i2c6 {
>
>  &mmc0 {
>  	max-frequency = <100000000>;
> +	assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
> +	assigned-clock-rates = <50000000>;
>  	bus-width = <8>;
>  	cap-mmc-highspeed;
>  	mmc-ddr-1_8v;
> @@ -221,6 +223,8 @@ &mmc0 {
>
>  &mmc1 {
>  	max-frequency = <100000000>;
> +	assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
> +	assigned-clock-rates = <50000000>;
>  	bus-width = <4>;
>  	no-sdio;
>  	no-mmc;
> --
> 2.34.1
>

Re: [PATCH v3 0/3] Change tuning implementation

[Ulf Hansson]

On Fri, 22 Sept 2023 at 08:28, William Qiu <william.qiu@starfivetech.com> wrote:
>
> Hi,
>
> This series of patches changes the tuning implementation, from the
> previous way of reading and writing system controller registers to
> reading and writing UHS_REG_EXT register, thus optimizing the tuning
> of obtaining delay-chain.
>
> Changes v2->v3:
> - Rebased to v6.6rc2.
> - Dropped redundant criteria.
> - Keeped "starfive,sysreg" in dts file.
>
> Changes v1->v2:
> - Rebased to v6.6rc1.
> - Keeped "starfive,sysreg" in dt-bindings but removed from required.
> - Changed the function interface name.
> - Maked the code implementation more concise.
>
> The patch series is based on v6.6rc2.
>
> William Qiu (3):
>   dt-bindings: mmc: Remove properties from required
>   mmc: starfive: Change tuning implementation
>   riscv: dts: starfive: add assigned-clock* to limit frquency
>
>  .../bindings/mmc/starfive,jh7110-mmc.yaml     |   2 -
>  .../jh7110-starfive-visionfive-2.dtsi         |   4 +
>  drivers/mmc/host/dw_mmc-starfive.c            | 137 +++++-------------
>  3 files changed, 44 insertions(+), 99 deletions(-)
>

Patch 1 -> 2 applied for next, thanks!

Kind regards
Uffe

Re: (subset) [PATCH v3 0/3] Change tuning implementation

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

On Fri, 22 Sep 2023 14:28:31 +0800, William Qiu wrote:
> This series of patches changes the tuning implementation, from the
> previous way of reading and writing system controller registers to
> reading and writing UHS_REG_EXT register, thus optimizing the tuning
> of obtaining delay-chain.
> 
> Changes v2->v3:
> - Rebased to v6.6rc2.
> - Dropped redundant criteria.
> - Keeped "starfive,sysreg" in dts file.
> 
> [...]

Applied to riscv-dt-for-next, thanks!

[3/3] riscv: dts: starfive: add assigned-clock* to limit frquency
      https://git.kernel.org/conor/c/af571133f7ae

Thanks,
Conor.