| Message ID | 20240131215044.3163469-5-jm@ti.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add tuning algorithm for delay chain | expand |
On 1/31/24 3:50 PM, Judith Mendez wrote: > Add ITAPDLYSEL to sdhci_j721e_4bit_set_clock function. > This allows to set the correct ITAPDLY for timings that > do not carry out tuning. > > Fixes: 1accbced1c32 ("mmc: sdhci_am654: Add Support for 4 bit IP on J721E") You are adding this as a new feature, and not having a feature doesn't mean the initial patch was broken. If this patch was backported to kernels only containing the above patch it would cause more issues, so no need for the fixes tags on this nor the last patch. Andrew > Signed-off-by: Judith Mendez <jm@ti.com> > --- > drivers/mmc/host/sdhci_am654.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c > index 5ac82bc70706..f5dc981c470d 100644 > --- a/drivers/mmc/host/sdhci_am654.c > +++ b/drivers/mmc/host/sdhci_am654.c > @@ -321,6 +321,7 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host, > unsigned char timing = host->mmc->ios.timing; > u32 otap_del_sel; > u32 itap_del_ena; > + u32 itap_del_sel; > u32 mask, val; > > /* Setup Output TAP delay */ > @@ -329,12 +330,17 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host, > mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; > val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT); > > + /* Setup Input TAP delay */ > itap_del_ena = sdhci_am654->itap_del_ena[timing]; > + itap_del_sel = sdhci_am654->itap_del_sel[timing]; > > - mask |= ITAPDLYENA_MASK; > - val |= (itap_del_ena << ITAPDLYENA_SHIFT); > + mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; > + val |= (itap_del_ena << ITAPDLYENA_SHIFT) | (itap_del_sel << ITAPDLYSEL_SHIFT); > > + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, > + 1 << ITAPCHGWIN_SHIFT); > regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0); > > regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK, > sdhci_am654->clkbuf_sel);
Hi Andrew, On 2/1/24 1:57 PM, Andrew Davis wrote: > On 1/31/24 3:50 PM, Judith Mendez wrote: >> Add ITAPDLYSEL to sdhci_j721e_4bit_set_clock function. >> This allows to set the correct ITAPDLY for timings that >> do not carry out tuning. >> >> Fixes: 1accbced1c32 ("mmc: sdhci_am654: Add Support for 4 bit IP on >> J721E") > > You are adding this as a new feature, and not having a feature doesn't mean > the initial patch was broken. If this patch was backported to kernels only > containing the above patch it would cause more issues, so no need for the > fixes tags on this nor the last patch. > Sure, will fix, thanks. >> Signed-off-by: Judith Mendez <jm@ti.com> >> --- >> drivers/mmc/host/sdhci_am654.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci_am654.c >> b/drivers/mmc/host/sdhci_am654.c >> index 5ac82bc70706..f5dc981c470d 100644 >> --- a/drivers/mmc/host/sdhci_am654.c >> +++ b/drivers/mmc/host/sdhci_am654.c >> @@ -321,6 +321,7 @@ static void sdhci_j721e_4bit_set_clock(struct >> sdhci_host *host, >> unsigned char timing = host->mmc->ios.timing; >> u32 otap_del_sel; >> u32 itap_del_ena; >> + u32 itap_del_sel; >> u32 mask, val; >> /* Setup Output TAP delay */ >> @@ -329,12 +330,17 @@ static void sdhci_j721e_4bit_set_clock(struct >> sdhci_host *host, >> mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; >> val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << >> OTAPDLYSEL_SHIFT); >> + /* Setup Input TAP delay */ >> itap_del_ena = sdhci_am654->itap_del_ena[timing]; >> + itap_del_sel = sdhci_am654->itap_del_sel[timing]; >> - mask |= ITAPDLYENA_MASK; >> - val |= (itap_del_ena << ITAPDLYENA_SHIFT); >> + mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; >> + val |= (itap_del_ena << ITAPDLYENA_SHIFT) | (itap_del_sel << >> ITAPDLYSEL_SHIFT); >> + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, >> + 1 << ITAPCHGWIN_SHIFT); >> regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); >> + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, >> 0); >> regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK, >> sdhci_am654->clkbuf_sel); ~ Judith
On 02/02/24 01:27, Andrew Davis wrote: > On 1/31/24 3:50 PM, Judith Mendez wrote: >> Add ITAPDLYSEL to sdhci_j721e_4bit_set_clock function. >> This allows to set the correct ITAPDLY for timings that >> do not carry out tuning. >> >> Fixes: 1accbced1c32 ("mmc: sdhci_am654: Add Support for 4 bit IP on >> J721E") > > You are adding this as a new feature, and not having a feature doesn't mean > the initial patch was broken. If this patch was backported to kernels only > containing the above patch it would cause more issues, so no need for the > fixes tags on this nor the last patch. > Not really a new features. Devices Datasheets have always been clear that static ITAPDLY needs to be configured when tuning isn't performed. Hence a bug as the initial patch (Fixes line) does enable such (affected) modes where tuning isn't performed but ITAPDLY isn't set either. > Andrew > >> Signed-off-by: Judith Mendez <jm@ti.com> >> --- >> drivers/mmc/host/sdhci_am654.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci_am654.c >> b/drivers/mmc/host/sdhci_am654.c >> index 5ac82bc70706..f5dc981c470d 100644 >> --- a/drivers/mmc/host/sdhci_am654.c >> +++ b/drivers/mmc/host/sdhci_am654.c >> @@ -321,6 +321,7 @@ static void sdhci_j721e_4bit_set_clock(struct >> sdhci_host *host, >> unsigned char timing = host->mmc->ios.timing; >> u32 otap_del_sel; >> u32 itap_del_ena; >> + u32 itap_del_sel; >> u32 mask, val; >> /* Setup Output TAP delay */ >> @@ -329,12 +330,17 @@ static void sdhci_j721e_4bit_set_clock(struct >> sdhci_host *host, >> mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; >> val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << >> OTAPDLYSEL_SHIFT); >> + /* Setup Input TAP delay */ >> itap_del_ena = sdhci_am654->itap_del_ena[timing]; >> + itap_del_sel = sdhci_am654->itap_del_sel[timing]; >> - mask |= ITAPDLYENA_MASK; >> - val |= (itap_del_ena << ITAPDLYENA_SHIFT); >> + mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; >> + val |= (itap_del_ena << ITAPDLYENA_SHIFT) | (itap_del_sel << >> ITAPDLYSEL_SHIFT); >> + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, >> + 1 << ITAPCHGWIN_SHIFT); >> regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); >> + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, >> 0); >> regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK, >> sdhci_am654->clkbuf_sel);
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c index 5ac82bc70706..f5dc981c470d 100644 --- a/drivers/mmc/host/sdhci_am654.c +++ b/drivers/mmc/host/sdhci_am654.c @@ -321,6 +321,7 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host, unsigned char timing = host->mmc->ios.timing; u32 otap_del_sel; u32 itap_del_ena; + u32 itap_del_sel; u32 mask, val; /* Setup Output TAP delay */ @@ -329,12 +330,17 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host, mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; val = (0x1 << OTAPDLYENA_SHIFT) | (otap_del_sel << OTAPDLYSEL_SHIFT); + /* Setup Input TAP delay */ itap_del_ena = sdhci_am654->itap_del_ena[timing]; + itap_del_sel = sdhci_am654->itap_del_sel[timing]; - mask |= ITAPDLYENA_MASK; - val |= (itap_del_ena << ITAPDLYENA_SHIFT); + mask |= ITAPDLYENA_MASK | ITAPDLYSEL_MASK; + val |= (itap_del_ena << ITAPDLYENA_SHIFT) | (itap_del_sel << ITAPDLYSEL_SHIFT); + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, + 1 << ITAPCHGWIN_SHIFT); regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, ITAPCHGWIN_MASK, 0); regmap_update_bits(sdhci_am654->base, PHY_CTRL5, CLKBUFSEL_MASK, sdhci_am654->clkbuf_sel);
Add ITAPDLYSEL to sdhci_j721e_4bit_set_clock function. This allows to set the correct ITAPDLY for timings that do not carry out tuning. Fixes: 1accbced1c32 ("mmc: sdhci_am654: Add Support for 4 bit IP on J721E") Signed-off-by: Judith Mendez <jm@ti.com> --- drivers/mmc/host/sdhci_am654.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)