[U-Boot] [PATCH] usbtty/omap: update to current API

[U-Boot] [PATCH] usbtty/omap: update to current API

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
index e738c56..448defa 100644
--- a/drivers/serial/usbtty.c
+++ b/drivers/serial/usbtty.c
@@ -22,14 +22,13 @@
  */
 
 #include <common.h>
-
+#include <config.h>		/* If defined, override Linux identifiers with
+				 * vendor specific ones */
 #include <circbuf.h>
 #include <devices.h>
 #include "usbtty.h"
 #include "usb_cdc_acm.h"
 #include "usbdescriptors.h"
-#include <config.h>		/* If defined, override Linux identifiers with
-				 * vendor specific ones */
 
 #if 0
 #define TTYDBG(fmt,args...)\
diff --git a/drivers/serial/usbtty.h b/drivers/serial/usbtty.h
index 71c47bc..2175d68 100644
--- a/drivers/serial/usbtty.h
+++ b/drivers/serial/usbtty.h
@@ -24,11 +24,11 @@
 #ifndef __USB_TTY_H__
 #define __USB_TTY_H__
 
-#include "usbdcore.h"
+#include <usbdcore.h>
 #if defined(CONFIG_PPC)
-#include "usbdcore_mpc8xx.h"
+#include <usbdcore_mpc8xx.h>
 #elif defined(CONFIG_ARM)
-#include "usbdcore_omap1510.h"
+#include <usbdcore_omap1510.h>
 #endif
 
 #include <version_autogenerated.h>
@@ -36,14 +36,25 @@
 /* If no VendorID/ProductID is defined in config.h, pretend to be Linux
  * DO NOT Reuse this Vendor/Product setup with protocol incompatible devices */
 
+#ifndef CONFIG_USBD_VENDORID
 #define CONFIG_USBD_VENDORID 0x0525	/* Linux/NetChip */
+#endif
+#ifndef CONFIG_USBD_PRODUCTID_GSERIAL
 #define CONFIG_USBD_PRODUCTID_GSERIAL 0xa4a6	/* gserial */
+#endif
+#ifndef CONFIG_USBD_PRODUCTID_CDCACM
 #define CONFIG_USBD_PRODUCTID_CDCACM  0xa4a7	/* CDC ACM */
+#endif
+#ifndef CONFIG_USBD_MANUFACTURER
 #define CONFIG_USBD_MANUFACTURER "Das U-Boot"
+#endif
+#ifndef CONFIG_USBD_PRODUCT_NAME
 #define CONFIG_USBD_PRODUCT_NAME U_BOOT_VERSION
+#endif
 
-
+#ifndef CONFIG_USBD_CONFIGURATION_STR
 #define CONFIG_USBD_CONFIGURATION_STR "TTY via USB"
+#endif
 
 #define CONFIG_USBD_SERIAL_OUT_ENDPOINT UDC_OUT_ENDPOINT
 #define CONFIG_USBD_SERIAL_OUT_PKTSIZE	UDC_OUT_PACKET_SIZE
diff --git a/drivers/usb/usbdcore_omap1510.c b/drivers/usb/usbdcore_omap1510.c
index 4e3239f..fb8cb5d 100644
--- a/drivers/usb/usbdcore_omap1510.c
+++ b/drivers/usb/usbdcore_omap1510.c
@@ -1064,7 +1064,7 @@ void omap1510_udc_noniso_irq (void)
  */
 
 /* Called to start packet transmission. */
-void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
+int udc_endpoint_write (struct usb_endpoint_instance *endpoint)
 {
 	unsigned short epnum =
 		endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
@@ -1081,6 +1081,8 @@ void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
 		/* deselect the endpoint FIFO */
 		outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
 	}
+
+	return 0;
 }
 
 /* Start to initialize h/w stuff */
diff --git a/include/usbdcore_omap1510.h b/include/usbdcore_omap1510.h
index 526fcd9..ece0e95 100644
--- a/include/usbdcore_omap1510.h
+++ b/include/usbdcore_omap1510.h
@@ -168,8 +168,8 @@
 #define UDC_IN_ENDPOINT	1
 #define UDC_IN_PACKET_SIZE 64
 #define UDC_INT_ENDPOINT 5
