Re: Renesas uPD720202 USB 3.0

[Mathias Nyman]

On 16.05.2018 20:38, Christian Brauns wrote:

Hi


I'm running 4.16.8-1-ARCH and that controller does not work, whereas I remember 
it was working sometime before.

Now I'm travelling, don't have the controller with me, especially because it 
does not work anymore.

This is what a part of dmesg looked like in 4.16.8-1-ARCH:


[  103.911001] e1000e :00:19.0: Some CPU C-states have been disabled in 
order to enable jumbo frames
[  124.510436] pci :05:00.0: [1912:0015] type 00 class 0x0c0330
[  124.510548] pci :05:00.0: reg 0x10: [mem 0x-0x1fff 64bit]
[  124.510949] pci :05:00.0: PME# supported from D0 D3hot D3cold
[  124.520156] pci :05:00.0: BAR 0: assigned [mem 0xf1c0-0xf1c01fff 
64bit]
[  124.520220] pci :05:00.0: enabling device ( -> 0002)
[  124.536237] xhci_hcd :05:00.0: Resetting
[  125.968432] xhci_hcd :05:00.0: xHCI Host Controller
[  125.968439] xhci_hcd :05:00.0: new USB bus registered, assigned bus 
number 3
[  125.973722] xhci_hcd :05:00.0: hcc params 0x014051cf hci version 0x100 
quirks 0x0090
[  125.974199] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002
[  125.974201] usb usb3: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[  125.974202] usb usb3: Product: xHCI Host Controller
[  125.974204] usb usb3: Manufacturer: Linux 4.16.8-1-ARCH xhci-hcd
[  125.974205] usb usb3: SerialNumber: :05:00.0
[  125.974357] hub 3-0:1.0: USB hub found
[  125.974420] hub 3-0:1.0: 2 ports detected
[  125.974569] xhci_hcd :05:00.0: xHCI Host Controller
[  125.974573] xhci_hcd :05:00.0: new USB bus registered, assigned bus 
number 4
[  125.977237] usb usb4: We don't know the algorithms for LPM for this host, 
disabling LPM.
[  125.977272] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003
[  125.977274] usb usb4: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[  125.977276] usb usb4: Product: xHCI Host Controller
[  125.977277] usb usb4: Manufacturer: Linux 4.16.8-1-ARCH xhci-hcd
[  125.977279] usb usb4: SerialNumber: :05:00.0
[  125.977416] hub 4-0:1.0: USB hub found
[  125.977479] hub 4-0:1.0: 2 ports detected
[  126.566269] xhci_hcd :05:00.0: xHCI host controller not responding, 
assume dead
[  126.566322] xhci_hcd :05:00.0: HC died; cleaning up
[  126.914313] usb usb3-port1: couldn't allocate usb_device
[  126.914362] usb usb4-port1: couldn't allocate usb_device
[  127.414926] xhci_hcd :05:00.0: remove, state 1
[  127.414933] usb usb4: USB disconnect, device number 1
[  127.415106] xhci_hcd :05:00.0: USB bus 4 deregistered
[  127.415110] xhci_hcd :05:00.0: remove, state 1
[  127.415114] usb usb3: USB disconnect, device number 1
[  127.415282] xhci_hcd :05:00.0: Host halt failed, -19
[  127.415288] xhci_hcd :05:00.0: Host not accessible, reset failed.
[  127.415525] xhci_hcd :05:00.0: USB bus 3 deregistered
[  127.520610] pci :05:00.0: [1912:0015] type 00 class 0x0c0330
[  127.520740] pci :05:00.0: reg 0x10: [mem 0x-0x1fff 64bit]
[  127.521183] pci :05:00.0: PME# supported from D0 D3hot D3cold
[  127.530190] pci :05:00.0: BAR 0: assigned [mem 0xf1c0-0xf1c01fff 
64bit]
[  127.530261] pci :05:00.0: enabling device ( -> 0002)
[  127.530733] xhci_hcd :05:00.0: Resetting
[  128.955196] xhci_hcd :05:00.0: xHCI Host Controller
[  128.955202] xhci_hcd :05:00.0: new USB bus registered, assigned bus 
number 3
[  128.960487] xhci_hcd :05:00.0: hcc params 0x014051cf hci version 0x100 
quirks 0x0090
[  128.960931] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002
[  128.960933] usb usb3: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[  128.960934] usb usb3: Product: xHCI Host Controller
[  128.960935] usb usb3: Manufacturer: Linux 4.16.8-1-ARCH xhci-hcd
[  128.960936] usb usb3: SerialNumber: :05:00.0
[  128.961054] hub 3-0:1.0: USB hub found
[  128.96] hub 3-0:1.0: 2 ports detected
[  128.961208] xhci_hcd :05:00.0: xHCI Host Controller
[  128.961211] xhci_hcd :05:00.0: new USB bus registered, assigned bus 
number 4
[  128.964011] usb usb4: We don't know the algorithms for LPM for this host, 
disabling LPM.
[  128.964043] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003
[  128.964046] usb usb4: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[  128.964048] usb usb4: Product: xHCI Host Controller
[  128.964049] usb usb4: Manufacturer: Linux 4.16.8-1-ARCH xhci-hcd
[  128.964051] usb usb4: SerialNumber: :05:00.0
[  128.964213] hub 4-0:1.0: USB hub found
[  128.964276] hub 4-0:1.0: 2 ports detected
[  129.554923] xhci_hcd :05:00.0: xHCI host controller not responding, 
assume dead
[  129.554979] xhci_hcd :05:00.0: HC died; cleaning up
[  129.900959] usb usb3-port1: couldn't allocate usb_device
[  129.901008] usb usb4-port1: couldn't allocate usb_device

For kernel 4.13.1-1-ARCH I have a similar output here, saved from th

Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work

[Alexander Kappner]

Hi Alan,

thanks for reviewing. (This is my first contribution that touches 
usb-storage, so please bear with me.)

> That's kind of weird.  Does the drive work under Windows in UAS mode?  

On the Windows 10 VM that I just spun up for testing this, access to the 
drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the 
answer is no. 

> If so, why are the WRITE(16) commands failing under Linux?

