[PATCH] usbip: usbip_host: fix NULL-ptr deref and use-after-free errors

2018-05-14 Thread Shuah Khan (Samsung OSG)
usbip_host updates device status without holding lock from stub probe,
disconnect and rebind code paths. When multiple requests to import a
device are received, these unprotected code paths step all over each
other and drive fails with NULL-ptr deref and use-after-free errors.

The driver uses a table lock to protect the busid array for adding and
deleting busids to the table. However, the probe, disconnect and rebind
paths get the busid table entry and update the status without holding
the busid table lock. Add a new finer grain lock to protect the busid
entry. This new lock will be held to search and update the busid entry
fields from get_busid_idx(), add_match_busid() and del_match_busid().

match_busid_show() does the same to access the busid entry fields.

get_busid_priv() changed to return the pointer to the busid entry holding
the busid lock. stub_probe(), stub_disconnect() and stub_device_rebind()
call put_busid_priv() to release the busid lock before returning. This
changes fixes the unprotected code paths eliminating the race conditions
in updating the busid entries.

Signed-off-by: Shuah Khan (Samsung OSG) 
---
 drivers/usb/usbip/stub.h  |  2 ++
 drivers/usb/usbip/stub_dev.c  | 33 +++--
 drivers/usb/usbip/stub_main.c | 40 +++-
 3 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/usbip/stub.h b/drivers/usb/usbip/stub.h
index 14a72357800a..35618ceb2791 100644
--- a/drivers/usb/usbip/stub.h
+++ b/drivers/usb/usbip/stub.h
@@ -73,6 +73,7 @@ struct bus_id_priv {
struct stub_device *sdev;
struct usb_device *udev;
char shutdown_busid;
+   spinlock_t busid_lock;
 };
 
 /* stub_priv is allocated from stub_priv_cache */
@@ -83,6 +84,7 @@ extern struct usb_device_driver stub_driver;
 
 /* stub_main.c */
 struct bus_id_priv *get_busid_priv(const char *busid);
+void put_busid_priv(struct bus_id_priv *bid);
 int del_match_busid(char *busid);
 void stub_device_cleanup_urbs(struct stub_device *sdev);
 
diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index 9d0425113c4b..c0d6ff1baa72 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -300,7 +300,7 @@ static int stub_probe(struct usb_device *udev)
struct stub_device *sdev = NULL;
const char *udev_busid = dev_name(&udev->dev);
struct bus_id_priv *busid_priv;
-   int rc;
+   int rc = 0;
 
dev_dbg(&udev->dev, "Enter probe\n");
 
@@ -317,13 +317,15 @@ static int stub_probe(struct usb_device *udev)
 * other matched drivers by the driver core.
 * See driver_probe_device() in driver/base/dd.c
 */
-   return -ENODEV;
+   rc = -ENODEV;
+   goto call_put_busid_priv;
}
 
if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) {
dev_dbg(&udev->dev, "%s is a usb hub device... skip!\n",
 udev_busid);
-   return -ENODEV;
+   rc = -ENODEV;
+   goto call_put_busid_priv;
}
 
if (!strcmp(udev->bus->bus_name, "vhci_hcd")) {
@@ -331,13 +333,16 @@ static int stub_probe(struct usb_device *udev)
"%s is attached on vhci_hcd... skip!\n",
udev_busid);
 
-   return -ENODEV;
+   rc = -ENODEV;
+   goto call_put_busid_priv;
}
 
/* ok, this is my device */
sdev = stub_device_alloc(udev);
-   if (!sdev)
-   return -ENOMEM;
+   if (!sdev) {
+   rc = -ENOMEM;
+   goto call_put_busid_priv;
+   }
 
dev_info(&udev->dev,
"usbip-host: register new device (bus %u dev %u)\n",
@@ -369,7 +374,9 @@ static int stub_probe(struct usb_device *udev)
}
busid_priv->status = STUB_BUSID_ALLOC;
 
-   return 0;
+   rc = 0;
+   goto call_put_busid_priv;
+
 err_files:
usb_hub_release_port(udev->parent, udev->portnum,
 (struct usb_dev_state *) udev);
@@ -379,6 +386,9 @@ static int stub_probe(struct usb_device *udev)
 
busid_priv->sdev = NULL;
stub_device_free(sdev);
+
+call_put_busid_priv:
+   put_busid_priv(busid_priv);
return rc;
 }
 
@@ -417,7 +427,7 @@ static void stub_disconnect(struct usb_device *udev)
/* get stub_device */
if (!sdev) {
dev_err(&udev->dev, "could not get device");
-   return;
+   goto call_put_busid_priv;
}
 
dev_set_drvdata(&udev->dev, NULL);
@@ -432,12 +442,12 @@ static void stub_disconnect(struct usb_device *udev)
  (struct usb_dev_state *) udev);
if (rc) {
dev_dbg(&udev->dev, "unable to release port\n");
-   return;
+   goto call_put_busid_priv;
}
 
/* If usb reset is c

RE: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

2018-05-14 Thread Yoshihiro Shimoda
Hi Heikki,

> From: Heikki Krogerus, Sent: Monday, May 14, 2018 10:28 PM
> 
> On Mon, May 14, 2018 at 06:15:57PM +0900, Yoshihiro Shimoda wrote:

> > diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
> > index d427e80..5a0da33 100644
> > --- a/drivers/base/devcon.c
> > +++ b/drivers/base/devcon.c

> > +static int generic_graph_match(struct device *dev, void *fwnode)
> > +{
> > +   return dev->fwnode == fwnode;
> > +}
> > +
> > +/**
> > + * device_connection_find_by_graph - Find two devices connected together
> > + * @dev: Device to find connected device
> > + * @port: identifier of the @dev port node
> > + * @endpoint: identifier of the @dev endpoint node
> > + *
> > + * Find a connection with @port and @endpoint by using graph between @dev 
> > and
> > + * another device. On success returns handle to the device that is 
> > connected
> > + * to @dev, with the reference count for the found device incremented. 
> > Returns
> > + * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
> > + * a connection was found but the other device has not been enumerated yet.
> > + */
> > +struct device *device_connection_find_by_graph(struct device *dev, u32 
> > port,
> > +  u32 endpoint)
> > +{
> > +   struct bus_type *bus;
> > +   struct fwnode_handle *remote;
> > +   struct device *conn;
> > +
> > +   remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
> > +   if (!remote)
> > +   return NULL;
> > +
> > +   for (bus = generic_match_buses[0]; bus; bus++) {
> > +   conn = bus_find_device(bus, NULL, remote, generic_graph_match);
> > +   if (conn)
> > +   return conn;
> > +   }
> > +
> > +   /*
> > +* We only get called if a connection was found, tell the caller to
> > +* wait for the other device to show up.
> > +*/
> > +   return ERR_PTR(-EPROBE_DEFER);
> > +}
> > +EXPORT_SYMBOL_GPL(device_connection_find_by_graph);
> 
> Why do we need more API for walking through the graph?

I thought there is difficult to find if a device has multiple ports or 
endpoints.
So, I'd like to use port and endpoint number for finding a device.

> I'm not sure exactly sure what is going on here, I'll try to study
> your patches more when I have time, but the approach looks wrong. That
> function looks like a helper, but just not that useful one.
> 
> We really should be able to use the existing functions. In practice
> device_connection_find_match() should eventually parse the graph, then
> fallback to build-in connections if no graph is found. Otherwise
> parsing graph here is not really useful at all.

I think using device_connection_find_match() for finding the graph becomes 
complicated.
The current arguments of the function is the below:

void *device_connection_find_match(struct device *dev, const char *con_id,
   void *data,
   void *(*match)(struct device_connection *con,
  int ep, void *data))

If finding the graph, the following arguments will be not used.
 - con_id
 - *con and ep of "match" arguments.

This is because these arguments are for the build-in connections.
So, should I modify the arguments of the function for finding both
the graph and built-in connections somehow?

Best regards,
Yoshihiro Shimoda

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


Re: USB Floppy Disk Driver

2018-05-14 Thread Randy Dunlap
On 05/14/2018 05:51 PM, Joshua Hudson wrote:
> Has anybody tried the USB floppy disk driver in awhile? I just did to
> read an old disk, and I couldn't read a single byte.
> 
> I started thinking maybe the hardware's bad, but dd didn't raise en
> error even after I pulled it out and got "USB Disconnect" on the
> screen from kernel log.
> 
> I still don't know if the code is broken or if my hardware's bad. It
> worked 15 years ago with a 2.6 series kernel.
> 
> Kernel version:
> 
> joshua@nova:~ϟ uname -a
> Linux nova 4.9.0-6-amd64 #1 SMP Debian 4.9.82-1+deb9u3 (2018-03-02)
> x86_64 GNU/Linux
> joshua@nova:~ϟ
> 

Both (!) of my USB floppy drives work [on 4.16.6].


-- 
~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: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

2018-05-14 Thread David R. Bild
On Mon, May 14, 2018 at 3:08 PM, Jason Gunthorpe  wrote:
> On Mon, May 14, 2018 at 02:59:36PM -0500, David R. Bild wrote:
>> On Mon, May 14, 2018 at 2:31 PM, Jason Gunthorpe  wrote:
 The driver can setup enough to use the TPM
>> > framework to send commands before completing registration. We use it
>> > in startup timeouts and other flows today.
>>
>>
>> That sounds perfect.  Can you point me to some usages in the code (or
>> relevant functions)?
>>
>
> Well, tpm_tis_core_init() does it
>
> Only the driver that calls tpm_chip_register() gets to use this
> capability, so you can't use "tpm_tis_spi" and the automatic binding
> if you need it.
>
> Looks like the spi binding and tpm_tis_core_init will need some gentle
> editing to allow a SPI based driver the same opportunity.

Thanks for the pointers.  I'll work with Peter to add this capability
for SPI drivers.

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


Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

2018-05-14 Thread Jason Gunthorpe
On Mon, May 14, 2018 at 02:59:36PM -0500, David R. Bild wrote:
> On Mon, May 14, 2018 at 2:31 PM, Jason Gunthorpe  wrote:
> >
> > On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:
> >
> > > 3) Allow the driver to register the TPM with TPM driver, but not yet
> > > expose the TPM to userspace.  Let the driver do some additional work
> > > (like set the platform hierarchy password) and then explicitly inform
> > > the TPM driver that it is safe to expose the TPM to userspace.  This
> > > would be my preferred approach.
> >
> > We already have this. The driver can setup enough to use the TPM
> > framework to send commands before completing registration. We use it
> > in startup timeouts and other flows today.
> 
> 
> That sounds perfect.  Can you point me to some usages in the code (or
> relevant functions)?
> 
> This driver registers with the TPM subsystem using the "tpm_tis_spi"
> driver like this:
> 
> "
>   static struct spi_board_info tpm_board_info = {
>   .modalias   = "tpm_tis_spi",
>   .max_speed_hz = 43 * 1000 * 1000, // Hz
>   .chip_select   = 0,
>   .mode= SPI_MODE_0
>   };
> 
>   struct spi_device *tpm = spi_new_device(spi_master, &tpm_board_info);
> "
> 
> I don't see how sending of commands before completing registration.
> At the very least, the "tpm_tis_spi" driver probably has to be
> changed?

Well, tpm_tis_core_init() does it

Only the driver that calls tpm_chip_register() gets to use this
capability, so you can't use "tpm_tis_spi" and the automatic binding
if you need it.

Looks like the spi binding and tpm_tis_core_init will need some gentle
editing to allow a SPI based driver the same opportunity.

Jason
--
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: Race related to e04a0442d33b "HID: core: remove the absolute need of hid_have_special_driver[]"

2018-05-14 Thread Heiner Kallweit
Am 14.05.2018 um 11:58 schrieb Benjamin Tissoires:
> Hi Heiner,
> 
> On Fri, May 11, 2018 at 12:11 AM, Heiner Kallweit  
> wrote:
>> Due to some other issue with one devices supported by hid-led I figured
>> out that it's no longer needed to list devices with own driver in
>> hid_have_special_driver[].
>>
>> So I removed the entries for the hid-led devices and got the following.
>> When I plugged in the device first the device-specific driver wasn't
>> loaded. After unplugging/re-plugging the device-specific driver was
>> loaded properly.
>>
>> It seems usbhid module wasn't fully loaded and active yet when
>> hid-generic checks for a device-specific driver for the first time.
>> This also explains why the second attempt was successful.
> 
> I don't think so. When you plugged in the device, the usb core stack
> emitted a uevent requesting usbhid to be loaded. Then udev loaded
> usbhid, which created the USB-HID transport layer device immediately
> and started probing devices attached to it. The kernel emitted then a
> uevent regarding 0003:0FC5:B080, and udev answered by loading
> hid-generic. hid-generic made its initialization of the HID device,
> and then only now usbhid finished its initialization where it emits
> "USB HID core driver".
> 
> On the second attempt, the uevent is still emitted by the kernel, but
> this time udev loaded hid-led, which is detected by the hid-sstack as
> a new candidate and it takes precedence over hid-generic.
> 
> Keep in mind that the kernel doesn't load external modules, but udev
> does. So here, it seems we expected udev to load hid-led the first
> time, but it didn't.
> 
>>
>> Did you come across similar races? At a first glance I'm not sure how
>> to prevent this race. Any idea?
> 
> This is related to udev and the way the uevents are emitted.
> Maybe if we see the logs from "udevadm monitor" I'll be able to tell a
> little bit more. Meanwhile, I'll try reproducing it locally.
> 
> Also, the v4.17-rc series has seen a little cleanup in the way
> hid-generic is unbound, so it is worth a try.
> 
I used the latest linux-next kernel, so I assume these changes should
be included. When trying to create the "udevadm monitor" log I found
that I can't reproduce the issue. Hmm, strange ..
I will keep an eye on it. Until then: sorry for the noise.

Heiner

> Cheers,
> Benjamin
> 
>>
>> [   15.785205] usb 2-5: new low-speed USB device number 4 using xhci_hcd
>> [   15.917766] usb 2-5: New USB device found, idVendor=0fc5, idProduct=b080, 
>> bcdDevice= 0.1f
>> [   15.917842] usb 2-5: New USB device strings: Mfr=1, Product=2, 
>> SerialNumber=0
>> [   15.917899] usb 2-5: Product: USB IO Controller
>> [   15.917941] usb 2-5: Manufacturer: Delcom Products Inc.
>> [   15.943667] hid-generic 0003:0FC5:B080.0001: hiddev96,hidraw0: USB HID 
>> v1.00 Device [Delcom Products Inc. USB IO Controller ] on 
>> usb-:00:14.0-5/input0
>> [   15.944171] usbcore: registered new interface driver usbhid
>> [   15.944204] usbhid: USB HID core driver
>> [  101.496255] usb 2-5: USB disconnect, device number 4
>> [  107.364402] usb 2-5: new low-speed USB device number 5 using xhci_hcd
>> [  107.496582] usb 2-5: New USB device found, idVendor=0fc5, idProduct=b080, 
>> bcdDevice= 0.1f
>> [  107.496659] usb 2-5: New USB device strings: Mfr=1, Product=2, 
>> SerialNumber=0
>> [  107.496716] usb 2-5: Product: USB IO Controller
>> [  107.496757] usb 2-5: Manufacturer: Delcom Products Inc.
>> [  107.510676] hid-led 0003:0FC5:B080.0002: hidraw0: USB HID v1.00 Device 
>> [Delcom Products Inc. USB IO Controller ] on usb-:00:14.0-5/input0
>> [  107.511975] hid-led 0003:0FC5:B080.0002: Delcom Visual Signal Indicator 
>> G2 initialized
> 

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


Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

2018-05-14 Thread David R. Bild
On Mon, May 14, 2018 at 2:31 PM, Jason Gunthorpe  wrote:
>
> On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:
>
> > 3) Allow the driver to register the TPM with TPM driver, but not yet
> > expose the TPM to userspace.  Let the driver do some additional work
> > (like set the platform hierarchy password) and then explicitly inform
> > the TPM driver that it is safe to expose the TPM to userspace.  This
> > would be my preferred approach.
>
> We already have this. The driver can setup enough to use the TPM
> framework to send commands before completing registration. We use it
> in startup timeouts and other flows today.


That sounds perfect.  Can you point me to some usages in the code (or
relevant functions)?

This driver registers with the TPM subsystem using the "tpm_tis_spi"
driver like this:

"
  static struct spi_board_info tpm_board_info = {
  .modalias   = "tpm_tis_spi",
  .max_speed_hz = 43 * 1000 * 1000, // Hz
  .chip_select   = 0,
  .mode= SPI_MODE_0
  };

  struct spi_device *tpm = spi_new_device(spi_master, &tpm_board_info);
"

I don't see how sending of commands before completing registration.
At the very least, the "tpm_tis_spi" driver probably has to be
changed?

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


Re: [PATCH v3 2/2] usb: misc: xapea00x: perform platform initialization of TPM

2018-05-14 Thread Jason Gunthorpe
On Thu, May 10, 2018 at 09:41:53AM -0500, David R. Bild wrote:

> 3) Allow the driver to register the TPM with TPM driver, but not yet
> expose the TPM to userspace.  Let the driver do some additional work
> (like set the platform hierarchy password) and then explicitly inform
> the TPM driver that it is safe to expose the TPM to userspace.  This
> would be my preferred approach.

We already have this. The driver can setup enough to use the TPM
framework to send commands before completing registration. We use it
in startup timeouts and other flows today.

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

2018-05-14 Thread russianneuromancer
Hello!

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

