All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays
@ 2021-07-09 11:53 Ashok Reddy Soma
  2021-07-09 11:53 ` [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value Ashok Reddy Soma
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, 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 v3:
 - Updated commmit description to explain why return type int is kept
 - Change "@degree" to "@degrees:" in 5/6, so revert it in 1/6 from
   sdhci_zynqmp_sampleclk_set_phase() and sdhci_versal_sdcardclk_set_phase()
 - Updated macro's with BIT() and GENMASK() for readability
 - Change "@degree" to "@degrees:" in sdhci_zynqmp_sampleclk_set_phase()
   and sdhci_versal_sdcardclk_set_phase() and update description with
   these warnings.

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


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
@ 2021-07-09 11:53 ` Ashok Reddy Soma
  2021-07-11 22:23   ` Jaehoon Chung
  2021-07-09 11:53 ` [PATCH v3 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values Ashok Reddy Soma
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, Ashok Reddy Soma

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

Keep the return type as int to return errors when the tapdelay's are
set via xilinx_pm_request() in future.

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

Changes in v3:
 - Updated commmit description to explain why return type int is kept
 - Change "@degree" to "@degrees:" in 5/6, so revert it in 1/6 from
   sdhci_zynqmp_sampleclk_set_phase() and sdhci_versal_sdcardclk_set_phase()

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

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

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index b79c4021b6..5bad5cb9e7 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;
 }
 
 /**
@@ -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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
  2021-07-09 11:53 ` [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value Ashok Reddy Soma
@ 2021-07-09 11:53 ` Ashok Reddy Soma
  2021-07-11 22:23   ` Jaehoon Chung
  2021-07-09 11:53 ` [PATCH v3 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays Ashok Reddy Soma
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, 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 5bad5cb9e7..f65a87a4e1 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
  2021-07-09 11:53 ` [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value Ashok Reddy Soma
  2021-07-09 11:53 ` [PATCH v3 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values Ashok Reddy Soma
@ 2021-07-09 11:53 ` Ashok Reddy Soma
  2021-07-11 22:24   ` Jaehoon Chung
  2021-07-09 11:53 ` [PATCH v3 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out Ashok Reddy Soma
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, 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>
---

Changes in v3:
 - Updated macro's with BIT() and GENMASK() for readability

 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 f65a87a4e1..bf638e9675 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	GENMASK(7, 0)
+#define SDHCI_ARASAN_OTAPDLY_REGISTER	0xF0FC
+#define SDHCI_ARASAN_OTAPDLY_SEL_MASK	GENMASK(5, 0)
+#define SDHCI_ITAPDLY_CHGWIN		BIT(9)
+#define SDHCI_ITAPDLY_ENABLE		BIT(8)
+#define SDHCI_OTAPDLY_ENABLE		BIT(6)
 
 #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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
                   ` (2 preceding siblings ...)
  2021-07-09 11:53 ` [PATCH v3 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays Ashok Reddy Soma
@ 2021-07-09 11:53 ` Ashok Reddy Soma
  2021-07-11 22:24   ` Jaehoon Chung
  2021-07-09 11:53 ` [PATCH v3 5/6] mmc: zynq_sdhci: Fix kernel doc warnings Ashok Reddy Soma
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, 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 bf638e9675..95d42ccef4 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 5/6] mmc: zynq_sdhci: Fix kernel doc warnings
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
                   ` (3 preceding siblings ...)
  2021-07-09 11:53 ` [PATCH v3 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out Ashok Reddy Soma
@ 2021-07-09 11:53 ` Ashok Reddy Soma
  2021-07-11 22:24   ` Jaehoon Chung
  2021-07-09 11:53 ` [PATCH v3 6/6] mmc: zynq_sdhci: Make variables/structure static Ashok Reddy Soma
  2021-07-12 11:27 ` [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Michal Simek
  6 siblings, 1 reply; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, Michal Simek,
	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: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>
---

Changes in v3:
 - Change "@degree" to "@degrees:" in sdhci_zynqmp_sampleclk_set_phase()
   and sdhci_versal_sdcardclk_set_phase() and update description with
   these warnings.

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

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

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 95d42ccef4..47862c0bf5 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.
+ * @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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH v3 6/6] mmc: zynq_sdhci: Make variables/structure static
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
                   ` (4 preceding siblings ...)
  2021-07-09 11:53 ` [PATCH v3 5/6] mmc: zynq_sdhci: Fix kernel doc warnings Ashok Reddy Soma
@ 2021-07-09 11:53 ` Ashok Reddy Soma
  2021-07-11 22:24   ` Jaehoon Chung
  2021-07-12 11:27 ` [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Michal Simek
  6 siblings, 1 reply; 14+ messages in thread
From: Ashok Reddy Soma @ 2021-07-09 11:53 UTC (permalink / raw)
  To: u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy, Michal Simek,
	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 47862c0bf5..ba87ee8dd5 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value
  2021-07-09 11:53 ` [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value Ashok Reddy Soma
@ 2021-07-11 22:23   ` Jaehoon Chung
  0 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-07-11 22:23 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot; +Cc: peng.fan, git, monstr, somaashokreddy

On 7/9/21 8:53 PM, Ashok Reddy Soma wrote:
> set_phase() functions are not modifying the ret value and returning
> the same uninitialized ret, return 0 instead.
> 
> Keep the return type as int to return errors when the tapdelay's are
> set via xilinx_pm_request() in future.
> 
> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> Changes in v3:
>  - Updated commmit description to explain why return type int is kept
>  - Change "@degree" to "@degrees:" in 5/6, so revert it in 1/6 from
>    sdhci_zynqmp_sampleclk_set_phase() and sdhci_versal_sdcardclk_set_phase()
> 
> Changes in v2:
>  - Changed "@degree" to "@degrees:" in function descriptions of tap
>    delay functions
> 
>  drivers/mmc/zynq_sdhci.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index b79c4021b6..5bad5cb9e7 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;
>  }
>  
>  /**
> @@ -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)
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values
  2021-07-09 11:53 ` [PATCH v3 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values Ashok Reddy Soma
@ 2021-07-11 22:23   ` Jaehoon Chung
  0 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-07-11 22:23 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot; +Cc: peng.fan, git, monstr, somaashokreddy

On 7/9/21 8:53 PM, Ashok Reddy Soma wrote:
> 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>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (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 5bad5cb9e7..f65a87a4e1 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) {
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays
  2021-07-09 11:53 ` [PATCH v3 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays Ashok Reddy Soma
@ 2021-07-11 22:24   ` Jaehoon Chung
  0 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-07-11 22:24 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot; +Cc: peng.fan, git, monstr, somaashokreddy

On 7/9/21 8:53 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>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> Changes in v3:
>  - Updated macro's with BIT() and GENMASK() for readability
> 
>  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 f65a87a4e1..bf638e9675 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	GENMASK(7, 0)
> +#define SDHCI_ARASAN_OTAPDLY_REGISTER	0xF0FC
> +#define SDHCI_ARASAN_OTAPDLY_SEL_MASK	GENMASK(5, 0)
> +#define SDHCI_ITAPDLY_CHGWIN		BIT(9)
> +#define SDHCI_ITAPDLY_ENABLE		BIT(8)
> +#define SDHCI_OTAPDLY_ENABLE		BIT(6)
>  
>  #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;
>  }
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out
  2021-07-09 11:53 ` [PATCH v3 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out Ashok Reddy Soma
@ 2021-07-11 22:24   ` Jaehoon Chung
  0 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-07-11 22:24 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot; +Cc: peng.fan, git, monstr, somaashokreddy

On 7/9/21 8:53 PM, Ashok Reddy Soma wrote:
> 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>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (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 bf638e9675..95d42ccef4 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
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 5/6] mmc: zynq_sdhci: Fix kernel doc warnings
  2021-07-09 11:53 ` [PATCH v3 5/6] mmc: zynq_sdhci: Fix kernel doc warnings Ashok Reddy Soma
@ 2021-07-11 22:24   ` Jaehoon Chung
  0 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-07-11 22:24 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot
  Cc: peng.fan, git, monstr, somaashokreddy, Michal Simek

On 7/9/21 8:53 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:236: warning: contents before sections
> 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>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> Changes in v3:
>  - Change "@degree" to "@degrees:" in sdhci_zynqmp_sampleclk_set_phase()
>    and sdhci_versal_sdcardclk_set_phase() and update description with
>    these warnings.
> 
> Changes in v2:
>  - Removed @degree warning from commit description since it is fixed in
>    patch 1/6.
> 
>  drivers/mmc/zynq_sdhci.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 95d42ccef4..47862c0bf5 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.
> + * @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)
>  {
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 6/6] mmc: zynq_sdhci: Make variables/structure static
  2021-07-09 11:53 ` [PATCH v3 6/6] mmc: zynq_sdhci: Make variables/structure static Ashok Reddy Soma
@ 2021-07-11 22:24   ` Jaehoon Chung
  0 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-07-11 22:24 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot
  Cc: peng.fan, git, monstr, somaashokreddy, Michal Simek

On 7/9/21 8:53 PM, Ashok Reddy Soma wrote:
> 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>


Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (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 47862c0bf5..ba87ee8dd5 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,
>  };
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays
  2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
                   ` (5 preceding siblings ...)
  2021-07-09 11:53 ` [PATCH v3 6/6] mmc: zynq_sdhci: Make variables/structure static Ashok Reddy Soma
@ 2021-07-12 11:27 ` Michal Simek
  6 siblings, 0 replies; 14+ messages in thread
From: Michal Simek @ 2021-07-12 11:27 UTC (permalink / raw)
  To: Ashok Reddy Soma, u-boot
  Cc: peng.fan, jh80.chung, git, monstr, somaashokreddy



On 7/9/21 1:53 PM, Ashok Reddy Soma wrote:
> 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 v3:
>  - Updated commmit description to explain why return type int is kept
>  - Change "@degree" to "@degrees:" in 5/6, so revert it in 1/6 from
>    sdhci_zynqmp_sampleclk_set_phase() and sdhci_versal_sdcardclk_set_phase()
>  - Updated macro's with BIT() and GENMASK() for readability
>  - Change "@degree" to "@degrees:" in sdhci_zynqmp_sampleclk_set_phase()
>    and sdhci_versal_sdcardclk_set_phase() and update description with
>    these warnings.
> 
> 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(-)
> 

Applied all.
M

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-07-12 11:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-09 11:53 [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Ashok Reddy Soma
2021-07-09 11:53 ` [PATCH v3 1/6] mmc: zynq_sdhci: Resolve uninitialized return value Ashok Reddy Soma
2021-07-11 22:23   ` Jaehoon Chung
2021-07-09 11:53 ` [PATCH v3 2/6] mmc: zynq_sdhci: Allow configuring zero Tap values Ashok Reddy Soma
2021-07-11 22:23   ` Jaehoon Chung
2021-07-09 11:53 ` [PATCH v3 3/6] mmc: zynq_sdhci: Use Mask writes for Tap delays Ashok Reddy Soma
2021-07-11 22:24   ` Jaehoon Chung
2021-07-09 11:53 ` [PATCH v3 4/6] mmc: zynq_sdhci: Split set_tapdelay function to in and out Ashok Reddy Soma
2021-07-11 22:24   ` Jaehoon Chung
2021-07-09 11:53 ` [PATCH v3 5/6] mmc: zynq_sdhci: Fix kernel doc warnings Ashok Reddy Soma
2021-07-11 22:24   ` Jaehoon Chung
2021-07-09 11:53 ` [PATCH v3 6/6] mmc: zynq_sdhci: Make variables/structure static Ashok Reddy Soma
2021-07-11 22:24   ` Jaehoon Chung
2021-07-12 11:27 ` [PATCH v3 0/6] This patch set fixes minor issues related to tapdelays Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.