-#define UDC_INT_PKTSIZE	16
-#define UDC_BULK_PKTSIZE 16
+#define UDC_INT_PACKET_SIZE	16
+#define UDC_BULK_PACKET_SIZE 16
 
 void udc_irq (void);
 /* Flow control */
@@ -177,7 +177,7 @@ void udc_set_nak(int epid);
 void udc_unset_nak (int epid);
 
 /* Higher level functions for abstracting away from specific device */
-void udc_endpoint_write(struct usb_endpoint_instance *endpoint);
+int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
 
 int  udc_init (void);
 
-- 
1.5.6.5

[U-Boot] [PATCH] usbtty/omap: update to current API

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1226254923-8378-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Please explain what exactly you are trying to change / fix here.

> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> index e738c56..448defa 100644
> --- a/drivers/serial/usbtty.c
> +++ b/drivers/serial/usbtty.c
> @@ -22,14 +22,13 @@
>   */
>  
>  #include <common.h>
> -
> +#include <config.h>		/* If defined, override Linux identifiers with
> +				 * vendor specific ones */

"If defined" - what needs to be defined to have any impact here?

And why do we care about Linux identifiers here? That's U-Boot, not
Linux.

> +#ifndef CONFIG_USBD_VENDORID
>  #define CONFIG_USBD_VENDORID 0x0525	/* Linux/NetChip */
> +#endif
> +#ifndef CONFIG_USBD_PRODUCTID_GSERIAL
>  #define CONFIG_USBD_PRODUCTID_GSERIAL 0xa4a6	/* gserial */
> +#endif
> +#ifndef CONFIG_USBD_PRODUCTID_CDCACM
>  #define CONFIG_USBD_PRODUCTID_CDCACM  0xa4a7	/* CDC ACM */
> +#endif
> +#ifndef CONFIG_USBD_MANUFACTURER
>  #define CONFIG_USBD_MANUFACTURER "Das U-Boot"
> +#endif
> +#ifndef CONFIG_USBD_PRODUCT_NAME
>  #define CONFIG_USBD_PRODUCT_NAME U_BOOT_VERSION
> +#endif
>  
> -
> +#ifndef CONFIG_USBD_CONFIGURATION_STR
>  #define CONFIG_USBD_CONFIGURATION_STR "TTY via USB"
> +#endif

All these new config options need to be documented in the README.

>  /* Called to start packet transmission. */
> -void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> +int udc_endpoint_write (struct usb_endpoint_instance *endpoint)
>  {
>  	unsigned short epnum =
>  		endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
> @@ -1081,6 +1081,8 @@ void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
>  		/* deselect the endpoint FIFO */
>  		outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
>  	}
> +
> +	return 0;
>  }

If the only way to exit the function is by returning 0  at  the  end,
then  we should rather leave this as is and have this function return
void - otherwise we suggest that different values could  be  returned
which is not the case.



Best regards,

Wolfgang Denk

-- 
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: wd at denx.de
It is practically impossible to teach good programming style to  stu-
dents that have had prior exposure to BASIC: as potential programmers
they are mentally mutilated beyond hope of regeneration.   - Dijkstra

[U-Boot] [PATCH] usbtty/omap: update to current API

[Jean-Christophe PLAGNIOL-VILLARD]

