Re: [PATCH 2/5] usb: musb: core: added helper functions for parsing DT

[Felipe Balbi]

Hi,

Sergei Shtylyov  writes:
> On 02/18/2016 11:18 AM, Felipe Balbi wrote:
>
> This adds two functions to get DT properties "mentor,power" and "dr_mode":
> musb_get_power() and musb_mode musb_get_mode()
>
> Signed-off-by: Petr Kulhavy 
 seems like I don't have patch 1/5. After fixing Sergei's comments,
 please resend with his Acked-by already in place.

 thanks
>>> Hi Felipe,
>>>
>>> I will do as soon as the patch 1/5 gets approved.
>>> It seem to be a bit stuck at the moment as Rob Herring from the DT wants
>>> the "mentor,power"
>>> to be represented as a regulator, whereas Sergei and me want to stick to
>>> the existing "mentor,power" integer property.
>>>
>>> As soon as this get clarified I will do the final updates and send the
>>> patch again.
>>> Maybe this is something you can help to clarify?
>>
>> I don't think that makes sense as a regulator. It's just a number which
>> gets passed to USB Core as is.
>
> Well, in case of DaVinci's it's an external GPIO controlled
> regulator indeed.

oh, I see. Not controller by SetPortPower. That's a shame.

>> However, it seems like everything in kernel right now is passing it as
>> 500. So why don't you deprecate that property, hardcode it to 500 and
>> avoid the problem altogether ?
>
> OMAP boards can only supply 100 mA, AFAIK. Isn't it too early for the 
> deprecation? :-)

 $ git --no-pager grep -e mentor,power
Documentation/devicetree/bindings/usb/am33xx-usb.txt:44:- mentor,power: Should 
be "500". This signifies the controller can supply up to
Documentation/devicetree/bindings/usb/am33xx-usb.txt:112:   
mentor,power = <500>;
Documentation/devicetree/bindings/usb/am33xx-usb.txt:157:   
mentor,power = <500>;
arch/arm/boot/dts/am33xx.dtsi:580:  mentor,power = 
<500>;
arch/arm/boot/dts/am33xx.dtsi:627:  mentor,power = 
<500>;
arch/arm/boot/dts/dm814x.dtsi:91:   mentor,power = 
<500>;
arch/arm/boot/dts/dm814x.dtsi:129:  mentor,power = 
<500>;
arch/arm/boot/dts/dm816x.dtsi:414:  mentor,power = 
<500>;
arch/arm/boot/dts/dm816x.dtsi:454:  mentor,power = 
<500>;
drivers/usb/musb/musb_dsps.c:744:   pdata.power = get_int_prop(dn, 
"mentor,power") / 2;

Even documentation says it _must_ be 500.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCHv3 00/11] Fixes and improvements to f_fs and f_midi

[Felipe Balbi]

Hi,

Michal Nazarewicz  writes:
>> Michal Nazarewicz  writes:
>>> Resending my previous two sets for f_fs and f_midi.  This time rebased
>>> on top of Felipe’s next branch.
>>>
>>> Dan Carpenter (1):
>>>   usb: gadget: f_midi: missing unlock on error path
>>>
>>> Du, Changbin (1):
>>>   usb: f_fs: avoid race condition with ffs_epfile_io_complete
>>>
>>> Felipe F. Tonello (1):
>>>   usb: gadget: f_midi: remove useless midi reference from port struct
>>>
>>> Michal Nazarewicz (8):
>>>   usb: f_fs: fix memory leak when ep changes during transfer
>>>   usb: f_fs: fix ffs_epfile_io returning success on req alloc failure
>>>   usb: f_fs: replace unnecessary goto with a return
>>>   usb: f_fs: refactor ffs_epfile_io
>>>   usb: gadget: f_midi: move some of f_midi_transmit to separate func
>>>   usb: gadget: f_midi: fix in_last_port looping logic
>>>   usb: gadget: f_midi: use flexible array member for gmidi_in_port
>>> elements
>>>   usb: gadget: f_midi: stash substream in gmidi_in_port structure
>
> On Thu, Feb 18 2016, Felipe Balbi wrote:
>> looks like I don't have these patches in my inbox, care to resend ?
>
> Patches freshly rebased on your next branch coming right up.  Also
> available at:
>
> git fetch https://github.com/mina86/linux.git for-felipe

thanks, I'll fetch your branch and apply the patches individually :-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v7 05/10] usb: dbc: add bulk out and bulk in interfaces

[Lu Baolu]

On 02/18/2016 09:32 PM, Mathias Nyman wrote:
> On 26.01.2016 14:58, Lu Baolu wrote:
>> This patch adds interfaces for bulk out and bulk in ops. These
>> interfaces could be used to implement early printk bootconsole
>> or hook to various system debuggers.
>>
>> Signed-off-by: Lu Baolu 
>> ---
>>   drivers/usb/early/xhci-dbc.c | 373 
>> +++
>>   include/linux/usb/xhci-dbc.h |  30 
>>   2 files changed, 403 insertions(+)
>>
>
> ...
>
>> +
>> +/*
>> + * Check and dispatch events in event ring. It also checks status
>> + * of hardware. This function will be called from multiple threads.
>> + * An atomic lock is applied to protect the access of event ring.
>> + */
>> +static int xdbc_check_event(void)
>> +{
>> +/* event ring is under checking by other thread? */
>> +if (!test_bit(XDBC_ATOMIC_EVENT, >atomic_flags) &&
>> +!test_and_set_bit(XDBC_ATOMIC_EVENT,
>> +>atomic_flags))
>> +return 0;
>
> homemade trylock, can't the real ones be used?
>
>> +
>> +xdbc_handle_events();
>> +
>> +test_and_clear_bit(XDBC_ATOMIC_EVENT, >atomic_flags);
>> +
>> +return 0;
>>  +}
>> +
>> +#defineBULK_IN_COMPLETED(p)((xdbcp->in_pending == (p)) && \
>> + xdbcp->in_complete)
>> +#defineBULK_OUT_COMPLETED(p)((xdbcp->out_pending == (p)) && \
>> + xdbcp->out_complete)
>> +
>
> ...
>
>> +}
>> +
>> +int xdbc_bulk_read(void *data, int size, int loops)
>> +{
>> +int ret;
>> +
>> +do {
>> +if (!test_bit(XDBC_ATOMIC_BULKIN, >atomic_flags) &&
>> +!test_and_set_bit(XDBC_ATOMIC_BULKIN,
>> +>atomic_flags))
>> +break;
>> +} while (1);
>
> homemeade spin_lock, can't the real one be used?
>
> If the xdbc_bulk_write() can be accessed from interrupt context (handler, 
> soft, timer) it
> may deadlock
>
>> +
>> +ret = xdbc_bulk_transfer(data, size, loops, true);
>> +
>> +test_and_clear_bit(XDBC_ATOMIC_BULKIN, >atomic_flags);
>> +
>> +return ret;
>> +}
>> +
>> +int xdbc_bulk_write(const char *bytes, int size)
>> +{
>> +int ret;
>> +
>> +do {
>> +if (!test_bit(XDBC_ATOMIC_BULKOUT, >atomic_flags) &&
>> +!test_and_set_bit(XDBC_ATOMIC_BULKOUT,
>> +>atomic_flags))
>> +break;
>> +} while (1);
>
> Another homemeade spin_lock, can't the real one be used?
>
> same issue here, deadlock if accessible from interrupt context.

I will try to rework this spin_lock with the real one and keep avoiding 
deadlock in mind.

>
>
> Would it make sense to have only one spinlock, and start one separate thread 
> for
> reading the event ring. The thread would,  lock, handle pending events, 
> unlock,
> then call shedule, in a loop. ehci early debug code has some variant of this.

Let me try to find this part of code.

>
> So the lock would be taken while events are being handled.
>
> The same lock would be used for bulk_read and bulk_write. Yes this would 
> prevent read and
> write at the same time, and the read and writes need to be modified to not 
> block until
> the reansfer is finished, just to write the TRBs on the ring, update ring 
> pointers,
> and ring the doorbell.
>
> Or is all this impossibe due to the earlyness of the code?

It's not only due to earlyness of the code. But also, these read/write ops were 
designed to
be used by a debugger (for example kgdb) as well. Using the kernel provided 
interface
might make things simple, but what should happen when the debugger is used
to debug the kernel subsystem itself?

So, it seems that I should implement read/write ops depends on the use case. 
For this
time being, let's focus on the boot console case.

Some transfers take place when the thread/lock subsystem is not initialized yet.
But after thread/lock subsystem is able to be used, we are able to use the real 
one.

Let me wrapper them in functions. For the transfers taken place before the 
subsystem
initialization (that's single thread context, no worry about deadlock), it will 
use the
current methods (it might be possible to drop lock due the single thread 
context),
and after the subsystem being initialized, it will use those provided by the 
kernel.

>
> -Mathias
>

Very appreciated for your time.

Regards,
-Baolu
--
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 19/19] usb: dwc3: Enable SuperSpeedPlus

[Felipe Balbi]

Hi,

John Youn  writes:
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 5bbdf5d9c35e..d8566ad60d9b 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -1633,9 +1633,14 @@ static int dwc3_gadget_start(struct usb_gadget *g,
reg |= DWC3_DSTS_SUPERSPEED_PLUS;
break;
case USB_SPEED_SUPER:   /* FALLTHROUGH */
 +  reg |= DWC3_DSTS_SUPERSPEED;
 +  break;
case USB_SPEED_UNKNOWN: /* FALTHROUGH */
default:
 -  reg |= DWC3_DSTS_SUPERSPEED;
 +  if (dwc_is_usb31(dwc))
 +  reg |= DWC3_DSTS_SUPERSPEED_PLUS;
 +  else
 +  reg |= DWC3_DSTS_SUPERSPEED;
}
}
dwc3_writel(dwc->regs, DWC3_DCFG, reg);

 the reason being that default speed should always be the fastest speed
 supported by current core and, after this patchset we start support
 superspeed plus.

 What do you think ?

>>>
>>> Hi Felipe,
>>>
>>> That check is already done on the dwc->maximum_speed parameter in the
>>> probe when set to UNKNOWN. Though that check also involves checking
>>> the GHWPARAMS3.SSPHY_IF. Which, I think you reported before that some
>>> systems may not set correctly. I was thinking those systems can just
>>> set maximum_speed if needed.
>> 
>> In that case, the default case statement here is unnecessary.
>> 
>
> The maximum_speed is not fully validated. It is only adjusted on
> USB_SPEED_UNKNOWN. So if a user passes in some completely invalid
> value then and we could end up in the default condition here.
>
> I'll fix it so that can't occur.
>
> But even then, I think it is reasonable to handle the default
> condition in the switch, if only to guard against programmer error in
> the future. Maybe print an error and have some safe fall-back. Are you
> ok with that?

sounds ok to me. :-)

-- 
balbi


signature.asc
Description: PGP signature

Re: driver migration

[tilman]

Hello

I configured and setup a more recent kernel:  V4.5.0-rc4

The driver compiles and inserts. When I plugin the USB-Dongle, the machine
freezes (not sure whether it freezes during the firmware download or after
it once it has reenumerated). 

I could observe  a kernel oops message starting with BUG. Probably a null
pointer dereference...

Cheers
Tilman

--
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 04/10] usb: dbc: add debug buffer

[Lu Baolu]

On 02/18/2016 07:43 PM, Mathias Nyman wrote:
> On 26.01.2016 14:58, Lu Baolu wrote:
>> "printk" is not suitable for dbc debugging especially when console
>> is in usage. This patch adds a debug buffer in dbc driver and puts
>> the debug messages in this local buffer. The debug buffer could be
>> dumped whenever the console is not in use. This part of code will
>> not be visible unless DBC_DEBUG is defined.
>>
>> Signed-off-by: Lu Baolu 
>> ---
>>   drivers/usb/early/xhci-dbc.c | 62 
>> ++--
>>   1 file changed, 60 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
>> index 41ce116..6855048 100644
>> --- a/drivers/usb/early/xhci-dbc.c
>> +++ b/drivers/usb/early/xhci-dbc.c
>> @@ -32,8 +32,64 @@ static struct xdbc_state xdbc_stat;
>>   static struct xdbc_state *xdbcp = _stat;
>>
>>   #ifdef DBC_DEBUG
>> -/* place holder */
>> -#definexdbc_traceprintk
>> +#defineXDBC_DEBUG_BUF_SIZE(PAGE_SIZE * 32)
>
> Does it really need to be this huge? minimum 4096 * 32 ~ 128k
> The kernel ring  buffer is about the same size (16k - 256k)

This debug buffer is only used when DBC_DEBUG is defined.

128k is a little large. I will change it to 16k.

>
>> +#defineMSG_MAX_LINE128
>
> with 128 characters per line this would fit ~1000 lines
>
>> +static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
>> +static void xdbc_trace(const char *fmt, ...)
>> +{
>> +int i, size;
>> +va_list args;
>> +static int pos;
>> +char temp_buf[MSG_MAX_LINE];
>> +
>> +if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
>> +return;
>> +
>> +memset(temp_buf, 0, MSG_MAX_LINE);
>> +va_start(args, fmt);
>> +vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
>> +va_end(args);
>> +
>> +i = 0;
>> +size = strlen(temp_buf);
>> +while (i < size) {
>> +xdbc_debug_buf[pos] = temp_buf[i];
>> +pos++;
>> +i++;
>> +
>> +if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
>> +break;
>> +}
>
> how about something like:
>
> size = min(XDBC_DEBUG_BUF_SIZE - pos, size)
> memcpy(xdbc_debug_buf + pos, temp_buf, size)
> pos += size;
>
> (might need some "-1" and off by one checking..)

Yes. This looks better.

>
>> +}
>> +
>> +static void xdbc_dump_debug_buffer(void)
>> +{
>> +int index = 0;
>> +int count = 0;
>> +char dump_buf[MSG_MAX_LINE];
>> +
>> +xdbc_trace("The end of DbC trace buffer\n");
>> +pr_notice("DBC debug buffer:\n");
>> +memset(dump_buf, 0, MSG_MAX_LINE);
>> +
>> +while (index < XDBC_DEBUG_BUF_SIZE) {
>> +if (!xdbc_debug_buf[index])
>> +break;
>> +
>> +if (xdbc_debug_buf[index] == '\n' ||
>> +count >= MSG_MAX_LINE - 1) {
>> +pr_notice("DBC: @%08x %s\n", index, dump_buf);
>
> Is showing the he index (position in debug buffer) useful here?

It helps to check the debug log especially when it shows the hardware
registers or memory block.

>
>
>> +memset(dump_buf, 0, MSG_MAX_LINE);
>> +count = 0;
>> +} else {
>> +dump_buf[count] = xdbc_debug_buf[index];
>> +count++;
>> +}
>> +
>> +index++;
>> +}
>
> So we have one huge buffer that xdbc keeps on filling as the initialization 
> progresses.
> It is never emptied, or overwritten (circular).
> When dumped it always dumps the whole thing, copying one character
> at a time.
>
> As this is only used for debugging during xdbc development/debugging, and 
> never enabled
> even if xdbc early printk is used, I don't think optimization really matters.

Yes, it's only a helper for the persons who develop and debug this driver.

>
> Perhaps take a look if we really need PAGE_SIZE * 32 bytes, is xdbc driver 
> even nearly
> writing that much debug data.

I will reduce the debug buffer size.

>  
> -Mathias
>
>

Thanks for your time.

Regards,
-Baolu
--
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

[GIT PULL] USB Chipidea fixes for v4.5-rc5

[Peter Chen]

The following changes since commit 18558cae0272f8fd9647e69d3fec1565a7949865:

  Linux 4.5-rc4 (2016-02-14 13:05:20 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ 
tags/usb-ci-v4.5-rc5

for you to fetch changes up to 8c0614ca312c847ca5409ea11c39434dec69631d:

  usb: chipidea: fix return value check in ci_hdrc_pci_probe() (2016-02-19 
14:13:44 +0800)


Some tiny bug fixes for chipidea driver


Alan (1):
  usb: chipidea: error on overflow for port_test_write

Wei Yongjun (1):
  usb: chipidea: fix return value check in ci_hdrc_pci_probe()

 drivers/usb/chipidea/ci_hdrc_pci.c | 4 ++--
 drivers/usb/chipidea/debug.c   | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 

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: [PATCH] usb: chipidea: fix return value check in ci_hdrc_pci_probe()

[Peter Chen]

On Tue, Feb 16, 2016 at 05:12:51PM +0800, Peter Chen wrote:
> On Sat, Feb 06, 2016 at 10:57:06PM +0800, weiyj...@163.com wrote:
> > From: Wei Yongjun 
> > 
> > In case of error, the function usb_phy_generic_register()
> > returns ERR_PTR() and never returns NULL. The NULL test in
> > the return value check should be replaced with IS_ERR().
> > 
> > Signed-off-by: Wei Yongjun 
> > ---
> >  drivers/usb/chipidea/ci_hdrc_pci.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c 
> > b/drivers/usb/chipidea/ci_hdrc_pci.c
> > index b59195e..b635ab6 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_pci.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_pci.c
> > @@ -85,8 +85,8 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
> >  
> > /* register a nop PHY */
> > ci->phy = usb_phy_generic_register();
> > -   if (!ci->phy)
> > -   return -ENOMEM;
> > +   if (IS_ERR(ci->phy))
> > +   return PTR_ERR(ci->phy);
> >  
> > memset(res, 0, sizeof(res));
> > res[0].start= pci_resource_start(pdev, 0);
> > 
> 
> The original code has several issues:
> First, usb_phy_generic_register returns pointers of platform_device
> Second, the generic USB PHY driver is related to struct usb_phy, but
> not struct phy.
> So, in this code, it needs to use ci->usb_phy to get the pointer
> of struct usb_phy used at generic USB PHY driver.
> 
> Would you mind sending the patches to fix them?
> 

oops, The ci in this patch is struct ci_hdrc_pci *, but not struct
ci_hdrc *. Your patch is ok, I will queue it, thanks.

-- 

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: Corsair HIDs not properly initializing

[Greg KH]

On Fri, Feb 19, 2016 at 03:44:29AM +, Cestarian Inhabitant wrote:
> see ccMSC's CKB git project for more details (troubleshooting section)

What is that?

> Many of the new corsair HID devices (mice and keyboards) fail to
> initialize correctly on kernel bootup. This is the workaround used to
> make them work.
> 
> (Some device ID's, like for the K60 are missing)

Missing where?

> The code I use in the kernel commandline to bypass the issue on my
> K95-RGB looks like so: usbhid.quirks=0x1B1C:0x1B11:0x408
> 
> The issue has been confirmed on recent (2016) Arch and Fedora kernels
> for K95-RGB and on Ubuntu's El Capitan with K60.
> 
> Please add support for these devices, and look into adding support for
> devices not listed in the above page (K60, K60-RGB, K50, K40, K30,
> K65(Non-RGB), K90, and the KATAR mouse; there may be some devices I
> left out but this should be the brunt of their current device names)

What page are you referring to here?

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

Corsair HIDs not properly initializing

[Cestarian Inhabitant]

see ccMSC's CKB git project for more details (troubleshooting section)

Many of the new corsair HID devices (mice and keyboards) fail to
initialize correctly on kernel bootup. This is the workaround used to
make them work.

(Some device ID's, like for the K60 are missing)

The code I use in the kernel commandline to bypass the issue on my
K95-RGB looks like so: usbhid.quirks=0x1B1C:0x1B11:0x408

The issue has been confirmed on recent (2016) Arch and Fedora kernels
for K95-RGB and on Ubuntu's El Capitan with K60.

Please add support for these devices, and look into adding support for
devices not listed in the above page (K60, K60-RGB, K50, K40, K30,
K65(Non-RGB), K90, and the KATAR mouse; there may be some devices I
left out but this should be the brunt of their current device names)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] usb: host: xhci-rcar: Use ARCH_RENESAS

[Simon Horman]

Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.

This is part of an ongoing process to migrate from ARCH_SHMOBILE to
ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.

Signed-off-by: Simon Horman 
---
 drivers/usb/host/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 438dcf6289b0..ed9a90f601e8 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -64,7 +64,7 @@ config USB_XHCI_MVEBU
 config USB_XHCI_RCAR
tristate "xHCI support for Renesas R-Car SoCs"
select USB_XHCI_PLATFORM
-   depends on ARCH_SHMOBILE || COMPILE_TEST
+   depends on ARCH_RENESAS || COMPILE_TEST
---help---
  Say 'Y' to enable the support for the xHCI host controller
  found in Renesas R-Car ARM SoCs.
-- 
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

[PATCH 1/2] usb: renesas_usbhs: Use ARCH_RENESAS

[Simon Horman]

Make use of ARCH_RENESAS in place of ARCH_SHMOBILE.

This is part of an ongoing process to migrate from ARCH_SHMOBILE to
ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.

Signed-off-by: Simon Horman 
---
 drivers/usb/renesas_usbhs/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/renesas_usbhs/Kconfig 
b/drivers/usb/renesas_usbhs/Kconfig
index ebc99ee076ce..b26d7c339c05 100644
--- a/drivers/usb/renesas_usbhs/Kconfig
+++ b/drivers/usb/renesas_usbhs/Kconfig
@@ -5,7 +5,7 @@
 config USB_RENESAS_USBHS
tristate 'Renesas USBHS controller'
depends on USB_GADGET
-   depends on ARCH_SHMOBILE || SUPERH || COMPILE_TEST
+   depends on ARCH_RENESAS || SUPERH || COMPILE_TEST
depends on EXTCON || !EXTCON # if EXTCON=m, USBHS cannot be built-in
default n
help
-- 
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

[PATCH 0/2] usb: Use ARCH_RENESAS

[Simon Horman]

Hi,

this short series makes use of of ARCH_RENESAS in place of ARCH_SHMOBILE.

This is part of an ongoing process to migrate from ARCH_SHMOBILE to
ARCH_RENESAS the motivation for which being that RENESAS seems to be a more
appropriate name than SHMOBILE for the majority of Renesas ARM based SoCs.

Simon Horman (2):
  usb: renesas_usbhs: Use ARCH_RENESAS
  usb: host: xhci-rcar: Use ARCH_RENESAS

 drivers/usb/host/Kconfig  | 2 +-
 drivers/usb/renesas_usbhs/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
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 v6 00/10] add HNP polling support for usb otg fsm

[Jun Li]

Hi Felipe

I didn't get any response for this request, if you need a resend
of the whole patchset, please let me know.

Li Jun

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Jun Li
> Sent: Monday, February 15, 2016 10:20 AM
> To: Felipe Balbi 
> Cc: linux-usb@vger.kernel.org; Peter Chen ;
> hzpeterc...@gmail.com; gre...@linuxfoundation.org
> Subject: RE: [PATCH v6 00/10] add HNP polling support for usb otg fsm
> 
> Resend with Felipe's new email address
> 
> > -Original Message-
> > From: Jun Li
> > Sent: Monday, February 15, 2016 10:16 AM
> > To: ba...@ti.com
> > Cc: linux-usb@vger.kernel.org; Peter Chen ;
> > hzpeterc...@gmail.com; gre...@linuxfoundation.org; Jun Li
> > 
> > Subject: RE: [PATCH v6 00/10] add HNP polling support for usb otg fsm
> >
> > Ping...
> >
> > > -Original Message-
> > > From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> > > ow...@vger.kernel.org] On Behalf Of Jun Li
> > > Sent: Thursday, January 28, 2016 2:50 PM
> > > To: ba...@ti.com
> > > Cc: linux-usb@vger.kernel.org; Peter Chen ;
> > > hzpeterc...@gmail.com; gre...@linuxfoundation.org
> > > Subject: RE: [PATCH v6 00/10] add HNP polling support for usb otg
> > > fsm
> > >
> > > Hi Felipe,
> > >
> > > > -Original Message-
> > > > From: Jun Li
> > > > Sent: Tuesday, January 26, 2016 3:19 PM
> > > > To: Peter Chen ; hzpeterc...@gmail.com;
> > > > ba...@ti.com; gre...@linuxfoundation.org
> > > > Cc: linux-usb@vger.kernel.org; Jun Li 
> > > > Subject: [PATCH v6 00/10] add HNP polling support for usb otg fsm
> > > >
> > > > HNP polling is a mechanism which allows the OTG device currently
> > > > acting as host to determine when the other attached OTG device
> > > > wishes to take the host role. When an OTG host, which supports
> > > > HNP, is connected to an OTG peripheral which also supports HNP it
> > > > shall poll the peripheral regularly to determine whether it
> > > > requires a role-swap and grant this at the earliest opportunity.
> > > > This patchset adds OTG HNP polling support, and enable for
> > > > chipidea usb otg fsm driver, more patches for pass OTG
> > > > certification will come
> > > later.
> > > >
> > > > changes for v6:
> > > > - Remove patch of disable irq while stop host role.
> > > > - Split B_AIDL_BDIS timer patch to be 2 patches(9/10 and 10/10).
> > > > - Add Peter's ack for patch 3, 7, 8/10.
> > > >
> > > > changes for v5:
> > > > - Instead of use stack memory, use kmalloc to allocate memory for
> host
> > > >   request flag one byte buffer(DMA read), and add host_req_flag
> > pointer
> > > >   in struct otg_fsm for reference to that flag buffer.
> > > > - Add one patch to disable irq while stop host role(7/10).
> > > > - Add one patch to fix B_AIDL_BDIS timing issue(10/10).
> > > >
> > > > changes for v4:
> > > > - Add OTG HNP capable check for connected device before sending
> > > > HNP polling
> > > >   in patch 3/8.
> > > > - Add comment to explain HNP test update in chipidea.txt in patch
> 8/8.
> > > > - Fix some typo.
> > > > - Add Peter's Ack in patch 1,2,4,5,6,7/8 of the series.
> > > >
> > > > Li Jun (10):
> > > >   usb: gadget: add hnp_polling_support and host_request_flag in
> > > > usb_gadget
> > > >   usb: add OTG status selector definition for HNP polling
> > > >   usb: common: otg-fsm: add HNP polling support
> > > >   usb: chipidea: udc: bypass otg status selector handling to gadget
> > > > driver
> > > >   usb: gadget: composite: handle otg status selector request from
> OTG
> > > > host
> > > >   usb: chipidea: otg: set host_request_flag for gadget
> > > >   usb: chipidea: otg: enable HNP polling support for gadget and host
> > > >   Documentation: usb: chipidea: Update test procedure for HNP
> polling
> > > >   usb: otg-fsm: add B_AIDL_BDIS timer
> > > >   usb: chipidea: otg: add A idle to B disconnect timer
> > > >
> > > >  Documentation/usb/chipidea.txt   |  9 +++--
> > > >  drivers/usb/chipidea/otg_fsm.c   | 29 --
> > > >  drivers/usb/chipidea/otg_fsm.h   |  2 +
> > > >  drivers/usb/chipidea/udc.c   |  3 +-
> > > >  drivers/usb/common/usb-otg-fsm.c | 87
> > > > 
> > > >  drivers/usb/gadget/composite.c   | 25 
> > > >  include/linux/usb/gadget.h   |  6 +++
> > > >  include/linux/usb/otg-fsm.h  | 15 +++
> > > >  include/uapi/linux/usb/ch9.h |  1 +
> > > >  9 files changed, 161 insertions(+), 16 deletions(-)
> > > >
> > > > --
> > > > 1.9.1
> > >
> > > Could you please take look this series again? Basically this is a
> > > updated version based on v4 you reviewed, for common and gadget
> > > part, the only major change is use dedicated buffer through kmalloc
> > > instead of use a stack variable for host request flag(usb control
> > > transfer, DMA
> > 

[RESEND PATCH v6 05/10] usb: gadget: composite: handle otg status selector request from OTG host

[Li Jun]

If gadget with HNP polling support receives GetStatus request of otg
status selector, it feedback to host with host request flag to indicate
if it wants to take host role.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/usb/gadget/composite.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 8b14c2a..709a4df 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1634,15 +1634,24 @@ composite_setup(struct usb_gadget *gadget, const struct 
usb_ctrlrequest *ctrl)
*((u8 *)req->buf) = value;
value = min(w_length, (u16) 1);
break;
-
-   /*
-* USB 3.0 additions:
-* Function driver should handle get_status request. If such cb
-* wasn't supplied we respond with default value = 0
-* Note: function driver should supply such cb only for the first
-* interface of the function
-*/
case USB_REQ_GET_STATUS:
+   if (gadget_is_otg(gadget) && gadget->hnp_polling_support &&
+   (w_index == OTG_STS_SELECTOR)) {
+   if (ctrl->bRequestType != (USB_DIR_IN |
+   USB_RECIP_DEVICE))
+   goto unknown;
+   *((u8 *)req->buf) = gadget->host_request_flag;
+   value = 1;
+   break;
+   }
+
+   /*
+* USB 3.0 additions:
+* Function driver should handle get_status request. If such cb
+* wasn't supplied we respond with default value = 0
+* Note: function driver should supply such cb only for the
+* first interface of the function
+*/
if (!gadget_is_superspeed(gadget))
goto unknown;
if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
-- 
1.9.1

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

[RESEND PATCH v6 08/10] Documentation: usb: chipidea: Update test procedure for HNP polling

[Li Jun]

Update HNP test procedure as HNP polling is supported.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 Documentation/usb/chipidea.txt | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index 05f735a..678741b 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -26,16 +26,17 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
On B-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
 
-   if HNP polling is not supported, also need:
-   On A-device:
-   echo 0 > /sys/bus/platform/devices/ci_hdrc.0/inputs/a_bus_req
-
B-device should take host role and enumrate A-device.
 
 4) A-device switch back to host.
On B-device:
echo 0 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
 
+   or, by introducing HNP polling, B-Host can know when A-peripheral wish
+   to be host role, so this role switch also can be trigged in A-peripheral
+   side by answering the polling from B-Host, this can be done on A-device:
+   echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/a_bus_req
+
A-device should switch back to host and enumrate B-device.
 
 5) Remove B-device(unplug micro B plug) and insert again in 10 seconds,
