Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-12 Thread Marek Vasut
Dear Tom Rini,

 On 12/10/12 19:47, Marek Vasut wrote:
  Dear Lukasz Majewski,
  
  Pantelis,
  
  [...]
  
  Hm hm ... I suspect it'd be nice to have a separate DFU custodian.
  That'd leverage some burden from me. I like that idea. I wonder if
  it'd be nice to start building such bigger net of custodians.
 
 So long as everyone involved plays nice, we have a maintainer of the
 code (Lukasz) and another user / developer of the code (Pantelis).  I
 don't think we need to go full on custodian right now.  Lukasz is
 reviewing and trying to understand what Pantelis is seeing since we're
 seeing some odd issues on the second platform to add DFU support.

WFM, but it'd leverage burden from me. And I honestly dont understand the DFU 
as 
much as Lukasz does.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-12 Thread Tom Rini
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 12/12/12 12:55, Marek Vasut wrote:
 Dear Tom Rini,
 
 On 12/10/12 19:47, Marek Vasut wrote:
 Dear Lukasz Majewski,
 
 Pantelis,
 
 [...]
 
 Hm hm ... I suspect it'd be nice to have a separate DFU
 custodian. That'd leverage some burden from me. I like that
 idea. I wonder if it'd be nice to start building such bigger
 net of custodians.
 
 So long as everyone involved plays nice, we have a maintainer of
 the code (Lukasz) and another user / developer of the code
 (Pantelis).  I don't think we need to go full on custodian right
 now.  Lukasz is reviewing and trying to understand what Pantelis
 is seeing since we're seeing some odd issues on the second
 platform to add DFU support.
 
 WFM, but it'd leverage burden from me. And I honestly dont
 understand the DFU as much as Lukasz does.

That's fine.  Lean on maintainers as much as they're agreeable to.
I'm sure (well, I hope and assume) Lukasz would like to ack anything
DFU related that goes in and if you don't want to pull for u-boot-usb
until he does, that's fine and good with me.

- -- 
Tom
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQyMbsAAoJENk4IS6UOR1W/nUP/05xGbixnO450kJyxMCQZGc2
0QHAy41jTqGnBKGLutZE0ged/lKxo2UwNbAklJc/EixILFLIjOaRpNlKARo7G4hH
NHX8SXYgHVUY4R1fWgciH/gMO+lNAsLOP30peR0KtQyL6EBmC7fEUxjEPUKsCKWy
RA8AOG9QBD88k1wt2GxEkuXlONpPO9/LTb39ZPrUgfo9iG8oTEcYWq98OfitWKvf
Rd3q2XxUpu1UxUxh4ThPE1uj9V5l+d5eeTYismOcB/WvjAAJ0o6H26MDov1Z49TP
9ezB++NEzAoh5VO2WMBmruLBUBZkvl+XQLHCLgGy62Zvw5EYolYdQu0+2TYsNB17
kxWDdZEi0t+y+uZ3qSXcxnTta/yAwvv0LeE6mODA1h8Q717qlBSkCch2Nr3jwYU5
LbPvnAVHX+f1gslGmiL192uCpQ0YtNE+0McHSQ55/P9s+aZrOnlkZApLvAfKyQJr
E9bufbgeey3JXa8O64mVNX8rDcvVPXftdP1Hqbrw42UkCK89KiskTjQ7PYO2yVpq
Ge/JnJzNnYDtf3YCbM8iIM7rUo228ugG8d7Wd9t0ISimT3zU05KoNdOOIqiCsmAd
FLyC9tjQqctic1kh2+KBBAaNy3rpKaDDb2LOpjYjaQsftDOUJ0zBxcCinh5GPI7n
yYnG1XxwV1sqT1Fov49x
=YfYD
-END PGP SIGNATURE-
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-12 Thread Marek Vasut
Dear Lukasz Majewski,

 Hi Pantelis,
 
  Tomorrow I will prepare output of USB Ellisys analizer on my side, so
  we could get clue what is going on.
 
 Please find attached output from USB ellisys analizer.
 
 (It is possible to download WinXP based program to view logs without
 USB analizer box).
 
 What I see in the current implementation stalls on GetDescriptor
 (Class: 0x21),but afterwards transmission is performed.

