dfu: Provide means to find difference between dfu-util -e and -R

PATCH Superseded Lukasz Majewski 3 participants 10 messages Aug 22, 2014 – Sep 1, 2014 USB

Share & tools

Lukasz Majewski

author

2014-08-22 14:08 UTC #0 lore raw

PATCH 1/2 dfu: Provide means to find difference between dfu-util -e and -R

This commit provides distinction between DFU device detach and reset. The -R behavior is preserved with proper handling of the dfu-util's -e switch, which detach the DFU device.

By running dfu-util -e; one can force device to finish the execution of dfu command on target and execute some other scripted commands.

Moreover, some naming has been changed - the dfu_reset() method now is known as dfu_detach(). New name better reflects the purpose of the code.

It was also necessary to increase the number of usb_gadget_handle_interrupts() calls since we also must wait for detection of the USB reset event.

Example usage:
1. -e (detach) switch
dfu-util -a0 -D file1.bin;dfu-util -a3 -D uImage;dfu-util -e

access to u-boot prompt.

2. -R (reset) switch
dfu-util -a0 -D file1.bin;dfu-util -R -a3 -D uImage

target board reset

Signed-off-by Lukasz Majewski <[email protected]>

---
 common/cmd_dfu.c           | 23 +++++++++++++++++++----
 drivers/dfu/dfu.c          | 31 ++++++++++++++++++++++++++-----
 drivers/usb/gadget/f_dfu.c |  2 +-
 include/dfu.h              |  5 ++++-
 4 files changed, 50 insertions(+), 11 deletions(-)

common/cmd_dfu.c +19 -4

index 2633b30..ffd6c17 100644
@@ -13,6 +13,8 @@
 #include <g_dnl.h>
 #include <usb.h>
 
+static bool dfu_reset;
+
 static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 4)

@@ -36,17 +38,28 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	int controller_index = simple_strtoul(usb_controller, NULL, 0);
 	board_usb_init(controller_index, USB_INIT_DEVICE);
-
+	dfu_clear_detach();
 	g_dnl_register("usb_dnl_dfu");
 	while (1) {
-		if (dfu_reset())
+		if (dfu_detach()) {
+			/*
+			 * Check if USB bus reset is performed after detach,
+			 * which indicates that -R switch has been passed to
+			 * dfu-util. In this case reboot the device
+			 */
+			if (dfu_usb_get_reset()) {
+				dfu_reset = true;
+				goto exit;
+			}
+
 			/*
 			 * This extra number of usb_gadget_handle_interrupts()
 			 * calls is necessary to assure correct transmission
 			 * completion with dfu-util
 			 */
-			if (++i == 10)
+			if (++i == 10000)
 				goto exit;
+		}
 
 		if (ctrlc())
 			goto exit;

@@ -58,9 +71,11 @@ exit:
 done:
 	dfu_free_entities();
 
-	if (dfu_reset())
+	if (dfu_reset)
 		run_command("reset", 0);
 
+	dfu_clear_detach();
+
 	return ret;
 }

drivers/dfu/dfu.c +26 -5

index 3512b14..55e6a83 100644
@@ -17,20 +17,41 @@
 #include <linux/list.h>
 #include <linux/compiler.h>
 
-static bool dfu_reset_request;
+static bool dfu_detach_request;
 static LIST_HEAD(dfu_list);
 static int dfu_alt_num;
 static int alt_num_cnt;
 static struct hash_algo *dfu_hash_algo;
 
-bool dfu_reset(void)
+/*
+ * The purpose of the dfu_usb_get_reset() function is to
+ * provide information if after USB_DETACH request
+ * being sent the dfu-util performed reset of USB
+ * bus.
+ *
+ * Described behaviour is the only way to distinct if
+ * user has typed -e (detach) or -R (reset) when invoking
+ * dfu-util command.
+ *
+ */
+__weak bool dfu_usb_get_reset(void)
+{
+	return true;
+}
+
+bool dfu_detach(void)
+{
+	return dfu_detach_request;
+}
+
+void dfu_trigger_detach(void)
 {
-	return dfu_reset_request;
+	dfu_detach_request = true;
 }
 
-void dfu_trigger_reset()
+void dfu_clear_detach(void)
 {
-	dfu_reset_request = true;
+	dfu_detach_request = false;
 }
 
 static int dfu_find_alt_num(const char *s)

drivers/usb/gadget/f_dfu.c +1 -1

index 9863dec..3e4f029 100644
@@ -372,7 +372,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
 		to_runtime_mode(f_dfu);
 		f_dfu->dfu_state = DFU_STATE_appIDLE;
 
-		dfu_trigger_reset();
+		dfu_trigger_detach();
 		break;
 	default:
 		f_dfu->dfu_state = DFU_STATE_dfuERROR;