14/05/2018 в 16:27 +0300, Mathias Nyman:
> On 11.05.2018 19:37, gert wrote:
> > 
> > Since the upgrade to Linux 4.15 (and also in 4.16), I'm
> > experiencing an issue where all my USB devices just die seemingly
> > without any cause. Both my laptop's internal (attached) keyboard as
> > well as my external keyboard die.
> > 
> > Replugging the external keyboard unfortunately does not solve the
> > problem. My touchpad, on the other hand, continues to work, though
> > it may internally be connected via PS/2.
> > 
> > After this happens, I have only been able to solve it by rebooting.
> > 
> > In the logs, the following error can be found.
> > 
> > xhci_hcd :3d:00.0: xHCI host controller not responding, assume
> > dead
> > 
> > Previously, similar issues occurred to users that could be fixed by
> > adding intel_iommu=false to the kernel parameters. This however
> > seems to be a different problem, as it newly occurs in this
> > specific kernel version and is not solved by the aforementioned
> > solution.
> > 
> > This was also posted at the Archlinux forums [1], where we managed
> > to pin down the issue to being related to xHCI and autosuspend
> > (power management). I'm using powertop's --auto-tune and disabling
> > the "good" setting for all xHCI controllers again makes the issue
> > disappear. Linux 4.14 and lower also do not expose this issue.
> > 
> > Please also find attached the complete journalctl output of one
> > boot from start to finish that exposed the issue, which may be
> > helpful during debugging. [1]
> > 
> 
> Hi
> 
> I got a report about a very similar issue, thread can be found at:
> https://marc.info/?l=linux-usb&m=152335174903017&w=2
> 
> I didn't get any feedback about the suggested patch.
> If you can test it, either by just by compiling my for-usb-linus
> branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git for-
> usb-linus
> 
> or alternatively applying the attached patch I would appreciate it.
> 
> If it doesn't help then we need to dig deeper into it, woith more
> detailed logs.
> 
> Thanks
> Mathias
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Enabling USB (auto)suspend for xHCI controllers incurs random device failures since kernel 4.15

2018-05-14 Thread russianneuromancer
Hello!

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

14/05/2018 в 16:27 +0300, Mathias Nyman:
> On 11.05.2018 19:37, gert wrote:
> > 
> > Since the upgrade to Linux 4.15 (and also in 4.16), I'm
> > experiencing an issue where all my USB devices just die seemingly
> > without any cause. Both my laptop's internal (attached) keyboard as
> > well as my external keyboard die.
> > 
> > Replugging the external keyboard unfortunately does not solve the
> > problem. My touchpad, on the other hand, continues to work, though
> > it may internally be connected via PS/2.
> > 
> > After this happens, I have only been able to solve it by rebooting.
> > 
> > In the logs, the following error can be found.
> > 
> > xhci_hcd :3d:00.0: xHCI host controller not responding, assume
> > dead
> > 
> > Previously, similar issues occurred to users that could be fixed by
> > adding intel_iommu=false to the kernel parameters. This however
> > seems to be a different problem, as it newly occurs in this
> > specific kernel version and is not solved by the aforementioned
> > solution.
> > 
> > This was also posted at the Archlinux forums [1], where we managed
> > to pin down the issue to being related to xHCI and autosuspend
> > (power management). I'm using powertop's --auto-tune and disabling
> > the "good" setting for all xHCI controllers again makes the issue
> > disappear. Linux 4.14 and lower also do not expose this issue.
> > 
> > Please also find attached the complete journalctl output of one
> > boot from start to finish that exposed the issue, which may be
> > helpful during debugging. [1]
> > 
> 
> Hi
> 
> I got a report about a very similar issue, thread can be found at:
> https://marc.info/?l=linux-usb&m=152335174903017&w=2
> 
> I didn't get any feedback about the suggested patch.
> If you can test it, either by just by compiling my for-usb-linus
> branch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git for-
> usb-linus
> 
> or alternatively applying the attached patch I would appreciate it.
> 
> If it doesn't help then we need to dig deeper into it, woith more
> detailed logs.
> 
> Thanks
> Mathias
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

2018-05-14 Thread Randy Dunlap
On 05/14/2018 02:15 AM, Yoshihiro Shimoda wrote:
> This patch adds a new API "device_connection_find_by_graph()" to
> find device connection by using graph.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/driver-api/device_connection.rst |  4 +--
>  drivers/base/devcon.c  | 43 
> ++
>  include/linux/device.h |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/device_connection.rst 
> b/Documentation/driver-api/device_connection.rst
> index affbc556..2e2d26f 100644
> --- a/Documentation/driver-api/device_connection.rst
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between 
> the two devices.
>  They are only descriptions which are not tied to either of the devices 
> directly.
>  A dependency between the two devices exists only if one of the two endpoint
>  devices requests a reference to the other. The descriptions themselves can be
> -defined in firmware (not yet supported) or they can be built-in.
> +defined in firmware or they can be built-in.
>  
>  Usage
>  -
> @@ -40,4 +40,4 @@ API
>  ---
>  
>  .. kernel-doc:: drivers/base/devcon.c
> -   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove
> +   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove device_connection_find_by_graph

BTW, that line above should be like:
  :functions: ...
i.e., no space after the first ':'.

I have sent a patch for that and Jon Corbet has applied it to his docs tree.

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


[PATCH] musb fixes for v4.17-rc6

2018-05-14 Thread Bin Liu
Hi Greg,

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

Regards,
-Bin.


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

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

-- 
1.9.1

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


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

2018-05-14 Thread Bin Liu
From: Daniel Glöckner 

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

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

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

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

Re: [PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

2018-05-14 Thread Heikki Krogerus
On Mon, May 14, 2018 at 06:15:57PM +0900, Yoshihiro Shimoda wrote:
> This patch adds a new API "device_connection_find_by_graph()" to
> find device connection by using graph.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/driver-api/device_connection.rst |  4 +--
>  drivers/base/devcon.c  | 43 
> ++
>  include/linux/device.h |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/device_connection.rst 
> b/Documentation/driver-api/device_connection.rst
> index affbc556..2e2d26f 100644
> --- a/Documentation/driver-api/device_connection.rst
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -19,7 +19,7 @@ Device connections alone do not create a dependency between 
> the two devices.
>  They are only descriptions which are not tied to either of the devices 
> directly.
>  A dependency between the two devices exists only if one of the two endpoint
>  devices requests a reference to the other. The descriptions themselves can be
> -defined in firmware (not yet supported) or they can be built-in.
> +defined in firmware or they can be built-in.
>  
>  Usage
>  -
> @@ -40,4 +40,4 @@ API
>  ---
>  
>  .. kernel-doc:: drivers/base/devcon.c
> -   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove
> +   : functions: device_connection_find_match device_connection_find 
> device_connection_add device_connection_remove device_connection_find_by_graph
> diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
> index d427e80..5a0da33 100644
> --- a/drivers/base/devcon.c
> +++ b/drivers/base/devcon.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  static DEFINE_MUTEX(devcon_lock);
>  static LIST_HEAD(devcon_list);
> @@ -134,3 +135,45 @@ void device_connection_remove(struct device_connection 
> *con)
>   mutex_unlock(&devcon_lock);
>  }
>  EXPORT_SYMBOL_GPL(device_connection_remove);
> +
> +static int generic_graph_match(struct device *dev, void *fwnode)
> +{
> + return dev->fwnode == fwnode;
> +}
> +
> +/**
> + * device_connection_find_by_graph - Find two devices connected together
> + * @dev: Device to find connected device
> + * @port: identifier of the @dev port node
> + * @endpoint: identifier of the @dev endpoint node
> + *
> + * Find a connection with @port and @endpoint by using graph between @dev and
> + * another device. On success returns handle to the device that is connected
> + * to @dev, with the reference count for the found device incremented. 
> Returns
> + * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
> + * a connection was found but the other device has not been enumerated yet.
> + */
> +struct device *device_connection_find_by_graph(struct device *dev, u32 port,
> +u32 endpoint)
> +{
> + struct bus_type *bus;
> + struct fwnode_handle *remote;
> + struct device *conn;
> +
> + remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
> + if (!remote)
> + return NULL;
> +
> + for (bus = generic_match_buses[0]; bus; bus++) {
> + conn = bus_find_device(bus, NULL, remote, generic_graph_match);
> + if (conn)
> + return conn;
> + }
> +
> + /*
> +  * We only get called if a connection was found, tell the caller to
> +  * wait for the other device to show up.
> +  */
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL_GPL(device_connection_find_by_graph);

Why do we need more API for walking through the graph?

I'm not sure exactly sure what is going on here, I'll try to study
your patches more when I have time, but the approach looks wrong. That
function looks like a helper, but just not that useful one.

We really should be able to use the existing functions. In practice
device_connection_find_match() should eventually parse the graph, then
fallback to build-in connections if no graph is found. Otherwise
parsing graph here is not really useful at all.


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

2018-05-14 Thread Mathias Nyman

On 11.05.2018 19:37, gert wrote:


Since the upgrade to Linux 4.15 (and also in 4.16), I'm experiencing an issue 
where all my USB devices just die seemingly without any cause. Both my laptop's 
internal (attached) keyboard as well as my external keyboard die.

Replugging the external keyboard unfortunately does not solve the problem. My 
touchpad, on the other hand, continues to work, though it may internally be 
connected via PS/2.

After this happens, I have only been able to solve it by rebooting.

In the logs, the following error can be found.

xhci_hcd :3d:00.0: xHCI host controller not responding, assume dead

Previously, similar issues occurred to users that could be fixed by adding 
intel_iommu=false to the kernel parameters. This however seems to be a 
different problem, as it newly occurs in this specific kernel version and is 
not solved by the aforementioned solution.

This was also posted at the Archlinux forums [1], where we managed to pin down the issue 
to being related to xHCI and autosuspend (power management). I'm using powertop's 
--auto-tune and disabling the "good" setting for all xHCI controllers again 
makes the issue disappear. Linux 4.14 and lower also do not expose this issue.

Please also find attached the complete journalctl output of one boot from start 
to finish that exposed the issue, which may be helpful during debugging. [1]



Hi

I got a report about a very similar issue, thread can be found at:
https://marc.info/?l=linux-usb&m=152335174903017&w=2

I didn't get any feedback about the suggested patch.
If you can test it, either by just by compiling my for-usb-linus branch:

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

or alternatively applying the attached patch I would appreciate it.

If it doesn't help then we need to dig deeper into it, woith more detailed logs.

Thanks
Mathias

>From d5fb6d354cd14d962d9f790f782ace0d06bf5002 Mon Sep 17 00:00:00 2001
From: Mathias Nyman 
Date: Fri, 4 May 2018 15:38:34 +0300
Subject: [PATCH] xhci: Fix perceived dead host due to runtime suspend race
 with event handler

Don't rely on event interrupt (EINT) bit alone to detect pending port
change in resume. If no change event is detected the host may be suspended
again, oterwise roothubs are resumed.

There is a lag in xHC setting EINT. If we don't notice the pending change
in resume, and the controller is runtime suspeded again, it causes the
event handler to assume host is dead as it will fail to read xHC registers
once PCI puts the controller to D3 state.

[  268.520969] xhci_hcd: xhci_resume: starting port polling.
[  268.520985] xhci_hcd: xhci_hub_status_data: stopping port polling.
[  268.521030] xhci_hcd: xhci_suspend: stopping port polling.
[  268.521040] xhci_hcd: // Setting command ring address to 0x349bd001
[  268.521139] xhci_hcd: Port Status Change Event for port 3
[  268.521149] xhci_hcd: resume root hub
[  268.521163] xhci_hcd: port resume event for port 3
[  268.521168] xhci_hcd: xHC is not running.
[  268.521174] xhci_hcd: handle_port_status: starting port polling.
[  268.596322] xhci_hcd: xhci_hc_died: xHCI host controller not responding, assume dead

The EINT lag is described in a additional note in xhci specs 4.19.2:

"Due to internal xHC scheduling and system delays, there will be a lag
between a change bit being set and the Port Status Change Event that it
generated being written to the Event Ring. If SW reads the PORTSC and
sees a change bit set, there is no guarantee that the corresponding Port
Status Change Event has already been written into the Event Ring."

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 37 ++---
 drivers/usb/host/xhci.h |  4 
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 711da33..18e9fcf 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -844,6 +844,38 @@ static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci)
 	spin_unlock_irqrestore(&xhci->lock, flags);
 }
 
