Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel

[Bin Liu]

Peter,

On Wed, Nov 28, 2018 at 01:16:32PM +0200, Peter Ujfalusi wrote:
> Hi,
> 
> On 28/11/2018 13.15, Peter Ujfalusi wrote:
> 
> forgot to fix up Vinod's email address.
> 
> > 
> > 
> > On 12/11/2018 17.40, Bin Liu wrote:
> > 
> > Can you fix up the subject line to:
> > dmaengine: ti: cppi4: delete channel from pending list when stop channel
> > 
> >> The driver defines three states for a cppi channel.
> >> - idle: .chan_busy == 0 && not in .pending list
> >> - pending: .chan_busy == 0 && in .pending list
> >> - busy: .chan_busy == 1 && not in .pending list
> >>
> >> There are cases in which the cppi channel could be in the pending state
> >> when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
> >> is called.
> >>
> >> cppi41_stop_chan() has a bug for these cases to set channels to idle state.
> >> It only checks the .chan_busy flag, but not the .pending list, then later
> >> when cppi41_runtime_resume() is called the channels in .pending list will
> >> be transitioned to busy state.
> >>
> >> Removing channels from the .pending list solves the problem.
> > 
> > So, let me see if I understand this correctly:
> > - client issued a transfer _after_ the cppi4 driver is suspended
> > - cppi41_dma_issue_pending() will place it to pending list and will not
> > start the transfer right away as cdd->is_suspended is true.
> > - on resume the cppi4 will pick up the pending transfers from the
> > pending list
> > 
> > This is so far a sane thing to do.
> > 
> > If I guess right, then after the issue_pending the client driver will
> > call terminate_all, presumably from it's suspend callback?
> > 
> > As per the purpose of terminate_all we should terminated all future
> > transfers on the channel, so clearing the pending list is the correct
> > thing to do.
> > 
> > With the fixed subject:
> > Reviewed-by: Peter Ujfalusi 
> > 
> > I have one question:
> > 
> >> Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is 
> >> busy")
> >> Cc: sta...@vger.kernel.org # v3.15+
> >> Signed-off-by: Bin Liu 
> >> ---
> >>  drivers/dma/ti/cppi41.c | 16 +++-
> >>  1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> >> index 1497da367710..e507ec36c0d3 100644
> >> --- a/drivers/dma/ti/cppi41.c
> >> +++ b/drivers/dma/ti/cppi41.c
> >> @@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
> >>  
> >>desc_phys = lower_32_bits(c->desc_phys);
> >>desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
> >> -  if (!cdd->chan_busy[desc_num])
> >> +  if (!cdd->chan_busy[desc_num]) {
> >> +  struct cppi41_channel *cc, *_ct;
> >> +
> >> +  /*
> >> +   * channels might still be in the pendling list if
> >> +   * cppi41_dma_issue_pending() is called after
> >> +   * cppi41_runtime_suspend() is called
> >> +   */
> >> +  list_for_each_entry_safe(cc, _ct, >pending, node) {
> >> +  if (cc != c)
> >> +  continue;
> >> +  list_del(>node);
> > 
> > If we delete from the pending list, are we going to leak memory?
> > I'm not familiar with the cppi4, it might not be an issue for it.

Here is no memory leak.
The elements added to the pending list are cppi41 channels which are
allocated in driver _probe(). No dynamic memory allocation happening
when operating this pending list.

Regards,
-Bin.

[PATCH] usb: musb: dsps: fix otg state machine

[Bin Liu]

Due to lack of ID pin interrupt event on AM335x devices, the musb dsps
driver uses polling to detect usb device attach for dual-role port.

But in the case if a micro-A cable adapter is attached without a USB device
attached to the cable, the musb state machine gets stuck in a_wait_vrise
state waiting for the MUSB_CONNECT interrupt which won't happen due to the
usb device is not attached. The state is stuck in a_wait_vrise even after
the micro-A cable is detached, which could cause VBUS retention if then the
dual-role port is attached to a host port.

To fix the problem, make a_wait_vrise as a transient state, then move the
state to either a_wait_bcon for host port or a_idle state for dual-role
port, if no usb device is attached to the port.

Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_dsps.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 23a0df79ef21..1e6d78b1334e 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -227,8 +227,13 @@ static int dsps_check_status(struct musb *musb, void 
*unused)
 
switch (musb->xceiv->otg->state) {
case OTG_STATE_A_WAIT_VRISE:
-   dsps_mod_timer_optional(glue);
-   break;
+   if (musb->port_mode == MUSB_HOST) {
+   musb->xceiv->otg->state = OTG_STATE_A_WAIT_BCON;
+   dsps_mod_timer_optional(glue);
+   break;
+   }
+   /* fall through */
+
case OTG_STATE_A_WAIT_BCON:
/* keep VBUS on for host-only mode */
if (musb->port_mode == MUSB_HOST) {
-- 
2.17.1

Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel

[Bin Liu]

Hi,

On Sat, Nov 24, 2018 at 07:58:03PM +0530, Vinod Koul wrote:
> On 12-11-18, 09:43, Bin Liu wrote:
> > The driver defines three states for a cppi channel.
> > - idle: .chan_busy == 0 && not in .pending list
> > - pending: .chan_busy == 0 && in .pending list
> > - busy: .chan_busy == 1 && not in .pending list
> > 
> > There are cases in which the cppi channel could be in the pending state
> > when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
> > is called.
> > 
> > cppi41_stop_chan() has a bug for these cases to set channels to idle state.
> > It only checks the .chan_busy flag, but not the .pending list, then later
> > when cppi41_runtime_resume() is called the channels in .pending list will
> > be transitioned to busy state.
> > 
> > Removing channels from the .pending list solves the problem.
> 
> I would like some testing, given that intent is to go to stable.
> Peter..?

FYI, this cppi41 dma driver is *only* used by musb controller driver. In
the past I received multiple reports for this issue, but I wasn't able
to reproduce it using similar test cases. The only way I could trigger
the issue is to do system suspend/resume test on AM335x with a USB hub
attached to the usb host port. This issue only happens once in the very
*first* time suspend/resume test after reboot.

Regards,
-Bin.

Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel

[Bin Liu]

Sorry, please ignore this. Used incorrect Vinod email address.

On Mon, Nov 12, 2018 at 09:40:49AM -0600, Bin Liu wrote:
> The driver defines three states for a cppi channel.
> - idle: .chan_busy == 0 && not in .pending list
> - pending: .chan_busy == 0 && in .pending list
> - busy: .chan_busy == 1 && not in .pending list
> 
> There are cases in which the cppi channel could be in the pending state
> when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
> is called.
> 
> cppi41_stop_chan() has a bug for these cases to set channels to idle state.
> It only checks the .chan_busy flag, but not the .pending list, then later
> when cppi41_runtime_resume() is called the channels in .pending list will
> be transitioned to busy state.
> 
> Removing channels from the .pending list solves the problem.
> 
> Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy")
> Cc: sta...@vger.kernel.org # v3.15+
> Signed-off-by: Bin Liu 
> ---
>  drivers/dma/ti/cppi41.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
> index 1497da367710..e507ec36c0d3 100644
> --- a/drivers/dma/ti/cppi41.c
> +++ b/drivers/dma/ti/cppi41.c
> @@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
>  
>   desc_phys = lower_32_bits(c->desc_phys);
>   desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
> - if (!cdd->chan_busy[desc_num])
> + if (!cdd->chan_busy[desc_num]) {
> + struct cppi41_channel *cc, *_ct;
> +
> + /*
> +  * channels might still be in the pendling list if
> +  * cppi41_dma_issue_pending() is called after
> +  * cppi41_runtime_suspend() is called
> +  */
> + list_for_each_entry_safe(cc, _ct, >pending, node) {
> + if (cc != c)
> + continue;
> + list_del(>node);
> + break;
> + }
>   return 0;
> + }
>  
>   ret = cppi41_tear_down_chan(c);
>   if (ret)
> -- 
> 2.17.1
> 

[PATCH] dma: cppi41: delete channel from pending list when stop channel

[Bin Liu]

The driver defines three states for a cppi channel.
- idle: .chan_busy == 0 && not in .pending list
- pending: .chan_busy == 0 && in .pending list
- busy: .chan_busy == 1 && not in .pending list

There are cases in which the cppi channel could be in the pending state
when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
is called.

cppi41_stop_chan() has a bug for these cases to set channels to idle state.
It only checks the .chan_busy flag, but not the .pending list, then later
when cppi41_runtime_resume() is called the channels in .pending list will
be transitioned to busy state.

Removing channels from the .pending list solves the problem.

Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy")
Cc: sta...@vger.kernel.org # v3.15+
Signed-off-by: Bin Liu 
---
 drivers/dma/ti/cppi41.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index 1497da367710..e507ec36c0d3 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
 
desc_phys = lower_32_bits(c->desc_phys);
desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
-   if (!cdd->chan_busy[desc_num])
+   if (!cdd->chan_busy[desc_num]) {
+   struct cppi41_channel *cc, *_ct;
+
+   /*
+* channels might still be in the pendling list if
+* cppi41_dma_issue_pending() is called after
+* cppi41_runtime_suspend() is called
+*/
+   list_for_each_entry_safe(cc, _ct, >pending, node) {
+   if (cc != c)
+   continue;
+   list_del(>node);
+   break;
+   }
return 0;
+   }
 
ret = cppi41_tear_down_chan(c);
if (ret)
-- 
2.17.1

[PATCH] dma: cppi41: delete channel from pending list when stop channel

[Bin Liu]

The driver defines three states for a cppi channel.
- idle: .chan_busy == 0 && not in .pending list
- pending: .chan_busy == 0 && in .pending list
- busy: .chan_busy == 1 && not in .pending list

There are cases in which the cppi channel could be in the pending state
when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend()
is called.

cppi41_stop_chan() has a bug for these cases to set channels to idle state.
It only checks the .chan_busy flag, but not the .pending list, then later
when cppi41_runtime_resume() is called the channels in .pending list will
be transitioned to busy state.

Removing channels from the .pending list solves the problem.

Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy")
Cc: sta...@vger.kernel.org # v3.15+
Signed-off-by: Bin Liu 
---
 drivers/dma/ti/cppi41.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c
index 1497da367710..e507ec36c0d3 100644
--- a/drivers/dma/ti/cppi41.c
+++ b/drivers/dma/ti/cppi41.c
@@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan)
 
desc_phys = lower_32_bits(c->desc_phys);
desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc);
-   if (!cdd->chan_busy[desc_num])
+   if (!cdd->chan_busy[desc_num]) {
+   struct cppi41_channel *cc, *_ct;
+
+   /*
+* channels might still be in the pendling list if
+* cppi41_dma_issue_pending() is called after
+* cppi41_runtime_suspend() is called
+*/
+   list_for_each_entry_safe(cc, _ct, >pending, node) {
+   if (cc != c)
+   continue;
+   list_del(>node);
+   break;
+   }
return 0;
+   }
 
ret = cppi41_tear_down_chan(c);
if (ret)
-- 
2.17.1

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

Joel,

On Wed, Oct 24, 2018 at 07:19:01PM +0200, Josep M. Mirats Tur wrote:
> Hi Joel,
> 
> The camera I have provides 640x480 as its lowest resolution.
> Hope the issue can be solved.

If your issue was same as mine - the uvc layer holds spinlock for too
long in urb->complete() which prevents musb to schedule next IN
transaction on time, it won't be that easy to solve. :( I had a try last
year, but the solution caused other problem, then haven't looked at this
issue since then.

Regards,
-Bin.


> Regards
> 
> On Wed, Oct 24, 2018 at 5:31 PM Joel Pepper  
> wrote:
> >
> > Hi Joseph, Hi Bin,
> >
> > I am currently investigating a bug with the BeagleBoneBlack (which also
> > uses the AM335x) and a Logitech Webcam, which might be connected.
> >
> > I am using a Logitech C270 and if I set the format to YUYV and the
> > framesize to 544x288 or larger (including specifically 640x480), any
> > ioctl VIDIOC_DQBUF will hang indefinetely. This might be the underlying
> > source of the timeout Joseph encountered.
> >
> > Joseph,
> >
> > have you tried setting the camera to the lowest resolution it can go? If
> > low resolutions work, but higher ones, like 640x480 don't, we might be
> > chasing the same problem. If not, I will look into my problem separately
> > and open a new thread.
> >
> >
> > Bin,
> >
> > my case happened using ISOC transfers, but my preliminary investigation
> > has shown lots of ISOC traffic flowing, apparently successfuly from the
> > camera to the BBB, which makes me suspect a problem closer to the
> > v4l2/musb boundary, but I'm still reading into the code. If you would
> > like to investigate that further, the C270 is rather affordable, but I
> > will gladly provide any details, usbmon captures etc. that you require.
> >
> >
> > Cheers,
> >
> > Joel Pepper
> >
> > On 19.10.2018 20:06, Josep M. Mirats Tur wrote:
> > > Hi Bin,
> > >
> > >> Do you know what is the usb throughput required in your test case? for
> > >> example image format, resolution, frame rate? Is the usb data transfer
> > >> bulk or isoch?
> > > Yes, the image format is 640 x 480
> > > The camera can run up to 30fps, although I do not need it and set up it 
> > > to 5
> > > The Usb data transfer is "bulk"
> > >
> > > Regards
> > > Josep M.
> > >
> > > On Fri, Oct 19, 2018 at 8:01 PM Bin Liu  wrote:
> > >> On Fri, Oct 19, 2018 at 07:48:20PM +0200, Josep M. Mirats Tur wrote:
> > >>> Hi,
> > >>>
> > >>> Yes, this would be the link:
> > >>> http://shop-orbbec3d-com.3dcartstores.com/Astra-Mini_p_40.html
> > >> Okay, It is about $160, a bit more than my bugdet...
> > >>
> > >>> In the meanwhile I'll check my application with other board.
> > >>>
> > >>> Do you think there might be a problem with the processor frequency?
> > >>> I've also crossing e-mails with ORBBEC engineers, and their only
> > >>> answer has been to suggest using an ARM processor capable of 1.8GHz
> > >>> Beagle is capable of 1GHz
> > >>> But I can not see why is this so important as long as USB is well
> > >>> synchronized and interrupts and driver work well. (Actually I'm not
> > >>> familiar with ARM USB system, is just intuition)
> > >> I guess the cpu clock requirement is only to ensure the whole system is
> > >> capable to process the usb packets, depending on the image resolution,
> > >> fps, and any other data processing. But lower cpu clock shouldn't cause
> > >> such problem that the usb controller generates rx interrupt but fifo has
> > >> no usb packet. If ARM cannot keep up, which doesn't send IN requests on
> > >> time, the camera should just drop data.
> > >>
> > >> Do you know what is the usb throughput required in your test case? for
> > >> example image format, resolution, frame rate? Is the usb data transfer
> > >> bulk or isoch?
> > >>
> > >> Regards,
> > >> -Bin.
> >
> >

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

Joel,

On Wed, Oct 24, 2018 at 05:31:25PM +0200, Joel Pepper wrote:
> Hi Joseph, Hi Bin,
> 
> I am currently investigating a bug with the BeagleBoneBlack (which also
> uses the AM335x) and a Logitech Webcam, which might be connected.
> 
> I am using a Logitech C270 and if I set the format to YUYV and the
> framesize to 544x288 or larger (including specifically 640x480), any
> ioctl VIDIOC_DQBUF will hang indefinetely. This might be the underlying
> source of the timeout Joseph encountered.
> 
> Joseph,
> 
> have you tried setting the camera to the lowest resolution it can go? If
> low resolutions work, but higher ones, like 640x480 don't, we might be
> chasing the same problem. If not, I will look into my problem separately
> and open a new thread.
> 
> 
> Bin,
> 
> my case happened using ISOC transfers, but my preliminary investigation
> has shown lots of ISOC traffic flowing, apparently successfuly from the
> camera to the BBB, which makes me suspect a problem closer to the
> v4l2/musb boundary, but I'm still reading into the code. If you would
> like to investigate that further, the C270 is rather affordable, but I
> will gladly provide any details, usbmon captures etc. that you require.

I am aware of an issue for 640x480@30fps with some cameras. I am in
full-day meetings this week. Once I get time, I will provide more
details for the issue I know of to see if you have face the same.

Regards,
-Bin.

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

On Fri, Oct 19, 2018 at 02:26:51PM -0400, Alan Stern wrote:
> On Fri, 19 Oct 2018, Bin Liu wrote:
> 
> > On Fri, Oct 19, 2018 at 07:19:17PM +0200, Josep M. Mirats Tur wrote:
> > > Hi Bin,
> > > 
> > > I included the following line in the /boot/uEnv.txt file (attached for
> > > your info):
> > > cmdline=musb_hdrc.use_dma=0
> > 
> > I am not sure about the 'cmdline=' part. Anyway, after your board is
> > booted, use command 'cat /proc/cmdline' to check if your setting is
> > correct.
> 
> There already was a "cmdline=" entry in the file:
> 
> > > cmdline=coherent_pool=1M net.ifnames=0 quiet
> > > 
> > > #In the event of edid real failures, uncomment this next line:
> > > #cmdline=coherent_pool=1M net.ifnames=0 quiet video=HDMI-A-1:1024x768@60e
> > > 
> > > #Use an overlayfs on top of a read-only root filesystem:
> > > #cmdline=coherent_pool=1M net.ifnames=0 quiet overlayroot=tmpfs
> > > 
> > > ##enable Generic eMMC Flasher:
> > > ##make sure, these tools are installed: dosfstools rsync
> > > #cmdline=init=/opt/scripts/tools/eMMC/init-eMMC-flasher-v3.sh
> > > 
> > > #jmmt disable dma for musb
> > > cmdline=musb_hdrc.use_dma=0
> 
> The "musb_hdrc.use_dma=0" should have been appended to the earlier 
> entry, not added separately.

right, but this is just a testing to disable cppi dma, other existing
cmdline params seem not related to usb, so I think it is okay to
overwrite.

Regards,
-Bin.

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

On Fri, Oct 19, 2018 at 07:48:20PM +0200, Josep M. Mirats Tur wrote:
> Hi,
> 
> Yes, this would be the link:
> http://shop-orbbec3d-com.3dcartstores.com/Astra-Mini_p_40.html

Okay, It is about $160, a bit more than my bugdet...

> In the meanwhile I'll check my application with other board.
> 
> Do you think there might be a problem with the processor frequency?
> I've also crossing e-mails with ORBBEC engineers, and their only
> answer has been to suggest using an ARM processor capable of 1.8GHz
> Beagle is capable of 1GHz
> But I can not see why is this so important as long as USB is well
> synchronized and interrupts and driver work well. (Actually I'm not
> familiar with ARM USB system, is just intuition)

I guess the cpu clock requirement is only to ensure the whole system is
capable to process the usb packets, depending on the image resolution,
fps, and any other data processing. But lower cpu clock shouldn't cause
such problem that the usb controller generates rx interrupt but fifo has
no usb packet. If ARM cannot keep up, which doesn't send IN requests on
time, the camera should just drop data.

Do you know what is the usb throughput required in your test case? for
example image format, resolution, frame rate? Is the usb data transfer
bulk or isoch?

Regards,
-Bin.

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

On Fri, Oct 19, 2018 at 07:33:47PM +0200, Josep M. Mirats Tur wrote:
> Hi,
> 
> It seems correct, this is the output of  sudo cat /proc/cmdline:
> console=ttyO0,115200n8 bone_capemgr.uboot_capemgr_enabled=1
> root=/dev/mmcblk1p1 ro rootfstype=ext4 rootwait musb_hdrc.use_dma=0

It looks correct. So the issue is not related to CPPI dma.
Do you have a link to buy the camera? If I can afford it, I might buy
one to take a look at this problem.

Regards,
-Bin.

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

On Fri, Oct 19, 2018 at 07:19:17PM +0200, Josep M. Mirats Tur wrote:
> Hi Bin,
> 
> I included the following line in the /boot/uEnv.txt file (attached for
> your info):
> cmdline=musb_hdrc.use_dma=0

I am not sure about the 'cmdline=' part. Anyway, after your board is
booted, use command 'cat /proc/cmdline' to check if your setting is
correct.

Regards,
-Bin.
> 
> Is this format correct?
> I rebooted and checked again but still the same errors from dmesg:
> 
> [  228.863997] usb 1-1: new high-speed USB device number 2 using musb-hdrc
> [  229.016708] usb 1-1: New USB device found, idVendor=2bc5, idProduct=0404
> [  229.023512] usb 1-1: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=0
> [  229.032841] usb 1-1: Product: ORBBEC Depth Sensor
> [  229.038752] usb 1-1: Manufacturer: Orbbec(R)
> [  249.221761] musb_host_rx 1965: Rx interrupt with no errors or packet!
> [  249.228346] musb_host_rx 1965: Rx interrupt with no errors or packet!
> [  249.238329] musb_host_rx 1965: Rx interrupt with no errors or packet!
> [  249.249025] musb_ep_program 916: broken !rx_reinit, ep2 csr 0003
> On Fri, Oct 19, 2018 at 7:00 PM Bin Liu  wrote:
> >
> > On Fri, Oct 19, 2018 at 06:31:15PM +0200, Josep M. Mirats Tur wrote:
> > > Bin,
> > >
> > > I'm sorry but I cannot find that menuconfig at the Beagle...
> >
> > Never mind, but add the following to your UBoot bootargs env instead to
> > disable dma.
> >
> > musb_hdrc.use_dma=0
> >
> > Regards,
> > -Bin.
> >

> #Docs: http://elinux.org/Beagleboard:U-boot_partitioning_layout_2.0
> 
> uname_r=4.14.71-ti-r80
> #uuid=
> #dtb=
> 
> ###U-Boot Overlays###
> ###Documentation: 
> http://elinux.org/Beagleboard:BeagleBoneBlack_Debian#U-Boot_Overlays
> ###Master Enable
> enable_uboot_overlays=1
> ###
> ###Overide capes with eeprom
> #uboot_overlay_addr0=/lib/firmware/.dtbo
> #uboot_overlay_addr1=/lib/firmware/.dtbo
> #uboot_overlay_addr2=/lib/firmware/.dtbo
> #uboot_overlay_addr3=/lib/firmware/.dtbo
> ###
> ###Additional custom capes
> #uboot_overlay_addr4=/lib/firmware/.dtbo
> #uboot_overlay_addr5=/lib/firmware/.dtbo
> #uboot_overlay_addr6=/lib/firmware/.dtbo
> #uboot_overlay_addr7=/lib/firmware/.dtbo
> ###
> ###Custom Cape
> #dtb_overlay=/lib/firmware/.dtbo
> ###
> ###Disable auto loading of virtual capes (emmc/video/wireless/adc)
> #disable_uboot_overlay_emmc=1
> disable_uboot_overlay_video=1
> disable_uboot_overlay_audio=1
> disable_uboot_overlay_wireless=1
> #disable_uboot_overlay_adc=1
> ###
> ###PRUSS OPTIONS
> ###pru_rproc (4.4.x-ti kernel)
> #uboot_overlay_pru=/lib/firmware/AM335X-PRU-RPROC-4-4-TI-00A0.dtbo
> ###pru_rproc (4.14.x-ti kernel)
> uboot_overlay_pru=/lib/firmware/AM335X-PRU-RPROC-4-14-TI-00A0.dtbo
> ###pru_uio (4.4.x-ti, 4.14.x-ti & mainline/bone kernel)
> uboot_overlay_pru=/lib/firmware/AM335X-PRU-UIO-00A0.dtbo
> ###
> ###Cape Universal Enable
> enable_uboot_cape_universal=1
> ###
> ###Debug: disable uboot autoload of Cape
> #disable_uboot_overlay_addr0=1
> #disable_uboot_overlay_addr1=1
> #disable_uboot_overlay_addr2=1
> #disable_uboot_overlay_addr3=1
> ###
> ###U-Boot fdt tweaks... (6 = 384KB)
> #uboot_fdt_buffer=0x6
> ###U-Boot Overlays###
> 
> cmdline=coherent_pool=1M net.ifnames=0 quiet
> 
> #In the event of edid real failures, uncomment this next line:
> #cmdline=coherent_pool=1M net.ifnames=0 quiet video=HDMI-A-1:1024x768@60e
> 
> #Use an overlayfs on top of a read-only root filesystem:
> #cmdline=coherent_pool=1M net.ifnames=0 quiet overlayroot=tmpfs
> 
> ##enable Generic eMMC Flasher:
> ##make sure, these tools are installed: dosfstools rsync
> #cmdline=init=/opt/scripts/tools/eMMC/init-eMMC-flasher-v3.sh
> 
> #jmmt disable dma for musb
> cmdline=musb_hdrc.use_dma=0

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

On Fri, Oct 19, 2018 at 06:31:15PM +0200, Josep M. Mirats Tur wrote:
> Bin,
> 
> I'm sorry but I cannot find that menuconfig at the Beagle...

Never mind, but add the following to your UBoot bootargs env instead to
disable dma.

musb_hdrc.use_dma=0

Regards,
-Bin.

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

On Fri, Oct 19, 2018 at 06:17:31PM +0200, Josep M. Mirats Tur wrote:
> Hi Bin,
> 
> I was referring to the output of usbmon I attached in the previous
> message (attached again)
> 
> I'm sorry but I do not know how to acces the kernel menuconfig to
> disable the MUSB CPPI DMA, how should I do that?

In kernel menuconfig, go to

Device Drivers  --->
[*] USB support  --->
   Inventra Highspeed Dual Role Controller (TI, ADI, AW, ...)
...
 TI DSPS platforms
*** MUSB DMA mode ***
[*] Disable DMA (always use PIO)

Please ensure the last option ("Disable DMA") is checked.

Regards,
-Bin.

Re: Error reading USB camera in BeagleBone with ARM Debian

[Bin Liu]

Hi,

On Fri, Oct 19, 2018 at 05:35:29PM +0200, Josep M. Mirats Tur wrote:
> Hi,
> 
> Thanks again for your answer.
> 
> I changed to the newest Debian image available for BeagleBone boards:
> The Debian 9.5 -  4.14.71-ti-r80
> I disabled the usb "autosuspend" by issuing the command
> echo -1 | sudo tee /sys/module/usbcore/parameters/autosuspend
> 
> I cannot  say if now there is data coming from the USB  as I cannot
> understand the output

what output?

> But I still get errors from the driver when I check the dmesg:
> 
> [ 6340.997467] usb 1-1: usbfs: usb_submit_urb returned -121
> [ 6341.198347] usb 1-1: usbfs: usb_submit_urb returned -121
> [ 6342.299444] usb 1-1: usbfs: usb_submit_urb returned -121
> [ 6343.867458] usb 1-1: usbfs: usb_submit_urb returned -121
> [ 6344.168455] usb 1-1: usbfs: usb_submit_urb returned -121
> [ 6596.714269] musb_host_rx 1965: Rx interrupt with no errors or packet!
> [ 6596.720873] musb_host_rx 1965: Rx interrupt with no errors or packet!

This message means RX EP interrupt happens, but the RX FIFO has no data.
I don't have any case showing this problem before, so not sure what
causes the problem.

Can you please try to disable MUSB CPPI DMA to see how it goes? You can
disable it in kernel menuconfig.

Regards,
-Bin.

[PATCH 1/1] usb: musb: dsps: do not disable CPPI41 irq in driver teardown

[Bin Liu]

TI AM335x CPPI 4.1 module uses a single register bit for CPPI interrupts
in both musb controllers. So disabling the CPPI irq in one musb driver
breaks the other musb module.

Since musb is already disabled before tearing down dma controller in
musb_remove(), it is safe to not disable CPPI irq in
musb_dma_controller_destroy().

Fixes: 255348289f71 ("usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in DSPS")
Cc: sta...@vger.kernel.org
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_dsps.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index df827ff57b0d..23a0df79ef21 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -658,16 +658,6 @@ dsps_dma_controller_create(struct musb *musb, void __iomem 
*base)
return controller;
 }
 
-static void dsps_dma_controller_destroy(struct dma_controller *c)
-{
-   struct musb *musb = c->musb;
-   struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
-   void __iomem *usbss_base = glue->usbss_base;
-
-   musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
-   cppi41_dma_controller_destroy(c);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static void dsps_dma_controller_suspend(struct dsps_glue *glue)
 {
@@ -697,7 +687,7 @@ static struct musb_platform_ops dsps_ops = {
 
 #ifdef CONFIG_USB_TI_CPPI41_DMA
.dma_init   = dsps_dma_controller_create,
-   .dma_exit   = dsps_dma_controller_destroy,
+   .dma_exit   = cppi41_dma_controller_destroy,
 #endif
.enable = dsps_musb_enable,
.disable= dsps_musb_disable,
-- 
2.17.1

[PATCH 0/1] musb fixes for v4.19-rc5

[Bin Liu]

Hi Greg,

Here is one musb fix for v4.19 rc, which fixes the interfering issue among the
two musb controllers in dsps glue driver. Please let me know if any change is
needed.

Regards,
-Bin.
---

Bin Liu (1):
  usb: musb: dsps: do not disable CPPI41 irq in driver teardown

 drivers/usb/musb/musb_dsps.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

-- 
2.17.1

Re: musb_hdrc HNP?

[Bin Liu]

Hi,

On Fri, Aug 31, 2018 at 06:35:50AM +, Takashi Matsuzawa wrote:
> Hello.
> I just confirmed what I wanted to see.
> I could do lsusb to list A-device (from b_host) and B-device (from a_host).
> Suspending from either side kicked role change between A-device and B-device 
> (in both direction).
> 
> I needed to wait 20ms after B-device seeing MUSB_INTR_CONNECT and before 
> calling musb_host_poke_root_hub().
> 
> I suppose seeing CONNECT inturrupt B-device can expect that A-device is 
> reset, but it may take some time and B-device may need to wait.
> On technial nodes, this is mentioned as something like this:
> 
> "Reset Signaling. If the RESET bit in the POWER register (bit 3) is set while 
> the controller is in Host mode, it will generate Reset signaling on the bus. 
> If the HSENAB bit in the POWER register (bit 5) was set, it will also try to 
> negotiate for high-speed operation. The software should keep the RESET bit 
> set for at least 20 ms to ensure correct resetting of the target device."
> 
> Note I still see errors like below after role change (b_host -> 
> b_peripheral), perhaps some necessary cleanup is not there.
> But anyway they role-switched in both direction..

Is commit [1] reverted or not in this experiment?

[1] 0a9134bd733b usb: musb: disable otg protocol support

> 
> [ 1204.225843] usb 2-1: USB disconnect, device number 7
> [ 1204.274238] hub 2-0:1.0: USB hub found
> [ 1204.282564] hub 2-0:1.0: 1 port detected
> [ 1204.496301] musb-hdrc musb-hdrc.1: Babble
> [ 1204.622799] musb_h_ep0_irq 1192: no URB for end 0
> [ 1208.474661] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1210.063965] zero gadget: high-speed config #3: source/sink
> [ 1212.566697] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1212.573607] usb usb2-port1: attempt power cycle
> [ 1216.974544] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1221.066539] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1221.073328] usb usb2-port1: unable to enumerate USB device
> debian@beaglebone:~/wk-b$ 

