| Message ID | 1308873788-22881-1-git-send-email-agust@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 24, 2011 at 8:03 AM, Anatolij Gustschin <agust@denx.de> wrote: > Commit 09ba0def introduced build breakage on ARM arch. > Fix it by setting accessors using a static inline function > which is a nop when compiling the driver for ARM arch. > > Don't use flush_dcache_range(), convert to the DMA API > usage instead. USB2.0CV Halt Endpoint Test succeeds on > PPC. Tested both on ARM i.MX31 and mpc5121 PPC. > > Signed-off-by: Anatolij Gustschin <agust@denx.de> This looks good to me. Acked-by: Eric Miao <eric.miao@linaro.org> > --- > drivers/usb/gadget/fsl_udc_core.c | 30 +++++++++++++++++++----------- > 1 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c > index 2cd9a60..eab033f 100644 > --- a/drivers/usb/gadget/fsl_udc_core.c > +++ b/drivers/usb/gadget/fsl_udc_core.c > @@ -46,7 +46,6 @@ > #include <asm/system.h> > #include <asm/unaligned.h> > #include <asm/dma.h> > -#include <asm/cacheflush.h> > > #include "fsl_usb2_udc.h" > > @@ -118,6 +117,17 @@ static void (*_fsl_writel)(u32 v, unsigned __iomem *p); > #define fsl_readl(p) (*_fsl_readl)((p)) > #define fsl_writel(v, p) (*_fsl_writel)((v), (p)) > > +static inline void fsl_set_accessors(struct fsl_usb2_platform_data *pdata) > +{ > + if (pdata->big_endian_mmio) { > + _fsl_readl = _fsl_readl_be; > + _fsl_writel = _fsl_writel_be; > + } else { > + _fsl_readl = _fsl_readl_le; > + _fsl_writel = _fsl_writel_le; > + } > +} > + > static inline u32 cpu_to_hc32(const u32 x) > { > return udc_controller->pdata->big_endian_desc > @@ -132,6 +142,8 @@ static inline u32 hc32_to_cpu(const u32 x) > : le32_to_cpu((__force __le32)x); > } > #else /* !CONFIG_PPC32 */ > +static inline void fsl_set_accessors(struct fsl_usb2_platform_data *pdata) {} > + > #define fsl_readl(addr) readl(addr) > #define fsl_writel(val32, addr) writel(val32, addr) > #define cpu_to_hc32(x) cpu_to_le32(x) > @@ -1348,9 +1360,6 @@ static void ch9getstatus(struct fsl_udc *udc, u8 request_type, u16 value, > /* Fill in the reqest structure */ > *((u16 *) req->req.buf) = cpu_to_le16(tmp); > > - /* flush cache for the req buffer */ > - flush_dcache_range((u32)req->req.buf, (u32)req->req.buf + 8); > - > req->ep = ep; > req->req.length = 2; > req->req.status = -EINPROGRESS; > @@ -1358,6 +1367,11 @@ static void ch9getstatus(struct fsl_udc *udc, u8 request_type, u16 value, > req->req.complete = NULL; > req->dtd_count = 0; > > + dma_sync_single_for_device(udc->gadget.dev.parent, > + req->req.dma, req->req.length, > + ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); > + req->mapped = 0; > + > /* prime the data phase */ > if ((fsl_req_to_dtd(req) == 0)) > fsl_queue_td(ep, req); > @@ -2470,13 +2484,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev) > } > > /* Set accessors only after pdata->init() ! */ > - if (pdata->big_endian_mmio) { > - _fsl_readl = _fsl_readl_be; > - _fsl_writel = _fsl_writel_be; > - } else { > - _fsl_readl = _fsl_readl_le; > - _fsl_writel = _fsl_writel_le; > - } > + fsl_set_accessors(pdata); > > #ifndef CONFIG_ARCH_MXC > if (pdata->have_sysif_regs) > -- > 1.7.1 > >
On Fri, Jun 24, 2011 at 02:03:08AM +0200, Anatolij Gustschin wrote: > Commit 09ba0def introduced build breakage on ARM arch. > Fix it by setting accessors using a static inline function > which is a nop when compiling the driver for ARM arch. > > Don't use flush_dcache_range(), convert to the DMA API > usage instead. USB2.0CV Halt Endpoint Test succeeds on > PPC. Tested both on ARM i.MX31 and mpc5121 PPC. While this may work, if you enable DMA API debugging, you'll get complaints. Please test your changes on ARM with CONFIG_DMA_API_DEBUG enabled. /* FIXME: fsl_alloc_request() ignores ep argument */ udc->status_req = container_of(fsl_alloc_request(NULL, GFP_KERNEL), struct fsl_req, req); /* allocate a small amount of memory to get valid address */ udc->status_req->req.buf = kmalloc(8, GFP_KERNEL); udc->status_req->req.dma = virt_to_phys(udc->status_req->req.buf); ... /* Borrow the per device status_req */ req = udc->status_req; /* Fill in the reqest structure */ *((u16 *) req->req.buf) = cpu_to_le16(tmp); /* flush cache for the req buffer */ flush_dcache_range((u32)req->req.buf, (u32)req->req.buf + 8); req->ep = ep; req->req.length = 2; req->req.status = -EINPROGRESS; req->req.actual = 0; req->req.complete = NULL; req->dtd_count = 0; /* prime the data phase */ if ((fsl_req_to_dtd(req) == 0)) fsl_queue_td(ep, req); ... I've no idea why this driver can't use dma_map_single() before queuing, and dma_unmap_single() once the req is complete. It looks like done() will do the unmap provided we map it correctly and set req->mapped. So, I think that virt_to_phys can be killed, and a dma_map_single() can be done in place of the flush_dcache_range to set req->req.dma, and req->mapped needs to be set to cause it to be unmapped.
On Fri, Jun 24, 2011 at 5:56 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Jun 24, 2011 at 02:03:08AM +0200, Anatolij Gustschin wrote: >> Commit 09ba0def introduced build breakage on ARM arch. >> Fix it by setting accessors using a static inline function >> which is a nop when compiling the driver for ARM arch. >> >> Don't use flush_dcache_range(), convert to the DMA API >> usage instead. USB2.0CV Halt Endpoint Test succeeds on >> PPC. Tested both on ARM i.MX31 and mpc5121 PPC. > > While this may work, if you enable DMA API debugging, you'll get > complaints. Please test your changes on ARM with CONFIG_DMA_API_DEBUG > enabled. > > > /* FIXME: fsl_alloc_request() ignores ep argument */ > udc->status_req = container_of(fsl_alloc_request(NULL, GFP_KERNEL), > struct fsl_req, req); > /* allocate a small amount of memory to get valid address */ > udc->status_req->req.buf = kmalloc(8, GFP_KERNEL); > udc->status_req->req.dma = virt_to_phys(udc->status_req->req.buf); > > ... > > /* Borrow the per device status_req */ > req = udc->status_req; > /* Fill in the reqest structure */ > *((u16 *) req->req.buf) = cpu_to_le16(tmp); > > /* flush cache for the req buffer */ > flush_dcache_range((u32)req->req.buf, (u32)req->req.buf + 8); > > req->ep = ep; > req->req.length = 2; > req->req.status = -EINPROGRESS; > req->req.actual = 0; > req->req.complete = NULL; > req->dtd_count = 0; > > /* prime the data phase */ > if ((fsl_req_to_dtd(req) == 0)) > fsl_queue_td(ep, req); > ... > > I've no idea why this driver can't use dma_map_single() before queuing, > and dma_unmap_single() once the req is complete. It looks like done() > will do the unmap provided we map it correctly and set req->mapped. > > So, I think that virt_to_phys can be killed, and a dma_map_single() can > be done in place of the flush_dcache_range to set req->req.dma, and > req->mapped needs to be set to cause it to be unmapped. > It looks like it's using dma_* API for endpoints other than ep0.
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c index 2cd9a60..eab033f 100644 --- a/drivers/usb/gadget/fsl_udc_core.c +++ b/drivers/usb/gadget/fsl_udc_core.c @@ -46,7 +46,6 @@ #include <asm/system.h> #include <asm/unaligned.h> #include <asm/dma.h> -#include <asm/cacheflush.h> #include "fsl_usb2_udc.h" @@ -118,6 +117,17 @@ static void (*_fsl_writel)(u32 v, unsigned __iomem *p); #define fsl_readl(p) (*_fsl_readl)((p)) #define fsl_writel(v, p) (*_fsl_writel)((v), (p)) +static inline void fsl_set_accessors(struct fsl_usb2_platform_data *pdata) +{ + if (pdata->big_endian_mmio) { + _fsl_readl = _fsl_readl_be; + _fsl_writel = _fsl_writel_be; + } else { + _fsl_readl = _fsl_readl_le; + _fsl_writel = _fsl_writel_le; + } +} + static inline u32 cpu_to_hc32(const u32 x) { return udc_controller->pdata->big_endian_desc @@ -132,6 +142,8 @@ static inline u32 hc32_to_cpu(const u32 x) : le32_to_cpu((__force __le32)x); } #else /* !CONFIG_PPC32 */ +static inline void fsl_set_accessors(struct fsl_usb2_platform_data *pdata) {} + #define fsl_readl(addr) readl(addr) #define fsl_writel(val32, addr) writel(val32, addr) #define cpu_to_hc32(x) cpu_to_le32(x) @@ -1348,9 +1360,6 @@ static void ch9getstatus(struct fsl_udc *udc, u8 request_type, u16 value, /* Fill in the reqest structure */ *((u16 *) req->req.buf) = cpu_to_le16(tmp); - /* flush cache for the req buffer */ - flush_dcache_range((u32)req->req.buf, (u32)req->req.buf + 8); - req->ep = ep; req->req.length = 2; req->req.status = -EINPROGRESS; @@ -1358,6 +1367,11 @@ static void ch9getstatus(struct fsl_udc *udc, u8 request_type, u16 value, req->req.complete = NULL; req->dtd_count = 0; + dma_sync_single_for_device(udc->gadget.dev.parent, + req->req.dma, req->req.length, + ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE); + req->mapped = 0; + /* prime the data phase */ if ((fsl_req_to_dtd(req) == 0)) fsl_queue_td(ep, req); @@ -2470,13 +2484,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev) } /* Set accessors only after pdata->init() ! */ - if (pdata->big_endian_mmio) { - _fsl_readl = _fsl_readl_be; - _fsl_writel = _fsl_writel_be; - } else { - _fsl_readl = _fsl_readl_le; - _fsl_writel = _fsl_writel_le; - } + fsl_set_accessors(pdata); #ifndef CONFIG_ARCH_MXC if (pdata->have_sysif_regs)
Commit 09ba0def introduced build breakage on ARM arch. Fix it by setting accessors using a static inline function which is a nop when compiling the driver for ARM arch. Don't use flush_dcache_range(), convert to the DMA API usage instead. USB2.0CV Halt Endpoint Test succeeds on PPC. Tested both on ARM i.MX31 and mpc5121 PPC. Signed-off-by: Anatolij Gustschin <agust@denx.de> --- drivers/usb/gadget/fsl_udc_core.c | 30 +++++++++++++++++++----------- 1 files changed, 19 insertions(+), 11 deletions(-)