* [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree
@ 2024-02-23 14:06 Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings Steffen Dirkwinkel
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-02-23 14:06 UTC (permalink / raw
To: u-boot
Cc: Steffen Dirkwinkel, Ashok Reddy Soma, Marcel Ziswiler,
Michal Simek, Simon Glass, Tom Rini, Venkatesh Yadav Abbarapu
From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
These are used during sdhci initialization as functions are called with
node_id even if we're not talking to the firmware.
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
configs/xilinx_zynqmp_virt_defconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig
index 1fcae45e95d..a96b088534a 100644
--- a/configs/xilinx_zynqmp_virt_defconfig
+++ b/configs/xilinx_zynqmp_virt_defconfig
@@ -107,7 +107,7 @@ CONFIG_PARTITION_TYPE_GUID=y
CONFIG_SPL_OF_CONTROL=y
CONFIG_OF_BOARD=y
CONFIG_OF_LIST="avnet-ultra96-rev1 zynqmp-a2197-revA zynqmp-e-a2197-00-revA zynqmp-e-a2197-00-revB zynqmp-g-a2197-00-revA zynqmp-m-a2197-01-revA zynqmp-m-a2197-02-revA zynqmp-m-a2197-03-revA zynqmp-p-a2197-00-revA zynqmp-zc1232-revA zynqmp-zc1254-revA zynqmp-zc1751-xm015-dc1 zynqmp-zc1751-xm016-dc2 zynqmp-zc1751-xm017-dc3 zynqmp-zc1751-xm018-dc4 zynqmp-zc1751-xm019-dc5 zynqmp-zcu100-revC zynqmp-zcu102-rev1.1 zynqmp-zcu102-rev1.0 zynqmp-zcu102-revA zynqmp-zcu102-revB zynqmp-zcu104-revA zynqmp-zcu104-revC zynqmp-zcu106-revA zynqmp-zcu106-rev1.0 zynqmp-zcu111-revA zynqmp-zcu1275-revA zynqmp-zcu1275-revB zynqmp-zcu1285-revA zynqmp-zcu208-revA zynqmp-zcu216-revA zynqmp-topic-miamimp-xilinx-xdp-v1r1 zynqmp-sm-k26-revA zynqmp-smk-k26-revA zynqmp-dlc21-revA"
-CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names interrupt-parent interrupts iommus power-domains"
+CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names interrupt-parent interrupts iommus"
CONFIG_ENV_IS_NOWHERE=y
CONFIG_ENV_IS_IN_FAT=y
CONFIG_ENV_IS_IN_NAND=y
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
2024-02-23 14:06 [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Steffen Dirkwinkel
@ 2024-02-23 14:06 ` Steffen Dirkwinkel
2024-03-14 6:45 ` Love Kumar
2024-02-23 14:06 ` [PATCH 3/5] mmc: zynq_sdhci: disable ITAPDLYENA for zero itap_delay Steffen Dirkwinkel
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-02-23 14:06 UTC (permalink / raw
To: u-boot
Cc: Steffen Dirkwinkel, Algapally Santosh Sagar, Ashok Reddy Soma,
Jaehoon Chung, Johan Jonker, Michal Simek, Peng Fan, Tom Rini
From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
Previously we were setting in tapdelay for SD1 every time even if SD0 was
requested.
The SD tapdelay settings are shifted by 16 bits between SD0 and SD1.
We can use that to make our tapdelay setup simpler. This is also how it
currently works in arm-trusted-firmware.
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
drivers/mmc/zynq_sdhci.c | 65 +++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 37 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 935540d1719..d4845245b2a 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -42,14 +42,11 @@
#define SD_OTAP_DLY 0xFF180318
#define SD0_DLL_RST BIT(2)
#define SD1_DLL_RST BIT(18)
+#define SD1_TAP_OFFSET 16
#define SD0_ITAPCHGWIN BIT(9)
-#define SD1_ITAPCHGWIN BIT(25)
#define SD0_ITAPDLYENA BIT(8)
-#define SD1_ITAPDLYENA BIT(24)
#define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
-#define SD1_ITAPDLYSEL_MASK GENMASK(23, 16)
#define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
-#define SD1_OTAPDLYSEL_MASK GENMASK(21, 16)
#define MIN_PHY_CLK_HZ 50000000
@@ -275,44 +272,32 @@ static int arasan_sdhci_config_dll(struct sdhci_host *host, unsigned int clock,
static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
{
int ret;
+ u32 shift;
- if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
- if (node_id == NODE_SD_0) {
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN,
- SD0_ITAPCHGWIN);
- if (ret)
- return ret;
-
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA,
- SD0_ITAPDLYENA);
- if (ret)
- return ret;
-
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK,
- itap_delay);
- if (ret)
- return ret;
+ if (node_id == NODE_SD_0)
+ shift = 0;
+ else if (node_id == NODE_SD_1)
+ shift = SD1_TAP_OFFSET;
+ else
+ return -EINVAL;
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN, 0);
- if (ret)
- return ret;
- }
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN,
- SD1_ITAPCHGWIN);
+ if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
+ ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN << shift,
+ SD0_ITAPCHGWIN << shift);
if (ret)
return ret;
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA,
- SD1_ITAPDLYENA);
+ ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA << shift,
+ SD0_ITAPDLYENA << shift);
if (ret)
return ret;
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
- (itap_delay << 16));
+ ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK << shift,
+ itap_delay << shift);
if (ret)
return ret;
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN, 0);
+ ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN << shift, 0);
if (ret)
return ret;
} else {
@@ -326,14 +311,20 @@ static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id, u32 otap_delay)
{
+ u32 shift;
+
+ if (node_id == NODE_SD_0)
+ shift = 0;
+ else if (node_id == NODE_SD_1)
+ shift = SD1_TAP_OFFSET;
+ else
+ return -EINVAL;
+
if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
- if (node_id == NODE_SD_0)
- return zynqmp_mmio_write(SD_OTAP_DLY,
- SD0_OTAPDLYSEL_MASK,
- otap_delay);
+ return zynqmp_mmio_write(SD_OTAP_DLY,
+ SD0_OTAPDLYSEL_MASK << shift,
+ otap_delay << shift);
- return zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
- (otap_delay << 16));
} else {
return xilinx_pm_request(PM_IOCTL, node_id,
IOCTL_SET_SD_TAPDELAY,
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] mmc: zynq_sdhci: disable ITAPDLYENA for zero itap_delay
2024-02-23 14:06 [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings Steffen Dirkwinkel
@ 2024-02-23 14:06 ` Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 4/5] mmc: zynq_sdhci: disable OTAPDLYENA Steffen Dirkwinkel
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-02-23 14:06 UTC (permalink / raw
To: u-boot
Cc: Steffen Dirkwinkel, Algapally Santosh Sagar, Ashok Reddy Soma,
Jaehoon Chung, Johan Jonker, Michal Simek, Peng Fan, Tom Rini
From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
This ports a change from arm-trusted-firmware:
commit: fe1fa205fca4d1dd4a1b1755942956dbca65d573 in arm-trusted-firmware
(https://github.com/ARM-software/arm-trusted-firmware/commit/fe1fa205fca4d1dd4a1b1755942956dbca65d573)
The OTAPDLYENA change from there will be in another commit.
We shouldn't have different behavior for u-boot-SPL and u-boot so let's
use the same logic here.
Message from arm-trusted-firmware:
plat: zynqmp: Disable ITAPDLYENA bit for zero ITAP delay
This patch disable the ITAPDLYENA bit for ITAP delay value zero.
As per IP design, it is recommended to disable the ITAPDLYENA bit
before auto-tuning.
Also disable OTAPDLYENA bit always as there is one issue in RTL
where SD0_OTAPDLYENA has been wrongly connected to both SD0 and SD1
controllers. Hence it is recommended to disable OTAPDLYENA bit always
for both the controllers.
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
drivers/mmc/zynq_sdhci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index d4845245b2a..79bb8ba66d9 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -287,8 +287,13 @@ static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
if (ret)
return ret;
- ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA << shift,
- SD0_ITAPDLYENA << shift);
+ if (itap_delay == 0)
+ ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA << shift,
+ 0);
+ else
+ ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA << shift,
+ SD0_ITAPDLYENA << shift);
+
if (ret)
return ret;
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] mmc: zynq_sdhci: disable OTAPDLYENA
2024-02-23 14:06 [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 3/5] mmc: zynq_sdhci: disable ITAPDLYENA for zero itap_delay Steffen Dirkwinkel
@ 2024-02-23 14:06 ` Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 5/5] zynqmp: boot without delay in psu_uboot_init Steffen Dirkwinkel
2024-03-01 7:11 ` [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Michal Simek
4 siblings, 0 replies; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-02-23 14:06 UTC (permalink / raw
To: u-boot
Cc: Steffen Dirkwinkel, Algapally Santosh Sagar, Ashok Reddy Soma,
Jaehoon Chung, Johan Jonker, Michal Simek, Peng Fan, Simon Glass,
Tom Rini
From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
This ports a change from arm-trusted-firmware:
commit: fe1fa205fca4d1dd4a1b1755942956dbca65d573 in arm-trusted-firmware
(https://github.com/ARM-software/arm-trusted-firmware/commit/fe1fa205fca4d1dd4a1b1755942956dbca65d573)
The ITAPDLYENA change is in another commit.
We shouldn't have different behavior for u-boot-SPL and u-boot so let's
use the same logic here.
Message from arm-trusted-firmware:
plat: zynqmp: Disable ITAPDLYENA bit for zero ITAP delay
This patch disable the ITAPDLYENA bit for ITAP delay value zero.
As per IP design, it is recommended to disable the ITAPDLYENA bit
before auto-tuning.
Also disable OTAPDLYENA bit always as there is one issue in RTL
where SD0_OTAPDLYENA has been wrongly connected to both SD0 and SD1
controllers. Hence it is recommended to disable OTAPDLYENA bit always
for both the controllers.
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
drivers/mmc/zynq_sdhci.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 79bb8ba66d9..06b782eb79a 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -45,6 +45,7 @@
#define SD1_TAP_OFFSET 16
#define SD0_ITAPCHGWIN BIT(9)
#define SD0_ITAPDLYENA BIT(8)
+#define SD0_OTAPDLYENA BIT(6)
#define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
#define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
@@ -316,6 +317,7 @@ static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id, u32 otap_delay)
{
+ int ret;
u32 shift;
if (node_id == NODE_SD_0)
@@ -326,6 +328,12 @@ static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id, u32 otap_delay)
return -EINVAL;
if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
+ ret = zynqmp_mmio_write(SD_OTAP_DLY,
+ SD0_OTAPDLYENA << shift,
+ 0);
+ if (ret)
+ return ret;
+
return zynqmp_mmio_write(SD_OTAP_DLY,
SD0_OTAPDLYSEL_MASK << shift,
otap_delay << shift);
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] zynqmp: boot without delay in psu_uboot_init
2024-02-23 14:06 [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Steffen Dirkwinkel
` (2 preceding siblings ...)
2024-02-23 14:06 ` [PATCH 4/5] mmc: zynq_sdhci: disable OTAPDLYENA Steffen Dirkwinkel
@ 2024-02-23 14:06 ` Steffen Dirkwinkel
2024-03-01 7:11 ` [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Michal Simek
4 siblings, 0 replies; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-02-23 14:06 UTC (permalink / raw
To: u-boot; +Cc: Steffen Dirkwinkel, Michal Simek, Tom Rini,
Venkatesh Yadav Abbarapu
From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
This seems to have been required for sd init. I don't need it anymore
with correct tapdelay / dll reset handling, but can't be sure this works
on all platforms.
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
board/xilinx/zynqmp/zynqmp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index ba49eb7be22..b36c72c7e54 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -72,9 +72,6 @@ int __maybe_unused psu_uboot_init(void)
writel(ZYNQMP_PS_SYSMON_ANALOG_BUS_VAL,
ZYNQMP_AMS_PS_SYSMON_ANALOG_BUS);
- /* Delay is required for clocks to be propagated */
- udelay(1000000);
-
return 0;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree
2024-02-23 14:06 [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Steffen Dirkwinkel
` (3 preceding siblings ...)
2024-02-23 14:06 ` [PATCH 5/5] zynqmp: boot without delay in psu_uboot_init Steffen Dirkwinkel
@ 2024-03-01 7:11 ` Michal Simek
4 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2024-03-01 7:11 UTC (permalink / raw
To: Steffen Dirkwinkel, u-boot
Cc: Steffen Dirkwinkel, Ashok Reddy Soma, Marcel Ziswiler,
Simon Glass, Tom Rini, Venkatesh Yadav Abbarapu
On 2/23/24 15:06, Steffen Dirkwinkel wrote:
> From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
>
> These are used during sdhci initialization as functions are called with
> node_id even if we're not talking to the firmware.
>
> Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> ---
>
> configs/xilinx_zynqmp_virt_defconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig
> index 1fcae45e95d..a96b088534a 100644
> --- a/configs/xilinx_zynqmp_virt_defconfig
> +++ b/configs/xilinx_zynqmp_virt_defconfig
> @@ -107,7 +107,7 @@ CONFIG_PARTITION_TYPE_GUID=y
> CONFIG_SPL_OF_CONTROL=y
> CONFIG_OF_BOARD=y
> CONFIG_OF_LIST="avnet-ultra96-rev1 zynqmp-a2197-revA zynqmp-e-a2197-00-revA zynqmp-e-a2197-00-revB zynqmp-g-a2197-00-revA zynqmp-m-a2197-01-revA zynqmp-m-a2197-02-revA zynqmp-m-a2197-03-revA zynqmp-p-a2197-00-revA zynqmp-zc1232-revA zynqmp-zc1254-revA zynqmp-zc1751-xm015-dc1 zynqmp-zc1751-xm016-dc2 zynqmp-zc1751-xm017-dc3 zynqmp-zc1751-xm018-dc4 zynqmp-zc1751-xm019-dc5 zynqmp-zcu100-revC zynqmp-zcu102-rev1.1 zynqmp-zcu102-rev1.0 zynqmp-zcu102-revA zynqmp-zcu102-revB zynqmp-zcu104-revA zynqmp-zcu104-revC zynqmp-zcu106-revA zynqmp-zcu106-rev1.0 zynqmp-zcu111-revA zynqmp-zcu1275-revA zynqmp-zcu1275-revB zynqmp-zcu1285-revA zynqmp-zcu208-revA zynqmp-zcu216-revA zynqmp-topic-miamimp-xilinx-xdp-v1r1 zynqmp-sm-k26-revA zynqmp-smk-k26-revA zynqmp-dlc21-revA"
> -CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names interrupt-parent interrupts iommus power-domains"
> +CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names interrupt-parent interrupts iommus"
> CONFIG_ENV_IS_NOWHERE=y
> CONFIG_ENV_IS_IN_FAT=y
> CONFIG_ENV_IS_IN_NAND=y
just FYI. I have asked my colleague to take this to regression and see if there
is any issue. I will let you know when I know more.
Thanks,
Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
2024-02-23 14:06 ` [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings Steffen Dirkwinkel
@ 2024-03-14 6:45 ` Love Kumar
2024-03-14 7:52 ` Steffen Dirkwinkel
0 siblings, 1 reply; 10+ messages in thread
From: Love Kumar @ 2024-03-14 6:45 UTC (permalink / raw
To: Steffen Dirkwinkel, u-boot
Cc: Steffen Dirkwinkel, Algapally Santosh Sagar, Ashok Reddy Soma,
Jaehoon Chung, Johan Jonker, Michal Simek, Peng Fan, Tom Rini,
venkatesh.abbarapu
Hi,
When we run in el3 for zynqmp board, we are seeing below issue with this
patch:
Model: ZynqMP MINI EMMC0
Board: Xilinx ZynqMP
DRAM: 512 MiB
EL Level: EL3
Secure Boot: not authenticated, not encrypted
Multiboot: 0
Core: 10 devices, 9 uclasses, devicetree: embed
MMC: sdhci@ff160000: 0
Loading Environment from <NULL>... OK
In: dcc
Out: dcc
Err: dcc
ZynqMP> mmc list
sdhci@ff160000: 0
ZynqMP> mmc dev 0 0
arasan_sdhci sdhci@ff160000: Error setting Input Tap Delay
sdhci_set_clock: Error while setting tap delay
sdhci_send_command: Timeout for status update: 00000000 00000001
ZynqMP>
Regards,
Love Kumar
On 23/02/24 7:36 pm, Steffen Dirkwinkel wrote:
> From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
>
> Previously we were setting in tapdelay for SD1 every time even if SD0 was
> requested.
> The SD tapdelay settings are shifted by 16 bits between SD0 and SD1.
> We can use that to make our tapdelay setup simpler. This is also how it
> currently works in arm-trusted-firmware.
>
> Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> ---
>
> drivers/mmc/zynq_sdhci.c | 65 +++++++++++++++++-----------------------
> 1 file changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 935540d1719..d4845245b2a 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -42,14 +42,11 @@
> #define SD_OTAP_DLY 0xFF180318
> #define SD0_DLL_RST BIT(2)
> #define SD1_DLL_RST BIT(18)
> +#define SD1_TAP_OFFSET 16
> #define SD0_ITAPCHGWIN BIT(9)
> -#define SD1_ITAPCHGWIN BIT(25)
> #define SD0_ITAPDLYENA BIT(8)
> -#define SD1_ITAPDLYENA BIT(24)
> #define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
> -#define SD1_ITAPDLYSEL_MASK GENMASK(23, 16)
> #define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
> -#define SD1_OTAPDLYSEL_MASK GENMASK(21, 16)
>
> #define MIN_PHY_CLK_HZ 50000000
>
> @@ -275,44 +272,32 @@ static int arasan_sdhci_config_dll(struct sdhci_host *host, unsigned int clock,
> static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
> {
> int ret;
> + u32 shift;
>
> - if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> - if (node_id == NODE_SD_0) {
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN,
> - SD0_ITAPCHGWIN);
> - if (ret)
> - return ret;
> -
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA,
> - SD0_ITAPDLYENA);
> - if (ret)
> - return ret;
> -
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK,
> - itap_delay);
> - if (ret)
> - return ret;
> + if (node_id == NODE_SD_0)
> + shift = 0;
> + else if (node_id == NODE_SD_1)
> + shift = SD1_TAP_OFFSET;
> + else
> + return -EINVAL;
>
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN, 0);
> - if (ret)
> - return ret;
> - }
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN,
> - SD1_ITAPCHGWIN);
> + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN << shift,
> + SD0_ITAPCHGWIN << shift);
> if (ret)
> return ret;
>
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA,
> - SD1_ITAPDLYENA);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA << shift,
> + SD0_ITAPDLYENA << shift);
> if (ret)
> return ret;
>
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
> - (itap_delay << 16));
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK << shift,
> + itap_delay << shift);
> if (ret)
> return ret;
>
> - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN, 0);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN << shift, 0);
> if (ret)
> return ret;
> } else {
> @@ -326,14 +311,20 @@ static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
>
> static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id, u32 otap_delay)
> {
> + u32 shift;
> +
> + if (node_id == NODE_SD_0)
> + shift = 0;
> + else if (node_id == NODE_SD_1)
> + shift = SD1_TAP_OFFSET;
> + else
> + return -EINVAL;
> +
> if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> - if (node_id == NODE_SD_0)
> - return zynqmp_mmio_write(SD_OTAP_DLY,
> - SD0_OTAPDLYSEL_MASK,
> - otap_delay);
> + return zynqmp_mmio_write(SD_OTAP_DLY,
> + SD0_OTAPDLYSEL_MASK << shift,
> + otap_delay << shift);
>
> - return zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
> - (otap_delay << 16));
> } else {
> return xilinx_pm_request(PM_IOCTL, node_id,
> IOCTL_SET_SD_TAPDELAY,
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
2024-03-14 6:45 ` Love Kumar
@ 2024-03-14 7:52 ` Steffen Dirkwinkel
2024-03-15 9:52 ` Abbarapu, Venkatesh
0 siblings, 1 reply; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-03-14 7:52 UTC (permalink / raw
To: Love Kumar, u-boot
Cc: Algapally Santosh Sagar, Ashok Reddy Soma, Jaehoon Chung,
Johan Jonker, Michal Simek, Peng Fan, Tom Rini,
venkatesh.abbarapu
Hi Love,
On Thu, 2024-03-14 at 12:15 +0530, Love Kumar wrote:
> Hi,
>
> When we run in el3 for zynqmp board, we are seeing below issue with this
> patch:
>
>
> Model: ZynqMP MINI EMMC0
> Board: Xilinx ZynqMP
> DRAM: 512 MiB
> EL Level: EL3
> Secure Boot: not authenticated, not encrypted
> Multiboot: 0
> Core: 10 devices, 9 uclasses, devicetree: embed
> MMC: sdhci@ff160000: 0
> Loading Environment from <NULL>... OK
> In: dcc
> Out: dcc
> Err: dcc
> ZynqMP> mmc list
> sdhci@ff160000: 0
> ZynqMP> mmc dev 0 0
> arasan_sdhci sdhci@ff160000: Error setting Input Tap Delay
> sdhci_set_clock: Error while setting tap delay
> sdhci_send_command: Timeout for status update: 00000000 00000001
> ZynqMP>
Thank you for testing this.
It looks like the zynqmp-mini-emmc0 device lacks the power-domain node and doesn't have
a sd node id. The code before my change would have ignored the unknown node id, applied
settings for sdhci1 instead of sdhci0 and returned without an error. Maybe there's a different
way to find out which sdhci we want to operate on?
Can you try setting the node id in device tree?
Something like this:
diff --git a/arch/arm/dts/zynqmp-mini-emmc0.dts b/arch/arm/dts/zynqmp-mini-emmc0.dts
index 02e80bd85e1..87c4a1b6ad0 100644
--- a/arch/arm/dts/zynqmp-mini-emmc0.dts
+++ b/arch/arm/dts/zynqmp-mini-emmc0.dts
@@ -7,6 +7,8 @@
* Siva Durga Prasad Paladugu <siva.durga.prasad.paladugu@amd.com>
*/
+#include <dt-bindings/power/xlnx-zynqmp-power.h>
+
/dts-v1/;
/ {
@@ -41,6 +43,12 @@
clock-frequency = <200000000>;
};
+ firmware {
+ zynqmp_firmware: zynqmp-firmware {
+ #power-domain-cells = <1>;
+ };
+ };
+
amba: amba {
compatible = "simple-bus";
#address-cells = <2>;
@@ -56,6 +64,7 @@
reg = <0x0 0xff160000 0x0 0x1000>;
clock-names = "clk_xin", "clk_ahb";
clocks = <&clk_xin &clk_xin>;
+ power-domains = <&zynqmp_firmware PD_SD_0>;
};
};
};
Thanks,
Steffen
>
> Regards,
> Love Kumar
>
> On 23/02/24 7:36 pm, Steffen Dirkwinkel wrote:
> > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> >
> > Previously we were setting in tapdelay for SD1 every time even if SD0 was
> > requested.
> > The SD tapdelay settings are shifted by 16 bits between SD0 and SD1.
> > We can use that to make our tapdelay setup simpler. This is also how it
> > currently works in arm-trusted-firmware.
> >
> > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> > ---
> >
> > drivers/mmc/zynq_sdhci.c | 65 +++++++++++++++++-----------------------
> > 1 file changed, 28 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> > index 935540d1719..d4845245b2a 100644
> > --- a/drivers/mmc/zynq_sdhci.c
> > +++ b/drivers/mmc/zynq_sdhci.c
> > @@ -42,14 +42,11 @@
> > #define SD_OTAP_DLY 0xFF180318
> > #define SD0_DLL_RST BIT(2)
> > #define SD1_DLL_RST BIT(18)
> > +#define SD1_TAP_OFFSET 16
> > #define SD0_ITAPCHGWIN BIT(9)
> > -#define SD1_ITAPCHGWIN BIT(25)
> > #define SD0_ITAPDLYENA BIT(8)
> > -#define SD1_ITAPDLYENA BIT(24)
> > #define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
> > -#define SD1_ITAPDLYSEL_MASK GENMASK(23, 16)
> > #define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
> > -#define SD1_OTAPDLYSEL_MASK GENMASK(21, 16)
> >
> > #define MIN_PHY_CLK_HZ 50000000
> >
> > @@ -275,44 +272,32 @@ static int arasan_sdhci_config_dll(struct sdhci_host *host, unsigned int clock,
> > static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
> > {
> > int ret;
> > + u32 shift;
> >
> > - if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > - if (node_id == NODE_SD_0) {
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN,
> > - SD0_ITAPCHGWIN);
> > - if (ret)
> > - return ret;
> > -
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA,
> > - SD0_ITAPDLYENA);
> > - if (ret)
> > - return ret;
> > -
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK,
> > - itap_delay);
> > - if (ret)
> > - return ret;
> > + if (node_id == NODE_SD_0)
> > + shift = 0;
> > + else if (node_id == NODE_SD_1)
> > + shift = SD1_TAP_OFFSET;
> > + else
> > + return -EINVAL;
> >
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN, 0);
> > - if (ret)
> > - return ret;
> > - }
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN,
> > - SD1_ITAPCHGWIN);
> > + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN << shift,
> > + SD0_ITAPCHGWIN << shift);
> > if (ret)
> > return ret;
> >
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA,
> > - SD1_ITAPDLYENA);
> > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA << shift,
> > + SD0_ITAPDLYENA << shift);
> > if (ret)
> > return ret;
> >
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
> > - (itap_delay << 16));
> > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK << shift,
> > + itap_delay << shift);
> > if (ret)
> > return ret;
> >
> > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN, 0);
> > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN << shift, 0);
> > if (ret)
> > return ret;
> > } else {
> > @@ -326,14 +311,20 @@ static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
> >
> > static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id, u32 otap_delay)
> > {
> > + u32 shift;
> > +
> > + if (node_id == NODE_SD_0)
> > + shift = 0;
> > + else if (node_id == NODE_SD_1)
> > + shift = SD1_TAP_OFFSET;
> > + else
> > + return -EINVAL;
> > +
> > if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > - if (node_id == NODE_SD_0)
> > - return zynqmp_mmio_write(SD_OTAP_DLY,
> > - SD0_OTAPDLYSEL_MASK,
> > - otap_delay);
> > + return zynqmp_mmio_write(SD_OTAP_DLY,
> > + SD0_OTAPDLYSEL_MASK << shift,
> > + otap_delay << shift);
> >
> > - return zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
> > - (otap_delay << 16));
> > } else {
> > return xilinx_pm_request(PM_IOCTL, node_id,
> > IOCTL_SET_SD_TAPDELAY,
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
2024-03-14 7:52 ` Steffen Dirkwinkel
@ 2024-03-15 9:52 ` Abbarapu, Venkatesh
2024-03-22 7:58 ` Steffen Dirkwinkel
0 siblings, 1 reply; 10+ messages in thread
From: Abbarapu, Venkatesh @ 2024-03-15 9:52 UTC (permalink / raw
To: Steffen Dirkwinkel, Kumar, Love, u-boot@lists.denx.de
Cc: Algapally Santosh Sagar, Ashok Reddy Soma, Jaehoon Chung,
Johan Jonker, Simek, Michal, Peng Fan, Tom Rini
Hi Steffen,
For mini u-boot cases ZYNQMP_FIRMWARE config is disabled as there won't be any ATF(TF-A) present.
Thanks
Venkatesh
> -----Original Message-----
> From: Steffen Dirkwinkel <lists@steffen.cc>
> Sent: Thursday, March 14, 2024 1:23 PM
> To: Kumar, Love <love.kumar@amd.com>; u-boot@lists.denx.de
> Cc: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>; Ashok
> Reddy Soma <ashok.reddy.soma@amd.com>; Jaehoon Chung
> <jh80.chung@samsung.com>; Johan Jonker <jbx6244@gmail.com>; Simek,
> Michal <michal.simek@amd.com>; Peng Fan <peng.fan@nxp.com>; Tom Rini
> <trini@konsulko.com>; Abbarapu, Venkatesh
> <venkatesh.abbarapu@amd.com>
> Subject: Re: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
>
> Hi Love,
>
> On Thu, 2024-03-14 at 12:15 +0530, Love Kumar wrote:
> > Hi,
> >
> > When we run in el3 for zynqmp board, we are seeing below issue with
> > this
> > patch:
> >
> >
> > Model: ZynqMP MINI EMMC0
> > Board: Xilinx ZynqMP
> > DRAM: 512 MiB
> > EL Level: EL3
> > Secure Boot: not authenticated, not encrypted
> > Multiboot: 0
> > Core: 10 devices, 9 uclasses, devicetree: embed
> > MMC: sdhci@ff160000: 0
> > Loading Environment from <NULL>... OK
> > In: dcc
> > Out: dcc
> > Err: dcc
> > ZynqMP> mmc list
> > sdhci@ff160000: 0
> > ZynqMP> mmc dev 0 0
> > arasan_sdhci sdhci@ff160000: Error setting Input Tap Delay
> > sdhci_set_clock: Error while setting tap delay
> > sdhci_send_command: Timeout for status update: 00000000 00000001
> > ZynqMP>
>
> Thank you for testing this.
> It looks like the zynqmp-mini-emmc0 device lacks the power-domain node
> and doesn't have a sd node id. The code before my change would have
> ignored the unknown node id, applied settings for sdhci1 instead of sdhci0
> and returned without an error. Maybe there's a different way to find out
> which sdhci we want to operate on?
>
> Can you try setting the node id in device tree?
> Something like this:
>
> diff --git a/arch/arm/dts/zynqmp-mini-emmc0.dts b/arch/arm/dts/zynqmp-
> mini-emmc0.dts
> index 02e80bd85e1..87c4a1b6ad0 100644
> --- a/arch/arm/dts/zynqmp-mini-emmc0.dts
> +++ b/arch/arm/dts/zynqmp-mini-emmc0.dts
> @@ -7,6 +7,8 @@
> * Siva Durga Prasad Paladugu <siva.durga.prasad.paladugu@amd.com>
> */
>
> +#include <dt-bindings/power/xlnx-zynqmp-power.h>
> +
> /dts-v1/;
>
> / {
> @@ -41,6 +43,12 @@
> clock-frequency = <200000000>;
> };
>
> + firmware {
> + zynqmp_firmware: zynqmp-firmware {
> + #power-domain-cells = <1>;
> + };
> + };
> +
> amba: amba {
> compatible = "simple-bus";
> #address-cells = <2>;
> @@ -56,6 +64,7 @@
> reg = <0x0 0xff160000 0x0 0x1000>;
> clock-names = "clk_xin", "clk_ahb";
> clocks = <&clk_xin &clk_xin>;
> + power-domains = <&zynqmp_firmware PD_SD_0>;
> };
> };
> };
>
> Thanks,
> Steffen
>
> >
> > Regards,
> > Love Kumar
> >
> > On 23/02/24 7:36 pm, Steffen Dirkwinkel wrote:
> > > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> > >
> > > Previously we were setting in tapdelay for SD1 every time even if
> > > SD0 was requested.
> > > The SD tapdelay settings are shifted by 16 bits between SD0 and SD1.
> > > We can use that to make our tapdelay setup simpler. This is also how
> > > it currently works in arm-trusted-firmware.
> > >
> > > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> > > ---
> > >
> > > drivers/mmc/zynq_sdhci.c | 65
> > > +++++++++++++++++-----------------------
> > > 1 file changed, 28 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> > > index 935540d1719..d4845245b2a 100644
> > > --- a/drivers/mmc/zynq_sdhci.c
> > > +++ b/drivers/mmc/zynq_sdhci.c
> > > @@ -42,14 +42,11 @@
> > > #define SD_OTAP_DLY 0xFF180318
> > > #define SD0_DLL_RST BIT(2)
> > > #define SD1_DLL_RST BIT(18)
> > > +#define SD1_TAP_OFFSET 16
> > > #define SD0_ITAPCHGWIN BIT(9)
> > > -#define SD1_ITAPCHGWIN BIT(25)
> > > #define SD0_ITAPDLYENA BIT(8)
> > > -#define SD1_ITAPDLYENA BIT(24)
> > > #define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
> > > -#define SD1_ITAPDLYSEL_MASK GENMASK(23, 16)
> > > #define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
> > > -#define SD1_OTAPDLYSEL_MASK GENMASK(21, 16)
> > >
> > > #define MIN_PHY_CLK_HZ 50000000
> > >
> > > @@ -275,44 +272,32 @@ static int arasan_sdhci_config_dll(struct
> > > sdhci_host *host, unsigned int clock,
> > > static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32
> > > itap_delay)
> > > {
> > > int ret;
> > > + u32 shift;
> > >
> > > - if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > > - if (node_id == NODE_SD_0) {
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> SD0_ITAPCHGWIN,
> > > - SD0_ITAPCHGWIN);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> SD0_ITAPDLYENA,
> > > - SD0_ITAPDLYENA);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> SD0_ITAPDLYSEL_MASK,
> > > - itap_delay);
> > > - if (ret)
> > > - return ret;
> > > + if (node_id == NODE_SD_0)
> > > + shift = 0;
> > > + else if (node_id == NODE_SD_1)
> > > + shift = SD1_TAP_OFFSET;
> > > + else
> > > + return -EINVAL;
> > >
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> SD0_ITAPCHGWIN, 0);
> > > - if (ret)
> > > - return ret;
> > > - }
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN,
> > > - SD1_ITAPCHGWIN);
> > > + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN
> << shift,
> > > + SD0_ITAPCHGWIN << shift);
> > > if (ret)
> > > return ret;
> > >
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA,
> > > - SD1_ITAPDLYENA);
> > > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA <<
> shift,
> > > + SD0_ITAPDLYENA << shift);
> > > if (ret)
> > > return ret;
> > >
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> SD1_ITAPDLYSEL_MASK,
> > > - (itap_delay << 16));
> > > + ret = zynqmp_mmio_write(SD_ITAP_DLY,
> SD0_ITAPDLYSEL_MASK << shift,
> > > + itap_delay << shift);
> > > if (ret)
> > > return ret;
> > >
> > > - ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN,
> 0);
> > > + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN
> << shift, 0);
> > > if (ret)
> > > return ret;
> > > } else {
> > > @@ -326,14 +311,20 @@ static inline int
> > > arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
> > >
> > > static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id, u32
> > > otap_delay)
> > > {
> > > + u32 shift;
> > > +
> > > + if (node_id == NODE_SD_0)
> > > + shift = 0;
> > > + else if (node_id == NODE_SD_1)
> > > + shift = SD1_TAP_OFFSET;
> > > + else
> > > + return -EINVAL;
> > > +
> > > if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > > - if (node_id == NODE_SD_0)
> > > - return zynqmp_mmio_write(SD_OTAP_DLY,
> > > - SD0_OTAPDLYSEL_MASK,
> > > - otap_delay);
> > > + return zynqmp_mmio_write(SD_OTAP_DLY,
> > > + SD0_OTAPDLYSEL_MASK <<
> shift,
> > > + otap_delay << shift);
> > >
> > > - return zynqmp_mmio_write(SD_OTAP_DLY,
> SD1_OTAPDLYSEL_MASK,
> > > - (otap_delay << 16));
> > > } else {
> > > return xilinx_pm_request(PM_IOCTL, node_id,
> > > IOCTL_SET_SD_TAPDELAY,
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
2024-03-15 9:52 ` Abbarapu, Venkatesh
@ 2024-03-22 7:58 ` Steffen Dirkwinkel
0 siblings, 0 replies; 10+ messages in thread
From: Steffen Dirkwinkel @ 2024-03-22 7:58 UTC (permalink / raw
To: Abbarapu, Venkatesh, Kumar, Love, u-boot@lists.denx.de
Cc: Jaehoon Chung, Johan Jonker, Simek, Michal, Peng Fan, Tom Rini
Hi Venkatesh,
we're not using the firmware when running in el3, so that should still
work. The sdhci driver needs to know which controller it's operating on
and uses the power-domain node for that. I can't add the power domain
node without a firmware node (i think?), but the firmware node without
a compatible shouldn't be used by anything?
Alternatively we could exit without an error from these functions if
our node id doesn't match sd0 or sd1. That wouldn't apply any settings,
but that's the current behaviour already.
Thanks
Steffen
On Fri, 2024-03-15 at 09:52 +0000, Abbarapu, Venkatesh wrote:
> Hi Steffen,
> For mini u-boot cases ZYNQMP_FIRMWARE config is disabled as there
> won't be any ATF(TF-A) present.
>
> Thanks
> Venkatesh
>
> > -----Original Message-----
> > From: Steffen Dirkwinkel <lists@steffen.cc>
> > Sent: Thursday, March 14, 2024 1:23 PM
> > To: Kumar, Love <love.kumar@amd.com>; u-boot@lists.denx.de
> > Cc: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>; Ashok
> > Reddy Soma <ashok.reddy.soma@amd.com>; Jaehoon Chung
> > <jh80.chung@samsung.com>; Johan Jonker <jbx6244@gmail.com>; Simek,
> > Michal <michal.simek@amd.com>; Peng Fan <peng.fan@nxp.com>; Tom
> > Rini
> > <trini@konsulko.com>; Abbarapu, Venkatesh
> > <venkatesh.abbarapu@amd.com>
> > Subject: Re: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay
> > settings
> >
> > Hi Love,
> >
> > On Thu, 2024-03-14 at 12:15 +0530, Love Kumar wrote:
> > > Hi,
> > >
> > > When we run in el3 for zynqmp board, we are seeing below issue
> > > with
> > > this
> > > patch:
> > >
> > >
> > > Model: ZynqMP MINI EMMC0
> > > Board: Xilinx ZynqMP
> > > DRAM: 512 MiB
> > > EL Level: EL3
> > > Secure Boot: not authenticated, not encrypted
> > > Multiboot: 0
> > > Core: 10 devices, 9 uclasses, devicetree: embed
> > > MMC: sdhci@ff160000: 0
> > > Loading Environment from <NULL>... OK
> > > In: dcc
> > > Out: dcc
> > > Err: dcc
> > > ZynqMP> mmc list
> > > sdhci@ff160000: 0
> > > ZynqMP> mmc dev 0 0
> > > arasan_sdhci sdhci@ff160000: Error setting Input Tap Delay
> > > sdhci_set_clock: Error while setting tap delay
> > > sdhci_send_command: Timeout for status update: 00000000 00000001
> > > ZynqMP>
> >
> > Thank you for testing this.
> > It looks like the zynqmp-mini-emmc0 device lacks the power-domain
> > node
> > and doesn't have a sd node id. The code before my change would have
> > ignored the unknown node id, applied settings for sdhci1 instead of
> > sdhci0
> > and returned without an error. Maybe there's a different way to
> > find out
> > which sdhci we want to operate on?
> >
> > Can you try setting the node id in device tree?
> > Something like this:
> >
> > diff --git a/arch/arm/dts/zynqmp-mini-emmc0.dts
> > b/arch/arm/dts/zynqmp-
> > mini-emmc0.dts
> > index 02e80bd85e1..87c4a1b6ad0 100644
> > --- a/arch/arm/dts/zynqmp-mini-emmc0.dts
> > +++ b/arch/arm/dts/zynqmp-mini-emmc0.dts
> > @@ -7,6 +7,8 @@
> > * Siva Durga Prasad Paladugu <siva.durga.prasad.paladugu@amd.com>
> > */
> >
> > +#include <dt-bindings/power/xlnx-zynqmp-power.h>
> > +
> > /dts-v1/;
> >
> > / {
> > @@ -41,6 +43,12 @@
> > clock-frequency = <200000000>;
> > };
> >
> > + firmware {
> > + zynqmp_firmware: zynqmp-firmware {
> > + #power-domain-cells = <1>;
> > + };
> > + };
> > +
> > amba: amba {
> > compatible = "simple-bus";
> > #address-cells = <2>;
> > @@ -56,6 +64,7 @@
> > reg = <0x0 0xff160000 0x0 0x1000>;
> > clock-names = "clk_xin", "clk_ahb";
> > clocks = <&clk_xin &clk_xin>;
> > + power-domains = <&zynqmp_firmware PD_SD_0>;
> > };
> > };
> > };
> >
> > Thanks,
> > Steffen
> >
> > >
> > > Regards,
> > > Love Kumar
> > >
> > > On 23/02/24 7:36 pm, Steffen Dirkwinkel wrote:
> > > > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> > > >
> > > > Previously we were setting in tapdelay for SD1 every time even
> > > > if
> > > > SD0 was requested.
> > > > The SD tapdelay settings are shifted by 16 bits between SD0 and
> > > > SD1.
> > > > We can use that to make our tapdelay setup simpler. This is
> > > > also how
> > > > it currently works in arm-trusted-firmware.
> > > >
> > > > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> > > > ---
> > > >
> > > > drivers/mmc/zynq_sdhci.c | 65
> > > > +++++++++++++++++-----------------------
> > > > 1 file changed, 28 insertions(+), 37 deletions(-)
> > > >
> > > > diff --git a/drivers/mmc/zynq_sdhci.c
> > > > b/drivers/mmc/zynq_sdhci.c
> > > > index 935540d1719..d4845245b2a 100644
> > > > --- a/drivers/mmc/zynq_sdhci.c
> > > > +++ b/drivers/mmc/zynq_sdhci.c
> > > > @@ -42,14 +42,11 @@
> > > > #define SD_OTAP_DLY 0xFF180318
> > > > #define SD0_DLL_RST BIT(2)
> > > > #define SD1_DLL_RST BIT(18)
> > > > +#define SD1_TAP_OFFSET 16
> > > > #define SD0_ITAPCHGWIN BIT(9)
> > > > -#define SD1_ITAPCHGWIN BIT(25)
> > > > #define SD0_ITAPDLYENA BIT(8)
> > > > -#define SD1_ITAPDLYENA BIT(24)
> > > > #define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
> > > > -#define SD1_ITAPDLYSEL_MASK GENMASK(23, 16)
> > > > #define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
> > > > -#define SD1_OTAPDLYSEL_MASK GENMASK(21, 16)
> > > >
> > > > #define MIN_PHY_CLK_HZ 50000000
> > > >
> > > > @@ -275,44 +272,32 @@ static int arasan_sdhci_config_dll(struct
> > > > sdhci_host *host, unsigned int clock,
> > > > static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id,
> > > > u32
> > > > itap_delay)
> > > > {
> > > > int ret;
> > > > + u32 shift;
> > > >
> > > > - if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3)
> > > > {
> > > > - if (node_id == NODE_SD_0) {
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > SD0_ITAPCHGWIN,
> > > > -
> > > > SD0_ITAPCHGWIN);
> > > > - if (ret)
> > > > - return ret;
> > > > -
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > SD0_ITAPDLYENA,
> > > > -
> > > > SD0_ITAPDLYENA);
> > > > - if (ret)
> > > > - return ret;
> > > > -
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > SD0_ITAPDLYSEL_MASK,
> > > > - itap_delay);
> > > > - if (ret)
> > > > - return ret;
> > > > + if (node_id == NODE_SD_0)
> > > > + shift = 0;
> > > > + else if (node_id == NODE_SD_1)
> > > > + shift = SD1_TAP_OFFSET;
> > > > + else
> > > > + return -EINVAL;
> > > >
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > SD0_ITAPCHGWIN, 0);
> > > > - if (ret)
> > > > - return ret;
> > > > - }
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > > > SD1_ITAPCHGWIN,
> > > > - SD1_ITAPCHGWIN);
> > > > + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3)
> > > > {
> > > > + ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > > > SD0_ITAPCHGWIN
> > << shift,
> > > > + SD0_ITAPCHGWIN <<
> > > > shift);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > > > SD1_ITAPDLYENA,
> > > > - SD1_ITAPDLYENA);
> > > > + ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > > > SD0_ITAPDLYENA <<
> > shift,
> > > > + SD0_ITAPDLYENA <<
> > > > shift);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > SD1_ITAPDLYSEL_MASK,
> > > > - (itap_delay << 16));
> > > > + ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > SD0_ITAPDLYSEL_MASK << shift,
> > > > + itap_delay << shift);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > > > SD1_ITAPCHGWIN,
> > 0);
> > > > + ret = zynqmp_mmio_write(SD_ITAP_DLY,
> > > > SD0_ITAPCHGWIN
> > << shift, 0);
> > > > if (ret)
> > > > return ret;
> > > > } else {
> > > > @@ -326,14 +311,20 @@ static inline int
> > > > arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 itap_delay)
> > > >
> > > > static inline int arasan_zynqmp_set_out_tapdelay(u32 node_id,
> > > > u32
> > > > otap_delay)
> > > > {
> > > > + u32 shift;
> > > > +
> > > > + if (node_id == NODE_SD_0)
> > > > + shift = 0;
> > > > + else if (node_id == NODE_SD_1)
> > > > + shift = SD1_TAP_OFFSET;
> > > > + else
> > > > + return -EINVAL;
> > > > +
> > > > if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3)
> > > > {
> > > > - if (node_id == NODE_SD_0)
> > > > - return zynqmp_mmio_write(SD_OTAP_DLY,
> > > > -
> > > > SD0_OTAPDLYSEL_MASK,
> > > > - otap_delay);
> > > > + return zynqmp_mmio_write(SD_OTAP_DLY,
> > > > + SD0_OTAPDLYSEL
> > > > _MASK <<
> > shift,
> > > > + otap_delay <<
> > > > shift);
> > > >
> > > > - return zynqmp_mmio_write(SD_OTAP_DLY,
> > SD1_OTAPDLYSEL_MASK,
> > > > - (otap_delay << 16));
> > > > } else {
> > > > return xilinx_pm_request(PM_IOCTL, node_id,
> > > >
> > > > IOCTL_SET_SD_TAPDELAY,
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-03-22 7:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-23 14:06 [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings Steffen Dirkwinkel
2024-03-14 6:45 ` Love Kumar
2024-03-14 7:52 ` Steffen Dirkwinkel
2024-03-15 9:52 ` Abbarapu, Venkatesh
2024-03-22 7:58 ` Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 3/5] mmc: zynq_sdhci: disable ITAPDLYENA for zero itap_delay Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 4/5] mmc: zynq_sdhci: disable OTAPDLYENA Steffen Dirkwinkel
2024-02-23 14:06 ` [PATCH 5/5] zynqmp: boot without delay in psu_uboot_init Steffen Dirkwinkel
2024-03-01 7:11 ` [PATCH 1/5] configs: zynqmp: don't remove power-domains for spl device tree 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.