9 Nov
2011
9 Nov
'11
11:22 p.m.
From: Holger Hans Peter Freyther <zecke(a)selfish.org>
This still needs to receive some more testing (untested on Windows
right now) and also consideration how/if we want to upgrade this in
the field. If we consider reflashing the DFU part safe enough. I would
argue that there is still SAMBA to fix things up if someone breaks the
bootloader.
Holger Hans Peter Freyther (1):
usb: Do not send ZLP when we have filled the window
firmware/src/dfu/dfu.c | 46 ++++++++++++++++----------------
firmware/src/dfu/dfu.h | 2 +-
firmware/src/os/pcd_enumerate.c | 54 +++++++++++++++++++++------------------
3 files changed, 53 insertions(+), 49 deletions(-)
--
1.7.7.2
9 Nov
9 Nov
11:22 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
From: Holger Hans Peter Freyther <zecke(a)selfish.org>
Only send the ZLP if we send less data than was required/asked for
by the host and it is a multiple of the bMaxPacketSize0 (which is
hardcoded to 8 right now).
This is completing the change done in fe88b83e80df8be0351ff38ee6 to
fix SIMtrace attached to OSX and not regress on windows.
Introduce another parameter to udp_ep0_send_data to specify the window
size (wLength) or if not available the default from USB 2.0 specification.
---
firmware/src/dfu/dfu.c | 46 ++++++++++++++++----------------
firmware/src/dfu/dfu.h | 2 +-
firmware/src/os/pcd_enumerate.c | 54 +++++++++++++++++++++------------------
3 files changed, 53 insertions(+), 49 deletions(-)
diff --git a/firmware/src/dfu/dfu.c b/firmware/src/dfu/dfu.c
index 5d6865c..9d1ad10 100644
--- a/firmware/src/dfu/dfu.c
+++ b/firmware/src/dfu/dfu.c
@@ -96,13 +96,15 @@ static void __dfufunc udp_init(void)
static void __dfufunc udp_ep0_send_zlp(void);
/* Send Data through the control endpoint */
-static void __dfufunc udp_ep0_send_data(const char *pData, u_int32_t length)
+static void __dfufunc udp_ep0_send_data(const char *pData, u_int32_t length,
+ u_int32_t window_length)
{
AT91PS_UDP pUdp = AT91C_BASE_UDP;
- u_int32_t cpt = 0, len_remain = length;
+ u_int32_t cpt = 0, len_remain;
AT91_REG csr;
- DEBUGE("send_data: %u bytes ", length);
+ len_remain = MIN(length, window_length);
+ DEBUGE("send_data: %u/%u bytes ", length, window_length);
do {
cpt = MIN(len_remain, 8);
@@ -135,7 +137,7 @@ static void __dfufunc udp_ep0_send_data(const char *pData, u_int32_t
length)
while (pUdp->UDP_CSR[0] & AT91C_UDP_TXCOMP) ;
}
- if ((length % 8) == 0) {
+ if ((length % 8) == 0 && length < window_length) {
/* if the length is a multiple of the EP size, we need
* to send another ZLP (zero-length packet) to tell the
* host the transfer has completed. */
@@ -366,7 +368,7 @@ static __dfufunc int handle_upload(u_int16_t __unused val, u_int16_t
len)
first_download = 1;
}
- udp_ep0_send_data((char *)ptr, len);
+ udp_ep0_send_data((char *)ptr, len, len);
ptr+= len;
return len;
@@ -409,7 +411,7 @@ static __dfufunc void handle_getstatus(void)
dstat.iString = 0;
/* FIXME: set dstat.bwPollTimeout */
- udp_ep0_send_data((char *)&dstat, sizeof(dstat));
+ udp_ep0_send_data((char *)&dstat, sizeof(dstat), sizeof(dstat));
}
static void __dfufunc handle_getstate(void)
@@ -417,7 +419,7 @@ static void __dfufunc handle_getstate(void)
u_int8_t u8 = dfu_state;
DEBUGE("getstate ");
- udp_ep0_send_data((char *)&u8, sizeof(u8));
+ udp_ep0_send_data((char *)&u8, sizeof(u8), sizeof(u8));
}
/* callback function for DFU requests */
@@ -806,15 +808,13 @@ static __dfufunc void dfu_udp_ep0_handler(void)
/* Return Device Descriptor */
udp_ep0_send_data((const char *)
&dfu_dev_descriptor,
- MIN(sizeof(dfu_dev_descriptor),
- wLength));
+ sizeof(dfu_dev_descriptor), wLength);
break;
case USB_DT_CONFIG:
/* Return Configuration Descriptor */
udp_ep0_send_data((const char *)
&dfu_cfg_descriptor,
- MIN(sizeof(dfu_cfg_descriptor),
- wLength));
+ sizeof(dfu_cfg_descriptor), wLength);
break;
case USB_DT_STRING:
/* Return String Descriptor */
@@ -825,14 +825,12 @@ static __dfufunc void dfu_udp_ep0_handler(void)
DEBUGE("bLength=%u, wLength=%u ",
usb_strings[desc_index]->bLength, wLength);
udp_ep0_send_data((const char *) usb_strings[desc_index],
- MIN(usb_strings[desc_index]->bLength,
- wLength));
+ usb_strings[desc_index]->bLength, wLength);
break;
case USB_DT_CS_DEVICE:
/* Return Function descriptor */
udp_ep0_send_data((const char *) &dfu_cfg_descriptor.func_dfu,
- MIN(sizeof(dfu_cfg_descriptor.func_dfu),
- wLength));
+ sizeof(dfu_cfg_descriptor.func_dfu), wLength);
break;
default:
udp_ep0_send_stall();
@@ -867,18 +865,20 @@ static __dfufunc void dfu_udp_ep0_handler(void)
break;
case STD_GET_CONFIGURATION:
DEBUGE("GET_CONFIG ");
- udp_ep0_send_data((char *)&(cur_config),
- sizeof(cur_config));
+ /* Table 9.4 wLength One */
+ udp_ep0_send_data((char *)&(cur_config), sizeof(cur_config), 1);
break;
case STD_GET_STATUS_ZERO:
DEBUGE("GET_STATUS_ZERO ");
wStatus = 0;
- udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
break;
case STD_GET_STATUS_INTERFACE:
DEBUGE("GET_STATUS_INTERFACE ");
wStatus = 0;
- udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
break;
case STD_GET_STATUS_ENDPOINT:
DEBUGE("GET_STATUS_ENDPOINT(EPidx=%u) ", wIndex&0x0f);
@@ -887,14 +887,14 @@ static __dfufunc void dfu_udp_ep0_handler(void)
if ((pUDP->UDP_GLBSTATE & AT91C_UDP_CONFG) && (wIndex == 0)) {
wStatus =
(pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
- udp_ep0_send_data((char *)&wStatus,
- sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
} else if ((pUDP->UDP_GLBSTATE & AT91C_UDP_FADDEN)
&& (wIndex == 0)) {
wStatus =
(pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
- udp_ep0_send_data((char *)&wStatus,
- sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
} else
udp_ep0_send_stall();
break;
diff --git a/firmware/src/dfu/dfu.h b/firmware/src/dfu/dfu.h
index c898197..27c63b5 100644
--- a/firmware/src/dfu/dfu.h
+++ b/firmware/src/dfu/dfu.h
@@ -124,7 +124,7 @@ struct _dfu_desc {
struct dfuapi {
void (*udp_init)(void);
- void (*ep0_send_data)(const char *data, u_int32_t len);
+ void (*ep0_send_data)(const char *data, u_int32_t len, u_int32_t wlen);
void (*ep0_send_zlp)(void);
void (*ep0_send_stall)(void);
int (*dfu_ep0_handler)(u_int8_t req_type, u_int8_t req,
diff --git a/firmware/src/os/pcd_enumerate.c b/firmware/src/os/pcd_enumerate.c
index c360d37..3a7397f 100644
--- a/firmware/src/os/pcd_enumerate.c
+++ b/firmware/src/os/pcd_enumerate.c
@@ -531,12 +531,12 @@ static void udp_ep0_handler(void)
if (*dfu->dfu_state != DFU_STATE_appIDLE)
udp_ep0_send_data((const char *)
dfu->dfu_dev_descriptor,
- MIN(dfu->dfu_dev_descriptor->bLength,
- wLength));
+ dfu->dfu_dev_descriptor->bLength,
+ wLength);
else
#endif
udp_ep0_send_data((const char *) &dev_descriptor,
- MIN(sizeof(dev_descriptor), wLength));
+ sizeof(dev_descriptor), wLength);
break;
case USB_DT_CONFIG:
/* Return Configuration Descriptor */
@@ -544,12 +544,12 @@ static void udp_ep0_handler(void)
if (*dfu->dfu_state != DFU_STATE_appIDLE)
udp_ep0_send_data((const char *)
dfu->dfu_cfg_descriptor,
- MIN(dfu->dfu_cfg_descriptor->ucfg.wTotalLength,
- wLength));
+ dfu->dfu_cfg_descriptor->ucfg.wTotalLength,
+ wLength);
else
#endif
udp_ep0_send_data((const char *) &cfg_descriptor,
- MIN(sizeof(cfg_descriptor), wLength));
+ sizeof(cfg_descriptor), wLength);
break;
case USB_DT_STRING:
#ifdef CONFIG_USB_STRING
@@ -559,8 +559,8 @@ static void udp_ep0_handler(void)
DEBUGE("bLength=%u, wLength=%u\n",
usb_strings[desc_index]->bLength, wLength);
udp_ep0_send_data((const char *) usb_strings[desc_index],
- MIN(usb_strings[desc_index]->bLength,
- wLength));
+ usb_strings[desc_index]->bLength,
+ wLength);
#else
goto out_stall;
#endif
@@ -568,7 +568,7 @@ static void udp_ep0_handler(void)
case USB_DT_CS_DEVICE:
/* Return Function descriptor */
udp_ep0_send_data((const char *) &dfu->dfu_cfg_descriptor->func_dfu,
- MIN(sizeof(dfu->dfu_cfg_descriptor->func_dfu), wLength));
+ sizeof(dfu->dfu_cfg_descriptor->func_dfu), wLength);
break;
case USB_DT_INTERFACE:
/* Return Interface descriptor */
@@ -578,27 +578,27 @@ static void udp_ep0_handler(void)
case 0:
udp_ep0_send_data((const char *)
&cfg_descriptor.uif,
- MIN(sizeof(cfg_descriptor.uif),
- wLength));
+ sizeof(cfg_descriptor.uif),
+ wLength);
break;
#ifdef CONFIG_DFU
case 1:
udp_ep0_send_data((const char *)
&cfg_descriptor.uif_dfu[0],
- MIN(sizeof(cfg_descriptor.uif_dfu[0]),
- wLength));
+ sizeof(cfg_descriptor.uif_dfu[0]),
+ wLength);
break;
case 2:
udp_ep0_send_data((const char *)
&cfg_descriptor.uif_dfu[1],
- MIN(sizeof(cfg_descriptor.uif_dfu[1]),
- wLength));
+ sizeof(cfg_descriptor.uif_dfu[1]),
+ wLength);
break;
case 3:
udp_ep0_send_data((const char *)
&cfg_descriptor.uif_dfu[2],
- MIN(sizeof(cfg_descriptor.uif_dfu[2]),
- wLength));
+ sizeof(cfg_descriptor.uif_dfu[2]),
+ wLength);
break;
#endif
default:
@@ -679,8 +679,9 @@ static void udp_ep0_handler(void)
switch (upcd.state) {
case USB_STATE_ADDRESS:
case USB_STATE_CONFIGURED:
+ /* Table 9.4 wLength One */
udp_ep0_send_data((char *)&(upcd.cur_config),
- sizeof(upcd.cur_config));
+ sizeof(upcd.cur_config), 1);
break;
default:
goto out_stall;
@@ -691,13 +692,15 @@ static void udp_ep0_handler(void)
DEBUGE("GET_INTERFACE ");
if (upcd.state != USB_STATE_CONFIGURED)
goto out_stall;
+ /* Table 9.4 wLength One */
udp_ep0_send_data((char *)&(upcd.cur_altsett),
- sizeof(upcd.cur_altsett));
+ sizeof(upcd.cur_altsett), 1);
break;
case STD_GET_STATUS_ZERO:
DEBUGE("GET_STATUS_ZERO ");
wStatus = 0;
- udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
break;
case STD_GET_STATUS_INTERFACE:
DEBUGE("GET_STATUS_INTERFACE ");
@@ -705,7 +708,8 @@ static void udp_ep0_handler(void)
(upcd.state == USB_STATE_ADDRESS && wIndex != 0))
goto out_stall;
wStatus = 0;
- udp_ep0_send_data((char *)&wStatus, sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
break;
case STD_GET_STATUS_ENDPOINT:
DEBUGE("GET_STATUS_ENDPOINT(EPidx=%u) ", wIndex&0x0f);
@@ -717,14 +721,14 @@ static void udp_ep0_handler(void)
if ((pUDP->UDP_GLBSTATE & AT91C_UDP_CONFG) && (wIndex <= 3)) {
wStatus =
(pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
- udp_ep0_send_data((char *)&wStatus,
- sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
} else if ((pUDP->UDP_GLBSTATE & AT91C_UDP_FADDEN)
&& (wIndex == 0)) {
wStatus =
(pUDP->UDP_CSR[wIndex] & AT91C_UDP_EPEDS) ? 0 : 1;
- udp_ep0_send_data((char *)&wStatus,
- sizeof(wStatus));
+ /* Table 9.4 wLength Two */
+ udp_ep0_send_data((char *)&wStatus, sizeof(wStatus), 2);
} else
goto out_stall;
break;
--
1.7.7.2
10 Nov
10 Nov
9:20 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
On 11/09/2011 11:22 PM, Holger Hans Peter Freyther wrote:
From: Holger Hans Peter Freyther
<zecke(a)selfish.org>
I enabled debug in dfu and added another debug statement. It appears to do the
right thing. I have attached the device to Windows running in KVM but I think
that a native windows will behave differently (bus reset, etc). I will try to
test with a native windows, please don't apply the patch yet.
main(76): entering main (idle) loop
send_data: 18/64 bytes stopped by status out send_data: 18/18 bytes NO ZLP 18 18
send_data: 68/9 bytes NO ZLP 68 9
send_data: 68/66 bytes NO ZLP 68 66
send_data: 4/255 bytes NO ZLP 4 255
send_data: 72/255 bytes set_txpktrdy_zlp
send_data: 100/255 bytes NO ZLP 100 255
send_data: 62/255 bytes NO ZLP 62 255
send_data: 54/255 bytes NO ZLP 54 255
send_data: 94/255 bytes NO ZLP 94 255
send_data: 92/255 bytes NO ZLP 92 255
send_data: 58/255 bytes NO ZLP 58 255
send_data: 1/1 bytes NO ZLP 1 1
send_data: 18/64 bytes stopped by status out send_data: 18/18 bytes NO ZLP 18 18
send_data: 68/66 bytes NO ZLP 68 66
send_data: 18/64 bytes NO ZLP 18 64
send_data: 18/64 bytes stopped by status out send_data: 18/18 bytes NO ZLP 18 18
send_data: 68/66 bytes NO ZLP 68 66
send_data: 18/18 bytes NO ZLP 18 18
send_data: 68/255 bytes NO ZLP 68 255
send_data: 4/255 bytes NO ZLP 4 255
send_data: 72/255 bytes set_txpktrdy_zlp
29 Nov
29 Nov
5:17 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
On 11/10/2011 09:20 PM, Holger Hans Peter Freyther wrote:
On 11/09/2011 11:22 PM, Holger Hans Peter Freyther
wrote:
> From: Holger Hans Peter Freyther <zecke(a)selfish.org>
Hi again,
i tested this patch by connecting the device to Mac OS X 10.6, Windows7 in
qemu/kvm (with -usb) and Dieter tested it by attaching yo his device. The
upgrade procedure is not tested yet but should be:
1.) flash the new DFU
2.) reboot with bootloader button pressed
3.) flash the new main_simtrace.bin
14 Dec
14 Dec
3:17 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
Hi Holger,
On Wed, Nov 09, 2011 at 11:22:13PM +0100, Holger Hans Peter Freyther wrote:
Only send the ZLP if we send less data than was
required/asked for
by the host and it is a multiple of the bMaxPacketSize0 (which is
hardcoded to 8 right now).
I've finally merged this.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
8:09 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
That should basically mean we shouldn't need the os x hack anymore?
Cheers,
Lukas
Sent from my iPhone
On Dec 14, 2011, at 3:31 PM, Harald Welte <laforge(a)gnumonks.org> wrote:
Hi Holger,
On Wed, Nov 09, 2011 at 11:22:13PM +0100, Holger Hans Peter Freyther wrote:
Only send the ZLP if we send less data than was
required/asked for
by the host and it is a multiple of the bMaxPacketSize0 (which is
hardcoded to 8 right now).
I've finally merged this.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
8:15 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
On 12/14/2011 08:09 PM, Lukas Kuzmiak wrote:
That should basically mean we shouldn't need the
os x hack anymore?
right. feel free to test the dfu upgrade path.
1.) reflash the dfu part
2.) reflash the main part
3.) reset/reboot
harald proposed to add ABI versioning to payloads, it might be a 30 minute
work and I think I will attempt to find some time for it.
15 Dec
15 Dec
10:26 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
Hi Holger,
I've just cloned the latest git, compiled with no patches, reflashed the
whole thing and can confirm it works with no problem on mac os x lion
(macbook air).
Cheers,
Lukas
On Wed, Dec 14, 2011 at 8:15 PM, Holger Hans Peter Freyther <
holger(a)freyther.de> wrote:
On 12/14/2011 08:09 PM, Lukas Kuzmiak wrote:
That should basically mean we shouldn't need
the os x hack anymore?
right. feel free to test the dfu upgrade path.
1.) reflash the dfu part
2.) reflash the main part
3.) reset/reboot
harald proposed to add ABI versioning to payloads, it might be a 30 minute
work and I think I will attempt to find some time for it.
17 Dec
17 Dec
9:20 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
Is there a compiled version of this new firmware somewhere ?
Cheers,
Sylvain
9:50 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
Haven't noticed any, but I can upload you one I've compiled if u want.
Cheers,
Lukas
Sent from my iPhone
On Dec 17, 2011, at 9:47 PM, Sylvain Munaut <246tnt(a)gmail.com> wrote:
Is there a compiled version of this new firmware
somewhere ?
Cheers,
Sylvain
10 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
> Is there a compiled version of this new firmware
somewhere ?
Haven't noticed any, but I can upload you one
I've compiled if u want.
Yes, I think you could put it in the wikipage
http://bb.osmocom.org/trac/wiki/SIMtrace/Firmware with the others.
(I think both main and dfu part are needed right ?)
Cheers,
Sylvain
10:08 p.m.
New subject: [PATCH] usb: Do not send ZLP when we have filled the window
On 12/17/2011 10:00 PM, Sylvain Munaut wrote:
Hi,
I would prefer to not upload non-tagged versions. I think there is one patch
from Kevin that is not merged yet.
holger
> Is there a
compiled version of this new firmware somewhere ?
Haven't noticed any, but I can upload you one
I've compiled if u want.
Yes, I think you could put it in the wikipage
http://bb.osmocom.org/trac/wiki/SIMtrace/Firmware with the others.
4522
days inactive
4560
days old
11 comments
4 participants
participants (4)
-
Harald Welte -
Holger Hans Peter Freyther -
Lukas Kuzmiak -
Sylvain Munaut