diff mbox series

[U-Boot,4/5] usb: gadget: f_sdp: Allow SPL to load and boot FIT via SDP

Message ID 20190603190029.10373-5-sjoerd.simons@collabora.co.uk
State Superseded
Delegated to: Stefano Babic
Headers show
Series Enable usage of SDP for i.MX6 Sabre Auto Boards | expand

Commit Message

Sjoerd Simons June 3, 2019, 7 p.m. UTC 
From: Frieder Schrempf <frieder.schrempf@kontron.de>

Add support for loading u-boot FIT images over the USB SPD protocol in
the SPL

[Small fixes to build]
Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

 common/spl/spl_sdp.c       | 11 ++++++++--
 drivers/usb/gadget/f_sdp.c | 41 ++++++++++++++++++++++++++++++++------
 include/sdp.h              |  4 +++-
 3 files changed, 47 insertions(+), 9 deletions(-)

Comments

Fabio Estevam June 3, 2019, 7:17 p.m. UTC | #1 
Hi Sjoerd,

On Mon, Jun 3, 2019 at 4:02 PM Sjoerd Simons
<sjoerd.simons@collabora.co.uk> wrote:
>
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> Add support for loading u-boot FIT images over the USB SPD protocol in
> the SPL
>
> [Small fixes to build]

Applied this patch against U-Boot master and it still does not build
for me when using mx6sabresd_defconfig:

cmd/usb_gadget_sdp.c: In function ‘do_sdp’:
cmd/usb_gadget_sdp.c:35:2: error: too few arguments to function ‘sdp_handle’
  sdp_handle(controller_index);
  ^~~~~~~~~~
In file included from cmd/usb_gadget_sdp.c:11:0:
include/sdp.h:15:5: note: declared here
 int sdp_handle(int controller_index, struct spl_image_info *spl_image);
     ^~~~~~~~~~
scripts/Makefile.build:278: recipe for target 'cmd/usb_gadget_sdp.o' failed
make[1]: *** [cmd/usb_gadget_sdp.o] Error
Sjoerd Simons June 3, 2019, 7:30 p.m. UTC | #2 
On Mon, 2019-06-03 at 16:17 -0300, Fabio Estevam wrote:
> Hi Sjoerd,
> 
> On Mon, Jun 3, 2019 at 4:02 PM Sjoerd Simons
> <sjoerd.simons@collabora.co.uk> wrote:
> > From: Frieder Schrempf <frieder.schrempf@kontron.de>
> > 
> > Add support for loading u-boot FIT images over the USB SPD protocol
> > in
> > the SPL
> > 
> > [Small fixes to build]
> 
> Applied this patch against U-Boot master and it still does not build
> for me when using mx6sabresd_defconfig:

