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

[PATCH v2 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

Changes in v2:
 - Changed "@degree" to "@degrees:" in function descriptions of tap
   delay functions
 - Removed @degree warning from commit description since it is fixed in
   patch 1/6.

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 v2 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>
---

Changes in v2:
 - Changed "@degree" to "@degrees:" in function descriptions of tap
   delay functions

 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..03600188ba 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -183,7 +183,7 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
  *
  * @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_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;
 }
 
 /**
@@ -239,7 +238,7 @@ static int sdhci_zynqmp_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_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;
 }
 
 /**
@@ -294,15 +292,14 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
  * 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_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;
 }
 
 /**
@@ -358,15 +355,14 @@ static int sdhci_versal_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_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

[PATCH v2 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>
---

(no changes since v1)

 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 03600188ba..b229c24a8b 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 v2 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>
---

(no changes since v1)

 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 b229c24a8b..44eb982b9a 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 v2 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>
---

(no changes since v1)

 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 44eb982b9a..dd06b56838 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 v2 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:236: warning: contents before sections
drivers/mmc/zynq_sdhci.c:291: warning: contents before sections
drivers/mmc/zynq_sdhci.c:354: warning: contents before sections
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>
---

Changes in v2:
 - Removed @degree warning from commit description since it is fixed in
   patch 1/6.

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

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index dd06b56838..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.
  * 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.
  * 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.
  * 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.
  * 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

[PATCH v2 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>
---

(no changes since v1)

 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

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

[Michal Simek]

On 7/9/21 12:46 PM, Ashok Reddy Soma wrote:
> 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>
> ---
> 
> Changes in v2:
>  - Changed "@degree" to "@degrees:" in function descriptions of tap
>    delay functions
> 
>  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..03600188ba 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -183,7 +183,7 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
>   *
>   * @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_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;
>  }
>  
>  /**
> @@ -239,7 +238,7 @@ static int sdhci_zynqmp_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_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;
>  }
>  
>  /**
> @@ -294,15 +292,14 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
>   * 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.

this should be also the part of 5/6

> + * 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;
>  }
>  
>  /**
> @@ -358,15 +355,14 @@ static int sdhci_versal_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.

this should be also the part of 5/6

> + * 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)
> 

M

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

[Michal Simek]

On 7/9/21 12:47 PM, Ashok Reddy Soma wrote:
> 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>
> ---
> 
> (no changes since v1)
> 
>  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 b229c24a8b..44eb982b9a 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

when you are touching this maybe better to use BIT and GENMASK macros.

>  
>  #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;
>  }
> 

M

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

[Ashok Reddy Soma]

HI Michal,

> -----Original Message-----
> From: Michal Simek <michal.simek@xilinx.com>
> Sent: Friday, July 9, 2021 4:37 PM
> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
> Cc: peng.fan@nxp.com; jh80.chung@samsung.com; git <git@xilinx.com>;
> monstr@monstr.eu; somaashokreddy@gmail.com
> Subject: Re: [PATCH v2 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays
> 
> 
> 
> On 7/9/21 12:47 PM, Ashok Reddy Soma wrote:
> > 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>
> > ---
> >
> > (no changes since v1)
> >
> >  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
> > b229c24a8b..44eb982b9a 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
> 
> when you are touching this maybe better to use BIT and GENMASK macros.
Sure,
Will change it in v3.

Thanks,
Ashok
> 
> >
> >  #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;
> >  }
> >
> 
> M

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

[Ashok Reddy Soma]

Hi Michal,

> -----Original Message-----
> From: Michal Simek <michal.simek@xilinx.com>
> Sent: Friday, July 9, 2021 4:35 PM
> To: Ashok Reddy Soma <ashokred@xilinx.com>; u-boot@lists.denx.de
> Cc: peng.fan@nxp.com; jh80.chung@samsung.com; git <git@xilinx.com>;
> monstr@monstr.eu; somaashokreddy@gmail.com
> Subject: Re: [PATCH v2 1/6] mmc: zynq_sdhci: Resolve uninitialized return value
> 
> 
> 
> On 7/9/21 12:46 PM, Ashok Reddy Soma wrote:
> > 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>
> > ---
> >
> > Changes in v2:
> >  - Changed "@degree" to "@degrees:" in function descriptions of tap
> >    delay functions
> >
> >  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..03600188ba 100644
> > --- a/drivers/mmc/zynq_sdhci.c
> > +++ b/drivers/mmc/zynq_sdhci.c
> > @@ -183,7 +183,7 @@ static int arasan_sdhci_execute_tuning(struct mmc
> *mmc, u8 opcode)
> >   *
> >   * @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_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;
> >  }
> >
> >  /**
> > @@ -239,7 +238,7 @@ static int sdhci_zynqmp_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_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;
> >  }
> >
> >  /**
> > @@ -294,15 +292,14 @@ static int
> sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host,
> >   * 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.
> 
> this should be also the part of 5/6
Sure will move it to 5/6.
> 
> > + * 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;
> >  }
> >
> >  /**
> > @@ -358,15 +355,14 @@ static int sdhci_versal_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.
> 
> this should be also the part of 5/6
will move it to 5/6 here too.

Thanks,
Ashok

> 
> > + * 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)
> >
> 
> M