On 17.08.2012 22:48, Marek Vasut wrote:
Dear Łukasz Dałek,
PXA25X chips don't support alternate settings so code in ether.c disables usage of CDC. But only code defined between DEV_CONFIG_CDC signals that network is up. This patch is fixing this bug by addding pxa_connect_gadget() which on pxa25x chips signals that network is up and do nothing on any other chips.
You're getting better, it's a good sign you're learning, I'm glad to see it :-)
It's my first big project in which I got involved so I'm ready for criticism.
Signed-off-by: Łukasz Dałekluk0104@gmail.com
drivers/usb/gadget/ether.c | 21 ++++++++++++++++++++- 1 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index d975fb6..964fe2e 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -44,7 +44,12 @@ extern struct platform_data brd;
unsigned packet_received, packet_sent;
-#define DEV_CONFIG_CDC 1 +#ifdef CONFIG_USB_GADGET_PXA2XX +# undef DEV_CONFIG_CDC +# define DEV_CONFIG_SUBSET 1
Can't we make it into a config value?
Does this work for you:
- Add:
CONFIG_USB_ETH_SUBSET CONFIG_USB_ETH_CDC
- Change this to
#if !defined(...SUBSET) || !defined(...CDC) || !defined(RNDIS) #define DEV_CONFIG_CDC /* preserve default behavior */ #endif
#if defined(...SUBSET) #define DEV_CONFIG_SUBSET #endif
#if defined(...CDC) #define DEV_CONFIG_CDC #endif
/* Note that RNDIS is already fixed */
Ok.
-- FROM HERE IT GOES INTO DIFFERENT PATCH --
- Replace DEV_CONFIG_CDC with CONFIG_USB_ETH_CDC, remove the last #if defined
above
Which one #if? That one I'd define in previous patch?
DTTO for SUBSET
define SUBSET in your header file
In my board's header file?
+#else +# define DEV_CONFIG_CDC 1 +#endif #define GFP_ATOMIC ((gfp_t) 0) #define GFP_KERNEL ((gfp_t) 0)
@@ -864,7 +869,9 @@ static struct usb_gadget_strings stringtab = {
/*======================================================================== ====*/ static u8 control_req[USB_BUFSIZ]; +#if defined(DEV_CONFIG_CDC) || defined(CONFIG_USB_ETH_RNDIS) static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(4))); +#endif
What trouble do you face here?
Tom, this aligned(4) doesn't look correct, some ALLOC_ALIGNED or friend would help here, right ? :)
If you don't define DEV_CONFIG_CDC nor ...RNDIS and define DEV_CONFIG_SUBSET then it won't compile (because of STATUS_BYTECOUNT).
/** @@ -1252,6 +1259,17 @@ static void rndis_command_complete(struct usb_ep *ep, struct usb_request *req)
#endif /* RNDIS */
+#ifdef CONFIG_USB_GADGET_PXA2XX +static inline void pxa_connect_gadget(void) +{
- debug("PXA connecting gadget...\n");
- l_ethdev.network_started = 1;
- printf("USB network up!\n");
Definitelly not printf(), but is this really PXA specific or is this part of the CDC subset goo?
I've took pattern from code in this file. What should I use instead of printf? CDC subset disable some code for PXA which signalise that network was started (l_ethdev... = 1) so this is quick-fix for this bug.
Łukasz Dałek