+static bool xhci_pending_portevent(struct xhci_hcd *xhci)
+{
+	int	port_index;
+	u32	status;
+	u32	portsc;
+
+	status = readl(&xhci->op_regs->status);
+	if (status & STS_EINT)
+		return true;
+	/*
+	 * Checking STS_EINT is not enough as there is a lag between a change
+	 * bit being set and the Port Status Change Event that it generated
+	 * being written to the Event Ring. See note in xhci 1.1 section 4.19.2.
+	 */
+
+	port_index = xhci->num_usb2_ports;
+	while (port_index--) {
+		portsc = readl(xhci->usb2_ports[port_index]);
+		if (portsc & PORT_CHANGE_MASK ||
+		(portsc & PORT_PLS_MASK) == XDEV_RESUME)
+			return true;
+	}
+	port_index = xhci->num_usb3_ports;
+	while (port_index--) {
+		portsc = readl(xhci->usb3_ports[port_index]);
+		if (portsc & PORT_CHANGE_MASK ||
+		(portsc & PORT_PLS_MASK) == XDEV_RESUME)
+			return true;
+	}
+	return false;
+}
+
 /*
  * Stop HC (not bus-specific)
  

Re: [RFC PATCH v3 0/5] usb: typec: Support for Alternate Modes

2018-05-14 Thread Heikki Krogerus
On Sat, May 12, 2018 at 02:42:47PM -0700, Guenter Roeck wrote:
> On 05/11/2018 06:18 AM, Heikki Krogerus wrote:
> > Hi,
> > 
> > This is the third version of my proposal for more complete alternate
> > mode support. In this version I'm including a proposal for the mux
> > handling. Basically, I'm proposing that every supported alternate will
> > have its own mux handle. That should allow us to support multiple
> > alternate modes at the same time. There is also a small change to how
> > I handled enter/exit mode commands. Now the alternate mode drivers
> > will need to check if Enter Mode command ACK/NAK. The ->enter callback
> > is not called in those cases separately. The typec_altmode_enter/exit
> > functions are used only when the command is initiated. Other than
> > that, only minor tuning.
> > 
> 
> I like the both the idea and the approach.

Thanks! :-)

> I browsed through the code, but I don't see anything obviously wrong
> with it. Too bad I wont have the time for an actual alternate mode
> implementation. Are you working on one, by any chance ? I would like
> to see this move forward, and an actual implementation would help
> to get there.

Yes, an alt mode implementation is definitely needed. I am working on
something, but I don't think I'm able to share anything before next
week unfortunately.


Thanks Guenter,

-- 
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: [RFC PATCH v3 3/5] usb: typec: Bus type for alternate modes

2018-05-14 Thread Heikki Krogerus
On Sat, May 12, 2018 at 06:25:09PM -0700, Randy Dunlap wrote:
> On 05/11/2018 06:18 AM, Heikki Krogerus wrote:
> > Introducing a simple bus for the alternate modes. Bus allows
> > binding drivers to the discovered alternate modes the
> > partners support.
> > 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >  Documentation/ABI/obsolete/sysfs-class-typec |  48 +++
> >  Documentation/ABI/testing/sysfs-bus-typec|  51 +++
> >  Documentation/ABI/testing/sysfs-class-typec  |  62 +--
> >  Documentation/driver-api/usb/typec_bus.rst   | 136 ++
> >  drivers/usb/typec/Makefile   |   2 +-
> >  drivers/usb/typec/bus.c  | 423 +++
> >  drivers/usb/typec/bus.h  |  38 ++
> >  drivers/usb/typec/class.c| 364 
> >  include/linux/mod_devicetable.h  |  15 +
> >  include/linux/usb/typec.h|  14 +-
> >  include/linux/usb/typec_altmode.h| 142 +++
> >  scripts/mod/devicetable-offsets.c|   4 +
> >  scripts/mod/file2alias.c |  13 +
> >  13 files changed, 1168 insertions(+), 144 deletions(-)
> >  create mode 100644 Documentation/ABI/obsolete/sysfs-class-typec
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-typec
> >  create mode 100644 Documentation/driver-api/usb/typec_bus.rst
> >  create mode 100644 drivers/usb/typec/bus.c
> >  create mode 100644 drivers/usb/typec/bus.h
> >  create mode 100644 include/linux/usb/typec_altmode.h
> 
> Hi,
> I have a few doc corrections for you.

Thanks, I'll fix all of them as you proposed.

> > diff --git a/Documentation/driver-api/usb/typec_bus.rst 
> > b/Documentation/driver-api/usb/typec_bus.rst
> > new file mode 100644
> > index ..4184e0925567
> > --- /dev/null
> > +++ b/Documentation/driver-api/usb/typec_bus.rst
> > @@ -0,0 +1,136 @@
> > +
> > +API for USB Type-C Alternate Mode drivers
> > +=
> > +
> > +Introduction
> > +
> > +
> > +Alternate modes require communication with the partner using Vendor Defined
> > +Messages (VDM) as defined in USB Type-C and USB Power Delivery 
> > Specifications.
> > +The communication is SVID (Standard or Vendor ID) specific, i.e. specific 
> > for
> > +every alternate mode, so every alternate mode will need custom driver.
> 
>a custom driver.
> 
> > +
> > +USB Type-C bus allows binding a driver to the discovered partner alternate
> > +modes by using the SVID and the mode number.
> > +
> > +USB Type-C Connector Class provides a device for every alternate mode a 
> > port
> > +supports, and separate device for every alternate mode the partner 
> > supports.
> > +The drivers for the alternate modes are bind to the partner alternate mode
> 
>are bound
> or just:   bind
> 
> > +devices, and the port alternate mode devices must be handled by the port
> > +drivers.
> > +
> > +When a new partner alternate mode device is registered, it is linked to the
> > +alternate mode device of the port that the partner is attached to, that has
> > +matching SVID and mode. Communication between the port driver and 
> > alternate mode
> > +driver will happen using the same API.
> > +
> > +The port alternate mode devices are used as a proxy between the partner 
> > and the
> > +alternate mode drivers, so the port drivers are only expected to pass the 
> > SVID
> > +specific commands from the alternate mode drivers to the partner, and from 
> > the
> > +partners to the alternate mode drivers. No direct SVID specific 
> > communication is
> > +needed from the port drivers, but the port drivers need to provide the 
> > operation
> > +callbacks for the port alternate mode devices, just like the alternate mode
> > +drivers need to provide them for the partner alternate mode devices.
> > +
> > +Usage:
> > +--
> > +
> > +General
> > +~~~
> > +
> > +By default, the alternate mode drivers are responsible for entering the 
> > mode.
> > +It is also possible to leave the decision about entering the mode to the 
> > user
> > +space (See Documentation/ABI/testing/sysfs-class-typec). Port drivers 
> > should not
> > +enter any modes on their own.
> > +
> > +``->vdm`` is the most important callback in the vector. It will be used to
> > +deliver all the SVID specific commands from the partner to the alternate 
> > mode
> > +driver, and vise versa in case of port drivers. The drivers send the SVID
> 
>vice versa
> 
> > +specific commands to each other using :c:func:`typec_altmode_vmd()`.
> > +
> > +If the communication with the partner using the SVID specific commands 
> > results
> > +in need to re-configure the pins on the connector, the alternate mode 
> > driver
> 
>   reconfigure
> 
> > +needs to notify the bus using :c:func:`typec_altmode_notify()`. The driver
> > +passes

Re: [RFC PATCH v3 5/5] usb: typec: tcpm: Support for Alternate Modes

2018-05-14 Thread Heikki Krogerus
On Sun, May 13, 2018 at 12:30:51AM +0300, Andy Shevchenko wrote:
> On Fri, May 11, 2018 at 4:18 PM, Heikki Krogerus
>  wrote:
> > This adds more complete handling of VDMs and registration of
> > partner alternate modes, and introduces callbacks for
> > alternate mode operations.
> >
> > Only DFP role is supported for now.
> 
> > +   for (i = 0; i < cnt; i++)
> > +   p[i] = le32_to_cpu(payload[i]);
> 
> I would recommend to consider to use le32_to_cpu_array().
> 
> Though, actually we have slightly different API for BE and LE cases.
> For LE existing would be renamed to rather le32_to_cpus_array() and
> establishing the former one in the similar way how be32_to_cpu_array()
> is implemented.

OK. I'll check it.


Thanks Andy,

-- 
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: Race related to e04a0442d33b "HID: core: remove the absolute need of hid_have_special_driver[]"

2018-05-14 Thread Benjamin Tissoires
Hi Heiner,

On Fri, May 11, 2018 at 12:11 AM, Heiner Kallweit  wrote:
> Due to some other issue with one devices supported by hid-led I figured
> out that it's no longer needed to list devices with own driver in
> hid_have_special_driver[].
>
> So I removed the entries for the hid-led devices and got the following.
> When I plugged in the device first the device-specific driver wasn't
> loaded. After unplugging/re-plugging the device-specific driver was
> loaded properly.
>
> It seems usbhid module wasn't fully loaded and active yet when
> hid-generic checks for a device-specific driver for the first time.
> This also explains why the second attempt was successful.

I don't think so. When you plugged in the device, the usb core stack
emitted a uevent requesting usbhid to be loaded. Then udev loaded
usbhid, which created the USB-HID transport layer device immediately
and started probing devices attached to it. The kernel emitted then a
uevent regarding 0003:0FC5:B080, and udev answered by loading
hid-generic. hid-generic made its initialization of the HID device,
and then only now usbhid finished its initialization where it emits
"USB HID core driver".

On the second attempt, the uevent is still emitted by the kernel, but
this time udev loaded hid-led, which is detected by the hid-sstack as
a new candidate and it takes precedence over hid-generic.

Keep in mind that the kernel doesn't load external modules, but udev
does. So here, it seems we expected udev to load hid-led the first
time, but it didn't.

>
> Did you come across similar races? At a first glance I'm not sure how
> to prevent this race. Any idea?

This is related to udev and the way the uevents are emitted.
Maybe if we see the logs from "udevadm monitor" I'll be able to tell a
little bit more. Meanwhile, I'll try reproducing it locally.

Also, the v4.17-rc series has seen a little cleanup in the way
hid-generic is unbound, so it is worth a try.

Cheers,
Benjamin

>
> [   15.785205] usb 2-5: new low-speed USB device number 4 using xhci_hcd
> [   15.917766] usb 2-5: New USB device found, idVendor=0fc5, idProduct=b080, 
> bcdDevice= 0.1f
> [   15.917842] usb 2-5: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=0
> [   15.917899] usb 2-5: Product: USB IO Controller
> [   15.917941] usb 2-5: Manufacturer: Delcom Products Inc.
> [   15.943667] hid-generic 0003:0FC5:B080.0001: hiddev96,hidraw0: USB HID 
> v1.00 Device [Delcom Products Inc. USB IO Controller ] on 
> usb-:00:14.0-5/input0
> [   15.944171] usbcore: registered new interface driver usbhid
> [   15.944204] usbhid: USB HID core driver
> [  101.496255] usb 2-5: USB disconnect, device number 4
> [  107.364402] usb 2-5: new low-speed USB device number 5 using xhci_hcd
> [  107.496582] usb 2-5: New USB device found, idVendor=0fc5, idProduct=b080, 
> bcdDevice= 0.1f
> [  107.496659] usb 2-5: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=0
> [  107.496716] usb 2-5: Product: USB IO Controller
> [  107.496757] usb 2-5: Manufacturer: Delcom Products Inc.
> [  107.510676] hid-led 0003:0FC5:B080.0002: hidraw0: USB HID v1.00 Device 
> [Delcom Products Inc. USB IO Controller ] on usb-:00:14.0-5/input0
> [  107.511975] hid-led 0003:0FC5:B080.0002: Delcom Visual Signal Indicator G2 
> initialized
--
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 14/14] staging: typec: tcpci: move tcpci driver out of staging

2018-05-14 Thread Jun Li
Hi
> -Original Message-
> From: Mats Karrman [mailto:mats.dev.l...@gmail.com]
> Sent: 2018年5月12日 5:37
> 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 14/14] staging: typec: tcpci: move tcpci driver out of
> staging
> 
> Hi Li Jun,
> 
> This patch takes away building the entire staging/typec tree but this is 
> still not
> empty after your patch, another driver "tcpci_rt1711h" is there.
> Better just remove tcpci from staging/typec/{Kconfig,Makefile}

Oh, yes, I should also move out tcpci_rt1711h driver, will update in next 
version.

Thanks
Li Jun
> 
> // Mats
> 
> On 2018-05-03 02:24, Li Jun wrote:
> 
> > Move TCPCI(Typec port controller interface) driver out of staging.
> >
> > Signed-off-by: Li Jun 
> > ---
> >   drivers/staging/Kconfig| 2 --
> >   drivers/staging/Makefile   | 1 -
> >   drivers/staging/typec/TODO | 5 -
> >   drivers/usb/typec/Kconfig  | 7 +++
> >   drivers/usb/typec/Makefile | 1 +
> >   drivers/{staging => usb}/typec/tcpci.c | 0
> >   drivers/{staging => usb}/typec/tcpci.h | 0
> >   7 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig index
> > d5926f0..d83ff66 100644
> > --- a/drivers/staging/Kconfig
> > +++ b/drivers/staging/Kconfig
> > @@ -112,8 +112,6 @@ source "drivers/staging/greybus/Kconfig"
> >
> >   source "drivers/staging/vc04_services/Kconfig"
> >
> > -source "drivers/staging/typec/Kconfig"
> > -
> >   source "drivers/staging/vboxvideo/Kconfig"
> >
> >   source "drivers/staging/pi433/Kconfig"
> > diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile index
> > 919753c..a71ec1f 100644
> > --- a/drivers/staging/Makefile
> > +++ b/drivers/staging/Makefile
> > @@ -2,7 +2,6 @@
> >   # Makefile for staging directory
> >
> >   obj-y += media/
> > -obj-y  += typec/
> >   obj-$(CONFIG_IPX) += ipx/
> >   obj-$(CONFIG_NCP_FS)  += ncpfs/
> >   obj-$(CONFIG_PRISM2_USB)  += wlan-ng/
> > diff --git a/drivers/staging/typec/TODO b/drivers/staging/typec/TODO
> > deleted file mode 100644 index 53fe2f7..000
> > --- a/drivers/staging/typec/TODO
> > +++ /dev/null
> > @@ -1,5 +0,0 @@
> > -tcpci:
> > -- Test with real hardware
> > -
> > -Please send patches to Guenter Roeck  and copy
> > -Heikki Krogerus .
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 2c8eab1..0a862fc 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -56,6 +56,13 @@ config TYPEC_TCPM
> >
> >   if TYPEC_TCPM
> >
> > +config TYPEC_TCPCI
> > +   tristate "Type-C Port Controller Interface driver"
> > +   depends on I2C
> > +   select REGMAP_I2C
> > +   help
> > + Type-C Port Controller driver for TCPCI-compliant controller.
> > +
> >   source "drivers/usb/typec/fusb302/Kconfig"
> >
> >   config TYPEC_WCOVE
> > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> > index 1f599a6..02758a1 100644
> > --- a/drivers/usb/typec/Makefile
> > +++ b/drivers/usb/typec/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_TYPEC_WCOVE)   += typec_wcove.o
> >   obj-$(CONFIG_TYPEC_UCSI)  += ucsi/
> >   obj-$(CONFIG_TYPEC_TPS6598X)  += tps6598x.o
> >   obj-$(CONFIG_TYPEC)   += mux/
> > +obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
> > diff --git a/drivers/staging/typec/tcpci.c b/drivers/usb/typec/tcpci.c
> > similarity index 100% rename from drivers/staging/typec/tcpci.c rename
> > to drivers/usb/typec/tcpci.c diff --git
> > a/drivers/staging/typec/tcpci.h b/drivers/usb/typec/tcpci.h similarity
> > index 100% rename from drivers/staging/typec/tcpci.h rename to
> > drivers/usb/typec/tcpci.h



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

2018-05-14 Thread Jun Li
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 
> >   #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.

> 
> > + * @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"

Thanks
Li Jun
> 
> BR // Mats
> 
> > + * @name: power role string
> > + *
> > + * This routine is used to find the typec_role by its string name of
> > + * preferred power role(Try.SRC or Try.SNK).
> > + *
> > + * Returns typec_role if success, otherwise negative error code.
> > + */
> > +int typec_find_preferred_role(const char *name) {
> > +   return match_string(typec_roles, ARRAY_SIZE(typec_roles), name); }
> > +EXPORT_SYMBOL_GPL(typec_find_preferred_role);
> > +
> > +/**
> > + * typec_find_data_type - Get the typec port data capability
> > + * @name: data type string
> > + *
> > + * This routine is used to find the typec_port_data by its string name.
> > + *
> > + * Returns typec_port_data if success, otherwise negative error code.
> > + */
> > +int typec_find_data_type(const char *name) {
> > +   return match_string(typec_data_types, ARRAY_SIZE(typec_data_types),
> > +   name);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_find_data_type);
> > +
> >   /* -- */
> >   /* API for Multiplexer/DeMultiplexer Switches */
> >
> > diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> > index 672b39b..00c93e7 100644
> > --- a/include/linux/usb/typec.h
> > +++ b/include/linux/usb/typec.h
> > @@ -267,4 +267,7 @@ int typec_set_orientation(struct typec_port *port,
> >   enum typec_orientation orientation);
> >   int typec_set_mode(struct typec_port *port, int mode);
> >
> > +int typec_find_power_type(const char *name); int
> > +typec_find_preferred_role(const char *name); int
> > +typec_find_data_type(const char *name);
> >   #endif /* __LINUX_USB_TYPEC_H */
> >


