[U-Boot] [PATCH] cmd: fpga: Remove fit image support passed without fpga device

[U-Boot] [PATCH] cmd: fpga: Remove fit image support passed without fpga device

[Michal Simek]

The patch applied in 2010
"cmd_fpga: cleanup help and check parameters"
(sha1: a790b5b2326be9d7c9ad9e3d9b51a8bfabc62d07"
was adding this checking

+       if (dev == FPGA_INVALID_DEVICE) {
+               puts("FPGA device not specified\n");
+               op = FPGA_NONE;
+       }

which simply broke one command flow which was
setenv fpga <dev>
fpga loadmk <addr> // legacy image
fpga loadmk <addr>:<fit_image_name> //fit image

Also this sequence for FIT image is completely broken
setenv fpga <dev>
setenv fpgadata <addr>:<fit_image_name>
fpga loadmk
(Note: For legacy images this is working fine).

even from code I don't think this has ever worked properly
for fit image (dev = FPGA_INVALID_DEVICE should be rejected
by fpga core). Fit image support was in 2008 added by:
"[new uImage] Add new uImage fromat support to fpga command"
(sha1: c28c4d193dbfb20b2dd3a5447640fd6de7fd0720)

Just a summary of these facts that none found this for pretty long time
it shouldn't be a problem to remove this flow (without fpga dev)
completely to simplify the code.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

I am rewriting cmd/fpga.c file to use u-boot subcommand and I found
that these flow are not working.
---
 cmd/fpga.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 74ae80c807e2..791fe5cb7718 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -127,31 +127,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	case 3:		/* fpga <op> <dev | data addr> */
 		dev = (int)simple_strtoul(argv[2], NULL, 16);
 		debug("%s: device = %d\n", __func__, dev);
-		/* FIXME - this is a really weak test */
-		if ((argc == 3) && (dev > fpga_count())) {
-			/* must be buffer ptr */
-			debug("%s: Assuming buffer pointer in arg 3\n",
-			      __func__);
-
-#if defined(CONFIG_FIT)
-			if (fit_parse_subimage(argv[2], (ulong)fpga_data,
-					       &fit_addr, &fit_uname)) {
-				fpga_data = (void *)fit_addr;
-				debug("*  fpga: subimage '%s' from FIT image ",
-				      fit_uname);
-				debug("at 0x%08lx\n", fit_addr);
-			} else
-#endif
-			{
-				fpga_data = (void *)(uintptr_t)dev;
-				debug("*  fpga: cmdline image addr = 0x%08lx\n",
-				      (ulong)fpga_data);
-			}
-
-			debug("%s: fpga_data = 0x%lx\n",
-			      __func__, (ulong)fpga_data);
-			dev = FPGA_INVALID_DEVICE;	/* reset device num */
-		}
 	}
 
 	if (dev == FPGA_INVALID_DEVICE) {
-- 
2.17.0

[U-Boot] [PATCH] cmd: fpga: Remove fit image support passed without fpga device

[Simon Glass]

+Tom

Hi,

On 5 June 2018 at 04:47, Michal Simek <michal.simek@xilinx.com> wrote:
> The patch applied in 2010
> "cmd_fpga: cleanup help and check parameters"
> (sha1: a790b5b2326be9d7c9ad9e3d9b51a8bfabc62d07"
> was adding this checking
>
> +       if (dev == FPGA_INVALID_DEVICE) {
> +               puts("FPGA device not specified\n");
> +               op = FPGA_NONE;
> +       }
>
> which simply broke one command flow which was
> setenv fpga <dev>
> fpga loadmk <addr> // legacy image
> fpga loadmk <addr>:<fit_image_name> //fit image
>
> Also this sequence for FIT image is completely broken
> setenv fpga <dev>
> setenv fpgadata <addr>:<fit_image_name>
> fpga loadmk
> (Note: For legacy images this is working fine).
>
> even from code I don't think this has ever worked properly
> for fit image (dev = FPGA_INVALID_DEVICE should be rejected
> by fpga core). Fit image support was in 2008 added by:
> "[new uImage] Add new uImage fromat support to fpga command"
> (sha1: c28c4d193dbfb20b2dd3a5447640fd6de7fd0720)
>
> Just a summary of these facts that none found this for pretty long time
> it shouldn't be a problem to remove this flow (without fpga dev)
> completely to simplify the code.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> I am rewriting cmd/fpga.c file to use u-boot subcommand and I found
> that these flow are not working.
> ---
>  cmd/fpga.c | 25 -------------------------
>  1 file changed, 25 deletions(-)

Seems like we could use a sandbox test here.

Regards,
Simon

[U-Boot] [PATCH] cmd: fpga: Remove fit image support passed without fpga device

[Michal Simek]

On 8.6.2018 23:59, Simon Glass wrote:
> +Tom
> 
> Hi,
> 
> On 5 June 2018 at 04:47, Michal Simek <michal.simek@xilinx.com> wrote:
>> The patch applied in 2010
>> "cmd_fpga: cleanup help and check parameters"
>> (sha1: a790b5b2326be9d7c9ad9e3d9b51a8bfabc62d07"
>> was adding this checking
>>
>> +       if (dev == FPGA_INVALID_DEVICE) {
>> +               puts("FPGA device not specified\n");
>> +               op = FPGA_NONE;
>> +       }
>>
>> which simply broke one command flow which was
>> setenv fpga <dev>
>> fpga loadmk <addr> // legacy image
>> fpga loadmk <addr>:<fit_image_name> //fit image
>>
>> Also this sequence for FIT image is completely broken
>> setenv fpga <dev>
>> setenv fpgadata <addr>:<fit_image_name>
>> fpga loadmk
>> (Note: For legacy images this is working fine).
>>
>> even from code I don't think this has ever worked properly
>> for fit image (dev = FPGA_INVALID_DEVICE should be rejected
>> by fpga core). Fit image support was in 2008 added by:
>> "[new uImage] Add new uImage fromat support to fpga command"
>> (sha1: c28c4d193dbfb20b2dd3a5447640fd6de7fd0720)
>>
>> Just a summary of these facts that none found this for pretty long time
>> it shouldn't be a problem to remove this flow (without fpga dev)
>> completely to simplify the code.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> I am rewriting cmd/fpga.c file to use u-boot subcommand and I found
>> that these flow are not working.
>> ---
>>  cmd/fpga.c | 25 -------------------------
>>  1 file changed, 25 deletions(-)
> 
> Seems like we could use a sandbox test here.

I have fpga python tests to cover all cases on zynq and this is what I
found when I tried to cover the whole file.

Thanks,
Michal