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

[PATCH 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>
---
 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 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>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 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>
---
 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..2ee66a94ccaa 100644
--- a/drivers/mmc/host/dw_mmc-starfive.c
+++ b/drivers/mmc/host/dw_mmc-starfive.c
@@ -20,13 +20,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 +38,66 @@ 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 mask = 0x1f;
+	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_raise >= 0)
+		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 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>
---
 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..2ee66a94ccaa 100644
--- a/drivers/mmc/host/dw_mmc-starfive.c
+++ b/drivers/mmc/host/dw_mmc-starfive.c
@@ -20,13 +20,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 +38,66 @@ 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 mask = 0x1f;
+	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_raise >= 0)
+		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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[William Qiu]

Drop unused properties and limit cclk_in to 50M, thus cancelling the
internal frequency and adopting the by-pass mode.

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

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;
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index e85464c328d0..7b8e841aeef8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
 			status = "disabled";
 		};
 
@@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
 			status = "disabled";
 		};
 
-- 
2.34.1

[PATCH 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[William Qiu]

Drop unused properties and limit cclk_in to 50M, thus cancelling the
internal frequency and adopting the by-pass mode.

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

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;
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index e85464c328d0..7b8e841aeef8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
 			status = "disabled";
 		};
 
@@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
 			status = "disabled";
 		};
 
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 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 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.6rc1.

William Qiu (3):
  dt-bindings: mmc: Remove properties from required
  mmc: starfive: Change tuning implementation
  riscv: dts: starfive: Drop unused properties and limit frquency

 .../bindings/mmc/starfive,jh7110-mmc.yaml     |   2 -
 .../jh7110-starfive-visionfive-2.dtsi         |   4 +
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |   2 -
 drivers/mmc/host/dw_mmc-starfive.c            | 137 +++++-------------
 4 files changed, 44 insertions(+), 101 deletions(-)

--
2.34.1

[PATCH v2 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 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.6rc1.

William Qiu (3):
  dt-bindings: mmc: Remove properties from required
  mmc: starfive: Change tuning implementation
  riscv: dts: starfive: Drop unused properties and limit frquency

 .../bindings/mmc/starfive,jh7110-mmc.yaml     |   2 -
 .../jh7110-starfive-visionfive-2.dtsi         |   4 +
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |   2 -
 drivers/mmc/host/dw_mmc-starfive.c            | 137 +++++-------------
 4 files changed, 44 insertions(+), 101 deletions(-)

--
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Rob Herring]

On Tue, Sep 12, 2023 at 04:13:59PM +0800, William Qiu 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 v1->v2:

Please don't send new versions as a reply to the prior version.

> - 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.6rc1.
> 
> William Qiu (3):
>   dt-bindings: mmc: Remove properties from required
>   mmc: starfive: Change tuning implementation
>   riscv: dts: starfive: Drop unused properties and limit frquency
> 
>  .../bindings/mmc/starfive,jh7110-mmc.yaml     |   2 -
>  .../jh7110-starfive-visionfive-2.dtsi         |   4 +
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |   2 -
>  drivers/mmc/host/dw_mmc-starfive.c            | 137 +++++-------------
>  4 files changed, 44 insertions(+), 101 deletions(-)
> 
> --
> 2.34.1
> 

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

[Rob Herring]

On Tue, Sep 12, 2023 at 04:13:59PM +0800, William Qiu 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 v1->v2:

Please don't send new versions as a reply to the prior version.

> - 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.6rc1.
> 
> William Qiu (3):
>   dt-bindings: mmc: Remove properties from required
>   mmc: starfive: Change tuning implementation
>   riscv: dts: starfive: Drop unused properties and limit frquency
> 
>  .../bindings/mmc/starfive,jh7110-mmc.yaml     |   2 -
>  .../jh7110-starfive-visionfive-2.dtsi         |   4 +
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |   2 -
>  drivers/mmc/host/dw_mmc-starfive.c            | 137 +++++-------------
>  4 files changed, 44 insertions(+), 101 deletions(-)
> 
> --
> 2.34.1
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 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>
---
 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 v2 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>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

