Hi Ravi,
> Hi Lukasz
>
> >> Adding support functions to run dfu spl commands.
> >>
> >> Signed-off-by: Ravi Babu <ravib...@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 <ravib...@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"
>
> Ok. Will change 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
>
> Fine.
>
> >
> >> + 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
>
> Actually not required, I felt it is better to use default environment
> inorder to get the latest environment from SPL which is being loaded.
> I will check once without set_default_env().
>
> >
> >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?
>
> This step is required, "dfu_alt_info" is one referred by dfu driver.
But is it necessary to store "dfu_alt_info" env variable to memory
medium (e.g. eMMC) or is it enough to use the default value stored in
RAM?
>
> >
> >If you want to store default envs in the medium (e.g. eMMC), I think
> >that it would be easier to call "saveenv".
>
> Ok.
>
> >Otherwise, I would stay with default envs, since we only use this
> >u-boot for flashing other binaries.
>
> Right.
>
> Regards
> Ravi
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