-- 
1.9.1

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

[RESEND PATCH v6 06/10] usb: chipidea: otg: set host_request_flag for gadget

[Li Jun]

Set host_request_flag if the current peripheral wants to take host role
via changing a_bus_req or b_bus_req by user application.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/otg_fsm.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index ba90dc6..cb28e76 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -66,6 +66,11 @@ set_a_bus_req(struct device *dev, struct device_attribute 
*attr,
return count;
}
ci->fsm.a_bus_req = 1;
+   if (ci->fsm.otg->state == OTG_STATE_A_PERIPHERAL) {
+   ci->gadget.host_request_flag = 1;
+   mutex_unlock(>fsm.lock);
+   return count;
+   }
}
 
ci_otg_queue_work(ci);
@@ -144,8 +149,14 @@ set_b_bus_req(struct device *dev, struct device_attribute 
*attr,
mutex_lock(>fsm.lock);
if (buf[0] == '0')
ci->fsm.b_bus_req = 0;
-   else if (buf[0] == '1')
+   else if (buf[0] == '1') {
ci->fsm.b_bus_req = 1;
+   if (ci->fsm.otg->state == OTG_STATE_B_PERIPHERAL) {
+   ci->gadget.host_request_flag = 1;
+   mutex_unlock(>fsm.lock);
+   return count;
+   }
+   }
 
ci_otg_queue_work(ci);
mutex_unlock(>fsm.lock);
-- 
1.9.1

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

[RESEND PATCH v6 10/10] usb: chipidea: otg: add A idle to B disconnect timer

[Li Jun]

B-device detects that bus is idle for more than TB_AIDL_BDIS min and
begins HNP by turning off pullup on DP, this allows the bus to discharge
to the SE0 state. This timer was missed and failed with PET test:
6.8.5 B-UUT HNP of USB OTG and EH automated compliance plan v1.2,
this patch is to fix this timing issue.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/otg_fsm.c | 12 ++--
 drivers/usb/chipidea/otg_fsm.h |  2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 9a963a7..de8e22e 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -209,6 +209,7 @@ static unsigned otg_timer_ms[] = {
TA_AIDL_BDIS,
TB_ASE0_BRST,
TA_BIDL_ADIS,
+   TB_AIDL_BDIS,
TB_SE0_SRP,
TB_SRP_FAIL,
0,
@@ -320,6 +321,12 @@ static int a_bidl_adis_tmout(struct ci_hdrc *ci)
return 0;
 }
 
+static int b_aidl_bdis_tmout(struct ci_hdrc *ci)
+{
+   ci->fsm.a_bus_suspend = 1;
+   return 0;
+}
+
 static int b_se0_srp_tmout(struct ci_hdrc *ci)
 {
ci->fsm.b_se0_srp = 1;
@@ -364,6 +371,7 @@ static int (*otg_timer_handlers[])(struct ci_hdrc *) = {
a_aidl_bdis_tmout,  /* A_AIDL_BDIS */
b_ase0_brst_tmout,  /* B_ASE0_BRST */
a_bidl_adis_tmout,  /* A_BIDL_ADIS */
+   b_aidl_bdis_tmout,  /* B_AIDL_BDIS */
b_se0_srp_tmout,/* B_SE0_SRP */
b_srp_fail_tmout,   /* B_SRP_FAIL */
NULL,   /* A_WAIT_ENUM */
@@ -655,9 +663,9 @@ static void ci_otg_fsm_event(struct ci_hdrc *ci)
break;
case OTG_STATE_B_PERIPHERAL:
if ((intr_sts & USBi_SLI) && port_conn && otg_bsess_vld) {
-   fsm->a_bus_suspend = 1;
-   ci_otg_queue_work(ci);
+   ci_otg_add_timer(ci, B_AIDL_BDIS);
} else if (intr_sts & USBi_PCI) {
+   ci_otg_del_timer(ci, B_AIDL_BDIS);
if (fsm->a_bus_suspend == 1)
fsm->a_bus_suspend = 0;
}
diff --git a/drivers/usb/chipidea/otg_fsm.h b/drivers/usb/chipidea/otg_fsm.h
index 262d6ef..6366fe3 100644
--- a/drivers/usb/chipidea/otg_fsm.h
+++ b/drivers/usb/chipidea/otg_fsm.h
@@ -62,6 +62,8 @@
 /* SSEND time before SRP */
 #define TB_SSEND_SRP (1500)/* minimum 1.5 sec, section:5.1.2 */
 
+#define TB_AIDL_BDIS (20)  /* 4ms ~ 150ms, section 5.2.1 */
+
 #if IS_ENABLED(CONFIG_USB_OTG_FSM)
 
 int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
-- 
1.9.1

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

[RESEND PATCH v6 09/10] usb: otg-fsm: add B_AIDL_BDIS timer

[Li Jun]

Add A-idle to B-disconnect timer, B-device detects that bus is idle
for more than TB_AIDL_BDIS min and begins HNP by turning off pullup
on D+. This allows the bus to discharge to the SE0 state.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 include/linux/usb/otg-fsm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h
index 3059a95..24198e1 100644
--- a/include/linux/usb/otg-fsm.h
+++ b/include/linux/usb/otg-fsm.h
@@ -60,6 +60,7 @@ enum otg_fsm_timer {
A_AIDL_BDIS,
B_ASE0_BRST,
A_BIDL_ADIS,
+   B_AIDL_BDIS,
 
/* Auxiliary timers */
B_SE0_SRP,
-- 
1.9.1

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

[RESEND PATCH v6 07/10] usb: chipidea: otg: enable HNP polling support for gadget and host

[Li Jun]

Enable HNP polling support for chipidea gadget and allocate memory
for host request flag when otg fsm init.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/otg_fsm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index cb28e76..9a963a7 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -797,6 +797,10 @@ int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
ci->fsm.otg->state = OTG_STATE_UNDEFINED;
ci->fsm.ops = _otg_ops;
+   ci->gadget.hnp_polling_support = 1;
+   ci->fsm.host_req_flag = devm_kzalloc(ci->dev, 1, GFP_KERNEL);
+   if (!ci->fsm.host_req_flag)
+   return -ENOMEM;
 
mutex_init(>fsm.lock);
 
-- 
1.9.1

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

[RESEND PATCH v6 04/10] usb: chipidea: udc: bypass otg status selector handling to gadget driver

[Li Jun]

Since gadget driver will handle this request, so controller driver bypass
it.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/usb/chipidea/udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 00250ab..065f5d9 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1067,7 +1067,8 @@ __acquires(ci->lock)
}
break;
case USB_REQ_GET_STATUS:
-   if (type != (USB_DIR_IN|USB_RECIP_DEVICE)   &&
+   if ((type != (USB_DIR_IN|USB_RECIP_DEVICE) ||
+   le16_to_cpu(req.wIndex) == OTG_STS_SELECTOR) &&
type != (USB_DIR_IN|USB_RECIP_ENDPOINT) &&
type != (USB_DIR_IN|USB_RECIP_INTERFACE))
goto delegate;
-- 
1.9.1

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

[RESEND PATCH v6 02/10] usb: add OTG status selector definition for HNP polling

[Li Jun]

A host is required to use the GetStatus command, with wIndex set to the
OTG status selector(F000H) to request the Host request flag from the
peripheral.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 include/uapi/linux/usb/ch9.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/usb/ch9.h b/include/uapi/linux/usb/ch9.h
index 4338eb7..a4deb1d 100644
--- a/include/uapi/linux/usb/ch9.h
+++ b/include/uapi/linux/usb/ch9.h
@@ -690,6 +690,7 @@ struct usb_otg20_descriptor {
 #define USB_OTG_HNP(1 << 1)/* swap host/device roles */
 #define USB_OTG_ADP(1 << 2)/* support ADP */
 
+#define OTG_STS_SELECTOR   0xF000  /* OTG status selector */
 /*-*/
 
 /* USB_DT_DEBUG:  for special highspeed devices, replacing serial console */
-- 
1.9.1

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

[RESEND PATCH v6 01/10] usb: gadget: add hnp_polling_support and host_request_flag in usb_gadget

[Li Jun]

Add 2 flags for USB OTG HNP polling, hnp_polling_support is to indicate
if the gadget can support HNP polling, host_request_flag is used for
gadget to store host request information from application, which can be
used to respond to HNP polling from host.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 include/linux/usb/gadget.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..737d823 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -595,6 +595,10 @@ struct usb_gadget_ops {
  * only supports HNP on a different root port.
  * @b_hnp_enable: OTG device feature flag, indicating that the A-Host
  * enabled HNP support.
+ * @hnp_polling_support: OTG device feature flag, indicating if the OTG device
+ * in peripheral mode can support HNP polling.
+ * @host_request_flag: OTG device feature flag, indicating if A-Peripheral
+ * or B-Peripheral wants to take host role.
  * @quirk_ep_out_aligned_size: epout requires buffer size to be aligned to
  * MaxPacketSize.
  * @is_selfpowered: if the gadget is self-powered.
@@ -642,6 +646,8 @@ struct usb_gadget {
unsignedb_hnp_enable:1;
unsigneda_hnp_support:1;
unsigneda_alt_hnp_support:1;
+   unsignedhnp_polling_support:1;
+   unsignedhost_request_flag:1;
unsignedquirk_ep_out_aligned_size:1;
unsignedquirk_altset_not_supp:1;
unsignedquirk_stall_not_supp:1;
-- 
1.9.1

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

[RESEND PATCH v6 00/10] add HNP polling support for usb otg fsm

[Li Jun]

HNP polling is a mechanism which allows the OTG device currently acting
as host to determine when the other attached OTG device wishes to take
the host role. When an OTG host, which supports HNP, is connected to an
OTG peripheral which also supports HNP it shall poll the peripheral
regularly to determine whether it requires a role-swap and grant this at
the earliest opportunity.
This patchset adds OTG HNP polling support, and enable for chipidea usb
otg fsm driver, more patches for pass OTG certification will come later.

changes for v6:
- Remove patch of disable irq while stop host role.
- Split B_AIDL_BDIS timer patch to be 2 patches(9/10 and 10/10).
- Add Peter's ack for patch 3, 7, 8/10.

changes for v5:
- Instead of use stack memory, use kmalloc to allocate memory for host
  request flag one byte buffer(DMA read), and add host_req_flag pointer
  in struct otg_fsm for reference to that flag buffer.
- Add one patch to disable irq while stop host role(7/10).
- Add one patch to fix B_AIDL_BDIS timing issue(10/10).

changes for v4:
- Add OTG HNP capable check for connected device before sending HNP polling
  in patch 3/8.
- Add comment to explain HNP test update in chipidea.txt in patch 8/8.
- Fix some typo.
- Add Peter's Ack in patch 1,2,4,5,6,7/8 of the series.

Li Jun (10):
  usb: gadget: add hnp_polling_support and host_request_flag in
usb_gadget
  usb: add OTG status selector definition for HNP polling
  usb: common: otg-fsm: add HNP polling support
  usb: chipidea: udc: bypass otg status selector handling to gadget
driver
  usb: gadget: composite: handle otg status selector request from OTG
host
  usb: chipidea: otg: set host_request_flag for gadget
  usb: chipidea: otg: enable HNP polling support for gadget and host
  Documentation: usb: chipidea: Update test procedure for HNP polling
  usb: otg-fsm: add B_AIDL_BDIS timer
  usb: chipidea: otg: add A idle to B disconnect timer

 Documentation/usb/chipidea.txt   |  9 +++--
 drivers/usb/chipidea/otg_fsm.c   | 29 --
 drivers/usb/chipidea/otg_fsm.h   |  2 +
 drivers/usb/chipidea/udc.c   |  3 +-
 drivers/usb/common/usb-otg-fsm.c | 87 
 drivers/usb/gadget/composite.c   | 25 
 include/linux/usb/gadget.h   |  6 +++
 include/linux/usb/otg-fsm.h  | 15 +++
 include/uapi/linux/usb/ch9.h |  1 +
 9 files changed, 161 insertions(+), 16 deletions(-)

-- 
1.9.1

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

Re: [PATCH 19/19] usb: dwc3: Enable SuperSpeedPlus

[John Youn]

On 2/17/2016 10:39 PM, Felipe Balbi wrote:
> John Youn  writes:
>> On 2/17/2016 12:39 AM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> John Youn  writes:
 Enable SuperSpeedPlus by programming the DCFG.speed and after
 enumerating, set gadget->speed appropriately.

 Signed-off-by: John Youn 
 ---
  drivers/usb/dwc3/gadget.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 73db723..5bbdf5d 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -1629,6 +1629,9 @@ static int dwc3_gadget_start(struct usb_gadget *g,
case USB_SPEED_HIGH:
reg |= DWC3_DSTS_HIGHSPEED;
break;
 +  case USB_SPEED_SUPER_PLUS:
 +  reg |= DWC3_DSTS_SUPERSPEED_PLUS;
 +  break;
case USB_SPEED_SUPER:   /* FALLTHROUGH */
case USB_SPEED_UNKNOWN: /* FALTHROUGH */
default:
>>>
>>> I'm thinking about amending this change to this patch:
>>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 5bbdf5d9c35e..d8566ad60d9b 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1633,9 +1633,14 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>> reg |= DWC3_DSTS_SUPERSPEED_PLUS;
>>> break;
>>> case USB_SPEED_SUPER:   /* FALLTHROUGH */
>>> +   reg |= DWC3_DSTS_SUPERSPEED;
>>> +   break;
>>> case USB_SPEED_UNKNOWN: /* FALTHROUGH */
>>> default:
>>> -   reg |= DWC3_DSTS_SUPERSPEED;
>>> +   if (dwc_is_usb31(dwc))
>>> +   reg |= DWC3_DSTS_SUPERSPEED_PLUS;
>>> +   else
>>> +   reg |= DWC3_DSTS_SUPERSPEED;
>>> }
>>> }
>>> dwc3_writel(dwc->regs, DWC3_DCFG, reg);
>>>
>>> the reason being that default speed should always be the fastest speed
>>> supported by current core and, after this patchset we start support
>>> superspeed plus.
>>>
>>> What do you think ?
>>>
>>
>> Hi Felipe,
>>
>> That check is already done on the dwc->maximum_speed parameter in the
>> probe when set to UNKNOWN. Though that check also involves checking
>> the GHWPARAMS3.SSPHY_IF. Which, I think you reported before that some
>> systems may not set correctly. I was thinking those systems can just
>> set maximum_speed if needed.
> 
> In that case, the default case statement here is unnecessary.
> 

The maximum_speed is not fully validated. It is only adjusted on
USB_SPEED_UNKNOWN. So if a user passes in some completely invalid
value then and we could end up in the default condition here.

I'll fix it so that can't occur.

But even then, I think it is reasonable to handle the default
condition in the switch, if only to guard against programmer error in
the future. Maybe print an error and have some safe fall-back. Are you
ok with that?

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: [PATCHv3 00/11] Fixes and improvements to f_fs and f_midi

[Michal Nazarewicz]

> Michal Nazarewicz  writes:
>> Resending my previous two sets for f_fs and f_midi.  This time rebased
>> on top of Felipe’s next branch.
>>
>> Dan Carpenter (1):
>>   usb: gadget: f_midi: missing unlock on error path
>>
>> Du, Changbin (1):
>>   usb: f_fs: avoid race condition with ffs_epfile_io_complete
>>
>> Felipe F. Tonello (1):
>>   usb: gadget: f_midi: remove useless midi reference from port struct
>>
>> Michal Nazarewicz (8):
>>   usb: f_fs: fix memory leak when ep changes during transfer
>>   usb: f_fs: fix ffs_epfile_io returning success on req alloc failure
>>   usb: f_fs: replace unnecessary goto with a return
>>   usb: f_fs: refactor ffs_epfile_io
>>   usb: gadget: f_midi: move some of f_midi_transmit to separate func
>>   usb: gadget: f_midi: fix in_last_port looping logic
>>   usb: gadget: f_midi: use flexible array member for gmidi_in_port
>> elements
>>   usb: gadget: f_midi: stash substream in gmidi_in_port structure

On Thu, Feb 18 2016, Felipe Balbi wrote:
> looks like I don't have these patches in my inbox, care to resend ?

Patches freshly rebased on your next branch coming right up.  Also
available at:

git fetch https://github.com/mina86/linux.git for-felipe

-- 
Best regards
Liege of Serenely Enlightened Majesty of Computer Science,
ミハウ “mina86” ナザレヴイツ   
--
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 v5 07/15] usb: ehci: add vbus-gpio parameter

[Antony Pavlov]

On Thu, 18 Feb 2016 21:31:20 +0300
Sergei Shtylyov  wrote:

> On 02/18/2016 09:06 PM, Antony Pavlov wrote:
[...]
> > so I use regulator in the TL-MR3020 board dts file:
> >
> >  reg_usb_vbus: reg_usb_vbus {
> >  compatible = "regulator-fixed";
> >  regulator-name = "usb_vbus";
> >  regulator-min-microvolt = <500>;
> 
> Not 0?
> 
> >  regulator-max-microvolt = <500>;
> >  gpio = < 8 GPIO_ACTIVE_HIGH>;
> 
> Where's the switch if both voltages are equal?

Here is a quote from 
linux/Documentation/devicetree/bindings/regulator/fixed-regulator.txt

Any property defined as part of the core regulator
binding, defined in regulator.txt, can also be used.
However a fixed voltage regulator is expected to have the
regulator-min-microvolt and regulator-max-microvolt
to be the same.

Moreover please see this of_get_fixed_voltage_config() code fragment
(please see linux/drivers/regulator/fixed.c for details):

if (init_data->constraints.min_uV == init_data->constraints.max_uV) {
config->microvolts = init_data->constraints.min_uV;
} else {
dev_err(dev,
 "Fixed regulator specified with variable voltages\n");
return ERR_PTR(-EINVAL);
}

-- 
Best regards,
  Antony Pavlov
--
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

[ANNOUNCE] patch queue empty

[Felipe Balbi]

Hi folks,

it looks like I've finally caught up with my patch queue. Please go
through my testing/fixes and testing/next and if you don't find your
patches in either branch, resend them as I, likely, don't have them
anymore.

cheers

-- 
balbi


signature.asc
Description: PGP signature

Re: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537

[Daniel Fraga]

On Thu, 18 Feb 2016 15:23:00 -0500 (EST)
Alan Stern  wrote:

> Something like the patch below (untested).

> + dev_info(>dev, "post reset hid_start_in -> %d\n", status);

Ok, so I got the following:

Feb 18 19:22:26 tux kernel: [  258.693120] usb 3-1.6: reset full-speed USB 
device number 6 using ehci-pci
Feb 18 19:22:26 tux kernel: [  258.783654] usbhid 3-1.6:1.0: post reset 
hid_start_in -> -22
Feb 18 19:22:26 tux kernel: [  259.883071] usb 3-1.6: reset full-speed USB 
device number 6 using ehci-pci
Feb 18 19:22:27 tux kernel: [  259.973529] usbhid 3-1.6:1.0: post reset 
hid_start_in -> -22
Feb 18 19:22:27 tux kernel: [  260.036047] usb 3-1.6: reset full-speed USB 
device number 6 using ehci-pci
Feb 18 19:22:27 tux kernel: [  260.126642] usbhid 3-1.6:1.0: post reset 
hid_start_in -> -22
Feb 18 19:22:28 tux kernel: [  260.188998] usb 3-1.6: reset full-speed USB 
device number 6 using ehci-pci
Feb 18 19:22:28 tux kernel: [  260.279629] usbhid 3-1.6:1.0: post reset 
hid_start_in -> -22
Feb 18 19:22:28 tux kernel: [  260.342019] usb 3-1.6: reset full-speed USB 
device number 6 using ehci-pci
Feb 18 19:22:28 tux kernel: [  260.432742] usbhid 3-1.6:1.0: post reset 
hid_start_in -> -22

(...)

And usbmon shows the following:

88024568da80 1732564945 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568da80 1732565020 C Ci:3:002:0 0 4 = 1101
88024568da80 1732575950 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568da80 1732576021 C Ci:3:002:0 0 4 = 03011000
88024568da80 1732576035 S Co:3:002:0 s 23 01 0014 0006  0
88024568da80 1732576154 C Co:3:002:0 0 0
88024568d540 1732626948 S Co:3:000:0 s 00 05 0006   0
88024568d540 1732627022 C Co:3:000:0 0 0
88024568d540 1732638972 S Ci:3:006:0 s 80 06 0100  0012 18 <
88024568d540 1732639272 C Ci:3:006:0 0 18 = 12010002 0008 f3043400 
1200040e 0001
88024568d540 1732639292 S Ci:3:006:0 s 80 06 0200  0029 41 <
88024568d540 1732639519 C Ci:3:006:0 0 41 = 09022900 010100e0 32090400 
00020300 0921 10010001 22b70307 05810340
88024568d540 1732639533 S Co:3:006:0 s 00 09 0001   0
88024568d540 1732639644 C Co:3:006:0 0 0
88024568d540 1732639671 S Ci:3:006:0 s 81 06 2200  03b7 951 <
88024568d540 1732644165 C Ci:3:006:0 0 951 = 050d0904 a1018501 0922a102 
09421500 25017501 95018102 75018103 75060951
88024568d540 1732644193 S Co:3:006:0 s 21 0a    0
88024568d540 1732644269 C Co:3:006:0 0 0
88024568da80 1732644989 S Co:3:002:0 s 23 03 0004 0006  0
88024568da80 1732645146 C Co:3:002:0 0 0
88024568d480 1732655954 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568d480 1732656023 C Ci:3:002:0 0 4 = 1101
88024568da80 1732666962 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568da80 1732667024 C Ci:3:002:0 0 4 = 03011000
88024568da80 1732667029 S Co:3:002:0 s 23 01 0014 0006  0
88024568da80 1732667144 C Co:3:002:0 0 0
88024568d780 1732717984 S Ci:3:000:0 s 80 06 0100  0040 64 <
88024568d780 1732718148 C Ci:3:000:0 0 8 = 12010002 0008
88024568d780 1732718168 S Co:3:002:0 s 23 03 0004 0006  0
88024568d780 1732718271 C Co:3:002:0 0 0
88024568d780 1732728982 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568d780 1732729059 C Ci:3:002:0 0 4 = 1101
88024568d780 1732739957 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568d780 1732740022 C Ci:3:002:0 0 4 = 03011000
88024568d780 1732740030 S Co:3:002:0 s 23 01 0014 0006  0
88024568d780 1732740146 C Co:3:002:0 0 0
88024568d780 1732790980 S Co:3:000:0 s 00 05 0006   0
88024568d780 1732791149 C Co:3:000:0 0 0
88024568d9c0 1732802963 S Ci:3:006:0 s 80 06 0100  0012 18 <
88024568d9c0 1732803282 C Ci:3:006:0 0 18 = 12010002 0008 f3043400 
1200040e 0001
88024568d9c0 1732803305 S Ci:3:006:0 s 80 06 0200  0029 41 <
88024568d9c0 1732803667 C Ci:3:006:0 0 41 = 09022900 010100e0 32090400 
00020300 0921 10010001 22b70307 05810340
88024568d9c0 1732803675 S Co:3:006:0 s 00 09 0001   0
88024568d9c0 1732803770 C Co:3:006:0 0 0
88024568d9c0 1732803797 S Ci:3:006:0 s 81 06 2200  03b7 951 <
88024568d9c0 1732808292 C Ci:3:006:0 0 951 = 050d0904 a1018501 0922a102 
09421500 25017501 95018102 75018103 75060951
88024568d9c0 1732808300 S Co:3:006:0 s 21 0a    0
88024568d9c0 1732808398 C Co:3:006:0 0 0
88024568d780 1732809002 S Co:3:002:0 s 23 03 0004 0006  0
88024568d780 1732809146 C Co:3:002:0 0 0
88024568d780 1732819951 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568d780 1732820023 C Ci:3:002:0 0 4 = 1101
88024568d9c0 1732830954 S Ci:3:002:0 s a3 00  0006 0004 4 <
88024568d9c0 1732831033 C Ci:3:002:0 0 4 = 03011000
88024568d9c0 1732831053 S Co:3:002:0 s 23 01 0014 0006  0
88024568d9c0 1732831147 C Co:3:002:0 0 0
88024568d0c0 1732881971 S Ci:3:000:0 s 80 06 

Re: [PATCHv3 00/11] Fixes and improvements to f_fs and f_midi

[Felipe Balbi]

Hi,

Michal Nazarewicz  writes:
> Resending my previous two sets for f_fs and f_midi.  This time rebased
> on top of Felipe’s next branch.
>
> Dan Carpenter (1):
>   usb: gadget: f_midi: missing unlock on error path
>
> Du, Changbin (1):
>   usb: f_fs: avoid race condition with ffs_epfile_io_complete
>
> Felipe F. Tonello (1):
>   usb: gadget: f_midi: remove useless midi reference from port struct
>
> Michal Nazarewicz (8):
>   usb: f_fs: fix memory leak when ep changes during transfer
>   usb: f_fs: fix ffs_epfile_io returning success on req alloc failure
>   usb: f_fs: replace unnecessary goto with a return
>   usb: f_fs: refactor ffs_epfile_io
>   usb: gadget: f_midi: move some of f_midi_transmit to separate func
>   usb: gadget: f_midi: fix in_last_port looping logic
>   usb: gadget: f_midi: use flexible array member for gmidi_in_port
> elements
>   usb: gadget: f_midi: stash substream in gmidi_in_port structure

looks like I don't have these patches in my inbox, care to resend ?

thanks

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] USB: cdc_subset: only build when one driver is enabled

[David Miller]

From: Arnd Bergmann 
Date: Wed, 17 Feb 2016 23:25:11 +0100

> This avoids a harmless randconfig warning I get when USB_NET_CDC_SUBSET
> is enabled, but all of the more specific drivers are not:
> 
> drivers/net/usb/cdc_subset.c:241:2: #warning You need to configure some 
> hardware for this driver
> 
> The current behavior is clearly intentional, giving a warning when
> a user picks a configuration that won't do anything good. The only
> reason for even addressing this is that I'm getting close to
> eliminating all 'randconfig' warnings on ARM, and this came up
> a couple of times.
> 
> My workaround is to not even build the module when none of the
> configurations are enable.
> 
> Alternatively we could simply remove the #warning (nothing wrong
> for compile-testing), turn it into a runtime warning, or
> change the Kconfig options into a menu to hide CONFIG_USB_NET_CDC_SUBSET.
> 
> Signed-off-by: Arnd Bergmann 

Applied, thanks 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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537

[Alan Stern]

On Thu, 18 Feb 2016, Daniel Fraga wrote:

> On Thu, 18 Feb 2016 14:30:15 -0500 (EST)
> Alan Stern  wrote:
> 
> > It looks like there's some problem in the usbhid driver.  Apparently
> > hid_start_in() gets some sort of error when it submits the input URB, 
> > because that URB doesn't show up in the usbmon output.
> > 
> > Can you add a debugging line near the end of hid_post_reset() in
> > drivers/hid/usbhid/hid-core.c to find out what the return value from
> > hid_start_in() is?
> 
>   Yes, can you provide the patch (or the specific line you need)
> so I can recompile it?

Something like the patch below (untested).

Alan Stern


Index: usb-4.4/drivers/hid/usbhid/hid-core.c
===
--- usb-4.4.orig/drivers/hid/usbhid/hid-core.c
+++ usb-4.4/drivers/hid/usbhid/hid-core.c
@@ -1458,6 +1458,7 @@ static int hid_post_reset(struct usb_int
spin_unlock_irq(>lock);
hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0);
status = hid_start_in(hid);
+   dev_info(>dev, "post reset hid_start_in -> %d\n", status);
if (status < 0)
hid_io_error(hid);
usbhid_restart_queues(usbhid);

--
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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537

[Daniel Fraga]

On Thu, 18 Feb 2016 14:30:15 -0500 (EST)
Alan Stern  wrote:

> It looks like there's some problem in the usbhid driver.  Apparently
> hid_start_in() gets some sort of error when it submits the input URB, 
> because that URB doesn't show up in the usbmon output.
> 
> Can you add a debugging line near the end of hid_post_reset() in
> drivers/hid/usbhid/hid-core.c to find out what the return value from
> hid_start_in() is?

Yes, can you provide the patch (or the specific line you need)
so I can recompile it?

Thanks.

-- 
Linux 4.4.1: Blurry Fish Butt
http://www.youtube.com/DanielFragaBR
http://exchangewar.info
Bitcoin: 12H6661yoLDUZaYPdah6urZS5WiXwTAUgL
--
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/5] usb: musb: core: added helper functions for parsing DT

[Sergei Shtylyov]

On 02/18/2016 11:18 AM, Felipe Balbi wrote:


This adds two functions to get DT properties "mentor,power" and "dr_mode":
musb_get_power() and musb_mode musb_get_mode()

Signed-off-by: Petr Kulhavy 

seems like I don't have patch 1/5. After fixing Sergei's comments,
please resend with his Acked-by already in place.

thanks

Hi Felipe,

I will do as soon as the patch 1/5 gets approved.
It seem to be a bit stuck at the moment as Rob Herring from the DT wants
the "mentor,power"
to be represented as a regulator, whereas Sergei and me want to stick to
the existing "mentor,power" integer property.

As soon as this get clarified I will do the final updates and send the
patch again.
Maybe this is something you can help to clarify?


I don't think that makes sense as a regulator. It's just a number which
gets passed to USB Core as is.


   Well, in case of DaVinci's it's an external GPIO controlled regulator indeed.


However, it seems like everything in kernel right now is passing it as
500. So why don't you deprecate that property, hardcode it to 500 and
avoid the problem altogether ?


   OMAP boards can only supply 100 mA, AFAIK. Isn't it too early for the 
deprecation? :-)


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: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537

[Alan Stern]

On Thu, 18 Feb 2016, Daniel Fraga wrote:

> On Thu, 18 Feb 2016 11:18:37 -0500 (EST)
> Alan Stern  wrote:
> 
> > No idea.  Can you collect a usbmon trace for bus 3, starting before the 
> > suspend and showing the problems?
> 
>   Ok Alan, I attached the requested usbmon output for bus 3 (before and 
> after S3).

It looks like there's some problem in the usbhid driver.  Apparently
hid_start_in() gets some sort of error when it submits the input URB, 
because that URB doesn't show up in the usbmon output.

Can you add a debugging line near the end of hid_post_reset() in
drivers/hid/usbhid/hid-core.c to find out what the return value from
hid_start_in() is?

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: [PATCH v5 1/1] USB: core: let USB device know device node

[Alan Stern]

On Tue, 16 Feb 2016, Peter Chen wrote:

> From: Peter Chen 
> 
> Although most of USB devices are hot-plug's, there are still some devices
> are hard wired on the board, eg, for HSIC and SSIC interface USB devices.
> If these kinds of USB devices are multiple functions, and they can supply
> other interfaces like i2c, gpios for other devices, we may need to
> describe these at device tree.
> 
> In this commit, it uses "reg" in dts as physical port number to match
> the phyiscal port number decided by USB core, if they are the same,
> then the device node is for the device we are creating for USB core.
> 
> Signed-off-by: Peter Chen 

Acked-by: 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: [PATCH v4] usb: devio: Add ioctl to disallow detaching kernel USB drivers.

[Alan Stern]

On Sun, 14 Feb 2016, Emilio López wrote:

> From: Reilly Grant 
> 
> The new USBDEVFS_DROP_PRIVILEGES ioctl allows a process to voluntarily
> relinquish the ability to issue other ioctls that may interfere with
> other processes and drivers that have claimed an interface on the
> device.
> 
> This commit also includes a simple utility to be able to test the
> ioctl, located at Documentation/usb/usbdevfs-drop-permissions.c
> 
> Example (with qemu-kvm's input device):
> 
> $ lsusb
> ...
> Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd
> 
> $ usb-devices
> ...
> C:  #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
> I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=usbhid
> 
> $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
> OK: privileges dropped!
> Available options:
> [0] Exit now
> [1] Reset device. Should fail if device is in use
> [2] Claim 4 interfaces. Should succeed where not in use
> [3] Narrow interface permission mask
> Which option shall I run?: 1
> ERROR: USBDEVFS_RESET failed! (1 - Operation not permitted)
> Which test shall I run next?: 2
> ERROR claiming if 0 (1 - Operation not permitted)
> ERROR claiming if 1 (1 - Operation not permitted)
> ERROR claiming if 2 (1 - Operation not permitted)
> ERROR claiming if 3 (1 - Operation not permitted)
> Which test shall I run next?: 0
> 
> After unbinding usbhid:
> 
> $ usb-devices
> ...
> I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=02 Driver=(none)
> 
> $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
> ...
> Which option shall I run?: 2
> OK: claimed if 0
> ERROR claiming if 1 (1 - Operation not permitted)
> ERROR claiming if 2 (1 - Operation not permitted)
> ERROR claiming if 3 (1 - Operation not permitted)
> Which test shall I run next?: 1
> OK: USBDEVFS_RESET succeeded
> Which test shall I run next?: 0
> 
> After unbinding usbhid and restricting the mask:
> 
> $ sudo ./usbdevfs-drop-permissions /dev/bus/usb/001/002
> ...
> Which option shall I run?: 3
> Insert new mask: 0
> OK: privileges dropped!
> Which test shall I run next?: 2
> ERROR claiming if 0 (1 - Operation not permitted)
> ERROR claiming if 1 (1 - Operation not permitted)
> ERROR claiming if 2 (1 - Operation not permitted)
> ERROR claiming if 3 (1 - Operation not permitted)
> 
> Signed-off-by: Reilly Grant 
> Signed-off-by: Emilio López 

Acked-by: 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: [PATCH] usb: host: ohci-pxa27x: propagate the irq error code

[Alan Stern]

On Sat, 13 Feb 2016, Robert Jarzmik wrote:

> In several drivers in the pxa architecture, it was found that the
> platform_get_irq() was not propagated. This breaks the the device-tree
> probe deferral path, if -EPROBE_DEFER is returned. Unfortunately, the
> error return in this case is transformed into -ENXIO, breaking the
> deferral mechanism.
> 
> Even if in this specific case the driver was not broken, because the
> interrupt controller is always probed before drivers, propagate the
> proper return code.
> 
> Signed-off-by: Robert Jarzmik 

Acked-by: Alan Stern 

> ---
>  drivers/usb/host/ohci-pxa27x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index e8c006e7a960..a667cf2d5788 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -435,7 +435,7 @@ int usb_hcd_pxa27x_probe (const struct hc_driver *driver, 
> struct platform_device
>   irq = platform_get_irq(pdev, 0);
>   if (irq < 0) {
>   pr_err("no resource of IORESOURCE_IRQ");
> - return -ENXIO;
> + return irq;
>   }
>  
>   usb_clk = devm_clk_get(>dev, NULL);
> 

--
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 v5 07/15] usb: ehci: add vbus-gpio parameter

[Sergei Shtylyov]

On 02/18/2016 09:06 PM, Antony Pavlov wrote:


This patch retrieves and configures the vbus control gpio via
the device tree.

This patch is based on a ehci-s5p.c commit fd81d59c90d38661
("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").

Signed-off-by: Antony Pavlov 
Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
  drivers/usb/host/ehci-platform.c | 22 ++
  1 file changed, 22 insertions(+)

diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index bd7082f2..0d95ced 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -28,6 +28,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
.power_off =ehci_platform_power_off,
  };

+static void setup_vbus_gpio(struct device *dev)
+{
+   int err;
+   int gpio;
+
+   if (!dev->of_node)
+   return;
+
+   gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
+   if (!gpio_is_valid(gpio))
+   return;
+
+   err = devm_gpio_request_one(dev, gpio,
+   GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
+   "ehci_vbus_gpio");
+   if (err)
+   dev_err(dev, "can't request ehci vbus gpio %d", gpio);



I don't understand this.  If you get an error here, what's the point of
allowing the probe to continue?  Shouldn't you return an error code so
the probe will fail?


Please ignore the 'usb: ehci: add vbus-gpio parameter' patch!

In the new AR9331 patchseries I use chipidea USB driver (thanks to Marek for 
the suggestion)
in the AR9331 dtsi-file:

 usb: usb@1b000100 {
 compatible = "chipidea,usb2";
 reg = <0x1b00 0x200>;

 interrupt-parent = <>;
 interrupts = <3>;
 resets = < 5>;

 phy-names = "usb-phy";
 phys = <_phy>;

 status = "disabled";
 };


so I use regulator in the TL-MR3020 board dts file:

 reg_usb_vbus: reg_usb_vbus {
 compatible = "regulator-fixed";
 regulator-name = "usb_vbus";
 regulator-min-microvolt = <500>;


   Not 0?


 regulator-max-microvolt = <500>;
 gpio = < 8 GPIO_ACTIVE_HIGH>;


   Where's the switch if both voltages are equal?


 enable-active-high;
 };

 {
 dr_mode = "host";
 vbus-supply = <_usb_vbus>;
 status = "okay";
};

As a result there is no need in adding vbus-gpio parameter to ehci anymore!

[...]

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 1/3] usb: fix endpoint descriptor buffer size decrement

[Alan Stern]

On Thu, 18 Feb 2016, Mathias Nyman wrote:

> An endpoint descriptor may be followed by both SuperSpeed and
> SuperSpeedPlus Isoc endpoint companion descriptors.
> These descriptors are all stored one after another in a buffer.
> 
> The new SuperSpeedPlus Isoc endpoint companion parsing incorrectly
> decreased the the remaining buffer size before comparing the size with the
> expected length of the descriptor.
> 
> This lead to possible failure in reading the SuperSpeed endpoint companion
> descriptor of the last endpoint, displaying an message like:
> 
> "No SuperSpeed endpoint companion for config 1 interface 0 altsetting 0
>  ep 129: using minimum values"
> 
> Fix it by decreasing the size after comparing it.
> 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/core/config.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 5eb1a87..47fb58c 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -76,7 +76,6 @@ static void usb_parse_ss_endpoint_companion(struct device 
> *ddev, int cfgno,
>*/
>   desc = (struct usb_ss_ep_comp_descriptor *) buffer;
>   buffer += desc->bLength;
> - size -= desc->bLength;
>  
>   if (desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP ||
>   size < USB_DT_SS_EP_COMP_SIZE) {
> @@ -101,6 +100,7 @@ static void usb_parse_ss_endpoint_companion(struct device 
> *ddev, int cfgno,
>   return;
>   }
>  
> + size -= desc->bLength;
>   memcpy(>ss_ep_comp, desc, USB_DT_SS_EP_COMP_SIZE);
>  
>   /* Check the various values */

It's generally better to change the two variables (buffer and size) at 
the same time.  At least, that's what people expect to see.  How about 
moving both statements down?

Also, wouldn't it be better in the original patch to call 
usb_parse_ssp_isoc_endpoint_companion() after all the SS endpoint 
companion stuff is finished, instead of in the middle?

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: OHCI-PCI: frame counter not updating, module disabled

[Alan Stern]

On Thu, 18 Feb 2016 m...@superbash.de wrote:

> This happens also with older kernel versions (3.x, etc.):
> 
> A small LCD display (ID 1908:0102 GEMBIRD) connected to USB is used for
> some years now. Computer (and display) runs always (24 hours/day).
> 'lcd4linux' is used to update the displays contents.
> 
> This works fine - but it fails after 1-2 days.
> 
> The log shows:
> 
> ohci-pci :00:12.0: frame counter not updating; disabled
> ohci-pci :00:12.0: HC died; cleaning up
> 
> The ohci-pci module will be disabled then, the display gets lost.

That error message indicates a problem in the OHCI hardware.  I don't 
what causes the problem or how to prevent it.

> A workaround for this is to reload this module every 24 hours
> (automated by a systemd service). So far this helps: I didn't have this
> error anymore. This workaround is tested for a few months now and it
> runs fine - but it's 'only' a workaround. ;)

That's okay.  Reinitializing the controller and the driver is a
perfectly reasonable workaround for a hardware problem.

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: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter

[Antony Pavlov]

On Thu, 18 Feb 2016 11:12:43 -0500 (EST)
Alan Stern  wrote:

> On Tue, 9 Feb 2016, Antony Pavlov wrote:
> 
> > This patch retrieves and configures the vbus control gpio via
> > the device tree.
> > 
> > This patch is based on a ehci-s5p.c commit fd81d59c90d38661
> > ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
> > 
> > Signed-off-by: Antony Pavlov 
> > Cc: Alan Stern 
> > Cc: Greg Kroah-Hartman 
> > Cc: linux-usb@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org
> > ---
> >  drivers/usb/host/ehci-platform.c | 22 ++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/usb/host/ehci-platform.c 
> > b/drivers/usb/host/ehci-platform.c
> > index bd7082f2..0d95ced 100644
> > --- a/drivers/usb/host/ehci-platform.c
> > +++ b/drivers/usb/host/ehci-platform.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
> > .power_off =ehci_platform_power_off,
> >  };
> >  
> > +static void setup_vbus_gpio(struct device *dev)
> > +{
> > +   int err;
> > +   int gpio;
> > +
> > +   if (!dev->of_node)
> > +   return;
> > +
> > +   gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
> > +   if (!gpio_is_valid(gpio))
> > +   return;
> > +
> > +   err = devm_gpio_request_one(dev, gpio,
> > +   GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
> > +   "ehci_vbus_gpio");
> > +   if (err)
> > +   dev_err(dev, "can't request ehci vbus gpio %d", gpio);
>
>
> I don't understand this.  If you get an error here, what's the point of 
> allowing the probe to continue?  Shouldn't you return an error code so 
> the probe will fail?

Please ignore the 'usb: ehci: add vbus-gpio parameter' patch!

In the new AR9331 patchseries I use chipidea USB driver (thanks to Marek for 
the suggestion)
in the AR9331 dtsi-file:

usb: usb@1b000100 {
compatible = "chipidea,usb2";
reg = <0x1b00 0x200>;

interrupt-parent = <>;
interrupts = <3>;
resets = < 5>;

phy-names = "usb-phy";
phys = <_phy>;

status = "disabled";
};


so I use regulator in the TL-MR3020 board dts file:

reg_usb_vbus: reg_usb_vbus {
compatible = "regulator-fixed";
regulator-name = "usb_vbus";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
gpio = < 8 GPIO_ACTIVE_HIGH>;
enable-active-high;
};

 {
dr_mode = "host";
vbus-supply = <_usb_vbus>;
status = "okay";
};

As a result there is no need in adding vbus-gpio parameter to ehci anymore!

> > +}
> > +
> >  static int ehci_platform_probe(struct platform_device *dev)
> >  {
> > struct usb_hcd *hcd;
> > @@ -174,6 +194,8 @@ static int ehci_platform_probe(struct platform_device 
> > *dev)
> > return irq;
> > }
> >  
> > +   setup_vbus_gpio(>dev);
> > +
> > hcd = usb_create_hcd(_platform_hc_driver, >dev,
> >  dev_name(>dev));
> > if (!hcd)
> > 
> 


-- 
-- 
Best regards,
  Antony Pavlov
--
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 v5 07/15] usb: ehci: add vbus-gpio parameter

[Marek Vasut]

On 02/18/2016 05:12 PM, Alan Stern wrote:
> On Tue, 9 Feb 2016, Antony Pavlov wrote:
> 
>> This patch retrieves and configures the vbus control gpio via
>> the device tree.
>>
>> This patch is based on a ehci-s5p.c commit fd81d59c90d38661
>> ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
>>
>> Signed-off-by: Antony Pavlov 
>> Cc: Alan Stern 
>> Cc: Greg Kroah-Hartman 
>> Cc: linux-usb@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>>  drivers/usb/host/ehci-platform.c | 22 ++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/usb/host/ehci-platform.c 
>> b/drivers/usb/host/ehci-platform.c
>> index bd7082f2..0d95ced 100644
>> --- a/drivers/usb/host/ehci-platform.c
>> +++ b/drivers/usb/host/ehci-platform.c
>> @@ -28,6 +28,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
>>  .power_off =ehci_platform_power_off,
>>  };
>>  
>> +static void setup_vbus_gpio(struct device *dev)
>> +{
>> +int err;
>> +int gpio;
>> +
>> +if (!dev->of_node)
>> +return;
>> +
>> +gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
>> +if (!gpio_is_valid(gpio))
>> +return;
>> +
>> +err = devm_gpio_request_one(dev, gpio,
>> +GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
>> +"ehci_vbus_gpio");
>> +if (err)
>> +dev_err(dev, "can't request ehci vbus gpio %d", gpio);
> 
> I don't understand this.  If you get an error here, what's the point of 
> allowing the probe to continue?  Shouldn't you return an error code so 
> the probe will fail?

The idea is I believe that if there is no vbus gpio specified, the port
might just not have vbus control, so the probe can continue. But this
patch is irrelevant anyway, since Alexey will switch to CI HDRC driver
and use standard regulator, as it should be done.


--
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: NEC uPD720200 xHCI Controller dies when Runtime PM enabled

[Mike Murdoch]

On 2016-02-18 16:12, Mathias Nyman wrote:
> On 16.02.2016 23:58, main.ha...@googlemail.com wrote:
>>
>>
>> On 2016-02-08 15:31, Mathias Nyman wrote:
>>> Hi
>>>
>>> On 06.02.2016 19:08, Mike Murdoch wrote:
 Bug ID: 111251

 Hello,

 I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on
 kernel 4.4.1-gentoo.

 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
 Controller (rev 04) (prog-if 30 [XHCI])
   Subsystem: Lenovo uPD720200 USB 3.0 Host Controller

 When runtime power control for this controller is disabled
 (/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller
 works fine and reaches over 120MB/s transfer rates.

 When runtime power control for this controller is enabled
 (/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects
 can be observed:

 - Transfer rates are much lower at around 30MB/s
 - During transfers, the controller dies after a couple of seconds:

 xhci_hcd :0e:00.0: xHCI host not responding to stop endpoint
 command.
 xhci_hcd :0e:00.0: Assuming host is dying, halting host.
 xhci_hcd :0e:00.0: Host not halted after 16000 microseconds.
 xhci_hcd :0e:00.0: Non-responsive xHCI host is not halting.
 xhci_hcd :0e:00.0: Completing active URBs anyway.
 xhci_hcd :0e:00.0: HC died; cleaning up
 sd 9:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR
 driverbyte=DRIVER_OK
 sd 9:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 19 a9 00 00 00 f0 00
 blk_update_request: I/O error, dev sdc, sector 1681664
 xhci_hcd :0e:00.0: Stopped the command ring failed, maybe the host
 is dead
 xhci_hcd :0e:00.0: Host not halted after 16000 microseconds.
 xhci_hcd :0e:00.0: Abort command ring failed
 xhci_hcd :0e:00.0: HC died; cleaning up

 At this point, a reboot is required to reactivate the controller,
 unloading and reloading the xhci_* modules does not work.

>>>
>>> With 120MB/s I assume it was a USB3 device.
>>> Was there any USB 2 device connected as well?
>>> Does this occur with only a USB2 device connected to xhci?
>>>
>>> xhci handles suspend/resume a bit differently for USB2 and USB3
>>> roothubs.
>>>
>>> Does this happen on older kernels as well? 4.3 or 4.2 based?
>>>
>>> For more xhci debugging, do:
>>> echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
>>> and check dmesg for more xhci info.
>>>
>>> If reloading the module did not help it is more likely that the
>>> controller is in some
>>> unexpected state.
>>> If however, it would instead be just bad timeout timer handling we
>>> could just return immediately
>>> in the timeout handler, and check if the usb device(s) continue to
>>> work normally.
>>>
>>> This could be done by editing drivers/usb/hosts/xhci-ring.c
>>>
>>> +++ b/drivers/usb/host/xhci-ring.c
>>> @@ -831,6 +831,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned
>>> long arg)
>>>  struct xhci_virt_ep *ep;
>>>  int ret, i, j;
>>>  unsigned long flags;
>>> +   return;
>>>
>>> -Mathias
>>>
>>>
>> Hello Mat,
>>
>> thanks for your response. I have experimented with your suggestions.
>>
>> As for your questions: No, there was only one USB3 stick connected to
>> the host controller during the tests. USB2 devices work fine too.
>>
>> Yes, I encountered this problem on a 4.1 series kernel aswell as the 4.4
>> series.
>>
>> I have enabled the debug controls and attached the results to this mail,
>> along with some commentary. I am hoping this works in the mailing list.
>>
>> I've also tried your suggested modification, and it does seem to work!
>> With it, the controller does not die, but it still sacrifices a lot of
>> speed (as I had mentioned in the first mail of this thread)
>>
>>
>> I hope this is helpful!
>>
>
> Thanks, it is helpful
>
> Looks like when the USB3 device is inserted it is first detected as a
> USB2 device,
> then immediately afterwars as a USB3 device, the usb2 device stops
> responding so 5
> seconds later we timeout, and kill everything.
>
> selected parts of the log:
>
> inserting usb3 storage device
> 20:03:33 xhci_hcd :0e:00.0: xhci_resume: starting port polling.
> 20:03:33 xhci_hcd :0e:00.0: Port Status Change Event for port 3
> 20:03:33 xhci_hcd :0e:00.0: get port status, actual port 0 status 
> = 0x202e1  /* PORT 0
> 20:03:33 xhci_hcd :0e:00.0: get port status, actual port 1 status 
> = 0x2a0 /* PORT 1
> 20:03:33 usb 1-1: new high-speed USB device number 2 using xhci_hcd
> 20:03:33 xhci_hcd :0e:00.0: Slot ID 1 Input Context:/*
> Found a HS device
> 20:03:33 xhci_hcd :0e:00.0: IN Endpoint 00 Context (ep_index 00):
> 20:03:33 xhci_hcd :0e:00.0: @8805fc8a5048 (virt) @a048
> (dma) 0xfffdf001 - deq
> 20:03:33 xhci_hcd :0e:00.0: Successful setup context command
>  *   now we have a device at SLOT 

Re: "reset full-speed USB device number 6 using ehci-pci" with Dell Inspiron 15R 5537

[Alan Stern]

On Tue, 9 Feb 2016, Dâniel Fraga wrote:

>   I'm using Linux 4.4.1 kernel on a Dell Inspiron 15R 5537 and
> everytime I wake up from S3 sleep, the syslog is full of the following
> message:
> 
> Feb  9 17:19:05 tux kernel: [19196.253206] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:05 tux kernel: [19196.406206] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:05 tux kernel: [19196.559239] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:06 tux kernel: [19197.738267] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:06 tux kernel: [19197.891218] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:07 tux kernel: [19198.044243] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:07 tux kernel: [19198.198231] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:08 tux kernel: [19199.388295] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:08 tux kernel: [19199.541307] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:08 tux kernel: [19199.694319] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:08 tux kernel: [19199.847338] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> Feb  9 17:19:09 tux kernel: [19201.026362] usb 3-1.6: reset full-speed USB 
> device number 6 using ehci-pci
> 
>   And it doesn't stop. The only way to stop it is to reload the ehci-pci 
> kernel module:
> 
> sudo modprobe -r ehci-pci; sudo modprobe ehci-pci
> 
>   lsusb returns:
> 
> Bus 003 Device 008: ID 064e:812e Suyin Corp. 
> Bus 003 Device 007: ID 0bda:0129 Realtek Semiconductor Corp. 
> Bus 003 Device 006: ID 04f3:0034 Elan Microelectronics Corp. 
> Bus 003 Device 005: ID 0cf3:0036 Atheros Communications, Inc. 
> Bus 003 Device 004: ID 1532:0016 Razer USA, Ltd 
> Bus 003 Device 003: ID 046d:c316 Logitech, Inc. HID-Compliant Keyboard
> Bus 003 Device 002: ID 8087:8000  
> Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 001 Device 006: ID 046d:0990 Logitech, Inc. QuickCam Pro 9000
> Bus 001 Device 005: ID 0424:2228 Standard Microsystems Corp. 9-in-2 Card 
> Reader
> Bus 001 Device 004: ID 0424:2602 Standard Microsystems Corp. 
> Bus 001 Device 003: ID 0424:2512 Standard Microsystems Corp. 
> Bus 001 Device 007: ID 051d:0002 American Power Conversion Uninterruptible 
> Power Supply
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> 
>   So, any hints on what's going on?

No idea.  Can you collect a usbmon trace for bus 3, starting before the 
suspend and showing the problems?

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: [RFC v5 07/15] usb: ehci: add vbus-gpio parameter

[Alan Stern]

On Tue, 9 Feb 2016, Antony Pavlov wrote:

> This patch retrieves and configures the vbus control gpio via
> the device tree.
> 
> This patch is based on a ehci-s5p.c commit fd81d59c90d38661
> ("USB: ehci-s5p: Add vbus setup function to the s5p ehci glue layer").
> 
> Signed-off-by: Antony Pavlov 
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Cc: linux-usb@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/usb/host/ehci-platform.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/usb/host/ehci-platform.c 
> b/drivers/usb/host/ehci-platform.c
> index bd7082f2..0d95ced 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -142,6 +143,25 @@ static struct usb_ehci_pdata ehci_platform_defaults = {
>   .power_off =ehci_platform_power_off,
>  };
>  
> +static void setup_vbus_gpio(struct device *dev)
> +{
> + int err;
> + int gpio;
> +
> + if (!dev->of_node)
> + return;
> +
> + gpio = of_get_named_gpio(dev->of_node, "vbus-gpio", 0);
> + if (!gpio_is_valid(gpio))
> + return;
> +
> + err = devm_gpio_request_one(dev, gpio,
> + GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
> + "ehci_vbus_gpio");
> + if (err)
> + dev_err(dev, "can't request ehci vbus gpio %d", gpio);

I don't understand this.  If you get an error here, what's the point of 
allowing the probe to continue?  Shouldn't you return an error code so 
the probe will fail?

Alan Stern

> +}
> +
>  static int ehci_platform_probe(struct platform_device *dev)
>  {
>   struct usb_hcd *hcd;
> @@ -174,6 +194,8 @@ static int ehci_platform_probe(struct platform_device 
> *dev)
>   return irq;
>   }
>  
> + setup_vbus_gpio(>dev);
> +
>   hcd = usb_create_hcd(_platform_hc_driver, >dev,
>dev_name(>dev));
>   if (!hcd)
> 

--
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: [Bug 112171] UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:873:47

[Alan Stern]

On Tue, 9 Feb 2016, Yaroslav Molochko wrote:

> After upgrade to  4.5.0-rc3 I've got following stack trace, never seen
> that before:
> 
> Feb 08 22:58:56 x kernel: UBSAN: Undefined behaviour in
> drivers/usb/host/ehci-hub.c:873:47
> Feb 08 22:58:56 x kernel: index -1 is out of range for type 'u32 [1]'
> Feb 08 22:58:56 x kernel: CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.5.0-rc3 #98
> Feb 08 22:58:56 x kernel: Hardware name: LENOVO 20BSCTO1WW/20BSCTO1WW,
> BIOS N14ET32W (1.10 ) 08/13/2015
> Feb 08 22:58:56 x kernel:   84b750fa
> 880224deb358 81832ca4
> Feb 08 22:58:56 x kernel:   84b750fa
> 880224deb380 
> Feb 08 22:58:56 x kernel:  880224deb370 818a6360
> 833ea920 880224deb3c8
> Feb 08 22:58:56 x kernel: Call Trace:
> Feb 08 22:58:56 x kernel:  [] dump_stack+0xaf/0x10c
> Feb 08 22:58:56 x kernel:  [] ubsan_epilogue+0x14/0x56
> Feb 08 22:58:56 x kernel:  []
> __ubsan_handle_out_of_bounds+0x86/0xb3
> Feb 08 22:58:56 x kernel:  [] ehci_hub_control+0xcf/0x141e

I have no idea why this didn't show up earlier.  In any case it doesn't 
matter; the -1 index is part of an address calculation for a pointer 
that doesn't get used in this pathway.

This seems to be a common problem with UBSAN.  It spots invalid or 
questionable calculations in situations where they make no difference 
because the results don't get used.

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

[PATCH 3/3] usb: host: xhci-plat: fix of_table.cocci warnings

[Mathias Nyman]

From: Julia Lawall 

Make sure (of/i2c/platform)_device_id tables are NULL terminated

Generated by: scripts/coccinelle/misc/of_table.cocci

Signed-off-by: Fengguang Wu 
Signed-off-by: Julia Lawall 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-plat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3aede6e..5c15e9b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -116,6 +116,7 @@ static const struct of_device_id usb_xhci_of_match[] = {
.compatible = "renesas,rcar-gen3-xhci",
.data = _plat_renesas_rcar_gen3,
},
+   {},
 };
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 #endif
-- 
1.9.1

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

[PATCH 0/3] usb and xhci fixes for usb-next 4.6

[Mathias Nyman]

Hi Greg

A few more patches for 4.6.
At least the first one would be important to get with the rest of the
USB 3.1 SSP Isoc patches to 4.6
Without it the SSP Isoc endpoint companion descriptor parsing
may mess up the normal SS endpoint companion parsing.

Julia Lawall (1):
  usb: host: xhci-plat: fix of_table.cocci warnings

Mathias Nyman (1):
  usb: fix endpoint descriptor buffer size decrement

Simon Horman (1):
  usb: host: xhci-plat: add R-Car Gen2 and Gen3 fallback compatibility
strings

 Documentation/devicetree/bindings/usb/usb-xhci.txt | 21 +
 drivers/usb/core/config.c  |  2 +-
 drivers/usb/host/xhci-plat.c   |  6 ++
 3 files changed, 24 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[PATCH 1/3] usb: fix endpoint descriptor buffer size decrement

[Mathias Nyman]

An endpoint descriptor may be followed by both SuperSpeed and
SuperSpeedPlus Isoc endpoint companion descriptors.
These descriptors are all stored one after another in a buffer.

The new SuperSpeedPlus Isoc endpoint companion parsing incorrectly
decreased the the remaining buffer size before comparing the size with the
expected length of the descriptor.

This lead to possible failure in reading the SuperSpeed endpoint companion
descriptor of the last endpoint, displaying an message like:

"No SuperSpeed endpoint companion for config 1 interface 0 altsetting 0
 ep 129: using minimum values"

Fix it by decreasing the size after comparing it.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/core/config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 5eb1a87..47fb58c 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -76,7 +76,6 @@ static void usb_parse_ss_endpoint_companion(struct device 
*ddev, int cfgno,
 */
desc = (struct usb_ss_ep_comp_descriptor *) buffer;
buffer += desc->bLength;
-   size -= desc->bLength;
 
if (desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP ||
size < USB_DT_SS_EP_COMP_SIZE) {
@@ -101,6 +100,7 @@ static void usb_parse_ss_endpoint_companion(struct device 
*ddev, int cfgno,
return;
}
 
+   size -= desc->bLength;
memcpy(>ss_ep_comp, desc, USB_DT_SS_EP_COMP_SIZE);
 
/* Check the various values */
-- 
1.9.1

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

[PATCH 2/3] usb: host: xhci-plat: add R-Car Gen2 and Gen3 fallback compatibility strings

[Mathias Nyman]

From: Simon Horman 

Add fallback compatibility strings for R-Car Gen2 and Gen3.

This is in keeping with the fallback scheme being adopted wherever
appropriate for drivers for Renesas SoCs.

Signed-off-by: Simon Horman 
Acked-by: Geert Uytterhoeven 
Acked-by: Gregory CLEMENT 
Signed-off-by: Mathias Nyman 
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt | 21 +
 drivers/usb/host/xhci-plat.c   |  5 +
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 0825732..6a17aa8 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -1,10 +1,23 @@
 USB xHCI controllers
 
 Required properties:
-  - compatible: should be one of "generic-xhci",
-"marvell,armada-375-xhci", "marvell,armada-380-xhci",
-"renesas,xhci-r8a7790", "renesas,xhci-r8a7791", "renesas,xhci-r8a7793",
-"renesas,xhci-r8a7795" (deprecated: "xhci-platform").
+  - compatible: should be one or more of
+
+- "generic-xhci" for generic XHCI device
+- "marvell,armada-375-xhci" for Armada 375 SoCs
+- "marvell,armada-380-xhci" for Armada 38x SoCs
+- "renesas,xhci-r8a7790" for r8a7790 SoC
+- "renesas,xhci-r8a7791" for r8a7791 SoC
+- "renesas,xhci-r8a7793" for r8a7793 SoC
+- "renesas,xhci-r8a7795" for r8a7795 SoC
+- "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 compatible device
+- "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
+- "xhci-platform" (deprecated)
+
+When compatible with the generic version, nodes must list the
+SoC-specific version corresponding to the platform first
+followed by the generic version.
+
   - reg: should contain address and length of the standard XHCI
 register set for the device.
   - interrupts: one XHCI interrupt should be described here.
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d39d6bf..3aede6e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -110,6 +110,11 @@ static const struct of_device_id usb_xhci_of_match[] = {
.compatible = "renesas,xhci-r8a7795",
.data = _plat_renesas_rcar_gen3,
}, {
+   .compatible = "renesas,rcar-gen2-xhci",
+   .data = _plat_renesas_rcar_gen2,
+   }, {
+   .compatible = "renesas,rcar-gen3-xhci",
+   .data = _plat_renesas_rcar_gen3,
},
 };
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
-- 
1.9.1

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

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Heikki Krogerus]

On Thu, Feb 18, 2016 at 02:44:25PM +0100, Oliver Neukum wrote:
> On Thu, 2016-02-18 at 15:25 +0200, Heikki Krogerus wrote:
> 
> Hi,
> 
> 
> > > We have port directories for port power switching. How is
> > > the connector directory linked to them?
> > 
> > I'm sorry, I don't think I understand this point.
> 
> Like this:
> 
> oneukum@linux-dtbq:/sys/bus/usb/devices/3-0:1.0> ls -l
> total 0
> -rw-r--r-- 1 root root 4096 Feb 18 14:34 authorized
> -r--r--r-- 1 root root 4096 Feb 18 14:34 bAlternateSetting
> -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceClass
> -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceNumber
> -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceProtocol
> -r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceSubClass
> -r--r--r-- 1 root root 4096 Feb 18 14:34 bNumEndpoints
> lrwxrwxrwx 1 root root0 Feb 17 15:59 driver
> -> ../../../../../bus/usb/drivers/hub
> drwxr-xr-x 3 root root0 Feb 18 09:35 ep_81
> -r--r--r-- 1 root root 4096 Feb 18 14:34 modalias
> drwxr-xr-x 2 root root0 Feb 18 09:35 power
> lrwxrwxrwx 1 root root0 Feb 17 15:59 subsystem
> -> ../../../../../bus/usb
> -r--r--r-- 1 root root 4096 Feb 18 14:34 supports_autosuspend
> -rw-r--r-- 1 root root 4096 Feb 18 14:34 uevent
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port1
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port10
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port11
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port12
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port13
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port14
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port15
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port2
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port3
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port4
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port5
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port6
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port7
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port8
> drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port9
> 
> usb*-port*
> 
> They correspond to the connectors a system has.
> It seems to me that we want a link connecting them
> if the correspondance is known.

Ah, got it. In case of ACPI enumerated UCSI, we will have the actual
ACPI device object for the port as a child device object. So when we
attach the port ACPI companion to the connector device we create in
the class, it will link us directly to the correct usb*-port*.

I have not done it so far because the same port ACPI device object
will also be bound to the usb peripheral once it gets enumerated, and
I was worried if that would cause a problem. But after talking to guys
that know more about ACPI then I do, I'm sure that is not going to be
a problem.

With ACPI, the binding should happen the same way even without UCSI.
What ever device driver registers the connector device should have the
port ACPI device object as it's child. So I'm thinking about doing
this in typec_register_port() and not in UCSI driver.


Thanks,

-- 
heikki
--
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: NEC uPD720200 xHCI Controller dies when Runtime PM enabled

[Mathias Nyman]

On 16.02.2016 23:58, main.ha...@googlemail.com wrote:



On 2016-02-08 15:31, Mathias Nyman wrote:

Hi

On 06.02.2016 19:08, Mike Murdoch wrote:

Bug ID: 111251

Hello,

I have a NEC uPD720200 USB3.0 controller in a Thinkpad W520 laptop on
kernel 4.4.1-gentoo.

0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
Controller (rev 04) (prog-if 30 [XHCI])
  Subsystem: Lenovo uPD720200 USB 3.0 Host Controller

When runtime power control for this controller is disabled
(/sys/bus/pci/devices/:0e:00.0/power/control = on), the controller
works fine and reaches over 120MB/s transfer rates.

When runtime power control for this controller is enabled
(/sys/bus/pci/devices/:0e:00.0/power/control = auto), two effects
can be observed:

- Transfer rates are much lower at around 30MB/s
- During transfers, the controller dies after a couple of seconds:

xhci_hcd :0e:00.0: xHCI host not responding to stop endpoint
command.
xhci_hcd :0e:00.0: Assuming host is dying, halting host.
xhci_hcd :0e:00.0: Host not halted after 16000 microseconds.
xhci_hcd :0e:00.0: Non-responsive xHCI host is not halting.
xhci_hcd :0e:00.0: Completing active URBs anyway.
xhci_hcd :0e:00.0: HC died; cleaning up
sd 9:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR
driverbyte=DRIVER_OK
sd 9:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 19 a9 00 00 00 f0 00
blk_update_request: I/O error, dev sdc, sector 1681664
xhci_hcd :0e:00.0: Stopped the command ring failed, maybe the host
is dead
xhci_hcd :0e:00.0: Host not halted after 16000 microseconds.
xhci_hcd :0e:00.0: Abort command ring failed
xhci_hcd :0e:00.0: HC died; cleaning up

At this point, a reboot is required to reactivate the controller,
unloading and reloading the xhci_* modules does not work.



With 120MB/s I assume it was a USB3 device.
Was there any USB 2 device connected as well?
Does this occur with only a USB2 device connected to xhci?

xhci handles suspend/resume a bit differently for USB2 and USB3 roothubs.

Does this happen on older kernels as well? 4.3 or 4.2 based?

For more xhci debugging, do:
echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control
and check dmesg for more xhci info.

If reloading the module did not help it is more likely that the
controller is in some
unexpected state.
If however, it would instead be just bad timeout timer handling we
could just return immediately
in the timeout handler, and check if the usb device(s) continue to
work normally.

This could be done by editing drivers/usb/hosts/xhci-ring.c

+++ b/drivers/usb/host/xhci-ring.c
@@ -831,6 +831,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned
long arg)
 struct xhci_virt_ep *ep;
 int ret, i, j;
 unsigned long flags;
+   return;

-Mathias



Hello Mat,

thanks for your response. I have experimented with your suggestions.

As for your questions: No, there was only one USB3 stick connected to
the host controller during the tests. USB2 devices work fine too.

Yes, I encountered this problem on a 4.1 series kernel aswell as the 4.4
series.

I have enabled the debug controls and attached the results to this mail,
along with some commentary. I am hoping this works in the mailing list.

I've also tried your suggested modification, and it does seem to work!
With it, the controller does not die, but it still sacrifices a lot of
speed (as I had mentioned in the first mail of this thread)


I hope this is helpful!



Thanks, it is helpful

Looks like when the USB3 device is inserted it is first detected as a USB2 
device,
then immediately afterwars as a USB3 device, the usb2 device stops responding 
so 5
seconds later we timeout, and kill everything.

selected parts of the log:

inserting usb3 storage device
20:03:33 xhci_hcd :0e:00.0: xhci_resume: starting port polling.
20:03:33 xhci_hcd :0e:00.0: Port Status Change Event for port 3
20:03:33 xhci_hcd :0e:00.0: get port status, actual port 0 status  = 
0x202e1  /* PORT 0
20:03:33 xhci_hcd :0e:00.0: get port status, actual port 1 status  = 0x2a0  
   /* PORT 1
20:03:33 usb 1-1: new high-speed USB device number 2 using xhci_hcd
20:03:33 xhci_hcd :0e:00.0: Slot ID 1 Input Context:
/* Found a HS device
20:03:33 xhci_hcd :0e:00.0: IN Endpoint 00 Context (ep_index 00):
20:03:33 xhci_hcd :0e:00.0: @8805fc8a5048 (virt) @a048 (dma) 
0xfffdf001 - deq
20:03:33 xhci_hcd :0e:00.0: Successful setup context command
 *   now we have a device at SLOT 1 with control endpoint 0 buffer at address  
0xfffdf000
20:03:33 xhci_hcd :0e:00.0: Slot ID 2 Input Context:
20:03:33 xhci_hcd :0e:00.0: IN Endpoint 00 Context (ep_index 00):
20:03:33 xhci_hcd :0e:00.0: @8800b68d7048 (virt) @2048 (dma) 
0xfffe1001 - deq
 * now we have another device at SLOT 2 with control endpoint buffer at 
0xfffe1000
20:03:33 usb 2-1: new SuperSpeed USB device number 3 using xhci_hcd /* 
found SS device
20:03:33 usb 2-1: 

Re: [PATCH v5 1/1] USB: core: let USB device know device node

[Rob Herring]

On Tue, Feb 16, 2016 at 04:42:52PM +0800, Peter Chen wrote:
> From: Peter Chen 
> 
> Although most of USB devices are hot-plug's, there are still some devices
> are hard wired on the board, eg, for HSIC and SSIC interface USB devices.
> If these kinds of USB devices are multiple functions, and they can supply
> other interfaces like i2c, gpios for other devices, we may need to
> describe these at device tree.
> 
> In this commit, it uses "reg" in dts as physical port number to match
> the phyiscal port number decided by USB core, if they are the same,
> then the device node is for the device we are creating for USB core.
> 
> Signed-off-by: Peter Chen 
> ---
> Changes for v5:
> - Refine the code how to get the device node at usb_alloc_dev according
>   to Alan's comment.
> - Point out "usbVID,PID" is the recommented compatible, and others
>   compatibles at binding doc could also be used.
> 
> Changes for v4:
> - The range of "reg" should be 1-31, changing device node address
>   style as in lower case hexadecimal with leading zeroes suppressed
>   [binding doc, usb-device.txt]
> - Improve the example at binding doc, it describes node from the top
>   (the controller)
> - Delete the struct of_node * within struct usb_device
> - Using usb_hcd_find_raw_port_number to get raw port number under root
>   hub port
> 
> Changes for v3:
> - typo: s/descirbe/describe/
> 
> Changes for v2:
> - Fix build error reported by kbuild robot, lack of "static" for
>   inline usb_of_get_child_node
> - Fix typo, "devcie_node" -> "device_node"
> - Add kernel-doc for of_node at struct usb_device
> 
> Changes from RFC:
> - Fix the error address for binding doc, and add compatible for binding doc
> - Change get child node API from "usb_of_find_node" to
>   "usb_of_get_child_node"
> - Delete unecessary header files
> - One typo
> 
>  .../devicetree/bindings/usb/usb-device.txt | 26 

Looks good to me. Thanks for driving this to completion.

Acked-by: Rob Herring 

>  drivers/usb/core/Makefile  |  2 +-
>  drivers/usb/core/of.c  | 47 
> ++
>  drivers/usb/core/usb.c | 10 +
>  include/linux/usb/of.h |  7 
>  5 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-device.txt
>  create mode 100644 drivers/usb/core/of.c
> 
--
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: Spansion's CMSIS-DAP + COM Port

[Oliver Neukum]

On Thu, 2016-02-18 at 13:05 +0100, Johan Hovold wrote:
> > I have a Starter Kit SK-FM4-176L-S6SE2CC
> > modprobe  usbserial vendor=0x1a6a product=0x2000
> > 
> > makes the USB Port appear nicely (works with gtkterm) and at the
> same
> > time one can use the CMIS-DAP  adapter with openocd.
> > 
> > As suggested by the driver I would like to recommend the
> > vendor/product IDs to be added.
> 
> This looks like a cdc-acm device to me. Any reason why you can't use
> that driver? If the driver is available, you should get a /dev/ttyACMn
> device node. If not, could you post the logs from when plugging the
> device in?

It definitely looks like that. What kind of device is it?

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

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Oliver Neukum]

On Thu, 2016-02-18 at 15:25 +0200, Heikki Krogerus wrote:

Hi,


> > We have port directories for port power switching. How is
> > the connector directory linked to them?
> 
> I'm sorry, I don't think I understand this point.

Like this:

oneukum@linux-dtbq:/sys/bus/usb/devices/3-0:1.0> ls -l
total 0
-rw-r--r-- 1 root root 4096 Feb 18 14:34 authorized
-r--r--r-- 1 root root 4096 Feb 18 14:34 bAlternateSetting
-r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceClass
-r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceNumber
-r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceProtocol
-r--r--r-- 1 root root 4096 Feb 18 14:34 bInterfaceSubClass
-r--r--r-- 1 root root 4096 Feb 18 14:34 bNumEndpoints
lrwxrwxrwx 1 root root0 Feb 17 15:59 driver
-> ../../../../../bus/usb/drivers/hub
drwxr-xr-x 3 root root0 Feb 18 09:35 ep_81
-r--r--r-- 1 root root 4096 Feb 18 14:34 modalias
drwxr-xr-x 2 root root0 Feb 18 09:35 power
lrwxrwxrwx 1 root root0 Feb 17 15:59 subsystem
-> ../../../../../bus/usb
-r--r--r-- 1 root root 4096 Feb 18 14:34 supports_autosuspend
-rw-r--r-- 1 root root 4096 Feb 18 14:34 uevent
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port1
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port10
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port11
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port12
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port13
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port14
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port15
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port2
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port3
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port4
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port5
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port6
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port7
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port8
drwxr-xr-x 3 root root0 Feb 18 09:35 usb3-port9

usb*-port*

They correspond to the connectors a system has.
It seems to me that we want a link connecting them
if the correspondance is known.

> > Likewise, if we have USB PD, we have to know how that
> > is linked to the connector directory.
> 
> So you mean when we have USB PD PHY or controller, right? That
> will be the parent of the connector device if we have one on the
> platform.

So the parentage is different on whether a PD controller is present?
That needs to be documented. And so we cannot deal with separate modules
for a PD driver?

[..]
> > Furthermore, do these files support poll?

At least the current role and mode can change, so in principle
poll() makes sense.

> > And lastly we can get "Attention" as a message connected
> > with a connector in an alternate mode. How does user space
> > learn about that?
> 
> The class should notify the userspace with uevent on
> connection/disconnection regardless what is being connected, or what
> mode the connector enters initially.

Yes, but "Attention" in the sense of 6.4.4.3.6 of the PD spec.
Does this need to be handled in the kernel? Do we generate a uevent
for that?

> So do you want to see that explained in the ABI document?

No.

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

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Heikki Krogerus]

Hi,

On Thu, Feb 18, 2016 at 10:35:41AM +0100, Oliver Neukum wrote:
> On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote:
> 
> Hi,
> 
> > P.S. That reminds me, here's my current draft for the
> > Documentation/ABI/. Could you take a look?
> 
> And I am afraid, that I have a few remarks not bound
> to a specific entry.
> 
> We have port directories for port power switching. How is
> the connector directory linked to them?

I'm sorry, I don't think I understand this point.

> Likewise, if we have USB PD, we have to know how that
> is linked to the connector directory.

So you mean when we have USB PD PHY or controller, right? That
will be the parent of the connector device if we have one on the
platform.

I think I'm misunderstanding this point as well..

> In addition, writes to those files have results. We need
> the error codes to be described.

Yes, I need to document those.

> Furthermore, do these files support poll?
> 
> And lastly we can get "Attention" as a message connected
> with a connector in an alternate mode. How does user space
> learn about that?

The class should notify the userspace with uevent on
connection/disconnection regardless what is being connected, or what
mode the connector enters initially.

So do you want to see that explained in the ABI document?

The uevent does not contain any details, but I thought that it's OK to
expect the userspace to read that separately. If this is not OK, let's
add a new uevent variable that specifies what was just connected.

I hope I did not misunderstand also this one.

> I am sorry to be this obnoxious, but this is an API which
> will be with us for a long time, so we better get it right.

I would not say you are being obnoxious. If you are, feel free :).
Your input is most welcome. Thanks a lot for that. And I agree, we
need to make this solid from the beginning.


Thanks,

-- 
heikki
--
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 05/10] usb: dbc: add bulk out and bulk in interfaces

[Mathias Nyman]

On 26.01.2016 14:58, Lu Baolu wrote:

This patch adds interfaces for bulk out and bulk in ops. These
interfaces could be used to implement early printk bootconsole
or hook to various system debuggers.

Signed-off-by: Lu Baolu 
---
  drivers/usb/early/xhci-dbc.c | 373 +++
  include/linux/usb/xhci-dbc.h |  30 
  2 files changed, 403 insertions(+)



...


+
+/*
+ * Check and dispatch events in event ring. It also checks status
+ * of hardware. This function will be called from multiple threads.
+ * An atomic lock is applied to protect the access of event ring.
+ */
+static int xdbc_check_event(void)
+{
+   /* event ring is under checking by other thread? */
+   if (!test_bit(XDBC_ATOMIC_EVENT, >atomic_flags) &&
+   !test_and_set_bit(XDBC_ATOMIC_EVENT,
+   >atomic_flags))
+   return 0;


homemade trylock, can't the real ones be used?


+
+   xdbc_handle_events();
+
+   test_and_clear_bit(XDBC_ATOMIC_EVENT, >atomic_flags);
+
+   return 0;
 +}
+
+#defineBULK_IN_COMPLETED(p)((xdbcp->in_pending == (p)) && \
+xdbcp->in_complete)
+#defineBULK_OUT_COMPLETED(p)   ((xdbcp->out_pending == (p)) && \
+xdbcp->out_complete)
+


