Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 19:24, Felipe Balbi  wrote:
>>> >> >> >
>>> >> >> > Seems you don't want to guarantee charger type detection is done
>>> >> >> > before gadget connection(pullup DP), right?
>>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>>> >> >> > state
>>> >> >> changes.
>>> >> >>
>>> >> >> I am not sure I get your point correctly, please correct me if I
>>> >> >> misunderstand you.
>>> >> >> We need to check the charger type at every event comes from the
>>> >> >> usb gadget state changes or the extcon device state changes, which
>>> >> >> means a new charger plugin or pullup.
>>> >> >>
>>> >> >
>>> >> > According to usb charger spec, my understanding is you can't do
>>> >> > real charger detection procedure *after* gadget _connection_(pullup
>>> >> > DP), also I don't
>>> >>
>>> >> Why can not? Charger detection is usually from PMIC.
>>> >
>>> > Charger detection process will impact DP/DM line state, see usb
>>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>>> >
>>> > "A PD is allowed to *disconnect* and repeat the charger detection
>>> > process multiple times while attached. The PD is required to wait for
>>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>>> > the charger detection process."
>>> >
>>> > As Peter mentioned, the charger detection should happen between VBUS
>>> > detection and gadget pull up DP for first plug in case. So when
>>> > gadget connect (pullup DP), you should already know the charger type.
>>>
>>> Make sense. In our company's solution, charger detection can be done by
>>> hardware from PMIC at first, then it will not affect the DP/DM line when
>>> gadget starts to enumeration.
>>
>> I see, charger type detection is done automatically by PMIC when VBUS
>> is detected in your case, you just assume the process is complete
>
> assuming this finishes before gadget starts is a bad idea. It would've
> been much more robust to delay usb_gadget_connect() until we KNOW
> charger detection has completed.

It is hardware action to detect the charger type quickly. It actually
*gets* the charger type and does not means *detect* charger type in
'usb_charger_detect_type()' function. Maybe I need to change the
function name as 'usb_charger_get_type()'.

If some udc drivers want to detect charger type in
'gadget->ops->get_charger_type()' callback, they should avoid
impacting DP/DM line state at the right gadget state. Thanks.

>
> --
> balbi



-- 
Baolin.wang
Best Regards
--
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

musb: kernel 3.18.29 build issue

[Yegor Yefremov]

Hello Greg,

3.18.x is missing this patch [1] in order to perform a successful
build. The reason: following patch [2] in 3.18.x uses a macro name
introduced in the missing patch [1].

Could you queue it for 3.18.30?

[1] http://marc.info/?l=git-commits-head=142403040503570=2
[2] 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/usb/musb/musb_cppi41.c?h=linux-3.18.y=7059616548ef5443e3cf70fa3befdc3d4f9c2da1

Thanks.

Yegor
--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 18:58, Jun Li  wrote:
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when
>> > gadget connect (pullup DP), you should already know the charger type.
>>
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration.
>
> I see, charger type detection is done automatically by PMIC when VBUS is
> detected in your case, you just assume the process is complete before SW
> do gadget connect. To make the framework common, you may do one time charger 
> type check when vbus is on, and save it to avoid repeat charger type check.

OK. I'll add one judgement to check if the charger type is set in
'usb_charger_detect_type()' function.

-- 
Baolin.wang
Best Regards
--
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: [GIT PULL] USB fixes for v4.6-rc2

[Greg Kroah-Hartman]

On Wed, Mar 30, 2016 at 01:48:28PM +0300, Felipe Balbi wrote:
> 
> Hi Greg,
> 
> here's the first set of fixes for v4.6-rc cycle. Let me know if you want
> anything to be changed ;-)
> 
> The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca:
> 
>   Linux 4.6-rc1 (2016-03-26 16:03:24 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> tags/fixes-for-v4.6-rc2

Pulled and pushed out, thanks.

greg k-h
--
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: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout

[Rajesh Bhagat]

> -Original Message-
> From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com]
> Sent: Tuesday, March 29, 2016 10:51 PM
> To: Rajesh Bhagat 
> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> ker...@vger.kernel.org; Sriram Dash 
> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI
> commmand timeout
> 
> On 28.03.2016 09:13, Rajesh Bhagat wrote:
> >
> >
> >> -Original Message-
> >> From: Mathias Nyman [mailto:mathias.ny...@linux.intel.com]
> >> Sent: Wednesday, March 23, 2016 7:52 PM
> >> To: Rajesh Bhagat 
> >> Cc: gre...@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
> >> ker...@vger.kernel.org; Sriram Dash 
> >> Subject: Re: [PATCH] usb: xhci: Fix incomplete PM resume operation
> >> due to XHCI commmand timeout
> >>
> >> On 23.03.2016 05:53, Rajesh Bhagat wrote:
> >>
> > IMO, The assumption that "xhci_abort_cmd_ring would always
> > generate an event and handle_cmd_completion would be called" will
> > not be always be true if HW
>  is in bad state.
> >
> > Please share your opinion.
> >
> 
>  writing the CA (command abort) bit in CRCR (command ring control
>  register)  will stop the command ring, and CRR (command ring
>  running) will be set
> >> to 0 by xHC.
>  xhci_abort_cmd_ring() polls this bit up to 5 seconds.
>  If it's not 0 then the driver considers the command abort as failed.
> 
>  The scenario you're thinking of is that xHC would still react to CA
>  bit set, it would stop the command ring and set CRR 0, but not send
>  a command
> >> completion event.
> 
>  Have you tried adding some debug to handle_cmd_completion() and see
>  if you receive any event after command abortion?
> 
> >>>
> >>> Yes. We have added debug prints at first line of
> >>> handle_cmd_completion, and we are not getting those prints. The last
> >>> print messages that we get are as below from xhci_alloc_dev while
> >>> resume
> >>> operation:
> >>>
> >>> xhci-hcd xhci-hcd.0.auto: Command timeout xhci-hcd xhci-hcd.0.auto:
> >>> Abort command ring
> >>>
> >>> May be somehow, USB controller is in bad state and not responding to
> >>> the
> >> commands.
> >>>
> >>> Please suggest how XHCI driver can handle such situations.
> >>>
> >>
> >> Restart the command timeout timer when writing the command abort bit.
> >> If we get theIf we get the abort event the timer is deleted.
> >>
> >> Otherwise if the timout triggers a second time we end up calling
> >> xhci_handle_command_timeout() with a stopped ring, This will call
> >> xhci_handle_stopped_cmd_ring(), turn the aborted command to no-op,
> >> restart the command ring, and finally when the no-op completes it
> >> should call the missing completion.
> >>
> >> If command ring doesn't start then additional code could be added to
> >> xhci_handle_command_timeout() that clears the command ring if it is
> >> called a second time (=current command is already in abort state and
> >> command ring is stopped when entering xhci_handle_command_timeout)
> >>
> >> There might be some details missing, I'm not able to test any of
> >> this, but try something like this:
> >>
> >> diff --git a/drivers/usb/host/xhci-ring.c
> >> b/drivers/usb/host/xhci-ring.c index 3e1d24c..576819e 100644
> >> --- a/drivers/usb/host/xhci-ring.c
> >> +++ b/drivers/usb/host/xhci-ring.c
> >> @@ -319,7 +319,10 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci)
> >>   xhci_halt(xhci);
> >>   return -ESHUTDOWN;
> >>   }
> >> -
> >> +   /* writing the CMD_RING_ABORT bit should create a command 
> >> completion
> >> +* event, add a command completion timeout for it as well
> >> +*/
> >> +   mod_timer(>cmd_timer, jiffies +
> >> + XHCI_CMD_DEFAULT_TIMEOUT);
> >>   return 0;
> >>}
> >
> > Hello Mathias,
> >
> > Thanks for the patch.
> >
> > After application of above patch, I'm getting following prints constantly:
> >
> > xhci-hcd xhci-hcd.0.auto: Command timeout xhci-hcd xhci-hcd.0.auto:
> > Abort command ring xhci-hcd xhci-hcd.0.auto: Command timeout on
> > stopped ring xhci-hcd xhci-hcd.0.auto: Turn aborted command be56e000
> > to no-op xhci-hcd xhci-hcd.0.auto: // Ding dong!
> > ...
> > xhci-hcd xhci-hcd.0.auto: Command timeout xhci-hcd xhci-hcd.0.auto:
> > Abort command ring xhci-hcd xhci-hcd.0.auto: Command timeout on
> > stopped ring xhci-hcd xhci-hcd.0.auto: Turn aborted command be56e000
> > to no-op xhci-hcd xhci-hcd.0.auto: // Ding dong!
> >
> > As expected, xhci_handle_command_timeout is called again and next time
> > ring state is __not__ CMD_RING_STATE_RUNNING, Hence
> > xhci_handle_stopped_cmd_ring is called which turn all the aborted
> > commands to no-ops and again makes the ring state as
> CMD_RING_STATE_RUNNING, and rings the door bell.
> >
> > But again in this case, no response 

Re: [PATCH] usb: dwc2: do not override forced dr_mode in gadget setup

[John Youn]

On 3/30/2016 8:08 AM, Przemek Rudy wrote:
> On 03/30/2016 12:15 PM, Felipe Balbi wrote:
>> John Youn  writes:
>>> [ text/plain ]
>>> On 3/16/2016 3:10 PM, Przemek Rudy wrote:
 The host/device mode set with dr_mode should be kept all the time,
 not being changed to OTG in gadget setup (by overriding CFGUSB_FORCEDEVMODE
 and CFGUSB_FORCEHOSTMODE bits).

 Signed-off-by: Przemek Rudy 
 ---
  drivers/usb/dwc2/gadget.c | 23 ++-
  1 file changed, 18 insertions(+), 5 deletions(-)

 diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
 index e9940dd..818f158 100644
 --- a/drivers/usb/dwc2/gadget.c
 +++ b/drivers/usb/dwc2/gadget.c
 @@ -2254,6 +2254,7 @@ void dwc2_hsotg_core_init_disconnected(struct 
 dwc2_hsotg *hsotg,
  {
u32 intmsk;
u32 val;
 +  u32 usbcfg;
  
/* Kill any ep0 requests as controller will be reinitialized */
kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
 @@ -2267,10 +2268,16 @@ void dwc2_hsotg_core_init_disconnected(struct 
 dwc2_hsotg *hsotg,
 * set configuration.
 */
  
 +  /* keep other bits untouched (so e.g. forced modes are not lost) */
 +  usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
 +  usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
 +  GUSBCFG_HNPCAP);
 +
/* set the PLL on, remove the HNP/SRP and set the PHY */
val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
 -  dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
 - (val << GUSBCFG_USBTRDTIM_SHIFT), hsotg->regs + GUSBCFG);
 +  usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
 +  (val << GUSBCFG_USBTRDTIM_SHIFT);
 +  dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
  
dwc2_hsotg_init_fifo(hsotg);
  
 @@ -3031,6 +3038,7 @@ static struct usb_ep_ops dwc2_hsotg_ep_ops = {
  static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
  {
u32 trdtim;
 +  u32 usbcfg;
/* unmask subset of endpoint interrupts */
  
dwc2_writel(DIEPMSK_TIMEOUTMSK | DIEPMSK_AHBERRMSK |
 @@ -3054,11 +3062,16 @@ static void dwc2_hsotg_init(struct dwc2_hsotg 
 *hsotg)
  
dwc2_hsotg_init_fifo(hsotg);
  
 +  /* keep other bits untouched (so e.g. forced modes are not lost) */
 +  usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
 +  usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
 +  GUSBCFG_HNPCAP);
 +
/* set the PLL on, remove the HNP/SRP and set the PHY */
trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
 -  dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
 -  (trdtim << GUSBCFG_USBTRDTIM_SHIFT),
 -  hsotg->regs + GUSBCFG);
 +  usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
 +  (trdtim << GUSBCFG_USBTRDTIM_SHIFT);
 +  dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
  
if (using_dma(hsotg))
__orr32(hsotg->regs + GAHBCFG, GAHBCFG_DMA_EN);

>>>
>>>
>>> Acked-by: John Youn 
>>> Tested-by: John Youn 
>>
>> I suppose this is for v4.7 merge window ?
>>
> Technically - patch was started against 4.5, as there are no conflict changes 
> around the file it fits for 4.6, 4.7.
> 
> 
> 

Hi Felipe,

Please queue for 4.6-rc.

Thanks,
John



--
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: dwc2: gadget: avoid null dereference on incomplete transfer

[John Youn]

On 3/30/2016 6:22 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Keeping  writes:
>> Setting up a gadget with the uac2 function results in:
>>
>>   Unable to handle kernel NULL pointer dereference at virtual address 
>> 0058
>>   ...
>>   PC is at dwc2_hsotg_irq+0x7f0/0x908
>>   LR is at dwc2_hsotg_irq+0x4c/0x908
>>   Backtrace:
>>   [] (dwc2_hsotg_irq) from [] 
>> (handle_irq_event_percpu+0x130/0x3ec)
>>   [] (handle_irq_event_percpu) from [] 
>> (handle_irq_event+0x48/0x6c)
>>
>> In all other loops we already skip endpoints that are null, so do so
>> here as well.
>>
>> Signed-off-by: John Keeping 
>> ---
>>  drivers/usb/dwc2/gadget.c | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 0abf73c..df43ec0 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2606,7 +2606,9 @@ irq_retry:
>>  for (idx = 1; idx < hsotg->num_of_eps; idx++) {
>>  hs_ep = hsotg->eps_in[idx];
>>  
>> -if (!hs_ep->isochronous || hs_ep->has_correct_parity)
>> +if (!hs_ep ||
>> +!hs_ep->isochronous ||
>> +hs_ep->has_correct_parity)
> 
> this is fine (even though choice of where to break line is a bit odd),
> but I have a question about how the rest of the code works (a bit
> off-topic, sorry)
> 
>>  continue;
>>  
>>  epctl_reg = DIEPCTL(idx);
> 
> So, this means that the first ISO endpoint without correct parity will
> be used. Isn't this a bit fragile ? What happens when you use a device
> with several different interfaces using several different endpoints ?
> 
> Isn't there a register where we can check which physical endpoint
> generated the IRQ ? Seems like you really wanna check what:
> 

We discussed this back when the patch was first submitted and
determined it should work fine like this. I don't remember exactly why
though.

But this ISOC parity stuff is a workaround and we have a series of
patches to correctly set up ISOC allowing us to remove it. We're doing
some final tests before we send them.

Regards,
John

--
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 v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Ruslan Bilovol]

Hi,

On Thu, Mar 31, 2016 at 1:08 AM, Bin Liu  wrote:
> Hi,
>
> On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders  wrote:
>> Bin Liu  writes:
>>
>>> Dirk,
>>>
>>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders  wrote:
 Bin Liu  writes:

> All,
>
> On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN  
> wrote:
>> Dirk, All,
>>
>> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>>> If choices consist of choice_values that depend on symbols set to 'm',
>>> those choice_values are not set to 'n' if the choice is changed from
>>> 'm' to 'y' (in which case only one active choice_value is allowed).
>>> Those values are also written to the config file causing modules to be
>>> built when they should not.
>>>
>>> The following config can be used to reproduce and examine the problem;
>>> with the frontend of your choice set "Choice 0" and "Choice 1" to 'm',
>>> then set "Tristate Choice" to 'y' and save the configuration:
>>>
>>> config modules
>>>   boolean modules
>>>   default y
>>>   option modules
>>>
>>> config dependency
>>>   tristate "Dependency"
>>>   default m
>>>
>>> choice
>>>   prompt "Tristate Choice"
>>>   default choice0
>>>
>>> config choice0
>>>   tristate "Choice 0"
>>>
>>> config choice1
>>>   tristate "Choice 1"
>>>   depends on dependency
>>>
>>> endchoice
>>>
>>> This patch sets choice_values' visibility that depend on symbols set
>>> to 'm' to 'n' if the corresponding choice is set to 'y'.  This makes
>>> them disappear from the choice list and will also cause the
>>> choice_values' value set to 'n' in sym_calc_value() and as a result
>>> they are written as "not set" to the resulting .config file.
>>>
>>> Reported-by: Sebastian Andrzej Siewior 
>>> Signed-off-by: Dirk Gouders 
>>> Tested-by: Sebastian Andrzej Siewior 
>>
>> Acked-by: "Yann E. MORIN" 
>>
>> It will be in my tree soon. Thanks!
>
> I don't see this patch in 3.16 but 3.16 does not have the issue any
> more. Anyone has an idea how the issue got fixed? I am trying to find
> the right patch to backport.

 With the above sample kconfig I still see the issue.  How did you
 notice the issue got fixed?
>>>
>>> I did not pay much attention on the above sample kconfig, but just
>>> focused on the USB gadget driver kconfig issue initially reported by
>>> Sebastian. I saw the issue exists in 3.14, but does not in 3.16,
>>> unless I messed up with my test. I will test 3.16 again some time next
>>> week.
>>
>> Hi Bin,
>>
>> I now also re-tested the initially reported steps to reproduce the
>> issue:
>>
>> 
>>> in USB gadget menu (that is Device Drivers ---> USB support ---> USB
>>> Gadget Support --->  USB Gadget Drivers) I can create a configuration
>>> which is "lost". Here is how to reproduce it:
>>>
>>> - first config two gadgets as M:
>>> USB Gadget Drivers
>>>   Audio Gadget
>>>   Ethernet Gadget
>>>   MIDI Gadget
>>>
>>>  save config & leave
>>>
>>> - now start menu config again and go to the same menu, now select
>>>   built-in:
>>>   <*>   USB Gadget Drivers (Ethernet Gadget
>>>   the ethernet gadget is chosen automatically because we can have only
>>>   one gadget selected.
>>>   save config & leave
>>>
>>> - step three, go back to the menu and you will see that everything is
>>>   as it was (the <*> is ignored).
>> 
>>
>> Here, I still see the problem (I was wondering if the issue has been
>> solved/gone by a kconfig-file modification).
>
> This issue was gone since 3.16, but came back again due to commit
> 1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules.
>

I can confirm this issue too, faced it on v4.5 (but didn't try v4.6-rc1 yet)

-- 
Best regards,
Ruslan Bilovol
--
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 v4] kconfig/symbol.c: handle choice_values that depend on 'm' symbols

[Bin Liu]

Hi,

On Fri, Aug 15, 2014 at 2:37 AM, Dirk Gouders  wrote:
> Bin Liu  writes:
>
>> Dirk,
>>
>> On Thu, Aug 14, 2014 at 1:52 AM, Dirk Gouders  wrote:
>>> Bin Liu  writes:
>>>
 All,

 On Mon, Nov 18, 2013 at 12:08 PM, Yann E. MORIN  
 wrote:
> Dirk, All,
>
> On 2013-11-07 15:05 +0100, Dirk Gouders spake thusly:
>> If choices consist of choice_values that depend on symbols set to 'm',
>> those choice_values are not set to 'n' if the choice is changed from
>> 'm' to 'y' (in which case only one active choice_value is allowed).
>> Those values are also written to the config file causing modules to be
>> built when they should not.
>>
>> The following config can be used to reproduce and examine the problem;
>> with the frontend of your choice set "Choice 0" and "Choice 1" to 'm',
>> then set "Tristate Choice" to 'y' and save the configuration:
>>
>> config modules
>>   boolean modules
>>   default y
>>   option modules
>>
>> config dependency
>>   tristate "Dependency"
>>   default m
>>
>> choice
>>   prompt "Tristate Choice"
>>   default choice0
>>
>> config choice0
>>   tristate "Choice 0"
>>
>> config choice1
>>   tristate "Choice 1"
>>   depends on dependency
>>
>> endchoice
>>
>> This patch sets choice_values' visibility that depend on symbols set
>> to 'm' to 'n' if the corresponding choice is set to 'y'.  This makes
>> them disappear from the choice list and will also cause the
>> choice_values' value set to 'n' in sym_calc_value() and as a result
>> they are written as "not set" to the resulting .config file.
>>
>> Reported-by: Sebastian Andrzej Siewior 
>> Signed-off-by: Dirk Gouders 
>> Tested-by: Sebastian Andrzej Siewior 
>
> Acked-by: "Yann E. MORIN" 
>
> It will be in my tree soon. Thanks!

 I don't see this patch in 3.16 but 3.16 does not have the issue any
 more. Anyone has an idea how the issue got fixed? I am trying to find
 the right patch to backport.
>>>
>>> With the above sample kconfig I still see the issue.  How did you
>>> notice the issue got fixed?
>>
>> I did not pay much attention on the above sample kconfig, but just
>> focused on the USB gadget driver kconfig issue initially reported by
>> Sebastian. I saw the issue exists in 3.14, but does not in 3.16,
>> unless I messed up with my test. I will test 3.16 again some time next
>> week.
>
> Hi Bin,
>
> I now also re-tested the initially reported steps to reproduce the
> issue:
>
> 
>> in USB gadget menu (that is Device Drivers ---> USB support ---> USB
>> Gadget Support --->  USB Gadget Drivers) I can create a configuration
>> which is "lost". Here is how to reproduce it:
>>
>> - first config two gadgets as M:
>> USB Gadget Drivers
>>   Audio Gadget
>>   Ethernet Gadget
>>   MIDI Gadget
>>
>>  save config & leave
>>
>> - now start menu config again and go to the same menu, now select
>>   built-in:
>>   <*>   USB Gadget Drivers (Ethernet Gadget
>>   the ethernet gadget is chosen automatically because we can have only
>>   one gadget selected.
>>   save config & leave
>>
>> - step three, go back to the menu and you will see that everything is
>>   as it was (the <*> is ignored).
> 
>
> Here, I still see the problem (I was wondering if the issue has been
> solved/gone by a kconfig-file modification).

This issue was gone since 3.16, but came back again due to commit
1fd6d08 ARM: omap2plus_defconfig: Enable n900 modem as loadable modules.

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: USB gadgets with configfs hang reboot

[Tony Lindgren]

* Alan Stern  [160330 12:26]:
> On Wed, 30 Mar 2016, Ivaylo Dimitrov wrote:
> 
> > >>> seems to be created twice :). Maybe the problem is that the first time
> > >>> musb-hdrc is probed it fails with -EPROBE_DEFER, however that failure is
> > >>> after gadget drivers got loaded and noone unloads them.
> > >>
> > >> gadget drivers will get added to a pending list, then later they'll
> > >> bind. But they shouldn't bind() twice, unless there are multiple
> > >> interfaces for them.
> > >>
> > >
> > > Well, then it seems we have problem, as the 2 unbind() calls are with
> > > one and the same "common" pointer (again, from memory).
> > >
> > >>> Just some wild guesses based on my memories as I've lost the logs (power
> > >>> outage). For sure I can recreate them if needed.
> > >>
> > >> okay.
> > >
> > > I will redo dump_stack() and printks and will provide logs as soon as I
> > > have some time, so to stop counting on my memories.
> > >
> > 
> > Please find attached the relevant logs. It really seems that g_nokia is 
> > probed twice, with all the gadgets in it created two times. I am 
> > starting to suspect 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb: 
> > gadget: udc-core: independent registration of gadgets and gadget 
> > drivers") has something to do with the problem, though reverting it 
> > resulted in g_nokia not being probed at all :)
> 
> The problem is not caused by nokia_bind() getting called twice.  The
> log clearly shows that nokia_bind() is called only once, but it calls
> usb_add_config() from two different places:
> 
> Jan  1 02:00:10 Nokia-N900 kernel: [8.002838] [] 
> (usb_add_config) from [] (nokia_bind+0x160/0x2f0)
> Jan  1 02:00:10 Nokia-N900 kernel: [8.014526] [] (nokia_bind) 
> from [] (composite_bind+0x68/0x1a0)
> ...
> Jan  1 02:00:10 Nokia-N900 kernel: [8.381286] [] 
> (usb_add_config) from [] (nokia_bind+0x178/0x2f0)
> Jan  1 02:00:10 Nokia-N900 kernel: [8.394348] [] (nokia_bind) 
> from [] (composite_bind+0x68/0x1a0)
> 
> Everything else along the two pathways is the same, so this is where
> the multiple binds come from.  And indeed, looking at the code in
> nokia_bind() you can see the two calls:
> 
> /* finally register the configuration */
> status = usb_add_config(cdev, _config_500ma_driver,
> nokia_bind_config);
> if (status < 0)
> goto err_msg_luns;
> 
> status = usb_add_config(cdev, _config_100ma_driver,
> nokia_bind_config);
> 
> This isn't supposed to cause any problems.  The two instances should 
> never run at the same time, because they belong to different configs.

Maybe drivers/usb/gadget/legacy/ether.c suffers from a similar issue
as that's what I noticed the $subject problem with.

Regards,

Tony
--
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 gadgets with configfs hang reboot

[Alan Stern]

On Wed, 30 Mar 2016, Ivaylo Dimitrov wrote:

> >>> seems to be created twice :). Maybe the problem is that the first time
> >>> musb-hdrc is probed it fails with -EPROBE_DEFER, however that failure is
> >>> after gadget drivers got loaded and noone unloads them.
> >>
> >> gadget drivers will get added to a pending list, then later they'll
> >> bind. But they shouldn't bind() twice, unless there are multiple
> >> interfaces for them.
> >>
> >
> > Well, then it seems we have problem, as the 2 unbind() calls are with
> > one and the same "common" pointer (again, from memory).
> >
> >>> Just some wild guesses based on my memories as I've lost the logs (power
> >>> outage). For sure I can recreate them if needed.
> >>
> >> okay.
> >
> > I will redo dump_stack() and printks and will provide logs as soon as I
> > have some time, so to stop counting on my memories.
> >
> 
> Please find attached the relevant logs. It really seems that g_nokia is 
> probed twice, with all the gadgets in it created two times. I am 
> starting to suspect 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb: 
> gadget: udc-core: independent registration of gadgets and gadget 
> drivers") has something to do with the problem, though reverting it 
> resulted in g_nokia not being probed at all :)

