Hi Ashok,
On 7/26/21 2:33 PM, Ashok Reddy Soma wrote:
> 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 ?
I didn't see other patch yet. Sorry.
After checked other patch, I will reply ASAP.
Best Regards,
Jaehoon Chung
>
> 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);
>>> };
>>>
>>>
>
