| Message ID | 20240415212747.2678974-4-jm@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Fix MMC tuning algorithm | expand |
Hi, > -----Original Message----- > From: Judith Mendez <jm@ti.com> > Sent: Tuesday, April 16, 2024 6:28 AM > To: Peng Fan <peng.fan@nxp.com>; Jaehoon Chung <jh80.chung@samsung.com>; Tom Rini <trini@konsulko.com> > Cc: Nitin Yadav <n-yadav@ti.com>; Simon Glass <sjg@chromium.org>; u-boot@lists.denx.de > Subject: [PATCH 3/5] mmc: am654_sdhci: Add itap_del_ena[] to store itapdlyena bit > > Set itap_del_ena if ITAPDLY is found in DT or if the tuning > algorithm was executed and found the optimal ITAPDLY. Add the > functionality to save ITAPDLYENA that can be referenced later > by storing the bit in array itap_del_ena[]. > > Signed-off-by: Judith Mendez <jm@ti.com> > --- > drivers/mmc/am654_sdhci.c | 30 ++++++++++++++++++++---------- > 1 file changed, 20 insertions(+), 10 deletions(-) > > diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c > index 1dd032e1e36..38f1ad28ec4 100644 > --- a/drivers/mmc/am654_sdhci.c > +++ b/drivers/mmc/am654_sdhci.c > @@ -92,6 +92,7 @@ struct am654_sdhci_plat { > bool non_removable; > u32 otap_del_sel[MMC_MODES_END]; > u32 itap_del_sel[MMC_MODES_END]; > + u32 itap_del_ena[MMC_MODES_END]; > u32 trm_icp; > u32 drv_strength; > u32 strb_sel; > @@ -223,8 +224,10 @@ static int am654_sdhci_setup_dll(struct am654_sdhci_plat *plat, > } > > static void am654_sdhci_write_itapdly(struct am654_sdhci_plat *plat, > - u32 itapdly) > + u32 itapdly, u32 enable) > { > + regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK, > + enable << ITAPDLYENA_SHIFT); > /* Set ITAPCHGWIN before writing to ITAPDLY */ > regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, > 1 << ITAPCHGWIN_SHIFT); > @@ -242,7 +245,8 @@ static void am654_sdhci_setup_delay_chain(struct am654_sdhci_plat *plat, > mask = SELDLYTXCLK_MASK | SELDLYRXCLK_MASK; > regmap_update_bits(plat->base, PHY_CTRL5, mask, val); > > - am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode]); > + am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode], > + plat->itap_del_ena[mode]); > } > > static int am654_sdhci_set_ios_post(struct sdhci_host *host) > @@ -443,6 +447,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) > struct udevice *dev = mmc->dev; > struct am654_sdhci_plat *plat = dev_get_plat(dev); > struct window fail_window[ITAPDLY_LENGTH]; > + int mode = mmc->selected_mode; > u8 curr_pass, itap; > u8 fail_index = 0; > u8 prev_pass = 1; > @@ -450,11 +455,10 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) > memset(fail_window, 0, sizeof(fail_window)); > > /* Enable ITAPDLY */ > - regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK, > - 1 << ITAPDLYENA_SHIFT); > + plat->itap_del_ena[mode] = 0x1; 0x1 means "enable"? I want to use a macro with meaning. > > for (itap = 0; itap < ITAPDLY_LENGTH; itap++) { > - am654_sdhci_write_itapdly(plat, itap); > + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]); > > curr_pass = !mmc_send_tuning(mmc, opcode, NULL); > > @@ -478,7 +482,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) > itap = am654_sdhci_calculate_itap(dev, fail_window, fail_index, > plat->dll_enable); > > - am654_sdhci_write_itapdly(plat, itap); > + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]); > > return 0; > } > @@ -515,6 +519,7 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host) > struct am654_sdhci_plat *plat = dev_get_plat(dev); > int mode = host->mmc->selected_mode; > u32 otap_del_sel; > + u32 itap_del_ena; > u32 itap_del_sel; > u32 mask, val; > > @@ -524,10 +529,11 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host) > val = (1 << OTAPDLYENA_SHIFT) | > (otap_del_sel << OTAPDLYSEL_SHIFT); > > + itap_del_ena = plat->itap_del_ena[mode]; > itap_del_sel = plat->itap_del_sel[mode]; > > mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; > - val = (1 << ITAPDLYENA_SHIFT) | > + val = (itap_del_ena << ITAPDLYENA_SHIFT) | > (itap_del_sel << ITAPDLYSEL_SHIFT); > > regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, > @@ -599,9 +605,13 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev, > cfg->host_caps &= ~td[i].capability; > } > > - if (td[i].itap_binding) > - dev_read_u32(dev, td[i].itap_binding, > - &plat->itap_del_sel[i]); > + if (td[i].itap_binding) { > + ret = dev_read_u32(dev, td[i].itap_binding, > + &plat->itap_del_sel[i]); > + > + if (!ret) > + plat->itap_del_ena[i] = 0x1; Ditto. Best Regards, Jaehoon Chung > + } > } > > return 0; > -- > 2.43.2
On 4/17/24 6:34 AM, Jaehoon Chung wrote: > Hi, > >> -----Original Message----- >> From: Judith Mendez <jm@ti.com> >> Sent: Tuesday, April 16, 2024 6:28 AM >> To: Peng Fan <peng.fan@nxp.com>; Jaehoon Chung <jh80.chung@samsung.com>; Tom Rini <trini@konsulko.com> >> Cc: Nitin Yadav <n-yadav@ti.com>; Simon Glass <sjg@chromium.org>; u-boot@lists.denx.de >> Subject: [PATCH 3/5] mmc: am654_sdhci: Add itap_del_ena[] to store itapdlyena bit >> >> Set itap_del_ena if ITAPDLY is found in DT or if the tuning >> algorithm was executed and found the optimal ITAPDLY. Add the >> functionality to save ITAPDLYENA that can be referenced later >> by storing the bit in array itap_del_ena[]. >> >> Signed-off-by: Judith Mendez <jm@ti.com> >> --- >> drivers/mmc/am654_sdhci.c | 30 ++++++++++++++++++++---------- >> 1 file changed, 20 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c >> index 1dd032e1e36..38f1ad28ec4 100644 >> --- a/drivers/mmc/am654_sdhci.c >> +++ b/drivers/mmc/am654_sdhci.c >> @@ -92,6 +92,7 @@ struct am654_sdhci_plat { >> bool non_removable; >> u32 otap_del_sel[MMC_MODES_END]; >> u32 itap_del_sel[MMC_MODES_END]; >> + u32 itap_del_ena[MMC_MODES_END]; >> u32 trm_icp; >> u32 drv_strength; >> u32 strb_sel; >> @@ -223,8 +224,10 @@ static int am654_sdhci_setup_dll(struct am654_sdhci_plat *plat, >> } >> >> static void am654_sdhci_write_itapdly(struct am654_sdhci_plat *plat, >> - u32 itapdly) >> + u32 itapdly, u32 enable) >> { >> + regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK, >> + enable << ITAPDLYENA_SHIFT); >> /* Set ITAPCHGWIN before writing to ITAPDLY */ >> regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, >> 1 << ITAPCHGWIN_SHIFT); >> @@ -242,7 +245,8 @@ static void am654_sdhci_setup_delay_chain(struct am654_sdhci_plat *plat, >> mask = SELDLYTXCLK_MASK | SELDLYRXCLK_MASK; >> regmap_update_bits(plat->base, PHY_CTRL5, mask, val); >> >> - am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode]); >> + am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode], >> + plat->itap_del_ena[mode]); >> } >> >> static int am654_sdhci_set_ios_post(struct sdhci_host *host) >> @@ -443,6 +447,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) >> struct udevice *dev = mmc->dev; >> struct am654_sdhci_plat *plat = dev_get_plat(dev); >> struct window fail_window[ITAPDLY_LENGTH]; >> + int mode = mmc->selected_mode; >> u8 curr_pass, itap; >> u8 fail_index = 0; >> u8 prev_pass = 1; >> @@ -450,11 +455,10 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) >> memset(fail_window, 0, sizeof(fail_window)); >> >> /* Enable ITAPDLY */ >> - regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK, >> - 1 << ITAPDLYENA_SHIFT); >> + plat->itap_del_ena[mode] = 0x1; > > 0x1 means "enable"? I want to use a macro with meaning. Sure, can do for V2, thanks. ~ Judith > >> >> for (itap = 0; itap < ITAPDLY_LENGTH; itap++) { >> - am654_sdhci_write_itapdly(plat, itap); >> + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]); >> >> curr_pass = !mmc_send_tuning(mmc, opcode, NULL); >> >> @@ -478,7 +482,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) >> itap = am654_sdhci_calculate_itap(dev, fail_window, fail_index, >> plat->dll_enable); >> >> - am654_sdhci_write_itapdly(plat, itap); >> + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]); >> >> return 0; >> } >> @@ -515,6 +519,7 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host) >> struct am654_sdhci_plat *plat = dev_get_plat(dev); >> int mode = host->mmc->selected_mode; >> u32 otap_del_sel; >> + u32 itap_del_ena; >> u32 itap_del_sel; >> u32 mask, val; >> >> @@ -524,10 +529,11 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host) >> val = (1 << OTAPDLYENA_SHIFT) | >> (otap_del_sel << OTAPDLYSEL_SHIFT); >> >> + itap_del_ena = plat->itap_del_ena[mode]; >> itap_del_sel = plat->itap_del_sel[mode]; >> >> mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; >> - val = (1 << ITAPDLYENA_SHIFT) | >> + val = (itap_del_ena << ITAPDLYENA_SHIFT) | >> (itap_del_sel << ITAPDLYSEL_SHIFT); >> >> regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, >> @@ -599,9 +605,13 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev, >> cfg->host_caps &= ~td[i].capability; >> } >> >> - if (td[i].itap_binding) >> - dev_read_u32(dev, td[i].itap_binding, >> - &plat->itap_del_sel[i]); >> + if (td[i].itap_binding) { >> + ret = dev_read_u32(dev, td[i].itap_binding, >> + &plat->itap_del_sel[i]); >> + >> + if (!ret) >> + plat->itap_del_ena[i] = 0x1; > > Ditto. > > Best Regards, > Jaehoon Chung > >> + } >> } >> >> return 0; >> -- >> 2.43.2 > >
diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c index 1dd032e1e36..38f1ad28ec4 100644 --- a/drivers/mmc/am654_sdhci.c +++ b/drivers/mmc/am654_sdhci.c @@ -92,6 +92,7 @@ struct am654_sdhci_plat { bool non_removable; u32 otap_del_sel[MMC_MODES_END]; u32 itap_del_sel[MMC_MODES_END]; + u32 itap_del_ena[MMC_MODES_END]; u32 trm_icp; u32 drv_strength; u32 strb_sel; @@ -223,8 +224,10 @@ static int am654_sdhci_setup_dll(struct am654_sdhci_plat *plat, } static void am654_sdhci_write_itapdly(struct am654_sdhci_plat *plat, - u32 itapdly) + u32 itapdly, u32 enable) { + regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK, + enable << ITAPDLYENA_SHIFT); /* Set ITAPCHGWIN before writing to ITAPDLY */ regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, 1 << ITAPCHGWIN_SHIFT); @@ -242,7 +245,8 @@ static void am654_sdhci_setup_delay_chain(struct am654_sdhci_plat *plat, mask = SELDLYTXCLK_MASK | SELDLYRXCLK_MASK; regmap_update_bits(plat->base, PHY_CTRL5, mask, val); - am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode]); + am654_sdhci_write_itapdly(plat, plat->itap_del_sel[mode], + plat->itap_del_ena[mode]); } static int am654_sdhci_set_ios_post(struct sdhci_host *host) @@ -443,6 +447,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) struct udevice *dev = mmc->dev; struct am654_sdhci_plat *plat = dev_get_plat(dev); struct window fail_window[ITAPDLY_LENGTH]; + int mode = mmc->selected_mode; u8 curr_pass, itap; u8 fail_index = 0; u8 prev_pass = 1; @@ -450,11 +455,10 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) memset(fail_window, 0, sizeof(fail_window)); /* Enable ITAPDLY */ - regmap_update_bits(plat->base, PHY_CTRL4, ITAPDLYENA_MASK, - 1 << ITAPDLYENA_SHIFT); + plat->itap_del_ena[mode] = 0x1; for (itap = 0; itap < ITAPDLY_LENGTH; itap++) { - am654_sdhci_write_itapdly(plat, itap); + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]); curr_pass = !mmc_send_tuning(mmc, opcode, NULL); @@ -478,7 +482,7 @@ static int am654_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) itap = am654_sdhci_calculate_itap(dev, fail_window, fail_index, plat->dll_enable); - am654_sdhci_write_itapdly(plat, itap); + am654_sdhci_write_itapdly(plat, itap, plat->itap_del_ena[mode]); return 0; } @@ -515,6 +519,7 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host) struct am654_sdhci_plat *plat = dev_get_plat(dev); int mode = host->mmc->selected_mode; u32 otap_del_sel; + u32 itap_del_ena; u32 itap_del_sel; u32 mask, val; @@ -524,10 +529,11 @@ static int j721e_4bit_sdhci_set_ios_post(struct sdhci_host *host) val = (1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT); + itap_del_ena = plat->itap_del_ena[mode]; itap_del_sel = plat->itap_del_sel[mode]; mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; - val = (1 << ITAPDLYENA_SHIFT) | + val = (itap_del_ena << ITAPDLYENA_SHIFT) | (itap_del_sel << ITAPDLYSEL_SHIFT); regmap_update_bits(plat->base, PHY_CTRL4, ITAPCHGWIN_MASK, @@ -599,9 +605,13 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev, cfg->host_caps &= ~td[i].capability; } - if (td[i].itap_binding) - dev_read_u32(dev, td[i].itap_binding, - &plat->itap_del_sel[i]); + if (td[i].itap_binding) { + ret = dev_read_u32(dev, td[i].itap_binding, + &plat->itap_del_sel[i]); + + if (!ret) + plat->itap_del_ena[i] = 0x1; + } } return 0;
Set itap_del_ena if ITAPDLY is found in DT or if the tuning algorithm was executed and found the optimal ITAPDLY. Add the functionality to save ITAPDLYENA that can be referenced later by storing the bit in array itap_del_ena[]. Signed-off-by: Judith Mendez <jm@ti.com> --- drivers/mmc/am654_sdhci.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)