[PATCH v2 0/3] mmc: hi6220-dwmmc: handle resets and clocks
Also allow setup fifoth_val in driver
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- Changes in v2: - dw-mmc: proceed if data busy found - hi6220-dw-mmc: add fifoth_val setup, separate hi3798mv200 data - Link to v1: https://lore.kernel.org/r/20240119-mmc-v1-1-aee6b2cf6902@outlook.com
--- Yang Xiwen (3): mmc: hi6220-dwmmc: handle clocks and resets if CONFIG_CLK and CONFIG_DM_RESET enabled mmc: dw_mmc: Don't return error if data busy timeout mmc: hi6220_dw_mmc: add fifoth_val to private data and set it in .probe
drivers/mmc/dw_mmc.c | 4 +-- drivers/mmc/hi6220_dw_mmc.c | 72 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 4 deletions(-) --- base-commit: f7cca7ccc5117eaafcc2bde91ad1bed6fee7cfc3 change-id: 20240119-mmc-9cf7f3455cb4
Best regards,
From: Yang Xiwen forbidden405@outlook.com
This can avoid hardcoding a clock rate in driver. Also can enable the clocks and deassert the resets if the pre-bootloader does not do this for us.
Currently only enabled for Hi3798MV200.
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- drivers/mmc/hi6220_dw_mmc.c | 61 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index 71962cd47e..a4b8072976 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -5,15 +5,24 @@ */
#include <common.h> +#include <clk.h> #include <dm.h> #include <dwmmc.h> #include <errno.h> #include <fdtdec.h> #include <malloc.h> +#include <reset.h> #include <asm/global_data.h> +#include <dm/device_compat.h>
DECLARE_GLOBAL_DATA_PTR;
+enum hi6220_dwmmc_clk_type { + HI6220_DWMMC_CLK_BIU, + HI6220_DWMMC_CLK_CIU, + HI6220_DWMMC_CLK_CNT, +}; + struct hi6220_dwmmc_plat { struct mmc_config cfg; struct mmc mmc; @@ -21,6 +30,8 @@ struct hi6220_dwmmc_plat {
struct hi6220_dwmmc_priv_data { struct dwmci_host host; + struct clk *clks[HI6220_DWMMC_CLK_CNT]; + struct reset_ctl_bulk rsts; };
struct hisi_mmc_data { @@ -32,7 +43,29 @@ static int hi6220_dwmmc_of_to_plat(struct udevice *dev) { struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host; + int ret;
+ if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) { + priv->clks[HI6220_DWMMC_CLK_BIU] = devm_clk_get(dev, "biu"); + if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_BIU])) { + ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_BIU]); + dev_err(dev, "Failed to get BIU clock(ret = %d).\n", ret); + return log_msg_ret("clk", ret); + } + + priv->clks[HI6220_DWMMC_CLK_CIU] = devm_clk_get(dev, "ciu"); + if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_CIU])) { + ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_CIU]); + dev_err(dev, "Failed to get CIU clock(ret = %d).\n", ret); + return log_msg_ret("clk", ret); + } + + ret = reset_get_bulk(dev, &priv->rsts); + if (ret) { + dev_err(dev, "Failed to get resets(ret = %d)", ret); + return log_msg_ret("rst", ret); + } + } host->name = dev->name; host->ioaddr = dev_read_addr_ptr(dev); host->buswidth = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), @@ -56,11 +89,37 @@ static int hi6220_dwmmc_probe(struct udevice *dev) struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host; struct hisi_mmc_data *mmc_data; + int ret;
mmc_data = (struct hisi_mmc_data *)dev_get_driver_data(dev);
- /* Use default bus speed due to absence of clk driver */ host->bus_hz = mmc_data->clock; + if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) { + ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_BIU]); + if (ret) { + dev_err(dev, "Failed to enable biu clock(ret = %d).\n", ret); + return log_msg_ret("clk", ret); + } + + ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_CIU]); + if (ret) { + dev_err(dev, "Failed to enable ciu clock(ret = %d).\n", ret); + return log_msg_ret("clk", ret); + } + + ret = reset_deassert_bulk(&priv->rsts); + if (ret) { + dev_err(dev, "Failed to deassert resets(ret = %d).\n", ret); + return log_msg_ret("rst", ret); + } + + host->bus_hz = clk_get_rate(priv->clks[HI6220_DWMMC_CLK_CIU]); + if (host->bus_hz <= 0) { + dev_err(dev, "Failed to get ciu clock rate(ret = %d).\n", ret); + return log_msg_ret("clk", ret); + } + } + dev_dbg(dev, "bus clock rate: %d.\n", host->bus_hz);
dwmci_setup_cfg(&plat->cfg, host, host->bus_hz, 400000); host->mmc = &plat->mmc;
Hi,
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
This can avoid hardcoding a clock rate in driver. Also can enable the clocks and deassert the resets if the pre-bootloader does not do this for us.
Currently only enabled for Hi3798MV200.
Signed-off-by: Yang Xiwen forbidden405@outlook.com
drivers/mmc/hi6220_dw_mmc.c | 61 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index 71962cd47e..a4b8072976 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -5,15 +5,24 @@ */
#include <common.h> +#include <clk.h> #include <dm.h> #include <dwmmc.h> #include <errno.h> #include <fdtdec.h> #include <malloc.h> +#include <reset.h> #include <asm/global_data.h> +#include <dm/device_compat.h>
DECLARE_GLOBAL_DATA_PTR;
+enum hi6220_dwmmc_clk_type {
- HI6220_DWMMC_CLK_BIU,
- HI6220_DWMMC_CLK_CIU,
- HI6220_DWMMC_CLK_CNT,
+};
struct hi6220_dwmmc_plat { struct mmc_config cfg; struct mmc mmc; @@ -21,6 +30,8 @@ struct hi6220_dwmmc_plat {
struct hi6220_dwmmc_priv_data { struct dwmci_host host;
- struct clk *clks[HI6220_DWMMC_CLK_CNT];
- struct reset_ctl_bulk rsts;
};
struct hisi_mmc_data { @@ -32,7 +43,29 @@ static int hi6220_dwmmc_of_to_plat(struct udevice *dev) { struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host;
- int ret;
If CONFIG_CLK and DM_RESET aren't enabled, this value is a dead code. It also needs to initialize.
- if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) {
priv->clks[HI6220_DWMMC_CLK_BIU] = devm_clk_get(dev, "biu");if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_BIU])) {ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_BIU]);dev_err(dev, "Failed to get BIU clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}priv->clks[HI6220_DWMMC_CLK_CIU] = devm_clk_get(dev, "ciu");if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_CIU])) {ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_CIU]);dev_err(dev, "Failed to get CIU clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = reset_get_bulk(dev, &priv->rsts);if (ret) {dev_err(dev, "Failed to get resets(ret = %d)", ret);return log_msg_ret("rst", ret);}- } host->name = dev->name; host->ioaddr = dev_read_addr_ptr(dev); host->buswidth = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
@@ -56,11 +89,37 @@ static int hi6220_dwmmc_probe(struct udevice *dev) struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host; struct hisi_mmc_data *mmc_data;
- int ret;
Ditto.
Best Regards, Jaehoon Chung
mmc_data = (struct hisi_mmc_data *)dev_get_driver_data(dev);
- /* Use default bus speed due to absence of clk driver */ host->bus_hz = mmc_data->clock;
if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) {
ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_BIU]);if (ret) {dev_err(dev, "Failed to enable biu clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_CIU]);if (ret) {dev_err(dev, "Failed to enable ciu clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = reset_deassert_bulk(&priv->rsts);if (ret) {dev_err(dev, "Failed to deassert resets(ret = %d).\n", ret);return log_msg_ret("rst", ret);}host->bus_hz = clk_get_rate(priv->clks[HI6220_DWMMC_CLK_CIU]);if (host->bus_hz <= 0) {dev_err(dev, "Failed to get ciu clock rate(ret = %d).\n", ret);return log_msg_ret("clk", ret);}}
dev_dbg(dev, "bus clock rate: %d.\n", host->bus_hz);
dwmci_setup_cfg(&plat->cfg, host, host->bus_hz, 400000); host->mmc = &plat->mmc;
On 4/3/2024 8:39 AM, Jaehoon Chung wrote:
Hi,
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
This can avoid hardcoding a clock rate in driver. Also can enable the clocks and deassert the resets if the pre-bootloader does not do this for us.
Currently only enabled for Hi3798MV200.
Signed-off-by: Yang Xiwen forbidden405@outlook.com
drivers/mmc/hi6220_dw_mmc.c | 61 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index 71962cd47e..a4b8072976 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -5,15 +5,24 @@ */
#include <common.h> +#include <clk.h> #include <dm.h> #include <dwmmc.h> #include <errno.h> #include <fdtdec.h> #include <malloc.h> +#include <reset.h> #include <asm/global_data.h> +#include <dm/device_compat.h>
DECLARE_GLOBAL_DATA_PTR;
+enum hi6220_dwmmc_clk_type {
- HI6220_DWMMC_CLK_BIU,
- HI6220_DWMMC_CLK_CIU,
- HI6220_DWMMC_CLK_CNT,
+};
- struct hi6220_dwmmc_plat { struct mmc_config cfg; struct mmc mmc;
@@ -21,6 +30,8 @@ struct hi6220_dwmmc_plat {
struct hi6220_dwmmc_priv_data { struct dwmci_host host;
struct clk *clks[HI6220_DWMMC_CLK_CNT];
struct reset_ctl_bulk rsts; };
struct hisi_mmc_data {
@@ -32,7 +43,29 @@ static int hi6220_dwmmc_of_to_plat(struct udevice *dev) { struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host;
- int ret;
If CONFIG_CLK and DM_RESET aren't enabled, this value is a dead code. It also needs to initialize.
I think a alternative solution is replacing the if stmt below with some `#ifdef`s just like some unittests code. So we can mask variable `ret' out if it's not used However, this seems not favored by checkpatch.pl.
- if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) {
priv->clks[HI6220_DWMMC_CLK_BIU] = devm_clk_get(dev, "biu");if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_BIU])) {ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_BIU]);dev_err(dev, "Failed to get BIU clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}priv->clks[HI6220_DWMMC_CLK_CIU] = devm_clk_get(dev, "ciu");if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_CIU])) {ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_CIU]);dev_err(dev, "Failed to get CIU clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = reset_get_bulk(dev, &priv->rsts);if (ret) {dev_err(dev, "Failed to get resets(ret = %d)", ret);return log_msg_ret("rst", ret);}- } host->name = dev->name; host->ioaddr = dev_read_addr_ptr(dev); host->buswidth = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
@@ -56,11 +89,37 @@ static int hi6220_dwmmc_probe(struct udevice *dev) struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host; struct hisi_mmc_data *mmc_data;
- int ret;
Ditto.
Best Regards, Jaehoon Chung
mmc_data = (struct hisi_mmc_data *)dev_get_driver_data(dev);
- /* Use default bus speed due to absence of clk driver */ host->bus_hz = mmc_data->clock;
if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) {
ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_BIU]);if (ret) {dev_err(dev, "Failed to enable biu clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_CIU]);if (ret) {dev_err(dev, "Failed to enable ciu clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = reset_deassert_bulk(&priv->rsts);if (ret) {dev_err(dev, "Failed to deassert resets(ret = %d).\n", ret);return log_msg_ret("rst", ret);}host->bus_hz = clk_get_rate(priv->clks[HI6220_DWMMC_CLK_CIU]);if (host->bus_hz <= 0) {dev_err(dev, "Failed to get ciu clock rate(ret = %d).\n", ret);return log_msg_ret("clk", ret);}}
dev_dbg(dev, "bus clock rate: %d.\n", host->bus_hz);
dwmci_setup_cfg(&plat->cfg, host, host->bus_hz, 400000); host->mmc = &plat->mmc;
Hi,
-----Original Message----- From: Yang Xiwen forbidden405@outlook.com Sent: Wednesday, April 3, 2024 10:16 AM To: Jaehoon Chung jh80.chung@samsung.com; Peng Fan peng.fan@nxp.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH v2 1/3] mmc: hi6220-dwmmc: handle clocks and resets if CONFIG_CLK and CONFIG_DM_RESET enabled
On 4/3/2024 8:39 AM, Jaehoon Chung wrote:
Hi,
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
This can avoid hardcoding a clock rate in driver. Also can enable the clocks and deassert the resets if the pre-bootloader does not do this for us.
Currently only enabled for Hi3798MV200.
Signed-off-by: Yang Xiwen forbidden405@outlook.com
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/hi6220_dw_mmc.c | 61 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index 71962cd47e..a4b8072976 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -5,15 +5,24 @@ */
#include <common.h> +#include <clk.h> #include <dm.h> #include <dwmmc.h> #include <errno.h> #include <fdtdec.h> #include <malloc.h> +#include <reset.h> #include <asm/global_data.h> +#include <dm/device_compat.h>
DECLARE_GLOBAL_DATA_PTR;
+enum hi6220_dwmmc_clk_type {
- HI6220_DWMMC_CLK_BIU,
- HI6220_DWMMC_CLK_CIU,
- HI6220_DWMMC_CLK_CNT,
+};
- struct hi6220_dwmmc_plat { struct mmc_config cfg; struct mmc mmc;
@@ -21,6 +30,8 @@ struct hi6220_dwmmc_plat {
struct hi6220_dwmmc_priv_data { struct dwmci_host host;
struct clk *clks[HI6220_DWMMC_CLK_CNT];
struct reset_ctl_bulk rsts; };
struct hisi_mmc_data {
@@ -32,7 +43,29 @@ static int hi6220_dwmmc_of_to_plat(struct udevice *dev) { struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host;
- int ret;
If CONFIG_CLK and DM_RESET aren't enabled, this value is a dead code. It also needs to initialize.
I think a alternative solution is replacing the if stmt below with some `#ifdef`s just like some unittests code. So we can mask variable `ret' out if it's not used However, this seems not favored by checkpatch.pl.
It's not a critical thing. If possible to change more generic, I will change them. Thanks!
Best Regards, Jaehoon Chung
- if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) {
priv->clks[HI6220_DWMMC_CLK_BIU] = devm_clk_get(dev, "biu");if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_BIU])) {ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_BIU]);dev_err(dev, "Failed to get BIU clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}priv->clks[HI6220_DWMMC_CLK_CIU] = devm_clk_get(dev, "ciu");if (IS_ERR(priv->clks[HI6220_DWMMC_CLK_CIU])) {ret = PTR_ERR(priv->clks[HI6220_DWMMC_CLK_CIU]);dev_err(dev, "Failed to get CIU clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = reset_get_bulk(dev, &priv->rsts);if (ret) {dev_err(dev, "Failed to get resets(ret = %d)", ret);return log_msg_ret("rst", ret);}- } host->name = dev->name; host->ioaddr = dev_read_addr_ptr(dev); host->buswidth = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
@@ -56,11 +89,37 @@ static int hi6220_dwmmc_probe(struct udevice *dev) struct hi6220_dwmmc_priv_data *priv = dev_get_priv(dev); struct dwmci_host *host = &priv->host; struct hisi_mmc_data *mmc_data;
- int ret;
Ditto.
Best Regards, Jaehoon Chung
mmc_data = (struct hisi_mmc_data *)dev_get_driver_data(dev);
- /* Use default bus speed due to absence of clk driver */ host->bus_hz = mmc_data->clock;
if (CONFIG_IS_ENABLED(CLK) && CONFIG_IS_ENABLED(DM_RESET)) {
ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_BIU]);if (ret) {dev_err(dev, "Failed to enable biu clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = clk_prepare_enable(priv->clks[HI6220_DWMMC_CLK_CIU]);if (ret) {dev_err(dev, "Failed to enable ciu clock(ret = %d).\n", ret);return log_msg_ret("clk", ret);}ret = reset_deassert_bulk(&priv->rsts);if (ret) {dev_err(dev, "Failed to deassert resets(ret = %d).\n", ret);return log_msg_ret("rst", ret);}host->bus_hz = clk_get_rate(priv->clks[HI6220_DWMMC_CLK_CIU]);if (host->bus_hz <= 0) {dev_err(dev, "Failed to get ciu clock rate(ret = %d).\n", ret);return log_msg_ret("clk", ret);}}
dev_dbg(dev, "bus clock rate: %d.\n", host->bus_hz);
dwmci_setup_cfg(&plat->cfg, host, host->bus_hz, 400000); host->mmc = &plat->mmc;
-- Regards, Yang Xiwen
From: Yang Xiwen forbidden405@outlook.com
As described in [1], some poor hardware or cards would fail to release the bus and keep driving data lines low. Ignore it and send the next cmd directly seems okay for most cases.
[1]: https://patchwork.kernel.org/project/linux-mmc/patch/1424458179-5456-1-git-s...
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- drivers/mmc/dw_mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 400066fa99..e103664145 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -262,8 +262,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
while (dwmci_readl(host, DWMCI_STATUS) & DWMCI_BUSY) { if (get_timer(start) > timeout) { - debug("%s: Timeout on data busy\n", __func__); - return -ETIMEDOUT; + debug("%s: Timeout on data busy, continue anyway\n", __func__); + break; } }
Hi,
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
As described in [1], some poor hardware or cards would fail to release the bus and keep driving data lines low. Ignore it and send the next cmd directly seems okay for most cases.
This patch seems to be same with previous patch, right?
Best Regards, Jaehoon Chung
Signed-off-by: Yang Xiwen forbidden405@outlook.com
drivers/mmc/dw_mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 400066fa99..e103664145 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -262,8 +262,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
while (dwmci_readl(host, DWMCI_STATUS) & DWMCI_BUSY) { if (get_timer(start) > timeout) {
debug("%s: Timeout on data busy\n", __func__);return -ETIMEDOUT;
debug("%s: Timeout on data busy, continue anyway\n", __func__);break;
On 4/3/2024 8:41 AM, Jaehoon Chung wrote:
Hi,
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
As described in [1], some poor hardware or cards would fail to release the bus and keep driving data lines low. Ignore it and send the next cmd directly seems okay for most cases.
This patch seems to be same with previous patch, right?
From my observation, this patch does fix some weird problems and is mostly okay for other dwmmc users. I can't say it is very well tested because of I can't come up of other tests i can do except some `mmc read` and `mmc write`.
Best Regards, Jaehoon Chung
Signed-off-by: Yang Xiwen forbidden405@outlook.com
drivers/mmc/dw_mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 400066fa99..e103664145 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -262,8 +262,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
while (dwmci_readl(host, DWMCI_STATUS) & DWMCI_BUSY) { if (get_timer(start) > timeout) {
debug("%s: Timeout on data busy\n", __func__);return -ETIMEDOUT;
debug("%s: Timeout on data busy, continue anyway\n", __func__);break;
Hi,
-----Original Message----- From: Yang Xiwen forbidden405@outlook.com Sent: Wednesday, April 3, 2024 10:20 AM To: Jaehoon Chung jh80.chung@samsung.com; Peng Fan peng.fan@nxp.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH v2 2/3] mmc: dw_mmc: Don't return error if data busy timeout
On 4/3/2024 8:41 AM, Jaehoon Chung wrote:
Hi,
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
As described in [1], some poor hardware or cards would fail to release the bus and keep driving data lines low. Ignore it and send the next cmd directly seems okay for most cases.
This patch seems to be same with previous patch, right?
From my observation, this patch does fix some weird problems and is mostly okay for other dwmmc users. I can't say it is very well tested because of I can't come up of other tests i can do except some `mmc read` and `mmc write`.
Best Regards, Jaehoon Chung
dianders@chromium.org/
Signed-off-by: Yang Xiwen forbidden405@outlook.com
Tested-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
drivers/mmc/dw_mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 400066fa99..e103664145 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -262,8 +262,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
while (dwmci_readl(host, DWMCI_STATUS) & DWMCI_BUSY) { if (get_timer(start) > timeout) {
debug("%s: Timeout on data busy\n", __func__);return -ETIMEDOUT;
debug("%s: Timeout on data busy, continue anyway\n", __func__);break;-- Regards, Yang Xiwen
From: Yang Xiwen forbidden405@outlook.com
The value defaults to 0 and is ignored by dw_mmc code, so the other users are not affected.
Setting this explicitly fixes some weird reading error found on Hi3798MV200.
Fixes: 8a5dc8140e62 ("mmc: hi6220_dw_mmc: add compatible for HC2910 support")
Signed-off-by: Yang Xiwen forbidden405@outlook.com --- drivers/mmc/hi6220_dw_mmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index a4b8072976..dc0210402b 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -37,6 +37,7 @@ struct hi6220_dwmmc_priv_data { struct hisi_mmc_data { unsigned int clock; bool use_fifo; + u32 fifoth_val; };
static int hi6220_dwmmc_of_to_plat(struct udevice *dev) @@ -125,6 +126,7 @@ static int hi6220_dwmmc_probe(struct udevice *dev) host->mmc = &plat->mmc;
host->fifo_mode = mmc_data->use_fifo; + host->fifoth_val = mmc_data->fifoth_val; host->mmc->priv = &priv->host; upriv->mmc = host->mmc; host->mmc->dev = dev; @@ -154,13 +156,20 @@ static const struct hisi_mmc_data hi6220_mmc_data = { .use_fifo = false, };
+static const struct hisi_mmc_data hi3798mv2x_mmc_data = { + .clock = 50000000, + .use_fifo = false, + // FIFO depth is 256 + .fifoth_val = MSIZE(4) | RX_WMARK(0x7f) | TX_WMARK(0x80), +}; + static const struct udevice_id hi6220_dwmmc_ids[] = { { .compatible = "hisilicon,hi6220-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798cv200-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798mv200-dw-mshc", - .data = (ulong)&hi6220_mmc_data }, + .data = (ulong)&hi3798mv2x_mmc_data }, { .compatible = "hisilicon,hi3660-dw-mshc", .data = (ulong)&hi3660_mmc_data }, { }
Hi
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
The value defaults to 0 and is ignored by dw_mmc code, so the other users are not affected.
Setting this explicitly fixes some weird reading error found on Hi3798MV200.
Fixes: 8a5dc8140e62 ("mmc: hi6220_dw_mmc: add compatible for HC2910 support")
Signed-off-by: Yang Xiwen forbidden405@outlook.com
drivers/mmc/hi6220_dw_mmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index a4b8072976..dc0210402b 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -37,6 +37,7 @@ struct hi6220_dwmmc_priv_data { struct hisi_mmc_data { unsigned int clock; bool use_fifo;
- u32 fifoth_val;
};
static int hi6220_dwmmc_of_to_plat(struct udevice *dev) @@ -125,6 +126,7 @@ static int hi6220_dwmmc_probe(struct udevice *dev) host->mmc = &plat->mmc;
host->fifo_mode = mmc_data->use_fifo;
- host->fifoth_val = mmc_data->fifoth_val; host->mmc->priv = &priv->host; upriv->mmc = host->mmc; host->mmc->dev = dev;
@@ -154,13 +156,20 @@ static const struct hisi_mmc_data hi6220_mmc_data = { .use_fifo = false, };
+static const struct hisi_mmc_data hi3798mv2x_mmc_data = {
- .clock = 50000000,
- .use_fifo = false,
- // FIFO depth is 256
Removed unnecessary comment.
Best Regards, Jaehoon Chung
- .fifoth_val = MSIZE(4) | RX_WMARK(0x7f) | TX_WMARK(0x80),
+};
static const struct udevice_id hi6220_dwmmc_ids[] = { { .compatible = "hisilicon,hi6220-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798cv200-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798mv200-dw-mshc",
.data = (ulong)&hi6220_mmc_data },
.data = (ulong)&hi3798mv2x_mmc_data },
On 4/3/2024 8:43 AM, Jaehoon Chung wrote:
Hi
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
The value defaults to 0 and is ignored by dw_mmc code, so the other users are not affected.
Setting this explicitly fixes some weird reading error found on Hi3798MV200.
Fixes: 8a5dc8140e62 ("mmc: hi6220_dw_mmc: add compatible for HC2910 support")
Signed-off-by: Yang Xiwen forbidden405@outlook.com
drivers/mmc/hi6220_dw_mmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index a4b8072976..dc0210402b 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -37,6 +37,7 @@ struct hi6220_dwmmc_priv_data { struct hisi_mmc_data { unsigned int clock; bool use_fifo;
u32 fifoth_val; };
static int hi6220_dwmmc_of_to_plat(struct udevice *dev)
@@ -125,6 +126,7 @@ static int hi6220_dwmmc_probe(struct udevice *dev) host->mmc = &plat->mmc;
host->fifo_mode = mmc_data->use_fifo;
- host->fifoth_val = mmc_data->fifoth_val; host->mmc->priv = &priv->host; upriv->mmc = host->mmc; host->mmc->dev = dev;
@@ -154,13 +156,20 @@ static const struct hisi_mmc_data hi6220_mmc_data = { .use_fifo = false, };
+static const struct hisi_mmc_data hi3798mv2x_mmc_data = {
- .clock = 50000000,
- .use_fifo = false,
- // FIFO depth is 256
Removed unnecessary comment.
Will do. It seems that this should also apply to hi3798cv200-dw-mshc. tests are needed from cv200 users.
Best Regards, Jaehoon Chung
- .fifoth_val = MSIZE(4) | RX_WMARK(0x7f) | TX_WMARK(0x80),
+};
- static const struct udevice_id hi6220_dwmmc_ids[] = { { .compatible = "hisilicon,hi6220-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798cv200-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798mv200-dw-mshc",
.data = (ulong)&hi6220_mmc_data },
.data = (ulong)&hi3798mv2x_mmc_data },
Hi,
-----Original Message----- From: Yang Xiwen forbidden405@outlook.com Sent: Wednesday, April 3, 2024 10:22 AM To: Jaehoon Chung jh80.chung@samsung.com; Peng Fan peng.fan@nxp.com Cc: u-boot@lists.denx.de Subject: Re: [PATCH v2 3/3] mmc: hi6220_dw_mmc: add fifoth_val to private data and set it in .probe
On 4/3/2024 8:43 AM, Jaehoon Chung wrote:
Hi
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
From: Yang Xiwen forbidden405@outlook.com
The value defaults to 0 and is ignored by dw_mmc code, so the other users are not affected.
Setting this explicitly fixes some weird reading error found on Hi3798MV200.
Fixes: 8a5dc8140e62 ("mmc: hi6220_dw_mmc: add compatible for HC2910 support")
Signed-off-by: Yang Xiwen forbidden405@outlook.com
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
drivers/mmc/hi6220_dw_mmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c index a4b8072976..dc0210402b 100644 --- a/drivers/mmc/hi6220_dw_mmc.c +++ b/drivers/mmc/hi6220_dw_mmc.c @@ -37,6 +37,7 @@ struct hi6220_dwmmc_priv_data { struct hisi_mmc_data { unsigned int clock; bool use_fifo;
u32 fifoth_val; };
static int hi6220_dwmmc_of_to_plat(struct udevice *dev)
@@ -125,6 +126,7 @@ static int hi6220_dwmmc_probe(struct udevice *dev) host->mmc = &plat->mmc;
host->fifo_mode = mmc_data->use_fifo;
- host->fifoth_val = mmc_data->fifoth_val; host->mmc->priv = &priv->host; upriv->mmc = host->mmc; host->mmc->dev = dev;
@@ -154,13 +156,20 @@ static const struct hisi_mmc_data hi6220_mmc_data = { .use_fifo = false, };
+static const struct hisi_mmc_data hi3798mv2x_mmc_data = {
- .clock = 50000000,
- .use_fifo = false,
- // FIFO depth is 256
Removed unnecessary comment.
Will do. It seems that this should also apply to hi3798cv200-dw-mshc. tests are needed from cv200 users.
In future, It can be removed. Others looks good to me.
Best Regards, Jaehoon Chung
Best Regards, Jaehoon Chung
- .fifoth_val = MSIZE(4) | RX_WMARK(0x7f) | TX_WMARK(0x80),
+};
- static const struct udevice_id hi6220_dwmmc_ids[] = { { .compatible = "hisilicon,hi6220-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798cv200-dw-mshc", .data = (ulong)&hi6220_mmc_data }, { .compatible = "hisilicon,hi3798mv200-dw-mshc",
.data = (ulong)&hi6220_mmc_data },
.data = (ulong)&hi3798mv2x_mmc_data },-- Regards, Yang Xiwen
participants (3)
-
Jaehoon Chung -
Yang Xiwen -
Yang Xiwen via B4 Relay