include/dfu.h +4 -1

index 7e0a999..f1a71c7 100644
@@ -150,11 +150,14 @@ struct dfu_entity *dfu_get_entity(int alt);
 char *dfu_extract_token(char** e, int *n);
 void dfu_trigger_reset(void);
 int dfu_get_alt(char *name);
-bool dfu_reset(void);
+bool dfu_detach(void);
+void dfu_trigger_detach(void);
+void dfu_clear_detach(void);
 int dfu_init_env_entities(char *interface, char *devstr);
 unsigned char *dfu_get_buf(struct dfu_entity *dfu);
 unsigned char *dfu_free_buf(void);
 unsigned long dfu_get_buf_size(void);
+bool dfu_usb_get_reset(void);
 
 int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
 int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num);

Signature · 1 line

2.0.0.rc2

Lukasz Majewski

author

2014-08-22 14:08 UTC #1 lore raw

PATCH 2/2 udc: dfu: s3c_udc: Provide function to check if USB reset was asserted

New dfu_usb_get_reset() method is necessary to distinct two different use cases of dfu-util program.
This method checks if the USB bus reset has been really performed after DFU DETACH.

Without this function the previous DFU behavior is preserved.

Signed-off-by Lukasz Majewski <[email protected]>

---
 drivers/usb/gadget/s3c_udc_otg.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/usb/gadget/s3c_udc_otg.c +5 -0

index 63d4487..ce2494c 100644
@@ -149,6 +149,11 @@ struct s3c_usbotg_reg *reg;
 struct s3c_usbotg_phy *phy;
 static unsigned int usb_phy_ctrl;
 