Regards,
-Bin.

> ===
> 
> ________
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Takashi Matsuzawa 
> Sent: Friday, August 31, 2018 1:50 PM
> To: Bin Liu
> Cc: linux-usb@vger.kernel.org
> Subject: Re: musb_hdrc HNP?
> 
> Hello.
> FYI.  I made a progress on this, but no solution yet.
> 
> >The smartphone does use HNP, it is not iPhone Carplay, correct?
> 
> At this point, I am trying to see original HNP behavior between two 
> pocketbeagles.
> (After seeing it works, I plan to replace B-device with a phone, and so 
> customization on A-device stack.)
> 
> >1. MUSB uses one register bit to report babble and reset event, so driver 
> >bug could report bus reset as babble if it doesn't trace the controller role 
> >correctly;
> 
> As mentioned below, it may be related to how driver on B-device responds to 
> RESET/BABBLE interrupts (or its sideeffets).
> 
> 1) I could see CONFIG_USB_OTG was not set on "Debian 9.4 2018-06-17" image so 
> I turned it on.
> 
> This improved the situation a bit, like A-device side b_hnp_enable flag 
> (which is set when B-device b_hnp_enable is set.)
> 
> 2) Now I can see A-device and B-device turns to expected modes.
> 
> A-device:
> 
> a_host -> a_peripheral
> After suspending port, sees DISCONNECT and RESET events.
> 
> B-device:
> 
> b_peripheral -> b_host
> Sees SUSPEND, CONNECT events.
> 
> 3) But I do not see B-device enumerate A-device at this point, and instead on 
> B-device (now b_host) RESET(or BUBBLE) events are seen.
> Then after that, immediately, SUSPEND is seen on A-device, looks like now 
> A-device is resuming as a_host and B-device back to b_peripheral.
> 
> 4) I expect at 2) B-device should enumerate A-device and both stays in new 
> mode (and I can, say do lsusb on B-device and see A-device listed), but it 
> does not happen.
> 
> Ignoring RESET(BUBBLE) events on B-device (b_host) at 3) did not improve the 
> situgation.  (Now I see SUSPEND on B-device instead of DISCONNECT.)
> 
> It may be that driver behavior after 2) (to be initiated as a_peripheral and 
> b_host and restarting) having some problem.
> 
> 
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Bin Liu 
> Sent: Monday, August 27, 2018 10:33 PM
> To: Takashi Matsuzawa
> Cc: linux-usb@vger.kernel.org
> Subject: Re: musb_hdrc HNP?
> 
> Hi,
> 
> On Mon, Aug 27, 2018 at 12:57:55AM +, Takashi Matsuzawa wrote:
> > Thank you for your suggestion.
> > Yes, I am aware that full-OTG support code is

Re: [PATCH v2] usb: musb: gadget: Fix big-endian arch issue

[Bin Liu]

Hi,

On Fri, Aug 03, 2018 at 09:03:36AM +, Alexey Spirkov wrote:
> Existing code is not applicable to big-endian machines
> ctrlrequest fields received in USB endian - i.e. in little-endian
> and should be converted to cpu endianness before usage.
> 
> Signed-off-by: Alexey Spirkov 
> ---
>  drivers/usb/musb/musb_gadget_ep0.c | 29 ++---
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_gadget_ep0.c 
> b/drivers/usb/musb/musb_gadget_ep0.c
> index 91a5027..90abacb 100644
> --- a/drivers/usb/musb/musb_gadget_ep0.c
> +++ b/drivers/usb/musb/musb_gadget_ep0.c
> @@ -82,7 +82,7 @@ static int service_tx_status_request(
> u16 tmp;
 ^
The patch cannot be applied. All the tabs are converted to whitespaces.

Regards,
-Bin.

Re: musb_hdrc HNP?

[Bin Liu]

Hi,

On Mon, Aug 27, 2018 at 12:57:55AM +, Takashi Matsuzawa wrote:
> Thank you for your suggestion.
> Yes, I am aware that full-OTG support code is being wiped out of the
> latest mainline kernels.

Okay. Let me know if reverting that patch can magically make HNP works.

> I am trying this for smartphone connectivity where OTG based
> role-switch is being used, which may not be of interest of everyone.

The smartphone does use HNP, it is not iPhone Carplay, correct?

> I picked pocketbeagle since it has 2.0 OTG controller (without hub),
> which matched what I wanted to prototype.

Pocketbeagle should be good, it uses TI AM335x which is the device I
have.

> (If anyone is aware similar low-cost board with proven kernel version,
> I would interested in hearing about it.)
> 
> I think I try debugging a bit further through ftrace, etc. and bus
> monitoring.
> 
> One thing I am curious is the "Babble" errors.
> If they are caused by hardware (e.g. noise on power line), they may
> not be solved by software (what it can do is recovering/reset from
> failure).

I highly doubt the babble error is caused by hardware. There are mainly
two types of sw bugs which cause babble.

1. MUSB uses one register bit to report babble and reset event, so
driver bug could report bus reset as babble if it doesn't trace the
controller role correctly;

2. true babble events due to controller misconfiguration at runtime.

> If it is likely to be caused by such, I may try adding ferite beeds,
> etc. to my prototype to see if anything change.

First you can try to liminate some variables, for example, disable CPPI
DMA, disable usb runtime PM.

Good luck.

Regards,
-Bin.

> ________
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Bin Liu 
> Sent: Friday, August 24, 2018 10:43 PM
> To: Takashi Matsuzawa
> Cc: linux-usb@vger.kernel.org
> Subject: Re: musb_hdrc HNP?
> 
> Hi,
> 
> On Thu, Aug 23, 2018 at 10:06:50PM +, Takashi Matsuzawa wrote:
> > Hello.
> >
> > I am trying HNP (host -> peripheral role change) using two
> > PocketBeagles, but without success.
> 
> Well, you are the very first one that I have ever heard who tries to use
> HNP on musb, at least on musb_dsps.
> 
> > If there any idea on where I, where should I ask this, or how can I
> > debug / fix, I really appreciate.
> 
> Being said, the controller itself does support HNP and other OTG
> protocols, but the musb driver might not, or have been broken for a very
> long time, since HNP probably never been tested on musb_dsps.
> 
> If you use kernel v4.18+, you can try to revert commit
> 
> 0a9134bd733b usb: musb: disable otg protocol support
> 
> to see if HNP works.
> 
> Regards,
> -Bin.
> 
> >
> > 1) What I did
> >
> > Two PocketBeagles (running the default Debian image).
> > Both added 2nd USB ports (musb_hdrc.1).
> > DTBs were modified so that the 2nd musb_hdrcs have dr_mode = "otg".
> > musb_dsps glue port seems to be used.
> > Tech reference manual says the SoC's musb core supports HNP.
> >
> > One is A (by GNDing ID pin) and another B, connected with USB cable.
> > modprobe g_zero on both so that VBUS power is on, and A becomes a_host
> > and b becomes b_peripheral (as read through debugfs mode value).
> >
> > Then I did the following, expecting HNP happens and A bevcomes
> > a_peripheral and B bcomes b_host:
> >
> > i) Send b_hnp_enable request on A.
> > (This does set DEVCTL.HOSTREQ bits on B's musb_hdrc.1 and maybe doing
> > some additinoal things in musb driver on B.)
> >
> > ii) Set POWER.SUSPENDM bit on A's musb_hdrc.1
> > (According to TRM, this should tell B's musb core to initiate HNP.)
> >
> > 2) Observation
> >
> > A: a_host -> a_wait_bcon -> a_idle -> a_wait_vrise -> a_host
> > B: b_peripheral -> b_idle -> b_peripheral
> >
> > And I see "musb_hdrc.1 Babble" messages on both A and B console.
> >
> > Looks like B disconnects once, but A = host/B = peripheral does not change.
> >
> > Looking into musb driver source, there are code for HNP maybe to help
> > musb core behavior.
> > (But I have not enabled driver debug message yet, which I may try next?)

Re: musb_hdrc HNP?

[Bin Liu]

Hi,

On Thu, Aug 23, 2018 at 10:06:50PM +, Takashi Matsuzawa wrote:
> Hello.
> 
> I am trying HNP (host -> peripheral role change) using two
> PocketBeagles, but without success.

Well, you are the very first one that I have ever heard who tries to use
HNP on musb, at least on musb_dsps. 

> If there any idea on where I, where should I ask this, or how can I
> debug / fix, I really appreciate.

Being said, the controller itself does support HNP and other OTG
protocols, but the musb driver might not, or have been broken for a very
long time, since HNP probably never been tested on musb_dsps.

If you use kernel v4.18+, you can try to revert commit

0a9134bd733b usb: musb: disable otg protocol support

to see if HNP works.

Regards,
-Bin.

> 
> 1) What I did
> 
> Two PocketBeagles (running the default Debian image).
> Both added 2nd USB ports (musb_hdrc.1).
> DTBs were modified so that the 2nd musb_hdrcs have dr_mode = "otg".
> musb_dsps glue port seems to be used.
> Tech reference manual says the SoC's musb core supports HNP.
> 
> One is A (by GNDing ID pin) and another B, connected with USB cable.
> modprobe g_zero on both so that VBUS power is on, and A becomes a_host
> and b becomes b_peripheral (as read through debugfs mode value).
> 
> Then I did the following, expecting HNP happens and A bevcomes
> a_peripheral and B bcomes b_host:
> 
> i) Send b_hnp_enable request on A.
> (This does set DEVCTL.HOSTREQ bits on B's musb_hdrc.1 and maybe doing
> some additinoal things in musb driver on B.)
> 
> ii) Set POWER.SUSPENDM bit on A's musb_hdrc.1
> (According to TRM, this should tell B's musb core to initiate HNP.)
> 
> 2) Observation
> 
> A: a_host -> a_wait_bcon -> a_idle -> a_wait_vrise -> a_host
> B: b_peripheral -> b_idle -> b_peripheral
> 
> And I see "musb_hdrc.1 Babble" messages on both A and B console.
> 
> Looks like B disconnects once, but A = host/B = peripheral does not change.
> 
> Looking into musb driver source, there are code for HNP maybe to help
> musb core behavior.
> (But I have not enabled driver debug message yet, which I may try next?)

[PATCH v2] usb: musb: dsps: do not disable CPPI41 irq in driver teardown

[Bin Liu]

TI AM335x CPPI 4.1 module uses a single register bit for CPPI interrupts
in both musb controllers. So disabling the CPPI irq in one musb driver
breaks the other musb module.

Since musb is already disabled before tearing down dma controller in
musb_remove(), it is safe to not disable CPPI irq in
musb_dma_controller_destroy().

Fixes: 255348289f71 ("usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in DSPS")
Cc: sta...@vger.kernel.org
Signed-off-by: Bin Liu 
---
v2: revise commit message

 drivers/usb/musb/musb_dsps.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index df827ff57b0d..23a0df79ef21 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -658,16 +658,6 @@ dsps_dma_controller_create(struct musb *musb, void __iomem 
*base)
return controller;
 }
 