On 20:59 Sun 09 Nov     , Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <1226254923-8378-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> Please explain what exactly you are trying to change / fix here.
> 
> > diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> > index e738c56..448defa 100644
> > --- a/drivers/serial/usbtty.c
> > +++ b/drivers/serial/usbtty.c
> > @@ -22,14 +22,13 @@
> >   */
> >  
> >  #include <common.h>
> > -
> > +#include <config.h>		/* If defined, override Linux identifiers with
> > +				 * vendor specific ones */
> 
> "If defined" - what needs to be defined to have any impact here?
> 
> And why do we care about Linux identifiers here? That's U-Boot, not
> Linux.
Yes I may remove the comment because I've allow to not overwrite them
> 
> > +#ifndef CONFIG_USBD_VENDORID
> >  #define CONFIG_USBD_VENDORID 0x0525	/* Linux/NetChip */
> > +#endif
> > +#ifndef CONFIG_USBD_PRODUCTID_GSERIAL
> >  #define CONFIG_USBD_PRODUCTID_GSERIAL 0xa4a6	/* gserial */
> > +#endif
> > +#ifndef CONFIG_USBD_PRODUCTID_CDCACM
> >  #define CONFIG_USBD_PRODUCTID_CDCACM  0xa4a7	/* CDC ACM */
> > +#endif
> > +#ifndef CONFIG_USBD_MANUFACTURER
> >  #define CONFIG_USBD_MANUFACTURER "Das U-Boot"
> > +#endif
> > +#ifndef CONFIG_USBD_PRODUCT_NAME
> >  #define CONFIG_USBD_PRODUCT_NAME U_BOOT_VERSION
> > +#endif
> >  
> > -
> > +#ifndef CONFIG_USBD_CONFIGURATION_STR
> >  #define CONFIG_USBD_CONFIGURATION_STR "TTY via USB"
> > +#endif
> 
> All these new config options need to be documented in the README.
> 

this not new config option this is present option which is supposed to be
define in the config file or use this defaul one.

I'll take a look on the README anyway and add it if nor present


> >  /* Called to start packet transmission. */
> > -void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> > +int udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> >  {
> >  	unsigned short epnum =
> >  		endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
> > @@ -1081,6 +1081,8 @@ void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> >  		/* deselect the endpoint FIFO */
> >  		outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
> >  	}
> > +
> > +	return 0;
> >  }
> 
> If the only way to exit the function is by returning 0  at  the  end,
> then  we should rather leave this as is and have this function return
> void - otherwise we suggest that different values could  be  returned
> which is not the case.

on mpc8xx they do return -1 if failed

and they share to same api

Best Regards,
J.

[U-Boot] [PATCH] usbtty/omap: update to current API

[Remy Bohmer]

Hello Jean-Christophe,

>> In message <1226254923-8378-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
>> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>
>> > diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c

What are we going to do with this patch?

Kind Regards,

Remy

[U-Boot] [PATCH] usbtty/omap: update to current API

[Jean-Christophe PLAGNIOL-VILLARD]

On 22:00 Wed 26 Nov     , Remy Bohmer wrote:
> Hello Jean-Christophe,
> 
> >> In message <1226254923-8378-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> >> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >>
> >> > diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> 
> What are we going to do with this patch?
I'll work on it next week

Best Regards,
J.

[U-Boot] [PATCH] usbtty/omap: update to current API

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20081127124413.GD3644@game.jcrosoft.org> you wrote:
> On 22:00 Wed 26 Nov     , Remy Bohmer wrote:
> > Hello Jean-Christophe,
> > 
> > >> In message <1226254923-8378-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> > >> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > >>
> > >> > diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> > 
> > What are we going to do with this patch?
> I'll work on it next week

Next week was a long time ago.

What happened to that patch?

Best regards,

Wolfgang Denk

-- 
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: wd at denx.de
Looks clean and obviously correct to me, but then _everything_ I
write always looks obviously correct to me.  - Linus Torvalds in
<Pine.LNX.4.10.10012090054360.791-100000@penguin.transmeta.com>

[U-Boot] [PATCH V2] usbtty/omap: update to current API

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
the doc about the CONFIG_USBD is already in the README

Best Regards,
J.
 drivers/serial/usbtty.c         |    6 ++----
 drivers/serial/usbtty.h         |   31 +++++++++++++++++++++----------
 drivers/usb/usbdcore_omap1510.c |    4 +++-
 include/usbdcore_omap1510.h     |    6 +++---
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
index e738c56..7eba470 100644
--- a/drivers/serial/usbtty.c
+++ b/drivers/serial/usbtty.c
@@ -22,16 +22,14 @@
  */
 
 #include <common.h>
