[U-Boot] Kconfig: update FASTBOOT_FLASH_MMC_DEV

handle FASTBOOT_FLASH_MMC_DEV default properly

Signed-off-by: Steve Rae <steve.rae@raedomain.com>
---
I was hoping that the FASTBOOT_FLASH_MMC_DEV Kconfig option could be
an integer (eg. 0, 1, or 2 etc.) or undefined (to signify that it
is not being used). However, it seems that (Kconfig experts please!)
this is not correct within Kconfig.
Therefore, I have implemented "-1" to signify that it is not used.
Is this the "best practice" for handling this scenario?

 cmd/fastboot/Kconfig            |  4 +++-
 common/Makefile                 |  4 +++-
 drivers/usb/gadget/f_fastboot.c | 12 ++++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

On Aug 27, 2016 15:16, "Steve Rae" <steve.rae@raedomain.com> wrote:
>
> handle FASTBOOT_FLASH_MMC_DEV default properly
>
> Signed-off-by: Steve Rae <steve.rae@raedomain.com>
> ---
> I was hoping that the FASTBOOT_FLASH_MMC_DEV Kconfig option could be
> an integer (eg. 0, 1, or 2 etc.) or undefined (to signify that it
> is not being used). However, it seems that (Kconfig experts please!)
> this is not correct within Kconfig.
> Therefore, I have implemented "-1" to signify that it is not used.
> Is this the "best practice" for handling this scenario?
>
>  cmd/fastboot/Kconfig            |  4 +++-
>  common/Makefile                 |  4 +++-
>  drivers/usb/gadget/f_fastboot.c | 12 ++++++++----
>  3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/cmd/fastboot/Kconfig b/cmd/fastboot/Kconfig
> index a93d1c0..fdd5475 100644
> --- a/cmd/fastboot/Kconfig
> +++ b/cmd/fastboot/Kconfig
> @@ -50,10 +50,12 @@ config FASTBOOT_FLASH
>
>  config FASTBOOT_FLASH_MMC_DEV
>         int "Define FASTBOOT MMC FLASH default device"
> +       default -1
>         help
>           The fastboot "flash" command requires additional information
>           regarding the non-volatile storage device. Define this to
> -         the eMMC device that fastboot should use to store the image.
> +         the eMMC device that fastboot should use to store the image,
> +         or define '-1' to disable storing to an eMMC device.
>
>  endif # USB_FUNCTION_FASTBOOT
>
> diff --git a/common/Makefile b/common/Makefile
> index 21619b3..56e95b3 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -142,12 +142,14 @@ obj-$(CONFIG_IO_TRACE) += iotrace.o
>  obj-y += memsize.o
>  obj-y += stdio.o
>
> -# This option is not just y/n - it can have a numeric value
>  ifdef CONFIG_FASTBOOT_FLASH
>  obj-y += image-sparse.o
> +# This option is not just y/n - it is a numeric value
>  ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> +ifneq ($(CONFIG_FASTBOOT_FLASH_MMC_DEV), -1)
>  obj-y += fb_mmc.o
>  endif
> +endif
>  ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
>  obj-y += fb_nand.o
>  endif
> diff --git a/drivers/usb/gadget/f_fastboot.c
b/drivers/usb/gadget/f_fastboot.c
> index 2160b1c..5df6b8b 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -21,7 +21,8 @@
>  #include <linux/compiler.h>
>  #include <version.h>
>  #include <g_dnl.h>
> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> +#if defined(CONFIG_FASTBOOT_FLASH_MMC_DEV) && \
> +          (CONFIG_FASTBOOT_FLASH_MMC_DEV != -1)
>  #include <fb_mmc.h>
>  #endif
>  #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> @@ -594,7 +595,8 @@ static void cb_flash(struct usb_ep *ep, struct
usb_request *req)
>         fb_response_str = response;
>
>         fastboot_fail("no flash device defined");
> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> +#if defined(CONFIG_FASTBOOT_FLASH_MMC_DEV) && \
> +          (CONFIG_FASTBOOT_FLASH_MMC_DEV != -1)
>         fb_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
>                            download_bytes);
>  #endif
> @@ -610,7 +612,8 @@ static void cb_flash(struct usb_ep *ep, struct
usb_request *req)
>  static void cb_oem(struct usb_ep *ep, struct usb_request *req)
>  {
>         char *cmd = req->buf;
> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> +#if defined(CONFIG_FASTBOOT_FLASH_MMC_DEV) && \
> +          (CONFIG_FASTBOOT_FLASH_MMC_DEV != -1)
>         if (strncmp("format", cmd + 4, 6) == 0) {
>                 char cmdbuf[32];
>                  sprintf(cmdbuf, "gpt write mmc %x $partitions",
> @@ -646,7 +649,8 @@ static void cb_erase(struct usb_ep *ep, struct
usb_request *req)
>         fb_response_str = response;
>
>         fastboot_fail("no flash device defined");
> -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
> +#if defined(CONFIG_FASTBOOT_FLASH_MMC_DEV) && \
> +          (CONFIG_FASTBOOT_FLASH_MMC_DEV != -1)
>         fb_mmc_erase(cmd);
>  #endif
>  #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
> --
> 1.8.5
>

ping...

Hi Steve,

On 27 August 2016 at 16:15, Steve Rae <steve.rae@raedomain.com> wrote:
> handle FASTBOOT_FLASH_MMC_DEV default properly
>
> Signed-off-by: Steve Rae <steve.rae@raedomain.com>
> ---
> I was hoping that the FASTBOOT_FLASH_MMC_DEV Kconfig option could be
> an integer (eg. 0, 1, or 2 etc.) or undefined (to signify that it
> is not being used). However, it seems that (Kconfig experts please!)
> this is not correct within Kconfig.
> Therefore, I have implemented "-1" to signify that it is not used.
> Is this the "best practice" for handling this scenario?

I think it might be better to have a bool option which
enables/disables the feature, as well as what you have here. Then you
don't need the -1 value.
>
>  cmd/fastboot/Kconfig            |  4 +++-
>  common/Makefile                 |  4 +++-
>  drivers/usb/gadget/f_fastboot.c | 12 ++++++++----
>  3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/cmd/fastboot/Kconfig b/cmd/fastboot/Kconfig
> index a93d1c0..fdd5475 100644
> --- a/cmd/fastboot/Kconfig
> +++ b/cmd/fastboot/Kconfig
> @@ -50,10 +50,12 @@ config FASTBOOT_FLASH
>

Regards,
Simon