Re: [PATCH v3 1/2] usb: dwc2: alloc dma aligned buffer for isoc split in
Dear Doug,
在 2018年05月11日 04:59, Doug Anderson 写道:
Hi,
On Wed, May 9, 2018 at 1:55 AM, wlf wrote:
+ } else if (hsotg->params.host_dma) {
Are you sure this is "else if"? Can't you have descriptor DMA enabled
in the controller and still need to do a normal DMA transfer if you
plug in a hub? Seems like this should be just "if".
Sorry, I don't understand the case "have descriptor DMA enabled in the
controller and still need to do a normal DMA transfer". But maybe it
still
has another problem if just use "if" here, because it will create kmem
caches for Slave mode which actually doesn't need aligned DMA buf.
So right now your code looks like:
if (hsotg->params.dma_desc_enable ||
hsotg->params.dma_desc_fs_enable) {
...
...
} else if (hsotg->params.host_dma) {
...
}
I've never played much with "descriptor DMA" on dwc2 because every
time I enabled it (last I tried was years ago) a whole bunch of
peripherals stopped working and I didn't dig (I just left it off).
Thus, I'm no expert on descriptor DMA. ...that being said, my
understanding is that if you enable descriptor DMA in the controller
then it will enable a smarter DMA mode for some of the transfers.
IIRC Descriptor DMA mode specifically can't handle splits. Is my
memory correct there? Presumably that means that when you enable
descriptor DMA in the controller the driver will automatically fall
back to normal Address DMA mode if things get too complicated. When
it falls back to Address DMA mode, presumably dwc2_hcd_init() won't
get called again. That means that any data structures that are needed
for Address DMA need to be allocated in dwc2_hcd_init() even if
descriptor DMA is enabled because we might need to fall back to
Address DMA.
Thus, I'm suggesting changing the code to:
if (hsotg->params.dma_desc_enable ||
hsotg->params.dma_desc_fs_enable) {
...
...
}
if (hsotg->params.host_dma) {
...
}
...as I said, I'm no expert and I could just be confused. If
something I say seems wrong please correct me.
Ah, I got it. Thanks for your detailed explanation. Although I don't know if
it's possible that dwc2 driver automatically fall back to normal Address DMA
mode when desc DMA work abnormally with split, I agree with your suggestion.
I'll fix it in V4 patch.
Hmm, I guess you're right. I did a quick search and I can't find any
evidence of a fallback like this. Maybe I dreamed that. I found some
old comment in the commit history that said:
I think you're right. I find that it's possible to fall back to Address
DMA mode if desc DMA initialization failure. The error case is：in
dwc2_hcd_init(), if it fails to create dwc2 generic desc cache or dwc2
hs isoc desc cache, then it will disable desc DMA and fall back to
Address DMA mode.
/*
* Disable descriptor dma mode by default as the HW can support
* it, but does not support it for SPLIT transactions.
* Disable it for FS devices as well.
*/
...and it looks there's currently nobody using descriptor DMA anyway
(assuming I read the code correctly). It does seem like people were
ever going to turn it on then it would have to be dynamic as I was
dreaming it was...
Yes, the dwc2 desc DMA mode can't support split transaction. So few
people use desc DMA mode for OTG Host mode. BTW, I find that the latest
code enable desc DMA mode by default for the OTG peripheral mode if the
dwc2 hardware support desc DMA.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] usb: dwc2: alloc dma aligned buffer for isoc split in
Dear Doug,
在 2018年05月11日 05:01, Doug Anderson 写道:
Hi,
On Wed, May 9, 2018 at 3:11 AM, William Wu wrote:
The commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in
a more supported way") rips out a lot of code to simply the
allocation of aligned DMA. However, it also introduces a new
issue when use isoc split in transfer.
In my test case, I connect the dwc2 controller with an usb hs
Hub (GL852G-12), and plug an usb fs audio device (Plantronics
headset) into the downstream port of Hub. Then use the usb mic
to record, we can find noise when playback.
It's because that the usb Hub uses an MDATA for the first
transaction and a DATA0 for the second transaction for the isoc
split in transaction. An typical isoc split in transaction sequence
like this:
- SSPLIT IN transaction
- CSPLIT IN transaction
- MDATA packet
- CSPLIT IN transaction
- DATA0 packet
The DMA address of MDATA (urb->dma) is always DWORD-aligned, but
the DMA address of DATA0 (urb->dma + qtd->isoc_split_offset) may
not be DWORD-aligned, it depends on the qtd->isoc_split_offset (the
length of MDATA). In my test case, the length of MDATA is usually
unaligned, this cause DATA0 packet transmission error.
This patch use kmem_cache to allocate aligned DMA buf for isoc
split in transaction. Note that according to usb 2.0 spec, the
maximum data payload size is 1023 bytes for each fs isoc ep,
and the maximum allowable interrupt data payload size is 64 bytes
or less for fs interrupt ep. So we set the size of object to be
1024 bytes in the kmem cache.
Signed-off-by: William Wu
---
Changes in v4:
- get rid of "qh->dw_align_buf_size"
- allocate unaligned_cache for Address DMA mode and Desc DMA mode
- freeing order opposite of allocation
You only did half of this. You changed the order under "error4" but
forgot to make dwc2_hcd_remove() match.
Ah, sorry, I'm careless.
- do dma_map_single() with the size of DWC2_KMEM_UNALIGNED_BUF_SIZE
Changes in v3:
- Modify the commit message
- Use Kmem_cache to allocate aligned DMA buf
- Fix coding style
Changes in v2:
- None
drivers/usb/dwc2/core.h | 3 ++
drivers/usb/dwc2/hcd.c | 87 ++--
drivers/usb/dwc2/hcd.h | 8
drivers/usb/dwc2/hcd_intr.c | 8
drivers/usb/dwc2/hcd_queue.c | 3 ++
5 files changed, 105 insertions(+), 4 deletions(-)
My only remaining comment is a really nitty detail. Unless someone
else feels strongly about it, I'm not sure it's worth spinning the
patch just for that nit unless someone else has review comments.
I believe I've looked at this enough to say:
Reviewed-by: Douglas Anderson
Thank you very much for your review. I will submit V5 patches to fix the
order of memory free in dwc2_hcd_remove()， and also add Review-by.
--
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 v5 0/2] usb: dwc2: fix isoc split in transfer issue
This patch fix dma unaligned problem and data lost problem for isoc split in transfer. Test on rk3288 platform, use an usb hs Hub (GL852G-12) and an usb fs audio device (Plantronics headset) to capture and playback. William Wu (2): usb: dwc2: alloc dma aligned buffer for isoc split in usb: dwc2: fix isoc split in transfer with no data drivers/usb/dwc2/core.h | 3 ++ drivers/usb/dwc2/hcd.c | 89 +--- drivers/usb/dwc2/hcd.h | 8 drivers/usb/dwc2/hcd_intr.c | 11 +- drivers/usb/dwc2/hcd_queue.c | 3 ++ 5 files changed, 107 insertions(+), 7 deletions(-) -- 2.0.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/2] usb: dwc2: fix isoc split in transfer with no data
If isoc split in transfer with no data (the length of DATA0
packet is zero), we can't simply return immediately. Because
the DATA0 can be the first transaction or the second transaction
for the isoc split in transaction. If the DATA0 packet with no
data is in the first transaction, we can return immediately.
But if the DATA0 packet with no data is in the second transaction
of isoc split in transaction sequence, we need to increase the
qtd->isoc_frame_index and giveback urb to device driver if needed,
otherwise, the MDATA packet will be lost.
A typical test case is that connect the dwc2 controller with an
usb hs Hub (GL852G-12), and plug an usb fs audio device (Plantronics
headset) into the downstream port of Hub. Then use the usb mic
to record, we can find noise when playback.
In the case, the isoc split in transaction sequence like this:
- SSPLIT IN transaction
- CSPLIT IN transaction
- MDATA packet (176 bytes)
- CSPLIT IN transaction
- DATA0 packet (0 byte)
This patch use both the length of DATA0 and qtd->isoc_split_offset
to check if the DATA0 is in the second transaction.
Signed-off-by: William Wu
---
Changes in v5:
- None
Changes in v4:
- None
Changes in v3:
- Remove "qtd->isoc_split_offset = 0" in the if test
Changes in v2:
- Modify the commit message
drivers/usb/dwc2/hcd_intr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index ba6229e..9751785 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -930,9 +930,8 @@ static int dwc2_xfercomp_isoc_split_in(struct dwc2_hsotg
*hsotg,
frame_desc = &qtd->urb->iso_descs[qtd->isoc_frame_index];
len = dwc2_get_actual_xfer_length(hsotg, chan, chnum, qtd,
DWC2_HC_XFER_COMPLETE, NULL);
- if (!len) {
+ if (!len && !qtd->isoc_split_offset) {
qtd->complete_split = 0;
- qtd->isoc_split_offset = 0;
return 0;
}
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] usb: dwc2: alloc dma aligned buffer for isoc split in
The commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in
a more supported way") rips out a lot of code to simply the
allocation of aligned DMA. However, it also introduces a new
issue when use isoc split in transfer.
In my test case, I connect the dwc2 controller with an usb hs
Hub (GL852G-12), and plug an usb fs audio device (Plantronics
headset) into the downstream port of Hub. Then use the usb mic
to record, we can find noise when playback.
It's because that the usb Hub uses an MDATA for the first
transaction and a DATA0 for the second transaction for the isoc
split in transaction. An typical isoc split in transaction sequence
like this:
- SSPLIT IN transaction
- CSPLIT IN transaction
- MDATA packet
- CSPLIT IN transaction
- DATA0 packet
The DMA address of MDATA (urb->dma) is always DWORD-aligned, but
the DMA address of DATA0 (urb->dma + qtd->isoc_split_offset) may
not be DWORD-aligned, it depends on the qtd->isoc_split_offset (the
length of MDATA). In my test case, the length of MDATA is usually
unaligned, this cause DATA0 packet transmission error.
This patch use kmem_cache to allocate aligned DMA buf for isoc
split in transaction. Note that according to usb 2.0 spec, the
maximum data payload size is 1023 bytes for each fs isoc ep,
and the maximum allowable interrupt data payload size is 64 bytes
or less for fs interrupt ep. So we set the size of object to be
1024 bytes in the kmem cache.
Signed-off-by: William Wu
Reviewed-by: Douglas Anderson
---
Changes in v5:
- freeing order opposite of allocation in dwc2_hcd_remove()
- Add Reviewed-by
Changes in v4:
- get rid of "qh->dw_align_buf_size"
- allocate unaligned_cache for Address DMA mode and Desc DMA mode
- freeing order opposite of allocation
- do dma_map_single() with the size of DWC2_KMEM_UNALIGNED_BUF_SIZE
Changes in v3:
- Modify the commit message
- Use Kmem_cache to allocate aligned DMA buf
- Fix coding style
Changes in v2:
- None
drivers/usb/dwc2/core.h | 3 ++
drivers/usb/dwc2/hcd.c | 89 +---
drivers/usb/dwc2/hcd.h | 8
drivers/usb/dwc2/hcd_intr.c | 8
drivers/usb/dwc2/hcd_queue.c | 3 ++
5 files changed, 106 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index d83be56..c1983f8 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -915,6 +915,7 @@ struct dwc2_hregs_backup {
* @frame_list_sz: Frame list size
* @desc_gen_cache: Kmem cache for generic descriptors
* @desc_hsisoc_cache: Kmem cache for hs isochronous descriptors
+ * @unaligned_cache:Kmem cache for DMA mode to handle non-aligned buf
*
* These are for peripheral mode:
*
@@ -1059,6 +1060,8 @@ struct dwc2_hsotg {
u32 frame_list_sz;
struct kmem_cache *desc_gen_cache;
struct kmem_cache *desc_hsisoc_cache;
+ struct kmem_cache *unaligned_cache;
+#define DWC2_KMEM_UNALIGNED_BUF_SIZE 1024
#endif /* CONFIG_USB_DWC2_HOST || CONFIG_USB_DWC2_DUAL_ROLE */
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 190f959..4e631ba 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1562,11 +1562,20 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg
*hsotg,
}
if (hsotg->params.host_dma) {
- dwc2_writel((u32)chan->xfer_dma,
- hsotg->regs + HCDMA(chan->hc_num));
+ dma_addr_t dma_addr;
+
+ if (chan->align_buf) {
+ if (dbg_hc(chan))
+ dev_vdbg(hsotg->dev, "align_buf\n");
+ dma_addr = chan->align_buf;
+ } else {
+ dma_addr = chan->xfer_dma;
+ }
+ dwc2_writel((u32)dma_addr, hsotg->regs + HCDMA(chan->hc_num));
+
if (dbg_hc(chan))
dev_vdbg(hsotg->dev, "Wrote %08lx to HCDMA(%d)\n",
-(unsigned long)chan->xfer_dma, chan->hc_num);
+(unsigned long)dma_addr, chan->hc_num);
}
/* Start the split */
@@ -2620,6 +2629,35 @@ static void dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
}
}
+static int dwc2_alloc_split_dma_aligned_buf(struct dwc2_hsotg *hsotg,
+ struct dwc2_qh *qh,
+ struct dwc2_host_chan *chan)
+{
+ if (!hsotg->unaligned_cache ||
+ chan->max_packet > DWC2_KMEM_UNALIGNED_BUF_SIZE)
+ return -ENOMEM;
+
+ if (!qh->dw_align_buf) {
+ qh->dw_align_buf = kmem_cache_alloc(hsotg->unaligned_cache,
+ GFP_ATOMIC | GFP_DMA);
+ if (!qh->dw_align_buf)
+ return -ENOMEM;
+ }
+
+ qh->dw_align_buf_dma = dma_map_single(hsotg->dev, qh->dw_align_buf,
+ DWC2_KMEM_UNA
[PATCH v2 3/3] usb: gadget: udc: atmel: Fix indenting
Fix the fallout of the conversion to GPIO descriptors in 3df034081021.
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index b9623e228609..2f586f2bda7e 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2277,15 +2277,15 @@ static int usba_udc_probe(struct platform_device *pdev)
if (udc->vbus_pin) {
irq_set_status_flags(gpiod_to_irq(udc->vbus_pin), IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(&pdev->dev,
- gpiod_to_irq(udc->vbus_pin), NULL,
- usba_vbus_irq_thread,
USBA_VBUS_IRQFLAGS,
- "atmel_usba_udc", udc);
- if (ret) {
- udc->vbus_pin = NULL;
- dev_warn(&udc->pdev->dev,
-"failed to request vbus irq; "
-"assuming always on\n");
- }
+ gpiod_to_irq(udc->vbus_pin), NULL,
+ usba_vbus_irq_thread, USBA_VBUS_IRQFLAGS,
+ "atmel_usba_udc", udc);
+ if (ret) {
+ udc->vbus_pin = NULL;
+ dev_warn(&udc->pdev->dev,
+"failed to request vbus irq; "
+"assuming always on\n");
+ }
}
ret = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
--
2.14.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 v2 1/3] usb: gadget: udc: atmel: GPIO inversion is handled by gpiod
When converting to GPIO descriptors, gpiod_get_value automatically
handles the line inversion flags from the device tree.
Do not invert the line twice.
Fixes: 3df034081021 ("usb: gadget: udc: atmel: convert to use GPIO descriptors")
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 3 +--
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 27c16399c7e8..0fe3d0feb8f7 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -417,7 +417,7 @@ static inline void usba_int_enb_set(struct usba_udc *udc,
u32 val)
static int vbus_is_present(struct usba_udc *udc)
{
if (udc->vbus_pin)
- return gpiod_get_value(udc->vbus_pin) ^ udc->vbus_pin_inverted;
+ return gpiod_get_value(udc->vbus_pin);
/* No Vbus detection: Assume always present */
return 1;
@@ -2076,7 +2076,6 @@ static struct usba_ep * atmel_udc_of_init(struct
platform_device *pdev,
udc->vbus_pin = devm_gpiod_get_optional(&pdev->dev, "atmel,vbus",
GPIOD_IN);
- udc->vbus_pin_inverted = gpiod_is_active_low(udc->vbus_pin);
if (fifo_mode == 0) {
pp = NULL;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 969ce8f3c3e2..d7eb7cf4fd5c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -326,7 +326,6 @@ struct usba_udc {
const struct usba_udc_errata *errata;
int irq;
struct gpio_desc *vbus_pin;
- int vbus_pin_inverted;
int num_ep;
int configured_ep;
struct usba_fifo_cfg *fifo_cfg;
--
2.14.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 v2 2/3] usb: gadget: udc: atmel: Remove obsolete include
The include defines the private platform_data structure used with AVR
platforms. It has no user since 7c55984e191f. Remove it.
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 1 -
include/linux/usb/atmel_usba_udc.h | 24
2 files changed, 25 deletions(-)
delete mode 100644 include/linux/usb/atmel_usba_udc.h
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 0fe3d0feb8f7..b9623e228609 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/include/linux/usb/atmel_usba_udc.h
b/include/linux/usb/atmel_usba_udc.h
deleted file mode 100644
index 9bb00df3b53f..
--- a/include/linux/usb/atmel_usba_udc.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Platform data definitions for Atmel USBA gadget driver.
- */
-#ifndef __LINUX_USB_USBA_H
-#define __LINUX_USB_USBA_H
-
-struct usba_ep_data {
- char*name;
- int index;
- int fifo_size;
- int nr_banks;
- int can_dma;
- int can_isoc;
-};
-
-struct usba_platform_data {
- int vbus_pin;
- int vbus_pin_inverted;
- int num_ep;
- struct usba_ep_data ep[0];
-};
-
-#endif /* __LINUX_USB_USBA_H */
--
2.14.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 v2 0/3] Fix an Atmel USBA UDC issue introducted in 4.17-rc1
The use of GPIO descriptors takes care of inversion flags declared in the device tree. The conversion of the Atmel USBA UDC driver introduced in 4.17-rc1 missed it, and as a result the inversion will not work. In addition, cleanup the code to remove an include left behind after the suppression of the support of platform_data to declare USBA controllers. These changes have not been tested on any hardware, as I do not have a board that needs to use inverted GPIOs. -- Changes in v2: - Use the correct format for the "Fixes:" tag - Collect "Acked-by:" tags Romain Izard (3): usb: gadget: udc: atmel: GPIO inversion is handled by gpiod usb: gadget: udc: atmel: Remove obsolete include usb: gadget: udc: atmel: Fix indenting drivers/usb/gadget/udc/atmel_usba_udc.c | 22 ++ drivers/usb/gadget/udc/atmel_usba_udc.h | 1 - include/linux/usb/atmel_usba_udc.h | 24 3 files changed, 10 insertions(+), 37 deletions(-) delete mode 100644 include/linux/usb/atmel_usba_udc.h -- 2.14.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
dwc2 device testing with Service Interval plus LPM
Hi all, I want to test a new feature of HSOTG core: Service Interval + LPM support in device mode. What I need to use from host side to be able generate appropriate ISOC IN traffic? Thanks, Grigor. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dwc2 device testing with Service Interval plus LPM
Hi, Grigor Tovmasyan writes: > Hi all, > > I want to test a new feature of HSOTG core: Service Interval + LPM support in > device mode. > What I need to use from host side to be able generate appropriate ISOC IN > traffic? Audio class, camera class, g_zero isoc tests... -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dwc2 device testing with Service Interval plus LPM
On 5/11/2018 2:42 PM, Felipe Balbi wrote: > > Hi, > > Grigor Tovmasyan writes: > >> Hi all, >> >> I want to test a new feature of HSOTG core: Service Interval + LPM support >> in device mode. >> What I need to use from host side to be able generate appropriate ISOC IN >> traffic? > > Audio class, camera class, g_zero isoc tests... > Currently I couldn't find host side driver with Service Interval support. Did xHCI driver support Service Interval? if yes how can I enable it? -- 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: usbcore: NULL pointer dereference after detaching USB disk with linux 4.17
Hi
On 10.05.2018 14:49, Jordan Glover wrote:
Hello,
Detaching plugged external usb disk with: "udisksctl power-off --block-device
" causes NULL pointer dereference and kernel hang. Tested with 4.17-rc4 on
Manjaro Linux config and my own custom config with two different usb disks. It doesn't happen
with 4.16.x. Below are logs registered with my own kernel config:
I'm able to reproduce this.
udisksd[1375]: Successfully sent SCSI command SYNCHRONIZE CACHE to /dev/sda
udisksd[1375]: Successfully sent SCSI command START STOP UNIT to /dev/sda
kernel: sd 0:0:0:0: [sda] Synchronizing SCSI cache
kernel: sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:
hostbyte=DID_ERROR driverbyte=DRIVER_OK
upowerd[1387]: unhandled action 'unbind' on
/sys/devices/pci:00/:00:14.0/usb2/2-3/2-3:1.0
laptop udisksd[1375]: Powered off /dev/sda - successfully wrote to sysfs path
/sys/devices/pci:00/:00:14.0/usb2/2-3/remove
kernel: usb 2-3: USB disconnect, device number 2
kernel: BUG: unable to handle kernel NULL pointer dereference at
001c
kernel: RIP: 0010:xhci_hub_control+0x1ee5/0x1ff0 [xhci_hcd]
looks like xhci issue, triggered by speed = xhci->devs[i]->udev->speed in
xhci_find_slot_id_by_port()
xhci->devs[i]->udev seems to be NULL, probably because of commit 44a182b9d177
("xhci: Fix use-after-free in xhci_free_virt_device")
That patch itself fixes another regression, I'll see igf there is a better
solution
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: usbcore: NULL pointer dereference after detaching USB disk with linux 4.17
On May 11, 2018 1:46 PM, Mathias Nyman wrote:
> Hi
>
> On 10.05.2018 14:49, Jordan Glover wrote:
>
> > Hello,
> >
> > Detaching plugged external usb disk with: "udisksctl power-off
> > --block-device " causes NULL pointer dereference and kernel hang.
> > Tested with 4.17-rc4 on Manjaro Linux config and my own custom config with
> > two different usb disks. It doesn't happen with 4.16.x. Below are logs
> > registered with my own kernel config:
>
> I'm able to reproduce this.
>
> > udisksd[1375]: Successfully sent SCSI command SYNCHRONIZE CACHE to /dev/sda
> >
> > udisksd[1375]: Successfully sent SCSI command START STOP UNIT to /dev/sda
> >
> > kernel: sd 0:0:0:0: [sda] Synchronizing SCSI cache
> >
> > kernel: sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:
> > hostbyte=DID_ERROR driverbyte=DRIVER_OK
> >
> > upowerd[1387]: unhandled action 'unbind' on
> > /sys/devices/pci:00/:00:14.0/usb2/2-3/2-3:1.0
> >
> > laptop udisksd[1375]: Powered off /dev/sda - successfully wrote to sysfs
> > path /sys/devices/pci:00/:00:14.0/usb2/2-3/remove
> >
> > kernel: usb 2-3: USB disconnect, device number 2
> >
> > kernel: BUG: unable to handle kernel NULL pointer dereference at
> > 001c
>
> > kernel: RIP: 0010:xhci_hub_control+0x1ee5/0x1ff0 [xhci_hcd]
>
> looks like xhci issue, triggered by speed = xhci->devs[i]->udev->speed in
>
> xhci_find_slot_id_by_port()
>
> xhci->devs[i]->udev seems to be NULL, probably because of commit 44a182b9d177
>
> ("xhci: Fix use-after-free in xhci_free_virt_device")
>
> That patch itself fixes another regression, I'll see igf there is a better
> solution
>
> Thanks
>
> -Mathias
Uh, it's a relief. I was afraid being on my own with this. Looking forward for
fix. Thank you.
Jordan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v3 5/5] usb: typec: tcpm: Support for Alternate Modes
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.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/tcpm.c | 156 +++
include/linux/usb/tcpm.h | 9 ---
2 files changed, 127 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index c13b986f9d7b..a08c87c21c33 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -168,13 +168,14 @@ enum pd_msg_request {
/* Alternate mode support */
#define SVID_DISCOVERY_MAX 16
+#define ALTMODE_DISCOVERY_MAX (SVID_DISCOVERY_MAX * MODE_DISCOVERY_MAX)
struct pd_mode_data {
int svid_index; /* current SVID index */
int nsvids;
u16 svids[SVID_DISCOVERY_MAX];
int altmodes; /* number of alternate modes*/
- struct typec_altmode_desc altmode_desc[SVID_DISCOVERY_MAX];
+ struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
};
struct pd_pps_data {
@@ -309,8 +310,8 @@ struct tcpm_port {
/* Alternate mode data */
struct pd_mode_data mode_data;
- struct typec_altmode *partner_altmode[SVID_DISCOVERY_MAX * 6];
- struct typec_altmode *port_altmode[SVID_DISCOVERY_MAX * 6];
+ struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
+ struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
/* Deadline in jiffies to exit src_try_wait state */
unsigned long max_wait;
@@ -644,14 +645,14 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
}
EXPORT_SYMBOL_GPL(tcpm_pd_transmit_complete);
-static int tcpm_mux_set(struct tcpm_port *port, enum tcpc_mux_mode mode,
+static int tcpm_mux_set(struct tcpm_port *port, int state,
enum usb_role usb_role,
enum typec_orientation orientation)
{
int ret;
- tcpm_log(port, "Requesting mux mode %d, usb-role %d, orientation %d",
-mode, usb_role, orientation);
+ tcpm_log(port, "Requesting mux state %d, usb-role %d, orientation %d",
+state, usb_role, orientation);
ret = typec_set_orientation(port->typec_port, orientation);
if (ret)
@@ -663,7 +664,7 @@ static int tcpm_mux_set(struct tcpm_port *port, enum
tcpc_mux_mode mode,
return ret;
}
- return typec_set_mode(port->typec_port, mode);
+ return typec_set_mode(port->typec_port, state);
}
static int tcpm_set_polarity(struct tcpm_port *port,
@@ -790,7 +791,7 @@ static int tcpm_set_roles(struct tcpm_port *port, bool
attached,
else
usb_role = USB_ROLE_DEVICE;
- ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role, orientation);
+ ret = tcpm_mux_set(port, TYPEC_STATE_USB, usb_role, orientation);
if (ret < 0)
return ret;
@@ -1017,36 +1018,57 @@ static void svdm_consume_modes(struct tcpm_port *port,
const __le32 *payload,
pmdata->altmodes, paltmode->svid,
paltmode->mode, paltmode->vdo);
- port->partner_altmode[pmdata->altmodes] =
- typec_partner_register_altmode(port->partner, paltmode);
- if (!port->partner_altmode[pmdata->altmodes]) {
- tcpm_log(port,
-"Failed to register modes for SVID 0x%04x",
-paltmode->svid);
- return;
- }
pmdata->altmodes++;
}
}
+static void tcpm_register_partner_altmodes(struct tcpm_port *port)
+{
+ struct pd_mode_data *modep = &port->mode_data;
+ struct typec_altmode *altmode;
+ int i;
+
+ for (i = 0; i < modep->altmodes; i++) {
+ altmode = typec_partner_register_altmode(port->partner,
+ &modep->altmode_desc[i]);
+ if (!altmode)
+ tcpm_log(port, "Failed to register partner SVID 0x%04x",
+modep->altmode_desc[i].svid);
+ port->partner_altmode[i] = altmode;
+ }
+}
+
#define supports_modal(port)
PD_IDH_MODAL_SUPP((port)->partner_ident.id_header)
static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
u32 *response)
{
- u32 p0 = le32_to_cpu(payload[0]);
- int cmd_type = PD_VDO_CMDT(p0);
- int cmd = PD_VDO_CMD(p0);
+ struct typec_altmode *adev;
+ struct typec_altmode *pdev;
struct pd_mode_data *modep;
+ u32 p[PD_MAX_PAYLOAD];
int rlen = 0;
- u16 svid;
+ int cmd_type;
+ int cmd;
int i;
+ for (i = 0; i < cnt; i++)
+ p[i] = le32_to_cpu(payload[i]);
+
+ cmd_type = PD_VDO_CMDT(p[0]);
+ cmd = PD_VDO_CM
[RFC PATCH v3 4/5] usb: typec: pi3usb30532: Start using generic state values
Instead of the tcpm specific mux states, using the generic
USB type-c connector state values.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/mux/pi3usb30532.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/mux/pi3usb30532.c
b/drivers/usb/typec/mux/pi3usb30532.c
index b0e88db60ecf..09b711dfe5c3 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -83,13 +83,15 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int
state)
new_conf = pi->conf;
switch (state) {
- case TYPEC_MUX_NONE:
+ case TYPEC_STATE_SAFE:
new_conf = PI3USB30532_CONF_OPEN;
break;
- case TYPEC_MUX_USB:
+ case TYPEC_STATE_USB:
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
- PI3USB30532_CONF_USB3;
+ PI3USB30532_CONF_USB3;
break;
+ /* To be replaced with actual DisplayPort connector states */
+ /*
case TYPEC_MUX_DP:
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
PI3USB30532_CONF_4LANE_DP;
@@ -98,6 +100,9 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int
state)
new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
PI3USB30532_CONF_USB3_AND_2LANE_DP;
break;
+ */
+ default:
+ break;
}
ret = pi3usb30532_set_conf(pi, new_conf);
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v3 1/5] usb: typec: mux: Get the mux identifier from function parameter
In order for the muxes to be usable with alternate modes,
the alternate mode devices will need also to be able to get
a handle to the muxes on top of the port devices. To make
that possible, the muxes need to be possible to request with
an identifier.
This will change the API so that the mux identifier is given
as a function parameter to typec_mux_get(), and the hard-coded
"typec-mux" is replaced with that value.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 2 +-
drivers/usb/typec/mux.c | 6 +++---
include/linux/usb/typec_mux.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 53df10df2f9d..3753950a798d 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1345,7 +1345,7 @@ struct typec_port *typec_register_port(struct device
*parent,
goto err_switch;
}
- port->mux = typec_mux_get(cap->fwnode ? &port->dev : parent);
+ port->mux = typec_mux_get(parent, "typec-mux");
if (IS_ERR(port->mux)) {
ret = PTR_ERR(port->mux);
goto err_mux;
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index f89093bd7185..7446d6d1e424 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -123,19 +123,19 @@ static void *typec_mux_match(struct device_connection
*con, int ep, void *data)
/**
* typec_mux_get - Find USB Type-C Multiplexer
* @dev: The caller device
+ * @name: Mux identifier
*
* Finds a mux linked to the caller. This function is primarily meant for the
* Type-C drivers. Returns a reference to the mux on success, NULL if no
* matching connection was found, or ERR_PTR(-EPROBE_DEFER) when a connection
* was found but the mux has not been enumerated yet.
*/
-struct typec_mux *typec_mux_get(struct device *dev)
+struct typec_mux *typec_mux_get(struct device *dev, const char *name)
{
struct typec_mux *mux;
mutex_lock(&mux_lock);
- mux = device_connection_find_match(dev, "typec-mux", NULL,
- typec_mux_match);
+ mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
if (!IS_ERR_OR_NULL(mux))
get_device(mux->dev);
mutex_unlock(&mux_lock);
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 12c1b057834b..79293f630ee1 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -47,7 +47,7 @@ void typec_switch_put(struct typec_switch *sw);
int typec_switch_register(struct typec_switch *sw);
void typec_switch_unregister(struct typec_switch *sw);
-struct typec_mux *typec_mux_get(struct device *dev);
+struct typec_mux *typec_mux_get(struct device *dev, const char *name);
void typec_mux_put(struct typec_mux *mux);
int typec_mux_register(struct typec_mux *mux);
void typec_mux_unregister(struct typec_mux *mux);
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH v3 3/5] usb: typec: Bus type for alternate modes
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 diff --git a/Documentation/ABI/obsolete/sysfs-class-typec b/Documentation/ABI/obsolete/sysfs-class-typec new file mode 100644 index ..32623514ee87 --- /dev/null +++ b/Documentation/ABI/obsolete/sysfs-class-typec @@ -0,0 +1,48 @@ +These files are deprecated and will be removed. The same files are available +under /sys/bus/typec (see Documentation/ABI/testing/sysfs-bus-typec). + +What: /sys/class/typec///svid +Date: April 2017 +Contact: Heikki Krogerus +Description: + The SVID (Standard or Vendor ID) assigned by USB-IF for this + alternate mode. + +What: /sys/class/typec///mode/ +Date: April 2017 +Contact: Heikki Krogerus +Description: + Every supported mode will have its own directory. The name of + a mode will be "mode" (for example mode1), where + is the actual index to the mode VDO returned by Discover Modes + USB power delivery command. + +What: /sys/class/typec///mode/description +Date: April 2017 +Contact: Heikki Krogerus +Description: + Shows description of the mode. The description is optional for + the drivers, just like with the Billboard Devices. + +What: /sys/class/typec///mode/vdo +Date: April 2017 +Contact: Heikki Krogerus +Description: + Shows the VDO in hexadecimal returned by Discover Modes command + for this mode. + +What: /sys/class/typec///mode/active +Date: April 2017 +Contact: Heikki Krogerus +Description: + Shows if the mode is active or not. The attribute can be used + for entering/exiting the mode with partners and cable plugs, and + with the port alternate modes it can be used for disabling + support for specific alternate modes. Entering/exiting modes is + supported as synchronous operation so write(2) to the attribute + does not return until the enter/exit mode operation has + finished. The attribute is notified when the mode is + entered/exited so poll(2) on the attribute wakes up. + Entering/exiting a mode will also generate uevent KOBJ_CHANGE. + + Valid values: yes, no diff --git a/Documentation/ABI/testing/sysfs-bus-typec b/Documentation/ABI/testing/sysfs-bus-typec new file mode 100644 index ..ead63f97d9a2 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-typec @@ -0,0 +1,51 @@ +What: /sys/bus/typec/devices/.../active +Date: April 2018 +Contact: Heikki Krogerus +Description: + Shows if the mode is active or not. The attribute can be used + for entering/exiting the mode. Entering/exiting modes is + supported as synchronous operation so write(2) to the attribute + does not return until the enter/exit mode operation has + finished. The attribute is notified when the mode is + entered/exited so poll(2) on the attribute wakes up. + Entering/exiting a mode will also generate uevent KOBJ_CHANGE. + + Valid values are boolean. + +What: /sys/bus/typec/devices/.../description +Date: April 2018 +Contact: Heikki Krogerus +Description: + Shows description of the mode. The description is optional for + the drivers, just like with the Billboard Devices. + +What: /sys/bus/typec/devices/.../mode +Date: April 2018 +Contact: Heikki Krogerus +Description: +
[RFC PATCH v3 2/5] usb: typec: Register a device for every mode
Before a device was created for every discovered SVID, but
this will create a device for every discovered mode of every
SVID. The idea is to make it easier to create mode specific
drivers once a bus for the alternate mode is added.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 172 --
drivers/usb/typec/tcpm.c | 45 +-
include/linux/usb/typec.h | 37 ++--
3 files changed, 83 insertions(+), 171 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 3753950a798d..b8a66babd2cd 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -13,31 +13,20 @@
#include
#include
-struct typec_mode {
- int index;
+struct typec_altmode {
+ struct device dev;
+ u16 svid;
+ u8 mode;
+
u32 vdo;
char*desc;
enum typec_port_typeroles;
-
- struct typec_altmode*alt_mode;
-
unsigned intactive:1;
+ struct attribute*attrs[5];
chargroup_name[6];
struct attribute_group group;
- struct attribute*attrs[5];
- struct device_attribute vdo_attr;
- struct device_attribute desc_attr;
- struct device_attribute active_attr;
- struct device_attribute roles_attr;
-};
-
-struct typec_altmode {
- struct device dev;
- u16 svid;
- int n_modes;
- struct typec_mode modes[ALTMODE_MAX_MODES];
- const struct attribute_group*mode_groups[ALTMODE_MAX_MODES];
+ const struct attribute_group*groups[2];
};
struct typec_plug {
@@ -177,23 +166,20 @@ static void typec_report_identity(struct device *dev)
/**
* typec_altmode_update_active - Report Enter/Exit mode
* @alt: Handle to the alternate mode
- * @mode: Mode index
* @active: True when the mode has been entered
*
* If a partner or cable plug executes Enter/Exit Mode command successfully,
the
* drivers use this routine to report the updated state of the mode.
*/
-void typec_altmode_update_active(struct typec_altmode *alt, int mode,
-bool active)
+void typec_altmode_update_active(struct typec_altmode *alt, bool active)
{
- struct typec_mode *m = &alt->modes[mode];
char dir[6];
- if (m->active == active)
+ if (alt->active == active)
return;
- m->active = active;
- snprintf(dir, sizeof(dir), "mode%d", mode);
+ alt->active = active;
+ snprintf(dir, sizeof(dir), "mode%d", alt->mode);
sysfs_notify(&alt->dev.kobj, dir, "active");
kobject_uevent(&alt->dev.kobj, KOBJ_CHANGE);
}
@@ -220,42 +206,36 @@ struct typec_port *typec_altmode2port(struct
typec_altmode *alt)
EXPORT_SYMBOL_GPL(typec_altmode2port);
static ssize_t
-typec_altmode_vdo_show(struct device *dev, struct device_attribute *attr,
- char *buf)
+vdo_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct typec_mode *mode = container_of(attr, struct typec_mode,
- vdo_attr);
+ struct typec_altmode *alt = to_altmode(dev);
- return sprintf(buf, "0x%08x\n", mode->vdo);
+ return sprintf(buf, "0x%08x\n", alt->vdo);
}
+static DEVICE_ATTR_RO(vdo);
static ssize_t
-typec_altmode_desc_show(struct device *dev, struct device_attribute *attr,
- char *buf)
+description_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct typec_mode *mode = container_of(attr, struct typec_mode,
- desc_attr);
+ struct typec_altmode *alt = to_altmode(dev);
- return sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
+ return sprintf(buf, "%s\n", alt->desc ? alt->desc : "");
}
+static DEVICE_ATTR_RO(description);
static ssize_t
-typec_altmode_active_show(struct device *dev, struct device_attribute *attr,
- char *buf)
+active_show(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct typec_mode *mode = container_of(attr, struct typec_mode,
- active_attr);
+ struct typec_altmode *alt = to_altmode(dev);
- return sprintf(buf, "%s\n", mode->active ? "yes" : "no");
+ return sprintf(buf, "%s\n", alt->active ? "yes" : "no");
}
-static ssize_t
-typec_altmode_active_store(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t size)
+static ssize_t active_store(struct device *dev, struct device_attribute *
[RFC PATCH v3 0/5] usb: typec: Support for Alternate Modes
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. v2 commit message: This is second version of my proposal for more complete USB Type-C Alternate Mode support. The original proposal can be read from here: https://www.spinics.net/lists/linux-usb/msg161098.html These patches now depend on series from Hans where he is introducing mux handling support for USB Type-C and USB in general: https://lkml.org/lkml/2018/3/2/340 The major difference compared to v1 is that I'm proposing change to the sysfs ABI we have for the alternate mode devices. The files are not changed, but they are moved to the parent directory from the mode folder. Since the alternate mode devices are not yet used and in practice not supported in mainline, I felt brave enough to propose that. The reason for removing the mode folder is because as in patch 1/3 I now create a device for every mode of every SVID, there will never be more then one mode folder. I.e. the folder serves no purpose. The mode is still kept for now, but it's just deprecated. There are no alternate mode drivers included yet in this version. Original commit message (subject was "usb: typec: alternate mode bus"): The bus allows SVID specific communication with the partners to be handled in separate drivers for each alternate mode. Alternate mode handling happens with two separate logical devices: 1. Partner alternate mode devices which represent the alternate modes on the partner. The driver for them will handle the alternate mode specific communication with the partner using VDMs. 2. Port alternate mode devices which represent connections from the USB Type-C port to devices on the platform. The drivers will be bind to the partner alternate modes. The alternate mode drivers will need to deliver the result of the negotiated pin configurations to the rest of the platform (towards the port alternate mode devices). This series includes API for that, however, not the final implementation yet. The connections to the other devices on the platform the ports have can be described by using the remote endpoint concept [1][2] on ACPI and DT platforms, but I have no solution for the "platform data" case where we have neither DT nor ACPI to describe the connections for us. [1] Documentation/devicetree/bindings/graph.txt [2] Documentation/acpi/dsd/graph.txt Heikki Krogerus (5): usb: typec: mux: Get the mux identifier from function parameter usb: typec: Register a device for every mode usb: typec: Bus type for alternate modes usb: typec: pi3usb30532: Start using generic state values usb: typec: tcpm: Support for Alternate Modes 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| 472 --- drivers/usb/typec/mux.c | 6 +- drivers/usb/typec/mux/pi3usb30532.c | 11 +- drivers/usb/typec/tcpm.c | 179 +-- include/linux/mod_devicetable.h | 15 + include/linux/usb/tcpm.h | 9 - include/linux/usb/typec.h| 51 +- include/linux/usb/typec_altmode.h| 142 ++ include/linux/usb/typec_mux.h| 2 +- scripts/mod/devicetable-offsets.c| 4 + scripts/mod/file2alias.c | 13 + 18 files changed, 1347 insertions(+), 317 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 -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] musb: fix remote wakeup racing with suspend
Hi, On Thu, May 03, 2018 at 04:43:25PM +0200, Daniel Glöckner wrote: > 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 Applied. Thanks. Regards, -Bin. -- 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 v2 1/6] dt-bindings: usb: add Renesas R-Car USB 3.0 role switch
On Mon, May 7, 2018 at 9:43 PM, Yoshihiro Shimoda
wrote:
> Hi Rob,
>
> Sorry for the delayed response. I had a vacation in last week.
>
>> From: Rob Herring, Sent: Saturday, April 28, 2018 5:06 AM
>>
>> On Thu, Apr 26, 2018 at 08:26:41PM +0900, Yoshihiro Shimoda wrote:
>> > This patch adds a new documentation for Renesas R-Car USB 3.0 role
>> > switch that can change the USB 3.0 role to either host or peripheral
>> > by a hardware register that is included in USB3.0 peripheral module.
>> >
>> > Signed-off-by: Yoshihiro Shimoda
>> > ---
>> > .../bindings/usb/renesas,rcar-usb3-role-sw.txt | 47
>> > ++
>> > 1 file changed, 47 insertions(+)
>> > create mode 100644
>> > Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt
>> b/Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt
>> > new file mode 100644
>> > index 000..e074c03
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/usb/renesas,rcar-usb3-role-sw.txt
>> > @@ -0,0 +1,47 @@
>> > +Renesas Electronics R-Car USB 3.0 role switch
>> > +
>> > +A renesas_usb3's node can contain this node.
>> > +
>> > +Required properties:
>> > + - compatible: Must contain "renesas,rcar-usb3-role-switch".
>> > +
>> > +Required nodes:
>> > + - The connection to a usb3.0 host node needs by using OF graph bindings.
>> > + - port@0 = USB 3.0 host port
>> > + - port@1 = USB 3.0 peripheral port
>> > +
>> > +Example of R-Car H3 ES2.0:
>> > + usb3_peri0: usb@ee02 {
>> > + compatible = "renesas,r8a7795-usb3-peri",
>> > +"renesas,rcar-gen3-usb3-peri";
>> > + reg = <0 0xee02 0 0x400>;
>> > + interrupts = ;
>> > + clocks = <&cpg CPG_MOD 328>;
>> > + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
>> > + resets = <&cpg 328>;
>> > +
>> > + usb3-role-sw {
>> > + compatible = "renesas,rcar-usb3-role-switch";
>>
>> You don't define any h/w resources. How is this device accessed?
>
> This device accesses one of registers in the usb3_peri0.
> In the detail, the usb3-role-sw uses 0xee020218 (32-bit register) only.
> (Unfortunately, the hardware design is not good...)
>
> In this case, should I describe the following in the usb3-role-sw node?
>
> reg = <0 0xee020218 0 4>;
>
> Or, shouldn't I add the usb3-role-sw node and a driver for usb3_peri0 should
> take care for it?
IMO, the driver should take care of it.
Rob
--
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: usbcore: NULL pointer dereference after detaching USB disk with linux 4.17
On 11.05.2018 15:11, Jordan Glover wrote:
On May 11, 2018 1:46 PM, Mathias Nyman wrote:
Hi
On 10.05.2018 14:49, Jordan Glover wrote:
Hello,
Detaching plugged external usb disk with: "udisksctl power-off --block-device
" causes NULL pointer dereference and kernel hang. Tested with 4.17-rc4 on
Manjaro Linux config and my own custom config with two different usb disks. It doesn't happen
with 4.16.x. Below are logs registered with my own kernel config:
I'm able to reproduce this.
udisksd[1375]: Successfully sent SCSI command SYNCHRONIZE CACHE to /dev/sda
udisksd[1375]: Successfully sent SCSI command START STOP UNIT to /dev/sda
kernel: sd 0:0:0:0: [sda] Synchronizing SCSI cache
kernel: sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result:
hostbyte=DID_ERROR driverbyte=DRIVER_OK
upowerd[1387]: unhandled action 'unbind' on
/sys/devices/pci:00/:00:14.0/usb2/2-3/2-3:1.0
laptop udisksd[1375]: Powered off /dev/sda - successfully wrote to sysfs path
/sys/devices/pci:00/:00:14.0/usb2/2-3/remove
kernel: usb 2-3: USB disconnect, device number 2
kernel: BUG: unable to handle kernel NULL pointer dereference at
001c
kernel: RIP: 0010:xhci_hub_control+0x1ee5/0x1ff0 [xhci_hcd]
looks like xhci issue, triggered by speed = xhci->devs[i]->udev->speed in
xhci_find_slot_id_by_port()
xhci->devs[i]->udev seems to be NULL, probably because of commit 44a182b9d177
("xhci: Fix use-after-free in xhci_free_virt_device")
That patch itself fixes another regression, I'll see igf there is a better
solution
Thanks
-Mathias
Uh, it's a relief. I was afraid being on my own with this. Looking forward for
fix. Thank you.
Ok, I think I found the reason.
Below details are mostly for myself to remember what's going on.
Some testcode found last.
It's a combination of USB3 devices lacking a real "disabled" link state,
udisksctl power-off using the "remove" sysfs entry causing a logical disconnect,
and xhci free_dev being async, not really freeing before returning.
udisksctl power-off uses the "remove" sysfs entry, setting removed bit,calls
logical_disaconnect and sets USB3 device to U3, and kicks hub thread.
Hub thread calls usb_disconnect, and end by calling xhci_free_dev callback.
xhci_free_dev returns before freeing anything. It queues xhci slot disabling
commands,
which when complete will free the slot (set xhci->devs[i] to NULL).
Before xhci->devs[i] is set to NULL the hub thread notices the port is enabled,
(U3)
and will try to disable it once again, setting it to U3 again.
xhci->devs[i]->udev will be NULL in 4.17-rc4 here, but we only check
xhci->devs[i]
before setting U3, and hence trigger the NULL pointer dereference.
xhci->devs[i]->udev is a better, earlier indicator to check if xhci slot is
about to
go be disabled and freed, and simply checking it as well makes sense.
Some usb core change in how we handle the whole thing might make sense, but to
fix this,
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
--
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
Fwd: Enabling USB (auto)suspend for xHCI controllers incurs random device failures since kernel 4.15
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] Posted to this mailing list as requested by Greg Kroah-Hartman. Apologies for not doing this sooner, but all the archive links on the site were dead for me, so I assumed (wrongly) that it was no longer used. Thanks! Architecture: x86-64 Kernel Version: 4.16.8 Bugzilla link: [2] [1] https://bugzilla.kernel.org/attachment.cgi?id=275907 [2] https://bugzilla.kernel.org/show_bug.cgi?id=199681 -- 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: general protection fault in usb_find_alt_setting
On Sun, Nov 12, 2017 at 10:06 AM, syzbot wrote: > Hello, > > syzkaller hit the following crash on > d9e0e63d9a6f88440eb201e1491fcf730272c706 > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master > compiler: gcc (GCC) 7.1.1 20170620 > .config is attached > Raw console output is attached. > > Unfortunately, I don't have any reproducer for this bug yet. This crash happened 779 times, but first 188d ago, and last 175d ago. Let's consider this fixed by something. #syz invalid > kasan: CONFIG_KASAN_INLINE enabled > kasan: GPF could be caused by NULL-ptr deref or user memory access > general protection fault: [#1] SMP KASAN > Dumping ftrace buffer: >(ftrace buffer empty) > Modules linked in: > CPU: 3 PID: 23503 Comm: syz-executor5 Not tainted 4.14.0-rc8-next-20171110+ > #12 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > task: 88007c5e0580 task.stack: 88006c3b8000 > RIP: 0010:usb_find_alt_setting+0x38/0x310 drivers/usb/core/usb.c:231 > RSP: 0018:88006c3bf610 EFLAGS: 00010247 > RAX: dc00 RBX: RCX: 83bf4473 > RDX: RSI: c90002773000 RDI: 0004 > RBP: 88006c3bf650 R08: ed000d877ee2 R09: ed000d877ee2 > R10: 0003 R11: ed000d877ee1 R12: 88007c668000 > R13: 00fd R14: 07fd R15: > FS: 7f10e9fc8700() GS:88007fd0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 20278000 CR3: 6e8fe000 CR4: 06e0 > DR0: 2008 DR1: 2000 DR2: > DR3: DR6: fffe0ff0 DR7: 0600 > Call Trace: > check_ctrlrecip+0xf3/0x290 drivers/usb/core/devio.c:831 > proc_control+0x13f/0xe30 drivers/usb/core/devio.c:1078 > usbdev_do_ioctl+0x2097/0x3670 drivers/usb/core/devio.c:2396 > SELinux: unrecognized netlink message: protocol=6 nlmsg_type=0 > sclass=netlink_xfrm_socket pig=23496 comm=syz-executor0 > usbdev_ioctl+0x25/0x30 drivers/usb/core/devio.c:2553 > vfs_ioctl fs/ioctl.c:46 [inline] > do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:686 > SYSC_ioctl fs/ioctl.c:701 [inline] > SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692 > entry_SYSCALL_64_fastpath+0x1f/0x96 > RIP: 0033:0x447c99 > RSP: 002b:7f10e9fc7bd8 EFLAGS: 0246 ORIG_RAX: 0010 > RAX: ffda RBX: 7f10e9fc86cc RCX: 00447c99 > RDX: 2003dffa RSI: c0185500 RDI: 0014 > RBP: 0086 R08: R09: > R10: R11: 0246 R12: > R13: 48d8 R14: 006e8978 R15: 7f10e9fc8700 > Code: 89 d5 53 48 89 fb 48 83 ec 18 48 89 7d c8 89 75 d0 e8 2d 3c b0 fd 48 > 8d 7b 04 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 48 > 89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 a1 02 00 > SELinux: unrecognized netlink message: protocol=0 nlmsg_type=0 > sclass=netlink_route_socket pig=23514 comm=syz-executor7 > RIP: usb_find_alt_setting+0x38/0x310 drivers/usb/core/usb.c:231 RSP: > 88006c3bf610 > ---[ end trace 53f2c0803d4e1797 ]--- > Kernel panic - not syncing: Fatal exception > Dumping ftrace buffer: >(ftrace buffer empty) > Kernel Offset: disabled > Rebooting in 86400 seconds.. > > > --- > This bug is generated by a dumb bot. It may contain errors. > See https://goo.gl/tpsmEJ for details. > Direct all questions to syzkal...@googlegroups.com. > Please credit me with: Reported-by: syzbot > > syzbot will keep track of this bug report. > Once a fix for this bug is committed, please reply to this email with: > #syz fix: exact-commit-title > To mark this as a duplicate of another syzbot report, please reply with: > #syz dup: exact-subject-of-another-report > If it's a one-off invalid bug report, please reply with: > #syz invalid > Note: if the crash happens again, it will cause creation of a new bug > report. > Note: all commands must start from beginning of the line in the email body. > > -- > You received this message because you are subscribed to the Google Groups > "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to syzkaller-bugs+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/syzkaller-bugs/94eb2c05b4ba7e98d2055dc57696%40google.com. > For more options, visit https://groups.google.com/d/optout. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/7] usb: typec: Generalize mux mode names
On 2018-05-11 13:14, Heikki Krogerus wrote: On Fri, May 11, 2018 at 11:05:55AM +0200, Mats Karrman wrote: On 2018-05-10 19:49, Heikki Krogerus wrote: On Thu, May 10, 2018 at 10:04:21AM +0200, Mats Karrman wrote: Hi, On 05/09/2018 02:49 PM, Heikki Krogerus wrote: On Tue, May 08, 2018 at 09:10:13PM +0200, Mats Karrman wrote: Hi, On 05/08/2018 04:25 PM, Heikki Krogerus wrote: Hi, On Mon, May 07, 2018 at 11:19:40PM +0200, Mats Karrman wrote: Even so, when the mux driver "set" function is called, it will just get the mode argument but since the mode (TYPEC_STATE_...) is overlapping for different AMs if I understand your proposal correctly, the mux also needs to know what AM is active. Does this imply that the mux set function signature need to change? My plan was actually to propose we get rid of the current mux handling (just leave the orientation switch) in favour of the notifications I'm introducing with the type-c bus for the alternate modes. The current mux handling is definitely not enough, and does not work in every scenario, like also you pointed out. So, the mux need to subscribe to each svid:mode pair it is interested in using typec_altmode_register_notifier() and then use those callbacks to switch the correct signals to the connector. And a driver for an off-the-shelf mux device could have the translation between svid:mode pairs and mux device specific control specified by of/acpi properties. Right? Yes. That is the plan. Would it work for you? I think so. I'll give it a go. When about do you think you'll post the next version of your RFC? Or do you have an updated series available somewhere public? I'll try to put together and post the next version tomorrow. My original plan was actually to use just the notifications with the muxes, but one thing to consider with the notifications is that in practice we have to increment the ref count for the alt mode devices when ever something registers a notifier. To me that does not feel ideal. The dependency should go the other way around in case of the muxes. That is why I liked the separate API and handling for the muxes felt better, as it will do just that. The mux is then a "service" that the port driver can as for, and if it gets a handle to a mux, the mux will have its ref count incremented. So I think fixing the mux API would perhaps be better after all. Thoughts? So, we're back to a mux API similar to the current one, where: - the port driver and the mux driver are connected through "graph" - alt mode driver finds its port mux using the typec class mux api - the mux mode setting function arguments now include both svid and mode I like that. One thought that popped up again is if we, somewhere down the line, will see some super device that support many different alternate modes on the same port and therefore will need to have multiple mux devices? However I think the mux api could be extended (later on) to support some aggregate mux device that manages multiple physical devices. If we simply had always a mux for every alternate mode, that would not be a problem. So the port would have its own mux, and every supported alternate mode also had its own. I think that removes the need to deal with the svid:mode when using the muxes, as they are already tied to a specific alternate modes, right? With a single mux device, for example pi3usb30532, the driver just needs to register a mux for the port and separate mux for DP, but I don't think that's a huge problem. Hmm... As an hypothetical example I have written a driver for another mux from TI and according to its data sheet: """ The HD3SS460 is a generic analog differential passive switch that can work for any high speed interface applications as long as it is biased at a common mode voltage range of 0-2V and has differential signaling with differential amplitude up to 1800mVpp """ What I am thinking is that it e.g. would be possible to use this/a mux with USBSS + 2ch DP + 2ch something else (HDMI?, ThunderBolt?, ???). The problem here is that it is a general mux device so the driver writer does not know what types of muxes to register. I guess it could also be configured using properties but that would be very complicated. Why? All the mux driver needs to get from device properties is the SVID and the mode. Sigh... Again, if the same mux handles signals for more than one alternate mode the driver won't know what alternate mode is intended if it only receives the connector state which use overlapping numbers for different alternate modes. Is there a problem providing both svid and sub-mode in the mux set call? The partner drivers should all know what svid they implement. By sub-mode, what do you mean? The SVID specific connector state value you already get with the mux ->set callback. The mode index number is not very useful (with DP for example it will always be 1). In any case, the mux driver will still need to interpret the SVID specific connector states, so wh
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)
BR // Mats
+ sink and power dual role.
+
Required nodes:
- any data bus to the connector should be modeled using the OF graph bindings
specified in bindings/graph.txt, unless the bus is between parent node and
@@ -73,3 +100,20 @@ ccic: s2mm005@33 {
};
};
};
+
+3. USB-C connector attached to a typec port controller(ptn5110), which has
+power delivery support and enables drp.
+
+typec: ptn5110@50 {
+ ...
+ usb_con: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ power-role = "dual";
+ try-power-role = "sink";
+ source-pdos = ;
+ sink-pdos = ;
+ op-sink-microwatt = <1000>;
+ };
+};
diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
new file mode 100644
index 000..7b7a92f
--- /dev/null
+++ b/include/dt-bindings/usb/pd.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_POWER_DELIVERY_H
+#define __DT_POWER_DELIVERY_H
+
+/* Power delivery Power Data Object definitions */
+#define PDO_TYPE_FIXED 0
+#define PDO_TYPE_BATT 1
+#define PDO_TYPE_VAR 2
+#define PDO_TYPE_APDO 3
+
+#define PDO_TYPE_SHIFT 30
+#define PDO_TYPE_MASK 0x3
+
+#define PDO_TYPE(t)((t) << PDO_TYPE_SHIFT)
+
+#define PDO_VOLT_MASK 0x3ff
+#define PDO_CURR_MASK 0x3ff
+#define PDO_PWR_MASK 0x3ff
+
+#define PDO_FIXED_DUAL_ROLE(1 << 29) /* Power role swap supported */
+#define PDO_FIXED_SUSPEND (1 << 28) /* USB Suspend supported (Source) */
+#define PDO_FIXED_HIGHER_CAP (1 << 28) /* Requires more than vSafe5V (Sink)
*/
+#define PDO_FIXED_EXTPOWER (1 << 27) /* Externally powered */
+#define PDO_FIXED_USB_COMM (1 << 26) /* USB communications capable */
+#define PDO_FIXED_DATA_SWAP(1 << 25) /* Data role swap supported */
+#define PDO_FIXED_VOLT_SHIFT 10 /* 50mV units */
+#define PDO_FIXED_CURR_SHIFT 0 /* 10mA units */
+
+#define PDO_FIXED_VOLT(mv) mv) / 50) & PDO_VOLT_MASK) <<
PDO_FIXED_VOLT_SHIFT)
+#define PDO_FIXED_CURR(ma) ma) / 10) & PDO_CURR_MASK) <<
PDO_FIXED_CURR_SHIFT)
+
+#define PDO_FIXED(mv, ma, flags) \
+ (PDO_TYPE(PDO_TYPE_FIXED) | (flags) | \
+PDO_FIXED_VOLT(mv) | PDO_FIXED_CURR(ma))
+
+#define VSAFE5V 5000 /* mv units */
+
+#define PDO_BATT_MAX_VOLT_SHIFT
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.
BR // Mats
+ { .compatible = "nxp,ptn5110", },
{},
};
MODULE_DEVICE_TABLE(of, tcpci_of_match);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 05/14] usb: typec: add API to get typec basic port power and data config
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...)
+ * @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()?
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 */
--
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: usbcore: NULL pointer dereference after detaching USB disk with linux 4.17
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.
Jordan
--
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
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}
// 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
--
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
Fwd: patch "xhci: Fix use-after-free in xhci_free_virt_device" causes kernel crashes on 4.16.8 on unplug
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.
-
commit f5331826b0b7a5f2db56a9020ddbb8ce16acdfc0
Author: Mathias Nyman
Date: Thu May 3 17:30:07 2018 +0300
xhci: Fix use-after-free in xhci_free_virt_device
commit 44a182b9d17765514fa2b1cc911e4e65134eef93 upstream.
KASAN found a use-after-free in xhci_free_virt_device+0x33b/0x38e
where xhci_free_virt_device() sets slot id to 0 if udev exists:
if (dev->udev && dev->udev->slot_id)
dev->udev->slot_id = 0;
dev->udev will be true even if udev is freed because dev->udev is
not set to NULL.
set dev->udev pointer to NULL in xhci_free_dev()
The original patch went to stable so this fix needs to be applied
there as well.
Fixes: a400efe455f7 ("xhci: zero usb device slot_id member when
disabling and freeing a xhci slot")
Cc:
Reported-by: Guenter Roeck
Reviewed-by: Guenter Roeck
Tested-by: Guenter Roeck
Signed-off-by: Mathias Nyman
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Greg Kroah-Hartman
:04 04 356ecdc13bf252535bd51b8558a8173dae546f19
d28f201228652e87e51ba48f5a0ad4539cca5c29 M drivers
Jacob Saunders.
--
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