[PATCH/RFC v3 1/4] base: devcon: add a new API to find the graph

2018-05-14 Thread Yoshihiro Shimoda
This patch adds a new API "device_connection_find_by_graph()" to
find device connection by using graph.

Signed-off-by: Yoshihiro Shimoda 
---
 Documentation/driver-api/device_connection.rst |  4 +--
 drivers/base/devcon.c  | 43 ++
 include/linux/device.h |  2 ++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/device_connection.rst 
b/Documentation/driver-api/device_connection.rst
index affbc556..2e2d26f 100644
--- a/Documentation/driver-api/device_connection.rst
+++ b/Documentation/driver-api/device_connection.rst
@@ -19,7 +19,7 @@ Device connections alone do not create a dependency between 
the two devices.
 They are only descriptions which are not tied to either of the devices 
directly.
 A dependency between the two devices exists only if one of the two endpoint
 devices requests a reference to the other. The descriptions themselves can be
-defined in firmware (not yet supported) or they can be built-in.
+defined in firmware or they can be built-in.
 
 Usage
 -
@@ -40,4 +40,4 @@ API
 ---
 
 .. kernel-doc:: drivers/base/devcon.c
-   : functions: device_connection_find_match device_connection_find 
device_connection_add device_connection_remove
+   : functions: device_connection_find_match device_connection_find 
device_connection_add device_connection_remove device_connection_find_by_graph
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index d427e80..5a0da33 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 
 static DEFINE_MUTEX(devcon_lock);
 static LIST_HEAD(devcon_list);
