Enable DFU for RAM on Allwinner devices
Subsystems (2)
cover Enable DFU for RAM on Allwinner devices
Hello,
DFU allows to transfer large files (such as initrd images) much faster than FEL.
Siarhei Siamashka (2):
sunxi: Enable DFU for RAM
musb: sunxi: Implement dfu_usb_get_reset()
drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-)
Signature · 1 line
2.4.10
PATCH 1/2 sunxi: Enable DFU for RAM
The DFU protocol implementation in U-Boot is much faster than the FEL protocol implementation in the boot ROM on Allwinner devices. Using DFU instead of FEL improves the USB transfer speed from 500-900 KB/s to 3.2-3.7 MB/s. This is particularly useful for reducing the time needed for booting systems with large initrd images.
FEL is still useful for loading the U-Boot bootloader and a boot script, which may then activate DFU in the following way:
setenv dfu_alt_info ${dfu_alt_info_ram}
dfu 0 ram 0
bootm ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}The rest of the files can be transferred to the device using the "dfu-util" tool.
--- include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
include/configs/sunxi-common.h +25 -5
@@ -337,6 +337,7 @@ extern int soft_i2c_gpio_scl;#define CONFIG_USB_GADGET_VBUS_DRAW 0#define CONFIG_USB_GADGET_DOWNLOAD +#define CONFIG_USB_FUNCTION_DFU #define CONFIG_USB_FUNCTION_FASTBOOT #define CONFIG_USB_FUNCTION_MASS_STORAGE #endif
@@ -347,6 +348,11 @@ extern int soft_i2c_gpio_scl;#define CONFIG_G_DNL_MANUFACTURER "Allwinner Technology"#endif +#ifdef CONFIG_USB_FUNCTION_DFU +#define CONFIG_CMD_DFU +#define CONFIG_DFU_RAM +#endif +#ifdef CONFIG_USB_FUNCTION_FASTBOOT #define CONFIG_CMD_FASTBOOT #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
@@ -394,13 +400,26 @@ extern int soft_i2c_gpio_scl;* 32M uncompressed kernel, 16M compressed kernel, 1M fdt, * 1M script, 1M pxe and the ramdisk at the end. */ + +#define KERNEL_ADDR_R __stringify(SDRAM_OFFSET(2000000)) +#define FDT_ADDR_R __stringify(SDRAM_OFFSET(3000000)) +#define SCRIPT_ADDR_R __stringify(SDRAM_OFFSET(3100000)) +#define PXEFILE_ADDR_R __stringify(SDRAM_OFFSET(3200000)) +#define RAMDISK_ADDR_R __stringify(SDRAM_OFFSET(3300000)) +#define MEM_LAYOUT_ENV_SETTINGS \ "bootm_size=0xa000000\0" \ - "kernel_addr_r=" __stringify(SDRAM_OFFSET(2000000)) "\0" \ - "fdt_addr_r=" __stringify(SDRAM_OFFSET(3000000)) "\0" \ - "scriptaddr=" __stringify(SDRAM_OFFSET(3100000)) "\0" \ - "pxefile_addr_r=" __stringify(SDRAM_OFFSET(3200000)) "\0" \ - "ramdisk_addr_r=" __stringify(SDRAM_OFFSET(3300000)) "\0" + "kernel_addr_r=" KERNEL_ADDR_R "\0" \ + "fdt_addr_r=" FDT_ADDR_R "\0" \ + "scriptaddr=" SCRIPT_ADDR_R "\0" \ + "pxefile_addr_r=" PXEFILE_ADDR_R "\0" \ + "ramdisk_addr_r=" RAMDISK_ADDR_R "\0" + +#define DFU_ALT_INFO_RAM \ + "dfu_alt_info_ram=" \ + "kernel ram " KERNEL_ADDR_R " 0x1000000;" \ + "fdt ram " FDT_ADDR_R " 0x100000;" \ + "ramdisk ram " RAMDISK_ADDR_R " 0x4000000\0"#ifdef CONFIG_MMC #define BOOT_TARGET_DEVICES_MMC (func) func(MMC, mmc, 0)
@@ -482,6 +501,7 @@ extern int soft_i2c_gpio_scl;#define CONFIG_EXTRA_ENV_SETTINGS \ CONSOLE_ENV_SETTINGS \ MEM_LAYOUT_ENV_SETTINGS \ + DFU_ALT_INFO_RAM \ "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \ "console=ttyS0,115200\0" \ BOOTCMD_SUNXI_COMPAT \
Signature · 1 line
2.4.10
PATCH 1/2 sunxi: Enable DFU for RAM
Hi,
Quoted 10 lines · Siarhei Siamashka
The DFU protocol implementation in U-Boot is much faster than the FEL protocol implementation in the boot ROM on Allwinner devices. Using DFU instead of FEL improves the USB transfer speed from 500-900 KB/s to 3.2-3.7 MB/s. This is particularly useful for reducing the time needed for booting systems with large initrd images.
FEL is still useful for loading the U-Boot bootloader and a boot script, which may then activate DFU in the following way:
Quoted 8 lines
setenv dfu_alt_info ${dfu_alt_info_ram} dfu 0 ram 0 bootm ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}The rest of the files can be transferred to the device using the "dfu-util" tool.
Signed-off-by Siarhei Siamashka <[email protected]>
Thanks, I've applied this series to my tree, and it will be part of the next pull-req.
Regards,
Hans
Quoted 69 lines
--- include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)include/configs/sunxi-common.h +25
-5@@ -337,6 +337,7 @@ extern int soft_i2c_gpio_scl;#define CONFIG_USB_GADGET_VBUS_DRAW 0#define CONFIG_USB_GADGET_DOWNLOAD +#define CONFIG_USB_FUNCTION_DFU #define CONFIG_USB_FUNCTION_FASTBOOT #define CONFIG_USB_FUNCTION_MASS_STORAGE #endif @@ -347,6 +348,11 @@ extern int soft_i2c_gpio_scl;#define CONFIG_G_DNL_MANUFACTURER "Allwinner Technology"#endif +#ifdef CONFIG_USB_FUNCTION_DFU +#define CONFIG_CMD_DFU +#define CONFIG_DFU_RAM +#endif +#ifdef CONFIG_USB_FUNCTION_FASTBOOT #define CONFIG_CMD_FASTBOOT #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR@@ -394,13 +400,26 @@ extern int soft_i2c_gpio_scl;* 32M uncompressed kernel, 16M compressed kernel, 1M fdt, * 1M script, 1M pxe and the ramdisk at the end. */ + +#define KERNEL_ADDR_R __stringify(SDRAM_OFFSET(2000000)) +#define FDT_ADDR_R __stringify(SDRAM_OFFSET(3000000)) +#define SCRIPT_ADDR_R __stringify(SDRAM_OFFSET(3100000)) +#define PXEFILE_ADDR_R __stringify(SDRAM_OFFSET(3200000)) +#define RAMDISK_ADDR_R __stringify(SDRAM_OFFSET(3300000)) +#define MEM_LAYOUT_ENV_SETTINGS \ "bootm_size=0xa000000\0" \ - "kernel_addr_r=" __stringify(SDRAM_OFFSET(2000000)) "\0" \ - "fdt_addr_r=" __stringify(SDRAM_OFFSET(3000000)) "\0" \ - "scriptaddr=" __stringify(SDRAM_OFFSET(3100000)) "\0" \ - "pxefile_addr_r=" __stringify(SDRAM_OFFSET(3200000)) "\0" \ - "ramdisk_addr_r=" __stringify(SDRAM_OFFSET(3300000)) "\0" + "kernel_addr_r=" KERNEL_ADDR_R "\0" \ + "fdt_addr_r=" FDT_ADDR_R "\0" \ + "scriptaddr=" SCRIPT_ADDR_R "\0" \ + "pxefile_addr_r=" PXEFILE_ADDR_R "\0" \ + "ramdisk_addr_r=" RAMDISK_ADDR_R "\0" + +#define DFU_ALT_INFO_RAM \ + "dfu_alt_info_ram=" \ + "kernel ram " KERNEL_ADDR_R " 0x1000000;" \ + "fdt ram " FDT_ADDR_R " 0x100000;" \ + "ramdisk ram " RAMDISK_ADDR_R " 0x4000000\0"#ifdef CONFIG_MMC #define BOOT_TARGET_DEVICES_MMC (func) func(MMC, mmc, 0)@@ -482,6 +501,7 @@ extern int soft_i2c_gpio_scl;#define CONFIG_EXTRA_ENV_SETTINGS \ CONSOLE_ENV_SETTINGS \ MEM_LAYOUT_ENV_SETTINGS \ + DFU_ALT_INFO_RAM \ "fdtfile=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0" \ "console=ttyS0,115200\0" \ BOOTCMD_SUNXI_COMPAT \
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
In reply to Siarhei SiamashkaThis is necessary to distinguish between the "dfu-util --detach" and the "dfu-util --reset" requests.
The default weak implementation of dfu_usb_get_reset() unconditionally reboots the device, but we want to be able to continue the boot.scr execution after writing the kernel, fdt and ramdisk to RAM via DFU.
--- drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/usb/musb-new/sunxi.c +12 -0
@@ -166,6 +166,17 @@ static void USBC_ConfigFIFO_Base(void) }/****************************************************************************** +* Needed for the DFU polling magic +******************************************************************************/ + +static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET ); +} + +/****************************************************************************** * MUSB Glue code ******************************************************************************/
@@ -176,6 +187,7 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)/* read and flush interrupts */ musb->int_usb =musb_readb (musb->mregs,MUSB_INTRUSB ); + last_int_usb = musb->int_usb;if (musb->int_usb)musb_writeb (musb->mregs,MUSB_INTRUSB , musb->int_usb); musb->int_tx =musb_readw (musb->mregs,MUSB_INTRTX );
Signature · 1 line
2.4.10
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 19 lines · Siarhei Siamashka
This is necessary to distinguish between the "dfu-util --detach" and the "dfu-util --reset" requests.
The default weak implementation of
dfu_usb_get_reset()unconditionally reboots the device, but we want to be able to continue the boot.scr execution after writing the kernel, fdt and ramdisk to RAM via DFU.Signed-off-by Siarhei Siamashka <[email protected]>--- drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/usb/musb-new/sunxi.c +0
-0@@ -166,6 +166,17 @@ static void USBC_ConfigFIFO_Base(void) }
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 177
Quoted 10 lines
drivers/usb/musb-new/sunxi.c +6
-0/************************************************************************* ***** + * Needed for the DFU polling magic +************************************************************************** ****/ + +static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );
The !! is not needed.
[...]
Best regards,
Marek Vasut
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Hello Marek,
Quoted 20 lines · Marek Vasut
Quoted 19 lines · Siarhei Siamashka
This is necessary to distinguish between the "dfu-util --detach" and the "dfu-util --reset" requests.
The default weak implementation of
dfu_usb_get_reset()unconditionally reboots the device, but we want to be able to continue the boot.scr execution after writing the kernel, fdt and ramdisk to RAM via DFU.Signed-off-by Siarhei Siamashka <[email protected]>--- drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/usb/musb-new/sunxi.c +0
-0@@ -166,6 +166,17 @@ static void USBC_ConfigFIFO_Base(void) }
Quoted 12 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 10 lines
drivers/usb/musb-new/sunxi.c +6
-0/************************************************************************* ***** + * Needed for the DFU polling magic +************************************************************************** ****/ + +static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Except if you want to be sure that you return 0 or 1 rather than 0 or (1 << something).
Quoted 4 lines
[...]
Best regards,
Marek Vasut
Amicalement,
Signature · 1 line
Albert.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 10 lines · Albert ARIBAUD
Quoted 7 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 5 lines
drivers/usb/musb-new/sunxi.c +5
-0+static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Except if you want to be sure that you return 0 or 1 rather than 0 or (1 << something).
Doesn't the bool return type already cause that to happen? (from the PoV of the caller@least)
Ian.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Hello Ian,
Quoted 9 lines · Ian Campbell
Quoted 8 lines · Albert ARIBAUD
Quoted 7 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 5 lines
drivers/usb/musb-new/sunxi.c +5
-0+static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Quoted 5 lines
Quoted 2 lines
Except if you want to be sure that you return 0 or 1 rather than 0 or (1 << something).
Doesn't the bool return type already cause that to happen? (from the PoV of the caller at least)
When all is said and done, a C bool is a C int, and anyway C does not perform value conversion (except for size and possibly sign extension) on type casts.
So no, types, bool or otherwise, do not cause any implicit '!!' to happen.
What happens is, wherever C expects a boolean value ('if', 'while'...) it considers 0 to be false and anything else to be true. But that's independent of the value's alleged type.
Quoted 1 line
Ian.
Amicalement,
Signature · 1 line
Albert.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 1 line · Albert ARIBAUD
Hello Ian,
Hi!
Quoted 22 lines
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 9 lines · <[email protected]> wrote:
Quoted 8 lines · Albert ARIBAUD
Quoted 7 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 5 lines
drivers/usb/musb-new/sunxi.c +5
-0+static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Quoted 5 lines
Quoted 2 lines
Except if you want to be sure that you return 0 or 1 rather than 0 or (1 << something).
Doesn't the bool return type already cause that to happen? (from the PoV of the caller at least)
When all is said and done, a C bool is a C int, and anyway C does not perform value conversion (except for size and possibly sign extension) on type casts.
Quoted 6 lines
So no, types, bool or otherwise, do not cause any implicit '!!' to happen.
What happens is, wherever C expects a boolean value ('if', 'while'...) it considers 0 to be false and anything else to be true. But that's independent of the value's alleged type.
Which is the case here -- one is not supposed to test boolean type for any particular value.
Best grouik,
Marek Vasut
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 30 lines · On Sun, 25 Oct 2015 14:29:59 +0100 Marek Vasut <[email protected]> wrote:
Quoted 1 line · Albert ARIBAUD
Hello Ian,
Hi!
Quoted 25 lines
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 9 lines · <[email protected]> wrote:
Quoted 8 lines · Albert ARIBAUD
Quoted 7 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 5 lines
drivers/usb/musb-new/sunxi.c +5
-0+static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Quoted 5 lines
Quoted 2 lines
Except if you want to be sure that you return 0 or 1 rather than 0 or (1 << something).
Doesn't the bool return type already cause that to happen? (from the PoV of the caller at least)
When all is said and done, a C bool is a C int, and anyway C does not perform value conversion (except for size and possibly sign extension) on type casts.
So no, types, bool or otherwise, do not cause any implicit '!!' to happen.
Quoted 6 lines
Quoted 3 lines
What happens is, wherever C expects a boolean value ('if', 'while'...) it considers 0 to be false and anything else to be true. But that's independent of the value's alleged type.
Which is the case here -- one is not supposed to test boolean type for any particular value.
Sure, this works fine as long as everyone has exactly the same idea about how this is supposed to work. Please consider the following code:
if (one_boolean_variable != another_boolean_variable) {
/* Sanity check failed, features X and Y must be either
both enabled or both disabled at the same time */
}The author of this hypothetical code may claim that a boolean variable must be always 0 or 1. And both of you will have a long and entertaining discussion as a result.
One more example:
#include <stdbool.h>
#include <stdio.h>
bool foo(void)
{
return 123;
}
int main(void)
{
printf("%d\n", (int)foo());
return 0;
}Guess what is printed after compiling and executing this code? Then replace "#include <stdbool.h>" with "typedef int bool;" and try it again. With the GCC compiler, the former prints "1" and the latter prints "123".
This stuff is a potential source of non-obvious bugs. Using "!!" is always safe, but may be in many cases redundant.
Signature · 2 lines
Best regards, Siarhei Siamashka
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 43 lines · Siarhei Siamashka
On Sun, 25 Oct 2015 14:29:59 +0100
Quoted 30 lines · Marek Vasut
Quoted 1 line · Albert ARIBAUD
Hello Ian,
Hi!
Quoted 25 lines
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 9 lines · <[email protected]> wrote:
Quoted 8 lines · Albert ARIBAUD
Quoted 7 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+static u8 last_int_usb; + +bool dfu_usb_get_reset(void) +{ + return !!(last_int_usb & MUSB_INTR_RESET);The !! is not needed.
Quoted 5 lines
Quoted 2 lines
Except if you want to be sure that you return 0 or 1 rather than 0 or (1 << something).
Doesn't the bool return type already cause that to happen? (from the PoV of the caller at least)
When all is said and done, a C bool is a C int, and anyway C does not perform value conversion (except for size and possibly sign extension) on type casts.
So no, types, bool or otherwise, do not cause any implicit '!!' to happen.
Quoted 6 lines
Quoted 3 lines
What happens is, wherever C expects a boolean value ('if', 'while'...) it considers 0 to be false and anything else to be true. But that's independent of the value's alleged type.
Which is the case here -- one is not supposed to test boolean type for any particular value.
Sure, this works fine as long as everyone has exactly the same idea about how this is supposed to work. Please consider the following code:
Quoted 7 lines
if (one_boolean_variable != another_boolean_variable) { /* Sanity check failed, features X and Y must be either both enabled or both disabled at the same time */ }The author of this hypothetical code may claim that a boolean variable must be always 0 or 1.
This assumption is wrong.
Quoted 11 lines
And both of you will have a long
and entertaining discussion as a result.One more example:
#include <stdbool.h> #include <stdio.h> bool foo(void) { return 123;
This is bloody confusing.
Quoted 5 lines
}
int main(void)
{
printf("%d\n", (int)foo());This is wrong -- the cast is outright incorrect.
Quoted 7 lines
return 0; }Guess what is printed after compiling and executing this code? Then replace "#include <
stdbool.h>" with "typedef int bool;" and try it again. With the GCC compiler, the former prints "1" and the latter prints "123".
The code is broken, so the result is undefined.
Quoted 2 lines
This stuff is a potential source of non-obvious bugs. Using "!!" is always safe, but may be in many cases redundant.
I'd expect that using !! will generate additional code and that's done for no reason at all.
Best regards,
Marek Vasut
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
In reply to Siarhei SiamashkaQuoted 2 lines · Siarhei Siamashka
replace "#include <
stdbool.h>" with "typedef int bool;" and try it again.
So don't do that then.
In u-boot the "bool" type comes from stdbool.h and it is therefore completely reasonable to assume that bool in u-boot has the properties which this implies.
Ian.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 5 lines · On Sun, 25 Oct 2015 19:24:04 +0000 Ian Campbell <[email protected]>…
Quoted 2 lines · Siarhei Siamashka
replace "#include <
stdbool.h>" with "typedef int bool;" and try it again.So don't do that then.
Yes, I'm not going to do this. It was just an artificial example, demonstrating how things work. I'm only a little bit worried about a potential rogue typedef unexpectedly coming from one of the include files one day.
Quoted 3 lines
In u-boot the "bool" type comes from
stdbool.hand it is therefore completely reasonable to assume that bool in u-boot has the properties which this implies.
Right, except that there are still a few bool related typedefs in the code:
https://github.com/u-boot/u-boot/blob/v2015.10/lib/lzma/Types.h#L83 https://github.com/u-boot/u-boot/blob/v2015.10/lib/bzip2/bzlib_private.h#L85
Yes, they come from third-party libraries and don't really redefine bool or _Bool. But the C language did not always have a native bool data type. And historically it had been a common practice to typedef bool to some integer type. One should not be very much surprised upon encountering such code in the wild even now.
Regarding the code efficiency. The following example:
#include <stdbool.h>
bool foo(int x)
{
return x & 4;
}
bool bar(int x)
{
return !!(x & 4);
}Compiles into the following code on ARM:
00000000 <foo>:
0: e7e00150 ubfx r0, r0, #2, #1
4: e12fff1e bx lr
00000008 <bar>:
8: e7e00150 ubfx r0, r0, #2, #1
c: e12fff1e bx lrBasically, there is exactly no difference. And removing "!!" only has a cosmetic value for the code that uses the bool definition from <stdbool.h>.
As I already said, I'm perfectly fine with either of these variants :-) The purists are welcome to start a crusade eliminating all of the uses of the "!!" construct in the U-Boot code. Probably starting with the other dfu_usb_get_reset() implementations for the sake of consistency:
https://github.com/u-boot/u-boot/blob/v2015.10/drivers/usb/gadget/s3c_udc_otg.c#L151 https://github.com/u-boot/u-boot/blob/v2015.10/drivers/usb/gadget/ci_udc.c#L1062
Signature · 2 lines
Best regards, Siarhei Siamashka
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
In reply to Albert ARIBAUDQuoted 5 lines · Albert ARIBAUD
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 2 lines
Doesn't the bool return type already cause that to happen? (from the PoV of the caller at least)
When all is said and done, a C bool is a C int,
Not if it is a _Bool (via stdbool.h or some other way).
A _Bool is always either 0 or 1, and scalar value which is converted to a _Bool is converted to either 0 or 1.
Quoted 2 lines
So no, types, bool or otherwise, do not cause any implicit '!!' to happen.
I believe this is not correct when _Bool is used.
In u-boot a bool is indeed a _Bool (or at least I don't see any other typedef's and I can see various includes on stdbool.h, I therefore didn't feel the need to check how bool is arrived at in this particular file).
Ian.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Hello Ian,
Quoted 15 lines · Ian Campbell
Quoted 5 lines · Albert ARIBAUD
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 2 lines
Doesn't the bool return type already cause that to happen? (from the PoV of the caller at least)
When all is said and done, a C bool is a C int,
Not if it is a
_Bool(viastdbool.hor some other way).A
_Boolis always either 0 or 1, and scalar value which is converted to a_Boolis converted to either 0 or 1.Quoted 2 lines
So no, types, bool or otherwise, do not cause any implicit '!!' to happen.
Quoted 6 lines
I believe this is not correct when
_Boolis used.In u-boot a bool is indeed a
_Bool(or at least I don't see any other typedef's and I can see various includes onstdbool.h, I therefore didn't feel the need to check how bool is arrived at in this particular file).
What you write is possibly correct for C++, but certainly not for C, for which booleans are integers, with no compiler-enforced constraint on their value domains.
Amicalement,
Signature · 1 line
Albert.1
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 26 lines · Albert ARIBAUD
Hello Ian,
Quoted 3 lines · Ian Campbell
Quoted 2 lines · Albert ARIBAUD
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 1 line
Doesn't the bool return type already cause that to happen?
(from the
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
PoV of the caller at least)
When all is said and done, a C bool is a C int,
Not if it is a
_Bool(viastdbool.hor some other way).A
_Boolis always either 0 or 1, and scalar value which isconverted to
Quoted 3 lines
a
_Boolis converted to either 0 or 1.Quoted 1 line
So no, types, bool or otherwise, do not cause any implicit '!!'
to
Quoted 5 lines
Quoted 1 line
happen.
I believe this is not correct when
_Boolis used.In u-boot a bool is indeed a
_Bool(or at least I don't see anyother
Quoted 8 lines
Quoted 2 lines
typedef's and I can see various includes on
stdbool.h, I therefore didn't feel the need to check how bool is arrived at in thisparticular
Quoted 1 line
file).
What you write is possibly correct for C++, but certainly not for C, for which booleans are integers, with no compiler-enforced constraint on their value domains.
I know next to nothing about C++ so I am certainly not confusing things with that.
The _Bool type in C99 is an integer which may take on exactly the values 0 or 1. Since the code which started this subthread was using "bool" from <stdbool.h> it is _Bool which is being discussed here.
The actual standard costs money (and is therefore unlinkable) but
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf is a late draft and I believe this aspect is unchanged. Section 6.3.1.2 is one relevant part explaining that a _Bool must always be either 0 or 1:
When any scalar value is converted to _Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1.
Many of the other clauses dealing with Integers (e.g. 6.3.1.3.1) have had "other than _Bool" or some similar wording added to them since _Bool does indeed behave a little differently.
So I'm afraid I disagree with your statement, at least for C >= C99 (I can't recall if _Bool was in C89, but I think the answer is no).
Ian.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Hello Ian,
Quoted 51 lines · Ian Campbell
Quoted 26 lines · Albert ARIBAUD
Hello Ian,
Quoted 3 lines · Ian Campbell
Quoted 2 lines · Albert ARIBAUD
On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
Quoted 1 line
Doesn't the bool return type already cause that to happen?
(from the
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
PoV of the caller at least)
When all is said and done, a C bool is a C int,
Not if it is a
_Bool(viastdbool.hor some other way).A
_Boolis always either 0 or 1, and scalar value which isconverted to
Quoted 3 lines
a
_Boolis converted to either 0 or 1.Quoted 1 line
So no, types, bool or otherwise, do not cause any implicit '!!'
to
Quoted 5 lines
Quoted 1 line
happen.
I believe this is not correct when
_Boolis used.In u-boot a bool is indeed a
_Bool(or at least I don't see anyother
Quoted 8 lines
Quoted 2 lines
typedef's and I can see various includes on
stdbool.h, I therefore didn't feel the need to check how bool is arrived at in thisparticular
Quoted 1 line
file).
What you write is possibly correct for C++, but certainly not for C, for which booleans are integers, with no compiler-enforced constraint on their value domains.
I know next to nothing about C++ so I am certainly not confusing things with that.
The
_Booltype in C99 is an integer which may take on exactly the values 0 or 1. Since the code which started this subthread was using "bool" from <stdbool.h> it is_Boolwhich is being discussed here.The actual standard costs money (and is therefore unlinkable) but
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf is a late draft and I believe this aspect is unchanged. Section 6.3.1.2 is one relevant part explaining that a_Boolmust always be either 0 or 1:When any scalar value is converted to
_Bool, the result is 0 if the value compares equal to 0; otherwise, the result is 1.
Quoted 6 lines
Many of the other clauses dealing with Integers (e.g. 6.3.1.3.1) have had "other than
_Bool" or some similar wording added to them since_Booldoes indeed behave a little differently.So I'm afraid I disagree with your statement, at least for C >= C99 (I can't recall if
_Boolwas in C89, but I think the answer is no).
I stand corrected: I've just checked it, and the conversion does indeed happen on return -- at least gcc 5.2.1 -- even when, like in U-Boot build files, c99 standard compliance is not specified.
If older GCCs (up to a point: how old a gcc are we wanting to support?) also work this way too, the '!!' is indeed unneeded.
Quoted 1 line
Ian.
Amicalement,
Signature · 1 line
Albert.
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
In reply to Marek VasutQuoted 20 lines · On Sun, 25 Oct 2015 12:00:09 +0100 Marek Vasut <[email protected]> wrote:
Quoted 19 lines · Siarhei Siamashka
This is necessary to distinguish between the "dfu-util --detach" and the "dfu-util --reset" requests.
The default weak implementation of
dfu_usb_get_reset()unconditionally reboots the device, but we want to be able to continue the boot.scr execution after writing the kernel, fdt and ramdisk to RAM via DFU.Signed-off-by Siarhei Siamashka <[email protected]>--- drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/usb/musb-new/sunxi.c +0
-0@@ -166,6 +166,17 @@ static void USBC_ConfigFIFO_Base(void) }
Quoted 12 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 10 lines
drivers/usb/musb-new/sunxi.c +6
-0/************************************************************************* ***** + * Needed for the DFU polling magic +************************************************************************** ****/ + +static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Thanks for your comment, I can surely change this in the updated version of the patch.
But I'm more interested to know if the USB people are happy with the current wacky way of interaction between the DFU code and the USB drivers. Doing dfu_usb_get_reset() polling up to 1000 times in a loop does not look exactly beautiful:
https://github.com/u-boot/u-boot/blob/v2015.10/common/cmd_dfu.c#L64
It might be also a bit racy (the RESET interrupt could be potentially missed sometimes).
Signature · 2 lines
Best regards, Siarhei Siamashka
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
Quoted 48 lines · Siarhei Siamashka
On Sun, 25 Oct 2015 12:00:09 +0100
Quoted 25 lines · Marek Vasut
Quoted 24 lines · Siarhei Siamashka
This is necessary to distinguish between the "dfu-util --detach" and the "dfu-util --reset" requests.
The default weak implementation of
dfu_usb_get_reset()unconditionally reboots the device, but we want to be able to continue the boot.scr execution after writing the kernel, fdt and ramdisk to RAM via DFU.Signed-off-by Siarhei Siamashka <[email protected]>---
drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)(quoted diff) +0
-0
b/drivers/usb/musb-new/sunxi.cindex a146c08..5eb8d19 100644drivers/usb/musb-new/sunxi.c +0
-0@@ -166,6 +166,17 @@ static void USBC_ConfigFIFO_Base(void) }/********************************************************************* **** Quoted 11 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 9 lines
***** + * Needed for the DFU polling magic
+
***********************************************************************
*** ****/ +drivers/usb/musb-new/sunxi.c +5
-0+static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Thanks for your comment, I can surely change this in the updated version of the patch.
But I'm more interested to know if the USB people are happy with the current wacky way of interaction between the DFU code and the USB drivers. Doing
dfu_usb_get_reset()polling up to 1000 times in a loop does not look exactly beautiful:
Quoted 4 lines
https://github.com/u-boot/u-boot/blob/v2015.10/common/cmd_dfu.c#L64It might be also a bit racy (the RESET interrupt could be potentially missed sometimes).
That's a question for Lukasz .
Best regards,
Marek Vasut
PATCH 2/2 musb: sunxi: Implement dfu_usb_get_reset()
In reply to Siarhei SiamashkaHello Siarhei,
Quoted 36 lines · Siarhei Siamashka
Quoted 20 lines · On Sun, 25 Oct 2015 12:00:09 +0100 Marek Vasut <[email protected]> wrote:
Quoted 19 lines · Siarhei Siamashka
This is necessary to distinguish between the "dfu-util --detach" and the "dfu-util --reset" requests.
The default weak implementation of
dfu_usb_get_reset()unconditionally reboots the device, but we want to be able to continue the boot.scr execution after writing the kernel, fdt and ramdisk to RAM via DFU.Signed-off-by Siarhei Siamashka <[email protected]>--- drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/usb/musb-new/sunxi.c +0
-0@@ -166,6 +166,17 @@ static void USBC_ConfigFIFO_Base(void) }Quoted 12 lines
drivers/usb/musb-new/sunxi.c static void USBC_ConfigFIFO_Base(void) · line 176
Quoted 10 lines
drivers/usb/musb-new/sunxi.c +6
-0/************************************************************************* ***** + * Needed for the DFU polling magic +************************************************************************** ****/ + +static u8 last_int_usb; + +bool dfu_usb_get_reset (void ) +{ +return !!(last_int_usb &MUSB_INTR_RESET );The !! is not needed.
Quoted 2 lines
Thanks for your comment, I can surely change this in the updated version of the patch.
For the sake of consistency, I'd rather you did not remove the '!!'. If you decide to remove it, then you should change the function's type to int and mention that it may return zero or non-zero. In C, boolean operators '!', '||' and '&&' always evaluate to 1 for true.
Amicalement,
Signature · 1 line
Albert.
cover Enable DFU for RAM on Allwinner devices
In reply to Siarhei SiamashkaQuoted 5 lines · Siarhei Siamashka
Hello,
DFU allows to transfer large files (such as initrd images) much faster than FEL.
Quoted 7 lines
Siarhei Siamashka (2):
sunxi: Enable DFU for RAM
musb: sunxi: Implementdfu_usb_get_reset()drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-)
Siarhei,
can you give some pointers how to test those patches. I have A20-OLinuXino-Micro and Cubietruck and would be glad to give them a try.
I think it would be interested to combine this method with Boris NAND support and get much better solution then {Live,Phoenix}Suit.
Best Regards,
Signature · 3 lines
Piotr Kr?l Embedded Systems Consultant http://3mdeb.com | @3mdeb_com
cover Enable DFU for RAM on Allwinner devices
Quoted 10 lines · On Mon, 26 Oct 2015 12:18:35 +0100 Piotr Kr?l <[email protected]> wrote:
Quoted 9 lines · Siarhei Siamashka
Hello,
DFU allows to transfer large files (such as initrd images) much faster than FEL.
Siarhei Siamashka (2):
sunxi: Enable DFU for RAM
musb: sunxi: Implementdfu_usb_get_reset()
Quoted 7 lines
Quoted 3 lines
drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-)Siarhei,
can you give some pointers how to test those patches. I have A20-OLinuXino-Micro and Cubietruck and would be glad to give them a try.
Hello,
I tried to provide some basic usage instructions as a part of the commit message:
https://patchwork.ozlabs.org/patch/535535/
But you also need the "sunxi: cubietruck: Enable the USB OTG controller" patch from Maxime Ripard to enable USB OTG on the Cubietruck:
https://patchwork.ozlabs.org/patch/530656/
Quoted 2 lines
I think it would be interested to combine this method with Boris NAND support and get much better solution then {Live,Phoenix}Suit.
At this stage I'm only interested in the DFU usage as a speed booster for FEL. Booting over USB via FEL allows us to temporarily run more or less complete system (kernel and rootfs on ramdisk) on any Allwinner device without modifying existing pre-installed software on non-volatile storage. This already works fine, but we were not quite happy about the data transfer speed.
If you are interested in flashing NAND, then you can probably have a look at the recent fastboot patches from Maxime.
DFU means "Device Firmware Upgrade" and it can be also used for flashing NAND or writing images to SD cards over USB (if we hook up this part of the DFU functionality). The main question is how many alternative NAND flashing methods do we need?
Signature · 2 lines
Best regards, Siarhei Siamashka
cover Enable DFU for RAM on Allwinner devices
Quoted 21 lines · Siarhei Siamashka
Quoted 10 lines · On Mon, 26 Oct 2015 12:18:35 +0100 Piotr Kr?l <[email protected]> wrote:
Quoted 9 lines · Siarhei Siamashka
Hello,
DFU allows to transfer large files (such as initrd images) much faster than FEL.
Siarhei Siamashka (2):
sunxi: Enable DFU for RAM
musb: sunxi: Implementdfu_usb_get_reset()Quoted 7 lines
Quoted 3 lines
drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-)Siarhei,
can you give some pointers how to test those patches. I have A20-OLinuXino-Micro and Cubietruck and would be glad to give them a try.
Quoted 6 lines
Hello,
I tried to provide some basic usage instructions as a part of the commit message:
https://patchwork.ozlabs.org/patch/535535/
Hi Siarhei,
unfortunately I'm not able to compile even clean master with enabled CONFIG_USB_MUSB_GADGET. I'm using Linaro toolchain and get this error:
arm-linux-gnueabihf-ld.bfd: error:
/home/pietrushnic/storage/wdc/projects/3mdeb/cubietruck/toolchains/gcc-linaro-4.9-2015.05-x86_64_arm-linux-gnueabihf/bin/../lib/gcc/arm-linux-gnueabihf/4.9.3/libgcc.a(_udivmoddi4.o) uses VFP register arguments, u-boot does not arm-linux-gnueabihf-ld.bfd: failed to merge target specific data of file /home/pietrushnic/storage/wdc/projects/3mdeb/cubietruck/toolchains/gcc-linaro-4.9-2015.05-x86_64_arm-linux-gnueabihf/bin/../lib/gcc/arm-linux-gnueabihf/4.9.3/libgcc.a(_udivmoddi4.o) Makefile:1183: recipe for target 'u-boot' failed
make: *** [u-boot] Error 1
CONFIG_USB_MUSB_GADGET automatically enable CONFIG_USB_MUSB_SUNXI. When I try with CONFIG_USB_MUSB_SUNXI not set I'm getting other failure:
(...)drivers/usb/musb-new/musb_gadget.c:133:8: error: 'DMA_TO_DEVICE' undeclared (first use in this function) (...)drivers/usb/musb-new/musb_gadget.c:134:8: error: 'DMA_FROM_DEVICE' undeclared (first use in this function)
: DMA_FROM_DEVICE);
^
(...)
drivers/usb/musb-new/musb_gadget.c:164:7: error: 'DMA_TO_DEVICE' undeclared (first use in this function)
? DMA_TO_DEVICE
^
drivers/usb/musb-new/musb_gadget.c:165:7: error: 'DMA_FROM_DEVICE' undeclared (first use in this function)
: DMA_FROM_DEVICE);My steps:
make CROSS_COMPILE=arm-linux-gnueabihf- Cubietruck_defconfig make CROSS_COMPILE=arm-linux-gnueabihf- menuconfig
# enable CONFIG_USB_MUSB_GADGET make CROSS_COMPILE=arm-linux-gnueabihf- -j$(nproc)
Any idea how to narrow VFP problem ?
Quoted 5 lines
But you also need the "sunxi: cubietruck: Enable the USB OTG controller" patch from Maxime Ripard to enable USB OTG on the Cubietruck:
https://patchwork.ozlabs.org/patch/530656/
Cannot apply this series to master cleanly. Should I try different tree ?
Quoted 10 lines
Quoted 2 lines
I think it would be interested to combine this method with Boris NAND support and get much better solution then {Live,Phoenix}Suit.
At this stage I'm only interested in the DFU usage as a speed booster for FEL. Booting over USB via FEL allows us to temporarily run more or less complete system (kernel and rootfs on ramdisk) on any Allwinner device without modifying existing pre-installed software on non-volatile storage. This already works fine, but we were not quite happy about the data transfer speed.
Quoted 7 lines
If you are interested in flashing NAND, then you can probably have a look at the recent fastboot patches from Maxime.
DFU means "Device Firmware Upgrade" and it can be also used for flashing NAND or writing images to SD cards over USB (if we hook up this part of the DFU functionality). The main question is how many alternative NAND flashing methods do we need?
IMHO one method that works would be great.
Best Regards,
Signature · 3 lines
Piotr Kr?l Embedded Systems Consultant http://3mdeb.com | @3mdeb_com
cover Enable DFU for RAM on Allwinner devices
Quoted 39 lines · On Wed, 28 Oct 2015 00:27:48 +0100 Piotr Kr?l <[email protected]> wrote:
Quoted 21 lines · Siarhei Siamashka
Quoted 10 lines · On Mon, 26 Oct 2015 12:18:35 +0100 Piotr Kr?l <[email protected]> wrote:
Quoted 9 lines · Siarhei Siamashka
Hello,
DFU allows to transfer large files (such as initrd images) much faster than FEL.
Siarhei Siamashka (2):
sunxi: Enable DFU for RAM
musb: sunxi: Implementdfu_usb_get_reset()Quoted 7 lines
Quoted 3 lines
drivers/usb/musb-new/sunxi.c | 12 ++++++++++++ include/configs/sunxi-common.h | 30 +++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-)Siarhei,
can you give some pointers how to test those patches. I have A20-OLinuXino-Micro and Cubietruck and would be glad to give them a try.Quoted 6 lines
Hello,
I tried to provide some basic usage instructions as a part of the commit message:
https://patchwork.ozlabs.org/patch/535535/Hi Siarhei,
unfortunately I'm not able to compile even clean master with enabledCONFIG_USB_MUSB_GADGET. I'm using Linaro toolchain and get this error:arm-linux-gnueabihf-ld.bfd: error:
/home/pietrushnic/storage/wdc/projects/3mdeb/cubietruck/toolchains/gcc-linaro-4.9-2015.05-x86_64_arm-linux-gnueabihf/bin/../lib/gcc/arm-linux-gnueabihf/4.9.3/libgcc.a(_udivmoddi4.o) uses VFP register arguments, u-boot does not arm-linux-gnueabihf-ld.bfd: failed to merge target specific data of file /home/pietrushnic/storage/wdc/projects/3mdeb/cubietruck/toolchains/gcc-linaro-4.9-2015.05-x86_64_arm-linux-gnueabihf/bin/../lib/gcc/arm-linux-gnueabihf/4.9.3/libgcc.a(_udivmoddi4.o) Makefile:1183: recipe for target 'u-boot' failed
make: *** [u-boot] Error 1
[...]
Quoted 7 lines
My steps:
make CROSS_COMPILE=arm-linux-gnueabihf-Cubietruck_defconfigmake CROSS_COMPILE=arm-linux-gnueabihf- menuconfig# enable CONFIG_USB_MUSB_GADGET make CROSS_COMPILE=arm-linux-gnueabihf- -j$(nproc)Any idea how to narrow VFP problem ?
Thanks for reporting this. I have also confirmed the problem with a hardfloat toolchain and just sent a patch:
https://patchwork.ozlabs.org/patch/537185/
Debugging this involved just checking all the symbols in the object files generated by a softfloat toolchain (it does not fail), then filtering out all the symbols which also exist in a successful hardfloat build (with fastboot config options disabled) and comparing the remaining symbols with libgcc.a
The culprit was "__aeabi_uldivmod".
Quoted 7 lines
Quoted 5 lines
But you also need the "sunxi: cubietruck: Enable the USB OTG controller" patch from Maxime Ripard to enable USB OTG on the Cubietruck:
https://patchwork.ozlabs.org/patch/530656/Cannot apply this series to master cleanly. Should I try different tree ?
This whole series is not needed if you are only interested in DFU.
We just need to ensure that USB OTG is eventually enabled on all sunxi boards (and also properly handles the ID pin to switch between the device and host roles).
Signature · 2 lines
Best regards, Siarhei Siamashka