I don't know exactly why they're failing, but another entry in the 
unusual_uas.h shows another SimpleTech device (only with a slightly 
different product ID) needing the same UAS quirk (see 
https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those 
drives are just buggy. 

> That doesn't quite make sense.  Since you prevent the uas driver from 
> binding to this device, it will end up using usb-storage no matter how 
> the kernel was built.  Therefore the second quirk flag has to go into 
> unusual_devs.h no matter what.

Yes that's what I was trying to get at. So even though the UAS flag would 
conceptually belong into unusual_uas, I'm putting both into unusual_devs to 
avoid having to create two separate entries for the same device.

> You don't describe the second quirk flag at all.  Are you sure it is 
> needed?

Yes. Without this flag, the device keeps throwing similar errors on 
usb-storage. That's the same result I get on a host that doesn't have UAS 
compiled in. Here's a dmesg:

[2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, 
bcdDevice=24.03
[2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[2.184980] usb 3-1: Product: G-DRIVE
[2.185447] usb 3-1: Manufacturer: HGST
[2.185829] usb 3-1: SerialNumber: AA015010004C
[2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected
[2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS
[2.202981] scsi host2: usb-storage 3-1:1.0
...
[3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE   2403 
PQ: 0 ANSI: 6
[3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0
[3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ 
CAPACITY(16).
[3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 
TB/3.64 TiB)
[3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks
[3.241255] sd 2:0:0:0: [sda] Write Protect is off
[3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08
[3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, 
supports DPO and FUA
[3.257893]  sda: sda1 sda9
[3.261402] sd 2:0:0:0: [sda] Attached SCSI disk
...
[   92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[   92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
[   92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[   92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 
00 00 00 00 00 08 00 00
[   92.437493] print_req_error: critical target error, dev sda, sector 0
[   92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page 
write
[   92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. Opts: 
(null)
[  101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
[  101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[  101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
00 18 00 00 00 08 00 00
[  101.452906] print_req_error: critical target error, dev sda, sector 
3905159192
[  101.453593] print_req_error: critical target error, dev sda, sector 
3905159192
[  101.454889] Aborting journal on device sda-8.
[  101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[  101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
[  101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[  101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
00 00 00 00 00 08 00 00

Source code comments describe this as a known problem (scsiglue.c:182):

/*
 * Some devices don't like MODE SENSE with page=0x3f,
 * which is the command used for checking if a device
 * is write-protected.  Now that we tell the sd driver
 * to do a 192-byte transfer with this command the
 * majority of devices work fine, but a few still can't
 * handle it.  The sd driver will simply assume those
 * devices are write-enabled.
 */
if (us->fflags & US_FL_NO_WP_DETECT)
sdev->skip_ms_page_3f = 1;

--agk 



On 05/16/2018 01:55 PM, Alan Stern wrote:
> On Wed, 16 May 2018, Alexander Kappner wrote:
> 
>> The "G-Drive" (sold by G-Technology) external USB 3.0 drive
>>  hangs on write access under UAS:
>>
>> [

Re: [PATCH] usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Thu, May 17, 2018 at 05:39:21AM +0200, Florian Zumbiehl wrote:
> Hi,
> 
> > > Note that the patch has only been fully tested with kernel 4.9.28, and
> > > test-compiled against 4.16.8. Tested only with the pl2303 adapters I have
> > > available (which seem to be type 0 if I interpret the code correctly), I
> > > don't have a clue whether this works with other versions of the chip.
> > 
> > What's the usb-devices (or lsusb -v) output for your device?
> 
> | T:  Bus=06 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#=  4 Spd=12  MxCh= 0
> | D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> | P:  Vendor=067b ProdID=2303 Rev=03.00
> | S:  Manufacturer=Prolific Technology Inc.
> | S:  Product=USB-Serial Controller
> | C:  #Ifs= 1 Cfg#= 1 Atr=80 MxPwr=100mA
> | I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

That looks like an HX device according to the current way we identify
these types (which may be wrong).

I have a similar device here but with Rev 04.00 and I can confirm that
your patch works with that one too.

> > > --- linux-4.16.8/drivers/usb/serial/pl2303.c.orig 2018-05-09 
> > > 09:53:14.0 +0200
> > > +++ linux-4.16.8/drivers/usb/serial/pl2303.c  2018-05-14 
> > > 04:32:27.860780856 +0200
> > > @@ -544,8 +544,12 @@ static void pl2303_set_termios(struct tt
> > >   int ret;
> > >   u8 control;
> > >  
> > > - if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
> > > - return;
> > > + if (old_termios &&
> > > + !(tty_termios_hw_change(&tty->termios, old_termios) ||
> > > +   ((tty->termios.c_iflag ^ old_termios->c_iflag) & (IXON | 
> > > IXANY)) ||
> > > +   tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP] ||
> > > +   tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART]))
> > 
> > Use a helper function for this (if everything is indeed needed, see
> > below).
> > 
> > > + return;
> > 
> > Odd indentation (which checkpatch would have caught).
> 
> Well, it did, but it didn't exactly offer a suggestion as to how to
> reformat things that would still be readable, nor does the style guide,
> AFAICS, so I kept it as it is.

You used double indentation (two tabs further) for the inner block
(return statement) instead of one.

Continuation lines (e.g. the broken if statement) should be indented
substantially to the right, which some subsystems interpret as at least
two tabs further (you used just one).

if (... &&
...) {
return;
}

> (Also, I doubt that had I encountered this
> condition moved out of line into a separate function that would have made
> it easier for me to follow what's going on.)

Depends on how you name your helper, I'd say. Another option could be to
use an intermediate bool. Either way, the above is hardly readable and
must be fixed.

> > >   buf = kzalloc(7, GFP_KERNEL);
> > >   if (!buf) {
> > > @@ -662,6 +666,9 @@ static void pl2303_set_termios(struct tt
> > >   pl2303_vendor_write(serial, 0x0, 0x41);
> > >   else
> > >   pl2303_vendor_write(serial, 0x0, 0x61);
> > > + } else if (I_IXON(tty) && !I_IXANY(tty) &&
> > > +START_CHAR(tty) == 0x11 && STOP_CHAR(tty) == 0x13) {
> > 
> > If the device does not support IXANY, I think you should just clear that
> > bit to indicate lack of support.
> > 
> > And the start and stop characters cannot be modified (as far as you
> > know)? Then we should probably set these in the termios whenever IXON is
> > set as well since usbserial does not support falling back to the
> > line-discipline IXON implementation.
> 
> I don't know how to enable IXANY or change the control characters, so I am
> working with what I do know (also, I guess the screenshot of the "ROM
> programming tool" in the "datasheet" suggests that none of that is
> supported).

Fair enough, that's why I asked. My device exhibits the same behaviour
with regards to IXANY by the way.

> Now, I agree that signaling lack of support would be better in general, but
> it does change what the code does for (still) unsupported cases. After all,
> usbserial in general does not signal that it doesn't support IXON/IXANY,
> but just pretends that it does. So, if there is a good reason why usbserial
> ignores POSIX on this point, I would think that would still apply for the
> remaining unsupported cases after partial support is implemented?!

There are historical reasons for a lot of things, but that's not
necessarily a reason to continue taking shortcuts.

> However, in any case, the part of setting particular control characters
> only when IXON is set seems like a very bad idea, in that it potentially
> changes settings where no change was requested, so userspace code cannot
> really be expected to check for that (and certainly not based on POSIX,
> which explicitly states that other fields have to stay unchanged). So, if
> we want to go wit

Re: `ucsi_acpi: probe of USBC000:00 failed with error -12` on Dell XPS 13 9370

[Heikki Krogerus]

Hi,

On Wed, May 16, 2018 at 04:13:31PM +, mario.limoncie...@dell.com wrote:
> 
> 
> > -Original Message-
> > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> > Sent: Wednesday, May 16, 2018 6:58 AM
> > To: Greg KH; Paul Menzel
> > Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Limonciello, 
> > Mario
> > Subject: Re: `ucsi_acpi: probe of USBC000:00 failed with error -12` on Dell 
> > XPS 13
> > 9370
> > 
> > Hi,
> > 
> > On Wed, May 16, 2018 at 10:02:26AM +0200, Greg KH wrote:
> > > On Tue, May 15, 2018 at 06:47:37PM +0200, Paul Menzel wrote:
> > > > Dear Greg,
> > > >
> > > >
> > > > As always, thank you for the prompt response.
> > > >
> > > >
> > > > On 05/15/18 18:00, Greg KH wrote:
> > > > > On Tue, May 15, 2018 at 04:34:03PM +0200, Paul Menzel wrote:
> > > >
> > > > > > Linux 4.17-rc5 shows the error below on the Dell XPS 13 9370 with 
> > > > > > Debian
> > > > > > Sid/unstable.
> > > > > >
> > > > > > ```
> > > > > > [???]
> > > > > > [0.440240] usb: port power management may be unreliable
> > > > > > [0.441358] usbcore: registered new interface driver usb-storage
> > > > > > [0.441367] usbcore: registered new interface driver 
> > > > > > usbserial_generic
> > > > > > [0.441369] usbserial: USB Serial support registered for generic
> > > > > > [0.441383] ioremap error for 0x3f799000-0x3f79a000, requested 
> > > > > > 0x2, got
> > > > > > 0x0
> > > > > > [0.441518] ucsi_acpi: probe of USBC000:00 failed with error -12
> > > > > > [???]
> > > > > > ```
> > > > > >
> > > > > > 1.  Are the ioremap and ucsi_acpi error related or is a separate 
> > > > > > report
> > > > > > needed?
> > > > >
> > > > > The ioremap error is what causes ucsi_acpi to fail the probe call (-12
> > > > > is "out of memory".)
> > > > >
> > > > > > 2.  Do you know the reason for the ucsi_acpi error?
> > > > >
> > > > > the call to ioremap failed.
> > > > >
> > > > > Does this device really have a working typec connector?
> > > >
> > > > Just to avoid misunderstandings, no device was connected to the laptop
> > > > during my test.
> > > >
> > > > But, from other boots, the Dell docking station TB16 kind of works with 
> > > > it,
> > > > so I???d say the USB Type-C connector is working.
> > >
> > > Ok, good, this might just be the acpi tables not set up properly for
> > > this type of connection.  Odd that the tables show it should work,
> > > Heikki should know more about this.
> > 
> > The firmware probable has not implemented UCSI on this board. I think
> > Dell always supplies the ACPI device node for UCSI in their acpi
> > tables. The _STA method in that device node is then used to inform the
> > OS if the interface exists or not. The return value for _STA comes
> > probable from BIOS, so this is most likely a BIOS problem.
> 
> Heikki,
> 
> I confirmed with internal team that UCSI is implemented on XPS 9370
> and was confirmed to be working properly with Windows 10 RS2+.

Just to double check: "UCSI was confirmed working properly", so not
"the Type-C ports were confirmed working properly"?

> The reason that _STA is responding on this device node now but wasn't
> previously is it wasn't exposed in Linux until 4.16 when the Win 10 RS2
> OSI string started to respond.

OK.

> Intel should internally have some XPS 9370 you can remotely access if
> you would like to poke around ACPI tables some.

I will try get access to XPS 9370, but with the acpi tables, if
somebody could just send me acpidump, that would be enough:

% acpidump -o xps9370_acpi.dump

> > Please note that UCSI will only supply status information to the
> > operating system, so the USB Type-C ports will function normally even
> > without it. The ports are handled in firmware on these platforms.
> > 
> > Paul, do you have the latest BIOS?
> > 
> > 
> > > > > Does normal USB devices work with it?
> > > >
> > > > Sorry for being ignorant, but could you please tell me what normal USB
> > > > devices are?
> > >
> > > If you plug a USB typeC device into this port, does it work?  A docking
> > > station is a little bit "different" in that it usually uses the PCIe
> > > connection, not the USB connectors.  Or at least that's how my Dell
> > > docking station works last time I tried it[1]
> 
> I think the best description here is "Non-Thunderbolt" USB type C device.
> Some examples:
> There are Dell docking stations with Thunderbolt (TB16) or without (WD15).
> 
> You can also pick up little dongles for ethernet or combo dongles for
> ethernet/VGA/HDMI/etc.
> 
> Anything non-Thunderbolt would satisfy what Greg was looking for.

Anything non-Thunderbolt and non-display.

With the display adapters you would be in DisplayPort alternate mode,
and you would again not be testing a normal USB device.

It never hurts to check that, but I think it's safe to assume that the
ports are functioning normally if the Thunderbolt dock was working.
Unless I'm mistaken, even the xHCI USB host controller behind the
USB

RE: dwc2: Regression on 96Boards Hikey due to enabling power down

[Artur Petrosyan]

Hi Mani,

We need some detailed information to perform debugging.

1. Could you please share the documentation of "96Boards HiKey" board, at least 
dwc core configuration parameters. Or dump of GHWCFG1-4.
2. Could you  please share with us full debug log of dwc2 loading and plug the 
USB device.
3. From short debug log seen that Host exited from hibernation after USB device 
plugged to the port. Do you mean that enumeration process didn't start? If not, 
could you please dump registers after "dwc2 f72c.usb: Host hibernation 
restore complete"
4. In which mode had the dwc2 been built. In host only mode or DRD mode?
5. And you mention that USB controller's DP/DM out is connected to a switch 
which switches between a USB type C port and a HUB (USB2513B). Is the device 
connected to USB type C port or to HUB (USB2513B) ? Switching connection done 
before dwc2 load or after?

Looking forward to your reply.

Regards,
Artur


-Original Message-
From: Manivannan Sadhasivam [mailto:manivannan.sadhasi...@linaro.org] 
Sent: Wednesday, May 16, 2018 17:23
To: linux-usb@vger.kernel.org
Cc: john.y...@synopsys.com; mvar...@synopsys.com; 
arthur.petros...@synopsys.com; grigor.tovmas...@synopsys.com; 
felipe.ba...@linux.intel.com
Subject: usb: dwc2: Regression on 96Boards Hikey due to enabling power down

Hello,

Commit 03ea6d6e9e1ff1b0222eb723eee5990d3511cc4d introduced the powerdown 
feature in USB DWC2 driver which stops USB from working on 96Boards HiKey board.

During bootup, USB host controller goes into hibernation state and when any USB 
device is plugged onto the port, the host finishes executing the GPWRDN 
interrupt handler but still it didn't wake up.

Below is the dmesg log after plugging a device onto USB port:

[   30.763414] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: 
dwc2_handle_gpwrdwn_intr called gpwrdn= 081411bb
[   30.763458] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: GPWRDN_LNSTSCHG
[   30.763492] dwc2 f72c.usb: dwc2_host_exit_hibernation: called with 
rem_wakeup = 1 reset = 0
[   30.763623] dwc2 f72c.usb: dwc2_restore_essential_regs: restoring 
essential regs
[   30.763671] dwc2 f72c.usb: restore done  generated here
[   30.976707] dwc2 f72c.usb: dwc2_restore_global_registers
[   30.976723] dwc2 f72c.usb: dwc2_restore_host_registers
[   30.976741] dwc2 f72c.usb: Host hibernation restore complete

I tried to manually reset the HUB and PHY after plugging in the device but that 
didn't help.

This issue has been detected in kernel 4.17-rc1.

Can anyone shed some light here? Any help would be greatly appreciated!

Note: On this board, the USB controller's DP/DM out is connected to a switch 
which switches between a USB type C port and a HUB (USB2513B).

Thanks,
Mani
--
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] usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Thu, May 17, 2018 at 10:29:14AM +0200, Johan Hovold wrote:
> On Thu, May 17, 2018 at 05:39:21AM +0200, Florian Zumbiehl wrote:

> > Now, I agree that signaling lack of support would be better in general, but
> > it does change what the code does for (still) unsupported cases. After all,
> > usbserial in general does not signal that it doesn't support IXON/IXANY,
> > but just pretends that it does. So, if there is a good reason why usbserial
> > ignores POSIX on this point, I would think that would still apply for the
> > remaining unsupported cases after partial support is implemented?!
> 
> There are historical reasons for a lot of things, but that's not
> necessarily a reason to continue taking shortcuts.

But on second thought, I think your approach here makes sense. If
usbserial ever gains generic IXON support, we'd just fall back to the
line-discipline implementation whenever the control characters do no
match.

Just clean up some of the lengthy conditionals (e.g. using intermediate
bools), and mention why you implemented things the way you did in the
commit message.

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] USB: serial: use tty_port_register_device()

[Johan Hovold]

On Wed, May 16, 2018 at 11:47:39AM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 16, 2018 at 11:42:07AM +0200, Johan Hovold wrote:
> > We already have the tty port when probing a usb-serial port so use
> > tty_port_register_device() directly instead of tty_port_install() later
> > to set up the port link.
> > 
> > This is a step towards enabling serdev for usb-serial (but we need to
> > determine how to handle hotplugging first).
> > 
> > Signed-off-by: Johan Hovold 
> 
> Reviewed-by: Greg Kroah-Hartman 

Thanks for reviewing. Now applied.

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: dwc2: Regression on 96Boards Hikey due to enabling power down

[Manivannan Sadhasivam]

Hi Artur,

Thanks for the reply!

On Thu, May 17, 2018 at 09:10:06AM +, Artur Petrosyan wrote:
> Hi Mani,
> 
> We need some detailed information to perform debugging.
> 
> 1. Could you please share the documentation of "96Boards HiKey" board, at 
> least dwc core configuration parameters. Or dump of GHWCFG1-4.

You can find the HiKey documentation here:
https://github.com/96boards/documentation/tree/master/consumer/hikey/hardware-docs

GHWCFG register dump:

GHWCFG1 = 0x
GHWCFG2 = 0x23affc70
GHWCFG3 = 0x0780d4e8
GHWCFG4 = 0xfff00060

> 2. Could you  please share with us full debug log of dwc2 loading and plug 
> the USB device.

Here is the full kernel log from the boot till USB device gets plugged in:
https://pastebin.ubuntu.com/p/3bZwWtk8wD/

> 3. From short debug log seen that Host exited from hibernation after USB 
> device plugged to the port. Do you mean that enumeration process didn't 
> start? If not, could you please dump registers after "dwc2 f72c.usb: Host 
> hibernation restore complete"

Yeah, I guess the enumeration process didn't happen at all. Here is the
register dump after plugging the device:

https://pastebin.ubuntu.com/p/3vxdkFGgp6/

> 4. In which mode had the dwc2 been built. In host only mode or DRD mode?

DWC2 in HiKey is built in Dual Role Device (DRD) mode.

> 5. And you mention that USB controller's DP/DM out is connected to a switch 
> which switches between a USB type C port and a HUB (USB2513B). Is the device 
> connected to USB type C port or to HUB (USB2513B) ? Switching connection done 
> before dwc2 load or after?
>

The USB device is connected to one of the ports from HUB and there is one
gpio which is used for switching the USB ports between Type-C and HUB. By
default, this gpio is unused and it pulled low, which means HUB will get
selected before dwc2 gets loaded.

Hope this information will help debugging the issue.

Thanks,
Mani

> Looking forward to your reply.
> 
> Regards,
> Artur
> 
> 
> -Original Message-
> From: Manivannan Sadhasivam [mailto:manivannan.sadhasi...@linaro.org] 
> Sent: Wednesday, May 16, 2018 17:23
> To: linux-usb@vger.kernel.org
> Cc: john.y...@synopsys.com; mvar...@synopsys.com; 
> arthur.petros...@synopsys.com; grigor.tovmas...@synopsys.com; 
> felipe.ba...@linux.intel.com
> Subject: usb: dwc2: Regression on 96Boards Hikey due to enabling power down
> 
> Hello,
> 
> Commit 03ea6d6e9e1ff1b0222eb723eee5990d3511cc4d introduced the powerdown 
> feature in USB DWC2 driver which stops USB from working on 96Boards HiKey 
> board.
> 
> During bootup, USB host controller goes into hibernation state and when any 
> USB device is plugged onto the port, the host finishes executing the GPWRDN 
> interrupt handler but still it didn't wake up.
> 
> Below is the dmesg log after plugging a device onto USB port:
> 
> [   30.763414] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: 
> dwc2_handle_gpwrdwn_intr called gpwrdn= 081411bb
> [   30.763458] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: GPWRDN_LNSTSCHG
> [   30.763492] dwc2 f72c.usb: dwc2_host_exit_hibernation: called with 
> rem_wakeup = 1 reset = 0
> [   30.763623] dwc2 f72c.usb: dwc2_restore_essential_regs: restoring 
> essential regs
> [   30.763671] dwc2 f72c.usb: restore done  generated here
> [   30.976707] dwc2 f72c.usb: dwc2_restore_global_registers
> [   30.976723] dwc2 f72c.usb: dwc2_restore_host_registers
> [   30.976741] dwc2 f72c.usb: Host hibernation restore complete
> 
> I tried to manually reset the HUB and PHY after plugging in the device but 
> that didn't help.
> 
> This issue has been detected in kernel 4.17-rc1.
> 
> Can anyone shed some light here? Any help would be greatly appreciated!
> 
> Note: On this board, the USB controller's DP/DM out is connected to a switch 
> which switches between a USB type C port and a HUB (USB2513B).
> 
> Thanks,
> Mani
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/7] usb: typec: Generalize mux mode names

[Heikki Krogerus]

On Wed, May 16, 2018 at 11:11:02PM +0200, Mats Karrman wrote:
> On 05/16/2018 01:43 PM, Heikki Krogerus wrote:
> 
> > On Tue, May 15, 2018 at 11:24:07PM +0200, Mats Karrman wrote:
> >> Hi Heikki,
> >>
> >> On 05/15/2018 09:30 AM, Heikki Krogerus wrote:
> >>
> >>> Hi Mats,
> >>>
> >>> On Fri, May 11, 2018 at 09:28:04PM +0200, Mats Karrman wrote:
>  On 2018-05-11 13:14, Heikki Krogerus wrote:
> 
> > On Fri, May 11, 2018 at 11:05:55AM +0200, Mats Karrman wrote:
> >> On 2018-05-10 19:49, Heikki Krogerus wrote:
> >>
> >>> On Thu, May 10, 2018 at 10:04:21AM +0200, Mats Karrman wrote:
>  Hi,
> 
>  On 05/09/2018 02:49 PM, Heikki Krogerus wrote:
> 
> > On Tue, May 08, 2018 at 09:10:13PM +0200, Mats Karrman wrote:
> >> Hi,
> >>
> >> On 05/08/2018 04:25 PM, Heikki Krogerus wrote:
> >>
> >>> Hi,
> >>>
> >>> On Mon, May 07, 2018 at 11:19:40PM +0200, Mats Karrman wrote:
> >> Even so, when the mux driver "set" function is called, it will 
> >> just get the
> >> mode argument but since the mode (TYPEC_STATE_...) is 
> >> overlapping for different
> >> AMs if I understand your proposal correctly, the mux also 
> >> needs to know what AM
> >> is active.
> >> Does this imply that the mux set function signature need to 
> >> change?
> > My plan was actually to propose we get rid of the current mux 
> > handling
> > (just leave the orientation switch) in favour of the 
> > notifications I'm
> > introducing with the type-c bus for the alternate modes. The 
> > current
> > mux handling is definitely not enough, and does not work in 
> > every
> > scenario, like also you pointed out.
>  So, the mux need to subscribe to each svid:mode pair it is 
>  interested in using
>  typec_altmode_register_notifier() and then use those callbacks 
>  to switch the correct
>  signals to the connector. And a driver for an off-the-shelf mux 
>  device could have
>  the translation between svid:mode pairs and mux device specific 
>  control specified by
>  of/acpi properties. Right?
> >>> Yes. That is the plan. Would it work for you?
> >> I think so. I'll give it a go. When about do you think you'll post 
> >> the next version
> >> of your RFC? Or do you have an updated series available somewhere 
> >> public?
> > I'll try to put together and post the next version tomorrow.
> >
> > My original plan was actually to use just the notifications with the
> > muxes, but one thing to consider with the notifications is that in
> > practice we have to increment the ref count for the alt mode devices
> > when ever something registers a notifier.
> >
> > To me that does not feel ideal. The dependency should go the other 
> > way
> > around in case of the muxes. That is why I liked the separate API 
> > and
> > handling for the muxes felt better, as it will do just that. The mux
> > is then a "service" that the port driver can as for, and if it gets 
> > a
> > handle to a mux, the mux will have its ref count incremented.
> >
> > So I think fixing the mux API would perhaps be better after all.
> > Thoughts?
>  So, we're back to a mux API similar to the current one, where:
>  - the port driver and the mux driver are connected through "graph"
>  - alt mode driver finds its port mux using the typec class mux api
>  - the mux mode setting function arguments now include both svid and 
>  mode
> 
>  I like that.
> 
>  One thought that popped up again is if we, somewhere down the line,
>  will see some super device that support many different alternate 
>  modes
>  on the same port and therefore will need to have multiple mux 
>  devices?
>  However I think the mux api could be extended (later on) to support 
>  some
>  aggregate mux device that manages multiple physical devices.
> >>> If we simply had always a mux for every alternate mode, that would not
> >>> be a problem. So the port would have its own mux, and every supported
> >>> alternate mode also had its own. I think that removes the need to deal
> >>> with the svid:mode when using the muxes, as they are already tied to a
> >>> specific alternate modes, right? With a single mux device, for example
> >>> pi3usb30532, the driver just needs to register a mux for the port and
> >>> separate mux for DP, but I don't think that's a huge problem.

Re: usb: dwc2: gadget: Fix memory leak in dwc2_gadget_init()

[Marek Szyprowski]

Hi All,

I've just noticed this patch has been applied to linux-next.

It breaks DWC2 driver initialization on Samsung Exynos-based boards
I've use for kernel daily tests.

Here is an example of the call stack:

Unable to handle kernel NULL pointer dereference at virtual address 000c
pgd = (ptrval)
[000c] *pgd=
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 25 Comm: kworker/0:1 Not tainted 4.17.0-rc5-next-20180517 #782
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: events deferred_probe_work_func
PC is at usb_ep_alloc_request+0x1c/0x200
LR is at composite_dev_prepare+0x20/0xec
pc : []    lr : []    psr: 6153
sp : d95f1d10  ip :   fp : 0002
r10: c1072d58  r9 : d82443d0  r8 : 
r7 : c1074180  r6 : c10ad460  r5 : c100746c  r4 : d8253980
r3 :   r2 : d82539b8  r1 : 014000c0  r0 : d82443d0
Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 4000406a  DAC: 0051
Process kworker/0:1 (pid: 25, stack limit = 0x(ptrval))
Stack: (0xd95f1d10 to 0xd95f2000)
...
[] (usb_ep_alloc_request) from [] 
(composite_dev_prepare+0x20/0xec)
[] (composite_dev_prepare) from [] 
(composite_bind+0x7c/0x1c8)
[] (composite_bind) from [] 
(udc_bind_to_driver+0x74/0x134)
[] (udc_bind_to_driver) from [] 
(check_pending_gadget_drivers+0x74/0xac)
[] (check_pending_gadget_drivers) from [] 
(usb_add_gadget_udc_release+0x190/0x1ec)
[] (usb_add_gadget_udc_release) from [] 
(dwc2_gadget_init+0x274/0x464)
[] (dwc2_gadget_init) from [] 
(dwc2_driver_probe+0x488/0x50c)
[] (dwc2_driver_probe) from [] 
(platform_drv_probe+0x48/0x9c)
[] (platform_drv_probe) from [] 
(driver_probe_device+0x2dc/0x4ac)
[] (driver_probe_device) from [] 
(bus_for_each_drv+0x74/0xb8)
[] (bus_for_each_drv) from [] 
(__device_attach+0xd4/0x168)
[] (__device_attach) from [] 
(bus_probe_device+0x88/0x90)
[] (bus_probe_device) from [] 
(deferred_probe_work_func+0x60/0x180)
[] (deferred_probe_work_func) from ad+0x28c/0x58c)
[] (worker_thread) from [] (kthread+0x138/0x168)
[] (kthread) from [] (ret_from_fork+0x14/0x20)


A proper fix for the potential memory leak is to add free to error path
instead of reordering usb_add_gadget_udc() call. The issue happens if DWC2
driver is initialized from deferred probe (in such case the gadget driver
is already registered).

Felipe: please drop or revert this patch in your -next branch.


Best regards
Marek Szyprowski, PhD
Samsung R&D Institute Poland

On 2018-04-16 12:16, Grigor Tovmasyan wrote:
> In dwc2_gadget_init() we allocate EP0 request via
> dwc2_hsotg_ep_alloc_request(). After that there are
> usb_add_gadget_udc() call and if it failed, then
> ctrl_req will not be freed and will cause memory leak.
>
> Reordered function calls in gadget_init: moved up usb_add_gadget_udc()
> before dwc2_hsotg_ep_alloc_request().
>
> Tested using kmemleak.
>
> Cc: Stefan Wahren 
> Signed-off-by: Grigor Tovmasyan 
> Acked-by: Minas Harutyunyan 
> ---
>   drivers/usb/dwc2/gadget.c | 8 
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 6c32bf26e48e..24000bda5c20 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4679,6 +4679,10 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
>   INIT_LIST_HEAD(&hsotg->gadget.ep_list);
>   hsotg->gadget.ep0 = &hsotg->eps_out[0]->ep;
>   
> + ret = usb_add_gadget_udc(dev, &hsotg->gadget);
> + if (ret)
> + return ret;
> +
>   /* allocate EP0 request */
>   
>   hsotg->ctrl_req = dwc2_hsotg_ep_alloc_request(&hsotg->eps_out[0]->ep,
> @@ -4698,10 +4702,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> epnum, 0);
>   }
>   
> - ret = usb_add_gadget_udc(dev, &hsotg->gadget);
> - if (ret)
> - return ret;
> -
>   dwc2_hsotg_dump(hsotg);
>   
>   return 0;
>

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

Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config

[Heikki Krogerus]

On Wed, May 16, 2018 at 10:55:36PM +0200, Mats Karrman wrote:
> Hi,
> 
> On 05/16/2018 02:25 PM, Heikki Krogerus wrote:
> 
> > Hi guys,
> >
> > On Tue, May 15, 2018 at 10:52:57PM +0200, Mats Karrman wrote:
> >> Hi,
> >>
> >> On 05/14/2018 11:36 AM, Jun Li wrote:
> >>
> >>> Hi
>  -Original Message-
>  From: Mats Karrman [mailto:mats.dev.l...@gmail.com]
>  Sent: 2018???5???12??? 3:56
>  To: Jun Li ; robh...@kernel.org; 
>  gre...@linuxfoundation.org;
>  heikki.kroge...@linux.intel.com; li...@roeck-us.net
>  Cc: a.ha...@samsung.com; cw00.c...@samsung.com;
>  shufan_...@richtek.com; Peter Chen ;
>  gso...@gmail.com; devicet...@vger.kernel.org; linux-usb@vger.kernel.org;
>  dl-linux-imx 
>  Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic 
>  port power
>  and data config
> 
>  Hi Li Jun,
> 
>  On 2018-05-03 02:24, Li Jun wrote:
> 
> > This patch adds 3 APIs to get the typec port power and data type, and
> > preferred power role by its name string.
> >
> > Signed-off-by: Li Jun 
> > ---
> >   drivers/usb/typec/class.c | 52
>  +++
> >   include/linux/usb/typec.h |  3 +++
> >   2 files changed, 55 insertions(+)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 53df10d..5981e18 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -9,6 +9,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> > I don't think you need that anymore.
> >
> >   #include 
> >   #include 
> >   #include 
> > @@ -802,6 +803,12 @@ static const char * const typec_port_types[] = {
> > [TYPEC_PORT_DRP] = "dual",
> >   };
> >
> > +static const char * const typec_data_types[] = {
> > +   [TYPEC_PORT_DFP] = "host",
> > +   [TYPEC_PORT_UFP] = "device",
> > +   [TYPEC_PORT_DRD] = "dual",
> > +};
> > +
> >   static const char * const typec_port_types_drp[] = {
> > [TYPEC_PORT_SRC] = "dual [source] sink",
> > [TYPEC_PORT_SNK] = "dual source [sink]", @@ -1252,6 +1259,51
>  @@
> > void typec_set_pwr_opmode(struct typec_port *port,
> >   }
> >   EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
> >
> > +/**
> > + * typec_find_power_type - Get the typec port power type
>  Why is this function called typec_find_power_type() and not
>  typec_find_port_type()?
>  It's called port_type in sysfs, having different names just adds 
>  confusion.
>  (Otherwise I agree power_type is a better name but...)
> >>> We have "port type" before the power and data role separation,
> >>> this API name's intention is to reflect the power cap, anyway I
> >>> leave this to be decided by Heikki then.
> > I really hate the "*_type" naming. It was understandable when there
> > was no separate power and data roles defined in the specification, but
> > now that there are, it's just confusing. IMO we should not use it
> > anywhere.
> >
> > So to me typec_find_type() is just as bad as typec_find_power_type()
> > because it has the "type" in it. I wonder if this function is
> > necessary at all? If it is, then perhaps we can think of some better
> > name for it, name that gives a better hint what it is used for.
> 
> I reread this patch and tried to see it more in the context of the other
> patches and the existing code. The naming of the existing string tables
> doesn't help in getting this right, however I have a proposal:
> 
> typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP
> typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD
> typec_find_power_role() to get to TYPEC_SINK/SOURCE
> 
> and sometime, if the use should arise 
> 
> typec_find_data_role() to get to TYPEC_DEVICE/HOST
> 
> I think it is fairly comprehensible, *_port_* concerns a capability and
> without *_port_* it is an actual state. Plus it matches the names of the
> constants.

Well, there are now four things to consider:

1) the roles (power and data) the port is capable of supporting
2) Try.SRC and Try.SNK, i.e. the preferred role
3) the current roles
4) the role(s) the user want's to limit the use of a port with DRP ports

The last one I don't know if it's relevant with these functions. It's
not information that we would get for example from firmware.

I also don't think we need to use these functions with the current
roles the port is in.

If the preferred role is "sink" or "source", so just like the power
role, we don't need separate function for it here.

So isn't two functions all we need here: one for the power and one for
data 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

[PATCH 0/3] Revised Renesas uPD72020x workaround for 32bit DMA issue

[Marc Zyngier]

Back around the 4.13 timeframe, we tried to address a rather bad issue
with the Renesas uPD72020x USB3 controller family, that have a
horrible issue with the programming of the base addresses which tend
to stick on XHCI reset. This makes transitionning from 64bit to 32bit
addresses completely unsafe. This was observed on a fairly popular
arm64 platform (AMD Opteron 1100, which has all of its memory above
4GB).

The fixe was to perform a PCI reset of the device, but we have
multiple reports that this generated regressions (the controller not
being usable after the patch was applied).

This series reverts the problematic patch, and tries to address it in
a more constrained way. If the controller is behind an IOMMU (and only
in that case), we zero its base addresses before reseting it. In the
affected configuration, this has the effect of putting the device in a
state where the XHCI reset will be effective.

It must be noted that in the absence of an IOMMU (and maybe even in
its presence), this device is completely unsafe, and may silently
corrupt memory.

Marc Zyngier (3):
  xhci: Allow more than 32 quirks
  xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers
  Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA
issue"

 drivers/usb/host/pci-quirks.c | 20 -
 drivers/usb/host/pci-quirks.h |  1 -
 drivers/usb/host/xhci-pci.c   | 15 --
 drivers/usb/host/xhci.c   | 65 +--
 drivers/usb/host/xhci.h   |  3 +-
 5 files changed, 70 insertions(+), 34 deletions(-)

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

[PATCH 3/3] Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"

[Marc Zyngier]

This reverts commit 8466489ef5ba48272ba4fa4ea9f8f403306de4c7.

Now that we can properly reset the uPD72020x without a hard PCI reset,
let's get rid of the existing quirks.

Signed-off-by: Marc Zyngier 
---
 drivers/usb/host/pci-quirks.c | 20 
 drivers/usb/host/pci-quirks.h |  1 -
 drivers/usb/host/xhci-pci.c   |  7 ---
 drivers/usb/host/xhci.c   | 21 -
 drivers/usb/host/xhci.h   |  2 +-
 5 files changed, 13 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 67ad4bb6919a..3625a5c1a41b 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -1268,23 +1268,3 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev)
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff);
-
-bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
-{
-   /*
-* Our dear uPD72020{1,2} friend only partially resets when
-* asked to via the XHCI interface, and may end up doing DMA
-* at the wrong addresses, as it keeps the top 32bit of some
-* addresses from its previous programming under obscure
-* circumstances.
-* Give it a good wack at probe time. Unfortunately, this
-* needs to happen before we've had a chance to discover any
-* quirk, or the system will be in a rather bad state.
-*/
-   if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
-   (pdev->device == 0x0014 || pdev->device == 0x0015))
-   return true;
-
-   return false;
-}
-EXPORT_SYMBOL_GPL(usb_xhci_needs_pci_reset);
diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
index 4ca0d9b7e463..63c633077d9e 100644
--- a/drivers/usb/host/pci-quirks.h
+++ b/drivers/usb/host/pci-quirks.h
@@ -16,7 +16,6 @@ void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev);
 void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev);
 void usb_disable_xhci_ports(struct pci_dev *xhci_pdev);
 void sb800_prefetch(struct device *dev, int on);
-bool usb_xhci_needs_pci_reset(struct pci_dev *pdev);
 bool usb_amd_pt_check_port(struct device *device, int port);
 #else
 struct pci_dev;
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index e0a0a12871e2..6372edf339d9 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -288,13 +288,6 @@ static int xhci_pci_probe(struct pci_dev *dev, const 
struct pci_device_id *id)
 
driver = (struct hc_driver *)id->driver_data;
 
-   /* For some HW implementation, a XHCI reset is just not enough... */
-   if (usb_xhci_needs_pci_reset(dev)) {
-   dev_info(&dev->dev, "Resetting\n");
-   if (pci_reset_function_locked(dev))
-   dev_warn(&dev->dev, "Reset failed");
-   }
-
/* Prevent runtime suspending between USB-2 and USB-3 initialization */
pm_runtime_get_noresume(&dev->dev);
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index cd6b48c43007..07272d1ce32a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4923,16 +4923,19 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
 
/*
 * Some Renesas controllers get into a weird state if they are
-* reset while programmed with 64bit addresses (they will
-* preserve the top half of the address in internal, non
-* visible registers). You end up with half the address coming
-* from the kernel, and the other half coming from the
-* firmware. Also, changing the programming leads to extra
-* accesses even if the controller is supposed to be
-* halted. The controller ends up with a fatal fault, and is
-* then ripe for being properly reset.
+* reset while programmed with 64bit addresses (they will preserve
+* the top half of the address in internal, non visible
+* registers). You end up with half the address coming from the
+* kernel, and the other half coming from the firmware. Also,
+* changing the programming leads to extra accesses even if the
+* controller is supposed to be halted. The controller ends up with
+* a fatal fault, and is then ripe for being properly reset.
+*
+* Special care is taken to only apply this if the device is behind
+* an iommu. Doing anything when there is no iommu is definitely
+* unsafe...
 */
-   if (xhci->quirks & XHCI_ZERO_64B_REGS) {
+   if ((xhci->quirks & XHCI_ZERO_64B_REGS) && dev->iommu_group) {
u64 val;
int i;
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 5ea0b52b49cc..758864767fd4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1831,7 +1831,7 @@ struct xhci_hcd {
 #define XHCI_HW_LPM_DISABLE(1 << 29)
 #define XHCI_SU

[PATCH 1/3] xhci: Allow more than 32 quirks

[Marc Zyngier]

We now have 32 different quirks, and the field that holds them
is full. Let's bump it up to the next stage so that we can handle
some more... The type is now an unsigned long long, which is 64bit
on most architectures.

Signed-off-by: Marc Zyngier 
---
 drivers/usb/host/xhci.c | 6 +++---
 drivers/usb/host/xhci.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 711da3306b14..8dba26d3de07 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -33,8 +33,8 @@ static int link_quirk;
 module_param(link_quirk, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(link_quirk, "Don't clear the chain bit on a link TRB");
 
-static unsigned int quirks;
-module_param(quirks, uint, S_IRUGO);
+static unsigned long long quirks;
+module_param(quirks, ullong, S_IRUGO);
 MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
 
 /* TODO: copied from ehci-hcd.c - can this be refactored? */
@@ -4963,7 +4963,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t 
get_quirks)
return retval;
xhci_dbg(xhci, "Called HCD init\n");
 
-   xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%08x\n",
+   xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%016llx\n",
  xhci->hcc_params, xhci->hci_version, xhci->quirks);
 
return 0;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 6dfc4867dbcf..42848dfc3445 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1787,7 +1787,7 @@ struct xhci_hcd {
 #define XHCI_STATE_DYING   (1 << 0)
 #define XHCI_STATE_HALTED  (1 << 1)
 #define XHCI_STATE_REMOVING(1 << 2)
-   unsigned intquirks;
+   unsigned long long  quirks;
 #defineXHCI_LINK_TRB_QUIRK (1 << 0)
 #define XHCI_RESET_EP_QUIRK(1 << 1)
 #define XHCI_NEC_HOST  (1 << 2)
-- 
2.14.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

[PATCH 2/3] xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers

[Marc Zyngier]

Some Renesas controllers get into a weird state if they are reset while
programmed with 64bit addresses (they will preserve the top half of the
address in internal, non visible registers).

You end up with half the address coming from the kernel, and the other
half coming from the firmware.

Also, changing the programming leads to extra accesses even if the
controller is supposed to be halted. The controller ends up with a fatal
fault, and is then ripe for being properly reset. On the flip side,
this is completely unsafe if the defvice isn't behind an IOMMU, so
we have to make sure that this is the case. Can you say "broken"?

This is an alternative method to the one introduced in 8466489ef5ba
("xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"),
which will subsequently be removed.

Signed-off-by: Marc Zyngier 
---
 drivers/usb/host/xhci-pci.c |  8 +--
 drivers/usb/host/xhci.c | 56 +
 drivers/usb/host/xhci.h |  1 +
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 85ffda85f8ab..e0a0a12871e2 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -196,11 +196,15 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_BROKEN_STREAMS;
}
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
-   pdev->device == 0x0014)
+   pdev->device == 0x0014) {
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
+   xhci->quirks |= XHCI_ZERO_64B_REGS;
+   }
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
-   pdev->device == 0x0015)
+   pdev->device == 0x0015) {
xhci->quirks |= XHCI_RESET_ON_RESUME;
+   xhci->quirks |= XHCI_ZERO_64B_REGS;
+   }
if (pdev->vendor == PCI_VENDOR_ID_VIA)
xhci->quirks |= XHCI_RESET_ON_RESUME;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8dba26d3de07..cd6b48c43007 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4921,6 +4921,62 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
if (retval)
return retval;
 
+   /*
+* Some Renesas controllers get into a weird state if they are
+* reset while programmed with 64bit addresses (they will
+* preserve the top half of the address in internal, non
+* visible registers). You end up with half the address coming
+* from the kernel, and the other half coming from the
+* firmware. Also, changing the programming leads to extra
+* accesses even if the controller is supposed to be
+* halted. The controller ends up with a fatal fault, and is
+* then ripe for being properly reset.
+*/
+   if (xhci->quirks & XHCI_ZERO_64B_REGS) {
+   u64 val;
+   int i;
+
+   xhci_info(xhci,
+ "Zeroing 64bit base registers, expecting fault\n");
+
+   /* Clear HSEIE so that faults do not get signaled */
+   val = readl(&xhci->op_regs->command);
+   val &= ~CMD_HSEIE;
+   writel(val, &xhci->op_regs->command);
+
+   /* Clear HSE (aka FATAL) */
+   val = readl(&xhci->op_regs->status);
+   val |= STS_FATAL;
+   writel(val, &xhci->op_regs->status);
+
+   /* Now zero the registers, and brace for impact */
+   val = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
+   if (upper_32_bits(val))
+   xhci_write_64(xhci, 0, &xhci->op_regs->dcbaa_ptr);
+   val = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+   if (upper_32_bits(val))
+   xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring);
+
+   for (i = 0; i < HCS_MAX_INTRS(xhci->hcs_params1); i++) {
+   struct xhci_intr_reg __iomem *ir;
+
+   ir = &xhci->run_regs->ir_set[i];
+   val = xhci_read_64(xhci, &ir->erst_base);
+   if (upper_32_bits(val))
+   xhci_write_64(xhci, 0, &ir->erst_base);
+   val= xhci_read_64(xhci, &ir->erst_dequeue);
+   if (upper_32_bits(val))
+   xhci_write_64(xhci, 0, &ir->erst_dequeue);
+   }
+
+   /* Wait for the fault to appear. It will be cleared on reset */
+   retval = xhci_handshake(&xhci->op_regs->status,
+   STS_FATAL, STS_FATAL,
+   XHCI_MAX_HALT_USEC);
+   if (!retval)
+   xhci_info(xhci, "Fault detected\n");
+   }
+
xhci_dbg(xhci, "Resetting HCD\n");
/* Reset the internal HC memor

RE: `ucsi_acpi: probe of USBC000:00 failed with error -12` on Dell XPS 13 9370

[Mario.Limonciello]

> -Original Message-
> From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> Sent: Thursday, May 17, 2018 4:00 AM
> To: Limonciello, Mario
> Cc: g...@kroah.com; pmenzel+linux-...@molgen.mpg.de; linux-
> u...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: `ucsi_acpi: probe of USBC000:00 failed with error -12` on Dell 
> XPS 13
> 9370
> 
> Hi,
> 
> On Wed, May 16, 2018 at 04:13:31PM +, mario.limoncie...@dell.com wrote:
> >
> >
> > > -Original Message-
> > > From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> > > Sent: Wednesday, May 16, 2018 6:58 AM
> > > To: Greg KH; Paul Menzel
> > > Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Limonciello, 
> > > Mario
> > > Subject: Re: `ucsi_acpi: probe of USBC000:00 failed with error -12` on 
> > > Dell XPS 13
> > > 9370
> > >
> > > Hi,
> > >
> > > On Wed, May 16, 2018 at 10:02:26AM +0200, Greg KH wrote:
> > > > On Tue, May 15, 2018 at 06:47:37PM +0200, Paul Menzel wrote:
> > > > > Dear Greg,
> > > > >
> > > > >
> > > > > As always, thank you for the prompt response.
> > > > >
> > > > >
> > > > > On 05/15/18 18:00, Greg KH wrote:
> > > > > > On Tue, May 15, 2018 at 04:34:03PM +0200, Paul Menzel wrote:
> > > > >
> > > > > > > Linux 4.17-rc5 shows the error below on the Dell XPS 13 9370 with 
> > > > > > > Debian
> > > > > > > Sid/unstable.
> > > > > > >
> > > > > > > ```
> > > > > > > [???]
> > > > > > > [0.440240] usb: port power management may be unreliable
> > > > > > > [0.441358] usbcore: registered new interface driver 
> > > > > > > usb-storage
> > > > > > > [0.441367] usbcore: registered new interface driver 
> > > > > > > usbserial_generic
> > > > > > > [0.441369] usbserial: USB Serial support registered for 
> > > > > > > generic
> > > > > > > [0.441383] ioremap error for 0x3f799000-0x3f79a000, requested 
> > > > > > > 0x2,
> got
> > > > > > > 0x0
> > > > > > > [0.441518] ucsi_acpi: probe of USBC000:00 failed with error 
> > > > > > > -12
> > > > > > > [???]
> > > > > > > ```
> > > > > > >
> > > > > > > 1.  Are the ioremap and ucsi_acpi error related or is a separate 
> > > > > > > report
> > > > > > > needed?
> > > > > >
> > > > > > The ioremap error is what causes ucsi_acpi to fail the probe call 
> > > > > > (-12
> > > > > > is "out of memory".)
> > > > > >
> > > > > > > 2.  Do you know the reason for the ucsi_acpi error?
> > > > > >
> > > > > > the call to ioremap failed.
> > > > > >
> > > > > > Does this device really have a working typec connector?
> > > > >
> > > > > Just to avoid misunderstandings, no device was connected to the laptop
> > > > > during my test.
> > > > >
> > > > > But, from other boots, the Dell docking station TB16 kind of works 
> > > > > with it,
> > > > > so I???d say the USB Type-C connector is working.
> > > >
> > > > Ok, good, this might just be the acpi tables not set up properly for
> > > > this type of connection.  Odd that the tables show it should work,
> > > > Heikki should know more about this.
> > >
> > > The firmware probable has not implemented UCSI on this board. I think
> > > Dell always supplies the ACPI device node for UCSI in their acpi
> > > tables. The _STA method in that device node is then used to inform the
> > > OS if the interface exists or not. The return value for _STA comes
> > > probable from BIOS, so this is most likely a BIOS problem.
> >
> > Heikki,
> >
> > I confirmed with internal team that UCSI is implemented on XPS 9370
> > and was confirmed to be working properly with Windows 10 RS2+.
> 
> Just to double check: "UCSI was confirmed working properly", so not
> "the Type-C ports were confirmed working properly"?

UCSI was confirmed working properly.  FWIW it's a certification requirement
in Windows.

> 
> > The reason that _STA is responding on this device node now but wasn't
> > previously is it wasn't exposed in Linux until 4.16 when the Win 10 RS2
> > OSI string started to respond.
> 
> OK.
> 
> > Intel should internally have some XPS 9370 you can remotely access if
> > you would like to poke around ACPI tables some.
> 
> I will try get access to XPS 9370, but with the acpi tables, if
> somebody could just send me acpidump, that would be enough:
> 
> % acpidump -o xps9370_acpi.dump
> 
> > > Please note that UCSI will only supply status information to the
> > > operating system, so the USB Type-C ports will function normally even
> > > without it. The ports are handled in firmware on these platforms.
> > >
> > > Paul, do you have the latest BIOS?
> > >
> > >
> > > > > > Does normal USB devices work with it?
> > > > >
> > > > > Sorry for being ignorant, but could you please tell me what normal USB
> > > > > devices are?
> > > >
> > > > If you plug a USB typeC device into this port, does it work?  A docking
> > > > station is a little bit "different" in that it usually uses the PCIe
> > > > connection, not the USB connectors.  Or at least that's how my Dell
> > > > docking station works last tim

Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work

[Oliver Neukum]

Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner:
> Yes. Without this flag, the device keeps throwing similar errors on 
> usb-storage. That's the same result I get on a host that doesn't have UAS 
> compiled in. Here's a dmesg:

Hi,

this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
by itself would make the device work. Can you please test that?
You will need the attached patch for the quirk to be supported.

Regards
Oliver
From 1ff6c9c9cc66ddb4cf7ca95bea589d6a30c63623 Mon Sep 17 00:00:00 2001
From: Oliver Neukum 
Date: Thu, 17 May 2018 14:46:47 +0200
Subject: [PATCH] UAS: add support for the quirk US_FL_NO_WP_DETECT

The assumption that this quirk can be limited to old storage
devices was too optimistic.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/storage/uas.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 6034c39b67d1..7ee67e8c1f1e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -836,6 +836,10 @@ static int uas_slave_configure(struct scsi_device *sdev)
 	if (devinfo->flags & US_FL_BROKEN_FUA)
 		sdev->broken_fua = 1;
 
+	/* UAS also needs to support FL_NO_WP_DETECT */
+	if (devinfo->flags & US_FL_NO_WP_DETECT)
+		sdev->skip_ms_page_3f = 1;
+
 	scsi_change_queue_depth(sdev, devinfo->qdepth - 2);
 	return 0;
 }
-- 
2.13.6

ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Carlos Manuel Santos]

Hello.
I'm having troubles with this NFC card reader. It seems kernel driver
pn533 is not working properly.
At this moment I have my work stalled. I need to add NFC support to a
software product and I can't get the device to work. NFC tools won't
work, the device is not detected.

This is what I get from "dmesg":

[4.182300] nfc: nfc_init: NFC Core ver 0.1
[4.182318] NET: Registered protocol family 39
[4.184676] hidraw: raw HID events driver (C) Jiri Kosina
[4.193366] [ cut here ]
[4.193366] transfer buffer not dma capable
[4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
[4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
intel_rapl_perf tpm_crb psmouse
[4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
sysimgblt fb_sys_fops ttm drm agpgart
[4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 4.16.8-1-ARCH #1
[4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
PRO A50-C, BIOS Version 7.50   09/26/2016
[4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
[4.193468] RSP: 0018:a3b44282f9f8 EFLAGS: 00010282
[4.193469] RAX:  RBX: 981fc9e320c0 RCX: 0001
[4.193470] RDX: 8001 RSI: 0002 RDI: 
[4.193471] RBP: 981fd42f R08: 000713ed01d2 R09: 001f
[4.193472] R10: 0344 R11: f300 R12: 014000c0
[4.193473] R13: fff5 R14: 981fd2592b98 R15: c0410280
[4.193475] FS:  7f4fb98d0d40() GS:981fe6d0()
knlGS:
[4.193476] CS:  0010 DS:  ES:  CR0: 80050033
[4.193477] CR2: 562b4a68f6e8 CR3: 0004532d6004 CR4: 003606e0
[4.193478] Call Trace:
[4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
[4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
[4.193495]  ? __kmalloc+0x19e/0x220
[4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
[4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
[4.193511]  driver_probe_device+0x2b9/0x460
[4.193514]  ? __driver_attach+0xb6/0xe0
[4.193516]  ? driver_probe_device+0x460/0x460
[4.193518]  ? bus_for_each_dev+0x76/0xc0
[4.193520]  ? bus_add_driver+0x152/0x230
[4.193522]  ? driver_register+0x6b/0xb0
[4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
[4.193531]  ? 0xc13b6000
[4.193534]  ? do_one_initcall+0x48/0x13b
[4.193537]  ? free_unref_page_commit+0x6a/0x100
[4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
[4.193542]  ? do_init_module+0x5a/0x210
[4.193544]  ? load_module+0x247a/0x29f0
[4.193549]  ? SyS_init_module+0x139/0x180
[4.193550]  ? SyS_init_module+0x139/0x180
[4.193554]  ? do_syscall_64+0x74/0x190
[4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
97 85 9a ec <0f> 0b 8b 53 64 e9 3a ff ff ff 65 48 8b 0c 25 00 5c 01 00
48 8b
[4.193589] ---[ end trace 37ff3cbaf04a5b5d ]---
[4.193612] usb 1-1: NFC: Reader power on cmd error -11
[4.193614] pn533_usb 1-1:1.0: NFC: Couldn't poweron the reader (error -11)
[4.193618] pn533_usb: probe of 1-1:1.0 failed with error -11
[4.193637] usbcore: registered new interface driver pn533_usb
[4.198216] usbcore: registered new interface driver usbhid


Please find the full dmesg in the link below:
https://pastebin.com/ck4sZuUY

Kind regards,
Carlos Santos
--
To unsubscribe from this list: send the line "u

RE: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config

[Jun Li]

Hi Mats,
> 
> Uhm, typing too fast again, I am. A better name would be just
> typec_find_role().
> What I mean is that the function could be used for any situation when
> someone wants to map a string to a TYPEC_{SOURCE,SINK} constant so it is
> unnecessary to limit its usage to just preferred role.

Get your idea, that will be a more general API for typec class.

Thanks
Li Jun

RE: [PATCH v5 12/14] staging: typec: tcpci: keep the not connecting cc line open

[Jun Li]

> -Original Message-
> From: Peter Chen
> Sent: 2018年5月16日 16:36
> To: Jun Li ; robh...@kernel.org; gre...@linuxfoundation.org;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net
> Cc: a.ha...@samsung.com; cw00.c...@samsung.com;
> shufan_...@richtek.com; gso...@gmail.com; devicet...@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx 
> Subject: RE: [PATCH v5 12/14] staging: typec: tcpci: keep the not connecting 
> cc
> line open
> 
> 
> >
> > While set polarity, we should keep the not connecting cc line to be open.
> >
> 
> keep the disconnected cc line open?

Okay, I will change.

Thanks
Li Jun
> 
> Peter
> 
> > Signed-off-by: Li Jun 
> > ---
> >  drivers/staging/typec/tcpci.c | 18 ++
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c 
> > index
> > 5c48810..5c0c5e3 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -185,15 +185,25 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> >   enum typec_cc_polarity polarity)  {
> > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > +   unsigned int reg;
> > int ret;
> >
> > -   ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> > -  (polarity == TYPEC_POLARITY_CC2) ?
> > -  TCPC_TCPC_CTRL_ORIENTATION : 0);
> > +   /* Keep the disconnect cc line open */
> > +   ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, ®);
> > if (ret < 0)
> > return ret;
> >
> > -   return 0;
> > +   if (polarity == TYPEC_POLARITY_CC2)
> > +   reg |= TCPC_ROLE_CTRL_CC_OPEN <<
> > TCPC_ROLE_CTRL_CC1_SHIFT;
> > +   else
> > +   reg |= TCPC_ROLE_CTRL_CC_OPEN <<
> > TCPC_ROLE_CTRL_CC2_SHIFT;
> > +   ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> > +  (polarity == TYPEC_POLARITY_CC2) ?
> > +  TCPC_TCPC_CTRL_ORIENTATION : 0);
> >  }
> >
> >  static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
> > --
> > 2.7.4

RE: [PATCH v5 01/14] dt-bindings: connector: add properties for typec

[Jun Li]

Hi
> -Original Message-
> From: Peter Chen
> Sent: 2018年5月16日 15:22
> To: Jun Li ; robh...@kernel.org; gre...@linuxfoundation.org;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net
> Cc: a.ha...@samsung.com; cw00.c...@samsung.com;
> shufan_...@richtek.com; gso...@gmail.com; devicet...@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx 
> Subject: RE: [PATCH v5 01/14] dt-bindings: connector: add properties for typec
> 
> 
> > Add bingdings supported by current typec driver, so user can pass all
> > those properties via dt.
> >
> 
> %s/bingdings/bindings

Will change

Thanks
Li Jun
> 
> Peter

RE: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config

[Jun Li]

Hi
> -Original Message-
> From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> Sent: 2018年5月16日 20:25
> To: Jun Li ; Mats Karrman 
> Cc: robh...@kernel.org; gre...@linuxfoundation.org; li...@roeck-us.net;
> a.ha...@samsung.com; cw00.c...@samsung.com; shufan_...@richtek.com;
> Peter Chen ; gso...@gmail.com;
> devicet...@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> 
> Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port 
> power
> and data config
> 
> Hi guys,
> 
> On Tue, May 15, 2018 at 10:52:57PM +0200, Mats Karrman wrote:
> > Hi,
> >
> > On 05/14/2018 11:36 AM, Jun Li wrote:
> >
> > > Hi
> > >> -Original Message-
> > >> From: Mats Karrman [mailto:mats.dev.l...@gmail.com]
> > >> Sent: 2018???5???12??? 3:56
> > >> To: Jun Li ; robh...@kernel.org;
> > >> gre...@linuxfoundation.org; heikki.kroge...@linux.intel.com;
> > >> li...@roeck-us.net
> > >> Cc: a.ha...@samsung.com; cw00.c...@samsung.com;
> > >> shufan_...@richtek.com; Peter Chen ;
> > >> gso...@gmail.com; devicet...@vger.kernel.org;
> > >> linux-usb@vger.kernel.org; dl-linux-imx 
> > >> Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec
> > >> basic port power and data config
> > >>
> > >> Hi Li Jun,
> > >>
> > >> On 2018-05-03 02:24, Li Jun wrote:
> > >>
> > >>> This patch adds 3 APIs to get the typec port power and data type,
> > >>> and preferred power role by its name string.
> > >>>
> > >>> Signed-off-by: Li Jun 
> > >>> ---
> > >>>   drivers/usb/typec/class.c | 52
> > >> +++
> > >>>   include/linux/usb/typec.h |  3 +++
> > >>>   2 files changed, 55 insertions(+)
> > >>>
> > >>> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > >>> index 53df10d..5981e18 100644
> > >>> --- a/drivers/usb/typec/class.c
> > >>> +++ b/drivers/usb/typec/class.c
> > >>> @@ -9,6 +9,7 @@
> > >>>   #include 
> > >>>   #include 
> > >>>   #include 
> > >>> +#include 
> 
> I don't think you need that anymore.

I will remove it.

> 
> > >>>   #include 
> > >>>   #include 
> > >>>   #include  @@ -802,6 +803,12 @@ static
> > >>> const char * const typec_port_types[] = {
> > >>> [TYPEC_PORT_DRP] = "dual",
> > >>>   };
> > >>>
> > >>> +static const char * const typec_data_types[] = {
> > >>> +   [TYPEC_PORT_DFP] = "host",
> > >>> +   [TYPEC_PORT_UFP] = "device",
> > >>> +   [TYPEC_PORT_DRD] = "dual",
> > >>> +};
> > >>> +
> > >>>   static const char * const typec_port_types_drp[] = {
> > >>> [TYPEC_PORT_SRC] = "dual [source] sink",
> > >>> [TYPEC_PORT_SNK] = "dual source [sink]", @@ -1252,6 +1259,51
> > >> @@
> > >>> void typec_set_pwr_opmode(struct typec_port *port,
> > >>>   }
> > >>>   EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
> > >>>
> > >>> +/**
> > >>> + * typec_find_power_type - Get the typec port power type
> > >> Why is this function called typec_find_power_type() and not
> > >> typec_find_port_type()?
> > >> It's called port_type in sysfs, having different names just adds 
> > >> confusion.
> > >> (Otherwise I agree power_type is a better name but...)
> > > We have "port type" before the power and data role separation, this
> > > API name's intention is to reflect the power cap, anyway I leave
> > > this to be decided by Heikki then.
> 
> I really hate the "*_type" naming. It was understandable when there was no
> separate power and data roles defined in the specification, but now that there
> are, it's just confusing. IMO we should not use it anywhere.

OK, know your preference now.

> 
> So to me typec_find_type() is just as bad as typec_find_power_type() because 
> it
> has the "type" in it. I wonder if this function is necessary at all? If it 
> is, then
> perhaps we can think of some better name for it, name that gives a better hint
> what it is used for.

We need a way to match a string to a value which has to be done via typec
class.
 
> 
> > >>> + * @name: port type string
> > >>> + *
> > >>> + * This routine is used to find the typec_port_type by its string name.
> > >>> + *
> > >>> + * Returns typec_port_type if success, otherwise negative error code.
> > >>> + */
> > >>> +int typec_find_power_type(const char *name) {
> > >>> +   return match_string(typec_port_types,
> ARRAY_SIZE(typec_port_types),
> > >>> +   name);
> > >>> +}
> > >>> +EXPORT_SYMBOL_GPL(typec_find_power_type);
> > >>> +
> > >>> +/**
> > >>> + * typec_find_preferred_role - Find the typec drp port preferred
> > >>> +power role
> > >> Why typec_find_preferred_role()? Could be used for any power_role
> > >> so why not typec_find_power_role()?
> > > I am not sure if I catch your point of this comment.
> > > For preferred role(if support try.sink or try.src) the only allowed
> > > power roles are "sink"
> > > "source"
> > > But for power role, the allowed type are "sink"
> > > "source"
> > > "dual"
> >
> > Uhm, typing too fast again, I am. A better name would be just
> typec_find_role().
> > What I

Re: [PATCH 1/3] xhci: Allow more than 32 quirks

[Greg KH]

On Thu, May 17, 2018 at 01:58:34PM +0100, Marc Zyngier wrote:
> We now have 32 different quirks, and the field that holds them
> is full. Let's bump it up to the next stage so that we can handle
> some more... The type is now an unsigned long long, which is 64bit
> on most architectures.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/usb/host/xhci.c | 6 +++---
>  drivers/usb/host/xhci.h | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 711da3306b14..8dba26d3de07 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -33,8 +33,8 @@ static int link_quirk;
>  module_param(link_quirk, int, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(link_quirk, "Don't clear the chain bit on a link TRB");
>  
> -static unsigned int quirks;
> -module_param(quirks, uint, S_IRUGO);
> +static unsigned long long quirks;
> +module_param(quirks, ullong, S_IRUGO);
>  MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
>  
>  /* TODO: copied from ehci-hcd.c - can this be refactored? */
> @@ -4963,7 +4963,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
> xhci_get_quirks_t get_quirks)
>   return retval;
>   xhci_dbg(xhci, "Called HCD init\n");
>  
> - xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%08x\n",
> + xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%016llx\n",
> xhci->hcc_params, xhci->hci_version, xhci->quirks);
>  
>   return 0;
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 6dfc4867dbcf..42848dfc3445 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1787,7 +1787,7 @@ struct xhci_hcd {
>  #define XHCI_STATE_DYING (1 << 0)
>  #define XHCI_STATE_HALTED(1 << 1)
>  #define XHCI_STATE_REMOVING  (1 << 2)
> - unsigned intquirks;
> + unsigned long long  quirks;

u64?

>  #define  XHCI_LINK_TRB_QUIRK (1 << 0)
>  #define XHCI_RESET_EP_QUIRK  (1 << 1)
>  #define XHCI_NEC_HOST(1 << 2)

And these all can use BIT(), right?

thanks,

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

Re: [PATCH 3/3] Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"

[Greg KH]

On Thu, May 17, 2018 at 01:58:36PM +0100, Marc Zyngier wrote:
> This reverts commit 8466489ef5ba48272ba4fa4ea9f8f403306de4c7.
> 
> Now that we can properly reset the uPD72020x without a hard PCI reset,
> let's get rid of the existing quirks.
> 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/usb/host/pci-quirks.c | 20 
>  drivers/usb/host/pci-quirks.h |  1 -
>  drivers/usb/host/xhci-pci.c   |  7 ---
>  drivers/usb/host/xhci.c   | 21 -
>  drivers/usb/host/xhci.h   |  2 +-
>  5 files changed, 13 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 67ad4bb6919a..3625a5c1a41b 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -1268,23 +1268,3 @@ static void quirk_usb_early_handoff(struct pci_dev 
> *pdev)
>  }
>  DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
>   PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff);
> -
> -bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
> -{
> - /*
> -  * Our dear uPD72020{1,2} friend only partially resets when
> -  * asked to via the XHCI interface, and may end up doing DMA
> -  * at the wrong addresses, as it keeps the top 32bit of some
> -  * addresses from its previous programming under obscure
> -  * circumstances.
> -  * Give it a good wack at probe time. Unfortunately, this
> -  * needs to happen before we've had a chance to discover any
> -  * quirk, or the system will be in a rather bad state.
> -  */
> - if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - (pdev->device == 0x0014 || pdev->device == 0x0015))
> - return true;
> -
> - return false;
> -}
> -EXPORT_SYMBOL_GPL(usb_xhci_needs_pci_reset);
> diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
> index 4ca0d9b7e463..63c633077d9e 100644
> --- a/drivers/usb/host/pci-quirks.h
> +++ b/drivers/usb/host/pci-quirks.h
> @@ -16,7 +16,6 @@ void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev);
>  void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev);
>  void usb_disable_xhci_ports(struct pci_dev *xhci_pdev);
>  void sb800_prefetch(struct device *dev, int on);
> -bool usb_xhci_needs_pci_reset(struct pci_dev *pdev);
>  bool usb_amd_pt_check_port(struct device *device, int port);
>  #else
>  struct pci_dev;
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index e0a0a12871e2..6372edf339d9 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -288,13 +288,6 @@ static int xhci_pci_probe(struct pci_dev *dev, const 
> struct pci_device_id *id)
>  
>   driver = (struct hc_driver *)id->driver_data;
>  
> - /* For some HW implementation, a XHCI reset is just not enough... */
> - if (usb_xhci_needs_pci_reset(dev)) {
> - dev_info(&dev->dev, "Resetting\n");
> - if (pci_reset_function_locked(dev))
> - dev_warn(&dev->dev, "Reset failed");
> - }
> -
>   /* Prevent runtime suspending between USB-2 and USB-3 initialization */
>   pm_runtime_get_noresume(&dev->dev);
>  
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index cd6b48c43007..07272d1ce32a 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4923,16 +4923,19 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
> xhci_get_quirks_t get_quirks)
>  
>   /*
>* Some Renesas controllers get into a weird state if they are
> -  * reset while programmed with 64bit addresses (they will
> -  * preserve the top half of the address in internal, non
> -  * visible registers). You end up with half the address coming
> -  * from the kernel, and the other half coming from the
> -  * firmware. Also, changing the programming leads to extra
> -  * accesses even if the controller is supposed to be
> -  * halted. The controller ends up with a fatal fault, and is
> -  * then ripe for being properly reset.
> +  * reset while programmed with 64bit addresses (they will preserve
> +  * the top half of the address in internal, non visible
> +  * registers). You end up with half the address coming from the
> +  * kernel, and the other half coming from the firmware. Also,
> +  * changing the programming leads to extra accesses even if the
> +  * controller is supposed to be halted. The controller ends up with
> +  * a fatal fault, and is then ripe for being properly reset.
> +  *
> +  * Special care is taken to only apply this if the device is behind
> +  * an iommu. Doing anything when there is no iommu is definitely
> +  * unsafe...
>*/
> - if (xhci->quirks & XHCI_ZERO_64B_REGS) {
> + if ((xhci->quirks & XHCI_ZERO_64B_REGS) && dev->iommu_group) {
>   u64 val;
>   int i;
>  
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 5ea0b52b4

Re: Fwd: usb: uas: device reset most the time while enumeration- usb3.0

[Oliver Neukum]

Am Donnerstag, den 17.05.2018, 12:29 +0530 schrieb Tushar Nimkar:
> Those commands issued from different layers say: sd.. uas.. scsi..
> so making them to go one after other. Once REPORT_LUN done go with 
> READ_CAPACITY_16.
> This is only for the UAS devices. I believe no disturb to BOT behavior.

Hi,

this is good news.

1. We cannot slow down all UAS devices because a few are broken. This
would need to be selective.
2. What is insufficient about "shost->async_scan" for your approach?

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 v5 05/14] usb: typec: add API to get typec basic port power and data config

[Jun Li]

Hi Mats

> I reread this patch and tried to see it more in the context of the other 
> patches
> and the existing code. The naming of the existing string tables doesn't help 
> in
> getting this right, however I have a proposal:
> 
> typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP
> typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD
> typec_find_power_role() to get to TYPEC_SINK/SOURCE
> 
> and sometime, if the use should arise
> 
> typec_find_data_role() to get to TYPEC_DEVICE/HOST
> 
> I think it is fairly comprehensible, *_port_* concerns a capability and 
> without
> *_port_* it is an actual state. Plus it matches the names of the constants.
> 

Reasonable to me, I was using "type" to concern a capability but it was not
preferred, if Heikki agree this proposal, I will change the API names like this
and include 4 APIs above.
 
Thanks
Li Jun

Re: `ucsi_acpi: probe of USBC000:00 failed with error -12` on Dell XPS 13 9370

[Heikki Krogerus]

Hi Mario,

On Thu, May 17, 2018 at 01:01:20PM +, mario.limoncie...@dell.com wrote:
> > > Heikki,
> > >
> > > I confirmed with internal team that UCSI is implemented on XPS 9370
> > > and was confirmed to be working properly with Windows 10 RS2+.
> > 
> > Just to double check: "UCSI was confirmed working properly", so not
> > "the Type-C ports were confirmed working properly"?
> 
> UCSI was confirmed working properly.  FWIW it's a certification requirement
> in Windows.

OK, thank you for confirming that.

> > > The reason that _STA is responding on this device node now but wasn't
> > > previously is it wasn't exposed in Linux until 4.16 when the Win 10 RS2
> > > OSI string started to respond.
> > 
> > OK.
> > 
> > > Intel should internally have some XPS 9370 you can remotely access if
> > > you would like to poke around ACPI tables some.
> > 
> > I will try get access to XPS 9370, but with the acpi tables, if
> > somebody could just send me acpidump, that would be enough:
> > 
> > % acpidump -o xps9370_acpi.dump

I now have the XPS 9370 ACPI tables.


Br,

-- 
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 v5 05/14] usb: typec: add API to get typec basic port power and data config

[Jun Li]

Hi Heikki,

> > I reread this patch and tried to see it more in the context of the
> > other patches and the existing code. The naming of the existing string
> > tables doesn't help in getting this right, however I have a proposal:
> >
> > typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP
> > typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD
> > typec_find_power_role() to get to TYPEC_SINK/SOURCE
> >
> > and sometime, if the use should arise
> >
> > typec_find_data_role() to get to TYPEC_DEVICE/HOST
> >
> > I think it is fairly comprehensible, *_port_* concerns a capability
> > and without *_port_* it is an actual state. Plus it matches the names
> > of the constants.
> 
> Well, there are now four things to consider:
> 
> 1) the roles (power and data) the port is capable of supporting
> 2) Try.SRC and Try.SNK, i.e. the preferred role
> 3) the current roles
> 4) the role(s) the user want's to limit the use of a port with DRP ports
> 
> The last one I don't know if it's relevant with these functions. It's not
> information that we would get for example from firmware.
> 
> I also don't think we need to use these functions with the current roles the 
> port
> is in.
> 
> If the preferred role is "sink" or "source", so just like the power role, we 
> don't
> need separate function for it here.
> 
> So isn't two functions all we need here: one for the power and one for data
> role?

Take power as an example, can we use one function to only look up
typec_port_types[]? as capability(typec_port_type) and state(typec_role)
are different enum, and the allowed strings are different.

static const char * const typec_roles[] = {
[TYPEC_SINK]= "sink",
[TYPEC_SOURCE]  = "source",
};

static const char * const typec_port_types[] = {
[TYPEC_PORT_SRC] = "source",
[TYPEC_PORT_SNK] = "sink",
[TYPEC_PORT_DRP] = "dual",
};

Thanks
Li Jun
> 
> 
> 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: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
> Hello.
> I'm having troubles with this NFC card reader. It seems kernel driver
> pn533 is not working properly.
> At this moment I have my work stalled. I need to add NFC support to a
> software product and I can't get the device to work. NFC tools won't
> work, the device is not detected.
> 
> This is what I get from "dmesg":
> 
> [4.182300] nfc: nfc_init: NFC Core ver 0.1
> [4.182318] NET: Registered protocol family 39
> [4.184676] hidraw: raw HID events driver (C) Jiri Kosina
> [4.193366] [ cut here ]
> [4.193366] transfer buffer not dma capable
> [4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
> usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> [4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
> snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
> snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
> snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
> snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
> intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
> snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
> mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
> ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
> videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
> snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
> intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
> intel_rapl_perf tpm_crb psmouse
> [4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
> wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
> sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
> intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
> rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
> ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
> crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
> serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
> libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
> mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
> sysimgblt fb_sys_fops ttm drm agpgart
> [4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 4.16.8-1-ARCH 
> #1
> [4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
> PRO A50-C, BIOS Version 7.50   09/26/2016
> [4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> [4.193468] RSP: 0018:a3b44282f9f8 EFLAGS: 00010282
> [4.193469] RAX:  RBX: 981fc9e320c0 RCX: 
> 0001
> [4.193470] RDX: 8001 RSI: 0002 RDI: 
> 
> [4.193471] RBP: 981fd42f R08: 000713ed01d2 R09: 
> 001f
> [4.193472] R10: 0344 R11: f300 R12: 
> 014000c0
> [4.193473] R13: fff5 R14: 981fd2592b98 R15: 
> c0410280
> [4.193475] FS:  7f4fb98d0d40() GS:981fe6d0()
> knlGS:
> [4.193476] CS:  0010 DS:  ES:  CR0: 80050033
> [4.193477] CR2: 562b4a68f6e8 CR3: 0004532d6004 CR4: 
> 003606e0
> [4.193478] Call Trace:
> [4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
> [4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
> [4.193495]  ? __kmalloc+0x19e/0x220
> [4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
> [4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
> [4.193511]  driver_probe_device+0x2b9/0x460
> [4.193514]  ? __driver_attach+0xb6/0xe0
> [4.193516]  ? driver_probe_device+0x460/0x460
> [4.193518]  ? bus_for_each_dev+0x76/0xc0
> [4.193520]  ? bus_add_driver+0x152/0x230
> [4.193522]  ? driver_register+0x6b/0xb0
> [4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
> [4.193531]  ? 0xc13b6000
> [4.193534]  ? do_one_initcall+0x48/0x13b
> [4.193537]  ? free_unref_page_commit+0x6a/0x100
> [4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
> [4.193542]  ? do_init_module+0x5a/0x210
> [4.193544]  ? load_module+0x247a/0x29f0
> [4.193549]  ? SyS_init_module+0x139/0x180
> [4.193550]  ? SyS_init_module+0x139/0x180
> [4.193554]  ? do_syscall_64+0x74/0x190
> [4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> [4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
> ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
> 97 85 9a ec <0f> 0b 8b 53 64 e9 3a ff ff ff 65 48 8b 0c 25 00 5c 01 00
> 48 8b
> [4.193589] ---[ end trace 37ff3cbaf04a5b5d ]---
> [4.193612] usb 1-1: NFC: Reader power on cmd error -11
> [4.193614] pn533_usb 1-1:1.0: NFC: Couldn't poweron the reader (error -11)
> [4.193618] pn533_usb: probe of 1-1:1.0 failed with error -11
> [4.193637] usbcore:

Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config

[Heikki Krogerus]

On Thu, May 17, 2018 at 02:05:41PM +, Jun Li wrote:
> Hi Heikki,
> 
> > > I reread this patch and tried to see it more in the context of the
> > > other patches and the existing code. The naming of the existing string
> > > tables doesn't help in getting this right, however I have a proposal:
> > >
> > > typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP
> > > typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD
> > > typec_find_power_role() to get to TYPEC_SINK/SOURCE
> > >
> > > and sometime, if the use should arise
> > >
> > > typec_find_data_role() to get to TYPEC_DEVICE/HOST
> > >
> > > I think it is fairly comprehensible, *_port_* concerns a capability
> > > and without *_port_* it is an actual state. Plus it matches the names
> > > of the constants.
> > 
> > Well, there are now four things to consider:
> > 
> > 1) the roles (power and data) the port is capable of supporting
> > 2) Try.SRC and Try.SNK, i.e. the preferred role
> > 3) the current roles
> > 4) the role(s) the user want's to limit the use of a port with DRP ports
> > 
> > The last one I don't know if it's relevant with these functions. It's not
> > information that we would get for example from firmware.
> > 
> > I also don't think we need to use these functions with the current roles 
> > the port
> > is in.
> > 
> > If the preferred role is "sink" or "source", so just like the power role, 
> > we don't
> > need separate function for it here.
> > 
> > So isn't two functions all we need here: one for the power and one for data
> > role?
> 
> Take power as an example, can we use one function to only look up
> typec_port_types[]? as capability(typec_port_type) and state(typec_role)
> are different enum, and the allowed strings are different.
> 
> static const char * const typec_roles[] = {
>   [TYPEC_SINK]= "sink",
>   [TYPEC_SOURCE]  = "source",
> };
> 
> static const char * const typec_port_types[] = {
>   [TYPEC_PORT_SRC] = "source",
>   [TYPEC_PORT_SNK] = "sink",
>   [TYPEC_PORT_DRP] = "dual",
> };

Where out side the class code we need to convert the current role, the
"state", with these functions?


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 v5 05/14] usb: typec: add API to get typec basic port power and data config

[Jun Li]

Hi
> -Original Message-
> From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> Sent: 2018年5月17日 22:24
> To: Jun Li 
> Cc: Mats Karrman ; robh...@kernel.org;
> gre...@linuxfoundation.org; li...@roeck-us.net; a.ha...@samsung.com;
> cw00.c...@samsung.com; shufan_...@richtek.com; Peter Chen
> ; gso...@gmail.com; devicet...@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx 
> Subject: Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port 
> power
> and data config
> 
> On Thu, May 17, 2018 at 02:05:41PM +, Jun Li wrote:
> > Hi Heikki,
> >
> > > > I reread this patch and tried to see it more in the context of the
> > > > other patches and the existing code. The naming of the existing
> > > > string tables doesn't help in getting this right, however I have a 
> > > > proposal:
> > > >
> > > > typec_find_port_power_role() to get to TYPEC_PORT_SRC/SNK/DRP
> > > > typec_find_port_data_role() to get to TYPEC_PORT_DFP/UFP/DRD
> > > > typec_find_power_role() to get to TYPEC_SINK/SOURCE
> > > >
> > > > and sometime, if the use should arise
> > > >
> > > > typec_find_data_role() to get to TYPEC_DEVICE/HOST
> > > >
> > > > I think it is fairly comprehensible, *_port_* concerns a
> > > > capability and without *_port_* it is an actual state. Plus it
> > > > matches the names of the constants.
> > >
> > > Well, there are now four things to consider:
> > >
> > > 1) the roles (power and data) the port is capable of supporting
> > > 2) Try.SRC and Try.SNK, i.e. the preferred role
> > > 3) the current roles
> > > 4) the role(s) the user want's to limit the use of a port with DRP
> > > ports
> > >
> > > The last one I don't know if it's relevant with these functions.
> > > It's not information that we would get for example from firmware.
> > >
> > > I also don't think we need to use these functions with the current
> > > roles the port is in.
> > >
> > > If the preferred role is "sink" or "source", so just like the power
> > > role, we don't need separate function for it here.
> > >
> > > So isn't two functions all we need here: one for the power and one
> > > for data role?
> >
> > Take power as an example, can we use one function to only look up
> > typec_port_types[]? as capability(typec_port_type) and
> > state(typec_role) are different enum, and the allowed strings are different.
> >
> > static const char * const typec_roles[] = {
> > [TYPEC_SINK]= "sink",
> > [TYPEC_SOURCE]  = "source",
> > };
> >
> > static const char * const typec_port_types[] = {
> > [TYPEC_PORT_SRC] = "source",
> > [TYPEC_PORT_SNK] = "sink",
> > [TYPEC_PORT_DRP] = "dual",
> > };
> 
> Where out side the class code we need to convert the current role, the 
> "state",
> with these functions?

Currently, the only call site is getting the preferred power role from firmware.

Thanks
Li Jun
> 
> 
> Thanks,
> 
> --
> heikki

Re: [usb-storage] Re: [PATCH] usb-storage: Add quirks to make G-Technology "G-Drive" work

[Alan Stern]

On Thu, 17 May 2018, Alexander Kappner wrote:

> Hi Alan,
> 
> thanks for reviewing. (This is my first contribution that touches 
> usb-storage, so please bear with me.)
> 
> > That's kind of weird.  Does the drive work under Windows in UAS mode?  
> 
> On the Windows 10 VM that I just spun up for testing this, access to the 
> drive uses "usbstor.inf" (rather than "uaspstor.sys"). So I believe the 
> answer is no. 
> 
> > If so, why are the WRITE(16) commands failing under Linux?
> 
> I don't know exactly why they're failing, but another entry in the 
> unusual_uas.h shows another SimpleTech device (only with a slightly 
> different product ID) needing the same UAS quirk (see 
> https://bugzilla.redhat.com/show_bug.cgi?id=1124119). So my guess is those 
> drives are just buggy. 

Okay, that's quite possible.

> > That doesn't quite make sense.  Since you prevent the uas driver from 
> > binding to this device, it will end up using usb-storage no matter how 
> > the kernel was built.  Therefore the second quirk flag has to go into 
> > unusual_devs.h no matter what.
> 
> Yes that's what I was trying to get at. So even though the UAS flag would 
> conceptually belong into unusual_uas, I'm putting both into unusual_devs to 
> avoid having to create two separate entries for the same device.
> 
> > You don't describe the second quirk flag at all.  Are you sure it is 
> > needed?
> 
> Yes. Without this flag, the device keeps throwing similar errors on 
> usb-storage. That's the same result I get on a host that doesn't have UAS 
> compiled in. Here's a dmesg:
> 
> [2.183472] usb 3-1: New USB device found, idVendor=4971, idProduct=8024, 
> bcdDevice=24.03
> [2.184285] usb 3-1: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=3
> [2.184980] usb 3-1: Product: G-DRIVE
> [2.185447] usb 3-1: Manufacturer: HGST
> [2.185829] usb 3-1: SerialNumber: AA015010004C
> [2.195509] usb-storage 3-1:1.0: USB Mass Storage device detected
> [2.198668] Floppy drive(s): fd0 is 2.88M AMI BIOS
> [2.202981] scsi host2: usb-storage 3-1:1.0
> ...
> [3.233085] scsi 2:0:0:0: Direct-Access G-DRIVE   2403 
> PQ: 0 ANSI: 6
> [3.234514] sd 2:0:0:0: Attached scsi generic sg1 type 0
> [3.235465] sd 2:0:0:0: [sda] Very big device. Trying to use READ 
> CAPACITY(16).
> [3.236847] sd 2:0:0:0: [sda] 7814037168 512-byte logical blocks: (4.00 
> TB/3.64 TiB)
> [3.237794] sd 2:0:0:0: [sda] 4096-byte physical blocks
> [3.241255] sd 2:0:0:0: [sda] Write Protect is off
> [3.242096] sd 2:0:0:0: [sda] Mode Sense: 47 00 10 08
> [3.243595] sd 2:0:0:0: [sda] Write cache: enabled, read cache: enabled, 
> supports DPO and FUA
> [3.257893]  sda: sda1 sda9
> [3.261402] sd 2:0:0:0: [sda] Attached SCSI disk
> ...
> [   92.433428] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
> driverbyte=DRIVER_SENSE
> [   92.434759] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
> [   92.435637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
> [   92.436401] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 00 
> 00 00 00 00 00 08 00 00
> [   92.437493] print_req_error: critical target error, dev sda, sector 0
> [   92.438211] Buffer I/O error on dev sda, logical block 0, lost sync page 
> write
> [   92.516692] EXT4-fs (sda): mounted filesystem with ordered data mode. 
> Opts: (null)
> [  101.449311] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
> driverbyte=DRIVER_SENSE
> [  101.450598] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
> [  101.451401] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
> [  101.452041] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
> 00 18 00 00 00 08 00 00
> [  101.452906] print_req_error: critical target error, dev sda, sector 
> 3905159192
> [  101.453593] print_req_error: critical target error, dev sda, sector 
> 3905159192
> [  101.454889] Aborting journal on device sda-8.
> [  101.457103] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
> driverbyte=DRIVER_SENSE
> [  101.457988] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request [current] 
> [  101.458637] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
> [  101.459250] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 e8 c4 
> 00 00 00 00 00 08 00 00

That's bizarre too.  Even though the only difference is a MODE SENSE 
command, the command that actually failed was WRITE(16).

> Source code comments describe this as a known problem (scsiglue.c:182):
> 
> /*
>  * Some devices don't like MODE SENSE with page=0x3f,
>  * which is the command used for checking if a device
>  * is write-protected.  Now that we tell the sd driver
>  * to do a 192-byte transfer with this command the
>  * majority of devices work fine, but a few still can't
>  * handle it.  The sd driver will s

Re: usb: dwc2: gadget: Fix memory leak in dwc2_gadget_init()

[Grigor Tovmasyan]

Hi Felipe,

Please drop this patch from your next branch.
I will provide another fix based on Marek's suggestion.

Thanks,
Grigor.

On 5/17/2018 4:18 PM, Marek Szyprowski wrote:
> Hi All,
> 
> I've just noticed this patch has been applied to linux-next.
> 
> It breaks DWC2 driver initialization on Samsung Exynos-based boards
> I've use for kernel daily tests.
> 
> Here is an example of the call stack:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 000c
> pgd = (ptrval)
> [000c] *pgd=
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 0 PID: 25 Comm: kworker/0:1 Not tainted 4.17.0-rc5-next-20180517 #782
> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> Workqueue: events deferred_probe_work_func
> PC is at usb_ep_alloc_request+0x1c/0x200
> LR is at composite_dev_prepare+0x20/0xec
> pc : []    lr : []    psr: 6153
> sp : d95f1d10  ip :   fp : 0002
> r10: c1072d58  r9 : d82443d0  r8 : 
> r7 : c1074180  r6 : c10ad460  r5 : c100746c  r4 : d8253980
> r3 :   r2 : d82539b8  r1 : 014000c0  r0 : d82443d0
> Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 4000406a  DAC: 0051
> Process kworker/0:1 (pid: 25, stack limit = 0x(ptrval))
> Stack: (0xd95f1d10 to 0xd95f2000)
> ...
> [] (usb_ep_alloc_request) from []
> (composite_dev_prepare+0x20/0xec)
> [] (composite_dev_prepare) from []
> (composite_bind+0x7c/0x1c8)
> [] (composite_bind) from []
> (udc_bind_to_driver+0x74/0x134)
> [] (udc_bind_to_driver) from []
> (check_pending_gadget_drivers+0x74/0xac)
> [] (check_pending_gadget_drivers) from []
> (usb_add_gadget_udc_release+0x190/0x1ec)
> [] (usb_add_gadget_udc_release) from []
> (dwc2_gadget_init+0x274/0x464)
> [] (dwc2_gadget_init) from []
> (dwc2_driver_probe+0x488/0x50c)
> [] (dwc2_driver_probe) from []
> (platform_drv_probe+0x48/0x9c)
> [] (platform_drv_probe) from []
> (driver_probe_device+0x2dc/0x4ac)
> [] (driver_probe_device) from []
> (bus_for_each_drv+0x74/0xb8)
> [] (bus_for_each_drv) from []
> (__device_attach+0xd4/0x168)
> [] (__device_attach) from []
> (bus_probe_device+0x88/0x90)
> [] (bus_probe_device) from []
> (deferred_probe_work_func+0x60/0x180)
> [] (deferred_probe_work_func) from ad+0x28c/0x58c)
> [] (worker_thread) from [] (kthread+0x138/0x168)
> [] (kthread) from [] (ret_from_fork+0x14/0x20)
> 
> 
> A proper fix for the potential memory leak is to add free to error path
> instead of reordering usb_add_gadget_udc() call. The issue happens if DWC2
> driver is initialized from deferred probe (in such case the gadget driver
> is already registered).
> 
> Felipe: please drop or revert this patch in your -next branch.
> 
> 
> Best regards
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 
> On 2018-04-16 12:16, Grigor Tovmasyan wrote:
>> In dwc2_gadget_init() we allocate EP0 request via
>> dwc2_hsotg_ep_alloc_request(). After that there are
>> usb_add_gadget_udc() call and if it failed, then
>> ctrl_req will not be freed and will cause memory leak.
>>
>> Reordered function calls in gadget_init: moved up usb_add_gadget_udc()
>> before dwc2_hsotg_ep_alloc_request().
>>
>> Tested using kmemleak.
>>
>> Cc: Stefan Wahren 
>> Signed-off-by: Grigor Tovmasyan 
>> Acked-by: Minas Harutyunyan 
>> ---
>>drivers/usb/dwc2/gadget.c | 8 
>>1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>> index 6c32bf26e48e..24000bda5c20 100644
>> --- a/drivers/usb/dwc2/gadget.c
>> +++ b/drivers/usb/dwc2/gadget.c
>> @@ -4679,6 +4679,10 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
>>  INIT_LIST_HEAD(&hsotg->gadget.ep_list);
>>  hsotg->gadget.ep0 = &hsotg->eps_out[0]->ep;
>>
>> +ret = usb_add_gadget_udc(dev, &hsotg->gadget);
>> +if (ret)
>> +return ret;
>> +
>>  /* allocate EP0 request */
>>
>>  hsotg->ctrl_req = dwc2_hsotg_ep_alloc_request(&hsotg->eps_out[0]->ep,
>> @@ -4698,10 +4702,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
>>epnum, 0);
>>  }
>>
>> -ret = usb_add_gadget_udc(dev, &hsotg->gadget);
>> -if (ret)
>> -return ret;
>> -
>>  dwc2_hsotg_dump(hsotg);
>>
>>  return 0;
>>
> 
> 

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

[RFC PATCH linux-next] USB: dwc3: dwc3_get_extcon() can be static

[kbuild test robot]

Fixes: 5f0b74e54890 ("USB: dwc3: get extcon device by OF graph bindings")
Signed-off-by: kbuild test robot 
---
 drd.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 2706824..218371f 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -440,7 +440,7 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
return NOTIFY_DONE;
 }
 
-struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
+static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 {
struct device *dev = dwc->dev;
struct device_node *np_phy, *np_conn;
--
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

[linux-next:master 7522/8111] drivers/usb/dwc3/drd.c:443:19: sparse: symbol 'dwc3_get_extcon' was not declared. Should it be static?

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git 
master
head:   fbbe3b8c2c9c5f84caf668703c26154cb4fbb9d1
commit: 5f0b74e54890c354d6ac0124ea7a96adf22845d0 [7522/8111] USB: dwc3: get 
extcon device by OF graph bindings
reproduce:
# apt-get install sparse
git checkout 5f0b74e54890c354d6ac0124ea7a96adf22845d0
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/usb/dwc3/drd.c:443:19: sparse: symbol 'dwc3_get_extcon' was not 
>> declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
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 00/12] usb: usbtmc: Changes needed for compatible IVI/VISA library

[Guido Kiener]

The working group "VISA for Linux" of the IVI Foundation
www.ivifoundation.org specifies common rules, shared libraries and
drivers to implement the specification of "VPP-4.3: The VISA Library"
on Linux to be compatible with implementations on other operating systems.

The USBTMC protocol is part of the "VISA Library" that is used by many
popular T&M applications.

An initial implementation for Linux based on libusb has been created.
While functional it has some drawbacks:
- Performance
- Requires root privileges to reclaim devices already claimed by
  the usbtmc driver

The following collaborative patches meet the requirements of the IVI
Foundation to implement the library based on the usbtmc driver.

Improvements in the data transfer rate of over 130 MByte/s for
usb 3.x connections have been measured.

Guido Kiener, Dave Penkler, Steve Bayless (12):
 01 Remove rigol_quirk
 02 Support Read Status Byte with SRQ per file handle
 03 Add ioctls to set/get usb timeout
 04 Add ioctls for trigger, EOM bit, TermChar
 05 Add ioctl for generic requests on control pipe
 06 Add vendor specific/asynchronous read/write ioctls
 07 Add ioctl USBTMC488_IOCTL_WAIT_SRQ
 08 Optimize read/write and add ioctls for auto_abort
 09 Fix ioctl USBTMC_IOCTL_CLEAR
 10 Add test functions to set HALT Feature (Stall)
 11 Add ioctls to  abort with specific tags
 12 Add ioctl to return API version of usbtmc driver 
 
 Documentation/ioctl/ioctl-number.txt |2 +-
 drivers/usb/class/usbtmc.c   | 1951 +-
 include/uapi/linux/usb/tmc.h |   60 +
 3 files changed, 1628 insertions(+), 385 deletions(-)

-- 
2.17.0

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

[PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle

[Guido Kiener]

- Add 'struct usbtmc_file_data' for each file handle to cache last
  srq_byte (=Status Byte with SRQ) received by usbtmc_interrupt(..)

- usbtmc488_ioctl_read_stb returns cached srq_byte when available for
  each file handle to avoid race conditions of concurrent applications.

- SRQ now sets EPOLLPRI instead of EPOLLIN

- Caches other values TermChar, TermCharEnabled, auto_abort in
  'struct usbtmc_file_data' will not be changed by sysfs device
  attributes during an open file session.
  Future ioctl functions can change these values.

- use consistent error return value ETIMEOUT instead of ETIME

Tested-by: Dave Penkler 
Reviewed-by: Steve Bayless 
Signed-off-by: Guido Kiener 
---
 drivers/usb/class/usbtmc.c | 176 -
 1 file changed, 136 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 529295a17579..5754354429d8 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -67,6 +67,7 @@ struct usbtmc_device_data {
const struct usb_device_id *id;
struct usb_device *usb_dev;
struct usb_interface *intf;
+   struct list_head file_list;
 
unsigned int bulk_in;
unsigned int bulk_out;
@@ -87,12 +88,12 @@ struct usbtmc_device_data {
intiin_interval;
struct urb*iin_urb;
u16iin_wMaxPacketSize;
-   atomic_t   srq_asserted;
 
/* coalesced usb488_caps from usbtmc_dev_capabilities */
__u8 usb488_caps;
 
/* attributes from the USB TMC spec for this device */
+   /* They are used as default values for file_data */
u8 TermChar;
bool TermCharEnabled;
bool auto_abort;
@@ -104,9 +105,26 @@ struct usbtmc_device_data {
struct mutex io_mutex;  /* only one i/o function running at a time */
wait_queue_head_t waitq;
struct fasync_struct *fasync;
+   spinlock_t dev_lock; /* lock for file_list */
 };
 #define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
 
+/*
+ * This structure holds private data for each USBTMC file handle.
+ */
+struct usbtmc_file_data {
+   struct usbtmc_device_data *data;
+   struct list_head file_elem;
+
+   u8 srq_byte;
+   atomic_t   srq_asserted;
+
+   /* These values are initialized with default values from device_data */
+   u8 TermChar;
+   bool   TermCharEnabled;
+   bool   auto_abort;
+};
+
 /* Forward declarations */
 static struct usb_driver usbtmc_driver;
 
@@ -114,6 +132,7 @@ static void usbtmc_delete(struct kref *kref)
 {
struct usbtmc_device_data *data = to_usbtmc_data(kref);
 
+   pr_debug("%s - called\n", __func__);
usb_put_dev(data->usb_dev);
kfree(data);
 }
@@ -122,7 +141,7 @@ static int usbtmc_open(struct inode *inode, struct file 
*filp)
 {
struct usb_interface *intf;
struct usbtmc_device_data *data;
-   int retval = 0;
+   struct usbtmc_file_data *file_data;
 
intf = usb_find_interface(&usbtmc_driver, iminor(inode));
if (!intf) {
@@ -130,21 +149,54 @@ static int usbtmc_open(struct inode *inode, struct file 
*filp)
return -ENODEV;
}
 
+   file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
+   if (!file_data)
+   return -ENOMEM;
+
+   pr_debug("%s - called\n", __func__);
+
data = usb_get_intfdata(intf);
/* Protect reference to data from file structure until release */
kref_get(&data->kref);
 
+   mutex_lock(&data->io_mutex);
+   file_data->data = data;
+
+   /* copy default values from device settings */
+   file_data->TermChar = data->TermChar;
+   file_data->TermCharEnabled = data->TermCharEnabled;
+   file_data->auto_abort = data->auto_abort;
+
+   INIT_LIST_HEAD(&file_data->file_elem);
+   spin_lock_irq(&data->dev_lock);
+   list_add_tail(&file_data->file_elem, &data->file_list);
+   spin_unlock_irq(&data->dev_lock);
+   mutex_unlock(&data->io_mutex);
+
/* Store pointer in file structure's private data field */
-   filp->private_data = data;
+   filp->private_data = file_data;
 
-   return retval;
+   return 0;
 }
 
 static int usbtmc_release(struct inode *inode, struct file *file)
 {
-   struct usbtmc_device_data *data = file->private_data;
+   struct usbtmc_file_data *file_data = file->private_data;
 
-   kref_put(&data->kref, usbtmc_delete);
+   pr_debug("%s - called\n", __func__);
+
+   /* prevent IO _AND_ usbtmc_interrupt */
+   mutex_lock(&file_data->data->io_mutex);
+   spin_lock_irq(&file_data->data->dev_lock);
+
+   list_del(&file_data->file_elem);
+
+   spin_unlock_irq(&file_data->data->dev_lock);
+   mutex_unlock(&file_data->data->io_mutex);
+
+   kref_put(&file_data->data->kref, usbtmc_delete);
+   file_data->data = NULL;
+   kfree(file

[PATCH 05/12] usb: usbtmc: Add ioctl for generic requests on control pipe

[Guido Kiener]

Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the
control pipe.  Used by specific applications of IVI Foundation,
Inc. to implement VISA API functions: viUsbControlIn/Out.

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c   | 61 
 include/uapi/linux/usb/tmc.h | 15 +
 2 files changed, 76 insertions(+)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 152e2daa9644..00c2e51a23a7 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany
  * Copyright (C) 2008 Novell, Inc.
  * Copyright (C) 2008 Greg Kroah-Hartman 
+ * Copyright (C) 2018, IVI Foundation, Inc.
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -1263,6 +1264,62 @@ static int usbtmc_ioctl_indicator_pulse(struct 
usbtmc_device_data *data)
return rv;
 }
 
+static int usbtmc_ioctl_request(struct usbtmc_device_data *data,
+   void __user *arg)
+{
+   struct device *dev = &data->intf->dev;
+   struct usbtmc_ctrlrequest request;
+   u8 *buffer = NULL;
+   int rv;
+   unsigned long res;
+
+   res = copy_from_user(&request, arg, sizeof(struct usbtmc_ctrlrequest));
+   if (res)
+   return -EFAULT;
+
+   buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL);
+   if (!buffer)
+   return -ENOMEM;
+
+   if ((request.req.bRequestType & USB_DIR_IN) == 0
+   && request.req.wLength) {
+   res = copy_from_user(buffer, request.data,
+request.req.wLength);
+   if (res) {
+   rv = -EFAULT;
+   goto exit;
+   }
+   }
+
+   rv = usb_control_msg(data->usb_dev,
+   usb_rcvctrlpipe(data->usb_dev, 0),
+   request.req.bRequest,
+   request.req.bRequestType,
+   request.req.wValue,
+   request.req.wIndex,
+   buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT);
+
+   if (rv < 0) {
+   dev_err(dev, "%s failed %d\n", __func__, rv);
+   goto exit;
+   }
+   if ((request.req.bRequestType & USB_DIR_IN)) {
+   if (rv > request.req.wLength) {
+   dev_warn(dev, "%s returned too much data: %d\n",
+__func__, rv);
+   rv = request.req.wLength;
+   }
+
+   res = copy_to_user(request.data, buffer, rv);
+   if (res)
+   rv = -EFAULT;
+   }
+
+ exit:
+   kfree(buffer);
+   return rv;
+}
+
 /*
  * Get the usb timeout value
  */
@@ -1379,6 +1436,10 @@ static long usbtmc_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
retval = usbtmc_ioctl_abort_bulk_in(data);
break;
 
+   case USBTMC_IOCTL_CTRL_REQUEST:
+   retval = usbtmc_ioctl_request(data, (void __user *)arg);
+   break;
+
case USBTMC_IOCTL_GET_TIMEOUT:
retval = usbtmc_ioctl_get_timeout(file_data,
  (void __user *)arg);
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 60369825691c..c1acec9ad834 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -4,6 +4,7 @@
  * Copyright (C) 2008 Novell, Inc.
  * Copyright (C) 2008 Greg Kroah-Hartman 
  * Copyright (C) 2015 Dave Penkler 
+ * Copyright (C) 2018, IVI Foundation, Inc.
  *
  * This file holds USB constants defined by the USB Device Class
  * and USB488 Subclass Definitions for Test and Measurement devices
@@ -40,6 +41,19 @@
 #define USBTMC488_REQUEST_GOTO_LOCAL   161
 #define USBTMC488_REQUEST_LOCAL_LOCKOUT162
 
+struct usbtmc_request {
+   __u8 bRequestType;
+   __u8 bRequest;
+   __u16 wValue;
+   __u16 wIndex;
+   __u16 wLength;
+} __attribute__ ((packed));
+
+struct usbtmc_ctrlrequest {
+   struct usbtmc_request req;
+   void __user *data;
+} __attribute__ ((packed));
+
 struct usbtmc_termchar {
__u8 term_char;
__u8 term_char_enabled; // bool
@@ -53,6 +67,7 @@ struct usbtmc_termchar {
 #define USBTMC_IOCTL_ABORT_BULK_IN _IO(USBTMC_IOC_NR, 4)
 #define USBTMC_IOCTL_CLEAR_OUT_HALT_IO(USBTMC_IOC_NR, 6)
 #define USBTMC_IOCTL_CLEAR_IN_HALT _IO(USBTMC_IOC_NR, 7)
+#define USBTMC_IOCTL_CTRL_REQUEST  _IOWR(USBTMC_IOC_NR, 8, struct 
usbtmc_ctrlrequest)
 #define USBTMC_IOCTL_GET_TIMEOUT   _IOR(USBTMC_IOC_NR, 9, unsigned int)
 #define USBTMC_IOCTL_SET_TIMEOUT   _IOW(USBTMC_IOC_NR, 10, unsigned int)
 #define USBTMC_IOCTL_EOM_ENABLE_IOW(USBTMC_IOC_NR, 11, __u8)
-- 
2.17.0

--
To unsubscribe from this list: send the line "unsubscribe 

[PATCH 04/12] usb: usbtmc: Add ioctls for trigger, EOM bit and TermChar

[Guido Kiener]

- add USBTMC488_IOCTL_TRIGGER to send TRIGGER Bulk-OUT header
  according to Subclass USB488 Specification

  The usbtmc trigger command is equivalent to the IEEE 488 GET (Group
  Execute Trigger) action. While the "*TRG" command can be sent as
  data to perform the same operation, in some situations an instrument
  will be busy and unable to process the data immediately in which
  case the USBTMC488_IOCTL_TRIGGER can be used to trigger the
  instrument with lower latency.

- add USBTMC_IOCTL_EOM_ENABLE to specify EOM bit for next write()
  call. Sets Bit 0 of field 'bmTransferAttributes' of DEV_DEP_MSG_OUT
  Bulk-OUT Header.

  Allows fine grained control over end of message handling on a
  per file descriptor basis.

- add USBTMC_IOCTL_CONFIG_TERMCHAR to control TermChar handling
  for next read(). Controls field 'TermChar' and Bit 1 of field
  'bmTransferAttributes' of REQUEST_DEV_DEP_MSG_IN BULK-OUT header.

  Allows enabling/disabling of terminating a read on reception of
  term_char individually for each read request.

Reviewed-by: Steve Bayless 
Signed-off-by: Dave Penkler 
Signed-off-by: Guido Kiener 
---
 drivers/usb/class/usbtmc.c   | 111 +--
 include/uapi/linux/usb/tmc.h |  11 
 2 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index ad7872003420..152e2daa9644 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -126,6 +126,7 @@ struct usbtmc_file_data {
u8 TermChar;
bool   TermCharEnabled;
bool   auto_abort;
+   u8 eom_val;
 };
 
 /* Forward declarations */
@@ -170,6 +171,7 @@ static int usbtmc_open(struct inode *inode, struct file 
*filp)
file_data->TermChar = data->TermChar;
file_data->TermCharEnabled = data->TermCharEnabled;
file_data->auto_abort = data->auto_abort;
+   file_data->eom_val = 1;
 
INIT_LIST_HEAD(&file_data->file_elem);
spin_lock_irq(&data->dev_lock);
@@ -577,6 +579,51 @@ static int usbtmc488_ioctl_simple(struct 
usbtmc_device_data *data,
return rv;
 }
 
+/*
+ * Sends a TRIGGER Bulk-OUT command message
+ * See the USBTMC-USB488 specification, Table 2.
+ *
+ * Also updates bTag_last_write.
+ */
+static int usbtmc488_ioctl_trigger(struct usbtmc_file_data *file_data)
+{
+   struct usbtmc_device_data *data = file_data->data;
+   int retval;
+   u8 *buffer;
+   int actual;
+
+   buffer = kzalloc(USBTMC_HEADER_SIZE, GFP_KERNEL);
+   if (!buffer)
+   return -ENOMEM;
+
+   buffer[0] = 128;
+   buffer[1] = data->bTag;
+   buffer[2] = ~data->bTag;
+
+   retval = usb_bulk_msg(data->usb_dev,
+ usb_sndbulkpipe(data->usb_dev,
+ data->bulk_out),
+ buffer, USBTMC_HEADER_SIZE,
+ &actual, file_data->timeout);
+
+   /* Store bTag (in case we need to abort) */
+   data->bTag_last_write = data->bTag;
+
+   /* Increment bTag -- and increment again if zero */
+   data->bTag++;
+   if (!data->bTag)
+   data->bTag++;
+
+   kfree(buffer);
+   if (retval < 0) {
+   dev_err(&data->intf->dev, "%s returned %d\n",
+   __func__, retval);
+   return retval;
+   }
+
+   return 0;
+}
+
 /*
  * Sends a REQUEST_DEV_DEP_MSG_IN message on the Bulk-OUT endpoint.
  * @transfer_size: number of bytes to request from the device.
@@ -829,7 +876,7 @@ static ssize_t usbtmc_write(struct file *filp, const char 
__user *buf,
buffer[8] = 0;
} else {
this_part = remaining;
-   buffer[8] = 1;
+   buffer[8] = file_data->eom_val;
}
 
/* Setup IO buffer for DEV_DEP_MSG_OUT message */
@@ -1251,6 +1298,47 @@ static int usbtmc_ioctl_set_timeout(struct 
usbtmc_file_data *file_data,
return 0;
 }
 
+/*
+ * enables/disables sending EOM on write
+ */
+static int usbtmc_ioctl_eom_enable(struct usbtmc_file_data *file_data,
+   void __user *arg)
+{
+   __u8 eom_enable;
+
+   if (copy_from_user(&eom_enable, arg, sizeof(eom_enable)))
+   return -EFAULT;
+
+   if (eom_enable > 1)
+   return -EINVAL;
+
+   file_data->eom_val = eom_enable;
+
+   return 0;
+}
+
+/*
+ * Configure TermChar and TermCharEnable
+ */
+static int usbtmc_ioctl_config_termc(struct usbtmc_file_data *file_data,
+   void __user *arg)
+{
+   struct usbtmc_termchar termc;
+
+   if (copy_from_user(&termc, arg, sizeof(termc)))
+   return -EFAULT;
+
+   if ((termc.term_char_enabled > 1) ||
+   (termc.term_char_enabled &&
+   !(file_data->data->capabilities.device_capabilities & 1)))
+

[PATCH 03/12] usb: usbtmc: Add ioctls to set/get usb timeout

[Guido Kiener]

Add ioctls USBTMC_IOCTL_GET_TIMEOUT / USBTMC_IOCTL_SET_TIMEOUT to
get/set I/O timeout for specific file handle.

Different operations on an instrument can take different lengths of
time thus it is important to be able to set the timeout slightly
longer than the expected duration of each operation to optimise the
responsiveness of the application. As the instrument may be shared by
multiple applications the timeout should be settable on a per file
descriptor basis.

Tested-by: Dave Penkler 
Reviewed-by: Steve Bayless 
Signed-off-by: Dave Penkler 
Signed-off-by: Guido Kiener 
---
 drivers/usb/class/usbtmc.c   | 58 +---
 include/uapi/linux/usb/tmc.h |  2 ++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 5754354429d8..ad7872003420 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -30,6 +30,8 @@
  */
 #define USBTMC_SIZE_IOBUFFER   2048
 
+/* Minimum USB timeout (in milliseconds) */
+#define USBTMC_MIN_TIMEOUT 100
 /* Default USB timeout (in milliseconds) */
 #define USBTMC_TIMEOUT 5000
 
@@ -116,6 +118,7 @@ struct usbtmc_file_data {
struct usbtmc_device_data *data;
struct list_head file_elem;
 
+   u32timeout;
u8 srq_byte;
atomic_t   srq_asserted;
 
@@ -163,6 +166,7 @@ static int usbtmc_open(struct inode *inode, struct file 
*filp)
file_data->data = data;
 
/* copy default values from device settings */
+   file_data->timeout = USBTMC_TIMEOUT;
file_data->TermChar = data->TermChar;
file_data->TermCharEnabled = data->TermCharEnabled;
file_data->auto_abort = data->auto_abort;
@@ -478,7 +482,7 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data 
*file_data,
rv = wait_event_interruptible_timeout(
data->waitq,
atomic_read(&data->iin_data_valid) != 0,
-   USBTMC_TIMEOUT);
+   file_data->timeout);
if (rv < 0) {
dev_dbg(dev, "wait interrupted %d\n", rv);
goto exit;
@@ -613,7 +617,8 @@ static int send_request_dev_dep_msg_in(struct 
usbtmc_file_data *file_data,
retval = usb_bulk_msg(data->usb_dev,
  usb_sndbulkpipe(data->usb_dev,
  data->bulk_out),
- buffer, USBTMC_HEADER_SIZE, &actual, 
USBTMC_TIMEOUT);
+ buffer, USBTMC_HEADER_SIZE,
+ &actual, file_data->timeout);
 
/* Store bTag (in case we need to abort) */
data->bTag_last_write = data->bTag;
@@ -681,7 +686,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
  usb_rcvbulkpipe(data->usb_dev,
  data->bulk_in),
  buffer, USBTMC_SIZE_IOBUFFER, &actual,
- USBTMC_TIMEOUT);
+ file_data->timeout);
 
dev_dbg(dev, "usb_bulk_msg: retval(%u), done(%zu), 
remaining(%zu), actual(%d)\n", retval, done, remaining, actual);
 
@@ -854,7 +859,7 @@ static ssize_t usbtmc_write(struct file *filp, const char 
__user *buf,
  usb_sndbulkpipe(data->usb_dev,
  data->bulk_out),
  buffer, n_bytes,
- &actual, USBTMC_TIMEOUT);
+ &actual, file_data->timeout);
if (retval != 0)
break;
n_bytes -= actual;
@@ -1211,6 +1216,41 @@ static int usbtmc_ioctl_indicator_pulse(struct 
usbtmc_device_data *data)
return rv;
 }
 
+/*
+ * Get the usb timeout value
+ */
+static int usbtmc_ioctl_get_timeout(struct usbtmc_file_data *file_data,
+   void __user *arg)
+{
+   __u32 timeout;
+
+   timeout = file_data->timeout;
+
+   return put_user(timeout, (__u32 __user *)arg);
+}
+
+/*
+ * Set the usb timeout value
+ */
+static int usbtmc_ioctl_set_timeout(struct usbtmc_file_data *file_data,
+   void __user *arg)
+{
+   __u32 timeout;
+
+   if (get_user(timeout, (__u32 __user *)arg))
+   return -EFAULT;
+
+   /* Note that timeout = 0 means
+* MAX_SCHEDULE_TIMEOUT in usb_control_msg
+*/
+   if (timeout < USBTMC_MIN_TIMEOUT)
+   return -EINVAL;
+
+   file_data->timeout = timeout;
+
+   return 0;
+}
+
 static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long 
arg)
 {
struct usbtmc_file_data *file_data;
@@ -1251,6 +1291,16 @

[PATCH 09/12] usb: usbtmc: Fix ioctl USBTMC_IOCTL_CLEAR

[Guido Kiener]

Insert a sleep of 50 ms between subsequent CHECK_CLEAR_STATUS
control requests to avoid stressing the instrument with repeated
requests.

Some instruments need time to cleanup internal I/O buffers.
Polling and repeated requests slow down the response time of
devices.

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c | 46 +++---
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 76b0a7b083a7..165b707991f3 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1643,20 +1643,17 @@ static ssize_t usbtmc_write(struct file *filp, const 
char __user *buf,
 
 static int usbtmc_ioctl_clear(struct usbtmc_device_data *data)
 {
-   struct usb_host_interface *current_setting;
-   struct usb_endpoint_descriptor *desc;
struct device *dev;
u8 *buffer;
int rv;
int n;
int actual = 0;
-   int max_size;
 
dev = &data->intf->dev;
 
dev_dbg(dev, "Sending INITIATE_CLEAR request\n");
 
-   buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
+   buffer = kmalloc(USBTMC_BUFSIZE, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
 
@@ -1664,7 +1661,7 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
 usb_rcvctrlpipe(data->usb_dev, 0),
 USBTMC_REQUEST_INITIATE_CLEAR,
 USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-0, 0, buffer, 1, USBTMC_TIMEOUT);
+0, 0, buffer, 1, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
@@ -1678,22 +1675,6 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
goto exit;
}
 
-   max_size = 0;
-   current_setting = data->intf->cur_altsetting;
-   for (n = 0; n < current_setting->desc.bNumEndpoints; n++) {
-   desc = ¤t_setting->endpoint[n].desc;
-   if (desc->bEndpointAddress == data->bulk_in)
-   max_size = usb_endpoint_maxp(desc);
-   }
-
-   if (max_size == 0) {
-   dev_err(dev, "Couldn't get wMaxPacketSize\n");
-   rv = -EPERM;
-   goto exit;
-   }
-
-   dev_dbg(dev, "wMaxPacketSize is %d\n", max_size);
-
n = 0;
 
 usbtmc_clear_check_status:
@@ -1704,7 +1685,7 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
 usb_rcvctrlpipe(data->usb_dev, 0),
 USBTMC_REQUEST_CHECK_CLEAR_STATUS,
 USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-0, 0, buffer, 2, USBTMC_TIMEOUT);
+0, 0, buffer, 2, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
@@ -1721,15 +1702,19 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
goto exit;
}
 
-   if (buffer[1] == 1)
+   if ((buffer[1] & 1) != 0) {
do {
dev_dbg(dev, "Reading from bulk in EP\n");
 
rv = usb_bulk_msg(data->usb_dev,
  usb_rcvbulkpipe(data->usb_dev,
  data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER,
- &actual, USBTMC_TIMEOUT);
+ buffer, USBTMC_BUFSIZE,
+ &actual, USB_CTRL_GET_TIMEOUT);
+
+   print_hex_dump_debug("usbtmc ", DUMP_PREFIX_NONE,
+16, 1, buffer, actual, true);
+
n++;
 
if (rv < 0) {
@@ -1737,10 +1722,15 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
rv);
goto exit;
}
-   } while ((actual == max_size) &&
+   } while ((actual == USBTMC_BUFSIZE) &&
  (n < USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
+   } else {
+   /* do not stress device with subsequent requests */
+   msleep(50);
+   n++;
+   }
 
-   if (actual == max_size) {
+   if (n >= USBTMC_MAX_READS_TO_CLEAR_BULK_IN) {
dev_err(dev, "Couldn't clear device buffer within %d cycles\n",
USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
rv = -EPERM;
@@ -1754,7 +1744,7 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
rv = usb_clear_halt(data->usb_dev,
   

[PATCH 11/12] usb: usbtmc: Add ioctls to abort with specific tags

[Guido Kiener]

- fix ioctls USBTMC_IOCTL_ABORT_BULK_OUT/IN

- add ioctls USBTMC_IOCTL_ABORT_BULK_OUT_TAG and
  USBTMC_IOCTL_ABORT_BULK_IN_TAG for test purpose.

  Useful for testing devices and client applications: The ioctls
  allow to test the abort mechanism and the response of the
  device when using different or wrong tag ids.

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c   | 151 ++-
 include/uapi/linux/usb/tmc.h |   2 +
 2 files changed, 81 insertions(+), 72 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 8b464598bee5..c24efe513556 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -277,18 +277,17 @@ static int usbtmc_release(struct inode *inode, struct 
file *file)
return 0;
 }
 
-static int usbtmc_ioctl_abort_bulk_in(struct usbtmc_device_data *data)
+static int usbtmc_ioctl_abort_bulk_in_tag(struct usbtmc_device_data *data,
+ u8 bTag)
 {
u8 *buffer;
struct device *dev;
int rv;
int n;
int actual;
-   struct usb_host_interface *current_setting;
-   int max_size;
 
dev = &data->intf->dev;
-   buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
+   buffer = kmalloc(USBTMC_BUFSIZE, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
 
@@ -296,86 +295,87 @@ static int usbtmc_ioctl_abort_bulk_in(struct 
usbtmc_device_data *data)
 usb_rcvctrlpipe(data->usb_dev, 0),
 USBTMC_REQUEST_INITIATE_ABORT_BULK_IN,
 USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
-data->bTag_last_read, data->bulk_in,
-buffer, 2, USBTMC_TIMEOUT);
+bTag, data->bulk_in,
+buffer, 2, USB_CTRL_GET_TIMEOUT);
 
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto exit;
}
 
-   dev_dbg(dev, "INITIATE_ABORT_BULK_IN returned %x\n", buffer[0]);
+   dev_dbg(dev, "INITIATE_ABORT_BULK_IN returned %x with tag %02x\n",
+   buffer[0], buffer[1]);
 
if (buffer[0] == USBTMC_STATUS_FAILED) {
+   /* No transfer in progress and the Bulk-OUT FIFO is empty. */
rv = 0;
goto exit;
}
 
-   if (buffer[0] != USBTMC_STATUS_SUCCESS) {
-   dev_err(dev, "INITIATE_ABORT_BULK_IN returned %x\n",
-   buffer[0]);
-   rv = -EPERM;
+   if (buffer[0] == USBTMC_STATUS_TRANSFER_NOT_IN_PROGRESS) {
+   /* The device returns this status if either:
+* - There is a transfer in progress, but the specified bTag
+*   does not match.
+* - There is no transfer in progress, but the Bulk-OUT FIFO
+*   is not empty.
+*/
+   rv = -ENOMSG;
goto exit;
}
 
-   max_size = 0;
-   current_setting = data->intf->cur_altsetting;
-   for (n = 0; n < current_setting->desc.bNumEndpoints; n++)
-   if (current_setting->endpoint[n].desc.bEndpointAddress ==
-   data->bulk_in)
-   max_size = 
usb_endpoint_maxp(¤t_setting->endpoint[n].desc);
-
-   if (max_size == 0) {
-   dev_err(dev, "Couldn't get wMaxPacketSize\n");
+   if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+   dev_err(dev, "INITIATE_ABORT_BULK_IN returned %x\n",
+   buffer[0]);
rv = -EPERM;
goto exit;
}
 
-   dev_dbg(&data->intf->dev, "wMaxPacketSize is %d\n", max_size);
-
n = 0;
 
-   do {
-   dev_dbg(dev, "Reading from bulk in EP\n");
+usbtmc_abort_bulk_in_status:
+   dev_dbg(dev, "Reading from bulk in EP\n");
 
-   rv = usb_bulk_msg(data->usb_dev,
- usb_rcvbulkpipe(data->usb_dev,
- data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER,
- &actual, USBTMC_TIMEOUT);
+   /* Data must be present. So use low timeout 300 ms */
+   rv = usb_bulk_msg(data->usb_dev,
+ usb_rcvbulkpipe(data->usb_dev,
+ data->bulk_in),
+ buffer, USBTMC_BUFSIZE,
+ &actual, 300);
 
-   n++;
+   print_hex_dump_debug("usbtmc ", DUMP_PREFIX_NONE, 16, 1,
+buffer, actual, true);
 
-   if (rv < 0) {
-   dev_err(dev, "usb_bulk_msg returned %d\n", rv);
+   n++;
+
+   if (rv < 0) {
+   dev_err(dev, "usb_bulk_msg returned %d\n", rv);
+   if (rv != -ETIMEDOUT)
   

[PATCH 12/12] usb: usbtmc: Add ioctl to return API version of usbtmc driver

[Guido Kiener]

- add ioctl USBTMC_IOCTL_API_VERSION to get current API version
- add info message to show API version
- replace USBTMC_TIMEOUT macros with common used USB_CTRL_GET_TIMEOUT
  or USB_CTRL_SET_TIMEOUT macros.
- delete some superfluous code lines.
- update ioctl-number.txt

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 Documentation/ioctl/ioctl-number.txt |  2 +-
 drivers/usb/class/usbtmc.c   | 38 +++-
 include/uapi/linux/usb/tmc.h |  1 +
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 7f7413e597f3..9112df246a77 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -200,7 +200,7 @@ Code  Seq#(hex) Include FileComments
 'X'01  linux/pktcdvd.h conflict!
 'Y'all linux/cyclades.h
 'Z'14-15   drivers/message/fusion/mptctl.h
-'['00-07   linux/usb/tmc.h USB Test and Measurement Devices
+'['00-3F   linux/usb/tmc.h USB Test and Measurement Devices

 'a'all linux/atm*.h, linux/sonet.h ATM on linux

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index c24efe513556..25c5418556ef 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -21,16 +21,14 @@
 #include 
 #include 
 
+/* Increment API VERSION when changing tmc.h with new flags or ioctls
+ * or when changing a significant behavior of the driver.
+ */
+#define USBTMC_API_VERSION (2)
 
 #define USBTMC_HEADER_SIZE 12
 #define USBTMC_MINOR_BASE  176
 
-/*
- * Size of driver internal IO buffer. Must be multiple of 4 and at least as
- * large as wMaxPacketSize (which is usually 512 bytes).
- */
-#define USBTMC_SIZE_IOBUFFER   2048
-
 /* Minimum USB timeout (in milliseconds) */
 #define USBTMC_MIN_TIMEOUT 100
 /* Default USB timeout (in milliseconds) */
@@ -512,8 +510,6 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data 
*file_data,
rv = put_user(stb, (__u8 __user *)arg);
dev_dbg(dev, "stb:0x%02x with srq received %d\n",
(unsigned int)stb, rv);
-   if (rv)
-   return -EFAULT;
return rv;
}
spin_unlock_irq(&data->dev_lock);
@@ -530,7 +526,7 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data 
*file_data,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
data->iin_bTag,
data->ifnum,
-   buffer, 0x03, USBTMC_TIMEOUT);
+   buffer, 0x03, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "stb usb_control_msg returned %d\n", rv);
goto exit;
@@ -569,10 +565,7 @@ static int usbtmc488_ioctl_read_stb(struct 
usbtmc_file_data *file_data,
stb = buffer[2];
}
 
-   if (put_user(stb, (__u8 __user *)arg))
-   rv = -EFAULT;
-   else
-   rv = 0;
+   rv = put_user(stb, (__u8 __user *)arg);
dev_dbg(dev, "stb:0x%02x received %d\n", (unsigned int)stb, rv);
 
  exit:
@@ -667,7 +660,7 @@ static int usbtmc488_ioctl_simple(struct usbtmc_device_data 
*data,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
wValue,
data->ifnum,
-   buffer, 0x01, USBTMC_TIMEOUT);
+   buffer, 0x01, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "simple usb_control_msg failed %d\n", rv);
goto exit;
@@ -1861,7 +1854,7 @@ static int get_capabilities(struct usbtmc_device_data 
*data)
rv = usb_control_msg(data->usb_dev, usb_rcvctrlpipe(data->usb_dev, 0),
 USBTMC_REQUEST_GET_CAPABILITIES,
 USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-0, 0, buffer, 0x18, USBTMC_TIMEOUT);
+0, 0, buffer, 0x18, USB_CTRL_GET_TIMEOUT);
if (rv < 0) {
dev_err(dev, "usb_control_msg returned %d\n", rv);
goto err_out;
@@ -1984,6 +1977,9 @@ static const struct attribute_group data_attr_grp = {
.attrs = data_attrs,
 };
 
+/*
+ * Flash activity indicator on device
+ */
 static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data)
 {
struct device *dev;
@@ -2000,7 +1996,7 @@ static int usbtmc_ioctl_indicator_pulse(struct 
usbtmc_device_data *data)
 usb_rcvctrlpipe(data->usb_dev, 0),
 USBTMC_REQUEST_INDICATOR_PULSE,
 USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-0, 0, buffer, 0x

[PATCH 08/12] usb: usbtmc: Optimize read/write and add ioctls for auto_abort

[Guido Kiener]

- Optimize read/write functions for better bandwidth and
  fixes reading of long transfers

- add ioctl USBTMC_IOCTL_AUTO_ABORT to configure auto_abort for
  each specific file handle.

- add ioctl USBTMC_IOCTL_MSG_IN_ATTR that returns the specific
  bmTransferAttributes field of the last DEV_DEP_MSG_IN Bulk-IN
  header. This header is received by the read() function. The
  meaning of the (u8) bitmap bmTransferAttributes is:
  Bit 0 = EOM flag is set when the last of a USBTMC message is
  received.
  Bit 1 = is set when the last byte is a termchar (e.g. '\n').
  Note that this bit is always zero when the device does not support
  termchar feature or when termchar detection is not enabled
  (see ioctl USBTMC_IOCTL_CONFIG_TERMCHAR).

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c   | 357 ---
 include/uapi/linux/usb/tmc.h |   3 +
 2 files changed, 206 insertions(+), 154 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 6b8b8510cfc4..76b0a7b083a7 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -131,6 +131,7 @@ struct usbtmc_file_data {
u8 srq_byte;
atomic_t   srq_asserted;
atomic_t   closing;
+   u8 bmTransferAttributes; /* member of DEV_DEP_MSG_IN */
 
/* These values are initialized with default values from device_data */
u8 TermChar;
@@ -1356,20 +1357,20 @@ static ssize_t usbtmc_read(struct file *filp, char 
__user *buf,
struct usbtmc_file_data *file_data;
struct usbtmc_device_data *data;
struct device *dev;
+   const size_t bufsize = USBTMC_BUFSIZE;
u32 n_characters;
u8 *buffer;
int actual;
-   size_t done;
-   size_t remaining;
+   u64 done = 0;
+   u64 remaining;
int retval;
-   size_t this_part;
 
/* Get pointer to private data structure */
file_data = filp->private_data;
data = file_data->data;
dev = &data->intf->dev;
 
-   buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
+   buffer = kmalloc(bufsize, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
 
@@ -1379,124 +1380,117 @@ static ssize_t usbtmc_read(struct file *filp, char 
__user *buf,
goto exit;
}
 
-   dev_dbg(dev, "usb_bulk_msg_in: count(%zu)\n", count);
+   if (count > INT_MAX)
+   count = INT_MAX;
+
+   dev_dbg(dev, "%s(count:%zu)\n", __func__, count);
 
retval = send_request_dev_dep_msg_in(file_data, count);
 
if (retval < 0) {
-   if (data->auto_abort)
+   if (file_data->auto_abort)
usbtmc_ioctl_abort_bulk_out(data);
goto exit;
}
 
/* Loop until we have fetched everything we requested */
remaining = count;
-   this_part = remaining;
-   done = 0;
 
-   while (remaining > 0) {
-   /* Send bulk URB */
-   retval = usb_bulk_msg(data->usb_dev,
- usb_rcvbulkpipe(data->usb_dev,
- data->bulk_in),
- buffer, USBTMC_SIZE_IOBUFFER, &actual,
- file_data->timeout);
+   /* Send bulk URB */
+   retval = usb_bulk_msg(data->usb_dev,
+ usb_rcvbulkpipe(data->usb_dev,
+ data->bulk_in),
+ buffer, bufsize, &actual,
+ file_data->timeout);
 
-   dev_dbg(dev, "usb_bulk_msg: retval(%u), done(%zu), 
remaining(%zu), actual(%d)\n", retval, done, remaining, actual);
+   dev_dbg(dev, "%s: bulk_msg retval(%u), actual(%d)\n",
+   __func__, retval, actual);
 
-   /* Store bTag (in case we need to abort) */
-   data->bTag_last_read = data->bTag;
+   /* Store bTag (in case we need to abort) */
+   data->bTag_last_read = data->bTag;
 
-   if (retval < 0) {
-   dev_dbg(dev, "Unable to read data, error %d\n", retval);
-   if (data->auto_abort)
-   usbtmc_ioctl_abort_bulk_in(data);
-   goto exit;
-   }
+   if (retval < 0) {
+   if (file_data->auto_abort)
+   usbtmc_ioctl_abort_bulk_in(data);
+   goto exit;
+   }
 
-   /* Parse header in first packet */
-   if (done == 0) {
-   /* Sanity checks for the header */
-   if (actual < USBTMC_HEADER_SIZE) {
-   dev_err(dev, "Device sent too small first 
packet: %u < %u\n", actual, USBTMC_HEADER_SIZE);
-   if (data->auto_abort)
-

[PATCH 10/12] usb: usbtmc: Add test functions to set HALT Feature (Stall)

[Guido Kiener]

The ioctls USBTMC_IOCTL_SET_OUT_HALT or USBTMC_IOCTL_SET_IN_HALT
send a SET_FEATURE(HALT) request to the corresponding OUT or IN pipe.

Useful for testing devices and client applications: The ioctls force
the device to simulate the error state at the specified pipe.

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c   | 78 ++--
 include/uapi/linux/usb/tmc.h |  4 ++
 2 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 165b707991f3..8b464598bee5 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -1754,34 +1754,80 @@ static int usbtmc_ioctl_clear(struct usbtmc_device_data 
*data)
return rv;
 }
 
+/*
+ * set pipe in halt state (stalled)
+ * Needed for test purpose or workarounds.
+ */
+static int usbtmc_set_halt(struct usb_device *dev, int pipe)
+{
+   int rv;
+   int endp = usb_pipeendpoint(pipe);
+
+   if (usb_pipein(pipe))
+   endp |= USB_DIR_IN;
+
+   rv = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+USB_REQ_SET_FEATURE, USB_RECIP_ENDPOINT,
+USB_ENDPOINT_HALT, endp, NULL, 0,
+USB_CTRL_SET_TIMEOUT);
+
+   return rv;
+}
+
+static int usbtmc_ioctl_set_out_halt(struct usbtmc_device_data *data)
+{
+   int rv;
+
+   dev_dbg(&data->intf->dev, "%s - called\n", __func__);
+
+   rv = usbtmc_set_halt(data->usb_dev,
+usb_sndbulkpipe(data->usb_dev, data->bulk_out));
+
+   if (rv < 0)
+   dev_err(&data->usb_dev->dev, "%s returned %d\n", __func__, rv);
+   return rv;
+}
+
+static int usbtmc_ioctl_set_in_halt(struct usbtmc_device_data *data)
+{
+   int rv;
+
+   dev_dbg(&data->intf->dev, "%s - called\n", __func__);
+
+   rv = usbtmc_set_halt(data->usb_dev,
+usb_rcvbulkpipe(data->usb_dev, data->bulk_in));
+
+   if (rv < 0)
+   dev_err(&data->usb_dev->dev, "%s returned %d\n", __func__, rv);
+   return rv;
+}
+
 static int usbtmc_ioctl_clear_out_halt(struct usbtmc_device_data *data)
 {
int rv;
 
+   dev_dbg(&data->intf->dev, "%s - called\n", __func__);
+
rv = usb_clear_halt(data->usb_dev,
usb_sndbulkpipe(data->usb_dev, data->bulk_out));
 
-   if (rv < 0) {
-   dev_err(&data->usb_dev->dev, "usb_control_msg returned %d\n",
-   rv);
-   return rv;
-   }
-   return 0;
+   if (rv < 0)
+   dev_err(&data->usb_dev->dev, "%s returned %d\n", __func__, rv);
+   return rv;
 }
 
 static int usbtmc_ioctl_clear_in_halt(struct usbtmc_device_data *data)
 {
int rv;
 
+   dev_dbg(&data->intf->dev, "%s - called\n", __func__);
+
rv = usb_clear_halt(data->usb_dev,
usb_rcvbulkpipe(data->usb_dev, data->bulk_in));
 
-   if (rv < 0) {
-   dev_err(&data->usb_dev->dev, "usb_control_msg returned %d\n",
-   rv);
-   return rv;
-   }
-   return 0;
+   if (rv < 0)
+   dev_err(&data->usb_dev->dev, "%s returned %d\n", __func__, rv);
+   return rv;
 }
 
 static int usbtmc_ioctl_cancel_io(struct usbtmc_file_data *file_data)
@@ -2241,6 +2287,14 @@ static long usbtmc_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
file_data->auto_abort = !!tmp_byte;
break;
 
+   case USBTMC_IOCTL_SET_OUT_HALT:
+   retval = usbtmc_ioctl_set_out_halt(data);
+   break;
+
+   case USBTMC_IOCTL_SET_IN_HALT:
+   retval = usbtmc_ioctl_set_in_halt(data);
+   break;
+
case USBTMC_IOCTL_CANCEL_IO:
retval = usbtmc_ioctl_cancel_io(file_data);
break;
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index e3bdfc1935ed..886fabf5dfea 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -101,6 +101,10 @@ struct usbtmc_message {
 #define USBTMC_IOCTL_MSG_IN_ATTR   _IOR(USBTMC_IOC_NR, 24, __u8)
 #define USBTMC_IOCTL_AUTO_ABORT_IOW(USBTMC_IOC_NR, 25, __u8)
 
+/* For test purpose only */
+#define USBTMC_IOCTL_SET_OUT_HALT  _IO(USBTMC_IOC_NR, 30)
+#define USBTMC_IOCTL_SET_IN_HALT   _IO(USBTMC_IOC_NR, 31)
+
 /* Cancel and cleanup asynchronous calls */
 #define USBTMC_IOCTL_CANCEL_IO _IO(USBTMC_IOC_NR, 35)
 #define USBTMC_IOCTL_CLEANUP_IO_IO(USBTMC_IOC_NR, 36)
-- 
2.17.0

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

[PATCH 01/12] usb: usbtmc: Remove rigol_quirk

[Guido Kiener]

All T&M instruments should also work with rigol_quirk = 1 code path.
So remove unnecessary code in rigol_quirk = 0 code path to simplify the driver.

Tested-by: Dave Penkler 
Reviewed-by: Steve Bayless 
Signed-off-by: Guido Kiener 
---
 drivers/usb/class/usbtmc.c | 81 ++
 1 file changed, 12 insertions(+), 69 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index bdb1de0c0cef..529295a17579 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -21,7 +21,6 @@
 #include 
 
 
-#define RIGOL  1
 #define USBTMC_HEADER_SIZE 12
 #define USBTMC_MINOR_BASE  176
 
@@ -93,8 +92,6 @@ struct usbtmc_device_data {
/* coalesced usb488_caps from usbtmc_dev_capabilities */
__u8 usb488_caps;
 
-   u8 rigol_quirk;
-
/* attributes from the USB TMC spec for this device */
u8 TermChar;
bool TermCharEnabled;
@@ -110,17 +107,6 @@ struct usbtmc_device_data {
 };
 #define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
 
-struct usbtmc_ID_rigol_quirk {
-   __u16 idVendor;
-   __u16 idProduct;
-};
-
-static const struct usbtmc_ID_rigol_quirk usbtmc_id_quirk[] = {
-   { 0x1ab1, 0x0588 },
-   { 0x1ab1, 0x04b0 },
-   { 0, 0 }
-};
-
 /* Forward declarations */
 static struct usb_driver usbtmc_driver;
 
@@ -603,16 +589,14 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
goto exit;
}
 
-   if (data->rigol_quirk) {
-   dev_dbg(dev, "usb_bulk_msg_in: count(%zu)\n", count);
+   dev_dbg(dev, "usb_bulk_msg_in: count(%zu)\n", count);
 
-   retval = send_request_dev_dep_msg_in(data, count);
+   retval = send_request_dev_dep_msg_in(data, count);
 
-   if (retval < 0) {
-   if (data->auto_abort)
-   usbtmc_ioctl_abort_bulk_out(data);
-   goto exit;
-   }
+   if (retval < 0) {
+   if (data->auto_abort)
+   usbtmc_ioctl_abort_bulk_out(data);
+   goto exit;
}
 
/* Loop until we have fetched everything we requested */
@@ -621,23 +605,6 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
done = 0;
 
while (remaining > 0) {
-   if (!data->rigol_quirk) {
-   dev_dbg(dev, "usb_bulk_msg_in: remaining(%zu), 
count(%zu)\n", remaining, count);
-
-   if (remaining > USBTMC_SIZE_IOBUFFER - 
USBTMC_HEADER_SIZE - 3)
-   this_part = USBTMC_SIZE_IOBUFFER - 
USBTMC_HEADER_SIZE - 3;
-   else
-   this_part = remaining;
-
-   retval = send_request_dev_dep_msg_in(data, this_part);
-   if (retval < 0) {
-   dev_err(dev, "usb_bulk_msg returned %d\n", retval);
-   if (data->auto_abort)
-   usbtmc_ioctl_abort_bulk_out(data);
-   goto exit;
-   }
-   }
-
/* Send bulk URB */
retval = usb_bulk_msg(data->usb_dev,
  usb_rcvbulkpipe(data->usb_dev,
@@ -658,7 +625,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
}
 
/* Parse header in first packet */
-   if ((done == 0) || !data->rigol_quirk) {
+   if (done == 0) {
/* Sanity checks for the header */
if (actual < USBTMC_HEADER_SIZE) {
dev_err(dev, "Device sent too small first 
packet: %u < %u\n", actual, USBTMC_HEADER_SIZE);
@@ -698,20 +665,11 @@ static ssize_t usbtmc_read(struct file *filp, char __user 
*buf,
actual -= USBTMC_HEADER_SIZE;
 
/* Check if the message is smaller than requested */
-   if (data->rigol_quirk) {
-   if (remaining > n_characters)
-   remaining = n_characters;
-   /* Remove padding if it exists */
-   if (actual > remaining)
-   actual = remaining;
-   }
-   else {
-   if (this_part > n_characters)
-   this_part = n_characters;
-   /* Remove padding if it exists */
-   if (actual > this_part)
-   actual = this_part;
-   }
+   if (remaining > n_characters)
+   remaining = n_characters;
+   /* Remove padding if it exists */
+  

[PATCH 07/12] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ

[Guido Kiener]

Wait until an SRQ (service request) is received on the interrupt pipe
or until the given period of time is expired. In contrast to the
poll() function this ioctl does not return when other (a)synchronous
I/O operations fail with EPOLLERR.

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c   | 60 ++--
 include/uapi/linux/usb/tmc.h |  1 +
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index de664a345e69..6b8b8510cfc4 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -594,6 +594,54 @@ static int usbtmc488_ioctl_read_stb(struct 
usbtmc_file_data *file_data,
return rv;
 }
 
+static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data,
+   unsigned int __user *arg)
+{
+   struct usbtmc_device_data *data = file_data->data;
+   struct device *dev = &data->intf->dev;
+   int rv;
+   unsigned int timeout;
+   unsigned long expire;
+
+   if (!data->iin_ep_present) {
+   dev_dbg(dev, "no interrupt endpoint present\n");
+   return -EFAULT;
+   }
+
+   if (get_user(timeout, arg))
+   return -EFAULT;
+
+   expire = msecs_to_jiffies(timeout);
+
+   mutex_unlock(&data->io_mutex);
+
+   rv = wait_event_interruptible_timeout(
+   data->waitq,
+   atomic_read(&file_data->srq_asserted) != 0 ||
+   atomic_read(&file_data->closing),
+   expire);
+
+   mutex_lock(&data->io_mutex);
+
+   /* Note! disconnect or close could be called in the meantime */
+   if (atomic_read(&file_data->closing) || data->zombie)
+   rv = -ENODEV;
+
+   if (rv < 0) {
+   /* dev can be invalid now! */
+   pr_debug("%s - wait interrupted %d\n", __func__, rv);
+   return rv;
+   }
+
+   if (rv == 0) {
+   dev_dbg(dev, "%s - wait timed out\n", __func__);
+   return -ETIMEDOUT;
+   }
+
+   dev_dbg(dev, "%s - srq asserted\n", __func__);
+   return 0;
+}
+
 static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
void __user *arg, unsigned int cmd)
 {
@@ -2149,6 +2197,11 @@ static long usbtmc_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
retval = usbtmc488_ioctl_trigger(file_data);
break;
 
+   case USBTMC488_IOCTL_WAIT_SRQ:
+   retval = usbtmc488_ioctl_wait_srq(file_data,
+ (unsigned int __user *)arg);
+   break;
+
case USBTMC_IOCTL_CANCEL_IO:
retval = usbtmc_ioctl_cancel_io(file_data);
break;
@@ -2290,6 +2343,7 @@ static void usbtmc_interrupt(struct urb *urb)
case -ESHUTDOWN:
case -EILSEQ:
case -ETIME:
+   case -EPIPE:
/* urb terminated, clean up */
dev_dbg(dev, "urb terminated, status: %d\n", status);
return;
@@ -2308,7 +2362,9 @@ static void usbtmc_free_int(struct usbtmc_device_data 
*data)
return;
usb_kill_urb(data->iin_urb);
kfree(data->iin_buffer);
+   data->iin_buffer = NULL;
usb_free_urb(data->iin_urb);
+   data->iin_urb = NULL;
kref_put(&data->kref, usbtmc_delete);
 }
 
@@ -2420,8 +2476,8 @@ static int usbtmc_probe(struct usb_interface *intf,
 
retcode = usb_register_dev(intf, &usbtmc_class);
if (retcode) {
-   dev_err(&intf->dev, "Not able to get a minor"
-   " (base %u, slice default): %d\n", USBTMC_MINOR_BASE,
+   dev_err(&intf->dev, "Not able to get a minor (base %u, slice 
default): %d\n",
+   USBTMC_MINOR_BASE,
retcode);
goto error_register;
}
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 1540716de293..35b63530121d 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -96,6 +96,7 @@ struct usbtmc_message {
 #define USBTMC488_IOCTL_GOTO_LOCAL _IO(USBTMC_IOC_NR, 20)
 #define USBTMC488_IOCTL_LOCAL_LOCKOUT  _IO(USBTMC_IOC_NR, 21)
 #define USBTMC488_IOCTL_TRIGGER_IO(USBTMC_IOC_NR, 22)
+#define USBTMC488_IOCTL_WAIT_SRQ   _IOW(USBTMC_IOC_NR, 23, unsigned int)
 
 /* Cancel and cleanup asynchronous calls */
 #define USBTMC_IOCTL_CANCEL_IO _IO(USBTMC_IOC_NR, 35)
-- 
2.17.0

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

[PATCH 06/12] usb: usbtmc: Add vendor specific/asynchronous read/write ioctls

[Guido Kiener]

- ioctl USBTMC_IOCTL_WRITE sends an (a)synchronous generic message
  to bulk OUT. The message is split into chunks of 4k (page size).
  Message size is aligned to 32 bit boundaries.

  With flag USBTMC_FLAG_ASYNC the ioctl is non blocking.
  With flag USBTMC_FLAG_APPEND additional urbs are queued and
  out_status/out_transfer_size is not reset. EPOLLOUT | EPOLLWRNORM
  is signaled when all submitted urbs are completed.

- ioctl USBTMC_IOCTL_WRITE_RESULT copies current out_transfer_size
  to given __u64 pointer and returns current out_status of the last
  USBTMC_IOCTL_WRITE call.

- ioctl USBTMC_IOCTL_READ reads an (a)synchronous generic message
  from bulk IN. Depending on transfer_size the function submits one
  (<=4kB) or more urbs (up to 16) with a size of 4k.

  The flag USBTMC_FLAG_IGNORE_TRAILER can be used when the transmission
  size is already known. Then the function does not truncate the
  transfer_size to a multiple of 4 kB, but does reserve extra space
  to receive the final short or zero length packet. Note that the
  instrument is allowed to send up to wMaxPacketSize - 1 bytes at the
  end of a message to avoid sending a zero length packet.

  With flag USBTMC_FLAG_ASYNC the ioctl is non blocking. When no
  received data is available, the read function submits as many urbs as
  needed to receive transfer_size bytes. However the number of flying
  urbs (=4kB) is limited to 16 even with subsequent calls of this ioctl.
  Returns -EAGAIN when non blocking and no data is received.
  Signals EPOLLIN | EPOLLRDNORM when asynchronous urbs are ready to
  be read.

  The usbtmc_message.message pointer can be NULL to just trigger
  reading data. However -EINVAL is returned when data is available,
  and the message pointer is NULL.

- ioctl USBTMC_IOCTL_CANCEL_IO cancels last USBTMC_IOCTL_READ and
  USBTMC_IOCTL_WRITE functions which returns -ECANCELED. A subsequent
  call to USBTMC_IOCTL_READ or USBTMC_IOCTL_WRITE_RESULT returns
  -ECANCELED with information about current transferred data.

- ioctl USBTMC_IOCTL_CLEANUP_IO kills all submitted urbs to OUT and
  IN bulk, and clears all received data from IN bulk. Internal
  transfer counters and error states are reset.

- Flush flying urbs when file handle is closed or device is
  suspended.

Signed-off-by: Guido Kiener 
Reviewed-by: Steve Bayless 
---
 drivers/usb/class/usbtmc.c   | 778 ++-
 include/uapi/linux/usb/tmc.h |  21 +
 2 files changed, 797 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 00c2e51a23a7..de664a345e69 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -36,6 +36,11 @@
 /* Default USB timeout (in milliseconds) */
 #define USBTMC_TIMEOUT 5000
 
+/* Max number of urbs used in write transfers */
+#define MAX_URBS_IN_FLIGHT 16
+/* I/O buffer size used in generic read/write functions */
+#define USBTMC_BUFSIZE (4096)
+
 /*
  * Maximum number of read cycles to empty bulk in endpoint during CLEAR and
  * ABORT_BULK_IN requests. Ends the loop if (for whatever reason) a short
@@ -79,6 +84,9 @@ struct usbtmc_device_data {
u8 bTag_last_write; /* needed for abort */
u8 bTag_last_read;  /* needed for abort */
 
+   /* packet size of IN bulk */
+   u16wMaxPacketSize;
+
/* data for interrupt in endpoint handling */
u8 bNotify1;
u8 bNotify2;
@@ -122,16 +130,34 @@ struct usbtmc_file_data {
u32timeout;
u8 srq_byte;
atomic_t   srq_asserted;
+   atomic_t   closing;
 
/* These values are initialized with default values from device_data */
u8 TermChar;
bool   TermCharEnabled;
bool   auto_abort;
u8 eom_val;
+
+   spinlock_t err_lock; /* lock for errors */
+
+   struct usb_anchor submitted;
+
+   /* data for generic_write */
+   struct semaphore limit_write_sem;
+   u64 out_transfer_size;
+   int out_status;
+
+   /* data for generic_read */
+   u64 in_transfer_size;
+   int in_status;
+   int in_urbs_used;
+   struct usb_anchor in_anchor;
+   wait_queue_head_t wait_bulk_in;
 };
 
 /* Forward declarations */
 static struct usb_driver usbtmc_driver;
+static void usbtmc_draw_down(struct usbtmc_file_data *file_data);
 
 static void usbtmc_delete(struct kref *kref)
 {
@@ -160,6 +186,12 @@ static int usbtmc_open(struct inode *inode, struct file 
*filp)
 
pr_debug("%s - called\n", __func__);
 
+   spin_lock_init(&file_data->err_lock);
+   sema_init(&file_data->limit_write_sem, MAX_URBS_IN_FLIGHT);
+   init_usb_anchor(&file_data->submitted);
+   init_usb_anchor(&file_data->in_anchor);
+   init_waitqueue_head(&file_data->wait_bulk_in);
+
data = usb_get_intfdata(intf);
/* Protect reference to data from file struct

Re: [PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle

[Greg KH]

On Thu, May 17, 2018 at 07:03:26PM +0200, Guido Kiener wrote:
> - Add 'struct usbtmc_file_data' for each file handle to cache last
>   srq_byte (=Status Byte with SRQ) received by usbtmc_interrupt(..)
> 
> - usbtmc488_ioctl_read_stb returns cached srq_byte when available for
>   each file handle to avoid race conditions of concurrent applications.
> 
> - SRQ now sets EPOLLPRI instead of EPOLLIN
> 
> - Caches other values TermChar, TermCharEnabled, auto_abort in
>   'struct usbtmc_file_data' will not be changed by sysfs device
>   attributes during an open file session.
>   Future ioctl functions can change these values.
> 
> - use consistent error return value ETIMEOUT instead of ETIME
> 
> Tested-by: Dave Penkler 
> Reviewed-by: Steve Bayless 
> Signed-off-by: Guido Kiener 
> ---
>  drivers/usb/class/usbtmc.c | 176 -
>  1 file changed, 136 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
> index 529295a17579..5754354429d8 100644
> --- a/drivers/usb/class/usbtmc.c
> +++ b/drivers/usb/class/usbtmc.c
> @@ -67,6 +67,7 @@ struct usbtmc_device_data {
>   const struct usb_device_id *id;
>   struct usb_device *usb_dev;
>   struct usb_interface *intf;
> + struct list_head file_list;
>  
>   unsigned int bulk_in;
>   unsigned int bulk_out;
> @@ -87,12 +88,12 @@ struct usbtmc_device_data {
>   intiin_interval;
>   struct urb*iin_urb;
>   u16iin_wMaxPacketSize;
> - atomic_t   srq_asserted;
>  
>   /* coalesced usb488_caps from usbtmc_dev_capabilities */
>   __u8 usb488_caps;
>  
>   /* attributes from the USB TMC spec for this device */
> + /* They are used as default values for file_data */
>   u8 TermChar;
>   bool TermCharEnabled;
>   bool auto_abort;
> @@ -104,9 +105,26 @@ struct usbtmc_device_data {
>   struct mutex io_mutex;  /* only one i/o function running at a time */
>   wait_queue_head_t waitq;
>   struct fasync_struct *fasync;
> + spinlock_t dev_lock; /* lock for file_list */
>  };
>  #define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
>  
> +/*
> + * This structure holds private data for each USBTMC file handle.
> + */
> +struct usbtmc_file_data {
> + struct usbtmc_device_data *data;
> + struct list_head file_elem;
> +
> + u8 srq_byte;
> + atomic_t   srq_asserted;
> +
> + /* These values are initialized with default values from device_data */
> + u8 TermChar;
> + bool   TermCharEnabled;

I don't remember, does TermChar and TermCharEnabled come from a
specification somewhere?  Is that why they are in InterCaps format?

And why duplicate these fields as they are already in the
device-specific data structure?

> + bool   auto_abort;
> +};
> +
>  /* Forward declarations */
>  static struct usb_driver usbtmc_driver;
>  
> @@ -114,6 +132,7 @@ static void usbtmc_delete(struct kref *kref)
>  {
>   struct usbtmc_device_data *data = to_usbtmc_data(kref);
>  
> + pr_debug("%s - called\n", __func__);
>   usb_put_dev(data->usb_dev);
>   kfree(data);
>  }
> @@ -122,7 +141,7 @@ static int usbtmc_open(struct inode *inode, struct file 
> *filp)
>  {
>   struct usb_interface *intf;
>   struct usbtmc_device_data *data;
> - int retval = 0;
> + struct usbtmc_file_data *file_data;
>  
>   intf = usb_find_interface(&usbtmc_driver, iminor(inode));
>   if (!intf) {
> @@ -130,21 +149,54 @@ static int usbtmc_open(struct inode *inode, struct file 
> *filp)
>   return -ENODEV;
>   }
>  
> + file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
> + if (!file_data)
> + return -ENOMEM;
> +
> + pr_debug("%s - called\n", __func__);

Please do not add "tracing" functions like this.  The kernel has a
wonderful built-in function tracing functionality, don't try to write
your own.  These lines should just be removed.


> +
>   data = usb_get_intfdata(intf);
>   /* Protect reference to data from file structure until release */
>   kref_get(&data->kref);
>  
> + mutex_lock(&data->io_mutex);
> + file_data->data = data;
> +
> + /* copy default values from device settings */
> + file_data->TermChar = data->TermChar;
> + file_data->TermCharEnabled = data->TermCharEnabled;
> + file_data->auto_abort = data->auto_abort;
> +
> + INIT_LIST_HEAD(&file_data->file_elem);
> + spin_lock_irq(&data->dev_lock);
> + list_add_tail(&file_data->file_elem, &data->file_list);
> + spin_unlock_irq(&data->dev_lock);
> + mutex_unlock(&data->io_mutex);
> +
>   /* Store pointer in file structure's private data field */
> - filp->private_data = data;
> + filp->private_data = file_data;
>  
> - return retval;
> + return 0;
>  }
>  
>  static int usbtmc_release(struct inode *inode, struct file *file)
>  {
> - struct u

Re: [PATCH 04/12] usb: usbtmc: Add ioctls for trigger, EOM bit and TermChar

[Greg KH]

On Thu, May 17, 2018 at 07:03:28PM +0200, Guido Kiener wrote:
> - add USBTMC488_IOCTL_TRIGGER to send TRIGGER Bulk-OUT header
>   according to Subclass USB488 Specification
> 
>   The usbtmc trigger command is equivalent to the IEEE 488 GET (Group
>   Execute Trigger) action. While the "*TRG" command can be sent as
>   data to perform the same operation, in some situations an instrument
>   will be busy and unable to process the data immediately in which
>   case the USBTMC488_IOCTL_TRIGGER can be used to trigger the
>   instrument with lower latency.
> 
> - add USBTMC_IOCTL_EOM_ENABLE to specify EOM bit for next write()
>   call. Sets Bit 0 of field 'bmTransferAttributes' of DEV_DEP_MSG_OUT
>   Bulk-OUT Header.
> 
>   Allows fine grained control over end of message handling on a
>   per file descriptor basis.
> 
> - add USBTMC_IOCTL_CONFIG_TERMCHAR to control TermChar handling
>   for next read(). Controls field 'TermChar' and Bit 1 of field
>   'bmTransferAttributes' of REQUEST_DEV_DEP_MSG_IN BULK-OUT header.
> 
>   Allows enabling/disabling of terminating a read on reception of
>   term_char individually for each read request.

Why isn't this 3 patches?  Please only do "one thing per patch".

thanks,

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

Re: [PATCH 00/12] usb: usbtmc: Changes needed for compatible IVI/VISA library

[Greg KH]

On Thu, May 17, 2018 at 07:03:24PM +0200, Guido Kiener wrote:
> The working group "VISA for Linux" of the IVI Foundation
> www.ivifoundation.org specifies common rules, shared libraries and
> drivers to implement the specification of "VPP-4.3: The VISA Library"
> on Linux to be compatible with implementations on other operating systems.
> 
> The USBTMC protocol is part of the "VISA Library" that is used by many
> popular T&M applications.
> 
> An initial implementation for Linux based on libusb has been created.
> While functional it has some drawbacks:
> - Performance
> - Requires root privileges to reclaim devices already claimed by
>   the usbtmc driver
> 
> The following collaborative patches meet the requirements of the IVI
> Foundation to implement the library based on the usbtmc driver.
> 
> Improvements in the data transfer rate of over 130 MByte/s for
> usb 3.x connections have been measured.

Why is the libusb version "slower"?  Last I checked, we could reach
line-speeds for USB quite easily from userspace with no problem.  If you
keep the endpoints full you should be just fine.  Unless you have a
horrid protocol that doesn't allow for that :)

thanks,

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

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

Adding the network and NFC developers as this really is a NFC driver
bug, not a USB core issue...

On Thu, May 17, 2018 at 04:12:17PM +0200, Greg KH wrote:
> On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
> > Hello.
> > I'm having troubles with this NFC card reader. It seems kernel driver
> > pn533 is not working properly.
> > At this moment I have my work stalled. I need to add NFC support to a
> > software product and I can't get the device to work. NFC tools won't
> > work, the device is not detected.
> > 
> > This is what I get from "dmesg":
> > 
> > [4.182300] nfc: nfc_init: NFC Core ver 0.1
> > [4.182318] NET: Registered protocol family 39
> > [4.184676] hidraw: raw HID events driver (C) Jiri Kosina
> > [4.193366] [ cut here ]
> > [4.193366] transfer buffer not dma capable
> > [4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
> > usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > [4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
> > snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
> > snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
> > snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
> > snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
> > intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
> > snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
> > mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
> > ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
> > videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
> > snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
> > intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
> > intel_rapl_perf tpm_crb psmouse
> > [4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
> > wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
> > sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
> > intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
> > rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
> > ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
> > crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
> > serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
> > libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
> > mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
> > sysimgblt fb_sys_fops ttm drm agpgart
> > [4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 
> > 4.16.8-1-ARCH #1
> > [4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
> > PRO A50-C, BIOS Version 7.50   09/26/2016
> > [4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > [4.193468] RSP: 0018:a3b44282f9f8 EFLAGS: 00010282
> > [4.193469] RAX:  RBX: 981fc9e320c0 RCX: 
> > 0001
> > [4.193470] RDX: 8001 RSI: 0002 RDI: 
> > 
> > [4.193471] RBP: 981fd42f R08: 000713ed01d2 R09: 
> > 001f
> > [4.193472] R10: 0344 R11: f300 R12: 
> > 014000c0
> > [4.193473] R13: fff5 R14: 981fd2592b98 R15: 
> > c0410280
> > [4.193475] FS:  7f4fb98d0d40() GS:981fe6d0()
> > knlGS:
> > [4.193476] CS:  0010 DS:  ES:  CR0: 80050033
> > [4.193477] CR2: 562b4a68f6e8 CR3: 0004532d6004 CR4: 
> > 003606e0
> > [4.193478] Call Trace:
> > [4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
> > [4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
> > [4.193495]  ? __kmalloc+0x19e/0x220
> > [4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
> > [4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
> > [4.193511]  driver_probe_device+0x2b9/0x460
> > [4.193514]  ? __driver_attach+0xb6/0xe0
> > [4.193516]  ? driver_probe_device+0x460/0x460
> > [4.193518]  ? bus_for_each_dev+0x76/0xc0
> > [4.193520]  ? bus_add_driver+0x152/0x230
> > [4.193522]  ? driver_register+0x6b/0xb0
> > [4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
> > [4.193531]  ? 0xc13b6000
> > [4.193534]  ? do_one_initcall+0x48/0x13b
> > [4.193537]  ? free_unref_page_commit+0x6a/0x100
> > [4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
> > [4.193542]  ? do_init_module+0x5a/0x210
> > [4.193544]  ? load_module+0x247a/0x29f0
> > [4.193549]  ? SyS_init_module+0x139/0x180
> > [4.193550]  ? SyS_init_module+0x139/0x180
> > [4.193554]  ? do_syscall_64+0x74/0x190
> > [4.193556]  ? entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > [4.193559] Code: 49 39 c9 73 30 80 3d 7d b5 02 00 00 41 bd f5 ff
> > ff ff 0f 85 57 ff ff ff 48 c7 c7 88 9d 6e c0 c6 05 63 b5 02 00 01 e8
> > 97 85 9a ec <0f> 0b 8b 53 64 e9 3a 

Re: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

On Thu, May 17, 2018 at 06:40:04PM +0200, Greg KH wrote:
> Adding the network and NFC developers as this really is a NFC driver
> bug, not a USB core issue...
> 
> On Thu, May 17, 2018 at 04:12:17PM +0200, Greg KH wrote:
> > On Thu, May 17, 2018 at 02:10:57PM +0100, Carlos Manuel Santos wrote:
> > > Hello.
> > > I'm having troubles with this NFC card reader. It seems kernel driver
> > > pn533 is not working properly.
> > > At this moment I have my work stalled. I need to add NFC support to a
> > > software product and I can't get the device to work. NFC tools won't
> > > work, the device is not detected.
> > > 
> > > This is what I get from "dmesg":
> > > 
> > > [4.182300] nfc: nfc_init: NFC Core ver 0.1
> > > [4.182318] NET: Registered protocol family 39
> > > [4.184676] hidraw: raw HID events driver (C) Jiri Kosina
> > > [4.193366] [ cut here ]
> > > [4.193366] transfer buffer not dma capable
> > > [4.193398] WARNING: CPU: 2 PID: 259 at drivers/usb/core/hcd.c:1584
> > > usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > > [4.193399] Modules linked in: usbhid(+) pn533_usb(+) pn533 hid nfc
> > > snd_soc_skl(+) rtsx_usb_ms snd_soc_skl_ipc memstick snd_hda_ext_core
> > > snd_soc_sst_dsp snd_soc_sst_ipc ecdh_generic snd_soc_acpi snd_soc_core
> > > snd_hda_codec_realtek(+) snd_hda_codec_generic snd_compress ac97_bus
> > > snd_pcm_dmaengine arc4 intel_rapl x86_pkg_temp_thermal
> > > intel_powerclamp coretemp kvm_intel snd_hda_intel kvm iTCO_wdt
> > > snd_hda_codec iTCO_vendor_support iwlmvm i915 nls_iso8859_1 nls_cp437
> > > mac80211 vfat fat ppdev irqbypass crct10dif_pclmul crc32_pclmul
> > > ghash_clmulni_intel uvcvideo pcbc snd_hda_core iwlwifi
> > > videobuf2_vmalloc videobuf2_memops aesni_intel videobuf2_v4l2
> > > snd_hwdep aes_x86_64 crypto_simd glue_helper cryptd snd_pcm
> > > intel_cstate videobuf2_common e1000e intel_uncore snd_timer cfg80211
> > > intel_rapl_perf tpm_crb psmouse
> > > [4.193427]  videodev pcspkr input_leds intel_wmi_thunderbolt
> > > wmi_bmof ptp snd pps_core i2c_i801 soundcore toshiba_acpi mei_me media
> > > sparse_keymap toshiba_bluetooth mei intel_gtt industrialio
> > > intel_pch_thermal shpchp parport_pc tpm_tis tpm_tis_core battery
> > > rfkill parport evdev rtc_cmos mac_hid tpm rng_core ac sg crypto_user
> > > ip_tables x_tables rtsx_usb_sdmmc mmc_core rtsx_usb ext4
> > > crc32c_generic crc16 mbcache jbd2 fscrypto sr_mod cdrom sd_mod
> > > serio_raw atkbd libps2 ahci libahci xhci_pci xhci_hcd crc32c_intel
> > > libata usbcore scsi_mod usb_common i8042 serio nouveau led_class
> > > mxm_wmi wmi i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
> > > sysimgblt fb_sys_fops ttm drm agpgart
> > > [4.193458] CPU: 2 PID: 259 Comm: systemd-udevd Not tainted 
> > > 4.16.8-1-ARCH #1
> > > [4.193459] Hardware name: TOSHIBA SATELLITE PRO A50-C/SATELLITE
> > > PRO A50-C, BIOS Version 7.50   09/26/2016
> > > [4.193467] RIP: 0010:usb_hcd_map_urb_for_dma+0x413/0x570 [usbcore]
> > > [4.193468] RSP: 0018:a3b44282f9f8 EFLAGS: 00010282
> > > [4.193469] RAX:  RBX: 981fc9e320c0 RCX: 
> > > 0001
> > > [4.193470] RDX: 8001 RSI: 0002 RDI: 
> > > 
> > > [4.193471] RBP: 981fd42f R08: 000713ed01d2 R09: 
> > > 001f
> > > [4.193472] R10: 0344 R11: f300 R12: 
> > > 014000c0
> > > [4.193473] R13: fff5 R14: 981fd2592b98 R15: 
> > > c0410280
> > > [4.193475] FS:  7f4fb98d0d40() GS:981fe6d0()
> > > knlGS:
> > > [4.193476] CS:  0010 DS:  ES:  CR0: 80050033
> > > [4.193477] CR2: 562b4a68f6e8 CR3: 0004532d6004 CR4: 
> > > 003606e0
> > > [4.193478] Call Trace:
> > > [4.193488]  usb_hcd_submit_urb+0x38d/0xb20 [usbcore]
> > > [4.193492]  ? pn533_usb_probe+0x61/0x4d0 [pn533_usb]
> > > [4.193495]  ? __kmalloc+0x19e/0x220
> > > [4.193498]  pn533_usb_probe+0x397/0x4d0 [pn533_usb]
> > > [4.193507]  usb_probe_interface+0xe4/0x2f0 [usbcore]
> > > [4.193511]  driver_probe_device+0x2b9/0x460
> > > [4.193514]  ? __driver_attach+0xb6/0xe0
> > > [4.193516]  ? driver_probe_device+0x460/0x460
> > > [4.193518]  ? bus_for_each_dev+0x76/0xc0
> > > [4.193520]  ? bus_add_driver+0x152/0x230
> > > [4.193522]  ? driver_register+0x6b/0xb0
> > > [4.193530]  ? usb_register_driver+0x7a/0x130 [usbcore]
> > > [4.193531]  ? 0xc13b6000
> > > [4.193534]  ? do_one_initcall+0x48/0x13b
> > > [4.193537]  ? free_unref_page_commit+0x6a/0x100
> > > [4.193539]  ? kmem_cache_alloc_trace+0xdc/0x1c0
> > > [4.193542]  ? do_init_module+0x5a/0x210
> > > [4.193544]  ? load_module+0x247a/0x29f0
> > > [4.193549]  ? SyS_init_module+0x139/0x180
> > > [4.193550]  ? SyS_init_module+0x139/0x180
> > > [4.193554]  ? do_syscall_64+0x74/0x190
> > > [4.19

Re: [PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle

[Randy Dunlap]

On 05/17/2018 09:20 AM, Greg KH wrote:
>> +file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
>> +if (!file_data)
>> +return -ENOMEM;
>> +
>> +pr_debug("%s - called\n", __func__);
> Please do not add "tracing" functions like this.  The kernel has a
> wonderful built-in function tracing functionality, don't try to write
> your own.  These lines should just be removed.


but pr_debug() works great with DYNAMIC_DEBUG.

Someone does not need to go all the way to tracing to get decent
debug capability.


-- 
~Randy
--
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: ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't poweron...

[Greg KH]

On Thu, May 17, 2018 at 03:17:01PM +0100, Carlos Manuel Santos wrote:
> Thank you for you quick reply. I didn't have the opportunity to test
> with older kernels, I'm afraid.
> I never built a kernel patch but I can give it a try! :)

Let's drag this back on the mailing list, it's better that way, as I'm
not the only USB developer here.

Anyway, I would recommend building and booting your own kernel first,
before trying to apply a patch.  Try downloading the 4.16.8 (or newer)
kernel from kernel.org, use the config file from your currently running
kernel (it can be found in /proc/config.gz) just copy it to the root of
your kernel source directory, uncompress it, and rename it to .config.

Then build and install it "like normal".  There are loads of tutorials
online for how to do this, and a whole free book as well (search for
Linux Kernel in a Nutshell).

I'll go work on the patch now...

thanks,

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

Re: linux-next: Tree for May 17 (usb/gadget/udc/aspeed-vhub/)

[Randy Dunlap]

On 05/17/2018 12:06 AM, Stephen Rothwell wrote:
> 
> The usb-gadget tree gained a conflict against the usb tree.
> 


on x86_64:

drivers/usb/gadget/udc/aspeed-vhub/hub.o: In function 
`ast_vhub_std_hub_request':
hub.c:(.text+0x3a7): undefined reference to `usb_gadget_get_string'

CONFIG_LIB_USBCOMPOSITE is not enabled, hence usbstring.c is not built.


Full randconfig file is attached.

-- 
~Randy
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.17.0-rc5 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_ARCH_HAS_FILTER_PGPROT=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_CONSTRUCTORS=y
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y

#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
CONFIG_COMPILE_TEST=y
CONFIG_LOCALVERSION=""
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
CONFIG_KERNEL_XZ=y
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
# CONFIG_CROSS_MEMORY_ATTACH is not set
CONFIG_USELIB=y
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_SIM=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR=y
CONFIG_GENERIC_IRQ_RESERVATION_MODE=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
# CONFIG_GENERIC_IRQ_DEBUGFS is not set
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
# CONFIG_NO_HZ is not set
CONFIG_HIGH_RES_TIMERS=y

#
# CPU/Task time and stats accounting
#
CONFIG_VIRT_CPU_ACCOUNTING=y
# CONFIG_TICK_CPU_ACCOUNTING is not set
CONFIG_VIRT_CPU_ACCOUNTING_GEN=y
CONFIG_IRQ_TIME_ACCOUNTING=y
# CONFIG_BSD_PROCESS_ACCT is not set
CONFIG_TASKSTATS=y
# CONFIG_TASK_DELAY_ACCT is not set
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y
CONFIG_CPU_ISOLATION=y

#
# RCU Subsystem
#
CONFIG_TINY_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
CONFIG_TINY_SRCU=y
CONFIG_TASKS_RCU=y
CONFIG_CONTEXT_TRACKING=y
# CONFIG_CONTEXT_TRACKING_FORCE is not set
CONFIG_BUILD_BIN2C=y
# CONFIG_IKCONFIG is not set
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y
CONFIG_ARCH_SUPPORTS_INT128=y
# CONFIG_CGROUPS is not set
CONFIG_NAMESPACES=y
# CONFIG_UTS_NS is not set
# CONFIG_IPC_NS is not set
# CONFIG_USER_NS is not set
CONFIG_PID_NS=y
# CONFIG_NET_NS is not set
# CONFIG_SCHED_AUTOGROUP is not set
CONFIG_SYSFS_DEPRECATED=y
CONFIG_SYSFS_DEPRECATED_V2=y
CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE=""
# CONFIG_RD_GZIP is not set
# CONFIG_RD_BZIP2 is not set
# CONFIG_RD_LZMA is not set
CONFIG_RD_XZ=y
# CONFIG_RD_LZO is not set
# CONFIG_RD_LZ4 is not set
# CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_SYSCTL=y
CONFIG_ANON_INODES=y
CONFIG_HAVE_UID16=y
CONFIG_SYSCTL_EXCEPTION_TRACE=y
CONFIG_HAVE_PCSPKR_PLATFORM=y
CONFIG_BPF=y
CONFIG_EXPERT=y
CONFIG_UID16=y
CONFIG_MULTIUSER=y
CONFIG_SGETMASK_SYSCALL=y
CONFIG_SYSFS_SYSCALL=y
# CONFIG_SYSCTL_SYSCALL is not set
CONFIG_FHANDLE=y
CONFIG_POSIX_TIMERS=y
# CONFIG_PRINTK is not set
CONFIG_BUG=y
CONFIG_ELF_CORE=y
# CONFIG_PCSPKR_PLATFORM is not set
# CONFIG_BASE_FULL is not set
CONFIG_FUTEX=y
CONFIG_FUTEX_P

Re: [PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle

[Greg KH]

On Thu, May 17, 2018 at 09:44:40AM -0700, Randy Dunlap wrote:
> On 05/17/2018 09:20 AM, Greg KH wrote:
> >> +  file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
> >> +  if (!file_data)
> >> +  return -ENOMEM;
> >> +
> >> +  pr_debug("%s - called\n", __func__);
> > Please do not add "tracing" functions like this.  The kernel has a
> > wonderful built-in function tracing functionality, don't try to write
> > your own.  These lines should just be removed.
> 
> 
> but pr_debug() works great with DYNAMIC_DEBUG.

Sure, but don't do that for "I made it to this function!" type calls.
We have been ripping them out for a long time now, don't add new ones
please.

> Someone does not need to go all the way to tracing to get decent
> debug capability.

ftrace is not hard to use, it's even easier than dynamic_debug at times
:)

thanks,

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

Re: USB, Help, Keyboard being dropped

[ant]

Did I not say it right?

Or perhaps a kernel or some other issue?


=


(no need to CC I'm subscribed to list),

Hello,

  My USB keyboard is being dropped by the kernel at various times
and I cannot figure out how to fix this.  Maybe I have messed up
somewhere but I think I'm reading the documents right.  :)

  I'm using a recent Debian kernel and running Debian Testing/Unstable
but the kernel is coming from Experimental.

Linux ant 4.17.0-rc3-amd64 #1 SMP Debian 4.17~rc3-1~exp1 (2018-04-29) x86_64 
GNU/Linux

  And I've also tried kernel straight from Linus tree, doesn't seem
to be any different.

  I've also made sure the keyboard is plugged into various USB
ports and of versions both 2.0 and 3.1 and also tried changing
through the bios any legacy and handoff settings to verify that
they didn't solve this problem.

  I've captured logs from journalctl and udevadm which makes me
think that this is coming from the kernel (they are at the end).
If it helps I have a combined log from the console captured by
script command.

  I've tried setting kernel parameters for power control in
sys/bus/usb and sys/bus/hid to stop any autosuspend and I've
used the usbhid.quirks in my kernel boot parameters like:

net.ifnames=0 usbhid.quirks=0x17f6:0x0830:0x0400

  I've also made sure that udev has rules like:

ACTION=="add|change", SUBSYSTEM=="usb", ATTR{idVendor}=="17f6", 
ATTR{idProduct}=="0830", TEST=="power/control", ATTR{power/control}="on"
ACTION=="add|change", SUBSYSTEM=="usb", ATTR{idVendor}=="17f6", 
ATTR{idProduct}=="0830", TEST=="power/autosuspend_delay_ms", 
ATTR{power/autosuspend_delay_ms}="-1"

  and the following script shows they seem to be set:

-
echo -e "\nDev\tControl\tMs\tProduct\n"
for d in /sys/bus/usb/devices/[0-9]* ; do
  if [[ -e $d/product ]] ; then
echo -e "`basename $d`\t`cat $d/power/control`\t`cat 
$d/power/autosuspend_delay_ms`\t`cat $d/product`" ;
  fi ;
done

echo -e "\nDev\tControl\n"
for d in /sys/bus/hid/devices/[0-9]* ; do
  echo -e "`basename $d`\t`cat $d/power/control`" ;
done
-


Dev Control Ms  Product

1-12on  20002.4G Mouse
1-4 on  2000DeskJet 930C
1-5 on  2000ASC495 Speakers
1-6 on  -1  Ruffian6_x Kbrd v3_16
1-8 on  2000Cruzer Glide

Dev Control

0003:17F6:0830.0002 auto
0003:1EA7:0064.0001 auto


-

lsusb finds device fine:

Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 005: ID 0781:5575 SanDisk Corp. 
Bus 001 Device 007: ID 17f6:0830 Unicomp, Inc 
Bus 001 Device 003: ID 04d2:ff05 Altec Lansing Technologies ADA-305 Speakers
Bus 001 Device 002: ID 03f0:1204 Hewlett-Packard DeskJet 930c
Bus 001 Device 006: ID 1ea7:0064  
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub


lsmod says:

Module  Size  Used by
cpufreq_powersave  16384  0
cpufreq_userspace  16384  0
cpufreq_conservative16384  0
usblp  24576  0
snd_hda_codec_hdmi 57344  1
nls_ascii  16384  2
nls_cp437  20480  2
snd_hda_codec_realtek   106496  1
vfat   24576  2
intel_rapl 24576  0
snd_hda_codec_generic86016  1 snd_hda_codec_realtek
fat77824  1 vfat
x86_pkg_temp_thermal16384  0
intel_powerclamp   16384  0
snd_hda_intel  45056  3
kvm_intel 233472  0
snd_hda_codec 151552  4 
snd_hda_codec_generic,snd_hda_codec_hdmi,snd_hda_intel,snd_hda_codec_realtek
snd_usb_audio 229376  0
snd_usbmidi_lib36864  1 snd_usb_audio
kvm   724992  1 kvm_intel
snd_hda_core   94208  5 
snd_hda_codec_generic,snd_hda_codec_hdmi,snd_hda_intel,snd_hda_codec,snd_hda_codec_realtek
mxm_wmi16384  0
snd_rawmidi40960  1 snd_usbmidi_lib
irqbypass  16384  1 kvm
crct10dif_pclmul   16384  0
crc32_pclmul   16384  0
snd_seq_device 16384  1 snd_rawmidi
snd_hwdep  16384  2 snd_usb_audio,snd_hda_codec
snd_pcm   118784  5 
snd_hda_codec_hdmi,snd_hda_intel,snd_usb_audio,snd_hda_codec,snd_hda_core
ghash_clmulni_intel16384  0
i915 1708032  4
efi_pstore 16384  0
intel_cstate   16384  0
intel_uncore  131072  0
evdev  28672  6
snd_timer  36864  1 snd_pcm
mei_me 45056  0
snd94208  18 
snd_hda_codec_generic,snd_seq_device,snd_hda_codec_hdmi,snd_hwdep,snd_hda_intel,snd_usb_audio,snd_usbmidi_lib,snd_hda_codec,snd_hda_codec_realtek,snd_timer,snd_pcm,snd_rawmidi
iTCO_wdt   16384  0
drm_kms_helper200704  1 i915
joydev 24576  0
intel_rapl_perf16384  0
soundcore  16384  1 snd
iTCO_vendor_support16384  1 iTCO_wdt
pcspkr 16384  0
efivars  

Re: [PATCH] usbip: vhci_sysfs: fix potential Spectre v1

[Gustavo A. R. Silva]

Hi Greg,

On 05/17/2018 01:51 AM, Greg Kroah-Hartman wrote:

On Wed, May 16, 2018 at 05:22:00PM -0500, Gustavo A. R. Silva wrote:

pdev_nr and rhport can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.

This issue was detected with the help of Smatch:
drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential
spectre issue 'vhcis'
drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential
spectre issue 'vhcis'
drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential
spectre issue 'vhci->vhci_hcd_ss->vdev'
drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential
spectre issue 'vhci->vhci_hcd_hs->vdev'


Nit, no need to line-wrap long error messages from tools :)



Got it.


Fix this by sanitizing pdev_nr and rhport before using them to index
vhcis and vhci->vhci_hcd_ss->vdev respectively.

Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].

[1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2

Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva 
---
  drivers/usb/usbip/vhci_sysfs.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 4880838..9045888 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -10,6 +10,8 @@
  #include 
  #include 
  
+#include 

+
  #include "usbip_common.h"
  #include "vhci.h"
  
@@ -235,6 +237,8 @@ static ssize_t detach_store(struct device *dev, struct device_attribute *attr,

if (!valid_port(pdev_nr, rhport))
return -EINVAL;
  
+	pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers);

+   rhport = array_index_nospec(rhport, VHCI_HC_PORTS);


Shouldn't we just do this in one place, in the valid_port() function?

That way it keeps the range checking logic in one place (now it is in 3
places in the function), which should make maintenance much simpler.



Yep, I thought about that, the thing is: what happens if the hardware is 
"trained" to predict that valid_port always evaluates to false, and then 
malicious values are stored in pdev_nr and nhport?


It seems to me that under this scenario we need to serialize 
instructions in this place.


What do you think?

Thanks
--
Gustavo


--
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-storage: Add quirks to make G-Technology "G-Drive" work

[Alexander Kappner]

Oliver and Alan,

thank for investigating.

> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
> by itself would make the device work. Can you please test that?

US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, 
even with the patch you included applied:

[   44.108417] JBD2: Clearing recovery information on journal
[   44.119593] sd 2:0:0:0: [sda] tag#1 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[   44.121605] sd 2:0:0:0: [sda] tag#1 Sense Key : Illegal Request 
[current] 
[   44.123254] sd 2:0:0:0: [sda] tag#1 Add. Sense: Invalid field in cdb
[   44.124798] sd 2:0:0:0: [sda] tag#1 CDB: Write(16) 8a 08 00 00 00 00 e8 
c4 00 00 00 00 00 08 00 00
[   44.126847] print_req_error: critical target error, dev sda, sector 
3905159168
[   44.128450] Buffer I/O error on dev sda, logical block 488144896, lost 
sync page write
[   44.130776] JBD2: Error -5 detected when updating journal superblock for 
sda-8.
[   44.141059] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK 
driverbyte=DRIVER_SENSE
[   44.141376] sd 2:0:0:0: [sda] tag#0 Sense Key : Illegal Request 
[current] 
[   44.141376] sd 2:0:0:0: [sda] tag#0 Add. Sense: Invalid field in cdb
[   44.141376] sd 2:0:0:0: [sda] tag#0 CDB: Write(16) 8a 08 00 00 00 00 00 
00 00 00 00 00 00 08 00 00
[   44.141376] print_req_error: critical target error, dev sda, sector 0
[   44.141376] Buffer I/O error on dev sda, logical block 0, lost sync page 
write

> That's bizarre too.  Even though the only difference is a MODE SENSE 
> command, the command that actually faliled was WRITE(16).
It looks to me like the MODE SENSE simply hangs the drive, so anything 
issued after that will fail. Of course the drive says it's the "current 
command" that caused the failure, but I wouldn't give too much credence to 
that. FYI -- this device is a consumer grade rotational drive that you can 
get for less than $200, so I wouldn't be surprised if the implementations 
have issues. 

Also, I noticed that copying onto the drive with dd works fine, whereas 
trying to mount a filesystem immediately crashes it. I suspect this is 
because check_disk_change is called on mount (which eventually calls down 
to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag 
comes into play).



On 05/17/2018 05:58 AM, Oliver Neukum wrote:
> Am Donnerstag, den 17.05.2018, 01:15 -0700 schrieb Alexander Kappner:
>> Yes. Without this flag, the device keeps throwing similar errors on 
>> usb-storage. That's the same result I get on a host that doesn't have UAS 
>> compiled in. Here's a dmesg:
> 
> Hi,
> 
> this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
> by itself would make the device work. Can you please test that?
> You will need the attached patch for the quirk to be supported.
> 
>   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] usb-storage: Add quirks to make G-Technology "G-Drive" work

[Alan Stern]

On Thu, 17 May 2018, Alexander Kappner wrote:

> Oliver and Alan,
> 
> thank for investigating.
> 
> > this is suspicious. You do not actually whether US_FL_NO_WP_DETECT
> > by itself would make the device work. Can you please test that?
> 
> US_FL_NO_WP_DETECT without US_FL_IGNORE_UAS does not make a difference, 
> even with the patch you included applied:

Are you certain Oliver's new code was executed?  If you put 
US_FL_NO_WP_DETECT only in unusual_devs.h and not in ususual_uas.h then 
it would not affect the uas driver.

> > That's bizarre too.  Even though the only difference is a MODE SENSE 
> > command, the command that actually faliled was WRITE(16).
> It looks to me like the MODE SENSE simply hangs the drive, so anything 
> issued after that will fail. Of course the drive says it's the "current 
> command" that caused the failure, but I wouldn't give too much credence to 
> that. FYI -- this device is a consumer grade rotational drive that you can 
> get for less than $200, so I wouldn't be surprised if the implementations 
> have issues. 
> 
> Also, I noticed that copying onto the drive with dd works fine, whereas 
> trying to mount a filesystem immediately crashes it. I suspect this is 
> because check_disk_change is called on mount (which eventually calls down 
> to sd_read_write_protect_flag, which is where the US_FL_NO_WP_DETECT flag 
> comes into play).

Was this tested with uas or usb-storage?

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] usbip: vhci_sysfs: fix potential Spectre v1

[Greg Kroah-Hartman]

On Thu, May 17, 2018 at 12:57:49PM -0500, Gustavo A. R. Silva wrote:
> Hi Greg,
> 
> On 05/17/2018 01:51 AM, Greg Kroah-Hartman wrote:
> > On Wed, May 16, 2018 at 05:22:00PM -0500, Gustavo A. R. Silva wrote:
> > > pdev_nr and rhport can be controlled by user-space, hence leading to
> > > a potential exploitation of the Spectre variant 1 vulnerability.
> > > 
> > > This issue was detected with the help of Smatch:
> > > drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential
> > > spectre issue 'vhcis'
> > > drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential
> > > spectre issue 'vhcis'
> > > drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential
> > > spectre issue 'vhci->vhci_hcd_ss->vdev'
> > > drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential
> > > spectre issue 'vhci->vhci_hcd_hs->vdev'
> > 
> > Nit, no need to line-wrap long error messages from tools :)
> > 
> 
> Got it.
> 
> > > Fix this by sanitizing pdev_nr and rhport before using them to index
> > > vhcis and vhci->vhci_hcd_ss->vdev respectively.
> > > 
> > > Notice that given that speculation windows are large, the policy is
> > > to kill the speculation on the first load and not worry if it can be
> > > completed with a dependent load/store [1].
> > > 
> > > [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2
> > > 
> > > Cc: sta...@vger.kernel.org
> > > Signed-off-by: Gustavo A. R. Silva 
> > > ---
> > >   drivers/usb/usbip/vhci_sysfs.c | 6 ++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/usb/usbip/vhci_sysfs.c 
> > > b/drivers/usb/usbip/vhci_sysfs.c
> > > index 4880838..9045888 100644
> > > --- a/drivers/usb/usbip/vhci_sysfs.c
> > > +++ b/drivers/usb/usbip/vhci_sysfs.c
> > > @@ -10,6 +10,8 @@
> > >   #include 
> > >   #include 
> > > +#include 
> > > +
> > >   #include "usbip_common.h"
> > >   #include "vhci.h"
> > > @@ -235,6 +237,8 @@ static ssize_t detach_store(struct device *dev, 
> > > struct device_attribute *attr,
> > >   if (!valid_port(pdev_nr, rhport))
> > >   return -EINVAL;
> > > + pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers);
> > > + rhport = array_index_nospec(rhport, VHCI_HC_PORTS);
> > 
> > Shouldn't we just do this in one place, in the valid_port() function?
> > 
> > That way it keeps the range checking logic in one place (now it is in 3
> > places in the function), which should make maintenance much simpler.
> > 
> 
> Yep, I thought about that, the thing is: what happens if the hardware is
> "trained" to predict that valid_port always evaluates to false, and then
> malicious values are stored in pdev_nr and nhport?
> 
> It seems to me that under this scenario we need to serialize instructions in
> this place.
> 
> What do you think?

I don't understand, it should not matter where you put the barrier.  Be
it a function call back or right after it, it does the same thing, it
stops speculation from crossing that barrier.

So it _should_ work either way, if I understand the issue correctly.

If not, what am I missing?

thanks,

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

Re: [PATCH] usbip: vhci_sysfs: fix potential Spectre v1

[Gustavo A. R. Silva]

On 05/17/2018 02:15 PM, Greg Kroah-Hartman wrote:

Shouldn't we just do this in one place, in the valid_port() function?

That way it keeps the range checking logic in one place (now it is in 3
places in the function), which should make maintenance much simpler.



Yep, I thought about that, the thing is: what happens if the hardware is
"trained" to predict that valid_port always evaluates to false, and then
malicious values are stored in pdev_nr and nhport?

It seems to me that under this scenario we need to serialize instructions in
this place.

What do you think?


I don't understand, it should not matter where you put the barrier.  Be
it a function call back or right after it, it does the same thing, it
stops speculation from crossing that barrier.



Yeah. It makes sense.


So it _should_ work either way, if I understand the issue correctly.

If not, what am I missing?



No. It seems I'm the one who was missing something.

I'll place the barrier into valid_port and send v2 shortly.

Thanks!
--
Gustavo
--
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: Fwd: Enabling USB (auto)suspend for xHCI controllers incurs random device failures since kernel 4.15

[gert]

Hello

Thanks for your quick response.

I'll be away on holidays for the coming two weeks, but after that I'll 
certainly try taking the patch for a test drive for a couple of days 
(as the issue seemed intermittent).


I'll also notify the users on the Arch linux forum of the patch, as 
there were others experiencing the same issue. The more testers, the 
merrier!


Op dinsdag 15 mei 2018 om 9:33 schreef Mathias Nyman 
:

Hi


I didn't get any feedback about the suggested patch.


If you talk about this one 
https://marc.info/?l=linux-usb&m=15252835690

2325&w=2 then unfortunately I missed it.

Mathias, please confirm, is this patch I need to check on Dell 5855?



Ah, no, it's a different one:

https://marc.info/?l=linux-usb&m=152544392822763&w=2
also found in my for-usb-linus branch:

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
for-usb-linus

https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=for-usb-linus

-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

[PATCH v2] usbip: vhci_sysfs: fix potential Spectre v1

[Gustavo A. R. Silva]

pdev_nr and rhport can be controlled by user-space, hence leading to
a potential exploitation of the Spectre variant 1 vulnerability.

This issue was detected with the help of Smatch:
drivers/usb/usbip/vhci_sysfs.c:238 detach_store() warn: potential spectre issue 
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:328 attach_store() warn: potential spectre issue 
'vhcis'
drivers/usb/usbip/vhci_sysfs.c:338 attach_store() warn: potential spectre issue 
'vhci->vhci_hcd_ss->vdev'
drivers/usb/usbip/vhci_sysfs.c:340 attach_store() warn: potential spectre issue 
'vhci->vhci_hcd_hs->vdev'

Fix this by sanitizing pdev_nr and rhport before using them to index
vhcis and vhci->vhci_hcd_ss->vdev respectively.

Notice that given that speculation windows are large, the policy is
to kill the speculation on the first load and not worry if it can be
completed with a dependent load/store [1].

[1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2

Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 - Place the barriers into valid_port.

 drivers/usb/usbip/vhci_sysfs.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 4880838..69db0c9 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -10,6 +10,8 @@
 #include 
 #include 
 
+#include 
+
 #include "usbip_common.h"
 #include "vhci.h"
 
@@ -211,10 +213,14 @@ static int valid_port(__u32 pdev_nr, __u32 rhport)
pr_err("pdev %u\n", pdev_nr);
return 0;
}
+   pdev_nr = array_index_nospec(pdev_nr, vhci_num_controllers);
+
if (rhport >= VHCI_HC_PORTS) {
pr_err("rhport %u\n", rhport);
return 0;
}
+   rhport = array_index_nospec(rhport, VHCI_HC_PORTS);
+
return 1;
 }
 
-- 
2.7.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 v3 0/1] usb: gadget: udc: fsl: Introduce FSL_USB2_PHY_UTMI_DUAL

[Tiago Brusamarello]

Initialization for SoCs with dual role phy is being bypassed since
FSL_USB2_PHY_UTMI_DUAL macro is not being evaluated in the FSL gadget
driver. In this state a controller configured in peripheral mode will
not work as a gadget. This patch addresses this issue.

Tested on 4.14.32 using a hardware with the T1024 SoC.

Nikhil Badola (1):
  drivers: usb: Introduce FSL_USB2_PHY_UTMI_DUAL for usb gadget

 drivers/usb/gadget/udc/fsl_udc_core.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.12.0

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

[PATCH v3 1/1] drivers: usb: Introduce FSL_USB2_PHY_UTMI_DUAL for usb gadget

[Tiago Brusamarello]

From: Nikhil Badola 

Introduce FSL_USB2_PHY_UTMI_DUAL in gadget driver for setting
phy in SOCs with utmi dual phy

Signed-off-by: Nikhil Badola 
Tested-by: Tiago Brusamarello 
---

Changes since v2:
 * Reformatted the patch so it can be applied in the main tree

Changes since v1:
 * Removed Freescale internal information from commit message
 
 drivers/usb/gadget/udc/fsl_udc_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index 56b517a38865..a3c092b4db20 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -253,6 +253,7 @@ static int dr_controller_setup(struct fsl_udc *udc)
portctrl |= PORTSCX_PTW_16BIT;
/* fall through */
case FSL_USB2_PHY_UTMI:
+   case FSL_USB2_PHY_UTMI_DUAL:
if (udc->pdata->have_sysif_regs) {
if (udc->pdata->controller_ver) {
/* controller version 1.6 or above */
-- 
2.12.0

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

RE: [PATCH v3 1/1] drivers: usb: Introduce FSL_USB2_PHY_UTMI_DUAL for usb gadget

[Leo Li]

> -Original Message-
> From: Tiago Brusamarello [mailto:tbr...@gmail.com]
> Sent: Thursday, May 17, 2018 4:29 PM
> To: Leo Li 
> Cc: linux-usb@vger.kernel.org; Nikhil Badola 
> Subject: [PATCH v3 1/1] drivers: usb: Introduce FSL_USB2_PHY_UTMI_DUAL
> for usb gadget
> 
> From: Nikhil Badola 
> 
> Introduce FSL_USB2_PHY_UTMI_DUAL in gadget driver for setting phy in
> SOCs with utmi dual phy
> 
> Signed-off-by: Nikhil Badola 
> Tested-by: Tiago Brusamarello 

Acked-by: Li Yang 

> ---
> 
> Changes since v2:
>  * Reformatted the patch so it can be applied in the main tree
> 
> Changes since v1:
>  * Removed Freescale internal information from commit message
> 
>  drivers/usb/gadget/udc/fsl_udc_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
> b/drivers/usb/gadget/udc/fsl_udc_core.c
> index 56b517a38865..a3c092b4db20 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -253,6 +253,7 @@ static int dr_controller_setup(struct fsl_udc *udc)
>   portctrl |= PORTSCX_PTW_16BIT;
>   /* fall through */
>   case FSL_USB2_PHY_UTMI:
> + case FSL_USB2_PHY_UTMI_DUAL:
>   if (udc->pdata->have_sysif_regs) {
>   if (udc->pdata->controller_ver) {
>   /* controller version 1.6 or above */
> --
> 2.12.0

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

Re: [PATCH] NFC: pn533: don't send USB data off of the stack

[kbuild test robot]

Hi Greg,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Greg-KH/NFC-pn533-don-t-send-USB-data-off-of-the-stack/20180518-100416
config: i386-randconfig-x013-201819 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/nfc/pn533/usb.c: In function 'pn533_usb_send_ack':
>> drivers/nfc/pn533/usb.c:163:15: error: invalid operands to binary | (have 
>> 'struct urb *' and 'int')
 phy->out_urb |= URB_FREE_BUFFER;
  ^~
   drivers/nfc/pn533/usb.c: In function 'pn533_usb_send_frame':
   drivers/nfc/pn533/usb.c:180:15: error: invalid operands to binary & (have 
'struct urb *' and 'int')
 phy->out_urb &= ~URB_FREE_BUFFER;
  ^~
   drivers/nfc/pn533/usb.c: In function 'pn533_acr122_poweron_rdr':
   drivers/nfc/pn533/usb.c:406:15: error: invalid operands to binary | (have 
'struct urb *' and 'int')
 phy->out_urb |= URB_FREE_BUFFER;
  ^~

vim +163 drivers/nfc/pn533/usb.c

   147  
   148  static int pn533_usb_send_ack(struct pn533 *dev, gfp_t flags)
   149  {
   150  struct pn533_usb_phy *phy = dev->phy;
   151  static const u8 ack[6] = {0x00, 0x00, 0xff, 0x00, 0xff, 0x00};
   152  /* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
   153  char *buffer;
   154  int rc;
   155  
   156  buffer = kmalloc(sizeof(ack), GFP_KERNEL);
   157  if (!buffer)
   158  return -ENOMEM;
   159  memcpy(buffer, ack, sizeof(ack));
   160  
   161  phy->out_urb->transfer_buffer = (u8 *)ack;
   162  phy->out_urb->transfer_buffer_length = sizeof(ack);
 > 163  phy->out_urb |= URB_FREE_BUFFER;
   164  rc = usb_submit_urb(phy->out_urb, flags);
   165  
   166  return rc;
   167  }
   168  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH] usbserial: pl2303 tx xon/xoff flow control

[Florian Zumbiehl]

Hi,

> That looks like an HX device according to the current way we identify
> these types (which may be wrong).

That at least matches the labeling on the chip, so I guess that might be
correct?

> Continuation lines (e.g. the broken if statement) should be indented
> substantially to the right, which some subsystems interpret as at least
> two tabs further (you used just one).
> 
>   if (... &&
>   ...) {
>   return;
>   }

Ah, that's what is meant by that (i.e., to the right relative to the body,
not (just) the surrounding code).

> Depends on how you name your helper, I'd say. Another option could be to
> use an intermediate bool. Either way, the above is hardly readable and
> must be fixed.

I don't think a descriptive name would make up for the overhead of the
indirection as it necessarily still hides what exactly is being tested from
view, but doesn't add anything to readability as the basic function of the
condition is obvious from the code itself (a list of fields being checked
for changes from old to new).

But assigning the result to a variable inline seems fine to me as that
doesn't introduce any indirection overhead.

> > I don't know how to enable IXANY or change the control characters, so I am
> > working with what I do know (also, I guess the screenshot of the "ROM
> > programming tool" in the "datasheet" suggests that none of that is
> > supported).
> 
> Fair enough, that's why I asked. My device exhibits the same behaviour
> with regards to IXANY by the way.

I am not sure I understand. Are you saying your device exhibits IXANY
behaviour if you enable IXON with the patch applied? Mine definitely does
not, I just re-checked, it receives other data just fine, but only resumes
transmission when ^Q is received.

> > Now, I agree that signaling lack of support would be better in general, but
> > it does change what the code does for (still) unsupported cases. After all,
> > usbserial in general does not signal that it doesn't support IXON/IXANY,
> > but just pretends that it does. So, if there is a good reason why usbserial
> > ignores POSIX on this point, I would think that would still apply for the
> > remaining unsupported cases after partial support is implemented?!
> 
> There are historical reasons for a lot of things, but that's not
> necessarily a reason to continue taking shortcuts.

Well, sure, I just assumed that there was a *good* reason for why it is the
way it is, for lack of any information to the contrary, and so just
preserved the existing behaviour for cases that I wasn't obviously
improving.

> > However, in any case, the part of setting particular control characters
> > only when IXON is set seems like a very bad idea, in that it potentially
> > changes settings where no change was requested, so userspace code cannot
> > really be expected to check for that (and certainly not based on POSIX,
> > which explicitly states that other fields have to stay unchanged). So, if
> > we want to go with indicating lack of support, I think the correct strategy
> > would be to always force the control characters to what the hardware
> > supports.
> 
> You're right, but since the defaults termios control characters match
> what the device uses, you can still reset them whenever user space tries
> to change them to indicate lack of support (instead of silently
> disabling sw flow control while leaving IXON set).

Well, yeah, that's what I suggested--while you had suggested to do so only
when IXON was set, which would be problematic IMO. Making them completely
unchangeable should be fine I'd think.

Regards, Florian
--
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] usbserial: pl2303 tx xon/xoff flow control

[Florian Zumbiehl]

Hi,

> > There are historical reasons for a lot of things, but that's not
> > necessarily a reason to continue taking shortcuts.
> 
> But on second thought, I think your approach here makes sense. If
> usbserial ever gains generic IXON support, we'd just fall back to the
> line-discipline implementation whenever the control characters do no
> match.

Which wouldn't preclude indicating lack of support while the support is
lacking?! I mean, unless there is a good reason to pretend ...

(And if anything, the usb serial framework could use that indication to
recognize the lack of support in a given driver for a given configuration,
while with the current code there is no way to determine when a
generic/software implementation would have to be enabled?!)

> bools), and mention why you implemented things the way you did in the
> commit message.

Not sure what to mention there?! I mean, for the IXON/!IXANY/^S/^Q case, I
implemented things the way I did because that makes the hardware behave the
way that a serial interface should behave when IXON/!IXANY/^S/^Q is
configured, obviously. For all other cases, I have no clue why the
behaviour is the way it is, as I am just preserving existing behaviour that
was decided by others before me and the reasons for which are unknown to
me.

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

Re: [RFC PATCH 1/7] usb: typec: Generalize mux mode names

[Mats Karrman]

On 2018-05-17 13:50, Heikki Krogerus wrote:


On Wed, May 16, 2018 at 11:11:02PM +0200, Mats Karrman wrote:

On 05/16/2018 01:43 PM, Heikki Krogerus wrote:


On Tue, May 15, 2018 at 11:24:07PM +0200, Mats Karrman wrote:

Hi Heikki,

On 05/15/2018 09:30 AM, Heikki Krogerus wrote:


Hi Mats,

On Fri, May 11, 2018 at 09:28:04PM +0200, Mats Karrman wrote:

On 2018-05-11 13:14, Heikki Krogerus wrote:


On Fri, May 11, 2018 at 11:05:55AM +0200, Mats Karrman wrote:

On 2018-05-10 19:49, Heikki Krogerus wrote:


On Thu, May 10, 2018 at 10:04:21AM +0200, Mats Karrman wrote:

Hi,

On 05/09/2018 02:49 PM, Heikki Krogerus wrote:


On Tue, May 08, 2018 at 09:10:13PM +0200, Mats Karrman wrote:

Hi,

On 05/08/2018 04:25 PM, Heikki Krogerus wrote:


Hi,

On Mon, May 07, 2018 at 11:19:40PM +0200, Mats Karrman wrote:

Even so, when the mux driver "set" function is called, it will just get the
mode argument but since the mode (TYPEC_STATE_...) is overlapping for different
AMs if I understand your proposal correctly, the mux also needs to know what AM
is active.
Does this imply that the mux set function signature need to change?

My plan was actually to propose we get rid of the current mux handling
(just leave the orientation switch) in favour of the notifications I'm
introducing with the type-c bus for the alternate modes. The current
mux handling is definitely not enough, and does not work in every
scenario, like also you pointed out.

So, the mux need to subscribe to each svid:mode pair it is interested in using
typec_altmode_register_notifier() and then use those callbacks to switch the 
correct
signals to the connector. And a driver for an off-the-shelf mux device could 
have
the translation between svid:mode pairs and mux device specific control 
specified by
of/acpi properties. Right?

Yes. That is the plan. Would it work for you?

I think so. I'll give it a go. When about do you think you'll post the next 
version
of your RFC? Or do you have an updated series available somewhere public?

I'll try to put together and post the next version tomorrow.

My original plan was actually to use just the notifications with the
muxes, but one thing to consider with the notifications is that in
practice we have to increment the ref count for the alt mode devices
when ever something registers a notifier.

To me that does not feel ideal. The dependency should go the other way
around in case of the muxes. That is why I liked the separate API and
handling for the muxes felt better, as it will do just that. The mux
is then a "service" that the port driver can as for, and if it gets a
handle to a mux, the mux will have its ref count incremented.

So I think fixing the mux API would perhaps be better after all.
Thoughts?

So, we're back to a mux API similar to the current one, where:
- the port driver and the mux driver are connected through "graph"
- alt mode driver finds its port mux using the typec class mux api
- the mux mode setting function arguments now include both svid and mode

I like that.

One thought that popped up again is if we, somewhere down the line,
will see some super device that support many different alternate modes
on the same port and therefore will need to have multiple mux devices?
However I think the mux api could be extended (later on) to support some
aggregate mux device that manages multiple physical devices.

If we simply had always a mux for every alternate mode, that would not
be a problem. So the port would have its own mux, and every supported
alternate mode also had its own. I think that removes the need to deal
with the svid:mode when using the muxes, as they are already tied to a
specific alternate modes, right? With a single mux device, for example
pi3usb30532, the driver just needs to register a mux for the port and
separate mux for DP, but I don't think that's a huge problem.

Hmm... As an hypothetical example I have written a driver for another mux
from TI and according to its data sheet:

"""
The HD3SS460 is a generic analog differential
passive switch that can work for any high speed
interface applications as long as it is biased at a
common mode voltage range of 0-2V and has
differential signaling with differential amplitude up to
1800mVpp
"""

What I am thinking is that it e.g. would be possible to use this/a mux with 
USBSS +
2ch DP + 2ch something else (HDMI?, ThunderBolt?, ???). The problem here is
that it is a general mux device so the driver writer does not know what types of
muxes to register. I guess it could also be configured using properties but that
would be very complicated.

Why? All the mux driver needs to get from device properties is the
SVID and the mode.

Sigh... Again, if the same mux handles signals for more than one alternate mode
the driver won't know what alternate mode is intended if it only receives the
connector state which use overlapping numbers for different alternate modes.

You are missing the point. We are now registering separate struct
typec_mux for eve

Re: [RFC PATCH linux-next] USB: dwc3: dwc3_get_extcon() can be static

[Andrzej Hajda]

On 17.05.2018 18:06, kbuild test robot wrote:
> Fixes: 5f0b74e54890 ("USB: dwc3: get extcon device by OF graph bindings")
> Signed-off-by: kbuild test robot 

It should be static of course, my bad.

Reviewed-by: Andrzej Hajda 

 --
Regards
Andrzej

> ---
>  drd.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 2706824..218371f 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -440,7 +440,7 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
>   return NOTIFY_DONE;
>  }
>  
> -struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
> +static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
>  {
>   struct device *dev = dwc->dev;
>   struct device_node *np_phy, *np_conn;
>
>
>

--
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] ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: fix memdup.cocci warnings

[Julia Lawall]

From: kbuild test robot 

 Use kmemdup rather than duplicating its implementation

Generated by: scripts/coccinelle/api/memdup.cocci

Fixes: fe1c559e1567 ("ACS ACR122U not working: pn533_usb 1-1:1.0: NFC: Couldn't 
poweron...")
CC: Greg KH 
Signed-off-by: kbuild test robot 
Signed-off-by: Julia Lawall 

---

url:
https://github.com/0day-ci/linux/commits/Greg-KH/NFC-pn533-don-t-send-USB-data-off-of-the-stack/20180518-100416
:: branch date: 54 minutes ago
:: commit date: 54 minutes ago

 usb.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -153,10 +153,9 @@ static int pn533_usb_send_ack(struct pn5
char *buffer;
int rc;

-   buffer = kmalloc(sizeof(ack), GFP_KERNEL);
+   buffer = kmemdup(ack, sizeof(ack), GFP_KERNEL);
if (!buffer)
return -ENOMEM;
-   memcpy(buffer, ack, sizeof(ack));

phy->out_urb->transfer_buffer = (u8 *)ack;
phy->out_urb->transfer_buffer_length = sizeof(ack);
@@ -390,10 +389,9 @@ static int pn533_acr122_poweron_rdr(stru

dev_dbg(&phy->udev->dev, "%s\n", __func__);

-   buffer = kmalloc(sizeof(cmd), GFP_KERNEL);
+   buffer = kmemdup(cmd, sizeof(cmd), GFP_KERNEL);
if (!buffer)
return -ENOMEM;
-   memcpy(buffer, cmd, sizeof(cmd));

init_completion(&arg.done);
cntx = phy->in_urb->context;  /* backup context */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: USB, Help, Keyboard being dropped

[Greg KH]

On Thu, May 17, 2018 at 01:43:35PM -0400, ant wrote:
> 
> Did I not say it right?
> 
> Or perhaps a kernel or some other issue?
> 
> 
> =
> 
> 
> (no need to CC I'm subscribed to list),
> 
> Hello,
> 
>   My USB keyboard is being dropped by the kernel at various times
> and I cannot figure out how to fix this.  Maybe I have messed up
> somewhere but I think I'm reading the documents right.  :)
> 
>   I'm using a recent Debian kernel and running Debian Testing/Unstable
> but the kernel is coming from Experimental.
> 
> Linux ant 4.17.0-rc3-amd64 #1 SMP Debian 4.17~rc3-1~exp1 (2018-04-29) x86_64 
> GNU/Linux
> 
>   And I've also tried kernel straight from Linus tree, doesn't seem
> to be any different.
> 
>   I've also made sure the keyboard is plugged into various USB
> ports and of versions both 2.0 and 3.1 and also tried changing
> through the bios any legacy and handoff settings to verify that
> they didn't solve this problem.
> 
>   I've captured logs from journalctl and udevadm which makes me
> think that this is coming from the kernel (they are at the end).
> If it helps I have a combined log from the console captured by
> script command.
> 

What do you mean exactly by "dropped"?  The kernel log you posted
doesn't show any USB devices going away, so it's a bit hard to know if
the kernel really thought the device is gone, or the device just stopped
working, right?

Does this keyboard work properly on other machines / operating systems?

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