Message ID | 1472336143-8532-1-git-send-email-steve.rae@raedomain.com |
---|---|
State | Not Applicable |
Delegated to: | Simon Glass |
Headers | show |
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
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
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(-)