[PATCH 0/6] This patch set fixes minor issues related to tapdelays

[PATCH 0/6] This patch set fixes minor issues related to tapdelays

[Ashok Reddy Soma]

This patch set fixes below issues in zynq_sdhc driver
 - Fix issues in tap delay functions where it returns uninitialized values
 - Allow configuring zero tap delay values
 - Split tapdelay functions to set input and output tap delay's separately.
 - Fix kernel doc warnings
 - Make local structures as static structures


Ashok Reddy Soma (4):
  mmc: zynq_sdhci: Resolve uninitialized return value
  mmc: zynq_sdhci: Allow configuring zero Tap values
  mmc: zynq_sdhci: Use Mask writes for Tap delays
  mmc: zynq_sdhci: Split set_tapdelay function to in and out

Michal Simek (2):
  mmc: zynq_sdhci: Fix kernel doc warnings
  mmc: zynq_sdhci: Make variables/structure static

 board/xilinx/zynqmp/tap_delays.c |  73 ++++++++--------
 drivers/mmc/zynq_sdhci.c         | 144 ++++++++++++++++---------------
 include/zynqmp_tap_delay.h       |   7 +-
 3 files changed, 115 insertions(+), 109 deletions(-)

-- 
2.17.1

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

[Ashok Reddy Soma]

set_phase() functions are not modifying the ret value and returning
the same uninitialized ret, return 0 instead.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@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)
-- 
2.17.1

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

[Jaehoon Chung]

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?

Best Regards,
Jaehoon Chung


> 
> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@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)
> 

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

[Ashok Reddy Soma]

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung <jh80.chung@samsung.com>
> Sent: Friday, July 9, 2021 3:08 PM
> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
> Cc: peng.fan@nxp.com; git <git@xilinx.com>; monstr@monstr.eu;
> somaashokreddy@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,
Ashok

> Best Regards,
> Jaehoon Chung
> 
> 
> >
> > Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@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)
> >

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

[Jaehoon Chung]

Hi Ashok,

On 7/9/21 7:15 PM, Ashok Reddy Soma wrote:
> Hi Jaehoon,
> 
>> -----Original Message-----
>> From: Jaehoon Chung <jh80.chung@samsung.com>
>> Sent: Friday, July 9, 2021 3:08 PM
>> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
>> Cc: peng.fan@nxp.com; git <git@xilinx.com>; monstr@monstr.eu;
>> somaashokreddy@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.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Ashok
> 
>> Best Regards,
>> Jaehoon Chung
>>
>>
>>>
>>> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@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)
>>>
> 

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

[Ashok Reddy Soma]

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung <jh80.chung@samsung.com>
> Sent: Friday, July 9, 2021 4:21 PM
> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
> Cc: peng.fan@nxp.com; git <git@xilinx.com>; monstr@monstr.eu;
> somaashokreddy@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@samsung.com>
> >> Sent: Friday, July 9, 2021 3:08 PM
> >> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
> >> Cc: peng.fan@nxp.com; git <git@xilinx.com>; monstr@monstr.eu;
> >> somaashokreddy@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@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)
> >>>
> >