-static void dsps_dma_controller_destroy(struct dma_controller *c)
-{
-   struct musb *musb = c->musb;
-   struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
-   void __iomem *usbss_base = glue->usbss_base;
-
-   musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
-   cppi41_dma_controller_destroy(c);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static void dsps_dma_controller_suspend(struct dsps_glue *glue)
 {
@@ -697,7 +687,7 @@ static struct musb_platform_ops dsps_ops = {
 
 #ifdef CONFIG_USB_TI_CPPI41_DMA
.dma_init   = dsps_dma_controller_create,
-   .dma_exit   = dsps_dma_controller_destroy,
+   .dma_exit   = cppi41_dma_controller_destroy,
 #endif
.enable = dsps_musb_enable,
.disable= dsps_musb_disable,
-- 
2.17.1

[PATCH] usb: musb: dsps: do not disable CPPI41 irq in driver teardown

[Bin Liu]

TI AM335x CPPI41 module uses a single register bit for CPPI interrupts in
both musb controllers. So disabling the CPPI irq in one musb driver breaks
the other musb module.

Since musb is already disabled before tearing down dma controller in
musb_remove(), it is safe to not disable CPPI irq in
musb_dma_controller_destroy().

Fixes: 255348289f71 ("usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in DSPS")

Cc: sta...@vger.kernel.org
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_dsps.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index df827ff57b0d..23a0df79ef21 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -658,16 +658,6 @@ dsps_dma_controller_create(struct musb *musb, void __iomem 
*base)
return controller;
 }
 
-static void dsps_dma_controller_destroy(struct dma_controller *c)
-{
-   struct musb *musb = c->musb;
-   struct dsps_glue *glue = dev_get_drvdata(musb->controller->parent);
-   void __iomem *usbss_base = glue->usbss_base;
-
-   musb_writel(usbss_base, USBSS_IRQ_CLEARR, USBSS_IRQ_PD_COMP);
-   cppi41_dma_controller_destroy(c);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static void dsps_dma_controller_suspend(struct dsps_glue *glue)
 {
@@ -697,7 +687,7 @@ static struct musb_platform_ops dsps_ops = {
 
 #ifdef CONFIG_USB_TI_CPPI41_DMA
.dma_init   = dsps_dma_controller_create,
-   .dma_exit   = dsps_dma_controller_destroy,
+   .dma_exit   = cppi41_dma_controller_destroy,
 #endif
.enable = dsps_musb_enable,
.disable= dsps_musb_disable,
-- 
2.17.1

Re: g_audio regression on dwc3 udc

[Bin Liu]

Hi Thinh,

On Fri, Aug 10, 2018 at 09:36:30PM +, Thinh Nguyen wrote:
> Hi Bin,
> 
> On 8/10/2018 12:05 PM, Bin Liu wrote:
> > Hi Felipe,
> >
> > I noticed a regression on g_audio (uac2) with dwc3 udc on v4.14:
> >
> > - nosound in playback
> > - playback can only play once, the following error happens from the 2nd
> >   playback
> >
> >   aplay: set_params:1361: Unable to install hw params:
> >
> > A manual bisect tells the two issues are seperate and the regression
> > happens in v4.11-rc2 with the following two patches.
> >
> > 40d829fb2ec63 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for short 
> > packets")
> >
> > which cases the first no-sound issue.
> >
> > ep->mult is already 1 from g_audio, so mult-- leads to incorrect value.
> > The following change solves it.
> 
> I believe v4.14.40 is missing this patch regarding that issue:
> ec5bb87e4e2a ("usb: dwc3: gadget: Fix PCM1 for ISOC EP with ep->mult
> less than 3")
> 
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index f92e2f2c7fbe..758cc258895c 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -930,10 +930,10 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep 
> > *dep, struct dwc3_trb *trb,
> > unsigned int mult = ep->mult - 1;
> > unsigned int maxp = 
> > usb_endpoint_maxp(ep->desc);
> >  
> > -   if (length <= (2 * maxp))
> > +   if (mult && length <= (2 * maxp))
> > mult--;
> >  
> > -   if (length <= maxp)
> > +   if (mult && length <= maxp)
> > mult--;
> >  
> > trb->size |= DWC3_TRB_SIZE_PCM1(mult);
> >
> > cf3113d893d4 ("usb: dwc3: gadget: properly increment dequeue pointer on 
> > ep_dequeue")
> >
> > which causes the multiple playback failure. The following hack covers it.
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index f92e2f2c7fbe..758cc258895c 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -1411,7 +1411,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
> > if (r == req) {
> > /* wait until it is processed */
> > dwc3_stop_active_transfer(dwc, dep->number, true);
> > -
> > +#if 0
> > /*
> >  * If request was already started, this means we 
> > had to
> >  * stop the transfer. With that we also need to 
> > ignore
> > @@ -1473,6 +1473,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
> > dwc3_ep_inc_deq(dep);
> > }
> > }
> > +#endif
> > goto out1;
> > }
> > dev_err(dwc->dev, "request %pK was not queued to %s\n",
> >
> > With the two changes above, g_audio works fine with v4.14.40, but not with
> > v4.18-rc7, in which the console is flooded with following message.
> >
> > u_audio_iso_complete: iso_complete status(-18) 0/256
> >
> 
> Pierre Le Magourou attempted to fix the second issue with this patch
> (though the patch may need rework):
> https://patchwork.kernel.org/patch/10509639/
> 
> You can try it out. It works fine when I tested it on HAPS platform
> (using Felipe's next branch).

Thanks for the hints. With the two patches you pointed, g_audio works
fine on v4.14.40.

Regards,
-Bin.

g_audio regression on dwc3 udc

[Bin Liu]

Hi Felipe,

I noticed a regression on g_audio (uac2) with dwc3 udc on v4.14:

- nosound in playback
- playback can only play once, the following error happens from the 2nd
  playback

  aplay: set_params:1361: Unable to install hw params:

A manual bisect tells the two issues are seperate and the regression
happens in v4.11-rc2 with the following two patches.

40d829fb2ec63 ("usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets")

which cases the first no-sound issue.

ep->mult is already 1 from g_audio, so mult-- leads to incorrect value.
The following change solves it.

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f92e2f2c7fbe..758cc258895c 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -930,10 +930,10 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, 
struct dwc3_trb *trb,
unsigned int mult = ep->mult - 1;
unsigned int maxp = usb_endpoint_maxp(ep->desc);
 
-   if (length <= (2 * maxp))
+   if (mult && length <= (2 * maxp))
mult--;
 
-   if (length <= maxp)
+   if (mult && length <= maxp)
mult--;
 
trb->size |= DWC3_TRB_SIZE_PCM1(mult);

cf3113d893d4 ("usb: dwc3: gadget: properly increment dequeue pointer on 
ep_dequeue")

which causes the multiple playback failure. The following hack covers it.

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f92e2f2c7fbe..758cc258895c 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1411,7 +1411,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
if (r == req) {
/* wait until it is processed */
dwc3_stop_active_transfer(dwc, dep->number, true);
-
+#if 0
/*
 * If request was already started, this means we had to
 * stop the transfer. With that we also need to ignore
@@ -1473,6 +1473,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
dwc3_ep_inc_deq(dep);
}
}
+#endif
goto out1;
}
dev_err(dwc->dev, "request %pK was not queued to %s\n",

With the two changes above, g_audio works fine with v4.14.40, but not with
v4.18-rc7, in which the console is flooded with following message.

u_audio_iso_complete: iso_complete status(-18) 0/256

Regards,
-Bin.

Re: [PATCH] Fix memory issue in non-DMA mode for MUSB gadget

[Bin Liu]

Hi,

Please don't top-posting.

On Fri, Aug 03, 2018 at 06:25:50AM +, Alexey Spirkov wrote:
> Hi, Bin,
> 
> Thanks for note, but what do you think - is 'dma_mapping_error'
> checking still needed before call of unmap_dma_buffer? If yes than
> I'll check DMA usage in this check instead of is_buffer_map.

No, unmap_dma_buffer() can be called unconditionally here because
unmap_dma_buffer() already calls is_buffer_mapped() internally.

So my comment below is to remove the 'if' condition check you modified.

Regards,
-Bin.

> 
> Best regards,
> Alexey Spirkov
> 
> -Original Message-
> From: Bin Liu  
> Sent: Thursday, August 2, 2018 7:15 PM
> To: Alexey Spirkov 
> Cc: Greg Kroah-Hartman ; 
> linux-usb@vger.kernel.org; and...@ncrmnt.org
> Subject: Re: [PATCH] Fix memory issue in non-DMA mode for MUSB gadget
> 
> Hi,
> 
> On Thu, Jul 26, 2018 at 12:52:57PM +, Alexey Spirkov wrote:
> > dma_mapping_error function unable to works in PowerPC arch when MUSB 
> > do not use DMA (illegal memory access). Proposed patch replace its 
> > usage to usual define for checking DMA mapping.
> > 
> > Signed-off-by: Alexey Spirkov 
> > ---
> >  drivers/usb/musb/musb_gadget.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/musb/musb_gadget.c 
> > b/drivers/usb/musb/musb_gadget.c index eae8b1b..3bc7c25 100644
> > --- a/drivers/usb/musb/musb_gadget.c
> > +++ b/drivers/usb/musb/musb_gadget.c
> > @@ -140,7 +140,7 @@ __acquires(ep->musb->lock)
> > ep->busy = 1;
> > spin_unlock(>lock);
> >  
> > -   if (!dma_mapping_error(>g.dev, request->dma))
> > +   if (req && is_buffer_mapped(req))
> 
> unmap_dma_buffer() checks for is_buffer_mapped(), so this line can be dropped.
> 
> > unmap_dma_buffer(req, musb);
> >  
> > trace_musb_req_gb(req);
> 
> Regards,
> -Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fix big-endian application issue for MUSB gadget

[Bin Liu]

On Thu, Aug 02, 2018 at 11:44:00AM -0500, Bin Liu wrote:
> Hi,
> 
> On Thu, Jul 26, 2018 at 12:52:53PM +, Alexey Spirkov wrote:
> > Existing code is not applicable to big-endian machines
> > ctrlrequest fields received in USB endian - i.e. in little-endian
> > and should be converted to cpu endianness before usage.
> > 
> > Signed-off-by: Alexey Spirkov 
> > ---
> >  drivers/usb/musb/musb_gadget_ep0.c | 33 -
> >  1 file changed, 20 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/usb/musb/musb_gadget_ep0.c 
> > b/drivers/usb/musb/musb_gadget_ep0.c
> > index 91a5027..5d5c933 100644
> > --- a/drivers/usb/musb/musb_gadget_ep0.c
> > +++ b/drivers/usb/musb/musb_gadget_ep0.c
> > @@ -82,7 +82,7 @@ static int service_tx_status_request(
> > u16 tmp;
> > void __iomem*regs;
> >  
> > -   epnum = (u8) ctrlrequest->wIndex;
> > +   epnum = (u8) le16_to_cpu(ctrlrequest->wIndex);
> > if (!epnum) {
> > result[0] = 0;
> > break;
> > @@ -217,14 +217,15 @@ __acquires(musb->lock)
> > case USB_REQ_SET_ADDRESS:
> > /* change it after the status stage */
> > musb->set_address = true;
> > -   musb->address = (u8) (ctrlrequest->wValue & 0x7f);
> > +   musb->address = (u8) (le16_to_cpu(ctrlrequest->wValue) &
> > +   0x7f);
> > handled = 1;
> > break;
> >  
> > case USB_REQ_CLEAR_FEATURE:
> > switch (recip) {
> > case USB_RECIP_DEVICE:
> > -   if (ctrlrequest->wValue
> > +   if (le16_to_cpu(ctrlrequest->wValue)
> > != USB_DEVICE_REMOTE_WAKEUP)
> > break;
> > musb->may_wakeup = 0;
> > @@ -234,7 +235,7 @@ __acquires(musb->lock)
> > break;
> > case USB_RECIP_ENDPOINT:{
> > const u8epnum =
> > -   ctrlrequest->wIndex & 0x0f;
> > +   le16_to_cpu(ctrlrequest->wIndex) & 0x0f;
> > struct musb_ep  *musb_ep;
> > struct musb_hw_ep   *ep;
> > struct musb_request *request;
> > @@ -243,12 +244,14 @@ __acquires(musb->lock)
> > u16 csr;
> >  
> > if (epnum == 0 || epnum >= MUSB_C_NUM_EPS ||
> > -   ctrlrequest->wValue != USB_ENDPOINT_HALT)
> > +   le16_to_cpu(ctrlrequest->wValue)
> > +   != USB_ENDPOINT_HALT)
> > break;
> >  
> > ep = musb->endpoints + epnum;
> > regs = ep->regs;
> > -   is_in = ctrlrequest->wIndex & USB_DIR_IN;
> > +   is_in = le16_to_cpu(ctrlrequest->wIndex) &
> > +   USB_DIR_IN;
> > if (is_in)
> > musb_ep = >ep_in;
> > else
> > @@ -300,17 +303,19 @@ __acquires(musb->lock)
> > switch (recip) {
> > case USB_RECIP_DEVICE:
> > handled = 1;
> > -   switch (ctrlrequest->wValue) {
> > +   switch (le16_to_cpu(ctrlrequest->wValue)) {
> > case USB_DEVICE_REMOTE_WAKEUP:
> > musb->may_wakeup = 1;
> > break;
> > case USB_DEVICE_TEST_MODE:
> > if (musb->g.speed != USB_SPEED_HIGH)
> > goto stall;
> > -   if (ctrlrequest->wIndex & 0xff)
> > +   if (le16_to_cpu(ctrlrequest->wIndex) &
> > +  

Re: [PATCH] Fix memory issue in non-DMA mode for MUSB gadget

[Bin Liu]

On Thu, Aug 02, 2018 at 11:14:32AM -0500, Bin Liu wrote:
> Hi,
> 
> On Thu, Jul 26, 2018 at 12:52:57PM +, Alexey Spirkov wrote:
> > dma_mapping_error function unable to works in PowerPC arch
> > when MUSB do not use DMA (illegal memory access). Proposed
> > patch replace its usage to usual define for checking DMA mapping.
> > 
> > Signed-off-by: Alexey Spirkov 
> > ---
> >  drivers/usb/musb/musb_gadget.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> > index eae8b1b..3bc7c25 100644
> > --- a/drivers/usb/musb/musb_gadget.c
> > +++ b/drivers/usb/musb/musb_gadget.c
> > @@ -140,7 +140,7 @@ __acquires(ep->musb->lock)
> > ep->busy = 1;
> > spin_unlock(>lock);
> >  
> > -   if (!dma_mapping_error(>g.dev, request->dma))
> > +   if (req && is_buffer_mapped(req))
> 
> unmap_dma_buffer() checks for is_buffer_mapped(), so this line can be
> dropped.
> 
> > unmap_dma_buffer(req, musb);
> >  
> > trace_musb_req_gb(req);

Please also fix the patch subject to "usb: musb: gadget: ..." in your
v2. I prefer

"usb: musb: gadget: fix illegal dma address check in non-DMA mode"

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fix big-endian application issue for MUSB gadget

[Bin Liu]

Hi,

On Thu, Jul 26, 2018 at 12:52:53PM +, Alexey Spirkov wrote:
> Existing code is not applicable to big-endian machines
> ctrlrequest fields received in USB endian - i.e. in little-endian
> and should be converted to cpu endianness before usage.
> 
> Signed-off-by: Alexey Spirkov 
> ---
>  drivers/usb/musb/musb_gadget_ep0.c | 33 -
>  1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_gadget_ep0.c 
> b/drivers/usb/musb/musb_gadget_ep0.c
> index 91a5027..5d5c933 100644
> --- a/drivers/usb/musb/musb_gadget_ep0.c
> +++ b/drivers/usb/musb/musb_gadget_ep0.c
> @@ -82,7 +82,7 @@ static int service_tx_status_request(
>   u16 tmp;
>   void __iomem*regs;
>  
> - epnum = (u8) ctrlrequest->wIndex;
> + epnum = (u8) le16_to_cpu(ctrlrequest->wIndex);
>   if (!epnum) {
>   result[0] = 0;
>   break;
> @@ -217,14 +217,15 @@ __acquires(musb->lock)
>   case USB_REQ_SET_ADDRESS:
>   /* change it after the status stage */
>   musb->set_address = true;
> - musb->address = (u8) (ctrlrequest->wValue & 0x7f);
> + musb->address = (u8) (le16_to_cpu(ctrlrequest->wValue) &
> + 0x7f);
>   handled = 1;
>   break;
>  
>   case USB_REQ_CLEAR_FEATURE:
>   switch (recip) {
>   case USB_RECIP_DEVICE:
> - if (ctrlrequest->wValue
> + if (le16_to_cpu(ctrlrequest->wValue)
>   != USB_DEVICE_REMOTE_WAKEUP)
>   break;
>   musb->may_wakeup = 0;
> @@ -234,7 +235,7 @@ __acquires(musb->lock)
>   break;
>   case USB_RECIP_ENDPOINT:{
>   const u8epnum =
> - ctrlrequest->wIndex & 0x0f;
> + le16_to_cpu(ctrlrequest->wIndex) & 0x0f;
>   struct musb_ep  *musb_ep;
>   struct musb_hw_ep   *ep;
>   struct musb_request *request;
> @@ -243,12 +244,14 @@ __acquires(musb->lock)
>   u16 csr;
>  
>   if (epnum == 0 || epnum >= MUSB_C_NUM_EPS ||
> - ctrlrequest->wValue != USB_ENDPOINT_HALT)
> + le16_to_cpu(ctrlrequest->wValue)
> + != USB_ENDPOINT_HALT)
>   break;
>  
>   ep = musb->endpoints + epnum;
>   regs = ep->regs;
> - is_in = ctrlrequest->wIndex & USB_DIR_IN;
> + is_in = le16_to_cpu(ctrlrequest->wIndex) &
> + USB_DIR_IN;
>   if (is_in)
>   musb_ep = >ep_in;
>   else
> @@ -300,17 +303,19 @@ __acquires(musb->lock)
>   switch (recip) {
>   case USB_RECIP_DEVICE:
>   handled = 1;
> - switch (ctrlrequest->wValue) {
> + switch (le16_to_cpu(ctrlrequest->wValue)) {
>   case USB_DEVICE_REMOTE_WAKEUP:
>   musb->may_wakeup = 1;
>   break;
>   case USB_DEVICE_TEST_MODE:
>   if (musb->g.speed != USB_SPEED_HIGH)
>   goto stall;
> - if (ctrlrequest->wIndex & 0xff)
> + if (le16_to_cpu(ctrlrequest->wIndex) &
> + 0xff)
>   goto stall;
>  
> - switch (ctrlrequest->wIndex >> 8) {
> + switch (le16_to_cpu(ctrlrequest->wIndex)
> +  >> 8) {
>   case 1:
>   pr_debug("TEST_J\n");
>   /* TEST_J */
> @@ -399,7 +404,7 @@ __acquires(musb->lock)
>  
>   case USB_RECIP_ENDPOINT:{
>   const u8epnum =
> -   

Re: [PATCH] Fix memory issue in non-DMA mode for MUSB gadget

[Bin Liu]

Hi,

On Thu, Jul 26, 2018 at 12:52:57PM +, Alexey Spirkov wrote:
> dma_mapping_error function unable to works in PowerPC arch
> when MUSB do not use DMA (illegal memory access). Proposed
> patch replace its usage to usual define for checking DMA mapping.
> 
> Signed-off-by: Alexey Spirkov 
> ---
>  drivers/usb/musb/musb_gadget.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index eae8b1b..3bc7c25 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -140,7 +140,7 @@ __acquires(ep->musb->lock)
>   ep->busy = 1;
>   spin_unlock(>lock);
>  
> - if (!dma_mapping_error(>g.dev, request->dma))
> + if (req && is_buffer_mapped(req))

unmap_dma_buffer() checks for is_buffer_mapped(), so this line can be
dropped.

>   unmap_dma_buffer(req, musb);
>  
>   trace_musb_req_gb(req);

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: core: handle hub C_PORT_OVER_CURRENT condition

[Bin Liu]

Based on USB2.0 Spec Section 11.12.5,

  "If a hub has per-port power switching and per-port current limiting,
  an over-current on one port may still cause the power on another port
  to fall below specific minimums. In this case, the affected port is
  placed in the Power-Off state and C_PORT_OVER_CURRENT is set for the
  port, but PORT_OVER_CURRENT is not set."

so let's check C_PORT_OVER_CURRENT too for over current condition.

Fixes: 08d1dec6f405 ("usb:hub set hub->change_bits when over-current happens")
Cc: 
Tested-by: Alessandro Antenucci 
Signed-off-by: Bin Liu 
---
 drivers/usb/core/hub.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index fcae521df29b..1fb266809966 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1142,10 +1142,14 @@ static void hub_activate(struct usb_hub *hub, enum 
hub_activation_type type)
 
if (!udev || udev->state == USB_STATE_NOTATTACHED) {
/* Tell hub_wq to disconnect the device or
-* check for a new connection
+* check for a new connection or over current condition.
+* Based on USB2.0 Spec Section 11.12.5,
+* C_PORT_OVER_CURRENT could be set while
+* PORT_OVER_CURRENT is not. So check for any of them.
 */
if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
-   (portstatus & USB_PORT_STAT_OVERCURRENT))
+   (portstatus & USB_PORT_STAT_OVERCURRENT) ||
+   (portchange & USB_PORT_STAT_C_OVERCURRENT))
set_bit(port1, hub->change_bits);
 
} else if (portstatus & USB_PORT_STAT_ENABLE) {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb hub driver bug in overcurrent handling?

[Bin Liu]

On Fri, Jul 13, 2018 at 04:35:33PM -0400, Alan Stern wrote:
> On Fri, 13 Jul 2018, Bin Liu wrote:
> 
> > Hi,
> > 
> > I got a report saying that when overcurrent happens in one of the hub
> > downstream ports, USB_PORT_STAT_OVERCURRENT is not set in portstatus,
> > instead USB_PORT_STAT_C_OVERCURRENT is set in portchange, then the
> > overcurrent condition is not handled in hub_event().
> > 
> > The following patch solves the issue.
> > 
> > t a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 442be7f312f6..118557acc74b 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -1144,7 +1144,8 @@ static void hub_activate(struct usb_hub *hub, enum 
> > hub_activation_type type)
> >  * check for a new connection
> >  */
> > if (udev || (portstatus & USB_PORT_STAT_CONNECTION) 
> > ||
> > -   (portstatus & USB_PORT_STAT_OVERCURRENT))
> > +   (portstatus & USB_PORT_STAT_OVERCURRENT) ||
> > +   (portchange & USB_PORT_STAT_C_OVERCURRENT))
> > set_bit(port1, hub->change_bits);
> >  
> > } else if (portstatus & USB_PORT_STAT_ENABLE) {
> 
> Please also update the immediately preceding comment.

Sure.

> 
> > The usb2.0 spec section 11.12.5 states
> > "If a hub has per-port power switching and per-port current limiting, an
> >  over-current on one port may still cause the power on another port to
> >  fall below specific minimums. In this case, the affected port is placed
> >  in the Power-Off state and C_PORT_OVER_CURRENT is set for the port, but
> >  PORT_OVER_CURRENT is not set."
> > 
> > So is the patch above a proper fix? or something else might be missing?
> 
> It certainly seems like a reasonable thing to do.  If you say it fixes 
> the problem, we can add it in.

Will send the patch.

Thanks,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb hub driver bug in overcurrent handling?

[Bin Liu]

On Fri, Jul 13, 2018 at 02:22:32PM -0500, Bin Liu wrote:
> Hi,
> 
> I got a report saying that when overcurrent happens in one of the hub
> downstream ports, USB_PORT_STAT_OVERCURRENT is not set in portstatus,
> instead USB_PORT_STAT_C_OVERCURRENT is set in portchange, then the
> overcurrent condition is not handled in hub_event().

not handled because neither hub->change_bits nor hub->event_bits is set.

> The following patch solves the issue.
> 
> t a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 442be7f312f6..118557acc74b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1144,7 +1144,8 @@ static void hub_activate(struct usb_hub *hub, enum 
> hub_activation_type type)
>  * check for a new connection
>  */
> if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
> -   (portstatus & USB_PORT_STAT_OVERCURRENT))
> +   (portstatus & USB_PORT_STAT_OVERCURRENT) ||
> +   (portchange & USB_PORT_STAT_C_OVERCURRENT))
> set_bit(port1, hub->change_bits);
>  
> } else if (portstatus & USB_PORT_STAT_ENABLE) {
> 
> The usb2.0 spec section 11.12.5 states
> "If a hub has per-port power switching and per-port current limiting, an
>  over-current on one port may still cause the power on another port to
>  fall below specific minimums. In this case, the affected port is placed
>  in the Power-Off state and C_PORT_OVER_CURRENT is set for the port, but
>  PORT_OVER_CURRENT is not set."
> 
> So is the patch above a proper fix? or something else might be missing?
> 
> Thanks,
> -Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usb hub driver bug in overcurrent handling?

[Bin Liu]

Hi,

I got a report saying that when overcurrent happens in one of the hub
downstream ports, USB_PORT_STAT_OVERCURRENT is not set in portstatus,
instead USB_PORT_STAT_C_OVERCURRENT is set in portchange, then the
overcurrent condition is not handled in hub_event().

The following patch solves the issue.

t a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 442be7f312f6..118557acc74b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1144,7 +1144,8 @@ static void hub_activate(struct usb_hub *hub, enum 
hub_activation_type type)
 * check for a new connection
 */
if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
-   (portstatus & USB_PORT_STAT_OVERCURRENT))
+   (portstatus & USB_PORT_STAT_OVERCURRENT) ||
+   (portchange & USB_PORT_STAT_C_OVERCURRENT))
set_bit(port1, hub->change_bits);
 
} else if (portstatus & USB_PORT_STAT_ENABLE) {

The usb2.0 spec section 11.12.5 states
"If a hub has per-port power switching and per-port current limiting, an
 over-current on one port may still cause the power on another port to
 fall below specific minimums. In this case, the affected port is placed
 in the Power-Off state and C_PORT_OVER_CURRENT is set for the port, but
 PORT_OVER_CURRENT is not set."

So is the patch above a proper fix? or something else might be missing?

Thanks,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring

[Bin Liu]

Hi laurent,

On Thu, May 24, 2018 at 10:59:18PM +0300, Laurent Pinchart wrote:
> Hi Felipe,
> 
> On Friday, 20 April 2018 13:57:23 EEST Felipe Balbi wrote:
> > Bin Liu <b-...@ti.com> writes:
> > >> Felipe Balbi <felipe.ba...@linux.intel.com> writes:
> > >>>>> Bin Liu <b-...@ti.com> writes:
> > >>>>>> On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote:
> > >>>>>>> Hi guys,
> > >>>>>>> 
> > >>>>>>> I've been working on this series for a while now. I feels like
> > >>>>>>> after this series the transfer management code is far easier to
> > >>>>>>> read and understand.
> > >>>>>>> 
> > >>>>>>> Based on my tests, I have no regressions. Tested g_mass_storage
> > >>>>>>> and all of testusb's tests (including ISOC).
> > >>>>>>> 
> > >>>>>>> Patches are also available on dwc3-improve-isoc-endpoints in my
> > >>>>>>> k.org tree. Test reports would be VERY, VERY, VERY welcome. Please
> > >>>>>>> give this a go so we avoid regressions on v4.18.
> > >>>>>> 
> > >>>>>> Have you tested g_webcam? I just tested your
> > >>>>>> dwc3-improve-isoc-endpoints
> > >>>>> 
> > >>>>> for isoc I only tested g_zero.
> > >>>> 
> > >>>> Then writting down details on what I observed probably won't help you
> > >>>> :(
> > >> 
> > >> I got webcam running here, it sure _is_ helpful to let me know how you
> > > 
> > > Great!
> > > 
> > >> trigger the problem ;-) Also, high-speed or super-speed?
> > > 
> > > Both. Long story short - super-speed doesn't stream the yuv video,
> > > gadget side kernel prints "VS request completed with status -18."
> > > flooding messages.
> > > 
> > > high-speed does stream the video, but stopping the host application
> > > (both yavta and luvcview) causes gadget side kernel prints error message
> > > (I didn't keep the log). Then restart the host application doesn't
> > > stream the video any more.
> 
> What is your test platform ? I'm using a TI AM574x EVM and I can't get video 
> to stream at all in high-speed. Super-speed seems out of question as the only 
> port supporting device mode on that board is connect to a DWC3 instance that 
> is limited to high-speed :-/ I'm testing v4.17-rc5 without this patch series 
> applied, please let me know if I should apply it first.

It worked at the last time I tested it without this patch series, but
don't remember the kernel version I tested.

I will be offline for two weeks. I can test it again once I am back if
you are unable to get it working by then.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/14] usb: musb: break the huge isr musb_stage0_irq() into small functions

[Bin Liu]

musb_stage0_irq() is 400+ lines long. Break its interrupt events
handling into each individual functions to make it easy to read.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c | 730 +++
 1 file changed, 384 insertions(+), 346 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index ce54f48314e1..a3a716197dc1 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -523,6 +523,383 @@ void musb_hnp_stop(struct musb *musb)
 
 static void musb_recover_from_babble(struct musb *musb);
 
+static void musb_handle_intr_resume(struct musb *musb, u8 devctl)
+{
+   musb_dbg(musb, "RESUME (%s)",
+   usb_otg_state_string(musb->xceiv->otg->state));
+
+   if (devctl & MUSB_DEVCTL_HM) {
+   switch (musb->xceiv->otg->state) {
+   case OTG_STATE_A_SUSPEND:
+   /* remote wakeup? */
+   musb->port1_status |=
+   (USB_PORT_STAT_C_SUSPEND << 16)
+   | MUSB_PORT_STAT_RESUME;
+   musb->rh_timer = jiffies
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
+   musb->xceiv->otg->state = OTG_STATE_A_HOST;
+   musb->is_active = 1;
+   musb_host_resume_root_hub(musb);
+   schedule_delayed_work(>finish_resume_work,
+   msecs_to_jiffies(USB_RESUME_TIMEOUT));
+   break;
+   case OTG_STATE_B_WAIT_ACON:
+   musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
+   musb->is_active = 1;
+   MUSB_DEV_MODE(musb);
+   break;
+   default:
+   WARNING("bogus %s RESUME (%s)\n",
+   "host",
+   usb_otg_state_string(musb->xceiv->otg->state));
+   }
+   } else {
+   switch (musb->xceiv->otg->state) {
+   case OTG_STATE_A_SUSPEND:
+   /* possibly DISCONNECT is upcoming */
+   musb->xceiv->otg->state = OTG_STATE_A_HOST;
+   musb_host_resume_root_hub(musb);
+   break;
+   case OTG_STATE_B_WAIT_ACON:
+   case OTG_STATE_B_PERIPHERAL:
+   /* disconnect while suspended?  we may
+* not get a disconnect irq...
+*/
+   if ((devctl & MUSB_DEVCTL_VBUS)
+   != (3 << MUSB_DEVCTL_VBUS_SHIFT)
+   ) {
+   musb->int_usb |= MUSB_INTR_DISCONNECT;
+   musb->int_usb &= ~MUSB_INTR_SUSPEND;
+   break;
+   }
+   musb_g_resume(musb);
+   break;
+   case OTG_STATE_B_IDLE:
+   musb->int_usb &= ~MUSB_INTR_SUSPEND;
+   break;
+   default:
+   WARNING("bogus %s RESUME (%s)\n",
+   "peripheral",
+   usb_otg_state_string(musb->xceiv->otg->state));
+   }
+   }
+}
+
+/* return IRQ_HANDLED to tell the caller to return immediately */
+static irqreturn_t musb_handle_intr_sessreq(struct musb *musb, u8 devctl)
+{
+   void __iomem *mbase = musb->mregs;
+
+   if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS
+   && (devctl & MUSB_DEVCTL_BDEVICE)) {
+   musb_dbg(musb, "SessReq while on B state");
+   return IRQ_HANDLED;
+   }
+
+   musb_dbg(musb, "SESSION_REQUEST (%s)",
+   usb_otg_state_string(musb->xceiv->otg->state));
+
+   /* IRQ arrives from ID pin sense or (later, if VBUS power
+* is removed) SRP.  responses are time critical:
+*  - turn on VBUS (with silicon-specific mechanism)
+*  - go through A_WAIT_VRISE
+*  - ... to A_WAIT_BCON.
+* a_wait_vrise_tmout triggers VBUS_ERROR transitions
+*/
+   musb_writeb(mbase, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
+   musb->ep0_stage = MUSB_EP0_START;
+   musb->xceiv->otg->state = OTG_STATE_A_IDLE;
+   MUSB_HST_MODE(musb);
+   musb_platform_set_vbus(musb, 1);
+
+   return IRQ_NONE;
+}
+
+static void musb_handle_intr_vbuserr(struct musb *musb, u8 devctl)
+{
+   int ignore = 0;
+
+   /* During connection as an A-Device, we may see a short
+ 

[PATCH 07/14] usb: musb: remove duplicated port mode enum

[Bin Liu]

include/linux/usb/musb.h already defines enum for musb port mode, so
remove the duplicate in musb_core.h and use the definition in musb.h.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c| 8 
 drivers/usb/musb/musb_core.h| 8 +---
 drivers/usb/musb/musb_dsps.c| 6 +++---
 drivers/usb/musb/musb_gadget.c  | 2 +-
 drivers/usb/musb/musb_host.c| 4 ++--
 drivers/usb/musb/musb_virthub.c | 2 +-
 drivers/usb/musb/sunxi.c| 8 
 7 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 84f25a2b078d..ce54f48314e1 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1038,7 +1038,7 @@ void musb_start(struct musb *musb)
 * (b) vbus present/connect IRQ, peripheral mode;
 * (c) peripheral initiates, using SRP
 */
-   if (musb->port_mode != MUSB_PORT_MODE_HOST &&
+   if (musb->port_mode != MUSB_HOST &&
musb->xceiv->otg->state != OTG_STATE_A_WAIT_BCON &&
(devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) {
musb->is_active = 1;
@@ -2323,19 +2323,19 @@ static void musb_deassert_reset(struct work_struct 
*work)
}
 
switch (musb->port_mode) {
-   case MUSB_PORT_MODE_HOST:
+   case MUSB_HOST:
status = musb_host_setup(musb, plat->power);
if (status < 0)
goto fail3;
status = musb_platform_set_mode(musb, MUSB_HOST);
break;
-   case MUSB_PORT_MODE_GADGET:
+   case MUSB_PERIPHERAL:
status = musb_gadget_setup(musb);
if (status < 0)
goto fail3;
status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
break;
-   case MUSB_PORT_MODE_DUAL_ROLE:
+   case MUSB_OTG:
status = musb_host_setup(musb, plat->power);
if (status < 0)
goto fail3;
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index f57323e50e44..04203b7126d5 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -53,12 +53,6 @@
 #define is_peripheral_active(m)(!(m)->is_host)
 #define is_host_active(m)  ((m)->is_host)
 
-enum {
-   MUSB_PORT_MODE_HOST = 1,
-   MUSB_PORT_MODE_GADGET,
-   MUSB_PORT_MODE_DUAL_ROLE,
-};
-
 /** CONSTANTS /
 
 #ifndef MUSB_C_NUM_EPS
@@ -351,7 +345,7 @@ struct musb {
 
u8  min_power;  /* vbus for periph, in mA/2 */
 
-   int port_mode;  /* MUSB_PORT_MODE_* */
+   enum musb_mode  port_mode;
boolsession;
unsigned long   quirk_retries;
boolis_host;
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index c513ecebc35a..71aec85b9bd4 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -183,7 +183,7 @@ static void dsps_musb_enable(struct musb *musb)
musb_writel(reg_base, wrp->coreintr_set, coremask);
/* start polling for ID change in dual-role idle mode */
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
-   musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+   musb->port_mode == MUSB_OTG)
dsps_mod_timer(glue, -1);
 }
 
@@ -231,7 +231,7 @@ static int dsps_check_status(struct musb *musb, void 
*unused)
break;
case OTG_STATE_A_WAIT_BCON:
/* keep VBUS on for host-only mode */
-   if (musb->port_mode == MUSB_PORT_MODE_HOST) {
+   if (musb->port_mode == MUSB_HOST) {
dsps_mod_timer_optional(glue);
break;
}
@@ -1028,7 +1028,7 @@ static int dsps_resume(struct device *dev)
musb_writel(mbase, wrp->tx_mode, glue->context.tx_mode);
musb_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
-   musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+   musb->port_mode == MUSB_OTG)
dsps_mod_timer(glue, -1);
 
pm_runtime_put(dev);
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 71c5835ea9cd..d082b0cbea93 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1823,7 +1823,7 @@ int musb_gadget_setup(struct musb *musb)
 
 void musb_gadget_cleanup(struct musb *musb)
 {
-   if (musb->port_mode == MUSB_PORT_MODE_HOST)
+   if (musb->port_mode == MUSB_HOST)
return;
 
can

[PATCH 08/14] usb: musb: remove unused members in struct musb_hdrc_config

[Bin Liu]

The following members in struct musb_hdrc_config are not used,
so remove them.

soft_con
utm_16
big_endian
mult_bulk_tx
mult_bulk_rx
high_iso_tx
high_iso_rx
dma
dma_channels
dyn_fifo_size
vendor_ctrl
vendor_stat
vendor_req
dma_req_chan
musb_hdrc_eps_bits

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/sunxi.c |  4 
 include/linux/usb/musb.h | 15 ---
 2 files changed, 19 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 8f7d378b7e7e..62ab2ca03779 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -651,10 +651,8 @@ static void sunxi_musb_writew(void __iomem *addr, unsigned 
offset, u16 data)
.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
.multipoint = true,
.dyn_fifo   = true,
-   .soft_con   = true,
.num_eps= SUNXI_MUSB_MAX_EP_NUM,
.ram_bits   = SUNXI_MUSB_RAM_BITS,
-   .dma= 0,
 };
 
 static struct musb_hdrc_config sunxi_musb_hdrc_config_h3 = {
@@ -662,10 +660,8 @@ static void sunxi_musb_writew(void __iomem *addr, unsigned 
offset, u16 data)
.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_h3),
.multipoint = true,
.dyn_fifo   = true,
-   .soft_con   = true,
.num_eps= SUNXI_MUSB_MAX_EP_NUM_H3,
.ram_bits   = SUNXI_MUSB_RAM_BITS,
-   .dma= 0,
 };
 
 
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 9eb908a98033..fc6c77918481 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -67,28 +67,13 @@ struct musb_hdrc_config {
/* MUSB configuration-specific details */
unsignedmultipoint:1;   /* multipoint device */
unsigneddyn_fifo:1 __deprecated; /* supports dynamic fifo 
sizing */
-   unsignedsoft_con:1 __deprecated; /* soft connect required */
-   unsignedutm_16:1 __deprecated; /* utm data witdh is 16 bits */
-   unsignedbig_endian:1;   /* true if CPU uses big-endian */
-   unsignedmult_bulk_tx:1; /* Tx ep required for multbulk pkts */
-   unsignedmult_bulk_rx:1; /* Rx ep required for multbulk pkts */
-   unsignedhigh_iso_tx:1;  /* Tx ep required for HB iso */
-   unsignedhigh_iso_rx:1;  /* Rx ep required for HD iso */
-   unsigneddma:1 __deprecated; /* supports DMA */
-   unsignedvendor_req:1 __deprecated; /* vendor registers required 
*/
 
/* need to explicitly de-assert the port reset after resume? */
unsignedhost_port_deassert_reset_at_resume:1;
 
u8  num_eps;/* number of endpoints _with_ ep0 */
-   u8  dma_channels __deprecated; /* number of dma channels */
-   u8  dyn_fifo_size;  /* dynamic size in bytes */
-   u8  vendor_ctrl __deprecated; /* vendor control reg width */
-   u8  vendor_stat __deprecated; /* vendor status reg witdh */
-   u8  dma_req_chan __deprecated; /* bitmask for required dma 
channels */
u8  ram_bits;   /* ram address size */
 
-   struct musb_hdrc_eps_bits *eps_bits __deprecated;
u32 maximum_speed;
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/14] usb: musb: gadget: fix to_musb_request() to not return NULL

[Bin Liu]

The gadget function drivers should ensure the usb_request parameter
passed in is not NULL. UDC core doesn't check if it is NULL, so MUSB
driver shouldn't have to check it either.

Convert to_musb_request() to a simple macro to not directly return NULL
to avoid warnings from code static analysis tools.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 9c34aca06db6..466877b471ef 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -60,10 +60,7 @@ struct musb_request {
enum buffer_map_state map_state;
 };
 
-static inline struct musb_request *to_musb_request(struct usb_request *req)
-{
-   return req ? container_of(req, struct musb_request, request) : NULL;
-}
+#define to_musb_request(r) container_of((r), struct musb_request, request)
 
 extern struct usb_request *
 musb_alloc_request(struct usb_ep *ep, gfp_t gfp_flags);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/14] usb: musb: dsps: remove duplicated get_musb_port_mode()

[Bin Liu]

musb_core already has musb_get_mode(), so remove the duplicate from
musb_dsps.c.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 6a60bc0490c5..c513ecebc35a 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -729,25 +729,6 @@ static int get_int_prop(struct device_node *dn, const char 
*s)
return val;
 }
 
-static int get_musb_port_mode(struct device *dev)
-{
-   enum usb_dr_mode mode;
-
-   mode = usb_get_dr_mode(dev);
-   switch (mode) {
-   case USB_DR_MODE_HOST:
-   return MUSB_PORT_MODE_HOST;
-
-   case USB_DR_MODE_PERIPHERAL:
-   return MUSB_PORT_MODE_GADGET;
-
-   case USB_DR_MODE_UNKNOWN:
-   case USB_DR_MODE_OTG:
-   default:
-   return MUSB_PORT_MODE_DUAL_ROLE;
-   }
-}
-
 static int dsps_create_musb_pdev(struct dsps_glue *glue,
struct platform_device *parent)
 {
@@ -807,7 +788,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
config->num_eps = get_int_prop(dn, "mentor,num-eps");
config->ram_bits = get_int_prop(dn, "mentor,ram-bits");
config->host_port_deassert_reset_at_resume = 1;
-   pdata.mode = get_musb_port_mode(dev);
+   pdata.mode = musb_get_mode(dev);
/* DT keeps this entry in mA, musb expects it as per USB spec */
pdata.power = get_int_prop(dn, "mentor,power") / 2;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/14] USB: musb: dsps: propagate device-tree node

[Bin Liu]

From: Johan Hovold <jo...@kernel.org>

To be able to use DSPS-based controllers with device-tree descriptions
of the USB topology, we need to associate the glue device's device-tree
node with the child controller device.

Note that this can also be used to eventually let USB core manage
generic phys.

Also note that the other glue drivers will require similar changes to be
able to describe their buses in DT.

Signed-off-by: Johan Hovold <jo...@kernel.org>
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 34e4dda1d6ac..cfe6bfcbeb5d 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -765,6 +765,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
musb->dev.parent= dev;
musb->dev.dma_mask  = _dmamask;
musb->dev.coherent_dma_mask = musb_dmamask;
+   device_set_of_node_from_dev(>dev, >dev);
 
glue->musb = musb;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/14] usb: musb: remove adjust_channel_params() callback from musb_platform_ops

[Bin Liu]

Now Blackfin support is removed, nobody uses adjust_channel_params() any
more, so remove it from struct musb_platform_ops.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.h | 4 
 drivers/usb/musb/musbhsdma.c | 8 
 2 files changed, 12 deletions(-)

diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index a4bf1e9e2d2c..f57323e50e44 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -138,7 +138,6 @@ enum musb_g_ep0_state {
  * @recover:   platform-specific babble recovery
  * @vbus_status: returns vbus status if possible
  * @set_vbus:  forces vbus status
- * @adjust_channel_params: pre check for standard dma channel_program func
  * @pre_root_reset_end: called before the root usb port reset flag gets cleared
  * @post_root_reset_end: called after the root usb port reset flag gets cleared
  * @phy_callback: optional callback function for the phy to call
@@ -184,9 +183,6 @@ struct musb_platform_ops {
int (*vbus_status)(struct musb *musb);
void(*set_vbus)(struct musb *musb, int on);
 
-   int (*adjust_channel_params)(struct dma_channel *channel,
-   u16 packet_sz, u8 *mode,
-   dma_addr_t *dma_addr, u32 *len);
void(*pre_root_reset_end)(struct musb *musb);
void(*post_root_reset_end)(struct musb *musb);
int (*phy_callback)(enum musb_vbus_id_status status);
diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 57d416a110a5..a688f7f87829 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -199,14 +199,6 @@ static int dma_channel_program(struct dma_channel *channel,
BUG_ON(channel->status == MUSB_DMA_STATUS_UNKNOWN ||
channel->status == MUSB_DMA_STATUS_BUSY);
 
-   /* Let targets check/tweak the arguments */
-   if (musb->ops->adjust_channel_params) {
-   int ret = musb->ops->adjust_channel_params(channel,
-   packet_sz, , _addr, );
-   if (ret)
-   return ret;
-   }
-
/*
 * The DMA engine in RTL1.8 and above cannot handle
 * DMA addresses that are not aligned to a 4 byte boundary.
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/14] usb: musb: merge musbhsdma.h into musbhsdma.c

[Bin Liu]

Now Blackfin support is removed, header musbhsdma.h is only included in
musbhsdma.c. So let's merge the content in musbhsdma.h to musbhsdma.c
and delete musbhsdma.h.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musbhsdma.c | 66 +++-
 drivers/usb/musb/musbhsdma.h | 72 
 2 files changed, 65 insertions(+), 73 deletions(-)
 delete mode 100644 drivers/usb/musb/musbhsdma.h

diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 4389fc3422bd..57d416a110a5 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -10,7 +10,71 @@
 #include 
 #include 
 #include "musb_core.h"
-#include "musbhsdma.h"
+
+#define MUSB_HSDMA_BASE0x200
+#define MUSB_HSDMA_INTR(MUSB_HSDMA_BASE + 0)
+#define MUSB_HSDMA_CONTROL 0x4
+#define MUSB_HSDMA_ADDRESS 0x8
+#define MUSB_HSDMA_COUNT   0xc
+
+#define MUSB_HSDMA_CHANNEL_OFFSET(_bchannel, _offset)  \
+   (MUSB_HSDMA_BASE + (_bchannel << 4) + _offset)
+
+#define musb_read_hsdma_addr(mbase, bchannel)  \
+   musb_readl(mbase,   \
+  MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_ADDRESS))
+
+#define musb_write_hsdma_addr(mbase, bchannel, addr) \
+   musb_writel(mbase, \
+   MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_ADDRESS), \
+   addr)
+
+#define musb_read_hsdma_count(mbase, bchannel) \
+   musb_readl(mbase,   \
+  MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_COUNT))
+
+#define musb_write_hsdma_count(mbase, bchannel, len) \
+   musb_writel(mbase, \
+   MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_COUNT), \
+   len)
+/* control register (16-bit): */
+#define MUSB_HSDMA_ENABLE_SHIFT0
+#define MUSB_HSDMA_TRANSMIT_SHIFT  1
+#define MUSB_HSDMA_MODE1_SHIFT 2
+#define MUSB_HSDMA_IRQENABLE_SHIFT 3
+#define MUSB_HSDMA_ENDPOINT_SHIFT  4
+#define MUSB_HSDMA_BUSERROR_SHIFT  8
+#define MUSB_HSDMA_BURSTMODE_SHIFT 9
+#define MUSB_HSDMA_BURSTMODE   (3 << MUSB_HSDMA_BURSTMODE_SHIFT)
+#define MUSB_HSDMA_BURSTMODE_UNSPEC0
+#define MUSB_HSDMA_BURSTMODE_INCR4 1
+#define MUSB_HSDMA_BURSTMODE_INCR8 2
+#define MUSB_HSDMA_BURSTMODE_INCR163
+
+#define MUSB_HSDMA_CHANNELS8
+
+struct musb_dma_controller;
+
+struct musb_dma_channel {
+   struct dma_channel  channel;
+   struct musb_dma_controller  *controller;
+   u32 start_addr;
+   u32 len;
+   u16 max_packet_sz;
+   u8  idx;
+   u8  epnum;
+   u8  transmit;
+};
+
+struct musb_dma_controller {
+   struct dma_controller   controller;
+   struct musb_dma_channel channel[MUSB_HSDMA_CHANNELS];
+   void*private_data;
+   void __iomem*base;
+   u8  channel_count;
+   u8  used_channels;
+   int irq;
+};
 
 static void dma_channel_release(struct dma_channel *channel);
 
diff --git a/drivers/usb/musb/musbhsdma.h b/drivers/usb/musb/musbhsdma.h
deleted file mode 100644
index 93665135aff1..
--- a/drivers/usb/musb/musbhsdma.h
+++ /dev/null
@@ -1,72 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * MUSB OTG driver - support for Mentor's DMA controller
- *
- * Copyright 2005 Mentor Graphics Corporation
- * Copyright (C) 2005-2007 by Texas Instruments
- */
-
-#define MUSB_HSDMA_BASE0x200
-#define MUSB_HSDMA_INTR(MUSB_HSDMA_BASE + 0)
-#define MUSB_HSDMA_CONTROL 0x4
-#define MUSB_HSDMA_ADDRESS 0x8
-#define MUSB_HSDMA_COUNT   0xc
-
-#define MUSB_HSDMA_CHANNEL_OFFSET(_bchannel, _offset)  \
-   (MUSB_HSDMA_BASE + (_bchannel << 4) + _offset)
-
-#define musb_read_hsdma_addr(mbase, bchannel)  \
-   musb_readl(mbase,   \
-  MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_ADDRESS))
-
-#define musb_write_hsdma_addr(mbase, bchannel, addr) \
-   musb_writel(mbase, \
-   MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_ADDRESS), \
-   addr)
-
-#define musb_read_hsdma_count(mbase, bchannel) \
-   musb_readl(mbase,   \
-  MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_COUNT))
-
-#define musb_write_hsdma_count(mbase, bchannel, len) \
-   musb_writel(mbase, \
-   MUSB_HSDMA_CHANNEL_OFFSET(bchannel, MUSB_HSDMA_COUNT), \
-   len)
-/* control register (16-bit): */
-#define MUSB_HSDMA_ENABLE_SHIFT0
-#defi

[PATCH 11/14] usb: musb: disable otg protocol support

[Bin Liu]

As decided in the discussion [1] we are deleting the otg protocol
support from the musb drivers.

First this patch disables the flags for enabling the otg protocols. We
will later gradually delete the otg protocol code from the musb drivers.

[1] https://www.spinics.net/lists/linux-usb/msg167003.html

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.c | 5 +
 drivers/usb/musb/musb_host.c   | 3 ++-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 5b8e6297ebed..eae8b1b1b45b 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1798,11 +1798,8 @@ int musb_gadget_setup(struct musb *musb)
 
/* this "gadget" abstracts/virtualizes the controller */
musb->g.name = musb_driver_name;
-#if IS_ENABLED(CONFIG_USB_MUSB_DUAL_ROLE)
-   musb->g.is_otg = 1;
-#elif IS_ENABLED(CONFIG_USB_MUSB_GADGET)
+   /* don't support otg protocols */
musb->g.is_otg = 0;
-#endif
INIT_DELAYED_WORK(>gadget_work, musb_gadget_work);
musb_g_init_endpoints(musb);
 
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index cd611a97f59e..8000c7c02f79 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2755,7 +2755,8 @@ int musb_host_setup(struct musb *musb, int power_budget)
musb->xceiv->otg->state = OTG_STATE_A_IDLE;
}
otg_set_host(musb->xceiv->otg, >self);
-   hcd->self.otg_port = 1;
+   /* don't support otg protocols */
+   hcd->self.otg_port = 0;
musb->xceiv->otg->host = >self;
hcd->power_budget = 2 * (power_budget ? : 250);
hcd->skip_phy_initialization = 1;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 14/14] usb: musb: gadget: fix to_musb_ep() to not return NULL

[Bin Liu]

UDC core ensures the usb_ep parameter passed in is not NULL, so
checking if (ep != NULL) is pointless.

Convert to_musb_ep() to a simple macro to not directly return NULL to
avoid warnings from code static analysis tools.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 466877b471ef..aaae24bb3c10 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -96,10 +96,7 @@ struct musb_ep {
u8  hb_mult;
 };
 
-static inline struct musb_ep *to_musb_ep(struct usb_ep *ep)
-{
-   return ep ? container_of(ep, struct musb_ep, end_point) : NULL;
-}
+#define to_musb_ep(ep) container_of((ep), struct musb_ep, end_point)
 
 static inline struct musb_request *next_request(struct musb_ep *ep)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/14] usb: musb: remove some register access wrapper functions

[Bin Liu]

The following wrappers were defined because of Blackfin support. Now
Blackfin support is removed, these wrappers are no longer needed, so
remove them.

musb_write_txfifosz
musb_write_txfifoadd
musb_write_rxfifosz
musb_write_rxfifoadd
musb_write_ulpi_buscontrol
musb_read_txfifosz
musb_read_txfifoadd
musb_read_rxfifosz
musb_read_rxfifoadd
musb_read_ulpi_buscontrol
musb_read_hwvers

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c | 42 -
 drivers/usb/musb/musb_regs.h | 55 
 2 files changed, 21 insertions(+), 76 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4ff6da1aa775..d3f9f7e5f353 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1240,25 +1240,25 @@ void musb_stop(struct musb *musb)
/* REVISIT error check:  be sure ep0 can both rx and tx ... */
switch (cfg->style) {
case FIFO_TX:
-   musb_write_txfifosz(mbase, c_size);
-   musb_write_txfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_TXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_TXFIFOADD, c_off);
hw_ep->tx_double_buffered = !!(c_size & MUSB_FIFOSZ_DPB);
hw_ep->max_packet_sz_tx = maxpacket;
break;
case FIFO_RX:
-   musb_write_rxfifosz(mbase, c_size);
-   musb_write_rxfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_RXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_RXFIFOADD, c_off);
hw_ep->rx_double_buffered = !!(c_size & MUSB_FIFOSZ_DPB);
hw_ep->max_packet_sz_rx = maxpacket;
break;
case FIFO_RXTX:
-   musb_write_txfifosz(mbase, c_size);
-   musb_write_txfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_TXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_TXFIFOADD, c_off);
hw_ep->rx_double_buffered = !!(c_size & MUSB_FIFOSZ_DPB);
hw_ep->max_packet_sz_rx = maxpacket;
 
-   musb_write_rxfifosz(mbase, c_size);
-   musb_write_rxfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_RXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_RXFIFOADD, c_off);
hw_ep->tx_double_buffered = hw_ep->rx_double_buffered;
hw_ep->max_packet_sz_tx = maxpacket;
 
@@ -1466,7 +1466,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
}
 
/* log release info */
-   musb->hwvers = musb_read_hwvers(mbase);
+   musb->hwvers = musb_readw(mbase, MUSB_HWVERS);
pr_debug("%s: %sHDRC RTL version %d.%d%s\n",
 musb_driver_name, type, MUSB_HWVERS_MAJOR(musb->hwvers),
 MUSB_HWVERS_MINOR(musb->hwvers),
@@ -2311,9 +2311,9 @@ static void musb_deassert_reset(struct work_struct *work)
 
/* program PHY to use external vBus if required */
if (plat->extvbus) {
-   u8 busctl = musb_read_ulpi_buscontrol(musb->mregs);
+   u8 busctl = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL);
busctl |= MUSB_ULPI_USE_EXTVBUS;
-   musb_write_ulpi_buscontrol(musb->mregs, busctl);
+   musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, busctl);
}
 
if (musb->xceiv->otg->default_a) {
@@ -2482,7 +2482,7 @@ static void musb_save_context(struct musb *musb)
 
musb->context.frame = musb_readw(musb_base, MUSB_FRAME);
musb->context.testmode = musb_readb(musb_base, MUSB_TESTMODE);
-   musb->context.busctl = musb_read_ulpi_buscontrol(musb->mregs);
+   musb->context.busctl = musb_readb(musb_base, MUSB_ULPI_BUSCONTROL);
musb->context.power = musb_readb(musb_base, MUSB_POWER);
musb->context.intrusbe = musb_readb(musb_base, MUSB_INTRUSBE);
musb->context.index = musb_readb(musb_base, MUSB_INDEX);
@@ -2511,13 +2511,13 @@ static void musb_save_context(struct musb *musb)
 
if (musb->dyn_fifo) {
musb->context.index_regs[i].txfifoadd =
-   musb_read_txfifoadd(musb_base);
+   musb_readw(musb_base, MUSB_TXFIFOADD);
musb->context.index_regs[i].rxfifoadd =
-   musb_read_rxfifoadd(musb_base);
+   musb_readw(musb_base, MUSB_RXFIFOADD);
musb->context.index_regs[i].txfifosz =
-   musb_read_txfifosz(musb_base);
+   musb_readb(musb_base, MUSB_TXFIFOSZ);
 

[PATCH 00/14] musb patches for v4.18-rc1

[Bin Liu]

Hi Greg,

Here are the musb patches for v4.18 rc1, mainly cleaning up the drivers after
BLACKFIN arch support is deleted, also having one patch from Johan to prepare
the USB PHY framework further work.

Please let me know if any change is needed.

Regards,
-Bin.


Bin Liu (13):
  usb: musb: merge musbhsdma.h into musbhsdma.c
  usb: musb: remove readl/writel from struct musb_platform_ops
  usb: musb: remove adjust_channel_params() callback from musb_platform_ops
  usb: musb: remove some register access wrapper functions
  usb: musb: remove duplicated quirks flag
  usb: musb: dsps: remove duplicated get_musb_port_mode()
  usb: musb: remove duplicated port mode enum
  usb: musb: remove unused members in struct musb_hdrc_config
  usb: musb: break the huge isr musb_stage0_irq() into small functions
  usb: musb: remove references to default_a of struct usb_otg
  usb: musb: disable otg protocol support
  usb: musb: gadget: fix to_musb_request() to not return NULL
  usb: musb: gadget: fix to_musb_ep() to not return NULL

Johan Hovold (1):
  USB: musb: dsps: propagate device-tree node

 drivers/usb/musb/am35x.c|   3 -
 drivers/usb/musb/da8xx.c|   2 -
 drivers/usb/musb/davinci.c  |  49 ++-
 drivers/usb/musb/musb_core.c| 833 
 drivers/usb/musb/musb_core.h|  16 +-
 drivers/usb/musb/musb_cppi41.c  |   4 +-
 drivers/usb/musb/musb_dma.h |  10 +-
 drivers/usb/musb/musb_dsps.c|  30 +-
 drivers/usb/musb/musb_gadget.c  |   8 +-
 drivers/usb/musb/musb_gadget.h  |  10 +-
 drivers/usb/musb/musb_host.c|   8 +-
 drivers/usb/musb/musb_io.h  |   6 +-
 drivers/usb/musb/musb_regs.h|  55 ---
 drivers/usb/musb/musb_virthub.c |   2 +-
 drivers/usb/musb/musbhsdma.c|  74 +++-
 drivers/usb/musb/musbhsdma.h|  72 
 drivers/usb/musb/omap2430.c |   5 -
 drivers/usb/musb/sunxi.c|  14 +-
 drivers/usb/musb/ux500.c|   2 -
 include/linux/usb/musb.h|  15 -
 20 files changed, 543 insertions(+), 675 deletions(-)
 delete mode 100644 drivers/usb/musb/musbhsdma.h

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/14] usb: musb: remove duplicated quirks flag

[Bin Liu]

Both musb_io and musb_platform_ops in struct musb define a quirks flag
for the same purpose.  Let's remove the one in struct musb_io, and use
that in struct musb_platform_ops instead.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c   | 10 --
 drivers/usb/musb/musb_cppi41.c |  4 ++--
 drivers/usb/musb/musb_dma.h| 10 +-
 drivers/usb/musb/musb_io.h |  2 --
 4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index d3f9f7e5f353..84f25a2b078d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1493,7 +1493,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
 
hw_ep->fifo = musb->io.fifo_offset(i) + mbase;
 #if IS_ENABLED(CONFIG_USB_MUSB_TUSB6010)
-   if (musb->io.quirks & MUSB_IN_TUSB) {
+   if (musb->ops->quirks & MUSB_IN_TUSB) {
hw_ep->fifo_async = musb->async + 0x400 +
musb->io.fifo_offset(i);
hw_ep->fifo_sync = musb->sync + 0x400 +
@@ -2176,11 +2176,9 @@ static void musb_deassert_reset(struct work_struct *work)
goto fail2;
}
 
-   if (musb->ops->quirks)
-   musb->io.quirks = musb->ops->quirks;
 
/* Most devices use indexed offset or flat offset */
-   if (musb->io.quirks & MUSB_INDEXED_EP) {
+   if (musb->ops->quirks & MUSB_INDEXED_EP) {
musb->io.ep_offset = musb_indexed_ep_offset;
musb->io.ep_select = musb_indexed_ep_select;
} else {
@@ -2188,7 +2186,7 @@ static void musb_deassert_reset(struct work_struct *work)
musb->io.ep_select = musb_flat_ep_select;
}
 
-   if (musb->io.quirks & MUSB_G_NO_SKB_RESERVE)
+   if (musb->ops->quirks & MUSB_G_NO_SKB_RESERVE)
musb->g.quirk_avoids_skb_reserve = 1;
 
/* At least tusb6010 has its own offsets */
@@ -2647,7 +2645,7 @@ static int musb_suspend(struct device *dev)
;
musb->flush_irq_work = false;
 
-   if (!(musb->io.quirks & MUSB_PRESERVE_SESSION))
+   if (!(musb->ops->quirks & MUSB_PRESERVE_SESSION))
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 
WARN_ON(!list_empty(>pending_list));
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index d0dd4f470bbe..7fbb8a307145 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -614,7 +614,7 @@ static int cppi41_dma_channel_abort(struct dma_channel 
*channel)
}
 
/* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */
-   if (musb->io.quirks & MUSB_DA8XX)
+   if (musb->ops->quirks & MUSB_DA8XX)
mdelay(250);
 
tdbit = 1 << cppi41_channel->port_num;
@@ -773,7 +773,7 @@ struct dma_controller *
controller->controller.is_compatible = cppi41_is_compatible;
controller->controller.musb = musb;
 
-   if (musb->io.quirks & MUSB_DA8XX) {
+   if (musb->ops->quirks & MUSB_DA8XX) {
controller->tdown_reg = DA8XX_USB_TEARDOWN;
controller->autoreq_reg = DA8XX_USB_AUTOREQ;
controller->set_dma_mode = da8xx_set_dma_mode;
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
index 0fc8cd0c2a5c..8f60271c0a9d 100644
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -44,31 +44,31 @@
 #endif
 
 #ifdef CONFIG_USB_UX500_DMA
-#define musb_dma_ux500(musb)   (musb->io.quirks & MUSB_DMA_UX500)
+#define musb_dma_ux500(musb)   (musb->ops->quirks & MUSB_DMA_UX500)
 #else
 #define musb_dma_ux500(musb)   0
 #endif
 
 #ifdef CONFIG_USB_TI_CPPI41_DMA
-#define musb_dma_cppi41(musb)  (musb->io.quirks & MUSB_DMA_CPPI41)
+#define musb_dma_cppi41(musb)  (musb->ops->quirks & MUSB_DMA_CPPI41)
 #else
 #define musb_dma_cppi41(musb)  0
 #endif
 
 #ifdef CONFIG_USB_TI_CPPI_DMA
-#define musb_dma_cppi(musb)(musb->io.quirks & MUSB_DMA_CPPI)
+#define musb_dma_cppi(musb)(musb->ops->quirks & MUSB_DMA_CPPI)
 #else
 #define musb_dma_cppi(musb)0
 #endif
 
 #ifdef CONFIG_USB_TUSB_OMAP_DMA
-#define tusb_dma_omap(musb)(musb->io.quirks & MUSB_DMA_TUSB_OMAP)
+#define tusb_dma_omap(musb)(musb->ops->quirks & MUSB_DMA_TUSB_OMAP)
 #else
 #define tusb_dma_omap(musb)0
 #endif
 
 #ifdef CONFIG_USB_INVENTRA_DMA
-#define musb_dma_inventra(musb)(musb->io.quirks & 
MUSB_DMA_INVENTRA)
+#define musb_dma_inventra(musb)(musb->ops->quirks & 
MUSB_DMA_INVENTRA)
 #else
 #defin

[PATCH 02/14] usb: musb: remove readl/writel from struct musb_platform_ops

[Bin Liu]

Now Blackfin support is removed, we no longer need function pointers for
musb_readl() and musb_writel().

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c | 34 --
 drivers/usb/musb/musb_core.h |  4 
 drivers/usb/musb/musb_io.h   |  4 ++--
 3 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index fb5e4523dc28..4ff6da1aa775 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -274,20 +274,6 @@ static void musb_default_writew(void __iomem *addr, 
unsigned offset, u16 data)
__raw_writew(data, addr + offset);
 }
 
-static u32 musb_default_readl(const void __iomem *addr, unsigned offset)
-{
-   u32 data = __raw_readl(addr + offset);
-
-   trace_musb_readl(__builtin_return_address(0), addr, offset, data);
-   return data;
-}
-
-static void musb_default_writel(void __iomem *addr, unsigned offset, u32 data)
-{
-   trace_musb_writel(__builtin_return_address(0), addr, offset, data);
-   __raw_writel(data, addr + offset);
-}
-
 /*
  * Load an endpoint's FIFO
  */
@@ -390,10 +376,20 @@ static void musb_default_read_fifo(struct musb_hw_ep 
*hw_ep, u16 len, u8 *dst)
 void (*musb_writew)(void __iomem *addr, unsigned offset, u16 data);
 EXPORT_SYMBOL_GPL(musb_writew);
 
-u32 (*musb_readl)(const void __iomem *addr, unsigned offset);
+u32 musb_readl(const void __iomem *addr, unsigned offset)
+{
+   u32 data = __raw_readl(addr + offset);
+
+   trace_musb_readl(__builtin_return_address(0), addr, offset, data);
+   return data;
+}
 EXPORT_SYMBOL_GPL(musb_readl);
 
-void (*musb_writel)(void __iomem *addr, unsigned offset, u32 data);
+void musb_writel(void __iomem *addr, unsigned offset, u32 data)
+{
+   trace_musb_writel(__builtin_return_address(0), addr, offset, data);
+   __raw_writel(data, addr + offset);
+}
 EXPORT_SYMBOL_GPL(musb_writel);
 
 #ifndef CONFIG_MUSB_PIO_ONLY
@@ -2158,8 +2154,6 @@ static void musb_deassert_reset(struct work_struct *work)
musb_writeb = musb_default_writeb;
musb_readw = musb_default_readw;
musb_writew = musb_default_writew;
-   musb_readl = musb_default_readl;
-   musb_writel = musb_default_writel;
 
/* The musb_platform_init() call:
 *   - adjusts musb->mregs
@@ -2226,10 +2220,6 @@ static void musb_deassert_reset(struct work_struct *work)
musb_readw = musb->ops->readw;
if (musb->ops->writew)
musb_writew = musb->ops->writew;
-   if (musb->ops->readl)
-   musb_readl = musb->ops->readl;
-   if (musb->ops->writel)
-   musb_writel = musb->ops->writel;
 
 #ifndef CONFIG_MUSB_PIO_ONLY
if (!musb->ops->dma_init || !musb->ops->dma_exit) {
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 8a74cb2907f8..a4bf1e9e2d2c 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -127,8 +127,6 @@ enum musb_g_ep0_state {
  * @writeb:write 8 bits
  * @readw: read 16 bits
  * @writew:write 16 bits
- * @readl: read 32 bits
- * @writel:write 32 bits
  * @read_fifo: reads the fifo
  * @write_fifo:writes to fifo
  * @dma_init:  platform specific dma init function
@@ -174,8 +172,6 @@ struct musb_platform_ops {
void(*writeb)(void __iomem *addr, unsigned offset, u8 data);
u16 (*readw)(const void __iomem *addr, unsigned offset);
void(*writew)(void __iomem *addr, unsigned offset, u16 data);
-   u32 (*readl)(const void __iomem *addr, unsigned offset);
-   void(*writel)(void __iomem *addr, unsigned offset, u32 data);
void(*read_fifo)(struct musb_hw_ep *hw_ep, u16 len, u8 *buf);
void(*write_fifo)(struct musb_hw_ep *hw_ep, u16 len, const u8 *buf);
struct dma_controller *
diff --git a/drivers/usb/musb/musb_io.h b/drivers/usb/musb/musb_io.h
index b7025b2e6e00..b4d870b836aa 100644
--- a/drivers/usb/musb/musb_io.h
+++ b/drivers/usb/musb/musb_io.h
@@ -39,7 +39,7 @@ struct musb_io {
 extern void (*musb_writeb)(void __iomem *addr, unsigned offset, u8 data);
 extern u16 (*musb_readw)(const void __iomem *addr, unsigned offset);
 extern void (*musb_writew)(void __iomem *addr, unsigned offset, u16 data);
-extern u32 (*musb_readl)(const void __iomem *addr, unsigned offset);
-extern void (*musb_writel)(void __iomem *addr, unsigned offset, u32 data);
+extern u32 musb_readl(const void __iomem *addr, unsigned offset);
+extern void musb_writel(void __iomem *addr, unsigned offset, u32 data);
 
 #endif
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/14] usb: musb: remove references to default_a of struct usb_otg

[Bin Liu]

musb drivers do not use the otg fsm framework, so referencing to
otg->default_a doesn't have any effect, so remove the references.

But tusb6010 glue driver uses it locally to control the vbus power, so
keep the references in tusb6010 only.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/am35x.c   |  3 ---
 drivers/usb/musb/da8xx.c   |  2 --
 drivers/usb/musb/davinci.c | 49 --
 drivers/usb/musb/musb_core.c   |  9 ++--
 drivers/usb/musb/musb_dsps.c   |  2 --
 drivers/usb/musb/musb_gadget.c |  1 -
 drivers/usb/musb/musb_host.c   |  1 -
 drivers/usb/musb/omap2430.c|  5 -
 drivers/usb/musb/sunxi.c   |  2 --
 drivers/usb/musb/ux500.c   |  2 --
 10 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 0ad664efda6b..660641ab1545 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -201,7 +201,6 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
struct device *dev = musb->controller;
struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
struct omap_musb_board_data *data = plat->board_data;
-   struct usb_otg *otg = musb->xceiv->otg;
unsigned long flags;
irqreturn_t ret = IRQ_NONE;
u32 epintr, usbintr;
@@ -264,14 +263,12 @@ static irqreturn_t am35x_musb_interrupt(int irq, void 
*hci)
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
-   otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
del_timer(>dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
-   otg->default_a = 0;
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
}
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b8295ce7c4fe..0e5929e81d26 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -280,7 +280,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
-   otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
del_timer(>dev_timer);
@@ -295,7 +294,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
 */
musb->is_active = 0;
MUSB_DEV_MODE(musb);
-   otg->default_a = 0;
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
}
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 2ad39dcd2f4c..fb6bbd254ab7 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -311,14 +311,12 @@ static irqreturn_t davinci_musb_interrupt(int irq, void 
*__hci)
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
-   otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
del_timer(>dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
-   otg->default_a = 0;
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
}
@@ -425,6 +423,9 @@ static int davinci_musb_init(struct musb *musb)
 
 static int davinci_musb_exit(struct musb *musb)
 {
+   int maxdelay = 30;
+   u8  devctl, warn = 0;
+
del_timer_sync(>dev_timer);
 
/* force VBUS off */
@@ -438,31 +439,27 @@ static int davinci_musb_exit(struct musb *musb)
 
davinci_musb_source_power(musb, 0 /*off*/, 1);
 
-   /* delay, to avoid problems with module reload */
-   if (musb->xceiv->otg->default_a) {
-   int maxdelay = 30;
-   u8  devctl, warn = 0;
+   /*
+* delay, to a

Re: Pandaboard OMAP4 MUSB short packet hang on UVC gadget

[Bin Liu]

Hi,

On Tue, May 15, 2018 at 10:58:16PM +0100, Kieran Bingham wrote:
> Hi Bin, Felipe,
> 
> We have been trying to get a UVC gadget operational on the Pandaboard ES
> platform, and we believe we've hit an issue with short packets on the MUSB in
> DMA mode.
 
I don't have a Pandaboard. Does it use MUSB internal DMA engine?

> Using the g_webcam module, I can instantiate a UVC video node, and use the
> uvc-gadget helper application [0] to handle the frame generation.
> 
> With DMA disabled using CONFIG_MUSB_PIO_ONLY I am able to process frames on 
> the
> host using yavta [1]
> 
> However when DMA is enabled, only the first frame is transmitted to the host,
> and I get an error from the musb_log debug prints which indicate that the
> MUSB_TXCSR_TXPKTRDY is not cleared following a short packet.

The MUSB_TXCSR_TXPKTRDY bit should be clear by the MUSB controller after
the TX packet is transmitted, there is no sw involved.

> A capture of the musb_log trace-points [2] shows the following at the end of 
> the
> log:
> 
> The 'mode' has been set to '0' instead of '1', buffer length is 902 instead of
> 1024 (expected behaviour for the last packet):
>   "ep13-Tx pkt_sz 1024, dma_addr 0xadfbbc00 length 902, mode 0"
> 
> And then there is a fault reported:
>   "ep13 old packet still ready , txcsr 2003"

I just checked the same testcase on Beaglebone Black which uses CPPI41
DMA, this bit is cleared after sent the 902-bytes end-of-frame packet.

> The host (linux, running the uvc driver with extra debug prints) reports:
> 
> [24517.711147] uvcvideo: decode_data: len: 1022, maxlen 460800
> [24517.759117] uvcvideo: decode_data: len: 1022, maxlen 459778
>  ...
> [24529.375018] uvcvideo: decode_data: len: 1022, maxlen 2944
> [24529.399073] uvcvideo: decode_data: len: 1022, maxlen 1922
> [24529.427014] uvcvideo: decode_data: len: 900, maxlen 900
> 
> And receives no further data following the end of the first frame.
> 
> 
> Are there any known issues on the musb-gadget with short-packet handling - or
> any other tests I can perform to check this ?

Nothing that I am aware of.
 
> Should I be looking to try to get the txstate() call to re-execute after a 
> delay
> (using musb_queue_resume_work() perhaps?) or does this indicate that the

I am not sure how that will make the controller to clear the TXPKTRDY
bit.

> hardware has jammed ? Stopping the pipeline (the yavta capture) and 
> restarting,

Likely that is the case.

> will successfully transfer (only) the first frame again.
> 
> Any other hints and tips appreciated!

Does the usecase ever worked on Pandaboard with older kernels? If so,
you can try to bisect the commit which causes the regression.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] musb fixes for v4.17-rc6

[Bin Liu]

Hi Greg,

Here is one musb fix for v4.17-rc6, which fixes a race condition in musb
transitioning from resume to suspend. Please let me know if any change is
needed.

Regards,
-Bin.


Daniel Glöckner (1):
  usb: musb: fix remote wakeup racing with suspend

 drivers/usb/musb/musb_host.c|  5 -
 drivers/usb/musb/musb_host.h|  7 +--
 drivers/usb/musb/musb_virthub.c | 25 +++--
 3 files changed, 24 insertions(+), 13 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: musb: fix remote wakeup racing with suspend

[Bin Liu]

From: Daniel Glöckner <d...@emlix.com>

It has been observed that writing 0xF2 to the power register while it
reads as 0xF4 results in the register having the value 0xF0, i.e. clearing
RESUME and setting SUSPENDM in one go does not work. It might also violate
the USB spec to transition directly from resume to suspend, especially
when not taking T_DRSMDN into account. But this is what happens when a
remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the
root hub and musb_bus_suspend being called.

This commit returns -EBUSY when musb_bus_suspend is called while remote
wakeup is signalled and thus avoids to reset the RESUME bit. Ignoring
this error when musb_port_suspend is called from musb_hub_control is ok.

Signed-off-by: Daniel Glöckner <d...@emlix.com>
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_host.c|  5 -
 drivers/usb/musb/musb_host.h|  7 +--
 drivers/usb/musb/musb_virthub.c | 25 +++--
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index e7f99d55922a..15a42cee0a9c 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2524,8 +2524,11 @@ static int musb_bus_suspend(struct usb_hcd *hcd)
 {
struct musb *musb = hcd_to_musb(hcd);
u8  devctl;
+   int ret;
 
-   musb_port_suspend(musb, true);
+   ret = musb_port_suspend(musb, true);
+   if (ret)
+   return ret;
 
if (!is_host_active(musb))
return 0;
diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h
index 72392bbcd0a4..2999845632ce 100644
--- a/drivers/usb/musb/musb_host.h
+++ b/drivers/usb/musb/musb_host.h
@@ -67,7 +67,7 @@ static inline struct musb_qh *first_qh(struct list_head *q)
 extern void musb_root_disconnect(struct musb *musb);
 extern void musb_host_resume_root_hub(struct musb *musb);
 extern void musb_host_poke_root_hub(struct musb *musb);
-extern void musb_port_suspend(struct musb *musb, bool do_suspend);
+extern int musb_port_suspend(struct musb *musb, bool do_suspend);
 extern void musb_port_reset(struct musb *musb, bool do_reset);
 extern void musb_host_finish_resume(struct work_struct *work);
 #else
@@ -99,7 +99,10 @@ static inline void musb_root_disconnect(struct musb *musb)   
{}
 static inline void musb_host_resume_root_hub(struct musb *musb){}
 static inline void musb_host_poll_rh_status(struct musb *musb) {}
 static inline void musb_host_poke_root_hub(struct musb *musb)  {}
-static inline void musb_port_suspend(struct musb *musb, bool do_suspend) {}
+static inline int musb_port_suspend(struct musb *musb, bool do_suspend)
+{
+   return 0;
+}
 static inline void musb_port_reset(struct musb *musb, bool do_reset) {}
 static inline void musb_host_finish_resume(struct work_struct *work) {}
 #endif
diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
index 5165d2b07ade..2f8dd9826e94 100644
--- a/drivers/usb/musb/musb_virthub.c
+++ b/drivers/usb/musb/musb_virthub.c
@@ -48,14 +48,14 @@ void musb_host_finish_resume(struct work_struct *work)
spin_unlock_irqrestore(>lock, flags);
 }
 
-void musb_port_suspend(struct musb *musb, bool do_suspend)
+int musb_port_suspend(struct musb *musb, bool do_suspend)
 {
struct usb_otg  *otg = musb->xceiv->otg;
u8  power;
void __iomem*mbase = musb->mregs;
 
if (!is_host_active(musb))
-   return;
+   return 0;
 
/* NOTE:  this doesn't necessarily put PHY into low power mode,
 * turning off its clock; that's a function of PHY integration and
@@ -66,16 +66,20 @@ void musb_port_suspend(struct musb *musb, bool do_suspend)
if (do_suspend) {
int retries = 1;
 
-   power &= ~MUSB_POWER_RESUME;
-   power |= MUSB_POWER_SUSPENDM;
-   musb_writeb(mbase, MUSB_POWER, power);
+   if (power & MUSB_POWER_RESUME)
+   return -EBUSY;
 
-   /* Needed for OPT A tests */
-   power = musb_readb(mbase, MUSB_POWER);
-   while (power & MUSB_POWER_SUSPENDM) {
+   if (!(power & MUSB_POWER_SUSPENDM)) {
+   power |= MUSB_POWER_SUSPENDM;
+   musb_writeb(mbase, MUSB_POWER, power);
+
+   /* Needed for OPT A tests */
power = musb_readb(mbase, MUSB_POWER);
-   if (retries-- < 1)
-   break;
+   while (power & MUSB_POWER_SUSPENDM) {
+   power = musb_readb(mbase, MUSB_POWER);
+   if (retries-- < 1)
+   break;
+   }
}
 
musb_dbg(musb, &qu

Re: [PATCH v2] musb: fix remote wakeup racing with suspend

[Bin Liu]

Hi,

On Thu, May 03, 2018 at 04:43:25PM +0200, Daniel Glöckner wrote:
> It has been observed that writing 0xF2 to the power register while it
> reads as 0xF4 results in the register having the value 0xF0, i.e. clearing
> RESUME and setting SUSPENDM in one go does not work. It might also violate
> the USB spec to transition directly from resume to suspend, especially
> when not taking T_DRSMDN into account. But this is what happens when a
> remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the
> root hub and musb_bus_suspend being called.
> 
> This commit returns -EBUSY when musb_bus_suspend is called while remote
> wakeup is signalled and thus avoids to reset the RESUME bit. Ignoring
> this error when musb_port_suspend is called from musb_hub_control is ok.
> 
> Signed-off-by: Daniel Glöckner 

Applied. Thanks.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] musb: fix remote wakeup racing with suspend

[Bin Liu]

On Wed, May 02, 2018 at 03:12:16AM +0200, Daniel Glöckner wrote:
> Hi,
> 
> On Tue, May 01, 2018 at 08:48:11AM -0500, Bin Liu wrote:
> > On Fri, Apr 27, 2018 at 03:00:05PM +0200, Daniel Glöckner wrote:
> > > It has been observed that writing 0xF2 to the power register while it
> > > reads as 0xF4 results in the register having the value 0xF0, i.e. clearing
> > > RESUME and setting SUSPENDM in one go does not work. It might also violate
> > > the USB spec to transition directly from resume to suspend, especially
> > > when not taking T_DRSMDN into account. But this is what happens when a
> > > remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the
> > > root hub and musb_bus_suspend being called.
> > > 
> > > This commit returns -EBUSY when musb_bus_suspend is called while remote
> > > wakeup is signalled and thus avoids to reset the RESUME bit. Remember that
> > > resume can be signalled only when the bus was already suspended, so it is
> > > safe to skip the following steps even when musb_hub_control ignores the
> > 
> > what do you mean by "skip the following steps"?
> 
> Setting USB_PORT_STAT_SUSPEND in musb->port1_status, changing
> musb->xceiv->otg->state, setting musb->is_active, etc.

Okay, but I am not sure how "even musb_hub_control ignores the error..."
is relevant in this statement. The steps are safely skipped in
musb_port_suspend() when MUSB_POWER_RESUME bit is set, no matter how
musb_hub_control() to deal with the return code from
musb_port_suspend(). Did I misunderstand anything?

> 
> > > error returned by musb_port_suspend.
> > > 
> > > The resume part of musb_port_suspend is modified to also accept a pending
> > > remote wakeup, to bring it to the end after T_DRSMDN has passed.
> > 
> > Can you please explain more here? I am not sure when musb_port_suspend()
> > is involved in resume by remote wakeup, and what case is a 'pending'
> > remote wakeup?
> 
> With 'pending' I was referring to the situation when MUSB_POWER_RESUME
> has been set by the controller in the power register as a result of
> of a detected remote wakeup.

Okay.

> 
> I see... finish_resume_work is already scheduled by musb_stage0_irq.

Yes.

> So the condition of the if statement probably does not need to be

I think so too. BTY, I don't think this pending remote wakeup situation
will trigger ClearPortFeature, which is the only case that invokes that
if statement you changed.

> changed. I'll test without that part and make a v2 patch if it works.
> 
> Btw., do you know why that 1 iterations while loop is needed in
> musb_port_suspend to pass the OTG Protocol Test as indicated by the
> comment?

Unfortunately I don't. There are many mysteries in the musb drivers :)

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Support gadget mode when the port is set to dual role

[Bin Liu]

On Tue, May 01, 2018 at 03:26:57PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> Le mardi 01 mai 2018 à 07:25 -0500, Bin Liu a écrit :
> > On Mon, Apr 30, 2018 at 11:08:42PM +0200, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > Le samedi 21 avril 2018 à 09:34 -0500, Bin Liu a écrit :
> > > > Okay, this came down to an argument that whether we should require
> > > > loading a gadget driver on a dual-role port to work in host mode,
> > > > which is currently required on musb since a long long time ago.
> > > > 
> > > > I understand the requirement is kinda unnecessary, but since it
> > > > already
> > > > exists on musb stack for a long time, I don't plan to change it.
> > > > Because I
> > > > cannot think of a use case in real products that doesn't
> > > > automatically
> > > > load a gadget function on the dual-role port.
> > > > 
> > > > If you can explain a use case in real world (not a engineering
> > > > lab)
> > > > that the gadget driver will not be loaded at linux booting up, but
> > > > later based on user's input, I will reconsider my decision. To
> > > > remove
> > > > this requirement from musb stack, the work is more than this
> > > > patch.
> > > 
> > > My use case here is to support common GNU/Linux-based distributions,
> > > not
> > > use-case-specific varieties of GNU/Linux-based rootfs. So my point
> > > here
> > > would be that most distros will (and probably should) ship g_ether
> > > as a
> > > module but without any particular reason to autoload it, or any
> > > other
> > > gadget module in particular, since the system is general-purpose.
> > 
> > This is the case I called it "in a engineering lab", not a real
> > product.
> 
> To me, this sounds more like "daily use with upstream like on any
> laptop/desktop" rather than an engineering lab, but that's not the main
> point here.
> 
> > > Then, imagine a user wants to plug a USB device through OTG (say,
> > > because it's the only USB port available at all on the tablet
> > > they're
> > > using), it simply won't work. It won't be obvious to that user that
> > > this
> > > is because no gadget is loaded, since what they want to do does not
> > > involve using gadget mode at any point.
> > 
> > If a tablet has a dual-role usb port, it is designed to use a gadget
> > driver,
> 
> I don't understand the logic behind this assertion. If it has a dual-
> role USB port, then its hardware allows both use cases. It's obvious
> that the use case is up to the user of the device since it can be
> switched by software and is not fixed at design time.

My view is the whole (embedded) system, not just Linux itself. If the
hardware designs a dual-role port, a gadget driver has to be used.
Otherwise, define the port as host-only, either in the hardware design,
or at least in device tree.

> >  which has to be loaded at some point. In the case you described
> > above, when the gadget driver will be loaded? and how?
> 
> Again, loading a gadget driver is not part of the use case. In what I
> described, the user only wants to use the dual-role port for its host
> capability and does not care about gadget at all. When the device is
> plugged into a host, it will simply charge and not propose any USB
> device features.

It sounds to me a hacking to an existing product, not designing a new
product. If so, please hack it completely, define the port dr_mode to
host in the board device tree, then the port should work for you.

> > If a gadget driver will never be used, a host-only port should be on
> > the board, not a dual-role port.
> 
> Here as well, I think the use case is separate from the hardware design.
>  I crafted this patch because I was in the use case I described, with a
> tablet that only features a micro B USB OTG port. The form factor simply

I guess you meant micro-AB port, microB doesn't have an ID pin, cannot
make MUSB to work in host mode.

> does not allow having a full USB A female host-only port.
> 
> > > Do you think this is a valid use case? It surely is a common one and
> > > perfectly depicts my situation.
> > 
> > As I explained above, I don't think so.
> 
> I am really surprised that using regular upstream GNU/Linux
> distributions out of the box is not a valid use case for the MUSB
> driver. The situation I'm describing is exactly the same as buying a
> laptop with a preinstalled OS and replacing it with a regular distro. In
&

Re: [PATCH] musb: fix remote wakeup racing with suspend

[Bin Liu]

Hi,

On Fri, Apr 27, 2018 at 03:00:05PM +0200, Daniel Glöckner wrote:
> It has been observed that writing 0xF2 to the power register while it
> reads as 0xF4 results in the register having the value 0xF0, i.e. clearing
> RESUME and setting SUSPENDM in one go does not work. It might also violate
> the USB spec to transition directly from resume to suspend, especially
> when not taking T_DRSMDN into account. But this is what happens when a
> remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the
> root hub and musb_bus_suspend being called.
> 
> This commit returns -EBUSY when musb_bus_suspend is called while remote
> wakeup is signalled and thus avoids to reset the RESUME bit. Remember that
> resume can be signalled only when the bus was already suspended, so it is
> safe to skip the following steps even when musb_hub_control ignores the

what do you mean by "skip the following steps"?

> error returned by musb_port_suspend.
> 
> The resume part of musb_port_suspend is modified to also accept a pending
> remote wakeup, to bring it to the end after T_DRSMDN has passed.

Can you please explain more here? I am not sure when musb_port_suspend()
is involved in resume by remote wakeup, and what case is a 'pending'
remote wakeup?

> 
> Signed-off-by: Daniel Glöckner 
> ---
>  drivers/usb/musb/musb_host.c|  5 -
>  drivers/usb/musb/musb_host.h|  7 +--
>  drivers/usb/musb/musb_virthub.c | 27 ---
>  3 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 3a8451a..d05cb03 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -2522,8 +2522,11 @@ static int musb_bus_suspend(struct usb_hcd *hcd)
>  {
>   struct musb *musb = hcd_to_musb(hcd);
>   u8  devctl;
> + int ret;
>  
> - musb_port_suspend(musb, true);
> + ret = musb_port_suspend(musb, true);
> + if (ret)
> + return ret;
>  
>   if (!is_host_active(musb))
>   return 0;
> diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h
> index 72392bb..2999845 100644
> --- a/drivers/usb/musb/musb_host.h
> +++ b/drivers/usb/musb/musb_host.h
> @@ -67,7 +67,7 @@ extern void musb_host_rx(struct musb *, u8);
>  extern void musb_root_disconnect(struct musb *musb);
>  extern void musb_host_resume_root_hub(struct musb *musb);
>  extern void musb_host_poke_root_hub(struct musb *musb);
> -extern void musb_port_suspend(struct musb *musb, bool do_suspend);
> +extern int musb_port_suspend(struct musb *musb, bool do_suspend);
>  extern void musb_port_reset(struct musb *musb, bool do_reset);
>  extern void musb_host_finish_resume(struct work_struct *work);
>  #else
> @@ -99,7 +99,10 @@ static inline void musb_root_disconnect(struct musb *musb) 
> {}
>  static inline void musb_host_resume_root_hub(struct musb *musb)  {}
>  static inline void musb_host_poll_rh_status(struct musb *musb)   {}
>  static inline void musb_host_poke_root_hub(struct musb *musb){}
> -static inline void musb_port_suspend(struct musb *musb, bool do_suspend) {}
> +static inline int musb_port_suspend(struct musb *musb, bool do_suspend)
> +{
> + return 0;
> +}
>  static inline void musb_port_reset(struct musb *musb, bool do_reset) {}
>  static inline void musb_host_finish_resume(struct work_struct *work) {}
>  #endif
> diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c
> index 5165d2b..41b44ce 100644
> --- a/drivers/usb/musb/musb_virthub.c
> +++ b/drivers/usb/musb/musb_virthub.c
> @@ -48,14 +48,14 @@ void musb_host_finish_resume(struct work_struct *work)
>   spin_unlock_irqrestore(>lock, flags);
>  }
>  
> -void musb_port_suspend(struct musb *musb, bool do_suspend)
> +int musb_port_suspend(struct musb *musb, bool do_suspend)
>  {
>   struct usb_otg  *otg = musb->xceiv->otg;
>   u8  power;
>   void __iomem*mbase = musb->mregs;
>  
>   if (!is_host_active(musb))
> - return;
> + return 0;
>  
>   /* NOTE:  this doesn't necessarily put PHY into low power mode,
>* turning off its clock; that's a function of PHY integration and
> @@ -66,16 +66,20 @@ void musb_port_suspend(struct musb *musb, bool do_suspend)
>   if (do_suspend) {
>   int retries = 1;
>  
> - power &= ~MUSB_POWER_RESUME;
> - power |= MUSB_POWER_SUSPENDM;
> - musb_writeb(mbase, MUSB_POWER, power);
> + if (power & MUSB_POWER_RESUME)
> + return -EBUSY;
>  
> - /* Needed for OPT A tests */
> - power = musb_readb(mbase, MUSB_POWER);
> - while (power & MUSB_POWER_SUSPENDM) {
> + if (!(power & MUSB_POWER_SUSPENDM)) {
> + power |= MUSB_POWER_SUSPENDM;
> + musb_writeb(mbase, MUSB_POWER, power);
> +
> + 

Re: [PATCH] usb: musb: Support gadget mode when the port is set to dual role

[Bin Liu]

On Mon, Apr 30, 2018 at 11:08:42PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> Le samedi 21 avril 2018 à 09:34 -0500, Bin Liu a écrit :
> > Okay, this came down to an argument that whether we should require
> > loading a gadget driver on a dual-role port to work in host mode,
> > which is currently required on musb since a long long time ago.
> > 
> > I understand the requirement is kinda unnecessary, but since it
> > already
> > exists on musb stack for a long time, I don't plan to change it.
> > Because I
> > cannot think of a use case in real products that doesn't automatically
> > load a gadget function on the dual-role port.
> > 
> > If you can explain a use case in real world (not a engineering lab)
> > that the gadget driver will not be loaded at linux booting up, but
> > later based on user's input, I will reconsider my decision. To remove
> > this requirement from musb stack, the work is more than this patch.
> 
> My use case here is to support common GNU/Linux-based distributions, not
> use-case-specific varieties of GNU/Linux-based rootfs. So my point here
> would be that most distros will (and probably should) ship g_ether as a
> module but without any particular reason to autoload it, or any other
> gadget module in particular, since the system is general-purpose.

This is the case I called it "in a engineering lab", not a real product.

> Then, imagine a user wants to plug a USB device through OTG (say,
> because it's the only USB port available at all on the tablet they're
> using), it simply won't work. It won't be obvious to that user that this
> is because no gadget is loaded, since what they want to do does not
> involve using gadget mode at any point.

If a tablet has a dual-role usb port, it is designed to use a gadget
driver, which has to be loaded at some point. In the case you described
above, when the gadget driver will be loaded? and how?

If a gadget driver will never be used, a host-only port should be on
the board, not a dual-role port.

> Do you think this is a valid use case? It surely is a common one and
> perfectly depicts my situation.

As I explained above, I don't think so.

> Note that in addition to Allwinner devices, I also have omap3/4/5
> devices for testing things. I don't think I have other MUSB-enabled

Much more than what I have ;)

> devices in my collection though, but I would be willing to test fixes to
> this issue on the ones I have.

Appreciated it, but someone has to make the patches first. The one you
posted might be a good start, but it is not complete. The first problem
I see is that musb_start() will be called twice, one in the place you
patched, the other is when the gadget driver is bound to the UDC.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: musb: host: fix potential NULL pointer dereference

[Bin Liu]

On Mon, Apr 30, 2018 at 11:24:48AM -0700, Greg Kroah-Hartman wrote:
> On Mon, Apr 30, 2018 at 01:11:34PM -0500, Bin Liu wrote:
> > On Mon, Apr 30, 2018 at 09:42:15AM -0700, Greg Kroah-Hartman wrote:
> > > On Mon, Apr 30, 2018 at 11:20:53AM -0500, Bin Liu wrote:
> > > > musb_start_urb() doesn't check the pass-in parameter if it is NULL.  But
> > > > in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is
> > > > returned from first_qh(), which could be NULL.
> > > > 
> > > > So wrap the musb_start_urb() call here with a if condition check to
> > > > avoid the potential NULL pointer dereference.
> > > > 
> > > > Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX 
> > > > endpoint")
> > > 
> > > Nit, you forgot a ' ', this should be:
> > >   f283862f3b5c ("usb: musb: NAK timeout scheme on bulk TX endpoint")
> > 
> > Sorry, thanks.
> > 
> > > You also had one extra id value in there, odd.  I'll edit this by
> > 
> > Not sure why 'git blame' gives that one extra on my computer. I will see
> > if I will figure it out...
> 
> Why use 'git blame'?  I use:
>   git show -s --abbrev-commit --abbrev=12 --pretty=format:"%h (\"%s\")%n"
> 
> to generate these types of lines.

My ~/.gitconfig already has
abbrev = 12

but I used 'git blame' to find the commit id which introduces the bug,
then just directly copied the commit id from there... only until now
know 'git blame' gives 13 chars...

I will create a shortcut for the command you gave above to avoid this 13
chars problem from now on.

Thanks,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: musb: host: fix potential NULL pointer dereference

[Bin Liu]

On Mon, Apr 30, 2018 at 09:42:15AM -0700, Greg Kroah-Hartman wrote:
> On Mon, Apr 30, 2018 at 11:20:53AM -0500, Bin Liu wrote:
> > musb_start_urb() doesn't check the pass-in parameter if it is NULL.  But
> > in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is
> > returned from first_qh(), which could be NULL.
> > 
> > So wrap the musb_start_urb() call here with a if condition check to
> > avoid the potential NULL pointer dereference.
> > 
> > Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")
> 
> Nit, you forgot a ' ', this should be:
>   f283862f3b5c ("usb: musb: NAK timeout scheme on bulk TX endpoint")

Sorry, thanks.

> You also had one extra id value in there, odd.  I'll edit this by

Not sure why 'git blame' gives that one extra on my computer. I will see
if I will figure it out...

> hand...

Thanks,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] usb: musb: host: fix potential NULL pointer dereference

[Bin Liu]

musb_start_urb() doesn't check the pass-in parameter if it is NULL.  But
in musb_bulk_nak_timeout() the parameter passed to musb_start_urb() is
returned from first_qh(), which could be NULL.

So wrap the musb_start_urb() call here with a if condition check to
avoid the potential NULL pointer dereference.

Fixes: f283862f3b5cb("usb: musb: NAK timeout scheme on bulk TX endpoint")

Cc: sta...@vger.kernel.org # v3.7+
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_host.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 4fa372c845e1..e7f99d55922a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -990,7 +990,9 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct 
musb_hw_ep *ep,
/* set tx_reinit and schedule the next qh */
ep->tx_reinit = 1;
}
-   musb_start_urb(musb, is_in, next_qh);
+
+   if (next_qh)
+   musb_start_urb(musb, is_in, next_qh);
}
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] usb: musb: trace: fix NULL pointer dereference in musb_g_tx()

[Bin Liu]

The usb_request pointer could be NULL in musb_g_tx(), where the
tracepoint call would trigger the NULL pointer dereference failure when
parsing the members of the usb_request pointer.

Move the tracepoint call to where the usb_request pointer is already
checked to solve the issue.

Fixes: fc78003e5345a("usb: musb: gadget: add usb-request tracepoints")

Cc: sta...@vger.kernel.org # v4.8+
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index e564695c6c8d..71c5835ea9cd 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -417,7 +417,6 @@ void musb_g_tx(struct musb *musb, u8 epnum)
req = next_request(musb_ep);
request = >request;
 
-   trace_musb_req_tx(req);
csr = musb_readw(epio, MUSB_TXCSR);
musb_dbg(musb, "<== %s, txcsr %04x", musb_ep->end_point.name, csr);
 
@@ -456,6 +455,8 @@ void musb_g_tx(struct musb *musb, u8 epnum)
u8  is_dma = 0;
boolshort_packet = false;
 
+   trace_musb_req_tx(req);
+
if (dma && (csr & MUSB_TXCSR_DMAENAB)) {
is_dma = 1;
csr |= MUSB_TXCSR_P_WZC_BITS;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] musb fixes for v4.17-rc4

[Bin Liu]

Hi Greg,

Here are musb fixes for v4.17-rc4 to fix two NULL pointer dereference cases.
Please let me know if any change is needed.

Regards,
-Bin.


Bin Liu (2):
  usb: musb: host: fix potential NULL pointer dereference
  usb: musb: trace: fix NULL pointer dereference in musb_g_tx()

 drivers/usb/musb/musb_gadget.c | 3 ++-
 drivers/usb/musb/musb_host.c   | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: musb: gadget: fix to_musb_request() to not return NULL

[Bin Liu]

On Thu, Apr 26, 2018 at 11:25:16AM +0530, Sekhar Nori wrote:
> On Thursday 26 April 2018 11:23 AM, Sekhar Nori wrote:
> > On Thursday 19 April 2018 01:35 AM, Bin Liu wrote:
> >> The gadget function drivers should ensure the usb_request parameter
> >> passed in is not NULL. UDC core doesn't check if it is NULL, so MUSB
> >> driver shouldn't have to check it either.
> >>
> >> Convert to_musb_request() to a simple macro to not directly return NULL
> >> to avoid warnings from code static analysis tools.
> >>
> >> Signed-off-by: Bin Liu <b-...@ti.com>
> >> ---
> >>  drivers/usb/musb/musb_gadget.h | 5 +
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/usb/musb/musb_gadget.h 
> >> b/drivers/usb/musb/musb_gadget.h
> >> index 9c34aca06db6..466877b471ef 100644
> >> --- a/drivers/usb/musb/musb_gadget.h
> >> +++ b/drivers/usb/musb/musb_gadget.h
> >> @@ -60,10 +60,7 @@ struct musb_request {
> >>enum buffer_map_state map_state;
> >>  };
> >>  
> >> -static inline struct musb_request *to_musb_request(struct usb_request 
> >> *req)
> >> -{
> >> -  return req ? container_of(req, struct musb_request, request) : NULL;
> >> -}
> >> +#define to_musb_request(r)(container_of(r, struct musb_request, 
> >> request))
> > 
> > Unnecessary parens over container_of
> 
> And 'r' can actually be parenthesized for safety.

Thanks. v2 is sent.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] usb: musb: host: fix potential NULL pointer dereference

[Bin Liu]

musb_start_urb() doesn't check the 3rd pass-in parameter if it is NULL.
But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb()
is returned from first_qh(), which could be NULL.

So wrap the musb_start_urb() call here with a if condition check to
avoid the potential NULL pointer dereference.

Signed-off-by: Bin Liu <b-...@ti.com>
---
v2: no changes.

 drivers/usb/musb/musb_host.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index e50438ae241e..218aadef5bbf 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -990,7 +990,9 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct 
musb_hw_ep *ep,
/* set tx_reinit and schedule the next qh */
ep->tx_reinit = 1;
}
-   musb_start_urb(musb, is_in, next_qh);
+
+   if (next_qh)
+   musb_start_urb(musb, is_in, next_qh);
}
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/3] usb: musb: gadget: fix to_musb_ep() to not return NULL

[Bin Liu]

UDC core ensures the usb_ep parameter passed in is not NULL, so
checking if (ep != NULL) is pointless.

Convert to_musb_ep() to a simple macro to not directly return NULL to
avoid warnings from code static analysis tools.

Signed-off-by: Bin Liu <b-...@ti.com>
---
v2: revised the parentheses.

 drivers/usb/musb/musb_gadget.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 06d60848337f..d02663660813 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -96,10 +96,7 @@ struct musb_ep {
u8  hb_mult;
 };
 
-static inline struct musb_ep *to_musb_ep(struct usb_ep *ep)
-{
-   return ep ? container_of(ep, struct musb_ep, end_point) : NULL;
-}
+#define to_musb_ep(ep) container_of((ep), struct musb_ep, end_point)
 
 static inline struct musb_request *next_request(struct musb_ep *ep)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] usb: musb: gadget: fix to_musb_request() to not return NULL

[Bin Liu]

The gadget function drivers should ensure the usb_request parameter
passed in is not NULL. UDC core doesn't check if it is NULL, so MUSB
driver shouldn't have to check it either.

Convert to_musb_request() to a simple macro to not directly return NULL
to avoid warnings from code static analysis tools.

Signed-off-by: Bin Liu <b-...@ti.com>
---
v2: revised the parentheses.

 drivers/usb/musb/musb_gadget.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 9c34aca06db6..06d60848337f 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -60,10 +60,7 @@ struct musb_request {
enum buffer_map_state map_state;
 };
 
-static inline struct musb_request *to_musb_request(struct usb_request *req)
-{
-   return req ? container_of(req, struct musb_request, request) : NULL;
-}
+#define to_musb_request(r) container_of((r), struct musb_request, request)
 
 extern struct usb_request *
 musb_alloc_request(struct usb_ep *ep, gfp_t gfp_flags);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] usb: musb: fix enumeration after resume

[Bin Liu]

From: Andreas Kemnade <andr...@kemnade.info>

commit 17539f2f4f0b7fa906b508765c8ada07a1e45f52 upstream.

On dm3730 there are enumeration problems after resume.
Investigation led to the cause that the MUSB_POWER_SOFTCONN
bit is not set. If it was set before suspend (because it
was enabled via musb_pullup()), it is set in
musb_restore_context() so the pullup is enabled. But then
musb_start() is called which overwrites MUSB_POWER and
therefore disables MUSB_POWER_SOFTCONN, so no pullup is
enabled and the device is not enumerated.

So let's do a subset of what musb_start() does
in the same way as musb_suspend() does it. Platform-specific
stuff it still called as there might be some phy-related stuff
which needs to be enabled.
Also interrupts are enabled, as it was the original idea
of calling musb_start() in musb_resume() according to
Commit 6fc6f4b87cb3 ("usb: musb: Disable interrupts on suspend,
enable them on resume")

Cc: sta...@vger.kernel.org # v4.9+
Signed-off-by: Andreas Kemnade <andr...@kemnade.info>
Tested-by: Tony Lindgren <t...@atomide.com>
Signed-off-by: Bin Liu <b-...@ti.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/usb/musb/musb_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 2d9a8067eaca..0736cc27b5e7 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2710,7 +2710,8 @@ static int musb_resume(struct device *dev)
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
 
-   musb_start(musb);
+   musb_enable_interrupts(musb);
+   musb_platform_enable(musb);
 
spin_lock_irqsave(>lock, flags);
error = musb_run_resume_work(musb);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] usb: musb: Fix external abort in musb_remove on omap2430

[Bin Liu]

From: Merlijn Wajer <merl...@wizzup.org>

commit 94e46a4f2d5eb14059e42f313c098d4854847376 upstream.

This fixes an oops on unbind / module unload (on the musb omap2430
platform).

musb_remove function now calls musb_platform_exit before disabling
runtime pm.

Cc: sta...@vger.kernel.org # v4.9+
Signed-off-by: Merlijn Wajer <merl...@wizzup.org>
Signed-off-by: Bin Liu <b-...@ti.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/usb/musb/musb_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index d4c9e9544ba0..579aa9accafc 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2485,10 +2485,11 @@ static int musb_remove(struct platform_device *pdev)
musb_generic_disable(musb);
spin_unlock_irqrestore(>lock, flags);
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
+   musb_platform_exit(musb);
+
pm_runtime_dont_use_autosuspend(musb->controller);
pm_runtime_put_sync(musb->controller);
pm_runtime_disable(musb->controller);
-   musb_platform_exit(musb);
musb_phy_callback = NULL;
if (musb->dma_controller)
musb_dma_controller_destroy(musb->dma_controller);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] musb fixes backport to v4.9+