-
+#include <config.h>
 #include <circbuf.h>
 #include <devices.h>
 #include "usbtty.h"
 #include "usb_cdc_acm.h"
 #include "usbdescriptors.h"
-#include <config.h>		/* If defined, override Linux identifiers with
-				 * vendor specific ones */
 
-#if 0
+#ifdef DEBUG
 #define TTYDBG(fmt,args...)\
 	serial_printf("[%s] %s %d: "fmt, __FILE__,__FUNCTION__,__LINE__,##args)
 #else
diff --git a/drivers/serial/usbtty.h b/drivers/serial/usbtty.h
index 71c47bc..ecefde5 100644
--- a/drivers/serial/usbtty.h
+++ b/drivers/serial/usbtty.h
@@ -24,11 +24,11 @@
 #ifndef __USB_TTY_H__
 #define __USB_TTY_H__
 
-#include "usbdcore.h"
+#include <usbdcore.h>
 #if defined(CONFIG_PPC)
-#include "usbdcore_mpc8xx.h"
+#include <usbdcore_mpc8xx.h>
 #elif defined(CONFIG_ARM)
-#include "usbdcore_omap1510.h"
+#include <usbdcore_omap1510.h>
 #endif
 
 #include <version_autogenerated.h>
@@ -36,14 +36,25 @@
 /* If no VendorID/ProductID is defined in config.h, pretend to be Linux
  * DO NOT Reuse this Vendor/Product setup with protocol incompatible devices */
 
-#define CONFIG_USBD_VENDORID 0x0525	/* Linux/NetChip */
-#define CONFIG_USBD_PRODUCTID_GSERIAL 0xa4a6	/* gserial */
-#define CONFIG_USBD_PRODUCTID_CDCACM  0xa4a7	/* CDC ACM */
-#define CONFIG_USBD_MANUFACTURER "Das U-Boot"
-#define CONFIG_USBD_PRODUCT_NAME U_BOOT_VERSION
-
+#ifndef CONFIG_USBD_VENDORID
+#define CONFIG_USBD_VENDORID		0x0525	/* Linux/NetChip */
+#endif
+#ifndef CONFIG_USBD_PRODUCTID_GSERIAL
+#define CONFIG_USBD_PRODUCTID_GSERIAL	0xa4a6	/* gserial */
+#endif
+#ifndef CONFIG_USBD_PRODUCTID_CDCACM
+#define CONFIG_USBD_PRODUCTID_CDCACM	0xa4a7	/* CDC ACM */
+#endif
+#ifndef CONFIG_USBD_MANUFACTURER
+#define CONFIG_USBD_MANUFACTURER	"Das U-Boot"
+#endif
+#ifndef CONFIG_USBD_PRODUCT_NAME
+#define CONFIG_USBD_PRODUCT_NAME	U_BOOT_VERSION
+#endif
 
-#define CONFIG_USBD_CONFIGURATION_STR "TTY via USB"
+#ifndef CONFIG_USBD_CONFIGURATION_STR
+#define CONFIG_USBD_CONFIGURATION_STR	"TTY via USB"
+#endif
 
 #define CONFIG_USBD_SERIAL_OUT_ENDPOINT UDC_OUT_ENDPOINT
 #define CONFIG_USBD_SERIAL_OUT_PKTSIZE	UDC_OUT_PACKET_SIZE
diff --git a/drivers/usb/usbdcore_omap1510.c b/drivers/usb/usbdcore_omap1510.c
index cb9dc44..6b7b61b 100644
--- a/drivers/usb/usbdcore_omap1510.c
+++ b/drivers/usb/usbdcore_omap1510.c
@@ -1061,7 +1061,7 @@ void omap1510_udc_noniso_irq (void)
  */
 
 /* Called to start packet transmission. */
-void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
+int udc_endpoint_write (struct usb_endpoint_instance *endpoint)
 {
 	unsigned short epnum =
 		endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
@@ -1078,6 +1078,8 @@ void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
 		/* deselect the endpoint FIFO */
 		outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
 	}
+
+	return 0;
 }
 
 /* Start to initialize h/w stuff */