Or run u-boot in qemu and tunnel it through usbmon :p

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-12 Thread Marek Vasut
Dear Robert P. J. Day,

 On Tue, 11 Dec 2012, Lukasz Majewski wrote:
  Hi Pantelis,
  
   Tomorrow I will prepare output of USB Ellisys analizer on my side, so
   we could get clue what is going on.
  
  Please find attached output from USB ellisys analizer.
 
   is it really appropriate to post 8M of output to a mailing
 list?  what's wrong with pastebin?

:rageface:

http://www.ietf.org/rfc/rfc1855.txt

 rday

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-11 Thread Lukasz Majewski
Hi Marek,

 Dear Lukasz Majewski,
 
  Pantelis,
 [...]
 
 Hm hm ... I suspect it'd be nice to have a separate DFU custodian.
 That'd leverage some burden from me. I like that idea. I wonder if
 it'd be nice to start building such bigger net of custodians.

I think,that this (political) decision shall be made by Wolfgang or Tom.

 
 Hm ?

No problem from my side. I would be honored to be DFU maintainer (as
part of USB subsystem).


 
 Best regards,
 Marek Vasut



-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-11 Thread Wolfgang Denk
Dear Marek Vasut,

In message 201212110147.49045.ma...@denx.de you wrote:

 Hm hm ... I suspect it'd be nice to have a separate DFU custodian. That'd 
 leverage some burden from me. I like that idea. I wonder if it'd be nice to 
 start building such bigger net of custodians.

I'm not sure about what the limit for a managable number of custodians
might be.  If the number grows too far, we need additional levels in
the hierarchy, concentrators of some kind - similar to what Albert is
doing for ARM.

My gut feeling is that we are not close to any such limit yet, on the
other hand I seriously doubt that somethign like DFU really needs the
formal establishment of a custodian.  Who are the regular users and
who the developers of this piece of code?

Best regards,

Wolfgang Denk

PS: I'm always surprised how the random generator manages to pick the
perfectly fitting quote for my signature :-)

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Conceptual integrity in turn dictates that the  design  must  proceed
from  one  mind,  or  from  a  very small number of agreeing resonant
minds.   - Frederick Brooks Jr., The Mythical Man Month
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-11 Thread Lukasz Majewski
Hi Lukasz,

 Tomorrow I will prepare output of USB Ellisys analizer on my side, so
 we could get clue what is going on.   

Since log itself waits for moderator approval, I will be more precise:

1. dfu-util version 0.1+svnexported

2. u-boot-denx master branch:

SHA1: d987274e214cbfc7a56504fb3f0575fc6d2c587a

-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-11 Thread Pantelis Antoniou
Hi Lukasz,

I bet transmission is performed, but with the default settings of dfu.
The DFU function descriptor is completely ignored.

An easy way to verify it is to check if the DFU version of the device
is the same one as the one stored in the descriptor. Same with the transmission
block size.

It might work, but only by accident.

I sure hope I'll have time today to send my captures as well.

Regards

-- Pantelis
 
On Dec 11, 2012, at 1:02 PM, Lukasz Majewski wrote:

 Hi Pantelis,
 
 Tomorrow I will prepare output of USB Ellisys analizer on my side, so
 we could get clue what is going on. 
 
 Please find attached output from USB ellisys analizer.
 
 (It is possible to download WinXP based program to view logs without
 USB analizer box).
 
 What I see in the current implementation stalls on GetDescriptor
 (Class: 0x21),but afterwards transmission is performed.
 
 
 
 -- 
 Best regards,
 
 Lukasz Majewski
 
 Samsung Poland RD Center | Linux Platform Group
 DFU_mainline_trats.tar

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-11 Thread Robert P. J. Day
On Tue, 11 Dec 2012, Lukasz Majewski wrote:

 Hi Pantelis,

  Tomorrow I will prepare output of USB Ellisys analizer on my side, so
  we could get clue what is going on.

 Please find attached output from USB ellisys analizer.

  is it really appropriate to post 8M of output to a mailing
list?  what's wrong with pastebin?

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-11 Thread Tom Rini
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 12/10/12 19:47, Marek Vasut wrote:
 Dear Lukasz Majewski,
 
 Pantelis,
 [...]
 
 Hm hm ... I suspect it'd be nice to have a separate DFU custodian.
 That'd leverage some burden from me. I like that idea. I wonder if
 it'd be nice to start building such bigger net of custodians.

So long as everyone involved plays nice, we have a maintainer of the
code (Lukasz) and another user / developer of the code (Pantelis).  I
don't think we need to go full on custodian right now.  Lukasz is
reviewing and trying to understand what Pantelis is seeing since we're
seeing some odd issues on the second platform to add DFU support.