+bool dfu_usb_get_reset(void)
+{
+	return !!(readl(&reg->gintsts) & INT_RESET);
+}
+
 void otg_phy_init(struct s3c_udc *dev)
 {
 	dev->pdata->phy_control(1);

Signature · 1 line

2.0.0.rc2

Stephen Warren

2014-08-22 18:06 UTC #2 lore raw

PATCH 2/2 udc: dfu: s3c_udc: Provide function to check if USB reset was asserted

Quoted 6 lines · Lukasz Majewski

New dfu_usb_get_reset() method is necessary to distinct two different use cases of dfu-util program.
This method checks if the USB bus reset has been really performed after DFU DETACH.
Without this function the previous DFU behavior is preserved.

Do you have HW that uses ci_udc, so you could implement/test this there too? If not, let me know and I'll see if I can.

Lukasz Majewski

2014-08-22 19:57 UTC #3 lore raw

PATCH 2/2 udc: dfu: s3c_udc: Provide function to check if USB reset was asserted

Quoted 11 lines · On Fri, 22 Aug 2014 12:06:49 -0600 Stephen Warren <[email protected]> wrote:

Quoted 6 lines · Lukasz Majewski
New dfu_usb_get_reset() method is necessary to distinct two different use cases of dfu-util program.
This method checks if the USB bus reset has been really performed after DFU DETACH.
Without this function the previous DFU behavior is preserved.
Do you have HW that uses ci_udc, so you could implement/test this there too? If not, let me know and I'll see if I can.

Unfortunately I'm using s3c_hsotg UDC. The function which you need only must return true if your's UDC has detected USB bus reset (enforced by dfu-util with -R switch).

Best regards,
Lukasz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140822/31ecf0d1/attachment.pgp>

Stephen Warren

2014-08-22 18:06 UTC #4 lore raw

PATCH 1/2 dfu: Provide means to find difference between dfu-util -e and -R

In reply to Lukasz Majewski

Quoted 6 lines · Lukasz Majewski

This commit provides distinction between DFU device detach and reset. The -R behavior is preserved with proper handling of the dfu-util's -e switch, which detach the DFU device.
By running dfu-util -e; one can force device to finish the execution of dfu command on target and execute some other scripted commands.

This sounds extremely useful; I wonder if UMS could have the same feature?

common/cmd_dfu.c line 16

Quoted 3 lines

common/cmd_dfu.c +1 -0
 
+static bool dfu_reset;

Can't this be a local variable inside the do_dfu() function?

Aside from that,

Reviewed-by Stephen Warren <[email protected]>

Lukasz Majewski

2014-08-22 19:59 UTC #5 lore raw

PATCH 1/2 dfu: Provide means to find difference between dfu-util -e and -R

Quoted 6 lines · On Fri, 22 Aug 2014 12:06:03 -0600 Stephen Warren <[email protected]> wrote:

Quoted 4 lines · Lukasz Majewski
This commit provides distinction between DFU device detach and reset. The -R behavior is preserved with proper handling of the dfu-util's -e switch, which detach the DFU device.

Quoted 6 lines

Quoted 3 lines
By running dfu-util -e; one can force device to finish the execution of dfu command on target and execute some other scripted commands.
This sounds extremely useful; I wonder if UMS could have the same feature?

It is worth considering. I will think about this

Quoted 6 lines

common/cmd_dfu.c line 16
Quoted 4 lines
common/cmd_dfu.c +1 -0
 
+static bool dfu_reset;
Can't this be a local variable inside the do_dfu() function?

I think I can put this variable there.

Quoted 3 lines

Aside from that,
Reviewed-by Stephen Warren <[email protected]>

Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140822/209561eb/attachment.pgp>

Lukasz Majewski

author

2014-08-25 09:07 UTC #6 lore raw

PATCH v2 1/2 dfu: Provide means to find difference between dfu-util -e and -R

In reply to Lukasz Majewski

This commit provides distinction between DFU device detach and reset. The -R behavior is preserved with proper handling of the dfu-util's -e switch, which detach the DFU device.

By running dfu-util -e; one can force device to finish the execution of dfu command on target and execute some other scripted commands.

Moreover, some naming has been changed - the dfu_reset() method now is known as dfu_detach(). New name better reflects the purpose of the code.

It was also necessary to increase the number of usb_gadget_handle_interrupts() calls since we also must wait for detection of the USB reset event.

Example usage:
1. -e (detach) switch
dfu-util -a0 -D file1.bin;dfu-util -a3 -D uImage;dfu-util -e

access to u-boot prompt.

2. -R (reset) switch
dfu-util -a0 -D file1.bin;dfu-util -R -a3 -D uImage

target board reset

Signed-off-by Lukasz Majewski <[email protected]>

Reviewed-by Stephen Warren <[email protected]>

---
Changes for v2:
- The 'dfu_reset' variable definition is now done inside the do_dfu function

---
 common/cmd_dfu.c           | 23 +++++++++++++++++++----
 drivers/dfu/dfu.c          | 31 ++++++++++++++++++++++++++-----
 drivers/usb/gadget/f_dfu.c |  2 +-
 include/dfu.h              |  5 ++++-
 4 files changed, 50 insertions(+), 11 deletions(-)

common/cmd_dfu.c +19 -4

index 2633b30..9e020b4 100644
@@ -15,6 +15,8 @@
 
 static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
+	bool dfu_reset = false;
+
 	if (argc < 4)
 		return CMD_RET_USAGE;

@@ -36,17 +38,28 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	int controller_index = simple_strtoul(usb_controller, NULL, 0);
 	board_usb_init(controller_index, USB_INIT_DEVICE);
-
+	dfu_clear_detach();
 	g_dnl_register("usb_dnl_dfu");
 	while (1) {
-		if (dfu_reset())
+		if (dfu_detach()) {
+			/*
+			 * Check if USB bus reset is performed after detach,
+			 * which indicates that -R switch has been passed to
+			 * dfu-util. In this case reboot the device
+			 */
+			if (dfu_usb_get_reset()) {
+				dfu_reset = true;
+				goto exit;
+			}
+
 			/*
 			 * This extra number of usb_gadget_handle_interrupts()
 			 * calls is necessary to assure correct transmission
 			 * completion with dfu-util
 			 */
-			if (++i == 10)
+			if (++i == 10000)
 				goto exit;
+		}
 
 		if (ctrlc())
 			goto exit;

@@ -58,9 +71,11 @@ exit:
 done:
 	dfu_free_entities();
 
-	if (dfu_reset())
+	if (dfu_reset)
 		run_command("reset", 0);
 
+	dfu_clear_detach();
+
 	return ret;
 }

drivers/dfu/dfu.c +26 -5

index 3512b14..55e6a83 100644
@@ -17,20 +17,41 @@
 #include <linux/list.h>
 #include <linux/compiler.h>
 
-static bool dfu_reset_request;
+static bool dfu_detach_request;
 static LIST_HEAD(dfu_list);
 static int dfu_alt_num;
 static int alt_num_cnt;
 static struct hash_algo *dfu_hash_algo;
 
-bool dfu_reset(void)
+/*
+ * The purpose of the dfu_usb_get_reset() function is to
+ * provide information if after USB_DETACH request
+ * being sent the dfu-util performed reset of USB
+ * bus.
+ *
+ * Described behaviour is the only way to distinct if
+ * user has typed -e (detach) or -R (reset) when invoking
+ * dfu-util command.
+ *
+ */
+__weak bool dfu_usb_get_reset(void)
+{
+	return true;
+}
+
+bool dfu_detach(void)
+{
+	return dfu_detach_request;
+}
+
+void dfu_trigger_detach(void)
 {
-	return dfu_reset_request;
+	dfu_detach_request = true;
 }
 
-void dfu_trigger_reset()
+void dfu_clear_detach(void)
 {
-	dfu_reset_request = true;
+	dfu_detach_request = false;
 }
 
 static int dfu_find_alt_num(const char *s)

drivers/usb/gadget/f_dfu.c +1 -1

index 9863dec..3e4f029 100644
@@ -372,7 +372,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
 		to_runtime_mode(f_dfu);
 		f_dfu->dfu_state = DFU_STATE_appIDLE;
 
-		dfu_trigger_reset();
+		dfu_trigger_detach();
 		break;
 	default:
 		f_dfu->dfu_state = DFU_STATE_dfuERROR;

include/dfu.h +4 -1

index 7e0a999..f1a71c7 100644
@@ -150,11 +150,14 @@ struct dfu_entity *dfu_get_entity(int alt);
 char *dfu_extract_token(char** e, int *n);
 void dfu_trigger_reset(void);
 int dfu_get_alt(char *name);
-bool dfu_reset(void);
+bool dfu_detach(void);
+void dfu_trigger_detach(void);
+void dfu_clear_detach(void);
 int dfu_init_env_entities(char *interface, char *devstr);
 unsigned char *dfu_get_buf(struct dfu_entity *dfu);
 unsigned char *dfu_free_buf(void);
 unsigned long dfu_get_buf_size(void);
+bool dfu_usb_get_reset(void);
 
 int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num);
 int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num);

