mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/1] Zynq: add support to chainload another barebox
@ 2021-05-03 10:07 Michael Graichen
  2021-05-05  8:15 ` Lucas Stach
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Graichen @ 2021-05-03 10:07 UTC (permalink / raw)
  To: barebox

Since OCRAM is only 192K this introduces CONFIG_ZYNQ_BUILD_FSBL so we can can chainload a more feature rich barebox via bootm.

>From 1f1a95eca42198d73c38cc12b9b44f061980cef8 Mon Sep 17 00:00:00 2001
From: Michael Graichen <michael.graichen@hotmail.com>
Date: Mon, 3 May 2021 12:03:05 +0200
Subject: [PATCH] zynq: add support to chainload another barebox

Signed-off-by: Michael Graichen <michael.graichen@hotmail.com>
---
 arch/arm/mach-zynq/Kconfig |  6 +++++-
 images/Makefile.zynq       | 21 +++++++++++++++------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
index 3e07633e5..0800b55e6 100644
--- a/arch/arm/mach-zynq/Kconfig
+++ b/arch/arm/mach-zynq/Kconfig
@@ -21,7 +21,11 @@ config ARCH_ZYNQ7000
 	select OFDEVICE
 	select RELOCATABLE

-
+config ZYNQ_BUILD_FSBL
+	prompt "build FSBL binary (BOOT.BIN)"
+	bool
+	help
+	  Say Y here if you want to build an FSBL binary for the Zynq.

 menu "select Zynq boards to be built"

diff --git a/images/Makefile.zynq b/images/Makefile.zynq
index b00e74869..39c72bd88 100644
--- a/images/Makefile.zynq
+++ b/images/Makefile.zynq
@@ -7,17 +7,26 @@ zynqcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \

 zynqcfg-tmp = $(subst $(comma),_,$(dot-target).zynqcfg.tmp)

-quiet_cmd_zynq_image = ZYNQIMG  $@
+quiet_cmd_zynq_image = ZYNQIMG $@
       cmd_zynq_image = \
          $(CPP) $(zynqcfg_cpp_flags) -o $(zynqcfg-tmp) $(CFG_$(@F)) ; \
          $(objtree)/scripts/zynq_mkimage -c $(zynqcfg-tmp) \
-           -f $(subst .zynqimg,,$@) -o $@
+           -f $(subst .zynqimg_fsbl,,$@) -o $@

-$(obj)/%.zynqimg: $(obj)/% FORCE
+$(obj)/%.zynqimg_fsbl: $(obj)/% FORCE
 	$(call if_changed,zynq_image)

 #------------------------------------------------------------------------------

-CFG_start_avnet_zedboard.pblb.zynqimg = $(board)/avnet-zedboard/zedboard.zynqcfg
-FILE_barebox-avnet-zedboard.img = start_avnet_zedboard.pblb.zynqimg
-image-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard.img
+FILE_barebox-avnet-zedboard.img = start_avnet_zedboard.pblb
+zynq-barebox-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard.img
+
+CFG_start_avnet_zedboard.pblb.zynqimg_fsbl = $(board)/avnet-zedboard/zedboard.zynqcfg
+FILE_barebox-avnet-zedboard-fsbl.img = start_avnet_zedboard.pblb.zynqimg_fsbl
+zynq-fsbl-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard-fsbl.img
+
+ifdef CONFIG_ZYNQ_BUILD_FSBL
+image-y += $(zynq-fsbl-y)
+else
+image-y += $(zynq-barebox-y)
+endif
--
2.25.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Zynq: add support to chainload another barebox
  2021-05-03 10:07 [PATCH 1/1] Zynq: add support to chainload another barebox Michael Graichen
@ 2021-05-05  8:15 ` Lucas Stach
  2021-05-05  8:27   ` Ahmad Fatoum
  2021-05-05  9:40   ` AW: " Michael Graichen
  0 siblings, 2 replies; 10+ messages in thread
From: Lucas Stach @ 2021-05-05  8:15 UTC (permalink / raw)
  To: Michael Graichen, barebox

Hi Michael,

Am Montag, dem 03.05.2021 um 10:07 +0000 schrieb Michael Graichen:
> Since OCRAM is only 192K this introduces CONFIG_ZYNQ_BUILD_FSBL so we can can chainload a more feature rich barebox via bootm.
> 
> From 1f1a95eca42198d73c38cc12b9b44f061980cef8 Mon Sep 17 00:00:00 2001
> From: Michael Graichen <michael.graichen@hotmail.com>
> Date: Mon, 3 May 2021 12:03:05 +0200
> Subject: [PATCH] zynq: add support to chainload another barebox

