[PATCH -next] usb: musb: cppi41: fix missing unlock on error in cppi41_dma_callback()
From: Wei Yongjun
Add the missing unlock before return from function cppi41_dma_callback()
in the error handling case.
Signed-off-by: Wei Yongjun
---
drivers/usb/musb/musb_cppi41.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index a74259e..6c12c86 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -150,8 +150,10 @@ static void cppi41_dma_callback(void *private_data)
remain_bytes,
direction,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
- if (WARN_ON(!dma_desc))
+ if (WARN_ON(!dma_desc)) {
+ spin_unlock_irqrestore(&musb->lock, flags);
return;
+ }
dma_desc->callback = cppi41_dma_callback;
dma_desc->callback_param = channel;
--
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 v1 02/49] USB: sg: prepare for enabling irq in complete()
On Sun, Aug 18, 2013 at 9:01 AM, Alan Stern wrote:
> On Sun, 18 Aug 2013, Ming Lei wrote:
>
>> Complete() will be run with interrupt enabled, so change to
>> spin_lock_irqsave().
>>
>> Cc: Alan Stern
>> Signed-off-by: Ming Lei
>> ---
>> drivers/usb/core/message.c |5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> index 82927e1..8bba734 100644
>> --- a/drivers/usb/core/message.c
>> +++ b/drivers/usb/core/message.c
>> @@ -266,8 +266,9 @@ static void sg_complete(struct urb *urb)
>> {
>> struct usb_sg_request *io = urb->context;
>> int status = urb->status;
>> + unsigned long flags;
>>
>> - spin_lock(&io->lock);
>> + spin_lock_irqsave(&io->lock, flags);
>>
>> /* In 2.5 we require hcds' endpoint queues not to progress after fault
>>* reports, until the completion callback (this!) returns. That lets
>> @@ -326,7 +327,7 @@ static void sg_complete(struct urb *urb)
>> if (!io->count)
>> complete(&io->complete);
>>
>> - spin_unlock(&io->lock);
>> + spin_unlock_irqrestore(&io->lock, flags);
>> }
>
> As far as I can see, these don't need to disable interrupts. All they
> protect against is the code in usb_sg_wait() and usb_sg_cancel(), which
> both run in process context.
Yes.
> But will lockdep complain if they don't disable interrupts?
Looks lockdep won't complain because the lock can't be held in
another hardirq context.
As I mentioned in 00/50, the patchset is basically a mechanical
change, so one patch can be dropped if anyone reviews and
concludes it isn't needed.
Thanks,
--
Ming Lei
--
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
USB configfs questions and help
Hi, Last week I discovered the new configfs for the USB gadget support in the kernel. I found the following threads on the mailing list: https://lkml.org/lkml/2012/6/21/154 https://lkml.org/lkml/2012/11/26/38 Now there is some discussion about having a userpace library to handle the creation of all the necessary files easily. But I cannot find a trace of it. Does it exist already? If not I will need to write code for it anyway to support it in the USB gadget handler daemon I maintain to support this. It loads/unloads modules, sets up usb gadget modes, can start programs needed etc ... (See usb_moded: https://gitorious.org/meego-middleware/usb_moded ) So if there is no code out there do handle the creation of those configfs structures I do not mind doing the work. Also I have some questions about the documentation. In the section about the gadget creation it states, I quote: "A gadget also needs its serial number, manufacturer and product strings. In order to have a place to store them, a strings subdirectory must be created for each language, e.g.: $ mkdir strings/0x409" What is meant with language here, and why does the example have a hex string as identifier. Are we rather talking about one of the USB identifiers here? The rest seems to be pretty clear so I'll start playing around with it. Thanks, Philippe -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb:gadget Fix comment for pointer to configfs
The documentation for the USB gadget fs is actually in Documentation/usb/gadget_configfs.txt. Signed-off-by: Philippe De Swert --- drivers/usb/gadget/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 8e93683..1c5c7de 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -572,7 +572,7 @@ config USB_CONFIGFS specified simply by creating appropriate directories in configfs. Associating functions with configurations is done by creating appropriate symbolic links. - For more information see Documentation/usb/gadget-configfs.txt. + For more information see Documentation/usb/gadget_configfs.txt. config USB_CONFIGFS_SERIAL boolean "Generic serial bulk in/out" -- 1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: octeon-usb and dwc2 in staging are for the same hw
Hi, On Sat, Aug 17, 2013 at 08:44:18PM +, Paul Zimmerman wrote: > > It doesn't get very far: > > > > External DMA Mode not supported > > dwc2_hcd_init() FAILED, returning -22 > > Hi Greg, all, > > After taking a look at the Octeon driver, it looks like that controller > uses a customized version of the DWC2 core - it has a different DMA > engine than the one provided by the standard hardware. So in fact these > two drivers are actually not "for the same hw". FWIW, I also tried forcing the DWC2 to non-DMA/FIFO mode, but could not get that working either. Do you support any HW running in slave mode? It seems the RX direction fails. I noticed the dwc2_read_packet() is hard-coded to channel 0 which looks odd, but changing that didn't change anything. A. -- 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 v1 26/49] input: cm109: prepare for enabling irq in complete()
Hi Dmitry, On Sun, Aug 18, 2013 at 11:37 AM, Dmitry Torokhov wrote: > Hi Ming, > > On Sun, Aug 18, 2013 at 12:24:51AM +0800, Ming Lei wrote: >> Complete() will be run with interrupt enabled, so change to >> spin_lock_irqsave(). > > I think cm109 needs some love in it's URB handling, but this patch does > not change anything, so: > > Acked-by: Dmitry Torokhov Thank you. > > Or do you want me to pick it up for my tree? IMO, it might be easier to merge these patches via one tree, so Greg, would you like to manage all these patches via your tree? If not, I have to push these patches on each subsystem, then you need to pick it up for your input tree... Thanks, -- Ming Lei -- 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 v1 02/49] USB: sg: prepare for enabling irq in complete()
On Sun, 18 Aug 2013, Ming Lei wrote: > > As far as I can see, these don't need to disable interrupts. All they > > protect against is the code in usb_sg_wait() and usb_sg_cancel(), which > > both run in process context. > > Yes. > > > But will lockdep complain if they don't disable interrupts? > > Looks lockdep won't complain because the lock can't be held in > another hardirq context. Don't be so sure. Suppose you have two mass-storage devices, one connected by EHCI and one connected by UHCI. The one using UHCI _will_ invoke the completion handler in hardirq context, because uhci-hcd doesn't support tasklets. Have you tested this? > As I mentioned in 00/50, the patchset is basically a mechanical > change, so one patch can be dropped if anyone reviews and > concludes it isn't needed. I'm afraid that it might be needed to keep lockdep happy, not to prevent real problems. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB configfs questions and help
On Sun, 18 Aug 2013, Philippe wrote: > Hi, > > Last week I discovered the new configfs for the USB gadget support in > the kernel. > > I found the following threads on the mailing list: > https://lkml.org/lkml/2012/6/21/154 > https://lkml.org/lkml/2012/11/26/38 > > Now there is some discussion about having a userpace library to handle > the creation of all the necessary files easily. But I cannot find a > trace of it. Does it exist already? No mention of it has appeared on the mailing list. > If not I will need to write code for > it anyway to support it in the USB gadget handler daemon I maintain to > support this. It loads/unloads modules, sets up usb gadget modes, can > start programs needed etc ... (See usb_moded: > https://gitorious.org/meego-middleware/usb_moded ) > So if there is no code out there do handle the creation of those > configfs structures I do not mind doing the work. > > Also I have some questions about the documentation. In the section about > the gadget creation it states, I quote: > > "A gadget also needs its serial number, manufacturer and product strings. > In order to have a place to store them, a strings subdirectory must be > created > for each language, e.g.: > > $ mkdir strings/0x409" > > What is meant with language here, and why does the example have a hex > string as identifier. Are we rather talking about one of the USB > identifiers here? The word "language" refers to human languages, like English, French, Mandarin, and so on. The hex number is a language identifier, as specified by the USB Interface Forum: http://www.usb.org/developers/docs/ (see the paragraph near the end about LANGID codes). The strings are not USB identifiers; they are USB descriptors. > The rest seems to be pretty clear so I'll start playing around with it. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: musb: Avoid null pointer dereference in debug logging
Since commit 511f3c53 usb_gadget_remove_driver will pass NULL for the driver argument. Signed-off-by: Maarten ter Huurne --- drivers/usb/musb/musb_gadget.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index ba70923..82e5386 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1935,7 +1935,8 @@ static int musb_gadget_stop(struct usb_gadget *g, stop_activity(musb, driver); otg_set_peripheral(musb->xceiv->otg, NULL); - dev_dbg(musb->controller, "unregistering driver %s\n", driver->function); + dev_dbg(musb->controller, "unregistering driver %s\n", + driver ? driver->function : "(removed)"); musb->is_active = 0; musb->gadget_driver = NULL; -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] Buffer size for ALSA USB PCM audio
Alan Stern wrote: > On Tue, 13 Aug 2013, Clemens Ladisch wrote: >>> The difference is that this version tries always to keep a period's >>> worth of bytes in the USB hardware queue. >> >> Having truncated URBs is possible only when URBs are shorter than one >> period, > > No. URBs are truncated when a full-sized URB would extend at least one > packet beyond the end of a frame. This thought was in the context of avoiding too-short queues. When there are multiple URBs per period, the queue is long enough anyway. >> so I think that a queue length of at least two periods, together >> with a minimum period size, should ensure the isochrounous scheduling >> threshold. > > This depends on how long a period is. In that patch, a period is guaranteed to be no smaller than the IST. >> And while we're at it: the default number of packets per URB was chosen >> before the driver had the ability to estimate the delay from the current >> frame number; it should now be quite safe to increase it. > > I don't understand how this delay estimation is supposed to work, or > what it is meant to accomplish. Can you explain? The "delay" is the difference between the time when a sample is written by the application and when that sample is actually played, and is important for things like A/V synchronization. It depends on 1) the amount of samples already in the ring buffer; 2) any processing done by the driver; and 3) any processing done by the hardware. Most drivers don't do any processing, and most of the hardware has very low delays, so it is common for drivers to pretend 2) and 3) are zero. snd-usb-audio computes 2) from the current number of packets in the queue, with the progress estimated based on the current frame. Without this computation, it was desirable to have short URBs because the delay would jump by a large amount whenever a URB was completed. Regards, Clemens -- 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 v1 26/49] input: cm109: prepare for enabling irq in complete()
On Sun, Aug 18, 2013 at 10:10:15PM +0800, Ming Lei wrote: > Hi Dmitry, > > On Sun, Aug 18, 2013 at 11:37 AM, Dmitry Torokhov > wrote: > > Hi Ming, > > > > On Sun, Aug 18, 2013 at 12:24:51AM +0800, Ming Lei wrote: > >> Complete() will be run with interrupt enabled, so change to > >> spin_lock_irqsave(). > > > > I think cm109 needs some love in it's URB handling, but this patch does > > not change anything, so: > > > > Acked-by: Dmitry Torokhov > > Thank you. > > > > > Or do you want me to pick it up for my tree? > > IMO, it might be easier to merge these patches via one tree, so > > Greg, would you like to manage all these patches via your tree? Yes, I can take them all. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: serial: fix stringify operator in usb-serial-simple
From: Yann Droneaud
usb-serial-simple uses an unknown stringify macro that make
all drivers being named "stringify(vendor)".
This can be a problem when two drivers have the same (wrong) name:
kernel: usbcore: registered new interface driver usb_serial_simple
kernel: usbserial: USB Serial support registered for stringify(vendor)
kernel Error: Driver 'stringify(vendor)' is already registered, aborting...
kernel: usbserial: problem -16 when registering driver stringify(vendor)
kernel: usbserial: USB Serial deregistering driver stringify(vendor)
kernel: usbcore: deregistering interface driver usb_serial_simple
Before the fix:
$ strings drivers/usb/serial/usb-serial-simple.o
usb_serial_simple
stringify(vendor)
After the fix:
$ strings drivers/usb/serial/usb-serial-simple.o
usb_serial_simple
funsoft
flashloader
vivopay
moto_modem
hp4x
suunto
siemens_mpi
This patch makes usb-serial-simple use the correct stringify operator.
Signed-off-by: Yann Droneaud
---
drivers/usb/serial/usb-serial-simple.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/usb-serial-simple.c
b/drivers/usb/serial/usb-serial-simple.c
index 6a06131..677c08c 100644
--- a/drivers/usb/serial/usb-serial-simple.c
+++ b/drivers/usb/serial/usb-serial-simple.c
@@ -29,7 +29,7 @@ static const struct usb_device_id vendor##_id_table[] = {
\
static struct usb_serial_driver vendor##_device = {\
.driver = { \
.owner =THIS_MODULE,\
- .name = "stringify(vendor)",\
+ .name = #vendor,\
}, \
.id_table = vendor##_id_table, \
.num_ports =1, \
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: fix stringify operator in usb-serial-simple
On Sun, Aug 18, 2013 at 09:29:00PM +0200, Yann Droneaud wrote: > From: Yann Droneaud > > usb-serial-simple uses an unknown stringify macro that make > all drivers being named "stringify(vendor)". > > This can be a problem when two drivers have the same (wrong) name: > > kernel: usbcore: registered new interface driver usb_serial_simple > kernel: usbserial: USB Serial support registered for stringify(vendor) > kernel Error: Driver 'stringify(vendor)' is already registered, > aborting... > kernel: usbserial: problem -16 when registering driver stringify(vendor) > kernel: usbserial: USB Serial deregistering driver stringify(vendor) > kernel: usbcore: deregistering interface driver usb_serial_simple > Ugh, sorry about that, I thought there used to be a stringify() macro that used to do this. Nice patch, I'll queue it up. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: octeon-usb and dwc2 in staging are for the same hw
> From: Aaro Koskinen [mailto:aaro.koski...@iki.fi] > Sent: Sunday, August 18, 2013 4:41 AM > > On Sat, Aug 17, 2013 at 08:44:18PM +, Paul Zimmerman wrote: > > > It doesn't get very far: > > > > > > External DMA Mode not supported > > > dwc2_hcd_init() FAILED, returning -22 > > > > Hi Greg, all, > > > > After taking a look at the Octeon driver, it looks like that controller > > uses a customized version of the DWC2 core - it has a different DMA > > engine than the one provided by the standard hardware. So in fact these > > two drivers are actually not "for the same hw". > > FWIW, I also tried forcing the DWC2 to non-DMA/FIFO mode, but could > not get that working either. Do you support any HW running in slave > mode? It seems the RX direction fails. I noticed the dwc2_read_packet() > is hard-coded to channel 0 which looks odd, but changing that didn't > change anything. I have tested FIFO mode on x86 only. It looks like there may be a problem with unaligned RX transfers on archs like ARM that have strict alignment requirements. See this comment in dwc2_read_packet(): * Todo: Account for the case where dest is not dword aligned. Hard-coding to FIFO 0 is correct for RX, since there is only one RX FIFO. -- Paul -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: fix stringify operator in usb-serial-simple
Le 18.08.2013 21:40, Greg Kroah-Hartman a écrit : On Sun, Aug 18, 2013 at 09:29:00PM +0200, Yann Droneaud wrote: Ugh, sorry about that, I thought there used to be a stringify() macro that used to do this. Nice patch, I'll queue it up. That's __stringify() which is defined in but: 1) inside a string (eg "__stringify(vendor)") it's gonna never work; 2) it's not required here, __stringify(vendor) would be needed if vendor was itself a macro and the macro content was to be converted to a string. Regards. -- Yann Droneaud OPTEYA -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] usb: gadget: USB_FUSB300 should depend on HAS_DMA
If NO_DMA=y: drivers/built-in.o: In function `fusb300_set_idma': drivers/usb/gadget/fusb300_udc.c:946: undefined reference to `usb_gadget_map_request' drivers/usb/gadget/fusb300_udc.c:958: undefined reference to `usb_gadget_unmap_request' Signed-off-by: Geert Uytterhoeven --- drivers/usb/gadget/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 8e93683..d2e6f4f 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -188,7 +188,7 @@ config USB_FSL_USB2 config USB_FUSB300 tristate "Faraday FUSB300 USB Peripheral Controller" - depends on !PHYS_ADDR_T_64BIT + depends on !PHYS_ADDR_T_64BIT && HAS_DMA help Faraday usb device controller FUSB300 driver -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] usb: gadget: USB_R8A66597 should depend on HAS_DMA
If NO_DMA=y: drivers/built-in.o: In function `sudmac_free_channel': drivers/usb/gadget/r8a66597-udc.c:676: undefined reference to `usb_gadget_unmap_request' drivers/built-in.o: In function `sudmac_alloc_channel': drivers/usb/gadget/r8a66597-udc.c:666: undefined reference to `usb_gadget_map_request' Signed-off-by: Geert Uytterhoeven --- drivers/usb/gadget/Kconfig |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index d2e6f4f..3c1cadd 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -246,6 +246,7 @@ config USB_PXA25X_SMALL config USB_R8A66597 tristate "Renesas R8A66597 USB Peripheral Controller" + depends on HAS_DMA help R8A66597 is a discrete USB host and peripheral controller chip that supports both full and high speed USB 2.0 data transfers. -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] usb: chipidea: USB_CHIPIDEA should depend on HAS_DMA
If NO_DMA=y: drivers/built-in.o: In function `dma_set_coherent_mask': include/linux/dma-mapping.h:93: undefined reference to `dma_supported' Signed-off-by: Geert Uytterhoeven --- drivers/usb/chipidea/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig index d1bd8ef..dbd5232 100644 --- a/drivers/usb/chipidea/Kconfig +++ b/drivers/usb/chipidea/Kconfig @@ -1,6 +1,6 @@ config USB_CHIPIDEA tristate "ChipIdea Highspeed Dual Role Controller" - depends on USB || USB_GADGET + depends on (USB || USB_GADGET) && HAS_DMA help Say Y here if your system has a dual role high speed USB controller based on ChipIdea silicon IP. Currently, only the -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] usb: gadget: USB_NET2272_DMA should depend on HAS_DMA
If NO_DMA=y: drivers/built-in.o: In function `net2272_done': drivers/usb/gadget/net2272.c:386: undefined reference to `usb_gadget_unmap_request' drivers/built-in.o: In function `net2272_queue': drivers/usb/gadget/net2272.c:848: undefined reference to `usb_gadget_map_request' Signed-off-by: Geert Uytterhoeven --- drivers/usb/gadget/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 3c1cadd..324dc61 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -403,7 +403,7 @@ config USB_NET2272 config USB_NET2272_DMA boolean "Support external DMA controller" - depends on USB_NET2272 + depends on USB_NET2272 && HAS_DMA help The NET2272 part can optionally support an external DMA controller, but your board has to have support in the -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: USB host support should depend on HAS_DMA
On Thu, Jul 11, 2013 at 1:12 AM, Arnd Bergmann wrote:
> On Wednesday 10 July 2013, Alan Stern wrote:
>> This isn't right. There are USB host controllers that use PIO, not
>> DMA. The HAS_DMA dependency should go with the controller driver, not
>> the USB core.
>>
>> On the other hand, the USB core does call various routines like
>> dma_unmap_single. It ought to be possible to compile these calls even
>> when DMA isn't enabled. That is, they should be defined as do-nothing
>> stubs.
>
> The asm-generic/dma-mapping-broken.h file intentionally causes link
> errors, but that could be changed.
>
> The better approach in my mind would be to replace code like
>
>
> if (hcd->self.uses_dma)
>
> with
>
> if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
>
> which will reliably cause that reference to be omitted from object code,
> but not stop giving link errors for drivers that actually require
> DMA.
This can be done for drivers/usb/core/hcd.c.
But I'm a bit puzzled by drivers/usb/core/buffer.c. E.g.
void *hcd_buffer_alloc(...)
{
/* some USB hosts just use PIO */
if (!bus->controller->dma_mask &&
!(hcd->driver->flags & HCD_LOCAL_MEM)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
if (size <= pool_max[i])
return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
}
return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
}
which is called from usb_hcd_map_urb_for_dma():
if (hcd->self.uses_dma) {
} else if (hcd->driver->flags & HCD_LOCAL_MEM) {
ret = hcd_alloc_coherent(
urb->dev->bus, mem_flags,
&urb->setup_dma,
(void **)&urb->setup_packet,
sizeof(struct usb_ctrlrequest),
DMA_TO_DEVICE);
...
}
So if DMA is not used (!hcd->self.uses_dma, i.e. bus->controller->dma_mask
is zero), and HCD_LOCAL_MEM is set, we still end up calling dma_pool_alloc()?
(Naively, I'm not so familiar with the USB code) I'd expect it to use
kmalloc() instead?
So I would change it to
if (!IS_ENABLED(CONFIG_HAS_DMA) ||
(!bus->controller->dma_mask &&
!(hcd->driver->flags & HCD_LOCAL_MEM))) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}
Thanks for your clarification!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB configfs questions and help
Hi, On 18/08/13 17:52, Alan Stern wrote: Now there is some discussion about having a userpace library to handle the creation of all the necessary files easily. But I cannot find a trace of it. Does it exist already? No mention of it has appeared on the mailing list. From: https://lkml.org/lkml/2012/11/27/341 "Couldn't we have a tool to manage all this? target has such a thing, you have just select a few things via a CLI tool and the tool creates the directories for you _and_ removes them later on. I don't want to push python on anyone but the removal magic is simply straight forward: unlink the disk ports, rmdir luns, tpgt,… " Or http://marc.info/?l=linux-usb&m=132431126927355&w=2 "> I understand that the echo/mkdir like interface is not perfect for > everybody. For target there is a python tool that handles it so you/the > user does not see configfs internals. The problem with python is that > small users which ship just a busybox as their RFS might consider python > as too big. Therefore I would suggest a small C program/library and Yeah, a library is the way to go. It also "standardizes" the usage, allows for lots of language bindings - so if you want to use python/ruby/perl/C#/etc you can - and allows the whole thing to go into products." Or maybe I misunderstood your answer and it means nothing like that made yet? And does somebody know where that python tool might be available? Unfortunately I did not have the time today to read all the code, but some mails regarding configfs suggest some paths are created by configfs itself. I did not immediately see anything backing that up, but any input is welcome. It is a bit hard to follow all the different threads as the search mixes things up. $ mkdir strings/0x409" What is meant with language here, and why does the example have a hex string as identifier. Are we rather talking about one of the USB identifiers here? The word "language" refers to human languages, like English, French, Mandarin, and so on. The hex number is a language identifier, as specified by the USB Interface Forum: http://www.usb.org/developers/docs/ (see the paragraph near the end about LANGID codes). Of course! I just never looked deep enough into that. The strings are not USB identifiers; they are USB descriptors. True. I always mix the exact terms up. Thanks Alan! Philippe -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: musb: Avoid null pointer dereference in debug logging
Hello. On 18-08-2013 20:21, Maarten ter Huurne wrote: Since commit 511f3c53 usb_gadget_remove_driver will pass NULL for the Please also specify that commit's summary line in parens. driver argument. Signed-off-by: Maarten ter Huurne --- drivers/usb/musb/musb_gadget.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) WBR, Sergei -- 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 v1 02/49] USB: sg: prepare for enabling irq in complete()
On Sun, Aug 18, 2013 at 10:44 PM, Alan Stern wrote: > On Sun, 18 Aug 2013, Ming Lei wrote: > >> > As far as I can see, these don't need to disable interrupts. All they >> > protect against is the code in usb_sg_wait() and usb_sg_cancel(), which >> > both run in process context. >> >> Yes. >> >> > But will lockdep complain if they don't disable interrupts? >> >> Looks lockdep won't complain because the lock can't be held in >> another hardirq context. > > Don't be so sure. Suppose you have two mass-storage devices, one > connected by EHCI and one connected by UHCI. The one using UHCI _will_ > invoke the completion handler in hardirq context, because uhci-hcd > doesn't support tasklets. > > Have you tested this? It can't be triggered since we don't enable local interrupts yet before calling completion handler. Also uhci-hcd/ohci-hcd should support tasklet later, even for xhci-hcd, there is only little performance loss with tasklet. > >> As I mentioned in 00/50, the patchset is basically a mechanical >> change, so one patch can be dropped if anyone reviews and >> concludes it isn't needed. > > I'm afraid that it might be needed to keep lockdep happy, not to > prevent real problems. Right, so the patch should be kept. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB configfs questions and help
On Mon, 19 Aug 2013, Philippe De Swert wrote: > Hi, > > On 18/08/13 17:52, Alan Stern wrote: > >> Now there is some discussion about having a userpace library to handle > >> the creation of all the necessary files easily. But I cannot find a > >> trace of it. Does it exist already? > > > > No mention of it has appeared on the mailing list. > > From: https://lkml.org/lkml/2012/11/27/341 > > "Couldn't we have a tool to manage all this? > target has such a thing, you have just select a few things via a CLI > tool and the tool creates the directories for you _and_ removes them > later on. > I don't want to push python on anyone but the removal magic is simply > straight forward: unlink the disk ports, rmdir luns, tpgt,� " > > Or http://marc.info/?l=linux-usb&m=132431126927355&w=2 > > "> I understand that the echo/mkdir like interface is not perfect for > > everybody. For target there is a python tool that handles it so you/the > > user does not see configfs internals. The problem with python is that > > small users which ship just a busybox as their RFS might consider python > > as too big. Therefore I would suggest a small C program/library and > > Yeah, a library is the way to go. It also "standardizes" the usage, > allows for lots of language bindings - so if you want to use > python/ruby/perl/C#/etc you can - and allows the whole thing to go into > products." > > Or maybe I misunderstood your answer and it means nothing like that made > yet? Right -- nobody has mentioned building such a tool yet. > And does somebody know where that python tool might be available? Not me. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: Cleanup error code in **_usb_get_phy_**() APIs
Hi,
On Thu, Aug 8, 2013 at 12:05 AM, Julius Werner wrote:
>> @@ -94,11 +94,11 @@ static int devm_usb_phy_match(struct device *dev, void
>> *res, void *match_data)
>> */
>> struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
>> {
>> - struct usb_phy **ptr, *phy;
>> + struct usb_phy *phy = ERR_PTR(-ENOMEM), **ptr;
>
> This looks a little roundabout, don't you think? Why don't you just
> directly have 'return ERR_PTR(-ENOMEM)' down there where you put 'goto
> err0'?
Ok, will change this.
>
>>
>> ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
>> if (!ptr)
>> - return NULL;
>> + goto err0;
>>
>> phy = usb_get_phy(type);
>> if (!IS_ERR(phy)) {
>> @@ -107,6 +107,7 @@ struct usb_phy *devm_usb_get_phy(struct device *dev,
>> enum usb_phy_type type)
>> } else
>> devres_free(ptr);
>>
>> +err0:
>> return phy;
>> }
>> EXPORT_SYMBOL_GPL(devm_usb_get_phy);
>
>> struct usb_phy *devm_usb_get_phy_dev(struct device *dev, u8 index)
>> {
>> - struct usb_phy **ptr, *phy;
>> + struct usb_phy *phy = ERR_PTR(-ENOMEM), **ptr;
>
> Same here
will change this too.
>
>>
>> ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
>> if (!ptr)
>> - return NULL;
>> + goto err0;
>>
>> phy = usb_get_phy_dev(dev, index);
>> if (!IS_ERR(phy)) {
>> @@ -267,6 +268,7 @@ struct usb_phy *devm_usb_get_phy_dev(struct device *dev,
>> u8 index)
>> } else
>> devres_free(ptr);
>>
>> +err0:
>> return phy;
>> }
>> EXPORT_SYMBOL_GPL(devm_usb_get_phy_dev);
>
>> @@ -142,7 +142,7 @@ extern void usb_remove_phy(struct usb_phy *);
>> /* helpers for direct access thru low-level io interface */
>> static inline int usb_phy_io_read(struct usb_phy *x, u32 reg)
>> {
>> - if (x->io_ops && x->io_ops->read)
>> + if (!IS_ERR(x) && x->io_ops && x->io_ops->read)
>
> I liked the ones where we had IS_ERR_OR_NULL() here (and in all the
> ones below)... you sometimes have to handle PHYs in
> platform-independent code where you don't want to worry about if this
> platform actually has a PHY driver there or not. Any reason you
> changed that?
The **get_phy_*() APIs never return a NULL pointer now, do we still
need to handle that in that case.
Or are we assuming that code will use these phy operations without
getting a phy in the first place ?
>
>> return x->io_ops->read(x, reg);
>>
>> return -EINVAL;
>> @@ -150,7 +150,7 @@ static inline int usb_phy_io_read(struct usb_phy *x, u32
>> reg)
>>
>> static inline int usb_phy_io_write(struct usb_phy *x, u32 val, u32 reg)
>> {
>> - if (x->io_ops && x->io_ops->write)
>> + if (!IS_ERR(x) && x->io_ops && x->io_ops->write)
>> return x->io_ops->write(x, val, reg);
>>
>> return -EINVAL;
>> @@ -159,7 +159,7 @@ static inline int usb_phy_io_write(struct usb_phy *x,
>> u32 val, u32 reg)
>> static inline int
>> usb_phy_init(struct usb_phy *x)
>> {
>> - if (x->init)
>> + if (!IS_ERR(x) && x->init)
>> return x->init(x);
>>
>> return 0;
>> @@ -168,26 +168,27 @@ usb_phy_init(struct usb_phy *x)
>> static inline void
>> usb_phy_shutdown(struct usb_phy *x)
>> {
>> - if (x->shutdown)
>> + if (!IS_ERR(x) && x->shutdown)
>> x->shutdown(x);
>> }
>>
>> static inline int
>> usb_phy_vbus_on(struct usb_phy *x)
>> {
>> - if (!x->set_vbus)
>> - return 0;
>> + if (!IS_ERR(x) && x->set_vbus)
>> + return x->set_vbus(x, true);
>>
>> - return x->set_vbus(x, true);
>> + return 0;
>> }
>>
>> static inline int
>> usb_phy_vbus_off(struct usb_phy *x)
>> {
>> - if (!x->set_vbus)
>> - return 0;
>> + if (!IS_ERR(x) && x->set_vbus)
>> + return x->set_vbus(x, false);
>> +
>> + return 0;
>>
>> - return x->set_vbus(x, false);
>> }
>>
>> /* for usb host and peripheral controller drivers */
>> @@ -249,8 +250,9 @@ static inline int usb_bind_phy(const char *dev_name, u8
>> index,
>> static inline int
>> usb_phy_set_power(struct usb_phy *x, unsigned mA)
>> {
>> - if (x && x->set_power)
>> + if (!IS_ERR(x) && x->set_power)
>> return x->set_power(x, mA);
>> +
>> return 0;
>> }
>>
>> @@ -258,28 +260,28 @@ usb_phy_set_power(struct usb_phy *x, unsigned mA)
>> static inline int
>> usb_phy_set_suspend(struct usb_phy *x, int suspend)
>> {
>> - if (x->set_suspend != NULL)
>> + if (!IS_ERR(x) && x->set_suspend != NULL)
>> return x->set_suspend(x, suspend);
>> - else
>> - return 0;
>> +
>> + return 0;
>> }
>>
>> static inline int
>> usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed)
>> {
>> - if (x->notify_connect)
>> + if (!
Re: [PATCH 4/4] usb: chipidea: USB_CHIPIDEA should depend on HAS_DMA
On Sun, Aug 18, 2013 at 10:20:44PM +0200, Geert Uytterhoeven wrote: > If NO_DMA=y: > > drivers/built-in.o: In function `dma_set_coherent_mask': > include/linux/dma-mapping.h:93: undefined reference to `dma_supported' > > Signed-off-by: Geert Uytterhoeven > --- > drivers/usb/chipidea/Kconfig |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig > index d1bd8ef..dbd5232 100644 > --- a/drivers/usb/chipidea/Kconfig > +++ b/drivers/usb/chipidea/Kconfig > @@ -1,6 +1,6 @@ > config USB_CHIPIDEA > tristate "ChipIdea Highspeed Dual Role Controller" > - depends on USB || USB_GADGET > + depends on (USB || USB_GADGET) && HAS_DMA > help > Say Y here if your system has a dual role high speed USB > controller based on ChipIdea silicon IP. Currently, only the I can't understand why the DMA can't be changed to fix this instead of changing every driver? -- Best Regards, Peter Chen -- 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 01/15] drivers: phy: add generic PHY framework
Felipe,
ping..
On Wednesday 14 August 2013 08:35 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 14 August 2013 04:34 AM, Tomasz Figa wrote:
>> On Wednesday 14 of August 2013 00:19:28 Sylwester Nawrocki wrote:
>>> W dniu 2013-08-13 14:05, Kishon Vijay Abraham I pisze:
On Tuesday 13 August 2013 05:07 PM, Tomasz Figa wrote:
> On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:
>> On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:
>>> On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I
>> wrote:
> IMHO we need a lookup method for PHYs, just like for clocks,
> regulators, PWMs or even i2c busses because there are complex
> cases
> when passing just a name using platform data will not work. I
> would
> second what Stephen said [1] and define a structure doing
> things
> in a
> DT-like way.
>
> Example;
>
> [platform code]
>
> static const struct phy_lookup my_phy_lookup[] = {
>
> PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1",
> "phy.2"),
The only problem here is that if *PLATFORM_DEVID_AUTO* is used
while
creating the device, the ids in the device name would change
and
PHY_LOOKUP wont be useful.
>>>
>>> I don't think this is a problem. All the existing lookup
>>> methods
>>> already
>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c,
>>> ...). You
>>> can simply add a requirement that the ID must be assigned
>>> manually,
>>> without using PLATFORM_DEVID_AUTO to use PHY lookup.
>>
>> And I'm saying that this idea, of using a specific name and id,
>> is
>> frought with fragility and will break in the future in various
>> ways
>> when
>> devices get added to systems, making these strings constantly
>> have
>> to be
>> kept up to date with different board configurations.
>>
>> People, NEVER, hardcode something like an id. The fact that
>> this
>> happens today with the clock code, doesn't make it right, it
>> makes
>> the
>> clock code wrong. Others have already said that this is wrong
>> there
>> as
>> well, as systems change and dynamic ids get used more and more.
>>
>> Let's not repeat the same mistakes of the past just because we
>> refuse to
>> learn from them...
>>
>> So again, the "find a phy by a string" functions should be
>> removed,
>> the
>> device id should be automatically created by the phy core just
>> to
>> make
>> things unique in sysfs, and no driver code should _ever_ be
>> reliant
>> on
>> the number that is being created, and the pointer to the phy
>> structure
>> should be used everywhere instead.
>>
>> With those types of changes, I will consider merging this
>> subsystem,
>> but
>> without them, sorry, I will not.
>
> I'll agree with Greg here, the very fact that we see people
> trying to
> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already
> points
> to a big problem in the framework.
>
> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will
> end up
> adding similar infrastructure to the driver themselves to make
> sure
> we
> don't end up with duplicate names in sysfs in case we have
> multiple
> instances of the same IP in the SoC (or several of the same PCIe
> card).
> I really don't want to go back to that.
If we are using PLATFORM_DEVID_AUTO, then I dont see any way we
can
give the correct binding information to the PHY framework. I think
we
can drop having this non-dt support in PHY framework? I see only
one
platform (OMAP3) going to be needing this non-dt support and we
can
use the USB PHY library for it.>
>>>
>>> you shouldn't drop support for non-DT platform, in any case we
>>> lived
>>> without DT (and still do) for years. Gotta find a better way ;-)
>>
>> hmm..
>>
>> how about passing the device names of PHY in platform data of the
>> controller? It should be deterministic as the PHY framework assigns
>> its
>> own id and we *don't* want to add any requirement that the ID must
>> be
>> assigned manually without using PLATFORM_DEVID_AUTO. We can get rid
>> of
>> *phy_init_data* in the v10 patch series
[PATCH] USB: musb: Avoid null pointer dereference in debug logging
Since commit 511f3c53 (usb: gadget: udc-core: fix a regression during gadget driver unbinding) usb_gadget_remove_driver will pass NULL for the driver argument. Signed-off-by: Maarten ter Huurne --- drivers/usb/musb/musb_gadget.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index ba70923..82e5386 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1935,7 +1935,8 @@ static int musb_gadget_stop(struct usb_gadget *g, stop_activity(musb, driver); otg_set_peripheral(musb->xceiv->otg, NULL); - dev_dbg(musb->controller, "unregistering driver %s\n", driver->function); + dev_dbg(musb->controller, "unregistering driver %s\n", + driver ? driver->function : "(removed)"); musb->is_active = 0; musb->gadget_driver = NULL; -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: cpp41: make it compile with CONFIG_BUG=n
On Fri, Aug 16, 2013 at 05:40:55PM +0200, Sebastian Andrzej Siewior wrote:
> From: Sebastian Andrzej Siewior
>
> Before Randy figures out that this does not compile with CONFIG_BUG=n
> here is a fix for it.
>
> Signed-off-by: Sebastian Andrzej Siewior
Acked-by: Vinod Koul
~Vinod
> ---
> drivers/dma/cppi41.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index 5dcebca..e696178 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -579,7 +579,7 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
> WARN_ON(!c->is_tx && !(pd0 & TD_DESC_IS_RX));
> WARN_ON((pd0 & 0x1f) != c->port_num);
> } else {
> - __WARN();
> + WARN_ON_ONCE(1);
> }
> c->td_seen = 1;
> }
> --
> 1.8.4.rc2
>
--
--
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