[PATCH 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values

[Ashok Reddy Soma]

Allow configuring ITAP and OTAP values with zero to avoid failures in
some cases (one of them is SD boot mode). Legacy, SDR12 modes require
to program the ITAP and OTAP values as zero, whereas for SDR50 and SDR104
modes ITAP value is zero.

In SD boot mode firmware configures the SD ITAP and OTAP values and
in this case u-boot has to re-configure required tap values(including zero)
based on the operating mode.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
---

 drivers/mmc/zynq_sdhci.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index cb785fd735..4f014da7ea 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -198,9 +198,7 @@ static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
 	 * ZynqMP does not set phase for <=25MHz clock.
 	 * If degrees is zero, no need to do anything.
 	 */
-	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300 ||
-	    timing == MMC_TIMING_LEGACY ||
-	    timing == MMC_TIMING_UHS_SDR12 || !degrees)
+	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300)
 		return 0;
 
 	switch (timing) {
@@ -253,9 +251,7 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
 	 * ZynqMP does not set phase for <=25MHz clock.
 	 * If degrees is zero, no need to do anything.
 	 */
-	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300 ||
-	    timing == MMC_TIMING_LEGACY ||
-	    timing == MMC_TIMING_UHS_SDR12 || !degrees)
+	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300)
 		return 0;
 
 	switch (timing) {
@@ -307,9 +303,7 @@ static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
 	 * Versal does not set phase for <=25MHz clock.
 	 * If degrees is zero, no need to do anything.
 	 */
-	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300 ||
-	    timing == MMC_TIMING_LEGACY ||
-	    timing == MMC_TIMING_UHS_SDR12 || !degrees)
+	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300)
 		return 0;
 
 	switch (timing) {
@@ -370,9 +364,7 @@ static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
 	 * Versal does not set phase for <=25MHz clock.
 	 * If degrees is zero, no need to do anything.
 	 */
-	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300 ||
-	    timing == MMC_TIMING_LEGACY ||
-	    timing == MMC_TIMING_UHS_SDR12 || !degrees)
+	if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300)
 		return 0;
 
 	switch (timing) {
-- 
2.17.1

[PATCH 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays

[Ashok Reddy Soma]

Restrict tap_delay value to the allowed size(8bits for itap and 6 bits
for otap) before writing to the tap delay register.

Clear ITAP and OTAP delay bits before updating with the new tap value
for Versal platform.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
---

 drivers/mmc/zynq_sdhci.c | 58 +++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 4f014da7ea..c600d97f1e 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -19,11 +19,13 @@
 #include <sdhci.h>
 #include <zynqmp_tap_delay.h>
 
-#define SDHCI_ARASAN_ITAPDLY_REGISTER   0xF0F8
-#define SDHCI_ARASAN_OTAPDLY_REGISTER   0xF0FC
-#define SDHCI_ITAPDLY_CHGWIN            0x200
-#define SDHCI_ITAPDLY_ENABLE            0x100
-#define SDHCI_OTAPDLY_ENABLE            0x40
+#define SDHCI_ARASAN_ITAPDLY_REGISTER	0xF0F8
+#define SDHCI_ARASAN_ITAPDLY_SEL_MASK	0xFF
+#define SDHCI_ARASAN_OTAPDLY_REGISTER	0xF0FC
+#define SDHCI_ARASAN_OTAPDLY_SEL_MASK	0x3F
+#define SDHCI_ITAPDLY_CHGWIN		0x200
+#define SDHCI_ITAPDLY_ENABLE		0x100
+#define SDHCI_OTAPDLY_ENABLE		0x40
 
 #define SDHCI_TUNING_LOOP_COUNT		40
 #define MMC_BANK2			0x2
@@ -297,6 +299,7 @@ static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
 	struct mmc *mmc = (struct mmc *)host->mmc;
 	u8 tap_delay, tap_max = 0;
 	int timing = mode2timing[mmc->selected_mode];
+	u32 regval;
 
 	/*
 	 * This is applicable for SDHCI_SPEC_300 and above
@@ -329,16 +332,16 @@ static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
 
 	tap_delay = (degrees * tap_max) / 360;
 
+	/* Limit output tap_delay value to 6 bits */
+	tap_delay &= SDHCI_ARASAN_OTAPDLY_SEL_MASK;
+
 	/* Set the Clock Phase */
-	if (tap_delay) {
-		u32 regval;
-
-		regval = sdhci_readl(host, SDHCI_ARASAN_OTAPDLY_REGISTER);
-		regval |= SDHCI_OTAPDLY_ENABLE;
-		sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER);
-		regval |= tap_delay;
-		sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER);
-	}
+	regval = sdhci_readl(host, SDHCI_ARASAN_OTAPDLY_REGISTER);
+	regval |= SDHCI_OTAPDLY_ENABLE;
+	sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER);
+	regval &= ~SDHCI_ARASAN_OTAPDLY_SEL_MASK;
+	regval |= tap_delay;
+	sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER);
 
 	return 0;
 }
@@ -358,6 +361,7 @@ static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
 	struct mmc *mmc = (struct mmc *)host->mmc;
 	u8 tap_delay, tap_max = 0;
 	int timing = mode2timing[mmc->selected_mode];
+	u32 regval;
 
 	/*
 	 * This is applicable for SDHCI_SPEC_300 and above
@@ -390,20 +394,20 @@ static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
 
 	tap_delay = (degrees * tap_max) / 360;
 
+	/* Limit input tap_delay value to 8 bits */
+	tap_delay &= SDHCI_ARASAN_ITAPDLY_SEL_MASK;
+
 	/* Set the Clock Phase */
