RE: [PATCH v2 3/3] mmc: hi6220_dw_mmc: add fifoth_val to private data and set it in .probe
Hi,
> -Original Message-
> From: Yang Xiwen
> Sent: Wednesday, April 3, 2024 10:22 AM
> To: Jaehoon Chung ; Peng Fan
> 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
> >>
> >> 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
Reviewed-by: Jaehoon Chung
> >> ---
> >> 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 = >mmc;
> >>
> >>host->fifo_mode = mmc_data->use_fifo;
> >> + host->fifoth_val = mmc_data->fifoth_val;
> >>host->mmc->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 = 5000,
> >> + .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)_mmc_data },
> >>{ .compatible = "hisilicon,hi3798cv200-dw-mshc",
> >> .data = (ulong)_mmc_data },
> >>{ .compatible = "hisilicon,hi3798mv200-dw-mshc",
> >> -.data = (ulong)_mmc_data },
> >> +.data = (ulong)_mmc_data },
> >>{ .compatible = "hisilicon,hi3660-dw-mshc",
> >> .data = (ulong)_mmc_data },
> >>{ }
>
>
> --
> Regards,
> Yang Xiwen
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
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
---
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 = >mmc;
host->fifo_mode = mmc_data->use_fifo;
+ host->fifoth_val = mmc_data->fifoth_val;
host->mmc->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 = 5000,
+ .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)_mmc_data },
{ .compatible = "hisilicon,hi3798cv200-dw-mshc",
.data = (ulong)_mmc_data },
{ .compatible = "hisilicon,hi3798mv200-dw-mshc",
- .data = (ulong)_mmc_data },
+ .data = (ulong)_mmc_data },
{ .compatible = "hisilicon,hi3660-dw-mshc",
.data = (ulong)_mmc_data },
{ }
--
Regards,
Yang Xiwen
Re: [PATCH v2 3/3] mmc: hi6220_dw_mmc: add fifoth_val to private data and set it in .probe
Hi
On 2/1/24 23:05, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen
>
> 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
> ---
> 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 = >mmc;
>
> host->fifo_mode = mmc_data->use_fifo;
> + host->fifoth_val = mmc_data->fifoth_val;
> host->mmc->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 = 5000,
> + .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)_mmc_data },
> { .compatible = "hisilicon,hi3798cv200-dw-mshc",
> .data = (ulong)_mmc_data },
> { .compatible = "hisilicon,hi3798mv200-dw-mshc",
> - .data = (ulong)_mmc_data },
> + .data = (ulong)_mmc_data },
> { .compatible = "hisilicon,hi3660-dw-mshc",
> .data = (ulong)_mmc_data },
> { }
[PATCH v2 3/3] mmc: hi6220_dw_mmc: add fifoth_val to private data and set it in .probe
From: Yang Xiwen
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
---
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 = >mmc;
host->fifo_mode = mmc_data->use_fifo;
+ host->fifoth_val = mmc_data->fifoth_val;
host->mmc->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 = 5000,
+ .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)_mmc_data },
{ .compatible = "hisilicon,hi3798cv200-dw-mshc",
.data = (ulong)_mmc_data },
{ .compatible = "hisilicon,hi3798mv200-dw-mshc",
- .data = (ulong)_mmc_data },
+ .data = (ulong)_mmc_data },
{ .compatible = "hisilicon,hi3660-dw-mshc",
.data = (ulong)_mmc_data },
{ }
--
2.43.0