...


+}
+
+int xdbc_bulk_read(void *data, int size, int loops)
+{
+   int ret;
+
+   do {
+   if (!test_bit(XDBC_ATOMIC_BULKIN, >atomic_flags) &&
+   !test_and_set_bit(XDBC_ATOMIC_BULKIN,
+   >atomic_flags))
+   break;
+   } while (1);


homemeade spin_lock, can't the real one be used?

If the xdbc_bulk_write() can be accessed from interrupt context (handler, soft, 
timer) it
may deadlock


+
+   ret = xdbc_bulk_transfer(data, size, loops, true);
+
+   test_and_clear_bit(XDBC_ATOMIC_BULKIN, >atomic_flags);
+
+   return ret;
+}
+
+int xdbc_bulk_write(const char *bytes, int size)
+{
+   int ret;
+
+   do {
+   if (!test_bit(XDBC_ATOMIC_BULKOUT, >atomic_flags) &&
+   !test_and_set_bit(XDBC_ATOMIC_BULKOUT,
+   >atomic_flags))
+   break;
+   } while (1);


Another homemeade spin_lock, can't the real one be used?

same issue here, deadlock if accessible from interrupt context.


Would it make sense to have only one spinlock, and start one separate thread for
reading the event ring. The thread would,  lock, handle pending events, unlock,
then call shedule, in a loop. ehci early debug code has some variant of this.

So the lock would be taken while events are being handled.

The same lock would be used for bulk_read and bulk_write. Yes this would 
prevent read and
write at the same time, and the read and writes need to be modified to not 
block until
the reansfer is finished, just to write the TRBs on the ring, update ring 
pointers,
and ring the doorbell.

Or is all this impossibe due to the earlyness of the code?

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

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Heikki Krogerus]

