Re: [PATCH 1/3] USB: serial: use static attribute groups for sysfs entries

[Takashi Iwai]

At Sun, 15 Feb 2015 14:25:05 +0700,
Johan Hovold wrote:
 
 On Thu, Feb 05, 2015 at 02:36:23PM +0100, Takashi Iwai wrote:
 
   static int usb_serial_device_probe(struct device *dev)
   {
  struct usb_serial_driver *driver;
  @@ -72,15 +79,9 @@ static int usb_serial_device_probe(struct device *dev)
  goto exit_with_autopm;
  }
   
  -   retval = device_create_file(dev, dev_attr_port_number);
  -   if (retval) {
  -   if (driver-port_remove)
  -   retval = driver-port_remove(port);
  -   goto exit_with_autopm;
  -   }
  -
  minor = port-minor;
  -   tty_register_device(usb_serial_tty_driver, minor, dev);
  +   tty_register_device_attr(usb_serial_tty_driver, minor, dev, NULL,
  +usb_serial_dev_groups);
 
 This is just wrong. You're moving a usb-serial-port device attribute to
 the tty class device (i.e. ABI breakage) and you're not updating the
 attribute operation either.
 
 I assume you haven't tested this, as the kernel would likely crash or,
 if you're lucky, just return a bogus port number when you try to read
 the attribute file.

Hm, right, drop this one.  Sorry for the noise.


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

Re: [PATCH 2/3] USB: serial: Handle the error from tty_register_device*()

[Takashi Iwai]

At Sun, 15 Feb 2015 14:32:39 +0700,
Johan Hovold wrote:
 
 On Thu, Feb 05, 2015 at 02:36:24PM +0100, Takashi Iwai wrote:
 
 No commit message?
 
  Signed-off-by: Takashi Iwai ti...@suse.de
  ---
   drivers/usb/serial/bus.c | 11 +--
   1 file changed, 9 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
  index b730366d524f..4b34afc51535 100644
  --- a/drivers/usb/serial/bus.c
  +++ b/drivers/usb/serial/bus.c
  @@ -80,8 +80,15 @@ static int usb_serial_device_probe(struct device *dev)
  }
   
  minor = port-minor;
  -   tty_register_device_attr(usb_serial_tty_driver, minor, dev, NULL,
  -usb_serial_dev_groups);
  +   dev = tty_register_device_attr(usb_serial_tty_driver, minor, dev, NULL,
  +  usb_serial_dev_groups);
 
 Please add a second struct device *tdev for the tty device, rather than
 reuse dev here.
 
  +   if (IS_ERR(dev)) {
  +   retval = PTR_ERR(dev);
  +   if (driver-port_remove)
  +   driver-port_remove(port);
  +   goto exit_with_autopm;
  +   }
  +
  dev_info(port-serial-dev-dev,
   %s converter now attached to ttyUSB%d\n,
   driver-description, minor);
 
 Looks good otherwise.
 
 Can you resend this one separately after you drop patch 1/3? Make sure
 you remove the port device attribute in the error path as well after you
 rebase.

If more change is needed, I leave this rather to you guys to work on :)


thanks,

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

[PATCH 1/1] usb: common: otg-fsm: only signal connect after switching to peripheral

[Peter Chen]

We should signal connect (pull up dp) after we have already
at peripheral mode, otherwise, the dp may be toggled due to
we reset controller or do disconnect during the initialization
for peripheral, then, the host may be confused during the
enumeration, eg, it finds the reset can't succeed, but the
device is still there, see below error message.

hub 1-0:1.0: USB hub found
hub 1-0:1.0: 1 port detected
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: cannot reset port 1 (err = -32)
hub 1-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
hub 1-0:1.0: unable to enumerate USB device on port 1

Signed-off-by: Peter Chen peter.c...@freescale.com
---
 drivers/usb/common/usb-otg-fsm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
index df8a14b..0cc68b5 100644
--- a/drivers/usb/common/usb-otg-fsm.c
+++ b/drivers/usb/common/usb-otg-fsm.c
@@ -154,9 +154,9 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
usb_otg_state new_state)
break;
case OTG_STATE_B_PERIPHERAL:
otg_chrg_vbus(fsm, 0);
-   otg_loc_conn(fsm, 1);
otg_loc_sof(fsm, 0);
otg_set_protocol(fsm, PROTO_GADGET);
+   otg_loc_conn(fsm, 1);
break;
case OTG_STATE_B_WAIT_ACON:
otg_chrg_vbus(fsm, 0);
@@ -219,10 +219,10 @@ static int otg_set_state(struct otg_fsm *fsm, enum 
usb_otg_state new_state)
 
break;
case OTG_STATE_A_PERIPHERAL:
-   otg_loc_conn(fsm, 1);
otg_loc_sof(fsm, 0);
otg_set_protocol(fsm, PROTO_GADGET);
otg_drv_vbus(fsm, 1);
+   otg_loc_conn(fsm, 1);
otg_add_timer(fsm, A_BIDL_ADIS);
break;
case OTG_STATE_A_WAIT_VFALL:
-- 
1.9.1

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

Re: [PATCH v3 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core

[Sneeker Yeh]

Hi Alan:

thanks for comment it,
and sorry that a little bit late for replying,

2015-02-12 23:18 GMT+08:00 Alan Stern st...@rowland.harvard.edu:
 On Thu, 12 Feb 2015, Mathias Nyman wrote:

 On 25.01.2015 10:13, Sneeker Yeh wrote:
  This issue is defined by a three-way race at disconnect, between
  1) Class driver interrupt endpoint resheduling attempts if the ISR gave an 
  ep
 error event due to device detach (it would try 3 times)
  2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
 asynchronously
  3) The hardware IP was configured in silicon with
 - DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
 - Synopsys IP version is  3.00a
  The IP will auto-suspend itself on device detach with some phy-specific 
  interval
  after CSC is cleared by 2)
 
  If 2) and 3) complete before 1), the interrupts it expects will not be 
  generated
  by the autosuspended IP, leading to a deadlock. Even later disconnection
  procedure would detect that corresponding urb is still in-progress and 
  issue a
  ep stop command, auto-suspended IP still won't respond to that command.

 If the Synopsys IP provides a way to do it, it would be better to turn
 off the autosuspend feature entirely.  Doesn't autosuspend violate the
 xHCI specification?

it's an IP parameter that can't be turn off via any register .

I guess Synopsis can insisted that xHCI specification doesn't
explicitly state that hardware designer shouldn't do auto-suspend when
device is attached, especially that definition of the auto-suspend is
their own specific low power state.
IIRC, this is point of view from Synopsis.

I wonder maybe they just didn't have a good assumption when
implementing auto-suspend, e.g. they shouldn't take PORTCSC clearing
as a commitment that software has done all the device detach task, so
that IP can go into suspend. it seems more reasonable to take
slot-disabling as a commitment to a start of auto-suspend.

anyway the errata would be disclosed recently, and Synopsis plan to
fix it in their new silicon version IP.
Old version IP has to live happily with some patch that can workaround
this monster i think.

Much appreciate,
Sneeker


 So did I understand correctly that the class driver submits a new urb which
 is enqueued by xhci_urb_enqueue() before the hub thread notices the device 
 is disconnected.
 Then hub thread clears CSC bit, controller suspends and the new urb is never 
 given back?

 Doesn't the CSC bit and PORT_CONNECT bit show the device is disconnected 
 when we enter
 xhci_enqueue_urb(), even it the hub thread doesn't know this yet?

 What if the device disconnects _after_ the new URB is enqueued?

 Would it make sense to check those bits in xhci_enqueue_urb, and just return 
 error
 in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be 
 a need for any quirk
 at all.

 That wouldn't help URBs that were already enqueued when the disconnect
 occurred.

 Alan Stern

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

RE: Usb serial driver change question in the linux kernel

[Li, Elvin]

Hi Johan,
Your patch fixed my issue. Thank you for support!

Regards
Elvin Li

-Original Message-
From: Johan Hovold [mailto:jhov...@gmail.com] On Behalf Of Johan Hovold
Sent: Sunday, February 15, 2015 1:16 PM
To: Li, Elvin
Cc: linux-usb@vger.kernel.org; linux-ser...@vger.kernel.org; Tian, Feng; Ni, 
Ruiyu; g...@kroah.com; jhov...@gmail.com
Subject: Re: Usb serial driver change question in the linux kernel

