[PATCH v2 2/3] mmc: starfive: Change tuning implementation
Emil Renner Berthing
emil.renner.berthing at canonical.com
Tue Sep 12 06:20:07 PDT 2023
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 at 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 at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
More information about the linux-riscv
mailing list