On Thu, Feb 18, 2016 at 10:21:48AM +0100, Oliver Neukum wrote:
> On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote:
> 
> Hi,
> 
> > The modes that can actually be selected have to be supported by both
> > the connector and the partner, and this is where I'm putting the ball
> > on the userspace at the moment. I'm not offering a list of
> > "possible_alternate_modes" where I list the combination, but instead
> > expect the userspace to be figure out that on it's own.
> > 
> > Do you think we should add "possible_alternate_modes" file?
> 
> No, what do we answer to the DFP if we recieve "Discover SVIDs"?
> I don't think that we always should answer with all we physically
> can. If, for example, the hardware could do Thunderbolt, but the OS
> is not prepared to handle it, we shouldn't offer it. So this is
> a policy decision to be made in user space. Hence we need
> an API to tell it to the kernel.

OK. Makes sense.

> > P.S. That reminds me, here's my current draft for the
> > Documentation/ABI/. Could you take a look?
> 
> OK
> 
> Here are my comments:
> 
> What: /sys/class/type-c/usbcN/connected
> 
>   Connection status of the USB Type-C connector usbcN. "yes" when
>   connected, otherwise "no".
> 
> Unnecessarily wordy. 0 and 1 would do

That works for me.

> What: /sys/class/type-c/usbcN/current_data_role
> 
> Again, 0 and 1 would do

I disagree with this one. What would 0 mean and what would 1? It would
require us to make an agreement about the "index" of the role, which
creates a small risk of somebody getting it wrong, but for what
purpose?

Why couldn't it be human readable "host" or "device" so there is never
no confusion about it.

> What: /sys/class/type-c/usbcN/partner_alternate_modes
> 
> You should say in which number base the values are given.
> 
> What: /sys/class/type-c/usbcN/partner_type
> 
> That could be combined with "connected"

Hmm, so in practice getting rid of "connected" completely.. I guess
it's OK.

> What: /sys/class/type-c/usbcN/supported_data_roles
> 
> A connector can be both. How is that expressed?

"host, device".

> What: /sys/class/type-c/usbcN/supported_power_roles
> 
> Again, what if it can do both?

"source, sink".

So these last two are now listing the values that can be entered to
the current_data_role and current_power_role.


Thanks,

-- 
heikki
--
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: TODO List For Linux USB

[Anil Nair]

Hi,

On Wed, Feb 17, 2016 at 4:21 PM, Andiry Xu  wrote:
> On Tue, Feb 16, 2016 at 10:43 PM, Anil Nair  wrote:
>> Hi,
>>
>> On Wed, Feb 17, 2016 at 12:00 PM, Greg KH  wrote:
>>> On Wed, Feb 17, 2016 at 11:47:56AM +0530, Anil Nair wrote:
 Hi All,

 I would like to contribute to Linux USB kernel but i am confused on
 where to start,
