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 <[email protected]> */ +#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 <[email protected]> > > > > 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 <[email protected]> > > --- > > > > 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,
