[PATCH 1/6] mmc: zynq_sdhci: Resolve uninitialized return value

[Ashok Reddy Soma]

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung <jh80.chung at samsung.com>
> Sent: Friday, July 9, 2021 4:21 PM
> To: Ashok Reddy Soma <ashokred at xilinx.com>; u-boot at lists.denx.de
> Cc: peng.fan at nxp.com; git <git at xilinx.com>; monstr at monstr.eu;
> somaashokreddy at gmail.com
> Subject: Re: [PATCH 1/6] mmc: zynq_sdhci: Resolve uninitialized return value
> 
> Hi Ashok,
> 
> On 7/9/21 7:15 PM, Ashok Reddy Soma wrote:
> > Hi Jaehoon,
> >
> >> -----Original Message-----
> >> From: Jaehoon Chung <jh80.chung at samsung.com>
> >> Sent: Friday, July 9, 2021 3:08 PM
> >> To: Ashok Reddy Soma <ashokred at xilinx.com>; u-boot at lists.denx.de
> >> Cc: peng.fan at nxp.com; git <git at xilinx.com>; monstr at monstr.eu;
> >> somaashokreddy at gmail.com
> >> Subject: Re: [PATCH 1/6] mmc: zynq_sdhci: Resolve uninitialized
> >> return value
> >>
> >> Hi Ashok,
> >>
> >> On 7/9/21 4:17 PM, Ashok Reddy Soma wrote:
> >>> set_phase() functions are not modifying the ret value and returning
> >>> the same uninitialized ret, return 0 instead.
> >>
> >> Why didn't you change from int to void?
> >>
> >
> > We are planning to change the way tapdelay's are set to use firmware
> function xilinx_pm_request() in place of arasan_zynqmp_set_out_tapdelay().
> > This return type int is a provision for that.
> 
> Thanks for kindly explanation.
> 
> Could you add the brief comment about that in commit-msg? Then It will be
> clarified more than now.
> Because there is no mentioned why keep to int type.
> Other things look good to me.
> 
Thanks for the review !! 
I will update it in the commit message and send v3.

Thanks,
Ashok