>>>
>>> If you don't know where to start, or what to do, why do you want to
>>> contribute?
>>
>> Because I have been on mailing list for quite a long time, I wanted to
>> contribute something back to the community.
>>
 If possible can you guys guide me regarding where to find the TODO
 list, so that I can start on with small patches.
>>>
>>> Run it, stress it, find problems, and start solving them.  Read the
>>> mailing list traffic and notice what others have problems with and work
>>> on solving them.
>>
>> Most issues are beyond my scope it requires more experience at
>> hardware level. Anyways i will observe and learn.
>>
>
> That's why starting with drivers may not be a good idea because it
> almost always requires some knowledge of hardware.
>
> Have you read the USB spec? Do you know the USB driver stack? If not,
> there is little chance you can do real contributions.
>
> Try something hardware-unrelated, like file system or mm...
>

Okay will try it.

> Thanks,
> Andiry
>
>>>
>>> Sorry, there is no TODO list.
>>
>> Ohh okay.
>>
>>>
>>> best of luck,
>>>
>>
>> Thanks.
>>
>>> greg k-h
>>
>>
>>
>> --
>> --
>> Regards,
>> Anil Nair
>> --
>> 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



-- 
--
Regards,
Anil Nair
--
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: option: add "4G LTE usb-modem U901"

[Johan Hovold]

On Fri, Feb 12, 2016 at 04:40:00PM +0100, Bjørn Mork wrote:
> Thomas reports:
> 
> T:  Bus=01 Lev=01 Prnt=01 Port=03 Cnt=01 Dev#=  4 Spd=480 MxCh= 0
> D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> P:  Vendor=05c6 ProdID=6001 Rev=00.00
> S:  Manufacturer=USB Modem
> S:  Product=USB Modem
> S:  SerialNumber=1234567890ABCDEF
> C:  #Ifs= 5 Cfg#= 1 Atr=e0 MxPwr=500mA
> I:  If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> I:  If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> I:  If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
> I:  If#= 4 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage
> 
> Cc: 
> Reported-by: Thomas Schäfer 
> Signed-off-by: Bjørn Mork 

Now applied.

Thanks,
Johan
--
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: Spansion's CMSIS-DAP + COM Port

[Johan Hovold]

[ +CC: Oliver ]

On Tue, Feb 09, 2016 at 09:27:39PM +0530, Thorsten Wilmer wrote:
> Hi,
> 
> I have a Starter Kit SK-FM4-176L-S6SE2CC
> modprobe  usbserial vendor=0x1a6a product=0x2000
> 
> makes the USB Port appear nicely (works with gtkterm) and at the same
> time one can use the CMIS-DAP  adapter with openocd.
> 
> As suggested by the driver I would like to recommend the
> vendor/product IDs to be added.

This looks like a cdc-acm device to me. Any reason why you can't use
that driver? If the driver is available, you should get a /dev/ttyACMn
device node. If not, could you post the logs from when plugging the
device in?

> Bus 001 Device 002: ID 1a6a:2000 Spansion Inc.
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   1.01
>   bDeviceClass0 (Defined at Interface level)
>   bDeviceSubClass 0
>   bDeviceProtocol 0
>   bMaxPacketSize064
>   idVendor   0x1a6a Spansion Inc.
>   idProduct  0x2000
>   bcdDevice1.60
>   iManufacturer   1 Spansion
>   iProduct2 Spansion CMSIS-DAP + COM Port
>   iSerial 0
>   bNumConfigurations  1
>   Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength   93
> bNumInterfaces  3
> bConfigurationValue 1
> iConfiguration  0
> bmAttributes 0x00
>   (Missing must-be-set bit!)
>   (Bus Powered)
> MaxPower   62mA
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber0
>   bAlternateSetting   0
>   bNumEndpoints   2
>   bInterfaceClass 3 Human Interface Device
>   bInterfaceSubClass  0 No Subclass
>   bInterfaceProtocol  0 None
>   iInterface  4 Spansion CMSIS-DAP
> HID Device Descriptor:
>   bLength 9
>   bDescriptorType33
>   bcdHID   1.11
>   bCountryCode0 Not supported
>   bNumDescriptors 1
>   bDescriptorType34 Report
>   wDescriptorLength  29
>  Report Descriptors:
>** UNAVAILABLE **
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x81  EP 1 IN
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0040  1x 64 bytes
> bInterval   1
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x02  EP 2 OUT
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0040  1x 64 bytes
> bInterval   1
> Interface Association:
>   bLength 8
>   bDescriptorType11
>   bFirstInterface 1
>   bInterfaceCount 2
>   bFunctionClass  2 Communications
>   bFunctionSubClass   2 Abstract (modem)
>   bFunctionProtocol   1 AT-commands (v.25ter)
>   iFunction   5 Spansion USB Serial Port
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber1
>   bAlternateSetting   0
>   bNumEndpoints   1
>   bInterfaceClass 2 Communications
>   bInterfaceSubClass  2 Abstract (modem)
>   bInterfaceProtocol  1 AT-commands (v.25ter)
>   iInterface  0
>   CDC Union:
> bMasterInterface0
> bSlaveInterface 1
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x83  EP 3 IN
> bmAttributes3
>   Transfer TypeInterrupt
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0040  1x 64 bytes
> bInterval 255
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber2
>   bAlternateSetting   0
>   bNumEndpoints   2
>   bInterfaceClass10 CDC Data
>   bInterfaceSubClass  0 Unused
>   bInterfaceProtocol  0
>   iInterface  0
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x04  EP 4 OUT
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   

Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data

[Felipe Balbi]

Hi,

Felipe Balbi  writes:
> Mika Westerberg  writes:
>> On Thu, Feb 18, 2016 at 01:31:33PM +0200, Heikki Krogerus wrote:
>>> On Thu, Feb 18, 2016 at 01:27:28PM +0200, Felipe Balbi wrote:
>>> > Heikki Krogerus  writes:
>>> > 
>>> > > On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote:
>>> > >> 
>>> > >> Hi,
>>> > >> 
>>> > >> Heikki Krogerus  writes:
>>> > >> > This should allow the core driver to drop handling of
>>> > >> > platform data and expect the platform specific details to
>>> > >> > always come from properties.
>>> > >> >
>>> > >> > Signed-off-by: Heikki Krogerus 
>>> > >> > Cc: Huang Rui 
>>> > >> > CC: John Youn 
>>> > >> 
>>> > >> doesn't compile dude:
>>> > >> 
>>> > >> make -k -j16 -- drivers/usb/dwc3/
>>> > >>   CHK include/config/kernel.release
>>> > >>   CHK include/generated/uapi/linux/version.h
>>> > >>   CHK include/generated/utsrelease.h
>>> > >>   CHK include/generated/timeconst.h
>>> > >>   CHK include/generated/bounds.h
>>> > >>   CHK include/generated/asm-offsets.h
>>> > >>   CALLscripts/checksyscalls.sh
>>> > >>   CC [M]  drivers/usb/dwc3/dwc3-pci.o
>>> > >>   CC [M]  drivers/usb/dwc3/dwc3-keystone.o
>>> > >>   CC [M]  drivers/usb/dwc3/dwc3-of-simple.o
>>> > >> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’:
>>> > >> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of 
>>> > >> function ‘platform_device_add_properties’ 
>>> > >> [-Werror=implicit-function-declaration]
>>> > >>   return platform_device_add_properties(dwc3, );
>>> > >
>>> > > Which kernel are you using? platform_device_add_properties() is
>>> > > available from v4.5-rc1 onwards.
>>> > 
>>> > v4.5-rc4, could it be you missed a header or the header misses a stub ?
>>> 
>>> Both the platform bus and property.c are always build in?
>>> 
>>> Adding Mika and Andy. Guys, do you have ideas?
>>
>> I would like to reproduce this. Felipe, can you send me your .config?
>
> It could have been the one attached. But I don't know for sure anymore

no, it wasn't this one. It's building now. Oh well...

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] USB: cp210x: add IDs for GE B650V3 and B850V3 boards

[Johan Hovold]

On Mon, Feb 01, 2016 at 02:57:25PM -0500, Akshay Bhat wrote:
> From: Ken Lin 
> 
> Add USB ID for cp2104/5 devices on GE B650v3 and B850v3 boards.
> 
> Signed-off-by: Ken Lin 
> Signed-off-by: Akshay Bhat 

Now applied, thanks.

Johan
--
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: option: add support for SIM7100E

[Johan Hovold]

On Sun, Jan 31, 2016 at 09:13:19PM +0100, Johan Hovold wrote:
> On Fri, Jan 29, 2016 at 12:07:30AM +0300, Andrey Skvortsov wrote:
> > $ lsusb:
> > Bus 001 Device 101: ID 1e0e:9001 Qualcomm / Option
> > 
> > $ usb-devices:
> > T:  Bus=01 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#=101 Spd=480  MxCh= 0
> > D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  2
> > P:  Vendor=1e0e ProdID=9001 Rev= 2.32
> > S:  Manufacturer=SimTech, Incorporated
> > S:  Product=SimTech, Incorporated
> > S:  SerialNumber=0123456789ABCDEF
> > C:* #Ifs= 7 Cfg#= 1 Atr=80 MxPwr=500mA
> > I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
> > I:* If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:* If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
> > I:* If#= 5 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
> > I:* If#= 6 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)
> > 
> > The last interface (6) is used for Android Composite ADB interface.
> > 
> > Serial port layout:
> > 0: QCDM/DIAG
> > 1: NMEA
> > 2: AT
> > 3: AT/PPP
> > 4: audio
> > 
> > Signed-off-by: Andrey Skvortsov 
> > ---
> >  v2: respin v1 against usb-linus branch of 
> > git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git
> >  move SIM7100E product id to ALINK entries
> >  move SIM7100E usb_device_id initialization to ALINK initializations
> 
> Thanks for respinning. I'll queue this one up for 4.5-rc.

Now applied, thanks.

Johan
--
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 04/10] usb: dbc: add debug buffer

[Mathias Nyman]

On 26.01.2016 14:58, Lu Baolu wrote:

"printk" is not suitable for dbc debugging especially when console
is in usage. This patch adds a debug buffer in dbc driver and puts
the debug messages in this local buffer. The debug buffer could be
dumped whenever the console is not in use. This part of code will
not be visible unless DBC_DEBUG is defined.

Signed-off-by: Lu Baolu 
---
  drivers/usb/early/xhci-dbc.c | 62 ++--
  1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index 41ce116..6855048 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -32,8 +32,64 @@ static struct xdbc_state xdbc_stat;
  static struct xdbc_state *xdbcp = _stat;

  #ifdef DBC_DEBUG
-/* place holder */
-#definexdbc_trace  printk
+#defineXDBC_DEBUG_BUF_SIZE (PAGE_SIZE * 32)


Does it really need to be this huge? minimum 4096 * 32 ~ 128k
The kernel ring  buffer is about the same size (16k - 256k)


+#defineMSG_MAX_LINE128


with 128 characters per line this would fit ~1000 lines


+static char xdbc_debug_buf[XDBC_DEBUG_BUF_SIZE];
+static void xdbc_trace(const char *fmt, ...)
+{
+   int i, size;
+   va_list args;
+   static int pos;
+   char temp_buf[MSG_MAX_LINE];
+
+   if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+   return;
+
+   memset(temp_buf, 0, MSG_MAX_LINE);
+   va_start(args, fmt);
+   vsnprintf(temp_buf, MSG_MAX_LINE - 1, fmt, args);
+   va_end(args);
+
+   i = 0;
+   size = strlen(temp_buf);
+   while (i < size) {
+   xdbc_debug_buf[pos] = temp_buf[i];
+   pos++;
+   i++;
+
+   if (pos >= XDBC_DEBUG_BUF_SIZE - 1)
+   break;
+   }


how about something like:

size = min(XDBC_DEBUG_BUF_SIZE - pos, size)
memcpy(xdbc_debug_buf + pos, temp_buf, size)
pos += size;

(might need some "-1" and off by one checking..)


+}
+
+static void xdbc_dump_debug_buffer(void)
+{
+   int index = 0;
+   int count = 0;
+   char dump_buf[MSG_MAX_LINE];
+
+   xdbc_trace("The end of DbC trace buffer\n");
+   pr_notice("DBC debug buffer:\n");
+   memset(dump_buf, 0, MSG_MAX_LINE);
+
+   while (index < XDBC_DEBUG_BUF_SIZE) {
+   if (!xdbc_debug_buf[index])
+   break;
+
+   if (xdbc_debug_buf[index] == '\n' ||
+   count >= MSG_MAX_LINE - 1) {
+   pr_notice("DBC: @%08x %s\n", index, dump_buf);


Is showing the he index (position in debug buffer) useful here?


+   memset(dump_buf, 0, MSG_MAX_LINE);
+   count = 0;
+   } else {
+   dump_buf[count] = xdbc_debug_buf[index];
+   count++;
+   }
+
+   index++;
+   }


So we have one huge buffer that xdbc keeps on filling as the initialization 
progresses.
It is never emptied, or overwritten (circular).
When dumped it always dumps the whole thing, copying one character
at a time.

As this is only used for debugging during xdbc development/debugging, and never 
enabled
even if xdbc early printk is used, I don't think optimization really matters.

Perhaps take a look if we really need PAGE_SIZE * 32 bytes, is xdbc driver even 
nearly
writing that much debug data.
 
-Mathias


--
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: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data

[Heikki Krogerus]

On Thu, Feb 18, 2016 at 01:27:28PM +0200, Felipe Balbi wrote:
> Heikki Krogerus  writes:
> 
> > On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote:
> >> 
> >> Hi,
> >> 
> >> Heikki Krogerus  writes:
> >> > This should allow the core driver to drop handling of
> >> > platform data and expect the platform specific details to
> >> > always come from properties.
> >> >
> >> > Signed-off-by: Heikki Krogerus 
> >> > Cc: Huang Rui 
> >> > CC: John Youn 
> >> 
> >> doesn't compile dude:
> >> 
> >> make -k -j16 -- drivers/usb/dwc3/
> >>   CHK include/config/kernel.release
> >>   CHK include/generated/uapi/linux/version.h
> >>   CHK include/generated/utsrelease.h
> >>   CHK include/generated/timeconst.h
> >>   CHK include/generated/bounds.h
> >>   CHK include/generated/asm-offsets.h
> >>   CALLscripts/checksyscalls.sh
> >>   CC [M]  drivers/usb/dwc3/dwc3-pci.o
> >>   CC [M]  drivers/usb/dwc3/dwc3-keystone.o
> >>   CC [M]  drivers/usb/dwc3/dwc3-of-simple.o
> >> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’:
> >> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of function 
> >> ‘platform_device_add_properties’ [-Werror=implicit-function-declaration]
> >>   return platform_device_add_properties(dwc3, );
> >
> > Which kernel are you using? platform_device_add_properties() is
> > available from v4.5-rc1 onwards.
> 
> v4.5-rc4, could it be you missed a header or the header misses a stub ?

Both the platform bus and property.c are always build in?

Adding Mika and Andy. Guys, do you have ideas?


Thanks,

-- 
heikki
--
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: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data

[Felipe Balbi]

Heikki Krogerus  writes:

> On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Heikki Krogerus  writes:
>> > This should allow the core driver to drop handling of
>> > platform data and expect the platform specific details to
>> > always come from properties.
>> >
>> > Signed-off-by: Heikki Krogerus 
>> > Cc: Huang Rui 
>> > CC: John Youn 
>> 
>> doesn't compile dude:
>> 
>> make -k -j16 -- drivers/usb/dwc3/
>>   CHK include/config/kernel.release
>>   CHK include/generated/uapi/linux/version.h
>>   CHK include/generated/utsrelease.h
>>   CHK include/generated/timeconst.h
>>   CHK include/generated/bounds.h
>>   CHK include/generated/asm-offsets.h
>>   CALLscripts/checksyscalls.sh
>>   CC [M]  drivers/usb/dwc3/dwc3-pci.o
>>   CC [M]  drivers/usb/dwc3/dwc3-keystone.o
>>   CC [M]  drivers/usb/dwc3/dwc3-of-simple.o
>> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’:
>> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of function 
>> ‘platform_device_add_properties’ [-Werror=implicit-function-declaration]
>>   return platform_device_add_properties(dwc3, );
>
> Which kernel are you using? platform_device_add_properties() is
> available from v4.5-rc1 onwards.

v4.5-rc4, could it be you missed a header or the header misses a stub ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCHv2 1/2] usb: dwc3: pci: use build-in properties instead of platform data

[Heikki Krogerus]

On Wed, Feb 17, 2016 at 04:24:26PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Heikki Krogerus  writes:
> > This should allow the core driver to drop handling of
> > platform data and expect the platform specific details to
> > always come from properties.
> >
> > Signed-off-by: Heikki Krogerus 
> > Cc: Huang Rui 
> > CC: John Youn 
> 
> doesn't compile dude:
> 
> make -k -j16 -- drivers/usb/dwc3/
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/bounds.h
>   CHK include/generated/asm-offsets.h
>   CALLscripts/checksyscalls.sh
>   CC [M]  drivers/usb/dwc3/dwc3-pci.o
>   CC [M]  drivers/usb/dwc3/dwc3-keystone.o
>   CC [M]  drivers/usb/dwc3/dwc3-of-simple.o
> drivers/usb/dwc3/dwc3-pci.c: In function ‘dwc3_pci_add_pset’:
> drivers/usb/dwc3/dwc3-pci.c:54:9: error: implicit declaration of function 
> ‘platform_device_add_properties’ [-Werror=implicit-function-declaration]
>   return platform_device_add_properties(dwc3, );

Which kernel are you using? platform_device_add_properties() is
available from v4.5-rc1 onwards.


Thanks,

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

Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

[Oliver Neukum]

On Thu, 2016-02-18 at 13:05 +0200, Heikki Krogerus wrote:

> > Since we have capability details of ports in user space, I believe
> > cable capability is also necessary for policy decision(power, alt
> > mode). Is that something we are cautiously leaving out ? pls explain
> 
> Adding the cable control to this interface will make it more complex
> from users perspective. However, nothing forces the user to control
> also the cable.

But we would like to indicate to the user that we cannot run
an alternate mode because the cable is incapable as opposed to the
device.

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

Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface

[Rajaram R]

On Thu, Feb 18, 2016 at 4:17 PM, Heikki Krogerus
 wrote:
> On Thu, Feb 18, 2016 at 04:07:54PM +0530, Rajaram R wrote:
>> On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus
>>  wrote:
>> > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote:
>> >>
>> >> Hi,
>> >>
>> >> Heikki Krogerus  writes:
>> >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote:
>> >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote:
>> >> >> > Hi,
>> >> >> >
>> >> >> > Oliver Neukum  writes:
>> >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote:
>> >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote:
>> >> >>
>> >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that
>> >> >> > >> > is to be supported, it needs to be defined now.
>> >> >> > >>
>> >> >> > >> When you say API, do you mean the API the class provides to the
>> >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this 
>> >> >> > >> case?
>> >> >> > >
>> >> >> > > The API to user space. That is the point. We cannot break user 
>> >> >> > > space.
>> >> >> > > Once this sysfs API is upstream we are stuck with it.
>> >> >> >
>> >> >> > yeah, in fact I have been wondering if sysfs is the best interface to
>> >> >>
>> >> >> That is the discussion we must have.
>> >> >>
>> >> >> > userspace. I talked with Heikki a few days back about this; I was
>> >> >> > wondering if something like what the NFC folks did with netlink 
>> >> >> > would be
>> >> >> > better here.
>> >> >>
>> >> >> I doubt that, because the main user is likely to be udev scripts.
>> >> >> They can easily deal with sysfs attributes.
>> >> >
>> >> > IMHO for high level interface like this, sysfs is ideal because of the
>> >> > simple fact that you only need a shell to access the files. netlink
>> >> > would make us depend on custom software, no?
>> >> >
>> >> > I'm not against using netlink, but what would be the benefit from it
>> >> > in this case?
>> >>
>> >> With HW we see nowadays, CC stack is hidden on some microcontroller, but
>> >> is it too far-fetched to consider a system where this is not the case ?
>> >
>> > There already are several USB PD stacks out there, like also Greg
>> > pointed out.
>> >
>> >> Specially when we consider things like power delivery which, I know, you
>> >> wanted to keep it out of this interface, however we would have two
>> >> 'stacks' competing for access to the same pins, right ?
>> >
>> > No. This class would be the top layer for the coming stack, where ever
>> > it ends up coming. The class is only the interface to the user space
>> > and nothing else.
>> >
>> > By saying we need to keep USB Type-C separate from USB PD I meant that
>> > the userspace access can not be mixed somewhere in layers of the USB
>> > PD/CC stack like it has been in the USB PD stacks I've seen so far.
>> > They assume that we always use the software USB PD stack with USB
>> > Type-C, which as we can see is not true when the stack is implemented
>> > in EC or firmware or some complex USB PD controller or what ever.
>> > However, the operations the userspace needs to do are exactly the same
>> > in both cases.
>> >
>> > - data role swapping
>> > - power role swapping (depends on USB PD)
>> > - Alternate Modes (depends on USB PD)
>> >
>> > And we really should not forget that we actually also have USB Type-C
>> > PHYs that can't do any USB PD communication over the CC pin, so USB PD
>> > is simply not always going to be available. But the data role swapping
>> > and also accessories are still available with them, as the do not need
>> > USB PD.
>> >
>> > This was the whole point with the class. It allows the different ways
>> > of dealing with Type-C ports to be exposed to userspace in the same
>> > way.
>> >
>> >> IIRC mode and role negotiation goes via CC pins using the power delivery
>> >> protocol. If I misunderstand anything, let me know.
>> >
>> > The data role swap with USB Type-C connectors is in no way tied to USB
>> > Power Delivery. The USB Type-C spec defines that when USB PD is
>>
>> Its not data role swap i guess its dual role, A Data role swap is tied
>> with USB PD,
>>
>> > available, DR_Swap USB PD function is used to swap the role, otherwise
>> > emulated disconnect will do the trick.
>>
>> I doubt a USB host with no device capability implement DRP ?? Also
>> emulated trick(??) is not spec requirement rt ?
>>
>> >
>> > Data role swapping is a must thing to have with USB Type-C connectors
>>
>> I guess you are referring to Dual role (DRP) and not data role (DRD).
>
> There is no term "DRD" in USB Type-C spec. A quote from Type-C spec

Yes, not in Spec 1.1 but a new term to differentiate data and power
roles . All I wanted to bring in some difference between data role
swap and DRP

> ch. 2.3.3:
>
> "Two methods are defined to allow a USB Type-C DRP to functionally
> swap 

Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

[Heikki Krogerus]

Hi Rajaram,

On Thu, Feb 18, 2016 at 01:04:48AM +0530, Rajaram R wrote:
> On Tue, Feb 9, 2016 at 10:31 PM, Heikki Krogerus
>  wrote:
> > Hi,
> >
> > The OS, or more precisely the user space, needs to be able to control
> > a few things regarding USB Type-C ports. The first thing that must be
> > allowed to be controlled is the data role. USB Type-C ports will
> > select the data role randomly with DRP ports. When USB PD is
> > supported, also independent (from data role) power role swapping can
> > be supported together with Alternate Mode control.
> >
> > I'm proposing with this set a Class for the Type-C connectors that
> > gives the user space control over those things on top of getting basic
> > details about the USB Type-C connectors and also partners. The details
> > include the capabilities of the port, the supported data and power
> > roles, supported accessories (audio and debug), supported Alternate
> > Modes, USB PD support and of course the type of the partner (USB, Alt
> > Mode, Accessory or Charger), and more or less the same details about
> > the partner.
> >
> > I'm not considering cables with this Class, and I have deliberately
> 
> Since we have capability details of ports in user space, I believe
> cable capability is also necessary for policy decision(power, alt
> mode). Is that something we are cautiously leaving out ? pls explain

Adding the cable control to this interface will make it more complex
from users perspective. However, nothing forces the user to control
also the cable.

I already decided to add vconn_swap support, so I'll try to add cable
alt mode control and capabilities as well.


Thanks,

-- 
heikki
--
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: type-c: USB Type-C Connector System Software Interface

[Heikki Krogerus]

On Thu, Feb 18, 2016 at 04:07:54PM +0530, Rajaram R wrote:
> On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus
>  wrote:
> > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote:
> >>
> >> Hi,
> >>
> >> Heikki Krogerus  writes:
> >> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote:
> >> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote:
> >> >> > Hi,
> >> >> >
> >> >> > Oliver Neukum  writes:
> >> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote:
> >> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote:
> >> >>
> >> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that
> >> >> > >> > is to be supported, it needs to be defined now.
> >> >> > >>
> >> >> > >> When you say API, do you mean the API the class provides to the
> >> >> > >> drivers? Or did you mean ABI which would be the sysfs in this case?
> >> >> > >
> >> >> > > The API to user space. That is the point. We cannot break user 
> >> >> > > space.
> >> >> > > Once this sysfs API is upstream we are stuck with it.
> >> >> >
> >> >> > yeah, in fact I have been wondering if sysfs is the best interface to
> >> >>
> >> >> That is the discussion we must have.
> >> >>
> >> >> > userspace. I talked with Heikki a few days back about this; I was
> >> >> > wondering if something like what the NFC folks did with netlink would 
> >> >> > be
> >> >> > better here.
> >> >>
> >> >> I doubt that, because the main user is likely to be udev scripts.
> >> >> They can easily deal with sysfs attributes.
> >> >
> >> > IMHO for high level interface like this, sysfs is ideal because of the
> >> > simple fact that you only need a shell to access the files. netlink
> >> > would make us depend on custom software, no?
> >> >
> >> > I'm not against using netlink, but what would be the benefit from it
> >> > in this case?
> >>
> >> With HW we see nowadays, CC stack is hidden on some microcontroller, but
> >> is it too far-fetched to consider a system where this is not the case ?
> >
> > There already are several USB PD stacks out there, like also Greg
> > pointed out.
> >
> >> Specially when we consider things like power delivery which, I know, you
> >> wanted to keep it out of this interface, however we would have two
> >> 'stacks' competing for access to the same pins, right ?
> >
> > No. This class would be the top layer for the coming stack, where ever
> > it ends up coming. The class is only the interface to the user space
> > and nothing else.
> >
> > By saying we need to keep USB Type-C separate from USB PD I meant that
> > the userspace access can not be mixed somewhere in layers of the USB
> > PD/CC stack like it has been in the USB PD stacks I've seen so far.
> > They assume that we always use the software USB PD stack with USB
> > Type-C, which as we can see is not true when the stack is implemented
> > in EC or firmware or some complex USB PD controller or what ever.
> > However, the operations the userspace needs to do are exactly the same
> > in both cases.
> >
> > - data role swapping
> > - power role swapping (depends on USB PD)
> > - Alternate Modes (depends on USB PD)
> >
> > And we really should not forget that we actually also have USB Type-C
> > PHYs that can't do any USB PD communication over the CC pin, so USB PD
> > is simply not always going to be available. But the data role swapping
> > and also accessories are still available with them, as the do not need
> > USB PD.
> >
> > This was the whole point with the class. It allows the different ways
> > of dealing with Type-C ports to be exposed to userspace in the same
> > way.
> >
> >> IIRC mode and role negotiation goes via CC pins using the power delivery
> >> protocol. If I misunderstand anything, let me know.
> >
> > The data role swap with USB Type-C connectors is in no way tied to USB
> > Power Delivery. The USB Type-C spec defines that when USB PD is
> 
> Its not data role swap i guess its dual role, A Data role swap is tied
> with USB PD,
> 
> > available, DR_Swap USB PD function is used to swap the role, otherwise
> > emulated disconnect will do the trick.
> 
> I doubt a USB host with no device capability implement DRP ?? Also
> emulated trick(??) is not spec requirement rt ?
> 
> >
> > Data role swapping is a must thing to have with USB Type-C connectors
> 
> I guess you are referring to Dual role (DRP) and not data role (DRD).

There is no term "DRD" in USB Type-C spec. A quote from Type-C spec
ch. 2.3.3:

"Two methods are defined to allow a USB Type-C DRP to functionally
swap data roles, one managed using USB PD DR_Swap and the other
emulating a disconnect/reconnect sequence (see Figure 4-16)"


Thanks,

-- 
heikki
--
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: type-c: USB Type-C Connector System Software Interface

[Heikki Krogerus]

Hi Peter,

On Thu, Feb 18, 2016 at 05:07:04PM +0800, Peter Chen wrote:
> On Wed, Feb 17, 2016 at 04:28:16PM +0200, Heikki Krogerus wrote:
> > On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote:
> > > IIRC mode and role negotiation goes via CC pins using the power delivery
> > > protocol. If I misunderstand anything, let me know.
> > 
> > The data role swap with USB Type-C connectors is in no way tied to USB
> > Power Delivery. The USB Type-C spec defines that when USB PD is
> > available, DR_Swap USB PD function is used to swap the role, otherwise
> > emulated disconnect will do the trick.
> 
> I am interested in how you design role swap on the fly without USB PD.
> Do you follow the spec like USB OTG 3.0 RSP (Role Swap Protocol) or
> just echo the /sys to give up current role, and swap to another?

No OTG with USB Type-C. You echo the wanted role to the
/sys/class/type-C/usbcN/data_role.

This operation from userspace is the same regardless was USB PD
supported or not. The actual operations needed for the role swap are
of course platform specific, and the responsibility of the drivers
that register the ports with type-c class.


Thanks,

-- 
heikki
--
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: type-c: USB Type-C Connector System Software Interface

[Oliver Neukum]

On Thu, 2016-02-18 at 12:30 +0200, Felipe Balbi wrote:
> Hi,
> 
> Oliver Neukum  writes:
> >> > What exactly are you sure about about?
> >> 
> >> heh, missed a NOT there :-)
> >
> > I am still confused :-)
> > Do you think a sysfs interface is good, bad or good
> > but insufficient?
> 
> I'm not sure it's the best interface. My fear is that as new
> requirements/features come along, the amount of files will continue to
> grow.

That will happen. The alternative, however is a "typectool" or
"usbpdtool" which would need to be updated for new features.

> >> I guess what I'm trying to say is that CC microcontroller might not be
> >> the one controlling the multiplexer which switches USB pins to another
> >> function. IOW:
> >> 
> >> Change to alternate mode X message
> >>  CC microcontroller interrupts CPU
> >>   read status to get X
> >>change multiplexer
> >> 
> >
> > Yes. But it seems to me that in this case we need a kernel driver
> > without an API to user space. There are necessarily internal users
> 
> that assumes kernel always knows all possible alternate modes. What do
> we about bogus requests (request alternate mode X when X is not
> supported) ?

Do as the spec says: NACK it.

The questions which modes we offer, if we are a slave, still remains.
And I think the API is deficient in that regard. But again that question
is orthogonal of both issue, handling of bogus requests and how the
CC pins are exported.

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

Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface

[Rajaram R]

On Wed, Feb 17, 2016 at 7:58 PM, Heikki Krogerus
 wrote:
> On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Heikki Krogerus  writes:
>> > On Wed, Feb 17, 2016 at 11:36:52AM +0100, Oliver Neukum wrote:
>> >> On Wed, 2016-02-17 at 12:29 +0200, Felipe Balbi wrote:
>> >> > Hi,
>> >> >
>> >> > Oliver Neukum  writes:
>> >> > > On Wed, 2016-02-17 at 09:58 +0200, Heikki Krogerus wrote:
>> >> > >> On Tue, Feb 16, 2016 at 02:39:47PM +0100, Oliver Neukum wrote:
>> >>
>> >> > >> > Yes, but we need an API. We can't keep adding to it. So if that
>> >> > >> > is to be supported, it needs to be defined now.
>> >> > >>
>> >> > >> When you say API, do you mean the API the class provides to the
>> >> > >> drivers? Or did you mean ABI which would be the sysfs in this case?
>> >> > >
>> >> > > The API to user space. That is the point. We cannot break user space.
>> >> > > Once this sysfs API is upstream we are stuck with it.
>> >> >
>> >> > yeah, in fact I have been wondering if sysfs is the best interface to
>> >>
>> >> That is the discussion we must have.
>> >>
>> >> > userspace. I talked with Heikki a few days back about this; I was
>> >> > wondering if something like what the NFC folks did with netlink would be
>> >> > better here.
>> >>
>> >> I doubt that, because the main user is likely to be udev scripts.
>> >> They can easily deal with sysfs attributes.
>> >
>> > IMHO for high level interface like this, sysfs is ideal because of the
>> > simple fact that you only need a shell to access the files. netlink
>> > would make us depend on custom software, no?
>> >
>> > I'm not against using netlink, but what would be the benefit from it
>> > in this case?
>>
>> With HW we see nowadays, CC stack is hidden on some microcontroller, but
>> is it too far-fetched to consider a system where this is not the case ?
>
> There already are several USB PD stacks out there, like also Greg
> pointed out.
>
>> Specially when we consider things like power delivery which, I know, you
>> wanted to keep it out of this interface, however we would have two
>> 'stacks' competing for access to the same pins, right ?
>
> No. This class would be the top layer for the coming stack, where ever
> it ends up coming. The class is only the interface to the user space
> and nothing else.
>
> By saying we need to keep USB Type-C separate from USB PD I meant that
> the userspace access can not be mixed somewhere in layers of the USB
> PD/CC stack like it has been in the USB PD stacks I've seen so far.
> They assume that we always use the software USB PD stack with USB
> Type-C, which as we can see is not true when the stack is implemented
> in EC or firmware or some complex USB PD controller or what ever.
> However, the operations the userspace needs to do are exactly the same
> in both cases.
>
> - data role swapping
> - power role swapping (depends on USB PD)
> - Alternate Modes (depends on USB PD)
>
> And we really should not forget that we actually also have USB Type-C
> PHYs that can't do any USB PD communication over the CC pin, so USB PD
> is simply not always going to be available. But the data role swapping
> and also accessories are still available with them, as the do not need
> USB PD.
>
> This was the whole point with the class. It allows the different ways
> of dealing with Type-C ports to be exposed to userspace in the same
> way.
>
>> IIRC mode and role negotiation goes via CC pins using the power delivery
>> protocol. If I misunderstand anything, let me know.
>
> The data role swap with USB Type-C connectors is in no way tied to USB
> Power Delivery. The USB Type-C spec defines that when USB PD is

Its not data role swap i guess its dual role, A Data role swap is tied
with USB PD,

> available, DR_Swap USB PD function is used to swap the role, otherwise
> emulated disconnect will do the trick.

I doubt a USB host with no device capability implement DRP ?? Also
emulated trick(??) is not spec requirement rt ?

>
> Data role swapping is a must thing to have with USB Type-C connectors

I guess you are referring to Dual role (DRP) and not data role (DRD).

> because of the fact that the role is selected randomly. Regardless was
> USB PD supported or not.
>
>
> Thanks,
>
> --
> heikki
> --
> 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
--
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 v3] usb: gadget: provide interface for legacy gadgets to get UDC name

[Marek Szyprowski]

Since commit 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb: gadget:
udc-core: independent registration of gadgets and gadget drivers") gadget
drivers can not assume that UDC drivers are already available on their
initialization. This broke the HACK, which was used in gadgetfs driver,
to get UDC controller name. This patch removes this hack and replaces it
by additional function in the UDC core (which is usefully only for legacy
drivers, please don't use it in the new code).

Reported-by: Vegard Nossum 
Signed-off-by: Marek Szyprowski 
Tested-by: Vegard Nossum 
---
changelog:
v3:
- simplified code a bit

v2: https://lkml.org/lkml/2016/2/8/234
- properly report udc gagdet driver name instead of udc device name

v1: https://lkml.org/lkml/2016/2/8/176
- initial version
---
 drivers/usb/gadget/legacy/inode.c | 28 +++-
 drivers/usb/gadget/udc/udc-core.c | 30 ++
 include/linux/usb/gadget.h|  1 +
 3 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 87fb0fd..5cdaf01 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -1699,28 +1699,6 @@ static struct usb_gadget_driver gadgetfs_driver = {
 };
 
 /*--*/
-
-static void gadgetfs_nop(struct usb_gadget *arg) { }
-
-static int gadgetfs_probe(struct usb_gadget *gadget,
-   struct usb_gadget_driver *driver)
-{
-   CHIP = gadget->name;
-   return -EISNAM;
-}
-
-static struct usb_gadget_driver probe_driver = {
-   .max_speed  = USB_SPEED_HIGH,
-   .bind   = gadgetfs_probe,
-   .unbind = gadgetfs_nop,
-   .setup  = (void *)gadgetfs_nop,
-   .disconnect = gadgetfs_nop,
-   .driver = {
-   .name   = "nop",
-   },
-};
-
-
 /* DEVICE INITIALIZATION
  *
  * fd = open ("/dev/gadget/$CHIP", O_RDWR)
@@ -1971,9 +1949,7 @@ gadgetfs_fill_super (struct super_block *sb, void *opts, 
int silent)
if (the_device)
return -ESRCH;
 
-   /* fake probe to determine $CHIP */
-   CHIP = NULL;
-   usb_gadget_probe_driver(_driver);
+   CHIP = usb_get_gadget_udc_name();
if (!CHIP)
return -ENODEV;
 
@@ -2034,6 +2010,8 @@ gadgetfs_kill_sb (struct super_block *sb)
put_dev (the_device);
the_device = NULL;
}
+   kfree(CHIP);
+   CHIP = NULL;
 }
 
 /*--*/
diff --git a/drivers/usb/gadget/udc/udc-core.c 
b/drivers/usb/gadget/udc/udc-core.c
index fd73a3e..fd30242 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -442,6 +442,36 @@ err1:
 EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
 
 /**
+ * usb_get_gadget_udc_name - get the name of the first UDC controller
+ * This functions returns the name of the first UDC controller in the system.
+ * Please note that this interface is usefull only for legacy drivers which
+ * assume that there is only one UDC controller in the system and they need to
+ * get its name before initialization. There is no guarantee that the UDC
+ * of the returned name will be still available, when gadget driver registers
+ * itself.
+ *
+ * Returns pointer to string with UDC controller name on success, NULL
+ * otherwise. Caller should kfree() returned string.
+ */
+char *usb_get_gadget_udc_name(void)
+{
+   struct usb_udc *udc;
+   char *name = NULL;
+
+   /* For now we take the first available UDC */
+   mutex_lock(_lock);
+   list_for_each_entry(udc, _list, list) {
+   if (!udc->driver) {
+   name = kstrdup(udc->gadget->name, GFP_KERNEL);
+   break;
+   }
+   }
+   mutex_unlock(_lock);
+   return name;
+}
+EXPORT_SYMBOL_GPL(usb_get_gadget_udc_name);
+
+/**
  * usb_add_gadget_udc - adds a new gadget to the udc class driver list
  * @parent: the parent device to this udc. Usually the controller
  * driver's device.
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..547d86d 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1126,6 +1126,7 @@ extern int usb_add_gadget_udc_release(struct device 
*parent,
struct usb_gadget *gadget, void (*release)(struct device *dev));
 extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget 
*gadget);
 extern void usb_del_gadget_udc(struct usb_gadget *gadget);
+extern char *usb_get_gadget_udc_name(void);
 
 /*-*/
 
-- 
1.9.2

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

Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface

[Felipe Balbi]

Hi,

Oliver Neukum  writes:
>> > What exactly are you sure about about?
>> 
>> heh, missed a NOT there :-)
>
> I am still confused :-)
> Do you think a sysfs interface is good, bad or good
> but insufficient?

I'm not sure it's the best interface. My fear is that as new
requirements/features come along, the amount of files will continue to
grow.

>> I guess what I'm trying to say is that CC microcontroller might not be
>> the one controlling the multiplexer which switches USB pins to another
>> function. IOW:
>> 
>> Change to alternate mode X message
>>  CC microcontroller interrupts CPU
>>   read status to get X
>>change multiplexer
>> 
>
> Yes. But it seems to me that in this case we need a kernel driver
> without an API to user space. There are necessarily internal users

that assumes kernel always knows all possible alternate modes. What do
we about bogus requests (request alternate mode X when X is not
supported) ?

> of the PD controller. There are also external users. So the CC pins
> should be seen as a bus, which in essence they are (it reminds me
> of ancient ethernet actually), and if you really want full user
> space access, you'd need the quivalent of an sg driver.
>
> The issue is orthogonal to the question how we support UCSI, except
> that UCSI is a user of the CC pins and PD and frankly I don't see the
> firmware and a driver access this sanely simultaneously.  Therefore
> I'd prefer we make an API here which does not depend on UCSI, but can,
> if necessary, work on top of a driver doing full hardware access.

fair enough.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface

[Oliver Neukum]

On Thu, 2016-02-18 at 09:08 +0200, Felipe Balbi wrote:

> Oliver Neukum  writes:
> >> Oliver Neukum  writes:

Hi,

> > What exactly are you sure about about?
> 
> heh, missed a NOT there :-)

I am still confused :-)
Do you think a sysfs interface is good, bad or good
but insufficient?

> >> that, eventually, we will need an actual stack exposed for the CC pin.
> >
> > The raw CC pin? What about the timing requirement?
> 
> well, not the _raw_ CC pin, but a situation where the microcontroller
> handling CC pin is "dumb", in the sense that it provides an interface
> for us to request/start arbitrary transactions. That will, in turn,
> shift the actual bits on the CC pin. Or something along these lines.

Well, how else would we send vendor specific messages?

> An example would be the alternate mode thing. CC microcontroller doesn't
> have to know what are the available alternate modes, but it needs to be
> able to tell processor what request is coming from the other end.

Indeed.

> I guess what I'm trying to say is that CC microcontroller might not be
> the one controlling the multiplexer which switches USB pins to another
> function. IOW:
> 
> Change to alternate mode X message
>  CC microcontroller interrupts CPU
>   read status to get X
>change multiplexer
> 

Yes. But it seems to me that in this case we need a kernel driver
without an API to user space. There are necessarily internal users
of the PD controller. There are also external users. So the CC pins
should be seen as a bus, which in essence they are (it reminds me
of ancient ethernet actually), and if you really want full user
space access, you'd need the quivalent of an sg driver.

The issue is orthogonal to the question how we support UCSI,
except that UCSI is a user of the CC pins and PD and frankly
I don't see the firmware and a driver access this sanely simultaneously.
Therefore I'd prefer we make an API here which does not depend on UCSI,
but can, if necessary, work on top of a driver doing full hardware
access.

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

Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface

[Oliver Neukum]

On Thu, 2016-02-18 at 17:29 +0800, Peter Chen wrote:

> Does this UCSI spec has some similar things with USB Type-C Port
> Controller Interface Spec at usb.org? If not, how to co-work
> together in future?

USB Type-C Port Controller Interface Spec:

What can a type C connector do?

UCSI spec:

How can the OS use ACPI to tell a type C connector what to do.

Obviously UCSI depends on the type C spec, but they cover different
things.

HTH
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

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Oliver Neukum]