Seems you imported this patch from somewhere and it left some traces in
the commit message?

Also I don't understand what this change is supposed to be doing. You
are building just another Barebox binary, with no real differences in
the configuration. I would much prefer a proper 2-stage loading in the
PBL, but that requires FAT support for the SDcard boot, which I didn't
get around to take a look at yet.

Regards,
Lucas

> Signed-off-by: Michael Graichen <michael.graichen@hotmail.com>
> ---
>  arch/arm/mach-zynq/Kconfig |  6 +++++-
>  images/Makefile.zynq       | 21 +++++++++++++++------
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
> index 3e07633e5..0800b55e6 100644
> --- a/arch/arm/mach-zynq/Kconfig
> +++ b/arch/arm/mach-zynq/Kconfig
> @@ -21,7 +21,11 @@ config ARCH_ZYNQ7000
>  	select OFDEVICE
>  	select RELOCATABLE
> 
> -
> +config ZYNQ_BUILD_FSBL
> +	prompt "build FSBL binary (BOOT.BIN)"
> +	bool
> +	help
> +	  Say Y here if you want to build an FSBL binary for the Zynq.
> 
>  menu "select Zynq boards to be built"
> 
> diff --git a/images/Makefile.zynq b/images/Makefile.zynq
> index b00e74869..39c72bd88 100644
> --- a/images/Makefile.zynq
> +++ b/images/Makefile.zynq
> @@ -7,17 +7,26 @@ zynqcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
> 
>  zynqcfg-tmp = $(subst $(comma),_,$(dot-target).zynqcfg.tmp)
> 
> -quiet_cmd_zynq_image = ZYNQIMG  $@
> +quiet_cmd_zynq_image = ZYNQIMG $@
>        cmd_zynq_image = \
>           $(CPP) $(zynqcfg_cpp_flags) -o $(zynqcfg-tmp) $(CFG_$(@F)) ; \
>           $(objtree)/scripts/zynq_mkimage -c $(zynqcfg-tmp) \
> -           -f $(subst .zynqimg,,$@) -o $@
> +           -f $(subst .zynqimg_fsbl,,$@) -o $@
> 
> -$(obj)/%.zynqimg: $(obj)/% FORCE
> +$(obj)/%.zynqimg_fsbl: $(obj)/% FORCE
>  	$(call if_changed,zynq_image)
> 
>  #------------------------------------------------------------------------------
> 
> -CFG_start_avnet_zedboard.pblb.zynqimg = $(board)/avnet-zedboard/zedboard.zynqcfg
> -FILE_barebox-avnet-zedboard.img = start_avnet_zedboard.pblb.zynqimg
> -image-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard.img
> +FILE_barebox-avnet-zedboard.img = start_avnet_zedboard.pblb
> +zynq-barebox-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard.img
> +
> +CFG_start_avnet_zedboard.pblb.zynqimg_fsbl = $(board)/avnet-zedboard/zedboard.zynqcfg
> +FILE_barebox-avnet-zedboard-fsbl.img = start_avnet_zedboard.pblb.zynqimg_fsbl
> +zynq-fsbl-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard-fsbl.img
> +
> +ifdef CONFIG_ZYNQ_BUILD_FSBL
> +image-y += $(zynq-fsbl-y)
> +else
> +image-y += $(zynq-barebox-y)
> +endif
> --
> 2.25.1
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Zynq: add support to chainload another barebox
  2021-05-05  8:15 ` Lucas Stach
@ 2021-05-05  8:27   ` Ahmad Fatoum
  2021-05-05  9:40   ` AW: " Michael Graichen
  1 sibling, 0 replies; 10+ messages in thread
From: Ahmad Fatoum @ 2021-05-05  8:27 UTC (permalink / raw)
  To: Lucas Stach, Michael Graichen, barebox

Hello Michael,
Hello Lucas,

On 05.05.21 10:15, Lucas Stach wrote:
> Hi Michael,
> 
> Am Montag, dem 03.05.2021 um 10:07 +0000 schrieb Michael Graichen:
>> Since OCRAM is only 192K this introduces CONFIG_ZYNQ_BUILD_FSBL so we can can chainload a more feature rich barebox via bootm.
>>
>> From 1f1a95eca42198d73c38cc12b9b44f061980cef8 Mon Sep 17 00:00:00 2001
>> From: Michael Graichen <michael.graichen@hotmail.com>
>> Date: Mon, 3 May 2021 12:03:05 +0200
>> Subject: [PATCH] zynq: add support to chainload another barebox
> 
> Seems you imported this patch from somewhere and it left some traces in
> the commit message?
> 
> Also I don't understand what this change is supposed to be doing. You
> are building just another Barebox binary, with no real differences in
> the configuration. I would much prefer a proper 2-stage loading in the
> PBL, but that requires FAT support for the SDcard boot, which I didn't
> get around to take a look at yet.

For the record, we now have two platforms where we do FAT in PBL, which
could be used as template:
 * arch/arm/boards/sama5d27*
 * arch/arm/boards/microchip-ksz9477-evb

There are more that build two barebox images with separate configs, but
recommendation is to do it in PBL, because you can then build both images
at once.

Cheers,
Ahmad
> 
> Regards,
> Lucas
> 
>> Signed-off-by: Michael Graichen <michael.graichen@hotmail.com>
>> ---
>>  arch/arm/mach-zynq/Kconfig |  6 +++++-
>>  images/Makefile.zynq       | 21 +++++++++++++++------
>>  2 files changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
>> index 3e07633e5..0800b55e6 100644
>> --- a/arch/arm/mach-zynq/Kconfig
>> +++ b/arch/arm/mach-zynq/Kconfig
>> @@ -21,7 +21,11 @@ config ARCH_ZYNQ7000
>>  	select OFDEVICE
>>  	select RELOCATABLE
>>
>> -
>> +config ZYNQ_BUILD_FSBL
>> +	prompt "build FSBL binary (BOOT.BIN)"
>> +	bool
>> +	help
>> +	  Say Y here if you want to build an FSBL binary for the Zynq.
>>
>>  menu "select Zynq boards to be built"
>>
>> diff --git a/images/Makefile.zynq b/images/Makefile.zynq
>> index b00e74869..39c72bd88 100644
>> --- a/images/Makefile.zynq
>> +++ b/images/Makefile.zynq
>> @@ -7,17 +7,26 @@ zynqcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
>>
>>  zynqcfg-tmp = $(subst $(comma),_,$(dot-target).zynqcfg.tmp)
>>
>> -quiet_cmd_zynq_image = ZYNQIMG  $@
>> +quiet_cmd_zynq_image = ZYNQIMG $@
>>        cmd_zynq_image = \
>>           $(CPP) $(zynqcfg_cpp_flags) -o $(zynqcfg-tmp) $(CFG_$(@F)) ; \
>>           $(objtree)/scripts/zynq_mkimage -c $(zynqcfg-tmp) \
>> -           -f $(subst .zynqimg,,$@) -o $@
>> +           -f $(subst .zynqimg_fsbl,,$@) -o $@
>>
>> -$(obj)/%.zynqimg: $(obj)/% FORCE
>> +$(obj)/%.zynqimg_fsbl: $(obj)/% FORCE
>>  	$(call if_changed,zynq_image)
>>
>>  #------------------------------------------------------------------------------
>>
>> -CFG_start_avnet_zedboard.pblb.zynqimg = $(board)/avnet-zedboard/zedboard.zynqcfg
>> -FILE_barebox-avnet-zedboard.img = start_avnet_zedboard.pblb.zynqimg
>> -image-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard.img
>> +FILE_barebox-avnet-zedboard.img = start_avnet_zedboard.pblb
>> +zynq-barebox-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard.img
>> +
>> +CFG_start_avnet_zedboard.pblb.zynqimg_fsbl = $(board)/avnet-zedboard/zedboard.zynqcfg
>> +FILE_barebox-avnet-zedboard-fsbl.img = start_avnet_zedboard.pblb.zynqimg_fsbl
>> +zynq-fsbl-$(CONFIG_MACH_ZEDBOARD) += barebox-avnet-zedboard-fsbl.img
>> +
>> +ifdef CONFIG_ZYNQ_BUILD_FSBL
>> +image-y += $(zynq-fsbl-y)
>> +else
>> +image-y += $(zynq-barebox-y)
>> +endif
>> --
>> 2.25.1
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
> 
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 10+ messages in thread

* AW: [PATCH 1/1] Zynq: add support to chainload another barebox
  2021-05-05  8:15 ` Lucas Stach
  2021-05-05  8:27   ` Ahmad Fatoum
@ 2021-05-05  9:40   ` Michael Graichen
  2021-05-17  8:23     ` Michael Tretter
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Graichen @ 2021-05-05  9:40 UTC (permalink / raw)
  To: Lucas Stach, barebox

Hey Lucas,

> Seems you imported this patch from somewhere and it left some traces in
> the commit message?

yes, sorry, my local mirror. I will fix that.

> Also I don't understand what this change is supposed to be doing. You
> are building just another Barebox binary, with no real differences in
> the configuration. I would much prefer a proper 2-stage loading in the
> PBL, but that requires FAT support for the SDcard boot, which I didn't
> get around to take a look at yet.

Of course, one would need a seperate configuration file. I'm using "zynq_fsbl_defconfig" for the small one and "zynq_defconfig" for the bigger one for my own tests.
I'm currently booting from SDcard and i have not realised any problems with the FAT support so far. But with the arasan-sdhci driver when writing, see my patch below which fixes it for me but I haven't dived into the drivers very deeply yet.

best regards
Michael


 drivers/mci/arasan-sdhci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
index 520bf30ff..669675369 100644
--- a/drivers/mci/arasan-sdhci.c
+++ b/drivers/mci/arasan-sdhci.c
@@ -265,7 +265,7 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
 	if (cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION)
 		mask |= SDHCI_CMD_INHIBIT_DATA;
 
-	ret = wait_on_timeout(10 * MSECOND,
+	ret = wait_on_timeout(100 * MSECOND,
 			!(sdhci_read32(&host->sdhci, SDHCI_PRESENT_STATE) & mask));
 
 	if (ret) {
@@ -277,8 +277,6 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
 	sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
 
 	mask = SDHCI_INT_CMD_COMPLETE;
-	if (data)
-		mask |= SDHCI_INT_DATA_AVAIL;
 
 	sdhci_set_cmd_xfer_mode(&host->sdhci, cmd, data, false, &command, &xfer);
 
-- 
2.25.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Zynq: add support to chainload another barebox
  2021-05-05  9:40   ` AW: " Michael Graichen
