On Fri, Mar 29, 2013 at 12:32:00AM +0100, Michael Grzeschik wrote:
> The prepared memory for the qhead needs to be contiguos and 2K aligned.
> We change the code from allocating extra buffer for every ep qhead to
> one big area. This patch lowers the amount of code to prepare the
> memory.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
> drivers/usb/chipidea/ci.h | 9 +++++++--
> drivers/usb/chipidea/udc.c | 32 +++++++++++++++-----------------
> drivers/usb/chipidea/udc.h | 2 +-
> 3 files changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 7dcd390..7319bf6 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -121,7 +121,7 @@ struct hw_bank {
> * @is_otg: if the device is otg-capable
> * @work: work for role changing
> * @wq: workqueue thread
> - * @qh_pool: allocation pool for queue heads
> + * @qh: allocation struct for queue heads
> * @td_pool: allocation pool for transfer descriptors
> * @gadget: device side representation for peripheral controller
> * @driver: gadget driver
> @@ -154,7 +154,12 @@ struct ci13xxx {
> struct work_struct vbus_work;
> struct workqueue_struct *wq;
>
> - struct dma_pool *qh_pool;
> + struct {
> + struct ci13xxx_qh *ptr;
> + dma_addr_t dma;
> + size_t size;
> + } qh;
> +
> struct dma_pool *td_pool;
>
> struct usb_gadget gadget;
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index dd4fe75..21f6558 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -13,9 +13,11 @@
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/dmapool.h>
> +#include <linux/dma-mapping.h>
> #include <linux/err.h>
> #include <linux/irqreturn.h>
> #include <linux/kernel.h>
> +#include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/pm_runtime.h>
> #include <linux/usb/ch9.h>
> @@ -1401,7 +1403,7 @@ static int ci13xxx_vbus_session(struct usb_gadget *_gadget, int is_active)
> pm_runtime_get_sync(&_gadget->dev);
> hw_device_reset(ci, USBMODE_CM_DC);
> hw_enable_vbus_intr(ci);
> - hw_device_state(ci, ci->ep0out->qh.dma);
> + hw_device_state(ci, ci->qh.dma);
> } else {
> hw_device_state(ci, 0);
> if (ci->platdata->notify_event)
> @@ -1504,8 +1506,8 @@ static int init_eps(struct ci13xxx *ci)
> mEp->ep.maxpacket = (unsigned short)~0;
>
> INIT_LIST_HEAD(&mEp->qh.queue);
> - mEp->qh.ptr = dma_pool_alloc(ci->qh_pool, GFP_KERNEL,
> - &mEp->qh.dma);
> + mEp->qh.ptr = &ci->qh.ptr[i * 2 + j];
> +
> if (mEp->qh.ptr == NULL)
> retval = -ENOMEM;
> else
> @@ -1533,13 +1535,8 @@ static int init_eps(struct ci13xxx *ci)
>
> static void destroy_eps(struct ci13xxx *ci)
> {
> - int i;
> -
> - for (i = 0; i < ci->hw_ep_max; i++) {
> - struct ci13xxx_ep *mEp = &ci->ci13xxx_ep[i];
> -
> - dma_pool_free(ci->qh_pool, mEp->qh.ptr, mEp->qh.dma);
> - }
> + dma_free_coherent(ci->dev, ci->qh.size,
> + ci->qh.ptr, ci->qh.dma);
> }
>
> /**
> @@ -1585,7 +1582,7 @@ static int ci13xxx_start(struct usb_gadget *gadget,
> }
> }
>
> - retval = hw_device_state(ci, ci->ep0out->qh.dma);
> + retval = hw_device_state(ci, ci->qh.dma);
> if (retval)
> pm_runtime_put_sync(&ci->gadget.dev);
>
> @@ -1736,11 +1733,14 @@ static int udc_start(struct ci13xxx *ci)
> ci->gadget.dev.parent = dev;
> ci->gadget.dev.release = udc_release;
>
> + ci->qh.size = ci->hw_ep_max * sizeof(struct ci13xxx_qh);
> + ci->qh.size = ALIGN(ci->qh.size, SZ_2K);
> +
What's the value of sizeof(struct ci13xxx_qh) after you pack it
as 64 bytes aligned?
QH is 48-bytes structure, but needs 64-bytes aligned, it can't
avoid wasting 16 bytes.
So, please make sure the memory layout is correct for qh arrays.
> /* alloc resources */
> - ci->qh_pool = dma_pool_create("ci13xxx_qh", dev,
> - sizeof(struct ci13xxx_qh),
> - 64, CI13XXX_PAGE_SIZE);
> - if (ci->qh_pool == NULL)
> + ci->qh.ptr = dma_alloc_coherent(dev, ci->qh.size,
> + &ci->qh.dma, GFP_ATOMIC);
> +
GFP_ATMOIC isn't needed.
> + if (ci->qh.ptr == NULL)
> return -ENOMEM;
>
> ci->td_pool = dma_pool_create("ci13xxx_td", dev,
> @@ -1814,7 +1814,6 @@ destroy_eps:
> free_pools:
> dma_pool_destroy(ci->td_pool);
> free_qh_pool:
> - dma_pool_destroy(ci->qh_pool);
You may need dma_free_coherent.
> return retval;
> }
>
> @@ -1836,7 +1835,6 @@ static void udc_stop(struct ci13xxx *ci)
> destroy_eps(ci);
>
> dma_pool_destroy(ci->td_pool);
> - dma_pool_destroy(ci->qh_pool);
>
> if (!IS_ERR_OR_NULL(ci->transceiver)) {
> otg_set_peripheral(ci->transceiver->otg, NULL);
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index 4b1315a..b7d3882 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -55,7 +55,7 @@ struct ci13xxx_qh {
> /* 9 */
> u32 RESERVED;
> struct usb_ctrlrequest setup;
> -} __attribute__ ((packed, aligned(4)));
> +} __attribute__ ((packed, aligned(64)));
>
> /**
> * struct ci13xxx_req - usb request representation
> --
> 1.8.2.rc2
>
>
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