@@ -134,3 +135,45 @@ void device_connection_remove(struct device_connection 
*con)
mutex_unlock(&devcon_lock);
 }
 EXPORT_SYMBOL_GPL(device_connection_remove);
+
+static int generic_graph_match(struct device *dev, void *fwnode)
+{
+   return dev->fwnode == fwnode;
+}
+
+/**
+ * device_connection_find_by_graph - Find two devices connected together
+ * @dev: Device to find connected device
+ * @port: identifier of the @dev port node
+ * @endpoint: identifier of the @dev endpoint node
+ *
+ * Find a connection with @port and @endpoint by using graph between @dev and
+ * another device. On success returns handle to the device that is connected
+ * to @dev, with the reference count for the found device incremented. Returns
+ * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when
+ * a connection was found but the other device has not been enumerated yet.
+ */
+struct device *device_connection_find_by_graph(struct device *dev, u32 port,
+  u32 endpoint)
+{
+   struct bus_type *bus;
+   struct fwnode_handle *remote;
+   struct device *conn;
+
+   remote = fwnode_graph_get_remote_node(dev_fwnode(dev), port, endpoint);
+   if (!remote)
+   return NULL;
+
+   for (bus = generic_match_buses[0]; bus; bus++) {
+   conn = bus_find_device(bus, NULL, remote, generic_graph_match);
+   if (conn)
+   return conn;
+   }
+
+   /*
+* We only get called if a connection was found, tell the caller to
+* wait for the other device to show up.
+*/
+   return ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL_GPL(device_connection_find_by_graph);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99..58f8544 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -751,6 +751,8 @@ void *device_connection_find_match(struct device *dev, 
const char *con_id,
 
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
+struct device *device_connection_find_by_graph(struct device *dev, u32 port,
+  u32 endpoint);
 
 /**
  * enum device_link_state - Device link states.
-- 
1.9.1

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


[PATCH/RFC v3 4/4] arm64: dts: renesas: r8a7795: add OF graph for usb role switch

2018-05-14 Thread Yoshihiro Shimoda
This patch adds OF graph properties for usb role switch in r8a7795
into USB3.0 host/peripheral nodes.

TODO:
 - need patches for other SoCs.

Signed-off-by: Yoshihiro Shimoda 
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 1d5e3ac..50d3312 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1746,6 +1746,12 @@
power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
resets = <&cpg 328>;
status = "disabled";
+
+   port {
+   usb3_host0_ep: endpoint {
+   remote-endpoint = <&usb3_peri0_ep>;
+   };
+   };
};
 
usb3_peri0: usb@ee02 {
@@ -1757,6 +1763,12 @@
power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
resets = <&cpg 328>;
status = "disabled";
+
+   port {
+   usb3_peri0_ep: endpoint {
+   remote-endpoint = <&usb3_host0_ep>;
+   };
+   };
};
 
usb_dmac0: dma-controller@e65a {
-- 
1.9.1

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


[PATCH/RFC v3 3/4] usb: gadget: udc: renesas_usb3: use usb role switch API

2018-05-14 Thread Yoshihiro Shimoda
This patch uses usb role switch API if the register suceeeded.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/gadget/udc/renesas_usb3.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index c878449..bfb5803 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -657,7 +657,11 @@ static void _usb3_set_mode(struct renesas_usb3 *usb3, bool 
host)
 
 static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 {
-   _usb3_set_mode(usb3, host);
+   if (usb3->role_sw)
+   usb_role_switch_set_role(usb3->role_sw, host ?
+USB_ROLE_HOST : USB_ROLE_DEVICE);
+   else
+   _usb3_set_mode(usb3, host);
 }
 
 static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable)
@@ -672,8 +676,8 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, 
bool host, bool a_dev)
 {
unsigned long flags;
 
-   spin_lock_irqsave(&usb3->lock, flags);
usb3_set_mode(usb3, host);
+   spin_lock_irqsave(&usb3->lock, flags);
usb3_vbus_out(usb3, a_dev);
/* for A-Peripheral or forced B-device mode */
if ((!host && a_dev) ||
-- 
1.9.1

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


[PATCH/RFC v3 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

2018-05-14 Thread Yoshihiro Shimoda
This patch adds role switch support for R-Car SoCs into the USB
3.0 peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB
3.0 dual-role device controller which has the USB 3.0 xHCI host
and Renesas USB 3.0 peripheral.

Unfortunately, the mode change register contains the USB 3.0 peripheral
controller side only. So, the USB 3.0 peripheral driver (renesas_usb3)
manages this register now. However, in peripheral mode, the host
should stop. Also the host hardware needs to reinitialize its own
registers when the mode changes from peripheral to host mode.
Otherwise, the host cannot work correctly (e.g. detect a device as
high-speed).

To achieve this by a driver, this role switch driver manages
the mode change register and attach/release the xhci-plat driver.

Signed-off-by: Yoshihiro Shimoda 
---
 .../devicetree/bindings/usb/renesas_usb3.txt   | 15 +
 drivers/usb/gadget/udc/Kconfig |  1 +
 drivers/usb/gadget/udc/renesas_usb3.c  | 68 +-
 3 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt 
b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 2c071bb5..f6105aa 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -19,6 +19,9 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - The connection to a usb3.0 host node needs by using OF graph bindings for
+usb role switch.
+   - port@0 = USB3.0 host port.
 
 Example of R-Car H3 ES1.x:
usb3_peri0: usb@ee02 {
@@ -27,6 +30,12 @@ Example of R-Car H3 ES1.x:
reg = <0 0xee02 0 0x400>;
interrupts = ;
clocks = <&cpg CPG_MOD 328>;
+
+   port {
+   usb3_peri0_ep: endpoint {
+   remote-endpoint = <&usb3_host0_ep>;
+   };
+   };
};
 
usb3_peri1: usb@ee06 {
@@ -35,4 +44,10 @@ Example of R-Car H3 ES1.x:
reg = <0 0xee06 0 0x400>;
interrupts = ;
clocks = <&cpg CPG_MOD 327>;
+
+   port {
+   usb3_peri1_ep: endpoint {
+   remote-endpoint = <&usb3_host1_ep>;
+   };
+   };
};
diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 0875d38..7e4a5dd 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -193,6 +193,7 @@ config USB_RENESAS_USB3
tristate 'Renesas USB3.0 Peripheral controller'
depends on ARCH_RENESAS || COMPILE_TEST
depends on EXTCON && HAS_DMA
+   select USB_ROLE_SWITCH
help
   Renesas USB3.0 Peripheral controller is a USB peripheral controller
   that supports super, high, and full speed USB 3.0 data transfers.
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 409cde4..c878449 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* register definitions */
 #define USB3_AXI_INT_STA   0x008
@@ -334,6 +335,9 @@ struct renesas_usb3 {
struct work_struct extcon_work;
struct phy *phy;
 
+   struct usb_role_switch *role_sw;
+   struct device *host_dev;
+
struct renesas_usb3_ep *usb3_ep;
int num_usb3_eps;
 
@@ -643,7 +647,7 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3)
}
 }
 
-static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
+static void _usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 {
if (host)
usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
@@ -651,6 +655,11 @@ static void usb3_set_mode(struct renesas_usb3 *usb3, bool 
host)
usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
 }
 
+static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
+{
+   _usb3_set_mode(usb3, host);
+}
+
 static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable)
 {
if (enable)
@@ -2294,6 +2303,41 @@ static int renesas_usb3_set_selfpowered(struct 
usb_gadget *gadget, int is_self)
.set_selfpowered= renesas_usb3_set_selfpowered,
 };
 
+static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
+{
+   struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+   enum usb_role cur_role;
+
+   pm_runtime_get_sync(dev);
+   cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+   pm_runtime_put(dev);
+
+   return cur_role;
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+   enum usb_role role)
+{
+   struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+   struct device *host = usb3->

[PATCH/RFC v3 0/4] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs

2018-05-14 Thread Yoshihiro Shimoda
This patch set is based on Felipe's usb.git / testing/next branch
(commit id = 5d1332a8eabd8bd5b8c322d45542968ee6f113be).

I still marked this patch set as "RFC". I would like to know whether
this way is good or not.

Changes from RFC v2:
 - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3
   because hardware resource (a register) is shared and remove individual
   usb role switch driver/dt-bindings for R-Car.
 - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver
   doesn't need such API now.

Changes from RFC:
 - Remove "device-connection-id" and "usb role switch driver" dt-bingings.
 - Remove drivers/of code.
 - Add a new API for find the connection by using graph on devcon.c and roles.c.
 - Use each new API on the rcar usb role switch and renesas_usb3 drivers.
 - Update the dtsi file for r8a7795.


Yoshihiro Shimoda (4):
  base: devcon: add a new API to find the graph
  usb: gadget: udc: renesas_usb3: Add register of usb role switch
  usb: gadget: udc: renesas_usb3: use usb role switch API
  arm64: dts: renesas: r8a7795: add OF graph for usb role switch

 .../devicetree/bindings/usb/renesas_usb3.txt   | 15 +
 Documentation/driver-api/device_connection.rst |  4 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi   | 12 
 drivers/base/devcon.c  | 43 +
 drivers/usb/gadget/udc/Kconfig |  1 +
 drivers/usb/gadget/udc/renesas_usb3.c  | 74 +-
 include/linux/device.h |  2 +
 7 files changed, 147 insertions(+), 4 deletions(-)

-- 
1.9.1

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


RE: [PATCH v5 03/14] staging: typec: tcpci: add compatible string for nxp ptn5110

2018-05-14 Thread Jun Li
Hi
> -Original Message-
> From: Mats Karrman [mailto:mats.dev.l...@gmail.com]
> Sent: 2018年5月12日 3:52
> 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 03/14] staging: typec: tcpci: add compatible string for
> nxp ptn5110
> 
> Hi Li Jun,
> 
> On 2018-05-03 02:24, Li Jun wrote:
> 
> > Add nxp ptn5110 typec controller compatible string: usb-tcpci,ptn5110,
> > which is a standard tcpci chip with power delivery support.
> >
> > Signed-off-by: Li Jun 
> > ---
> >   drivers/staging/typec/tcpci.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index 076d97e..741a80a 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -576,6 +576,7 @@ MODULE_DEVICE_TABLE(i2c, tcpci_id);
> >   #ifdef CONFIG_OF
> >   static const struct of_device_id tcpci_of_match[] = {
> > { .compatible = "usb,tcpci", },
> 
> Either this line should go away, or a "generic TCPCI controller" line should 
> be
> added to the DT documentation.

This binding name didn't follow the rule ", ",
AFAIK there is no user on upstream kernel, if no objection, I can remove it.

Li Jun
> 
> BR // Mats
> 
> > +   { .compatible = "nxp,ptn5110", },
> > {},
> >   };
> >   MODULE_DEVICE_TABLE(of, tcpci_of_match);
> >
N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

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

2018-05-14 Thread Jun Li
Hi
> -Original Message-
> From: Mats Karrman [mailto:mats.dev.l...@gmail.com]
> Sent: 2018年5月12日 3:49
> 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 01/14] dt-bindings: connector: add properties for typec
> 
> Hi Li Jun,
> 
> On 2018-05-03 02:24, Li Jun wrote:
> 
> > Add bingdings supported by current typec driver, so user can pass all
> > those properties via dt.
> >
> > Signed-off-by: Li Jun 
> > ---
> >   .../bindings/connector/usb-connector.txt   | 44
> +++
> >   include/dt-bindings/usb/pd.h   | 62
> ++
> >   2 files changed, 106 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index e1463f1..4b19de6d0 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -15,6 +15,33 @@ Optional properties:
> >   - type: size of the connector, should be specified in case of USB-A, USB-B
> > non-fullsize connectors: "mini", "micro".
> >
> > +Optional properties for usb-c-connector:
> > +- power-role: should be one of "source", "sink" or "dual"(DRP) if
> > +typec
> > +  connector has power support.
> > +- try-power-role: preferred power role if "dual"(DRP) can support
> > +Try.SNK
> > +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> > +- data-role: should be one of "host", "device", "dual"(DRD) if typec
> > +  connector supports USB data.
> > +
> > +Required properties for usb-c-connector with power delivery support:
> > +- source-pdos: An array of u32 with each entry providing supported
> > +power
> > +  source data object(PDO), the detailed bit definitions of PDO can be
> > +found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > +6.4.1.2
> > +  Source_Capabilities Message, the order of each entry(PDO) should
> > +follow
> > +  the PD spec chapter 6.4.1. Required for power source and power dual
> role.
> > +  User can specify the source PDO array via PDO_FIXED/BATT/VAR()
> > +defined in
> > +  dt-bindings/usb/pd.h.
> > +- sink-pdos: An array of u32 with each entry providing supported
> > +power
> > +  sink data object(PDO), the detailed bit definitions of PDO can be
> > +found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > +6.4.1.3
> > +  Sink Capabilities Message, the order of each entry(PDO) should
> > +follow
> > +  the PD spec chapter 6.4.1. Required for power sink and power dual role.
> > +  User can specify the sink PDO array via PDO_FIXED/BATT/VAR()
> > +defined in
> > +  dt-bindings/usb/pd.h.
> > +- op-sink-microwatt: Sink required operating power in microwatt, if
> > +source
> > +  can't offer the power, Capability Mismatch is set, required for
> > +power
> 
> ...set. Required...
> (new sentence, otherwise it's unclear what is required; op-sink-microwatt or
> Capability Mismatch set)

OK, will update as you suggested.

Thanks
Li Jun
N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: usbcore: NULL pointer dereference after detaching USB disk with linux 4.17

2018-05-14 Thread Mathias Nyman

On 11.05.2018 23:06, Jordan Glover wrote:

On May 11, 2018 6:14 PM, Mathias Nyman  wrote:


I think that just adding the below code should be enough for 4.17

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c

index 72ebbc9..32cd52c 100644

--- a/drivers/usb/host/xhci-hub.c

+++ b/drivers/usb/host/xhci-hub.c

@@ -354,7 +354,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct 
xhci_hcd *xhci,

slot_id = 0;

for (i = 0; i < MAX_HC_SLOTS; i++) {

- if (!xhci->devs[i])
 
 


- if (!xhci->devs[i] || !xhci->devs[i]->udev)
 
continue;

speed = xhci->devs[i]->udev->speed;
 
if (((speed >= USB_SPEED_SUPER) == (hcd->speed >= HCD_USB3))
 
 


-Mathias


I can confirm that above patch fixes this. I saw that offending commit was
backported to 4.16.8 so it needs this fix as well. Thank you.
​


Thanks, sent patch.
Added stable tags and Reported-by/Tested-by: tags

-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 0/1] xhci fix for usb-linus 4.17

2018-05-14 Thread Mathias Nyman
Hi Greg

One more xhci fix for 4.17.
Commit: 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
Revealed a race by causing a NULL pointer dereference regression in
4.17-rc4 and stable. Fix it.

-Mathias

Mathias Nyman (1):
  xhci: Fix USB3 NULL pointer dereference at logical disconnect.

 drivers/usb/host/xhci-hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
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 1/1] xhci: Fix USB3 NULL pointer dereference at logical disconnect.

2018-05-14 Thread Mathias Nyman
Hub driver will try to disable a USB3 device twice at logical disconnect,
racing with xhci_free_dev() callback from the first port disable.

This can be triggered with "udisksctl power-off --block-device "
or by writing "1" to the "remove" sysfs file for a USB3 device
in 4.17-rc4.

USB3 devices don't have a similar disabled link state as USB2 devices,
and use a U3 suspended link state instead. In this state the port
is still enabled and connected.

hub_port_connect() first disconnects the device, then later it notices
that device is still enabled (due to U3 states) it will try to disable
the port again (set to U3).

The xhci_free_dev() called during device disable is async, so checking
for existing xhci->devs[i] when setting link state to U3 the second time
was successful, even if device was being freed.

The regression was caused by, and whole thing revealed by,
Commit 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
which sets xhci->devs[i]->udev to NULL before xhci_virt_dev() returned.
and causes a NULL pointer dereference the second time we try to set U3.

Fix this by checking xhci->devs[i]->udev exists before setting link state.

The original patch went to stable so this fix needs to be applied there as
well.

Fixes: 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
Cc: 
Reported-by: Jordan Glover 
Tested-by: Jordan Glover 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 72ebbc9..32cd52c 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -354,7 +354,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct 
xhci_hcd *xhci,
 
slot_id = 0;
for (i = 0; i < MAX_HC_SLOTS; i++) {
-   if (!xhci->devs[i])
+   if (!xhci->devs[i] || !xhci->devs[i]->udev)
continue;
speed = xhci->devs[i]->udev->speed;
if (((speed >= USB_SPEED_SUPER) == (hcd->speed >= HCD_USB3))
-- 
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


Re: [PATCH 1/2] USB: dwc3: get extcon device by OF graph bindings

2018-05-14 Thread Andrzej Hajda
On 31.01.2018 16:57, Andrzej Hajda wrote:
> extcon device is used to detect host/device connection. Since extcon
> OF property is deprecated, alternative method should be added.
> This method uses OF graph bindings to locate extcon.
>
> Signed-off-by: Andrzej Hajda 

Ping, 3.5 months passed.

Regards
Andrzej

> ---
> Hi all,
>
> This patch implements alternative method to get extcon from DWC3.
> The code works but is hacky, as DWC3 must traverse different DT nodes
> to get extcon, in case of TM2 it is USB-PHY and MUIC, but other
> platforms can have different paths.
> I would be glad if it can be merged as is for now, but additional work
> must be done to make it generic.
> I guess on DT binding side it is OK. So the problem should be addressed
> in the code.
> My rough idea is to implement kind of extcon aliases/forwarder mechanism,
> ie. USB-PHY will expect on its output remote port extcon, and it should 
> register
> extcon-forwarder pointing to this extcon. This way DWC3 can look for the 
> extcon
> on its PHY phandle, and it will receive via forwarding mechanism extcon
> exposed by MUIC.
> As I said this is rough idea for discussion, other propositions are welcome.
>
> Regards
> Andrzej
> ---
>  drivers/usb/dwc3/drd.c | 41 -
>  1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index cc8ab9a8e9d2..eee2eca3d513 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include "debug.h"
>  #include "core.h"
> @@ -38,24 +39,38 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
>   return NOTIFY_DONE;
>  }
>  
> +struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
> +{
> + struct device *dev = dwc->dev;
> + struct device_node *np_phy, *np_conn;
> + struct extcon_dev *edev;
> +
> + if (of_property_read_bool(dev->of_node, "extcon"))
> + return extcon_get_edev_by_phandle(dwc->dev, 0);
> +
> + np_phy = of_parse_phandle(dev->of_node, "phys", 0);
> + np_conn = of_graph_get_remote_node(np_phy, -1, -1);
> + edev = extcon_get_edev_by_of_node(np_conn);
> + of_node_put(np_conn);
> + of_node_put(np_phy);
> +
> + return edev;
> +}
> +
>  int dwc3_drd_init(struct dwc3 *dwc)
>  {
>   int ret;
>  
> - if (dwc->dev->of_node) {
> - if (of_property_read_bool(dwc->dev->of_node, "extcon"))
> - dwc->edev = extcon_get_edev_by_phandle(dwc->dev, 0);
> -
> - if (IS_ERR(dwc->edev))
> - return PTR_ERR(dwc->edev);
> + dwc->edev = dwc3_get_extcon(dwc);
> + if (IS_ERR(dwc->edev))
> + return PTR_ERR(dwc->edev);
>  
> - dwc->edev_nb.notifier_call = dwc3_drd_notifier;
> - ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> -&dwc->edev_nb);
> - if (ret < 0) {
> - dev_err(dwc->dev, "couldn't register cable notifier\n");
> - return ret;
> - }
> + dwc->edev_nb.notifier_call = dwc3_drd_notifier;
> + ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> +&dwc->edev_nb);
> + if (ret < 0) {
> + dev_err(dwc->dev, "couldn't register cable notifier\n");
> + return ret;
>   }
>  
>   dwc3_drd_update(dwc);


--
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: patch "xhci: Fix use-after-free in xhci_free_virt_device" causes kernel crashes on 4.16.8 on unplug

2018-05-14 Thread Mathias Nyman

On 12.05.2018 04:21, Jacob Saunders wrote:

On Linux kernel 4.16.8 (using Arch Linux) if I eject a USB 3.0 device from my system 
(unplug or "udisksctl power-off --block-device /dev/sde), it freezes instantly 
with a null pointer error in XHCI. I've been unable to successfully capture a kernel 
log of the error (and my cameras did not return usable results) and the screen 
begins scrolling near-immediately with hung processors in a VT.  I have done a git 
bisection (between 4.16.7 and 4.16.7) narrowing it down to the following commit. 
Sending this both here and to the stable list, apologies if I'm doing it wrong. I am 
not subscribed to this list, so CC me please if replying to this.



Thanks, I got a similar report earlier, with a soltion.
see:
https://marc.info/?l=linux-usb&m=152595298422367&w=2

I'll send it out in a few minutes

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


Re: [PATCH v2] usb: chipidea: Hook into mux framework to toggle usb switch

2018-05-14 Thread Peter Rosin
On 2018-04-18 07:48, yos...@codeaurora.org wrote:
> On 2018-04-17 17:11, Peter Rosin wrote:
>> On 2018-04-17 15:52, Yossi Mansharoff wrote:
>>> On the db410c 96boards platform we have a TC7USB40MU on the board
>>> to mux the D+/D- lines coming from the controller between a micro
>>> usb "device" port and a USB hub for "host" roles[1]. During a
>>> role switch, we need to toggle this mux to forward the D+/D-
>>> lines to either the port or the hub. Add the necessary code to do
>>> the role switch in chipidea core via the generic mux framework.
>>> Board configurations like on db410c are expected to change roles
>>> via the sysfs API described in
>>> Documentation/ABI/testing/sysfs-platform-chipidea-usb2.
>>
>> Ok, so this is v2. Please describe what is different from v1.
>> I have told you before that this information helps.
>>
>>> [1] 
>>> https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
>>
>> This link returns 404 for me.
>>
>> Cheers,
>> Peter
> 
> 
> Hi,
> This patch was split apart from the original patch into two patches
> one for chipidea and the other for bindings.
> this patch has no other changes two the code.
> 
> I will update the link.

Just a note: I will not feed the mux_control_get_optional patch upstream
until I feel confident that this patch is going also heading upstream.

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