[PATCH 0/2] mci: fix warning and linker error

[PATCH 0/2] mci: fix warning and linker error

[Steffen Trumtrar]

This series fixes the following warning in mci-core:

   drivers/mci/mci-core.c:1455:17: warning: the address of 'status' will always evaluate as 'true' [-Waddress]

And the following linker error:

   arm-v7a-linux-gnueabihf-ld: drivers/mci/arasan-sdhci.o: in function `arasan_zynqmp_sdcardclk_set_phase': drivers/mci/arasan-sdhci.c:467: undefined reference to `zynqmp_pm_set_sd_tapdelay'
   arm-v7a-linux-gnueabihf-ld: drivers/mci/arasan-sdhci.c:472: undefined reference to `zynqmp_pm_sd_dll_reset'
   arm-v7a-linux-gnueabihf-ld: drivers/mci/arasan-sdhci.o: in function `arasan_zynqmp_sampleclk_set_phase': drivers/mci/arasan-sdhci.c:372: undefined reference to `zynqmp_pm_sd_dll_reset'
   arm-v7a-linux-gnueabihf-ld: /home/str/work/customers/bosch.be.ppm2/worktree/barebox/bosch.be.ppm2/drivers/mci/arasan-sdhci.c:398: undefined reference to `zynqmp_pm_set_sd_tapdelay'
   make[1]: *** [Makefile:900: .tmp_barebox1] Error 1

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
Steffen Trumtrar (2):
      mci: mci-core: fix mci_switch_status call
      mci: arasan: fix build for non-ZynqMP

 drivers/mci/arasan-sdhci.c | 3 ++-
 drivers/mci/mci-core.c     | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)
---
base-commit: bbda2f90b296e953bbef20d00c58447eb88ff84f
change-id: 20240326-worktree-barebox-bosch-be-ppm4-0e9bfd511418

Best regards,
-- 
Steffen Trumtrar <s.trumtrar@pengutronix.de>

[PATCH 1/2] mci: mci-core: fix mci_switch_status call

[Steffen Trumtrar]

mci_switch_status needs to be called with a boolean instead of a u32.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/mci/mci-core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 414bcf6f06..78c65c8614 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1431,8 +1431,6 @@ static int mmc_select_hs200(struct mci *mci)
 	/* find out maximum bus width and then try DDR if supported */
 	err = mci_mmc_select_bus_width(mci);
 	if (err > 0) {
-		u32 status;
-
 		/* TODO  actually set drive strength instead of 0. Currently unsupported. */
 		val = EXT_CSD_TIMING_HS200 | 0 << EXT_CSD_DRV_STR_SHIFT;
 		err = mci_switch(mci, EXT_CSD_HS_TIMING, val);
@@ -1452,7 +1450,7 @@ static int mmc_select_hs200(struct mci *mci)
 		mci_set_ios(mci);
 		mci_set_clock(mci, mci->host->hs_max_dtr);
 
-		err = mci_switch_status(mci, &status);
+		err = mci_switch_status(mci, true);
 
 		/*
 		 * mmc_select_timing() assumes timing has not changed if

-- 
2.43.2

[PATCH 2/2] mci: arasan: fix build for non-ZynqMP

[Steffen Trumtrar]

Registering sdclk only makes sense on the ZynqMP architecture. Guard
calling the function with a IS_ENABLED()

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/mci/arasan-sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
index f01396d7ee..b7dd98049f 100644
--- a/drivers/mci/arasan-sdhci.c
+++ b/drivers/mci/arasan-sdhci.c
@@ -772,7 +772,8 @@ static int arasan_sdhci_probe(struct device *dev)
 
 	mci->f_min = 50000000 / 256;
 
-	arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
+	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
+		arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
 
 	arasan_dt_parse_clk_phases(dev, &arasan_sdhci->clk_data);
 

-- 
2.43.2

Re: [PATCH 2/2] mci: arasan: fix build for non-ZynqMP

[Sascha Hauer]

On Tue, Mar 26, 2024 at 12:50:42PM +0100, Steffen Trumtrar wrote:
> Registering sdclk only makes sense on the ZynqMP architecture. Guard
> calling the function with a IS_ENABLED()
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/mci/arasan-sdhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> index f01396d7ee..b7dd98049f 100644
> --- a/drivers/mci/arasan-sdhci.c
> +++ b/drivers/mci/arasan-sdhci.c
> @@ -772,7 +772,8 @@ static int arasan_sdhci_probe(struct device *dev)
>  
>  	mci->f_min = 50000000 / 256;
>  
> -	arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> +	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
> +		arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);

CONFIG_ARCH_ZYNQMP being enabled doesn't necessarily mean the code
actually runs on Zynqmp. Does this need a runtime check for other
architectures?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/2] mci: fix warning and linker error

[Sascha Hauer]

On Tue, 26 Mar 2024 12:50:40 +0100, Steffen Trumtrar wrote:
> This series fixes the following warning in mci-core:
> 
>    drivers/mci/mci-core.c:1455:17: warning: the address of 'status' will always evaluate as 'true' [-Waddress]
> 
> And the following linker error:
> 
>    arm-v7a-linux-gnueabihf-ld: drivers/mci/arasan-sdhci.o: in function `arasan_zynqmp_sdcardclk_set_phase': drivers/mci/arasan-sdhci.c:467: undefined reference to `zynqmp_pm_set_sd_tapdelay'
>    arm-v7a-linux-gnueabihf-ld: drivers/mci/arasan-sdhci.c:472: undefined reference to `zynqmp_pm_sd_dll_reset'
>    arm-v7a-linux-gnueabihf-ld: drivers/mci/arasan-sdhci.o: in function `arasan_zynqmp_sampleclk_set_phase': drivers/mci/arasan-sdhci.c:372: undefined reference to `zynqmp_pm_sd_dll_reset'
>    arm-v7a-linux-gnueabihf-ld: /home/str/work/customers/bosch.be.ppm2/worktree/barebox/bosch.be.ppm2/drivers/mci/arasan-sdhci.c:398: undefined reference to `zynqmp_pm_set_sd_tapdelay'
>    make[1]: *** [Makefile:900: .tmp_barebox1] Error 1
> 
> [...]

Applied, thanks!

[1/2] mci: mci-core: fix mci_switch_status call
      https://git.pengutronix.de/cgit/barebox/commit/?id=7818608d7e69 (link may not be stable)
[2/2] mci: arasan: fix build for non-ZynqMP
      https://git.pengutronix.de/cgit/barebox/commit/?id=98bfb54c0a0f (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH 2/2] mci: arasan: fix build for non-ZynqMP

[Sascha Hauer]

On Tue, Mar 26, 2024 at 01:34:49PM +0100, Sascha Hauer wrote:
> On Tue, Mar 26, 2024 at 12:50:42PM +0100, Steffen Trumtrar wrote:
> > Registering sdclk only makes sense on the ZynqMP architecture. Guard
> > calling the function with a IS_ENABLED()
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >  drivers/mci/arasan-sdhci.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> > index f01396d7ee..b7dd98049f 100644
> > --- a/drivers/mci/arasan-sdhci.c
> > +++ b/drivers/mci/arasan-sdhci.c
> > @@ -772,7 +772,8 @@ static int arasan_sdhci_probe(struct device *dev)
> >  
> >  	mci->f_min = 50000000 / 256;
> >  
> > -	arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> > +	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
> > +		arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> 
> CONFIG_ARCH_ZYNQMP being enabled doesn't necessarily mean the code
> actually runs on Zynqmp. Does this need a runtime check for other
> architectures?

The arasan MMC driver is currently only used on ZynqMP, so it's OK for
now.

In Linux the driver the ZynqMP specifics are only used with the "xlnx,zynqmp-8.9a"
compatible whereas our driver binds to the "arasan,sdhci-8.9a"
compatible. This makes it more clear that this is really a ZynqMP
specific path that is taken here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/2] mci: arasan: fix build for non-ZynqMP

[Lucas Stach]

Am Dienstag, dem 02.04.2024 um 10:33 +0200 schrieb Sascha Hauer:
> On Tue, Mar 26, 2024 at 01:34:49PM +0100, Sascha Hauer wrote:
> > On Tue, Mar 26, 2024 at 12:50:42PM +0100, Steffen Trumtrar wrote:
> > > Registering sdclk only makes sense on the ZynqMP architecture. Guard
> > > calling the function with a IS_ENABLED()
> > > 
> > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > ---
> > >  drivers/mci/arasan-sdhci.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> > > index f01396d7ee..b7dd98049f 100644
> > > --- a/drivers/mci/arasan-sdhci.c
> > > +++ b/drivers/mci/arasan-sdhci.c
> > > @@ -772,7 +772,8 @@ static int arasan_sdhci_probe(struct device *dev)
> > >  
> > >  	mci->f_min = 50000000 / 256;
> > >  
> > > -	arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> > > +	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
> > > +		arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> > 
> > CONFIG_ARCH_ZYNQMP being enabled doesn't necessarily mean the code
> > actually runs on Zynqmp. Does this need a runtime check for other
> > architectures?
> 
> The arasan MMC driver is currently only used on ZynqMP, so it's OK for
> now.
> 
That's not true. The driver is also used on the Zynq7000.

Regards,
Lucas

> In Linux the driver the ZynqMP specifics are only used with the "xlnx,zynqmp-8.9a"
> compatible whereas our driver binds to the "arasan,sdhci-8.9a"
> compatible. This makes it more clear that this is really a ZynqMP
> specific path that is taken here.
> 
> Sascha
> 

Re: [PATCH 2/2] mci: arasan: fix build for non-ZynqMP

[Sascha Hauer]

On Tue, Apr 02, 2024 at 10:43:24AM +0200, Lucas Stach wrote:
> Am Dienstag, dem 02.04.2024 um 10:33 +0200 schrieb Sascha Hauer:
> > On Tue, Mar 26, 2024 at 01:34:49PM +0100, Sascha Hauer wrote:
> > > On Tue, Mar 26, 2024 at 12:50:42PM +0100, Steffen Trumtrar wrote:
> > > > Registering sdclk only makes sense on the ZynqMP architecture. Guard
> > > > calling the function with a IS_ENABLED()
> > > > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > > ---
> > > >  drivers/mci/arasan-sdhci.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> > > > index f01396d7ee..b7dd98049f 100644
> > > > --- a/drivers/mci/arasan-sdhci.c
> > > > +++ b/drivers/mci/arasan-sdhci.c
> > > > @@ -772,7 +772,8 @@ static int arasan_sdhci_probe(struct device *dev)
> > > >  
> > > >  	mci->f_min = 50000000 / 256;
> > > >  
> > > > -	arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> > > > +	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
> > > > +		arasan_sdhci_register_sdclk(&arasan_sdhci->clk_data, clk_xin, dev);
> > > 
> > > CONFIG_ARCH_ZYNQMP being enabled doesn't necessarily mean the code
> > > actually runs on Zynqmp. Does this need a runtime check for other
> > > architectures?
> > 
> > The arasan MMC driver is currently only used on ZynqMP, so it's OK for
> > now.
> > 
> That's not true. The driver is also used on the Zynq7000.

You're right. I didn't remember we have support for that in barebox as
well. In this case this note becomes relevant:

> 
> > In Linux the driver the ZynqMP specifics are only used with the "xlnx,zynqmp-8.9a"
> > compatible whereas our driver binds to the "arasan,sdhci-8.9a"
> > compatible. This makes it more clear that this is really a ZynqMP
> > specific path that is taken here.

We have to put the ZynqMP specific stuff behind the "xlnx,zynqmp-8.9a"
compatible to make sure Zynq7000 doesn't take this path.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |