Hi Jaehoon, Thanks for the review.
> -----Original Message----- > From: Jaehoon Chung <jh80.ch...@samsung.com> > Sent: Monday, July 26, 2021 3:18 AM > To: Ashok Reddy Soma <ashok...@xilinx.com>; u-boot@lists.denx.de > Cc: peng....@nxp.com; faiz_ab...@ti.com; s...@chromium.org; > mich...@walle.cc; git <g...@xilinx.com>; mon...@monstr.eu; > somaashokre...@gmail.com; T Karthik Reddy <tkart...@xilinx.com> > Subject: Re: [PATCH 1/7] mmc: sdhci: Return error in case of failure > > Hi Ashok, > > On 7/24/21 5:10 PM, Ashok Reddy Soma wrote: > > From: T Karthik Reddy <t.karthik.re...@xilinx.com> > > > > set_delay() function is from sdhci host ops, which does not return any > > error due to void return type. Get return values from input and output > > set clock phase functions inside arasan_sdhci_set_tapdelay() and > > return the errors. > > > > Change return type to int for arasan_sdhci_set_tapdelay() and also for > > set_delay() in sdhci_ops structure. > > Could you separate the patch to sdhci and zync_sdhci part? Sure, i will split in to two patches. > > > > > Signed-off-by: T Karthik Reddy <t.karthik.re...@xilinx.com> > > Signed-off-by: Ashok Reddy Soma <ashok.reddy.s...@xilinx.com> > > --- > > > > drivers/mmc/sdhci.c | 8 ++++++-- > > drivers/mmc/zynq_sdhci.c | 21 ++++++++++++++++----- > > include/sdhci.h | 2 +- > > 3 files changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index > > d9ab6a0a83..f144602eec 100644 > > --- a/drivers/mmc/sdhci.c > > +++ b/drivers/mmc/sdhci.c > > @@ -366,6 +366,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int > > clock) { > > struct sdhci_host *host = mmc->priv; > > unsigned int div, clk = 0, timeout; > > + int ret; > > > > /* Wait max 20 ms */ > > timeout = 200; > > @@ -386,8 +387,11 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int > clock) > > if (clock == 0) > > return 0; > > > > - if (host->ops && host->ops->set_delay) > > - host->ops->set_delay(host); > > + if (host->ops && host->ops->set_delay) { > > + ret = host->ops->set_delay(host); > > + if (ret) > > + return ret; > > how about adding debug(). It's helpful to debug when it's failed. Ok, I will add a debug print here. Any comments for other patches in this series or shall I send V2 with these changes ? Thanks, Ashok > > Best Regards, > Jaehoon Chung > > > + } > > > > if (SDHCI_GET_VERSION(host) >= SDHCI_SPEC_300) { > > /* > > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index > > ba87ee8dd5..9fb3603c7e 100644 > > --- a/drivers/mmc/zynq_sdhci.c > > +++ b/drivers/mmc/zynq_sdhci.c > > @@ -422,7 +422,7 @@ static int sdhci_versal_sampleclk_set_phase(struct > sdhci_host *host, > > return 0; > > } > > > > -static void arasan_sdhci_set_tapdelay(struct sdhci_host *host) > > +static int arasan_sdhci_set_tapdelay(struct sdhci_host *host) > > { > > struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev); > > struct arasan_sdhci_clk_data *clk_data = &priv->clk_data; @@ -431,18 > > +431,29 @@ static void arasan_sdhci_set_tapdelay(struct sdhci_host *host) > > u8 timing = mode2timing[mmc->selected_mode]; > > u32 iclk_phase = clk_data->clk_phase_in[timing]; > > u32 oclk_phase = clk_data->clk_phase_out[timing]; > > + int ret; > > > > dev_dbg(dev, "%s, host:%s, mode:%d\n", __func__, host->name, > > timing); > > > > if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) && > > device_is_compatible(dev, "xlnx,zynqmp-8.9a")) { > > - sdhci_zynqmp_sampleclk_set_phase(host, iclk_phase); > > - sdhci_zynqmp_sdcardclk_set_phase(host, oclk_phase); > > + ret = sdhci_zynqmp_sampleclk_set_phase(host, iclk_phase); > > + if (ret) > > + return ret; > > + ret = sdhci_zynqmp_sdcardclk_set_phase(host, oclk_phase); > > + if (ret) > > + return ret; > > } else if (IS_ENABLED(CONFIG_ARCH_VERSAL) && > > device_is_compatible(dev, "xlnx,versal-8.9a")) { > > - sdhci_versal_sampleclk_set_phase(host, iclk_phase); > > - sdhci_versal_sdcardclk_set_phase(host, oclk_phase); > > + ret = sdhci_versal_sampleclk_set_phase(host, iclk_phase); > > + if (ret) > > + return ret; > > + ret = sdhci_versal_sdcardclk_set_phase(host, oclk_phase); > > + if (ret) > > + return ret; > > } > > + > > + return 0; > > } > > > > static void arasan_dt_read_clk_phase(struct udevice *dev, unsigned > > char timing, diff --git a/include/sdhci.h b/include/sdhci.h index > > 0ae9471ad7..44a0d84e5a 100644 > > --- a/include/sdhci.h > > +++ b/include/sdhci.h > > @@ -268,7 +268,7 @@ struct sdhci_ops { > > int (*set_ios_post)(struct sdhci_host *host); > > void (*set_clock)(struct sdhci_host *host, u32 div); > > int (*platform_execute_tuning)(struct mmc *host, u8 opcode); > > - void (*set_delay)(struct sdhci_host *host); > > + int (*set_delay)(struct sdhci_host *host); > > int (*deferred_probe)(struct sdhci_host *host); > > }; > > > >