On Tue, Sep 12, 2023 at 04:14:00PM +0800, William Qiu wrote:
> 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>

$subject probably should be more specific about what binding is being
modified.
Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1274 bytes --]

On Tue, Sep 12, 2023 at 04:14:00PM +0800, William Qiu wrote:
> 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>

$subject probably should be more specific about what binding is being
modified.
Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  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
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 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>
---
 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..ad8f39c62fed 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_raise >= 0)
+		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 v2 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>
---
 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..ad8f39c62fed 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_raise >= 0)
+		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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Emil Renner Berthing]

William Qiu wrote:
> 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>
> ---
>  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..ad8f39c62fed 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_raise >= 0)
> +		smpl_fall = grade - 1;

If you switch the order of the two if's above and below you won't need the
smpl_raise >= 0 clause here. With that cleaned up:

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

>
> -		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
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Emil Renner Berthing]

William Qiu wrote:
> 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>
> ---
>  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..ad8f39c62fed 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_raise >= 0)
> +		smpl_fall = grade - 1;

If you switch the order of the two if's above and below you won't need the
smpl_raise >= 0 clause here. With that cleaned up:

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

>
> -		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
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[William Qiu]

Drop unused properties and limit cclk_in to 50M, thus cancelling the
internal frequency and adopting the by-pass mode.

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

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;
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index e85464c328d0..7b8e841aeef8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
 			status = "disabled";
 		};
 
@@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
 			status = "disabled";
 		};
 
-- 
2.34.1

[PATCH v2 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[William Qiu]

Drop unused properties and limit cclk_in to 50M, thus cancelling the
internal frequency and adopting the by-pass mode.

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

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;
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index e85464c328d0..7b8e841aeef8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
 			status = "disabled";
 		};
 
@@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
 			fifo-depth = <32>;
 			fifo-watermark-aligned;
 			data-addr = <0>;
-			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
 			status = "disabled";
 		};
 
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[Emil Renner Berthing]

William Qiu wrote:
> Drop unused properties and limit cclk_in to 50M, thus cancelling the
> internal frequency and adopting the by-pass mode.

That's two unrelated changes which should really be in different patches. But
again the hardware still has the relevant field in the syscon registers even if
the driver doesn't use it, so maybe just leave them and just keep this patch
adding the assigned-clock* properties.

/Emil

>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi                      | 2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> 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;
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index e85464c328d0..7b8e841aeef8 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
>  			fifo-depth = <32>;
>  			fifo-watermark-aligned;
>  			data-addr = <0>;
> -			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
>  			status = "disabled";
>  		};
>
> @@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
>  			fifo-depth = <32>;
>  			fifo-watermark-aligned;
>  			data-addr = <0>;
> -			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
>  			status = "disabled";
>  		};
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[Emil Renner Berthing]

William Qiu wrote:
> Drop unused properties and limit cclk_in to 50M, thus cancelling the
> internal frequency and adopting the by-pass mode.

That's two unrelated changes which should really be in different patches. But
again the hardware still has the relevant field in the syscon registers even if
the driver doesn't use it, so maybe just leave them and just keep this patch
adding the assigned-clock* properties.

/Emil

>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi                      | 2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> 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;
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index e85464c328d0..7b8e841aeef8 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
>  			fifo-depth = <32>;
>  			fifo-watermark-aligned;
>  			data-addr = <0>;
> -			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
>  			status = "disabled";
>  		};
>
> @@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
>  			fifo-depth = <32>;
>  			fifo-watermark-aligned;
>  			data-addr = <0>;
> -			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
>  			status = "disabled";
>  		};
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[William Qiu]

On 2023/9/12 21:23, Emil Renner Berthing wrote:
> William Qiu wrote:
>> Drop unused properties and limit cclk_in to 50M, thus cancelling the
>> internal frequency and adopting the by-pass mode.
> 
> That's two unrelated changes which should really be in different patches. But
> again the hardware still has the relevant field in the syscon registers even if
> the driver doesn't use it, so maybe just leave them and just keep this patch
> adding the assigned-clock* properties.
> 
> /Emil
> 
>>
Will update.

Best Regards,
William
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi                      | 2 --
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> 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;
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index e85464c328d0..7b8e841aeef8 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
>>  			fifo-depth = <32>;
>>  			fifo-watermark-aligned;
>>  			data-addr = <0>;
>> -			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
>>  			status = "disabled";
>>  		};
>>
>> @@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
>>  			fifo-depth = <32>;
>>  			fifo-watermark-aligned;
>>  			data-addr = <0>;
>> -			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
>>  			status = "disabled";
>>  		};
>>
>> --
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/3] riscv: dts: starfive: Drop unused properties and limit frquency

[William Qiu]

On 2023/9/12 21:23, Emil Renner Berthing wrote:
> William Qiu wrote:
>> Drop unused properties and limit cclk_in to 50M, thus cancelling the
>> internal frequency and adopting the by-pass mode.
> 
> That's two unrelated changes which should really be in different patches. But
> again the hardware still has the relevant field in the syscon registers even if
> the driver doesn't use it, so maybe just leave them and just keep this patch
> adding the assigned-clock* properties.
> 
> /Emil
> 
>>
Will update.

Best Regards,
William
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi                      | 2 --
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> 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;
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index e85464c328d0..7b8e841aeef8 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -870,7 +870,6 @@ mmc0: mmc@16010000 {
>>  			fifo-depth = <32>;
>>  			fifo-watermark-aligned;
>>  			data-addr = <0>;
>> -			starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>;
>>  			status = "disabled";
>>  		};
>>
>> @@ -886,7 +885,6 @@ mmc1: mmc@16020000 {
>>  			fifo-depth = <32>;
>>  			fifo-watermark-aligned;
>>  			data-addr = <0>;
>> -			starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>;
>>  			status = "disabled";
>>  		};
>>
>> --
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Emil Renner Berthing]

William Qiu wrote:
> 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.

nit: again the device tree should be a description of the hardware, so the
justification here shouldn't be that the Linux driver doesn't use the field,
but that it turns out the registers aren't required for the peripheral to work
properly. Don't respin the series just for this though.

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

> Signed-off-by: William Qiu <william.qiu@starfivetech.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
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Emil Renner Berthing]

William Qiu wrote:
> 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.

nit: again the device tree should be a description of the hardware, so the
justification here shouldn't be that the Linux driver doesn't use the field,
but that it turns out the registers aren't required for the peripheral to work
properly. Don't respin the series just for this though.

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

> Signed-off-by: William Qiu <william.qiu@starfivetech.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
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

On Tue, Sep 12, 2023 at 02:12:44AM -0700, Emil Renner Berthing wrote:
> William Qiu wrote:
> > 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.
> 
> nit: again the device tree should be a description of the hardware, so the
> justification here shouldn't be that the Linux driver doesn't use the field,
> but that it turns out the registers aren't required for the peripheral to work
> properly. Don't respin the series just for this though.
> 
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

The fact that I can't actually apply this without breaking bisection
kinda hints at removing it in this patch is incorrect.

> 
> > Signed-off-by: William Qiu <william.qiu@starfivetech.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
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1881 bytes --]

On Tue, Sep 12, 2023 at 02:12:44AM -0700, Emil Renner Berthing wrote:
> William Qiu wrote:
> > 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.
> 
> nit: again the device tree should be a description of the hardware, so the
> justification here shouldn't be that the Linux driver doesn't use the field,
> but that it turns out the registers aren't required for the peripheral to work
> properly. Don't respin the series just for this though.
> 
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

The fact that I can't actually apply this without breaking bisection
kinda hints at removing it in this patch is incorrect.

> 
> > Signed-off-by: William Qiu <william.qiu@starfivetech.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
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv