usbtty/omap: update to current API

PATCH New Jean-Christophe PLAGNIOL-VILLARD 4 participants 12 messages Nov 9, 2008 – Dec 7, 2008 1 message not in the archive USB

lore

Share & tools

lore

Jean-Christophe PLAGNIOL-VILLARD

author

2008-11-09 18:22 UTC #0 lore raw

PATCH 1/1 usbtty/omap: update to current API

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

drivers/serial/usbtty.c +2 -3

index e738c56..448defa 100644
@@ -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...)\

drivers/serial/usbtty.h +15 -4

index 71c47bc..2175d68 100644
@@ -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

drivers/usb/usbdcore_omap1510.c +3 -1

index 4e3239f..fb8cb5d 100644
@@ -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 */

include/usbdcore_omap1510.h +3 -3

index 526fcd9..ece0e95 100644
@@ -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);

Signature · 1 line

1.5.6.5

Wolfgang Denk

2008-11-09 19:59 UTC #1 lore raw

PATCH 1/1 usbtty/omap: update to current API

Dear Jean-Christophe PLAGNIOL-VILLARD,

Quoted 1 line · In message <[email protected]> you wrote:

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

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

drivers/serial/usbtty.c line 26

Quoted 11 lines

drivers/serial/usbtty.c +2 -1

index e738c56..448defa 100644
@@ -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.

Quoted 10 lines

drivers/serial/usbtty.c +7 -0

+#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

drivers/serial/usbtty.c

Quoted 10 lines

drivers/serial/usbtty.c +5 -1

 #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.

drivers/serial/usbtty.c void udc_endpoint_write (struct usb_endpoint_instance *endpoint) · line 1086

Quoted 13 lines

/* Called to start packet transmission. */

drivers/serial/usbtty.c +3 -1

-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

Signature · 6 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jean-Christophe PLAGNIOL-VILLARD

author

2008-11-10 16:05 UTC #3 lore raw

PATCH 1/1 usbtty/omap: update to current API

Quoted 21 lines · Wolfgang Denk

Dear Jean-Christophe PLAGNIOL-VILLARD,
Quoted 1 line · In message <[email protected]> you wrote:
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Please explain what exactly you are trying to change / fix here.
drivers/serial/usbtty.c line 26
Quoted 11 lines
drivers/serial/usbtty.c +2 -1
index e738c56..448defa 100644
@@ -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?

Quoted 2 lines

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

Quoted 17 lines

drivers/serial/usbtty.c +10 -0

+#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

Quoted 7 lines

drivers/serial/usbtty.c
Quoted 4 lines
drivers/serial/usbtty.c +2 -1
-
+#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

Quoted 10 lines

/* Called to start packet transmission. */

drivers/serial/usbtty.c +1 -1

-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);
 	}

Quoted 8 lines

drivers/serial/usbtty.c void udc_endpoint_write (struct usb_endpoint_instance *endpoint) · line 1086
Quoted 3 lines
drivers/serial/usbtty.c +2 -0
+
+	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.

Remy Bohmer

2008-11-26 21:00 UTC #4 lore raw

PATCH 1/1 usbtty/omap: update to current API

Hello Jean-Christophe,

Quoted 4 lines

Quoted 4 lines
drivers/serial/usbtty.c
Quoted 3 lines · In message <[email protected]> you wrote:
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
drivers/serial/usbtty.c +0 -0

What are we going to do with this patch?

Kind Regards,

Remy

Jean-Christophe PLAGNIOL-VILLARD

author

2008-11-27 12:44 UTC #5 lore raw

PATCH 1/1 usbtty/omap: update to current API

Quoted 8 lines · Remy Bohmer

Hello Jean-Christophe,
Quoted 4 lines
Quoted 4 lines
drivers/serial/usbtty.c
Quoted 3 lines · In message <[email protected]> you wrote:
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
drivers/serial/usbtty.c +0 -0
What are we going to do with this patch?

I'll work on it next week

Best Regards,
J.

Wolfgang Denk

2008-12-07 00:38 UTC #6 lore raw

PATCH 1/1 usbtty/omap: update to current API

Dear Jean-Christophe PLAGNIOL-VILLARD,

Quoted 10 lines · In message <[email protected]> you wrote:

