[U-Boot] [PATCH v2 00/17] cmd: fpga: Fix fpga command handling and add some fpga tests

[U-Boot] [PATCH v2 00/17] cmd: fpga: Fix fpga command handling and add some fpga tests

[Michal Simek]

Hi,

We are using this framework for a while and by adding more and more
features it requires small redesigned how commands are handled.
This is something what I have put together to improve readability of
this code and also remove code which is bogus and completely untested.

The patch
https://lists.denx.de/pipermail/u-boot/2018-June/330651.html
is also included in this series to have full picture.

Thanks,
Michal

Changes in v2:
- Extend non complete commit message

Changes in v1:
- Remove debug message checking (unfortunatelly we can't parse them)
- Add dependency on image_format_legacy and gzip
- Setup all FIT tests and legacy uimage tests
- New patch from RFC

Michal Simek (17):
  cmd: fpga: Remove fit image support passed without fpga device
  test/py: Extend fpga command to test all fpga load types
  cmd: fpga: Move error handling to do_fpga()
  cmd: fpga: Move fpga_get_op to avoid local function declaration
  cmd: fpga: Cleanup error handling in connection to FPGA_NONE
  cmd: fpga: Move parameter checking for loadfs/loads
  cmd: fpga: Remove parameter checking from fpga loadfs command
  cmd: fpga: Clean wrong_parms handling
  cmd: fpga: Create new do_fpga_wrapper for using u-boot subcommands
  cmd: fpga: Extract fpga info command to separate function
  cmd: fpga: Fix dump and all direct fpga load commands
  cmd: fpga: Fix loadfs command
  cmd: fpga: Fix loadmk command
  cmd: fpga: Add support for missing fpga loadmk commands
  cmd: fpga: Use CMD_RET_FAILURE instead of simple 1
  cmd: fpga: Fix loads command
  MAINTAINERS: Add myself as the FPGA maintainer

 MAINTAINERS                |   8 +
 cmd/fpga.c                 | 652 +++++++++++++++++++++++----------------------
 test/py/tests/test_fpga.py | 552 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 896 insertions(+), 316 deletions(-)
 create mode 100644 test/py/tests/test_fpga.py

-- 
1.9.1

[U-Boot] [PATCH v2 01/17] 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

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) {
-- 
1.9.1

[U-Boot] [PATCH v2 02/17] test/py: Extend fpga command to test all fpga load types

[Michal Simek]

Add support for info, load, loadp, loadb, loadbp, loadmk_legacy,
loadmk_legacy_gz, loadmk_fit, loadfs also with variable support.

There are probably missing failed tests.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1:
- Remove debug message checking (unfortunatelly we can't parse them)
- Add dependency on image_format_legacy and gzip
- Setup all FIT tests and legacy uimage tests

 test/py/tests/test_fpga.py | 552 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 552 insertions(+)
 create mode 100644 test/py/tests/test_fpga.py

diff --git a/test/py/tests/test_fpga.py b/test/py/tests/test_fpga.py
new file mode 100644
index 000000000000..7459ce586740
--- /dev/null
+++ b/test/py/tests/test_fpga.py
@@ -0,0 +1,552 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2018, Xilinx Inc.
+#
+# Michal Simek
+# Siva Durga Prasad Paladugu
+
+import pytest
+import re
+import random
+import u_boot_utils
+
+"""
+Note: This test relies on boardenv_* containing configuration values to define
+the network available and files to be used for testing. Without this, this test
+will be automatically skipped.
+
+For example:
+
+# True if a DHCP server is attached to the network, and should be tested.
+env__net_dhcp_server = True
+
+# A list of environment variables that should be set in order to configure a
+# static IP. In this test case we atleast need serverip for performing tftpb
+# to get required files.
+env__net_static_env_vars = [
+    ("ipaddr", "10.0.0.100"),
+    ("netmask", "255.255.255.0"),
+    ("serverip", "10.0.0.1"),
+]
+
+# Details regarding the files that may be read from a TFTP server. .
+env__fpga_secure_readable_file = {
+    "fn": "auth_bhdr_ppk1_bit.bin",
+    "enckupfn": "auth_bhdr_enc_kup_load_bit.bin",
+    "addr": 0x1000000,
+    "keyaddr": 0x100000,
+    "keyfn": "key.txt",
+}
+
+env__fpga_under_test = {
+    "dev": 0,
+    "addr" : 0x1000000,
+    "bitstream_load": "compress.bin",
+    "bitstream_load_size": 1831960,
+    "bitstream_loadp": "compress_pr.bin",
+    "bitstream_loadp_size": 423352,
+    "bitstream_loadb": "compress.bit",
+    "bitstream_loadb_size": 1832086,
+    "bitstream_loadbp": "compress_pr.bit",
+    "bitstream_loadbp_size": 423491,
+    "mkimage_legacy": "download.ub",
+    "mkimage_legacy_size": 13321468,
+    "mkimage_legacy_gz": "download.gz.ub",
+    "mkimage_legacy_gz_size": 53632,
+    "mkimage_fit": "download-fit.ub",
+    "mkimage_fit_size": 13322784,
+    "loadfs": "mmc 0 compress.bin",
+    "loadfs_size": 1831960,
+    "loadfs_block_size": 0x10000,
+}
+"""
+
+import test_net
+
+def check_dev(u_boot_console):
+    f = u_boot_console.config.env.get('env__fpga_under_test', None)
+    if not f:
+        pytest.skip('No FPGA to test')
+
+    dev = f.get('dev', -1)
+    if dev < 0:
+        pytest.fail('No dev specified via env__fpga_under_test')
+
+    return dev, f
+
+def load_file_from_var(u_boot_console, name):
+    dev, f = check_dev(u_boot_console)
+
+    addr = f.get('addr', -1)
+    if addr < 0:
+        pytest.fail('No address specified via env__fpga_under_test')
+
+    test_net.test_net_dhcp(u_boot_console)
+    test_net.test_net_setup_static(u_boot_console)
+    bit = f['%s' % (name)]
+    bit_size = f['%s_size' % (name)]
+
+    expected_tftp = 'Bytes transferred = %d' % bit_size
+    output = u_boot_console.run_command('tftpboot %x %s' % (addr, bit))
+    assert expected_tftp in output
+
+    return f, dev, addr, bit, bit_size
+
+###### FPGA FAIL test ######
+expected_usage = 'fpga - loadable FPGA image support'
+
+ at pytest.mark.xfail
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_fail(u_boot_console):
+    # Test non valid fpga subcommand
+    expected = 'fpga: non existing command'
+    output = u_boot_console.run_command('fpga broken 0')
+    #assert expected in output
+    assert expected_usage in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_help(u_boot_console):
+    # Just show help
+    output = u_boot_console.run_command('fpga')
+    assert expected_usage in output
+
+
+###### FPGA DUMP tests ######
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_dump(u_boot_console):
+    pytest.skip('Not implemented now')
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_dump_variable(u_boot_console):
+    # Same as above but via "fpga" variable
+    pytest.skip('Not implemented now')
+
+###### FPGA INFO tests ######
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_info_fail(u_boot_console):
+    # Maybe this can be skipped completely
+    dev, f = check_dev(u_boot_console)
+
+    # Multiple parameters to fpga info should fail
+    expected = 'fpga: more parameters passed'
+    output = u_boot_console.run_command('fpga info 0 0')
+    #assert expected in output
+    assert expected_usage in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_info_list(u_boot_console):
+    # Maybe this can be skipped completely
+    dev, f = check_dev(u_boot_console)
+
+    # Code is design in a way that if fpga dev is not passed it should
+    # return list of all fpga devices in the system
+    u_boot_console.run_command('setenv fpga')
+    output = u_boot_console.run_command('fpga info')
+    assert expected_usage not in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_info(u_boot_console):
+    dev, f = check_dev(u_boot_console)
+
+    output = u_boot_console.run_command('fpga info %x' % (dev))
+    assert expected_usage not in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_info_variable(u_boot_console):
+    dev, f = check_dev(u_boot_console)
+
+    #
+    # fpga variable is storing device number which doesn't need to be passed
+    #
+    u_boot_console.run_command('setenv fpga %x' % (dev))
+
+    output = u_boot_console.run_command('fpga info')
+    # Variable cleanup
+    u_boot_console.run_command('setenv fpga')
+    assert expected_usage not in output
+
+###### FPGA LOAD tests ######
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_load_fail(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_load')
+
+    for cmd in ['dump', 'load', 'loadb']:
+	# missing dev parameter
+	expected = 'fpga: incorrect parameters passed'
+	output = u_boot_console.run_command('fpga %s %x $filesize' % (cmd, addr))
+	#assert expected in output
+	assert expected_usage in output
+
+	# more parameters - 0 at the end
+	expected = 'fpga: more parameters passed'
+	output = u_boot_console.run_command('fpga %s %x %x $filesize 0' % (cmd, dev, addr))
+	#assert expected in output
+	assert expected_usage in output
+
+	# 0 address
+	expected = 'fpga: zero fpga_data address'
+	output = u_boot_console.run_command('fpga %s %x 0 $filesize' % (cmd, dev))
+	#assert expected in output
+	assert expected_usage in output
+
+	# 0 filesize
+	expected = 'fpga: zero size'
+	output = u_boot_console.run_command('fpga %s %x %x 0' % (cmd, dev, addr))
+	#assert expected in output
+	assert expected_usage in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_load(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_load')
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga load %x %x $filesize && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadp')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadp(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_load')
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga load %x %x $filesize && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+    # And load also partial bistream
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_loadp')
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadp %x %x $filesize && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadb(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_loadb')
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadb %x %x $filesize && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadbp')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadbp(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_loadb')
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadb %x %x $filesize && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+    # And load also partial bistream in bit format
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'bitstream_loadbp')
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadbp %x %x $filesize && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+###### FPGA LOADMK tests ######
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('image_format_legacy')
+def test_fpga_loadmk_fail(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_legacy')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    # load image but pass incorrect address to show error message
+    expected = 'Unknown image type'
+    output = u_boot_console.run_command('fpga loadmk %x %x' % (dev, addr + 0x10))
+    assert expected in output
+
+    # Pass more parameters then command expects - 0 at the end
+    output = u_boot_console.run_command('fpga loadmk %x %x 0' % (dev, addr))
+    #assert expected in output
+    assert expected_usage in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('image_format_legacy')
+def test_fpga_loadmk_legacy(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_legacy')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x %x && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.xfail
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('image_format_legacy')
+def test_fpga_loadmk_legacy_variable_fpga(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_legacy')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    u_boot_console.run_command('setenv fpga %x' % (dev))
+
+    # this testcase should cover case which looks like it is supported but dev pointer is broken by loading mkimage address
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x && echo %s' % (addr, expected_text))
+    u_boot_console.run_command('setenv fpga')
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('image_format_legacy')
+def test_fpga_loadmk_legacy_variable_fpgadata(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_legacy')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    u_boot_console.run_command('setenv fpgadata %x' % (addr))
+
+    # this testcase should cover case which looks like it is supported but dev pointer is broken by loading mkimage address
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x && echo %s' % (dev, expected_text))
+    u_boot_console.run_command('setenv fpgadata')
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('image_format_legacy')
+def test_fpga_loadmk_legacy_variable(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_legacy')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    u_boot_console.run_command('setenv fpga %x' % (dev))
+    u_boot_console.run_command('setenv fpgadata %x' % (addr))
+
+    # this testcase should cover case which looks like it is supported but dev pointer is broken by loading mkimage address
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk && echo %s' % (expected_text))
+    u_boot_console.run_command('setenv fpga')
+    u_boot_console.run_command('setenv fpgadata')
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('image_format_legacy')
+ at pytest.mark.buildconfigspec('gzip')
+def test_fpga_loadmk_legacy_gz(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_legacy_gz')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x %x && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('fit')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadmk_fit(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_fit')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x %x:fpga && echo %s' % (dev, addr, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('fit')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadmk_fit_variable_fpga(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_fit')
+
+    u_boot_console.run_command('imi %x' % (addr))
+    # FIXME this should fail - broken support in past
+    u_boot_console.run_command('setenv fpga %x' % (dev))
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x:fpga && echo %s' % (addr, expected_text))
+    u_boot_console.run_command('setenv fpga')
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('fit')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadmk_fit_variable_fpgadata(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_fit')
+
+    u_boot_console.run_command('imi %x' % (addr))
+    # FIXME this should fail - broken support in past
+    u_boot_console.run_command('setenv fpgadata %x:fpga' % (addr))
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk %x && echo %s' % (dev, expected_text))
+    u_boot_console.run_command('setenv fpgadata')
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_loadmk')
+ at pytest.mark.buildconfigspec('fit')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadmk_fit_variable(u_boot_console):
+    f, dev, addr, bit, bit_size = load_file_from_var(u_boot_console, 'mkimage_fit')
+
+    u_boot_console.run_command('imi %x' % (addr))
+
+    u_boot_console.run_command('setenv fpga %x' % (dev))
+    u_boot_console.run_command('setenv fpgadata %x:fpga' % (addr))
+
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadmk && echo %s' % (expected_text))
+    u_boot_console.run_command('setenv fpga')
+    u_boot_console.run_command('setenv fpgadata')
+    assert expected_text in output
+
+###### FPGA LOAD tests ######
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+def test_fpga_loadfs_fail(u_boot_console):
+    dev, f = check_dev(u_boot_console)
+
+    addr = f.get('addr', -1)
+    if addr < 0:
+        pytest.fail('No address specified via env__fpga_under_test')
+
+    bit = f['loadfs']
+    bit_size = f['loadfs_size']
+    block_size = f['loadfs_block_size']
+
+    # less params - dev number removed
+    expected = 'fpga: incorrect parameters passed'
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %s' % (addr, bit_size, block_size, bit))
+    #assert expected in output
+    assert expected_usage in output
+
+    # one more param - 0 at the end
+    # This is the longest command that's why there is no message from cmd/fpga.c
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %x %s 0' % (dev, addr, bit_size, block_size, bit))
+    assert expected_usage in output
+
+    # zero address 0
+    expected = 'fpga: zero fpga_data address'
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %x %s' % (dev, 0, bit_size, block_size, bit))
+    #assert expected in output
+    assert expected_usage in output
+
+    # bit_size 0
+    expected = 'fpga: zero size'
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %x %s' % (dev, addr, 0, block_size, bit))
+    #assert expected in output
+    assert expected_usage in output
+
+    # block size 0
+    # FIXME this should pass but it failing too
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %x %s' % (dev, addr, bit_size, 0, bit))
+    assert expected_usage in output
+
+    # non existing bitstream name
+    expected = 'Unable to read file noname'
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %x mmc 0 noname' % (dev, addr, bit_size, block_size))
+    assert expected in output
+    assert expected_usage in output
+
+    # -1 dev number
+    expected = 'fpga_fsload: Invalid device number -1'
+    output = u_boot_console.run_command('fpga loadfs %d %x %x %x mmc 0 noname' % (-1, addr, bit_size, block_size))
+    assert expected in output
+    assert expected_usage in output
+
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_fpga_loadfs(u_boot_console):
+    dev, f = check_dev(u_boot_console)
+
+    addr = f.get('addr', -1)
+    if addr < 0:
+        pytest.fail('No address specified via env__fpga_under_test')
+
+    bit = f['loadfs']
+    bit_size = f['loadfs_size']
+    block_size = f['loadfs_block_size']
+
+    # This should be done better
+    expected_text = 'FPGA loaded successfully'
+    output = u_boot_console.run_command('fpga loadfs %x %x %x %x %s && echo %s' % (dev, addr, bit_size, block_size, bit, expected_text))
+    assert expected_text in output
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_load_secure')
+ at pytest.mark.buildconfigspec('cmd_net')
+ at pytest.mark.buildconfigspec('cmd_dhcp')
+ at pytest.mark.buildconfigspec('net')
+def test_fpga_secure_bit_auth(u_boot_console):
+
+    test_net.test_net_dhcp(u_boot_console)
+    test_net.test_net_setup_static(u_boot_console)
+
+    f = u_boot_console.config.env.get('env__fpga_secure_readable_file', None)
+    if not f:
+        pytest.skip('No TFTP readable file to read')
+
+    addr = f.get('addr', None)
+    if not addr:
+      addr = u_boot_utils.find_ram_base(u_boot_console)
+
+    expected_tftp = 'Bytes transferred = '
+    fn = f['fn']
+    output = u_boot_console.run_command('tftpboot %x %s' % (addr, fn))
+    assert expected_tftp in output
+
+    expected_zynqmpsecure = 'Bitstream successfully loaded'
+    output = u_boot_console.run_command('fpga loads 0 %x $filesize 0 2' % (addr))
+    assert expected_zynqmpsecure in output
+
+
+ at pytest.mark.buildconfigspec('cmd_fpga')
+ at pytest.mark.buildconfigspec('cmd_fpga_load_secure')
+ at pytest.mark.buildconfigspec('cmd_net')
+ at pytest.mark.buildconfigspec('cmd_dhcp')
+@pytest.mark.buildconfigspec('net')
+def test_fpga_secure_bit_img_auth_kup(u_boot_console):
+
+    test_net.test_net_dhcp(u_boot_console)
+    test_net.test_net_setup_static(u_boot_console)
+
+    f = u_boot_console.config.env.get('env__fpga_secure_readable_file', None)
+    if not f:
+        pytest.skip('No TFTP readable file to read')
+
+    keyaddr = f.get('keyaddr', None)
+    if not keyaddr:
+      addr = u_boot_utils.find_ram_base(u_boot_console)
+    expected_tftp = 'Bytes transferred = '
+    keyfn = f['keyfn']
+    output = u_boot_console.run_command('tftpboot %x %s' % (keyaddr, keyfn))
+    assert expected_tftp in output
+
+    addr = f.get('addr', None)
+    if not addr:
+      addr = u_boot_utils.find_ram_base(u_boot_console)
+    expected_tftp = 'Bytes transferred = '
+    fn = f['enckupfn']
+    output = u_boot_console.run_command('tftpboot %x %s' % (addr, fn))
+    assert expected_tftp in output
+
+    expected_zynqmpsecure = 'Bitstream successfully loaded'
+    output = u_boot_console.run_command('fpga loads 0 %x $filesize 0 1 %x' % (addr, keyaddr))
+    assert expected_zynqmpsecure in output
-- 
1.9.1

[U-Boot] [PATCH v2 03/17] cmd: fpga: Move error handling to do_fpga()

[Michal Simek]

Clean fpga_get_op() error handling by moving checking/print to do_fpga.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 791fe5cb7718..abe683720285 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -74,6 +74,9 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	op = (int)fpga_get_op(argv[1]);
 
 	switch (op) {
+	case FPGA_NONE:
+		printf("Unknown fpga operation \"%s\"\n", argv[1]);
+		return CMD_RET_USAGE;
 #if defined(CONFIG_CMD_FPGA_LOADFS)
 	case FPGA_LOADFS:
 		if (argc < 9)
@@ -360,9 +363,6 @@ static int fpga_get_op(char *opstr)
 		op = FPGA_LOADS;
 #endif
 
-	if (op == FPGA_NONE)
-		printf("Unknown fpga operation \"%s\"\n", opstr);
-
 	return op;
 }
 
-- 
1.9.1

[U-Boot] [PATCH v2 04/17] cmd: fpga: Move fpga_get_op to avoid local function declaration

[Michal Simek]

Move fpga_get_op() to top of file to remove local function declaration
and also remove useless retyping.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 85 ++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index abe683720285..de8505e9d4c8 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -13,9 +13,6 @@
 #include <fs.h>
 #include <malloc.h>
 
-/* Local functions */
-static int fpga_get_op(char *opstr);
-
 /* Local defines */
 enum {
 	FPGA_NONE = -1,
@@ -30,6 +27,46 @@ enum {
 	FPGA_LOADS,
 };
 
+/*
+ * Map op to supported operations.  We don't use a table since we
+ * would just have to relocate it from flash anyway.
+ */
+static int fpga_get_op(char *opstr)
+{
+	int op = FPGA_NONE;
+
+	if (!strcmp("info", opstr))
+		op = FPGA_INFO;
+	else if (!strcmp("loadb", opstr))
+		op = FPGA_LOADB;
+	else if (!strcmp("load", opstr))
+		op = FPGA_LOAD;
+#if defined(CONFIG_CMD_FPGA_LOADP)
+	else if (!strcmp("loadp", opstr))
+		op = FPGA_LOADP;
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADBP)
+	else if (!strcmp("loadbp", opstr))
+		op = FPGA_LOADBP;
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+	else if (!strcmp("loadfs", opstr))
+		op = FPGA_LOADFS;
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADMK)
+	else if (!strcmp("loadmk", opstr))
+		op = FPGA_LOADMK;
+#endif
+	else if (!strcmp("dump", opstr))
+		op = FPGA_DUMP;
+#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
+	else if (!strcmp("loads", opstr))
+		op = FPGA_LOADS;
+#endif
+
+	return op;
+}
+
 /* ------------------------------------------------------------------------- */
 /* command form:
  *   fpga <op> <device number> <data addr> <datasize>
@@ -71,7 +108,7 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		return CMD_RET_USAGE;
 	}
 
-	op = (int)fpga_get_op(argv[1]);
+	op = fpga_get_op(argv[1]);
 
 	switch (op) {
 	case FPGA_NONE:
@@ -326,46 +363,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	return rc;
 }
 
-/*
- * Map op to supported operations.  We don't use a table since we
- * would just have to relocate it from flash anyway.
- */
-static int fpga_get_op(char *opstr)
-{
-	int op = FPGA_NONE;
-
-	if (!strcmp("info", opstr))
-		op = FPGA_INFO;
-	else if (!strcmp("loadb", opstr))
-		op = FPGA_LOADB;
-	else if (!strcmp("load", opstr))
-		op = FPGA_LOAD;
-#if defined(CONFIG_CMD_FPGA_LOADP)
-	else if (!strcmp("loadp", opstr))
-		op = FPGA_LOADP;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADBP)
-	else if (!strcmp("loadbp", opstr))
-		op = FPGA_LOADBP;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	else if (!strcmp("loadfs", opstr))
-		op = FPGA_LOADFS;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADMK)
-	else if (!strcmp("loadmk", opstr))
-		op = FPGA_LOADMK;
-#endif
-	else if (!strcmp("dump", opstr))
-		op = FPGA_DUMP;
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	else if (!strcmp("loads", opstr))
-		op = FPGA_LOADS;
-#endif
-
-	return op;
-}
-
 #if defined(CONFIG_CMD_FPGA_LOADFS) || defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 U_BOOT_CMD(fpga, 9, 1, do_fpga,
 #else
-- 
1.9.1

[U-Boot] [PATCH v2 05/17] cmd: fpga: Cleanup error handling in connection to FPGA_NONE

[Michal Simek]

Incorrect command is already handled and FPGA_NONE should be used only
one. In case of error CMD_RET_USAGE can be returned directly without any
addition logic around.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index de8505e9d4c8..af2f514dca00 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -171,11 +171,10 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 
 	if (dev == FPGA_INVALID_DEVICE) {
 		puts("FPGA device not specified\n");
-		op = FPGA_NONE;
+		return CMD_RET_USAGE;
 	}
 
 	switch (op) {
-	case FPGA_NONE:
 	case FPGA_INFO:
 		break;
 #if defined(CONFIG_CMD_FPGA_LOADFS)
@@ -219,13 +218,10 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 
 	if (wrong_parms) {
 		puts("Wrong parameters for FPGA request\n");
-		op = FPGA_NONE;
+		return CMD_RET_USAGE;
 	}
 
 	switch (op) {
-	case FPGA_NONE:
-		return CMD_RET_USAGE;
-
 	case FPGA_INFO:
 		rc = fpga_info(dev);
 		break;
-- 
1.9.1

[U-Boot] [PATCH v2 06/17] cmd: fpga: Move parameter checking for loadfs/loads

[Michal Simek]

There is no reason to check parameters in separate switch before main
one. This patch is simplifying error path and checking parameters right
after assignment.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 44 +++++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index af2f514dca00..48902286f1d5 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -123,6 +123,14 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		fpga_fsinfo.interface = argv[6];
 		fpga_fsinfo.dev_part = argv[7];
 		fpga_fsinfo.filename = argv[8];
+
+		/* Blocksize can be zero */
+		if (!fpga_fsinfo.interface || !fpga_fsinfo.dev_part ||
+		    !fpga_fsinfo.filename) {
+			puts("ERR: Wrong interface, dev_part or filename\n");
+			return CMD_RET_USAGE;
+		}
+
 		argc = 5;
 		break;
 #endif
@@ -136,6 +144,19 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 								     NULL, 16);
 		fpga_sec_info.encflag = (u8)simple_strtoul(argv[6], NULL, 16);
 		fpga_sec_info.authflag = (u8)simple_strtoul(argv[5], NULL, 16);
+
+		if (fpga_sec_info.authflag >= FPGA_NO_ENC_OR_NO_AUTH &&
+		    fpga_sec_info.encflag >= FPGA_NO_ENC_OR_NO_AUTH) {
+			puts("ERR: Use <fpga load> for NonSecure bitstream\n");
+			return CMD_RET_USAGE;
+		}
+
+		if (fpga_sec_info.encflag == FPGA_ENC_USR_KEY &&
+		    !fpga_sec_info.userkey_addr) {
+			puts("ERR: User key not provided\n");
+			return CMD_RET_USAGE;
+		}
+
 		argc = 5;
 		break;
 #endif
@@ -177,29 +198,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	switch (op) {
 	case FPGA_INFO:
 		break;
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	case FPGA_LOADFS:
-		/* Blocksize can be zero */
-		if (!fpga_fsinfo.interface || !fpga_fsinfo.dev_part ||
-		    !fpga_fsinfo.filename)
-			wrong_parms = 1;
-		break;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	case FPGA_LOADS:
-		if (fpga_sec_info.authflag >= FPGA_NO_ENC_OR_NO_AUTH &&
-		    fpga_sec_info.encflag >= FPGA_NO_ENC_OR_NO_AUTH) {
-			puts("ERR: use <fpga load> for NonSecure bitstream\n");
-			wrong_parms = 1;
-		}
-
-		if (fpga_sec_info.encflag == FPGA_ENC_USR_KEY &&
-		    !fpga_sec_info.userkey_addr) {
-			wrong_parms = 1;
-			puts("ERR:User key not provided\n");
-		}
-		break;
-#endif
 	case FPGA_LOAD:
 	case FPGA_LOADP:
 	case FPGA_LOADB:
-- 
1.9.1

[U-Boot] [PATCH v2 07/17] cmd: fpga: Remove parameter checking from fpga loadfs command

[Michal Simek]

Parameter checking is dead code because all the time there must be all
params assigned. If they are not assigned there is no 9th parameters
passed and checking before return CMD_RET_USAGE.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 48902286f1d5..b03dd9dc0ace 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -124,13 +124,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		fpga_fsinfo.dev_part = argv[7];
 		fpga_fsinfo.filename = argv[8];
 
-		/* Blocksize can be zero */
-		if (!fpga_fsinfo.interface || !fpga_fsinfo.dev_part ||
-		    !fpga_fsinfo.filename) {
-			puts("ERR: Wrong interface, dev_part or filename\n");
-			return CMD_RET_USAGE;
-		}
-
 		argc = 5;
 		break;
 #endif
-- 
1.9.1

[U-Boot] [PATCH v2 08/17] cmd: fpga: Clean wrong_parms handling

[Michal Simek]

There is no reason to check parameters in separate switch. Check them
directly when they are read. Also there is no reason to check loadmk
case separately because fpga_data address must be non zero too.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 35 ++++++++---------------------------
 1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index b03dd9dc0ace..0e5f4117c02e 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -83,7 +83,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	char *devstr = env_get("fpga");
 	char *datastr = env_get("fpgadata");
 	int rc = FPGA_FAIL;
-	int wrong_parms = 0;
 #if defined(CONFIG_FIT)
 	const char *fit_uname = NULL;
 	ulong fit_addr;
@@ -160,7 +159,10 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	switch (argc) {
 	case 5:		/* fpga <op> <dev> <data> <datasize> */
 		data_size = simple_strtoul(argv[4], NULL, 16);
-
+		if (!data_size) {
+			puts("Zero data_size\n");
+			return CMD_RET_USAGE;
+		}
 	case 4:		/* fpga <op> <dev> <data> */
 #if defined(CONFIG_FIT)
 		if (fit_parse_subimage(argv[3], (ulong)fpga_data,
@@ -177,7 +179,10 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 			      (ulong)fpga_data);
 		}
 		debug("%s: fpga_data = 0x%lx\n", __func__, (ulong)fpga_data);
-
+		if (!fpga_data) {
+			puts("Zero fpga_data address\n");
+			return CMD_RET_USAGE;
+		}
 	case 3:		/* fpga <op> <dev | data addr> */
 		dev = (int)simple_strtoul(argv[2], NULL, 16);
 		debug("%s: device = %d\n", __func__, dev);
@@ -190,30 +195,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 
 	switch (op) {
 	case FPGA_INFO:
-		break;
-	case FPGA_LOAD:
-	case FPGA_LOADP:
-	case FPGA_LOADB:
-	case FPGA_LOADBP:
-	case FPGA_DUMP:
-		if (!fpga_data || !data_size)
-			wrong_parms = 1;
-		break;
-#if defined(CONFIG_CMD_FPGA_LOADMK)
-	case FPGA_LOADMK:
-		if (!fpga_data)
-			wrong_parms = 1;
-		break;
-#endif
-	}
-
-	if (wrong_parms) {
-		puts("Wrong parameters for FPGA request\n");
-		return CMD_RET_USAGE;
-	}
-
-	switch (op) {
-	case FPGA_INFO:
 		rc = fpga_info(dev);
 		break;
 
-- 
1.9.1

[U-Boot] [PATCH v2 09/17] cmd: fpga: Create new do_fpga_wrapper for using u-boot subcommands

[Michal Simek]

Create command wrapper to clean fpga subcommands.
The function logic is taken from cmd_dm.c

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 0e5f4117c02e..ac12af2fa06d 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -331,10 +331,48 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	return rc;
 }
 
+static cmd_tbl_t fpga_commands[] = {
+};
+
+static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
+			   char *const argv[])
+{
+	cmd_tbl_t *fpga_cmd;
+	int ret;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	fpga_cmd = find_cmd_tbl(argv[1], fpga_commands,
+				ARRAY_SIZE(fpga_commands));
+
+	/* This should be removed when all functions are converted */
+	if (!fpga_cmd)
+		return do_fpga(cmdtp, flag, argc, argv);
+
+	/* FIXME This can't be reached till all functions are converted */
+	if (!fpga_cmd) {
+		debug("fpga: non existing command\n");
+		return CMD_RET_USAGE;
+	}
+
+	argc -= 2;
+	argv += 2;
+
+	if (argc > fpga_cmd->maxargs) {
+		debug("fpga: more parameters passed\n");
+		return CMD_RET_USAGE;
+	}
+
+	ret = fpga_cmd->cmd(fpga_cmd, flag, argc, argv);
+
+	return cmd_process_error(fpga_cmd, ret);
+}
+
 #if defined(CONFIG_CMD_FPGA_LOADFS) || defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-U_BOOT_CMD(fpga, 9, 1, do_fpga,
+U_BOOT_CMD(fpga, 9, 1, do_fpga_wrapper,
 #else
-U_BOOT_CMD(fpga, 6, 1, do_fpga,
+U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper,
 #endif
 	   "loadable FPGA image support",
 	   "[operation type] [device number] [image address] [image size]\n"
-- 
1.9.1

[U-Boot] [PATCH v2 10/17] cmd: fpga: Extract fpga info command to separate function

[Michal Simek]

Move fpga info to U_BOOT_CMD_MKENT subcommand.
Also use strtol instead of simple_strtoul. The reason is that if -1 is
passed (or fpga info without "fpga" variable) the list of all fpgas is
shown.
This functionality is in the fpga core but it couldn't be performed.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index ac12af2fa06d..039803870b02 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -13,10 +13,26 @@
 #include <fs.h>
 #include <malloc.h>
 
+static long do_fpga_get_device(char *arg)
+{
+	long dev = FPGA_INVALID_DEVICE;
+	char *devstr = env_get("fpga");
+
+	if (devstr)
+		/* Should be strtol to handle -1 cases */
+		dev = simple_strtol(devstr, NULL, 16);
+
+	if (arg)
+		dev = simple_strtol(arg, NULL, 16);
+
+	debug("%s: device = %ld\n", __func__, dev);
+
+	return dev;
+}
+
 /* Local defines */
 enum {
 	FPGA_NONE = -1,
-	FPGA_INFO,
 	FPGA_LOAD,
 	FPGA_LOADB,
 	FPGA_DUMP,
@@ -35,9 +51,7 @@ static int fpga_get_op(char *opstr)
 {
 	int op = FPGA_NONE;
 
-	if (!strcmp("info", opstr))
-		op = FPGA_INFO;
-	else if (!strcmp("loadb", opstr))
+	if (!strcmp("loadb", opstr))
 		op = FPGA_LOADB;
 	else if (!strcmp("load", opstr))
 		op = FPGA_LOAD;
@@ -194,10 +208,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	}
 
 	switch (op) {
-	case FPGA_INFO:
-		rc = fpga_info(dev);
-		break;
-
 	case FPGA_LOAD:
 		rc = fpga_load(dev, fpga_data, data_size, BIT_FULL);
 		break;
@@ -331,7 +341,16 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	return rc;
 }
 
+static int do_fpga_info(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	long dev = do_fpga_get_device(argv[0]);
+
+	return fpga_info(dev);
+}
+
 static cmd_tbl_t fpga_commands[] = {
+	U_BOOT_CMD_MKENT(info, 1, 1, do_fpga_info, "", ""),
 };
 
 static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
-- 
1.9.1

[U-Boot] [PATCH v2 11/17] cmd: fpga: Fix dump and all direct fpga load commands

[Michal Simek]

Separate dump, load, loadb, loadp and loadbp commands to separate
functions to make it clear how they are called and what parameters they
need.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

Maybe they can be still groupped together with one switch/case but it
can be done later.

---
 cmd/fpga.c | 166 +++++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 122 insertions(+), 44 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 039803870b02..9c715db80512 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -30,15 +30,42 @@ static long do_fpga_get_device(char *arg)
 	return dev;
 }
 
+static int do_fpga_check_params(long *dev, long *fpga_data, size_t *data_size,
+				cmd_tbl_t *cmdtp, int argc, char *const argv[])
+{
+	size_t local_data_size;
+	long local_fpga_data;
+
+	debug("%s %d, %d\n", __func__, argc, cmdtp->maxargs);
+
+	if (argc != cmdtp->maxargs) {
+		debug("fpga: incorrect parameters passed\n");
+		return CMD_RET_USAGE;
+	}
+
+	*dev = do_fpga_get_device(argv[0]);
+
+	local_fpga_data = simple_strtol(argv[1], NULL, 16);
+	if (!local_fpga_data) {
+		debug("fpga: zero fpga_data address\n");
+		return CMD_RET_USAGE;
+	}
+	*fpga_data = local_fpga_data;
+
+	local_data_size = simple_strtoul(argv[2], NULL, 16);
+	if (!local_data_size) {
+		debug("fpga: zero size\n");
+		return CMD_RET_USAGE;
+	}
+	*data_size = local_data_size;
+
+	return 0;
+}
+
 /* Local defines */
 enum {
 	FPGA_NONE = -1,
-	FPGA_LOAD,
-	FPGA_LOADB,
-	FPGA_DUMP,
 	FPGA_LOADMK,
-	FPGA_LOADP,
-	FPGA_LOADBP,
 	FPGA_LOADFS,
 	FPGA_LOADS,
 };
@@ -51,28 +78,14 @@ static int fpga_get_op(char *opstr)
 {
 	int op = FPGA_NONE;
 
-	if (!strcmp("loadb", opstr))
-		op = FPGA_LOADB;
-	else if (!strcmp("load", opstr))
-		op = FPGA_LOAD;
-#if defined(CONFIG_CMD_FPGA_LOADP)
-	else if (!strcmp("loadp", opstr))
-		op = FPGA_LOADP;
-#endif
-#if defined(CONFIG_CMD_FPGA_LOADBP)
-	else if (!strcmp("loadbp", opstr))
-		op = FPGA_LOADBP;
-#endif
 #if defined(CONFIG_CMD_FPGA_LOADFS)
-	else if (!strcmp("loadfs", opstr))
+	if (!strcmp("loadfs", opstr))
 		op = FPGA_LOADFS;
 #endif
 #if defined(CONFIG_CMD_FPGA_LOADMK)
 	else if (!strcmp("loadmk", opstr))
 		op = FPGA_LOADMK;
 #endif
-	else if (!strcmp("dump", opstr))
-		op = FPGA_DUMP;
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 	else if (!strcmp("loads", opstr))
 		op = FPGA_LOADS;
@@ -208,26 +221,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	}
 
 	switch (op) {
-	case FPGA_LOAD:
-		rc = fpga_load(dev, fpga_data, data_size, BIT_FULL);
-		break;
-
-#if defined(CONFIG_CMD_FPGA_LOADP)
-	case FPGA_LOADP:
-		rc = fpga_load(dev, fpga_data, data_size, BIT_PARTIAL);
-		break;
-#endif
-
-	case FPGA_LOADB:
-		rc = fpga_loadbitstream(dev, fpga_data, data_size, BIT_FULL);
-		break;
-
-#if defined(CONFIG_CMD_FPGA_LOADBP)
-	case FPGA_LOADBP:
-		rc = fpga_loadbitstream(dev, fpga_data, data_size, BIT_PARTIAL);
-		break;
-#endif
-
 #if defined(CONFIG_CMD_FPGA_LOADFS)
 	case FPGA_LOADFS:
 		rc = fpga_fsload(dev, fpga_data, data_size, &fpga_fsinfo);
@@ -330,10 +323,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		break;
 #endif
 
-	case FPGA_DUMP:
-		rc = fpga_dump(dev, fpga_data, data_size);
-		break;
-
 	default:
 		printf("Unknown operation\n");
 		return CMD_RET_USAGE;
@@ -349,8 +338,97 @@ static int do_fpga_info(cmd_tbl_t *cmdtp, int flag, int argc,
 	return fpga_info(dev);
 }
 
+static int do_fpga_dump(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	size_t data_size = 0;
+	long fpga_data, dev;
+	int ret;
+
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
+
+	return fpga_dump(dev, (void *)fpga_data, data_size);
+}
+
+static int do_fpga_load(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	size_t data_size = 0;
+	long fpga_data, dev;
+	int ret;
+
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
+
+	return fpga_load(dev, (void *)fpga_data, data_size, BIT_FULL);
+}
+
+static int do_fpga_loadb(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char * const argv[])
+{
+	size_t data_size = 0;
+	long fpga_data, dev;
+	int ret;
+
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
+
+	return fpga_loadbitstream(dev, (void *)fpga_data, data_size, BIT_FULL);
+}
+
+#if defined(CONFIG_CMD_FPGA_LOADP)
+static int do_fpga_loadp(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char * const argv[])
+{
+	size_t data_size = 0;
+	long fpga_data, dev;
+	int ret;
+
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
+
+	return fpga_load(dev, (void *)fpga_data, data_size, BIT_PARTIAL);
+}
+#endif
+
+#if defined(CONFIG_CMD_FPGA_LOADBP)
+static int do_fpga_loadbp(cmd_tbl_t *cmdtp, int flag, int argc,
+			  char * const argv[])
+{
+	size_t data_size = 0;
+	long fpga_data, dev;
+	int ret;
+
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
+
+	return fpga_loadbitstream(dev, (void *)fpga_data, data_size,
+				  BIT_PARTIAL);
+}
+#endif
+
 static cmd_tbl_t fpga_commands[] = {
 	U_BOOT_CMD_MKENT(info, 1, 1, do_fpga_info, "", ""),
+	U_BOOT_CMD_MKENT(dump, 3, 1, do_fpga_dump, "", ""),
+	U_BOOT_CMD_MKENT(load, 3, 1, do_fpga_load, "", ""),
+	U_BOOT_CMD_MKENT(loadb, 3, 1, do_fpga_loadb, "", ""),
+#if defined(CONFIG_CMD_FPGA_LOADP)
+	U_BOOT_CMD_MKENT(loadp, 3, 1, do_fpga_loadp, "", ""),
+#endif
+#if defined(CONFIG_CMD_FPGA_LOADBP)
+	U_BOOT_CMD_MKENT(loadbp, 3, 1, do_fpga_loadbp, "", ""),
+#endif
 };
 
 static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
-- 
1.9.1

[U-Boot] [PATCH v2 12/17] cmd: fpga: Fix loadfs command

[Michal Simek]

Convert loadfs command to fpga subcommands.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 58 ++++++++++++++++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 9c715db80512..826f63371a66 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -66,7 +66,6 @@ static int do_fpga_check_params(long *dev, long *fpga_data, size_t *data_size,
 enum {
 	FPGA_NONE = -1,
 	FPGA_LOADMK,
-	FPGA_LOADFS,
 	FPGA_LOADS,
 };
 
@@ -78,12 +77,8 @@ static int fpga_get_op(char *opstr)
 {
 	int op = FPGA_NONE;
 
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	if (!strcmp("loadfs", opstr))
-		op = FPGA_LOADFS;
-#endif
 #if defined(CONFIG_CMD_FPGA_LOADMK)
-	else if (!strcmp("loadmk", opstr))
+	if (!strcmp("loadmk", opstr))
 		op = FPGA_LOADMK;
 #endif
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
@@ -114,10 +109,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	const char *fit_uname = NULL;
 	ulong fit_addr;
 #endif
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	fpga_fs_info fpga_fsinfo;
-	fpga_fsinfo.fstype = FS_TYPE_ANY;
-#endif
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 	struct fpga_secure_info fpga_sec_info;
 
@@ -140,19 +131,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	case FPGA_NONE:
 		printf("Unknown fpga operation \"%s\"\n", argv[1]);
 		return CMD_RET_USAGE;
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	case FPGA_LOADFS:
-		if (argc < 9)
-			return CMD_RET_USAGE;
-		fpga_fsinfo.blocksize = (unsigned int)
-					simple_strtoul(argv[5], NULL, 16);
-		fpga_fsinfo.interface = argv[6];
-		fpga_fsinfo.dev_part = argv[7];
-		fpga_fsinfo.filename = argv[8];
-
-		argc = 5;
-		break;
-#endif
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 	case FPGA_LOADS:
 		if (argc < 7)
@@ -221,18 +199,11 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	}
 
 	switch (op) {
-#if defined(CONFIG_CMD_FPGA_LOADFS)
-	case FPGA_LOADFS:
-		rc = fpga_fsload(dev, fpga_data, data_size, &fpga_fsinfo);
-		break;
-#endif
-
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 	case FPGA_LOADS:
 		rc = fpga_loads(dev, fpga_data, data_size, &fpga_sec_info);
 		break;
 #endif
-
 #if defined(CONFIG_CMD_FPGA_LOADMK)
 	case FPGA_LOADMK:
 		switch (genimg_get_format(fpga_data)) {
@@ -330,6 +301,30 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	return rc;
 }
 
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+static int do_fpga_loadfs(cmd_tbl_t *cmdtp, int flag, int argc,
+			  char *const argv[])
+{
+	size_t data_size = 0;
+	long fpga_data, dev;
+	int ret;
+	fpga_fs_info fpga_fsinfo;
+
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
+
+	fpga_fsinfo.fstype = FS_TYPE_ANY;
+	fpga_fsinfo.blocksize = (unsigned int)simple_strtoul(argv[3], NULL, 16);
+	fpga_fsinfo.interface = argv[4];
+	fpga_fsinfo.dev_part = argv[5];
+	fpga_fsinfo.filename = argv[6];
+
+	return fpga_fsload(dev, (void *)fpga_data, data_size, &fpga_fsinfo);
+}
+#endif
+
 static int do_fpga_info(cmd_tbl_t *cmdtp, int flag, int argc,
 			char * const argv[])
 {
@@ -429,6 +424,9 @@ static cmd_tbl_t fpga_commands[] = {
 #if defined(CONFIG_CMD_FPGA_LOADBP)
 	U_BOOT_CMD_MKENT(loadbp, 3, 1, do_fpga_loadbp, "", ""),
 #endif
+#if defined(CONFIG_CMD_FPGA_LOADFS)
+	U_BOOT_CMD_MKENT(loadfs, 7, 1, do_fpga_loadfs, "", ""),
+#endif
 };
 
 static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
-- 
1.9.1

[U-Boot] [PATCH v2 13/17] cmd: fpga: Fix loadmk command

[Michal Simek]

Convert loadmk command to fpga subcommands. Not all combinations are
working but they have never worked properly. This will be fixed later.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 238 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 123 insertions(+), 115 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 826f63371a66..9cb0116af734 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -65,7 +65,6 @@ static int do_fpga_check_params(long *dev, long *fpga_data, size_t *data_size,
 /* Local defines */
 enum {
 	FPGA_NONE = -1,
-	FPGA_LOADMK,
 	FPGA_LOADS,
 };
 
@@ -77,12 +76,8 @@ static int fpga_get_op(char *opstr)
 {
 	int op = FPGA_NONE;
 
-#if defined(CONFIG_CMD_FPGA_LOADMK)
-	if (!strcmp("loadmk", opstr))
-		op = FPGA_LOADMK;
-#endif
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	else if (!strcmp("loads", opstr))
+	if (!strcmp("loads", opstr))
 		op = FPGA_LOADS;
 #endif
 
@@ -102,24 +97,13 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	int op, dev = FPGA_INVALID_DEVICE;
 	size_t data_size = 0;
 	void *fpga_data = NULL;
-	char *devstr = env_get("fpga");
-	char *datastr = env_get("fpgadata");
 	int rc = FPGA_FAIL;
-#if defined(CONFIG_FIT)
-	const char *fit_uname = NULL;
-	ulong fit_addr;
-#endif
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
 	struct fpga_secure_info fpga_sec_info;
 
 	memset(&fpga_sec_info, 0, sizeof(fpga_sec_info));
 #endif
 
-	if (devstr)
-		dev = (int) simple_strtoul(devstr, NULL, 16);
-	if (datastr)
-		fpga_data = (void *)simple_strtoul(datastr, NULL, 16);
-
 	if (argc > 9 || argc < 2) {
 		debug("%s: Too many or too few args (%d)\n", __func__, argc);
 		return CMD_RET_USAGE;
@@ -169,15 +153,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 			return CMD_RET_USAGE;
 		}
 	case 4:		/* fpga <op> <dev> <data> */
-#if defined(CONFIG_FIT)
-		if (fit_parse_subimage(argv[3], (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 *)simple_strtoul(argv[3], NULL, 16);
 			debug("*  fpga: cmdline image address = 0x%08lx\n",
@@ -204,95 +179,6 @@ int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 		rc = fpga_loads(dev, fpga_data, data_size, &fpga_sec_info);
 		break;
 #endif
-#if defined(CONFIG_CMD_FPGA_LOADMK)
-	case FPGA_LOADMK:
-		switch (genimg_get_format(fpga_data)) {
-#if defined(CONFIG_IMAGE_FORMAT_LEGACY)
-		case IMAGE_FORMAT_LEGACY:
-			{
-				image_header_t *hdr =
-						(image_header_t *)fpga_data;
-				ulong data;
-				uint8_t comp;
-
-				comp = image_get_comp(hdr);
-				if (comp == IH_COMP_GZIP) {
-#if defined(CONFIG_GZIP)
-					ulong image_buf = image_get_data(hdr);
-					data = image_get_load(hdr);
-					ulong image_size = ~0UL;
-
-					if (gunzip((void *)data, ~0UL,
-						   (void *)image_buf,
-						   &image_size) != 0) {
-						puts("GUNZIP: error\n");
-						return 1;
-					}
-					data_size = image_size;
-#else
-					puts("Gunzip image is not supported\n");
-					return 1;
-#endif
-				} else {
-					data = (ulong)image_get_data(hdr);
-					data_size = image_get_data_size(hdr);
-				}
-				rc = fpga_load(dev, (void *)data, data_size,
-					       BIT_FULL);
-			}
-			break;
-#endif
-#if defined(CONFIG_FIT)
-		case IMAGE_FORMAT_FIT:
-			{
-				const void *fit_hdr = (const void *)fpga_data;
-				int noffset;
-				const void *fit_data;
-
-				if (fit_uname == NULL) {
-					puts("No FIT subimage unit name\n");
-					return 1;
-				}
-
-				if (!fit_check_format(fit_hdr)) {
-					puts("Bad FIT image format\n");
-					return 1;
-				}
-
-				/* get fpga component image node offset */
-				noffset = fit_image_get_node(fit_hdr,
-							     fit_uname);
-				if (noffset < 0) {
-					printf("Can't find '%s' FIT subimage\n",
-					       fit_uname);
-					return 1;
-				}
-
-				/* verify integrity */
-				if (!fit_image_verify(fit_hdr, noffset)) {
-					puts ("Bad Data Hash\n");
-					return 1;
-				}
-
-				/* get fpga subimage data address and length */
-				if (fit_image_get_data(fit_hdr, noffset,
-						       &fit_data, &data_size)) {
-					puts("Fpga subimage data not found\n");
-					return 1;
-				}
-
-				rc = fpga_load(dev, fit_data, data_size,
-					       BIT_FULL);
-			}
-			break;
-#endif
-		default:
-			puts("** Unknown image type\n");
-			rc = FPGA_FAIL;
-			break;
-		}
-		break;
-#endif
 
 	default:
 		printf("Unknown operation\n");
@@ -413,6 +299,125 @@ static int do_fpga_loadbp(cmd_tbl_t *cmdtp, int flag, int argc,
 }
 #endif
 
+#if defined(CONFIG_CMD_FPGA_LOADMK)
+static int do_fpga_loadmk(cmd_tbl_t *cmdtp, int flag, int argc,
+			  char * const argv[])
+{
+	size_t data_size = 0;
+	void *fpga_data = NULL;
+#if defined(CONFIG_FIT)
+	const char *fit_uname = NULL;
+	ulong fit_addr;
+#endif
+	ulong dev = do_fpga_get_device(argv[0]);
+	char *datastr = env_get("fpgadata");
+
+	if (datastr)
+		fpga_data = (void *)simple_strtoul(datastr, NULL, 16);
+
+	if (argc == 2) {
+#if defined(CONFIG_FIT)
+		if (fit_parse_subimage(argv[1], (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 *)simple_strtoul(argv[1], NULL, 16);
+			debug("*  fpga: cmdline image address = 0x%08lx\n",
+			      (ulong)fpga_data);
+		}
+		debug("%s: fpga_data = 0x%lx\n", __func__, (ulong)fpga_data);
+		if (!fpga_data) {
+			puts("Zero fpga_data address\n");
+			return CMD_RET_USAGE;
+		}
+	}
+
+	switch (genimg_get_format(fpga_data)) {
+#if defined(CONFIG_IMAGE_FORMAT_LEGACY)
+	case IMAGE_FORMAT_LEGACY:
+	{
+		image_header_t *hdr = (image_header_t *)fpga_data;
+		ulong data;
+		u8 comp;
+
+		comp = image_get_comp(hdr);
+		if (comp == IH_COMP_GZIP) {
+#if defined(CONFIG_GZIP)
+			ulong image_buf = image_get_data(hdr);
+			ulong image_size = ~0UL;
+
+			data = image_get_load(hdr);
+
+			if (gunzip((void *)data, ~0UL, (void *)image_buf,
+				   &image_size) != 0) {
+				puts("GUNZIP: error\n");
+				return 1;
+			}
+			data_size = image_size;
+#else
+			puts("Gunzip image is not supported\n");
+			return 1;
+#endif
+		} else {
+			data = (ulong)image_get_data(hdr);
+			data_size = image_get_data_size(hdr);
+		}
+		return fpga_load(dev, (void *)data, data_size,
+				  BIT_FULL);
+	}
+#endif
+#if defined(CONFIG_FIT)
+	case IMAGE_FORMAT_FIT:
+	{
+		const void *fit_hdr = (const void *)fpga_data;
+		int noffset;
+		const void *fit_data;
+
+		if (!fit_uname) {
+			puts("No FIT subimage unit name\n");
+			return 1;
+		}
+
+		if (!fit_check_format(fit_hdr)) {
+			puts("Bad FIT image format\n");
+			return 1;
+		}
+
+		/* get fpga component image node offset */
+		noffset = fit_image_get_node(fit_hdr, fit_uname);
+		if (noffset < 0) {
+			printf("Can't find '%s' FIT subimage\n", fit_uname);
+			return 1;
+		}
+
+		/* verify integrity */
+		if (!fit_image_verify(fit_hdr, noffset)) {
+			puts("Bad Data Hash\n");
+			return 1;
+		}
+
+		/* get fpga subimage data address and length */
+		if (fit_image_get_data(fit_hdr, noffset, &fit_data,
+				       &data_size)) {
+			puts("Fpga subimage data not found\n");
+			return 1;
+		}
+
+		return fpga_load(dev, fit_data, data_size, BIT_FULL);
+	}
+#endif
+	default:
+		puts("** Unknown image type\n");
+		return FPGA_FAIL;
+	}
+}
+#endif
+
 static cmd_tbl_t fpga_commands[] = {
 	U_BOOT_CMD_MKENT(info, 1, 1, do_fpga_info, "", ""),
 	U_BOOT_CMD_MKENT(dump, 3, 1, do_fpga_dump, "", ""),
@@ -427,6 +432,9 @@ static cmd_tbl_t fpga_commands[] = {
 #if defined(CONFIG_CMD_FPGA_LOADFS)
 	U_BOOT_CMD_MKENT(loadfs, 7, 1, do_fpga_loadfs, "", ""),
 #endif
+#if defined(CONFIG_CMD_FPGA_LOADMK)
+	U_BOOT_CMD_MKENT(loadmk, 2, 1, do_fpga_loadmk, "", ""),
+#endif
 };
 
 static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
-- 
1.9.1

[U-Boot] [PATCH v2 14/17] cmd: fpga: Add support for missing fpga loadmk commands

[Michal Simek]

There are ways how to call fpga loadmk

1. Full command
fpga loadmk [dev] [address]

2. Dev setup via variable
set fpga [dev]
fpga loadmk [address]

3. Address setup via variable
set fpgadata [address]
fpga loadmk [dev]

4. Dev and address setup via variables
set fpga [dev]
set fpgadata [address]
fpga loadmk

Before this patch only cases 1 and 3 are working but the part of code
was trying to support also cases 2 and 4.
This patch is adding support for cases 2 and 4 to have all of
combinations supported.

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

Changes in v2:
- Extend non complete commit message

Changes in v1:
- New patch from RFC

 cmd/fpga.c | 55 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 9cb0116af734..de9d19dd8e91 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -22,7 +22,7 @@ static long do_fpga_get_device(char *arg)
 		/* Should be strtol to handle -1 cases */
 		dev = simple_strtol(devstr, NULL, 16);
 
-	if (arg)
+	if (dev == FPGA_INVALID_DEVICE && arg)
 		dev = simple_strtol(arg, NULL, 16);
 
 	debug("%s: device = %ld\n", __func__, dev);
@@ -312,29 +312,44 @@ static int do_fpga_loadmk(cmd_tbl_t *cmdtp, int flag, int argc,
 	ulong dev = do_fpga_get_device(argv[0]);
 	char *datastr = env_get("fpgadata");
 
-	if (datastr)
-		fpga_data = (void *)simple_strtoul(datastr, NULL, 16);
+	debug("fpga: argc %x, dev %lx, datastr %s\n", argc, dev, datastr);
+
+	if (dev == FPGA_INVALID_DEVICE) {
+		debug("fpga: Invalid fpga device\n");
+		return CMD_RET_USAGE;
+	}
+
+	if (argc == 0 && !datastr) {
+		debug("fpga: No datastr passed\n");
+		return CMD_RET_USAGE;
+	}
 
 	if (argc == 2) {
+		datastr = argv[1];
+		debug("fpga: Full command with two args\n");
+	} else if (argc == 1 && !datastr) {
+		debug("fpga: Dev is setup - fpgadata passed\n");
+		datastr = argv[0];
+	}
+
 #if defined(CONFIG_FIT)
-		if (fit_parse_subimage(argv[1], (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
+	if (fit_parse_subimage(datastr, (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 *)simple_strtoul(argv[1], NULL, 16);
-			debug("*  fpga: cmdline image address = 0x%08lx\n",
-			      (ulong)fpga_data);
-		}
-		debug("%s: fpga_data = 0x%lx\n", __func__, (ulong)fpga_data);
-		if (!fpga_data) {
-			puts("Zero fpga_data address\n");
-			return CMD_RET_USAGE;
-		}
+	{
+		fpga_data = (void *)simple_strtoul(datastr, NULL, 16);
+		debug("*  fpga: cmdline image address = 0x%08lx\n",
+		      (ulong)fpga_data);
+	}
+	debug("%s: fpga_data = 0x%lx\n", __func__, (ulong)fpga_data);
+	if (!fpga_data) {
+		puts("Zero fpga_data address\n");
+		return CMD_RET_USAGE;
 	}
 
 	switch (genimg_get_format(fpga_data)) {
-- 
1.9.1

[U-Boot] [PATCH v2 15/17] cmd: fpga: Use CMD_RET_FAILURE instead of simple 1

[Michal Simek]

Use standard return command failure macro.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index de9d19dd8e91..89fae86b8253 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -371,7 +371,7 @@ static int do_fpga_loadmk(cmd_tbl_t *cmdtp, int flag, int argc,
 			if (gunzip((void *)data, ~0UL, (void *)image_buf,
 				   &image_size) != 0) {
 				puts("GUNZIP: error\n");
-				return 1;
+				return CMD_RET_FAILURE;
 			}
 			data_size = image_size;
 #else
@@ -395,32 +395,32 @@ static int do_fpga_loadmk(cmd_tbl_t *cmdtp, int flag, int argc,
 
 		if (!fit_uname) {
 			puts("No FIT subimage unit name\n");
-			return 1;
+			return CMD_RET_FAILURE;
 		}
 
 		if (!fit_check_format(fit_hdr)) {
 			puts("Bad FIT image format\n");
-			return 1;
+			return CMD_RET_FAILURE;
 		}
 
 		/* get fpga component image node offset */
 		noffset = fit_image_get_node(fit_hdr, fit_uname);
 		if (noffset < 0) {
 			printf("Can't find '%s' FIT subimage\n", fit_uname);
-			return 1;
+			return CMD_RET_FAILURE;
 		}
 
 		/* verify integrity */
 		if (!fit_image_verify(fit_hdr, noffset)) {
 			puts("Bad Data Hash\n");
-			return 1;
+			return CMD_RET_FAILURE;
 		}
 
 		/* get fpga subimage data address and length */
 		if (fit_image_get_data(fit_hdr, noffset, &fit_data,
 				       &data_size)) {
 			puts("Fpga subimage data not found\n");
-			return 1;
+			return CMD_RET_FAILURE;
 		}
 
 		return fpga_load(dev, fit_data, data_size, BIT_FULL);
@@ -428,7 +428,7 @@ static int do_fpga_loadmk(cmd_tbl_t *cmdtp, int flag, int argc,
 #endif
 	default:
 		puts("** Unknown image type\n");
-		return FPGA_FAIL;
+		return CMD_RET_FAILURE;
 	}
 }
 #endif
-- 
1.9.1

[U-Boot] [PATCH v2 16/17] cmd: fpga: Fix loads command

[Michal Simek]

Convert last loads command to fpga subcommands.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 cmd/fpga.c | 148 +++++++++++++++----------------------------------------------
 1 file changed, 36 insertions(+), 112 deletions(-)

diff --git a/cmd/fpga.c b/cmd/fpga.c
index 89fae86b8253..88a8e3f3186b 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -62,130 +62,57 @@ static int do_fpga_check_params(long *dev, long *fpga_data, size_t *data_size,
 	return 0;
 }
 
-/* Local defines */
-enum {
-	FPGA_NONE = -1,
-	FPGA_LOADS,
-};
-
-/*
- * Map op to supported operations.  We don't use a table since we
- * would just have to relocate it from flash anyway.
- */
-static int fpga_get_op(char *opstr)
-{
-	int op = FPGA_NONE;
-
 #if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	if (!strcmp("loads", opstr))
-		op = FPGA_LOADS;
-#endif
-
-	return op;
-}
-
-/* ------------------------------------------------------------------------- */
-/* command form:
- *   fpga <op> <device number> <data addr> <datasize>
- * where op is 'load', 'dump', or 'info'
- * If there is no device number field, the fpga environment variable is used.
- * If there is no data addr field, the fpgadata environment variable is used.
- * The info command requires no data address field.
- */
-int do_fpga(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+int do_fpga_loads(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
-	int op, dev = FPGA_INVALID_DEVICE;
 	size_t data_size = 0;
-	void *fpga_data = NULL;
-	int rc = FPGA_FAIL;
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
+	long fpga_data, dev;
+	int ret;
 	struct fpga_secure_info fpga_sec_info;
 
 	memset(&fpga_sec_info, 0, sizeof(fpga_sec_info));
-#endif
 
-	if (argc > 9 || argc < 2) {
-		debug("%s: Too many or too few args (%d)\n", __func__, argc);
+	if (argc < 5) {
+		debug("fpga: incorrect parameters passed\n");
 		return CMD_RET_USAGE;
 	}
 
-	op = fpga_get_op(argv[1]);
-
-	switch (op) {
-	case FPGA_NONE:
-		printf("Unknown fpga operation \"%s\"\n", argv[1]);
+	if (argc == 6)
+		fpga_sec_info.userkey_addr = (u8 *)(uintptr_t)
+					      simple_strtoull(argv[5],
+							      NULL, 16);
+	else
+		/*
+		 * If 6th parameter is not passed then do_fpga_check_params
+		 * will get 5 instead of expected 6 which means that function
+		 * return CMD_RET_USAGE. Increase number of params +1 to pass
+		 * this.
+		 */
+		argc++;
+
+	fpga_sec_info.encflag = (u8)simple_strtoul(argv[4], NULL, 16);
+	fpga_sec_info.authflag = (u8)simple_strtoul(argv[3], NULL, 16);
+
+	if (fpga_sec_info.authflag >= FPGA_NO_ENC_OR_NO_AUTH &&
+	    fpga_sec_info.encflag >= FPGA_NO_ENC_OR_NO_AUTH) {
+		debug("fpga: Use <fpga load> for NonSecure bitstream\n");
 		return CMD_RET_USAGE;
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	case FPGA_LOADS:
-		if (argc < 7)
-			return CMD_RET_USAGE;
-		if (argc == 8)
-			fpga_sec_info.userkey_addr = (u8 *)(uintptr_t)
-						     simple_strtoull(argv[7],
-								     NULL, 16);
-		fpga_sec_info.encflag = (u8)simple_strtoul(argv[6], NULL, 16);
-		fpga_sec_info.authflag = (u8)simple_strtoul(argv[5], NULL, 16);
-
-		if (fpga_sec_info.authflag >= FPGA_NO_ENC_OR_NO_AUTH &&
-		    fpga_sec_info.encflag >= FPGA_NO_ENC_OR_NO_AUTH) {
-			puts("ERR: Use <fpga load> for NonSecure bitstream\n");
-			return CMD_RET_USAGE;
-		}
-
-		if (fpga_sec_info.encflag == FPGA_ENC_USR_KEY &&
-		    !fpga_sec_info.userkey_addr) {
-			puts("ERR: User key not provided\n");
-			return CMD_RET_USAGE;
-		}
-
-		argc = 5;
-		break;
-#endif
-	default:
-		break;
 	}
 
-	switch (argc) {
-	case 5:		/* fpga <op> <dev> <data> <datasize> */
-		data_size = simple_strtoul(argv[4], NULL, 16);
-		if (!data_size) {
-			puts("Zero data_size\n");
-			return CMD_RET_USAGE;
-		}
-	case 4:		/* fpga <op> <dev> <data> */
-		{
-			fpga_data = (void *)simple_strtoul(argv[3], NULL, 16);
-			debug("*  fpga: cmdline image address = 0x%08lx\n",
-			      (ulong)fpga_data);
-		}
-		debug("%s: fpga_data = 0x%lx\n", __func__, (ulong)fpga_data);
-		if (!fpga_data) {
-			puts("Zero fpga_data address\n");
-			return CMD_RET_USAGE;
-		}
-	case 3:		/* fpga <op> <dev | data addr> */
-		dev = (int)simple_strtoul(argv[2], NULL, 16);
-		debug("%s: device = %d\n", __func__, dev);
-	}
-
-	if (dev == FPGA_INVALID_DEVICE) {
-		puts("FPGA device not specified\n");
+	if (fpga_sec_info.encflag == FPGA_ENC_USR_KEY &&
+	    !fpga_sec_info.userkey_addr) {
+		debug("fpga: User key not provided\n");
 		return CMD_RET_USAGE;
 	}
 
-	switch (op) {
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
-	case FPGA_LOADS:
-		rc = fpga_loads(dev, fpga_data, data_size, &fpga_sec_info);
-		break;
-#endif
+	ret = do_fpga_check_params(&dev, &fpga_data, &data_size,
+				   cmdtp, argc, argv);
+	if (ret)
+		return ret;
 
-	default:
-		printf("Unknown operation\n");
-		return CMD_RET_USAGE;
-	}
-	return rc;
+	return fpga_loads(dev, (void *)fpga_data, data_size, &fpga_sec_info);
 }
+#endif
 
 #if defined(CONFIG_CMD_FPGA_LOADFS)
 static int do_fpga_loadfs(cmd_tbl_t *cmdtp, int flag, int argc,
@@ -450,6 +377,9 @@ static cmd_tbl_t fpga_commands[] = {
 #if defined(CONFIG_CMD_FPGA_LOADMK)
 	U_BOOT_CMD_MKENT(loadmk, 2, 1, do_fpga_loadmk, "", ""),
 #endif
+#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
+	U_BOOT_CMD_MKENT(loads, 6, 1, do_fpga_loads, "", ""),
+#endif
 };
 
 static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
@@ -463,12 +393,6 @@ static int do_fpga_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	fpga_cmd = find_cmd_tbl(argv[1], fpga_commands,
 				ARRAY_SIZE(fpga_commands));
-
-	/* This should be removed when all functions are converted */
-	if (!fpga_cmd)
-		return do_fpga(cmdtp, flag, argc, argv);
-
-	/* FIXME This can't be reached till all functions are converted */
 	if (!fpga_cmd) {
 		debug("fpga: non existing command\n");
 		return CMD_RET_USAGE;
-- 
1.9.1

[U-Boot] [PATCH v2 17/17] MAINTAINERS: Add myself as the FPGA maintainer

[Michal Simek]

FPGA subsystem requires special care that's why it should be maintained
via one tree.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None
Changes in v1: None

 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 58b61ac05882..c9e414006999 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -376,6 +376,14 @@ F:	test/py/tests/test_efi*
 F:	cmd/bootefi.c
 F:	tools/file2include.c
 
+FPGA
+M:	Michal Simek <michal.simek@xilinx.com>
+S:	Maintained
+T:	git git://git.denx.de/u-boot-microblaze.git
+F:	drivers/fpga/
+F:	cmd/fpga.c
+F:	include/fpga.h
+
 FLATTENED DEVICE TREE
 M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
-- 
1.9.1

[U-Boot] [PATCH v2 14/17] cmd: fpga: Add support for missing fpga loadmk commands

[Simon Glass]

On 8 August 2018 at 05:37, Michal Simek <michal.simek@xilinx.com> wrote:
> There are ways how to call fpga loadmk
>
> 1. Full command
> fpga loadmk [dev] [address]
>
> 2. Dev setup via variable
> set fpga [dev]
> fpga loadmk [address]
>
> 3. Address setup via variable
> set fpgadata [address]
> fpga loadmk [dev]
>
> 4. Dev and address setup via variables
> set fpga [dev]
> set fpgadata [address]
> fpga loadmk
>
> Before this patch only cases 1 and 3 are working but the part of code
> was trying to support also cases 2 and 4.
> This patch is adding support for cases 2 and 4 to have all of
> combinations supported.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Changes in v2:
> - Extend non complete commit message
>
> Changes in v1:
> - New patch from RFC
>
>  cmd/fpga.c | 55 +++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 35 insertions(+), 20 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>