Ah; that has CONFIG_CMD_USB_SDP which i didn't enable in the sabre auto
defconfig (and thus didn't notice the failure). Will fix that.

> cmd/usb_gadget_sdp.c: In function ‘do_sdp’:
> cmd/usb_gadget_sdp.c:35:2: error: too few arguments to function
> ‘sdp_handle’
>   sdp_handle(controller_index);
>   ^~~~~~~~~~
> In file included from cmd/usb_gadget_sdp.c:11:0:
> include/sdp.h:15:5: note: declared here
>  int sdp_handle(int controller_index, struct spl_image_info
> *spl_image);
>      ^~~~~~~~~~
> scripts/Makefile.build:278: recipe for target 'cmd/usb_gadget_sdp.o'
> failed
> make[1]: *** [cmd/usb_gadget_sdp.o] Error
Lukasz Majewski June 3, 2019, 9:18 p.m. UTC | #3 
Hi Sjoerd,

> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Add support for loading u-boot FIT images over the USB SPD protocol in
> the SPL
> 
> [Small fixes to build]
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Reviewed-by: Lukasz Majewski <lukma@denx.de>

> ---
> 
>  common/spl/spl_sdp.c       | 11 ++++++++--
>  drivers/usb/gadget/f_sdp.c | 41
> ++++++++++++++++++++++++++++++++------ include/sdp.h              |
> 4 +++- 3 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/common/spl/spl_sdp.c b/common/spl/spl_sdp.c
> index 807256e908..dc57171966 100644
> --- a/common/spl/spl_sdp.c
> +++ b/common/spl/spl_sdp.c
> @@ -26,9 +26,16 @@ static int spl_sdp_load_image(struct
> spl_image_info *spl_image, }
>  
>  	/* This command typically does not return but jumps to an
> image */
> -	sdp_handle(controller_index);
> +	sdp_handle(controller_index, spl_image);
>  	pr_err("SDP ended\n");
> +	/*
> +	 * This command either loads a legacy image, jumps and never
> returns,
> +	 * or it loads a FIT image and returns it to be handled by
> the SPL
> +	 * code.
> +	 */
> +	ret = sdp_handle(controller_index, spl_image);
> +	debug("SDP ended\n");
>  
> -	return -EINVAL;
> +	return ret;
>  }
>  SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_BOARD,
> spl_sdp_load_image); diff --git a/drivers/usb/gadget/f_sdp.c
> b/drivers/usb/gadget/f_sdp.c index ae97ab2b49..2a23160d91 100644
> --- a/drivers/usb/gadget/f_sdp.c
> +++ b/drivers/usb/gadget/f_sdp.c
> @@ -638,7 +638,18 @@ static u32 sdp_jump_imxheader(void *address)
>  	return 0;
>  }
>  
> -static void sdp_handle_in_ep(void)
> +#ifdef CONFIG_SPL_LOAD_FIT
> +static ulong sdp_fit_read(struct spl_load_info *load, ulong sector,
> +			  ulong count, void *buf)
> +{
> +	debug("%s: sector %lx, count %lx, buf %lx\n",
> +	      __func__, sector, count, (ulong)buf);
> +	memcpy(buf, (void *)(load->dev + sector), count);
> +	return count;
> +}
> +#endif
> +
> +static void sdp_handle_in_ep(struct spl_image_info *spl_image)
>  {
>  	u8 *data = sdp_func->in_req->buf;
>  	u32 status;
> @@ -689,11 +700,26 @@ static void sdp_handle_in_ep(void)
>  
>  		/* If imx header fails, try some U-Boot specific
> headers */ if (status) {
> +			image_header_t *header =
> +				sdp_ptr(sdp_func->jmp_address);
>  #ifdef CONFIG_SPL_BUILD
> +#ifdef CONFIG_SPL_LOAD_FIT
> +			if (image_get_magic(header) == FDT_MAGIC) {
> +				struct spl_load_info load;
> +
> +				debug("Found FIT\n");
> +				load.dev = header;
> +				load.bl_len = 1;
> +				load.read = sdp_fit_read;
> +				spl_load_simple_fit(spl_image,
> &load, 0,
> +						    header);
> +
> +				return;
> +			}
> +#endif
>  			/* In SPL, allow jumps to U-Boot images */
>  			struct spl_image_info spl_image = {};
> -			spl_parse_image_header(&spl_image,
> -				(struct image_header
> *)sdp_func->jmp_address);
> +			spl_parse_image_header(&spl_image, header);
>  			jump_to_image_no_args(&spl_image);
>  #else
>  			/* In U-Boot, allow jumps to scripts */
> @@ -715,19 +741,22 @@ static void sdp_handle_in_ep(void)
>  	};
>  }
>  
> -void sdp_handle(int controller_index)
> +int sdp_handle(int controller_index, struct spl_image_info
> *spl_image) {
>  	printf("SDP: handle requests...\n");
>  	while (1) {
>  		if (ctrlc()) {
>  			puts("\rCTRL+C - Operation aborted.\n");
> -			return;
> +			return -EINVAL;
>  		}
>  
> +		if (spl_image->flags & SPL_FIT_FOUND)
> +			return 0;
> +
>  		WATCHDOG_RESET();
>  		usb_gadget_handle_interrupts(controller_index);
>  
> -		sdp_handle_in_ep();
> +		sdp_handle_in_ep(spl_image);
>  	}
>  }
>  
> diff --git a/include/sdp.h b/include/sdp.h
> index f6252d027f..f30e2bca19 100644
> --- a/include/sdp.h
> +++ b/include/sdp.h
> @@ -9,7 +9,9 @@
>  #ifndef __SDP_H_
>  #define __SDP_H_
>  
> +#include <spl.h>
> +
>  int sdp_init(int controller_index);
> -void sdp_handle(int controller_index);
> +int sdp_handle(int controller_index, struct spl_image_info
> *spl_image); 
>  #endif /* __SDP_H_ */




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Frieder Schrempf June 4, 2019, 7:09 a.m. UTC | #4 
Hi Sjoerd,

On 03.06.19 21:00, Sjoerd Simons wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Add support for loading u-boot FIT images over the USB SPD protocol in
> the SPL

I always get confused by this acronym too, but *SPD* is the Social 
Democratic Party of Germany, currently in a deep crisis after the recent 
election and after their leader resigned from all offices two days ago.

And *SDP* is the Serial Download Protocol ;)

> 
> [Small fixes to build]
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>

Thanks a lot for preparing the patch and sending it!

Regards,
Frieder