-	if (tap_delay) {
-		u32 regval;
-
-		regval = sdhci_readl(host, SDHCI_ARASAN_ITAPDLY_REGISTER);
-		regval |= SDHCI_ITAPDLY_CHGWIN;
-		sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
-		regval |= SDHCI_ITAPDLY_ENABLE;
-		sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
-		regval |= tap_delay;
-		sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
-		regval &= ~SDHCI_ITAPDLY_CHGWIN;
-		sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
-	}
+	regval = sdhci_readl(host, SDHCI_ARASAN_ITAPDLY_REGISTER);
+	regval |= SDHCI_ITAPDLY_CHGWIN;
+	sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
+	regval |= SDHCI_ITAPDLY_ENABLE;
+	sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
+	regval &= ~SDHCI_ARASAN_ITAPDLY_SEL_MASK;
+	regval |= tap_delay;
+	sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
+	regval &= ~SDHCI_ITAPDLY_CHGWIN;
+	sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER);
 
 	return 0;
 }
-- 
2.17.1

[PATCH 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out

[Ashok Reddy Soma]

Split arasan_zynqmp_set_tapdelay() to handle input and output tapdelays
separately. This is required to handle zero values for ITAP and OTAP
values. If we dont split, we will have to remove the if() in the
function, which makes ITAP values to be overwritten when OTAP values are
called to set and vice-versa.

Restrict tap_delay value calculated to max allowed 8 bits for ITAP and 6
bits for OTAP for ZynqMP.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
---

 board/xilinx/zynqmp/tap_delays.c | 73 +++++++++++++++++---------------
 drivers/mmc/zynq_sdhci.c         | 10 ++++-
 include/zynqmp_tap_delay.h       |  7 +--
 3 files changed, 50 insertions(+), 40 deletions(-)

diff --git a/board/xilinx/zynqmp/tap_delays.c b/board/xilinx/zynqmp/tap_delays.c
index 1cab25f00a..d16bbb8eff 100644
--- a/board/xilinx/zynqmp/tap_delays.c
+++ b/board/xilinx/zynqmp/tap_delays.c
@@ -50,48 +50,51 @@ void zynqmp_dll_reset(u8 deviceid)
 		zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, 0x0);
 }
 
-void arasan_zynqmp_set_tapdelay(u8 deviceid, u32 itap_delay, u32 otap_delay)
+void arasan_zynqmp_set_in_tapdelay(u8 deviceid, u32 itap_delay)
 {
 	if (deviceid == 0) {
-		zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK,
-				  SD0_DLL_RST);
-		/* Program ITAP */
-		if (itap_delay) {
-			zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN_MASK,
-					  SD0_ITAPCHGWIN);
-			zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA_MASK,
-					  SD0_ITAPDLYENA);
-			zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK,
-					  itap_delay);
-			zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN_MASK,
-					  0x0);
-		}
+		zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, SD0_DLL_RST);
 
-		/* Program OTAP */
-		if (otap_delay)
-			zynqmp_mmio_write(SD_OTAP_DLY, SD0_OTAPDLYSEL_MASK,
-					  otap_delay);
+		/* Program ITAP delay */
+		zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN_MASK,
+				  SD0_ITAPCHGWIN);
+		zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA_MASK,
+				  SD0_ITAPDLYENA);
+		zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK, itap_delay);
+		zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN_MASK, 0x0);
 
 		zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, 0x0);
 	} else {
-		zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK,
-				  SD1_DLL_RST);
-		/* Program ITAP */
-		if (itap_delay) {
-			zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN_MASK,
-					  SD1_ITAPCHGWIN);
-			zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA_MASK,
-					  SD1_ITAPDLYENA);
-			zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
-					  (itap_delay << 16));
-			zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN_MASK,
-					  0x0);
-		}
+		zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, SD1_DLL_RST);
+
+		/* Program ITAP delay */
+		zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN_MASK,
+				  SD1_ITAPCHGWIN);
+		zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA_MASK,
+				  SD1_ITAPDLYENA);
+		zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
+				  (itap_delay << 16));
+		zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN_MASK, 0x0);
+
+		zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, 0x0);
+	}
+}
+
+void arasan_zynqmp_set_out_tapdelay(u8 deviceid, u32 otap_delay)
+{
+	if (deviceid == 0) {
+		zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, SD0_DLL_RST);
+
+		/* Program OTAP delay */
+		zynqmp_mmio_write(SD_OTAP_DLY, SD0_OTAPDLYSEL_MASK, otap_delay);
+
+		zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, 0x0);
+	} else {
+		zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, SD1_DLL_RST);
 
-		/* Program OTAP */
-		if (otap_delay)
-			zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
-					  (otap_delay << 16));
+		/* Program OTAP delay */
+		zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
+				  (otap_delay << 16));
 
 		zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, 0x0);
 	}
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index c600d97f1e..926569f5d0 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -226,7 +226,10 @@ static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
 
 	tap_delay = (degrees * tap_max) / 360;
 
-	arasan_zynqmp_set_tapdelay(priv->deviceid, 0, tap_delay);
+	/* Limit output tap_delay value to 6 bits */
+	tap_delay &= SDHCI_ARASAN_OTAPDLY_SEL_MASK;
+
+	arasan_zynqmp_set_out_tapdelay(priv->deviceid, tap_delay);
 
 	return 0;
 }
@@ -279,7 +282,10 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
 
 	tap_delay = (degrees * tap_max) / 360;
 
-	arasan_zynqmp_set_tapdelay(priv->deviceid, tap_delay, 0);
+	/* Limit input tap_delay value to 8 bits */
+	tap_delay &= SDHCI_ARASAN_ITAPDLY_SEL_MASK;
+
+	arasan_zynqmp_set_in_tapdelay(priv->deviceid, tap_delay);
 
 	return 0;
 }
diff --git a/include/zynqmp_tap_delay.h b/include/zynqmp_tap_delay.h
index 7b713438f7..1c1e3e7dee 100644
--- a/include/zynqmp_tap_delay.h
+++ b/include/zynqmp_tap_delay.h
@@ -10,11 +10,12 @@
 
 #ifdef CONFIG_ARCH_ZYNQMP
 void zynqmp_dll_reset(u8 deviceid);
-void arasan_zynqmp_set_tapdelay(u8 device_id, u32 itap_delay, u32 otap_delay);
+void arasan_zynqmp_set_in_tapdelay(u8 device_id, u32 itap_delay);
+void arasan_zynqmp_set_out_tapdelay(u8 device_id, u32 otap_delay);
 #else
 inline void zynqmp_dll_reset(u8 deviceid) {}
-inline void arasan_zynqmp_set_tapdelay(u8 device_id, u32 itap_delay,
-				       u32 otap_delay) {}
+inline void arasan_zynqmp_set_in_tapdelay(u8 device_id, u32 itap_delay) {}
+inline void arasan_zynqmp_set_out_tapdelay(u8 device_id, u32 otap_delay) {}
 #endif
 
 #endif
-- 
2.17.1

[PATCH 5/6] mmc: zynq_sdhci: Fix kernel doc warnings

[Ashok Reddy Soma]

From: Michal Simek <michal.simek@xilinx.com>

Fix these kernel doc warnings:
drivers/mmc/zynq_sdhci.c:181: warning: contents before sections
drivers/mmc/zynq_sdhci.c:187: warning: Function parameter or member 'degrees' not described in 'sdhci_zynqmp_sdcardclk_set_phase'
drivers/mmc/zynq_sdhci.c:236: warning: contents before sections
drivers/mmc/zynq_sdhci.c:242: warning: Function parameter or member 'degrees' not described in 'sdhci_zynqmp_sampleclk_set_phase'
drivers/mmc/zynq_sdhci.c:291: warning: contents before sections
drivers/mmc/zynq_sdhci.c:297: warning: Function parameter or member 'degrees' not described in 'sdhci_versal_sdcardclk_set_phase'
drivers/mmc/zynq_sdhci.c:354: warning: contents before sections
drivers/mmc/zynq_sdhci.c:360: warning: Function parameter or member 'degrees' not described in 'sdhci_versal_sampleclk_set_phase'
drivers/mmc/zynq_sdhci.c:467: warning: contents before sections

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
---

 drivers/mmc/zynq_sdhci.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 926569f5d0..1e6efb1cd4 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -181,11 +181,11 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
 /**
  * sdhci_zynqmp_sdcardclk_set_phase - Set the SD Output Clock Tap Delays
  *
- * 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.
+ * @degrees:		The clock phase shift between 0 - 359.
  * Return: 0
+ *
+ * Set the SD Output Clock Tap Delays for Output path
  */
 static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
 					    int degrees)
@@ -237,11 +237,11 @@ static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
 /**
  * sdhci_zynqmp_sampleclk_set_phase - Set the SD Input Clock Tap Delays
  *
- * 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.
+ * @degrees:		The clock phase shift between 0 - 359.
  * Return: 0
+ *
+ * Set the SD Input Clock Tap Delays for Input path
  */
 static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
 					    int degrees)
@@ -293,11 +293,11 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
 /**
  * sdhci_versal_sdcardclk_set_phase - Set the SD Output Clock Tap Delays
  *
- * 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.
+ * @degrees:		The clock phase shift between 0 - 359.
  * Return: 0
+ *
+ * Set the SD Output Clock Tap Delays for Output path
  */
 static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
 					    int degrees)
@@ -355,11 +355,11 @@ static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
 /**
  * sdhci_versal_sampleclk_set_phase - Set the SD Input Clock Tap Delays
  *
- * 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.
+ * @degrees:		The clock phase shift between 0 - 359.
  * Return: 0
+ *
+ * Set the SD Input Clock Tap Delays for Input path
  */
 static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
 					    int degrees)
@@ -467,9 +467,9 @@ static void arasan_dt_read_clk_phase(struct udevice *dev, unsigned char timing,
 /**
  * arasan_dt_parse_clk_phases - Read Tap Delay values from DT
  *
- * Called at initialization to parse the values of Tap Delays.
- *
  * @dev:                Pointer to our struct udevice.
+ *
+ * Called at initialization to parse the values of Tap Delays.
  */
 static void arasan_dt_parse_clk_phases(struct udevice *dev)
 {
-- 
2.17.1

Re: [PATCH 5/6] mmc: zynq_sdhci: Fix kernel doc warnings

[Jaehoon Chung]

Hi Ashok,

On 7/9/21 4:17 PM, Ashok Reddy Soma wrote:
> From: Michal Simek <michal.simek@xilinx.com>
> 
> Fix these kernel doc warnings:
> drivers/mmc/zynq_sdhci.c:181: warning: contents before sections
> drivers/mmc/zynq_sdhci.c:187: warning: Function parameter or member 'degrees' not described in 'sdhci_zynqmp_sdcardclk_set_phase'
> drivers/mmc/zynq_sdhci.c:236: warning: contents before sections
> drivers/mmc/zynq_sdhci.c:242: warning: Function parameter or member 'degrees' not described in 'sdhci_zynqmp_sampleclk_set_phase'
> drivers/mmc/zynq_sdhci.c:291: warning: contents before sections
> drivers/mmc/zynq_sdhci.c:297: warning: Function parameter or member 'degrees' not described in 'sdhci_versal_sdcardclk_set_phase'
> drivers/mmc/zynq_sdhci.c:354: warning: contents before sections
> drivers/mmc/zynq_sdhci.c:360: warning: Function parameter or member 'degrees' not described in 'sdhci_versal_sampleclk_set_phase'
> drivers/mmc/zynq_sdhci.c:467: warning: contents before sections

I didn't understand this patch. You are changing from "@degrees:" to "@degree" on [PATCH 1/6].
But reverted them in this patch.

Best Regards,
Jaehoon Chung


> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
> ---
> 
>  drivers/mmc/zynq_sdhci.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 926569f5d0..1e6efb1cd4 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -181,11 +181,11 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
>  /**
>   * sdhci_zynqmp_sdcardclk_set_phase - Set the SD Output Clock Tap Delays
>   *
> - * 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.
> + * @degrees:		The clock phase shift between 0 - 359.
>   * Return: 0
> + *
> + * Set the SD Output Clock Tap Delays for Output path
>   */
>  static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
>  					    int degrees)
> @@ -237,11 +237,11 @@ static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
>  /**
>   * sdhci_zynqmp_sampleclk_set_phase - Set the SD Input Clock Tap Delays
>   *
> - * 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.
> + * @degrees:		The clock phase shift between 0 - 359.
>   * Return: 0
> + *
> + * Set the SD Input Clock Tap Delays for Input path
>   */
>  static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
>  					    int degrees)
> @@ -293,11 +293,11 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
>  /**
>   * sdhci_versal_sdcardclk_set_phase - Set the SD Output Clock Tap Delays
>   *
> - * 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.
> + * @degrees:		The clock phase shift between 0 - 359.
>   * Return: 0
> + *
> + * Set the SD Output Clock Tap Delays for Output path
>   */
>  static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
>  					    int degrees)
> @@ -355,11 +355,11 @@ static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
>  /**
>   * sdhci_versal_sampleclk_set_phase - Set the SD Input Clock Tap Delays
>   *
> - * 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.
> + * @degrees:		The clock phase shift between 0 - 359.
>   * Return: 0
> + *
> + * Set the SD Input Clock Tap Delays for Input path
>   */
>  static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
>  					    int degrees)
> @@ -467,9 +467,9 @@ static void arasan_dt_read_clk_phase(struct udevice *dev, unsigned char timing,
>  /**
>   * arasan_dt_parse_clk_phases - Read Tap Delay values from DT
>   *
> - * Called at initialization to parse the values of Tap Delays.
> - *
>   * @dev:                Pointer to our struct udevice.
> + *
> + * Called at initialization to parse the values of Tap Delays.
>   */
>  static void arasan_dt_parse_clk_phases(struct udevice *dev)
>  {
> 

RE: [PATCH 5/6] mmc: zynq_sdhci: Fix kernel doc warnings

[Ashok Reddy Soma]

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung <jh80.chung@samsung.com>
> Sent: Friday, July 9, 2021 3:12 PM
> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
> Cc: peng.fan@nxp.com; git <git@xilinx.com>; monstr@monstr.eu;
> somaashokreddy@gmail.com; Michal Simek <michals@xilinx.com>
> Subject: Re: [PATCH 5/6] mmc: zynq_sdhci: Fix kernel doc warnings
> 
> Hi Ashok,
> 
> On 7/9/21 4:17 PM, Ashok Reddy Soma wrote:
> > From: Michal Simek <michal.simek@xilinx.com>
> >
> > Fix these kernel doc warnings:
> > drivers/mmc/zynq_sdhci.c:181: warning: contents before sections
> > drivers/mmc/zynq_sdhci.c:187: warning: Function parameter or member
> 'degrees' not described in 'sdhci_zynqmp_sdcardclk_set_phase'
> > drivers/mmc/zynq_sdhci.c:236: warning: contents before sections
> > drivers/mmc/zynq_sdhci.c:242: warning: Function parameter or member
> 'degrees' not described in 'sdhci_zynqmp_sampleclk_set_phase'
> > drivers/mmc/zynq_sdhci.c:291: warning: contents before sections
> > drivers/mmc/zynq_sdhci.c:297: warning: Function parameter or member
> 'degrees' not described in 'sdhci_versal_sdcardclk_set_phase'
> > drivers/mmc/zynq_sdhci.c:354: warning: contents before sections
> > drivers/mmc/zynq_sdhci.c:360: warning: Function parameter or member
> 'degrees' not described in 'sdhci_versal_sampleclk_set_phase'
> > drivers/mmc/zynq_sdhci.c:467: warning: contents before sections
> 
> I didn't understand this patch. You are changing from "@degrees:" to
> "@degree" on [PATCH 1/6].
> But reverted them in this patch.

I made a mistake in patch 1/6, Michal corrected it in this patch. 
I will correct/move this change to 1/6 and send V2.

Thanks,
Ashok

> 
> Best Regards,
> Jaehoon Chung
> 
> 
> >
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
> > ---
> >
> >  drivers/mmc/zynq_sdhci.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index
> > 926569f5d0..1e6efb1cd4 100644
> > --- a/drivers/mmc/zynq_sdhci.c
> > +++ b/drivers/mmc/zynq_sdhci.c
> > @@ -181,11 +181,11 @@ static int arasan_sdhci_execute_tuning(struct
> > mmc *mmc, u8 opcode)
> >  /**
> >   * sdhci_zynqmp_sdcardclk_set_phase - Set the SD Output Clock Tap Delays
> >   *
> > - * 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.
> > + * @degrees:		The clock phase shift between 0 - 359.
> >   * Return: 0
> > + *
> > + * Set the SD Output Clock Tap Delays for Output path
> >   */
> >  static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
> >  					    int degrees)
> > @@ -237,11 +237,11 @@ static int
> > sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host,
> >  /**
> >   * sdhci_zynqmp_sampleclk_set_phase - Set the SD Input Clock Tap Delays
> >   *
> > - * 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.
> > + * @degrees:		The clock phase shift between 0 - 359.
> >   * Return: 0
> > + *
> > + * Set the SD Input Clock Tap Delays for Input path
> >   */
> >  static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
> >  					    int degrees)
> > @@ -293,11 +293,11 @@ static int
> > sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
> >  /**
> >   * sdhci_versal_sdcardclk_set_phase - Set the SD Output Clock Tap Delays
> >   *
> > - * 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.
> > + * @degrees:		The clock phase shift between 0 - 359.
> >   * Return: 0
> > + *
> > + * Set the SD Output Clock Tap Delays for Output path
> >   */
> >  static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
> >  					    int degrees)
> > @@ -355,11 +355,11 @@ static int
> > sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host,
> >  /**
> >   * sdhci_versal_sampleclk_set_phase - Set the SD Input Clock Tap Delays
> >   *
> > - * 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.
> > + * @degrees:		The clock phase shift between 0 - 359.
> >   * Return: 0
> > + *
> > + * Set the SD Input Clock Tap Delays for Input path
> >   */
> >  static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host,
> >  					    int degrees)
> > @@ -467,9 +467,9 @@ static void arasan_dt_read_clk_phase(struct
> > udevice *dev, unsigned char timing,
> >  /**
> >   * arasan_dt_parse_clk_phases - Read Tap Delay values from DT
> >   *
> > - * Called at initialization to parse the values of Tap Delays.
> > - *
> >   * @dev:                Pointer to our struct udevice.
> > + *
> > + * Called at initialization to parse the values of Tap Delays.
> >   */
> >  static void arasan_dt_parse_clk_phases(struct udevice *dev)  {
> >

[PATCH 6/6] mmc: zynq_sdhci: Make variables/structure static

[Ashok Reddy Soma]

From: Michal Simek <michal.simek@xilinx.com>

All these variables/structure are local and should be static.

Issues are reported by sparse:
drivers/mmc/zynq_sdhci.c:49:11: warning: symbol 'zynqmp_iclk_phases' was not declared. Should it be static?
drivers/mmc/zynq_sdhci.c:50:11: warning: symbol 'zynqmp_oclk_phases' was not declared. Should it be static?
drivers/mmc/zynq_sdhci.c:53:11: warning: symbol 'versal_iclk_phases' was not declared. Should it be static?
drivers/mmc/zynq_sdhci.c:54:11: warning: symbol 'versal_oclk_phases' was not declared. Should it be static?
drivers/mmc/zynq_sdhci.c:546:24: warning: symbol 'arasan_ops' was not declared. Should it be static?

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>
---

 drivers/mmc/zynq_sdhci.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 1e6efb1cd4..85a436b242 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -50,12 +50,16 @@ struct arasan_sdhci_priv {
 
 #if defined(CONFIG_ARCH_ZYNQMP) || defined(CONFIG_ARCH_VERSAL)
 /* Default settings for ZynqMP Clock Phases */
-const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63,  0,   0, 183, 54,  0, 0};
-const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0};
+static const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63,  0,
+					 0, 183, 54,  0, 0};
+static const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72,
+					 135, 48, 72, 135, 0};
 
 /* Default settings for Versal Clock Phases */
-const u32 versal_iclk_phases[] = {0, 132, 132, 0, 132, 0, 0, 162, 90, 0, 0};
-const u32 versal_oclk_phases[] = {0,  60, 48, 0, 48, 72, 90, 36, 60, 90, 0};
+static const u32 versal_iclk_phases[] = {0, 132, 132, 0, 132,
+					 0, 0, 162, 90, 0, 0};
+static const u32 versal_oclk_phases[] = {0,  60, 48, 0, 48, 72,
+					 90, 36, 60, 90, 0};
 
 static const u8 mode2timing[] = {
 	[MMC_LEGACY] = MMC_TIMING_LEGACY,
@@ -541,8 +545,8 @@ static void arasan_sdhci_set_control_reg(struct sdhci_host *host)
 		sdhci_set_uhs_timing(host);
 }
 
-const struct sdhci_ops arasan_ops = {
-	.platform_execute_tuning = &arasan_sdhci_execute_tuning,
+static const struct sdhci_ops arasan_ops = {
+	.platform_execute_tuning	= &arasan_sdhci_execute_tuning,
 	.set_delay = &arasan_sdhci_set_tapdelay,
 	.set_control_reg = &arasan_sdhci_set_control_reg,
 };
-- 
2.17.1