Signature · 1 line

2.0.0.rc2

Lukasz Majewski

author

2014-08-25 09:07 UTC #7 lore raw

PATCH v2 2/2 udc: dfu: s3c_udc: Provide function to check if USB reset was asserted

New dfu_usb_get_reset() method is necessary to distinct two different use cases of dfu-util program.
This method checks if the USB bus reset has been really performed after DFU DETACH.

Without this function the previous DFU behavior is preserved.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v2:
- None

---
 drivers/usb/gadget/s3c_udc_otg.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/usb/gadget/s3c_udc_otg.c +5 -0

index 63d4487..ce2494c 100644
@@ -149,6 +149,11 @@ struct s3c_usbotg_reg *reg;
 struct s3c_usbotg_phy *phy;
 static unsigned int usb_phy_ctrl;
 
+bool dfu_usb_get_reset(void)
+{
+	return !!(readl(&reg->gintsts) & INT_RESET);
+}
+
 void otg_phy_init(struct s3c_udc *dev)
 {
 	dev->pdata->phy_control(1);

Signature · 1 line

2.0.0.rc2

Stephen Warren

2014-08-25 20:01 UTC #8 lore raw

PATCH 1/2 dfu: Provide means to find difference between dfu-util -e and -R

In reply to Lukasz Majewski

Quoted 17 lines · Lukasz Majewski

This commit provides distinction between DFU device detach and reset. The -R behavior is preserved with proper handling of the dfu-util's -e switch, which detach the DFU device.
By running dfu-util -e; one can force device to finish the execution of dfu command on target and execute some other scripted commands.
Moreover, some naming has been changed - the dfu_reset() method now is known as dfu_detach(). New name better reflects the purpose of the code.
It was also necessary to increase the number of usb_gadget_handle_interrupts() calls since we also must wait for detection of the USB reset event.
Example usage:
1. -e (detach) switch
dfu-util -a0 -D file1.bin;dfu-util -a3 -D uImage;dfu-util -e

Quoted 6 lines

access to u-boot prompt.
2. -R (reset) switch
dfu-util -a0 -D file1.bin;dfu-util -R -a3 -D uImage
target board reset

Tested-by Stephen Warren <[email protected]>

Lukasz Majewski

author

2014-09-01 07:27 UTC #9 lore raw

PATCH 1/2 dfu: Provide means to find difference between dfu-util -e and -R

Hi Stephen,

Quoted 21 lines · Lukasz Majewski

Quoted 20 lines
This commit provides distinction between DFU device detach and reset. The -R behavior is preserved with proper handling of the dfu-util's -e switch, which detach the DFU device.
By running dfu-util -e; one can force device to finish the execution of dfu command on target and execute some other scripted commands.
Moreover, some naming has been changed - the dfu_reset() method now is known as dfu_detach(). New name better reflects the purpose of the code.
It was also necessary to increase the number of usb_gadget_handle_interrupts() calls since we also must wait for detection of the USB reset event.
Example usage:
1. -e (detach) switch
dfu-util -a0 -D file1.bin;dfu-util -a3 -D uImage;dfu-util -e

Quoted 8 lines

Quoted 6 lines
access to u-boot prompt.
2. -R (reset) switch
dfu-util -a0 -D file1.bin;dfu-util -R -a3 -D uImage
target board reset
Tested-by Stephen Warren <[email protected]>

Applied to u-boot-dfu.

Thanks!

Signature · 5 lines

Best regards,

Lukasz Majewski

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