diff --git a/include/usbdcore_omap1510.h b/include/usbdcore_omap1510.h
index 526fcd9..ece0e95 100644
--- a/include/usbdcore_omap1510.h
+++ b/include/usbdcore_omap1510.h
@@ -168,8 +168,8 @@
 #define UDC_IN_ENDPOINT	1
 #define UDC_IN_PACKET_SIZE 64
 #define UDC_INT_ENDPOINT 5
-#define UDC_INT_PKTSIZE	16
-#define UDC_BULK_PKTSIZE 16
+#define UDC_INT_PACKET_SIZE	16
+#define UDC_BULK_PACKET_SIZE 16
 
 void udc_irq (void);
 /* Flow control */
@@ -177,7 +177,7 @@ void udc_set_nak(int epid);
 void udc_unset_nak (int epid);
 
 /* Higher level functions for abstracting away from specific device */
-void udc_endpoint_write(struct usb_endpoint_instance *endpoint);
+int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
 
 int  udc_init (void);
 
-- 
1.5.6.5

[U-Boot] [PATCH V2] usbtty/omap: update to current API

[Remy Bohmer]

Hello,

2008/12/7 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> the doc about the CONFIG_USBD is already in the README
>
> Best Regards,
> J.

Applied to U-boot-usb tree ('next' branch)

Thanks,

Remy