[Bin Liu]

Hi,

Here are several patches backported to v4.9+ to fix runtime PM problems in musb
drivers.

Regards,
-Bin.


Andreas Kemnade (1):
  usb: musb: fix enumeration after resume

Merlijn Wajer (2):
  usb: musb: call pm_runtime_{get,put}_sync before reading vbus registers
  usb: musb: Fix external abort in musb_remove on omap2430

 drivers/usb/musb/musb_core.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] usb: musb: call pm_runtime_{get,put}_sync before reading vbus registers

[Bin Liu]

From: Merlijn Wajer <merl...@wizzup.org>

commit df6b074dbe248d8c43a82131e8fd429e401841a5 upstream.

Without pm_runtime_{get,put}_sync calls in place, reading
vbus status via /sys causes the following error:

Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa0ab060
pgd = b333e822
[fa0ab060] *pgd=48011452(bad)

[] (musb_default_readb) from [] (musb_vbus_show+0x58/0xe4)
[] (musb_vbus_show) from [] (dev_attr_show+0x20/0x44)
[] (dev_attr_show) from [] (sysfs_kf_seq_show+0x80/0xdc)
[] (sysfs_kf_seq_show) from [] (seq_read+0x250/0x448)
[] (seq_read) from [] (__vfs_read+0x1c/0x118)
[] (__vfs_read) from [] (vfs_read+0x90/0x144)
[] (vfs_read) from [] (SyS_read+0x3c/0x74)
[] (SyS_read) from [] (ret_fast_syscall+0x0/0x54)

Solution was suggested by Tony Lindgren <t...@atomide.com>.

Cc: sta...@vger.kernel.org # v4.9+
Signed-off-by: Merlijn Wajer <merl...@wizzup.org>
Acked-by: Tony Lindgren <t...@atomide.com>
Signed-off-by: Bin Liu <b-...@ti.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/usb/musb/musb_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 0736cc27b5e7..d4c9e9544ba0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1774,6 +1774,7 @@ int musb_mailbox(enum musb_vbus_id_status status)
int vbus;
u8  devctl;
 
+   pm_runtime_get_sync(dev);
spin_lock_irqsave(>lock, flags);
val = musb->a_wait_bcon;
vbus = musb_platform_get_vbus_status(musb);
@@ -1787,6 +1788,7 @@ int musb_mailbox(enum musb_vbus_id_status status)
vbus = 0;
}
spin_unlock_irqrestore(>lock, flags);
+   pm_runtime_put_sync(dev);
 
return sprintf(buf, "Vbus %s, timeout %lu msec\n",
vbus ? "on" : "off", val);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: howto debug xhci driver?

[Bin Liu]

On Mon, Apr 23, 2018 at 07:21:12PM +0530, Tushar Nimkar wrote:
> Hi,
> 
> On 2018-04-23 18:58, Bin Liu wrote:
> >On Mon, Apr 23, 2018 at 11:14:55AM +0530, Tushar Nimkar wrote:
> >>On 2018-04-21 00:03, Bin Liu wrote:
> >>>Hi,
> >>>
> >>>On Tue, Mar 20, 2018 at 02:28:00PM -0700, Paul Zimmerman wrote:
> >>>>Forgot to CC linux-usb.
> >>>>
> >>>>
> >>>> Forwarded Message 
> >>>>Subject: Re: howto debug xhci driver?
> >>>>Date: Tue, 20 Mar 2018 13:56:21 -0700
> >>>>From: Paul Zimmerman <pauld...@gmail.com>
> >>>>To: Bin Liu <b-...@ti.com>
> >>>>CC: Felipe Balbi <felipe.ba...@linux.intel.com>
> >>>>
> >>>>Hi,
> >>>>
> >>>>Bin Liu <b-...@ti.com> writes:
> >>>>>>Bin Liu <b-...@ti.com> writes:
> >>>>>>>>>>>>BTY, the issue I am trying to debug is when reading
> >>>>>>>>>>>>bulk IN data from a USB2.0 device, if the device
> >>>>>>>>>>>>doesn't have data to transmit and NAKs the IN packet,
> >>>>>>>>>>>>after 4 pairs of IN-NAK transactions, xhci stops
> >>>>>>>>>>>>sending further IN tokens until the next SOF, which
> >>>>>>>>>>>>leaves ~90us gape on the bus.
> >>>>>>>>>>>>
> >>>>>>>>>>>>But when reading data from a USB2.0 thumb drive, this
> >>>>>>>>>>>>issue doesn't happen, even if the device NAKs the IN
> >>>>>>>>>>>>tokens, xhci still keeps sending IN tokens, which is
> >>>>>>>>>>>>way more than 4 pairs of IN-NAK transactions.
> >>>>>>>>>>>
> >>>>>>>>>>>Thumb drive has Bulk endpoints, what is the other
> >>>>>>>>>>>device's transfer type?
> >>>>>>>>>>
> >>>>>>>>>>It is bulk too. I asked for device descriptors. This is a
> >>>>>>>>>>remote debug effort for me, I don't have that device...
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>>Any one has a clue on what causes xhci to stop sending
> >>>>>>>>>>>>IN tokens after the device NAK'd 4 times?
> >>>>>>>>>
> >>>>>>>>>By accident, I reproduced the issue if addng a hub in the
> >>>>>>>>>middle... any comments about why a hub changes this xhci
> >>>>>>>>>behavior is appreciated.
> >>>>>>>>
> >>>>>>>>none off the top of my head. Maybe Mathias can suggest
> >>>>>>>>something.
> >>>>>>>
> >>>>>>>The issue seems to be not related to how many bulk IN-NAK pairs
> >>>>>>>before host stops sending IN token, but the host stops IN token
> >>>>>>>if 1) the device ever NAK'd an bulk IN token, and 2) there is
> >>>>>>>about 90~100us left to the next SOF. Then all the rest of
> >>>>>>>bandwidth is wasted.
> >>>>>>>
> >>>>>>>Is it about xhci bandwidth schduling? /me started reading...
> >>>>>>
> >>>>>>is this AM4 or AM5? Perhaps go after Synopsys' known errata list?
> >>>>>
> >>>>>I see the issue on both AM4 & AM5. I don't have access to the
> >>>>>errata list, I guess I should talk to TI internal for the list?
> >>>>
> >>>>I have a hazy recollection of something like this being a "feature" of
> >>>>the Synopsys core, to cut down on the excessive number of IN-NAK
> >>>>transactions you can sometimes get on the USB bus. So yep, you
> >>>>should talk to your Synopsys guy about this.
> >>>
> >>>Thanks for the information. We have been talking to Synopsis for this
> >>>issue, the progress is slow, one of the reasons is that the DWC3
> >>>version
> >>>is very old :(
> >>Bin, What is the version no? I have seen similar thing but on USB3.0
> >
> >On multiple versions: from 2.02a to 2.60a.
> suggest you to check errata list if not.

Yeah, stuck at here right now - too slow to get a copy of the errata :(

> >
> >>with dwc3 3.00a
> >
> >Do you mean you only see the problem in super-speed but not high-speed
> >with 3.00a?
> yes so far..
> >
> >>May I know how you confirmed/debug missing IN-ACK?
> >
> >Using bus protocol analyzer to capture bus traces. However my
> >issue is not
> >about missing IN-ACK, but IN-NAK - the host stops sending IN tokens too
> >early if the device NAK'd previous IN packets.
> >
> >Please confirm you mean IN-NAK instead in your case?
> it is IN-ACK, ss device send ERDY there should be IN-ACK saying NumP
> > 0 from host :)

Sounds like we face two different issues then :)

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] USB: musb: host: prevent core phy initialisation