The problem is not caused by nokia_bind() getting called twice.  The
log clearly shows that nokia_bind() is called only once, but it calls
usb_add_config() from two different places:

Jan  1 02:00:10 Nokia-N900 kernel: [8.002838] [] (usb_add_config) 
from [] (nokia_bind+0x160/0x2f0)
Jan  1 02:00:10 Nokia-N900 kernel: [8.014526] [] (nokia_bind) 
from [] (composite_bind+0x68/0x1a0)
...
Jan  1 02:00:10 Nokia-N900 kernel: [8.381286] [] (usb_add_config) 
from [] (nokia_bind+0x178/0x2f0)
Jan  1 02:00:10 Nokia-N900 kernel: [8.394348] [] (nokia_bind) 
from [] (composite_bind+0x68/0x1a0)

Everything else along the two pathways is the same, so this is where
the multiple binds come from.  And indeed, looking at the code in
nokia_bind() you can see the two calls:

/* finally register the configuration */
status = usb_add_config(cdev, _config_500ma_driver,
nokia_bind_config);
if (status < 0)
goto err_msg_luns;

status = usb_add_config(cdev, _config_100ma_driver,
nokia_bind_config);

This isn't supposed to cause any problems.  The two instances should 
never run at the same time, because they belong to different configs.

Alan Stern

--
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 gadgets with configfs hang reboot

[Ivaylo Dimitrov]

On 30.03.2016 21:50, Ivaylo Dimitrov wrote:



On 30.03.2016 17:01, Ivaylo Dimitrov wrote:



On 30.03.2016 16:38, Felipe Balbi wrote:


Hi,

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

On 16.01.2016 12:40, Ivaylo Dimitrov wrote:

Hi,

On 16.01.2016 00:48, Tony Lindgren wrote:

Hi all,

Looks like there's some issue with the USB gadgets and
configfs.

I'm seeing rmmod of the UDC driver cause a warning and then
reboot
hangs the system. This happens at least with v4.4, and I've
reproduced
it with dwc3 and musb so it seems to be generic.



Having configfs is not needed, disabling usb gadgets (#
CONFIG_USB_MUSB_GADGET is not set) seems to solved at least
poweroff
hang issue on N900. Also, g_nokia is not a module in the
config I use,
so I guess the problem is not related whether gadgets are
modular or
not. Unfortunately I was not able to test reboot, as rootfs
became
corrupted after the first poweroff :( . So it looks like my
theory that
onenand corruption on N900 is because poweroff/reboot hangs is
wrong.

Ivo



Is there any progress on the issue?




Doing Nokia-N900:/sys/bus/platform/drivers/musb-hdrc# echo
musb-hdrc.0.auto > unbind results in:

<1>[ 1418.511260] Unable to handle kernel paging request at virtual
address 6c6c757a
<7>[ 1418.677215] pvr: Xorg: cleaning up 49 unfreed resources
<1>[ 1418.683349] pgd = c0004000
<1>[ 1418.739959] [6c6c757a] *pgd=
<0>[ 1418.746307] Internal error: Oops: 5 [#1] PREEMPT ARM
<4>[ 1418.753997] Modules linked in: sha256_generic hmac drbg
ansi_cprng
ctr ccm vfat fat rfcomm sd_mod scsi_mod bnep bluetooth omaplfb
pvrsrvkm
ipv6 bq2415x_charger uinput radio_platform_si4713 joydev cmt_speech
hsi_char video_bus_switch arc4 wl1251_spi wl1251 isp1704_charger
gpio_keys mac80211 smc91x mii cfg80211 omap_wdt crc7 omap_sham
tsc2005
tsc200x_core bq27xxx_battery_i2c si4713 adp1653 tsl2563 leds_lp5523
leds_lp55xx_common bq27xxx_battery rtc_twl twl4030_wdt et8ek8
ad5820
v4l2_common smiaregs twl4030_vibra videodev ff_memless
lis3lv02d_i2c
lis3lv02d media input_polldev omap_ssi_port ti_soc_thermal
nokia_modem
ssi_protocol omap_ssi hsi rx51_battery
<4>[ 1418.835906] CPU: 0 PID: 53 Comm: file-storage Not tainted
4.5.0-rc5+ #59
<4>[ 1418.846130] Hardware name: Nokia RX-51 board
<4>[ 1418.853820] task: ceb8a300 ti: ce008000 task.ti: ce008000
<4>[ 1418.862792] PC is at handle_exception+0xa8/0x418
<4>[ 1418.871002] LR is at recalc_sigpending+0x18/0x7c
<4>[ 1418.879241] pc : []lr : []psr:
8013
<4>[ 1418.879241] sp : ce009ea0  ip :   fp : 
<4>[ 1418.898284] r10:   r9 :   r8 : 
<4>[ 1418.907287] r7 : c031d8d0  r6 : 6c6c7566  r5 :   r4
: cebe1600
<4>[ 1418.917663] r3 : 6f642820  r2 :   r1 :   r0
: 
<4>[ 1418.928039] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA
ARM
Segment none
<4>[ 1418.939025] Control: 10c5387d  Table: 8e244019  DAC: 0051
<0>[ 1418.948516] Process file-storage (pid: 53, stack limit =
0xce008210)
<0>[ 1418.958679] Stack: (0xce009ea0 to 0xce00a000)
<0>[ 1418.966735] 9ea0: 000f   0b07

0001 03ff 0001
<0>[ 1418.978973] 9ec0: ceb8a300 ceb8a300  c004841c

0002 ce888000 c0451a50
<0>[ 1418.991180] 9ee0:    0008
cebe1600
0001 c0717dd0 0001
<0>[ 1419.003387] 9f00:   ce009f14 c044ddf4

c031c020 0042 ce009f30
<0>[ 1419.015686] 9f20: ce009f30  cebe1600 c031d958

c044d864 a013 
<0>[ 1419.027923] 9f40: cebe1600 c031d8d0 cebfa100 cebfa100

cebe1600 c031d8d0 
<0>[ 1419.040130] 9f60:    c00474e4
dc4d900d
 31bc92e7 cebe1600
<0>[ 1419.052429] 9f80:  ce009f84 ce009f84 
ce009f90
ce009f90 ce009fac cebfa100
<0>[ 1419.064697] 9fa0: c0047418   c000f218

  
<0>[ 1419.076934] 9fc0:    

  
<0>[ 1419.089050] 9fe0:    
0013
 2000 3891
<4>[ 1419.101043] [] (handle_exception) from []
(fsg_main_thread+0x88/0x13dc)
<4>[ 1419.113189] [] (fsg_main_thread) from []
(kthread+0xcc/0xe0)
<4>[ 1419.124267] [] (kthread) from []
(ret_from_fork+0x14/0x3c)
<0>[ 1419.135101] Code: 1a15 ea40 e5946038 e0866285
(e5963014)
<4>[ 1419.330841] ---[ end trace 3377457e25b0732c ]---
<0>[ 1419.340972] Kernel panic - not syncing: Fatal exception

weirdly, I have that log only in mtdoops, but not in dmesg.
However,
after that oops "reboot" command does not hang, but reboots the
device.



So, what is handle_exception + 0xa8 ? You can figure that out either
with gdb or addr2line assuming your vmlinux has dbg symbols.

For gdb you would:

gdb vmlinux
(gdb) l *(handle_exception + 

Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()

[John Youn]

On 3/23/2016 11:52 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn  writes:
>> [ text/plain ]
>> On 3/21/2016 11:40 PM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> John Youn  writes:
 [ text/plain ]
 On 3/18/2016 12:17 PM, John Youn wrote:
> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>
>> heh, +john
>>
>> Felipe Balbi  writes:
>>> [ text/plain ]
>>>
>>> Hi,
>>>
>>> Roger Quadros  writes:
 [ text/plain ]
 We will need this function for a workaround.
 The function issues a softreset only to the device
 controller and performs minimal re-initialization
 so that the device controller can be usable.

 As some code is similar to dwc3_core_init() take out
 common code into dwc3_get_gctl_quirks().

 We add a new member (prtcap_mode) to struct dwc3 to
 keep track of the current mode in the PRTCAPDIR register.

 Signed-off-by: Roger Quadros 
>>>
>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>> needs this because of a poor silicon integration which took an IP with a
>>> known erratum where it can't be made to work on lower speeds and STILL
>>> was integrated without a superspeed PHY.
>>>
>>> There's a reason why I never tried to push this upstream myself ;-)
>>>
>>> I'm really thinking we might be better off adding a quirk flag to skip
>>> the metastability workaround and allow this ONE silicon to set the
>>> controller to lower speed.
>>>
>>> John, can you check with your colleagues if we would ever fall into
>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>> probe and never touch it again ? I would assume we don't really fall
>>> into the metastability workaround, right ? We're not doing any sort of
>>> PM for dwc3...
>>>

 Hi Felipe,

 Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
 I don't see an issue with that as long as we always ignore
 dwc->maximum_speed when programming DCFG.speed for all affected
 versions of the core. As long as the DCFG.speed = SS, you should not
 hit the STAR.
>>>
>>> I actually mean changing DCFG.speed during driver probe and never
>>> touching it again. Would that still cause problems ?
>>>
>>
>> In that case I'm not sure. The engineer who would know is off until
>> next week so I'll get back to you as soon as I can talk to him about
>> it.
> 

So the engineers said that this issue can occur while set to HS and
the run/stop bit is changed so it seems that won't work.

Regards,
John
--
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 v7 1/4] gadget: Introduce the usb charger framework

[Mark Brown]

On Wed, Mar 30, 2016 at 01:09:00PM +0300, Felipe Balbi wrote:
> Baolin Wang  writes:

> > +#include 
> > +#include 
> > +#include 
> > +#include 

> not very nice to depend on either of or platform_device here. What about
> PCI-based devices ?

The header inclusion shouldn't be conditional though.  But looking at
the patch I can't immediately see any use of these in the code anyway.

> > +static DEVICE_ATTR_RW(sdp_limit);

> why RW ? Who's going to use these ? Also, you're not documenting this
> new sysfs file.

If they end up not writeable should we just remove them entirely since
they should just be the spec values?


signature.asc
Description: PGP signature

Re: [PATCH 001/001] MUSB MULTIPOINT HIGH SPEED DUAL-ROLE CONTROLLER Fix trivial typo in constant name EP_MODE_AUTOREG_NONE

[Bin Liu]

Hi,

On Wed, Mar 30, 2016 at 10:23:31AM -0500, Bin Liu wrote:
> Hi,
> 
> 
> On Wed, Mar 30, 2016 at 06:56:28AM +0800, Antonio Victor Hilario wrote:
> > I'd been using kernel 3.18.10-29 on a set of Beaglebone Black boards, and 
> > had found and corrected this on my local build tree.
> > 
> > Kernel build fails when the source file drivers/usb/musb/musb_cppi41.c is 
> > built, with these kernel options enabled:
> > 
> > CONFIG_USB_MUSB_HDRC=y
> > CONFIG_USB_TI_CPPI41_DMA=y
> > 
> > The build fails with these errors, due to a misspelled constant name 
> > EP_MODE_AUTOREQ_NONE:
> > 
> > drivers/usb/musb/musb_cppi41.c: In function 'cppi41_dma_channel_abort':
> > drivers/usb/musb/musb_cppi41.c:544:43: error: 'EP_MODE_AUTOREQ_NONE' 
> > undeclared (first use in this function)
> >cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
> >^
> > drivers/usb/musb/musb_cppi41.c:544:43: note: each undeclared identifier is 
> > reported only once for each function it appears in
> > scripts/Makefile.build:257: recipe for target 
> > 'drivers/usb/musb/musb_cppi41.o' failed
> > make[3]: *** [drivers/usb/musb/musb_cppi41.o] Error 1
> > scripts/Makefile.build:402: recipe for target 'drivers/usb/musb' failed
> > make[2]: *** [drivers/usb/musb] Error 2
> > scripts/Makefile.build:402: recipe for target 'drivers/usb' failed
> > make[1]: *** [drivers/usb] Error 2
> > Makefile:937: recipe for target 'drivers' failed
> > 
> 
> This is due to the stable kernel misses patch 0149b07 (usb: musb:
> cppi41: correct the macro name EP_MODE_AUTOREG_*) from the upstream when
> back porting.
> 
> I believe this has been corrected in v3.12, but not sure about other
> stable kernels - I wasn't cc'd in these backport notification emails.

FYI -
http://article.gmane.org/gmane.linux.kernel.stable/167063/match=cppi41

> 
> 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 001/001] MUSB MULTIPOINT HIGH SPEED DUAL-ROLE CONTROLLER Fix trivial typo in constant name EP_MODE_AUTOREG_NONE

[Bin Liu]

Hi,


On Wed, Mar 30, 2016 at 06:56:28AM +0800, Antonio Victor Hilario wrote:
> I'd been using kernel 3.18.10-29 on a set of Beaglebone Black boards, and had 
> found and corrected this on my local build tree.
> 
> Kernel build fails when the source file drivers/usb/musb/musb_cppi41.c is 
> built, with these kernel options enabled:
> 
> CONFIG_USB_MUSB_HDRC=y
> CONFIG_USB_TI_CPPI41_DMA=y
> 
> The build fails with these errors, due to a misspelled constant name 
> EP_MODE_AUTOREQ_NONE:
> 
> drivers/usb/musb/musb_cppi41.c: In function 'cppi41_dma_channel_abort':
> drivers/usb/musb/musb_cppi41.c:544:43: error: 'EP_MODE_AUTOREQ_NONE' 
> undeclared (first use in this function)
>cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
>^
> drivers/usb/musb/musb_cppi41.c:544:43: note: each undeclared identifier is 
> reported only once for each function it appears in
> scripts/Makefile.build:257: recipe for target 
> 'drivers/usb/musb/musb_cppi41.o' failed
> make[3]: *** [drivers/usb/musb/musb_cppi41.o] Error 1
> scripts/Makefile.build:402: recipe for target 'drivers/usb/musb' failed
> make[2]: *** [drivers/usb/musb] Error 2
> scripts/Makefile.build:402: recipe for target 'drivers/usb' failed
> make[1]: *** [drivers/usb] Error 2
> Makefile:937: recipe for target 'drivers' failed
> 

This is due to the stable kernel misses patch 0149b07 (usb: musb:
cppi41: correct the macro name EP_MODE_AUTOREG_*) from the upstream when
back porting.

I believe this has been corrected in v3.12, but not sure about other
stable kernels - I wasn't cc'd in these backport notification emails.

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: dwc2: do not override forced dr_mode in gadget setup

[Przemek Rudy]

On 03/30/2016 12:15 PM, Felipe Balbi wrote:
> John Youn  writes:
>> [ text/plain ]
>> On 3/16/2016 3:10 PM, Przemek Rudy wrote:
>>> The host/device mode set with dr_mode should be kept all the time,
>>> not being changed to OTG in gadget setup (by overriding CFGUSB_FORCEDEVMODE
>>> and CFGUSB_FORCEHOSTMODE bits).
>>>
>>> Signed-off-by: Przemek Rudy 
>>> ---
>>>  drivers/usb/dwc2/gadget.c | 23 ++-
>>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>> index e9940dd..818f158 100644
>>> --- a/drivers/usb/dwc2/gadget.c
>>> +++ b/drivers/usb/dwc2/gadget.c
>>> @@ -2254,6 +2254,7 @@ void dwc2_hsotg_core_init_disconnected(struct 
>>> dwc2_hsotg *hsotg,
>>>  {
>>> u32 intmsk;
>>> u32 val;
>>> +   u32 usbcfg;
>>>  
>>> /* Kill any ep0 requests as controller will be reinitialized */
>>> kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>>> @@ -2267,10 +2268,16 @@ void dwc2_hsotg_core_init_disconnected(struct 
>>> dwc2_hsotg *hsotg,
>>>  * set configuration.
>>>  */
>>>  
>>> +   /* keep other bits untouched (so e.g. forced modes are not lost) */
>>> +   usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
>>> +   usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
>>> +   GUSBCFG_HNPCAP);
>>> +
>>> /* set the PLL on, remove the HNP/SRP and set the PHY */
>>> val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
>>> -   dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>>> -  (val << GUSBCFG_USBTRDTIM_SHIFT), hsotg->regs + GUSBCFG);
>>> +   usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>>> +   (val << GUSBCFG_USBTRDTIM_SHIFT);
>>> +   dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
>>>  
>>> dwc2_hsotg_init_fifo(hsotg);
>>>  
>>> @@ -3031,6 +3038,7 @@ static struct usb_ep_ops dwc2_hsotg_ep_ops = {
>>>  static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
>>>  {
>>> u32 trdtim;
>>> +   u32 usbcfg;
>>> /* unmask subset of endpoint interrupts */
>>>  
>>> dwc2_writel(DIEPMSK_TIMEOUTMSK | DIEPMSK_AHBERRMSK |
>>> @@ -3054,11 +3062,16 @@ static void dwc2_hsotg_init(struct dwc2_hsotg 
>>> *hsotg)
>>>  
>>> dwc2_hsotg_init_fifo(hsotg);
>>>  
>>> +   /* keep other bits untouched (so e.g. forced modes are not lost) */
>>> +   usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
>>> +   usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
>>> +   GUSBCFG_HNPCAP);
>>> +
>>> /* set the PLL on, remove the HNP/SRP and set the PHY */
>>> trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
>>> -   dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>>> -   (trdtim << GUSBCFG_USBTRDTIM_SHIFT),
>>> -   hsotg->regs + GUSBCFG);
>>> +   usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>>> +   (trdtim << GUSBCFG_USBTRDTIM_SHIFT);
>>> +   dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
>>>  
>>> if (using_dma(hsotg))
>>> __orr32(hsotg->regs + GAHBCFG, GAHBCFG_DMA_EN);
>>>
>>
>>
>> Acked-by: John Youn 
>> Tested-by: John Youn 
> 
> I suppose this is for v4.7 merge window ?
> 
Technically - patch was started against 4.5, as there are no conflict changes 
around the file it fits for 4.6, 4.7.


--
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: ehci and ohci reset number doubt

[Alan Stern]

On Wed, 30 Mar 2016, Lipengcheng wrote:

> Hi,
>   > The ohci-platform and ehci-platform drivers already perform their own 
> root-hub and bus resets.
>   I don't understand the sentence clearly.
> 
> Synopsis control VERSION： DesignWare Cores USB2.0 Host-AHB Controller, 
> Version 2.98a
> The synopsis has the three source clocks for EHCI Host Controller 
> implementation:
> A, System (AHB BIU) Clock
> B, PHY Clock(BUS clock)
> C, UTMI PHY Clock(per port)
> 
> A clock corresponding to a reset,

No, it doesn't.  Clocks and resets are different things.

> so the ehci control should have the
> same number reset. We are ready to put the utmi phy clock and utmi
> reset into the phy module. The ehci-platform is on the lack of a
> reset number.

I don't know why you think ehci-platform doesn't have a reset.  Just 
look near the start of the ehci-platform.c source file:

struct ehci_platform_priv {
struct clk *clks[EHCI_MAX_CLKS];
struct reset_control *rst;
...

Alan Stern

--
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 gadgets with configfs hang reboot

[Pali Rohár]

On Wednesday 30 March 2016 16:01:53 Ivaylo Dimitrov wrote:
> On 30.03.2016 16:38, Felipe Balbi wrote:
> > Hi,
> > 
> > Ivaylo Dimitrov  writes:
> >>> Ivaylo Dimitrov  writes:
> > Ivaylo Dimitrov  writes:
> >>> Ivaylo Dimitrov  writes:
>  On 16.01.2016 12:40, Ivaylo Dimitrov wrote:
> > Hi,
> > 
> > On 16.01.2016 00:48, Tony Lindgren wrote:
> >> Hi all,
> >> 
> >> Looks like there's some issue with the USB gadgets and
> >> configfs.
> >> 
> >> I'm seeing rmmod of the UDC driver cause a warning and
> >> then reboot hangs the system. This happens at least with
> >> v4.4, and I've reproduced it with dwc3 and musb so it
> >> seems to be generic.
> > 
> > Having configfs is not needed, disabling usb gadgets (#
> > CONFIG_USB_MUSB_GADGET is not set) seems to solved at least
> > poweroff hang issue on N900. Also, g_nokia is not a module
> > in the config I use, so I guess the problem is not related
> > whether gadgets are modular or not. Unfortunately I was
> > not able to test reboot, as rootfs became corrupted after
> > the first poweroff :( . So it looks like my theory that
> > onenand corruption on N900 is because poweroff/reboot
> > hangs is wrong.
> > 
> > Ivo
>  
>  Is there any progress on the issue?
> >> 
> >> Doing Nokia-N900:/sys/bus/platform/drivers/musb-hdrc# echo
> >> musb-hdrc.0.auto > unbind results in:
> >> 
> >> <1>[ 1418.511260] Unable to handle kernel paging request at
> >> virtual address 6c6c757a
> >> <7>[ 1418.677215] pvr: Xorg: cleaning up 49 unfreed resources
> >> <1>[ 1418.683349] pgd = c0004000
> >> <1>[ 1418.739959] [6c6c757a] *pgd=
> >> <0>[ 1418.746307] Internal error: Oops: 5 [#1] PREEMPT ARM
> >> <4>[ 1418.753997] Modules linked in: sha256_generic hmac drbg
> >> ansi_cprng ctr ccm vfat fat rfcomm sd_mod scsi_mod bnep
> >> bluetooth omaplfb pvrsrvkm ipv6 bq2415x_charger uinput
> >> radio_platform_si4713 joydev cmt_speech hsi_char
> >> video_bus_switch arc4 wl1251_spi wl1251 isp1704_charger
> >> gpio_keys mac80211 smc91x mii cfg80211 omap_wdt crc7
> >> omap_sham tsc2005 tsc200x_core bq27xxx_battery_i2c si4713
> >> adp1653 tsl2563 leds_lp5523 leds_lp55xx_common
> >> bq27xxx_battery rtc_twl twl4030_wdt et8ek8 ad5820 v4l2_common
> >> smiaregs twl4030_vibra videodev ff_memless lis3lv02d_i2c
> >> lis3lv02d media input_polldev omap_ssi_port ti_soc_thermal
> >> nokia_modem ssi_protocol omap_ssi hsi rx51_battery
> >> <4>[ 1418.835906] CPU: 0 PID: 53 Comm: file-storage Not
> >> tainted 4.5.0-rc5+ #59
> >> <4>[ 1418.846130] Hardware name: Nokia RX-51 board
> >> <4>[ 1418.853820] task: ceb8a300 ti: ce008000 task.ti:
> >> ce008000 <4>[ 1418.862792] PC is at
> >> handle_exception+0xa8/0x418 <4>[ 1418.871002] LR is at
> >> recalc_sigpending+0x18/0x7c <4>[ 1418.879241] pc :
> >> []lr : []psr: 8013 <4>[
> >> 1418.879241] sp : ce009ea0  ip :   fp :  <4>[
> >> 1418.898284] r10:   r9 :   r8 :  <4>[
> >> 1418.907287] r7 : c031d8d0  r6 : 6c6c7566  r5 :   r4
> >> : cebe1600 <4>[ 1418.917663] r3 : 6f642820  r2 :   r1
> >> :   r0 :  <4>[ 1418.928039] Flags: Nzcv  IRQs
> >> on  FIQs on  Mode SVC_32  ISA ARM Segment none
> >> <4>[ 1418.939025] Control: 10c5387d  Table: 8e244019  DAC:
> >> 0051 <0>[ 1418.948516] Process file-storage (pid: 53,
> >> stack limit = 0xce008210) <0>[ 1418.958679] Stack:
> >> (0xce009ea0 to 0xce00a000)
> >> <0>[ 1418.966735] 9ea0: 000f   0b07
> >>  0001 03ff 0001
> >> <0>[ 1418.978973] 9ec0: ceb8a300 ceb8a300  c004841c
> >>  0002 ce888000 c0451a50
> >> <0>[ 1418.991180] 9ee0:    0008
> >> cebe1600 0001 c0717dd0 0001
> >> <0>[ 1419.003387] 9f00:   ce009f14 c044ddf4
> >>  c031c020 0042 ce009f30
> >> <0>[ 1419.015686] 9f20: ce009f30  cebe1600 c031d958
> >>  c044d864 a013 
> >> <0>[ 1419.027923] 9f40: cebe1600 c031d8d0 cebfa100 cebfa100
> >>  cebe1600 c031d8d0 
> >> <0>[ 1419.040130] 9f60:    c00474e4
> >> dc4d900d  31bc92e7 cebe1600
> >> <0>[ 1419.052429] 9f80:  ce009f84 ce009f84 
> >> ce009f90 ce009f90 ce009fac cebfa100
> >> <0>[ 1419.064697] 9fa0: c0047418   c000f218
> >>    
> >> <0>[ 1419.076934] 9fc0:    
> >>  

Re: [PATCH 2/3] usb: dwc3: increase maximum number of TRBs per endpoint

[Bin Liu]

Hi,

On Wed, Mar 30, 2016 at 09:14:18AM +0300, Felipe Balbi wrote:
> 
> Hi again,
> 
> Felipe Balbi  writes:
> > Bin Liu  writes:
> >> On Fri, Mar 18, 2016 at 08:59:48AM +0200, Felipe Balbi wrote:
> >>> 
> >>> Hi,
> >>> 
> >>> Bin Liu  writes:
> >>> > [ text/plain ]
> >>> > Hi,
> >>> >
> >>> > On Fri, Mar 11, 2016 at 6:54 AM, Felipe Balbi
> >>> >  wrote:
> >>> >> previously we were using a maximum of 32 TRBs per
> >>> >> endpoint. With each TRB being 16 bytes long, we were
> >>> >> using 512 bytes of memory for each endpoint.
> >>> >>
> >>> >> However, SLAB/SLUB will always allocate PAGE_SIZE
> >>> >> chunks. In order to better utilize the memory we
> >>> >> allocate and to allow deeper queues for gadgets
> >>> >> which would benefit from it (g_ether comes to mind),
> >>> >> let's increase the maximum to 256 TRBs which rounds
> >>> >> up to 4096 bytes for each endpoint.
> >>> >
> >>> > Do we want to increase the same for event ring buffers as
> >>> > while, which is allocated by dma_alloc_coherent(), which
> >>> > is also at PAGE_SIZE chunks, right?
> >>> 
> >>> I could, but that's much less important. Currently we have up to 2
> >>
> >> I agree it is less important, the only issue I see is wasting of memory.
> >> The device I am working on right now has two dwc3 controllers, each
> >> allocates 16 event buffers, so for the total of 128KB allocated pages,
> >> only 8KB is really used, 120KB is wasted.
> >>
> >> Seems dma pool makes more sense in here?
> >
> > I don't know. I think the real thing is that I really need to revisit
> > that part of the code/databook. The whole "multiple interrupters" seems
> > like it's only really necessary for host side. Which means that we could
> > drop all the loops for multiple event buffers and always use a single
> > one.
> >
> > Do you wanna test the following ?
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 17fd81447c9f..ebb3ee9c06f1 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -237,8 +237,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
> > unsigned length)
> > int num;
> > int i;
> >  
> > -   num = DWC3_NUM_INT(dwc->hwparams.hwparams1);
> > -   dwc->num_event_buffers = num;
> > +   dwc->num_event_buffers = 1;
> >  
> > dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * num,
> > GFP_KERNEL);
> >
> > I'll re-read what these bits actually mean. I have a strong feeling we
> > could (should?) be ignoring them for the peripheral side.
> 
> Okay, so when we're configuring the endpoints, we could route endpoint
> interrupts to different event buffers. I really think that's really
> unimportant for us, specially since we end up using a single IRQ line.
> 
> I guess I'll just go ahead and remove that code. If it turns out we
> decide to use it, we shouldn't really be using a loop in the hardirq
> handler anyway.

Sounds good to me, I only see one evt buffer is used in all my devices,
even thought multi buffers are allocated based on hwparams1.

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: USB gadgets with configfs hang reboot

[Ivaylo Dimitrov]

On 30.03.2016 16:38, Felipe Balbi wrote:


Hi,

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

On 16.01.2016 12:40, Ivaylo Dimitrov wrote:

Hi,

On 16.01.2016 00:48, Tony Lindgren wrote:

Hi all,

Looks like there's some issue with the USB gadgets and configfs.

I'm seeing rmmod of the UDC driver cause a warning and then reboot
hangs the system. This happens at least with v4.4, and I've reproduced
it with dwc3 and musb so it seems to be generic.



Having configfs is not needed, disabling usb gadgets (#
CONFIG_USB_MUSB_GADGET is not set) seems to solved at least poweroff
hang issue on N900. Also, g_nokia is not a module in the config I use,
so I guess the problem is not related whether gadgets are modular or
not. Unfortunately I was not able to test reboot, as rootfs became
corrupted after the first poweroff :( . So it looks like my theory that
onenand corruption on N900 is because poweroff/reboot hangs is wrong.

Ivo



Is there any progress on the issue?




Doing Nokia-N900:/sys/bus/platform/drivers/musb-hdrc# echo
musb-hdrc.0.auto > unbind results in:

<1>[ 1418.511260] Unable to handle kernel paging request at virtual
address 6c6c757a
<7>[ 1418.677215] pvr: Xorg: cleaning up 49 unfreed resources
<1>[ 1418.683349] pgd = c0004000
<1>[ 1418.739959] [6c6c757a] *pgd=
<0>[ 1418.746307] Internal error: Oops: 5 [#1] PREEMPT ARM
<4>[ 1418.753997] Modules linked in: sha256_generic hmac drbg ansi_cprng
ctr ccm vfat fat rfcomm sd_mod scsi_mod bnep bluetooth omaplfb pvrsrvkm
ipv6 bq2415x_charger uinput radio_platform_si4713 joydev cmt_speech
hsi_char video_bus_switch arc4 wl1251_spi wl1251 isp1704_charger
gpio_keys mac80211 smc91x mii cfg80211 omap_wdt crc7 omap_sham tsc2005
tsc200x_core bq27xxx_battery_i2c si4713 adp1653 tsl2563 leds_lp5523
leds_lp55xx_common bq27xxx_battery rtc_twl twl4030_wdt et8ek8 ad5820
v4l2_common smiaregs twl4030_vibra videodev ff_memless lis3lv02d_i2c
lis3lv02d media input_polldev omap_ssi_port ti_soc_thermal nokia_modem
ssi_protocol omap_ssi hsi rx51_battery
<4>[ 1418.835906] CPU: 0 PID: 53 Comm: file-storage Not tainted
4.5.0-rc5+ #59
<4>[ 1418.846130] Hardware name: Nokia RX-51 board
<4>[ 1418.853820] task: ceb8a300 ti: ce008000 task.ti: ce008000
<4>[ 1418.862792] PC is at handle_exception+0xa8/0x418
<4>[ 1418.871002] LR is at recalc_sigpending+0x18/0x7c
<4>[ 1418.879241] pc : []lr : []psr: 8013
<4>[ 1418.879241] sp : ce009ea0  ip :   fp : 
<4>[ 1418.898284] r10:   r9 :   r8 : 
<4>[ 1418.907287] r7 : c031d8d0  r6 : 6c6c7566  r5 :   r4 : cebe1600
<4>[ 1418.917663] r3 : 6f642820  r2 :   r1 :   r0 : 
<4>[ 1418.928039] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment none
<4>[ 1418.939025] Control: 10c5387d  Table: 8e244019  DAC: 0051
<0>[ 1418.948516] Process file-storage (pid: 53, stack limit = 0xce008210)
<0>[ 1418.958679] Stack: (0xce009ea0 to 0xce00a000)
<0>[ 1418.966735] 9ea0: 000f   0b07 
0001 03ff 0001
<0>[ 1418.978973] 9ec0: ceb8a300 ceb8a300  c004841c 
0002 ce888000 c0451a50
<0>[ 1418.991180] 9ee0:    0008 cebe1600
0001 c0717dd0 0001
<0>[ 1419.003387] 9f00:   ce009f14 c044ddf4 
c031c020 0042 ce009f30
<0>[ 1419.015686] 9f20: ce009f30  cebe1600 c031d958 
c044d864 a013 
<0>[ 1419.027923] 9f40: cebe1600 c031d8d0 cebfa100 cebfa100 
cebe1600 c031d8d0 
<0>[ 1419.040130] 9f60:    c00474e4 dc4d900d
 31bc92e7 cebe1600
<0>[ 1419.052429] 9f80:  ce009f84 ce009f84  ce009f90
ce009f90 ce009fac cebfa100
<0>[ 1419.064697] 9fa0: c0047418   c000f218 
  
<0>[ 1419.076934] 9fc0:     
  
<0>[ 1419.089050] 9fe0:     0013
 2000 3891
<4>[ 1419.101043] [] (handle_exception) from []
(fsg_main_thread+0x88/0x13dc)
<4>[ 1419.113189] [] (fsg_main_thread) from []
(kthread+0xcc/0xe0)
<4>[ 1419.124267] [] (kthread) from []
(ret_from_fork+0x14/0x3c)
<0>[ 1419.135101] Code: 1a15 ea40 e5946038 e0866285 (e5963014)
<4>[ 1419.330841] ---[ end trace 3377457e25b0732c ]---
<0>[ 1419.340972] Kernel panic - not syncing: Fatal exception

weirdly, I have that log only in mtdoops, but not in dmesg. However,
after that oops "reboot" command does not hang, but reboots the device.



So, what is handle_exception + 0xa8 ? You can figure that out either
with gdb or addr2line assuming your vmlinux has dbg symbols.

For gdb you would:

gdb vmlinux
(gdb) l *(handle_exception + 0xa8)



Yeah, sorry I didn't do it with the previous mail.

Reading symbols from

Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

[Heiner Kallweit]

On Wed, Mar 30, 2016 at 3:03 PM, Pavel Machek  wrote:
> Hi!
>
>> >Ok, so:
>> >
>> >a) Do we want RGB leds to be handled by existing subsystem, or do we
>> >need separate layer on top of that?
>> >
>> >b) Does RGB make sense, or HSV? RGB is quite widely used in graphics,
>> >and it is what hardware implements. (But we'd need to do gamma
>> >correction).
>> >
>> >c) My hardware has "acceleration engine", LED is independend from
>> >CPU. That's rather big deal. Does yours? It seems to be quite common,
>> >at least in cellphones.
>> >
>> >Ideally, I'd like to have "triggers", but different ones. As in: if
>> >charging, do yellow " .xX" pattern. If fully charged, do green steady
>> >light. If message is waiting, do blue " x x" pattern. If none of
>> >above, do slow white blinking. (Plus priorities of events). But that's
>> >quite different from existing support...)
>>
>> Please note that HSV colour scheme allows to neatly project monochrome
>> brightness semantics on the RGB realm. I.e. you can have fixed
>> hue and saturation, and by changing the brightness component a perceived
>> colour intensity can be altered.
>
> I see HSV has some advantages. But we already have LEDs with multiple
> colors, and kernel already handles them:
>
> pavel@duo:~$ ls -1 /sys/class/leds/
> tpacpi:green:batt
> tpacpi:orange:batt
>
> This is physically 2 leds but hidden under one indicator, so you got
> "off", "green", "orange" and "green+orange".

That's a good example. As long as you can recognize green+orange as
separate lights/colors
(w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
but "multiple
LED devices".

In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
And it's not only about using a handful of discrete colors but about
displaying any arbitrary
color.
So far the kernel exposes the physical RGB LEDs as separate LEDs only
and I can't use
a trigger to e.g. set "magenta with 50% brightness".

Heiner

> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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: rework CONFIG_USB_COMMON logic

[Felipe Balbi]

Arnd Bergmann  writes:

> [ text/plain ]
> On Wednesday 30 March 2016 13:31:01 Felipe Balbi wrote:
>> Arnd Bergmann  writes:
>> > [ text/plain ]
>> > The phy-am335x driver selects 'USB_COMMON', but all other drivers
>> > use 'depends on' for that symbol, and it depends on USB || USB_GADGET
>> > itself, which causes a Kconfig warning:
>> >
>> > warning: (AM335X_PHY_USB) selects USB_COMMON which has unmet direct 
>> > dependencies (USB_SUPPORT && (USB || USB_GADGET))
>> >
>> > As suggested by Felipe Balbi, this turns the logic around, and makes
>> > 'USB_COMMON' selected by everything else that needs it, so we can
>> > remove the dependencies.
>> >
>> > Fixes: 59f042f644c5 ("usb: phy: phy-am335x: bypass first VBUS sensing for 
>> > host-only mode")
>> > Signed-off-by: Arnd Bergmann 
>> > Acked-by: Felipe Balbi 
>> > Reviewed-by: Peter Chen 
>> > ---
>> > I seem to have dropped the ball on this one after my initial submission
>> > when it wasn't clear who should merge the patch.
>> >
>> > Please apply to the USB or the USB gadget tree, whoever gets there
>> > first.
>> 
>> seems like this is one of those "can wait for next merge window" right ?
>> I'll take it through my tree. No issues.
>
> Correct, it's not urgent at all, I just want it to disappear from
> my backlog of unapplied patches.

https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next=5412212bf636f75cba8a0e3637e666b279e7067a

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 2/2] usb:dwc3: pass arch data to xhci-hcd child

[Felipe Balbi]

Grygorii Strashko  writes:

> [ text/plain ]
> On 03/30/2016 04:10 PM, Felipe Balbi wrote:
>> "Thang Q. Nguyen"  writes:
>> 
>>> [ text/plain ]
>>> From: "Thang Q. Nguyen" 
>>>
>>> The xhci-hcd child node needs to inherit archdata attribute to use
>>> dma_ops functions and attributes. This patch enables the USB DWC3
>>> driver to pass archdata attributes to its xhci-hcd child node.
>>>
>>> Changes from v2:
>>> - None
>>>
>>> Changes from v1:
>>> - None
>> 
>> changes should be between tearline and diffstat.
>> 
>
> uh. This become a real problem :(, especially with LPAE enabled.
> DMA properties need to be inherited not only here, but also in
> usb_add_gadget_udc_release(). And probably in other places
>  where devices are created manually - the worst case : device is created
> manually but doesn't belong to any bus.
>
> And DMA configuration must include dma_pfn_offset also!
> And how about iommu staff?
>
> FYI. Solution used for PCI
> c49b8fc of/pci: Add of_pci_dma_configure() to update DMA configuration
>
> Rejected: introduce dma_init_dev_from_parent() or smth. like this
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/378317

I like this very much. Meanwhile, we need something (although, $subject
is not very good).

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: rework CONFIG_USB_COMMON logic

[Arnd Bergmann]

On Wednesday 30 March 2016 13:31:01 Felipe Balbi wrote:
> Arnd Bergmann  writes:
> > [ text/plain ]
> > The phy-am335x driver selects 'USB_COMMON', but all other drivers
> > use 'depends on' for that symbol, and it depends on USB || USB_GADGET
> > itself, which causes a Kconfig warning:
> >
> > warning: (AM335X_PHY_USB) selects USB_COMMON which has unmet direct 
> > dependencies (USB_SUPPORT && (USB || USB_GADGET))
> >
> > As suggested by Felipe Balbi, this turns the logic around, and makes
> > 'USB_COMMON' selected by everything else that needs it, so we can
> > remove the dependencies.
> >
> > Fixes: 59f042f644c5 ("usb: phy: phy-am335x: bypass first VBUS sensing for 
> > host-only mode")
> > Signed-off-by: Arnd Bergmann 
> > Acked-by: Felipe Balbi 
> > Reviewed-by: Peter Chen 
> > ---
> > I seem to have dropped the ball on this one after my initial submission
> > when it wasn't clear who should merge the patch.
> >
> > Please apply to the USB or the USB gadget tree, whoever gets there
> > first.
> 
> seems like this is one of those "can wait for next merge window" right ?
> I'll take it through my tree. No issues.

Correct, it's not urgent at all, I just want it to disappear from
my backlog of unapplied patches.

Arnd
--
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 v3 2/2] usb:dwc3: pass arch data to xhci-hcd child

[Grygorii Strashko]

On 03/30/2016 04:10 PM, Felipe Balbi wrote:
> "Thang Q. Nguyen"  writes:
> 
>> [ text/plain ]
>> From: "Thang Q. Nguyen" 
>>
>> The xhci-hcd child node needs to inherit archdata attribute to use
>> dma_ops functions and attributes. This patch enables the USB DWC3
>> driver to pass archdata attributes to its xhci-hcd child node.
>>
>> Changes from v2:
>>  - None
>>
>> Changes from v1:
>>  - None
> 
> changes should be between tearline and diffstat.
> 

uh. This become a real problem :(, especially with LPAE enabled.
DMA properties need to be inherited not only here, but also in
usb_add_gadget_udc_release(). And probably in other places
 where devices are created manually - the worst case : device is created
manually but doesn't belong to any bus.

And DMA configuration must include dma_pfn_offset also!
And how about iommu staff?

FYI. Solution used for PCI
c49b8fc of/pci: Add of_pci_dma_configure() to update DMA configuration

Rejected: introduce dma_init_dev_from_parent() or smth. like this
http://permalink.gmane.org/gmane.linux.ports.arm.kernel/378317
https://lkml.org/lkml/2014/11/4/519


-- 
regards,
-grygorii
--
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 gadgets with configfs hang reboot

[Felipe Balbi]

Hi,

Ivaylo Dimitrov  writes:
>> Ivaylo Dimitrov  writes:
 Ivaylo Dimitrov  writes:
>> Ivaylo Dimitrov  writes:
>>> On 16.01.2016 12:40, Ivaylo Dimitrov wrote:
 Hi,

 On 16.01.2016 00:48, Tony Lindgren wrote:
> Hi all,
>
> Looks like there's some issue with the USB gadgets and configfs.
>
> I'm seeing rmmod of the UDC driver cause a warning and then reboot
> hangs the system. This happens at least with v4.4, and I've reproduced
> it with dwc3 and musb so it seems to be generic.
>

 Having configfs is not needed, disabling usb gadgets (#
 CONFIG_USB_MUSB_GADGET is not set) seems to solved at least poweroff
 hang issue on N900. Also, g_nokia is not a module in the config I use,
 so I guess the problem is not related whether gadgets are modular or
 not. Unfortunately I was not able to test reboot, as rootfs became
 corrupted after the first poweroff :( . So it looks like my theory that
 onenand corruption on N900 is because poweroff/reboot hangs is wrong.

 Ivo
>>>
>>>
>>> Is there any progress on the issue?
>>
>
> Doing Nokia-N900:/sys/bus/platform/drivers/musb-hdrc# echo
> musb-hdrc.0.auto > unbind results in:
>
> <1>[ 1418.511260] Unable to handle kernel paging request at virtual
> address 6c6c757a
> <7>[ 1418.677215] pvr: Xorg: cleaning up 49 unfreed resources
> <1>[ 1418.683349] pgd = c0004000
> <1>[ 1418.739959] [6c6c757a] *pgd=
> <0>[ 1418.746307] Internal error: Oops: 5 [#1] PREEMPT ARM
> <4>[ 1418.753997] Modules linked in: sha256_generic hmac drbg ansi_cprng
> ctr ccm vfat fat rfcomm sd_mod scsi_mod bnep bluetooth omaplfb pvrsrvkm
> ipv6 bq2415x_charger uinput radio_platform_si4713 joydev cmt_speech
> hsi_char video_bus_switch arc4 wl1251_spi wl1251 isp1704_charger
> gpio_keys mac80211 smc91x mii cfg80211 omap_wdt crc7 omap_sham tsc2005
> tsc200x_core bq27xxx_battery_i2c si4713 adp1653 tsl2563 leds_lp5523
> leds_lp55xx_common bq27xxx_battery rtc_twl twl4030_wdt et8ek8 ad5820
> v4l2_common smiaregs twl4030_vibra videodev ff_memless lis3lv02d_i2c
> lis3lv02d media input_polldev omap_ssi_port ti_soc_thermal nokia_modem
> ssi_protocol omap_ssi hsi rx51_battery
> <4>[ 1418.835906] CPU: 0 PID: 53 Comm: file-storage Not tainted
> 4.5.0-rc5+ #59
> <4>[ 1418.846130] Hardware name: Nokia RX-51 board
> <4>[ 1418.853820] task: ceb8a300 ti: ce008000 task.ti: ce008000
> <4>[ 1418.862792] PC is at handle_exception+0xa8/0x418
> <4>[ 1418.871002] LR is at recalc_sigpending+0x18/0x7c
> <4>[ 1418.879241] pc : []lr : []psr: 8013
> <4>[ 1418.879241] sp : ce009ea0  ip :   fp : 
> <4>[ 1418.898284] r10:   r9 :   r8 : 
> <4>[ 1418.907287] r7 : c031d8d0  r6 : 6c6c7566  r5 :   r4 : 
> cebe1600
> <4>[ 1418.917663] r3 : 6f642820  r2 :   r1 :   r0 : 
> 
> <4>[ 1418.928039] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
> Segment none
> <4>[ 1418.939025] Control: 10c5387d  Table: 8e244019  DAC: 0051
> <0>[ 1418.948516] Process file-storage (pid: 53, stack limit = 0xce008210)
> <0>[ 1418.958679] Stack: (0xce009ea0 to 0xce00a000)
> <0>[ 1418.966735] 9ea0: 000f   0b07 
> 0001 03ff 0001
> <0>[ 1418.978973] 9ec0: ceb8a300 ceb8a300  c004841c 
> 0002 ce888000 c0451a50
> <0>[ 1418.991180] 9ee0:    0008 cebe1600
> 0001 c0717dd0 0001
> <0>[ 1419.003387] 9f00:   ce009f14 c044ddf4 
> c031c020 0042 ce009f30
> <0>[ 1419.015686] 9f20: ce009f30  cebe1600 c031d958 
> c044d864 a013 
> <0>[ 1419.027923] 9f40: cebe1600 c031d8d0 cebfa100 cebfa100 
> cebe1600 c031d8d0 
> <0>[ 1419.040130] 9f60:    c00474e4 dc4d900d
>  31bc92e7 cebe1600
> <0>[ 1419.052429] 9f80:  ce009f84 ce009f84  ce009f90
> ce009f90 ce009fac cebfa100
> <0>[ 1419.064697] 9fa0: c0047418   c000f218 
>   
> <0>[ 1419.076934] 9fc0:     
>   
> <0>[ 1419.089050] 9fe0:     0013
>  2000 3891
> <4>[ 1419.101043] [] (handle_exception) from []
> (fsg_main_thread+0x88/0x13dc)
> <4>[ 1419.113189] [] (fsg_main_thread) from []
> (kthread+0xcc/0xe0)
> <4>[ 1419.124267] [] (kthread) from []
> (ret_from_fork+0x14/0x3c)
> <0>[ 1419.135101] Code: 1a15 

Re: USB gadgets with configfs hang reboot

[Ivaylo Dimitrov]

Hi,

On 30.03.2016 13:22, Felipe Balbi wrote:


Hi,

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

Ivaylo Dimitrov  writes:

On 16.01.2016 12:40, Ivaylo Dimitrov wrote:

Hi,

On 16.01.2016 00:48, Tony Lindgren wrote:

Hi all,

Looks like there's some issue with the USB gadgets and configfs.

I'm seeing rmmod of the UDC driver cause a warning and then reboot
hangs the system. This happens at least with v4.4, and I've reproduced
it with dwc3 and musb so it seems to be generic.



Having configfs is not needed, disabling usb gadgets (#
CONFIG_USB_MUSB_GADGET is not set) seems to solved at least poweroff
hang issue on N900. Also, g_nokia is not a module in the config I use,
so I guess the problem is not related whether gadgets are modular or
not. Unfortunately I was not able to test reboot, as rootfs became
corrupted after the first poweroff :( . So it looks like my theory that
onenand corruption on N900 is because poweroff/reboot hangs is wrong.

Ivo



Is there any progress on the issue?




Doing Nokia-N900:/sys/bus/platform/drivers/musb-hdrc# echo
musb-hdrc.0.auto > unbind results in:

<1>[ 1418.511260] Unable to handle kernel paging request at virtual
address 6c6c757a
<7>[ 1418.677215] pvr: Xorg: cleaning up 49 unfreed resources
<1>[ 1418.683349] pgd = c0004000
<1>[ 1418.739959] [6c6c757a] *pgd=
<0>[ 1418.746307] Internal error: Oops: 5 [#1] PREEMPT ARM
<4>[ 1418.753997] Modules linked in: sha256_generic hmac drbg ansi_cprng
ctr ccm vfat fat rfcomm sd_mod scsi_mod bnep bluetooth omaplfb pvrsrvkm
ipv6 bq2415x_charger uinput radio_platform_si4713 joydev cmt_speech
hsi_char video_bus_switch arc4 wl1251_spi wl1251 isp1704_charger
gpio_keys mac80211 smc91x mii cfg80211 omap_wdt crc7 omap_sham tsc2005
tsc200x_core bq27xxx_battery_i2c si4713 adp1653 tsl2563 leds_lp5523
leds_lp55xx_common bq27xxx_battery rtc_twl twl4030_wdt et8ek8 ad5820
v4l2_common smiaregs twl4030_vibra videodev ff_memless lis3lv02d_i2c
lis3lv02d media input_polldev omap_ssi_port ti_soc_thermal nokia_modem
ssi_protocol omap_ssi hsi rx51_battery
<4>[ 1418.835906] CPU: 0 PID: 53 Comm: file-storage Not tainted
4.5.0-rc5+ #59
<4>[ 1418.846130] Hardware name: Nokia RX-51 board
<4>[ 1418.853820] task: ceb8a300 ti: ce008000 task.ti: ce008000
<4>[ 1418.862792] PC is at handle_exception+0xa8/0x418
<4>[ 1418.871002] LR is at recalc_sigpending+0x18/0x7c
<4>[ 1418.879241] pc : []lr : []psr: 8013
<4>[ 1418.879241] sp : ce009ea0  ip :   fp : 
<4>[ 1418.898284] r10:   r9 :   r8 : 
<4>[ 1418.907287] r7 : c031d8d0  r6 : 6c6c7566  r5 :   r4 : cebe1600
<4>[ 1418.917663] r3 : 6f642820  r2 :   r1 :   r0 : 
<4>[ 1418.928039] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment none
<4>[ 1418.939025] Control: 10c5387d  Table: 8e244019  DAC: 0051
<0>[ 1418.948516] Process file-storage (pid: 53, stack limit = 0xce008210)
<0>[ 1418.958679] Stack: (0xce009ea0 to 0xce00a000)
<0>[ 1418.966735] 9ea0: 000f   0b07 
0001 03ff 0001
<0>[ 1418.978973] 9ec0: ceb8a300 ceb8a300  c004841c 
0002 ce888000 c0451a50
<0>[ 1418.991180] 9ee0:    0008 cebe1600
0001 c0717dd0 0001
<0>[ 1419.003387] 9f00:   ce009f14 c044ddf4 
c031c020 0042 ce009f30
<0>[ 1419.015686] 9f20: ce009f30  cebe1600 c031d958 
c044d864 a013 
<0>[ 1419.027923] 9f40: cebe1600 c031d8d0 cebfa100 cebfa100 
cebe1600 c031d8d0 
<0>[ 1419.040130] 9f60:    c00474e4 dc4d900d
 31bc92e7 cebe1600
<0>[ 1419.052429] 9f80:  ce009f84 ce009f84  ce009f90
ce009f90 ce009fac cebfa100
<0>[ 1419.064697] 9fa0: c0047418   c000f218 
  
<0>[ 1419.076934] 9fc0:     
  
<0>[ 1419.089050] 9fe0:     0013
 2000 3891
<4>[ 1419.101043] [] (handle_exception) from []
(fsg_main_thread+0x88/0x13dc)
<4>[ 1419.113189] [] (fsg_main_thread) from []
(kthread+0xcc/0xe0)
<4>[ 1419.124267] [] (kthread) from []
(ret_from_fork+0x14/0x3c)
<0>[ 1419.135101] Code: 1a15 ea40 e5946038 e0866285 (e5963014)
<4>[ 1419.330841] ---[ end trace 3377457e25b0732c ]---
<0>[ 1419.340972] Kernel panic - not syncing: Fatal exception

weirdly, I have that log only in mtdoops, but not in dmesg. However,
after that oops "reboot" command does not hang, but reboots the device.



So, what is handle_exception + 0xa8 ? You can figure that out either
with gdb or addr2line assuming your vmlinux has dbg symbols.

For gdb you would:

gdb vmlinux
(gdb) l *(handle_exception + 0xa8)



Yeah, sorry I didn't do it with the previous mail.

Reading symbols from

Re: [PATCH] usb: dwc2: gadget: avoid null dereference on incomplete transfer

[Felipe Balbi]

Hi,

John Keeping  writes:
> Setting up a gadget with the uac2 function results in:
>
>   Unable to handle kernel NULL pointer dereference at virtual address 0058
>   ...
>   PC is at dwc2_hsotg_irq+0x7f0/0x908
>   LR is at dwc2_hsotg_irq+0x4c/0x908
>   Backtrace:
>   [] (dwc2_hsotg_irq) from [] 
> (handle_irq_event_percpu+0x130/0x3ec)
>   [] (handle_irq_event_percpu) from [] 
> (handle_irq_event+0x48/0x6c)
>
> In all other loops we already skip endpoints that are null, so do so
> here as well.
>
> Signed-off-by: John Keeping 
> ---
>  drivers/usb/dwc2/gadget.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0abf73c..df43ec0 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2606,7 +2606,9 @@ irq_retry:
>   for (idx = 1; idx < hsotg->num_of_eps; idx++) {
>   hs_ep = hsotg->eps_in[idx];
>  
> - if (!hs_ep->isochronous || hs_ep->has_correct_parity)
> + if (!hs_ep ||
> + !hs_ep->isochronous ||
> + hs_ep->has_correct_parity)

this is fine (even though choice of where to break line is a bit odd),
but I have a question about how the rest of the code works (a bit
off-topic, sorry)

>   continue;
>  
>   epctl_reg = DIEPCTL(idx);

So, this means that the first ISO endpoint without correct parity will
be used. Isn't this a bit fragile ? What happens when you use a device
with several different interfaces using several different endpoints ?

Isn't there a register where we can check which physical endpoint
generated the IRQ ? Seems like you really wanna check what:

#define DIEPINT(_a) HSOTG_REG(0x908 + ((_a) * 0x20))

say about eps_in[idx].

> @@ -2623,7 +2625,9 @@ irq_retry:
>   for (idx = 1; idx < hsotg->num_of_eps; idx++) {
>   hs_ep = hsotg->eps_out[idx];
>  
> - if (!hs_ep->isochronous || hs_ep->has_correct_parity)
> + if (!hs_ep ||
> + !hs_ep->isochronous ||
> + hs_ep->has_correct_parity)
>   continue;
>  
>   epctl_reg = DOEPCTL(idx);

ditto for eps_out[idx] and:

#define DOEPINT(_a) HSOTG_REG(0xB08 + ((_a) * 0x20))

comments ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 2/2] usb:dwc3: pass arch data to xhci-hcd child

[Felipe Balbi]

"Thang Q. Nguyen"  writes:

> [ text/plain ]
> From: "Thang Q. Nguyen" 
>
> The xhci-hcd child node needs to inherit archdata attribute to use
> dma_ops functions and attributes. This patch enables the USB DWC3
> driver to pass archdata attributes to its xhci-hcd child node.
>
> Changes from v2:
>   - None
>
> Changes from v1:
>   - None

changes should be between tearline and diffstat.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 1/2] usb:dwc3: Enable support for 64-bit system

[Felipe Balbi]

Hi,

"Thang Q. Nguyen"  writes:
> From: "Thang Q. Nguyen" 
>
> Add 64-bit DMA operation support to the USB DWC3 driver.
> First attempt to set the coherent DMA mask for 64-bit DMA.
> If that failed, attempt again with 32-bit DMA.
>
> Changes from v2:
>   - None.
>
> Changes from v1:
>   - Remove WARN_ON if dma_mask is NULL

these changes lines should be between the tearline (---) and diffstat
below.

> Signed-off-by: Thang Q. Nguyen 
> ---
>  drivers/usb/dwc3/core.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index de5e01f..2479c24 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -831,6 +831,21 @@ static int dwc3_probe(struct platform_device *pdev)
>   dwc->mem = mem;
>   dwc->dev = dev;
>  
> + /* Try to set 64-bit DMA first */
> + if (!pdev->dev.dma_mask)
> + /* Platform did not initialize dma_mask */
> + ret = dma_coerce_mask_and_coherent(>dev,
> +DMA_BIT_MASK(64));
> + else
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
> +
> + /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
> + if (ret) {
> + ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> + }

Also, why is it so that you need this now ? glue layers are copying dma
mask from parent device and that should be set properly. This really
shouldn't be necessary in dwc3-core; it would mean that glue layer
didn't set this device up properly.

-- 
balbi


signature.asc
Description: PGP signature

RE: Getting a gadget serial device working on Atmel SAMA5D31

[Felipe Balbi]

Hi Gil,

Gil Weber  writes:
>>> To make sure, I have also performed some additional tests. The problem
>>> seems unrelated to the gadget being used: I have tried various
>>> combinations (zero, ethernet, etc.), even setting up an ACM device
>>> using configfs, and the result is the same: a host detects the device
>>> for a brief moment, but nothing after that.
>>
>>Odd. At Gil (now in Cc) has at91 working for him on his board. Gil, care
>>to confirm ?
>
> Just to confirm, ethernet gadget is working on my board with kernel 4.4...
> I haven't notice any issue since I applied the patch you gave me. 
> I have also tried 4.5.

Thanks for confirming ;-)

-- 
balbi


signature.asc
Description: PGP signature

RE: Getting a gadget serial device working on Atmel SAMA5D31

[Gil Weber]

Hi,

>> To make sure, I have also performed some additional tests. The problem
>> seems unrelated to the gadget being used: I have tried various
>> combinations (zero, ethernet, etc.), even setting up an ACM device
>> using configfs, and the result is the same: a host detects the device
>> for a brief moment, but nothing after that.
>
>Odd. At Gil (now in Cc) has at91 working for him on his board. Gil, care
>to confirm ?

Just to confirm, ethernet gadget is working on my board with kernel 4.4...
I haven't notice any issue since I applied the patch you gave me. 
I have also tried 4.5.

Regards,
Gil

--
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: gadget: f_fs: Fix EFAULT generation for async read operations

[Lars-Peter Clausen]

In the current implementation functionfs generates a EFAULT for async read
operations if the read buffer size is larger than the URB data size. Since
a application does not necessarily know how much data the host side is
going to send it typically supplies a buffer larger than the actual data,
which will then result in a EFAULT error.

This behaviour was introduced while refactoring the code to use iov_iter
interface in commit c993c39b8639 ("gadget/function/f_fs.c: use put iov_iter
into io_data"). The original code took the minimum over the URB size and
the user buffer size and then attempted to copy that many bytes using
copy_to_user(). If copy_to_user() could not copy all data a EFAULT error
was generated. Restore the original behaviour by only generating a EFAULT
error when the number of bytes copied is not the size of the URB and the
target buffer has not been fully filled.

Commit 342f39a6c8d3 ("usb: gadget: f_fs: fix check in read operation")
already fixed the same problem for the synchronous read path.

Fixes: c993c39b8639 ("gadget/function/f_fs.c: use put iov_iter into io_data")
Signed-off-by: Lars-Peter Clausen 
---
Changes since v1:
* copy_to_iter() can fail, make sure that is handled. Test this case by
  mapping part of the target buffer read-only.

Signed-off-by: Lars-Peter Clausen 
---
 drivers/usb/gadget/function/f_fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 8cfce10..3b27aa0 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -650,7 +650,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
if (io_data->read && ret > 0) {
use_mm(io_data->mm);
ret = copy_to_iter(io_data->buf, ret, _data->data);
-   if (iov_iter_count(_data->data))
+   if (ret != io_data->req->actual && 
iov_iter_count(_data->data))
ret = -EFAULT;
unuse_mm(io_data->mm);
}
-- 
2.1.4

--
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 001/001] MUSB MULTIPOINT HIGH SPEED DUAL-ROLE CONTROLLER Fix trivial typo in constant name EP_MODE_AUTOREG_NONE

[Antonio Victor Hilario]

Hi, Sergei,

You are correct.  I had only thought to submit the least number of changes to 
eliminate the problem.

A better fix is to make the leading part of the names EP_MODE_AUTOREQ 
consistent for the [mode] parameter to cppi41_set_autoreq_mode( struct 
cppi41_dma_channel *cppi41_channel, unsigned mode ), as follows:

---
--- linux-3.18.29/drivers/usb/musb/musb_cppi41.c2016-03-18 
02:09:52.0 +0800
+++ linux-3.18.1/drivers/usb/musb/musb_cppi41.c 2016-03-30 19:58:31.486965871 
+0800
@@ -9,9 +9,9 @@

#define RNDIS_REG(x) (0x80 + ((x - 1) * 4))

-#define EP_MODE_AUTOREG_NONE   0
-#define EP_MODE_AUTOREG_ALL_NEOP   1
-#define EP_MODE_AUTOREG_ALWAYS 3
+#define EP_MODE_AUTOREQ_NONE   0
+#define EP_MODE_AUTOREQ_ALL_NEOP   1
+#define EP_MODE_AUTOREQ_ALWAYS 3

#define EP_MODE_DMA_TRANSPARENT 0
#define EP_MODE_DMA_RNDIS   1
@@ -396,19 +396,19 @@ static bool cppi41_configure_channel(str

/* auto req */
cppi41_set_autoreq_mode(cppi41_channel,
-   EP_MODE_AUTOREG_ALL_NEOP);
+   EP_MODE_AUTOREQ_ALL_NEOP);
} else {
musb_writel(musb->ctrl_base,
RNDIS_REG(cppi41_channel->port_num), 0);
cppi41_set_dma_mode(cppi41_channel,
EP_MODE_DMA_TRANSPARENT);
cppi41_set_autoreq_mode(cppi41_channel,
-   EP_MODE_AUTOREG_NONE);
+   EP_MODE_AUTOREQ_NONE);
}
} else {
/* fallback mode */
cppi41_set_dma_mode(cppi41_channel, EP_MODE_DMA_TRANSPARENT);
-   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREG_NONE);
+   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
len = min_t(u32, packet_sz, len);
}
cppi41_channel->prog_len = len;
---


> On 30 Mar 2016, at 19:29, Sergei Shtylyov 
>  wrote:
> 
> Hello.
> 
> On 3/30/2016 1:56 AM, Antonio Victor Hilario wrote:
> 
>> I'd been using kernel 3.18.10-29 on a set of Beaglebone Black boards, and 
>> had found and corrected this on my local build tree.
>> 
>> Kernel build fails when the source file drivers/usb/musb/musb_cppi41.c is 
>> built, with these kernel options enabled:
>> 
>> CONFIG_USB_MUSB_HDRC=y
>> CONFIG_USB_TI_CPPI41_DMA=y
>> 
>> The build fails with these errors, due to a misspelled constant name 
>> EP_MODE_AUTOREQ_NONE:
>> 
>> drivers/usb/musb/musb_cppi41.c: In function 'cppi41_dma_channel_abort':
>> drivers/usb/musb/musb_cppi41.c:544:43: error: 'EP_MODE_AUTOREQ_NONE' 
>> undeclared (first use in this function)
>>   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
>>   ^
>> drivers/usb/musb/musb_cppi41.c:544:43: note: each undeclared identifier is 
>> reported only once for each function it appears in
>> scripts/Makefile.build:257: recipe for target 
>> 'drivers/usb/musb/musb_cppi41.o' failed
>> make[3]: *** [drivers/usb/musb/musb_cppi41.o] Error 1
>> scripts/Makefile.build:402: recipe for target 'drivers/usb/musb' failed
>> make[2]: *** [drivers/usb/musb] Error 2
>> scripts/Makefile.build:402: recipe for target 'drivers/usb' failed
>> make[1]: *** [drivers/usb] Error 2
>> Makefile:937: recipe for target 'drivers' failed
>> 
>> Signed-off-by:  Antonio VA Hilario 
>> ---
>> 
>> --- drivers/usb/musb/musb_cppi41.c   2016-03-18 02:09:52.0 +0800
>> +++ ../../linux-3.18.29/drivers/usb/musb/musb_cppi41.c   2016-02-08 
>> 13:30:43.334822382 +0800
>> @@ -541,7 +541,7 @@ static int cppi41_dma_channel_abort(stru
>>  csr &= ~MUSB_TXCSR_DMAENAB;
>>  musb_writew(epio, MUSB_TXCSR, csr);
>>  } else {
>> -cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
>> +cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREG_NONE);
> 
>  That simply doesn't make sense. Need to rename the #define, the typo is 
> there.
> 
> MBR, Sergei

--
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: Getting a gadget serial device working on Atmel SAMA5D31

[JB]

Hello,

>> To make sure, I have also performed some additional tests. The problem
>> seems unrelated to the gadget being used: I have tried various
>> combinations (zero, ethernet, etc.), even setting up an ACM device
>> using configfs, and the result is the same: a host detects the device
>> for a brief moment, but nothing after that.
>
> Odd. At Gil (now in Cc) has at91 working for him on his board. Gil, care
> to confirm ?

Since this is a SAMA5D31 device, it uses atmel_usba_udc instead of at91_udc.

And thank you, I will try your suggestions later today.

Bálint
--
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: Getting a gadget serial device working on Atmel SAMA5D31

[Felipe Balbi]

Hi,

JB  writes:
>> did you check that getty is still running ? That "deactivated" message
>> means that whatever had /dev/ttyGS0 opened, decided to close it. This
>> could mean that getty got a SIGHUP and just decided to close that fd.
>
> I have only briefly experimented with getty (I also tried opening the
> port with minicom). Would this even matter? Even if the serial port is
> not open, shouldn't the device be enumerated by its host?

yes and no ;-)

ACM might need a userspace component to implement, e.g. AT command
support. We shouldn't connect to host until that userspace component is
ready.

If you just want a serial console, then the generic serial (non-ACM) is
a better bet. Just load gserial with use_obex=0 use_acm=0:

# modprobe g_serial use_obex=0 use_acm=0

> To make sure, I have also performed some additional tests. The problem
> seems unrelated to the gadget being used: I have tried various
> combinations (zero, ethernet, etc.), even setting up an ACM device
> using configfs, and the result is the same: a host detects the device
> for a brief moment, but nothing after that.

Odd. At Gil (now in Cc) has at91 working for him on his board. Gil, care
to confirm ?

JB, I guess you need to add more debugging messages around at91
driver. Try to figure out if VBUS is dropping, or something crazy like
that. There's no reason for the gadget to just disconnect after
enumeration starting.

cheers

-- 
balbi


signature.asc
Description: PGP signature

Re: Getting a gadget serial device working on Atmel SAMA5D31

[JB]

Hello,

> did you check that getty is still running ? That "deactivated" message
> means that whatever had /dev/ttyGS0 opened, decided to close it. This
> could mean that getty got a SIGHUP and just decided to close that fd.

I have only briefly experimented with getty (I also tried opening the
port with minicom). Would this even matter? Even if the serial port is
not open, shouldn't the device be enumerated by its host?

To make sure, I have also performed some additional tests. The problem
seems unrelated to the gadget being used: I have tried various
combinations (zero, ethernet, etc.), even setting up an ACM device
using configfs, and the result is the same: a host detects the device
for a brief moment, but nothing after that.

Thanks,
Bálint
--
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 001/001] MUSB MULTIPOINT HIGH SPEED DUAL-ROLE CONTROLLER Fix trivial typo in constant name EP_MODE_AUTOREG_NONE

[Sergei Shtylyov]

Hello.

On 3/30/2016 1:56 AM, Antonio Victor Hilario wrote:


I'd been using kernel 3.18.10-29 on a set of Beaglebone Black boards, and had 
found and corrected this on my local build tree.

Kernel build fails when the source file drivers/usb/musb/musb_cppi41.c is 
built, with these kernel options enabled:

CONFIG_USB_MUSB_HDRC=y
CONFIG_USB_TI_CPPI41_DMA=y

The build fails with these errors, due to a misspelled constant name 
EP_MODE_AUTOREQ_NONE:

drivers/usb/musb/musb_cppi41.c: In function 'cppi41_dma_channel_abort':
drivers/usb/musb/musb_cppi41.c:544:43: error: 'EP_MODE_AUTOREQ_NONE' undeclared 
(first use in this function)
cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
^
drivers/usb/musb/musb_cppi41.c:544:43: note: each undeclared identifier is 
reported only once for each function it appears in
scripts/Makefile.build:257: recipe for target 'drivers/usb/musb/musb_cppi41.o' 
failed
make[3]: *** [drivers/usb/musb/musb_cppi41.o] Error 1
scripts/Makefile.build:402: recipe for target 'drivers/usb/musb' failed
make[2]: *** [drivers/usb/musb] Error 2
scripts/Makefile.build:402: recipe for target 'drivers/usb' failed
make[1]: *** [drivers/usb] Error 2
Makefile:937: recipe for target 'drivers' failed

Signed-off-by:  Antonio VA Hilario 
---

--- drivers/usb/musb/musb_cppi41.c  2016-03-18 02:09:52.0 +0800
+++ ../../linux-3.18.29/drivers/usb/musb/musb_cppi41.c  2016-02-08 
13:30:43.334822382 +0800
@@ -541,7 +541,7 @@ static int cppi41_dma_channel_abort(stru
csr &= ~MUSB_TXCSR_DMAENAB;
musb_writew(epio, MUSB_TXCSR, csr);
} else {
-   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
+   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREG_NONE);


   That simply doesn't make sense. Need to rename the #define, the typo is 
there.

MBR, Sergei

--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi,

Jun Li  writes:
>> -Original Message-
>> From: Baolin Wang [mailto:baolin.w...@linaro.org]
>> Sent: Wednesday, March 30, 2016 5:31 PM
>> To: Jun Li 
>> Cc: Peter Chen ; Felipe Balbi ;
>> Greg KH ; Sebastian Reichel ;
>> Dmitry Eremin-Solenikov ; David Woodhouse
>> ; Peter Chen ; Alan Stern
>> ; r.bald...@samsung.com; Yoshihiro Shimoda
>> ; Lee Jones ; Mark
>> Brown ; Charles Keepax
>> ; patc...@opensource.wolfsonmicro.com;
>> Linux PM list ; USB ;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>> 
>> On 30 March 2016 at 16:07, Jun Li  wrote:
>> > Hi
>> 
>> >> On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> >> >> It is not for udc driver but for power users who want to
>> >> >> >> negotiate with USB subsystem.
>> >> >> >>
>> >> >> >
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when
>> > gadget connect (pullup DP), you should already know the charger type.
>> 
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration. 
>
> I see, charger type detection is done automatically by PMIC when VBUS
> is detected in your case, you just assume the process is complete

assuming this finishes before gadget starts is a bad idea. It would've
been much more robust to delay usb_gadget_connect() until we KNOW
charger detection has completed.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: dwc3: rename __dwc3_cleanup_done_trbs to __dwc3_cleanup_one_trb

[Du, Changbin]

> > @@ -1873,7 +1873,7 @@ static void dwc3_gadget_free_endpoints(struct
> dwc3 *dwc)
> >
> >  /* 
> > -- 
> > */
> >
> > -static int __dwc3_cleanup_done_trbs(struct dwc3 *dwc, struct dwc3_ep
> *dep,
> > +static int __dwc3_cleanup_one_trb(struct dwc3 *dwc, struct dwc3_ep
> *dep,
> 
> I would rather just remove that 's' from the end so the function's name
> stays as __dwc3_cleanup_done_trb() ;-)
> 
> Care to do that ?
> 
> thanks
> 
> --
> Balbi

IMO, "one_trb" is a little more clear. But both is okay for me. :) 

Thanks, 
Du, Changbin
--
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: dwc3: rename __dwc3_cleanup_done_trbs to __dwc3_cleanup_one_trb

[Felipe Balbi]

Hi,

changbin...@intel.com writes:
> [ text/plain ]
> From: "Du, Changbin" 
>
> Actually, the function only clean one trb. So rename the function.
>
> Signed-off-by: Du, Changbin 

okay :-)

> ---
>  drivers/usb/dwc3/gadget.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 3ac170f..f84be3d 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1873,7 +1873,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
>  
>  /* 
> -- */
>  
> -static int __dwc3_cleanup_done_trbs(struct dwc3 *dwc, struct dwc3_ep *dep,
> +static int __dwc3_cleanup_one_trb(struct dwc3 *dwc, struct dwc3_ep *dep,

I would rather just remove that 's' from the end so the function's name
stays as __dwc3_cleanup_done_trb() ;-)

Care to do that ?

thanks

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Wednesday, March 30, 2016 5:31 PM
> To: Jun Li 
> Cc: Peter Chen ; Felipe Balbi ;
> Greg KH ; Sebastian Reichel ;
> Dmitry Eremin-Solenikov ; David Woodhouse
> ; Peter Chen ; Alan Stern
> ; r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 16:07, Jun Li  wrote:
> > Hi
> 
> >> On 30 March 2016 at 10:54, Jun Li  wrote:
> >> >> >> It is not for udc driver but for power users who want to
> >> >> >> negotiate with USB subsystem.
> >> >> >>
> >> >> >
> >> >> > Seems you don't want to guarantee charger type detection is done
> >> >> > before gadget connection(pullup DP), right?
> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> > state
> >> >> changes.
> >> >>
> >> >> I am not sure I get your point correctly, please correct me if I
> >> >> misunderstand you.
> >> >> We need to check the charger type at every event comes from the
> >> >> usb gadget state changes or the extcon device state changes, which
> >> >> means a new charger plugin or pullup.
> >> >>
> >> >
> >> > According to usb charger spec, my understanding is you can't do
> >> > real charger detection procedure *after* gadget _connection_(pullup
> >> > DP), also I don't
> >>
> >> Why can not? Charger detection is usually from PMIC.
> >
> > Charger detection process will impact DP/DM line state, see usb
> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >
> > "A PD is allowed to *disconnect* and repeat the charger detection
> > process multiple times while attached. The PD is required to wait for
> > a time of at least TCP_VDM_EN max between disconnecting and restarting
> > the charger detection process."
> >
> > As Peter mentioned, the charger detection should happen between VBUS
> > detection and gadget pull up DP for first plug in case. So when
> > gadget connect (pullup DP), you should already know the charger type.
> 
> Make sense. In our company's solution, charger detection can be done by
> hardware from PMIC at first, then it will not affect the DP/DM line when
> gadget starts to enumeration. 

I see, charger type detection is done automatically by PMIC when VBUS is
detected in your case, you just assume the process is complete before SW
do gadget connect. To make the framework common, you may do one time charger 
type check when vbus is on, and save it to avoid repeat charger type check.

> In the 'usb_charger_detect_type()', it
> usually get the charger type from type registers has been done by hardware
> from PMIC, which can not affect the DP/DM line.
> 
> >
> > Li Jun
> >
> >>
> >> > think it's necessary to check charger type at every event from usb
> >> gadget.
> >>
> >> My meaning is not every event from usb gadget. When the usb gadget
> >> state changes or the extcon device (maybe GPIO detection) state
> >> changes, which means charger plugin or pullup, we need to check the
> >> charger type to set current.
> >
> > From your below code, you call usb_charger_notify_others() in every
> > state change.
> 
> I think it does not matter. In case the usb charger missed some gadget
> state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
> Thanks.
> 
> >
> > if (uchger->old_gadget_state != state) {
> > uchger->old_gadget_state = state;
> >
> > if (state >= USB_STATE_ATTACHED)
> > uchger_state = USB_CHARGER_PRESENT;
> > else if (state == USB_STATE_NOTATTACHED)
> > uchger_state = USB_CHARGER_REMOVE;
> > else
> > /* this else will never happen */
> > uchger_state = USB_CHARGER_DEFAULT;
> >
> > usb_charger_notify_others(uchger, uchger_state); }
> >
> >>
> >> > Something in gadget driver you can utilize is only vbus detection,
> >> > and report diff current by diff usb state if it's a SDP.
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
> 
> 
> 
> --
> Baolin.wang
> Best Regards

Re: [PATCH] usb: phy: bcm-ns-usb3: new driver for USB 3.0 PHY on Northstar

[Felipe Balbi]

Rafał Miłecki  writes:
> [ text/plain ]
> On 30 March 2016 at 12:13, Felipe Balbi  wrote:
>> Rafał Miłecki  writes:
>>> Northstar is a family of SoCs used in home routers. They have USB 2.0
>>> and 3.0 controllers with PHYs that need to be properly initialized.
>>> This driver provides PHY init support in a generic way and can be bound
>>> with XHCI controller driver.
>>>
>>> Signed-off-by: Rafał Miłecki 
>>> ---
>>>  .../devicetree/bindings/usb/ns-usb3-phy.txt|  14 ++
>>>  drivers/usb/phy/Kconfig|   8 +
>>>  drivers/usb/phy/Makefile   |   1 +
>>>  drivers/usb/phy/phy-bcm-ns-usb3.c  | 256 
>>> +
>>
>> new drivers should be using drivers/phy/ framework instead. Sorry.
>
> How can I use generic PHY driver (/drivers/phy/) with xhci-platform
> (drivers/usb/host/xhci-plat.c)? I didn't think it's possible.

well, you can always patch xhci-plat.c, right ? ;-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: f_midi: Fixed a bug when buflen was smaller than wMaxPacketSize

[Felipe Balbi]

Michal Nazarewicz  writes:
> [ text/plain ]
> On Wed, Mar 09 2016, Felipe F. Tonello wrote:
>> buflen by default (256) is smaller than wMaxPacketSize (512) in high-speed
>> devices.
>>
>> That caused the OUT endpoint to freeze if the host send any data packet of
>> length greater than 256 bytes.
>>
>> This is an example dump of what happended on that enpoint:
>> HOST:   [DATA][Length=260][...]
>> DEVICE: [NAK]
>> HOST:   [PING]
>> DEVICE: [NAK]
>> HOST:   [PING]
>> DEVICE: [NAK]
>> ...
>> HOST:   [PING]
>> DEVICE: [NAK]
>>
>> This patch fixes this problem by setting the minimum usb_request's buffer 
>> size
>> for the OUT endpoint as its wMaxPacketSize.
>>
>> Signed-off-by: Felipe F. Tonello 
>
> Acked-by: Michal Nazarewicz 
>
> But see comment below:
>
>> ---
>>  drivers/usb/gadget/function/f_midi.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c 
>> b/drivers/usb/gadget/function/f_midi.c
>> index 8475e3dc82d4..826ba641f29d 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -366,7 +366,9 @@ static int f_midi_set_alt(struct usb_function *f, 
>> unsigned intf, unsigned alt)
>>  /* allocate a bunch of read buffers and queue them all at once. */
>>  for (i = 0; i < midi->qlen && err == 0; i++) {
>>  struct usb_request *req =
>> -midi_alloc_ep_req(midi->out_ep, midi->buflen);
>> +midi_alloc_ep_req(midi->out_ep,
>> +max_t(unsigned, midi->buflen,
>> +bulk_out_desc.wMaxPacketSize));
>
> Or, just:
>
> + midi_alloc_ep_req(midi->out_ep,
> +   bulk_out_desc.wMaxPacketSize);
>
> Packet cannot be greater than wMaxPacketSize so there is no need to
> allocate more (if buflen > wMaxPacketSize).

a USB packet, right. that's correct. But a struct usb_request can point
to whatever size buffer it wants and UDC is required to split that into
wMaxPacketSize transfers.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v10 1/9] dt-bindings: phy: Add NVIDIA Tegra XUSB pad controller binding

[Marcel Ziswiler]

On Tue, 2016-03-29 at 19:08 +0200, Thierry Reding wrote:
> On Tue, Mar 29, 2016 at 05:24:59PM +0200, Marcel Ziswiler wrote:
> > 
> > Hi Thierry
> > 
> > On Fri, 2016-03-04 at 17:19 +0100, Thierry Reding wrote:
> > > 
> > > From: Thierry Reding  > > .org
> > > > 
> > > > 
> > > The NVIDIA Tegra XUSB pad controller provides a set of pads, each
> > > with a
> > > set of lanes that are used for PCIe, SATA and USB.
> > I finally got around trying this both on Jetson TK1 as well as our
> > own
> > Toradex Apalis TK1 module we are about to mainline.
> Looking forward to it.

Yes, I'm just running some final tests.

> > 
> > I actually
> > applied
> > your patch set on top of 4.6.0-rc1. While USB 3.0 seems to work
> > fine I
> > noticed PCIe and SATA no more to come up right with the following
> > message:
> > 
> > [2.794458] tegra-pcie 1003000.pcie-controller: PLL failed to
> > lock:
> > -110
> > [2.801177] tegra-pcie 1003000.pcie-controller: failed to power
> > on
> > PHY: -110
> > [2.809031] tegra-pcie: probe of 1003000.pcie-controller failed
> > with
> > error -110
> > 
> > Do you happen to know what could be the issue?
> That's to be expected. You'll need this one:
> 
>   http://patchwork.ozlabs.org/patch/596548/
> 
> which I had hoped would make v4.6-rc1, but didn't. I'll have to
> respin
> and send out again. I didn't know that SATA failed in the same way,
> but
> I'll need to recheck and see if it needs a similar change.

Ah, I missed that one which you already sent out a v3 still applying
perfectly:

http://article.gmane.org/gmane.linux.ports.tegra/25396

With that it looks much better again:

ubuntu@tegra-ubuntu:~$ lspci
00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1)
01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)

I also found a more capable test device:

ubuntu@tegra-ubuntu:~$ sudo hdparm -t /dev/sda

/dev/sda:
 Timing buffered disk reads: 576 MB in  3.01 seconds = 191.55 MB/sec

However on Apalis TK1 this broke the second USB 3.0 host port which now
refuses to work. Still works plugging in USB 2.0 gear though.

Hm, it works if plugged in during boot. Strange. What could be the
issue if hot-plugging USB 3.0 devices on the second port does not quite
work?

I'm also wondering whether the Tegra EHCI stuff is still required as
there are now quite many USB buses showing up:

ubuntu@tegra-ubuntu:~$ lsusb
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 005 Device 002: ID 174c:07d1 ASMedia Technology Inc.
Bus 005 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 004 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

I also noticed xhci to register lots of ports:

[3.603688] tegra-xusb 7009.usb: Firmware timestamp: 2014-09-16
02:10:07 UTC
[3.611109] tegra-xusb 7009.usb: xHCI Host Controller
[3.616560] tegra-xusb 7009.usb: new USB bus registered,
assigned bus number 4
[3.626052] tegra-xusb 7009.usb: hcc params 0x0184f525 hci
version 0x100 quirks 0x00010010
[3.634738] tegra-xusb 7009.usb: irq 346, io mem 0x7009
[3.641572] hub 4-0:1.0: USB hub found
[3.645404] hub 4-0:1.0: 6 ports detected

And:

[3.650144] tegra-xusb 7009.usb: xHCI Host Controller
[3.655599] tegra-xusb 7009.usb: new USB bus registered,
assigned bus number 5
[3.663267] usb usb5: We don't know the algorithms for LPM for this
host, disabling LPM.
[3.696888] usb usb5: No SuperSpeed endpoint companion for config
1  interface 0 altsetting 0 ep 129: using minimum values
[3.715028] hub 5-0:1.0: USB hub found
[3.718819] hub 5-0:1.0: 2 ports detected

> > As for USB I do get some message about the endpoint companion but
> > do
> > not know whether or not this is to be expected:
> > 
> > [ 1021.575301] usb 4-1: new SuperSpeed USB device number 2 using
> > tegra-
> > xusb
> > [ 1021.598913] usb 4-1: No SuperSpeed endpoint companion for config
> > 1  interface 0 altsetting 0 ep 2: using minimum values
> I'm not exactly sure why that message appears, but I think it is
> harmless.

OK. Yes, at least it does not seem to have any adverse effects.

> > Otherwise it looks good:
> > 
> > ubuntu@tegra-ubuntu:~$ lsusb
> > Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > Bus 004 Device 002: ID 0951:1666 Kingston Technology DataTraveler
> > G4
> > Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> > Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > 
> > And performs satisfactorily (up from around 24 MB/sec with just USB
> > 2.0):
> > 
> > ubuntu@tegra-ubuntu:~$ sudo hdparm -t /dev/sda
> > 
> > /dev/sda:

[GIT PULL] USB fixes for v4.6-rc2

[Felipe Balbi]

Hi Greg,

here's the first set of fixes for v4.6-rc cycle. Let me know if you want
anything to be changed ;-)

The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca:

  Linux 4.6-rc1 (2016-03-26 16:03:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
tags/fixes-for-v4.6-rc2

for you to fetch changes up to 7e8ac87a44746b03a37d296fdb3e6b5d96350952:

  usb: phy: qcom-8x16: fix regulator API abuse (2016-03-30 13:34:04 +0300)


usb: fixes for v4.6-rc2

A couple fixes for the new SuperSpeedPlus that went
in during this merge window. Also two minor fixes
for dwc3 (one for setting endpoint naming correctly
and another to improve time spent in our reset
sequence) and an old bug fix on Atmel UDC which was
disabling endpoints when it shouldn't.

There's also a build breakage fix to the qcom phy
driver and a fix to the f_midi gadget to avoid a
race condition with f_midi_transmit().


Arnd Bergmann (1):
  usb: phy: qcom-8x16: fix regulator API abuse

Felipe Balbi (3):
  usb: dwc3: core: improve reset sequence
  usb: dwc3: gadget: fix endpoint renaming
  usb: gadget: udc: atmel: don't disable enpdoints we don't own

Felipe F. Tonello (1):
  usb: gadget: f_midi: added spinlock on transmit function

Jiebing Li (1):
  usb: dwc3: gadget: release spin lock during gadget resume

John Youn (2):
  usb: gadget: composite: Access SSP Dev Cap fields properly
  usb: ch9: Fix SSP Device Cap wFunctionalitySupport type

 drivers/usb/dwc3/core.c | 48 +-
 drivers/usb/dwc3/gadget.c   | 11 +++--
 drivers/usb/gadget/composite.c  |  8 ++--
 drivers/usb/gadget/function/f_midi.c|  9 +
 drivers/usb/gadget/udc/atmel_usba_udc.c | 14 ---
 drivers/usb/phy/phy-qcom-8x16-usb.c | 72 +
 include/uapi/linux/usb/ch9.h|  2 +-
 7 files changed, 52 insertions(+), 112 deletions(-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: rework CONFIG_USB_COMMON logic

[Felipe Balbi]

Arnd Bergmann  writes:
> [ text/plain ]
> The phy-am335x driver selects 'USB_COMMON', but all other drivers
> use 'depends on' for that symbol, and it depends on USB || USB_GADGET
> itself, which causes a Kconfig warning:
>
> warning: (AM335X_PHY_USB) selects USB_COMMON which has unmet direct 
> dependencies (USB_SUPPORT && (USB || USB_GADGET))
>
> As suggested by Felipe Balbi, this turns the logic around, and makes
> 'USB_COMMON' selected by everything else that needs it, so we can
> remove the dependencies.
>
> Fixes: 59f042f644c5 ("usb: phy: phy-am335x: bypass first VBUS sensing for 
> host-only mode")
> Signed-off-by: Arnd Bergmann 
> Acked-by: Felipe Balbi 
> Reviewed-by: Peter Chen 
> ---
> I seem to have dropped the ball on this one after my initial submission
> when it wasn't clear who should merge the patch.
>
> Please apply to the USB or the USB gadget tree, whoever gets there
> first.

seems like this is one of those "can wait for next merge window" right ?
I'll take it through my tree. No issues.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3] usb: phy: mxs: Add DT bindings to configure TX settings

[Felipe Balbi]

Hi,

Peter Chen  writes:
> [ text/plain ]
> On Wed, Mar 23, 2016 at 02:17:27PM -0400, Jaret Cantu wrote:
>> On 03/23/2016 01:37 PM, Jaret Cantu wrote:
>> >On 03/23/2016 12:36 AM, Peter Chen wrote:
>> >>On Mon, Mar 21, 2016 at 12:32:27PM -0400, Jaret Cantu wrote:
>> >>>The TX settings can be calibrated for particular hardware.  The
>> >>>phy is reset by Linux, so this cannot be handled by the bootloader.
>> >>>
>> >>>The TRM mentions that the maximum resistance should be used for the
>> >>>DN/DP calibration in order to pass USB certification.
>> >>>
>> >>>The values for the TX registers are poorly described in the TRM.
>> >>>The meanings of the register values were taken from another
>> >>>Freescale-provided document:
>> >>>https://community.freescale.com/message/566147#comment-566912
>> >>>
>> >>>Signed-off-by: Jaret Cantu 
>> >>>---
>> >>>v3. Added unit suffix (-ohms) to tx-cal-45-d*
>> >>>
>> >>>v2. Copying devicetree list
>> >>> Removed prettifying extra whitespace
>> >>> Removed unnecessary register rewrite on resume
>> >>> Use min and max constants for clarity
>> >>>
>> >>>  .../devicetree/bindings/phy/mxs-usb-phy.txt|   10 
>> >>>  drivers/usb/phy/phy-mxs-usb.c  |   58
>> >>>
>> >>>  2 files changed, 68 insertions(+)
>> >>>
>> >>>diff --git a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>index 379b84a..1d25b04 100644
>> >>>--- a/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>+++ b/Documentation/devicetree/bindings/phy/mxs-usb-phy.txt
>> >>>@@ -12,6 +12,16 @@ Required properties:
>> >>>  - interrupts: Should contain phy interrupt
>> >>>  - fsl,anatop: phandle for anatop register, it is only for imx6 SoC
>> >>>series
>> >>>
>> >>>+
>> >>>+if (!of_property_read_u32(np, "fsl,tx-d-cal", ) &&
>> >>>+val >= MXS_PHY_TX_D_CAL_MIN && val <= MXS_PHY_TX_D_CAL_MAX) {
>> >>>+/* scale to 4-bit value */
>> >>>+val = (MXS_PHY_TX_D_CAL_MAX - val) * 0xF
>> >>>+/ (MXS_PHY_TX_D_CAL_MAX - MXS_PHY_TX_D_CAL_MIN);
>> >>>+mxs_phy->tx_reg_mask |= GM_USBPHY_TX_D_CAL(~0);
>> >>>+mxs_phy->tx_reg_set  |= GM_USBPHY_TX_D_CAL(val);
>> >>>+}
>> >>>+
>> >>
>> >>I have tested "tx-d-cal", but it seems incorrect according to the xls you
>> >>have provided, would you please check it again or am I wrong?
>> >
>> >Gah! You're right. Some of the D_CAL values need to be rounded up to
>> >match the xls.  And even then, the value for 86 still doesn't play nice.
>> >  I was really hoping to avoid using a table for these values.
>> >
>> >The TXCALDP/DN values use a much simpler 1-to-1 scale across the 16
>> >possible register values and so are unaffected by a similar issue.  I
>> >rechecked their numbers just to be sure.
>> 
>> The solution looks to be to scale D_CAL starting from 80 instead of
>> 79.  If you look at the xls listing, the jump from 79 to 83 is the
>> only time two adjacent register values result in a change greater
>> than 2% or 3%.
>> 
>> This will result in some code ugliness as the minimum allowed
>> percentage (79), per the Freescale document, and the point at which
>> we are scaling the percentage values to register values (80) are
>> different.
>> 
>> And, as mentioned before, the values will also have to be rounded up.
>> 
>> This quick shell code confirms that these sorts of calculations
>> match up with the values in the spreadsheet:
>> 
>> for d in 119 116 114 112 109 106 103 100 97 95 93 90 88 86 83 79; do
>> echo "$d="$(( ( (119-$d) * 0xf + (119-80)/2 ) / (119-80) ));
>> d=$((d+1)); done
>> 
>> 
>> I can't find any formula which would hit all of those same
>> percentages without rounding up.
>> 
>
> Then, we had to use table for it. Besides, IC team confirms the default 
> value and the step for TXCAL45DP/DN are changed at next generation SoCs,
> so I am wondering how we describe it at binding doc.

so I take it we're not yet ready to move forward with this ?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: dwc3: rename __dwc3_cleanup_done_trbs to __dwc3_cleanup_one_trb

[changbin . du]

From: "Du, Changbin" 

Actually, the function only clean one trb. So rename the function.

Signed-off-by: Du, Changbin 
---
 drivers/usb/dwc3/gadget.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ac170f..f84be3d 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1873,7 +1873,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 
 /* -- 
*/
 
-static int __dwc3_cleanup_done_trbs(struct dwc3 *dwc, struct dwc3_ep *dep,
+static int __dwc3_cleanup_one_trb(struct dwc3 *dwc, struct dwc3_ep *dep,
struct dwc3_request *req, struct dwc3_trb *trb,
const struct dwc3_event_depevt *event, int status)
 {
@@ -1975,7 +1975,7 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, 
struct dwc3_ep *dep,
slot %= DWC3_TRB_NUM;
trb = >trb_pool[slot];
 
-   ret = __dwc3_cleanup_done_trbs(dwc, dep, req, trb,
+   ret = __dwc3_cleanup_one_trb(dwc, dep, req, trb,
event, status);
if (ret)
break;
-- 
2.5.0

--
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 gadgets with configfs hang reboot

[Felipe Balbi]

Hi,

Ivaylo Dimitrov  writes:
>> Ivaylo Dimitrov  writes:
 Ivaylo Dimitrov  writes:
> On 16.01.2016 12:40, Ivaylo Dimitrov wrote:
>> Hi,
>>
>> On 16.01.2016 00:48, Tony Lindgren wrote:
>>> Hi all,
>>>
>>> Looks like there's some issue with the USB gadgets and configfs.
>>>
>>> I'm seeing rmmod of the UDC driver cause a warning and then reboot
>>> hangs the system. This happens at least with v4.4, and I've reproduced
>>> it with dwc3 and musb so it seems to be generic.
>>>
>>
>> Having configfs is not needed, disabling usb gadgets (#
>> CONFIG_USB_MUSB_GADGET is not set) seems to solved at least poweroff
>> hang issue on N900. Also, g_nokia is not a module in the config I use,
>> so I guess the problem is not related whether gadgets are modular or
>> not. Unfortunately I was not able to test reboot, as rootfs became
>> corrupted after the first poweroff :( . So it looks like my theory that
>> onenand corruption on N900 is because poweroff/reboot hangs is wrong.
>>
>> Ivo
>
>
> Is there any progress on the issue?

>>>
>>> Doing Nokia-N900:/sys/bus/platform/drivers/musb-hdrc# echo
>>> musb-hdrc.0.auto > unbind results in:
>>>
>>> <1>[ 1418.511260] Unable to handle kernel paging request at virtual
>>> address 6c6c757a
>>> <7>[ 1418.677215] pvr: Xorg: cleaning up 49 unfreed resources
>>> <1>[ 1418.683349] pgd = c0004000
>>> <1>[ 1418.739959] [6c6c757a] *pgd=
>>> <0>[ 1418.746307] Internal error: Oops: 5 [#1] PREEMPT ARM
>>> <4>[ 1418.753997] Modules linked in: sha256_generic hmac drbg ansi_cprng
>>> ctr ccm vfat fat rfcomm sd_mod scsi_mod bnep bluetooth omaplfb pvrsrvkm
>>> ipv6 bq2415x_charger uinput radio_platform_si4713 joydev cmt_speech
>>> hsi_char video_bus_switch arc4 wl1251_spi wl1251 isp1704_charger
>>> gpio_keys mac80211 smc91x mii cfg80211 omap_wdt crc7 omap_sham tsc2005
>>> tsc200x_core bq27xxx_battery_i2c si4713 adp1653 tsl2563 leds_lp5523
>>> leds_lp55xx_common bq27xxx_battery rtc_twl twl4030_wdt et8ek8 ad5820
>>> v4l2_common smiaregs twl4030_vibra videodev ff_memless lis3lv02d_i2c
>>> lis3lv02d media input_polldev omap_ssi_port ti_soc_thermal nokia_modem
>>> ssi_protocol omap_ssi hsi rx51_battery
>>> <4>[ 1418.835906] CPU: 0 PID: 53 Comm: file-storage Not tainted
>>> 4.5.0-rc5+ #59
>>> <4>[ 1418.846130] Hardware name: Nokia RX-51 board
>>> <4>[ 1418.853820] task: ceb8a300 ti: ce008000 task.ti: ce008000
>>> <4>[ 1418.862792] PC is at handle_exception+0xa8/0x418
>>> <4>[ 1418.871002] LR is at recalc_sigpending+0x18/0x7c
>>> <4>[ 1418.879241] pc : []lr : []psr: 8013
>>> <4>[ 1418.879241] sp : ce009ea0  ip :   fp : 
>>> <4>[ 1418.898284] r10:   r9 :   r8 : 
>>> <4>[ 1418.907287] r7 : c031d8d0  r6 : 6c6c7566  r5 :   r4 : cebe1600
>>> <4>[ 1418.917663] r3 : 6f642820  r2 :   r1 :   r0 : 
>>> <4>[ 1418.928039] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
>>> Segment none
>>> <4>[ 1418.939025] Control: 10c5387d  Table: 8e244019  DAC: 0051
>>> <0>[ 1418.948516] Process file-storage (pid: 53, stack limit = 0xce008210)
>>> <0>[ 1418.958679] Stack: (0xce009ea0 to 0xce00a000)
>>> <0>[ 1418.966735] 9ea0: 000f   0b07 
>>> 0001 03ff 0001
>>> <0>[ 1418.978973] 9ec0: ceb8a300 ceb8a300  c004841c 
>>> 0002 ce888000 c0451a50
>>> <0>[ 1418.991180] 9ee0:    0008 cebe1600
>>> 0001 c0717dd0 0001
>>> <0>[ 1419.003387] 9f00:   ce009f14 c044ddf4 
>>> c031c020 0042 ce009f30
>>> <0>[ 1419.015686] 9f20: ce009f30  cebe1600 c031d958 
>>> c044d864 a013 
>>> <0>[ 1419.027923] 9f40: cebe1600 c031d8d0 cebfa100 cebfa100 
>>> cebe1600 c031d8d0 
>>> <0>[ 1419.040130] 9f60:    c00474e4 dc4d900d
>>>  31bc92e7 cebe1600
>>> <0>[ 1419.052429] 9f80:  ce009f84 ce009f84  ce009f90
>>> ce009f90 ce009fac cebfa100
>>> <0>[ 1419.064697] 9fa0: c0047418   c000f218 
>>>   
>>> <0>[ 1419.076934] 9fc0:     
>>>   
>>> <0>[ 1419.089050] 9fe0:     0013
>>>  2000 3891
>>> <4>[ 1419.101043] [] (handle_exception) from []
>>> (fsg_main_thread+0x88/0x13dc)
>>> <4>[ 1419.113189] [] (fsg_main_thread) from []
>>> (kthread+0xcc/0xe0)
>>> <4>[ 1419.124267] [] (kthread) from []
>>> (ret_from_fork+0x14/0x3c)
>>> <0>[ 1419.135101] Code: 1a15 ea40 e5946038 e0866285 (e5963014)
>>> <4>[ 1419.330841] ---[ end trace 3377457e25b0732c ]---
>>> <0>[ 1419.340972] Kernel panic - not syncing: Fatal exception
>>>
>>> weirdly, I have that log only in mtdoops, but not in dmesg. However,
>>> after that 

Re: [PATCH] usb: phy: bcm-ns-usb3: new driver for USB 3.0 PHY on Northstar

[Rafał Miłecki]

On 30 March 2016 at 12:13, Felipe Balbi  wrote:
> Rafał Miłecki  writes:
>> Northstar is a family of SoCs used in home routers. They have USB 2.0
>> and 3.0 controllers with PHYs that need to be properly initialized.
>> This driver provides PHY init support in a generic way and can be bound
>> with XHCI controller driver.
>>
>> Signed-off-by: Rafał Miłecki 
>> ---
>>  .../devicetree/bindings/usb/ns-usb3-phy.txt|  14 ++
>>  drivers/usb/phy/Kconfig|   8 +
>>  drivers/usb/phy/Makefile   |   1 +
>>  drivers/usb/phy/phy-bcm-ns-usb3.c  | 256 
>> +
>
> new drivers should be using drivers/phy/ framework instead. Sorry.

How can I use generic PHY driver (/drivers/phy/) with xhci-platform
(drivers/usb/host/xhci-plat.c)? I didn't think it's possible.

-- 
Rafał
--
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: dwc2: do not override forced dr_mode in gadget setup

[Felipe Balbi]

John Youn  writes:
> [ text/plain ]
> On 3/16/2016 3:10 PM, Przemek Rudy wrote:
>> The host/device mode set with dr_mode should be kept all the time,
>> not being changed to OTG in gadget setup (by overriding CFGUSB_FORCEDEVMODE
>> and CFGUSB_FORCEHOSTMODE bits).
>> 
>> Signed-off-by: Przemek Rudy 
>> ---
>>  drivers/usb/dwc2/gadget.c | 23 ++-
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index e9940dd..818f158 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -2254,6 +2254,7 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>  {
>>  u32 intmsk;
>>  u32 val;
>> +u32 usbcfg;
>>  
>>  /* Kill any ep0 requests as controller will be reinitialized */
>>  kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
>> @@ -2267,10 +2268,16 @@ void dwc2_hsotg_core_init_disconnected(struct 
>> dwc2_hsotg *hsotg,
>>   * set configuration.
>>   */
>>  
>> +/* keep other bits untouched (so e.g. forced modes are not lost) */
>> +usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
>> +usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
>> +GUSBCFG_HNPCAP);
>> +
>>  /* set the PLL on, remove the HNP/SRP and set the PHY */
>>  val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
>> -dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>> -   (val << GUSBCFG_USBTRDTIM_SHIFT), hsotg->regs + GUSBCFG);
>> +usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>> +(val << GUSBCFG_USBTRDTIM_SHIFT);
>> +dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
>>  
>>  dwc2_hsotg_init_fifo(hsotg);
>>  
>> @@ -3031,6 +3038,7 @@ static struct usb_ep_ops dwc2_hsotg_ep_ops = {
>>  static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
>>  {
>>  u32 trdtim;
>> +u32 usbcfg;
>>  /* unmask subset of endpoint interrupts */
>>  
>>  dwc2_writel(DIEPMSK_TIMEOUTMSK | DIEPMSK_AHBERRMSK |
>> @@ -3054,11 +3062,16 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
>>  
>>  dwc2_hsotg_init_fifo(hsotg);
>>  
>> +/* keep other bits untouched (so e.g. forced modes are not lost) */
>> +usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
>> +usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
>> +GUSBCFG_HNPCAP);
>> +
>>  /* set the PLL on, remove the HNP/SRP and set the PHY */
>>  trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
>> -dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>> -(trdtim << GUSBCFG_USBTRDTIM_SHIFT),
>> -hsotg->regs + GUSBCFG);
>> +usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
>> +(trdtim << GUSBCFG_USBTRDTIM_SHIFT);
>> +dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
>>  
>>  if (using_dma(hsotg))
>>  __orr32(hsotg->regs + GAHBCFG, GAHBCFG_DMA_EN);
>> 
>
>
> Acked-by: John Youn 
> Tested-by: John Youn 

I suppose this is for v4.7 merge window ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: phy: bcm-ns-usb3: new driver for USB 3.0 PHY on Northstar

[Felipe Balbi]

Rafał Miłecki  writes:
> Northstar is a family of SoCs used in home routers. They have USB 2.0
> and 3.0 controllers with PHYs that need to be properly initialized.
> This driver provides PHY init support in a generic way and can be bound
> with XHCI controller driver.
>
> Signed-off-by: Rafał Miłecki 
> ---
>  .../devicetree/bindings/usb/ns-usb3-phy.txt|  14 ++
>  drivers/usb/phy/Kconfig|   8 +
>  drivers/usb/phy/Makefile   |   1 +
>  drivers/usb/phy/phy-bcm-ns-usb3.c  | 256 
> +

new drivers should be using drivers/phy/ framework instead. Sorry.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> This patch introduces the usb charger driver based on usb gadget that
> makes an enhancement to a power driver. It works well in practice but
> that requires a system with suitable hardware.
>
> The basic conception of the usb charger is that, when one usb charger
> is added or removed by reporting from the usb gadget state change or
> the extcon device state change, the usb charger will report to power
> user to set the current limitation.
>
> The usb charger will register notifiees on the usb gadget or the extcon
> device to get notified the usb charger state. It also supplies the
> notification mechanism to userspace When the usb charger state is changed.
>
> Power user will register a notifiee on the usb charger to get notified
> by status changes from the usb charger. It will report to power user
> to set the current limitation when detecting the usb charger is added
> or removed from extcon device state or usb gadget state.
>
> This patch doesn't yet integrate with the gadget code, so some functions
> which rely on the 'gadget' are not completed, that will be implemented
> in the following patches.
>
> Signed-off-by: Baolin Wang 
> ---
>  drivers/usb/gadget/Kconfig  |7 +
>  drivers/usb/gadget/Makefile |1 +
>  drivers/usb/gadget/charger.c|  669 
> +++

It seems to me this should be part of udc-core's functionality. Maybe
move this to drivers/usb/gadget/udc and statically link it to
udc-core.ko ?

> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index af5d922..82a5b3c 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -133,6 +133,13 @@ config U_SERIAL_CONSOLE
>   help
>  It supports the serial gadget can be used as a console.
>  
> +config USB_CHARGER
> + bool "USB charger support"
> + help
> +   The usb charger driver based on the usb gadget that makes an
> +   enhancement to a power driver which can set the current limitation
> +   when the usb charger is added or removed.

sure you don't depend on anything ?

> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 598a67d..1e421c1 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -10,3 +10,4 @@ libcomposite-y  := usbstring.o config.o 
> epautoconf.o
>  libcomposite-y   += composite.o functions.o configfs.o 
> u_f.o
>  
>  obj-$(CONFIG_USB_GADGET) += udc/ function/ legacy/
> +obj-$(CONFIG_USB_CHARGER)+= charger.o
> diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
> new file mode 100644
> index 000..82a9973
> --- /dev/null
> +++ b/drivers/usb/gadget/charger.c
> @@ -0,0 +1,669 @@
> +/*
> + * charger.c -- USB charger driver
> + *
> + * Copyright (C) 2015 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

not very nice to depend on either of or platform_device here. What about
PCI-based devices ?

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DEFAULT_CUR_PROTECT  (50)

Where is this coming from ? Also, () are not necessary.

> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)

According to the spec we should always be talking about unit loads (1
unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
work for SS capable ports and SS gadgets (we have quite a few of them,
actually). You're missing the opportunity of charging at 900mA.

> +#define DEFAULT_DCP_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
> +#define DEFAULT_CDP_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
> +#define DEFAULT_ACA_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
> +#define UCHGER_STATE_LENGTH  (50)

what is this UCHGER_STATE_LENGTH ? And also, why don't you spell it out?
Is this weird name coming from a spec ? Which spec ?

> +static DEFINE_IDA(usb_charger_ida);
> +static struct bus_type usb_charger_subsys = {
> + .name   = "usb-charger",
> + .dev_name   = "usb-charger",
> +};
> +
> +static struct usb_charger *dev_to_uchger(struct device *udev)

nit-picking but I'd rather call this struct device *dev. Also, I'm not
sure this fits as a bus_type. There's no "usb charger" bus. There's USB
bus and its VBUS/GND lines. Why are you using a bus_type here ?

> +{
> + return container_of(udev, struct usb_charger, dev);
> +}
> +
> +static ssize_t sdp_limit_show(struct device *dev,
> +   struct device_attribute *attr,
> +   char *buf)
> +{
> + struct usb_charger *uchger = dev_to_uchger(dev);

[PATCH v6 0/3] usb: otg-fsm: Add documentation and some trivial cleanups

[Roger Quadros]

Hi,

Add documentation for struct otg_fsm with some trivial cleanups.
All patches have been Acked by otg-fsm maintainer (Peter Chen).

cheers,
-roger

Roger Quadros (3):
  usb: otg-fsm: Add documentation for struct otg_fsm
  usb: otg-fsm: support multiple instances
  usb: otg-fsm: Prevent build warning "VDBG" redefined

 drivers/usb/chipidea/otg_fsm.c   |   1 +
 drivers/usb/common/usb-otg-fsm.c |  20 +++
 drivers/usb/phy/phy-fsl-usb.c|   1 +
 include/linux/usb/otg-fsm.h  | 110 +++
 4 files changed, 100 insertions(+), 32 deletions(-)

-- 
2.5.0

--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 17:19, Peter Chen  wrote:
> On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
>> >> > - Third, since composite driver covers 500mA (and more for CDP) after 
>> >> > set
>> >> > configuration and 2mA after suspend, and vbus handler covers connect
>> >> > and disconnect. I can't see any reasons we need to notify gadget state
>> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>> >>
>> >> In some solutions, gadget does not negotiate with the current. They
>> >> just send out one signal to power driver to set the current when the
>> >> gadget state is changed (plugin or not). So we need to check the
>> >> charger state by the gadget state to notify the charger IC to set
>> >> current.
>> >>
>> >
>> > Would you give some examples?
>>
>> OK. I explain it in detail. Now charger detection can be from gadget
>> itself or PMIC, and we focus on gadget detection. Charger IC (charger
>> driver) is separate with gadget.
>> When the usb cable is plugin, we need to report the plugin event to
>> charger driver to set current after setting configuration for gadget.
>> The usb charger is responsible for reporting plugin event to charger
>> driver. But how usb charger get the plugin event? It can get the
>> plugin event from gadget state (if the gadget state is more than
>> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
>> gadget state to usb charger.
>>
>
> Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
> in your patch 2/4. Then, we need to make sure 
> usb_charger_set_cur_limit_by_type
> is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).

That's right.

>
> It seems you have not implemented usb_charger_plug_by_gadget in your patch 
> set.

It is implemented in patch 3/4.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards
--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 16:07, Jun Li  wrote:
> Hi

>> On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> >> It is not for udc driver but for power users who want to negotiate
>> >> >> with USB subsystem.
>> >> >>
>> >> >
>> >> > Seems you don't want to guarantee charger type detection is done
>> >> > before gadget connection(pullup DP), right?
>> >> > I see you call usb_charger_detect_type() in each gadget usb state
>> >> changes.
>> >>
>> >> I am not sure I get your point correctly, please correct me if I
>> >> misunderstand you.
>> >> We need to check the charger type at every event comes from the usb
>> >> gadget state changes or the extcon device state changes, which means
>> >> a new charger plugin or pullup.
>> >>
>> >
>> > According to usb charger spec, my understanding is you can't do real
>> > charger detection procedure *after* gadget _connection_(pullup DP),
>> > also I don't
>>
>> Why can not? Charger detection is usually from PMIC.
>
> Charger detection process will impact DP/DM line state, see usb charger
> spec v1.2 for detail detection process, section 4.6.3 says:
>
> "A PD is allowed to *disconnect* and repeat the charger detection process
> multiple times while attached. The PD is required to wait for a time of at
> least TCP_VDM_EN max between disconnecting and restarting the charger
> detection process."
>
> As Peter mentioned, the charger detection should happen between VBUS
> detection and gadget pull up DP for first plug in case. So when
> gadget connect (pullup DP), you should already know the charger type.

Make sense. In our company's solution, charger detection can be done
by hardware from PMIC at first, then it will not affect the DP/DM line
when gadget starts to enumeration. In the 'usb_charger_detect_type()',
it usually get the charger type from type registers has been done by
hardware from PMIC, which can not affect the DP/DM line.

>
> Li Jun
>
>>
>> > think it's necessary to check charger type at every event from usb
>> gadget.
>>
>> My meaning is not every event from usb gadget. When the usb gadget state
>> changes or the extcon device (maybe GPIO detection) state changes, which
>> means charger plugin or pullup, we need to check the charger type to set
>> current.
>
> From your below code, you call usb_charger_notify_others() in
> every state change.

I think it does not matter. In case the usb charger missed some gadget
state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
Thanks.

>
> if (uchger->old_gadget_state != state) {
> uchger->old_gadget_state = state;
>
> if (state >= USB_STATE_ATTACHED)
> uchger_state = USB_CHARGER_PRESENT;
> else if (state == USB_STATE_NOTATTACHED)
> uchger_state = USB_CHARGER_REMOVE;
> else
> /* this else will never happen */
> uchger_state = USB_CHARGER_DEFAULT;
>
> usb_charger_notify_others(uchger, uchger_state);
> }
>
>>
>> > Something in gadget driver you can utilize is only vbus detection, and
>> > report diff current by diff usb state if it's a SDP.
>>
>> --
>> Baolin.wang
>> Best Regards



-- 
Baolin.wang
Best Regards
--
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: Getting a gadget serial device working on Atmel SAMA5D31

[Felipe Balbi]

Hi,

JB  writes:
> [ text/plain ]
>> IIRC ACM will only do anything after you open /dev/ttyGS0 on the
>> peripheral side. Are you running getty on ttyGS0 ?
>
> I did try that, that would explain why it appears only once.

did you check that getty is still running ? That "deactivated" message
means that whatever had /dev/ttyGS0 opened, decided to close it. This
could mean that getty got a SIGHUP and just decided to close that fd.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
> >> > - Third, since composite driver covers 500mA (and more for CDP) after set
> >> > configuration and 2mA after suspend, and vbus handler covers connect
> >> > and disconnect. I can't see any reasons we need to notify gadget state
> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> >>
> >> In some solutions, gadget does not negotiate with the current. They
> >> just send out one signal to power driver to set the current when the
> >> gadget state is changed (plugin or not). So we need to check the
> >> charger state by the gadget state to notify the charger IC to set
> >> current.
> >>
> >
> > Would you give some examples?
> 
> OK. I explain it in detail. Now charger detection can be from gadget
> itself or PMIC, and we focus on gadget detection. Charger IC (charger
> driver) is separate with gadget.
> When the usb cable is plugin, we need to report the plugin event to
> charger driver to set current after setting configuration for gadget.
> The usb charger is responsible for reporting plugin event to charger
> driver. But how usb charger get the plugin event? It can get the
> plugin event from gadget state (if the gadget state is more than
> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
> gadget state to usb charger.
> 

Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
in your patch 2/4. Then, we need to make sure usb_charger_set_cur_limit_by_type
is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).

It seems you have not implemented usb_charger_plug_by_gadget in your patch set.

-- 
Best Regards,
Peter Chen
--
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] xhci: Detect out-of-bounds access to usb2_ports

[Christian M . Amsüss]

Out-of-bounds access was triggered on a Thinkpad Yoga during suspend or
by rmmod'ing the xhci-pci module, sometimes leading to a general
protection fault.

This is a defensive symptom fix; the error proper must occur somewhere
else, eg. when the udev with its portnum is left lingering around even
after the xhci struct has its num_usb2_ports reduced to zero, or maybe
when the udev is assigned to the wrong xhci. Anyway, this keeps nasty
crashes from happening in hard to debug situations (during suspend, not
even kdump could provide an error message).


Signed-off-by: Christian M. Amsüss 
---

On Mon, Dec 14, 2015 at 07:42:54PM +0100, chrysn wrote:
> [1.] One line summary of the problem:
> 
> general_protection when rmmod'ing xhci-pci

Some other observations around the issue: 

* rmmod'ing/modprobing xhci-pci is just a more convenient way of
  reproducing the issue; the problematic code section is being passed
  through on suspends as well (I placed debug messages there and
  observed them during suspend), I just seem never to have managed to
  get a kdump of such a situation / was always stuck with a black screen
  and dead device. The issue is more easily reproducible when the device
  is under any kind of load; a plain rmmod/modprobe loop with some sleep
  inbetween often does not trigger the issue, because while the
  out-of-bounds access still occurs, the addresses involved haven't
  changed, and there are still sufficiently valid pointers around for no
  general protection error to occur. Restarting a postgresql server in
  an endless loop in another terminal speeds up testing.

* This a symptom fix; I probably won't look deeper on my own, but would
  be happy to provide more details or test patches on the machine this
  occurs on.

 drivers/usb/host/xhci.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d51ee0c..3c1f1f3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4171,6 +4171,12 @@ int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 
port_array = xhci->usb2_ports;
port_num = udev->portnum - 1;
+   if (port_num >= xhci->num_usb2_ports) {
+   dev_warn(>dev, "%s: udev portnum exceeds num_usb2_ports, 
pretending there is no power management.\n",
+   __func__);
+   spin_unlock_irqrestore(>lock, flags);
+   return 0;
+   }
pm_addr = port_array[port_num] + PORTPMSC;
pm_val = readl(pm_addr);
hlpm_addr = port_array[port_num] + PORTHLPMC;
-- 
2.8.0.rc3


signature.asc
Description: PGP signature

[PATCH 001/001] MUSB Fix trivial typo in constant name EP_MODE_AUTOREG_NONE

[Antonio Victor Hilario]

Kernel build for an ARM target (Beaglebone Black) fails when the source file 
drivers/usb/musb/musb_cppi41.c is built, with these kernel options enabled:

CONFIG_USB_MUSB_HDRC=y
CONFIG_USB_TI_CPPI41_DMA=y

The build fails with these errors, due to a misspelled constant name 
EP_MODE_AUTOREQ_NONE:

drivers/usb/musb/musb_cppi41.c: In function 'cppi41_dma_channel_abort':
drivers/usb/musb/musb_cppi41.c:544:43: error: 'EP_MODE_AUTOREQ_NONE' undeclared 
(first use in this function)
 cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
 ^
drivers/usb/musb/musb_cppi41.c:544:43: note: each undeclared identifier is 
reported only once for each function it appears in
scripts/Makefile.build:257: recipe for target 'drivers/usb/musb/musb_cppi41.o' 
failed
make[3]: *** [drivers/usb/musb/musb_cppi41.o] Error 1
scripts/Makefile.build:402: recipe for target 'drivers/usb/musb' failed
make[2]: *** [drivers/usb/musb] Error 2
scripts/Makefile.build:402: recipe for target 'drivers/usb' failed
make[1]: *** [drivers/usb] Error 2
Makefile:937: recipe for target 'drivers' failed

Signed-off-by:  Antonio VA Hilario 
---

--- drivers/usb/musb/musb_cppi41.c  2016-03-18 02:09:52.0 +0800
+++ ../../linux-3.18.29/drivers/usb/musb/musb_cppi41.c  2016-02-08 
13:30:43.334822382 +0800
@@ -541,7 +541,7 @@ static int cppi41_dma_channel_abort(stru
csr &= ~MUSB_TXCSR_DMAENAB;
musb_writew(epio, MUSB_TXCSR, csr);
} else {
-   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREQ_NONE);
+   cppi41_set_autoreq_mode(cppi41_channel, EP_MODE_AUTOREG_NONE);

/* delay to drain to cppi dma pipeline for isoch */
udelay(250);

--
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: Getting a gadget serial device working on Atmel SAMA5D31

[JB]

> IIRC ACM will only do anything after you open /dev/ttyGS0 on the
> peripheral side. Are you running getty on ttyGS0 ?

I did try that, that would explain why it appears only once.
--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

Hi

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Wednesday, March 30, 2016 2:15 PM
> To: Jun Li 
> Cc: Peter Chen ; Felipe Balbi ;
> Greg KH ; Sebastian Reichel ;
> Dmitry Eremin-Solenikov ; David Woodhouse
> ; Peter Chen ; Alan Stern
> ; r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 10:54, Jun Li  wrote:
> >> >> It is not for udc driver but for power users who want to negotiate
> >> >> with USB subsystem.
> >> >>
> >> >
> >> > Seems you don't want to guarantee charger type detection is done
> >> > before gadget connection(pullup DP), right?
> >> > I see you call usb_charger_detect_type() in each gadget usb state
> >> changes.
> >>
> >> I am not sure I get your point correctly, please correct me if I
> >> misunderstand you.
> >> We need to check the charger type at every event comes from the usb
> >> gadget state changes or the extcon device state changes, which means
> >> a new charger plugin or pullup.
> >>
> >
> > According to usb charger spec, my understanding is you can't do real
> > charger detection procedure *after* gadget _connection_(pullup DP),
> > also I don't
> 
> Why can not? Charger detection is usually from PMIC.

Charger detection process will impact DP/DM line state, see usb charger
spec v1.2 for detail detection process, section 4.6.3 says:

"A PD is allowed to *disconnect* and repeat the charger detection process
multiple times while attached. The PD is required to wait for a time of at
least TCP_VDM_EN max between disconnecting and restarting the charger
detection process."

As Peter mentioned, the charger detection should happen between VBUS
detection and gadget pull up DP for first plug in case. So when
gadget connect (pullup DP), you should already know the charger type.

Li Jun
 
> 
> > think it's necessary to check charger type at every event from usb
> gadget.
> 
> My meaning is not every event from usb gadget. When the usb gadget state
> changes or the extcon device (maybe GPIO detection) state changes, which
> means charger plugin or pullup, we need to check the charger type to set
> current.

From your below code, you call usb_charger_notify_others() in
every state change.

if (uchger->old_gadget_state != state) {
uchger->old_gadget_state = state;

if (state >= USB_STATE_ATTACHED)
uchger_state = USB_CHARGER_PRESENT;
else if (state == USB_STATE_NOTATTACHED)
uchger_state = USB_CHARGER_REMOVE;
else
/* this else will never happen */
uchger_state = USB_CHARGER_DEFAULT;

usb_charger_notify_others(uchger, uchger_state);
}

> 
> > Something in gadget driver you can utilize is only vbus detection, and
> > report diff current by diff usb state if it's a SDP.
> 
> --
> Baolin.wang
> Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 15:42, Peter Chen  wrote:
> On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
>> On 30 March 2016 at 10:05, Peter Chen  wrote:
>> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >>
>> >> > > > I am afraid I still not find the user (udc driver) for this 
>> >> > > > framework, I would
>> >> > > > like to see how udc driver block the enumeration until the charger 
>> >> > > > detection
>> >> > > > has finished, or am I missing something?
>> >>
>> >> > > It is not for udc driver but for power users who want to negotiate
>> >> > > with USB subsystem.
>> >>
>> >> > Then, where is the code the test user to decide what kinds of USB 
>> >> > charger
>> >> > (SDP, CDP, DCP) is connecting now?
>> >>
>> >> Even without detection of CDP and DCP we have configurability within SDP
>> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> >> higher 500mA limit which can be negotiated.
>> >
>> > Well, things may be a little complicated.
>>
>> I try to address your comments, maybe Mark need to do some complements. 
>> Thanks.
>>
>> >
>> > - First, how to design get_charger_type for each udc driver?
>> > Since the charger detection process affects dp/dm signal, it can't be done
>> > during the enumeration or after that. So, the detection process can be only
>> > done after vbus has detected and before udc pull up dp.
>> >
>> > Then, when the get_charger_type do real charger detection? My suggestion 
>> > is,
>> > if the charger type is unknown, we do real one, else, we just return the
>> > stored type value.
>>
>> I think that is why we let udc driver to implement the
>> 'gadget->ops->get_charger_type()' callback, which can be controlled by
>> udc driver.
>>
>> >
>> > - Second, When to notify charger IC to charger:
>> > For SDP and CDP, it needs to notify charger IC after set configuration
>> > has finished.
>> > For DCP (and ACA, I am not sure), can notify charger IC after charger
>> > detection has finished or later.
>> >
>> > So, when we get charger is present, we need to notify charger IC at once
>> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
>> > composite core to notify charger IC after set configuration has finished,
>> > like the patch 2/4 does.
>>
>> But mostly charger IC is separate with gadget, so the charger IC
>> doesn't need to worry about the gadget status.
>>
>
> The reason why we need to combine charger IC with USB gadget (charger)
> driver is the charger IC should not charge as much as it wants, it
> needs to consider USB charger style and USB connection
> (un-configuration vs configuration) situation.

Yes, I agree with the charger IC should not charge as much as it
wants, but we don't want to combine two separated things. Like you
said, udc driver can implement 'get_charger_type()' callback to check
the charger type at the right time, then set the current after
checking the charger type (now the gadget has been set configuration).

>
>> >
>> > - Third, since composite driver covers 500mA (and more for CDP) after set
>> > configuration and 2mA after suspend, and vbus handler covers connect
>> > and disconnect. I can't see any reasons we need to notify gadget state
>> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>>
>> In some solutions, gadget does not negotiate with the current. They
>> just send out one signal to power driver to set the current when the
>> gadget state is changed (plugin or not). So we need to check the
>> charger state by the gadget state to notify the charger IC to set
>> current.
>>
>
> Would you give some examples?

OK. I explain it in detail. Now charger detection can be from gadget
itself or PMIC, and we focus on gadget detection. Charger IC (charger
driver) is separate with gadget.
When the usb cable is plugin, we need to report the plugin event to
charger driver to set current after setting configuration for gadget.
The usb charger is responsible for reporting plugin event to charger
driver. But how usb charger get the plugin event? It can get the
plugin event from gadget state (if the gadget state is more than
'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
gadget state to usb charger.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards
--
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: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[Peter Chen]

 
> 
> On 16-03-29 00:24:46, Peter Chen wrote:
> >
> > >
> > > On 2016-03-25 00:40, Peter Chen wrote:
> > > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > > >> Hello Peter,
> > > >>
> > > >> The existing usage of extcon in Chipidea driver relies on OTG
> > > >> registers. In case of SoC with dual role device but not a true
> > > >> OTG controller, this does not work. Such SoC's should specify the
> > > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > > >> without checking any of the OTG registers in my opinion.
> > > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > > >> have a true 5 pin OTG implemented.
> > > >
> > > > Sorry to reply you late due to my new born baby.
> > > >
> > > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > > When working at device mode, can you read vbus status through OTGSC?
> > > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not
> > > > the board.
> > >
> > > I think the IP is actually OTG capable, the registers are there, but
> > > the signals seem not to be available on the SoC package. That is also what
> the RM says...
> > >
> > > Quotes from the RM:
> > >
> > > "OTG controller should be treated as Dual role controller that
> > > allows the controller to act as either a Host or a device with no
> > > support for HNP/SRP."
> > >
> > > And later, in Chapter 11.1:
> > >
> > > "The USB is not a true OTG. It can be configured by software to
> > > function either as peripheral or as host. The ID pin, which is
> > > unique for OTG operation, is not present in this implementation.
> > > There are no five pin interface. The user will get four pin host/ device
> interface."
> > >
> >
> > Get it, thanks. I am doing a patch for covering this case and vbus always-on
> case.
> 
> If I may ask at this point, how would your implementation be covering this 
> case
> for Vybrid?
> 

Yes, if the vbus and id status are from the extcon, it will not read register 
OTGSC.

Peter

Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

[Jacek Anaszewski]

On 03/29/2016 11:43 PM, Pavel Machek wrote:

Hi!


First, please Cc me on RGB color support.


Add generic support for RGB Color LED's.

Basic idea is to use enum led_brightness also for the hue and saturation
color components.This allows to implement the color extension w/o
changes to struct led_classdev.

Select LEDS_RGB to enable building drivers using the RGB extension.

Flag LED_SET_HUE_SAT allows to specify that hue / saturation
should be overridden even if the provided values are zero.

Some examples for writing values to /sys/class/leds//brightness:
(now also hex notation can be used)

255 -> set full brightness and keep existing color if set
0 -> switch LED off but keep existing color so that it can be restored
  if the LED is switched on again later
0x100 -> switch LED off and set also hue and saturation to 0
0x00 -> set full brightness, full saturation and set hue to 0
(red)


Umm, that's rather strange interface -- and three values in single sysfs
file is actually forbidden.

Plus, it is very much unlike existing interfaces for RGB LEDs, which
we already have supported in the tree. (At least nokia N900 and Sony
motion controller already contain supported three-color LEDs).

Now... yes, there's work to be done for the 3-color LEDs. Currently,
they are treated as three different LEDs. (Which makes some sense, you
can use "battery charging" trigger for LED, and CPU activity trigger
for green, for example). It would be good to have some kind of
grouping, so that userspace can tell "these 3 leds are actually
combined into one light".


At first thanks for the review comments.
Treating the three physical LEDs of a RGB LED as separate LED devices
might have been implemented due to the lack of alternatives.


To be fair... they _are_ separate LED devices. In N900 case, you can
even see light comming from slightly different places if you look closely.


With one trigger controlling the red LED and another controlling the green
LED we may end up with a yellow light. Not sure whether this is what
we want.


Well, it should be understandable for most people.


One driver for this extension was the idea of triggers using color
to visualize states etc.
Therefore it's not only about userspace controlling the color.
As a trigger is bound to a led_classdev we need a led_classdev
representing a RGB LED device.

And ok: If required the sysfs interface can be splitted into separate
attributes for hue, saturation, and (existing) brightness.


Required.

Ok, so:

a) Do we want RGB leds to be handled by existing subsystem, or do we
need separate layer on top of that?

b) Does RGB make sense, or HSV? RGB is quite widely used in graphics,
and it is what hardware implements. (But we'd need to do gamma
correction).

c) My hardware has "acceleration engine", LED is independend from
CPU. That's rather big deal. Does yours? It seems to be quite common,
at least in cellphones.

Ideally, I'd like to have "triggers", but different ones. As in: if
charging, do yellow " .xX" pattern. If fully charged, do green steady
light. If message is waiting, do blue " x x" pattern. If none of
above, do slow white blinking. (Plus priorities of events). But that's
quite different from existing support...)


Please note that HSV colour scheme allows to neatly project monochrome
brightness semantics on the RGB realm. I.e. you can have fixed
hue and saturation, and by changing the brightness component a perceived
colour intensity can be altered.

--
Best regards,
Jacek Anaszewski
--
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 v5 4/4] leds: core: add support for RGB LED's

[Jacek Anaszewski]

On 03/29/2016 10:42 PM, Heiner Kallweit wrote:

Am 29.03.2016 um 12:05 schrieb Pavel Machek:

On Tue 2016-03-01 22:36:12, Heiner Kallweit wrote:

Export a function to convert HSV color values to RGB.
It's intended to be called by drivers for RGB LEDs.

Signed-off-by: Heiner Kallweit 
---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set
   This is needed in cases when we have monochrome and color LEDs
   as well in a system.
v4:
- Export led_hsv_to_rgb and let the device driver call it instead
   of doing the conversion in the core
v5:
- don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
   is set but warn
---
  drivers/leds/led-class.c|  7 +++
  drivers/leds/led-rgb-core.c | 36 
  include/linux/leds.h|  8 
  3 files changed, 51 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 8a3748a..a4b144e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct 
led_classdev *led_cdev)
char name[64];
int ret;

+   /*
+* for now reading back the color is not supported as multiple
+* HSV -> RGB -> HSV conversions may distort the color due to
+* rounding issues in the conversion algorithm
+*/
+   WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
+


Backtrace is useless here, you may want to add some ()s and you don't
really want user to be causing messages in syslog this easily.


I agree that the backtrace doesn't provide a benefit here.


I requested it to provide a verbose notification for a LED RGB class
driver developer, in case they try to implement a brightness_get
op, which would be harmful in case of HSV interface for RGB LEDs.


However I don't see how a user could create syslog entries.
The warn condition can be true only for drivers implementing the RGB extension
in a not supported way (by setting flag LED_DEV_CAP_RGB and implementing
brightness_get).

Pavel









--
Best regards,
Jacek Anaszewski
--
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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Jacek Anaszewski]

Hi Heiner and Pavel,

On 03/29/2016 10:38 PM, Heiner Kallweit wrote:

Am 29.03.2016 um 12:02 schrieb Pavel Machek:

Hi!

First, please Cc me on RGB color support.


Add generic support for RGB Color LED's.

Basic idea is to use enum led_brightness also for the hue and saturation
color components.This allows to implement the color extension w/o
changes to struct led_classdev.

Select LEDS_RGB to enable building drivers using the RGB extension.

Flag LED_SET_HUE_SAT allows to specify that hue / saturation
should be overridden even if the provided values are zero.

Some examples for writing values to /sys/class/leds//brightness:
(now also hex notation can be used)

255 -> set full brightness and keep existing color if set
0 -> switch LED off but keep existing color so that it can be restored
  if the LED is switched on again later
0x100 -> switch LED off and set also hue and saturation to 0
0x00 -> set full brightness, full saturation and set hue to 0
(red)


Umm, that's rather strange interface -- and three values in single sysfs
file is actually forbidden.

Plus, it is very much unlike existing interfaces for RGB LEDs, which
we already have supported in the tree. (At least nokia N900 and Sony
motion controller already contain supported three-color LEDs).

Now... yes, there's work to be done for the 3-color LEDs. Currently,
they are treated as three different LEDs. (Which makes some sense, you
can use "battery charging" trigger for LED, and CPU activity trigger
for green, for example). It would be good to have some kind of
grouping, so that userspace can tell "these 3 leds are actually
combined into one light".


At first thanks for the review comments.
Treating the three physical LEDs of a RGB LED as separate LED devices
might have been implemented due to the lack of alternatives.
With one trigger controlling the red LED and another controlling the green
LED we may end up with a yellow light. Not sure whether this is what we want.

One driver for this extension was the idea of triggers using color
to visualize states etc.
Therefore it's not only about userspace controlling the color.
As a trigger is bound to a led_classdev we need a led_classdev
representing a RGB LED device.

And ok: If required the sysfs interface can be splitted into separate
attributes for hue, saturation, and (existing) brightness.


It would have the same downsides as in case of having r, g and b in
separate attributes, i.e. - problems with setting LED colour in
a consistent way. This way LED blinking in whatever colour couldn't
be supported reliably. It was one of your primary rationale standing
behind this design, if I remember correctly. Second - what about
triggers? We've had a long discussion about it and this design turned
out to be most fitting.

It's hard to address these requirements by having the settings in
separate attributes, due to synchronization issues, and LED trigger
mechanism specificity.

There is a question whether we can bend the sysfs "one value per sysfs
file" rule down to RGB LEDs needs.

Of course other brilliant ideas on how to approach the problem are
more than expected.

--
Best regards,
Jacek Anaszewski
--
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 v2] USB: whci-hcd: add more checks for dma mapping error

[Vladimir Zapolskiy]

Hi Alexey,

On 26.03.2016 21:42, Alexey Khoroshilov wrote:
> Fixing checks for dma mapping error in qset_fill_page_list(),
> I have missed two similar problems in qset_add_urb_sg() and
> in qset_add_urb_sg_linearize().
> 
> v2: check validity of dma_addr with dma_mapping_error()
> in qset_free_std() as suggested by Vladimir Zapolskiy.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Alexey Khoroshilov 

Reviewed-by: Vladimir Zapolskiy 

> ---
>  drivers/usb/host/whci/qset.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
> index 1a8e960d073b..c0e6812426b3 100644
> --- a/drivers/usb/host/whci/qset.c
> +++ b/drivers/usb/host/whci/qset.c
> @@ -314,7 +314,7 @@ void qset_free_std(struct whc *whc, struct whc_std *std)
>   kfree(std->bounce_buf);
>   }
>   if (std->pl_virt) {
> - if (std->dma_addr)
> + if (!dma_mapping_error(whc->wusbhc.dev, std->dma_addr))
>   dma_unmap_single(whc->wusbhc.dev, std->dma_addr,
>std->num_pointers * sizeof(struct 
> whc_page_list_entry),
>DMA_TO_DEVICE);
> @@ -535,9 +535,11 @@ static int qset_add_urb_sg(struct whc *whc, struct 
> whc_qset *qset, struct urb *u
>   list_for_each_entry(std, >stds, list_node) {
>   if (std->ntds_remaining == -1) {
>   pl_len = std->num_pointers * sizeof(struct 
> whc_page_list_entry);
> - std->ntds_remaining = ntds--;
>   std->dma_addr = dma_map_single(whc->wusbhc.dev, 
> std->pl_virt,
>  pl_len, DMA_TO_DEVICE);
> + if (dma_mapping_error(whc->wusbhc.dev, std->dma_addr))
> + return -EFAULT;
> + std->ntds_remaining = ntds--;
>   }
>   }
>   return 0;
> @@ -618,6 +620,8 @@ static int qset_add_urb_sg_linearize(struct whc *whc, 
> struct whc_qset *qset,
>  
>   std->dma_addr = dma_map_single(>umc->dev, std->bounce_buf, 
> std->len,
>  is_out ? DMA_TO_DEVICE : 
> DMA_FROM_DEVICE);
> + if (dma_mapping_error(>umc->dev, std->dma_addr))
> + return -EFAULT;

>From whc_probe() looks like >umc->dev is the same as whc->wusbhc.dev,
so the change is correct, but I would suggest to unify the pointer to a device.

Still the driver has many problems, e.g. double kfree() -- error path
in qset_fill_page_list() and qset_free_stds() etc.

>   if (qset_fill_page_list(whc, std, mem_flags) < 0)
>   return -ENOMEM;
> 

--
With best wishes,
Vladimir
--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
> On 30 March 2016 at 10:05, Peter Chen  wrote:
> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >>
> >> > > > I am afraid I still not find the user (udc driver) for this 
> >> > > > framework, I would
> >> > > > like to see how udc driver block the enumeration until the charger 
> >> > > > detection
> >> > > > has finished, or am I missing something?
> >>
> >> > > It is not for udc driver but for power users who want to negotiate
> >> > > with USB subsystem.
> >>
> >> > Then, where is the code the test user to decide what kinds of USB charger
> >> > (SDP, CDP, DCP) is connecting now?
> >>
> >> Even without detection of CDP and DCP we have configurability within SDP
> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
> >> higher 500mA limit which can be negotiated.
> >
> > Well, things may be a little complicated.
> 
> I try to address your comments, maybe Mark need to do some complements. 
> Thanks.
> 
> >
> > - First, how to design get_charger_type for each udc driver?
> > Since the charger detection process affects dp/dm signal, it can't be done
> > during the enumeration or after that. So, the detection process can be only
> > done after vbus has detected and before udc pull up dp.
> >
> > Then, when the get_charger_type do real charger detection? My suggestion is,
> > if the charger type is unknown, we do real one, else, we just return the
> > stored type value.
> 
> I think that is why we let udc driver to implement the
> 'gadget->ops->get_charger_type()' callback, which can be controlled by
> udc driver.
> 
> >
> > - Second, When to notify charger IC to charger:
> > For SDP and CDP, it needs to notify charger IC after set configuration
> > has finished.
> > For DCP (and ACA, I am not sure), can notify charger IC after charger
> > detection has finished or later.
> >
> > So, when we get charger is present, we need to notify charger IC at once
> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> > composite core to notify charger IC after set configuration has finished,
> > like the patch 2/4 does.
> 
> But mostly charger IC is separate with gadget, so the charger IC
> doesn't need to worry about the gadget status.
> 

The reason why we need to combine charger IC with USB gadget (charger)
driver is the charger IC should not charge as much as it wants, it
needs to consider USB charger style and USB connection
(un-configuration vs configuration) situation.

> >
> > - Third, since composite driver covers 500mA (and more for CDP) after set
> > configuration and 2mA after suspend, and vbus handler covers connect
> > and disconnect. I can't see any reasons we need to notify gadget state
> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> 
> In some solutions, gadget does not negotiate with the current. They
> just send out one signal to power driver to set the current when the
> gadget state is changed (plugin or not). So we need to check the
> charger state by the gadget state to notify the charger IC to set
> current.
> 

Would you give some examples?

-- 
Best Regards,
Peter Chen
--
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 point of sale keyboard shows err 19 of hub has too many ports

[Oliver Neukum]

On Tue, 2016-03-29 at 17:58 -0700, Greg KH wrote:
> On Tue, Mar 29, 2016 at 05:42:50PM +0100, reggie macdonald wrote:
> > Hi I made a bug report on bugzilla and was told to forward it to this email.
> > 
> > Please see this thread: https://bbs.archlinux.org/viewtopic.php?id=210446
> > 
> > for current information related to the problem.

And your logs show the problem:

Hub Descriptor:
  bLength  34
  bDescriptorType   3
  nNbrPorts85

I doubt it having so many ports.

> > 
> > To summarise:
> >  
> > Works in windows and also bios/arch options menu but stops working when 
> > system loads with error message of hub has too many ports.  
> >   --
> 
> If your device says it has too many ports, there's not much we can do
> about it in Linux, right?  I suggest buying a new keyboard, it's going
> to be much cheaper in the end.
> 
> good luck!

Your hub's firmware is broken. How many physical ports does it have?

Regards
Oliver


--
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: dwc3: remove num_event_buffers

[Felipe Balbi]

We never, ever route any of the other event buffers
so we might as well drop support for them.

Until someone has a real, proper benefit for
multiple event buffers, we will rely on a single
one. This also helps reduce memory footprint of
dwc3.ko which won't allocate memory for the extra
event buffers.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.c   | 81 +++
 drivers/usb/dwc3/core.h   |  2 --
 drivers/usb/dwc3/gadget.c | 38 +++---
 3 files changed, 44 insertions(+), 77 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 67d183a18baa..9e5c57c7943d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -203,13 +203,10 @@ static struct dwc3_event_buffer 
*dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
 static void dwc3_free_event_buffers(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
-   int i;
 
-   for (i = 0; i < dwc->num_event_buffers; i++) {
-   evt = dwc->ev_buffs[i];
-   if (evt)
-   dwc3_free_one_event_buffer(dwc, evt);
-   }
+   evt = dwc->ev_buffs[0];
+   if (evt)
+   dwc3_free_one_event_buffer(dwc, evt);
 }
 
 /**
@@ -222,27 +219,19 @@ static void dwc3_free_event_buffers(struct dwc3 *dwc)
  */
 static int dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned length)
 {
-   int num;
-   int i;
-
-   num = DWC3_NUM_INT(dwc->hwparams.hwparams1);
-   dwc->num_event_buffers = num;
+   struct dwc3_event_buffer *evt;
 
-   dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * num,
+   dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs),
GFP_KERNEL);
if (!dwc->ev_buffs)
return -ENOMEM;
 
-   for (i = 0; i < num; i++) {
-   struct dwc3_event_buffer*evt;
-
-   evt = dwc3_alloc_one_event_buffer(dwc, length);
-   if (IS_ERR(evt)) {
-   dev_err(dwc->dev, "can't allocate event buffer\n");
-   return PTR_ERR(evt);
-   }
-   dwc->ev_buffs[i] = evt;
+   evt = dwc3_alloc_one_event_buffer(dwc, length);
+   if (IS_ERR(evt)) {
+   dev_err(dwc->dev, "can't allocate event buffer\n");
+   return PTR_ERR(evt);
}
+   dwc->ev_buffs[0] = evt;
 
return 0;
 }
@@ -256,25 +245,22 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
unsigned length)
 static int dwc3_event_buffers_setup(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
-   int n;
 
-   for (n = 0; n < dwc->num_event_buffers; n++) {
-   evt = dwc->ev_buffs[n];
-   dwc3_trace(trace_dwc3_core,
-   "Event buf %p dma %08llx length %d\n",
-   evt->buf, (unsigned long long) evt->dma,
-   evt->length);
-
-   evt->lpos = 0;
-
-   dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(n),
-   lower_32_bits(evt->dma));
-   dwc3_writel(dwc->regs, DWC3_GEVNTADRHI(n),
-   upper_32_bits(evt->dma));
-   dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(n),
-   DWC3_GEVNTSIZ_SIZE(evt->length));
-   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(n), 0);
-   }
+   evt = dwc->ev_buffs[0];
+   dwc3_trace(trace_dwc3_core,
+   "Event buf %p dma %08llx length %d\n",
+   evt->buf, (unsigned long long) evt->dma,
+   evt->length);
+
+   evt->lpos = 0;
+
+   dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(0),
+   lower_32_bits(evt->dma));
+   dwc3_writel(dwc->regs, DWC3_GEVNTADRHI(0),
+   upper_32_bits(evt->dma));
+   dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0),
+   DWC3_GEVNTSIZ_SIZE(evt->length));
+   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), 0);
 
return 0;
 }
@@ -282,19 +268,16 @@ static int dwc3_event_buffers_setup(struct dwc3 *dwc)
 static void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
-   int n;
 
-   for (n = 0; n < dwc->num_event_buffers; n++) {
-   evt = dwc->ev_buffs[n];
+   evt = dwc->ev_buffs[0];
 
-   evt->lpos = 0;
+   evt->lpos = 0;
 
-   dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(n), 0);
-   dwc3_writel(dwc->regs, DWC3_GEVNTADRHI(n), 0);
-   dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(n), DWC3_GEVNTSIZ_INTMASK
-   | DWC3_GEVNTSIZ_SIZE(0));
-   dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(n), 0);
-   }
+   dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(0), 

[PATCH 2/3] usb: dwc3: drop ev_buffs array

[Felipe Balbi]

we will be using a single event buffer and that
renders ev_buffs array unnecessary. Let's remove it
in favor of a single pointer to a single event
buffer.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/core.c   | 13 -
 drivers/usb/dwc3/core.h   |  2 +-
 drivers/usb/dwc3/gadget.c |  4 ++--
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9e5c57c7943d..05b7ec30266f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -204,7 +204,7 @@ static void dwc3_free_event_buffers(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
 
-   evt = dwc->ev_buffs[0];
+   evt = dwc->ev_buf;
if (evt)
dwc3_free_one_event_buffer(dwc, evt);
 }
@@ -221,17 +221,12 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
unsigned length)
 {
struct dwc3_event_buffer *evt;
 
-   dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs),
-   GFP_KERNEL);
-   if (!dwc->ev_buffs)
-   return -ENOMEM;
-
evt = dwc3_alloc_one_event_buffer(dwc, length);
if (IS_ERR(evt)) {
dev_err(dwc->dev, "can't allocate event buffer\n");
return PTR_ERR(evt);
}
-   dwc->ev_buffs[0] = evt;
+   dwc->ev_buf = evt;
 
return 0;
 }
@@ -246,7 +241,7 @@ static int dwc3_event_buffers_setup(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
 
-   evt = dwc->ev_buffs[0];
+   evt = dwc->ev_buf;
dwc3_trace(trace_dwc3_core,
"Event buf %p dma %08llx length %d\n",
evt->buf, (unsigned long long) evt->dma,
@@ -269,7 +264,7 @@ static void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
 {
struct dwc3_event_buffer*evt;
 
-   evt = dwc->ev_buffs[0];
+   evt = dwc->ev_buf;
 
evt->lpos = 0;
 
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index be03999e2dfd..df72234a805b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -753,7 +753,7 @@ struct dwc3 {
struct platform_device  *xhci;
struct resource xhci_resources[DWC3_XHCI_RESOURCES_NUM];
 
-   struct dwc3_event_buffer **ev_buffs;
+   struct dwc3_event_buffer *ev_buf;
struct dwc3_ep  *eps[DWC3_ENDPOINTS_NUM];
 
struct usb_gadget   gadget;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 5e6a4956655d..96dfde011c76 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2609,7 +2609,7 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3 
*dwc)
int left;
u32 reg;
 
-   evt = dwc->ev_buffs[0];
+   evt = dwc->ev_buf;
left = evt->count;
 
if (!(evt->flags & DWC3_EVENT_PENDING))
@@ -2668,7 +2668,7 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3 *dwc)
u32 count;
u32 reg;
 
-   evt = dwc->ev_buffs[0];
+   evt = dwc->ev_buf;
 
count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
count &= DWC3_GEVNTCOUNT_MASK;
-- 
2.8.0.rc2

--
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: dwc3: gadget: pass ev_buff as cookie to irq handler

[Felipe Balbi]

we don't plan on using multiple event buffers, but
if we find a good use case for it, this little trick
will help us avoid a loop in hardirq handler looping
for each and every event buffer.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/gadget.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 96dfde011c76..ba16e1dd32b0 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1536,7 +1536,7 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 
irq = platform_get_irq(to_platform_device(dwc->dev), 0);
ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
-   IRQF_SHARED, "dwc3", dwc);
+   IRQF_SHARED, "dwc3", dwc->ev_buf);
if (ret) {
dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
irq, ret);
@@ -1636,7 +1636,7 @@ err2:
 err1:
spin_unlock_irqrestore(>lock, flags);
 
-   free_irq(irq, dwc);
+   free_irq(irq, dwc->ev_buf);
 
 err0:
return ret;
@@ -2602,14 +2602,13 @@ static void dwc3_process_event_entry(struct dwc3 *dwc,
}
 }
 
-static irqreturn_t dwc3_process_event_buf(struct dwc3 *dwc)
+static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt)
 {
-   struct dwc3_event_buffer *evt;
+   struct dwc3 *dwc = evt->dwc;
irqreturn_t ret = IRQ_NONE;
int left;
u32 reg;
 
-   evt = dwc->ev_buf;
left = evt->count;
 
if (!(evt->flags & DWC3_EVENT_PENDING))
@@ -2649,27 +2648,26 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3 
*dwc)
return ret;
 }
 
-static irqreturn_t dwc3_thread_interrupt(int irq, void *_dwc)
+static irqreturn_t dwc3_thread_interrupt(int irq, void *_evt)
 {
-   struct dwc3 *dwc = _dwc;
+   struct dwc3_event_buffer *evt = _evt;
+   struct dwc3 *dwc = evt->dwc;
unsigned long flags;
irqreturn_t ret = IRQ_NONE;
 
spin_lock_irqsave(>lock, flags);
-   ret = dwc3_process_event_buf(dwc);
+   ret = dwc3_process_event_buf(evt);
spin_unlock_irqrestore(>lock, flags);
 
return ret;
 }
 
-static irqreturn_t dwc3_check_event_buf(struct dwc3 *dwc)
+static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
 {
-   struct dwc3_event_buffer *evt;
+   struct dwc3 *dwc = evt->dwc;
u32 count;
u32 reg;
 
-   evt = dwc->ev_buf;
-
count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
count &= DWC3_GEVNTCOUNT_MASK;
if (!count)
@@ -2686,11 +2684,11 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3 
*dwc)
return IRQ_WAKE_THREAD;
 }
 
-static irqreturn_t dwc3_interrupt(int irq, void *_dwc)
+static irqreturn_t dwc3_interrupt(int irq, void *_evt)
 {
-   struct dwc3 *dwc = _dwc;
+   struct dwc3_event_buffer*evt = _evt;
 
-   return dwc3_check_event_buf(dwc);
+   return dwc3_check_event_buf(evt);
 }
 
 /**
-- 
2.8.0.rc2

--
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 v2 5/6] usb: host: xhci: plat: make use of new methods in xhci_plat_priv

[Felipe Balbi]

Hi,

Yoshihiro Shimoda  writes:
> Hi,
>
>> Sent: Wednesday, March 30, 2016 2:59 PM
>> 
>> Yoshihiro Shimoda  writes:
>> > [ text/plain ]
>> > Hi Felipe,
>> >
>> > Thank you for the patch!
>> 
>> hey, no problem :-)
>> 
>> >> Sent: Tuesday, March 29, 2016 7:11 PM
>> > < snip >
>> >>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
>> >>   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>> >>   .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
>> >> + .init_quirk = xhci_rcar_init_quirk,
>> >>  };
>> >
>> > I should have said explicitly before, but the xhci_plat_renesas_rcar_gen2
>> > also needs ".plat_start = xhci_rcar_start" like the variable of gen3.
>> 
>> good catch, I really missed that one :-)
>
> I got it :)
>
>> Here's an updated version of $subject:
>
> Thank you for the update!
> Perhaps we need to rebase for the patch 6/6. Anyway, about this patch series:

depends on how Mathias wants this to be handled. Mathias, do you want me
to send the full series again ?

> Acked-by: Yoshihiro Shimoda 

Thanks :-)

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v2 5/6] usb: host: xhci: plat: make use of new methods in xhci_plat_priv

[Yoshihiro Shimoda]

Hi,

> Sent: Wednesday, March 30, 2016 2:59 PM
> 
> Yoshihiro Shimoda  writes:
> > [ text/plain ]
> > Hi Felipe,
> >
> > Thank you for the patch!
> 
> hey, no problem :-)
> 
> >> Sent: Tuesday, March 29, 2016 7:11 PM
> > < snip >
> >>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
> >>.type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
> >>.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
> >> +  .init_quirk = xhci_rcar_init_quirk,
> >>  };
> >
> > I should have said explicitly before, but the xhci_plat_renesas_rcar_gen2
> > also needs ".plat_start = xhci_rcar_start" like the variable of gen3.
> 
> good catch, I really missed that one :-)

I got it :)

> Here's an updated version of $subject:

Thank you for the update!
Perhaps we need to rebase for the patch 6/6. Anyway, about this patch series:

Acked-by: Yoshihiro Shimoda 

Best regards,
Yoshihiro Shimoda

> 8< cut here 
> 
> From 33607d25f042d4972dcf2a95f35f051c74b12e51 Mon Sep 17 00:00:00 2001
> From: Felipe Balbi 
> Date: Tue, 29 Mar 2016 13:03:31 +0300
> Subject: [PATCH] usb: host: xhci: plat: make use of new methods in
>  xhci_plat_priv
> 
> Now that the code has been refactored enough,
> switching over to using ->plat_start() and
> ->init_quirk() becomes a very simple patch.
> 
> After this patch, there are no further uses for
> xhci_plat_type_is() which will be removed in a
> follow-up patch.
> 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/host/xhci-plat.c | 42 +++---
>  1 file changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index d2a4f2be6a75..9e84992805d6 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -37,6 +37,24 @@ static const struct xhci_driver_overrides 
> xhci_plat_overrides __initconst = {
>   .start = xhci_plat_start,
>  };
> 
> +static void xhci_priv_plat_start(struct usb_hcd *hcd)
> +{
> + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +
> + if (priv->plat_start)
> + priv->plat_start(hcd);
> +}
> +
> +static int xhci_priv_init_quirk(struct usb_hcd *hcd)
> +{
> + struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
> +
> + if (!priv->init_quirk)
> + return 0;
> +
> + return priv->init_quirk(hcd);
> +}
> +
>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>  {
>   /*
> @@ -52,44 +70,38 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>  {
>   int ret;
> 
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
> - ret = xhci_rcar_init_quirk(hcd);
> - if (ret)
> - return ret;
> - }
> 
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_MARVELL_ARMADA)) {
> - ret = xhci_mvebu_mbus_init_quirk(hcd);
> - if (ret)
> - return ret;
> - }
> + ret = xhci_priv_init_quirk(hcd);
> + if (ret)
> + return ret;
> 
>   return xhci_gen_setup(hcd, xhci_plat_quirks);
>  }
> 
>  static int xhci_plat_start(struct usb_hcd *hcd)
>  {
> - if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
> - xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
> - xhci_rcar_start(hcd);
> -
> + xhci_priv_plat_start(hcd);
>   return xhci_run(hcd);
>  }
> 
>  #ifdef CONFIG_OF
>  static const struct xhci_plat_priv xhci_plat_marvell_armada = {
>   .type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
> + .init_quirk = xhci_mvebu_mbus_init_quirk,
>  };
> 
>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
>   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>   .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
> + .init_quirk = xhci_rcar_init_quirk,
> + .plat_start = xhci_rcar_start,
>  };
> 
>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
>   .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
>   .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
> + .init_quirk = xhci_rcar_init_quirk,
> + .plat_start = xhci_rcar_start,
>  };
> 
>  static const struct of_device_id usb_xhci_of_match[] = {
> --
> 2.8.0.rc2
> 
> 
> 
> 
> --
> balbi
--
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 v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 10:05, Peter Chen  wrote:
> On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>>
>> > > > I am afraid I still not find the user (udc driver) for this framework, 
>> > > > I would
>> > > > like to see how udc driver block the enumeration until the charger 
>> > > > detection
>> > > > has finished, or am I missing something?
>>
>> > > It is not for udc driver but for power users who want to negotiate
>> > > with USB subsystem.
>>
>> > Then, where is the code the test user to decide what kinds of USB charger
>> > (SDP, CDP, DCP) is connecting now?
>>
>> Even without detection of CDP and DCP we have configurability within SDP
>> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> higher 500mA limit which can be negotiated.
>
> Well, things may be a little complicated.

I try to address your comments, maybe Mark need to do some complements. Thanks.

>
> - First, how to design get_charger_type for each udc driver?
> Since the charger detection process affects dp/dm signal, it can't be done
> during the enumeration or after that. So, the detection process can be only
> done after vbus has detected and before udc pull up dp.
>
> Then, when the get_charger_type do real charger detection? My suggestion is,
> if the charger type is unknown, we do real one, else, we just return the
> stored type value.

I think that is why we let udc driver to implement the
'gadget->ops->get_charger_type()' callback, which can be controlled by
udc driver.

>
> - Second, When to notify charger IC to charger:
> For SDP and CDP, it needs to notify charger IC after set configuration
> has finished.
> For DCP (and ACA, I am not sure), can notify charger IC after charger
> detection has finished or later.
>
> So, when we get charger is present, we need to notify charger IC at once
> for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> composite core to notify charger IC after set configuration has finished,
> like the patch 2/4 does.

But mostly charger IC is separate with gadget, so the charger IC
doesn't need to worry about the gadget status.

>
> - Third, since composite driver covers 500mA (and more for CDP) after set
> configuration and 2mA after suspend, and vbus handler covers connect
> and disconnect. I can't see any reasons we need to notify gadget state
> for power driver, do we really need to have usb_charger_plug_by_gadget?

In some solutions, gadget does not negotiate with the current. They
just send out one signal to power driver to set the current when the
gadget state is changed (plugin or not). So we need to check the
charger state by the gadget state to notify the charger IC to set
current.

>
> --
> Best Regards,
> eter Chen



-- 
Baolin.wang
Best Regards
--
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: Getting a gadget serial device working on Atmel SAMA5D31

[Felipe Balbi]

JB  writes:
> Hello,
>
>> 4.5.0+, which patches do you have on top of 4.5 vanilla ?
>
> No code patches, just the configuration and dts file for the board.
>
>> Also, can you try this patch:
>
> I tried it, but the result is the same.
>
> Sometimes I can see another entry in the log:
>
> g_serial gadget: suspend
>
> And there is a single instance where this is followed by a few other entries:
>
> gadget: high-speed config #2: CDC ACM config
> gadget: reset acm control interface 0
> g_serial gadget: reset acm ttyGS0
> g_serial gadget: activate acm ttyGS0
> g_serial gadget: acm ttyGS0 serial state 
> g_serial gadget: non-core control reqa1.21 v i l7
> g_serial gadget: acm ttyGS0 reqa1.21 v i l7
> g_serial gadget: non-core control req21.22 v i l0
> g_serial gadget: acm ttyGS0 req21.22 v i l0
> g_serial gadget: non-core control req21.20 v i l7
> g_serial gadget: acm ttyGS0 req21.20 v i l7
> g_serial gadget: non-core control reqa1.21 v i l7
> g_serial gadget: acm ttyGS0 reqa1.21 v i l7
> g_serial gadget: reset config
> g_serial gadget: acm ttyGS0 deactivated
>
> Any other leads?

IIRC ACM will only do anything after you open /dev/ttyGS0 on the
peripheral side. Are you running getty on ttyGS0 ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [RFC PATCH 0/4] Implement USB device/host switch for Vybrid

[maitysanchayan]

Hello Peter,

On 16-03-29 00:24:46, Peter Chen wrote:
>  
> > 
> > On 2016-03-25 00:40, Peter Chen wrote:
> > > On Tue, Mar 15, 2016 at 02:08:26PM +0530, Sanchayan Maity wrote:
> > >> Hello Peter,
> > >>
> > >> The existing usage of extcon in Chipidea driver relies on OTG
> > >> registers. In case of SoC with dual role device but not a true OTG
> > >> controller, this does not work. Such SoC's should specify the
> > >> existing CI_HDRC_DUAL_ROLE_NOT_OTG flag and do the role switch
> > >> without checking any of the OTG registers in my opinion.
> > >> This is the case for Vybrid which uses a Chipidea IP but does not
> > >> have a true 5 pin OTG implemented.
> > >
> > > Sorry to reply you late due to my new born baby.
> > >
> > > Are you sure Vybrid is NOT OTG core? Afaik, it is uses the same IP
> > > base with other Freescale SoCs, just the IP core is 2.40a.
> > > When working at device mode, can you read vbus status through OTGSC?
> > > And if there is an ID pin (input pin) for Vybrid? I mean SoC, not the
> > > board.
> > 
> > I think the IP is actually OTG capable, the registers are there, but the 
> > signals
> > seem not to be available on the SoC package. That is also what the RM 
> > says...
> > 
> > Quotes from the RM:
> > 
> > "OTG controller should be treated as
> > Dual role controller that allows the
> > controller to act as either a Host or a
> > device with no support for HNP/SRP."
> > 
> > And later, in Chapter 11.1:
> > 
> > "The USB is not a true OTG. It can be configured by software to function 
> > either
> > as peripheral or as host. The ID pin, which is unique for OTG operation, is 
> > not
> > present in this implementation. There are no five pin interface. The user 
> > will
> > get four pin host/ device interface."
> > 
> 
> Get it, thanks. I am doing a patch for covering this case and vbus always-on 
> case.

If I may ask at this point, how would your implementation be covering this case
for Vybrid?

- Sanchayan.
--
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 2/3] usb: dwc3: increase maximum number of TRBs per endpoint

[Felipe Balbi]

Hi again,

Felipe Balbi  writes:
> Bin Liu  writes:
>> On Fri, Mar 18, 2016 at 08:59:48AM +0200, Felipe Balbi wrote:
>>> 
>>> Hi,
>>> 
>>> Bin Liu  writes:
>>> > [ text/plain ]
>>> > Hi,
>>> >
>>> > On Fri, Mar 11, 2016 at 6:54 AM, Felipe Balbi
>>> >  wrote:
>>> >> previously we were using a maximum of 32 TRBs per
>>> >> endpoint. With each TRB being 16 bytes long, we were
>>> >> using 512 bytes of memory for each endpoint.
>>> >>
>>> >> However, SLAB/SLUB will always allocate PAGE_SIZE
>>> >> chunks. In order to better utilize the memory we
>>> >> allocate and to allow deeper queues for gadgets
>>> >> which would benefit from it (g_ether comes to mind),
>>> >> let's increase the maximum to 256 TRBs which rounds
>>> >> up to 4096 bytes for each endpoint.
>>> >
>>> > Do we want to increase the same for event ring buffers as
>>> > while, which is allocated by dma_alloc_coherent(), which
>>> > is also at PAGE_SIZE chunks, right?
>>> 
>>> I could, but that's much less important. Currently we have up to 2
>>
>> I agree it is less important, the only issue I see is wasting of memory.
>> The device I am working on right now has two dwc3 controllers, each
>> allocates 16 event buffers, so for the total of 128KB allocated pages,
>> only 8KB is really used, 120KB is wasted.
>>
>> Seems dma pool makes more sense in here?
>
> I don't know. I think the real thing is that I really need to revisit
> that part of the code/databook. The whole "multiple interrupters" seems
> like it's only really necessary for host side. Which means that we could
> drop all the loops for multiple event buffers and always use a single
> one.
>
> Do you wanna test the following ?
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 17fd81447c9f..ebb3ee9c06f1 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -237,8 +237,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
> unsigned length)
>   int num;
>   int i;
>  
> - num = DWC3_NUM_INT(dwc->hwparams.hwparams1);
> - dwc->num_event_buffers = num;
> + dwc->num_event_buffers = 1;
>  
>   dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * num,
>   GFP_KERNEL);
>
> I'll re-read what these bits actually mean. I have a strong feeling we
> could (should?) be ignoring them for the peripheral side.

Okay, so when we're configuring the endpoints, we could route endpoint
interrupts to different event buffers. I really think that's really
unimportant for us, specially since we end up using a single IRQ line.

I guess I'll just go ahead and remove that code. If it turns out we
decide to use it, we shouldn't really be using a loop in the hardirq
handler anyway.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Seems you don't want to guarantee charger type detection is done
>> > before gadget connection(pullup DP), right?
>> > I see you call usb_charger_detect_type() in each gadget usb state
>> changes.
>>
>> I am not sure I get your point correctly, please correct me if I
>> misunderstand you.
>> We need to check the charger type at every event comes from the usb gadget
>> state changes or the extcon device state changes, which means a new
>> charger plugin or pullup.
>>
>
> According to usb charger spec, my understanding is you can't do real charger
> detection procedure *after* gadget _connection_(pullup DP), also I don't

Why can not? Charger detection is usually from PMIC.

> think it's necessary to check charger type at every event from usb gadget.

My meaning is not every event from usb gadget. When the usb gadget
state changes or the extcon device (maybe GPIO detection) state
changes, which means charger plugin or pullup, we need to check the
charger type to set current.

> Something in gadget driver you can utilize is only vbus detection, and
> report diff current by diff usb state if it's a SDP.

-- 
Baolin.wang
Best Regards
--
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 2/3] usb: dwc3: increase maximum number of TRBs per endpoint

[Felipe Balbi]

Hi,

Bin Liu  writes:
> On Fri, Mar 18, 2016 at 08:59:48AM +0200, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Bin Liu  writes:
>> > [ text/plain ]
>> > Hi,
>> >
>> > On Fri, Mar 11, 2016 at 6:54 AM, Felipe Balbi
>> >  wrote:
>> >> previously we were using a maximum of 32 TRBs per
>> >> endpoint. With each TRB being 16 bytes long, we were
>> >> using 512 bytes of memory for each endpoint.
>> >>
>> >> However, SLAB/SLUB will always allocate PAGE_SIZE
>> >> chunks. In order to better utilize the memory we
>> >> allocate and to allow deeper queues for gadgets
>> >> which would benefit from it (g_ether comes to mind),
>> >> let's increase the maximum to 256 TRBs which rounds
>> >> up to 4096 bytes for each endpoint.
>> >
>> > Do we want to increase the same for event ring buffers as
>> > while, which is allocated by dma_alloc_coherent(), which
>> > is also at PAGE_SIZE chunks, right?
>> 
>> I could, but that's much less important. Currently we have up to 2
>
> I agree it is less important, the only issue I see is wasting of memory.
> The device I am working on right now has two dwc3 controllers, each
> allocates 16 event buffers, so for the total of 128KB allocated pages,
> only 8KB is really used, 120KB is wasted.
>
> Seems dma pool makes more sense in here?

I don't know. I think the real thing is that I really need to revisit
that part of the code/databook. The whole "multiple interrupters" seems
like it's only really necessary for host side. Which means that we could
drop all the loops for multiple event buffers and always use a single
one.

Do you wanna test the following ?

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 17fd81447c9f..ebb3ee9c06f1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -237,8 +237,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, 
unsigned length)
int num;
int i;
 
-   num = DWC3_NUM_INT(dwc->hwparams.hwparams1);
-   dwc->num_event_buffers = num;
+   dwc->num_event_buffers = 1;
 
dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * num,
GFP_KERNEL);

I'll re-read what these bits actually mean. I have a strong feeling we
could (should?) be ignoring them for the peripheral side.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v2 5/6] usb: host: xhci: plat: make use of new methods in xhci_plat_priv

[Felipe Balbi]

Yoshihiro Shimoda  writes:
> [ text/plain ]
> Hi Felipe,
>
> Thank you for the patch!

hey, no problem :-)

>> Sent: Tuesday, March 29, 2016 7:11 PM
> < snip >
>>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
>>  .type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
>>  .firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
>> +.init_quirk = xhci_rcar_init_quirk,
>>  };
>
> I should have said explicitly before, but the xhci_plat_renesas_rcar_gen2
> also needs ".plat_start = xhci_rcar_start" like the variable of gen3.

good catch, I really missed that one :-)

Here's an updated version of $subject:

8< cut here 

From 33607d25f042d4972dcf2a95f35f051c74b12e51 Mon Sep 17 00:00:00 2001
From: Felipe Balbi 
Date: Tue, 29 Mar 2016 13:03:31 +0300
Subject: [PATCH] usb: host: xhci: plat: make use of new methods in
 xhci_plat_priv

Now that the code has been refactored enough,
switching over to using ->plat_start() and
->init_quirk() becomes a very simple patch.

After this patch, there are no further uses for
xhci_plat_type_is() which will be removed in a
follow-up patch.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-plat.c | 42 +++---
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d2a4f2be6a75..9e84992805d6 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -37,6 +37,24 @@ static const struct xhci_driver_overrides 
xhci_plat_overrides __initconst = {
.start = xhci_plat_start,
 };
 
+static void xhci_priv_plat_start(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (priv->plat_start)
+   priv->plat_start(hcd);
+}
+
+static int xhci_priv_init_quirk(struct usb_hcd *hcd)
+{
+   struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+   if (!priv->init_quirk)
+   return 0;
+
+   return priv->init_quirk(hcd);
+}
+
 static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
/*
@@ -52,44 +70,38 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
 {
int ret;
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3)) {
-   ret = xhci_rcar_init_quirk(hcd);
-   if (ret)
-   return ret;
-   }
 
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_MARVELL_ARMADA)) {
-   ret = xhci_mvebu_mbus_init_quirk(hcd);
-   if (ret)
-   return ret;
-   }
+   ret = xhci_priv_init_quirk(hcd);
+   if (ret)
+   return ret;
 
return xhci_gen_setup(hcd, xhci_plat_quirks);
 }
 
 static int xhci_plat_start(struct usb_hcd *hcd)
 {
-   if (xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2) ||
-   xhci_plat_type_is(hcd, XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3))
-   xhci_rcar_start(hcd);
-
+   xhci_priv_plat_start(hcd);
return xhci_run(hcd);
 }
 
 #ifdef CONFIG_OF
 static const struct xhci_plat_priv xhci_plat_marvell_armada = {
.type = XHCI_PLAT_TYPE_MARVELL_ARMADA,
+   .init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
.type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN2,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
+   .init_quirk = xhci_rcar_init_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen3 = {
.type = XHCI_PLAT_TYPE_RENESAS_RCAR_GEN3,
.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V2,
+   .init_quirk = xhci_rcar_init_quirk,
+   .plat_start = xhci_rcar_start,
 };
 
 static const struct of_device_id usb_xhci_of_match[] = {
-- 
2.8.0.rc2




-- 
balbi


signature.asc
Description: PGP signature