Hi,

John Youn <[email protected]> writes:
> Currently GEVNTCOUNT is written in the threaded interrupt handler while
> processing each event. This commit moves the GEVNTCOUNT write to the
> hard IRQ. We then copy the events to a separate buffer for the event
> handler to read from.
>
> This change is in preparation of working around an issue in core version
> 3.00a where the interrupt cannot be de-asserted in the normal way.
> However, if we enable interrupt moderation, we can also de-assert it by
> writing to GEVNTCOUNT.
>
> Signed-off-by: John Youn <[email protected]>
> ---
>  drivers/usb/dwc3/core.c   |  4 ++++
>  drivers/usb/dwc3/core.h   |  2 ++
>  drivers/usb/dwc3/gadget.c | 22 +++++++++++++---------
>  3 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index af42346..f0bb6df 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -251,6 +251,10 @@ static struct dwc3_event_buffer 
> *dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
>  
>       evt->dwc        = dwc;
>       evt->length     = length;
> +     evt->cache      = devm_kzalloc(dwc->dev, length, GFP_KERNEL);
> +     if (!evt->cache)
> +             return ERR_PTR(-ENOMEM);
> +
>       evt->buf        = dma_alloc_coherent(dwc->dev, length,
>                       &evt->dma, GFP_KERNEL);
>       if (!evt->buf)
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 354de24..bf63756 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -472,6 +472,7 @@ struct dwc3_trb;
>  /**
>   * struct dwc3_event_buffer - Software event buffer representation
>   * @buf: _THE_ buffer
> + * @cache: The buffer cache used in the threaded interrupt
>   * @length: size of this buffer
>   * @lpos: event offset
>   * @count: cache of last read event count register
> @@ -481,6 +482,7 @@ struct dwc3_trb;
>   */
>  struct dwc3_event_buffer {
>       void                    *buf;
> +     void                    *cache;
>       unsigned                length;
>       unsigned int            lpos;
>       unsigned int            count;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 22ccc34..f07dd84 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2829,18 +2829,16 @@ static irqreturn_t dwc3_process_event_buf(struct 
> dwc3_event_buffer *evt)
>  {
>       struct dwc3 *dwc = evt->dwc;
>       irqreturn_t ret = IRQ_NONE;
> -     int left;
> +     int idx;
>       u32 reg;
>  
> -     left = evt->count;
> -
>       if (!(evt->flags & DWC3_EVENT_PENDING))
>               return IRQ_NONE;
>  
> -     while (left > 0) {
> +     for (idx = 0; idx < evt->count; idx += 4) {

why so many changes on the same patch? Why did you remove left and
evt->lpos? You can still use both for traversing through your cache,
right?

>               union dwc3_event event;
>  
> -             event.raw = *(u32 *) (evt->buf + evt->lpos);
> +             event.raw = *(u32 *)(evt->cache + idx);
>  
>               dwc3_process_event_entry(dwc, &event);
>  
> @@ -2853,10 +2851,6 @@ static irqreturn_t dwc3_process_event_buf(struct 
> dwc3_event_buffer *evt)
>                * boundary so I worry about that once we try to handle
>                * that.
>                */
> -             evt->lpos = (evt->lpos + 4) % DWC3_EVENT_BUFFERS_SIZE;
> -             left -= 4;
> -
> -             dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 4);
>       }
>  
>       evt->count = 0;
> @@ -2889,6 +2883,7 @@ static irqreturn_t dwc3_check_event_buf(struct 
> dwc3_event_buffer *evt)
>  {
>       struct dwc3 *dwc = evt->dwc;
>       u32 count;
> +     u32 amount;

please keep reverse xmas tree ordering ;-)

>       u32 reg;
>  
>       if (pm_runtime_suspended(dwc->dev)) {
> @@ -2906,6 +2901,15 @@ static irqreturn_t dwc3_check_event_buf(struct 
> dwc3_event_buffer *evt)
>       evt->count = count;
>       evt->flags |= DWC3_EVENT_PENDING;
>  
> +     amount = min(count, DWC3_EVENT_BUFFERS_SIZE - evt->lpos);
> +     memcpy(evt->cache, evt->buf + evt->lpos, amount);
> +
> +     if (amount < count)
> +             memcpy(evt->cache + amount, evt->buf, count - amount);
> +
> +     evt->lpos = (evt->lpos + count) % DWC3_EVENT_BUFFERS_SIZE;

evt->lpos is only for the driver, if you don't touch it here you can
still use it on the bottom half handler.

> +     dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);

should you mask first just to make sure we don't have other interrupts
as soon as we clear the ones we just read?

-- 
balbi

Attachment: signature.asc
Description: PGP signature

Reply via email to