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,
