[PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings

Abbarapu, Venkatesh venkatesh.abbarapu at amd.com
Fri Mar 15 10:52:36 CET 2024


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 at steffen.cc>
> Sent: Thursday, March 14, 2024 1:23 PM
> To: Kumar, Love <love.kumar at amd.com>; u-boot at lists.denx.de
> Cc: Algapally Santosh Sagar <santoshsagar.algapally at amd.com>; Ashok
> Reddy Soma <ashok.reddy.soma at amd.com>; Jaehoon Chung
> <jh80.chung at samsung.com>; Johan Jonker <jbx6244 at gmail.com>; Simek,
> Michal <michal.simek at amd.com>; Peng Fan <peng.fan at nxp.com>; Tom Rini
> <trini at konsulko.com>; Abbarapu, Venkatesh
> <venkatesh.abbarapu at 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 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