[PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings
Steffen Dirkwinkel
lists at steffen.cc
Thu Mar 14 08:52:59 CET 2024
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 at ff160000: 0
> Loading Environment from <NULL>... OK
> In: dcc
> Out: dcc
> Err: dcc
> ZynqMP> mmc list
> sdhci at ff160000: 0
> ZynqMP> mmc dev 0 0
> arasan_sdhci sdhci at 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 at 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 at 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 at 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,
More information about the U-Boot
mailing list