- -- 
Tom
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQxzi6AAoJENk4IS6UOR1WfOcP/3ix59xKvaGPxQcW9XLBbOjj
XQC1jwqsJHTMlYY+gabgCH+A8sCiL8X5qHV7fbMu9RttS1cYvojxojk0dcIxpYdI
bQCAnKFKBNCAHzbXSqFJPnfrw8tZs7c0Ug+KfzscG+W6jacrCyb0U+NEV6QJdO+y
w+emS4zKFRMFxS6RSeAJcj5EKUa/ozn+O43oQamDl38MQ5Tut2UNZF6gf293Xw8E
+mquY3vAQiDe05x1hEt7GALwgEWrwfFVU1l5c+Xs75ERFhC2boDd8EL42GRz2HRq
X9flEOiF8za+CojRJ3yLR67jMgP4p+zmWUSPQdbqnRjrw5rzM/o2K3fSFlykmQeW
SmMaaB/imf3kJPskMsEQu9CGAJl/jWvUhM+wsaVp+YEMLTcOrQt9AzPFOGS/Zpee
hvIYdTrhGaXNDGo02kCBIvp/X2/rVBt8x4r3zhB3dDZHGxE3c4bGed6iftO8cBxA
2NODvJ2ZDGgN8i3WKo18sg4K4W5ocGck77iBFx1grfja6jr9Xcwsn3h1QIVePTXe
qMMjO7h58VrRojOr1+UwWECEbZXqr0wV8HAzTtu0KgQ4v6AcD43bHjaCg9gwWJ74
aCMmvTpW96l/8m1NAIeAWmX3fgzriMUsI/06aIpEQdFsmRF+BC452YgcyRHJbeBT
vE06Y5LBMbtGFixSgq5b
=gqji
-END PGP SIGNATURE-
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-10 Thread Lukasz Majewski
Hi Pantelis,

 DFU is a bit peculiar. It needs to hook to composite setup and
 return it's function descriptor.
 
 So when get-descriptor request comes with a type of DFU_DT_FUNC
 we iterate over the configs, and functions, and when we find
 the DFU function we call the setup method which is prepared
 to return the appropriate function descriptor.

Sorry, but could you be more informative here? Have you had any non
standard problems? I wonder why dfu-util on my linux works OK without
this patch?

 
 Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
 ---
  drivers/usb/gadget/composite.c | 27 +++
  drivers/usb/gadget/ep0.c   |  1 +
  drivers/usb/gadget/f_dfu.c |  6 --
  3 files changed, 32 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/gadget/composite.c
 b/drivers/usb/gadget/composite.c index ebb5131..6496436 100644
 --- a/drivers/usb/gadget/composite.c
 +++ b/drivers/usb/gadget/composite.c
 @@ -773,6 +773,33 @@ composite_setup(struct usb_gadget *gadget, const
 struct usb_ctrlrequest *ctrl) if (value = 0)
   value = min(w_length, (u16) value);
   break;
 +
 +#ifdef CONFIG_DFU_FUNCTION
 + /* DFU is mighty weird */
^^ - please explain this
wiredness. 

I don't recall such a hacks in linux kernel composite.c (any special
#ifdef). Am I missing something important in DFU?


Does your device have any special requirement, so you need this hack?

I generally don't like the idea to patch composite gadget code with
#ifdefs for special function. Please convince me.

 + case DFU_DT_FUNC:
 + w_value = 0xff;
 + list_for_each_entry(c, cdev-configs, list)
 {
 + if (w_value != 0) {
 + w_value--;
 + continue;
 + }
 +
 + list_for_each_entry(f,
 c-functions, list) { +
 + /* DFU function only */
 + if (strcmp(f-name,
 dfu) != 0)
 + continue;
 +
 + value = f-setup(f, ctrl);
 + goto dfu_func_done;
 + }
 + }
 +dfu_func_done:
 + if (value = 0)
 + value = min(w_length, (u16) value);
 + break;
 +#endif
 +
   default:
   goto unknown;
   }
 diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
 index aa8f916..971d846 100644
 --- a/drivers/usb/gadget/ep0.c
 +++ b/drivers/usb/gadget/ep0.c
 @@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct
 usb_device_instance *device, break;
  
   case USB_DESCRIPTOR_TYPE_CONFIGURATION:
 + case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
^- why do you need that?
   {
   struct usb_configuration_descriptor
   *configuration_descriptor;
 diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
 index 10547e3..6494f5e 100644
 --- a/drivers/usb/gadget/f_dfu.c
 +++ b/drivers/usb/gadget/f_dfu.c
 @@ -534,8 +534,10 @@ dfu_handle(struct usb_function *f, const struct
 usb_ctrlrequest *ctrl) value = min(len, (u16) sizeof(dfu_func));
   memcpy(req-buf, dfu_func, value);
   }
 - } else /* DFU specific request */
 - value = dfu_state[f_dfu-dfu_state] (f_dfu, ctrl,
 gadget, req);
 + return value;
 + }
 +
 + value = dfu_state[f_dfu-dfu_state] (f_dfu, ctrl, gadget,
 req); 

Why do you change state even after receiving req_type ==
USB_TYPE_STANDARD? I would expect to change the dfu state only when DFU
specific request appears.



   if (value = 0) {
   req-length = value;



-- 
Best regards,

Lukasz Majewski

Samsung Poland RD Center | Linux Platform Group
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-10 Thread Pantelis Antoniou
Hi Lukasz,

On Dec 10, 2012, at 7:11 PM, Lukasz Majewski wrote:

 Hi Pantelis,
 
 DFU is a bit peculiar. It needs to hook to composite setup and
 return it's function descriptor.
 
 So when get-descriptor request comes with a type of DFU_DT_FUNC
 we iterate over the configs, and functions, and when we find
 the DFU function we call the setup method which is prepared
 to return the appropriate function descriptor.
 
 Sorry, but could you be more informative here? Have you had any non
 standard problems? I wonder why dfu-util on my linux works OK without
 this patch?
 

I have absolutely no idea why it works at your side.

At our side it just didn't work at all without the patches.

If I had to guess maybe your gadget h/w takes care of replying
properly for the DFU case.

 
 Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
 ---
 drivers/usb/gadget/composite.c | 27 +++
 drivers/usb/gadget/ep0.c   |  1 +
 drivers/usb/gadget/f_dfu.c |  6 --
 3 files changed, 32 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/gadget/composite.c
 b/drivers/usb/gadget/composite.c index ebb5131..6496436 100644
 --- a/drivers/usb/gadget/composite.c
 +++ b/drivers/usb/gadget/composite.c
 @@ -773,6 +773,33 @@ composite_setup(struct usb_gadget *gadget, const
 struct usb_ctrlrequest *ctrl) if (value = 0)
  value = min(w_length, (u16) value);
  break;
 +
 +#ifdef CONFIG_DFU_FUNCTION
 +/* DFU is mighty weird */
   ^^ - please explain this
   wiredness. 
 
 I don't recall such a hacks in linux kernel composite.c (any special
 #ifdef). Am I missing something important in DFU?
 
 
 Does your device have any special requirement, so you need this hack?
 
 I generally don't like the idea to patch composite gadget code with
 #ifdefs for special function. Please convince me.

It doesn't work otherwise. I have no idea why you think I would be hacking
around there if the thing worked at all. And trust me on that, it just doesn't
without those patches, not to mention the way it unceremoniously blows up if
you transfer anything larger than the buffer set aside originally.

The way I see it, instead of complaining you should be rejoicing since now
DFU will be used in an actual production environment. 

More users == less bugs.

When I get a few free cycles I will post a tcpdump capture of the DFU USB
transaction hanging.

 
 +case DFU_DT_FUNC:
 +w_value = 0xff;
 +list_for_each_entry(c, cdev-configs, list)
 {
 +if (w_value != 0) {
 +w_value--;
 +continue;
 +}
 +
 +list_for_each_entry(f,
 c-functions, list) { +
 +/* DFU function only */
 +if (strcmp(f-name,
 dfu) != 0)
 +continue;
 +
 +value = f-setup(f, ctrl);
 +goto dfu_func_done;
 +}
 +}
 +dfu_func_done:
 +if (value = 0)
 +value = min(w_length, (u16) value);
 +break;
 +#endif
 +
  default:
  goto unknown;
  }
 diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
 index aa8f916..971d846 100644
 --- a/drivers/usb/gadget/ep0.c
 +++ b/drivers/usb/gadget/ep0.c
 @@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct
 usb_device_instance *device, break;
 
  case USB_DESCRIPTOR_TYPE_CONFIGURATION:
 +case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
   ^- why do you need that?
  {
  struct usb_configuration_descriptor
  *configuration_descriptor;
 diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
 index 10547e3..6494f5e 100644
 --- a/drivers/usb/gadget/f_dfu.c
 +++ b/drivers/usb/gadget/f_dfu.c
 @@ -534,8 +534,10 @@ dfu_handle(struct usb_function *f, const struct
 usb_ctrlrequest *ctrl) value = min(len, (u16) sizeof(dfu_func));
  memcpy(req-buf, dfu_func, value);
  }
 -} else /* DFU specific request */
 -value = dfu_state[f_dfu-dfu_state] (f_dfu, ctrl,
 gadget, req);
 +return value;
 +}
 +
 +value = dfu_state[f_dfu-dfu_state] (f_dfu, ctrl, gadget,
 req); 
 
 Why do you change state even after receiving req_type ==
 USB_TYPE_STANDARD? I would expect to change the dfu state only when DFU
 specific request appears.
 
 

 
  if (value = 0) {
  req-length = value;
 
 
 
 -- 
 Best regards,
 
 Lukasz Majewski
 
 Samsung Poland RD Center | Linux Platform Group

Regards


Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-10 Thread Lukasz Majewski
Pantelis,

 Hi Lukasz,
 
 On Dec 10, 2012, at 7:11 PM, Lukasz Majewski wrote:
 
  Hi Pantelis,
  
  DFU is a bit peculiar. It needs to hook to composite setup and
  return it's function descriptor.
  
  So when get-descriptor request comes with a type of DFU_DT_FUNC
  we iterate over the configs, and functions, and when we find
  the DFU function we call the setup method which is prepared
  to return the appropriate function descriptor.
  
  Sorry, but could you be more informative here? Have you had any non
  standard problems? I wonder why dfu-util on my linux works OK
  without this patch?
  
 
 I have absolutely no idea why it works at your side.
 
 At our side it just didn't work at all without the patches.
 
 If I had to guess maybe your gadget h/w takes care of replying
 properly for the DFU case.
 
  
  Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
  ---
  drivers/usb/gadget/composite.c | 27 +++
  drivers/usb/gadget/ep0.c   |  1 +
  drivers/usb/gadget/f_dfu.c |  6 --
  3 files changed, 32 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/usb/gadget/composite.c
  b/drivers/usb/gadget/composite.c index ebb5131..6496436 100644
  --- a/drivers/usb/gadget/composite.c
  +++ b/drivers/usb/gadget/composite.c
  @@ -773,6 +773,33 @@ composite_setup(struct usb_gadget *gadget,
  const struct usb_ctrlrequest *ctrl) if (value = 0)
 value = min(w_length, (u16) value);
 break;
  +
  +#ifdef CONFIG_DFU_FUNCTION
  +  /* DFU is mighty weird */
  ^^ - please explain this
  wiredness. 
  
  I don't recall such a hacks in linux kernel composite.c (any special
  #ifdef). Am I missing something important in DFU?
  
  
  Does your device have any special requirement, so you need this
  hack?
  
  I generally don't like the idea to patch composite gadget code
  with #ifdefs for special function. Please convince me.
 
 It doesn't work otherwise. I have no idea why you think I would be
 hacking around there if the thing worked at all. And trust me on
 that, it just doesn't without those patches, not to mention the way
 it unceremoniously blows up if you transfer anything larger than the
 buffer set aside originally.
 
 The way I see it, instead of complaining you should be rejoicing
 since now DFU will be used in an actual production environment. 

I'm not complaining. I try to resolve the problem, since this can make
dfu support better at u-boot. 


Moreover I'm aware that USB is tricky, so I want to understand your
problem.

Tomorrow I will prepare output of USB Ellisys analizer on my side, so we
could get clue what is going on.

 
 More users == less bugs.

I'm really happy, that you have posted patches for NAND flashing. 

Without your determination at debugging, problem with buffer overflow
wouldn't be discovered.

We only needs to share knowledge and provide solution acceptable for
community and us.

I'm open.

 
 When I get a few free cycles I will post a tcpdump capture of the DFU
 USB transaction hanging.

Yes, please. 

 
  
  +  case DFU_DT_FUNC:
  +  w_value = 0xff;
  +  list_for_each_entry(c, cdev-configs,
  list) {
  +  if (w_value != 0) {
  +  w_value--;
  +  continue;
  +  }
  +
  +  list_for_each_entry(f,
  c-functions, list) { +
  +  /* DFU function only */
  +  if (strcmp(f-name,
  dfu) != 0)
  +  continue;
  +
  +  value = f-setup(f, ctrl);
  +  goto dfu_func_done;
  +  }
  +  }
  +dfu_func_done:
  +  if (value = 0)
  +  value = min(w_length, (u16)
  value);
  +  break;
  +#endif
  +
 default:
 goto unknown;
 }
  diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
  index aa8f916..971d846 100644
  --- a/drivers/usb/gadget/ep0.c
  +++ b/drivers/usb/gadget/ep0.c
  @@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct
  usb_device_instance *device, break;
  
 case USB_DESCRIPTOR_TYPE_CONFIGURATION:
  +  case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
  ^- why do you need that?
 {
 struct usb_configuration_descriptor
 *configuration_descriptor;
  diff --git a/drivers/usb/gadget/f_dfu.c
  b/drivers/usb/gadget/f_dfu.c index 10547e3..6494f5e 100644
  --- a/drivers/usb/gadget/f_dfu.c
  +++ b/drivers/usb/gadget/f_dfu.c
  @@ -534,8 +534,10 @@ dfu_handle(struct usb_function *f, const
  struct usb_ctrlrequest *ctrl) value = min(len, (u16)
  

Re: [U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-12-10 Thread Marek Vasut
Dear Lukasz Majewski,

 Pantelis,
[...]

Hm hm ... I suspect it'd be nice to have a separate DFU custodian. That'd 
leverage some burden from me. I like that idea. I wonder if it'd be nice to 
start building such bigger net of custodians.

Hm ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v3 6/9] dfu: Send correct DFU response from composite_setup

2012-11-30 Thread Pantelis Antoniou
DFU is a bit peculiar. It needs to hook to composite setup and
return it's function descriptor.

So when get-descriptor request comes with a type of DFU_DT_FUNC
we iterate over the configs, and functions, and when we find
the DFU function we call the setup method which is prepared
to return the appropriate function descriptor.

Signed-off-by: Pantelis Antoniou pa...@antoniou-consulting.com
---
 drivers/usb/gadget/composite.c | 27 +++
 drivers/usb/gadget/ep0.c   |  1 +
 drivers/usb/gadget/f_dfu.c |  6 --
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index ebb5131..6496436 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -773,6 +773,33 @@ composite_setup(struct usb_gadget *gadget, const struct 
usb_ctrlrequest *ctrl)
if (value = 0)
value = min(w_length, (u16) value);
break;
+
+#ifdef CONFIG_DFU_FUNCTION
+   /* DFU is mighty weird */
+   case DFU_DT_FUNC:
+   w_value = 0xff;
+   list_for_each_entry(c, cdev-configs, list) {
+   if (w_value != 0) {
+   w_value--;
+   continue;
+   }
+
+   list_for_each_entry(f, c-functions, list) {
+
+   /* DFU function only */
+   if (strcmp(f-name, dfu) != 0)
+   continue;
+
+   value = f-setup(f, ctrl);
+   goto dfu_func_done;
+   }
+   }
+dfu_func_done:
+   if (value = 0)
+   value = min(w_length, (u16) value);
+   break;
+#endif
+
default:
goto unknown;
}
diff --git a/drivers/usb/gadget/ep0.c b/drivers/usb/gadget/ep0.c
index aa8f916..971d846 100644
--- a/drivers/usb/gadget/ep0.c
+++ b/drivers/usb/gadget/ep0.c
@@ -221,6 +221,7 @@ static int ep0_get_descriptor (struct usb_device_instance 
*device,
break;
 
case USB_DESCRIPTOR_TYPE_CONFIGURATION:
+   case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION:
{
struct usb_configuration_descriptor
*configuration_descriptor;
diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 10547e3..6494f5e 100644
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -534,8 +534,10 @@ dfu_handle(struct usb_function *f, const struct 
usb_ctrlrequest *ctrl)
value = min(len, (u16) sizeof(dfu_func));
memcpy(req-buf, dfu_func, value);
}
-   } else /* DFU specific request */
-   value = dfu_state[f_dfu-dfu_state] (f_dfu, ctrl, gadget, req);
+   return value;
+   }
+
+   value = dfu_state[f_dfu-dfu_state] (f_dfu, ctrl, gadget, req);
 
if (value = 0) {
req-length = value;
-- 
1.7.12

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot