On 8/03/24 02:57, Judith Mendez wrote: > Currently the sdhci_am654 driver only supports one tuning > algorithm which should be used only when DLL is enabled. The > ITAPDLY is selected from the largest passing window and the > buffer is viewed as a circular buffer. > > The new algorithm should be used when the delay chain > is enabled. The ITAPDLY is selected from the largest passing > window and the buffer is not viewed as a circular buffer. > > This implementation is based off of the following paper: [1]. > > Also add support for multiple failing windows. > > [1] https://www.ti.com/lit/an/spract9/spract9.pdf > > Fixes: 13ebeae68ac9 ("mmc: sdhci_am654: Add support for software tuning") > Signed-off-by: Judith Mendez <jm@xxxxxx> One question further below, and one cosmetic change, but resolve those and you may add to all patches in this patch set: Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> > --- > Changelog: > v2->v3: > - Fix return for tuning algorithm > - Fix ITAPDLY_LAST_INDEX > - Use reverse fir tree order for variable declarations > - Remove unnecessary parenthesis > --- > drivers/mmc/host/sdhci_am654.c | 112 +++++++++++++++++++++++++++------ > 1 file changed, 92 insertions(+), 20 deletions(-) > > diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c > index d659c59422e1..d11b0d769e6c 100644 > --- a/drivers/mmc/host/sdhci_am654.c > +++ b/drivers/mmc/host/sdhci_am654.c > @@ -149,10 +149,17 @@ struct sdhci_am654_data { > int strb_sel; > u32 flags; > u32 quirks; > + bool dll_enable; > > #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0) > }; > > +struct window { > + u8 start; > + u8 end; > + u8 length; > +}; > + > struct sdhci_am654_driver_data { > const struct sdhci_pltfm_data *pdata; > u32 flags; > @@ -290,10 +297,12 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) > > regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); > > - if (timing > MMC_TIMING_UHS_SDR25 && clock >= CLOCK_TOO_SLOW_HZ) > + if (timing > MMC_TIMING_UHS_SDR25 && clock >= CLOCK_TOO_SLOW_HZ) { > sdhci_am654_setup_dll(host, clock); > - else > + sdhci_am654->dll_enable = true; > + } else { > sdhci_am654_setup_delay_chain(sdhci_am654, timing); V2 patch had here: sdhci_am654->dll_enable = false; Was its removal intended? > + } > > regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK, > sdhci_am654->clkbuf_sel); > @@ -408,39 +417,102 @@ static u32 sdhci_am654_cqhci_irq(struct sdhci_host *host, u32 intmask) > return 0; > } > > -#define ITAP_MAX 32 > +#define ITAPDLY_LENGTH 32 > +#define ITAPDLY_LAST_INDEX (ITAPDLY_LENGTH - 1) > + > +static u32 sdhci_am654_calculate_itap(struct sdhci_host *host, struct window > + *fail_window, u8 num_fails, bool circular_buffer) > +{ > + u8 itap = 0, start_fail = 0, end_fail = 0, pass_length = 0; > + u8 first_fail_start = 0, last_fail_end = 0; > + struct device *dev = mmc_dev(host->mmc); > + struct window pass_window = {0, 0, 0}; > + int prev_fail_end = -1; > + Unnecessary blank line > + u8 i; > + > + if (!num_fails) > + return ITAPDLY_LAST_INDEX >> 1; > + > + if (fail_window->length == ITAPDLY_LENGTH) { > + dev_err(dev, "No passing ITAPDLY, return 0\n"); > + return 0; > + } > + > + first_fail_start = fail_window->start; > + last_fail_end = fail_window[num_fails - 1].end; > + > + for (i = 0; i < num_fails; i++) { > + start_fail = fail_window[i].start; > + end_fail = fail_window[i].end; > + pass_length = start_fail - (prev_fail_end + 1); > + > + if (pass_length > pass_window.length) { > + pass_window.start = prev_fail_end + 1; > + pass_window.length = pass_length; > + } > + prev_fail_end = end_fail; > + } > + > + if (!circular_buffer) > + pass_length = ITAPDLY_LAST_INDEX - last_fail_end; > + else > + pass_length = ITAPDLY_LAST_INDEX - last_fail_end + first_fail_start; > + > + if (pass_length > pass_window.length) { > + pass_window.start = last_fail_end + 1; > + pass_window.length = pass_length; > + } > + > + if (!circular_buffer) > + itap = pass_window.start + (pass_window.length >> 1); > + else > + itap = (pass_window.start + (pass_window.length >> 1)) % ITAPDLY_LENGTH; > + > + return (itap > ITAPDLY_LAST_INDEX) ? ITAPDLY_LAST_INDEX >> 1 : itap; > +} > + > static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host, > u32 opcode) > { > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host); > - int cur_val, prev_val = 1, fail_len = 0, pass_window = 0, pass_len; > - u32 itap; > + struct window fail_window[ITAPDLY_LENGTH]; > + u8 curr_pass, itap; > + u8 fail_index = 0; > + u8 prev_pass = 1; > + > + memset(fail_window, 0, sizeof(fail_window)); > > /* Enable ITAPDLY */ > regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPDLYENA_MASK, > 1 << ITAPDLYENA_SHIFT); > > - for (itap = 0; itap < ITAP_MAX; itap++) { > + for (itap = 0; itap < ITAPDLY_LENGTH; itap++) { > sdhci_am654_write_itapdly(sdhci_am654, itap); > > - cur_val = !mmc_send_tuning(host->mmc, opcode, NULL); > - if (cur_val && !prev_val) > - pass_window = itap; > + curr_pass = !mmc_send_tuning(host->mmc, opcode, NULL); > > - if (!cur_val) > - fail_len++; > + if (!curr_pass && prev_pass) > + fail_window[fail_index].start = itap; > > - prev_val = cur_val; > + if (!curr_pass) { > + fail_window[fail_index].end = itap; > + fail_window[fail_index].length++; > + } > + > + if (curr_pass && !prev_pass) > + fail_index++; > + > + prev_pass = curr_pass; > } > - /* > - * Having determined the length of the failing window and start of > - * the passing window calculate the length of the passing window and > - * set the final value halfway through it considering the range as a > - * circular buffer > - */ > - pass_len = ITAP_MAX - fail_len; > - itap = (pass_window + (pass_len >> 1)) % ITAP_MAX; > + > + if (fail_window[fail_index].length != 0) > + fail_index++; > + > + itap = sdhci_am654_calculate_itap(host, fail_window, fail_index, > + sdhci_am654->dll_enable); > + > sdhci_am654_write_itapdly(sdhci_am654, itap); > > return 0;