> ---
> 
>   common/spl/spl_sdp.c       | 11 ++++++++--
>   drivers/usb/gadget/f_sdp.c | 41 ++++++++++++++++++++++++++++++++------
>   include/sdp.h              |  4 +++-
>   3 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/common/spl/spl_sdp.c b/common/spl/spl_sdp.c
> index 807256e908..dc57171966 100644
> --- a/common/spl/spl_sdp.c
> +++ b/common/spl/spl_sdp.c
> @@ -26,9 +26,16 @@ static int spl_sdp_load_image(struct spl_image_info *spl_image,
>   	}
>   
>   	/* This command typically does not return but jumps to an image */
> -	sdp_handle(controller_index);
> +	sdp_handle(controller_index, spl_image);
>   	pr_err("SDP ended\n");
> +	/*
> +	 * This command either loads a legacy image, jumps and never returns,
> +	 * or it loads a FIT image and returns it to be handled by the SPL
> +	 * code.
> +	 */
> +	ret = sdp_handle(controller_index, spl_image);
> +	debug("SDP ended\n");
>   
> -	return -EINVAL;
> +	return ret;
>   }
>   SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_BOARD, spl_sdp_load_image);
> diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c
> index ae97ab2b49..2a23160d91 100644
> --- a/drivers/usb/gadget/f_sdp.c
> +++ b/drivers/usb/gadget/f_sdp.c
> @@ -638,7 +638,18 @@ static u32 sdp_jump_imxheader(void *address)
>   	return 0;
>   }
>   
> -static void sdp_handle_in_ep(void)
> +#ifdef CONFIG_SPL_LOAD_FIT
> +static ulong sdp_fit_read(struct spl_load_info *load, ulong sector,
> +			  ulong count, void *buf)
> +{
> +	debug("%s: sector %lx, count %lx, buf %lx\n",
> +	      __func__, sector, count, (ulong)buf);
> +	memcpy(buf, (void *)(load->dev + sector), count);
> +	return count;
> +}
> +#endif
> +
> +static void sdp_handle_in_ep(struct spl_image_info *spl_image)
>   {
>   	u8 *data = sdp_func->in_req->buf;
>   	u32 status;
> @@ -689,11 +700,26 @@ static void sdp_handle_in_ep(void)
>   
>   		/* If imx header fails, try some U-Boot specific headers */
>   		if (status) {
> +			image_header_t *header =
> +				sdp_ptr(sdp_func->jmp_address);
>   #ifdef CONFIG_SPL_BUILD
> +#ifdef CONFIG_SPL_LOAD_FIT
> +			if (image_get_magic(header) == FDT_MAGIC) {
> +				struct spl_load_info load;
> +
> +				debug("Found FIT\n");
> +				load.dev = header;
> +				load.bl_len = 1;
> +				load.read = sdp_fit_read;
> +				spl_load_simple_fit(spl_image, &load, 0,
> +						    header);
> +
> +				return;
> +			}
> +#endif
>   			/* In SPL, allow jumps to U-Boot images */
>   			struct spl_image_info spl_image = {};
> -			spl_parse_image_header(&spl_image,
> -				(struct image_header *)sdp_func->jmp_address);
> +			spl_parse_image_header(&spl_image, header);
>   			jump_to_image_no_args(&spl_image);
>   #else
>   			/* In U-Boot, allow jumps to scripts */
> @@ -715,19 +741,22 @@ static void sdp_handle_in_ep(void)
>   	};
>   }
>   
> -void sdp_handle(int controller_index)
> +int sdp_handle(int controller_index, struct spl_image_info *spl_image)
>   {
>   	printf("SDP: handle requests...\n");
>   	while (1) {
>   		if (ctrlc()) {
>   			puts("\rCTRL+C - Operation aborted.\n");
> -			return;
> +			return -EINVAL;
>   		}
>   
> +		if (spl_image->flags & SPL_FIT_FOUND)
> +			return 0;
> +
>   		WATCHDOG_RESET();
>   		usb_gadget_handle_interrupts(controller_index);
>   
> -		sdp_handle_in_ep();
> +		sdp_handle_in_ep(spl_image);
>   	}
>   }
>   
> diff --git a/include/sdp.h b/include/sdp.h
> index f6252d027f..f30e2bca19 100644
> --- a/include/sdp.h
> +++ b/include/sdp.h
> @@ -9,7 +9,9 @@
>   #ifndef __SDP_H_
>   #define __SDP_H_
>   
> +#include <spl.h>
> +
>   int sdp_init(int controller_index);
> -void sdp_handle(int controller_index);
> +int sdp_handle(int controller_index, struct spl_image_info *spl_image);
>   
>   #endif /* __SDP_H_ */
>
Fabio Estevam June 4, 2019, 1:50 p.m. UTC | #5 
Hi Frieder,