[Bin Liu]

From: Johan Hovold <jo...@kernel.org>

Set the new HCD flag which prevents USB core from trying to manage our
phys.

This is needed to be able to associate the controller platform device
with the glue device device-tree node on the BBB which uses legacy USB
phys. Otherwise, the generic phy lookup in usb_phy_roothub_init() and
thus HCD registration fails repeatedly with -EPROBE_DEFER (see commit
178a0bce05cb ("usb: core: hcd: integrate the PHY wrapper into the HCD
core")).

Note that a related phy-lookup issue was recently worked around in the
phy core by commit b7563e2796f8 ("phy: work around 'phys' references to
usb-nop-xceiv devices"). Something similar may now be needed for other
USB phys, and in particular if we eventually want to let USB core manage
musb generic phys.

Cc: Arnd Bergmann <a...@arndb.de>
Cc: Martin Blumenstingl <martin.blumensti...@googlemail.com>
Signed-off-by: Johan Hovold <jo...@kernel.org>
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 3a8451a15f7f..4fa372c845e1 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2754,6 +2754,7 @@ int musb_host_setup(struct musb *musb, int power_budget)
hcd->self.otg_port = 1;
musb->xceiv->otg->host = >self;
hcd->power_budget = 2 * (power_budget ? : 250);
+   hcd->skip_phy_initialization = 1;
 
ret = usb_add_hcd(hcd, 0, 0);
if (ret < 0)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] USB: musb: dsps: drop duplicate phy initialisation

[Bin Liu]

From: Johan Hovold <jo...@kernel.org>

Since commit 39cee200c23e ("usb: musb: core: call init and shutdown for
the usb phy") the musb USB phy is initialised by musb_core, but the
original initialisation in the dsps-glue init callback was left in
place resulting in two calls to phy init during probe (and similarly,
two shutdowns on remove).

Drop the duplicate phy init and shutdown calls from the dsps glue in
favour of the ones in musb core, which other glue drivers rely on.

Note however that any generic phy is still initialised in the glue init
callback (just as for the other drivers).

Cc: Uwe Kleine-König <u.kleine-koe...@pengutronix.de>
Signed-off-by: Johan Hovold <jo...@kernel.org>
Acked-by: Uwe Kleine-König <u.kleine-koe...@pengutronix.de>
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 05a679d5e3a2..6a60bc0490c5 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -451,7 +451,6 @@ static int dsps_musb_init(struct musb *musb)
if (!rev)
return -ENODEV;
 
-   usb_phy_init(musb->xceiv);
if (IS_ERR(musb->phy))  {
musb->phy = NULL;
} else {
@@ -501,7 +500,6 @@ static int dsps_musb_exit(struct musb *musb)
struct dsps_glue *glue = dev_get_drvdata(dev->parent);
 
del_timer_sync(>dev_timer);
-   usb_phy_shutdown(musb->xceiv);
phy_power_off(musb->phy);
phy_exit(musb->phy);
debugfs_remove_recursive(glue->dbgfs_root);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] musb fixes for v4.17-rc3

[Bin Liu]

Hi Greg,

Here are musb fixes for v4.17-rc3, which fix two bugs in phy handling. Please
let me know if any change is needed.

Regards,
-Bin.


Johan Hovold (2):
  USB: musb: dsps: drop duplicate phy initialisation
  USB: musb: host: prevent core phy initialisation

 drivers/usb/musb/musb_dsps.c | 2 --
 drivers/usb/musb/musb_host.c | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: howto debug xhci driver?

[Bin Liu]

On Mon, Apr 23, 2018 at 11:14:55AM +0530, Tushar Nimkar wrote:
> On 2018-04-21 00:03, Bin Liu wrote:
> >Hi,
> >
> >On Tue, Mar 20, 2018 at 02:28:00PM -0700, Paul Zimmerman wrote:
> >>Forgot to CC linux-usb.
> >>
> >>
> >> Forwarded Message 
> >>Subject: Re: howto debug xhci driver?
> >>Date: Tue, 20 Mar 2018 13:56:21 -0700
> >>From: Paul Zimmerman <pauld...@gmail.com>
> >>To: Bin Liu <b-...@ti.com>
> >>CC: Felipe Balbi <felipe.ba...@linux.intel.com>
> >>
> >>Hi,
> >>
> >>Bin Liu <b-...@ti.com> writes:
> >>>>Bin Liu <b-...@ti.com> writes:
> >>>>>>>>>>BTY, the issue I am trying to debug is when reading
> >>>>>>>>>>bulk IN data from a USB2.0 device, if the device
> >>>>>>>>>>doesn't have data to transmit and NAKs the IN packet,
> >>>>>>>>>>after 4 pairs of IN-NAK transactions, xhci stops
> >>>>>>>>>>sending further IN tokens until the next SOF, which
> >>>>>>>>>>leaves ~90us gape on the bus.
> >>>>>>>>>>
> >>>>>>>>>>But when reading data from a USB2.0 thumb drive, this
> >>>>>>>>>>issue doesn't happen, even if the device NAKs the IN
> >>>>>>>>>>tokens, xhci still keeps sending IN tokens, which is
> >>>>>>>>>>way more than 4 pairs of IN-NAK transactions.
> >>>>>>>>>
> >>>>>>>>>Thumb drive has Bulk endpoints, what is the other
> >>>>>>>>>device's transfer type?
> >>>>>>>>
> >>>>>>>>It is bulk too. I asked for device descriptors. This is a
> >>>>>>>>remote debug effort for me, I don't have that device...
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>Any one has a clue on what causes xhci to stop sending
> >>>>>>>>>>IN tokens after the device NAK'd 4 times?
> >>>>>>>
> >>>>>>>By accident, I reproduced the issue if addng a hub in the
> >>>>>>>middle... any comments about why a hub changes this xhci
> >>>>>>>behavior is appreciated.
> >>>>>>
> >>>>>>none off the top of my head. Maybe Mathias can suggest
> >>>>>>something.
> >>>>>
> >>>>>The issue seems to be not related to how many bulk IN-NAK pairs
> >>>>>before host stops sending IN token, but the host stops IN token
> >>>>>if 1) the device ever NAK'd an bulk IN token, and 2) there is
> >>>>>about 90~100us left to the next SOF. Then all the rest of
> >>>>>bandwidth is wasted.
> >>>>>
> >>>>>Is it about xhci bandwidth schduling? /me started reading...
> >>>>
> >>>>is this AM4 or AM5? Perhaps go after Synopsys' known errata list?
> >>>
> >>>I see the issue on both AM4 & AM5. I don't have access to the
> >>>errata list, I guess I should talk to TI internal for the list?
> >>
> >>I have a hazy recollection of something like this being a "feature" of
> >>the Synopsys core, to cut down on the excessive number of IN-NAK
> >>transactions you can sometimes get on the USB bus. So yep, you
> >>should talk to your Synopsys guy about this.
> >
> >Thanks for the information. We have been talking to Synopsis for this
> >issue, the progress is slow, one of the reasons is that the DWC3
> >version
> >is very old :(
> Bin, What is the version no? I have seen similar thing but on USB3.0

On multiple versions: from 2.02a to 2.60a.

> with dwc3 3.00a

Do you mean you only see the problem in super-speed but not high-speed
with 3.00a?

> May I know how you confirmed/debug missing IN-ACK?

Using bus protocol analyzer to capture bus traces. However my issue is not
about missing IN-ACK, but IN-NAK - the host stops sending IN tokens too
early if the device NAK'd previous IN packets.

Please confirm you mean IN-NAK instead in your case?

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Support gadget mode when the port is set to dual role

[Bin Liu]

On Sat, Apr 21, 2018 at 12:59:23PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> Le vendredi 20 avril 2018 à 09:25 -0500, Bin Liu a écrit :
> > On Thu, Mar 29, 2018 at 01:57:24PM +0200, Paul Kocialkowski wrote:
> > > Hi,
> > > 
> > > On Thu, 2018-03-29 at 11:23 +0200, Maxime Ripard wrote:
> > > > On Wed, Mar 28, 2018 at 11:52:13PM +0200, Paul Kocialkowski wrote:
> > > > > This allows dual-role ports to be reported as having gadget mode
> > > > > by
> > > > > the
> > > > > musb_has_gadget helper. This is required to enable MUSB at all
> > > > > with
> > > > > MUSB
> > > > > glue layers that set the port mode to MUSB_PORT_MODE_DUAL_ROLE
> > > > > at
> > > > > init.
> > > > > 
> > > > > Most notably, this allows calling musb_start when needed in the
> > > > > virtual
> > > > > MUSB root HUB, regardless of whether the current mode should be
> > > > > gadget
> > > > > or host.
> > > > > 
> > > > > This fixes USB OTG on Allwinner devices that I could test it
> > > > > with,
> > > > > mainly A20 devices.
> > > > > 
> > > > > Signed-off-by: Paul Kocialkowski <cont...@paulk.fr>
> > > > 
> > > > Surely there's more to it than that. The gadget mode of A20 boards
> > > > have been working in the past, including when compiling with mUSB
> > > > setup as dual role.
> > > > 
> > > > Is this a regression since a particular commit? Or is there
> > > > another,
> > > > deeper issue overlooked in the commit log?
> > > 
> > > The root of the issue here is that musb_start is not called at any
> > > point
> > > without this patch. My understanding of the flow is the following:
> > > when
> > > the PHY detects that there was a VBUS/ID change, it will notify its
> > > listeners (mainly the musb sunxi glue layer). This will then
> > > schedule
> > > the driver's work (sunxi_musb_work), which does nothing since the
> > > SUNXI_MUSB_FL_ENABLED bit was never set. This bit is only set after
> > > calling sunxi_musb_enable, which is called from
> > > musb_platform_enable,
> > > that originates from musb_start.
> > > 
> > > Currently I see two places where musb_start is called:
> > > * musb_virthub
> > > * musb_gadget
> > > 
> > > In the latter case, it is in turn called from udc_start, which
> > > should
> > > probably (correct me if I'm wrong) happen later in the call chain
> > > than
> > > ID/VBUS change notification time.
> > 
> > I don't think it is correct that udc_start() is triggered by ID/VBUS
> > events, but I don't have an Allwinner platform to verify the callflow.
> 
> Yes you're right, I didn't make myself very clear here. I didn't
> investigate the udc_start call path much since it was apparently not the
> culprit.
> 
> > Have you tried to load with a gadget driver? When a gadget function is
> > bound to UDC, udc_start() is triggered, which in turn calls
> > musb_start().
> 
> It does work under that scenario, although my used case here is using
> musb with DUAL_ROLE but no gadget driver loaded. That it, I want the
> musb_start call to originate from the virtual hub, not from the gadget
> side.
> 
> > > In the former case, musb_start is called in the root controller hub
> > > control, when setting the USB_PORT_FEAT_POWER feature. This looks
> > > perfectly legit and IMO this is where it should be initially calling
> > > musb_start in the dual role case. The kernel is indeed setting the
> > 
> > No actually. A dual-role port should be in b_idle state by default, so
> > logically all actions should go to the gadget path until the port
> > switches to host mode.
> 
> It makes sense that the port should be in b_idle state by default, but
> here it fails to switch to host mode when the ID pin detects that it
> should. Or does b_idle state entail that a gadget must be loaded (per
> the USB spec), and thus nothing should (ever) happen until that happens?
> 
> I find it really odd to need a gadget device to trigger host mode.
> This patch does fix the issue, but I am puzzled as to why it is needed
> in the first place. The comment above it mentions that "In OTG mode we
> have to wait until we loaded a gadget. We don't really need a gadget if
> we operate as a host but we should not start a session 

[PATCH] usb: musb: trace: fix NULL pointer dereference in musb_g_tx()

[Bin Liu]

The usb_request pointer could be NULL in musb_g_tx(), where the
tracepoint call would trigger the NULL pointer dereference failure when
parsing the members of the usb_request pointer.

Move the tracepoint call to where the usb_request pointer is already
checked to solve the issue.

Fixes: fc78003e5345a("usb: musb: gadget: add usb-request tracepoints")

Cc: sta...@vger.kernel.org # v4.8+
Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index e564695c6c8d..71c5835ea9cd 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -417,7 +417,6 @@ void musb_g_tx(struct musb *musb, u8 epnum)
req = next_request(musb_ep);
request = >request;
 
-   trace_musb_req_tx(req);
csr = musb_readw(epio, MUSB_TXCSR);
musb_dbg(musb, "<== %s, txcsr %04x", musb_ep->end_point.name, csr);
 
@@ -456,6 +455,8 @@ void musb_g_tx(struct musb *musb, u8 epnum)
u8  is_dma = 0;
boolshort_packet = false;
 
+   trace_musb_req_tx(req);
+
if (dma && (csr & MUSB_TXCSR_DMAENAB)) {
is_dma = 1;
csr |= MUSB_TXCSR_P_WZC_BITS;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: howto debug xhci driver?

[Bin Liu]

Hi,

On Tue, Mar 20, 2018 at 02:28:00PM -0700, Paul Zimmerman wrote:
> Forgot to CC linux-usb.
> 
> 
>  Forwarded Message 
> Subject: Re: howto debug xhci driver?
> Date: Tue, 20 Mar 2018 13:56:21 -0700
> From: Paul Zimmerman <pauld...@gmail.com>
> To: Bin Liu <b-...@ti.com>
> CC: Felipe Balbi <felipe.ba...@linux.intel.com>
> 
> Hi,
> 
> Bin Liu <b-...@ti.com> writes:
> >>Bin Liu <b-...@ti.com> writes:
> >>>>>>>>BTY, the issue I am trying to debug is when reading
> >>>>>>>>bulk IN data from a USB2.0 device, if the device
> >>>>>>>>doesn't have data to transmit and NAKs the IN packet,
> >>>>>>>>after 4 pairs of IN-NAK transactions, xhci stops
> >>>>>>>>sending further IN tokens until the next SOF, which
> >>>>>>>>leaves ~90us gape on the bus.
> >>>>>>>>
> >>>>>>>>But when reading data from a USB2.0 thumb drive, this
> >>>>>>>>issue doesn't happen, even if the device NAKs the IN
> >>>>>>>>tokens, xhci still keeps sending IN tokens, which is
> >>>>>>>>way more than 4 pairs of IN-NAK transactions.
> >>>>>>>
> >>>>>>>Thumb drive has Bulk endpoints, what is the other
> >>>>>>>device's transfer type?
> >>>>>>
> >>>>>>It is bulk too. I asked for device descriptors. This is a
> >>>>>>remote debug effort for me, I don't have that device...
> >>>>>>
> >>>>>>>
> >>>>>>>>Any one has a clue on what causes xhci to stop sending
> >>>>>>>>IN tokens after the device NAK'd 4 times?
> >>>>>
> >>>>>By accident, I reproduced the issue if addng a hub in the
> >>>>>middle... any comments about why a hub changes this xhci
> >>>>>behavior is appreciated.
> >>>>
> >>>>none off the top of my head. Maybe Mathias can suggest
> >>>>something.
> >>>
> >>>The issue seems to be not related to how many bulk IN-NAK pairs
> >>>before host stops sending IN token, but the host stops IN token
> >>>if 1) the device ever NAK'd an bulk IN token, and 2) there is
> >>>about 90~100us left to the next SOF. Then all the rest of
> >>>bandwidth is wasted.
> >>>
> >>>Is it about xhci bandwidth schduling? /me started reading...
> >>
> >>is this AM4 or AM5? Perhaps go after Synopsys' known errata list?
> >
> >I see the issue on both AM4 & AM5. I don't have access to the
> >errata list, I guess I should talk to TI internal for the list?
> 
> I have a hazy recollection of something like this being a "feature" of
> the Synopsys core, to cut down on the excessive number of IN-NAK
> transactions you can sometimes get on the USB bus. So yep, you
> should talk to your Synopsys guy about this.

Thanks for the information. We have been talking to Synopsis for this
issue, the progress is slow, one of the reasons is that the DWC3 version
is very old :(

However, I just realized that in this case even though DWC3 in host mode
doesn't fully utilize the bus bandwidth, it doesn't violate any of the
USB Specs, as the Specs don't define flow control for bulk IN transfer.
The USB device shouldn't use bulk protocol at the first place if it has
bus bandwidth requirements. Isoch probably is a better choice. I will
check if there is anything can be done on the USB device side to solve
the problem.

Thanks all for the help.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Support gadget mode when the port is set to dual role

[Bin Liu]

On Thu, Mar 29, 2018 at 01:57:24PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Thu, 2018-03-29 at 11:23 +0200, Maxime Ripard wrote:
> > On Wed, Mar 28, 2018 at 11:52:13PM +0200, Paul Kocialkowski wrote:
> > > This allows dual-role ports to be reported as having gadget mode by
> > > the
> > > musb_has_gadget helper. This is required to enable MUSB at all with
> > > MUSB
> > > glue layers that set the port mode to MUSB_PORT_MODE_DUAL_ROLE at
> > > init.
> > > 
> > > Most notably, this allows calling musb_start when needed in the
> > > virtual
> > > MUSB root HUB, regardless of whether the current mode should be
> > > gadget
> > > or host.
> > > 
> > > This fixes USB OTG on Allwinner devices that I could test it with,
> > > mainly A20 devices.
> > > 
> > > Signed-off-by: Paul Kocialkowski 
> > 
> > Surely there's more to it than that. The gadget mode of A20 boards
> > have been working in the past, including when compiling with mUSB
> > setup as dual role.
> >
> > Is this a regression since a particular commit? Or is there another,
> > deeper issue overlooked in the commit log?
> 
> The root of the issue here is that musb_start is not called at any point
> without this patch. My understanding of the flow is the following: when
> the PHY detects that there was a VBUS/ID change, it will notify its
> listeners (mainly the musb sunxi glue layer). This will then schedule
> the driver's work (sunxi_musb_work), which does nothing since the
> SUNXI_MUSB_FL_ENABLED bit was never set. This bit is only set after
> calling sunxi_musb_enable, which is called from musb_platform_enable,
> that originates from musb_start.
> 
> Currently I see two places where musb_start is called:
> * musb_virthub
> * musb_gadget
> 
> In the latter case, it is in turn called from udc_start, which should
> probably (correct me if I'm wrong) happen later in the call chain than
> ID/VBUS change notification time.

I don't think it is correct that udc_start() is triggered by ID/VBUS
events, but I don't have an Allwinner platform to verify the callflow.

Have you tried to load with a gadget driver? When a gadget function is
bound to UDC, udc_start() is triggered, which in turn calls
musb_start().

> 
> In the former case, musb_start is called in the root controller hub
> control, when setting the USB_PORT_FEAT_POWER feature. This looks
> perfectly legit and IMO this is where it should be initially calling
> musb_start in the dual role case. The kernel is indeed setting the

No actually. A dual-role port should be in b_idle state by default, so
logically all actions should go to the gadget path until the port
switches to host mode.

> feature, only that it fails to enable musb without this patch.
> 
> First, I'd like to make sure that this understanding of the flow is
> correct as I may have missed something here. Does it make sense?

I am guessing you didn't load a gadget driver when testing?

> Then, it seems that the offending commit is: be9d39881fc4f
> ("usb: musb: host: rely on port_mode to call musb_start()")
> 
> That itself fixed: ae44df2e21b5
> ("usb: musb: call musb_start() only once in OTG mode")
> 
> Still, this commit was authored in June 2015, so almost 3 years ago.
> In the meantime, the sunxi driver has received feature improvements, so
> it seems hard to believe that it was broken all this time...
> 
> Cheers,

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring

[Bin Liu]

On Fri, Apr 20, 2018 at 01:57:23PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Bin Liu <b-...@ti.com> writes:
> >> Felipe Balbi <felipe.ba...@linux.intel.com> writes:
> >> >>> Bin Liu <b-...@ti.com> writes:
> >> >>> > On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote:
> >> >>> >> Hi guys,
> >> >>> >> 
> >> >>> >> I've been working on this series for a while now. I feels like after
> >> >>> >> this series the transfer management code is far easier to read and
> >> >>> >> understand.
> >> >>> >> 
> >> >>> >> Based on my tests, I have no regressions. Tested g_mass_storage and 
> >> >>> >> all
> >> >>> >> of testusb's tests (including ISOC).
> >> >>> >> 
> >> >>> >> Patches are also available on dwc3-improve-isoc-endpoints in my 
> >> >>> >> k.org
> >> >>> >> tree. Test reports would be VERY, VERY, VERY welcome. Please give 
> >> >>> >> this a
> >> >>> >> go so we avoid regressions on v4.18.
> >> >>> >
> >> >>> > Have you tested g_webcam? I just tested your 
> >> >>> > dwc3-improve-isoc-endpoints
> >> >>> 
> >> >>> for isoc I only tested g_zero.
> >> >>
> >> >> Then writting down details on what I observed probably won't help you :(
> >> 
> >> I got webcam running here, it sure _is_ helpful to let me know how you
> >
> > Great!
> >
> >> trigger the problem ;-) Also, high-speed or super-speed?
> >
> > Both. Long story short - super-speed doesn't stream the yuv video,
> > gadget side kernel prints "VS request completed with status -18."
> > flooding messages.
> >
> > high-speed does stream the video, but stopping the host application
> > (both yavta and luvcview) causes gadget side kernel prints error message
> > (I didn't keep the log). Then restart the host application doesn't
> > stream the video any more.
> >
> > To run the test:
> > gadget side:
> > # modprobe g_webcam (streaming_maxpacket=3072 for super-speed)
> > # uvc-gadget
> > host side:
> > $ yavta -c -f YUYV -t 1/30 -s 640x360 -n4 /dev/video1, or
> > $ luvcview -d /dev/video1 -f yuv
> 
> okay, found the problem. This is actually a problem on webcam gadget
> which kills the stream in case of Missed Isoc. The reason why it "works"
> with dwc3 today is because dwc3, up until now, is really harsh whenever
> we miss and interval.
> 
> Currently we stop the transfer and wait for the next XferNotReady. This
> may cause us to loose many frames.
> 
> I've been talking with Laurent Pinchart (in Cc) about what would be the
> best way to sort this out and, our conclusion, is that webcam gadget
> should have a way for a single buffer to be treated as erroneous, but it
> shouldn't stop the stream.
> 
> There's also another error in webcam gadget where default interval,
> maxburst and maxpacket aren't very good for HS or SS.
> 
> Note that streaming_interval is, by default, 1. Which for FS means 1ms,
> but for HS/SS it means 1 * 125us. So host side is actually polling
> webcam gadget every uFrame. I got webcam gadget to behave really well
> with streaming_interval=4 streaming_maxburst=16
> streaming_maxpacket=3072. With that, in SS, I can get around 100
> fps. There are a few cases when FPS drops to 33, but that's because it
> coincides with a missed isoc and webcam stops and restarts the stream.

Great!
Drop me an email whenever it is ready to re-test, in case I missed any
related conversation/patch in the mailing list.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring

[Bin Liu]

On Thu, Apr 19, 2018 at 02:18:21PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Balbi <felipe.ba...@linux.intel.com> writes:
> >>> Bin Liu <b-...@ti.com> writes:
> >>> > On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote:
> >>> >> Hi guys,
> >>> >> 
> >>> >> I've been working on this series for a while now. I feels like after
> >>> >> this series the transfer management code is far easier to read and
> >>> >> understand.
> >>> >> 
> >>> >> Based on my tests, I have no regressions. Tested g_mass_storage and all
> >>> >> of testusb's tests (including ISOC).
> >>> >> 
> >>> >> Patches are also available on dwc3-improve-isoc-endpoints in my k.org
> >>> >> tree. Test reports would be VERY, VERY, VERY welcome. Please give this 
> >>> >> a
> >>> >> go so we avoid regressions on v4.18.
> >>> >
> >>> > Have you tested g_webcam? I just tested your dwc3-improve-isoc-endpoints
> >>> 
> >>> for isoc I only tested g_zero.
> >>
> >> Then writting down details on what I observed probably won't help you :(
> 
> I got webcam running here, it sure _is_ helpful to let me know how you

Great!

> trigger the problem ;-) Also, high-speed or super-speed?

Both. Long story short - super-speed doesn't stream the yuv video,
gadget side kernel prints "VS request completed with status -18."
flooding messages.

high-speed does stream the video, but stopping the host application
(both yavta and luvcview) causes gadget side kernel prints error message
(I didn't keep the log). Then restart the host application doesn't
stream the video any more.

To run the test:
gadget side:
# modprobe g_webcam (streaming_maxpacket=3072 for super-speed)
# uvc-gadget
host side:
$ yavta -c -f YUYV -t 1/30 -s 640x360 -n4 /dev/video1, or
$ luvcview -d /dev/video1 -f yuv

> 
> >>> > branch on TI VAYU evm, it has issues in both high-speed and super-speed
> >>> > connections. It works fine with v4.17-rc1.
> >>> >
> >>> > If it suppose to work with g-webcam, please let me know if you want
> >>> > the details of the problems I see - the log is very long in super-speed
> >>> > failure.
> >>> 
> >>> Sure, give me some details of what's going on. Some bisection would be
> >>> great too, then I'll know which patch to blame and where to focus fixes.
> >>
> >> Roger has a copy of your previous work here:
> >>
> >> g...@github.com:rogerq/linux.git, branch balbi-dwc3-improve-isoc-endpoints
> >>
> >> I just tested it and it works fine with g_webcam.
> >>
> >> Do you think this information will help you? If so it saves me time on
> >> bisect ;)
> >
> > What's the SHA1 on that? I can diff it locally and see if I can find
> > problems.
> 
> I got this one from Roger. There's nothing in there that should cause
> any changes to the behavior.
> 
> Please capture tracepoints of both working and failing cases and send to
> me. More details at [1]

I will try to capture the traces tomorrow.

> 
> [1] 
> https://www.kernel.org/doc/html/v4.16/driver-api/usb/dwc3.html#reporting-bugs

Regards,
-Bin.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] usb: musb: host: fix potential NULL pointer dereference

[Bin Liu]

musb_start_urb() doesn't check the 3rd pass-in parameter if it is NULL.
But in musb_bulk_nak_timeout() the parameter passed to musb_start_urb()
is returned from first_qh(), which could be NULL.

So wrap the musb_start_urb() call here with a if condition check to
avoid the potential NULL pointer dereference.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_host.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index e50438ae241e..218aadef5bbf 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -990,7 +990,9 @@ static void musb_bulk_nak_timeout(struct musb *musb, struct 
musb_hw_ep *ep,
/* set tx_reinit and schedule the next qh */
ep->tx_reinit = 1;
}
-   musb_start_urb(musb, is_in, next_qh);
+
+   if (next_qh)
+   musb_start_urb(musb, is_in, next_qh);
}
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] usb: musb: gadget: fix to_musb_request() to not return NULL

[Bin Liu]

The gadget function drivers should ensure the usb_request parameter
passed in is not NULL. UDC core doesn't check if it is NULL, so MUSB
driver shouldn't have to check it either.

Convert to_musb_request() to a simple macro to not directly return NULL
to avoid warnings from code static analysis tools.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 9c34aca06db6..466877b471ef 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -60,10 +60,7 @@ struct musb_request {
enum buffer_map_state map_state;
 };
 
-static inline struct musb_request *to_musb_request(struct usb_request *req)
-{
-   return req ? container_of(req, struct musb_request, request) : NULL;
-}
+#define to_musb_request(r) (container_of(r, struct musb_request, request))
 
 extern struct usb_request *
 musb_alloc_request(struct usb_ep *ep, gfp_t gfp_flags);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] usb: musb: gadget: fix to_musb_ep() to not return NULL

[Bin Liu]

UDC core ensures the usb_ep parameter passed in is not NULL, so
checking if (ep != NULL) is pointless.

Convert to_musb_ep() to a simple macro to not directly return NULL to
avoid warnings from code static analysis tools.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
index 466877b471ef..aaae24bb3c10 100644
--- a/drivers/usb/musb/musb_gadget.h
+++ b/drivers/usb/musb/musb_gadget.h
@@ -96,10 +96,7 @@ struct musb_ep {
u8  hb_mult;
 };
 
-static inline struct musb_ep *to_musb_ep(struct usb_ep *ep)
-{
-   return ep ? container_of(ep, struct musb_ep, end_point) : NULL;
-}
+#define to_musb_ep(ep) (container_of(ep, struct musb_ep, end_point))
 
 static inline struct musb_request *next_request(struct musb_ep *ep)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] USB: musb: dsps: phy fix and DT-topology support