@ 2021-05-17  8:23     ` Michael Tretter
  2021-05-18  8:09       ` AW: " Michael Graichen
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Tretter @ 2021-05-17  8:23 UTC (permalink / raw)
  To: Michael Graichen; +Cc: barebox

On Wed, 05 May 2021 09:40:29 +0000, Michael Graichen wrote:
[...]
> I'm currently booting from SDcard and i have not realised any problems with
> the FAT support so far. But with the arasan-sdhci driver when writing, see
> my patch below which fixes it for me but I haven't dived into the drivers
> very deeply yet.
> 
> best regards
> Michael
> 
> 
>  drivers/mci/arasan-sdhci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> index 520bf30ff..669675369 100644
> --- a/drivers/mci/arasan-sdhci.c
> +++ b/drivers/mci/arasan-sdhci.c
> @@ -265,7 +265,7 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
>  	if (cmd->cmdidx != MMC_CMD_STOP_TRANSMISSION)
>  		mask |= SDHCI_CMD_INHIBIT_DATA;
>  
> -	ret = wait_on_timeout(10 * MSECOND,
> +	ret = wait_on_timeout(100 * MSECOND,
>  			!(sdhci_read32(&host->sdhci, SDHCI_PRESENT_STATE) & mask));
>  
>  	if (ret) {
> @@ -277,8 +277,6 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
>  	sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
>  
>  	mask = SDHCI_INT_CMD_COMPLETE;
> -	if (data)
> -		mask |= SDHCI_INT_DATA_AVAIL;

This looks familiar. I have a similar patch that sets SDHCI_INT_DATA_AVAIL
only if the command is a READ, but I didn't yet have time to verify, that this
is the correct fix.

Michael

>  
>  	sdhci_set_cmd_xfer_mode(&host->sdhci, cmd, data, false, &command, &xfer);

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* AW: [PATCH 1/1] Zynq: add support to chainload another barebox
  2021-05-17  8:23     ` Michael Tretter
@ 2021-05-18  8:09       ` Michael Graichen
  2021-05-19  7:38         ` [PATCH 1/2] mci: arasan: wait for data available only on read Michael Tretter
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Graichen @ 2021-05-18  8:09 UTC (permalink / raw)
  To: Michael Tretter; +Cc: barebox

Hey Michael, 

> This looks familiar. I have a similar patch that sets SDHCI_INT_DATA_AVAIL
> only if the command is a READ, but I didn't yet have time to verify, that this
> is the correct fix.

Can you please send it to me?

Thanks,
Michael

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] mci: arasan: wait for data available only on read
  2021-05-18  8:09       ` AW: " Michael Graichen
@ 2021-05-19  7:38         ` Michael Tretter
  2021-05-19  7:38           ` [PATCH 2/2] mci: arasan: configure data transfer only if we actually have data Michael Tretter
  2021-05-25  7:16           ` [PATCH 1/2] mci: arasan: wait for data available only on read Sascha Hauer
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Tretter @ 2021-05-19  7:38 UTC (permalink / raw)
  To: Michael Graichen; +Cc: barebox, m.tretter

Only READ data transfers actually send a data available interrupt.
Therefore, check if the transfer is a read and wait for the data only in
this case.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---

Hi Michael,

On Tue, 18 May 2021 08:09:47 +0000, Michael Graichen wrote:
> > This looks familiar. I have a similar patch that sets SDHCI_INT_DATA_AVAIL
> > only if the command is a READ, but I didn't yet have time to verify, that this
> > is the correct fix.
>
> Can you please send it to me?

Here you are. I am not sure, if the fix is correct. I also added another
patch to handle situations where data is NULL, but I am not entirely sure
about that one either.

Michael
---
 drivers/mci/arasan-sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
index 520bf30ff952..399966e8cf10 100644
--- a/drivers/mci/arasan-sdhci.c
+++ b/drivers/mci/arasan-sdhci.c
@@ -277,7 +277,7 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
 	sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
 
 	mask = SDHCI_INT_CMD_COMPLETE;
-	if (data)
+	if (data && data->flags == MMC_DATA_READ)
 		mask |= SDHCI_INT_DATA_AVAIL;
 
 	sdhci_set_cmd_xfer_mode(&host->sdhci, cmd, data, false, &command, &xfer);
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] mci: arasan: configure data transfer only if we actually have data
  2021-05-19  7:38         ` [PATCH 1/2] mci: arasan: wait for data available only on read Michael Tretter
@ 2021-05-19  7:38           ` Michael Tretter
  2021-05-25  7:17             ` Sascha Hauer
  2021-05-25  7:16           ` [PATCH 1/2] mci: arasan: wait for data available only on read Sascha Hauer
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Tretter @ 2021-05-19  7:38 UTC (permalink / raw)
  To: Michael Graichen; +Cc: barebox, m.tretter

If we don't have any data to transfer, we must not set the block size
and block count.

If data is NULL, accessing data to get the block size and block count is
a NULL pointer dereference.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/mci/arasan-sdhci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
index 399966e8cf10..3d738774e825 100644
--- a/drivers/mci/arasan-sdhci.c
+++ b/drivers/mci/arasan-sdhci.c
@@ -283,10 +283,12 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
 	sdhci_set_cmd_xfer_mode(&host->sdhci, cmd, data, false, &command, &xfer);
 
 	sdhci_write8(&host->sdhci, SDHCI_TIMEOUT_CONTROL, TIMEOUT_VAL);
-	sdhci_write16(&host->sdhci, SDHCI_TRANSFER_MODE, xfer);
-	sdhci_write16(&host->sdhci, SDHCI_BLOCK_SIZE, SDHCI_DMA_BOUNDARY_512K |
-			    SDHCI_TRANSFER_BLOCK_SIZE(data->blocksize));
-	sdhci_write16(&host->sdhci, SDHCI_BLOCK_COUNT, data->blocks);
+	if (data) {
+		sdhci_write16(&host->sdhci, SDHCI_TRANSFER_MODE, xfer);
+		sdhci_write16(&host->sdhci, SDHCI_BLOCK_SIZE,
+			      SDHCI_DMA_BOUNDARY_512K | SDHCI_TRANSFER_BLOCK_SIZE(data->blocksize));
+		sdhci_write16(&host->sdhci, SDHCI_BLOCK_COUNT, data->blocks);
+	}
 	sdhci_write32(&host->sdhci, SDHCI_ARGUMENT, cmd->cmdarg);
 	sdhci_write16(&host->sdhci, SDHCI_COMMAND, command);
 
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] mci: arasan: wait for data available only on read
  2021-05-19  7:38         ` [PATCH 1/2] mci: arasan: wait for data available only on read Michael Tretter
  2021-05-19  7:38           ` [PATCH 2/2] mci: arasan: configure data transfer only if we actually have data Michael Tretter
@ 2021-05-25  7:16           ` Sascha Hauer
  1 sibling, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2021-05-25  7:16 UTC (permalink / raw)
  To: Michael Tretter; +Cc: Michael Graichen, barebox

On Wed, May 19, 2021 at 09:38:54AM +0200, Michael Tretter wrote:
> Only READ data transfers actually send a data available interrupt.
> Therefore, check if the transfer is a read and wait for the data only in
> this case.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> 
> Hi Michael,
> 
> On Tue, 18 May 2021 08:09:47 +0000, Michael Graichen wrote:
> > > This looks familiar. I have a similar patch that sets SDHCI_INT_DATA_AVAIL
> > > only if the command is a READ, but I didn't yet have time to verify, that this
> > > is the correct fix.
> >
> > Can you please send it to me?
> 
> Here you are. I am not sure, if the fix is correct. I also added another
> patch to handle situations where data is NULL, but I am not entirely sure
> about that one either.
> 
> Michael
> ---
>  drivers/mci/arasan-sdhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> index 520bf30ff952..399966e8cf10 100644
> --- a/drivers/mci/arasan-sdhci.c
> +++ b/drivers/mci/arasan-sdhci.c
> @@ -277,7 +277,7 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
>  	sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
>  
>  	mask = SDHCI_INT_CMD_COMPLETE;
> -	if (data)
> +	if (data && data->flags == MMC_DATA_READ)
>  		mask |= SDHCI_INT_DATA_AVAIL;

Waiting for Data avaialable only makes sense when reading. Applied,
thanks

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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mci: arasan: configure data transfer only if we actually have data
  2021-05-19  7:38           ` [PATCH 2/2] mci: arasan: configure data transfer only if we actually have data Michael Tretter
@ 2021-05-25  7:17             ` Sascha Hauer
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Hauer @ 2021-05-25  7:17 UTC (permalink / raw)
  To: Michael Tretter; +Cc: Michael Graichen, barebox

On Wed, May 19, 2021 at 09:38:55AM +0200, Michael Tretter wrote:
> If we don't have any data to transfer, we must not set the block size
> and block count.
> 
> If data is NULL, accessing data to get the block size and block count is
> a NULL pointer dereference.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  drivers/mci/arasan-sdhci.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mci/arasan-sdhci.c b/drivers/mci/arasan-sdhci.c
> index 399966e8cf10..3d738774e825 100644
> --- a/drivers/mci/arasan-sdhci.c
> +++ b/drivers/mci/arasan-sdhci.c
> @@ -283,10 +283,12 @@ static int arasan_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd,
>  	sdhci_set_cmd_xfer_mode(&host->sdhci, cmd, data, false, &command, &xfer);
>  
>  	sdhci_write8(&host->sdhci, SDHCI_TIMEOUT_CONTROL, TIMEOUT_VAL);
> -	sdhci_write16(&host->sdhci, SDHCI_TRANSFER_MODE, xfer);
> -	sdhci_write16(&host->sdhci, SDHCI_BLOCK_SIZE, SDHCI_DMA_BOUNDARY_512K |
> -			    SDHCI_TRANSFER_BLOCK_SIZE(data->blocksize));
> -	sdhci_write16(&host->sdhci, SDHCI_BLOCK_COUNT, data->blocks);
> +	if (data) {
> +		sdhci_write16(&host->sdhci, SDHCI_TRANSFER_MODE, xfer);
> +		sdhci_write16(&host->sdhci, SDHCI_BLOCK_SIZE,
> +			      SDHCI_DMA_BOUNDARY_512K | SDHCI_TRANSFER_BLOCK_SIZE(data->blocksize));
> +		sdhci_write16(&host->sdhci, SDHCI_BLOCK_COUNT, data->blocks);
> +	}

I also stumbled upon this already. Applied, thanks

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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-05-25  7:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 10:07 [PATCH 1/1] Zynq: add support to chainload another barebox Michael Graichen
2021-05-05  8:15 ` Lucas Stach
2021-05-05  8:27   ` Ahmad Fatoum
2021-05-05  9:40   ` AW: " Michael Graichen
2021-05-17  8:23     ` Michael Tretter
2021-05-18  8:09       ` AW: " Michael Graichen
2021-05-19  7:38         ` [PATCH 1/2] mci: arasan: wait for data available only on read Michael Tretter
2021-05-19  7:38           ` [PATCH 2/2] mci: arasan: configure data transfer only if we actually have data Michael Tretter
2021-05-25  7:17             ` Sascha Hauer
2021-05-25  7:16           ` [PATCH 1/2] mci: arasan: wait for data available only on read Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox