it's modified from hi3798cv200 driver, but quite a lot of code gets
rewritten because of the hardware differences. Actually cv200 DWMMC core
is called HIMCIV200 while mv200 DWMMC core is called HIMCIV300 in
downstream.
Signed-off-by: Yang Xiwen <[email protected]>
---
Changes in v6:
- apply the comments to the first patch, add their trailers
- Link to v5: https://lore.kernel.org/r/[email protected]
Changes in v5:
- pick the dependant patch: https://lore.kernel.org/all/[email protected]/
to fix the bot build error.
- edit the semantic meaning of hisilicon,sap-dll-reg property (Rob Herring)
The suggestion is from the CRG driver side:
https://lore.kernel.org/all/[email protected]/
- Link to v4: https://lore.kernel.org/r/[email protected]
Changes in v4:
- rename dw_mmc-hi3798 back to hi3798cv200 - Suggested by Krzysztof Kozlowski.
- add r-bs to patch 1 and 2 - Reviewed by Krzysztof Kozlowski.
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- dw_mmc-hi3798: fix bot error (Rob Herring)
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- dw_mmc-hi3798mv200: use dev_err_probe() helper - Suggested by Krzysztof Kozlowski.
- dw_mmc-hi3798mv200: add missing err=0;
- dw_mmc-hi3798c(m)v200: remove unused MODULE_ALIAS() - Suggested by Krzysztof Kozlowski.
- binding: rename the binding, a lot of tweaks suggested by Krzysztof Kozlowski.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Yang Xiwen (5):
mmc: host: mmc_of_parse_clk_phase(): Pass struct device * instead of mmc_host *
mmc: dw_mmc-hi3798cv200: remove MODULE_ALIAS()
mmc: dw_mmc: add support for hi3798mv200
dt-bindings: mmc: dw-mshc-hi3798cv200: convert to YAML
dt-bindings: mmc: hisilicon,hi3798cv200-dw-mshc: add Hi3798MV200 binding
.../bindings/mmc/hi3798cv200-dw-mshc.txt | 40 ----
.../mmc/hisilicon,hi3798cv200-dw-mshc.yaml | 97 +++++++++
drivers/mmc/core/host.c | 4 +-
drivers/mmc/host/Kconfig | 9 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/dw_mmc-hi3798cv200.c | 1 -
drivers/mmc/host/dw_mmc-hi3798mv200.c | 239 +++++++++++++++++++++
drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
include/linux/mmc/host.h | 2 +-
9 files changed, 349 insertions(+), 46 deletions(-)
---
base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
change-id: 20240121-b4-mmc-hi3798mv200-a5730edf122c
Best regards,
--
Yang Xiwen <[email protected]>
From: Yang Xiwen <[email protected]>
Add support for Hi3798MV200 specific extension.
Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/mmc/host/Kconfig | 9 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/dw_mmc-hi3798mv200.c | 239 ++++++++++++++++++++++++++++++++++
3 files changed, 249 insertions(+)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 81f2c4e05287..aebc587f77a7 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -798,6 +798,15 @@ config MMC_DW_HI3798CV200
Synopsys DesignWare Memory Card Interface driver. Select this option
for platforms based on HiSilicon Hi3798CV200 SoC.
+config MMC_DW_HI3798MV200
+ tristate "Hi3798MV200 specific extensions for Synopsys DW Memory Card Interface"
+ depends on MMC_DW
+ select MMC_DW_PLTFM
+ help
+ This selects support for HiSilicon Hi3798MV200 SoC specific extensions to the
+ Synopsys DesignWare Memory Card Interface driver. Select this option
+ for platforms based on HiSilicon Hi3798MV200 SoC.
+
config MMC_DW_K3
tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
depends on MMC_DW
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index d0be4465f3ec..f53f86d200ac 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_MMC_DW_PLTFM) += dw_mmc-pltfm.o
obj-$(CONFIG_MMC_DW_BLUEFIELD) += dw_mmc-bluefield.o
obj-$(CONFIG_MMC_DW_EXYNOS) += dw_mmc-exynos.o
obj-$(CONFIG_MMC_DW_HI3798CV200) += dw_mmc-hi3798cv200.o
+obj-$(CONFIG_MMC_DW_HI3798MV200) += dw_mmc-hi3798mv200.o
obj-$(CONFIG_MMC_DW_K3) += dw_mmc-k3.o
obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o
obj-$(CONFIG_MMC_DW_ROCKCHIP) += dw_mmc-rockchip.o
diff --git a/drivers/mmc/host/dw_mmc-hi3798mv200.c b/drivers/mmc/host/dw_mmc-hi3798mv200.c
new file mode 100644
index 000000000000..73aaa21040ea
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-hi3798mv200.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Modified from dw_mmc-hi3798cv200.c
+ *
+ * Copyright (c) 2024 Yang Xiwen <[email protected]>
+ * Copyright (c) 2018 HiSilicon Technologies Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mmc/host.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "dw_mmc.h"
+#include "dw_mmc-pltfm.h"
+
+#define SDMMC_TUNING_CTRL 0x118
+#define SDMMC_TUNING_FIND_EDGE BIT(5)
+
+#define ALL_INT_CLR 0x1ffff
+
+/* DLL ctrl reg */
+#define SAP_DLL_CTRL_DLLMODE BIT(16)
+
+struct dw_mci_hi3798mv200_priv {
+ struct clk *sample_clk;
+ struct clk *drive_clk;
+ struct regmap *crg_reg;
+ u32 sap_dll_offset;
+ struct mmc_clk_phase_map phase_map;
+};
+
+static void dw_mci_hi3798mv200_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+ struct dw_mci_hi3798mv200_priv *priv = host->priv;
+ struct mmc_clk_phase phase = priv->phase_map.phase[ios->timing];
+ u32 val;
+
+ val = mci_readl(host, ENABLE_SHIFT);
+ if (ios->timing == MMC_TIMING_MMC_DDR52
+ || ios->timing == MMC_TIMING_UHS_DDR50)
+ val |= SDMMC_ENABLE_PHASE;
+ else
+ val &= ~SDMMC_ENABLE_PHASE;
+ mci_writel(host, ENABLE_SHIFT, val);
+
+ val = mci_readl(host, DDR_REG);
+ if (ios->timing == MMC_TIMING_MMC_HS400)
+ val |= SDMMC_DDR_HS400;
+ else
+ val &= ~SDMMC_DDR_HS400;
+ mci_writel(host, DDR_REG, val);
+
+ if (clk_set_rate(host->ciu_clk, ios->clock))
+ dev_warn(host->dev, "Failed to set rate to %u\n", ios->clock);
+ else
+ // CLK_MUX_ROUND_NEAREST is enabled for this clock
+ // The actual clock rate is not what we setted, but a rounded value
+ // so we should get the rate once again
+ host->bus_hz = clk_get_rate(host->ciu_clk);
+
+ if (phase.valid) {
+ clk_set_phase(priv->drive_clk, phase.out_deg);
+ clk_set_phase(priv->sample_clk, phase.in_deg);
+ } else {
+ dev_warn(host->dev,
+ "The phase entry for timing mode %d is missing in device tree.\n",
+ ios->timing);
+ }
+}
+
+static inline int dw_mci_hi3798mv200_enable_tuning(struct dw_mci_slot *slot)
+{
+ struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
+
+ return regmap_clear_bits(priv->crg_reg, priv->sap_dll_offset, SAP_DLL_CTRL_DLLMODE);
+}
+
+static inline int dw_mci_hi3798mv200_disable_tuning(struct dw_mci_slot *slot)
+{
+ struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
+
+ return regmap_set_bits(priv->crg_reg, priv->sap_dll_offset, SAP_DLL_CTRL_DLLMODE);
+}
+
+static int dw_mci_hi3798mv200_execute_tuning_mix_mode(struct dw_mci_slot *slot,
+ u32 opcode)
+{
+ static const int degrees[] = { 0, 45, 90, 135, 180, 225, 270, 315 };
+ struct dw_mci *host = slot->host;
+ struct dw_mci_hi3798mv200_priv *priv = host->priv;
+ int raise_point = -1, fall_point = -1;
+ int err, prev_err = -1;
+ int found = 0;
+ int regval;
+ int i;
+ int ret;
+
+ // enable tuning
+ ret = dw_mci_hi3798mv200_enable_tuning(slot);
+ if (ret < 0)
+ return ret;
+ for (i = 0; i < ARRAY_SIZE(degrees); i++) {
+ clk_set_phase(priv->sample_clk, degrees[i]);
+ mci_writel(host, RINTSTS, ALL_INT_CLR);
+
+ err = mmc_send_tuning(slot->mmc, opcode, NULL);
+ if (!err) {
+ regval = mci_readl(host, TUNING_CTRL);
+ if (regval & SDMMC_TUNING_FIND_EDGE)
+ err = 1;
+ else
+ found = 1;
+ };
+
+ if (i > 0) {
+ if (err && !prev_err)
+ fall_point = i - 1;
+ if (!err && prev_err)
+ raise_point = i;
+ }
+
+ if (raise_point != -1 && fall_point != -1)
+ goto tuning_out;
+
+ prev_err = err;
+ err = 0;
+ }
+
+tuning_out:
+ ret = dw_mci_hi3798mv200_disable_tuning(slot);
+ if (ret < 0)
+ return ret;
+ if (found) {
+ if (raise_point == -1)
+ raise_point = 0;
+ if (fall_point == -1)
+ fall_point = ARRAY_SIZE(degrees) - 1;
+ if (fall_point < raise_point) {
+ if ((raise_point + fall_point) >
+ (ARRAY_SIZE(degrees) - 1))
+ i = fall_point / 2;
+ else
+ i = (raise_point + ARRAY_SIZE(degrees) - 1) / 2;
+ } else {
+ i = (raise_point + fall_point) / 2;
+ }
+
+ // use the same phase table for both HS200 and HS400
+ priv->phase_map.phase[MMC_TIMING_MMC_HS200].in_deg = degrees[i];
+ priv->phase_map.phase[MMC_TIMING_MMC_HS400].in_deg = degrees[i];
+
+ clk_set_phase(priv->sample_clk, degrees[i]);
+ dev_dbg(host->dev, "Tuning clk_sample[%d, %d], set[%d]\n",
+ raise_point, fall_point, degrees[i]);
+ err = 0;
+ } else {
+ dev_err(host->dev, "No valid clk_sample shift! use default\n");
+ err = -EINVAL;
+ }
+
+ mci_writel(host, RINTSTS, ALL_INT_CLR);
+ return err;
+}
+
+static int dw_mci_hi3798mv200_init(struct dw_mci *host)
+{
+ struct dw_mci_hi3798mv200_priv *priv;
+ struct device_node *np = host->dev->of_node;
+ int ret;
+
+ priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
+
+ priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
+ if (IS_ERR(priv->sample_clk))
+ return dev_err_probe(host->dev, PTR_ERR(priv->sample_clk),
+ "failed to get enabled ciu-sample clock\n");
+
+ priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
+ if (IS_ERR(priv->drive_clk))
+ return dev_err_probe(host->dev, PTR_ERR(priv->drive_clk),
+ "failed to get enabled ciu-drive clock\n");
+
+ priv->crg_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
+ if (IS_ERR(priv->crg_reg))
+ return dev_err_probe(host->dev, PTR_ERR(priv->crg_reg),
+ "failed to get CRG reg\n");
+
+ ret = of_property_read_u32_index(np, "hisilicon,sap-dll-reg", 1, &priv->sap_dll_offset);
+ if (ret)
+ return dev_err_probe(host->dev, ret, "failed to get sample DLL register offset\n");
+
+ host->priv = priv;
+ return 0;
+}
+
+static const struct dw_mci_drv_data hi3798mv200_data = {
+ .common_caps = MMC_CAP_CMD23,
+ .init = dw_mci_hi3798mv200_init,
+ .set_ios = dw_mci_hi3798mv200_set_ios,
+ .execute_tuning = dw_mci_hi3798mv200_execute_tuning_mix_mode,
+};
+
+static const struct of_device_id dw_mci_hi3798mv200_match[] = {
+ { .compatible = "hisilicon,hi3798mv200-dw-mshc" },
+ {},
+};
+
+static int dw_mci_hi3798mv200_probe(struct platform_device *pdev)
+{
+ return dw_mci_pltfm_register(pdev, &hi3798mv200_data);
+}
+
+static void dw_mci_hi3798mv200_remove(struct platform_device *pdev)
+{
+ dw_mci_pltfm_remove(pdev);
+}
+
+MODULE_DEVICE_TABLE(of, dw_mci_hi3798mv200_match);
+static struct platform_driver dw_mci_hi3798mv200_driver = {
+ .probe = dw_mci_hi3798mv200_probe,
+ .remove_new = dw_mci_hi3798mv200_remove,
+ .driver = {
+ .name = "dwmmc_hi3798mv200",
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .of_match_table = dw_mci_hi3798mv200_match,
+ },
+};
+module_platform_driver(dw_mci_hi3798mv200_driver);
+
+MODULE_DESCRIPTION("HiSilicon Hi3798MV200 Specific DW-MSHC Driver Extension");
+MODULE_LICENSE("GPL");
--
2.43.0
From: Yang Xiwen <[email protected]>
Parsing dt usually happens very early, sometimes even before struct
mmc_host is allocated (e.g. dw_mci_probe() and dw_mci_parse_dt() in
dw_mmc.c). Looking at the source of mmc_of_parse_clk_phase(), it's
actually not mandatory to have an initialized mmc_host first, instead we
can pass struct device * to it directly.
Also fix the only current user (sdhci-of-aspeed.c).
Reviewed-by: Paul Menzel <[email protected]>
Acked-by: Andrew Jeffery <[email protected]>
Signed-off-by: Yang Xiwen <[email protected]>
---
drivers/mmc/core/host.c | 4 +---
drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
include/linux/mmc/host.h | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index cf396e8f34e9..8b2844ac5dc5 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -234,10 +234,8 @@ static void mmc_of_parse_timing_phase(struct device *dev, const char *prop,
}
void
-mmc_of_parse_clk_phase(struct mmc_host *host, struct mmc_clk_phase_map *map)
+mmc_of_parse_clk_phase(struct device *dev, struct mmc_clk_phase_map *map)
{
- struct device *dev = host->parent;
-
mmc_of_parse_timing_phase(dev, "clk-phase-legacy",
&map->phase[MMC_TIMING_LEGACY]);
mmc_of_parse_timing_phase(dev, "clk-phase-mmc-hs",
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 42d54532cabe..430c1f90037b 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -435,7 +435,7 @@ static int aspeed_sdhci_probe(struct platform_device *pdev)
goto err_sdhci_add;
if (dev->phase_desc)
- mmc_of_parse_clk_phase(host->mmc, &dev->phase_map);
+ mmc_of_parse_clk_phase(&pdev->dev, &dev->phase_map);
ret = sdhci_add_host(host);
if (ret)
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 2f445c651742..5894bf912f7b 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -539,7 +539,7 @@ struct mmc_host *devm_mmc_alloc_host(struct device *dev, int extra);
int mmc_add_host(struct mmc_host *);
void mmc_remove_host(struct mmc_host *);
void mmc_free_host(struct mmc_host *);
-void mmc_of_parse_clk_phase(struct mmc_host *host,
+void mmc_of_parse_clk_phase(struct device *dev,
struct mmc_clk_phase_map *map);
int mmc_of_parse(struct mmc_host *host);
int mmc_of_parse_voltage(struct mmc_host *host, u32 *mask);
--
2.43.0
From: Yang Xiwen <[email protected]>
convert the legacy txt binding to modern YAML and rename to
hisilicon,hi3798cv200-dw-mshc.yaml. No semantic change.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Yang Xiwen <[email protected]>
---
.../bindings/mmc/hi3798cv200-dw-mshc.txt | 40 ------------
.../mmc/hisilicon,hi3798cv200-dw-mshc.yaml | 75 ++++++++++++++++++++++
2 files changed, 75 insertions(+), 40 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
deleted file mode 100644
index a0693b7145f2..000000000000
--- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-* Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
- Storage Host Controller
-
-Read synopsys-dw-mshc.txt for more details
-
-The Synopsys designware mobile storage host controller is used to interface
-a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
-differences between the core Synopsys dw mshc controller properties described
-by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
-specific extensions to the Synopsys Designware Mobile Storage Host Controller.
-
-Required Properties:
-- compatible: Should contain "hisilicon,hi3798cv200-dw-mshc".
-- clocks: A list of phandle + clock-specifier pairs for the clocks listed
- in clock-names.
-- clock-names: Should contain the following:
- "ciu" - The ciu clock described in synopsys-dw-mshc.txt.
- "biu" - The biu clock described in synopsys-dw-mshc.txt.
- "ciu-sample" - Hi3798CV200 extended phase clock for ciu sampling.
- "ciu-drive" - Hi3798CV200 extended phase clock for ciu driving.
-
-Example:
-
- emmc: mmc@9830000 {
- compatible = "hisilicon,hi3798cv200-dw-mshc";
- reg = <0x9830000 0x10000>;
- interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&crg HISTB_MMC_CIU_CLK>,
- <&crg HISTB_MMC_BIU_CLK>,
- <&crg HISTB_MMC_SAMPLE_CLK>,
- <&crg HISTB_MMC_DRV_CLK>;
- clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
- fifo-depth = <256>;
- clock-frequency = <200000000>;
- cap-mmc-highspeed;
- mmc-ddr-1_8v;
- mmc-hs200-1_8v;
- non-removable;
- bus-width = <8>;
- };
diff --git a/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml
new file mode 100644
index 000000000000..f3dc973cb490
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/hisilicon,hi3798cv200-dw-mshc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
+
+maintainers:
+ - Yang Xiwen <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - hisilicon,hi3798cv200-dw-mshc
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: bus interface unit clock
+ - description: card interface unit clock
+ - description: card input sample phase clock
+ - description: controller output drive phase clock
+
+ clock-names:
+ items:
+ - const: ciu
+ - const: biu
+ - const: ciu-sample
+ - const: ciu-drive
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+allOf:
+ - $ref: synopsys-dw-mshc-common.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/histb-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ mmc@9830000 {
+ compatible = "hisilicon,hi3798cv200-dw-mshc";
+ reg = <0x9830000 0x10000>;
+ interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&crg HISTB_MMC_CIU_CLK>,
+ <&crg HISTB_MMC_BIU_CLK>,
+ <&crg HISTB_MMC_SAMPLE_CLK>,
+ <&crg HISTB_MMC_DRV_CLK>;
+ clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
+ resets = <&crg 0xa0 4>;
+ reset-names = "reset";
+ pinctrl-names = "default";
+ pinctrl-0 = <&emmc_pins_1 &emmc_pins_2
+ &emmc_pins_3 &emmc_pins_4>;
+ fifo-depth = <256>;
+ clock-frequency = <200000000>;
+ cap-mmc-highspeed;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ non-removable;
+ bus-width = <8>;
+ };
--
2.43.0
From: Yang Xiwen <[email protected]>
Add binding and an extra property for Hi3798MV200 DWMMC specific extension.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Yang Xiwen <[email protected]>
---
.../mmc/hisilicon,hi3798cv200-dw-mshc.yaml | 24 +++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml
index f3dc973cb490..41c9b22523e7 100644
--- a/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml
+++ b/Documentation/devicetree/bindings/mmc/hisilicon,hi3798cv200-dw-mshc.yaml
@@ -4,7 +4,7 @@
$id: http://devicetree.org/schemas/mmc/hisilicon,hi3798cv200-dw-mshc.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
+title: Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
maintainers:
- Yang Xiwen <[email protected]>
@@ -13,6 +13,7 @@ properties:
compatible:
enum:
- hisilicon,hi3798cv200-dw-mshc
+ - hisilicon,hi3798mv200-dw-mshc
reg:
maxItems: 1
@@ -34,6 +35,15 @@ properties:
- const: ciu-sample
- const: ciu-drive
+ hisilicon,sap-dll-reg:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ DWMMC core on Hi3798MV2x SoCs has a delay-locked-loop(DLL) attached to card data input path.
+ It is integrated into CRG core on the SoC and has to be controlled during tuning.
+ items:
+ - description: A phandle pointed to the CRG syscon node
+ - description: Sample DLL register offset in CRG address space
+
required:
- compatible
- reg
@@ -44,6 +54,18 @@ required:
allOf:
- $ref: synopsys-dw-mshc-common.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: hisilicon,hi3798mv200-dw-mshc
+ then:
+ required:
+ - hisilicon,sap-dll-reg
+ else:
+ properties:
+ hisilicon,sap-dll-reg: false
+
unevaluatedProperties: false
examples:
--
2.43.0
On Wed, 21 Feb 2024 at 13:45, Yang Xiwen via B4 Relay
<[email protected]> wrote:
>
> From: Yang Xiwen <[email protected]>
>
> Add support for Hi3798MV200 specific extension.
>
> Signed-off-by: Yang Xiwen <[email protected]>
> ---
> drivers/mmc/host/Kconfig | 9 ++
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/dw_mmc-hi3798mv200.c | 239 ++++++++++++++++++++++++++++++++++
> 3 files changed, 249 insertions(+)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 81f2c4e05287..aebc587f77a7 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -798,6 +798,15 @@ config MMC_DW_HI3798CV200
> Synopsys DesignWare Memory Card Interface driver. Select this option
> for platforms based on HiSilicon Hi3798CV200 SoC.
>
> +config MMC_DW_HI3798MV200
> + tristate "Hi3798MV200 specific extensions for Synopsys DW Memory Card Interface"
> + depends on MMC_DW
> + select MMC_DW_PLTFM
> + help
> + This selects support for HiSilicon Hi3798MV200 SoC specific extensions to the
> + Synopsys DesignWare Memory Card Interface driver. Select this option
> + for platforms based on HiSilicon Hi3798MV200 SoC.
> +
> config MMC_DW_K3
> tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
> depends on MMC_DW
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index d0be4465f3ec..f53f86d200ac 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_MMC_DW_PLTFM) += dw_mmc-pltfm.o
> obj-$(CONFIG_MMC_DW_BLUEFIELD) += dw_mmc-bluefield.o
> obj-$(CONFIG_MMC_DW_EXYNOS) += dw_mmc-exynos.o
> obj-$(CONFIG_MMC_DW_HI3798CV200) += dw_mmc-hi3798cv200.o
> +obj-$(CONFIG_MMC_DW_HI3798MV200) += dw_mmc-hi3798mv200.o
> obj-$(CONFIG_MMC_DW_K3) += dw_mmc-k3.o
> obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o
> obj-$(CONFIG_MMC_DW_ROCKCHIP) += dw_mmc-rockchip.o
> diff --git a/drivers/mmc/host/dw_mmc-hi3798mv200.c b/drivers/mmc/host/dw_mmc-hi3798mv200.c
> new file mode 100644
> index 000000000000..73aaa21040ea
> --- /dev/null
> +++ b/drivers/mmc/host/dw_mmc-hi3798mv200.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Modified from dw_mmc-hi3798cv200.c
> + *
> + * Copyright (c) 2024 Yang Xiwen <[email protected]>
> + * Copyright (c) 2018 HiSilicon Technologies Co., Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mmc/host.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "dw_mmc.h"
> +#include "dw_mmc-pltfm.h"
> +
> +#define SDMMC_TUNING_CTRL 0x118
> +#define SDMMC_TUNING_FIND_EDGE BIT(5)
> +
> +#define ALL_INT_CLR 0x1ffff
> +
> +/* DLL ctrl reg */
> +#define SAP_DLL_CTRL_DLLMODE BIT(16)
> +
> +struct dw_mci_hi3798mv200_priv {
> + struct clk *sample_clk;
> + struct clk *drive_clk;
> + struct regmap *crg_reg;
> + u32 sap_dll_offset;
> + struct mmc_clk_phase_map phase_map;
> +};
> +
> +static void dw_mci_hi3798mv200_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> +{
> + struct dw_mci_hi3798mv200_priv *priv = host->priv;
> + struct mmc_clk_phase phase = priv->phase_map.phase[ios->timing];
> + u32 val;
> +
> + val = mci_readl(host, ENABLE_SHIFT);
> + if (ios->timing == MMC_TIMING_MMC_DDR52
> + || ios->timing == MMC_TIMING_UHS_DDR50)
> + val |= SDMMC_ENABLE_PHASE;
> + else
> + val &= ~SDMMC_ENABLE_PHASE;
> + mci_writel(host, ENABLE_SHIFT, val);
> +
> + val = mci_readl(host, DDR_REG);
> + if (ios->timing == MMC_TIMING_MMC_HS400)
> + val |= SDMMC_DDR_HS400;
> + else
> + val &= ~SDMMC_DDR_HS400;
> + mci_writel(host, DDR_REG, val);
> +
> + if (clk_set_rate(host->ciu_clk, ios->clock))
> + dev_warn(host->dev, "Failed to set rate to %u\n", ios->clock);
> + else
> + // CLK_MUX_ROUND_NEAREST is enabled for this clock
> + // The actual clock rate is not what we setted, but a rounded value
> + // so we should get the rate once again
Please use proper comments sections (/* .... */) and not "//".
> + host->bus_hz = clk_get_rate(host->ciu_clk);
> +
> + if (phase.valid) {
> + clk_set_phase(priv->drive_clk, phase.out_deg);
> + clk_set_phase(priv->sample_clk, phase.in_deg);
> + } else {
> + dev_warn(host->dev,
> + "The phase entry for timing mode %d is missing in device tree.\n",
> + ios->timing);
> + }
> +}
> +
> +static inline int dw_mci_hi3798mv200_enable_tuning(struct dw_mci_slot *slot)
> +{
> + struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
> +
> + return regmap_clear_bits(priv->crg_reg, priv->sap_dll_offset, SAP_DLL_CTRL_DLLMODE);
> +}
> +
> +static inline int dw_mci_hi3798mv200_disable_tuning(struct dw_mci_slot *slot)
> +{
> + struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
> +
> + return regmap_set_bits(priv->crg_reg, priv->sap_dll_offset, SAP_DLL_CTRL_DLLMODE);
> +}
> +
> +static int dw_mci_hi3798mv200_execute_tuning_mix_mode(struct dw_mci_slot *slot,
> + u32 opcode)
> +{
> + static const int degrees[] = { 0, 45, 90, 135, 180, 225, 270, 315 };
> + struct dw_mci *host = slot->host;
> + struct dw_mci_hi3798mv200_priv *priv = host->priv;
> + int raise_point = -1, fall_point = -1;
> + int err, prev_err = -1;
> + int found = 0;
> + int regval;
> + int i;
> + int ret;
> +
> + // enable tuning
Looks like a redundant comment, please drop it.
> + ret = dw_mci_hi3798mv200_enable_tuning(slot);
> + if (ret < 0)
> + return ret;
A newline here would improve the readability I think.
> + for (i = 0; i < ARRAY_SIZE(degrees); i++) {
> + clk_set_phase(priv->sample_clk, degrees[i]);
> + mci_writel(host, RINTSTS, ALL_INT_CLR);
> +
> + err = mmc_send_tuning(slot->mmc, opcode, NULL);
> + if (!err) {
> + regval = mci_readl(host, TUNING_CTRL);
> + if (regval & SDMMC_TUNING_FIND_EDGE)
> + err = 1;
> + else
> + found = 1;
> + };
> +
> + if (i > 0) {
> + if (err && !prev_err)
> + fall_point = i - 1;
> + if (!err && prev_err)
> + raise_point = i;
> + }
> +
> + if (raise_point != -1 && fall_point != -1)
> + goto tuning_out;
> +
> + prev_err = err;
> + err = 0;
> + }
> +
> +tuning_out:
> + ret = dw_mci_hi3798mv200_disable_tuning(slot);
> + if (ret < 0)
> + return ret;
> + if (found) {
> + if (raise_point == -1)
> + raise_point = 0;
> + if (fall_point == -1)
> + fall_point = ARRAY_SIZE(degrees) - 1;
> + if (fall_point < raise_point) {
> + if ((raise_point + fall_point) >
> + (ARRAY_SIZE(degrees) - 1))
> + i = fall_point / 2;
> + else
> + i = (raise_point + ARRAY_SIZE(degrees) - 1) / 2;
> + } else {
> + i = (raise_point + fall_point) / 2;
> + }
> +
> + // use the same phase table for both HS200 and HS400
Don't use "//" for comments.
> + priv->phase_map.phase[MMC_TIMING_MMC_HS200].in_deg = degrees[i];
> + priv->phase_map.phase[MMC_TIMING_MMC_HS400].in_deg = degrees[i];
> +
> + clk_set_phase(priv->sample_clk, degrees[i]);
> + dev_dbg(host->dev, "Tuning clk_sample[%d, %d], set[%d]\n",
> + raise_point, fall_point, degrees[i]);
> + err = 0;
> + } else {
> + dev_err(host->dev, "No valid clk_sample shift! use default\n");
> + err = -EINVAL;
> + }
> +
> + mci_writel(host, RINTSTS, ALL_INT_CLR);
> + return err;
The entire code in dw_mci_hi3798mv200_execute_tuning_mix_mode() looks
rather messy to me. A lot of variables are being used, set and reset
from everywhere.
Would you mind having a closer look and try to improve it a bit, so it
becomes easier to follow what is going on?
> +}
> +
> +static int dw_mci_hi3798mv200_init(struct dw_mci *host)
> +{
> + struct dw_mci_hi3798mv200_priv *priv;
> + struct device_node *np = host->dev->of_node;
> + int ret;
> +
> + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
> +
> + priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
> + if (IS_ERR(priv->sample_clk))
> + return dev_err_probe(host->dev, PTR_ERR(priv->sample_clk),
> + "failed to get enabled ciu-sample clock\n");
> +
> + priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
> + if (IS_ERR(priv->drive_clk))
> + return dev_err_probe(host->dev, PTR_ERR(priv->drive_clk),
> + "failed to get enabled ciu-drive clock\n");
> +
> + priv->crg_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
> + if (IS_ERR(priv->crg_reg))
> + return dev_err_probe(host->dev, PTR_ERR(priv->crg_reg),
> + "failed to get CRG reg\n");
> +
> + ret = of_property_read_u32_index(np, "hisilicon,sap-dll-reg", 1, &priv->sap_dll_offset);
> + if (ret)
> + return dev_err_probe(host->dev, ret, "failed to get sample DLL register offset\n");
> +
> + host->priv = priv;
> + return 0;
> +}
> +
> +static const struct dw_mci_drv_data hi3798mv200_data = {
> + .common_caps = MMC_CAP_CMD23,
> + .init = dw_mci_hi3798mv200_init,
> + .set_ios = dw_mci_hi3798mv200_set_ios,
> + .execute_tuning = dw_mci_hi3798mv200_execute_tuning_mix_mode,
> +};
> +
> +static const struct of_device_id dw_mci_hi3798mv200_match[] = {
> + { .compatible = "hisilicon,hi3798mv200-dw-mshc" },
> + {},
> +};
> +
> +static int dw_mci_hi3798mv200_probe(struct platform_device *pdev)
> +{
> + return dw_mci_pltfm_register(pdev, &hi3798mv200_data);
> +}
> +
> +static void dw_mci_hi3798mv200_remove(struct platform_device *pdev)
> +{
> + dw_mci_pltfm_remove(pdev);
> +}
> +
> +MODULE_DEVICE_TABLE(of, dw_mci_hi3798mv200_match);
> +static struct platform_driver dw_mci_hi3798mv200_driver = {
> + .probe = dw_mci_hi3798mv200_probe,
> + .remove_new = dw_mci_hi3798mv200_remove,
> + .driver = {
> + .name = "dwmmc_hi3798mv200",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + .of_match_table = dw_mci_hi3798mv200_match,
> + },
> +};
> +module_platform_driver(dw_mci_hi3798mv200_driver);
> +
> +MODULE_DESCRIPTION("HiSilicon Hi3798MV200 Specific DW-MSHC Driver Extension");
> +MODULE_LICENSE("GPL");
>
Kind regards
Uffe
On Wed, 21 Feb 2024 at 13:45, Yang Xiwen via B4 Relay
<[email protected]> wrote:
>
> it's modified from hi3798cv200 driver, but quite a lot of code gets
> rewritten because of the hardware differences. Actually cv200 DWMMC core
> is called HIMCIV200 while mv200 DWMMC core is called HIMCIV300 in
> downstream.
>
> Signed-off-by: Yang Xiwen <[email protected]>
Please re-order the patches in the series so the changes to the DT
bindings come prior to the driver changes that use the new bindings.
Other than the above and the few comments I had on patch3, this looks
good to me.
Kind regards
Uffe
> ---
> Changes in v6:
> - apply the comments to the first patch, add their trailers
> - Link to v5: https://lore.kernel.org/r/[email protected]
>
> Changes in v5:
> - pick the dependant patch: https://lore.kernel.org/all/[email protected]/
> to fix the bot build error.
> - edit the semantic meaning of hisilicon,sap-dll-reg property (Rob Herring)
> The suggestion is from the CRG driver side:
> https://lore.kernel.org/all/[email protected]/
> - Link to v4: https://lore.kernel.org/r/[email protected]
>
> Changes in v4:
> - rename dw_mmc-hi3798 back to hi3798cv200 - Suggested by Krzysztof Kozlowski.
> - add r-bs to patch 1 and 2 - Reviewed by Krzysztof Kozlowski.
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - dw_mmc-hi3798: fix bot error (Rob Herring)
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - dw_mmc-hi3798mv200: use dev_err_probe() helper - Suggested by Krzysztof Kozlowski.
> - dw_mmc-hi3798mv200: add missing err=0;
> - dw_mmc-hi3798c(m)v200: remove unused MODULE_ALIAS() - Suggested by Krzysztof Kozlowski.
> - binding: rename the binding, a lot of tweaks suggested by Krzysztof Kozlowski.
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Yang Xiwen (5):
> mmc: host: mmc_of_parse_clk_phase(): Pass struct device * instead of mmc_host *
> mmc: dw_mmc-hi3798cv200: remove MODULE_ALIAS()
> mmc: dw_mmc: add support for hi3798mv200
> dt-bindings: mmc: dw-mshc-hi3798cv200: convert to YAML
> dt-bindings: mmc: hisilicon,hi3798cv200-dw-mshc: add Hi3798MV200 binding
>
> .../bindings/mmc/hi3798cv200-dw-mshc.txt | 40 ----
> .../mmc/hisilicon,hi3798cv200-dw-mshc.yaml | 97 +++++++++
> drivers/mmc/core/host.c | 4 +-
> drivers/mmc/host/Kconfig | 9 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/dw_mmc-hi3798cv200.c | 1 -
> drivers/mmc/host/dw_mmc-hi3798mv200.c | 239 +++++++++++++++++++++
> drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
> include/linux/mmc/host.h | 2 +-
> 9 files changed, 349 insertions(+), 46 deletions(-)
> ---
> base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
> change-id: 20240121-b4-mmc-hi3798mv200-a5730edf122c
>
> Best regards,
> --
> Yang Xiwen <[email protected]>
>
On 2/27/2024 11:16 PM, Ulf Hansson wrote:
> On Wed, 21 Feb 2024 at 13:45, Yang Xiwen via B4 Relay
> <[email protected]> wrote:
>>
>> From: Yang Xiwen <[email protected]>
>>
>> Add support for Hi3798MV200 specific extension.
>>
>> Signed-off-by: Yang Xiwen <[email protected]>
>> ---
>> drivers/mmc/host/Kconfig | 9 ++
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/dw_mmc-hi3798mv200.c | 239 ++++++++++++++++++++++++++++++++++
>> 3 files changed, 249 insertions(+)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 81f2c4e05287..aebc587f77a7 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -798,6 +798,15 @@ config MMC_DW_HI3798CV200
>> Synopsys DesignWare Memory Card Interface driver. Select this option
>> for platforms based on HiSilicon Hi3798CV200 SoC.
>>
>> +config MMC_DW_HI3798MV200
>> + tristate "Hi3798MV200 specific extensions for Synopsys DW Memory Card Interface"
>> + depends on MMC_DW
>> + select MMC_DW_PLTFM
>> + help
>> + This selects support for HiSilicon Hi3798MV200 SoC specific extensions to the
>> + Synopsys DesignWare Memory Card Interface driver. Select this option
>> + for platforms based on HiSilicon Hi3798MV200 SoC.
>> +
>> config MMC_DW_K3
>> tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
>> depends on MMC_DW
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index d0be4465f3ec..f53f86d200ac 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -51,6 +51,7 @@ obj-$(CONFIG_MMC_DW_PLTFM) += dw_mmc-pltfm.o
>> obj-$(CONFIG_MMC_DW_BLUEFIELD) += dw_mmc-bluefield.o
>> obj-$(CONFIG_MMC_DW_EXYNOS) += dw_mmc-exynos.o
>> obj-$(CONFIG_MMC_DW_HI3798CV200) += dw_mmc-hi3798cv200.o
>> +obj-$(CONFIG_MMC_DW_HI3798MV200) += dw_mmc-hi3798mv200.o
>> obj-$(CONFIG_MMC_DW_K3) += dw_mmc-k3.o
>> obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o
>> obj-$(CONFIG_MMC_DW_ROCKCHIP) += dw_mmc-rockchip.o
>> diff --git a/drivers/mmc/host/dw_mmc-hi3798mv200.c b/drivers/mmc/host/dw_mmc-hi3798mv200.c
>> new file mode 100644
>> index 000000000000..73aaa21040ea
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-hi3798mv200.c
>> @@ -0,0 +1,239 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Modified from dw_mmc-hi3798cv200.c
>> + *
>> + * Copyright (c) 2024 Yang Xiwen <[email protected]>
>> + * Copyright (c) 2018 HiSilicon Technologies Co., Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "dw_mmc.h"
>> +#include "dw_mmc-pltfm.h"
>> +
>> +#define SDMMC_TUNING_CTRL 0x118
>> +#define SDMMC_TUNING_FIND_EDGE BIT(5)
>> +
>> +#define ALL_INT_CLR 0x1ffff
>> +
>> +/* DLL ctrl reg */
>> +#define SAP_DLL_CTRL_DLLMODE BIT(16)
>> +
>> +struct dw_mci_hi3798mv200_priv {
>> + struct clk *sample_clk;
>> + struct clk *drive_clk;
>> + struct regmap *crg_reg;
>> + u32 sap_dll_offset;
>> + struct mmc_clk_phase_map phase_map;
>> +};
>> +
>> +static void dw_mci_hi3798mv200_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> +{
>> + struct dw_mci_hi3798mv200_priv *priv = host->priv;
>> + struct mmc_clk_phase phase = priv->phase_map.phase[ios->timing];
>> + u32 val;
>> +
>> + val = mci_readl(host, ENABLE_SHIFT);
>> + if (ios->timing == MMC_TIMING_MMC_DDR52
>> + || ios->timing == MMC_TIMING_UHS_DDR50)
>> + val |= SDMMC_ENABLE_PHASE;
>> + else
>> + val &= ~SDMMC_ENABLE_PHASE;
>> + mci_writel(host, ENABLE_SHIFT, val);
>> +
>> + val = mci_readl(host, DDR_REG);
>> + if (ios->timing == MMC_TIMING_MMC_HS400)
>> + val |= SDMMC_DDR_HS400;
>> + else
>> + val &= ~SDMMC_DDR_HS400;
>> + mci_writel(host, DDR_REG, val);
>> +
>> + if (clk_set_rate(host->ciu_clk, ios->clock))
>> + dev_warn(host->dev, "Failed to set rate to %u\n", ios->clock);
>> + else
>> + // CLK_MUX_ROUND_NEAREST is enabled for this clock
>> + // The actual clock rate is not what we setted, but a rounded value
>> + // so we should get the rate once again
>
> Please use proper comments sections (/* .... */) and not "//".
>
>> + host->bus_hz = clk_get_rate(host->ciu_clk);
>> +
>> + if (phase.valid) {
>> + clk_set_phase(priv->drive_clk, phase.out_deg);
>> + clk_set_phase(priv->sample_clk, phase.in_deg);
>> + } else {
>> + dev_warn(host->dev,
>> + "The phase entry for timing mode %d is missing in device tree.\n",
>> + ios->timing);
>> + }
>> +}
>> +
>> +static inline int dw_mci_hi3798mv200_enable_tuning(struct dw_mci_slot *slot)
>> +{
>> + struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
>> +
>> + return regmap_clear_bits(priv->crg_reg, priv->sap_dll_offset, SAP_DLL_CTRL_DLLMODE);
>> +}
>> +
>> +static inline int dw_mci_hi3798mv200_disable_tuning(struct dw_mci_slot *slot)
>> +{
>> + struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
>> +
>> + return regmap_set_bits(priv->crg_reg, priv->sap_dll_offset, SAP_DLL_CTRL_DLLMODE);
>> +}
>> +
>> +static int dw_mci_hi3798mv200_execute_tuning_mix_mode(struct dw_mci_slot *slot,
>> + u32 opcode)
>> +{
>> + static const int degrees[] = { 0, 45, 90, 135, 180, 225, 270, 315 };
>> + struct dw_mci *host = slot->host;
>> + struct dw_mci_hi3798mv200_priv *priv = host->priv;
>> + int raise_point = -1, fall_point = -1;
>> + int err, prev_err = -1;
>> + int found = 0;
>> + int regval;
>> + int i;
>> + int ret;
>> +
>> + // enable tuning
>
> Looks like a redundant comment, please drop it.
>
>> + ret = dw_mci_hi3798mv200_enable_tuning(slot);
>> + if (ret < 0)
>> + return ret;
>
> A newline here would improve the readability I think.
>
>> + for (i = 0; i < ARRAY_SIZE(degrees); i++) {
>> + clk_set_phase(priv->sample_clk, degrees[i]);
>> + mci_writel(host, RINTSTS, ALL_INT_CLR);
>> +
>> + err = mmc_send_tuning(slot->mmc, opcode, NULL);
>> + if (!err) {
>> + regval = mci_readl(host, TUNING_CTRL);
>> + if (regval & SDMMC_TUNING_FIND_EDGE)
>> + err = 1;
>> + else
>> + found = 1;
>> + };
>> +
>> + if (i > 0) {
>> + if (err && !prev_err)
>> + fall_point = i - 1;
>> + if (!err && prev_err)
>> + raise_point = i;
>> + }
>> +
>> + if (raise_point != -1 && fall_point != -1)
>> + goto tuning_out;
>> +
>> + prev_err = err;
>> + err = 0;
>> + }
>> +
>> +tuning_out:
>> + ret = dw_mci_hi3798mv200_disable_tuning(slot);
>> + if (ret < 0)
>> + return ret;
>> + if (found) {
>> + if (raise_point == -1)
>> + raise_point = 0;
>> + if (fall_point == -1)
>> + fall_point = ARRAY_SIZE(degrees) - 1;
>> + if (fall_point < raise_point) {
>> + if ((raise_point + fall_point) >
>> + (ARRAY_SIZE(degrees) - 1))
>> + i = fall_point / 2;
>> + else
>> + i = (raise_point + ARRAY_SIZE(degrees) - 1) / 2;
>> + } else {
>> + i = (raise_point + fall_point) / 2;
>> + }
>> +
>> + // use the same phase table for both HS200 and HS400
>
> Don't use "//" for comments.
>
>> + priv->phase_map.phase[MMC_TIMING_MMC_HS200].in_deg = degrees[i];
>> + priv->phase_map.phase[MMC_TIMING_MMC_HS400].in_deg = degrees[i];
>> +
>> + clk_set_phase(priv->sample_clk, degrees[i]);
>> + dev_dbg(host->dev, "Tuning clk_sample[%d, %d], set[%d]\n",
>> + raise_point, fall_point, degrees[i]);
>> + err = 0;
>> + } else {
>> + dev_err(host->dev, "No valid clk_sample shift! use default\n");
>> + err = -EINVAL;
>> + }
>> +
>> + mci_writel(host, RINTSTS, ALL_INT_CLR);
>> + return err;
>
> The entire code in dw_mci_hi3798mv200_execute_tuning_mix_mode() looks
> rather messy to me. A lot of variables are being used, set and reset
> from everywhere.
>
> Would you mind having a closer look and try to improve it a bit, so it
> becomes easier to follow what is going on?
That might because the code here is modified from dw-mmc_hi3798cv200.c.
And That file is already a mess. I'll try to improve it soon.
>
>> +}
>> +
>> +static int dw_mci_hi3798mv200_init(struct dw_mci *host)
>> +{
>> + struct dw_mci_hi3798mv200_priv *priv;
>> + struct device_node *np = host->dev->of_node;
>> + int ret;
>> +
>> + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
>> +
>> + priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
>> + if (IS_ERR(priv->sample_clk))
>> + return dev_err_probe(host->dev, PTR_ERR(priv->sample_clk),
>> + "failed to get enabled ciu-sample clock\n");
>> +
>> + priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
>> + if (IS_ERR(priv->drive_clk))
>> + return dev_err_probe(host->dev, PTR_ERR(priv->drive_clk),
>> + "failed to get enabled ciu-drive clock\n");
>> +
>> + priv->crg_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
>> + if (IS_ERR(priv->crg_reg))
>> + return dev_err_probe(host->dev, PTR_ERR(priv->crg_reg),
>> + "failed to get CRG reg\n");
>> +
>> + ret = of_property_read_u32_index(np, "hisilicon,sap-dll-reg", 1, &priv->sap_dll_offset);
>> + if (ret)
>> + return dev_err_probe(host->dev, ret, "failed to get sample DLL register offset\n");
>> +
>> + host->priv = priv;
>> + return 0;
>> +}
>> +
>> +static const struct dw_mci_drv_data hi3798mv200_data = {
>> + .common_caps = MMC_CAP_CMD23,
>> + .init = dw_mci_hi3798mv200_init,
>> + .set_ios = dw_mci_hi3798mv200_set_ios,
>> + .execute_tuning = dw_mci_hi3798mv200_execute_tuning_mix_mode,
>> +};
>> +
>> +static const struct of_device_id dw_mci_hi3798mv200_match[] = {
>> + { .compatible = "hisilicon,hi3798mv200-dw-mshc" },
>> + {},
>> +};
>> +
>> +static int dw_mci_hi3798mv200_probe(struct platform_device *pdev)
>> +{
>> + return dw_mci_pltfm_register(pdev, &hi3798mv200_data);
>> +}
>> +
>> +static void dw_mci_hi3798mv200_remove(struct platform_device *pdev)
>> +{
>> + dw_mci_pltfm_remove(pdev);
>> +}
>> +
>> +MODULE_DEVICE_TABLE(of, dw_mci_hi3798mv200_match);
>> +static struct platform_driver dw_mci_hi3798mv200_driver = {
>> + .probe = dw_mci_hi3798mv200_probe,
>> + .remove_new = dw_mci_hi3798mv200_remove,
>> + .driver = {
>> + .name = "dwmmc_hi3798mv200",
>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> + .of_match_table = dw_mci_hi3798mv200_match,
>> + },
>> +};
>> +module_platform_driver(dw_mci_hi3798mv200_driver);
>> +
>> +MODULE_DESCRIPTION("HiSilicon Hi3798MV200 Specific DW-MSHC Driver Extension");
>> +MODULE_LICENSE("GPL");
>>
>
> Kind regards
> Uffe
--
Best regards,
Yang Xiwen