[Bin Liu]

On Wed, Apr 18, 2018 at 08:46:40PM +0200, Johan Hovold wrote:
> On Wed, Apr 18, 2018 at 11:20:15AM -0500, Bin Liu wrote:
> > Johan,
> > 
> > On Fri, Apr 13, 2018 at 05:15:02PM +0200, Johan Hovold wrote:
> > > I've been carrying a patch out-of-tree since my work on improving the
> > > USB device-tree support which is needed to be able to describe USB
> > > topologies for musb based controllers.
> > > 
> > > This patch, which associates the platform controller device with the
> > > glue device device-tree node, did not play well with the recent changes
> > > which added generic phy support to USB core however.
> > > 
> > > Like the recent dwc2 regression fixed by Arnd after the device-tree
> > > #phy-cell changes, the generic phy code in USB core can now also fail
> > > indefinitly with -EPROBE_DEFER when the controller uses a legacy USB
> > > phy.
> > > 
> > > The second patch addresses this for musb, which handles its own (legacy
> > > and generic) phys, but something more may possibly now be needed for
> > > other platforms with legacy phys.
> > > 
> > > In the process of debugging this, I stumbled over another issue which
> > > caused the dsps legacy phy init two be called twice on every probe and
> > > which is fixed by the first patch.
> > > 
> > > Johan
> > > 
> > > 
> > > Johan Hovold (3):
> > >   USB: musb: dsps: drop duplicate phy initialisation
> > >   USB: musb: host: prevent core phy initialisation
> > 
> > Are the two bugs only affecting you with your out-of-tree patch? It
> > seems don't have any functional impact for me. I need to make a decision
> > if these two patches need to go to the stable trees...
> 
> The first bug is independent of the third patch (the "out-of-tree"
> patch), but as Uwe also noted it seems to be mostly benign since it took
> two years for it to be discovered. For that reason, and to minimise the
> risk of any stable regressions, I did not mark it for stable.
> 
> The second fix is really only needed with the third of_node patch since
> I don't think any of the glue drivers propagates the device-tree node in
> this way currently. Hence it could also wait for 3.18, and it is in any
> case not stable material as the generic-phy support in USB core is new
> in 3.17.

Great, thanks for confirming. I will not send them for stable trees.

> 
> Note that other host controllers may have a device-tree node with
> associated legacy-phys however and that this could now lead to similar
> problems starting with 3.17.

regards,
-Bin.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] USB: musb: dsps: phy fix and DT-topology support

[Bin Liu]

Johan,

On Fri, Apr 13, 2018 at 05:15:02PM +0200, Johan Hovold wrote:
> I've been carrying a patch out-of-tree since my work on improving the
> USB device-tree support which is needed to be able to describe USB
> topologies for musb based controllers.
> 
> This patch, which associates the platform controller device with the
> glue device device-tree node, did not play well with the recent changes
> which added generic phy support to USB core however.
> 
> Like the recent dwc2 regression fixed by Arnd after the device-tree
> #phy-cell changes, the generic phy code in USB core can now also fail
> indefinitly with -EPROBE_DEFER when the controller uses a legacy USB
> phy.
> 
> The second patch addresses this for musb, which handles its own (legacy
> and generic) phys, but something more may possibly now be needed for
> other platforms with legacy phys.
> 
> In the process of debugging this, I stumbled over another issue which
> caused the dsps legacy phy init two be called twice on every probe and
> which is fixed by the first patch.
> 
> Johan
> 
> 
> Johan Hovold (3):
>   USB: musb: dsps: drop duplicate phy initialisation
>   USB: musb: host: prevent core phy initialisation

Are the two bugs only affecting you with your out-of-tree patch? It
seems don't have any functional impact for me. I need to make a decision
if these two patches need to go to the stable trees...

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] usb: musb: break the huge isr musb_stage0_irq() into small functions

[Bin Liu]

On Wed, Apr 18, 2018 at 05:14:05PM +0300, Dan Carpenter wrote:
> On Wed, Apr 18, 2018 at 08:33:52AM -0500, Bin Liu wrote:
> > Hi Dan,
> > 
> > I appreciate you scaning  the patches and reporting the issues.

The report is valuable!

> > 
> 
> These are kbuild stuff so I basically just forward them.  It's no
> effort.
> 
> > On Wed, Apr 18, 2018 at 10:25:34AM +0300, Dan Carpenter wrote:
> > > Hi Bin,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on balbi-usb/next]
> > > [also build test WARNING on v4.17-rc1 next-20180417]
> > > [if your patch is applied to the wrong git tree, please drop us a note to 
> > > help improve the system]
> > > 
> > > url:
> > > https://github.com/0day-ci/linux/commits/Bin-Liu/usb-musb-cleanup/20180417-133633
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> > > 
> > > smatch warnings:
> > > drivers/usb/musb/musb_core.c:797 musb_handle_intr_connect() error: we 
> > > previously assumed 'musb->hcd' could be null (see line 783)
> > 
> > It appears the condition check was introduced back in 2013 in an attempt
> > to separate host-only and gadget-only configurations for musb [1], but it
> > seems the work is not complete, because the musb->hcd allocation is
> > unconditional. So in the current code, musb->hcd won't be NULL. 
> > 
> > But I am now hesitated to remove this 'if(musb->hcd)' check to solve
> > this smatch warning, because I am still debating on either continue the
> > work to separate the two configurations or clean up the uncompleted code
> > left by the attempt to not separate them.
> > 
> > So I want to leave the patch as is for now, is it reasonable?
> > 
> 
> Fine by me.  Right now it's sort of hard for Smatch to parse the code
> well enough for it to know that ->hcd is always non-NULL but I'm
> hopeful that maybe in a couple years that will be possible...

To me, the report is good enough :) In this case, it tells the driver
requires some work, which is valid.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring

[Bin Liu]

On Wed, Apr 18, 2018 at 08:20:28AM +0300, Felipe Balbi wrote:
> 
> Hi Bin,
> 
> Bin Liu <b-...@ti.com> writes:
> > On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote:
> >> Hi guys,
> >> 
> >> I've been working on this series for a while now. I feels like after
> >> this series the transfer management code is far easier to read and
> >> understand.
> >> 
> >> Based on my tests, I have no regressions. Tested g_mass_storage and all
> >> of testusb's tests (including ISOC).
> >> 
> >> Patches are also available on dwc3-improve-isoc-endpoints in my k.org
> >> tree. Test reports would be VERY, VERY, VERY welcome. Please give this a
> >> go so we avoid regressions on v4.18.
> >
> > Have you tested g_webcam? I just tested your dwc3-improve-isoc-endpoints
> 
> for isoc I only tested g_zero.

Then writting down details on what I observed probably won't help you :(

> > branch on TI VAYU evm, it has issues in both high-speed and super-speed
> > connections. It works fine with v4.17-rc1.
> >
> > If it suppose to work with g-webcam, please let me know if you want
> > the details of the problems I see - the log is very long in super-speed
> > failure.
> 
> Sure, give me some details of what's going on. Some bisection would be
> great too, then I'll know which patch to blame and where to focus fixes.

Roger has a copy of your previous work here:

g...@github.com:rogerq/linux.git, branch balbi-dwc3-improve-isoc-endpoints

I just tested it and it works fine with g_webcam.

Do you think this information will help you? If so it saves me time on
bisect ;)

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/11] usb: musb: break the huge isr musb_stage0_irq() into small functions

[Bin Liu]

Hi Dan,

I appreciate you scaning  the patches and reporting the issues.

On Wed, Apr 18, 2018 at 10:25:34AM +0300, Dan Carpenter wrote:
> Hi Bin,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on balbi-usb/next]
> [also build test WARNING on v4.17-rc1 next-20180417]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Bin-Liu/usb-musb-cleanup/20180417-133633
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> 
> smatch warnings:
> drivers/usb/musb/musb_core.c:797 musb_handle_intr_connect() error: we 
> previously assumed 'musb->hcd' could be null (see line 783)

It appears the condition check was introduced back in 2013 in an attempt
to separate host-only and gadget-only configurations for musb [1], but it
seems the work is not complete, because the musb->hcd allocation is
unconditional. So in the current code, musb->hcd won't be NULL. 

But I am now hesitated to remove this 'if(musb->hcd)' check to solve
this smatch warning, because I am still debating on either continue the
work to separate the two configurations or clean up the uncompleted code
left by the attempt to not separate them.

So I want to leave the patch as is for now, is it reasonable?

[1] commit 74c2e93600581("usb: musb: factor out hcd initalization")

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring

[Bin Liu]

Felipe,

On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote:
> Hi guys,
> 
> I've been working on this series for a while now. I feels like after
> this series the transfer management code is far easier to read and
> understand.
> 
> Based on my tests, I have no regressions. Tested g_mass_storage and all
> of testusb's tests (including ISOC).
> 
> Patches are also available on dwc3-improve-isoc-endpoints in my k.org
> tree. Test reports would be VERY, VERY, VERY welcome. Please give this a
> go so we avoid regressions on v4.18.

Have you tested g_webcam? I just tested your dwc3-improve-isoc-endpoints
branch on TI VAYU evm, it has issues in both high-speed and super-speed
connections. It works fine with v4.17-rc1.

If it suppose to work with g-webcam, please let me know if you want
the details of the problems I see - the log is very long in super-speed
failure.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] USB: musb: dsps: propagate device-tree node

[Bin Liu]

Johan,

On Fri, Apr 13, 2018 at 05:15:05PM +0200, Johan Hovold wrote:
> To be able to use DSPS-based controllers with device-tree descriptions
> of the USB topology, we need to associate the glue device's device-tree
> node with the child controller device.
> 
> Note that this can also be used to eventually let USB core manage
> generic phys.
> 
> Also note that the other glue drivers will require similar changes to be
> able to describe their buses in DT.
> 
> Signed-off-by: Johan Hovold 