>  drivers/serial/usbtty.c         |    6 ++----
>  drivers/serial/usbtty.h         |   31 +++++++++++++++++++++----------
>  drivers/usb/usbdcore_omap1510.c |    4 +++-
>  include/usbdcore_omap1510.h     |    6 +++---
>  4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> index e738c56..7eba470 100644
> --- a/drivers/serial/usbtty.c
> +++ b/drivers/serial/usbtty.c
> @@ -22,16 +22,14 @@
>  */
>
>  #include <common.h>
> -
> +#include <config.h>
>  #include <circbuf.h>
>  #include <devices.h>
>  #include "usbtty.h"
>  #include "usb_cdc_acm.h"
>  #include "usbdescriptors.h"
> -#include <config.h>            /* If defined, override Linux identifiers with
> -                                * vendor specific ones */
>
> -#if 0
> +#ifdef DEBUG
>  #define TTYDBG(fmt,args...)\
>        serial_printf("[%s] %s %d: "fmt, __FILE__,__FUNCTION__,__LINE__,##args)
>  #else
> diff --git a/drivers/serial/usbtty.h b/drivers/serial/usbtty.h
> index 71c47bc..ecefde5 100644
> --- a/drivers/serial/usbtty.h
> +++ b/drivers/serial/usbtty.h
> @@ -24,11 +24,11 @@
>  #ifndef __USB_TTY_H__
>  #define __USB_TTY_H__
>
> -#include "usbdcore.h"
> +#include <usbdcore.h>
>  #if defined(CONFIG_PPC)
> -#include "usbdcore_mpc8xx.h"
> +#include <usbdcore_mpc8xx.h>
>  #elif defined(CONFIG_ARM)
> -#include "usbdcore_omap1510.h"
> +#include <usbdcore_omap1510.h>
>  #endif
>
>  #include <version_autogenerated.h>
> @@ -36,14 +36,25 @@
>  /* If no VendorID/ProductID is defined in config.h, pretend to be Linux
>  * DO NOT Reuse this Vendor/Product setup with protocol incompatible devices */
>
> -#define CONFIG_USBD_VENDORID 0x0525    /* Linux/NetChip */
> -#define CONFIG_USBD_PRODUCTID_GSERIAL 0xa4a6   /* gserial */
> -#define CONFIG_USBD_PRODUCTID_CDCACM  0xa4a7   /* CDC ACM */
> -#define CONFIG_USBD_MANUFACTURER "Das U-Boot"
> -#define CONFIG_USBD_PRODUCT_NAME U_BOOT_VERSION
> -
> +#ifndef CONFIG_USBD_VENDORID
> +#define CONFIG_USBD_VENDORID           0x0525  /* Linux/NetChip */
> +#endif
> +#ifndef CONFIG_USBD_PRODUCTID_GSERIAL
> +#define CONFIG_USBD_PRODUCTID_GSERIAL  0xa4a6  /* gserial */
> +#endif
> +#ifndef CONFIG_USBD_PRODUCTID_CDCACM
> +#define CONFIG_USBD_PRODUCTID_CDCACM   0xa4a7  /* CDC ACM */
> +#endif
> +#ifndef CONFIG_USBD_MANUFACTURER
> +#define CONFIG_USBD_MANUFACTURER       "Das U-Boot"
> +#endif
> +#ifndef CONFIG_USBD_PRODUCT_NAME
> +#define CONFIG_USBD_PRODUCT_NAME       U_BOOT_VERSION
> +#endif
>
> -#define CONFIG_USBD_CONFIGURATION_STR "TTY via USB"
> +#ifndef CONFIG_USBD_CONFIGURATION_STR
> +#define CONFIG_USBD_CONFIGURATION_STR  "TTY via USB"
> +#endif
>
>  #define CONFIG_USBD_SERIAL_OUT_ENDPOINT UDC_OUT_ENDPOINT
>  #define CONFIG_USBD_SERIAL_OUT_PKTSIZE UDC_OUT_PACKET_SIZE
> diff --git a/drivers/usb/usbdcore_omap1510.c b/drivers/usb/usbdcore_omap1510.c
> index cb9dc44..6b7b61b 100644
> --- a/drivers/usb/usbdcore_omap1510.c
> +++ b/drivers/usb/usbdcore_omap1510.c
> @@ -1061,7 +1061,7 @@ void omap1510_udc_noniso_irq (void)
>  */
>
>  /* Called to start packet transmission. */
> -void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> +int udc_endpoint_write (struct usb_endpoint_instance *endpoint)
>  {
>        unsigned short epnum =
>                endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
> @@ -1078,6 +1078,8 @@ void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
>                /* deselect the endpoint FIFO */
>                outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
>        }
> +
> +       return 0;
>  }
>
>  /* Start to initialize h/w stuff */
> diff --git a/include/usbdcore_omap1510.h b/include/usbdcore_omap1510.h
> index 526fcd9..ece0e95 100644
> --- a/include/usbdcore_omap1510.h
> +++ b/include/usbdcore_omap1510.h
> @@ -168,8 +168,8 @@
>  #define UDC_IN_ENDPOINT        1
>  #define UDC_IN_PACKET_SIZE 64
>  #define UDC_INT_ENDPOINT 5
> -#define UDC_INT_PKTSIZE        16
> -#define UDC_BULK_PKTSIZE 16
> +#define UDC_INT_PACKET_SIZE    16
> +#define UDC_BULK_PACKET_SIZE 16
>
>  void udc_irq (void);
>  /* Flow control */
> @@ -177,7 +177,7 @@ void udc_set_nak(int epid);
>  void udc_unset_nak (int epid);
>
>  /* Higher level functions for abstracting away from specific device */
> -void udc_endpoint_write(struct usb_endpoint_instance *endpoint);
> +int udc_endpoint_write(struct usb_endpoint_instance *endpoint);
>
>  int  udc_init (void);
>
> --
> 1.5.6.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH V2] usbtty/omap: update to current API

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:43 Sun 07 Dec     , Remy Bohmer wrote:
> Hello,
> 
> 2008/12/7 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> > the doc about the CONFIG_USBD is already in the README
> >
> > Best Regards,
> > J.
> 
> Applied to U-boot-usb tree ('next' branch)
It's a fix not a new features

Best Regards,
J.

[U-Boot] [PATCH V2] usbtty/omap: update to current API

[Remy Bohmer]

Hello,

>> Applied to U-boot-usb tree ('next' branch)
> It's a fix not a new features

Understood... I moved it to the master branch...

remy

>
> Best Regards,
> J.
>

[U-Boot] [PATCH V2] usbtty/omap: update to current API

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:08 Sun 07 Dec     , Remy Bohmer wrote:
> Hello,
> 
> >> Applied to U-boot-usb tree ('next' branch)
> > It's a fix not a new features
> 
> Understood... I moved it to the master branch...
Tks