> Best Regards,
> Jaehoon Chung
> 
> >
> > Thanks,
> > Ashok
> >
> >> Best Regards,
> >> Jaehoon Chung
> >>
> >>
> >>>
> >>> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma at xilinx.com>
> >>> ---
> >>>
> >>>  drivers/mmc/zynq_sdhci.c | 24 ++++++++++--------------
> >>>  1 file changed, 10 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> >>> index
> >>> b79c4021b6..cb785fd735 100644
> >>> --- a/drivers/mmc/zynq_sdhci.c
> >>> +++ b/drivers/mmc/zynq_sdhci.c
> >>> @@ -182,8 +182,8 @@ static int arasan_sdhci_execute_tuning(struct
> >>> mmc
> >> *mmc, u8 opcode)
> >>>   * Set the SD Output Clock Tap Delays for Output path
> >>>   *
> >>>   * @host:		Pointer to the sdhci_host structure.
> >>> - * @degrees:		The clock phase shift between 0 - 359.
> >>> - * Return: 0 on success and error value on error
> >>> + * @degrees		The clock phase shift between 0 - 359.
> >>> + * Return: 0
> >>>   */
> >>>  static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
> >>>  					    int degrees)
> >>> @@ -191,7 +191,6 @@ static int
> >>> sdhci_zynqmp_sdcardclk_set_phase(struct
> >> sdhci_host *host,
> >>>  	struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev);
> >>>  	struct mmc *mmc = (struct mmc *)host->mmc;
> >>>  	u8 tap_delay, tap_max = 0;
> >>> -	int ret;
> >>>  	int timing = mode2timing[mmc->selected_mode];
> >>>
> >>>  	/*
> >>> @@ -229,7 +228,7 @@ static int
> >>> sdhci_zynqmp_sdcardclk_set_phase(struct
> >>> sdhci_host *host,
> >>>
> >>>  	arasan_zynqmp_set_tapdelay(priv->deviceid, 0, tap_delay);
> >>>
> >>> -	return ret;
> >>> +	return 0;
> >>>  }
> >>>
> >>>  /**
> >>> @@ -238,8 +237,8 @@ static int
> >>> sdhci_zynqmp_sdcardclk_set_phase(struct
> >> sdhci_host *host,
> >>>   * Set the SD Input Clock Tap Delays for Input path
> >>>   *
> >>>   * @host:		Pointer to the sdhci_host structure.
> >>> - * @degrees:		The clock phase shift between 0 - 359.
> >>> - * Return: 0 on success and error value on error
> >>> + * @degrees		The clock phase shift between 0 - 359.
> >>> + * Return: 0
> >>>   */
> >>>  static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
> >>>  					    int degrees)
> >>> @@ -247,7 +246,6 @@ static int
> >>> sdhci_zynqmp_sampleclk_set_phase(struct
> >> sdhci_host *host,
> >>>  	struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev);
> >>>  	struct mmc *mmc = (struct mmc *)host->mmc;
> >>>  	u8 tap_delay, tap_max = 0;
> >>> -	int ret;
> >>>  	int timing = mode2timing[mmc->selected_mode];
> >>>
> >>>  	/*
> >>> @@ -285,7 +283,7 @@ static int
> >>> sdhci_zynqmp_sampleclk_set_phase(struct
> >>> sdhci_host *host,
> >>>
> >>>  	arasan_zynqmp_set_tapdelay(priv->deviceid, tap_delay, 0);
> >>>
> >>> -	return ret;
> >>> +	return 0;
> >>>  }
> >>>
> >>>  /**
> >>> @@ -295,14 +293,13 @@ static int
> >> sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
> >>>   *
> >>>   * @host:		Pointer to the sdhci_host structure.
> >>>   * @degrees		The clock phase shift between 0 - 359.
> >>> - * Return: 0 on success and error value on error
> >>> + * Return: 0
> >>>   */
> >>>  static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
> >>>  					    int degrees)
> >>>  {
> >>>  	struct mmc *mmc = (struct mmc *)host->mmc;
> >>>  	u8 tap_delay, tap_max = 0;
> >>> -	int ret;
> >>>  	int timing = mode2timing[mmc->selected_mode];
> >>>
> >>>  	/*
> >>> @@ -349,7 +346,7 @@ static int
> >>> sdhci_versal_sdcardclk_set_phase(struct
> >> sdhci_host *host,
> >>>  		sdhci_writel(host, regval,
> >> SDHCI_ARASAN_OTAPDLY_REGISTER);
> >>>  	}
> >>>
> >>> -	return ret;
> >>> +	return 0;
> >>>  }
> >>>
> >>>  /**
> >>> @@ -359,14 +356,13 @@ static int
> >>> sdhci_versal_sdcardclk_set_phase(struct
> >> sdhci_host *host,
> >>>   *
> >>>   * @host:		Pointer to the sdhci_host structure.
> >>>   * @degrees		The clock phase shift between 0 - 359.
> >>> - * Return: 0 on success and error value on error
> >>> + * Return: 0
> >>>   */
> >>>  static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
> >>>  					    int degrees)
> >>>  {
> >>>  	struct mmc *mmc = (struct mmc *)host->mmc;
> >>>  	u8 tap_delay, tap_max = 0;
> >>> -	int ret;
> >>>  	int timing = mode2timing[mmc->selected_mode];
> >>>
> >>>  	/*
> >>> @@ -417,7 +413,7 @@ static int
> >>> sdhci_versal_sampleclk_set_phase(struct
> >> sdhci_host *host,
> >>>  		sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
> >>>  	}
> >>>
> >>> -	return ret;
> >>> +	return 0;
> >>>  }
> >>>
> >>>  static void arasan_sdhci_set_tapdelay(struct sdhci_host *host)
> >>>
> >