I will take other two patches for v4.17 rc cycles, but is there any
problem if I take this patch for v4.18-rc1?

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/11] usb: musb: remove some register access wrapper functions

[Bin Liu]

The following wrappers were defined because of Blackfin support. Now
Blackfin support is removed, these wrappers are no longer needed, so
remove them.

musb_write_txfifosz
musb_write_txfifoadd
musb_write_rxfifosz
musb_write_rxfifoadd
musb_write_ulpi_buscontrol
musb_read_txfifosz
musb_read_txfifoadd
musb_read_rxfifosz
musb_read_rxfifoadd
musb_read_ulpi_buscontrol
musb_read_hwvers

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c | 42 -
 drivers/usb/musb/musb_regs.h | 55 
 2 files changed, 21 insertions(+), 76 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 4ff6da1aa775..d3f9f7e5f353 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1240,25 +1240,25 @@ void musb_stop(struct musb *musb)
/* REVISIT error check:  be sure ep0 can both rx and tx ... */
switch (cfg->style) {
case FIFO_TX:
-   musb_write_txfifosz(mbase, c_size);
-   musb_write_txfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_TXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_TXFIFOADD, c_off);
hw_ep->tx_double_buffered = !!(c_size & MUSB_FIFOSZ_DPB);
hw_ep->max_packet_sz_tx = maxpacket;
break;
case FIFO_RX:
-   musb_write_rxfifosz(mbase, c_size);
-   musb_write_rxfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_RXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_RXFIFOADD, c_off);
hw_ep->rx_double_buffered = !!(c_size & MUSB_FIFOSZ_DPB);
hw_ep->max_packet_sz_rx = maxpacket;
break;
case FIFO_RXTX:
-   musb_write_txfifosz(mbase, c_size);
-   musb_write_txfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_TXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_TXFIFOADD, c_off);
hw_ep->rx_double_buffered = !!(c_size & MUSB_FIFOSZ_DPB);
hw_ep->max_packet_sz_rx = maxpacket;
 
-   musb_write_rxfifosz(mbase, c_size);
-   musb_write_rxfifoadd(mbase, c_off);
+   musb_writeb(mbase, MUSB_RXFIFOSZ, c_size);
+   musb_writew(mbase, MUSB_RXFIFOADD, c_off);
hw_ep->tx_double_buffered = hw_ep->rx_double_buffered;
hw_ep->max_packet_sz_tx = maxpacket;
 
@@ -1466,7 +1466,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
}
 
/* log release info */
-   musb->hwvers = musb_read_hwvers(mbase);
+   musb->hwvers = musb_readw(mbase, MUSB_HWVERS);
pr_debug("%s: %sHDRC RTL version %d.%d%s\n",
 musb_driver_name, type, MUSB_HWVERS_MAJOR(musb->hwvers),
 MUSB_HWVERS_MINOR(musb->hwvers),
@@ -2311,9 +2311,9 @@ static void musb_deassert_reset(struct work_struct *work)
 
/* program PHY to use external vBus if required */
if (plat->extvbus) {
-   u8 busctl = musb_read_ulpi_buscontrol(musb->mregs);
+   u8 busctl = musb_readb(musb->mregs, MUSB_ULPI_BUSCONTROL);
busctl |= MUSB_ULPI_USE_EXTVBUS;
-   musb_write_ulpi_buscontrol(musb->mregs, busctl);
+   musb_writeb(musb->mregs, MUSB_ULPI_BUSCONTROL, busctl);
}
 
if (musb->xceiv->otg->default_a) {
@@ -2482,7 +2482,7 @@ static void musb_save_context(struct musb *musb)
 
musb->context.frame = musb_readw(musb_base, MUSB_FRAME);
musb->context.testmode = musb_readb(musb_base, MUSB_TESTMODE);
-   musb->context.busctl = musb_read_ulpi_buscontrol(musb->mregs);
+   musb->context.busctl = musb_readb(musb_base, MUSB_ULPI_BUSCONTROL);
musb->context.power = musb_readb(musb_base, MUSB_POWER);
musb->context.intrusbe = musb_readb(musb_base, MUSB_INTRUSBE);
musb->context.index = musb_readb(musb_base, MUSB_INDEX);
@@ -2511,13 +2511,13 @@ static void musb_save_context(struct musb *musb)
 
if (musb->dyn_fifo) {
musb->context.index_regs[i].txfifoadd =
-   musb_read_txfifoadd(musb_base);
+   musb_readw(musb_base, MUSB_TXFIFOADD);
musb->context.index_regs[i].rxfifoadd =
-   musb_read_rxfifoadd(musb_base);
+   musb_readw(musb_base, MUSB_RXFIFOADD);
musb->context.index_regs[i].txfifosz =
-   musb_read_txfifosz(musb_base);
+   musb_readb(musb_base, MUSB_TXFIFOSZ);
 

[PATCH 10/11] usb: musb: remove references to default_a of struct usb_otg

[Bin Liu]

musb drivers do not use the otg fsm framework, so referencing to
otg->default_a doesn't have any effect, so remove the references.

But tusb6010 glue driver uses it locally to control the vbus power, so
keep the references in tusb6010 only.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/am35x.c   |  3 ---
 drivers/usb/musb/da8xx.c   |  2 --
 drivers/usb/musb/davinci.c | 49 --
 drivers/usb/musb/musb_core.c   |  9 ++--
 drivers/usb/musb/musb_dsps.c   |  2 --
 drivers/usb/musb/musb_gadget.c |  1 -
 drivers/usb/musb/musb_host.c   |  1 -
 drivers/usb/musb/omap2430.c|  5 -
 drivers/usb/musb/sunxi.c   |  2 --
 drivers/usb/musb/ux500.c   |  2 --
 10 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 0ad664efda6b..660641ab1545 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -201,7 +201,6 @@ static irqreturn_t am35x_musb_interrupt(int irq, void *hci)
struct device *dev = musb->controller;
struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
struct omap_musb_board_data *data = plat->board_data;
-   struct usb_otg *otg = musb->xceiv->otg;
unsigned long flags;
irqreturn_t ret = IRQ_NONE;
u32 epintr, usbintr;
@@ -264,14 +263,12 @@ static irqreturn_t am35x_musb_interrupt(int irq, void 
*hci)
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
-   otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
del_timer(>dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
-   otg->default_a = 0;
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
}
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index b8295ce7c4fe..0e5929e81d26 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -280,7 +280,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
-   otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
del_timer(>dev_timer);
@@ -295,7 +294,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
 */
musb->is_active = 0;
MUSB_DEV_MODE(musb);
-   otg->default_a = 0;
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
}
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 2ad39dcd2f4c..fb6bbd254ab7 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -311,14 +311,12 @@ static irqreturn_t davinci_musb_interrupt(int irq, void 
*__hci)
WARNING("VBUS error workaround (delay coming)\n");
} else if (drvvbus) {
MUSB_HST_MODE(musb);
-   otg->default_a = 1;
musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
portstate(musb->port1_status |= USB_PORT_STAT_POWER);
del_timer(>dev_timer);
} else {
musb->is_active = 0;
MUSB_DEV_MODE(musb);
-   otg->default_a = 0;
musb->xceiv->otg->state = OTG_STATE_B_IDLE;
portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
}
@@ -425,6 +423,9 @@ static int davinci_musb_init(struct musb *musb)
 
 static int davinci_musb_exit(struct musb *musb)
 {
+   int maxdelay = 30;
+   u8  devctl, warn = 0;
+
del_timer_sync(>dev_timer);
 
/* force VBUS off */
@@ -438,31 +439,27 @@ static int davinci_musb_exit(struct musb *musb)
 
davinci_musb_source_power(musb, 0 /*off*/, 1);
 
-   /* delay, to avoid problems with module reload */
-   if (musb->xceiv->otg->default_a) {
-   int maxdelay = 30;
-   u8  devctl, warn = 0;
+   /*
+* delay, to a

[PATCH 09/11] usb: musb: break the huge isr musb_stage0_irq() into small functions

[Bin Liu]

musb_stage0_irq() is 400+ lines long. Break its interrupt events
handling into each individual functions to make it easy to read.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c | 730 +++
 1 file changed, 384 insertions(+), 346 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index ce54f48314e1..a3a716197dc1 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -523,6 +523,383 @@ void musb_hnp_stop(struct musb *musb)
 
 static void musb_recover_from_babble(struct musb *musb);
 
+static void musb_handle_intr_resume(struct musb *musb, u8 devctl)
+{
+   musb_dbg(musb, "RESUME (%s)",
+   usb_otg_state_string(musb->xceiv->otg->state));
+
+   if (devctl & MUSB_DEVCTL_HM) {
+   switch (musb->xceiv->otg->state) {
+   case OTG_STATE_A_SUSPEND:
+   /* remote wakeup? */
+   musb->port1_status |=
+   (USB_PORT_STAT_C_SUSPEND << 16)
+   | MUSB_PORT_STAT_RESUME;
+   musb->rh_timer = jiffies
+   + msecs_to_jiffies(USB_RESUME_TIMEOUT);
+   musb->xceiv->otg->state = OTG_STATE_A_HOST;
+   musb->is_active = 1;
+   musb_host_resume_root_hub(musb);
+   schedule_delayed_work(>finish_resume_work,
+   msecs_to_jiffies(USB_RESUME_TIMEOUT));
+   break;
+   case OTG_STATE_B_WAIT_ACON:
+   musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
+   musb->is_active = 1;
+   MUSB_DEV_MODE(musb);
+   break;
+   default:
+   WARNING("bogus %s RESUME (%s)\n",
+   "host",
+   usb_otg_state_string(musb->xceiv->otg->state));
+   }
+   } else {
+   switch (musb->xceiv->otg->state) {
+   case OTG_STATE_A_SUSPEND:
+   /* possibly DISCONNECT is upcoming */
+   musb->xceiv->otg->state = OTG_STATE_A_HOST;
+   musb_host_resume_root_hub(musb);
+   break;
+   case OTG_STATE_B_WAIT_ACON:
+   case OTG_STATE_B_PERIPHERAL:
+   /* disconnect while suspended?  we may
+* not get a disconnect irq...
+*/
+   if ((devctl & MUSB_DEVCTL_VBUS)
+   != (3 << MUSB_DEVCTL_VBUS_SHIFT)
+   ) {
+   musb->int_usb |= MUSB_INTR_DISCONNECT;
+   musb->int_usb &= ~MUSB_INTR_SUSPEND;
+   break;
+   }
+   musb_g_resume(musb);
+   break;
+   case OTG_STATE_B_IDLE:
+   musb->int_usb &= ~MUSB_INTR_SUSPEND;
+   break;
+   default:
+   WARNING("bogus %s RESUME (%s)\n",
+   "peripheral",
+   usb_otg_state_string(musb->xceiv->otg->state));
+   }
+   }
+}
+
+/* return IRQ_HANDLED to tell the caller to return immediately */
+static irqreturn_t musb_handle_intr_sessreq(struct musb *musb, u8 devctl)
+{
+   void __iomem *mbase = musb->mregs;
+
+   if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS
+   && (devctl & MUSB_DEVCTL_BDEVICE)) {
+   musb_dbg(musb, "SessReq while on B state");
+   return IRQ_HANDLED;
+   }
+
+   musb_dbg(musb, "SESSION_REQUEST (%s)",
+   usb_otg_state_string(musb->xceiv->otg->state));
+
+   /* IRQ arrives from ID pin sense or (later, if VBUS power
+* is removed) SRP.  responses are time critical:
+*  - turn on VBUS (with silicon-specific mechanism)
+*  - go through A_WAIT_VRISE
+*  - ... to A_WAIT_BCON.
+* a_wait_vrise_tmout triggers VBUS_ERROR transitions
+*/
+   musb_writeb(mbase, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
+   musb->ep0_stage = MUSB_EP0_START;
+   musb->xceiv->otg->state = OTG_STATE_A_IDLE;
+   MUSB_HST_MODE(musb);
+   musb_platform_set_vbus(musb, 1);
+
+   return IRQ_NONE;
+}
+
+static void musb_handle_intr_vbuserr(struct musb *musb, u8 devctl)
+{
+   int ignore = 0;
+
+   /* During connection as an A-Device, we may see a short
+ 

[PATCH 11/11] usb: musb: disable otg protocol support

[Bin Liu]

As decided in the discussion [1] we are deleting the otg protocol
support from the musb drivers.

First this patch disables the flags for enabling the otg protocols. We
will later gradually delete the otg protocol code from the musb drivers.

[1] https://www.spinics.net/lists/linux-usb/msg167003.html

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_gadget.c | 5 +
 drivers/usb/musb/musb_host.c   | 3 ++-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 63e9d4a4395f..b0d86d895420 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1797,11 +1797,8 @@ int musb_gadget_setup(struct musb *musb)
 
/* this "gadget" abstracts/virtualizes the controller */
musb->g.name = musb_driver_name;
-#if IS_ENABLED(CONFIG_USB_MUSB_DUAL_ROLE)
-   musb->g.is_otg = 1;
-#elif IS_ENABLED(CONFIG_USB_MUSB_GADGET)
+   /* don't support otg protocols */
musb->g.is_otg = 0;
-#endif
INIT_DELAYED_WORK(>gadget_work, musb_gadget_work);
musb_g_init_endpoints(musb);
 
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 34b97cae5ec1..e50438ae241e 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2750,7 +2750,8 @@ int musb_host_setup(struct musb *musb, int power_budget)
musb->xceiv->otg->state = OTG_STATE_A_IDLE;
}
otg_set_host(musb->xceiv->otg, >self);
-   hcd->self.otg_port = 1;
+   /* don't support otg protocols */
+   hcd->self.otg_port = 0;
musb->xceiv->otg->host = >self;
hcd->power_budget = 2 * (power_budget ? : 250);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/11] usb: musb: remove duplicated quirks flag

[Bin Liu]

Both musb_io and musb_platform_ops in struct musb define a quirks flag
for the same purpose.  Let's remove the one in struct musb_io, and use
that in struct musb_platform_ops instead.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c   | 10 --
 drivers/usb/musb/musb_cppi41.c |  4 ++--
 drivers/usb/musb/musb_dma.h| 10 +-
 drivers/usb/musb/musb_io.h |  2 --
 4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index d3f9f7e5f353..84f25a2b078d 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1493,7 +1493,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
 
hw_ep->fifo = musb->io.fifo_offset(i) + mbase;
 #if IS_ENABLED(CONFIG_USB_MUSB_TUSB6010)
-   if (musb->io.quirks & MUSB_IN_TUSB) {
+   if (musb->ops->quirks & MUSB_IN_TUSB) {
hw_ep->fifo_async = musb->async + 0x400 +
musb->io.fifo_offset(i);
hw_ep->fifo_sync = musb->sync + 0x400 +
@@ -2176,11 +2176,9 @@ static void musb_deassert_reset(struct work_struct *work)
goto fail2;
}
 
-   if (musb->ops->quirks)
-   musb->io.quirks = musb->ops->quirks;
 
/* Most devices use indexed offset or flat offset */
-   if (musb->io.quirks & MUSB_INDEXED_EP) {
+   if (musb->ops->quirks & MUSB_INDEXED_EP) {
musb->io.ep_offset = musb_indexed_ep_offset;
musb->io.ep_select = musb_indexed_ep_select;
} else {
@@ -2188,7 +2186,7 @@ static void musb_deassert_reset(struct work_struct *work)
musb->io.ep_select = musb_flat_ep_select;
}
 
-   if (musb->io.quirks & MUSB_G_NO_SKB_RESERVE)
+   if (musb->ops->quirks & MUSB_G_NO_SKB_RESERVE)
musb->g.quirk_avoids_skb_reserve = 1;
 
/* At least tusb6010 has its own offsets */
@@ -2647,7 +2645,7 @@ static int musb_suspend(struct device *dev)
;
musb->flush_irq_work = false;
 
-   if (!(musb->io.quirks & MUSB_PRESERVE_SESSION))
+   if (!(musb->ops->quirks & MUSB_PRESERVE_SESSION))
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 
WARN_ON(!list_empty(>pending_list));
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index d0dd4f470bbe..7fbb8a307145 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -614,7 +614,7 @@ static int cppi41_dma_channel_abort(struct dma_channel 
*channel)
}
 
/* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */
-   if (musb->io.quirks & MUSB_DA8XX)
+   if (musb->ops->quirks & MUSB_DA8XX)
mdelay(250);
 
tdbit = 1 << cppi41_channel->port_num;
@@ -773,7 +773,7 @@ struct dma_controller *
controller->controller.is_compatible = cppi41_is_compatible;
controller->controller.musb = musb;
 
-   if (musb->io.quirks & MUSB_DA8XX) {
+   if (musb->ops->quirks & MUSB_DA8XX) {
controller->tdown_reg = DA8XX_USB_TEARDOWN;
controller->autoreq_reg = DA8XX_USB_AUTOREQ;
controller->set_dma_mode = da8xx_set_dma_mode;
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
index 0fc8cd0c2a5c..8f60271c0a9d 100644
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -44,31 +44,31 @@
 #endif
 
 #ifdef CONFIG_USB_UX500_DMA
-#define musb_dma_ux500(musb)   (musb->io.quirks & MUSB_DMA_UX500)
+#define musb_dma_ux500(musb)   (musb->ops->quirks & MUSB_DMA_UX500)
 #else
 #define musb_dma_ux500(musb)   0
 #endif
 
 #ifdef CONFIG_USB_TI_CPPI41_DMA
-#define musb_dma_cppi41(musb)  (musb->io.quirks & MUSB_DMA_CPPI41)
+#define musb_dma_cppi41(musb)  (musb->ops->quirks & MUSB_DMA_CPPI41)
 #else
 #define musb_dma_cppi41(musb)  0
 #endif
 
 #ifdef CONFIG_USB_TI_CPPI_DMA
-#define musb_dma_cppi(musb)(musb->io.quirks & MUSB_DMA_CPPI)
+#define musb_dma_cppi(musb)(musb->ops->quirks & MUSB_DMA_CPPI)
 #else
 #define musb_dma_cppi(musb)0
 #endif
 
 #ifdef CONFIG_USB_TUSB_OMAP_DMA
-#define tusb_dma_omap(musb)(musb->io.quirks & MUSB_DMA_TUSB_OMAP)
+#define tusb_dma_omap(musb)(musb->ops->quirks & MUSB_DMA_TUSB_OMAP)
 #else
 #define tusb_dma_omap(musb)0
 #endif
 
 #ifdef CONFIG_USB_INVENTRA_DMA
-#define musb_dma_inventra(musb)(musb->io.quirks & 
MUSB_DMA_INVENTRA)
+#define musb_dma_inventra(musb)(musb->ops->quirks & 
MUSB_DMA_INVENTRA)
 #else
 #defin

[PATCH 07/11] usb: musb: remove duplicated port mode enum

[Bin Liu]

include/linux/usb/musb.h already defines enum for musb port mode, so
remove the duplicate in musb_core.h and use the definition in musb.h.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_core.c| 8 
 drivers/usb/musb/musb_core.h| 8 +---
 drivers/usb/musb/musb_dsps.c| 6 +++---
 drivers/usb/musb/musb_gadget.c  | 2 +-
 drivers/usb/musb/musb_host.c| 4 ++--
 drivers/usb/musb/musb_virthub.c | 2 +-
 drivers/usb/musb/sunxi.c| 8 
 7 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 84f25a2b078d..ce54f48314e1 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1038,7 +1038,7 @@ void musb_start(struct musb *musb)
 * (b) vbus present/connect IRQ, peripheral mode;
 * (c) peripheral initiates, using SRP
 */
-   if (musb->port_mode != MUSB_PORT_MODE_HOST &&
+   if (musb->port_mode != MUSB_HOST &&
musb->xceiv->otg->state != OTG_STATE_A_WAIT_BCON &&
(devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS) {
musb->is_active = 1;
@@ -2323,19 +2323,19 @@ static void musb_deassert_reset(struct work_struct 
*work)
}
 
switch (musb->port_mode) {
-   case MUSB_PORT_MODE_HOST:
+   case MUSB_HOST:
status = musb_host_setup(musb, plat->power);
if (status < 0)
goto fail3;
status = musb_platform_set_mode(musb, MUSB_HOST);
break;
-   case MUSB_PORT_MODE_GADGET:
+   case MUSB_PERIPHERAL:
status = musb_gadget_setup(musb);
if (status < 0)
goto fail3;
status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
break;
-   case MUSB_PORT_MODE_DUAL_ROLE:
+   case MUSB_OTG:
status = musb_host_setup(musb, plat->power);
if (status < 0)
goto fail3;
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index f57323e50e44..04203b7126d5 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -53,12 +53,6 @@
 #define is_peripheral_active(m)(!(m)->is_host)
 #define is_host_active(m)  ((m)->is_host)
 
-enum {
-   MUSB_PORT_MODE_HOST = 1,
-   MUSB_PORT_MODE_GADGET,
-   MUSB_PORT_MODE_DUAL_ROLE,
-};
-
 /** CONSTANTS /
 
 #ifndef MUSB_C_NUM_EPS
@@ -351,7 +345,7 @@ struct musb {
 
u8  min_power;  /* vbus for periph, in mA/2 */
 
-   int port_mode;  /* MUSB_PORT_MODE_* */
+   enum musb_mode  port_mode;
boolsession;
unsigned long   quirk_retries;
boolis_host;
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index d077eefb5e2c..951ac63f3526 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -183,7 +183,7 @@ static void dsps_musb_enable(struct musb *musb)
musb_writel(reg_base, wrp->coreintr_set, coremask);
/* start polling for ID change in dual-role idle mode */
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
-   musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+   musb->port_mode == MUSB_OTG)
dsps_mod_timer(glue, -1);
 }
 
@@ -231,7 +231,7 @@ static int dsps_check_status(struct musb *musb, void 
*unused)
break;
case OTG_STATE_A_WAIT_BCON:
/* keep VBUS on for host-only mode */
-   if (musb->port_mode == MUSB_PORT_MODE_HOST) {
+   if (musb->port_mode == MUSB_HOST) {
dsps_mod_timer_optional(glue);
break;
}
@@ -1030,7 +1030,7 @@ static int dsps_resume(struct device *dev)
musb_writel(mbase, wrp->tx_mode, glue->context.tx_mode);
musb_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
if (musb->xceiv->otg->state == OTG_STATE_B_IDLE &&
-   musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+   musb->port_mode == MUSB_OTG)
dsps_mod_timer(glue, -1);
 
pm_runtime_put(dev);
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index e564695c6c8d..967e8e2f6318 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1822,7 +1822,7 @@ int musb_gadget_setup(struct musb *musb)
 
 void musb_gadget_cleanup(struct musb *musb)
 {
-   if (musb->port_mode == MUSB_PORT_MODE_HOST)
+   if (musb->port_mode == MUSB_HOST)
return;
 
can

[PATCH 08/11] usb: musb: remove unused members in struct musb_hdrc_config

[Bin Liu]

The following members in struct musb_hdrc_config are not used,
so remove them.

soft_con
utm_16
big_endian
mult_bulk_tx
mult_bulk_rx
high_iso_tx
high_iso_rx
dma
dma_channels
dyn_fifo_size
vendor_ctrl
vendor_stat
vendor_req
dma_req_chan
musb_hdrc_eps_bits

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/sunxi.c |  4 
 include/linux/usb/musb.h | 15 ---
 2 files changed, 19 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 8f7d378b7e7e..62ab2ca03779 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -651,10 +651,8 @@ static void sunxi_musb_writew(void __iomem *addr, unsigned 
offset, u16 data)
.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
.multipoint = true,
.dyn_fifo   = true,
-   .soft_con   = true,
.num_eps= SUNXI_MUSB_MAX_EP_NUM,
.ram_bits   = SUNXI_MUSB_RAM_BITS,
-   .dma= 0,
 };
 
 static struct musb_hdrc_config sunxi_musb_hdrc_config_h3 = {
@@ -662,10 +660,8 @@ static void sunxi_musb_writew(void __iomem *addr, unsigned 
offset, u16 data)
.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_h3),
.multipoint = true,
.dyn_fifo   = true,
-   .soft_con   = true,
.num_eps= SUNXI_MUSB_MAX_EP_NUM_H3,
.ram_bits   = SUNXI_MUSB_RAM_BITS,
-   .dma= 0,
 };
 
 
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 9eb908a98033..fc6c77918481 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -67,28 +67,13 @@ struct musb_hdrc_config {
/* MUSB configuration-specific details */
unsignedmultipoint:1;   /* multipoint device */
unsigneddyn_fifo:1 __deprecated; /* supports dynamic fifo 
sizing */
-   unsignedsoft_con:1 __deprecated; /* soft connect required */
-   unsignedutm_16:1 __deprecated; /* utm data witdh is 16 bits */
-   unsignedbig_endian:1;   /* true if CPU uses big-endian */
-   unsignedmult_bulk_tx:1; /* Tx ep required for multbulk pkts */
-   unsignedmult_bulk_rx:1; /* Rx ep required for multbulk pkts */
-   unsignedhigh_iso_tx:1;  /* Tx ep required for HB iso */
-   unsignedhigh_iso_rx:1;  /* Rx ep required for HD iso */
-   unsigneddma:1 __deprecated; /* supports DMA */
-   unsignedvendor_req:1 __deprecated; /* vendor registers required 
*/
 
/* need to explicitly de-assert the port reset after resume? */
unsignedhost_port_deassert_reset_at_resume:1;
 
u8  num_eps;/* number of endpoints _with_ ep0 */
-   u8  dma_channels __deprecated; /* number of dma channels */
-   u8  dyn_fifo_size;  /* dynamic size in bytes */
-   u8  vendor_ctrl __deprecated; /* vendor control reg width */
-   u8  vendor_stat __deprecated; /* vendor status reg witdh */
-   u8  dma_req_chan __deprecated; /* bitmask for required dma 
channels */
u8  ram_bits;   /* ram address size */
 
-   struct musb_hdrc_eps_bits *eps_bits __deprecated;
u32 maximum_speed;
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/11] usb: musb: dsps: remove duplicated get_musb_port_mode()

[Bin Liu]

musb_core already has musb_get_mode(), so remove the duplicate from
musb_dsps.c.

Signed-off-by: Bin Liu <b-...@ti.com>
---
 drivers/usb/musb/musb_dsps.c | 21 +
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 05a679d5e3a2..d077eefb5e2c 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -731,25 +731,6 @@ static int get_int_prop(struct device_node *dn, const char 
*s)
return val;
 }
 
-static int get_musb_port_mode(struct device *dev)
-{
-   enum usb_dr_mode mode;
-
-   mode = usb_get_dr_mode(dev);
-   switch (mode) {
-   case USB_DR_MODE_HOST:
-   return MUSB_PORT_MODE_HOST;
-
-   case USB_DR_MODE_PERIPHERAL:
-   return MUSB_PORT_MODE_GADGET;
-
-   case USB_DR_MODE_UNKNOWN:
-   case USB_DR_MODE_OTG:
-   default:
-   return MUSB_PORT_MODE_DUAL_ROLE;
-   }
-}
-
 static int dsps_create_musb_pdev(struct dsps_glue *glue,
struct platform_device *parent)
 {
@@ -809,7 +790,7 @@ static int dsps_create_musb_pdev(struct dsps_glue *glue,
config->num_eps = get_int_prop(dn, "mentor,num-eps");
config->ram_bits = get_int_prop(dn, "mentor,ram-bits");
config->host_port_deassert_reset_at_resume = 1;
-   pdata.mode = get_musb_port_mode(dev);
+   pdata.mode = musb_get_mode(dev);
/* DT keeps this entry in mA, musb expects it as per USB spec */
pdata.power = get_int_prop(dn, "mentor,power") / 2;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html