[U-Boot] [PATCH 1/3] usbether: Fixed bug when using with PXA25X chips

[Marek Vasut <marex@denx.de>]

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

> Signed-off-by: ?ukasz Da?ek <luk0104@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:
1) Add:
CONFIG_USB_ETH_SUBSET
CONFIG_USB_ETH_CDC

2) 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 */

-- FROM HERE IT GOES INTO DIFFERENT PATCH --

3) Replace DEV_CONFIG_CDC with CONFIG_USB_ETH_CDC, remove the last #if defined 
above

4) DTTO for SUBSET

5) define SUBSET in your header file

is it clear?


> +#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 ? :)

> 
>  /**
> @@ -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?

> +}
> +#else
> +static inline void pxa_connect_gadget(void) { }
> +#endif
> +
>  /*
>   * The setup() callback implements all the ep0 functionality that's not
>   * handled lower down.  CDC has a number of less-common features:
> @@ -1352,6 +1370,7 @@ eth_setup(struct usb_gadget *gadget, const struct
> usb_ctrlrequest *ctrl) if (gadget_is_pxa(gadget)) {
>  			value = eth_set_config(dev, DEV_CONFIG_VALUE,
>  						GFP_ATOMIC);
> +			pxa_connect_gadget();
>  			goto done_set_intf;
>  		}

Best regards,
Marek Vasut