On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote:

Hi,

> P.S. That reminds me, here's my current draft for the
> Documentation/ABI/. Could you take a look?

And I am afraid, that I have a few remarks not bound
to a specific entry.

We have port directories for port power switching. How is
the connector directory linked to them?

Likewise, if we have USB PD, we have to know how that
is linked to the connector directory.

In addition, writes to those files have results. We need
the error codes to be described.

Furthermore, do these files support poll?

And lastly we can get "Attention" as a message connected
with a connector in an alternate mode. How does user space
learn about that?

I am sorry to be this obnoxious, but this is an API which
will be with us for a long time, so we better get it right.

HTH
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

Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface

[Peter Chen]

On Wed, Feb 10, 2016 at 12:30:42PM +0200, Heikki Krogerus wrote:
> On Tue, Feb 09, 2016 at 10:21:55AM -0800, Greg KH wrote:
> > On Tue, Feb 09, 2016 at 07:01:22PM +0200, Heikki Krogerus wrote:
> > > USB Type-C Connector System Software Interface (UCSI) is a
> > > specification that defines registers and data structures
> > > used to interface with the USB Type-C connectors on a system.
> > > 
> > > The specification is public and available at:
> > > http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html
> > > 
> > 
> > What does this driver / code actually do?  Why is it needed?  What
> > interface to the rest of the kernel / userspace does it provide?
> 
> I will fix this describe these things in the commit message. I'll just
> but some UCSI background in case somebody is interested. So UCSI is in
> practice a standard for USB Type-C controllers..
> 

Does this UCSI spec has some similar things with USB Type-C Port
Controller Interface Spec at usb.org? If not, how to co-work
together in future?

Peter

> UCSI is the control interface for USB Type-C connectors (regardless
> was USB PD supported or not) in MS Windows, so most likely all new HW
> platforms designed to work also with Windows that are equipped with
> USB Type-C will have UCSI device for controlling the USB Type-C ports.
> In some cases the hardware for Type-C will be just a PHY like fusb30x
> on these platforms (it's cheaper then USB PD or complete USB Type-C
> controller), but in those cases the PHY is probable attached to an EC
> or is completely controlled by system FW like BIOS together with any
> USB PD communication in cases where USB PD is supported, and is in any
> case not visible to the OS. Instead UCSI device is exposed to the OS
> to give it means to apply its policies to the USB Type-C port.
> 
> > Why would we care about this?
> 
> I'll try to explain why it's important to export the control of USB
> Type-C ports to the user space in my answer to your comments to the
> first patch of this series, the one introducing the class.
> 
> But surely everybody agrees that decision about the policies regarding
> USB Type-C ports, like which data role to use, do we charge or are we
> letting the other end charge, etc., belongs to the user? If you plug
> your phone to your desktop, I would imagine that you want to see the
> phone primarily as the USB device and the desktop as host, and to
> achieve the device role, you don't want to be forced to unplug/replug
> your phone from the desktop until you achieve device role, right?
> 
> > You need to describe this a lot better than you did...
> 
> Sure thing. I'm sorry about the poor description. I send these out too
> hastily.
> 
> 
> Thanks,
> 
> -- 
> heikki
> --
> 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

-- 

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: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

[Heikki Krogerus]

On Wed, Feb 17, 2016 at 07:53:47PM +0100, Oliver Neukum wrote:
> On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote:
> > Hi,
> > 
> > The OS, or more precisely the user space, needs to be able to control
> > a few things regarding USB Type-C ports. The first thing that must be
> > allowed to be controlled is the data role. USB Type-C ports will
> > select the data role randomly with DRP ports. When USB PD is
> > supported, also independent (from data role) power role swapping can
> > be supported together with Alternate Mode control.
> 
> What about S4? We need to restore the alternate mode upon resume,
> if we are the DFP and as that might involve storage devices it
> needs to be done in kernel space.

That I'm expecting to be the responsibility of the drivers registering
the ports at them moment, because I'm afraid of all the possible
different kind of platform specific oddities that need to be
considered.

But I guess all we would need to do in the class is store the roles
and mode during suspend, and restore them during resuming with
dr_swap, pr_swap and set_alt_mode hooks if they changed.


Thanks,

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

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Oliver Neukum]

On Thu, 2016-02-18 at 10:47 +0200, Heikki Krogerus wrote:

Hi,

> The modes that can actually be selected have to be supported by both
> the connector and the partner, and this is where I'm putting the ball
> on the userspace at the moment. I'm not offering a list of
> "possible_alternate_modes" where I list the combination, but instead
> expect the userspace to be figure out that on it's own.
> 
> Do you think we should add "possible_alternate_modes" file?

No, what do we answer to the DFP if we recieve "Discover SVIDs"?
I don't think that we always should answer with all we physically
can. If, for example, the hardware could do Thunderbolt, but the OS
is not prepared to handle it, we shouldn't offer it. So this is
a policy decision to be made in user space. Hence we need
an API to tell it to the kernel.

> P.S. That reminds me, here's my current draft for the
> Documentation/ABI/. Could you take a look?

OK

Here are my comments:

What:   /sys/class/type-c/usbcN/connected

Connection status of the USB Type-C connector usbcN. "yes" when
connected, otherwise "no".

Unnecessarily wordy. 0 and 1 would do

What:   /sys/class/type-c/usbcN/current_data_role

Again, 0 and 1 would do

What:   /sys/class/type-c/usbcN/partner_alternate_modes

You should say in which number base the values are given.

What:   /sys/class/type-c/usbcN/partner_type

That could be combined with "connected"

What:   /sys/class/type-c/usbcN/supported_data_roles

A connector can be both. How is that expressed?

What:   /sys/class/type-c/usbcN/supported_power_roles

Again, what if it can do both?



--
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: type-c: USB Type-C Connector System Software Interface

[Peter Chen]

On Wed, Feb 17, 2016 at 04:28:16PM +0200, Heikki Krogerus wrote:
> On Wed, Feb 17, 2016 at 03:36:46PM +0200, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> 
> > IIRC mode and role negotiation goes via CC pins using the power delivery
> > protocol. If I misunderstand anything, let me know.
> 
> The data role swap with USB Type-C connectors is in no way tied to USB
> Power Delivery. The USB Type-C spec defines that when USB PD is
> available, DR_Swap USB PD function is used to swap the role, otherwise
> emulated disconnect will do the trick.
> 

I am interested in how you design role swap on the fly without USB PD.
Do you follow the spec like USB OTG 3.0 RSP (Role Swap Protocol) or
just echo the /sys to give up current role, and swap to another?

-- 

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 v2] usb: usbtmc: Fix disconnect/poll interaction

[Dave Penkler]

When the device is disconnected poll waiters were not being woken.

Changes for v2:
  - add commit summary
  - add Fixes and Reported-by tags   

Fixes: eb6b92ecc0f9 ("Add support for receiving USBTMC USB488 SRQ notifications 
via poll/select")
Reported-by: Oliver Neukum 
Signed-off-by: Dave Penkler 
---
 drivers/usb/class/usbtmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 419c72e..917a55c 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1525,13 +1525,14 @@ static void usbtmc_disconnect(struct usb_interface 
*intf)
dev_dbg(>dev, "usbtmc_disconnect called\n");
 
data = usb_get_intfdata(intf);
-   usbtmc_free_int(data);
usb_deregister_dev(intf, _class);
sysfs_remove_group(>dev.kobj, _attr_grp);
sysfs_remove_group(>dev.kobj, _attr_grp);
mutex_lock(>io_mutex);
data->zombie = 1;
+   wake_up_all(>waitq);
mutex_unlock(>io_mutex);
+   usbtmc_free_int(data);
kref_put(>kref, usbtmc_delete);
 }
 
-- 
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] usb: USB Type-C Connector Class

[Heikki Krogerus]

Hi Oliver,

On Wed, Feb 17, 2016 at 03:07:27PM +0100, Oliver Neukum wrote:
> On Tue, 2016-02-09 at 19:01 +0200, Heikki Krogerus wrote:
> > 1. connected - Connection status of the connector
> > 2. alternate_mode - The current Alternate Mode
> > 3. alternate_modes - Lists all Alternate Modes the connector supports
> > 4. partner_alt_modes - Lists partner's Alternate Modes when connected
> 
> Now that I think about it, there's a gap.
> Which SVIDs do we expose if we are UFP (slave)?

In the alternate_modes listing the connectors alt modes, we can not
have modes that the hardware can not support of course, and it is the
responsibility of the drivers registering the type-c ports with this
clss to make sure they are not part of the list.

In partner alternate modes, we will list all alternate modes the
partner supports, even the ones our connector doesn't.

The modes that can actually be selected have to be supported by both
the connector and the partner, and this is where I'm putting the ball
on the userspace at the moment. I'm not offering a list of
"possible_alternate_modes" where I list the combination, but instead
expect the userspace to be figure out that on it's own.

Do you think we should add "possible_alternate_modes" file?


P.S. That reminds me, here's my current draft for the
Documentation/ABI/. Could you take a look?


Thanks,

-- 
heikki
What:   /sys/class/type-c/
Date:   February 2016
Contact:Heikki Krogerus 
Description:
USB Type-C Connectors can be used for other purposes than just
connecting USB peripherals to USB host. Devices with USB Type-C
connector can be of one of the following types:

1. USB - In normal USB use
2. Accessory Mode - Audio or Debug. Special purpose modes for
the connector which are defined in the USB
Type-C specification. When an Accessory is
connected to the connector, it can not be
used for anything besides what the Accessory
Mode defines, including USB.
3. Alternate Mode - Modes where the connector can be used also
for other purposes then just connecting USB
peripherals to USB hosts, and which are not
defined in USB Type-C Specification. The
Alternate Modes depend on USB Power Delivery
support as they are controlled with protocol
defined in USB Power Delivery specification.

There is also only one type of USB Type-C Connector, so
the connector will be the same for both host and peripheral. If
two systems that are both dual-role capable (can act as both USB
Host and USB Device) are connected together, the roles are
selected randomly.

This class gives the userspace the control over the USB role and
the Alternate Mode with USB Type-C connectors.

What:   /sys/class/type-c/usbcN/
Date:   February 2016
Contact:Heikki Krogerus 
Description:
A /sys/class/type-C/usbcN directory is created for each USB
Type-C connector in the system. The N is the index of the
connector.

What:   /sys/class/type-c/usbcN/connected
Date:   February 2016
Contact:Heikki Krogerus 
Description:
Connection status of the USB Type-C connector usbcN. "yes" when
connected, otherwise "no".

What:   /sys/class/type-c/usbcN/current_alternate_mode
Date:   February 2016
Contact:Heikki Krogerus 
Description:
When connected to an Alternate Mode, displays the current Mode
ID and the Standard of Vendor ID (SVID) of the Alternate Mode in
form:

,

This file is writable and can be used to enter other modes both
the connector usbcN and the partner support. The modes are
Alternate Mode specific, and before they are to be changed with
this file, the exact details of the modes under the given SVID
should be known by the user.

What:   /sys/class/type-c/usbcN/current_data_role
Date:   February 2016
Contact:Heikki Krogerus 
Description:
The current USB data role, host or device. This file can be used
by the userspace to execute data role swap by writing the
requested role to it.

[PATCH] usb: storage: remove the US_DEBUG macro

[Victor Dodon]

Get rid of the US_DEBUG macro and use instead empty inline function definitions
when CONFIG_USB_STORAGE_DEBUG is not defined

Signed-off-by: Victor Dodon 
---
 drivers/usb/storage/debug.h  | 18 +++---
 drivers/usb/storage/ene_ub6250.c |  1 -
 drivers/usb/storage/freecom.c| 12 
 drivers/usb/storage/transport.c  |  2 --
 drivers/usb/storage/usb.c|  2 +-
 5 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/storage/debug.h b/drivers/usb/storage/debug.h
index 6b365ce..50070f0 100644
--- a/drivers/usb/storage/debug.h
+++ b/drivers/usb/storage/debug.h
@@ -52,9 +52,22 @@ void usb_stor_show_sense(const struct us_data *us, unsigned 
char key,
 unsigned char asc, unsigned char ascq);
 __printf(2, 3) void usb_stor_dbg(const struct us_data *us,
 const char *fmt, ...);
-
-#define US_DEBUG(x)x
 #else
+static inline void _usb_stor_show_command(const struct us_data *us,
+  struct scsi_cmnd *srb)
+{
+}
+#define usb_stor_show_command(us, srb) \
+   do { if (0) _usb_stor_show_command(us, srb); } while (0)
+
+static inline void _usb_stor_show_sense(const struct us_data *us,
+   unsigned char key, unsigned char asc,
+   unsigned char ascq)
+{
+}
+#define usb_stor_show_sense(us, key, asc, ascq)\
+   do { if (0) _usb_stor_show_sense(us, key, asc, ascq); } while 0
+
 __printf(2, 3)
 static inline void _usb_stor_dbg(const struct us_data *us,
 const char *fmt, ...)
@@ -62,7 +75,6 @@ static inline void _usb_stor_dbg(const struct us_data *us,
 }
 #define usb_stor_dbg(us, fmt, ...) \
do { if (0) _usb_stor_dbg(us, fmt, ##__VA_ARGS__); } while (0)
-#define US_DEBUG(x)
 #endif
 
 #endif
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index f3cf4ce..73cfd84 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -2296,7 +2296,6 @@ static int ene_transport(struct scsi_cmnd *srb, struct 
us_data *us)
int result = 0;
struct ene_ub6250_info *info = (struct ene_ub6250_info *)(us->extra);
 
-   /*US_DEBUG(usb_stor_show_command(us, srb)); */
scsi_set_resid(srb, 0);
if (unlikely(!(info->SD_Status.Ready || info->MS_Status.Ready))) {
result = ene_init(us);
diff --git a/drivers/usb/storage/freecom.c b/drivers/usb/storage/freecom.c
index 3f2b089..9c37175 100644
--- a/drivers/usb/storage/freecom.c
+++ b/drivers/usb/storage/freecom.c
@@ -44,6 +44,10 @@ MODULE_LICENSE("GPL");
 
 #ifdef CONFIG_USB_STORAGE_DEBUG
 static void pdump(struct us_data *us, void *ibuffer, int length);
+#else
+static inline void pdump(struct us_data *us, void *ibuffer, int length)
+{
+}
 #endif
 
 /* Bits of HD_STATUS */
@@ -245,7 +249,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct 
us_data *us)
memcpy (fcb->Atapi, srb->cmnd, 12);
memset (fcb->Filler, 0, sizeof (fcb->Filler));
 
-   US_DEBUG(pdump(us, srb->cmnd, 12));
+   pdump(us, srb->cmnd, 12);
 
/* Send it out. */
result = usb_stor_bulk_transfer_buf (us, opipe, fcb,
@@ -267,7 +271,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct 
us_data *us)
if (result != USB_STOR_XFER_GOOD)
return USB_STOR_TRANSPORT_ERROR;
 
-   US_DEBUG(pdump(us, (void *)fst, partial));
+   pdump(us, (void *)fst, partial);
 
/* The firmware will time-out commands after 20 seconds. Some commands
 * can legitimately take longer than this, so we use a different
@@ -308,7 +312,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct 
us_data *us)
if (result != USB_STOR_XFER_GOOD)
return USB_STOR_TRANSPORT_ERROR;
 
-   US_DEBUG(pdump(us, (void *)fst, partial));
+   pdump(us, (void *)fst, partial);
}
 
if (partial != 4)
@@ -365,7 +369,7 @@ static int freecom_transport(struct scsi_cmnd *srb, struct 
us_data *us)
usb_stor_dbg(us, "Waiting for status\n");
result = usb_stor_bulk_transfer_buf (us, ipipe, fst,
FCM_PACKET_LENGTH, );
-   US_DEBUG(pdump(us, (void *)fst, partial));
+   pdump(us, (void *)fst, partial);
 
if (partial != 4 || result > USB_STOR_XFER_SHORT)
return USB_STOR_TRANSPORT_ERROR;
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 5e67f63..2f883a8 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -784,9 +784,7 @@ Retry_Sense:
usb_stor_dbg(us, "-- code: 0x%x, key: 0x%x, ASC: 0x%x, ASCQ: 
0x%x\n",
 

Re: [PATCH 2/5] usb: musb: core: added helper functions for parsing DT

[Felipe Balbi]

Hi,

Petr Kulhavy  writes:
> On 17.02.2016 15:58, Felipe Balbi wrote:
>> Hi,
>>
>> Petr Kulhavy  writes:
>>> This adds two functions to get DT properties "mentor,power" and "dr_mode":
>>> musb_get_power() and musb_mode musb_get_mode()
>>>
>>> Signed-off-by: Petr Kulhavy 
>> seems like I don't have patch 1/5. After fixing Sergei's comments,
>> please resend with his Acked-by already in place.
>>
>> thanks
> Hi Felipe,
>
> I will do as soon as the patch 1/5 gets approved.
> It seem to be a bit stuck at the moment as Rob Herring from the DT wants 
> the "mentor,power"
> to be represented as a regulator, whereas Sergei and me want to stick to 
> the existing "mentor,power" integer property.
>
> As soon as this get clarified I will do the final updates and send the 
> patch again.
> Maybe this is something you can help to clarify?

I don't think that makes sense as a regulator. It's just a number which
gets passed to USB Core as is.

However, it seems like everything in kernel right now is passing it as
500. So why don't you deprecate that property, hardcode it to 500 and
avoid the problem altogether ?

The likelihood of new MUSB designs is rather low, anyway.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 RESEND] usb: gadget: provide interface for legacy gadgets to get UDC name

[Felipe Balbi]

Hi,

Marek Szyprowski  writes:
> Since commit 855ed04a3758b205e84b269f92d26ab36ed8e2f7 ("usb: gadget:
> udc-core: independent registration of gadgets and gadget drivers") gadget
> drivers can not assume that UDC drivers are already available on their
> initialization. This broke the HACK, which was used in gadgetfs driver,
> to get UDC controller name. This patch removes this hack and replaces it
> by additional function in the UDC core (which is usefully only for legacy
> drivers, please don't use it in the new code).
>
> Reported-by: Vegard Nossum 
> Signed-off-by: Marek Szyprowski 
> Tested-by: Vegard Nossum 
> ---
> changelog:
> v2:
> - properly report udc gagdet driver name instead of udc device name
>
> v1: https://lkml.org/lkml/2016/2/8/176
> - initial version
> ---
>  drivers/usb/gadget/legacy/inode.c | 29 ++---
>  drivers/usb/gadget/udc/udc-core.c | 33 +
>  include/linux/usb/gadget.h|  1 +
>  3 files changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/inode.c 
> b/drivers/usb/gadget/legacy/inode.c
> index 7e179f81d05c..7a62a2f7bc18 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -227,7 +227,7 @@ static void put_ep (struct ep_data *data)
>   * implicitly, starting with the driver name and then endpoint names.
>   */
>  
> -static const char *CHIP;
> +static char CHIP[32];
>  
>  /*--*/
>  
> @@ -1697,28 +1697,6 @@ static struct usb_gadget_driver gadgetfs_driver = {
>  };
>  
>  /*--*/
> -
> -static void gadgetfs_nop(struct usb_gadget *arg) { }
> -
> -static int gadgetfs_probe(struct usb_gadget *gadget,
> - struct usb_gadget_driver *driver)
> -{
> - CHIP = gadget->name;
> - return -EISNAM;
> -}
> -
> -static struct usb_gadget_driver probe_driver = {
> - .max_speed  = USB_SPEED_HIGH,
> - .bind   = gadgetfs_probe,
> - .unbind = gadgetfs_nop,
> - .setup  = (void *)gadgetfs_nop,
> - .disconnect = gadgetfs_nop,
> - .driver = {
> - .name   = "nop",
> - },
> -};
> -
> -
>  /* DEVICE INITIALIZATION
>   *
>   * fd = open ("/dev/gadget/$CHIP", O_RDWR)
> @@ -1968,10 +1946,7 @@ gadgetfs_fill_super (struct super_block *sb, void 
> *opts, int silent)
>   if (the_device)
>   return -ESRCH;
>  
> - /* fake probe to determine $CHIP */
> - CHIP = NULL;
> - usb_gadget_probe_driver(_driver);
> - if (!CHIP)
> + if (usb_get_gadget_udc_name(CHIP, sizeof(CHIP)) != 0)
>   return -ENODEV;
>  
>   /* superblock */
> diff --git a/drivers/usb/gadget/udc/udc-core.c 
> b/drivers/usb/gadget/udc/udc-core.c
> index fd73a3ea07c2..4bde2e110e44 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -442,6 +442,39 @@ err1:
>  EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
>  
>  /**
> + * usb_get_gadget_udc_name - get the name of the first UDC controller
> + * @dst_name
> + * @len

gotta describe these and add a blank line after @len too.

This also seems a little too big for -rc.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH RESEND] usb: gadget: gadgetfs: unregister gadget only if it got successfully registered

[Marek Szyprowski]

Gadgetfs driver called usb_gadget_unregister_driver unconditionally, even
if it didn't register it earlier due to other failures. This patch fixes
this.

Reported-by: Vegard Nossum 
Signed-off-by: Marek Szyprowski 
Tested-by: Vegard Nossum 
---
 drivers/usb/gadget/legacy/inode.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 7a62a2f7bc18..cde6a2133c90 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -130,7 +130,8 @@ struct dev_data {
setup_can_stall : 1,
setup_out_ready : 1,
setup_out_error : 1,
-   setup_abort : 1;
+   setup_abort : 1,
+   gadget_registered : 1;
unsignedsetup_wLength;
 
/* the rest is basically write-once */
@@ -1179,7 +1180,8 @@ dev_release (struct inode *inode, struct file *fd)
 
/* closing ep0 === shutdown all */
 
-   usb_gadget_unregister_driver (_driver);
+   if (dev->gadget_registered)
+   usb_gadget_unregister_driver (_driver);
 
/* at this point "good" hardware has disconnected the
 * device from USB; the host won't see it any more.
@@ -1825,6 +1827,7 @@ dev_config (struct file *fd, const char __user *buf, 
size_t len, loff_t *ptr)
 * kick in after the ep0 descriptor is closed.
 */
value = len;
+   dev->gadget_registered = true;
}
return value;
 
-- 
1.9.2

--
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