[PATCH -next] usb: cdns3: Fix sheduling with locks held.
Patch fix issue in cdns3_ep0_feature_handle_device function.
The function usleep_range can't be used there because this function is
called with locks held and IRQs disabled in
cdns3_device_thread_irq_handler().
To resolve this issue patch replaces usleep_range with mdelay.
Reported-by: Dan Carpenter
Signed-off-by: Pawel Laszczak
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
---
drivers/usb/cdns3/ep0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
index 44f652e8b5a2..0445da0a5a0c 100644
--- a/drivers/usb/cdns3/ep0.c
+++ b/drivers/usb/cdns3/ep0.c
@@ -332,7 +332,7 @@ static int cdns3_ep0_feature_handle_device(struct
cdns3_device *priv_dev,
* for sending status stage.
* This time should be less then 3ms.
*/
- usleep_range(1000, 2000);
+ mdelay(1);
cdns3_set_register_bit(&priv_dev->regs->usb_cmd,
USB_CMD_STMODE |
USB_STS_TMODE_SEL(tmode - 1));
--
2.17.1
Re: [PATCH -next] usb: cdns3: Fix sheduling with locks held.
On Thu, Sep 26, 2019 at 08:00:30AM +0100, Pawel Laszczak wrote:
> Patch fix issue in cdns3_ep0_feature_handle_device function.
>
> The function usleep_range can't be used there because this function is
> called with locks held and IRQs disabled in
> cdns3_device_thread_irq_handler().
>
> To resolve this issue patch replaces usleep_range with mdelay.
>
> Reported-by: Dan Carpenter
> Signed-off-by: Pawel Laszczak
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> ---
> drivers/usb/cdns3/ep0.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Is this v2 of the patch?
If so, it needs to be said so in the subject line, and below the ---
line describe what changed from the previous one.
That should all be described in the kernel documentation, right?
v3 please?
thanks,
greg k-h
RE: [PATCH -next] usb: cdns3: Fix sheduling with locks held.
Hi,
>
>
>On Thu, Sep 26, 2019 at 08:00:30AM +0100, Pawel Laszczak wrote:
>> Patch fix issue in cdns3_ep0_feature_handle_device function.
>>
>> The function usleep_range can't be used there because this function is
>> called with locks held and IRQs disabled in
>> cdns3_device_thread_irq_handler().
>>
>> To resolve this issue patch replaces usleep_range with mdelay.
>>
>> Reported-by: Dan Carpenter
>> Signed-off-by: Pawel Laszczak
>> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
>> ---
>> drivers/usb/cdns3/ep0.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>Is this v2 of the patch?
Really. It's the same patch as previous one. I added only
Reported-by and Fixes.
Ok I will prepare the next one :(.
>If so, it needs to be said so in the subject line, and below the ---
>line describe what changed from the previous one.
>
>That should all be described in the kernel documentation, right?
>
>v3 please?
>
>thanks,
>
>greg k-h
Thenks,
Pawell
[PATCH v3 -next] usb: cdns3: Fix sheduling with locks held.
Patch fix issue in cdns3_ep0_feature_handle_device function.
The function usleep_range can't be used there because this function is
called with locks held and IRQs disabled in
cdns3_device_thread_irq_handler().
To resolve this issue patch replaces usleep_range with mdelay.
Reported-by: Dan Carpenter
Signed-off-by: Pawel Laszczak
Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
---
v2: added Reported-by and Fixes tags
v3: added version of the patch
---
drivers/usb/cdns3/ep0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
index 44f652e8b5a2..0445da0a5a0c 100644
--- a/drivers/usb/cdns3/ep0.c
+++ b/drivers/usb/cdns3/ep0.c
@@ -332,7 +332,7 @@ static int cdns3_ep0_feature_handle_device(struct
cdns3_device *priv_dev,
* for sending status stage.
* This time should be less then 3ms.
*/
- usleep_range(1000, 2000);
+ mdelay(1);
cdns3_set_register_bit(&priv_dev->regs->usb_cmd,
USB_CMD_STMODE |
USB_STS_TMODE_SEL(tmode - 1));
--
2.17.1
Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted
On 26.9.2019 8.45, Felipe Balbi wrote: Hi, David Laight writes: From: Mathias Nyman Sent: 25 September 2019 15:48 On 24.9.2019 17.45, alex zheng wrote: Hi Mathias, ... Logs show your transfer ring has four segments, but hardware fails to jump from the last segment back to first) Last TRB (LINK TRB) of each segment points to the next segment, last segments link trb points back to first segment. In your case: 0x1d117000 -> 0x1eb09000 -> 0x1eb0a000 -> 0x1dbda000 -> (back to 0x1d117000) For some reason your hardware doesn't treat the last TRB at the last segment as a LINK TRB, instead it just issues a transfer event for it, and continues to the next address instead of jumping back to first segment: That could be a cache coherency (or flushing (etc)) issue. The Link TRB is written very early, right after the ring segment is allocated, and before any other TRBs. 255 other TRBs were written and handled by hw on this segment after this, so not very likely a flushing/cache coherency issue. XHCI has a HW-configurable maximum number of segments in a ring. AFAICT, xhci driver doesn't take that into consideration today. Perhaps the HW in question doesn't like more than 3 segments. Mathias, what was the register to check this? Do you remember? I only recall a limit for the event ring in the HSCPARAMS2 register(ERST MAX), not for transfer rings. Other things to look at would be - check that Toggle Cycle bit is correct for last segments link TRB (incomplete logs) - some old xHCI hardware needed the Chain bit set in link TRB for some isoc rings - was ring recently expanded?, usually rings start with only two segments Mathias
[PATCH 3/4] USB: usblcd: drop redundant lcd mutex
Drop the redundant lcd mutex introduced by commit 925ce689bb31 ("USB:
autoconvert trivial BKL users to private mutex") which replaced an
earlier BKL use.
The lock serialised calls to open() against other open() and a custom
ioctl() returning the bcdDevice (sic!), but neither is needed.
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index b898650a5570..732eb1f81368 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -30,7 +30,6 @@
#define IOCTL_GET_DRV_VERSION 2
-static DEFINE_MUTEX(lcd_mutex);
static const struct usb_device_id id_table[] = {
{ .idVendor = 0x10D2, .match_flags = USB_DEVICE_ID_MATCH_VENDOR, },
{ },
@@ -81,12 +80,10 @@ static int lcd_open(struct inode *inode, struct file *file)
struct usb_interface *interface;
int subminor, r;
- mutex_lock(&lcd_mutex);
subminor = iminor(inode);
interface = usb_find_interface(&lcd_driver, subminor);
if (!interface) {
- mutex_unlock(&lcd_mutex);
printk(KERN_ERR "USBLCD: %s - error, can't find device for
minor %d\n",
__func__, subminor);
return -ENODEV;
@@ -101,13 +98,11 @@ static int lcd_open(struct inode *inode, struct file *file)
r = usb_autopm_get_interface(interface);
if (r < 0) {
kref_put(&dev->kref, lcd_delete);
- mutex_unlock(&lcd_mutex);
return r;
}
/* save our object in the file's private structure */
file->private_data = dev;
- mutex_unlock(&lcd_mutex);
return 0;
}
@@ -176,14 +171,12 @@ static long lcd_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
switch (cmd) {
case IOCTL_GET_HARD_VERSION:
- mutex_lock(&lcd_mutex);
bcdDevice = le16_to_cpu((dev->udev)->descriptor.bcdDevice);
sprintf(buf, "%1d%1d.%1d%1d",
(bcdDevice & 0xF000)>>12,
(bcdDevice & 0xF00)>>8,
(bcdDevice & 0xF0)>>4,
(bcdDevice & 0xF));
- mutex_unlock(&lcd_mutex);
if (copy_to_user((void __user *)arg, buf, strlen(buf)) != 0)
return -EFAULT;
break;
--
2.23.0
[PATCH 4/4] USB: usblcd: use pr_err()
Replace the one remaining printk with pr_err().
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index 732eb1f81368..61e9e987fe4a 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -84,7 +84,7 @@ static int lcd_open(struct inode *inode, struct file *file)
interface = usb_find_interface(&lcd_driver, subminor);
if (!interface) {
- printk(KERN_ERR "USBLCD: %s - error, can't find device for
minor %d\n",
+ pr_err("USBLCD: %s - error, can't find device for minor %d\n",
__func__, subminor);
return -ENODEV;
}
--
2.23.0
[PATCH 0/4] USB: usblcd: disconnect fix and locking clean ups
This series fixes a failure to stop I/O on disconnect() in the usblcd driver. Turns out there was a lot of legacy cruft in this driver which could simply be removed. The first patch is marked for stable and could go into v5.4 while the rest is v5.5 material. Posting all at once for completeness. I was tempted to rip out the custom ioctls() used to retrieve the driver version and bcdDevice (sic!), but decided to leave them in. I doubt anyone would miss them though so perhaps we should give it a go? Tested using a mockup device. Johan Johan Hovold (4): USB: usblcd: fix I/O after disconnect USB: usblcd: drop redundant disconnect mutex USB: usblcd: drop redundant lcd mutex USB: usblcd: use pr_err() drivers/usb/misc/usblcd.c | 60 +-- 1 file changed, 33 insertions(+), 27 deletions(-) -- 2.23.0
[PATCH 3/4] USB: usblcd: drop redundant lcd mutex
Drop the redundant lcd mutex introduced by commit 925ce689bb31 ("USB:
autoconvert trivial BKL users to private mutex") which replaced an
earlier BKL use.
The lock serialised calls to open() against other open() and a custom
ioctl() returning the bcdDevice (sic!), but neither is needed.
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index b898650a5570..732eb1f81368 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -30,7 +30,6 @@
#define IOCTL_GET_DRV_VERSION 2
-static DEFINE_MUTEX(lcd_mutex);
static const struct usb_device_id id_table[] = {
{ .idVendor = 0x10D2, .match_flags = USB_DEVICE_ID_MATCH_VENDOR, },
{ },
@@ -81,12 +80,10 @@ static int lcd_open(struct inode *inode, struct file *file)
struct usb_interface *interface;
int subminor, r;
- mutex_lock(&lcd_mutex);
subminor = iminor(inode);
interface = usb_find_interface(&lcd_driver, subminor);
if (!interface) {
- mutex_unlock(&lcd_mutex);
printk(KERN_ERR "USBLCD: %s - error, can't find device for
minor %d\n",
__func__, subminor);
return -ENODEV;
@@ -101,13 +98,11 @@ static int lcd_open(struct inode *inode, struct file *file)
r = usb_autopm_get_interface(interface);
if (r < 0) {
kref_put(&dev->kref, lcd_delete);
- mutex_unlock(&lcd_mutex);
return r;
}
/* save our object in the file's private structure */
file->private_data = dev;
- mutex_unlock(&lcd_mutex);
return 0;
}
@@ -176,14 +171,12 @@ static long lcd_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
switch (cmd) {
case IOCTL_GET_HARD_VERSION:
- mutex_lock(&lcd_mutex);
bcdDevice = le16_to_cpu((dev->udev)->descriptor.bcdDevice);
sprintf(buf, "%1d%1d.%1d%1d",
(bcdDevice & 0xF000)>>12,
(bcdDevice & 0xF00)>>8,
(bcdDevice & 0xF0)>>4,
(bcdDevice & 0xF));
- mutex_unlock(&lcd_mutex);
if (copy_to_user((void __user *)arg, buf, strlen(buf)) != 0)
return -EFAULT;
break;
--
2.23.0
[PATCH 0/4] USB: usblcd: disconnect fix and locking clean ups
This series fixes a failure to stop I/O on disconnect() in the usblcd driver. Turns out there was a lot of legacy cruft in this driver which could simply be removed. The first patch is marked for stable and could go into v5.4 while the rest is v5.5 material. Posting all at once for completeness. I was tempted to rip out the custom ioctls() used to retrieve the driver version and bcdDevice (sic!), but decided to leave them in. I doubt anyone would miss them though so perhaps we should give it a go? Tested using a mockup device. Johan Johan Hovold (4): USB: usblcd: fix I/O after disconnect USB: usblcd: drop redundant disconnect mutex USB: usblcd: drop redundant lcd mutex USB: usblcd: use pr_err() drivers/usb/misc/usblcd.c | 60 +-- 1 file changed, 33 insertions(+), 27 deletions(-) -- 2.23.0
[PATCH 1/4] USB: usblcd: fix I/O after disconnect
Make sure to stop all I/O on disconnect by adding a disconnected flag
which is used to prevent new I/O from being started and by stopping all
ongoing I/O before returning.
This also fixes a potential use-after-free on driver unbind in case the
driver data is freed before the completion handler has run.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable # 7bbe990c989e
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index 9ba4a4e68d91..aa982d3ca36b 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -57,6 +58,8 @@ struct usb_lcd {
using up all RAM */
struct usb_anchor submitted; /* URBs to wait for
before suspend */
+ struct rw_semaphore io_rwsem;
+ unsigned long disconnected:1;
};
#define to_lcd_dev(d) container_of(d, struct usb_lcd, kref)
@@ -142,6 +145,13 @@ static ssize_t lcd_read(struct file *file, char __user *
buffer,
dev = file->private_data;
+ down_read(&dev->io_rwsem);
+
+ if (dev->disconnected) {
+ retval = -ENODEV;
+ goto out_up_io;
+ }
+
/* do a blocking bulk read to get data from the device */
retval = usb_bulk_msg(dev->udev,
usb_rcvbulkpipe(dev->udev,
@@ -158,6 +168,9 @@ static ssize_t lcd_read(struct file *file, char __user *
buffer,
retval = bytes_read;
}
+out_up_io:
+ up_read(&dev->io_rwsem);
+
return retval;
}
@@ -237,11 +250,18 @@ static ssize_t lcd_write(struct file *file, const char
__user * user_buffer,
if (r < 0)
return -EINTR;
+ down_read(&dev->io_rwsem);
+
+ if (dev->disconnected) {
+ retval = -ENODEV;
+ goto err_up_io;
+ }
+
/* create a urb, and a buffer for it, and copy the data to the urb */
urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb) {
retval = -ENOMEM;
- goto err_no_buf;
+ goto err_up_io;
}
buf = usb_alloc_coherent(dev->udev, count, GFP_KERNEL,
@@ -278,6 +298,7 @@ static ssize_t lcd_write(struct file *file, const char
__user * user_buffer,
the USB core will eventually free it entirely */
usb_free_urb(urb);
+ up_read(&dev->io_rwsem);
exit:
return count;
error_unanchor:
@@ -285,7 +306,8 @@ static ssize_t lcd_write(struct file *file, const char
__user * user_buffer,
error:
usb_free_coherent(dev->udev, count, buf, urb->transfer_dma);
usb_free_urb(urb);
-err_no_buf:
+err_up_io:
+ up_read(&dev->io_rwsem);
up(&dev->limit_sem);
return retval;
}
@@ -325,6 +347,7 @@ static int lcd_probe(struct usb_interface *interface,
kref_init(&dev->kref);
sema_init(&dev->limit_sem, USB_LCD_CONCURRENT_WRITES);
+ init_rwsem(&dev->io_rwsem);
init_usb_anchor(&dev->submitted);
dev->udev = usb_get_dev(interface_to_usbdev(interface));
@@ -422,6 +445,12 @@ static void lcd_disconnect(struct usb_interface *interface)
/* give back our minor */
usb_deregister_dev(interface, &lcd_class);
+ down_write(&dev->io_rwsem);
+ dev->disconnected = 1;
+ up_write(&dev->io_rwsem);
+
+ usb_kill_anchored_urbs(&dev->submitted);
+
/* decrement our usage count */
kref_put(&dev->kref, lcd_delete);
--
2.23.0
[PATCH 2/4] USB: usblcd: drop redundant disconnect mutex
Drop the redundant disconnect mutex which was introduced after the
open-disconnect race had been addressed generally in USB core by commit
d4ead16f50f9 ("USB: prevent char device open/deregister race").
Specifically, the rw-semaphore in core guarantees that all calls to
open() will have completed and that no new calls to open() will occur
after usb_deregister_dev() returns. Hence there is no need use the
driver data as an inverted disconnected flag.
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index aa982d3ca36b..b898650a5570 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -37,9 +37,6 @@ static const struct usb_device_id id_table[] = {
};
MODULE_DEVICE_TABLE(usb, id_table);
-static DEFINE_MUTEX(open_disc_mutex);
-
-
struct usb_lcd {
struct usb_device *udev; /* init: probe_lcd */
struct usb_interface*interface; /* the interface for
@@ -95,17 +92,10 @@ static int lcd_open(struct inode *inode, struct file *file)
return -ENODEV;
}
- mutex_lock(&open_disc_mutex);
dev = usb_get_intfdata(interface);
- if (!dev) {
- mutex_unlock(&open_disc_mutex);
- mutex_unlock(&lcd_mutex);
- return -ENODEV;
- }
/* increment our usage count for the device */
kref_get(&dev->kref);
- mutex_unlock(&open_disc_mutex);
/* grab a power reference */
r = usb_autopm_get_interface(interface);
@@ -388,7 +378,6 @@ static int lcd_probe(struct usb_interface *interface,
/* something prevented us from registering this driver */
dev_err(&interface->dev,
"Not able to get a minor for this device.\n");
- usb_set_intfdata(interface, NULL);
goto error;
}
@@ -434,14 +423,9 @@ static int lcd_resume(struct usb_interface *intf)
static void lcd_disconnect(struct usb_interface *interface)
{
- struct usb_lcd *dev;
+ struct usb_lcd *dev = usb_get_intfdata(interface);
int minor = interface->minor;
- mutex_lock(&open_disc_mutex);
- dev = usb_get_intfdata(interface);
- usb_set_intfdata(interface, NULL);
- mutex_unlock(&open_disc_mutex);
-
/* give back our minor */
usb_deregister_dev(interface, &lcd_class);
--
2.23.0
[PATCH 1/4] USB: usblcd: fix I/O after disconnect
Make sure to stop all I/O on disconnect by adding a disconnected flag
which is used to prevent new I/O from being started and by stopping all
ongoing I/O before returning.
This also fixes a potential use-after-free on driver unbind in case the
driver data is freed before the completion handler has run.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable # 7bbe990c989e
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index 9ba4a4e68d91..aa982d3ca36b 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -57,6 +58,8 @@ struct usb_lcd {
using up all RAM */
struct usb_anchor submitted; /* URBs to wait for
before suspend */
+ struct rw_semaphore io_rwsem;
+ unsigned long disconnected:1;
};
#define to_lcd_dev(d) container_of(d, struct usb_lcd, kref)
@@ -142,6 +145,13 @@ static ssize_t lcd_read(struct file *file, char __user *
buffer,
dev = file->private_data;
+ down_read(&dev->io_rwsem);
+
+ if (dev->disconnected) {
+ retval = -ENODEV;
+ goto out_up_io;
+ }
+
/* do a blocking bulk read to get data from the device */
retval = usb_bulk_msg(dev->udev,
usb_rcvbulkpipe(dev->udev,
@@ -158,6 +168,9 @@ static ssize_t lcd_read(struct file *file, char __user *
buffer,
retval = bytes_read;
}
+out_up_io:
+ up_read(&dev->io_rwsem);
+
return retval;
}
@@ -237,11 +250,18 @@ static ssize_t lcd_write(struct file *file, const char
__user * user_buffer,
if (r < 0)
return -EINTR;
+ down_read(&dev->io_rwsem);
+
+ if (dev->disconnected) {
+ retval = -ENODEV;
+ goto err_up_io;
+ }
+
/* create a urb, and a buffer for it, and copy the data to the urb */
urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb) {
retval = -ENOMEM;
- goto err_no_buf;
+ goto err_up_io;
}
buf = usb_alloc_coherent(dev->udev, count, GFP_KERNEL,
@@ -278,6 +298,7 @@ static ssize_t lcd_write(struct file *file, const char
__user * user_buffer,
the USB core will eventually free it entirely */
usb_free_urb(urb);
+ up_read(&dev->io_rwsem);
exit:
return count;
error_unanchor:
@@ -285,7 +306,8 @@ static ssize_t lcd_write(struct file *file, const char
__user * user_buffer,
error:
usb_free_coherent(dev->udev, count, buf, urb->transfer_dma);
usb_free_urb(urb);
-err_no_buf:
+err_up_io:
+ up_read(&dev->io_rwsem);
up(&dev->limit_sem);
return retval;
}
@@ -325,6 +347,7 @@ static int lcd_probe(struct usb_interface *interface,
kref_init(&dev->kref);
sema_init(&dev->limit_sem, USB_LCD_CONCURRENT_WRITES);
+ init_rwsem(&dev->io_rwsem);
init_usb_anchor(&dev->submitted);
dev->udev = usb_get_dev(interface_to_usbdev(interface));
@@ -422,6 +445,12 @@ static void lcd_disconnect(struct usb_interface *interface)
/* give back our minor */
usb_deregister_dev(interface, &lcd_class);
+ down_write(&dev->io_rwsem);
+ dev->disconnected = 1;
+ up_write(&dev->io_rwsem);
+
+ usb_kill_anchored_urbs(&dev->submitted);
+
/* decrement our usage count */
kref_put(&dev->kref, lcd_delete);
--
2.23.0
[PATCH 2/4] USB: usblcd: drop redundant disconnect mutex
Drop the redundant disconnect mutex which was introduced after the
open-disconnect race had been addressed generally in USB core by commit
d4ead16f50f9 ("USB: prevent char device open/deregister race").
Specifically, the rw-semaphore in core guarantees that all calls to
open() will have completed and that no new calls to open() will occur
after usb_deregister_dev() returns. Hence there is no need use the
driver data as an inverted disconnected flag.
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index aa982d3ca36b..b898650a5570 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -37,9 +37,6 @@ static const struct usb_device_id id_table[] = {
};
MODULE_DEVICE_TABLE(usb, id_table);
-static DEFINE_MUTEX(open_disc_mutex);
-
-
struct usb_lcd {
struct usb_device *udev; /* init: probe_lcd */
struct usb_interface*interface; /* the interface for
@@ -95,17 +92,10 @@ static int lcd_open(struct inode *inode, struct file *file)
return -ENODEV;
}
- mutex_lock(&open_disc_mutex);
dev = usb_get_intfdata(interface);
- if (!dev) {
- mutex_unlock(&open_disc_mutex);
- mutex_unlock(&lcd_mutex);
- return -ENODEV;
- }
/* increment our usage count for the device */
kref_get(&dev->kref);
- mutex_unlock(&open_disc_mutex);
/* grab a power reference */
r = usb_autopm_get_interface(interface);
@@ -388,7 +378,6 @@ static int lcd_probe(struct usb_interface *interface,
/* something prevented us from registering this driver */
dev_err(&interface->dev,
"Not able to get a minor for this device.\n");
- usb_set_intfdata(interface, NULL);
goto error;
}
@@ -434,14 +423,9 @@ static int lcd_resume(struct usb_interface *intf)
static void lcd_disconnect(struct usb_interface *interface)
{
- struct usb_lcd *dev;
+ struct usb_lcd *dev = usb_get_intfdata(interface);
int minor = interface->minor;
- mutex_lock(&open_disc_mutex);
- dev = usb_get_intfdata(interface);
- usb_set_intfdata(interface, NULL);
- mutex_unlock(&open_disc_mutex);
-
/* give back our minor */
usb_deregister_dev(interface, &lcd_class);
--
2.23.0
[PATCH 4/4] USB: usblcd: use pr_err()
Replace the one remaining printk with pr_err().
Signed-off-by: Johan Hovold
---
drivers/usb/misc/usblcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c
index 732eb1f81368..61e9e987fe4a 100644
--- a/drivers/usb/misc/usblcd.c
+++ b/drivers/usb/misc/usblcd.c
@@ -84,7 +84,7 @@ static int lcd_open(struct inode *inode, struct file *file)
interface = usb_find_interface(&lcd_driver, subminor);
if (!interface) {
- printk(KERN_ERR "USBLCD: %s - error, can't find device for
minor %d\n",
+ pr_err("USBLCD: %s - error, can't find device for minor %d\n",
__func__, subminor);
return -ENODEV;
}
--
2.23.0
Re: [PATCH 0/4] USB: usblcd: disconnect fix and locking clean ups
On Thu, Sep 26, 2019 at 11:12:24AM +0200, Johan Hovold wrote: > This series fixes a failure to stop I/O on disconnect() in the usblcd > driver. Turns out there was a lot of legacy cruft in this driver which > could simply be removed. My apologies for the double post. Johan
[PATCH 09/14] usb: typec: ucsi: Simplified interface registration and I/O API
Adding more simplified API for interface registration and
read and write operations.
The registration is split into separate creation and
registration phases. That allows the drivers to properly
initialize the interface before registering it if necessary.
The read and write operations are supplied in a completely
separate struct ucsi_operations that is passed to the
ucsi_register() function during registration. The new read
and write operations will work more traditionally so that
the read callback function reads a requested amount of data
from an offset, and the write callback functions write the
given data to the offset. The drivers will have to support
both non-blocking writing and blocking writing. In blocking
writing the driver itself is responsible of waiting for the
completion event.
The new API makes it possible for the drivers to perform
tasks also independently of the core ucsi.c, and that should
allow for example quirks to be handled completely in the
drivers without the need to touch ucsi.c.
The old API is kept until all drivers have been converted to
the new API.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi.c | 326 +++---
drivers/usb/typec/ucsi/ucsi.h | 57 ++
2 files changed, 354 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index edd722fb88b8..2ba890327b9d 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -98,6 +98,98 @@ static int ucsi_ack(struct ucsi *ucsi, u8 ack)
return ret;
}
+static int ucsi_acknowledge_command(struct ucsi *ucsi)
+{
+ u64 ctrl;
+
+ ctrl = UCSI_ACK_CC_CI;
+ ctrl |= UCSI_ACK_COMMAND_COMPLETE;
+
+ return ucsi->ops->sync_write(ucsi, UCSI_CONTROL, &ctrl, sizeof(ctrl));
+}
+
+static int ucsi_acknowledge_connector_change(struct ucsi *ucsi)
+{
+ u64 ctrl;
+
+ ctrl = UCSI_ACK_CC_CI;
+ ctrl |= UCSI_ACK_CONNECTOR_CHANGE;
+
+ return ucsi->ops->async_write(ucsi, UCSI_CONTROL, &ctrl, sizeof(ctrl));
+}
+
+static int ucsi_exec_command(struct ucsi *ucsi, u64 command);
+
+static int ucsi_read_error(struct ucsi *ucsi)
+{
+ u16 error;
+ int ret;
+
+ /* Acknowlege the command that failed */
+ ret = ucsi_acknowledge_command(ucsi);
+ if (ret)
+ return ret;
+
+ ret = ucsi_exec_command(ucsi, UCSI_GET_ERROR_STATUS);
+ if (ret < 0)
+ return ret;
+
+ ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, &error, sizeof(error));
+ if (ret)
+ return ret;
+
+ switch (error) {
+ case UCSI_ERROR_INCOMPATIBLE_PARTNER:
+ return -EOPNOTSUPP;
+ case UCSI_ERROR_CC_COMMUNICATION_ERR:
+ return -ECOMM;
+ case UCSI_ERROR_CONTRACT_NEGOTIATION_FAIL:
+ return -EPROTO;
+ case UCSI_ERROR_DEAD_BATTERY:
+ dev_warn(ucsi->dev, "Dead battery condition!\n");
+ return -EPERM;
+ /* The following mean a bug in this driver */
+ case UCSI_ERROR_INVALID_CON_NUM:
+ case UCSI_ERROR_UNREGONIZED_CMD:
+ case UCSI_ERROR_INVALID_CMD_ARGUMENT:
+ dev_err(ucsi->dev, "possible UCSI driver bug (0x%x)\n", error);
+ return -EINVAL;
+ default:
+ dev_err(ucsi->dev, "%s: error without status\n", __func__);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)
+{
+ u32 cci;
+ int ret;
+
+ ret = ucsi->ops->sync_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd));
+ if (ret)
+ return ret;
+
+ ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+ if (ret)
+ return ret;
+
+ if (cci & UCSI_CCI_BUSY)
+ return -EBUSY;
+
+ if (!(cci & UCSI_CCI_COMMAND_COMPLETE))
+ return -EIO;
+
+ if (cci & UCSI_CCI_NOT_SUPPORTED)
+ return -EOPNOTSUPP;
+
+ if (cci & UCSI_CCI_ERROR)
+ return ucsi_read_error(ucsi);
+
+ return UCSI_CCI_LENGTH(cci);
+}
+
static int ucsi_run_command(struct ucsi *ucsi, struct ucsi_control *ctrl,
void *data, size_t size)
{
@@ -106,6 +198,26 @@ static int ucsi_run_command(struct ucsi *ucsi, struct
ucsi_control *ctrl,
u16 error;
int ret;
+ if (ucsi->ops) {
+ ret = ucsi_exec_command(ucsi, ctrl->raw_cmd);
+ if (ret < 0)
+ return ret;
+
+ data_length = ret;
+
+ if (data) {
+ ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, data,
size);
+ if (ret)
+ return ret;
+ }
+
+ ret = ucsi_acknowledge_command(ucsi);
+ if (ret)
+ return ret;
+
+ return data_length;
+ }
+
ret = ucsi_command(ucsi,
[PATCH 06/14] usb: typec: ucsi: Start using struct typec_operations
Supplying the operation callbacks as part of a struct
typec_operations instead of as part of struct
typec_capability during port registration.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ba288b964dc8..edd722fb88b8 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -17,9 +17,6 @@
#include "ucsi.h"
#include "trace.h"
-#define to_ucsi_connector(_cap_) container_of(_cap_, struct ucsi_connector, \
- typec_cap)
-
/*
* UCSI_TIMEOUT_MS - PPM communication timeout
*
@@ -713,10 +710,9 @@ static int ucsi_role_cmd(struct ucsi_connector *con,
struct ucsi_control *ctrl)
return ret;
}
-static int
-ucsi_dr_swap(const struct typec_capability *cap, enum typec_data_role role)
+static int ucsi_dr_swap(struct typec_port *port, enum typec_data_role role)
{
- struct ucsi_connector *con = to_ucsi_connector(cap);
+ struct ucsi_connector *con = typec_get_drvdata(port);
struct ucsi_control ctrl;
int ret = 0;
@@ -748,10 +744,9 @@ ucsi_dr_swap(const struct typec_capability *cap, enum
typec_data_role role)
return ret < 0 ? ret : 0;
}
-static int
-ucsi_pr_swap(const struct typec_capability *cap, enum typec_role role)
+static int ucsi_pr_swap(struct typec_port *port, enum typec_role role)
{
- struct ucsi_connector *con = to_ucsi_connector(cap);
+ struct ucsi_connector *con = typec_get_drvdata(port);
struct ucsi_control ctrl;
int ret = 0;
@@ -788,6 +783,11 @@ ucsi_pr_swap(const struct typec_capability *cap, enum
typec_role role)
return ret;
}
+static const struct typec_operations ucsi_ops = {
+ .dr_set = ucsi_dr_swap,
+ .pr_set = ucsi_pr_swap
+};
+
static struct fwnode_handle *ucsi_find_fwnode(struct ucsi_connector *con)
{
struct fwnode_handle *fwnode;
@@ -843,8 +843,8 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
*accessory = TYPEC_ACCESSORY_DEBUG;
cap->fwnode = ucsi_find_fwnode(con);
- cap->dr_set = ucsi_dr_swap;
- cap->pr_set = ucsi_pr_swap;
+ cap->driver_data = con;
+ cap->ops = &ucsi_ops;
/* Register the connector */
con->port = typec_register_port(ucsi->dev, cap);
--
2.23.0
[PATCH 01/14] usb: typec: Copy everything from struct typec_capability during registration
Copying everything from struct typec_capability to struct
typec_port during port registration.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 55 +--
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 94a3eda62add..3835e2d9fba6 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -46,8 +46,14 @@ struct typec_port {
enum typec_role vconn_role;
enum typec_pwr_opmode pwr_opmode;
enum typec_port_typeport_type;
+ enum typec_port_typefixed_role;
+ enum typec_port_dataport_roles;
+ enum typec_accessoryaccessory[TYPEC_MAX_ACCESSORY];
struct mutexport_type_lock;
+ u16 revision;
+ u16 pd_revision;
+
enum typec_orientation orientation;
struct typec_switch *sw;
struct typec_mux*mux;
@@ -950,7 +956,7 @@ preferred_role_store(struct device *dev, struct
device_attribute *attr,
int role;
int ret;
- if (port->cap->type != TYPEC_PORT_DRP) {
+ if (port->fixed_role != TYPEC_PORT_DRP) {
dev_dbg(dev, "Preferred role only supported with DRP ports\n");
return -EOPNOTSUPP;
}
@@ -982,7 +988,7 @@ preferred_role_show(struct device *dev, struct
device_attribute *attr,
{
struct typec_port *port = to_typec_port(dev);
- if (port->cap->type != TYPEC_PORT_DRP)
+ if (port->fixed_role != TYPEC_PORT_DRP)
return 0;
if (port->prefer_role < 0)
@@ -1009,7 +1015,7 @@ static ssize_t data_role_store(struct device *dev,
return ret;
mutex_lock(&port->port_type_lock);
- if (port->cap->data != TYPEC_PORT_DRD) {
+ if (port->port_roles != TYPEC_PORT_DRD) {
ret = -EOPNOTSUPP;
goto unlock_and_ret;
}
@@ -1029,7 +1035,7 @@ static ssize_t data_role_show(struct device *dev,
{
struct typec_port *port = to_typec_port(dev);
- if (port->cap->data == TYPEC_PORT_DRD)
+ if (port->port_roles == TYPEC_PORT_DRD)
return sprintf(buf, "%s\n", port->data_role == TYPEC_HOST ?
"[host] device" : "host [device]");
@@ -1044,7 +1050,7 @@ static ssize_t power_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
- if (!port->cap->pd_revision) {
+ if (!port->pd_revision) {
dev_dbg(dev, "USB Power Delivery not supported\n");
return -EOPNOTSUPP;
}
@@ -1064,9 +1070,9 @@ static ssize_t power_role_store(struct device *dev,
return ret;
mutex_lock(&port->port_type_lock);
- if (port->port_type != TYPEC_PORT_DRP) {
+ if (port->fixed_role != TYPEC_PORT_DRP) {
dev_dbg(dev, "port type fixed at \"%s\"",
-typec_port_power_roles[port->port_type]);
+typec_port_power_roles[port->fixed_role]);
ret = -EOPNOTSUPP;
goto unlock_and_ret;
}
@@ -1086,7 +1092,7 @@ static ssize_t power_role_show(struct device *dev,
{
struct typec_port *port = to_typec_port(dev);
- if (port->cap->type == TYPEC_PORT_DRP)
+ if (port->fixed_role == TYPEC_PORT_DRP)
return sprintf(buf, "%s\n", port->pwr_role == TYPEC_SOURCE ?
"[source] sink" : "source [sink]");
@@ -1102,7 +1108,7 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
int ret;
enum typec_port_type type;
- if (!port->cap->port_type_set || port->cap->type != TYPEC_PORT_DRP) {
+ if (!port->cap->port_type_set || port->fixed_role != TYPEC_PORT_DRP) {
dev_dbg(dev, "changing port type not supported\n");
return -EOPNOTSUPP;
}
@@ -1114,7 +1120,7 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
type = ret;
mutex_lock(&port->port_type_lock);
- if (port->port_type == type) {
+ if (port->fixed_role == type) {
ret = size;
goto unlock_and_ret;
}
@@ -1123,7 +1129,7 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
if (ret)
goto unlock_and_ret;
- port->port_type = type;
+ port->fixed_role = type;
ret = size;
unlock_and_ret:
@@ -1137,11 +1143,11 @@ port_type_show(struct device *dev, struct
device_attribute *attr,
{
struct typec_port *port = to_typec_port(dev);
- if (port->cap->type == TYPEC_PORT_DRP)
+ if (port->fixed_role == TYPEC_PORT_DRP)
return sprintf(buf, "%s\n",
- typ
[PATCH 03/14] usb: typec: Separate the operations vector
Introducing struct typec_operations which has the same
callbacks as struct typec_capability. The old callbacks are
kept for now, but after all users have been converted, they
will be removed.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 90 +--
include/linux/usb/typec.h | 19 +
2 files changed, 76 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9fab0be8f08c..542be63795db 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -59,6 +59,7 @@ struct typec_port {
struct typec_mux*mux;
const struct typec_capability *cap;
+ const struct typec_operations *ops;
};
#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
@@ -961,11 +962,6 @@ preferred_role_store(struct device *dev, struct
device_attribute *attr,
return -EOPNOTSUPP;
}
- if (!port->cap->try_role) {
- dev_dbg(dev, "Setting preferred role not supported\n");
- return -EOPNOTSUPP;
- }
-
role = sysfs_match_string(typec_roles, buf);
if (role < 0) {
if (sysfs_streq(buf, "none"))
@@ -974,9 +970,18 @@ preferred_role_store(struct device *dev, struct
device_attribute *attr,
return -EINVAL;
}
- ret = port->cap->try_role(port->cap, role);
- if (ret)
- return ret;
+ if (port->ops && port->ops->try_role) {
+ ret = port->ops->try_role(port, role);
+ if (ret)
+ return ret;
+ } else if (port->cap && port->cap->try_role) {
+ ret = port->cap->try_role(port->cap, role);
+ if (ret)
+ return ret;
+ } else {
+ dev_dbg(dev, "Setting preferred role not supported\n");
+ return -EOPNOTSUPP;
+ }
port->prefer_role = role;
return size;
@@ -1005,11 +1010,6 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
- if (!port->cap->dr_set) {
- dev_dbg(dev, "data role swapping not supported\n");
- return -EOPNOTSUPP;
- }
-
ret = sysfs_match_string(typec_data_roles, buf);
if (ret < 0)
return ret;
@@ -1020,9 +1020,19 @@ static ssize_t data_role_store(struct device *dev,
goto unlock_and_ret;
}
- ret = port->cap->dr_set(port->cap, ret);
- if (ret)
+ if (port->ops && port->ops->dr_set) {
+ ret = port->ops->dr_set(port, ret);
+ if (ret)
+ goto unlock_and_ret;
+ } else if (port->cap && port->cap->dr_set) {
+ ret = port->cap->dr_set(port->cap, ret);
+ if (ret)
+ goto unlock_and_ret;
+ } else {
+ dev_dbg(dev, "data role swapping not supported\n");
+ ret = -EOPNOTSUPP;
goto unlock_and_ret;
+ }
ret = size;
unlock_and_ret:
@@ -1055,11 +1065,6 @@ static ssize_t power_role_store(struct device *dev,
return -EOPNOTSUPP;
}
- if (!port->cap->pr_set) {
- dev_dbg(dev, "power role swapping not supported\n");
- return -EOPNOTSUPP;
- }
-
if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
dev_dbg(dev, "partner unable to swap power role\n");
return -EIO;
@@ -1077,11 +1082,21 @@ static ssize_t power_role_store(struct device *dev,
goto unlock_and_ret;
}
- ret = port->cap->pr_set(port->cap, ret);
- if (ret)
+ if (port->ops && port->ops->pr_set) {
+ ret = port->ops->pr_set(port, ret);
+ if (ret)
+ goto unlock_and_ret;
+ } else if (port->cap && port->cap->pr_set) {
+ ret = port->cap->pr_set(port->cap, ret);
+ if (ret)
+ goto unlock_and_ret;
+ } else {
+ dev_dbg(dev, "power role swapping not supported\n");
+ ret = -EOPNOTSUPP;
goto unlock_and_ret;
-
+ }
ret = size;
+
unlock_and_ret:
mutex_unlock(&port->port_type_lock);
return ret;
@@ -1108,7 +1123,8 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
int ret;
enum typec_port_type type;
- if (!port->cap->port_type_set || port->fixed_role != TYPEC_PORT_DRP) {
+ if ((!port->ops || !port->ops->port_type_set) ||
+ !port->cap->port_type_set || port->fixed_role != TYPEC_PORT_DRP) {
dev_dbg(dev, "changing port type not supported\n");
return -EOPNOTSUPP;
}
@@ -1125,7 +1141,10 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
goto unlock_an
[PATCH 04/14] usb: typec: tcpm: Start using struct typec_operations
Supplying the operation callbacks as part of a struct
typec_operations instead of as part of struct
typec_capability during port registration.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/tcpm/tcpm.c | 47 ---
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 96562744101c..0fde7e042d5f 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -390,12 +390,6 @@ static enum tcpm_state tcpm_default_state(struct tcpm_port
*port)
return SRC_UNATTACHED;
}
-static inline
-struct tcpm_port *typec_cap_to_tcpm(const struct typec_capability *cap)
-{
- return container_of(cap, struct tcpm_port, typec_caps);
-}
-
static bool tcpm_port_is_disconnected(struct tcpm_port *port)
{
return (!port->attached && port->cc1 == TYPEC_CC_OPEN &&
@@ -3970,10 +3964,9 @@ void tcpm_pd_hard_reset(struct tcpm_port *port)
}
EXPORT_SYMBOL_GPL(tcpm_pd_hard_reset);
-static int tcpm_dr_set(const struct typec_capability *cap,
- enum typec_data_role data)
+static int tcpm_dr_set(struct typec_port *p, enum typec_data_role data)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
int ret;
mutex_lock(&port->swap_lock);
@@ -4038,10 +4031,9 @@ static int tcpm_dr_set(const struct typec_capability
*cap,
return ret;
}
-static int tcpm_pr_set(const struct typec_capability *cap,
- enum typec_role role)
+static int tcpm_pr_set(struct typec_port *p, enum typec_role role)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
int ret;
mutex_lock(&port->swap_lock);
@@ -4082,10 +4074,9 @@ static int tcpm_pr_set(const struct typec_capability
*cap,
return ret;
}
-static int tcpm_vconn_set(const struct typec_capability *cap,
- enum typec_role role)
+static int tcpm_vconn_set(struct typec_port *p, bool source)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
int ret;
mutex_lock(&port->swap_lock);
@@ -4096,7 +4087,7 @@ static int tcpm_vconn_set(const struct typec_capability
*cap,
goto port_unlock;
}
- if (role == port->vconn_role) {
+ if (source == port->vconn_role) {
ret = 0;
goto port_unlock;
}
@@ -4122,9 +4113,9 @@ static int tcpm_vconn_set(const struct typec_capability
*cap,
return ret;
}
-static int tcpm_try_role(const struct typec_capability *cap, int role)
+static int tcpm_try_role(struct typec_port *p, int role)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
struct tcpc_dev *tcpc = port->tcpc;
int ret = 0;
@@ -4331,10 +4322,9 @@ static void tcpm_init(struct tcpm_port *port)
tcpm_set_state(port, PORT_RESET, 0);
}
-static int tcpm_port_type_set(const struct typec_capability *cap,
- enum typec_port_type type)
+static int tcpm_port_type_set(struct typec_port *p, enum typec_port_type type)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
mutex_lock(&port->lock);
if (type == port->port_type)
@@ -4359,6 +4349,14 @@ static int tcpm_port_type_set(const struct
typec_capability *cap,
return 0;
}
+static const struct typec_operations tcpm_ops = {
+ .try_role = tcpm_try_role,
+ .dr_set = tcpm_dr_set,
+ .pr_set = tcpm_pr_set,
+ .vconn_set = tcpm_vconn_set,
+ .port_type_set = tcpm_port_type_set
+};
+
void tcpm_tcpc_reset(struct tcpm_port *port)
{
mutex_lock(&port->lock);
@@ -4770,11 +4768,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev,
struct tcpc_dev *tcpc)
port->typec_caps.fwnode = tcpc->fwnode;
port->typec_caps.revision = 0x0120; /* Type-C spec release 1.2 */
port->typec_caps.pd_revision = 0x0300; /* USB-PD spec release 3.0 */
- port->typec_caps.dr_set = tcpm_dr_set;
- port->typec_caps.pr_set = tcpm_pr_set;
- port->typec_caps.vconn_set = tcpm_vconn_set;
- port->typec_caps.try_role = tcpm_try_role;
- port->typec_caps.port_type_set = tcpm_port_type_set;
+ port->typec_caps.driver_data = port;
+ port->typec_caps.ops = &tcpm_ops;
port->partner_desc.identity = &port->partner_ident;
port->port_type = port->typec_caps.type;
--
2.23.0
[PATCH 05/14] usb: typec: tps6598x: Start using struct typec_operations
Supplying the operation callbacks as part of a struct
typec_operations instead of as part of struct
typec_capability during port registration. After this there
is not need to keep the capabilities stored anywhere in the
driver.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/tps6598x.c | 49 +++-
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index a38d1409f15b..0698addd1185 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -94,7 +94,6 @@ struct tps6598x {
struct typec_port *port;
struct typec_partner *partner;
struct usb_pd_identity partner_identity;
- struct typec_capability typec_cap;
};
/*
@@ -307,11 +306,10 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const
char *cmd,
return 0;
}
-static int
-tps6598x_dr_set(const struct typec_capability *cap, enum typec_data_role role)
+static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
{
- struct tps6598x *tps = container_of(cap, struct tps6598x, typec_cap);
const char *cmd = (role == TYPEC_DEVICE) ? "SWUF" : "SWDF";
+ struct tps6598x *tps = typec_get_drvdata(port);
u32 status;
int ret;
@@ -338,11 +336,10 @@ tps6598x_dr_set(const struct typec_capability *cap, enum
typec_data_role role)
return ret;
}
-static int
-tps6598x_pr_set(const struct typec_capability *cap, enum typec_role role)
+static int tps6598x_pr_set(struct typec_port *port, enum typec_role role)
{
- struct tps6598x *tps = container_of(cap, struct tps6598x, typec_cap);
const char *cmd = (role == TYPEC_SINK) ? "SWSk" : "SWSr";
+ struct tps6598x *tps = typec_get_drvdata(port);
u32 status;
int ret;
@@ -369,6 +366,11 @@ tps6598x_pr_set(const struct typec_capability *cap, enum
typec_role role)
return ret;
}
+static const struct typec_operations tps6598x_ops = {
+ .dr_set = tps6598x_dr_set,
+ .pr_set = tps6598x_pr_set,
+};
+
static irqreturn_t tps6598x_interrupt(int irq, void *data)
{
struct tps6598x *tps = data;
@@ -448,6 +450,7 @@ static const struct regmap_config tps6598x_regmap_config = {
static int tps6598x_probe(struct i2c_client *client)
{
+ struct typec_capability typec_cap = { };
struct tps6598x *tps;
u32 status;
u32 conf;
@@ -492,40 +495,40 @@ static int tps6598x_probe(struct i2c_client *client)
if (ret < 0)
return ret;
- tps->typec_cap.revision = USB_TYPEC_REV_1_2;
- tps->typec_cap.pd_revision = 0x200;
- tps->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
- tps->typec_cap.pr_set = tps6598x_pr_set;
- tps->typec_cap.dr_set = tps6598x_dr_set;
+ typec_cap.revision = USB_TYPEC_REV_1_2;
+ typec_cap.pd_revision = 0x200;
+ typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+ typec_cap.driver_data = tps;
+ typec_cap.ops = &tps6598x_ops;
switch (TPS_SYSCONF_PORTINFO(conf)) {
case TPS_PORTINFO_SINK_ACCESSORY:
case TPS_PORTINFO_SINK:
- tps->typec_cap.type = TYPEC_PORT_SNK;
- tps->typec_cap.data = TYPEC_PORT_UFP;
+ typec_cap.type = TYPEC_PORT_SNK;
+ typec_cap.data = TYPEC_PORT_UFP;
break;
case TPS_PORTINFO_DRP_UFP_DRD:
case TPS_PORTINFO_DRP_DFP_DRD:
- tps->typec_cap.type = TYPEC_PORT_DRP;
- tps->typec_cap.data = TYPEC_PORT_DRD;
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_DRD;
break;
case TPS_PORTINFO_DRP_UFP:
- tps->typec_cap.type = TYPEC_PORT_DRP;
- tps->typec_cap.data = TYPEC_PORT_UFP;
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_UFP;
break;
case TPS_PORTINFO_DRP_DFP:
- tps->typec_cap.type = TYPEC_PORT_DRP;
- tps->typec_cap.data = TYPEC_PORT_DFP;
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_DFP;
break;
case TPS_PORTINFO_SOURCE:
- tps->typec_cap.type = TYPEC_PORT_SRC;
- tps->typec_cap.data = TYPEC_PORT_DFP;
+ typec_cap.type = TYPEC_PORT_SRC;
+ typec_cap.data = TYPEC_PORT_DFP;
break;
default:
return -ENODEV;
}
- tps->port = typec_register_port(&client->dev, &tps->typec_cap);
+ tps->port = typec_register_port(&client->dev, &typec_cap);
if (IS_ERR(tps->port))
return PTR_ERR(tps->port);
--
2.23.0
[PATCH 02/14] usb: typec: Introduce typec_get_drvdata()
Leaving the private driver_data pointer of the port device
to the port drivers.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 11 +++
include/linux/usb/typec.h | 4
2 files changed, 15 insertions(+)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 3835e2d9fba6..9fab0be8f08c 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1492,6 +1492,16 @@ EXPORT_SYMBOL_GPL(typec_set_mode);
/* --- */
+/**
+ * typec_get_drvdata - Return private driver data pointer
+ * @port: USB Type-C port
+ */
+void *typec_get_drvdata(struct typec_port *port)
+{
+ return dev_get_drvdata(&port->dev);
+}
+EXPORT_SYMBOL_GPL(typec_get_drvdata);
+
/**
* typec_port_register_altmode - Register USB Type-C Port Alternate Mode
* @port: USB Type-C Port that supports the alternate mode
@@ -1604,6 +1614,7 @@ struct typec_port *typec_register_port(struct device
*parent,
port->dev.fwnode = cap->fwnode;
port->dev.type = &typec_port_dev_type;
dev_set_name(&port->dev, "port%d", id);
+ dev_set_drvdata(&port->dev, cap->driver_data);
port->sw = typec_switch_get(&port->dev);
if (IS_ERR(port->sw)) {
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 7df4ecabc78a..8b90cd77331c 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -179,6 +179,7 @@ struct typec_partner_desc {
* @sw: Cable plug orientation switch
* @mux: Multiplexer switch for Alternate/Accessory Modes
* @fwnode: Optional fwnode of the port
+ * @driver_data: Private pointer for driver specific info
* @try_role: Set data role preference for DRP port
* @dr_set: Set Data Role
* @pr_set: Set Power Role
@@ -198,6 +199,7 @@ struct typec_capability {
struct typec_switch *sw;
struct typec_mux*mux;
struct fwnode_handle*fwnode;
+ void*driver_data;
int (*try_role)(const struct typec_capability *,
int role);
@@ -241,6 +243,8 @@ int typec_set_orientation(struct typec_port *port,
enum typec_orientation typec_get_orientation(struct typec_port *port);
int typec_set_mode(struct typec_port *port, int mode);
+void *typec_get_drvdata(struct typec_port *port);
+
int typec_find_port_power_role(const char *name);
int typec_find_power_role(const char *name);
int typec_find_port_data_role(const char *name);
--
2.23.0
[PATCH 08/14] usb: typec: ucsi: ccg: Remove run_isr flag
The "run_isr" flag is used for preventing the driver from
calling the interrupt service routine in its runtime resume
callback when the driver is expecting completion to a
command, but what that basically does is that it hides the
real problem. The real problem is that the controller is
allowed to suspend in the middle of command execution.
As a more appropriate fix for the problem, using autosuspend
delay time that matches UCSI_TIMEOUT_MS (5s). That prevents
the controller from suspending while still in the middle of
executing a command.
This fixes a potential deadlock. Both ccg_read() and
ccg_write() are called with the mutex already taken at least
from ccg_send_command(). In ccg_read() and ccg_write, the
mutex is only acquired so that run_isr flag can be set.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi_ccg.c | 42 +++
1 file changed, 4 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 907e20e1a71e..d772fce51905 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -195,7 +195,6 @@ struct ucsi_ccg {
/* fw build with vendor information */
u16 fw_build;
- bool run_isr; /* flag to call ISR routine during resume */
struct work_struct pm_work;
};
@@ -224,18 +223,6 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8
*data, u32 len)
if (quirks && quirks->max_read_len)
max_read_len = quirks->max_read_len;
- if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
- uc->fw_version <= CCG_OLD_FW_VERSION) {
- mutex_lock(&uc->lock);
- /*
-* Do not schedule pm_work to run ISR in
-* ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
-* since we are already in ISR path.
-*/
- uc->run_isr = false;
- mutex_unlock(&uc->lock);
- }
-
pm_runtime_get_sync(uc->dev);
while (rem_len > 0) {
msgs[1].buf = &data[len - rem_len];
@@ -278,18 +265,6 @@ static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8
*data, u32 len)
msgs[0].len = len + sizeof(rab);
msgs[0].buf = buf;
- if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
- uc->fw_version <= CCG_OLD_FW_VERSION) {
- mutex_lock(&uc->lock);
- /*
-* Do not schedule pm_work to run ISR in
-* ucsi_ccg_runtime_resume() after pm_runtime_get_sync()
-* since we are already in ISR path.
-*/
- uc->run_isr = false;
- mutex_unlock(&uc->lock);
- }
-
pm_runtime_get_sync(uc->dev);
status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
if (status < 0) {
@@ -1130,7 +1105,6 @@ static int ucsi_ccg_probe(struct i2c_client *client,
uc->ppm.sync = ucsi_ccg_sync;
uc->dev = dev;
uc->client = client;
- uc->run_isr = true;
mutex_init(&uc->lock);
INIT_WORK(&uc->work, ccg_update_firmware);
INIT_WORK(&uc->pm_work, ccg_pm_workaround_work);
@@ -1188,6 +1162,8 @@ static int ucsi_ccg_probe(struct i2c_client *client,
pm_runtime_set_active(uc->dev);
pm_runtime_enable(uc->dev);
+ pm_runtime_use_autosuspend(uc->dev);
+ pm_runtime_set_autosuspend_delay(uc->dev, 5000);
pm_runtime_idle(uc->dev);
return 0;
@@ -1229,7 +1205,6 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct ucsi_ccg *uc = i2c_get_clientdata(client);
- bool schedule = true;
/*
* Firmware version 3.1.10 or earlier, built for NVIDIA has known issue
@@ -1237,17 +1212,8 @@ static int ucsi_ccg_runtime_resume(struct device *dev)
* Schedule a work to call ISR as a workaround.
*/
if (uc->fw_build == CCG_FW_BUILD_NVIDIA &&
- uc->fw_version <= CCG_OLD_FW_VERSION) {
- mutex_lock(&uc->lock);
- if (!uc->run_isr) {
- uc->run_isr = true;
- schedule = false;
- }
- mutex_unlock(&uc->lock);
-
- if (schedule)
- schedule_work(&uc->pm_work);
- }
+ uc->fw_version <= CCG_OLD_FW_VERSION)
+ schedule_work(&uc->pm_work);
return 0;
}
--
2.23.0
[PATCH 00/14] usb: typec: UCSI driver overhaul
Hi Ajay, Here's the pretty much complete rewrite of the I/O handling that I was talking about. The first seven patches are not actually related to this stuff, but I'm including them here because the rest of the series is made on top of them. I'm including also that fix patch I send you earlier. After this it should be easier to handle quirks. My idea how to handle the multi-instance connector alt modes is that we "emulate" the PPM in ucsi_ccg.c in order to handle them, so ucsi.c is not touched at all. We can now get the connector alternate modes that the actual controller supplies during probe - before registering the ucsi interface - and squash all alt modes with the same SVID into one that we supply to the ucsi.c when ever it sends GET_ALTERNATE_MODES command. Also other alt mode commands like SET_NEW_CAM can have special processing in ucsi_ccg.c and ucsi_ccg.c alone. There should not be any problem with that anymore. thanks, Heikki Krogerus (14): usb: typec: Copy everything from struct typec_capability during registration usb: typec: Introduce typec_get_drvdata() usb: typec: Separate the operations vector usb: typec: tcpm: Start using struct typec_operations usb: typec: tps6598x: Start using struct typec_operations usb: typec: ucsi: Start using struct typec_operations usb: typec: Remove the callback members from struct typec_capability usb: typec: ucsi: ccg: Remove run_isr flag usb: typec: ucsi: Simplified interface registration and I/O API usb: typec: ucsi: acpi: Move to the new API usb: typec: ucsi: ccg: Move to the new API usb: typec: ucsi: Remove the old API usb: typec: ucsi: Remove struct ucsi_control usb: typec: ucsi: Remove all bit-fields drivers/usb/typec/class.c| 125 +++--- drivers/usb/typec/tcpm/tcpm.c| 47 +-- drivers/usb/typec/tps6598x.c | 49 +-- drivers/usb/typec/ucsi/displayport.c | 26 +- drivers/usb/typec/ucsi/trace.c | 11 - drivers/usb/typec/ucsi/trace.h | 79 +--- drivers/usb/typec/ucsi/ucsi.c| 592 ++- drivers/usb/typec/ucsi/ucsi.h| 410 +++ drivers/usb/typec/ucsi/ucsi_acpi.c | 96 - drivers/usb/typec/ucsi/ucsi_ccg.c| 214 -- include/linux/usb/typec.h| 38 +- 11 files changed, 785 insertions(+), 902 deletions(-) -- 2.23.0
[PATCH 07/14] usb: typec: Remove the callback members from struct typec_capability
There are no more users for them.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 65 +--
include/linux/usb/typec.h | 17 --
2 files changed, 22 insertions(+), 60 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 542be63795db..58e83fc54aa6 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -58,7 +58,6 @@ struct typec_port {
struct typec_switch *sw;
struct typec_mux*mux;
- const struct typec_capability *cap;
const struct typec_operations *ops;
};
@@ -970,19 +969,15 @@ preferred_role_store(struct device *dev, struct
device_attribute *attr,
return -EINVAL;
}
- if (port->ops && port->ops->try_role) {
- ret = port->ops->try_role(port, role);
- if (ret)
- return ret;
- } else if (port->cap && port->cap->try_role) {
- ret = port->cap->try_role(port->cap, role);
- if (ret)
- return ret;
- } else {
+ if (!port->ops || !port->ops->try_role) {
dev_dbg(dev, "Setting preferred role not supported\n");
return -EOPNOTSUPP;
}
+ ret = port->ops->try_role(port, role);
+ if (ret)
+ return ret;
+
port->prefer_role = role;
return size;
}
@@ -1020,20 +1015,16 @@ static ssize_t data_role_store(struct device *dev,
goto unlock_and_ret;
}
- if (port->ops && port->ops->dr_set) {
- ret = port->ops->dr_set(port, ret);
- if (ret)
- goto unlock_and_ret;
- } else if (port->cap && port->cap->dr_set) {
- ret = port->cap->dr_set(port->cap, ret);
- if (ret)
- goto unlock_and_ret;
- } else {
+ if (!port->ops || !port->ops->dr_set) {
dev_dbg(dev, "data role swapping not supported\n");
ret = -EOPNOTSUPP;
goto unlock_and_ret;
}
+ ret = port->ops->dr_set(port, ret);
+ if (ret)
+ goto unlock_and_ret;
+
ret = size;
unlock_and_ret:
mutex_unlock(&port->port_type_lock);
@@ -1082,21 +1073,17 @@ static ssize_t power_role_store(struct device *dev,
goto unlock_and_ret;
}
- if (port->ops && port->ops->pr_set) {
- ret = port->ops->pr_set(port, ret);
- if (ret)
- goto unlock_and_ret;
- } else if (port->cap && port->cap->pr_set) {
- ret = port->cap->pr_set(port->cap, ret);
- if (ret)
- goto unlock_and_ret;
- } else {
+ if (!port->ops || !port->ops->pr_set) {
dev_dbg(dev, "power role swapping not supported\n");
ret = -EOPNOTSUPP;
goto unlock_and_ret;
}
ret = size;
+ ret = port->ops->pr_set(port, ret);
+ if (ret)
+ goto unlock_and_ret;
+
unlock_and_ret:
mutex_unlock(&port->port_type_lock);
return ret;
@@ -1124,7 +,7 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
enum typec_port_type type;
if ((!port->ops || !port->ops->port_type_set) ||
- !port->cap->port_type_set || port->fixed_role != TYPEC_PORT_DRP) {
+ port->fixed_role != TYPEC_PORT_DRP) {
dev_dbg(dev, "changing port type not supported\n");
return -EOPNOTSUPP;
}
@@ -1141,10 +1128,7 @@ port_type_store(struct device *dev, struct
device_attribute *attr,
goto unlock_and_ret;
}
- if (port->ops && port->ops->port_type_set)
- ret = port->ops->port_type_set(port, type);
- else
- ret = port->cap->port_type_set(port->cap, type);
+ ret = port->ops->port_type_set(port, type);
if (ret)
goto unlock_and_ret;
@@ -1204,19 +1188,15 @@ static ssize_t vconn_source_store(struct device *dev,
if (ret)
return ret;
- if (port->ops && port->ops->vconn_set) {
- ret = port->ops->vconn_set(port, source);
- if (ret)
- return ret;
- } else if (port->cap && port->cap->vconn_set) {
- ret = port->cap->vconn_set(port->cap, (enum typec_role)source);
- if (ret)
- return ret;
- } else {
+ if (!port->ops || !port->ops->vconn_set) {
dev_dbg(dev, "VCONN swapping not supported\n");
return -EOPNOTSUPP;
}
+ ret = port->ops->vconn_set(port, source);
+ if (ret)
+ return ret;
+
return size;
}
@@ -1619,7 +1599,6 @@ struct typec_port *typec_register_port(struct device
*parent,
mutex_init(&port->po
Re: [PATCH 1/1] usb: host: xhci: update event ring dequeue pointer on purpose
On 24.9.2019 11.35, Peter Chen wrote:
On some situations, the software handles TRB events slower
than adding TRBs, then xhci_handle_event can't return zero
long time, the xHC will consider the event ring is full,
and trigger "Event Ring Full" error, but in fact, the software
has already finished lots of events, just no chance to
update ERDP (event ring dequeue pointer).
In this commit, we force update ERDP if half of TRBS_PER_SEGMENT
events have handled to avoid "Event Ring Full" error.
Signed-off-by: Peter Chen
---
drivers/usb/host/xhci-ring.c | 53
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e220bcbee173..92b6b07cf33d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2737,6 +2737,35 @@ static int xhci_handle_event(struct xhci_hcd *xhci)
return 1;
}
+/*
+ * Update Event Ring Dequeue Pointer:
+ * - When all events have finished
+ * - To avoid "Event Ring Full Error" condition
+ */
+static void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
+ union xhci_trb *event_ring_deq)
+{
+ u64 temp_64;
+ dma_addr_t deq;
+
+ temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
+ /* If necessary, update the HW's version of the event ring deq ptr. */
+ if (event_ring_deq != xhci->event_ring->dequeue) {
+ deq = xhci_trb_virt_to_dma(xhci->event_ring->deq_seg,
+ xhci->event_ring->dequeue);
+ if (deq == 0)
+ xhci_warn(xhci, "WARN something wrong with SW event "
+ "ring dequeue ptr.\n");
+ /* Update HC event ring dequeue pointer */
+ temp_64 &= ERST_PTR_MASK;
+ temp_64 |= ((u64) deq & (u64) ~ERST_PTR_MASK);
+ }
+
+ /* Clear the event handler busy flag (RW1C) */
+ temp_64 |= ERST_EHB;
+ xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue);
+}
+
/*
* xHCI spec says we can get an interrupt, and if the HC has an error
condition,
* we might get bad data out of the event ring. Section 4.10.2.7 has a list
of
@@ -2748,9 +2777,9 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
union xhci_trb *event_ring_deq;
irqreturn_t ret = IRQ_NONE;
unsigned long flags;
- dma_addr_t deq;
u64 temp_64;
u32 status;
+ int event_loop = 0;
spin_lock_irqsave(&xhci->lock, flags);
/* Check if the xHC generated the interrupt, or the irq is shared */
@@ -2804,24 +2833,14 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
/* FIXME this should be a delayed service routine
* that clears the EHB.
*/
- while (xhci_handle_event(xhci) > 0) {}
-
- temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
- /* If necessary, update the HW's version of the event ring deq ptr. */
- if (event_ring_deq != xhci->event_ring->dequeue) {
- deq = xhci_trb_virt_to_dma(xhci->event_ring->deq_seg,
- xhci->event_ring->dequeue);
- if (deq == 0)
- xhci_warn(xhci, "WARN something wrong with SW event "
- "ring dequeue ptr.\n");
- /* Update HC event ring dequeue pointer */
- temp_64 &= ERST_PTR_MASK;
- temp_64 |= ((u64) deq & (u64) ~ERST_PTR_MASK);
+ while (xhci_handle_event(xhci) > 0) {
+ if (event_loop++ < TRBS_PER_SEGMENT / 2)
+ continue;
+ xhci_update_erst_dequeue(xhci, event_ring_deq);
+ event_loop = 0;
}
- /* Clear the event handler busy flag (RW1C); event ring is empty. */
- temp_64 |= ERST_EHB;
- xhci_write_64(xhci, temp_64, &xhci->ir_set->erst_dequeue);
+ xhci_update_erst_dequeue(xhci, event_ring_deq);
Otherwise looks good, but in rare cases when we handle TRBS_PER_SEGMENT/2
events
we might write the ERST twice in a row with the same dequeue value.
xHCI specification section 4.9.4 forbids this:
"Note: Software writes to the ERDP register shall always advance the Event Ring
Dequeue Pointer value, i.e. software shall not write the same value to the ERDP
register on two consecutive write operations."
-Mathias
Re: BUG report: usb: dwc3: Link TRB triggered an intterupt without IOC being setted
Hi, Mathias Nyman 于2019年9月26日周四 下午4:19写道： > > On 26.9.2019 8.45, Felipe Balbi wrote: > > > > Hi, > > > > David Laight writes: > >> From: Mathias Nyman > >>> Sent: 25 September 2019 15:48 > >>> > >>> On 24.9.2019 17.45, alex zheng wrote: > Hi Mathias, > >> ... > >>> Logs show your transfer ring has four segments, but hardware fails to > >>> jump from the last segment back to first) > >>> > >>> Last TRB (LINK TRB) of each segment points to the next segment, > >>> last segments link trb points back to first segment. > >>> > >>> In your case: > >>> 0x1d117000 -> 0x1eb09000 -> 0x1eb0a000 -> 0x1dbda000 -> (back to > >>> 0x1d117000) > >>> > >>> For some reason your hardware doesn't treat the last TRB at the last > >>> segment > >>> as a LINK TRB, instead it just issues a transfer event for it, and > >>> continues to > >>> the next address instead of jumping back to first segment: > >> > >> That could be a cache coherency (or flushing (etc)) issue. > > The Link TRB is written very early, right after the ring segment is allocated, > and before any other TRBs. 255 other TRBs were written and handled by hw > on this segment after this, so not very likely a flushing/cache coherency > issue. > I add a flush_cache_all() after queue_trb everytime but it make no use. It seems not a flushing/cache coherency issus. flush like this: inc_enq(xhci, ring, more_trbs_coming); + flush_cache_all(); > > > > XHCI has a HW-configurable maximum number of segments in a ring. AFAICT, > > xhci driver doesn't take that into consideration today. Perhaps the HW > > in question doesn't like more than 3 segments. > > > > Mathias, what was the register to check this? Do you remember? > > > > I only recall a limit for the event ring in the HSCPARAMS2 register(ERST MAX), > not for transfer rings. > > Other things to look at would be > > - check that Toggle Cycle bit is correct for last segments link TRB > (incomplete logs) I dump an other error log, more complete logs see attached file(transfer_error_0926.cap), in the log: the error link TRB: 0x1d00dff0: TRB 1d068000 status 'Invalid' len 0 slot 0 ep 0 type 'Link' flags e:c and last segment link TRB: 0x1eb0aff0: TRB 1d00d000 status 'Invalid' len 0 slot 0 ep 0 type 'Link' flags e:C > - some old xHCI hardware needed the Chain bit set in link TRB for some isoc > rings xhci ver is 1.1: 6.888570] c1 46 (kworker/u8:1) xhci-hcd xhci-hcd.0.auto: HCIVERSION: 0x110 > - was ring recently expanded?, usually rings start with only two segments The extra segments are expanded after raw data test run a while, especially when the RNDIS test(iperf3) begin to run. Other info: 1. This issue seems only happened when the raw bulk data test and the rndis test(other pair endpoints) run at the same time, and happens more often if we queue trb more quick. 2. The raw bulk data test case is a libusb test use ep4(in) & ep3(out) to transfer raw bulk data, and I use iperf3(tcp) to test USB rndis. 3. The log file attached only show ep4(in) enqueue/dequeue log for more readable, 4. More test result show as below: 1) run just one raw bulk data test --> (always fine) 2) run raw rulk data test + rndis test run at the same time --> (transfer error in 10 minutes) 3) run two raw bulk data test run at the same time (with two pair endpoint) --> (transfer error in 10 minutes) 5. I try to modify the DWC3 hw registers like TX/RX FIFO size, GTXTHRCFG/GRXTHRCFG , but also did not work. 6. Related interface info: 8801 I:* If#= 0 Alt= 0 #EPs= 1 Cls=e0(wlcon) Sub=01 Prot=03 Driver=rndis_host 8802 E: Ad=82(I) Atr=03(Int.) MxPS= 8 Ivl=32ms 8803 I:* If#= 1 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=rndis_host-> used in rndis test 8804 E: Ad=81(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms 8805 E: Ad=01(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms 8809 I:* If#= 3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=43 Prot=01 Driver=(none)-> used in raw bulk test 8810 E: Ad=03(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms 8811 E: Ad=84(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms 8820 I:* If#= 7 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=43 Prot=01 Driver=(none) > used in double raw bulk test 8821 E: Ad=06(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms 8822 E: Ad=88(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms It seems that there are some conflicts when multiple endpoints work at the same time on our SOC. Are there any other way can try? > > Mathias
[PATCH 10/14] usb: typec: ucsi: acpi: Move to the new API
Replacing the old "cmd" and "sync" callbacks with an
implementation of struct ucsi_operations. The ACPI
notification (interrupt) handler will from now on read the
CCI (Command Status and Connector Change Indication)
register, and call ucsi_connector_change() function and/or
complete pending command completions based on it.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi_acpi.c | 96 --
1 file changed, 77 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c
b/drivers/usb/typec/ucsi/ucsi_acpi.c
index a18112a83fae..876510acbf9f 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -19,7 +19,9 @@
struct ucsi_acpi {
struct device *dev;
struct ucsi *ucsi;
- struct ucsi_ppm ppm;
+ void __iomem *base;
+ struct completion complete;
+ unsigned long flags;
guid_t guid;
};
@@ -39,27 +41,78 @@ static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func)
return 0;
}
-static int ucsi_acpi_cmd(struct ucsi_ppm *ppm, struct ucsi_control *ctrl)
+static int ucsi_acpi_read(struct ucsi *ucsi, unsigned int offset,
+ void *val, size_t val_len)
{
- struct ucsi_acpi *ua = container_of(ppm, struct ucsi_acpi, ppm);
+ struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+ int ret;
+
+ ret = ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_READ);
+ if (ret)
+ return ret;
- ppm->data->ctrl.raw_cmd = ctrl->raw_cmd;
+ memcpy(val, ua->base + offset, val_len);
+
+ return 0;
+}
+
+static int ucsi_acpi_async_write(struct ucsi *ucsi, unsigned int offset,
+const void *val, size_t val_len)
+{
+ struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+
+ memcpy(ua->base + offset, val, val_len);
return ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_WRITE);
}
-static int ucsi_acpi_sync(struct ucsi_ppm *ppm)
+static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset,
+ const void *val, size_t val_len)
{
- struct ucsi_acpi *ua = container_of(ppm, struct ucsi_acpi, ppm);
+ struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+ int ret;
+
+ set_bit(COMMAND_PENDING, &ua->flags);
+
+ ret = ucsi_acpi_async_write(ucsi, offset, val, val_len);
+ if (ret)
+ goto out_clear_bit;
+
+ if (!wait_for_completion_timeout(&ua->complete, msecs_to_jiffies(5000)))
+ ret = -ETIMEDOUT;
- return ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_READ);
+out_clear_bit:
+ clear_bit(COMMAND_PENDING, &ua->flags);
+
+ return ret;
}
+static const struct ucsi_operations ucsi_acpi_ops = {
+ .read = ucsi_acpi_read,
+ .sync_write = ucsi_acpi_sync_write,
+ .async_write = ucsi_acpi_async_write
+};
+
static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
{
struct ucsi_acpi *ua = data;
+ u32 cci;
+ int ret;
+
+ ret = ucsi_acpi_read(ua->ucsi, UCSI_CCI, &cci, sizeof(cci));
+ if (ret) {
+ dev_err(ua->dev, "failed to read CCI\n");
+ return;
+ }
+
+ if (UCSI_CCI_CONNECTOR(cci))
+ ucsi_connector_change(ua->ucsi, UCSI_CCI_CONNECTOR(cci));
- ucsi_notify(ua->ucsi);
+ if (test_bit(COMMAND_PENDING, &ua->flags) &&
+ (cci & (UCSI_CCI_BUSY |
+ UCSI_CCI_ACK_COMPLETE |
+ UCSI_CCI_COMMAND_COMPLETE)))
+ complete(&ua->complete);
}
static int ucsi_acpi_probe(struct platform_device *pdev)
@@ -90,35 +143,39 @@ static int ucsi_acpi_probe(struct platform_device *pdev)
* it can not be requested here, and we can not use
* devm_ioremap_resource().
*/
- ua->ppm.data = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!ua->ppm.data)
+ ua->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!ua->base)
return -ENOMEM;
- if (!ua->ppm.data->version)
- return -ENODEV;
-
ret = guid_parse(UCSI_DSM_UUID, &ua->guid);
if (ret)
return ret;
- ua->ppm.cmd = ucsi_acpi_cmd;
- ua->ppm.sync = ucsi_acpi_sync;
+ init_completion(&ua->complete);
ua->dev = &pdev->dev;
+ ua->ucsi = ucsi_create(&pdev->dev, &ucsi_acpi_ops);
+ if (IS_ERR(ua->ucsi))
+ return PTR_ERR(ua->ucsi);
+
+ ucsi_set_drvdata(ua->ucsi, ua);
+
status = acpi_install_notify_handler(ACPI_HANDLE(&pdev->dev),
ACPI_DEVICE_NOTIFY,
ucsi_acpi_notify, ua);
if (ACPI_FAILURE(status)) {
dev_err(&pdev->dev, "failed to install notify handler\n");
+ ucsi_destroy(ua->ucsi);
return -ENODEV;
}
- ua->ucsi = ucsi_register_ppm(&pdev->dev, &ua->p
[PATCH 11/14] usb: typec: ucsi: ccg: Move to the new API
Replacing the old "cmd" and "sync" callbacks with an
implementation of struct ucsi_operations. The interrupt
handler will from now on read the CCI (Command Status and
Connector Change Indication) register, and call
ucsi_connector_change() function and/or complete pending
command completions based on it.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/ucsi_ccg.c | 172 +++---
1 file changed, 87 insertions(+), 85 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
b/drivers/usb/typec/ucsi/ucsi_ccg.c
index d772fce51905..bbe87a71295c 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -176,8 +176,8 @@ struct ccg_resp {
struct ucsi_ccg {
struct device *dev;
struct ucsi *ucsi;
- struct ucsi_ppm ppm;
struct i2c_client *client;
+
struct ccg_dev_info info;
/* version info for boot, primary and secondary */
struct version_info version[FW2 + 1];
@@ -196,6 +196,8 @@ struct ucsi_ccg {
/* fw build with vendor information */
u16 fw_build;
struct work_struct pm_work;
+
+ struct completion complete;
};
static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
@@ -243,7 +245,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data,
u32 len)
return 0;
}
-static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+static int ccg_write(struct ucsi_ccg *uc, u16 rab, const u8 *data, u32 len)
{
struct i2c_client *client = uc->client;
unsigned char *buf;
@@ -317,88 +319,91 @@ static int ucsi_ccg_init(struct ucsi_ccg *uc)
return -ETIMEDOUT;
}
-static int ucsi_ccg_send_data(struct ucsi_ccg *uc)
+static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset,
+void *val, size_t val_len)
{
- u8 *ppm = (u8 *)uc->ppm.data;
- int status;
- u16 rab;
+ u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(offset);
- rab = CCGX_RAB_UCSI_DATA_BLOCK(offsetof(struct ucsi_data, message_out));
- status = ccg_write(uc, rab, ppm +
- offsetof(struct ucsi_data, message_out),
- sizeof(uc->ppm.data->message_out));
- if (status < 0)
- return status;
-
- rab = CCGX_RAB_UCSI_DATA_BLOCK(offsetof(struct ucsi_data, ctrl));
- return ccg_write(uc, rab, ppm + offsetof(struct ucsi_data, ctrl),
-sizeof(uc->ppm.data->ctrl));
+ return ccg_read(ucsi_get_drvdata(ucsi), reg, val, val_len);
}
-static int ucsi_ccg_recv_data(struct ucsi_ccg *uc)
+static int ucsi_ccg_async_write(struct ucsi *ucsi, unsigned int offset,
+ const void *val, size_t val_len)
{
- u8 *ppm = (u8 *)uc->ppm.data;
- int status;
- u16 rab;
+ u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(offset);
- rab = CCGX_RAB_UCSI_DATA_BLOCK(offsetof(struct ucsi_data, cci));
- status = ccg_read(uc, rab, ppm + offsetof(struct ucsi_data, cci),
- sizeof(uc->ppm.data->cci));
- if (status < 0)
- return status;
-
- rab = CCGX_RAB_UCSI_DATA_BLOCK(offsetof(struct ucsi_data, message_in));
- return ccg_read(uc, rab, ppm + offsetof(struct ucsi_data, message_in),
- sizeof(uc->ppm.data->message_in));
+ return ccg_write(ucsi_get_drvdata(ucsi), reg, val, val_len);
}
-static int ucsi_ccg_ack_interrupt(struct ucsi_ccg *uc)
+static int ucsi_ccg_sync_write(struct ucsi *ucsi, unsigned int offset,
+ const void *val, size_t val_len)
{
- int status;
- unsigned char data;
+ struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi);
+ int ret;
- status = ccg_read(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
- if (status < 0)
- return status;
+ mutex_lock(&uc->lock);
+ pm_runtime_get_sync(uc->dev);
+ set_bit(DEV_CMD_PENDING, &uc->flags);
- return ccg_write(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
-}
+ ret = ucsi_ccg_async_write(ucsi, offset, val, val_len);
+ if (ret)
+ goto err_clear_bit;
-static int ucsi_ccg_sync(struct ucsi_ppm *ppm)
-{
- struct ucsi_ccg *uc = container_of(ppm, struct ucsi_ccg, ppm);
- int status;
+ if (!wait_for_completion_timeout(&uc->complete, msecs_to_jiffies(5000)))
+ ret = -ETIMEDOUT;
- status = ucsi_ccg_recv_data(uc);
- if (status < 0)
- return status;
+err_clear_bit:
+ clear_bit(DEV_CMD_PENDING, &uc->flags);
+ pm_runtime_put_sync(uc->dev);
+ mutex_unlock(&uc->lock);
- /* ack interrupt to allow next command to run */
- return ucsi_ccg_ack_interrupt(uc);
+ return ret;
}
-static int ucsi_ccg_cmd(struct ucsi_ppm *ppm, struct ucsi_control *ctrl)
-{
- struct ucsi_ccg *uc = container_of(ppm, struct ucsi_ccg, ppm);
-
- ppm->data->ctrl.raw_cmd = ctr
[PATCH 13/14] usb: typec: ucsi: Remove struct ucsi_control
That data structure was used for constructing the commands
before executing them, but it was never really useful. Using
the structure just complicated the driver. The commands are
64-bit wide, so it is enough to simply fill a u64 variable.
No data structures needed.
This simplifies the driver considerable and makes it much
easier to for example add support for big endian systems
later on.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/displayport.c | 18 +--
drivers/usb/typec/ucsi/trace.c | 11 --
drivers/usb/typec/ucsi/trace.h | 50 +-
drivers/usb/typec/ucsi/ucsi.c| 110 +++--
drivers/usb/typec/ucsi/ucsi.h| 231 +--
5 files changed, 118 insertions(+), 302 deletions(-)
diff --git a/drivers/usb/typec/ucsi/displayport.c
b/drivers/usb/typec/ucsi/displayport.c
index 132007cbb96c..f8d3503933d0 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -49,7 +49,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt)
{
struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
struct ucsi *ucsi = dp->con->ucsi;
- struct ucsi_control ctrl;
+ u64 command;
u8 cur = 0;
u16 ver;
int ret;
@@ -65,8 +65,8 @@ static int ucsi_displayport_enter(struct typec_altmode *alt)
return -EOPNOTSUPP;
}
- UCSI_CMD_GET_CURRENT_CAM(ctrl, dp->con->num);
- ret = ucsi_send_command(dp->con->ucsi, &ctrl, &cur, sizeof(cur));
+ command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(dp->con->num);
+ ret = ucsi_send_command(dp->con->ucsi, command, &cur, sizeof(cur));
if (ret < 0) {
ret = ucsi->ops->read(ucsi, UCSI_VERSION, &ver, sizeof(ver));
if (ret)
@@ -107,7 +107,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt)
static int ucsi_displayport_exit(struct typec_altmode *alt)
{
struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
- struct ucsi_control ctrl;
+ u64 command;
int ret = 0;
mutex_lock(&dp->con->lock);
@@ -121,8 +121,8 @@ static int ucsi_displayport_exit(struct typec_altmode *alt)
goto out_unlock;
}
- ctrl.raw_cmd = UCSI_CMD_SET_NEW_CAM(dp->con->num, 0, dp->offset, 0);
- ret = ucsi_send_command(dp->con->ucsi, &ctrl, NULL, 0);
+ command = UCSI_CMD_SET_NEW_CAM(dp->con->num, 0, dp->offset, 0);
+ ret = ucsi_send_command(dp->con->ucsi, command, NULL, 0);
if (ret < 0)
goto out_unlock;
@@ -176,14 +176,14 @@ static int ucsi_displayport_status_update(struct ucsi_dp
*dp)
static int ucsi_displayport_configure(struct ucsi_dp *dp)
{
u32 pins = DP_CONF_GET_PIN_ASSIGN(dp->data.conf);
- struct ucsi_control ctrl;
+ u64 command;
if (!dp->override)
return 0;
- ctrl.raw_cmd = UCSI_CMD_SET_NEW_CAM(dp->con->num, 1, dp->offset, pins);
+ command = UCSI_CMD_SET_NEW_CAM(dp->con->num, 1, dp->offset, pins);
- return ucsi_send_command(dp->con->ucsi, &ctrl, NULL, 0);
+ return ucsi_send_command(dp->con->ucsi, command, NULL, 0);
}
static int ucsi_displayport_vdm(struct typec_altmode *alt,
diff --git a/drivers/usb/typec/ucsi/trace.c b/drivers/usb/typec/ucsi/trace.c
index 1dabafb74320..48ad1dc1b1b2 100644
--- a/drivers/usb/typec/ucsi/trace.c
+++ b/drivers/usb/typec/ucsi/trace.c
@@ -33,17 +33,6 @@ const char *ucsi_cmd_str(u64 raw_cmd)
return ucsi_cmd_strs[(cmd >= ARRAY_SIZE(ucsi_cmd_strs)) ? 0 : cmd];
}
-static const char * const ucsi_ack_strs[] = {
- [0] = "",
- [UCSI_ACK_EVENT]= "event",
- [UCSI_ACK_CMD] = "command",
-};
-
-const char *ucsi_ack_str(u8 ack)
-{
- return ucsi_ack_strs[(ack >= ARRAY_SIZE(ucsi_ack_strs)) ? 0 : ack];
-}
-
const char *ucsi_cci_str(u32 cci)
{
if (cci & GENMASK(7, 0)) {
diff --git a/drivers/usb/typec/ucsi/trace.h b/drivers/usb/typec/ucsi/trace.h
index 6e3d510b236e..2262229dae8e 100644
--- a/drivers/usb/typec/ucsi/trace.h
+++ b/drivers/usb/typec/ucsi/trace.h
@@ -10,54 +10,18 @@
#include
const char *ucsi_cmd_str(u64 raw_cmd);
-const char *ucsi_ack_str(u8 ack);
const char *ucsi_cci_str(u32 cci);
const char *ucsi_recipient_str(u8 recipient);
-DECLARE_EVENT_CLASS(ucsi_log_ack,
- TP_PROTO(u8 ack),
- TP_ARGS(ack),
- TP_STRUCT__entry(
- __field(u8, ack)
- ),
- TP_fast_assign(
- __entry->ack = ack;
- ),
- TP_printk("ACK %s", ucsi_ack_str(__entry->ack))
-);
-
-DEFINE_EVENT(ucsi_log_ack, ucsi_ack,
- TP_PROTO(u8 ack),
- TP_ARGS(ack)
-);
-
-DECLARE_EVENT_CLASS(ucsi_log_control,
- TP_PROTO(struct ucsi_control *ctrl),
- TP_ARGS(ctrl),
- TP_STRUCT__entry(
- __field(u64, ctrl)
- ),
- TP_fast_assign(
- __entry->ctrl = ctrl->raw_cmd;
-
[PATCH 12/14] usb: typec: ucsi: Remove the old API
The drivers now only use the new API, so removing the old one.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/displayport.c | 8 +-
drivers/usb/typec/ucsi/trace.h | 17 --
drivers/usb/typec/ucsi/ucsi.c| 346 +++
drivers/usb/typec/ucsi/ucsi.h| 41
4 files changed, 41 insertions(+), 371 deletions(-)
diff --git a/drivers/usb/typec/ucsi/displayport.c
b/drivers/usb/typec/ucsi/displayport.c
index 6c103697c582..132007cbb96c 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -48,8 +48,10 @@ struct ucsi_dp {
static int ucsi_displayport_enter(struct typec_altmode *alt)
{
struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
+ struct ucsi *ucsi = dp->con->ucsi;
struct ucsi_control ctrl;
u8 cur = 0;
+ u16 ver;
int ret;
mutex_lock(&dp->con->lock);
@@ -66,7 +68,11 @@ static int ucsi_displayport_enter(struct typec_altmode *alt)
UCSI_CMD_GET_CURRENT_CAM(ctrl, dp->con->num);
ret = ucsi_send_command(dp->con->ucsi, &ctrl, &cur, sizeof(cur));
if (ret < 0) {
- if (dp->con->ucsi->ppm->data->version > 0x0100) {
+ ret = ucsi->ops->read(ucsi, UCSI_VERSION, &ver, sizeof(ver));
+ if (ret)
+ return ret;
+
+ if (ver > 0x0100) {
mutex_unlock(&dp->con->lock);
return ret;
}
diff --git a/drivers/usb/typec/ucsi/trace.h b/drivers/usb/typec/ucsi/trace.h
index 783ec9c72055..6e3d510b236e 100644
--- a/drivers/usb/typec/ucsi/trace.h
+++ b/drivers/usb/typec/ucsi/trace.h
@@ -75,23 +75,6 @@ DEFINE_EVENT(ucsi_log_command, ucsi_reset_ppm,
TP_ARGS(ctrl, ret)
);
-DECLARE_EVENT_CLASS(ucsi_log_cci,
- TP_PROTO(u32 cci),
- TP_ARGS(cci),
- TP_STRUCT__entry(
- __field(u32, cci)
- ),
- TP_fast_assign(
- __entry->cci = cci;
- ),
- TP_printk("CCI=%08x %s", __entry->cci, ucsi_cci_str(__entry->cci))
-);
-
-DEFINE_EVENT(ucsi_log_cci, ucsi_notify,
- TP_PROTO(u32 cci),
- TP_ARGS(cci)
-);
-
DECLARE_EVENT_CLASS(ucsi_log_connector_status,
TP_PROTO(int port, struct ucsi_connector_status *status),
TP_ARGS(port, status),
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 2ba890327b9d..ea149a115834 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -36,68 +36,6 @@
*/
#define UCSI_SWAP_TIMEOUT_MS 5000
-static inline int ucsi_sync(struct ucsi *ucsi)
-{
- if (ucsi->ppm && ucsi->ppm->sync)
- return ucsi->ppm->sync(ucsi->ppm);
- return 0;
-}
-
-static int ucsi_command(struct ucsi *ucsi, struct ucsi_control *ctrl)
-{
- int ret;
-
- trace_ucsi_command(ctrl);
-
- set_bit(COMMAND_PENDING, &ucsi->flags);
-
- ret = ucsi->ppm->cmd(ucsi->ppm, ctrl);
- if (ret)
- goto err_clear_flag;
-
- if (!wait_for_completion_timeout(&ucsi->complete,
-msecs_to_jiffies(UCSI_TIMEOUT_MS))) {
- dev_warn(ucsi->dev, "PPM NOT RESPONDING\n");
- ret = -ETIMEDOUT;
- }
-
-err_clear_flag:
- clear_bit(COMMAND_PENDING, &ucsi->flags);
-
- return ret;
-}
-
-static int ucsi_ack(struct ucsi *ucsi, u8 ack)
-{
- struct ucsi_control ctrl;
- int ret;
-
- trace_ucsi_ack(ack);
-
- set_bit(ACK_PENDING, &ucsi->flags);
-
- UCSI_CMD_ACK(ctrl, ack);
- ret = ucsi->ppm->cmd(ucsi->ppm, &ctrl);
- if (ret)
- goto out_clear_bit;
-
- /* Waiting for ACK with ACK CMD, but not with EVENT for now */
- if (ack == UCSI_ACK_EVENT)
- goto out_clear_bit;
-
- if (!wait_for_completion_timeout(&ucsi->complete,
-msecs_to_jiffies(UCSI_TIMEOUT_MS)))
- ret = -ETIMEDOUT;
-
-out_clear_bit:
- clear_bit(ACK_PENDING, &ucsi->flags);
-
- if (ret)
- dev_err(ucsi->dev, "%s: failed\n", __func__);
-
- return ret;
-}
-
static int ucsi_acknowledge_command(struct ucsi *ucsi)
{
u64 ctrl;
@@ -193,115 +131,26 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)
static int ucsi_run_command(struct ucsi *ucsi, struct ucsi_control *ctrl,
void *data, size_t size)
{
- struct ucsi_control _ctrl;
- u8 data_length;
- u16 error;
+ u8 length;
int ret;
- if (ucsi->ops) {
- ret = ucsi_exec_command(ucsi, ctrl->raw_cmd);
- if (ret < 0)
- return ret;
-
- data_length = ret;
+ ret = ucsi_exec_command(ucsi, ctrl->raw_cmd);
+ if (ret < 0)
+ return ret;
- if (data) {
- ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, data,
size);
-
[PATCH 14/14] usb: typec: ucsi: Remove all bit-fields
We can't use bit fields with data that is received or send
to/from the device.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/ucsi/trace.h | 12 ++---
drivers/usb/typec/ucsi/ucsi.c | 52 +++
drivers/usb/typec/ucsi/ucsi.h | 93 +-
3 files changed, 85 insertions(+), 72 deletions(-)
diff --git a/drivers/usb/typec/ucsi/trace.h b/drivers/usb/typec/ucsi/trace.h
index 2262229dae8e..a0d3a934d3d9 100644
--- a/drivers/usb/typec/ucsi/trace.h
+++ b/drivers/usb/typec/ucsi/trace.h
@@ -56,13 +56,13 @@ DECLARE_EVENT_CLASS(ucsi_log_connector_status,
TP_fast_assign(
__entry->port = port - 1;
__entry->change = status->change;
- __entry->opmode = status->pwr_op_mode;
- __entry->connected = status->connected;
- __entry->pwr_dir = status->pwr_dir;
- __entry->partner_flags = status->partner_flags;
- __entry->partner_type = status->partner_type;
+ __entry->opmode = UCSI_CONSTAT_PWR_OPMODE(status->flags);
+ __entry->connected = !!(status->flags & UCSI_CONSTAT_CONNECTED);
+ __entry->pwr_dir = !!(status->flags & UCSI_CONSTAT_PWR_DIR);
+ __entry->partner_flags =
UCSI_CONSTAT_PARTNER_FLAGS(status->flags);
+ __entry->partner_type =
UCSI_CONSTAT_PARTNER_TYPE(status->flags);
__entry->request_data_obj = status->request_data_obj;
- __entry->bc_status = status->bc_status;
+ __entry->bc_status = UCSI_CONSTAT_BC_STATUS(status->pwr_status);
),
TP_printk("port%d status: change=%04x, opmode=%x, connected=%d, "
"sourcing=%d, partner_flags=%x, partner_type=%x, "
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 6fd4ff46d728..accf54d987ad 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -393,7 +393,7 @@ static void ucsi_unregister_altmodes(struct ucsi_connector
*con, u8 recipient)
static void ucsi_pwr_opmode_change(struct ucsi_connector *con)
{
- switch (con->status.pwr_op_mode) {
+ switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) {
case UCSI_CONSTAT_PWR_OPMODE_PD:
typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD);
break;
@@ -411,6 +411,7 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector
*con)
static int ucsi_register_partner(struct ucsi_connector *con)
{
+ u8 pwr_opmode = UCSI_CONSTAT_PWR_OPMODE(con->status.flags);
struct typec_partner_desc desc;
struct typec_partner *partner;
@@ -419,7 +420,7 @@ static int ucsi_register_partner(struct ucsi_connector *con)
memset(&desc, 0, sizeof(desc));
- switch (con->status.partner_type) {
+ switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) {
case UCSI_CONSTAT_PARTNER_TYPE_DEBUG:
desc.accessory = TYPEC_ACCESSORY_DEBUG;
break;
@@ -430,7 +431,7 @@ static int ucsi_register_partner(struct ucsi_connector *con)
break;
}
- desc.usb_pd = con->status.pwr_op_mode == UCSI_CONSTAT_PWR_OPMODE_PD;
+ desc.usb_pd = pwr_opmode == UCSI_CONSTAT_PWR_OPMODE_PD;
partner = typec_register_partner(con->port, &desc);
if (IS_ERR(partner)) {
@@ -462,7 +463,7 @@ static void ucsi_partner_change(struct ucsi_connector *con)
if (!con->partner)
return;
- switch (con->status.partner_type) {
+ switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) {
case UCSI_CONSTAT_PARTNER_TYPE_UFP:
typec_set_data_role(con->port, TYPEC_HOST);
break;
@@ -492,6 +493,7 @@ static void ucsi_handle_connector_change(struct work_struct
*work)
struct ucsi_connector *con = container_of(work, struct ucsi_connector,
work);
struct ucsi *ucsi = con->ucsi;
+ enum typec_role role;
u64 command;
int ret;
@@ -506,11 +508,13 @@ static void ucsi_handle_connector_change(struct
work_struct *work)
goto out_unlock;
}
+ role = !!(con->status.flags & UCSI_CONSTAT_PWR_DIR);
+
if (con->status.change & UCSI_CONSTAT_POWER_OPMODE_CHANGE)
ucsi_pwr_opmode_change(con);
if (con->status.change & UCSI_CONSTAT_POWER_DIR_CHANGE) {
- typec_set_pwr_role(con->port, con->status.pwr_dir);
+ typec_set_pwr_role(con->port, role);
/* Complete pending power role swap */
if (!completion_done(&con->complete))
@@ -518,9 +522,9 @@ static void ucsi_handle_connector_change(struct work_struct
*work)
}
if (con->status.change & UCSI_CONSTAT_CONNECT_CHANGE) {
- typec_set_pwr_role(con->port, con->status.pwr_dir);
+ typec_set_pwr_role(con->port, role);
- switch (con->status.p
RE: [PATCH 00/14] usb: typec: UCSI driver overhaul
Hi Heikki, > -Original Message- > From: Heikki Krogerus > Sent: Thursday, September 26, 2019 3:07 AM > To: Ajay Gupta > Cc: linux-usb@vger.kernel.org > Subject: [PATCH 00/14] usb: typec: UCSI driver overhaul > > Hi Ajay, > > Here's the pretty much complete rewrite of the I/O handling that I was > talking about. The first seven patches are not actually related to > this stuff, but I'm including them here because the rest of the series > is made on top of them. I'm including also that fix patch I send you > earlier. > > After this it should be easier to handle quirks. My idea how to handle > the multi-instance connector alt modes is that we "emulate" the PPM in > ucsi_ccg.c in order to handle them, so ucsi.c is not touched at all. > > We can now get the connector alternate modes that the actual > controller supplies during probe - before registering the ucsi > interface - and squash all alt modes with the same SVID into one that > we supply to the ucsi.c when ever it sends GET_ALTERNATE_MODES > command. Also other alt mode commands like SET_NEW_CAM can have > special processing in ucsi_ccg.c and ucsi_ccg.c alone. There should > not be any problem with that anymore. I took the changes and loaded on my GPU system and do not see altmode devices under /sys/bus/typec/devices/*. Its empty. Below error is seen "ucsi_ccg 4-0008: con1: failed to register alternate modes" ucsi_run_command() is returning -16. I will review the ccg changes and try to debug above issue. Thanks > nvpublic > > thanks, > > Heikki Krogerus (14): > usb: typec: Copy everything from struct typec_capability during > registration > usb: typec: Introduce typec_get_drvdata() > usb: typec: Separate the operations vector > usb: typec: tcpm: Start using struct typec_operations > usb: typec: tps6598x: Start using struct typec_operations > usb: typec: ucsi: Start using struct typec_operations > usb: typec: Remove the callback members from struct typec_capability > usb: typec: ucsi: ccg: Remove run_isr flag > usb: typec: ucsi: Simplified interface registration and I/O API > usb: typec: ucsi: acpi: Move to the new API > usb: typec: ucsi: ccg: Move to the new API > usb: typec: ucsi: Remove the old API > usb: typec: ucsi: Remove struct ucsi_control > usb: typec: ucsi: Remove all bit-fields > > drivers/usb/typec/class.c| 125 +++--- > drivers/usb/typec/tcpm/tcpm.c| 47 +-- > drivers/usb/typec/tps6598x.c | 49 +-- > drivers/usb/typec/ucsi/displayport.c | 26 +- > drivers/usb/typec/ucsi/trace.c | 11 - > drivers/usb/typec/ucsi/trace.h | 79 +--- > drivers/usb/typec/ucsi/ucsi.c| 592 ++- > drivers/usb/typec/ucsi/ucsi.h| 410 +++ > drivers/usb/typec/ucsi/ucsi_acpi.c | 96 - > drivers/usb/typec/ucsi/ucsi_ccg.c| 214 -- > include/linux/usb/typec.h| 38 +- > 11 files changed, 785 insertions(+), 902 deletions(-) > > -- > 2.23.0
