| Message ID | 20170510084231.19302-6-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Peter Ujfalusi <peter.ujfalusi@ti.com> [170510 01:45]: > We have one register for each EP to set the maximum packet size for both > TX and RX. > If for example an RX programming would happen before the previous TX > transfer finishes we would reset the TX packet side. > > To fix this issue, only modify the TX or RX part of the register. Oh this is a nice one. I think we've always had this. Bin, care to merge this one as a fix? This should have: Fixes: 550a7375fe72 ("USB: Add MUSB and TUSB support") Tested-by: Tony Lindgren <tony@atomide.com> -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote: > We have one register for each EP to set the maximum packet size for both > TX and RX. > If for example an RX programming would happen before the previous TX > transfer finishes we would reset the TX packet side. > > To fix this issue, only modify the TX or RX part of the register. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/usb/musb/tusb6010_omap.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c > index db2e4c379ccf..4e1a6e4a61b8 100644 > --- a/drivers/usb/musb/tusb6010_omap.c > +++ b/drivers/usb/musb/tusb6010_omap.c > @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, > > if (chdat->tx) { > /* Send transfer_packet_sz packets at a time */ > - musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, > - chdat->transfer_packet_sz); > + u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); checkpatch.pl complains about declaration and assignment together. > + psize &= ~0x7ff; > + psize |= chdat->transfer_packet_sz; > + musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize); > > musb_writel(ep_conf, TUSB_EP_TX_OFFSET, > TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len)); > } else { > /* Receive transfer_packet_sz packets at a time */ > - musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, > - chdat->transfer_packet_sz << 16); > + u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); and at here too. > + psize &= ~(0x7ff << 16); > + psize |= (chdat->transfer_packet_sz << 16); > + musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize); > > musb_writel(ep_conf, TUSB_EP_RX_OFFSET, > TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len)); Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-05-10 at 12:07 -0500, Bin Liu wrote: > On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote: > > We have one register for each EP to set the maximum packet size for both > > TX and RX. > > If for example an RX programming would happen before the previous TX > > transfer finishes we would reset the TX packet side. > > > > To fix this issue, only modify the TX or RX part of the register. [] > > diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c [] > > @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, > > > > if (chdat->tx) { > > /* Send transfer_packet_sz packets at a time */ > > - musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, > > - chdat->transfer_packet_sz); > > + u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); > > checkpatch.pl complains about declaration and assignment together. No it doesn't. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017-05-11 02:16, Joe Perches wrote: > On Wed, 2017-05-10 at 12:07 -0500, Bin Liu wrote: >> On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote: >>> We have one register for each EP to set the maximum packet size for both >>> TX and RX. >>> If for example an RX programming would happen before the previous TX >>> transfer finishes we would reset the TX packet side. >>> >>> To fix this issue, only modify the TX or RX part of the register. > [] >>> diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c > [] >>> @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, >>> >>> if (chdat->tx) { >>> /* Send transfer_packet_sz packets at a time */ >>> - musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, >>> - chdat->transfer_packet_sz); >>> + u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); >> >> checkpatch.pl complains about declaration and assignment together. > > No it doesn't. It 'only' complains about: WARNING: Missing a blank line after declarations which is valid. - Péter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 11, 2017 at 09:19:17AM +0300, Peter Ujfalusi wrote: > > > On 2017-05-11 02:16, Joe Perches wrote: > >On Wed, 2017-05-10 at 12:07 -0500, Bin Liu wrote: > >>On Wed, May 10, 2017 at 11:42:27AM +0300, Peter Ujfalusi wrote: > >>>We have one register for each EP to set the maximum packet size for both > >>>TX and RX. > >>>If for example an RX programming would happen before the previous TX > >>>transfer finishes we would reset the TX packet side. > >>> > >>>To fix this issue, only modify the TX or RX part of the register. > >[] > >>>diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c > >[] > >>>@@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, > >>> if (chdat->tx) { > >>> /* Send transfer_packet_sz packets at a time */ > >>>- musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, > >>>- chdat->transfer_packet_sz); > >>>+ u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); > >> > >>checkpatch.pl complains about declaration and assignment together. > > > >No it doesn't. > > It 'only' complains about: > WARNING: Missing a blank line after declarations It was it. My bad, I was multi-tasking and didn't read the log carefully. > > which is valid. So will you update the patch to move the declaration to the beginning of the function to avoid this WARNING. I would just fix it locally if you prefer. Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bin, On 2017-05-11 17:12, Bin Liu wrote: >> which is valid. > > So will you update the patch to move the declaration to the beginning of > the function to avoid this WARNING. I would just fix it locally if you > prefer. I was waiting for Vinod or someone from the DMAengine guys to say something for the first patch to send the v3, but I guess I can do it right away. - Péter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 12, 2017 at 09:53:55AM +0300, Peter Ujfalusi wrote: > Bin, > > On 2017-05-11 17:12, Bin Liu wrote: > >>which is valid. > > > >So will you update the patch to move the declaration to the beginning of > >the function to avoid this WARNING. I would just fix it locally if you > >prefer. > > I was waiting for Vinod or someone from the DMAengine guys to say > something for the first patch to send the v3, but I guess I can do > it right away. Yeah, this patch can go to the -rc in next week, but the rest will go to -next, so you can send this patch seperately. Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c index db2e4c379ccf..4e1a6e4a61b8 100644 --- a/drivers/usb/musb/tusb6010_omap.c +++ b/drivers/usb/musb/tusb6010_omap.c @@ -389,15 +389,19 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, if (chdat->tx) { /* Send transfer_packet_sz packets at a time */ - musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, - chdat->transfer_packet_sz); + u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); + psize &= ~0x7ff; + psize |= chdat->transfer_packet_sz; + musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize); musb_writel(ep_conf, TUSB_EP_TX_OFFSET, TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len)); } else { /* Receive transfer_packet_sz packets at a time */ - musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, - chdat->transfer_packet_sz << 16); + u32 psize = musb_readl(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET); + psize &= ~(0x7ff << 16); + psize |= (chdat->transfer_packet_sz << 16); + musb_writel(ep_conf, TUSB_EP_MAX_PACKET_SIZE_OFFSET, psize); musb_writel(ep_conf, TUSB_EP_RX_OFFSET, TUSB_EP_CONFIG_XFR_SIZE(chdat->transfer_len));
We have one register for each EP to set the maximum packet size for both TX and RX. If for example an RX programming would happen before the previous TX transfer finishes we would reset the TX packet side. To fix this issue, only modify the TX or RX part of the register. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/usb/musb/tusb6010_omap.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)