beaglebone JTAG (FT2232H)

2013-01-27 Thread Raphael Graf
The diff below makes the jtag and serial interfaces of the beaglebone (FT2232H)
work simultaneously.
This is how the beaglebone shows up:

uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev 2.00/b.b2 addr 
3
uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI BeagleBone/XDS100V2" 
rev 2.00/7.00 addr 4
ucom0 at uftdi0 portno 2
ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev 2.00/7.00 
addr 4

Like this, openocd (using ugen) and cu both work.

It is just assumed that interface 0 is used for jtag, not sure if this is
always true. I'm also unsure if the diff would break other devices.
Can someone comment on this?

Thanks


Index: uftdi.c
===
RCS file: /cvs/src/sys/dev/usb/uftdi.c,v
retrieving revision 1.63
diff -u -p -r1.63 uftdi.c
--- uftdi.c 11 Sep 2012 16:04:44 -  1.63
+++ uftdi.c 27 Jan 2013 13:20:53 -
@@ -748,28 +748,21 @@ int
 uftdi_match(struct device *parent, void *match, void *aux)
 {
struct usb_attach_arg *uaa = aux;
-   usbd_status err;
u_int8_t nifaces;

if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
return (UMATCH_NONE);

/* Get the number of interfaces. */
-   if (uaa->iface != NULL) {
+   if (uaa->iface != NULL)
nifaces = uaa->nifaces;
-   } else {
-   err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
-   if (err)
-   return (UMATCH_NONE);
-   err = usbd_interface_count(uaa->device, &nifaces);
-   if (err)
-   return (UMATCH_NONE);
-   usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
-   }
+   else
+   return UMATCH_NONE;

/* JTAG on USB interface 0 */
if (uaa->vendor == USB_VENDOR_FTDI &&
-   uaa->product == USB_PRODUCT_FTDI_OPENRD &&
+   (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
+   uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
uaa->ifaceno == 0)
return (UMATCH_NONE);





Re: beaglebone JTAG (FT2232H)

2013-01-28 Thread Stuart Henderson
Have you checked if this is still necessary? libusb on OpenBSD is now able to 
talk to devices claimed by drivers other than ugen.


Raphael Graf  wrote:

>The diff below makes the jtag and serial interfaces of the beaglebone
>(FT2232H)
>work simultaneously.
>This is how the beaglebone shows up:
>
>uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev
>2.00/b.b2 addr 3
>uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI
>BeagleBone/XDS100V2" rev 2.00/7.00 addr 4
>ucom0 at uftdi0 portno 2
>ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev
>2.00/7.00 addr 4
>
>Like this, openocd (using ugen) and cu both work.
>
>It is just assumed that interface 0 is used for jtag, not sure if this
>is
>always true. I'm also unsure if the diff would break other devices.
>Can someone comment on this?
>
>Thanks
>
>
>Index: uftdi.c
>===
>RCS file: /cvs/src/sys/dev/usb/uftdi.c,v
>retrieving revision 1.63
>diff -u -p -r1.63 uftdi.c
>--- uftdi.c11 Sep 2012 16:04:44 -  1.63
>+++ uftdi.c27 Jan 2013 13:20:53 -
>@@ -748,28 +748,21 @@ int
> uftdi_match(struct device *parent, void *match, void *aux)
> {
>   struct usb_attach_arg *uaa = aux;
>-  usbd_status err;
>   u_int8_t nifaces;
>
>   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
>   return (UMATCH_NONE);
>
>   /* Get the number of interfaces. */
>-  if (uaa->iface != NULL) {
>+  if (uaa->iface != NULL)
>   nifaces = uaa->nifaces;
>-  } else {
>-  err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
>-  if (err)
>-  return (UMATCH_NONE);
>-  err = usbd_interface_count(uaa->device, &nifaces);
>-  if (err)
>-  return (UMATCH_NONE);
>-  usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
>-  }
>+  else
>+  return UMATCH_NONE;
>
>   /* JTAG on USB interface 0 */
>   if (uaa->vendor == USB_VENDOR_FTDI &&
>-  uaa->product == USB_PRODUCT_FTDI_OPENRD &&
>+  (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
>+  uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
>   uaa->ifaceno == 0)
>   return (UMATCH_NONE);




Re: beaglebone JTAG (FT2232H)

2013-01-29 Thread Raphael Graf
On Mon, January 28, 2013 11:29 am, Stuart Henderson wrote:
> Have you checked if this is still necessary? libusb on OpenBSD is now able to 
> talk to devices claimed by drivers other than ugen.

Yes, I suspect it is still necessary to use ugen.
It seems like libusb does not support bulk transfers otherwise.


uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev 2.00/b.b2 addr 
3
uftdi0 at uhub8 port 1 configuration 1 interface 0 "FTDI BeagleBone/XDS100V2" 
rev 2.00/7.00 addr 4
ucom0 at uftdi0 portno 1
uftdi1 at uhub8 port 1 configuration 1 interface 1 "FTDI BeagleBone/XDS100V2" 
rev 2.00/7.00 addr 4
ucom1 at uftdi1 portno 2

$ openocd -f /usr/local/share/openocd/scripts/board/ti_beaglebone.cfg
Open On-Chip Debugger 0.6.1 (2013-01-29-22:25)
Licensed under GNU GPL v2
For bug reports, read
http://openocd.sourceforge.net/doc/doxygen/bugs.html
Info : only one transport option; autoselect 'jtag'
adapter speed: 16000 kHz
trst_and_srst separate srst_gates_jtag trst_push_pull srst_open_drain
Warn : unable to open ftdi device (trying more): device not found
Error: ftdi_write_data: usb bulk write failed
Error: couldn't initialize data bits low byte
Error: couldn't initialize FT2232 with 'xds100v2' layout
in procedure 'init'

>
>
> Raphael Graf  wrote:
>
>>The diff below makes the jtag and serial interfaces of the beaglebone
>>(FT2232H)
>>work simultaneously.
>>This is how the beaglebone shows up:
>>
>>uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev
>>2.00/b.b2 addr 3
>>uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI
>>BeagleBone/XDS100V2" rev 2.00/7.00 addr 4
>>ucom0 at uftdi0 portno 2
>>ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev
>>2.00/7.00 addr 4
>>
>>Like this, openocd (using ugen) and cu both work.
>>
>>It is just assumed that interface 0 is used for jtag, not sure if this
>>is
>>always true. I'm also unsure if the diff would break other devices.
>>Can someone comment on this?
>>
>>Thanks
>>
>>
>>Index: uftdi.c
>>===
>>RCS file: /cvs/src/sys/dev/usb/uftdi.c,v
>>retrieving revision 1.63
>>diff -u -p -r1.63 uftdi.c
>>--- uftdi.c   11 Sep 2012 16:04:44 -  1.63
>>+++ uftdi.c   27 Jan 2013 13:20:53 -
>>@@ -748,28 +748,21 @@ int
>> uftdi_match(struct device *parent, void *match, void *aux)
>> {
>>  struct usb_attach_arg *uaa = aux;
>>- usbd_status err;
>>  u_int8_t nifaces;
>>
>>  if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
>>  return (UMATCH_NONE);
>>
>>  /* Get the number of interfaces. */
>>- if (uaa->iface != NULL) {
>>+ if (uaa->iface != NULL)
>>  nifaces = uaa->nifaces;
>>- } else {
>>- err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
>>- if (err)
>>- return (UMATCH_NONE);
>>- err = usbd_interface_count(uaa->device, &nifaces);
>>- if (err)
>>- return (UMATCH_NONE);
>>- usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
>>- }
>>+ else
>>+ return UMATCH_NONE;
>>
>>  /* JTAG on USB interface 0 */
>>  if (uaa->vendor == USB_VENDOR_FTDI &&
>>- uaa->product == USB_PRODUCT_FTDI_OPENRD &&
>>+ (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
>>+ uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
>>  uaa->ifaceno == 0)
>>  return (UMATCH_NONE);
>
>



Re: beaglebone JTAG (FT2232H)

2013-01-30 Thread Martin Pieuchot
On 27/01/13(Sun) 16:13, Raphael Graf wrote:
> The diff below makes the jtag and serial interfaces of the beaglebone 
> (FT2232H)
> work simultaneously.
> This is how the beaglebone shows up:
> 
> uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev 2.00/b.b2 
> addr 3
> uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI BeagleBone/XDS100V2" 
> rev 2.00/7.00 addr 4
> ucom0 at uftdi0 portno 2
> ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev 
> 2.00/7.00 addr 4
> 
> Like this, openocd (using ugen) and cu both work.
> 
> It is just assumed that interface 0 is used for jtag, not sure if this is
> always true. I'm also unsure if the diff would break other devices.
> Can someone comment on this?

Your diff looks correct and after looking at how the USB drivers are
matched it seems obvious to me that changing the configuration in this
function is not the right thing to do. 

I just rewrote your diff to keep only one (iface == NULL) check, I think
it is clearer like that. Are you ok with this version?

Tested with:
uftdi0 at uhub3 port 1 configuration 1 interface 0 "FTDI USB Serial Converter" 
rev 2.00/6.00 addr 2

M.

Index: uftdi.c
===
RCS file: /home/ncvs/src/sys/dev/usb/uftdi.c,v
retrieving revision 1.63
diff -u -p -r1.63 uftdi.c
--- uftdi.c 11 Sep 2012 16:04:44 -  1.63
+++ uftdi.c 30 Jan 2013 09:28:59 -
@@ -748,39 +748,29 @@ int
 uftdi_match(struct device *parent, void *match, void *aux)
 {
struct usb_attach_arg *uaa = aux;
-   usbd_status err;
-   u_int8_t nifaces;
 
-   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
+   /*
+* Only attach when looping against interfaces to allow ugen(4)
+* to take over the interface 0, JTAG on USB on some models.
+*/
+   if (uaa->iface == NULL)
return (UMATCH_NONE);
 
-   /* Get the number of interfaces. */
-   if (uaa->iface != NULL) {
-   nifaces = uaa->nifaces;
-   } else {
-   err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
-   if (err)
-   return (UMATCH_NONE);
-   err = usbd_interface_count(uaa->device, &nifaces);
-   if (err)
-   return (UMATCH_NONE);
-   usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
-   }
+   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
+   return (UMATCH_NONE);
 
/* JTAG on USB interface 0 */
if (uaa->vendor == USB_VENDOR_FTDI &&
-   uaa->product == USB_PRODUCT_FTDI_OPENRD &&
+   (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
+   uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
uaa->ifaceno == 0)
return (UMATCH_NONE);
 
-   if (nifaces <= 1)
+   if (uaa->nifaces <= 1)
return (UMATCH_VENDOR_PRODUCT);
 
/* Dual UART chip */
-   if (uaa->iface != NULL)
-   return (UMATCH_VENDOR_IFACESUBCLASS);
-   else
-   return (UMATCH_NONE);
+   return (UMATCH_VENDOR_IFACESUBCLASS);
 }
 
 void



Re: beaglebone JTAG (FT2232H)

2013-01-31 Thread Martin Pieuchot
On 30/01/13(Wed) 13:19, Raphael Graf wrote:
> On Wed, January 30, 2013 10:52 am, Martin Pieuchot wrote:
> > On 27/01/13(Sun) 16:13, Raphael Graf wrote:
> >> The diff below makes the jtag and serial interfaces of the beaglebone 
> >> (FT2232H)
> >> work simultaneously.
> >> This is how the beaglebone shows up:
> >>
> >> uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev 2.00/b.b2 
> >> addr 3
> >> uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI 
> >> BeagleBone/XDS100V2" rev 2.00/7.00 addr 4
> >> ucom0 at uftdi0 portno 2
> >> ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev 
> >> 2.00/7.00 addr 4
> >>
> >> Like this, openocd (using ugen) and cu both work.
> >>
> >> It is just assumed that interface 0 is used for jtag, not sure if this is
> >> always true. I'm also unsure if the diff would break other devices.
> >> Can someone comment on this?
> >
> > Your diff looks correct and after looking at how the USB drivers are
> > matched it seems obvious to me that changing the configuration in this
> > function is not the right thing to do.
> >
> > I just rewrote your diff to keep only one (iface == NULL) check, I think
> > it is clearer like that. Are you ok with this version?
> 
> Yes, thanks.
> Could you take a quick look at the uftdi_attach function?
> I think some 'if (uaa->iface == NULL)' code could be removed there.

Right, here's an updated diff, is it still ok for you?

M.

Index: uftdi.c
===
RCS file: /home/ncvs/src/sys/dev/usb/uftdi.c,v
retrieving revision 1.63
diff -u -p -r1.63 uftdi.c
--- uftdi.c 11 Sep 2012 16:04:44 -  1.63
+++ uftdi.c 31 Jan 2013 08:59:50 -
@@ -748,39 +748,29 @@ int
 uftdi_match(struct device *parent, void *match, void *aux)
 {
struct usb_attach_arg *uaa = aux;
-   usbd_status err;
-   u_int8_t nifaces;
 
-   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
+   /*
+* Only match when looping against interfaces to allow ugen(4)
+* to take over the interface 0, JTAG on USB, on some models.
+*/
+   if (uaa->iface == NULL)
return (UMATCH_NONE);
 
-   /* Get the number of interfaces. */
-   if (uaa->iface != NULL) {
-   nifaces = uaa->nifaces;
-   } else {
-   err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
-   if (err)
-   return (UMATCH_NONE);
-   err = usbd_interface_count(uaa->device, &nifaces);
-   if (err)
-   return (UMATCH_NONE);
-   usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
-   }
+   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
+   return (UMATCH_NONE);
 
/* JTAG on USB interface 0 */
if (uaa->vendor == USB_VENDOR_FTDI &&
-   uaa->product == USB_PRODUCT_FTDI_OPENRD &&
+   (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
+   uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
uaa->ifaceno == 0)
return (UMATCH_NONE);
 
-   if (nifaces <= 1)
+   if (uaa->nifaces <= 1)
return (UMATCH_VENDOR_PRODUCT);
 
/* Dual UART chip */
-   if (uaa->iface != NULL)
-   return (UMATCH_VENDOR_IFACESUBCLASS);
-   else
-   return (UMATCH_NONE);
+   return (UMATCH_VENDOR_IFACESUBCLASS);
 }
 
 void
@@ -789,34 +779,15 @@ uftdi_attach(struct device *parent, stru
struct uftdi_softc *sc = (struct uftdi_softc *)self;
struct usb_attach_arg *uaa = aux;
usbd_device_handle dev = uaa->device;
-   usbd_interface_handle iface;
+   usbd_interface_handle iface = uaa->iface;
usb_interface_descriptor_t *id;
usb_endpoint_descriptor_t *ed;
char *devname = sc->sc_dev.dv_xname;
int i;
-   usbd_status err;
struct ucom_attach_args uca;
 
DPRINTFN(10,("\nuftdi_attach: sc=%p\n", sc));
 
-   if (uaa->iface == NULL) {
-   /* Move the device into the configured state. */
-   err = usbd_set_config_index(dev, UFTDI_CONFIG_INDEX, 1);
-   if (err) {
-   printf("%s: failed to set configuration, err=%s\n",
-   sc->sc_dev.dv_xname, usbd_errstr(err));
-   goto bad;
-   }
-
-   err = usbd_device2interface_handle(dev, UFTDI_IFACE_INDEX, 
&iface);
-   if (err) {
-   printf("%s: failed to get interface, err=%s\n",
-   sc->sc_dev.dv_xname, usbd_errstr(err));
-   goto bad;
-   }
-   } else
-   iface = uaa->iface;
-
id = usbd_get_interface_descriptor(iface);
 
sc->sc_udev = dev;
@@ -871,10 +842,7 @@ uftdi_attach(struct device *parent, stru
goto bad;
}
 
-   if (uaa->if

Re: beaglebone JTAG (FT2232H)

2013-01-31 Thread Raphael Graf
On Thu, January 31, 2013 10:31 am, Martin Pieuchot wrote:
> On 30/01/13(Wed) 13:19, Raphael Graf wrote:
>> On Wed, January 30, 2013 10:52 am, Martin Pieuchot wrote:
>> > On 27/01/13(Sun) 16:13, Raphael Graf wrote:
>> >> The diff below makes the jtag and serial interfaces of the beaglebone 
>> >> (FT2232H)
>> >> work simultaneously.
>> >> This is how the beaglebone shows up:
>> >>
>> >> uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev 
>> >> 2.00/b.b2 addr 3
>> >> uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI 
>> >> BeagleBone/XDS100V2" rev 2.00/7.00 addr 4
>> >> ucom0 at uftdi0 portno 2
>> >> ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev 
>> >> 2.00/7.00 addr 4
>> >>
>> >> Like this, openocd (using ugen) and cu both work.
>> >>
>> >> It is just assumed that interface 0 is used for jtag, not sure if this is
>> >> always true. I'm also unsure if the diff would break other devices.
>> >> Can someone comment on this?
>> >
>> > Your diff looks correct and after looking at how the USB drivers are
>> > matched it seems obvious to me that changing the configuration in this
>> > function is not the right thing to do.
>> >
>> > I just rewrote your diff to keep only one (iface == NULL) check, I think
>> > it is clearer like that. Are you ok with this version?
>>
>> Yes, thanks.
>> Could you take a quick look at the uftdi_attach function?
>> I think some 'if (uaa->iface == NULL)' code could be removed there.
>
> Right, here's an updated diff, is it still ok for you?

Yes, i've tested it again with the beaglebone.

Thanks!

r

>
> M.
>
> Index: uftdi.c
> ===
> RCS file: /home/ncvs/src/sys/dev/usb/uftdi.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 uftdi.c
> --- uftdi.c   11 Sep 2012 16:04:44 -  1.63
> +++ uftdi.c   31 Jan 2013 08:59:50 -
> @@ -748,39 +748,29 @@ int
>  uftdi_match(struct device *parent, void *match, void *aux)
>  {
>   struct usb_attach_arg *uaa = aux;
> - usbd_status err;
> - u_int8_t nifaces;
>
> - if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
> + /*
> +  * Only match when looping against interfaces to allow ugen(4)
> +  * to take over the interface 0, JTAG on USB, on some models.
> +  */
> + if (uaa->iface == NULL)
>   return (UMATCH_NONE);
>
> - /* Get the number of interfaces. */
> - if (uaa->iface != NULL) {
> - nifaces = uaa->nifaces;
> - } else {
> - err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
> - if (err)
> - return (UMATCH_NONE);
> - err = usbd_interface_count(uaa->device, &nifaces);
> - if (err)
> - return (UMATCH_NONE);
> - usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
> - }
> + if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
> + return (UMATCH_NONE);
>
>   /* JTAG on USB interface 0 */
>   if (uaa->vendor == USB_VENDOR_FTDI &&
> - uaa->product == USB_PRODUCT_FTDI_OPENRD &&
> + (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
> + uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
>   uaa->ifaceno == 0)
>   return (UMATCH_NONE);
>
> - if (nifaces <= 1)
> + if (uaa->nifaces <= 1)
>   return (UMATCH_VENDOR_PRODUCT);
>
>   /* Dual UART chip */
> - if (uaa->iface != NULL)
> - return (UMATCH_VENDOR_IFACESUBCLASS);
> - else
> - return (UMATCH_NONE);
> + return (UMATCH_VENDOR_IFACESUBCLASS);
>  }
>
>  void
> @@ -789,34 +779,15 @@ uftdi_attach(struct device *parent, stru
>   struct uftdi_softc *sc = (struct uftdi_softc *)self;
>   struct usb_attach_arg *uaa = aux;
>   usbd_device_handle dev = uaa->device;
> - usbd_interface_handle iface;
> + usbd_interface_handle iface = uaa->iface;
>   usb_interface_descriptor_t *id;
>   usb_endpoint_descriptor_t *ed;
>   char *devname = sc->sc_dev.dv_xname;
>   int i;
> - usbd_status err;
>   struct ucom_attach_args uca;
>
>   DPRINTFN(10,("\nuftdi_attach: sc=%p\n", sc));
>
> - if (uaa->iface == NULL) {
> - /* Move the device into the configured state. */
> - err = usbd_set_config_index(dev, UFTDI_CONFIG_INDEX, 1);
> - if (err) {
> - printf("%s: failed to set configuration, err=%s\n",
> - sc->sc_dev.dv_xname, usbd_errstr(err));
> - goto bad;
> - }
> -
> - err = usbd_device2interface_handle(dev, UFTDI_IFACE_INDEX, 
> &iface);
> - if (err) {
> - printf("%s: failed to get interface, err=%s\n",
> - sc->sc_dev.dv_xname, usbd_errstr(err));
> - goto bad;
> - }
> - } else
> - iface = uaa->iface;
> -
>   id = 

Re: beaglebone JTAG (FT2232H)

2013-02-01 Thread Martin Pieuchot
On 31/01/13(Thu) 11:21, Raphael Graf wrote:
> On Thu, January 31, 2013 10:31 am, Martin Pieuchot wrote:
> > On 30/01/13(Wed) 13:19, Raphael Graf wrote:
> >> On Wed, January 30, 2013 10:52 am, Martin Pieuchot wrote:
> >> > On 27/01/13(Sun) 16:13, Raphael Graf wrote:
> >> >> The diff below makes the jtag and serial interfaces of the beaglebone 
> >> >> (FT2232H)
> >> >> work simultaneously.
> >> >> This is how the beaglebone shows up:
> >> >>
> >> >> uhub8 at uhub0 port 1 "Standard Microsystems product 0x2412" rev 
> >> >> 2.00/b.b2 addr 3
> >> >> uftdi0 at uhub8 port 1 configuration 1 interface 1 "FTDI 
> >> >> BeagleBone/XDS100V2" rev 2.00/7.00 addr 4
> >> >> ucom0 at uftdi0 portno 2
> >> >> ugen0 at uhub8 port 1 configuration 1 "FTDI BeagleBone/XDS100V2" rev 
> >> >> 2.00/7.00 addr 4
> >> >>
> >> >> Like this, openocd (using ugen) and cu both work.
> >> >>
> >> >> It is just assumed that interface 0 is used for jtag, not sure if this 
> >> >> is
> >> >> always true. I'm also unsure if the diff would break other devices.
> >> >> Can someone comment on this?
> >> >
> >> > Your diff looks correct and after looking at how the USB drivers are
> >> > matched it seems obvious to me that changing the configuration in this
> >> > function is not the right thing to do.
> >> >
> >> > I just rewrote your diff to keep only one (iface == NULL) check, I think
> >> > it is clearer like that. Are you ok with this version?
> >>
> >> Yes, thanks.
> >> Could you take a quick look at the uftdi_attach function?
> >> I think some 'if (uaa->iface == NULL)' code could be removed there.
> >
> > Right, here's an updated diff, is it still ok for you?
> 
> Yes, i've tested it again with the beaglebone.

Unfortunately I can't commit this diff as it is, as it may break other
dual UART 2232C chip that may not be used for JTAG. But trying to change
the configuration in the match() methods looks definitively wrong to me.

Anyway this is yet another case showing that our generic USB support
needs definitively more love.

> > Index: uftdi.c
> > ===
> > RCS file: /home/ncvs/src/sys/dev/usb/uftdi.c,v
> > retrieving revision 1.63
> > diff -u -p -r1.63 uftdi.c
> > --- uftdi.c 11 Sep 2012 16:04:44 -  1.63
> > +++ uftdi.c 31 Jan 2013 08:59:50 -
> > @@ -748,39 +748,29 @@ int
> >  uftdi_match(struct device *parent, void *match, void *aux)
> >  {
> > struct usb_attach_arg *uaa = aux;
> > -   usbd_status err;
> > -   u_int8_t nifaces;
> >
> > -   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
> > +   /*
> > +* Only match when looping against interfaces to allow ugen(4)
> > +* to take over the interface 0, JTAG on USB, on some models.
> > +*/
> > +   if (uaa->iface == NULL)
> > return (UMATCH_NONE);
> >
> > -   /* Get the number of interfaces. */
> > -   if (uaa->iface != NULL) {
> > -   nifaces = uaa->nifaces;
> > -   } else {
> > -   err = usbd_set_config_index(uaa->device, UFTDI_CONFIG_INDEX, 1);
> > -   if (err)
> > -   return (UMATCH_NONE);
> > -   err = usbd_interface_count(uaa->device, &nifaces);
> > -   if (err)
> > -   return (UMATCH_NONE);
> > -   usbd_set_config_index(uaa->device, USB_UNCONFIG_INDEX, 1);
> > -   }
> > +   if (usb_lookup(uftdi_devs, uaa->vendor, uaa->product) == NULL)
> > +   return (UMATCH_NONE);
> >
> > /* JTAG on USB interface 0 */
> > if (uaa->vendor == USB_VENDOR_FTDI &&
> > -   uaa->product == USB_PRODUCT_FTDI_OPENRD &&
> > +   (uaa->product == USB_PRODUCT_FTDI_OPENRD ||
> > +   uaa->product == USB_PRODUCT_FTDI_SERIAL_2232C) &&
> > uaa->ifaceno == 0)
> > return (UMATCH_NONE);
> >
> > -   if (nifaces <= 1)
> > +   if (uaa->nifaces <= 1)
> > return (UMATCH_VENDOR_PRODUCT);
> >
> > /* Dual UART chip */
> > -   if (uaa->iface != NULL)
> > -   return (UMATCH_VENDOR_IFACESUBCLASS);
> > -   else
> > -   return (UMATCH_NONE);
> > +   return (UMATCH_VENDOR_IFACESUBCLASS);
> >  }
> >
> >  void
> > @@ -789,34 +779,15 @@ uftdi_attach(struct device *parent, stru
> > struct uftdi_softc *sc = (struct uftdi_softc *)self;
> > struct usb_attach_arg *uaa = aux;
> > usbd_device_handle dev = uaa->device;
> > -   usbd_interface_handle iface;
> > +   usbd_interface_handle iface = uaa->iface;
> > usb_interface_descriptor_t *id;
> > usb_endpoint_descriptor_t *ed;
> > char *devname = sc->sc_dev.dv_xname;
> > int i;
> > -   usbd_status err;
> > struct ucom_attach_args uca;
> >
> > DPRINTFN(10,("\nuftdi_attach: sc=%p\n", sc));
> >
> > -   if (uaa->iface == NULL) {
> > -   /* Move the device into the configured state. */
> > -   err = usbd_set_config_index(dev, UFTDI_CONFIG_INDEX, 1);
> > -   if (err) {
> > -   printf("%s: failed to set configuration, err=%s\n",
> > -   sc->sc_d