2023-09-22 06:29:10

by William Qiu

[permalink] [raw]
Subject: [PATCH v3 0/3] Change tuning implementation

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


2023-09-22 06:35:09

by William Qiu

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

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 <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Reviewed-by: Emil Renner Berthing <[email protected]>
---
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

2023-09-22 06:44:18

by William Qiu

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

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 <[email protected]>
Reviewed-by: Emil Renner Berthing <[email protected]>
---
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

2023-09-22 06:44:31

by William Qiu

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

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 <[email protected]>
---
.../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

2023-09-22 20:28:07

by Emil Renner Berthing

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

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 <[email protected]>

Thanks!

Reviewed-by: Emil Renner Berthing <[email protected]>

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

2023-09-26 16:05:27

by Ulf Hansson

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

On Fri, 22 Sept 2023 at 08:28, William Qiu <[email protected]> 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

2023-09-30 09:00:03

by Conor Dooley

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

From: Conor Dooley <[email protected]>

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.