| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <D2C8DF24C22DB24BA948A8B0D097284002F220EA@SJEXCHMB05.corp.ad.broadcom.com>
Date: Wed, 26 Aug 2015 14:44:51 +0000
From: Roman Bacik <rbacik@...adcom.com>
To: John Youn <John.Youn@...opsys.com>,
Scott Branden <sbranden@...adcom.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>
Subject: RE: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
> -----Original Message-----
> From: John Youn [mailto:John.Youn@...opsys.com]
> Sent: August-25-15 7:06 PM
> To: Roman Bacik; John Youn; Scott Branden; Greg Kroah-Hartman; linux-
> usb@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org; bcm-kernel-feedback-list
> Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous mode
>
> On 8/25/2015 3:00 PM, Roman Bacik wrote:
> >> -----Original Message-----
> >> From: John Youn [mailto:John.Youn@...opsys.com]
> >> Sent: August-25-15 2:52 PM
> >> To: Scott Branden; John Youn; Greg Kroah-Hartman; linux-
> >> usb@...r.kernel.org
> >> Cc: linux-kernel@...r.kernel.org; bcm-kernel-feedback-list; Roman
> >> Bacik
> >> Subject: Re: [PATCH 1/1] usb: dwc2: gadget: parity fix in isochronous
> >> mode
> >>
> >> On 8/18/2015 8:45 AM, Scott Branden wrote:
> >>> From: Roman Bacik <rbacik@...adcom.com>
> >>>
> >>> USB OTG driver in isochronous mode has to set the parity of the
> >>> receiving microframe. The parity is set to even by default. This
> >>> causes problems for an audio gadget, if the host starts transmitting
> >>> on odd
> >> microframes.
> >>>
> >>> This fix uses Incomplete Periodic Transfer interrupt to toggle
> >>> between even and odd parity until the Transfer Complete interrupt is
> received.
> >>>
> >>> Signed-off-by: Roman Bacik <rbacik@...adcom.com>
> >>> Reviewed-by: Abhinav Ratna <aratna@...adcom.com>
> >>> Reviewed-by: Srinath Mannam <srinath.mannam@...adcom.com>
> >>> Reviewed-by: Scott Branden <sbranden@...adcom.com>
> >>> Signed-off-by: Scott Branden <sbranden@...adcom.com>
> >>> ---
> >>> drivers/usb/dwc2/core.h | 1 +
> >>> drivers/usb/dwc2/gadget.c | 48
> >> ++++++++++++++++++++++++++++++++++++++++++++++-
> >>> drivers/usb/dwc2/hw.h | 1 +
> >>> 3 files changed, 49 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index
> >>> 0ed87620..954d1cd 100644
> >>> --- a/drivers/usb/dwc2/core.h
> >>> +++ b/drivers/usb/dwc2/core.h
> >>> @@ -150,6 +150,7 @@ struct s3c_hsotg_ep {
> >>> unsigned int periodic:1;
> >>> unsigned int isochronous:1;
> >>> unsigned int send_zlp:1;
> >>> + unsigned int parity_set:1;
> >>>
> >>> char name[10];
> >>> };
> >>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> >>> index 4d47b7c..28e4393 100644
> >>> --- a/drivers/usb/dwc2/gadget.c
> >>> +++ b/drivers/usb/dwc2/gadget.c
> >>> @@ -1954,6 +1954,8 @@ static void s3c_hsotg_epint(struct dwc2_hsotg
> >> *hsotg, unsigned int idx,
> >>> ints &= ~DXEPINT_XFERCOMPL;
> >>>
> >>> if (ints & DXEPINT_XFERCOMPL) {
> >>> + if (hs_ep->isochronous && !hs_ep->parity_set)
> >>> + hs_ep->parity_set = 1;
> >>> if (hs_ep->isochronous && hs_ep->interval == 1) {
> >>> if (ctrl & DXEPCTL_EOFRNUM)
> >>> ctrl |= DXEPCTL_SETEVENFR;
> >>> @@ -2316,7 +2318,8 @@ void s3c_hsotg_core_init_disconnected(struct
> >> dwc2_hsotg *hsotg,
> >>> GINTSTS_CONIDSTSCHNG | GINTSTS_USBRST |
> >>> GINTSTS_RESETDET | GINTSTS_ENUMDONE |
> >>> GINTSTS_OTGINT | GINTSTS_USBSUSP |
> >>> - GINTSTS_WKUPINT,
> >>> + GINTSTS_WKUPINT |
> >>> + GINTSTS_INCOMPL_SOIN | GINTSTS_INCOMPL_SOOUT,
> >>> hsotg->regs + GINTMSK);
> >>>
> >>> if (using_dma(hsotg))
> >>> @@ -2581,6 +2584,48 @@ irq_retry:
> >>> s3c_hsotg_dump(hsotg);
> >>> }
> >>>
> >>> + if (gintsts & GINTSTS_INCOMPL_SOIN) {
> >>> + u32 idx;
> >>> + struct s3c_hsotg_ep *hs_ep;
> >>> +
> >>> + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOIN\n",
> >> __func__);
> >>> + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) {
> >>> + hs_ep = hsotg->eps_in[idx];
> >>> + if (hs_ep->isochronous && !hs_ep->parity_set) {
> >>> + u32 epctl_reg = DIEPCTL(idx);
> >>> + u32 ctrl = readl(hsotg->regs + epctl_reg);
> >>> +
> >>> + if (ctrl & DXEPCTL_EOFRNUM)
> >>> + ctrl |= DXEPCTL_SETEVENFR;
> >>> + else
> >>> + ctrl |= DXEPCTL_SETODDFR;
> >>> + writel(ctrl, hsotg->regs + epctl_reg);
> >>> + }
> >>> + }
> >>> + writel(GINTSTS_INCOMPL_SOIN, hsotg->regs + GINTSTS);
> >>> + }
> >>> +
> >>> + if (gintsts & GINTSTS_INCOMPL_SOOUT) {
> >>> + u32 idx;
> >>> + struct s3c_hsotg_ep *hs_ep;
> >>> +
> >>> + dev_dbg(hsotg->dev, "%s: GINTSTS_INCOMPL_SOOUT\n",
> >> __func__);
> >>> + for (idx = 1; idx < MAX_EPS_CHANNELS; idx++) {
> >>> + hs_ep = hsotg->eps_out[idx];
> >>> + if (hs_ep->isochronous && !hs_ep->parity_set) {
> >>> + u32 epctl_reg = DOEPCTL(idx);
> >>> + u32 ctrl = readl(hsotg->regs + epctl_reg);
> >>> +
> >>> + if (ctrl & DXEPCTL_EOFRNUM)
> >>> + ctrl |= DXEPCTL_SETEVENFR;
> >>> + else
> >>> + ctrl |= DXEPCTL_SETODDFR;
> >>> + writel(ctrl, hsotg->regs + epctl_reg);
> >>> + }
> >>> + }
> >>> + writel(GINTSTS_INCOMPL_SOOUT, hsotg->regs + GINTSTS);
> >>> + }
> >>> +
> >>> /*
> >>> * if we've had fifo events, we should try and go around the
> >>> * loop again to see if there's any point in returning yet.
> >>> @@ -2667,6 +2712,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep
> >> *ep,
> >>> hs_ep->periodic = 0;
> >>> hs_ep->halted = 0;
> >>> hs_ep->interval = desc->bInterval;
> >>> + hs_ep->parity_set = 0;
> >>
> >>
> >> I'm not quite sure what the parity_set flag does in this patch.
> >> Shouldn't you be able to just toggle the even/odd frame when you get
> >> the interrupt?
> >>
> >> John
> >>
> >
> > When Transfer Complete interrupt is received, we have the correct parity.
> Therefore we set the flag and we stop toggling. The parity_set flag indicates
> whether we have the correct parity set.
> > Regards,
> >
> > Roman
> >
>
> I'm not sure that "parity" is the proper term in this context but I can't think of
> a more concise way to phrase it.
>
> What if the host switches again some time after the first xfer complete?
>
> What function or gadget driver are you using to test this? Did you test both
> the ISO IN and OUT case?
>
> John
>
>
This patch is related to isochronous or periodic transfers. So server uses the same microframe number and hence parity for transfers. I tested both IN and OUT directions using audio gadget on Altera SocKit and Broadcom development boards.
Regards,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists