* [PATCH 0/2] Fix issues with phy configurations @ 2019-04-25 15:57 Faiz Abbas 2019-04-25 15:57 ` [PATCH 1/2] mmc: sdhci_am654: Fix minor " Faiz Abbas 2019-04-25 15:57 ` [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write Faiz Abbas 0 siblings, 2 replies; 8+ messages in thread From: Faiz Abbas @ 2019-04-25 15:57 UTC (permalink / raw) To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, faiz_abbas The following patches fix issues with phy configurations for sdhci_am654 driver. Faiz Abbas (2): mmc: sdhci_am654: Fix minor phy configurations mmc: sdhci_am654: Fix SLOTTYPE write drivers/mmc/host/sdhci_am654.c | 37 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) -- 2.19.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] mmc: sdhci_am654: Fix minor phy configurations 2019-04-25 15:57 [PATCH 0/2] Fix issues with phy configurations Faiz Abbas @ 2019-04-25 15:57 ` Faiz Abbas 2019-04-26 5:50 ` Adrian Hunter 2019-04-25 15:57 ` [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write Faiz Abbas 1 sibling, 1 reply; 8+ messages in thread From: Faiz Abbas @ 2019-04-25 15:57 UTC (permalink / raw) To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, faiz_abbas Fix the following minor things: 1. Line wrapping with the regmap_*() functions is way more conservative than required by the 80 character rule. Expand the function calls out to use less number of lines. 2. Add an error message if the DLL fails to lock. Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> --- drivers/mmc/host/sdhci_am654.c | 37 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c index eea183e90f1b..866a9082705f 100644 --- a/drivers/mmc/host/sdhci_am654.c +++ b/drivers/mmc/host/sdhci_am654.c @@ -88,8 +88,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) int ret; if (sdhci_am654->dll_on) { - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, - ENDLL_MASK, 0); + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0); sdhci_am654->dll_on = false; } @@ -101,8 +100,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; val = (1 << OTAPDLYENA_SHIFT) | (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT); - regmap_update_bits(sdhci_am654->base, PHY_CTRL4, - mask, val); + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); switch (clock) { case 200000000: sel50 = 0; @@ -120,8 +118,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) /* Configure PHY DLL frequency */ mask = SEL50_MASK | SEL100_MASK; val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT); - regmap_update_bits(sdhci_am654->base, PHY_CTRL5, - mask, val); + regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val); /* Configure DLL TRIM */ mask = DLL_TRIM_ICP_MASK; val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT; @@ -129,19 +126,21 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) /* Configure DLL driver strength */ mask |= DR_TY_MASK; val |= sdhci_am654->drv_strength << DR_TY_SHIFT; - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, - mask, val); + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val); /* Enable DLL */ - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, - ENDLL_MASK, 0x1 << ENDLL_SHIFT); + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, + 0x1 << ENDLL_SHIFT); /* * Poll for DLL ready. Use a one second timeout. * Works in all experiments done so far */ - ret = regmap_read_poll_timeout(sdhci_am654->base, - PHY_STAT1, val, - val & DLLRDY_MASK, - 1000, 1000000); + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1, + val, val & DLLRDY_MASK, 1000, + 1000000); + if (ret) { + dev_err(mmc_dev(host->mmc), "DLL failed to relock\n"); + return; + } sdhci_am654->dll_on = true; } @@ -186,8 +185,7 @@ static int sdhci_am654_init(struct sdhci_host *host) /* Reset OTAP to default value */ mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; - regmap_update_bits(sdhci_am654->base, PHY_CTRL4, - mask, 0x0); + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, 0x0); regmap_read(sdhci_am654->base, PHY_STAT1, &val); if (~val & CALDONE_MASK) { @@ -201,15 +199,14 @@ static int sdhci_am654_init(struct sdhci_host *host) } /* Enable pins by setting IO mux to 0 */ - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, - IOMUX_ENABLE_MASK, 0); + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, IOMUX_ENABLE_MASK, 0); /* Set slot type based on SD or eMMC */ if (host->mmc->caps & MMC_CAP_NONREMOVABLE) ctl_cfg_2 = SLOTTYPE_EMBEDDED; - regmap_update_bits(sdhci_am654->base, CTL_CFG_2, - ctl_cfg_2, SLOTTYPE_MASK); + regmap_update_bits(sdhci_am654->base, CTL_CFG_2, ctl_cfg_2, + SLOTTYPE_MASK); return sdhci_add_host(host); } -- 2.19.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci_am654: Fix minor phy configurations 2019-04-25 15:57 ` [PATCH 1/2] mmc: sdhci_am654: Fix minor " Faiz Abbas @ 2019-04-26 5:50 ` Adrian Hunter 2019-05-07 16:29 ` Faiz Abbas 0 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2019-04-26 5:50 UTC (permalink / raw) To: Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson On 25/04/19 6:57 PM, Faiz Abbas wrote: > Fix the following minor things: > > 1. Line wrapping with the regmap_*() functions is way more conservative > than required by the 80 character rule. Expand the function calls out to > use less number of lines. > > 2. Add an error message if the DLL fails to lock. Please make the white space changes a separate patch. Also I would prefer not to use "fix" in the subject unless the patch fixes driver behaviour. > > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > --- > drivers/mmc/host/sdhci_am654.c | 37 ++++++++++++++++------------------ > 1 file changed, 17 insertions(+), 20 deletions(-) > > diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c > index eea183e90f1b..866a9082705f 100644 > --- a/drivers/mmc/host/sdhci_am654.c > +++ b/drivers/mmc/host/sdhci_am654.c > @@ -88,8 +88,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) > int ret; > > if (sdhci_am654->dll_on) { > - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, > - ENDLL_MASK, 0); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, 0); > > sdhci_am654->dll_on = false; > } > @@ -101,8 +100,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) > mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; > val = (1 << OTAPDLYENA_SHIFT) | > (sdhci_am654->otap_del_sel << OTAPDLYSEL_SHIFT); > - regmap_update_bits(sdhci_am654->base, PHY_CTRL4, > - mask, val); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, val); > switch (clock) { > case 200000000: > sel50 = 0; > @@ -120,8 +118,7 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) > /* Configure PHY DLL frequency */ > mask = SEL50_MASK | SEL100_MASK; > val = (sel50 << SEL50_SHIFT) | (sel100 << SEL100_SHIFT); > - regmap_update_bits(sdhci_am654->base, PHY_CTRL5, > - mask, val); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL5, mask, val); > /* Configure DLL TRIM */ > mask = DLL_TRIM_ICP_MASK; > val = sdhci_am654->trm_icp << DLL_TRIM_ICP_SHIFT; > @@ -129,19 +126,21 @@ static void sdhci_am654_set_clock(struct sdhci_host *host, unsigned int clock) > /* Configure DLL driver strength */ > mask |= DR_TY_MASK; > val |= sdhci_am654->drv_strength << DR_TY_SHIFT; > - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, > - mask, val); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, mask, val); > /* Enable DLL */ > - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, > - ENDLL_MASK, 0x1 << ENDLL_SHIFT); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, ENDLL_MASK, > + 0x1 << ENDLL_SHIFT); > /* > * Poll for DLL ready. Use a one second timeout. > * Works in all experiments done so far > */ > - ret = regmap_read_poll_timeout(sdhci_am654->base, > - PHY_STAT1, val, > - val & DLLRDY_MASK, > - 1000, 1000000); > + ret = regmap_read_poll_timeout(sdhci_am654->base, PHY_STAT1, > + val, val & DLLRDY_MASK, 1000, > + 1000000); > + if (ret) { > + dev_err(mmc_dev(host->mmc), "DLL failed to relock\n"); > + return; > + } > > sdhci_am654->dll_on = true; > } > @@ -186,8 +185,7 @@ static int sdhci_am654_init(struct sdhci_host *host) > > /* Reset OTAP to default value */ > mask = OTAPDLYENA_MASK | OTAPDLYSEL_MASK; > - regmap_update_bits(sdhci_am654->base, PHY_CTRL4, > - mask, 0x0); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL4, mask, 0x0); > > regmap_read(sdhci_am654->base, PHY_STAT1, &val); > if (~val & CALDONE_MASK) { > @@ -201,15 +199,14 @@ static int sdhci_am654_init(struct sdhci_host *host) > } > > /* Enable pins by setting IO mux to 0 */ > - regmap_update_bits(sdhci_am654->base, PHY_CTRL1, > - IOMUX_ENABLE_MASK, 0); > + regmap_update_bits(sdhci_am654->base, PHY_CTRL1, IOMUX_ENABLE_MASK, 0); > > /* Set slot type based on SD or eMMC */ > if (host->mmc->caps & MMC_CAP_NONREMOVABLE) > ctl_cfg_2 = SLOTTYPE_EMBEDDED; > > - regmap_update_bits(sdhci_am654->base, CTL_CFG_2, > - ctl_cfg_2, SLOTTYPE_MASK); > + regmap_update_bits(sdhci_am654->base, CTL_CFG_2, ctl_cfg_2, > + SLOTTYPE_MASK); > > return sdhci_add_host(host); > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mmc: sdhci_am654: Fix minor phy configurations 2019-04-26 5:50 ` Adrian Hunter @ 2019-05-07 16:29 ` Faiz Abbas 0 siblings, 0 replies; 8+ messages in thread From: Faiz Abbas @ 2019-05-07 16:29 UTC (permalink / raw) To: Adrian Hunter, Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson Hi Adrian, On 26/04/19 11:20 AM, Adrian Hunter wrote: > On 25/04/19 6:57 PM, Faiz Abbas wrote: >> Fix the following minor things: >> >> 1. Line wrapping with the regmap_*() functions is way more conservative >> than required by the 80 character rule. Expand the function calls out to >> use less number of lines. >> >> 2. Add an error message if the DLL fails to lock. > > Please make the white space changes a separate patch. > > Also I would prefer not to use "fix" in the subject unless the patch fixes > driver behaviour. > Ok. Two different patches. No "fix" in the subject. Sending v2. Thanks, Faiz ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write 2019-04-25 15:57 [PATCH 0/2] Fix issues with phy configurations Faiz Abbas 2019-04-25 15:57 ` [PATCH 1/2] mmc: sdhci_am654: Fix minor " Faiz Abbas @ 2019-04-25 15:57 ` Faiz Abbas 2019-04-26 6:00 ` Adrian Hunter 1 sibling, 1 reply; 8+ messages in thread From: Faiz Abbas @ 2019-04-25 15:57 UTC (permalink / raw) To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, faiz_abbas In the call to regmap_update_bits() for SLOTTYPE, the mask and value fields are exchanged. Fix this. Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> --- drivers/mmc/host/sdhci_am654.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c index 866a9082705f..613b151a73c5 100644 --- a/drivers/mmc/host/sdhci_am654.c +++ b/drivers/mmc/host/sdhci_am654.c @@ -205,8 +205,8 @@ static int sdhci_am654_init(struct sdhci_host *host) if (host->mmc->caps & MMC_CAP_NONREMOVABLE) ctl_cfg_2 = SLOTTYPE_EMBEDDED; - regmap_update_bits(sdhci_am654->base, CTL_CFG_2, ctl_cfg_2, - SLOTTYPE_MASK); + regmap_update_bits(sdhci_am654->base, CTL_CFG_2, SLOTTYPE_MASK, + ctl_cfg_2); return sdhci_add_host(host); } -- 2.19.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write 2019-04-25 15:57 ` [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write Faiz Abbas @ 2019-04-26 6:00 ` Adrian Hunter 2019-05-07 16:27 ` Faiz Abbas 0 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2019-04-26 6:00 UTC (permalink / raw) To: Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson On 25/04/19 6:57 PM, Faiz Abbas wrote: > In the call to regmap_update_bits() for SLOTTYPE, the mask and value > fields are exchanged. Fix this. Could you also comment on whether this has any known effect on the driver. > > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > --- > drivers/mmc/host/sdhci_am654.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c > index 866a9082705f..613b151a73c5 100644 > --- a/drivers/mmc/host/sdhci_am654.c > +++ b/drivers/mmc/host/sdhci_am654.c > @@ -205,8 +205,8 @@ static int sdhci_am654_init(struct sdhci_host *host) > if (host->mmc->caps & MMC_CAP_NONREMOVABLE) > ctl_cfg_2 = SLOTTYPE_EMBEDDED; > > - regmap_update_bits(sdhci_am654->base, CTL_CFG_2, ctl_cfg_2, > - SLOTTYPE_MASK); > + regmap_update_bits(sdhci_am654->base, CTL_CFG_2, SLOTTYPE_MASK, > + ctl_cfg_2); > > return sdhci_add_host(host); > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write 2019-04-26 6:00 ` Adrian Hunter @ 2019-05-07 16:27 ` Faiz Abbas 2019-05-08 6:04 ` Adrian Hunter 0 siblings, 1 reply; 8+ messages in thread From: Faiz Abbas @ 2019-05-07 16:27 UTC (permalink / raw) To: Adrian Hunter, Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson Hi Adrian, On 26/04/19 11:30 AM, Adrian Hunter wrote: > On 25/04/19 6:57 PM, Faiz Abbas wrote: >> In the call to regmap_update_bits() for SLOTTYPE, the mask and value >> fields are exchanged. Fix this. > > Could you also comment on whether this has any known effect on the driver. > This call was basically a NOP but it was the correct way around in u-boot so it was just taking that value instead. No effect that was known to me. Found this out just by inspection. Thanks, Faiz ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write 2019-05-07 16:27 ` Faiz Abbas @ 2019-05-08 6:04 ` Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: Adrian Hunter @ 2019-05-08 6:04 UTC (permalink / raw) To: Faiz Abbas, Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson On 7/05/19 7:27 PM, Faiz Abbas wrote: > Hi Adrian, > > On 26/04/19 11:30 AM, Adrian Hunter wrote: >> On 25/04/19 6:57 PM, Faiz Abbas wrote: >>> In the call to regmap_update_bits() for SLOTTYPE, the mask and value >>> fields are exchanged. Fix this. >> >> Could you also comment on whether this has any known effect on the driver. >> > > This call was basically a NOP but it was the correct way around in > u-boot so it was just taking that value instead. No effect that was > known to me. Found this out just by inspection. Please put that in the commit message. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-05-08 6:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-25 15:57 [PATCH 0/2] Fix issues with phy configurations Faiz Abbas 2019-04-25 15:57 ` [PATCH 1/2] mmc: sdhci_am654: Fix minor " Faiz Abbas 2019-04-26 5:50 ` Adrian Hunter 2019-05-07 16:29 ` Faiz Abbas 2019-04-25 15:57 ` [PATCH 2/2] mmc: sdhci_am654: Fix SLOTTYPE write Faiz Abbas 2019-04-26 6:00 ` Adrian Hunter 2019-05-07 16:27 ` Faiz Abbas 2019-05-08 6:04 ` Adrian Hunter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).