Best Regards,
J.

[U-Boot] [PATCH] usbtty/omap: update to current API

[Bryan O'Donoghue]

On Mon, 10 Nov 2008 17:05:00 +0100
Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:

> > Please explain what exactly you are trying to change / fix here.
> > 
> > > diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
> > > index e738c56..448defa 100644
> > > --- a/drivers/serial/usbtty.c
> > > +++ b/drivers/serial/usbtty.c
> > > @@ -22,14 +22,13 @@
> > >   */
> > >  
> > >  #include <common.h>
> > > -
> > > +#include <config.h>		/* If defined, override Linux
> > > identifiers with
> > > +				 * vendor specific ones */
> > 
> > "If defined" - what needs to be defined to have any impact here?
> > 
> > And why do we care about Linux identifiers here? That's U-Boot, not
> > Linux.
> Yes I may remove the comment because I've allow to not overwrite them

Greetings.

The idea there is that the producer of a USB device is supposed to get
themselves a VendorID and typically you'd incrementally iterate your
ProductID's upwards as you release more devices.

usbtty.h in Das U-Boot "borrows" the Linux VendorID and ProductID because our
CDC-ACM implementation and usbtty - "g_serial" implementation is protocol
compatible with Linux.

In theory though if one were producing a device that for example had a USB port
in place of an RS232 port you'd replace VendorID and ProductID with your own
VendorID and ProductID.

You'd then diseminate - for example a Windows .ini file so that you could map
the cdc-acm driver that ships with Windows to your USB device.

In order to have any kind of meaningful test of the USB stack though we need a
default VendorID AND ProductID - hence the re-use of the Linux identifiers.

You really are supposed to go and get your OWN VendorID if you are producing a
USB enabled device.

This is stated in the README albeit not very cogently by me.

<snip>

> > > +#ifndef CONFIG_USBD_CONFIGURATION_STR
> > >  #define CONFIG_USBD_CONFIGURATION_STR "TTY via USB"
> > > +#endif
> > 
> > All these new config options need to be documented in the README.
> > 
> 
> this not new config option this is present option which is supposed to be
> define in the config file or use this defaul one.
> 
> I'll take a look on the README anyway and add it if nor present

<snip>


> > >  /* Called to start packet transmission. */
> > > -void udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> > > +int udc_endpoint_write (struct usb_endpoint_instance *endpoint)
> > >  {
> > >  	unsigned short epnum =
> > >  		endpoint->endpoint_address & USB_ENDPOINT_NUMBER_MASK;
> > > @@ -1081,6 +1081,8 @@ void udc_endpoint_write (struct
> > > usb_endpoint_instance *endpoint) /* deselect the endpoint FIFO */
> > >  		outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
> > >  	}
> > > +
> > > +	return 0;
> > >  }
> > 
> > If the only way to exit the function is by returning 0  at  the  end,
> > then  we should rather leave this as is and have this function return
> > void - otherwise we suggest that different values could  be  returned
> > which is not the case.
> 
> on mpc8xx they do return -1 if failed
> 
> and they share to same api


Indeed. As the API stands the OMAP stuff would have to be updated.

Basically in usbtty.c - what's happening is our terminal has gone to write a
USB endpoint but in midst of doing that - he received some data.

Since USB can't just throw away the data he was writing - the RX data just
pre-empts the write until a later time.

The REASON we have to deal with the RX event straight away is that it might be
a control packet - which the USB defines - if memory serves - as having to be
dealt with immediately.

Hence modification of omap as above I believe is consistent with the
requirements of the protocol !!!!!

I'd actually ACK this patch - if I read my email more often !

Taking WD's point - there may be a better or more elegant way to preempt the
endpoint write - deal with the RX data - and come back later to finish off the
write...

For the moment though I'd certainly say that the patch provided bring OMAP
inline with what's supposed to be happening in the code - even if the wisdom or
elegance of what it's trying to do - i.e. fit in with a bit of code I built
ontop of the original usbtty - is questionable !


Cheers,
BOD