Quoted 8 lines · Remy Bohmer
Hello Jean-Christophe,
Quoted 4 lines
Quoted 4 lines
drivers/serial/usbtty.c
Quoted 3 lines · In message <[email protected]> you wrote:
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
drivers/serial/usbtty.c +0 -0
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

Signature · 6 lines

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
<[email protected]>

Jean-Christophe PLAGNIOL-VILLARD

author

2008-12-07 08:45 UTC #7 lore raw

PATCH v2 1/1 usbtty/omap: update to current API

In reply to Jean-Christophe PLAGNIOL-VILLARD

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

---
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(-)

drivers/serial/usbtty.c +2 -4

index e738c56..7eba470 100644
@@ -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

drivers/serial/usbtty.h +21 ~~-10~~

index 71c47bc..ecefde5 100644
@@ -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

drivers/usb/usbdcore_omap1510.c +3 -1

index cb9dc44..6b7b61b 100644
@@ -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 */

include/usbdcore_omap1510.h +3 -3

index 526fcd9..ece0e95 100644
@@ -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);

Signature · 1 line

1.5.6.5

Remy Bohmer

2008-12-07 10:43 UTC #8 lore raw

PATCH v2 1/1 usbtty/omap: update to current API

Hello,

2008/12/7 Jean-Christophe PLAGNIOL-VILLARD <[email protected]>:

Quoted 6 lines

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
---
the doc about the CONFIG_USBD is already in the README
Best Regards,
J.

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

Thanks,

Remy

Quoted 135 lines

 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(-)

drivers/serial/usbtty.c +2 -4

index e738c56..7eba470 100644
@@ -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

drivers/serial/usbtty.h +21 ~~-10~~

index 71c47bc..ecefde5 100644
@@ -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

drivers/usb/usbdcore_omap1510.c +3 -1

index cb9dc44..6b7b61b 100644
@@ -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 */

include/usbdcore_omap1510.h +3 -4

index 526fcd9..ece0e95 100644
@@ -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

Jean-Christophe PLAGNIOL-VILLARD

author

2008-12-07 10:40 UTC #9 lore raw

PATCH v2 1/1 usbtty/omap: update to current API

Quoted 11 lines · Remy Bohmer

Hello,
2008/12/7 Jean-Christophe PLAGNIOL-VILLARD <[email protected]>:
Quoted 6 lines
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
---
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.

Remy Bohmer

2008-12-07 11:08 UTC #10 lore raw

PATCH v2 1/1 usbtty/omap: update to current API

Hello,

Quoted 2 lines

Quoted 1 line
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

Quoted 4 lines

Best Regards,
J.

Jean-Christophe PLAGNIOL-VILLARD

author

2008-12-07 11:06 UTC #11 lore raw

PATCH v2 1/1 usbtty/omap: update to current API

Quoted 6 lines · Remy Bohmer

Hello,
Quoted 2 lines
Quoted 1 line
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.

Bryan O'Donoghue

2008-11-26 23:54 UTC #12 lore raw

PATCH 1/1 usbtty/omap: update to current API

In reply to Jean-Christophe PLAGNIOL-VILLARD

Quoted 17 lines · On Mon, 10 Nov 2008 17:05:00 +0100 Jean-Christophe PLAGNIOL-VILLARD…

Quoted 17 lines
Please explain what exactly you are trying to change / fix here.
drivers/serial/usbtty.c line 26
Quoted 12 lines
drivers/serial/usbtty.c +1 -1
index e738c56..448defa 100644
@@ -22,14 +22,13 @@
  */
 
 #include <common.h>
-
+#include <config.h>		/* If defined, override Linux
identifiers with
drivers/serial/usbtty.c +1 -0
+				 * vendor specific ones */
"If defined" - what needs to be defined to have any impact here?

Quoted 3 lines

Quoted 2 lines
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>

Quoted 11 lines

Quoted 6 lines
drivers/serial/usbtty.c
Quoted 3 lines
drivers/serial/usbtty.c +2 -0
+#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>

Quoted 19 lines

Quoted 10 lines
Quoted 10 lines
/* Called to start packet transmission. */
drivers/serial/usbtty.c +1 -1
-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 */
drivers/serial/usbtty.c +0 -0
 		outw (UDC_EP_Dir | epnum, UDC_EP_NUM);
 	}
Quoted 8 lines
drivers/serial/usbtty.c
Quoted 3 lines
drivers/serial/usbtty.c +2 -0
+
+	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.

Quoted 3 lines

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