On Sun, Feb 15, 2015 at 12:43:51AM +, Li, Elvin wrote:
 Hi All,
 I am using usb_debug driver in Linux kernel  to manage
 Ajays USB debug device (Vendor Id: 0525 Product Id:
 127a) which follows EHCI debug device specification.
 drivers\usb\serial\usb_debug.c was developed by Greg
 Kroah-Hartman, it worked very well with Ajays debug
 device until one change to usb-serial.c. I could see
 the Johan's change at
 
 https://github.com/torvalds/linux/commit/5083fd7bdfe6760577235a724cf6dccae13652c2.

 Does anyone has ideas how the issue could be resolved?

Thanks for the report. The commit mentioned above should simply be reverted; a 
bulk-out buffer size smaller than the end-point size is indeed valid.

Could you verify that the patch below fixes the issues you're seeing?

Thanks,
Johan


From fffe54908863974c20adf1b1bf4e24e35a8d921b Mon Sep 17 00:00:00 2001
From: Johan Hovold jo...@kernel.org
Date: Sun, 15 Feb 2015 11:57:53 +0700
Subject: [PATCH] Revert USB: serial: make bulk_out_size a lower limit

This reverts commit 5083fd7bdfe6760577235a724cf6dccae13652c2.

A bulk-out size smaller than the end-point size is indeed valid. The offending 
commit broke the usb-debug driver for EHCI debug devices, which use 8-byte 
buffers.

Fixes: 5083fd7bdfe6 (USB: serial: make bulk_out_size a lower limit)
Reported-by: Li, Elvin elvin...@intel.com
Cc: stable sta...@vger.kernel.org # v3.15
Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/usb/serial/usb-serial.c | 5 +++--
 include/linux/usb/serial.h  | 3 +--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c 
index 475723c006f9..19842370a07f 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -940,8 +940,9 @@ static int usb_serial_probe(struct usb_interface *interface,
port = serial-port[i];
if (kfifo_alloc(port-write_fifo, PAGE_SIZE, GFP_KERNEL))
goto probe_error;
-   buffer_size = max_t(int, serial-type-bulk_out_size,
-   usb_endpoint_maxp(endpoint));
+   buffer_size = serial-type-bulk_out_size;
+   if (!buffer_size)
+   buffer_size = usb_endpoint_maxp(endpoint);
port-bulk_out_size = buffer_size;
port-bulk_out_endpointAddress = endpoint-bEndpointAddress;
 
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index 
9bb547c7bce7..704a1ab8240c 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -190,8 +190,7 @@ static inline void usb_set_serial_data(struct usb_serial 
*serial, void *data)
  * @num_ports: the number of different ports this device will have.
  * @bulk_in_size: minimum number of bytes to allocate for bulk-in buffer
  * (0 = end-point size)
- * @bulk_out_size: minimum number of bytes to allocate for bulk-out buffer
- * (0 = end-point size)
+ * @bulk_out_size: bytes to allocate for bulk-out buffer (0 = end-point 
+ size)
  * @calc_num_ports: pointer to a function to determine how many ports this
  * device has dynamically.  It will be called after the probe()
  * callback is called, but before attach()
--
2.0.5

--
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/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core

[Sneeker Yeh]

hi Mathias:

thanks for reviewing these patch,
and sorry for replying lately~

2015-02-12 21:50 GMT+08:00 Mathias Nyman mathias.ny...@intel.com:
 On 25.01.2015 10:13, Sneeker Yeh wrote:
 This issue is defined by a three-way race at disconnect, between
 1) Class driver interrupt endpoint resheduling attempts if the ISR gave an ep
error event due to device detach (it would try 3 times)
 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
asynchronously
 3) The hardware IP was configured in silicon with
- DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
- Synopsys IP version is  3.00a
 The IP will auto-suspend itself on device detach with some phy-specific 
 interval
 after CSC is cleared by 2)

 If 2) and 3) complete before 1), the interrupts it expects will not be 
 generated
 by the autosuspended IP, leading to a deadlock. Even later disconnection
 procedure would detect that corresponding urb is still in-progress and issue 
 a
 ep stop command, auto-suspended IP still won't respond to that command.


 So did I understand correctly that the class driver submits a new urb which
 is enqueued by xhci_urb_enqueue() before the hub thread notices the device is 
 disconnected.
 Then hub thread clears CSC bit, controller suspends and the new urb is never 
 given back?

yes.


 Doesn't the CSC bit and PORT_CONNECT bit show the device is disconnected when 
 we enter
 xhci_enqueue_urb(), even it the hub thread doesn't know this yet?

 Would it make sense to check those bits in xhci_enqueue_urb, and just return 
 error
 in the xhci_urb_enqueue() if device is not connected? Then there wouldn't be 
 a need for any quirk
 at all.

ya I tried this before, it would work if i stop enqueue when i found
device detached,
but sometime it won't work when device might be detached just right
after i done enqueue, just like Alan mentioned.


 If that doesn't work then this patch looks good in general. See comments below

 this defect would result in this when device detached:
 ---
 [   99.603544] usb 4-1: USB disconnect, device number 2
 [  104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop 
 endpoint command.
 [  104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting 
 host.
 [  104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000 
 microseconds.
 [  104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not 
 halting.
 [  104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
 [  104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
 --
 As a result, when device detached, we desired to postpone PORTCSC clear 
 behind
 disable slot. it's found that all executed ep command related to 
 disconnetion,
 are executed before disable slot.

 Signed-off-by: Sneeker Yeh sneeker@tw.fujitsu.com
 ---
  drivers/usb/host/xhci-hub.c |4 
  drivers/usb/host/xhci.c |   29 +
  drivers/usb/host/xhci.h |   24 
  3 files changed, 57 insertions(+)

 diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
 index a7865c4..3b8f7fc 100644
 --- a/drivers/usb/host/xhci-hub.c
 +++ b/drivers/usb/host/xhci-hub.c
 @@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd 
 *xhci, u16 wValue,
   port_change_bit = warm(BH) reset;
   break;
   case USB_PORT_FEAT_C_CONNECTION:
 + if ((xhci-quirks  XHCI_DISCONNECT_QUIRK) 
 + !(readl(addr)  PORT_CONNECT))
 + return;
 +

 New port status event are prevented until CSC is cleared, not sure if there's 
 any harm in that?

hub thread would be aware of device detach before he try to clear PORTCSC, IIUC,
Despite I skip clearing PORTCSC here, whole device detach procedure
still behave normally.


   status = PORT_CSC;
   port_change_bit = connect;
   break;
 diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
 index c50d8d2..aa8e02a 100644
 --- a/drivers/usb/host/xhci.c
 +++ b/drivers/usb/host/xhci.c
 @@ -3584,6 +3584,33 @@ command_cleanup:
   return ret;
  }

 +static void xhci_try_to_clear_csc(struct usb_hcd *hcd, int dev_port_num)
 +{
 + int max_ports;
 + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 + __le32 __iomem **port_array;
 + u32 status;
 +
 + /* print debug info */
 + if (hcd-speed == HCD_USB3) {
 + max_ports = xhci-num_usb3_ports;
 + port_array = xhci-usb3_ports;
 + } else {
 + max_ports = xhci-num_usb2_ports;
 + port_array = xhci-usb2_ports;
 + }
 +
 + if (dev_port_num  max_ports) {
 + xhci_err(xhci, %s() port number invalid, __func__);
 + return;
 + }
 + status = readl(port_array[dev_port_num - 1]);
 +
 + /* write 1 to clear */
 + if (!(status  PORT_CONNECT)  

[GIT PULL] USB driver patches for 3.20-rc1

[Greg KH]

The following changes since commit e36f014edff70fc02b3d3d79cead1d58f289332e:

  Linux 3.19-rc7 (2015-02-01 20:07:21 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ 
tags/usb-3.20-rc1

for you to fetch changes up to 4d4bac4499e9955521af80198063ef9c2f2bd634:

  Merge tag 'usb-for-v3.20' of 
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next 
(2015-02-04 11:03:20 -0800)


USB patches for 3.20-rc1

Here's the big pull request for the USB driver tree for 3.20-rc1.

Nothing major happening here, just lots of gadget driver updates, new
device ids, and a bunch of cleanups.

All of these have been in linux-next for a while with no reported
issues.

Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org


Alan Stern (4):
  USB: don't cancel queued resets when unbinding drivers
  USB: usbfs: allow URBs to be reaped after disconnection
  USB: add flag for HCDs that can't receive wakeup requests (isp1760-hcd)
  USB: fix use-after-free bug in usb_hcd_unlink_urb()

Amit Virdi (2):
  usb: dwc3: gadget: Remove redundant check
  usb: dwc3: Remove current_trb as it is unused

Andreas Herrmann (2):
  USB: host: Remove hard-coded octeon platform information for ehci/ohci
  USB: host: Introduce flag to enable use of 64-bit dma_mask for 
ehci-platform

Andrzej Pietrasiewicz (26):
  Documentation: usb: gadget_serial: update generic serial setup instruction
  Documentation: usb: ACM function testing
  Documentation: usb: ECM function testing
  Documentation: usb: ECM subset function testing
  Documentation: usb: EEM function testing
  Documentation: usb: FFS function testing
  Documentation: usb: HID function testing
  Documentation: usb: LOOPBACK function testing
  Documentation: usb: MASS STORAGE function testing
  Documentation: usb: MIDI function testing
  Documentation: usb: NCM function testing
  Documentation: usb: OBEX function testing
  Documentation: usb: PHONET function testing
  Documentation: usb: RNDIS function testing
  Documentation: usb: SERIAL function testing
  Documentation: usb: SOURCESINK function testing
  Documentation: usb: UAC1 function testing
  Documentation: usb: UAC2 function testing
  Documentation: usb: UVC function testing
  usb: gadget: hid: consistently use 2^n - 1 for max values
  usb: gadget: f_uvc: rename a macro to avoid conflicts
  usb: gadget: uvc: verify descriptors presence
  usb: gadget: uvc: configfs support in uvc function
  usb: gadget: uvc: preserve the address passed to kfree()
  usb: gadget: uvc: use explicit type instead of void *
  usb: gadget: uvc: comments for iterating over streaming hierarchy

Andy Shevchenko (6):
  USB: use %*ph specifier in mikrotek driver
  USB: use %*ph specifier in uss720 driver
  usb: gadget: ethernet: re-use %pM specifier to print MAC
  usb: host: pci_quirks: joing string literals
  ehci-pci: disable for Intel MID platforms
  ehci-pci: disable for Intel MID platforms (update)

Arnd Bergmann (3):
  usb: musb: add generic usb phy dependencies
  usb: musb: add omap-control dependency
  usb: dwc2: fix USB core dependencies

Arun Ramamurthy (2):
  usb: ohci-platform: add support for multiple phys per controller
  usb: ehci-platform: add support for multiple phys per controller

Asaf Vertz (2):
  usb: host: max3421-hcd: use time_after()
  usb: gadget: zero: fix format string warnings

Bin Liu (3):
  usb: musb: cppi41: correct the macro name EP_MODE_AUTOREG_*
  usb: musb: cppi41: improve rx channel abort routine
  usb: musb: fix device hotplug behind hub

Boris Brezillon (4):
  usb: atmel_usba_udc: Rework at91sam9rl errata handling
  usb: atmel_usba_udc: Add at91sam9g45 and at91sam9x5 errata handling
  usb: atmel_usba_udc: Mask status with enabled irqs
  usb: gadget: atmel_usba: Cache INT_ENB register value

Chris Rorvick (1):
  usb: Fix typo in `struct usb_host_interface' comment

Christoph Jaeger (1):
  usb: gadget: Kconfig: use bool instead of boolean

Colin Ian King (1):
  USB: mos7840: remove unused code

Dan Carpenter (6):
  usb: gadget: udc: clean up a printk
  usb: gadget: udc: remove bogus NULL check
  usb: gadget: uvc: fix some error codes
  usb: gadget: uvc: remove an impossible condition
  usb: gadget: uvc: memory leak in uvcg_frame_make()
  usb: gadget: uvc: cleanup UVCG_FRAME_ATTR macro

Deepak Das (1):
  usb: core: hub: modify hub reset logic in hub driver

Dmitry Torokhov (1):
  usb: musb: blackfin: remove incorrect __exit_p()

Fabio Estevam (2):
  Documentation: usb: phy: nop: Fix the description of 'vcc-supply'
  usb: phy: phy-generic: Fix USB PHY gpio reset

Re: [PATCH 1/1] MAINTAINERS: add entry for USB OTG FSM

[Greg KH]

On Sun, Feb 15, 2015 at 12:22:59PM +0800, Peter Chen wrote:
 Add MAINTAINER entry for USB OTG Finite State Machine
 
 Cc: Felipe Balbi ba...@ti.com
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  MAINTAINERS | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/MAINTAINERS b/MAINTAINERS
 index f25de35..bcd6b6b 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -10021,6 +10021,13 @@ S:   Maintained
  F:   Documentation/usb/ohci.txt
  F:   drivers/usb/host/ohci*
  
 +USB OTG FSM (Finite State Machine)
 +M:   Peter Chen peter.c...@freescale.com
 +T:   git git://github.com/hzpeterchen/linux-usb.git


I'll queue this up, but you might want to think about getting a
kernel.org account as I can't ever take pull requests from github.com.
And your end-goal is to have me be able to trust you enough to take pull
requests, right?  {hint}

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

[PATCH v3] usb: dwc2: gadget reuse ahbcfg assigned from platform

[Zhangfei Gao]

Reuse ahbcfg if assigned from platform

Input from John:
AHB_SINGLE, NOTI_ALL_DMA_WRIT, REM_MEM_SUPP, HBSTLEN,
and INV_DESC_ENDIANNESS only apply in DMA mode and are
ignored in slave mode operation.

Signed-off-by: Zhangfei Gao zhangfei@linaro.org
---
 drivers/usb/dwc2/gadget.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 15aa578..5726fbe 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2314,14 +2314,19 @@ void s3c_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
GINTSTS_USBSUSP | GINTSTS_WKUPINT,
hsotg-regs + GINTMSK);
 
+   if (hsotg-core_params  hsotg-core_params-ahbcfg != -1)
+   val = hsotg-core_params-ahbcfg  ~GAHBCFG_CTRL_MASK;
+   else
+   val = 0;
+
if (using_dma(hsotg))
writel(GAHBCFG_GLBL_INTR_EN | GAHBCFG_DMA_EN |
-  (GAHBCFG_HBSTLEN_INCR4  GAHBCFG_HBSTLEN_SHIFT),
-  hsotg-regs + GAHBCFG);
+  (val ? val : GAHBCFG_HBSTLEN_INCR4 
+  GAHBCFG_HBSTLEN_SHIFT), hsotg-regs + GAHBCFG);
else
writel(((hsotg-dedicated_fifos) ? (GAHBCFG_NP_TXF_EMP_LVL |
GAHBCFG_P_TXF_EMP_LVL) : 0) 
|
-  GAHBCFG_GLBL_INTR_EN,
+  GAHBCFG_GLBL_INTR_EN | val,
   hsotg-regs + GAHBCFG);
 
/*
-- 
1.9.1

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

Re: [possible fix] HL-340 USB don't work correctly (ch340 based usb-rs232 adapter)

[Eddi De Pieri]

Hi Karl,

I don't know but as documented by usbsnoop log the value written from
kernel driver make my device to fail.

Windows driver after some assumption  write back the original value (0xc3)

Regards,

Eddi

On Sun, Feb 8, 2015 at 2:54 AM, Karl Palsson ka...@tweak.net.au wrote:

 My work in


 Eddi De Pieri e...@depieri.net wrote:
 Bus 003 Device 014: ID 1a86:7523 QinHeng Electronics HL-340 USB-Serial
 adapter

 ch431 kernel driver don't work correctly with this device,
 when URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER answer/send wrong character

 After some experiment  by passing command retrieved from usbsnoop log
 it seems that ch341.c  starts working if comment out following line

 // r = ch341_control_out(dev, 0x9a, 0x2518, 0x0050);

 my windows driver usbsnoop  | grep 18 25
 07: 03 ms 000398 ms c0 95 18 25 00 00 02 00   c3 00
 36: 04 ms 000620 ms 40 9a 18 25 c2 00 00 00 
 75: 03 ms 011673 ms 40 9a 18 25 c3 00 00 00 

 as you can see I get c3 while who first logged windows driver sniff get
 56.

 My work in progress updates to this driver had already marked that line
 as suspicious.
 My draft comment is this sets break + 5 bit mode, why?!  On a ch340,
 this _should_ have just been ignored, as it doesn't support 5 bit mode.
 Also, break on this device is actually by disabling the rx and tx
 sides, there's a bit in that register for both directions to be enabled
 individually.

 It's a little sad that your hl340 device fails, as those are the only
 ones that have been working for me so far.  ch341 devices don't work
 reliably at all.

 please note that my device after following command
 r = ch341_control_in(dev, 0x5f, 0, 0, buffer, size); (
 answer 0x3000, so it should be a more recent hardware version.

 ch34x linux driver from the oem confirmed that 0x5f means hardware
 version)

 I need someone that have same device or other devices that needs
 ch341.ko to check that it works properly even if you comment out
 before sending a patch back to the ML.


 It needs a lot more than just commenting out that line unfortunately.
 Good news, I'm working on this again, bad news, I said that 6 months ago
 too :(

 My current work is at https://github.com/karlp/linux/tree/ch341-3.18.6
 but note that this is only the initial cleanup, no fixes at this stage.

 Sincerely,
 Karl Palsson
--
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: Kernel Ooops upon USB disconnect of a Western Digital My Passport 1TB drive

[Athlion]

Just one more FYI, by blacklisting the ses module, the oops goes away.

On Tue, Feb 3, 2015 at 6:05 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Tue, 3 Feb 2015, Athlion wrote:

 On Mon, Feb 2, 2015 at 7:27 PM, Alan Stern st...@rowland.harvard.edu wrote:

  Had you made any changes to the runtime suspend settings?
  blk_post_runtime_resume wouldn't be called unless the drive had gone
  into runtime suspend.  And even then, it's not likely to be called
  after you unplug the USB cable.
 
  Alan Stern

 I have enabled SCSI ATA Bus power management with this udev rule:
 ACTION==add, SUBSYSTEM==scsi_host, KERNEL==host*,
 ATTR{link_power_management_policy}=min_power

 That won't have any effect on SCSI over USB, as far as I know.

 ( And generally almost all of actions herein:
 https://wiki.archlinux.org/index.php/Power_saving#Laptop_Mode )

 Here's another with dynamic debugging on (and some of the stack trace):

 [ 1565.610481] usb usb2: usb wakeup-resume
 [ 1565.613019] usb usb2: usb auto-resume
 [ 1565.615599] hub 2-0:1.0: hub_resume
 [ 1565.617876] usb usb2-port1: status 0507 change 
 [ 1565.620125] hub 2-0:1.0: state 7 ports 3 chg  evt 
 [ 1565.623471] hub 2-0:1.0: state 7 ports 3 chg  evt 
 [ 1565.650219] hub 2-0:1.0: state 7 ports 3 chg  evt 0002
 [ 1565.663520] usb 2-1: usb wakeup-resume
 [ 1565.665518] usb 2-1: finish resume
 [ 1565.667684] hub 2-1:1.0: hub_resume
 [ 1565.669812] usb 2-1-port2: status 0101 change 0001
 [ 1565.773645] usb usb2-port1: resume, status 0
 [ 1565.776031] hub 2-1:1.0: state 7 ports 8 chg 0004 evt 
 [ 1565.778664] usb 2-1-port2: status 0101, change , 12 Mb/s
 [ 1565.847291] usb 2-1.2: new high-speed USB device number 3 using ehci-pci
 [ 1565.860480] usb 2-1-port2: not reset yet, waiting 10ms
 [ 1565.945202] usb 2-1.2: default language 0x0409
 [ 1565.947882] usb 2-1.2: udev 3, busnum 2, minor = 130
 [ 1565.950384] usb 2-1.2: usb_probe_device
 [ 1565.952704] usb 2-1.2: configuration #1 chosen from 1 choice
 [ 1565.955145] usb 2-1.2: adding 2-1.2:1.0 (config #1, interface 0)
 [ 1565.957630] hub 2-1:1.0: state 7 ports 8 chg  evt 0004
 [ 1566.005124] usb-storage 2-1.2:1.0: usb_probe_interface
 [ 1566.006655] usb-storage 2-1.2:1.0: usb_probe_interface - got id
 [ 1566.008169] usb-storage 2-1.2:1.0: USB Mass Storage device detected
 [ 1566.010155] scsi host6: usb-storage 2-1.2:1.0
 [ 1566.012149] usbcore: registered new interface driver usb-storage
 [ 1566.016061] usbcore: registered new interface driver uas
 [ 1567.015852] scsi 6:0:0:0: Direct-Access WD   My Passport
 0748 1016 PQ: 0 ANSI: 6
 [ 1567.018966] scsi 6:0:0:1: Enclosure WD   SES Device
   1016 PQ: 0 ANSI: 6

 Your device contains two Logical Units.  One of them is the My Passport
 disk drive, and the other is the SES Device enclosure.

 [ 1567.027072] scsi 6:0:0:1: scsi_runtime_idle
 [ 1567.029695] sd 6:0:0:0: [sdb] Spinning up disk...
 [ 1567.032764] scsi 6:0:0:1: scsi_runtime_suspend
 [ 1568.031907] .ready
 [ 1569.060225] sd 6:0:0:0: [sdb] 1953458176 512-byte logical blocks:
 (1.00 TB/931 GiB)
 [ 1569.063857] sd 6:0:0:0: [sdb] Write Protect is off
 [ 1569.065613] ses 6:0:0:1: Attached Enclosure device
 [ 1569.068492] sd 6:0:0:0: [sdb] Mode Sense: 47 00 10 08
 [ 1569.071598] sd 6:0:0:0: [sdb] No Caching mode page found
 [ 1569.073628] sd 6:0:0:0: [sdb] Assuming drive cache: write through
 [ 1569.087891]  sdb: sdb1
 [ 1569.093382] sd 6:0:0:0: [sdb] Attached SCSI disk
 [ 1615.830142] hub 2-1:1.0: state 7 ports 8 chg  evt 0004
 [ 1615.833066] usb 2-1-port2: status 0100, change 0001, 12 Mb/s
 [ 1615.835764] usb 2-1.2: USB disconnect, device number 3
 [ 1615.838471] usb 2-1.2: unregistering device
 [ 1615.841199] usb 2-1.2: unregistering interface 2-1.2:1.0
 [ 1615.844996] scsi target6:0:0: scsi_runtime_idle
 [ 1615.848746] scsi target6:0:0: scsi_runtime_suspend
 [ 1615.868925] scsi target6:0:0: scsi_runtime_resume
 [ 1615.870975] ses 6:0:0:1: scsi_runtime_resume
 [ 1615.872913] BUG: unable to handle kernel NULL pointer dereference
 at 01a0
 [ 1615.874896] IP: [812850d5] blk_post_runtime_resume+0x65/0x80
 [ 1615.876843] PGD 0
 [ 1615.878765] Oops: 0002 [#1] PREEMPT SMP
 [ 1615.880734] Modules linked in: ses enclosure uas usb_storage
 netconsole joydev mousedev snd_hda_codec_hdmi snd_hda_codec_conexant
 snd_hda_codec_generic coretemp intel_rapl x86_pkg_temp_thermal
 intel_powerclamp kvm_intel kvm arc4 crct10dif_pclmul iwldvm
 crc32_pclmul iTCO_wdt iTCO_vendor_support crc32c_intel mac80211
 ghash_clmulni_intel ip6t_REJECT nf_reject_ipv6 aesni_intel xt_hl
 aes_x86_64 lrw gf128mul ip6t_rt psmouse iwlwifi glue_helper
 ablk_helper cryptd i2c_i801 serio_raw cfg80211 nf_conntrack_ipv6
 nf_defrag_ipv6 snd_hda_intel wmi thinkpad_acpi nvram thermal rfkill
 ipt_REJECT nf_reject_ipv4 hwmon tpm_tis ac tpm battery
 snd_hda_controller snd_hda_codec snd_hwdep evdev snd_pcm mac_hid
 xt_limit e1000e snd_timer xt_tcpudp mei_me snd mei ptp soundcore
 shpchp 

[PATCH] usb: dwc3: Moved PCI IDS to linux/pci_ids.h

[Joseph Kogut]

Moved DWC3 PCI IDS to linux/pci_ids.h per the FIXME.

Signed-off-by: Joseph Kogut joseph.ko...@gmail.com
---
 drivers/usb/dwc3/dwc3-pci.c | 10 +-
 include/linux/pci_ids.h |  8 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 8d95056..19ca7f6 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -20,19 +20,11 @@
 #include linux/module.h
 #include linux/slab.h
 #include linux/pci.h
+#include linux/pci_ids.h
 #include linux/platform_device.h
 
 #include platform_data.h
 
-/* FIXME define these in linux/pci_ids.h */
-#define PCI_VENDOR_ID_SYNOPSYS 0x16c3
-#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
-#define PCI_DEVICE_ID_INTEL_BYT0x0f37
-#define PCI_DEVICE_ID_INTEL_MRFLD  0x119e
-#define PCI_DEVICE_ID_INTEL_BSW0x22B7
-#define PCI_DEVICE_ID_INTEL_SPTLP  0x9d30
-#define PCI_DEVICE_ID_INTEL_SPTH   0xa130
-
 static int dwc3_pci_quirks(struct pci_dev *pdev)
 {
if (pdev-vendor == PCI_VENDOR_ID_AMD 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index e63c02a..6fc5cdd 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2312,6 +2312,9 @@
 #define PCI_VENDOR_ID_NETCELL  0x169c
 #define PCI_DEVICE_ID_REVOLUTION   0x0044
 
+#define PCI_VENDOR_ID_SYNOPSYS 0x16c3
+#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB30xabcd
+
 #define PCI_VENDOR_ID_CENATEK  0x16CA
 #define PCI_DEVICE_ID_CENATEK_IDE  0x0001
 
@@ -2566,12 +2569,14 @@
 #define PCI_DEVICE_ID_INTEL_QUARK_X1000_ILB0x095E
 #define PCI_DEVICE_ID_INTEL_I960   0x0960
 #define PCI_DEVICE_ID_INTEL_I960RM 0x0962
+#define PCI_DEVICE_ID_INTEL_BYT0x0f37
 #define PCI_DEVICE_ID_INTEL_CENTERTON_ILB  0x0c60
 #define PCI_DEVICE_ID_INTEL_8257X_SOL  0x1062
 #define PCI_DEVICE_ID_INTEL_82573E_SOL 0x1085
 #define PCI_DEVICE_ID_INTEL_82573L_SOL 0x108F
 #define PCI_DEVICE_ID_INTEL_82815_MC   0x1130
 #define PCI_DEVICE_ID_INTEL_82815_CGC  0x1132
+#define PCI_DEVICE_ID_INTEL_MRFLD  0x119e
 #define PCI_DEVICE_ID_INTEL_82092AA_0  0x1221
 #define PCI_DEVICE_ID_INTEL_7505_0 0x2550
 #define PCI_DEVICE_ID_INTEL_7205_0 0x255d
@@ -2593,6 +2598,7 @@
 #define PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI  0x1e31
 #define PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MIN   0x1e40
 #define PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MAX   0x1e5f
+#define PCI_DEVICE_ID_INTEL_BSW0x22B7
 #define PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN   0x2310
 #define PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX   0x231f
 #define PCI_DEVICE_ID_INTEL_82801AA_0  0x2410
@@ -2891,6 +2897,8 @@
 #define PCI_DEVICE_ID_INTEL_84460GX0x84ea
 #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500
 #define PCI_DEVICE_ID_INTEL_IXP28000x9004
+#define PCI_DEVICE_ID_INTEL_SPTLP  0x9d30
+#define PCI_DEVICE_ID_INTEL_SPTH   0xa130
 #define PCI_DEVICE_ID_INTEL_S21152BB   0xb152
 
 #define PCI_VENDOR_ID_SCALEMP  0x8686
-- 
2.3.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: Re: [possible fix] HL-340 USB don't work correctly (ch340 based usb-rs232 adapter)

[Karl Palsson]

Eddi De Pieri e...@depieri.net wrote:
 Hi Karl,
 
 I don't know but as documented by usbsnoop log the value written from
 kernel driver make my device to fail.
 
 Windows driver after some assumption  write back the original value
 (0xc3)
 

Ok,

I'm still progressing on more of that init code, but what I've got
pushed to https://github.com/karlp/linux/tree/ch341-3.18.6 at the moment
is _substantially_ more reliable than the original code.  It might
already fix  your problems.

With the original code, many software packages I tried wouldn't actually
work unless something else had setup the tty explicitly ahead of time.  

I'm still continuing to add the parity and stopbit configurations, and
then need to review break/clear and hardware flow control.

Cheers

Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers

[Ruslan Bilovol]

Hi Alan,

On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern st...@rowland.harvard.edu wrote:
 On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:

  Why bother matching by name?  Why not simply take the first
  available
  UDC?

 Because you may have more than one udc. This would allow to pick one by
 name just like using configfs interface.

 Clearly it would be more flexible to allow the user to do the matching,
 the way configfs does (sysfs too).

   Main feature of my path is not only deferred binding of gadget
  driver,
   but also possibility to register/unregister udc at any time.
   This is useful for user who can load, for example, udc module
   if needed and unload it safely, not touching gadget driver.
 
  We can already do that with the existing code.  There's no need for
  a
  patch.
 
  Also, it's not clear that the existing gadget drivers will work
  properly if they are unbound from one UDC and then bound again to
  another one.  They were not written with that sort of thing in
  mind.
 

 What you have described is one of basics configfs features.
 You should be able to bind and unbind your gadget whenever you want
 and it should work properly after doing:

 ## create gadget
 $ echo udc.0  UDC
 $ echo   UDC
 $ echo udc.1  UDC

 Function shouldn't care which udc it has been bound previously.
 Only current one is important and on each unbind each function
 should cleanup its state and prepare to be bound to another udc.
 Configfs interface doesn't prohibit this and I haven't seen any
 info about such restriction.

 It's not prohibited, but it also was never required.  Therefore it may
 not be implemented in all gadget drivers.

  If some function is not working in
 such situation there is a bug in that function and it should be fixed.

 That's fine.  I wasn't pointing out a fundamental limitation, just a
 fact that will have to be taken into account.

 Anyway, instead of going through all this, why not do what I suggested
 earlier (see http://marc.info/?l=linux-usbm=139888691230119w=2) and
 create a gadget bus type?  That would give userspace explicit control
 over which gadget driver was bound to which UDC.

 Or maybe that's not a very good fit with the existing code, since most
 gadget drivers assume they will be bound to only one UDC at a time.  So
 instead, why not create a sysfs interface that allows userspace to
 control which gadget drivers are bound to which UDCs?

 As I recall, the original problem people were complaining about was
 deferred probing.  They didn't need fancy matching; all they wanted was
 the ability to bind a gadget driver to a UDC some time after the gadget
 driver was loaded and initialized.  Something simple like Robert
 Baldyga's patch is enough to do that.

I looked over my patch and see that it doesn't automatically rebind
gadget driver to another available UDC after previous UDC is unbound.
The Gadget Bus mentioned previously is good thing but so far it seems there
is no users of such approach. So I left only deferred registration
from my patch.
I still keep it inside of udc-core since we also need to keep tracking UDC name
if somebody wanted to bind gadget driver to specific UDC and it looks
like good place to maintain this. I'll send second version of patches soon

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

Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers

[Ruslan Bilovol]

Hi Krzysztof,

On Tue, Feb 10, 2015 at 10:47 AM, Krzysztof Opasiak
k.opas...@samsung.com wrote:


 -Original Message-
 From: Ruslan Bilovol [mailto:ruslan.bilo...@gmail.com]
 Sent: Tuesday, February 10, 2015 12:46 AM
 To: Alan Stern
 Cc: Krzysztof Opasiak; Peter Chen; linux-usb@vger.kernel.org;
 linux-ker...@vger.kernel.org; Balbi, Felipe;
 gre...@linuxfoundation.org; Andrzej Pietrasiewicz
 Subject: Re: [PATCH 1/2] usb: gadget: udc-core: independent
 registration of gadgets and gadget drivers

 Hi guys,

 On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern
 st...@rowland.harvard.edu wrote:
  On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:
 
   Why bother matching by name?  Why not simply take the first
   available
   UDC?
 
  Because you may have more than one udc. This would allow to pick
 one by
  name just like using configfs interface.
 
  Clearly it would be more flexible to allow the user to do the
 matching,
  the way configfs does (sysfs too).
 
Main feature of my path is not only deferred binding of
 gadget
   driver,
but also possibility to register/unregister udc at any time.
This is useful for user who can load, for example, udc
 module
if needed and unload it safely, not touching gadget driver.
  
   We can already do that with the existing code.  There's no
 need for
   a
   patch.
  
   Also, it's not clear that the existing gadget drivers will
 work
   properly if they are unbound from one UDC and then bound again
 to
   another one.  They were not written with that sort of thing in
   mind.
  
 
  What you have described is one of basics configfs features.
  You should be able to bind and unbind your gadget whenever you
 want
  and it should work properly after doing:
 
  ## create gadget
  $ echo udc.0  UDC
  $ echo   UDC
  $ echo udc.1  UDC
 
  Function shouldn't care which udc it has been bound previously.
  Only current one is important and on each unbind each function
  should cleanup its state and prepare to be bound to another udc.
  Configfs interface doesn't prohibit this and I haven't seen any
  info about such restriction.

 Thank you Krzysztof for this explanation that makes things more
 clear
 for reviewers.

 
  It's not prohibited, but it also was never required.  Therefore
 it may
  not be implemented in all gadget drivers.
 
   If some function is not working in
  such situation there is a bug in that function and it should be
 fixed.
 
  That's fine.  I wasn't pointing out a fundamental limitation,
 just a
  fact that will have to be taken into account.

 I also don't see any restrictions to bind/unbind gadget driver few
 times
 and in case of such bug we just can fix it. I didn't see any issue
 with
 gadget drivers that I used for verification, however, to be honest,
 I didn't
 test it with all possible ones.
 Anyway, it should work in legacy way (one gadget driver is bound to
 one uds)
 so current behavior at least is not broken.

 
  Anyway, instead of going through all this, why not do what I
 suggested
  earlier (see http://marc.info/?l=linux-usbm=139888691230119w=2)
 and
  create a gadget bus type?  That would give userspace explicit
 control
  over which gadget driver was bound to which UDC.

 Exactly, my patch transforms udc-core to something like tiny bus
 with very basic
 functionality. But in mentioned mailthread I see good ideas that is
 possible to
 implement with small overhead.

 
  Or maybe that's not a very good fit with the existing code, since
 most
  gadget drivers assume they will be bound to only one UDC at a
 time.  So
  instead, why not create a sysfs interface that allows userspace
 to
  control which gadget drivers are bound to which UDCs?
 
  As I recall, the original problem people were complaining about
 was
  deferred probing.  They didn't need fancy matching; all they
 wanted was
  the ability to bind a gadget driver to a UDC some time after the
 gadget
  driver was loaded and initialized.  Something simple like Robert
  Baldyga's patch is enough to do that.

 This simplicity is also a limitation. As I mentioned before,
 sometimes it is
 needed to be able to bind/unbind gadget driver multiple times to
 different UDCs.
 A real case I faced recently is SoC with HighSpeed-only and
 SuperSpeed-only
 UDCs. SuperSpeed-only UDC can't work on USB 2.0 speeds and vice
 versa.
 The system has USB3.0 USB connector with soldered HS lines from
 mentioned HS-only and SS lines from SS-only UDCs. Each UDC has it's
 own
 device driver, so if we want to use both of them with one gadget
 driver, we
 must be able to bind/unbind it multiple times to different UDCs.
 Another one usecase is users who can unload udc drivers without
 carrying
 about gadget drivers.
 Third usecase is, for example, developers who can switch between
 dummy_hcd
 and real UDC hardware without unloading gadget driver.


 Just a stupid question. Why don't you use configfs composite gadget
 instead of legacy gadgets?

 In my opinion all things which you have described are 

[PATCH v2 0/4] usb: renesas_usbhs: add support for USB-DMAC

[Yoshihiro Shimoda]

This patch set is based on Felipe's usb.git / testing/next branch.
(commit id = 6461d69d12508fe166be5c6c31d5a9da02a4dfb5)

To use the USB-DMAC, we have to add some device nodes for USB-DMAC
in dts file. If we don't have such device nodes, this driver will
use PIO.

Changes from v1:
 - Change local_irq_save/restore() area in patch 1.
 - Modify the commit log in patch 1.
 - fix some comments in patch 4.

Yoshihiro Shimoda (4):
  usb: renesas_usbhs: fix spinlock suspected in a gadget complete
function
  usb: renesas_usbhs: add the channel number in dma-names
  usb: renesas_usbhs: fix the sequence in xfer_work()
  usb: renesas_usbhs: add support for USB-DMAC

 .../devicetree/bindings/usb/renesas_usbhs.txt  |5 +-
 drivers/usb/renesas_usbhs/common.c |   19 ++
 drivers/usb/renesas_usbhs/common.h |7 +
 drivers/usb/renesas_usbhs/fifo.c   |  189 ++--
 drivers/usb/renesas_usbhs/fifo.h   |1 +
 drivers/usb/renesas_usbhs/mod_gadget.c |   11 ++
 drivers/usb/renesas_usbhs/pipe.c   |   39 
 drivers/usb/renesas_usbhs/pipe.h   |1 +
 include/linux/usb/renesas_usbhs.h  |2 +
 9 files changed, 258 insertions(+), 16 deletions(-)

-- 
1.7.9.5

--
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 3/4] usb: renesas_usbhs: fix the sequence in xfer_work()

[Yoshihiro Shimoda]

This patch fixes the setup sequence in xfer_work(). Otherwise,
sometimes a usb transaction will get stuck.

Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
 drivers/usb/renesas_usbhs/fifo.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index 28d10eb..b737661 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -822,10 +822,10 @@ static void xfer_work(struct work_struct *work)
fifo-name, usbhs_pipe_number(pipe), pkt-length, pkt-zero);
 
usbhs_pipe_running(pipe, 1);
-   usbhs_pipe_set_trans_count_if_bulk(pipe, pkt-trans);
-   usbhs_pipe_enable(pipe);
usbhsf_dma_start(pipe, fifo);
+   usbhs_pipe_set_trans_count_if_bulk(pipe, pkt-trans);
dma_async_issue_pending(chan);
+   usbhs_pipe_enable(pipe);
 }
 
 /*
-- 
1.7.9.5

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

[PATCH v2 1/4] usb: renesas_usbhs: fix spinlock suspected in a gadget complete function

[Yoshihiro Shimoda]

According to the gadget.h, a complete function will always be called
with interrupts disabled. However, sometimes usbhsg_queue_pop() function
is called with interrupts enabled. So, this function should calls
local_irq_save() before this calls the usb_gadget_giveback_request().
Otherwise, there is possible to cause a spinlock suspected in a gadget
complete function.

Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
 drivers/usb/renesas_usbhs/mod_gadget.c |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c 
b/drivers/usb/renesas_usbhs/mod_gadget.c
index e0384af..104bddf 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -126,11 +126,22 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
struct usbhsg_gpriv *gpriv = usbhsg_uep_to_gpriv(uep);
struct usbhs_pipe *pipe = usbhsg_uep_to_pipe(uep);
struct device *dev = usbhsg_gpriv_to_dev(gpriv);
+   unsigned long flags;
 
dev_dbg(dev, pipe %d : queue pop\n, usbhs_pipe_number(pipe));
 
ureq-req.status = status;
+   /*
+* According to the gadget.h, a complete function will always be
+* called with interrupts disabled. However, sometimes this function
+* is called with interrupts enabled. (e.g. complete a DMAC transfer or
+* write data and done is set immediately when PIO.) So, this function
+* should calls local_irq_save() before this calls the
+* usb_gadget_giveback_request().
+*/
+   local_irq_save(flags);
usb_gadget_giveback_request(uep-ep, ureq-req);
+   local_irq_restore(flags);
 }
 
 static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
-- 
1.7.9.5

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

[PATCH v2 2/4] usb: renesas_usbhs: add the channel number in dma-names

[Yoshihiro Shimoda]

To connect the channel of USB-DMAC to USBHS DnFIFO number, this patch
adds this channel/FIFO number in dma-names. Otherwise, this driver
needs to add analysis code for device tree.

Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
 .../devicetree/bindings/usb/renesas_usbhs.txt  |5 -
 drivers/usb/renesas_usbhs/fifo.c   |   20 +---
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 61b045b..dc2a18f 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -15,7 +15,10 @@ Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be usb
   - dmas: Must contain a list of references to DMA specifiers.
-  - dma-names : Must contain a list of DMA names, tx or rx.
+  - dma-names : Must contain a list of DMA names:
+   - tx0 ... txn
+   - rx0 ... rxn
+- This n means DnFIFO in USBHS module.
 
 Example:
usbhs: usb@e659 {
diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index d891bff..28d10eb 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -1069,23 +1069,29 @@ static void usbhsf_dma_init_pdev(struct usbhs_fifo 
*fifo)
fifo-rx_slave);
 }
 
-static void usbhsf_dma_init_dt(struct device *dev, struct usbhs_fifo *fifo)
+static void usbhsf_dma_init_dt(struct device *dev, struct usbhs_fifo *fifo,
+  int channel)
 {
-   fifo-tx_chan = dma_request_slave_channel_reason(dev, tx);
+   char name[16];
+
+   snprintf(name, sizeof(name), tx%d, channel);
+   fifo-tx_chan = dma_request_slave_channel_reason(dev, name);
if (IS_ERR(fifo-tx_chan))
fifo-tx_chan = NULL;
-   fifo-rx_chan = dma_request_slave_channel_reason(dev, rx);
+
+   snprintf(name, sizeof(name), rx%d, channel);
+   fifo-rx_chan = dma_request_slave_channel_reason(dev, name);
if (IS_ERR(fifo-rx_chan))
fifo-rx_chan = NULL;
 }
 
-static void usbhsf_dma_init(struct usbhs_priv *priv,
-   struct usbhs_fifo *fifo)
+static void usbhsf_dma_init(struct usbhs_priv *priv, struct usbhs_fifo *fifo,
+   int channel)
 {
struct device *dev = usbhs_priv_to_dev(priv);
 
if (dev-of_node)
-   usbhsf_dma_init_dt(dev, fifo);
+   usbhsf_dma_init_dt(dev, fifo, channel);
else
usbhsf_dma_init_pdev(fifo);
 
@@ -1231,7 +1237,7 @@ do {  
\
usbhs_get_dparam(priv, d##channel##_tx_id); \
fifo-rx_slave.shdma_slave.slave_id =   \
usbhs_get_dparam(priv, d##channel##_rx_id); \
-   usbhsf_dma_init(priv, fifo);\
+   usbhsf_dma_init(priv, fifo, channel);   \
 } while (0)
 
 #define USBHS_DFIFO_INIT(priv, fifo, channel)  \
-- 
1.7.9.5

--
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 4/4] usb: renesas_usbhs: add support for USB-DMAC

[Yoshihiro Shimoda]

Some Renesas SoCs have the USB-DMAC. It is able to terminate transfers
when a short packet is received, even if less bytes than the transfer
counter size have been received. Also, it is able to send a short
packet even if the packet size is not multiples of 8bytes.

Since the previous code has used the interruption of USBHS controller
when receiving packets even if this driver has used a dmac, a lot of
interruptions has happened. This patch will reduce such interruptions.

This patch allows to use the USB-DMAC on R-Car H2 and M2.

Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
 drivers/usb/renesas_usbhs/common.c |   19 +
 drivers/usb/renesas_usbhs/common.h |7 ++
 drivers/usb/renesas_usbhs/fifo.c   |  165 ++--
 drivers/usb/renesas_usbhs/fifo.h   |1 +
 drivers/usb/renesas_usbhs/pipe.c   |   39 +
 drivers/usb/renesas_usbhs/pipe.h   |1 +
 include/linux/usb/renesas_usbhs.h  |2 +
 7 files changed, 228 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index 4cf77d3..0f7e850 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -276,6 +276,16 @@ int usbhs_set_device_config(struct usbhs_priv *priv, int 
devnum,
 }
 
 /*
+ * interrupt functions
+ */
+void usbhs_xxxsts_clear(struct usbhs_priv *priv, u16 sts_reg, u16 bit)
+{
+   u16 pipe_mask = (u16)GENMASK(usbhs_get_dparam(priv, pipe_size), 0);
+
+   usbhs_write(priv, sts_reg, ~(1  bit)  pipe_mask);
+}
+
+/*
  * local functions
  */
 static void usbhsc_set_buswait(struct usbhs_priv *priv)
@@ -487,6 +497,15 @@ static struct renesas_usbhs_platform_info 
*usbhs_parse_dt(struct device *dev)
if (gpio  0)
dparam-enable_gpio = gpio;
 
+   switch (dparam-type) {
+   case USBHS_TYPE_R8A7790:
+   case USBHS_TYPE_R8A7791:
+   dparam-has_usb_dmac = 1;
+   break;
+   default:
+   break;
+   }
+
return info;
 }
 
diff --git a/drivers/usb/renesas_usbhs/common.h 
b/drivers/usb/renesas_usbhs/common.h
index fc96e92..8c5fc12 100644
--- a/drivers/usb/renesas_usbhs/common.h
+++ b/drivers/usb/renesas_usbhs/common.h
@@ -193,6 +193,7 @@ struct usbhs_priv;
 #define TYPE_BULK  (1  14)
 #define TYPE_INT   (2  14)
 #define TYPE_ISO   (3  14)
+#define BFRE   (1  10)   /* BRDY Interrupt Operation Spec. */
 #define DBLB   (1  9)/* Double Buffer Mode */
 #define SHTNAK (1  7)/* Pipe Disable in Transfer End */
 #define DIR_OUT(1  4)/* Transfer Direction */
@@ -216,6 +217,7 @@ struct usbhs_priv;
 #defineACLRM   (1  9)/* Buffer Auto-Clear Mode */
 #define SQCLR  (1  8)/* Toggle Bit Clear */
 #define SQSET  (1  7)/* Toggle Bit Set */
+#define SQMON  (1  6)/* Toggle Bit Check */
 #define PBUSY  (1  5)/* Pipe Busy */
 #define PID_MASK   (0x3)   /* Response PID */
 #define  PID_NAK   0
@@ -324,6 +326,11 @@ int usbhs_set_device_config(struct usbhs_priv *priv, int 
devnum, u16 upphub,
   u16 hubport, u16 speed);
 
 /*
+ * interrupt functions
+ */
+void usbhs_xxxsts_clear(struct usbhs_priv *priv, u16 sts_reg, u16 bit);
+
+/*
  * data
  */
 struct usbhs_priv *usbhs_pdev_to_priv(struct platform_device *pdev);
diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c
index b737661..8597cf9 100644
--- a/drivers/usb/renesas_usbhs/fifo.c
+++ b/drivers/usb/renesas_usbhs/fifo.c
@@ -813,7 +813,8 @@ static void xfer_work(struct work_struct *work)
desc-callback  = usbhsf_dma_complete;
desc-callback_param= pipe;
 
-   if (dmaengine_submit(desc)  0) {
+   pkt-cookie = dmaengine_submit(desc);
+   if (pkt-cookie  0) {
dev_err(dev, Failed to submit dma descriptor\n);
return;
}
@@ -838,6 +839,7 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
struct usbhs_fifo *fifo;
int len = pkt-length - pkt-actual;
int ret;
+   uintptr_t align_mask;
 
if (usbhs_pipe_is_busy(pipe))
return 0;
@@ -847,10 +849,14 @@ static int usbhsf_dma_prepare_push(struct usbhs_pkt *pkt, 
int *is_done)
usbhs_pipe_is_dcp(pipe))
goto usbhsf_pio_prepare_push;
 
-   if (len  0x7) /* 8byte alignment */
+   /* check data length if this driver don't use USB-DMAC */
+   if (!usbhs_get_dparam(priv, has_usb_dmac)  len  0x7)
goto usbhsf_pio_prepare_push;
 
-   if ((uintptr_t)(pkt-buf + pkt-actual)  0x7) /* 8byte alignment */
+   /* check buffer alignment */
+   align_mask = usbhs_get_dparam(priv, has_usb_dmac) ?
+   USBHS_USB_DMAC_XFER_SIZE - 1 : 0x7;
+   if 

RE: [PATCH 1/1] MAINTAINERS: add entry for USB OTG FSM

[Peter Chen]

 
 On Sun, Feb 15, 2015 at 12:22:59PM +0800, Peter Chen wrote:
  Add MAINTAINER entry for USB OTG Finite State Machine
 
  Cc: Felipe Balbi ba...@ti.com
  Signed-off-by: Peter Chen peter.c...@freescale.com
  ---
   MAINTAINERS | 7 +++
   1 file changed, 7 insertions(+)
 
  diff --git a/MAINTAINERS b/MAINTAINERS index f25de35..bcd6b6b 100644
  --- a/MAINTAINERS
  +++ b/MAINTAINERS
  @@ -10021,6 +10021,13 @@ S: Maintained
   F: Documentation/usb/ohci.txt
   F: drivers/usb/host/ohci*
 
  +USB OTG FSM (Finite State Machine)
  +M: Peter Chen peter.c...@freescale.com
  +T: git git://github.com/hzpeterchen/linux-usb.git
 
 
 I'll queue this up, but you might want to think about getting a kernel.org
 account as I can't ever take pull requests from github.com.
 And your end-goal is to have me be able to trust you enough to take pull
 requests, right?  {hint}
 

Thanks, I will.

Peter
--
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: Kernel crash on usb-storage disconnect, multiqueue in use

[Bruno Prémont]

On Thu, 12 Feb 2015 07:56:47 +0100 Bart Van Assche wrote:
 On 02/11/15 22:25, Bruno Prémont wrote:
  Since 3.19-rcX kernel crashes when I disconnect an external
  HDD drive (not sure since which kernel revision tough early
  3.19-rcs possibly are not affected).
  
  It looks like this crash is related to the fact that I enable
  multiqueue (CONFIG_SCSI_MQ_DEFAULT=y).
 
 Can you repeat your test with kernel 3.19, which includes the pull
 request https://lkml.org/lkml/2015/2/3/606 ?

Yes crash is fixed in v3.19 (just a two-line IO error left on
SCSI cache flush).

Thanks,
Bruno


 Bart.
 
--
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 serial driver change question in the linux kernel

[Johan Hovold]

On Sun, Feb 15, 2015 at 01:26:08PM +, Li, Elvin wrote:
 Hi Johan,
   Your patch fixed my issue. Thank you for support!

Great, thanks for testing.

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

[PATCH] USB: console: add dummy __module_get

[Johan Hovold]

Add call to __module_get when initialising the fake tty in
usb_console_setup to match the module_put in release_one_tty.

Note that the tty-driver (i.e. usb-serial core) must be compiled-in to
enable the usb console so the __module_get is essentially a noop as
driver-owner will be null.

Reported-by: Ben Hutchings b...@decadent.org.uk
Signed-off-by: Johan Hovold jo...@kernel.org
---
 drivers/usb/serial/console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index 29fa1c3d0089..3806e7014199 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -14,6 +14,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 
 #include linux/kernel.h
+#include linux/module.h
 #include linux/slab.h
 #include linux/tty.h
 #include linux/console.h
@@ -144,6 +145,7 @@ static int usb_console_setup(struct console *co, char 
*options)
init_ldsem(tty-ldisc_sem);
INIT_LIST_HEAD(tty-tty_files);
kref_get(tty-driver-kref);
+   __module_get(tty-driver-owner);
tty-ops = usb_console_fake_tty_ops;
if (tty_init_termios(tty)) {
retval = -ENOMEM;
-- 
2.0.5

--
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: console: fix potential use after free

[Ben Hutchings]

On Wed, 2015-02-11 at 14:55 +0800, Johan Hovold wrote:
 On Tue, Feb 10, 2015 at 08:39:26PM +, Ben Hutchings wrote:
  On Mon, 2015-01-05 at 16:04 +0100, Johan Hovold wrote:
   Use tty kref to release the fake tty in usb_console_setup to avoid use
   after free if the underlying serial driver has acquired a reference.
   
   Note that using the tty destructor release_one_tty requires some more
   state to be initialised.
  [...]
   --- a/drivers/usb/serial/console.c
   +++ b/drivers/usb/serial/console.c
  [...]
   @@ -137,14 +139,17 @@ static int usb_console_setup(struct console *co, 
   char *options)
 goto reset_open_count;
 }
 kref_init(tty-kref);
   - tty_port_tty_set(port-port, tty);
 tty-driver = usb_serial_tty_driver;
 tty-index = co-index;
 init_ldsem(tty-ldisc_sem);
   + INIT_LIST_HEAD(tty-tty_files);
   + kref_get(tty-driver-kref);
   + tty-ops = usb_console_fake_tty_ops;
  [...]
  
  Do we also need:
  __module_get(tty-driver-owner);
  or am I missing something?
 
 When the console setup code is running, and while the console is open,
 everything is pinned through a reference to the usb-serial-driver module
 (the tty layer also pins usb-serial core directly).

 We should be holding a reference to the usb-serial driver module (which
 in turns pins usb-serial core, which is the tty driver) for when the
 console isn't open as well, although that is not directly related to the
 fix in question.

Sure, but release_one_tty() expects that the tty has a reference to the
module and it will drop that reference.  (Normally tty_init_dev() takes
that reference.)  I think that means that for a USB serial console we
take one reference to the module when setting up but drop two references
when cleaning up.

Ben.

 Yes, the USB console is a bit of a hack.


-- 
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] USB: console: fix potential use after free

[Johan Hovold]

On Mon, Feb 16, 2015 at 03:49:17AM +, Ben Hutchings wrote:
 On Wed, 2015-02-11 at 14:55 +0800, Johan Hovold wrote:
  On Tue, Feb 10, 2015 at 08:39:26PM +, Ben Hutchings wrote:
   On Mon, 2015-01-05 at 16:04 +0100, Johan Hovold wrote:
Use tty kref to release the fake tty in usb_console_setup to avoid use
after free if the underlying serial driver has acquired a reference.

Note that using the tty destructor release_one_tty requires some more
state to be initialised.
   [...]
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
   [...]
@@ -137,14 +139,17 @@ static int usb_console_setup(struct console *co, 
char *options)
goto reset_open_count;
}
kref_init(tty-kref);
-   tty_port_tty_set(port-port, tty);
tty-driver = usb_serial_tty_driver;
tty-index = co-index;
init_ldsem(tty-ldisc_sem);
+   INIT_LIST_HEAD(tty-tty_files);
+   kref_get(tty-driver-kref);
+   tty-ops = usb_console_fake_tty_ops;
   [...]
   
   Do we also need:
 __module_get(tty-driver-owner);
   or am I missing something?
  
  When the console setup code is running, and while the console is open,
  everything is pinned through a reference to the usb-serial-driver module
  (the tty layer also pins usb-serial core directly).
 
  We should be holding a reference to the usb-serial driver module (which
  in turns pins usb-serial core, which is the tty driver) for when the
  console isn't open as well, although that is not directly related to the
  fix in question.
 
 Sure, but release_one_tty() expects that the tty has a reference to the
 module and it will drop that reference.  (Normally tty_init_dev() takes
 that reference.)  I think that means that for a USB serial console we
 take one reference to the module when setting up but drop two references
 when cleaning up.

Ah, but the tty-driver owner will be null as usb-serial has to be
compiled in to enable the usb console (and therefore the point I made
above about a missing reference is also moot).

So nothing is leaking, but I should probably add a comment or a call to
__module_get() nonetheless to make this clear.

  Yes, the USB console is a bit of a hack.

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