Re: [PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values

2024-04-18 Thread Judith Mendez 

On 4/17/24 6:28 AM, Jaehoon Chung wrote:

Hi Judith,


-Original Message-
From: Judith Mendez 
Sent: Tuesday, April 16, 2024 6:28 AM
To: Peng Fan ; Jaehoon Chung ; Tom Rini 

Cc: Nitin Yadav ; Simon Glass ; 
u-boot@lists.denx.de
Subject: [PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values

From: Nitin Yadav 

U-Boot is failing to boot class U1 UHS SD cards due to incorrect
OTAP and ITAP delay select values. Update OTAP and ITAP delay select
values from DT.

Fixes: c7d106b4eb3 ("mmc: am654_sdhci: Update output tap delay writes")
Signed-off-by: Nitin Yadav 
Signed-off-by: Judith Mendez 
---
  drivers/mmc/am654_sdhci.c | 23 +++
  1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
index e5ad00e2531..1dd032e1e36 100644
--- a/drivers/mmc/am654_sdhci.c
+++ b/drivers/mmc/am654_sdhci.c
@@ -513,12 +513,27 @@ static int j721e_4bit_sdhci_set_ios_post(struct 
sdhci_host *host)
  {
struct udevice *dev = host->mmc->dev;
struct am654_sdhci_plat *plat = dev_get_plat(dev);
-   u32 otap_del_sel, mask, val;
+   int mode = host->mmc->selected_mode;
+   u32 otap_del_sel;
+   u32 itap_del_sel;
+   u32 mask, val;
+
+   otap_del_sel = plat->otap_del_sel[mode];

-   otap_del_sel = plat->otap_del_sel[host->mmc->selected_mode];
mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-   val = (1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
+   val = (1 << OTAPDLYENA_SHIFT) |
+ (otap_del_sel << OTAPDLYSEL_SHIFT);


Is there any reason to touch this? And I can't understood this, this val 
doesn’t use anywhere.
Because val is resetting the below. It seems same value, right?


This change is syncing with upstream kernel driver.

The second val assignment is supposed to be val |=,
will fix for v2.




+
+   itap_del_sel = plat->itap_del_sel[mode];
+
+   mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK;


Can it be set at above?

mask |= OTAPDLYENA_MASK | OTAPDLYSEL_MASK |
ITAPDLYENA_MASK | ITAPDLYSEL_MASK;


This is also syncing with upstream. I am hoping
to leave this as is.

~ Judith



Best Regards,
Jaehoon Chung




+   val = (1 << ITAPDLYENA_SHIFT) |
+ (itap_del_sel << ITAPDLYSEL_SHIFT);
+
+   regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK,
+  1 << ITAPCHGWIN_SHIFT);
regmap_update_bits(plat->base, PHY_CTRL4, mask, val);
+   regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);

regmap_update_bits(plat->base, PHY_CTRL5, CLKBUFSEL_MASK,
   plat->clkbuf_sel);
@@ -572,7 +587,7 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
 * Remove the corresponding capability if an otap-del-sel
 * value is not found
 */
-   for (i = MMC_HS; i <= MMC_HS_400; i++) {
+   for (i = MMC_LEGACY; i <= MMC_HS_400; i++) {
ret = dev_read_u32(dev, td[i].otap_binding,
   >otap_del_sel[i]);
if (ret) {
--
2.43.2

RE: [PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values

2024-04-17 Thread Jaehoon Chung 
Hi Judith,

> -Original Message-
> From: Judith Mendez 
> Sent: Tuesday, April 16, 2024 6:28 AM
> To: Peng Fan ; Jaehoon Chung ; Tom 
> Rini 
> Cc: Nitin Yadav ; Simon Glass ; 
> u-boot@lists.denx.de
> Subject: [PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values
> 
> From: Nitin Yadav 
> 
> U-Boot is failing to boot class U1 UHS SD cards due to incorrect
> OTAP and ITAP delay select values. Update OTAP and ITAP delay select
> values from DT.
> 
> Fixes: c7d106b4eb3 ("mmc: am654_sdhci: Update output tap delay writes")
> Signed-off-by: Nitin Yadav 
> Signed-off-by: Judith Mendez 
> ---
>  drivers/mmc/am654_sdhci.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index e5ad00e2531..1dd032e1e36 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -513,12 +513,27 @@ static int j721e_4bit_sdhci_set_ios_post(struct 
> sdhci_host *host)
>  {
>   struct udevice *dev = host->mmc->dev;
>   struct am654_sdhci_plat *plat = dev_get_plat(dev);
> - u32 otap_del_sel, mask, val;
> + int mode = host->mmc->selected_mode;
> + u32 otap_del_sel;
> + u32 itap_del_sel;
> + u32 mask, val;
> +
> + otap_del_sel = plat->otap_del_sel[mode];
> 
> - otap_del_sel = plat->otap_del_sel[host->mmc->selected_mode];
>   mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
> - val = (1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
> + val = (1 << OTAPDLYENA_SHIFT) |
> +   (otap_del_sel << OTAPDLYSEL_SHIFT);

Is there any reason to touch this? And I can't understood this, this val 
doesn’t use anywhere.
Because val is resetting the below. It seems same value, right?

> +
> + itap_del_sel = plat->itap_del_sel[mode];
> +
> + mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK;

Can it be set at above?

mask |= OTAPDLYENA_MASK | OTAPDLYSEL_MASK | 
ITAPDLYENA_MASK | ITAPDLYSEL_MASK;

Best Regards,
Jaehoon Chung



> + val = (1 << ITAPDLYENA_SHIFT) |
> +   (itap_del_sel << ITAPDLYSEL_SHIFT);
> +
> + regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK,
> +1 << ITAPCHGWIN_SHIFT);
>   regmap_update_bits(plat->base, PHY_CTRL4, mask, val);
> + regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
> 
>   regmap_update_bits(plat->base, PHY_CTRL5, CLKBUFSEL_MASK,
>  plat->clkbuf_sel);
> @@ -572,7 +587,7 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
>* Remove the corresponding capability if an otap-del-sel
>* value is not found
>*/
> - for (i = MMC_HS; i <= MMC_HS_400; i++) {
> + for (i = MMC_LEGACY; i <= MMC_HS_400; i++) {
>   ret = dev_read_u32(dev, td[i].otap_binding,
>  >otap_del_sel[i]);
>   if (ret) {
> --
> 2.43.2

[PATCH 2/5] mmc: am654_sdhci: Fix OTAP/ITAP delay values

2024-04-15 Thread Judith Mendez 
From: Nitin Yadav 

U-Boot is failing to boot class U1 UHS SD cards due to incorrect
OTAP and ITAP delay select values. Update OTAP and ITAP delay select
values from DT.

Fixes: c7d106b4eb3 ("mmc: am654_sdhci: Update output tap delay writes")
Signed-off-by: Nitin Yadav 
Signed-off-by: Judith Mendez 
---
 drivers/mmc/am654_sdhci.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
index e5ad00e2531..1dd032e1e36 100644
--- a/drivers/mmc/am654_sdhci.c
+++ b/drivers/mmc/am654_sdhci.c
@@ -513,12 +513,27 @@ static int j721e_4bit_sdhci_set_ios_post(struct 
sdhci_host *host)
 {
struct udevice *dev = host->mmc->dev;
struct am654_sdhci_plat *plat = dev_get_plat(dev);
-   u32 otap_del_sel, mask, val;
+   int mode = host->mmc->selected_mode;
+   u32 otap_del_sel;
+   u32 itap_del_sel;
+   u32 mask, val;
+
+   otap_del_sel = plat->otap_del_sel[mode];
 
-   otap_del_sel = plat->otap_del_sel[host->mmc->selected_mode];
mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK;
-   val = (1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT);
+   val = (1 << OTAPDLYENA_SHIFT) |
+ (otap_del_sel << OTAPDLYSEL_SHIFT);
+
+   itap_del_sel = plat->itap_del_sel[mode];
+
+   mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK;
+   val = (1 << ITAPDLYENA_SHIFT) |
+ (itap_del_sel << ITAPDLYSEL_SHIFT);
+
+   regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK,
+  1 << ITAPCHGWIN_SHIFT);
regmap_update_bits(plat->base, PHY_CTRL4, mask, val);
+   regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0);
 
regmap_update_bits(plat->base, PHY_CTRL5, CLKBUFSEL_MASK,
   plat->clkbuf_sel);
@@ -572,7 +587,7 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
 * Remove the corresponding capability if an otap-del-sel
 * value is not found
 */
-   for (i = MMC_HS; i <= MMC_HS_400; i++) {
+   for (i = MMC_LEGACY; i <= MMC_HS_400; i++) {
ret = dev_read_u32(dev, td[i].otap_binding,
   >otap_del_sel[i]);
if (ret) {
-- 
2.43.2