[U-Boot] [RFC PATCH v2 3/5] spl: dfu: adding dfu support functions for SPL-DFU

* [U-Boot] [RFC PATCH v2 3/5] spl: dfu: adding dfu support functions for SPL-DFU
       [not found] ` <1469193550-19125-4-git-send-email-ravibabu@ti.com>
@ 2016-07-25 10:05   ` Lukasz Majewski
  2016-07-25 13:08     ` B, Ravi
  0 siblings, 1 reply; 8+ messages in thread
From: Lukasz Majewski @ 2016-07-25 10:05 UTC (permalink / raw)
  To: u-boot

Hi Ravi,

> Adding support functions to run dfu spl commands.
> 
> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> ---
>  common/spl/Makefile  |    1 +
>  common/spl/spl_dfu.c |   57
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/spl.h        |    8 +++++++ 3 files changed, 66 insertions(+)
>  create mode 100644 common/spl/spl_dfu.c
> 
> diff --git a/common/spl/Makefile b/common/spl/Makefile
> index 2e0f695..0850da0 100644
> --- a/common/spl/Makefile
> +++ b/common/spl/Makefile
> @@ -21,4 +21,5 @@ obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o
>  obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o
>  obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o
>  obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
> +obj-$(CONFIG_SPL_DFU_SUPPORT) += spl_dfu.o
>  endif
> diff --git a/common/spl/spl_dfu.c b/common/spl/spl_dfu.c
> new file mode 100644
> index 0000000..e8d0ba1
> --- /dev/null
> +++ b/common/spl/spl_dfu.c
> @@ -0,0 +1,57 @@
> +/*
> + * (C) Copyright 2016
> + * Texas Instruments, <www.ti.com>
> + *
> + * Ravi B <ravibabu@ti.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <spl.h>
> +#include <linux/compiler.h>
> +#include <errno.h>
> +#include <watchdog.h>
> +#include <console.h>
> +#include <g_dnl.h>
> +#include <usb.h>
> +#include <dfu.h>
> +#include <environment.h>
> +
> +static int run_dfu(int usb_index, char *interface, char *devstring)
> +{
> +	int ret;
> +
> +	ret = dfu_init_env_entities(interface, devstring);
> +	if (ret) {
> +		dfu_free_entities();
> +		goto exit;
> +	}
> +
> +	run_usb_dnl_gadget(usb_index, "usb_dnl_dfu");
> +exit:
> +	dfu_free_entities();
> +	return ret;
> +}
> +
> +int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface,
				     ^^^^^^^^^^^^^^ this is a bit
				     misleading.
				     I would rename it to "alt_info_str"

> char *devstr) +{
> +	char *str_env;
> +	int ret;
> +
> +	/* set default environment */
> +	set_default_env(0);

set_default_env() accepts const char *s as the argument. Please replace
0 -> NULL

> +	str_env = getenv(dfu_alt_info);
> +	if (!str_env) {
> +		error("\"dfu_alt_info\" env variable not
> defined!\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = setenv("dfu_alt_info", str_env);
> +	if (ret) {
> +		error("unable to set env variable
> \"dfu_alt_info\"!\n");
> +		return -EINVAL;
> +	}

I'm a bit confused with this env flow.

Is it required on your platform to:

1. set_default_env(NULL) - which sets default envs (one which are
mostly defined at ./include/<board_config> file) in RAM

2. call getenv with "dfu_alt_info_ram", which reads this value from RAM

3. call setenv() to store already in ram available env variable to some
medium?

If you want to store default envs in the medium (e.g. eMMC), I think
that it would be easier to call "saveenv".

Otherwise, I would stay with default envs, since we only use this
u-boot for flashing other binaries.

> +
> +	/* invoke dfu command */
> +	return run_dfu(usbctrl, interface, devstr);
> +}
> diff --git a/include/spl.h b/include/spl.h
> index 2360466..1524e26 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -140,4 +140,12 @@ void spl_board_init(void);
>   */
>  bool spl_was_boot_source(void);
>  
> +/**
> + * spl_dfu_cmd- run dfu command with chosen mmc device interface
> + * @param usb_index - usb controller number
> + * @param mmc_dev -  mmc device nubmer
> + *
> + * @return 0 on success, otherwise error code
> + */
> +int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface,
> char *devstr); #endif

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

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