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 <li...@steffen.cc> > Sent: Thursday, March 14, 2024 1:23 PM > To: Kumar, Love <love.ku...@amd.com>; u-boot@lists.denx.de > Cc: Algapally Santosh Sagar <santoshsagar.algapa...@amd.com>; Ashok > Reddy Soma <ashok.reddy.s...@amd.com>; Jaehoon Chung > <jh80.ch...@samsung.com>; Johan Jonker <jbx6...@gmail.com>; Simek, > Michal <michal.si...@amd.com>; Peng Fan <peng....@nxp.com>; Tom Rini > <tr...@konsulko.com>; Abbarapu, Venkatesh > <venkatesh.abbar...@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.palad...@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.dirkwin...@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.dirkwin...@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,