On Tue, Jun 4, 2019 at 4:09 AM Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:

> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>
> Thanks a lot for preparing the patch and sending it!

As I reported yesterday to Sjoerd, this patch needs some adjustment as
it breaks mx6sabresd_defconfig.

Thanks
diff mbox series

Patch

diff --git a/common/spl/spl_sdp.c b/common/spl/spl_sdp.c
index 807256e908..dc57171966 100644
--- a/common/spl/spl_sdp.c
+++ b/common/spl/spl_sdp.c
@@ -26,9 +26,16 @@  static int spl_sdp_load_image(struct spl_image_info *spl_image,
 	}
 
 	/* This command typically does not return but jumps to an image */
-	sdp_handle(controller_index);
+	sdp_handle(controller_index, spl_image);
 	pr_err("SDP ended\n");
+	/*
+	 * This command either loads a legacy image, jumps and never returns,
+	 * or it loads a FIT image and returns it to be handled by the SPL
+	 * code.
+	 */
+	ret = sdp_handle(controller_index, spl_image);
+	debug("SDP ended\n");
 
-	return -EINVAL;
+	return ret;
 }
 SPL_LOAD_IMAGE_METHOD("USB SDP", 0, BOOT_DEVICE_BOARD, spl_sdp_load_image);
diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c
index ae97ab2b49..2a23160d91 100644
--- a/drivers/usb/gadget/f_sdp.c
+++ b/drivers/usb/gadget/f_sdp.c
@@ -638,7 +638,18 @@  static u32 sdp_jump_imxheader(void *address)
 	return 0;
 }
 
-static void sdp_handle_in_ep(void)
+#ifdef CONFIG_SPL_LOAD_FIT
+static ulong sdp_fit_read(struct spl_load_info *load, ulong sector,
+			  ulong count, void *buf)
+{
+	debug("%s: sector %lx, count %lx, buf %lx\n",
+	      __func__, sector, count, (ulong)buf);
+	memcpy(buf, (void *)(load->dev + sector), count);
+	return count;
+}
+#endif
+
+static void sdp_handle_in_ep(struct spl_image_info *spl_image)
 {
 	u8 *data = sdp_func->in_req->buf;
 	u32 status;
@@ -689,11 +700,26 @@  static void sdp_handle_in_ep(void)
 
 		/* If imx header fails, try some U-Boot specific headers */
 		if (status) {
+			image_header_t *header =
+				sdp_ptr(sdp_func->jmp_address);
 #ifdef CONFIG_SPL_BUILD
+#ifdef CONFIG_SPL_LOAD_FIT
+			if (image_get_magic(header) == FDT_MAGIC) {
+				struct spl_load_info load;
+
+				debug("Found FIT\n");
+				load.dev = header;
+				load.bl_len = 1;
+				load.read = sdp_fit_read;
+				spl_load_simple_fit(spl_image, &load, 0,
+						    header);
+
+				return;
+			}
+#endif
 			/* In SPL, allow jumps to U-Boot images */
 			struct spl_image_info spl_image = {};
-			spl_parse_image_header(&spl_image,
-				(struct image_header *)sdp_func->jmp_address);
+			spl_parse_image_header(&spl_image, header);
 			jump_to_image_no_args(&spl_image);
 #else
 			/* In U-Boot, allow jumps to scripts */
@@ -715,19 +741,22 @@  static void sdp_handle_in_ep(void)
 	};
 }
 
-void sdp_handle(int controller_index)
+int sdp_handle(int controller_index, struct spl_image_info *spl_image)
 {
 	printf("SDP: handle requests...\n");
 	while (1) {
 		if (ctrlc()) {
 			puts("\rCTRL+C - Operation aborted.\n");
-			return;
+			return -EINVAL;
 		}
 
+		if (spl_image->flags & SPL_FIT_FOUND)
+			return 0;
+
 		WATCHDOG_RESET();
 		usb_gadget_handle_interrupts(controller_index);
 
-		sdp_handle_in_ep();
+		sdp_handle_in_ep(spl_image);
 	}
 }
 
diff --git a/include/sdp.h b/include/sdp.h
index f6252d027f..f30e2bca19 100644
--- a/include/sdp.h
+++ b/include/sdp.h
@@ -9,7 +9,9 @@ 
 #ifndef __SDP_H_
 #define __SDP_H_
 
+#include <spl.h>
+
 int sdp_init(int controller_index);
-void sdp_handle(int controller_index);
+int sdp_handle(int controller_index, struct spl_image_info *spl_image);
 
 #endif /* __SDP_H_ */