> -----Original Message-----
> From: David Brownell [mailto:[EMAIL PROTECTED]
> Sent: Friday, April 20, 2007 11:20 AM
> To: linux-usb-devel@lists.sourceforge.net
> Cc: Li Yang-r58472; [EMAIL PROTECTED]; Schmid Bruce-R62923
> Subject: Re: [linux-usb-devel] [PATCH 1/3]USB: add Freescale high-speed USB 
> SOC
> device controller driver
> 
> On Thursday 19 April 2007 7:53 pm, Li Yang-r58472 wrote:
> > 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]; linux-usb-devel@lists.sourceforge.net; 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)
> > > > +{...
> > > > +}
> > >
> > > 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
> 
> No.  See <linux/usb_gadget.h> for the definition of usb_ep_alloc_buffer();
> it says explicitly that such mapping calls are not required.
> 
> The only portable way to guarantee that is to allocate coherent memory.

Then I can remove the dma_sync_single calls from ep_queue() and done()?
> 
> 
> > > > +               } 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;
> > > > + ...
> > >
> > > 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.
> 
> That's the usual way:  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.
> 
> Well, you included _partial_ OTG support ... managing those flags
> when is_otg is set (but nothing sets it yet).  :)
> 
> I guess it's just my taste that I'd include a comment right there
> highlighting that this doesn't actually implement the OTG feature
> it claims to implement ... but that's OK until something sets is_otg.

Is it ok that I indicate this in the patch comment?

- 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/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to