Re: [PATCH 1/3]USB: add Freescale high-speed USB SOC device controller driver

Hi Dave,

> -----Original Message-----
> From: David Brownell [mailto:[email protected]]
> Sent: Friday, April 20, 2007 2:24 AM
> To: Li Yang-r58472
> Cc: [email protected]; [email protected]; Schmid Bruce-R62923
> Subject: Re: [PATCH 1/3]USB: add Freescale high-speed USB SOC device controller
> driver
> 
> On Thursday 19 April 2007 2:53 am, Li Yang wrote:
> > +static void *fsl_alloc_buffer(struct usb_ep *_ep, unsigned bytes,
> > +               dma_addr_t *dma, gfp_t gfp_flags)
> > +{
> > +       void *retval = NULL;
> > +
> > +       if (!bytes)
> > +               return 0;
> > +
> > +       /* We alloc normal(cachable) kernel memory, as this buffer will
> > +        * be sync'ed upon transmittion. */
> > +
> > +       retval = kmalloc(bytes, gfp_flags);
> > +       if (retval)
> > +               *dma = virt_to_phys(retval);
> > +       return retval;
> > +}
> 
> You still need to fix this to use dma_alloc_coherent(), and its
> sibling function to use dma_free_coherent().
> 
> This particular code assumes !CONFIG_NOT_COHERENT_CACHE ... which
> ISTR you've said is not true on all the relevant PPC platforms, and
> is certainly not true on the versions found (already?) in Freescale
> ARM products.

This buffer is synced using dma_sync_single_*/dma_(un)map_single when the request is queued and when the transmission is complete.  To maintain cache coherency, we do either allocate a coherent (non-cacheable) memory or sync them when needed.  By allocating cacheable memory, we will get better performance while processing the payload of a USB request.

> 
> 
> > +               } else if ((setup->bRequestType & USB_RECIP_MASK)
> > +                               == USB_RECIP_DEVICE) {
> > +                       if (!udc->gadget.is_otg)
> > +                               break;
> > +                       else if (setup->bRequest ==
> USB_DEVICE_B_HNP_ENABLE)
> > +                               udc->gadget.b_hnp_enable = 1;
> > +                       else if (setup->bRequest ==
> USB_DEVICE_A_HNP_SUPPORT)
> > +                               udc->gadget.a_hnp_support = 1;
> > +                       else if (setup->bRequest ==
> >
> +                                       USB_DEVICE_A_ALT_HNP_SUPPORT)
> > +                               udc->gadget.a_alt_hnp_support = 1;
> > +                       rc = 0;
> > +               }
> 
> And I'm curious about that stuff too ... should have asked before.
> Is this a case where the hardware somehow snooping that request?

No, the hardware provide little assist on OTG.  We need to maintain the OTG states by software.
> 
> Because if it isn't, you're missing logic to poke some OTG register
> when B_HNP_ENABLE is set ... nothing will happen when the device
> suspends, no role switch or anything.

We have done the code for OTG, but it is not ready to be submitted for now.  So I decided not to include them in this patch.

- Leo

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel