Recall: [PATCH] xhci: remove unused variable 'addr' in inc_deq() and inc_enq().
Wang, Lin X would like to recall the message, "[PATCH] xhci: remove unused variable 'addr' in inc_deq() and inc_enq().".-- 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: gadget: nokia: fix error recovery path for optional functions
On 12/13/2013 02:46 PM, Andrzej Pietrasiewicz wrote:
> diff --git a/drivers/usb/gadget/nokia.c b/drivers/usb/gadget/nokia.c
> index 0a8099a..3ab3861 100644
> --- a/drivers/usb/gadget/nokia.c
> +++ b/drivers/usb/gadget/nokia.c
> @@ -126,9 +126,9 @@ static int __init nokia_bind_config(struct
> usb_configuration *c)
> struct usb_function *f_ecm;
> struct usb_function *f_obex2 = NULL;
> int status = 0;
> - int obex1_stat = 0;
> - int obex2_stat = 0;
> - int phonet_stat = 0;
> + int obex1_stat = -1;
> + int obex2_stat = -1;
> + int phonet_stat = -1;
I haven't look at the whole context but please do -ENODEV or something
like that but don't do -1.
> if (!IS_ERR(fi_phonet)) {
> f_phonet = usb_get_function(fi_phonet);
>
Sebastian
--
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: Bug#704242: Driver for PL-2303 HX not working
Hello Greg, Am 14.12.2013 18:08, schrieb Greg KH: On Fri, Dec 13, 2013 at 04:06:35PM +0100, Karsten Malcher wrote: Hello together, is there anything new for the PL-2303 HX? It would be fine if it could work like in the past. Have you tried a newer kernel in a while? A number of things have been hopefully fixed since June when you last looked. greg k-h no at this time i have not tested it - sorry. Although i am fascinated from your work i personally prefer to use stable releases for my daily work. This bug here shows that several functionalites in detail can be lost with an update. Then it takes much time to find workarounds - specially when hardware is not running any more. When i look in the Debian repositories i only see kernel 3.11.10 for testing and unstable. The latest one is 3.12.3 in experimental. I will see to upgrade one copy of my installation to testing on another partition with 3.11.10. Do you mean that will hopefully show a resolution for this problem? Regards Karsten -- 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] usbtest: Fix BOS control test for USB 2.01 devices.
Hi Sarah, On Fri, Dec 13, 2013 at 01:56:19PM -0800, Sarah Sharp wrote: > Commit c952a8ba7136505cd1ca01735cc748ddc08c7d2f "usb: usbtest: add a > test case to support bos for queue control" will cause USB 2.01 and USB > 2.10 devices with a BOS descriptor to fail case 15 of the control test. > > The Link PM errata (released in 2007, updated in 2011) says: > > "The value of the bcdUSB field in the standard USB 2.0 Device Descriptor > is used to indicate that the device supports the request to read the BOS > Descriptor (i.e. GetDescriptor(BOS)). Devices that support the BOS > descriptor must have a bcdUSB value of 0201H or larger." > > The current code says that non-SuperSpeed devices *must* return -EPIPE, > as this comment shows: > > /* sign of this variable means: > * -: tested code must return this (negative) error code > * +: tested code may return this (negative too) error code > */ > int expected = 0; > > This means the test will fail with USB 2.01 and USB 2.10 devices that > provide a BOS descriptor. Change it to only require a stall response if > the USB device bcdUSB is less than 2.01. > > Signed-off-by: Sarah Sharp > Cc: Huang Rui Thanks to catch and fix it. Acked-by: Huang Rui > --- > drivers/usb/misc/usbtest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c > index bff058ea222e..446ff55e3c58 100644 > --- a/drivers/usb/misc/usbtest.c > +++ b/drivers/usb/misc/usbtest.c > @@ -1224,7 +1224,7 @@ test_ctrl_queue(struct usbtest_dev *dev, struct > usbtest_param *param) > len = > le16_to_cpu(udev->bos->desc->wTotalLength); > else > len = sizeof(struct usb_bos_descriptor); > - if (udev->speed != USB_SPEED_SUPER) > + if (le16_to_cpu(udev->descriptor.bcdUSB) < 0x0201) > expected = -EPIPE; > break; > default: > -- > 1.8.3.3 > > -- 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] xhci: fix array index out of the bounds in function last_trb() and last_trb_on_last_seg()
In function last_trb() and last_trb_on_last_seg(), incorrect array index value
'TRBS_PER_SEGMENT' is used to determine the last element in an event ring
segment, which lead to the out-of-bounds
of index. But according to the current logic of event ring operation, this
"bug" brings no problems and the driver works as desired. This patch only fix
this array index out-of-bounds issue and there is
no need no modify corresponding ring operation.
Signed-off-by: Lin Wang
---
drivers/usb/host/xhci-ring.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d26cd94..3f93b07 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -98,21 +98,21 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci,
struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci->event_ring)
- return (trb == &seg->trbs[TRBS_PER_SEGMENT]) &&
+ return (trb == &seg->trbs[TRBS_PER_SEGMENT - 1]) &&
(seg->next == xhci->event_ring->first_seg);
else
return le32_to_cpu(trb->link.control) & LINK_TOGGLE;
}
-/* Is this TRB a link TRB or was the last TRB the last TRB in this event ring
- * segment? I.e. would the updated event TRB pointer step off the end of the
- * event seg?
+/* Is this TRB a link TRB or was the last TRB in this event ring segment?
+ * I.e. would the updated event TRB pointer step off the end of the event
+ * seg?
*/
static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci->event_ring)
- return trb == &seg->trbs[TRBS_PER_SEGMENT];
+ return trb == &seg->trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb->link.control);
}
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: remove unused variable 'addr' in inc_deq() and inc_enq()
This patch removes the variable 'addr' assigned but never used in function
inc_deq() and inc_enq().
Signed-off-by: Lin Wang
---
drivers/usb/host/xhci-ring.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d26cd94..afa28ce 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -156,8 +156,6 @@ static void next_trb(struct xhci_hcd *xhci,
*/
static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
{
- unsigned long long addr;
-
ring->deq_updates++;
/*
@@ -186,8 +184,6 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring
*ring)
ring->dequeue++;
}
} while (last_trb(xhci, ring, ring->deq_seg, ring->dequeue));
-
- addr = (unsigned long long) xhci_trb_virt_to_dma(ring->deq_seg,
ring->dequeue);
}
/*
@@ -212,7 +208,6 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring
*ring,
{
u32 chain;
union xhci_trb *next;
- unsigned long long addr;
chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
/* If this is not event ring, there is one less usable TRB */
@@ -264,7 +259,6 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring
*ring,
ring->enqueue = ring->enq_seg->trbs;
next = ring->enqueue;
}
- addr = (unsigned long long) xhci_trb_virt_to_dma(ring->enq_seg,
ring->enqueue);
}
/*
--
1.7.12.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: Bug#704242: Driver for PL-2303 HX not working
On Sun, Dec 15, 2013 at 11:32:33AM +0100, Karsten Malcher wrote: > Hello Greg, > > Am 14.12.2013 18:08, schrieb Greg KH: > > On Fri, Dec 13, 2013 at 04:06:35PM +0100, Karsten Malcher wrote: > >> Hello together, > >> > >> is there anything new for the PL-2303 HX? > >> It would be fine if it could work like in the past. > > Have you tried a newer kernel in a while? A number of things have been > > hopefully fixed since June when you last looked. > > > > greg k-h > > no at this time i have not tested it - sorry. > Although i am fascinated from your work i personally prefer to use stable > releases for my daily work. > This bug here shows that several functionalites in detail can be lost with an > update. > Then it takes much time to find workarounds - specially when hardware is not > running any more. > > When i look in the Debian repositories i only see kernel 3.11.10 for testing > and unstable. > The latest one is 3.12.3 in experimental. > I will see to upgrade one copy of my installation to testing on another > partition with 3.11.10. > > Do you mean that will hopefully show a resolution for this problem? I do not know, 3.11 is pretty old, sorry. -- 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/3] pm: make PM macros more smart
On Thu 2013-12-12 21:18:23, David Cohen wrote: > This patch makes SET_SYSTEM_SLEEP_PM_OPS() and SET_RUNTIME_PM_OPS() more > smart. > > Despite those macros check for '#ifdef CONFIG_PM_SLEEP/RUNTIME' to avoid > setting the callbacks when such #ifdef's aren't defined, they don't > handle compiler to avoid messages like that: > > drivers/usb/host/xhci-plat.c:200:12: warning: ???xhci_plat_suspend??? defined > but not used [-Wunused-function] > drivers/usb/host/xhci-plat.c:208:12: warning: ???xhci_plat_resume??? defined > but not used [-Wunused-function] > > As result, those macros get rid of #ifdef's when setting callbacks but > not when implementing them. > > With this patch, drivers using SET_*_PM_OPS() macros don't need to #ifdef > the callbacks implementation as well. Well... Interesting trickery, but it means that resulting kernel will be bigge due to the dead functions no? That may be acceptable tradeoff, but I guess its worth discussing... -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SuperSpeed ISO transfer returning EINVAL?
Hello everyone, I'm experimenting with the Kinect v2 on Linux (using libusbx-1.0.17). I've noticed that when trying to replicate the ISO transfers observed on Windows (8 packets of 0x8400 bytes each), I'm getting back -EINVAL from the usbfs IOCTL. For background info, I'm attaching a Wireshark dissection of the packet in question (as observed on Windows) and the lsusb -v output for the Kinect v2 itself. When I use a lower packet size, e.g. 0x4000, everything seems to work (although some data is obviously missing due to the reduced transfer size). A packet size of 0x6000 still seems to work spuriously, although I'm now getting a lot of -ENOMEM errors. This happens on 3.8 as well as on 3.11 (Ubuntu LTS kernels). usbfs_snoop doesn't help much here; I'm just seeing the USBDEVFS_SUBMITURB IOCTL being acknowledged in my syslog. Does somebody have any suggestions as to what the EINVAL actually means here, and how to fix this? Thanks & best regards, Florian -- SENT FROM MY DEC VT50 TERMINAL Bus 004 Device 005: ID 045e:02c4 Microsoft Corp. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 ? bDeviceProtocol 1 Interface Association bMaxPacketSize0 9 idVendor 0x045e Microsoft Corp. idProduct 0x02c4 bcdDevice1.00 iManufacturer 1 Microsoft iProduct2 Xbox NUI Sensor iSerial 4 016592534347 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 214 bNumInterfaces 4 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower 36mA Interface Association: bLength 8 bDescriptorType11 bFirstInterface 0 bInterfaceCount 2 bFunctionClass255 Vendor Specific Class bFunctionSubClass 255 Vendor Specific Subclass bFunctionProtocol 0 iFunction 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 7 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 7 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 7 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 19 Transfer TypeInterrupt Synch Type None Usage Type Feedback wMaxPacketSize 0x0040 1x 64 bytes bInterval 14 bMaxBurst 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber1 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber1 bAlternateSetting 1 bNumEndpoints 1 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84
Re: [RFC/PATCH 1/3] pm: make PM macros more smart
On Sun, Dec 15, 2013 at 06:51:12PM +0100, Pavel Machek wrote:
> On Thu 2013-12-12 21:18:23, David Cohen wrote:
> > This patch makes SET_SYSTEM_SLEEP_PM_OPS() and SET_RUNTIME_PM_OPS() more
> > smart.
> >
> > Despite those macros check for '#ifdef CONFIG_PM_SLEEP/RUNTIME' to avoid
> > setting the callbacks when such #ifdef's aren't defined, they don't
> > handle compiler to avoid messages like that:
> >
> > drivers/usb/host/xhci-plat.c:200:12: warning: ???xhci_plat_suspend???
> > defined but not used [-Wunused-function]
> > drivers/usb/host/xhci-plat.c:208:12: warning: ???xhci_plat_resume???
> > defined but not used [-Wunused-function]
> >
> > As result, those macros get rid of #ifdef's when setting callbacks but
> > not when implementing them.
> >
> > With this patch, drivers using SET_*_PM_OPS() macros don't need to #ifdef
> > the callbacks implementation as well.
>
> Well... Interesting trickery, but it means that resulting kernel
> will be bigge due to the dead functions no?
Actually, it doesn't get bigger. Before sending the patch I did this
dummy test app:
#include
#define USE_IT_OR_LOOSE_IT(fn) ((void *)((unsigned long)(fn) - (unsigned
long)(fn)))
#ifdef MAKE_ME_NULL
static int func1(int a)
{
printf("Hey!!\n");
return 0;
}
#endif
struct global_data {
int (*func)(int);
};
static struct global_data gd = {
#ifdef MAKE_ME_NULL
.func = USE_IT_OR_LOOSE_IT(func1),
#endif
};
int main(void)
{
#ifdef MAKE_ME_NULL
printf("MAKE_ME_NULL is defined\n");
#else
printf("MAKE_ME_NULL is NOT defined\n");
#endif
printf("%p\n", gd.func);
return 0;
}
Then I compiled 2 .S files:
$ gcc -DMAKE_ME_NULL test1.c -O2 -S -o test_makemenull.S
$ gcc test1.c -O2 -S -o test_no_makemenull.S
This is the diff between both:
--- test_makemenull.S 2013-12-15 11:07:02.635992492 -0800
+++ test_no_makemenull.S2013-12-15 11:07:10.639992359 -0800
@@ -1,7 +1,7 @@
.file "test1.c"
.section.rodata.str1.1,"aMS",@progbits,1
.LC0:
- .string "MAKE_ME_NULL is defined"
+ .string "MAKE_ME_NULL is NOT defined"
.LC1:
.string "%p\n"
.section.text.startup,"ax",@progbits
@@ -9,7 +9,7 @@
.globl main
.type main, @function
main:
-.LFB12:
+.LFB11:
.cfi_startproc
subq$8, %rsp
.cfi_def_cfa_offset 16
@@ -24,7 +24,7 @@
.cfi_def_cfa_offset 8
ret
.cfi_endproc
-.LFE12:
+.LFE11:
.size main, .-main
.ident "GCC: (Debian 4.8.2-1) 4.8.2"
.section.note.GNU-stack,"",@progbits
My conclusion is gcc's -O2 handles this situation pretty well, which
makes my patch to have not much actual side effects on kernel binary.
Br, David Cohen
>
> That may be acceptable tradeoff, but I guess its worth discussing...
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] fix our current target reap infrastructure.
On Sat, 14 Dec 2013, James Bottomley wrote: > > The refcount test and state change race with scsi_alloc_target(). > > Maybe the race won't occur in practice, but to be safe you should hold > > shost->host_lock throughout that time interval, as the original code > > here does. > > You mean the fact that using a state model to indicate whether we should > destroy a target without bothering to refcount isn't robust against two > threads of execution running through a scan on the same target? I meant that the patch you posted suffers from a race when one thread is adding a device to a target and another thread is removing an existing device below that target at the same time. Suppose the target's reap_ref count is initially equal to 1: Thread 0Thread 1 In scsi_alloc_target(): In scsi_target_reap(): lock host_lock scsi_target_reap_ref_put(): find existing starget starget->reap_ref drops to 0 incr starget->reap_ref In scsi_target_reap_ref_release(): unlock host_lockdevice_del(&starget->dev); starget->state == STARGET_DEL? No => okay to use starget set starget->state = STARGET_DEL Result: We end up using starget _and_ removing it. The only way to avoid this race would be to guarantee that we never add and remove devices below the same target at the same time. In theory this is feasible, but I don't know if you want to do it. This doesn't seem to be what you are talking about above. In any case, it is a bug. > Yes, it > could be construed as a bug, but it's a bug in the old code as well. The old code is immune to the bug I just described, because the existing scsi_target_reap() holds the host_lock while manipulating starget->state. Case A: Thread 0 acquires the host_lock first. Then thread 0 increments reap_ref while holding the lock. Later on, thread 1 acquires the lock and decrements reap_ref, but the value doesn't drop to 0 (because of the prior increment). Thus the target doesn't get reaped. Case B: Thread 1 acquires the host_lock first. Then thread 1 decrements reap_ref, sees that it drop to 0, and sets the state to STARGET_DEL, all before releasing the lock. Later on, thread 0 acquires the lock and increments reap_ref futilely, but sees that the state has already been changed to STARGET_DEL. Thus, it delays and retries instead of using starget. > > This means the kref approach won't work so easily. You might as well > > leave reap_ref as an ordinary int. > > Actually, no, we can better fix it using krefs. We just force > everything through the kref put instead of special casing the not made > visible destruction case. We can then case the release routine to fix > this, like below. I suppose since this is a separate bug I'll keep it > as a separate patch. It looks like you misunderstood the problem; the description in my previous email was perhaps excessively brief. Hopefully the explanation above is sufficiently explicit to make everything clear. This new, separate patch doesn't fix it. To fix this bug while using krefs would require that you hold the host_lock while doing the kref_put(). The release routine could set the state to STARGET_DEL, but then you would have to use the execute_in_usercontext mechanism to do the rest of the release work. That's doable, but it seems simpler to keep the code the way it is. In which case there's no need to change reap_ref into an atomic kref, because it is never modified outside the scope of the host_lock. 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: [RFC] fix our current target reap infrastructure.
On Sun, 2013-12-15 at 16:32 -0500, Alan Stern wrote:
> On Sat, 14 Dec 2013, James Bottomley wrote:
>
> > > The refcount test and state change race with scsi_alloc_target().
> > > Maybe the race won't occur in practice, but to be safe you should hold
> > > shost->host_lock throughout that time interval, as the original code
> > > here does.
> >
> > You mean the fact that using a state model to indicate whether we should
> > destroy a target without bothering to refcount isn't robust against two
> > threads of execution running through a scan on the same target?
>
> I meant that the patch you posted suffers from a race when one thread
> is adding a device to a target and another thread is removing an
> existing device below that target at the same time. Suppose the
> target's reap_ref count is initially equal to 1:
>
> Thread 0Thread 1
>
> In scsi_alloc_target(): In scsi_target_reap():
> lock host_lock scsi_target_reap_ref_put():
> find existing starget starget->reap_ref drops to 0
> incr starget->reap_ref In scsi_target_reap_ref_release():
> unlock host_lockdevice_del(&starget->dev);
> starget->state == STARGET_DEL?
> No => okay to use starget set starget->state = STARGET_DEL
>
> Result: We end up using starget _and_ removing it. The only way to
> avoid this race would be to guarantee that we never add and remove
> devices below the same target at the same time. In theory this is
> feasible, but I don't know if you want to do it.
>
> This doesn't seem to be what you are talking about above. In any case,
> it is a bug.
No, I was thinking of the two thread scan bug (i.e. two scan threads)
not one scan and one remove, which is a bug in the old code. This is a
race between put and get when the kref is incremented from zero (an
illegal operation which triggers a warn on).
The way to mediate this is to check for the kref already being zero
condition, like below.
James
---
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 327c0e92..303d471 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -473,7 +473,13 @@ static struct scsi_target *scsi_alloc_target(struct device
*parent,
return starget;
found:
- kref_get(&found_target->reap_ref);
+ if (!kref_get_unless_zero(&found_target->reap_ref))
+ /*
+* release routine already fired. Target is dead, but
+* STARGET_DEL may not yet be set (set in the release
+* routine), so set here as well, just in case
+*/
+ found_target->state = STARGET_DEL;
spin_unlock_irqrestore(shost->host_lock, flags);
if (found_target->state != STARGET_DEL) {
put_device(dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: gadget: Add UDC driver for Aeroflex Gaisler GRUSBDC
On 12/13/2013 08:52 PM, Felipe Balbi wrote:
Hi,
On Fri, Dec 13, 2013 at 08:48:30AM +0100, Andreas Larsson wrote:
On Wed, Dec 04, 2013 at 09:13:58AM +0100, Andreas Larsson wrote:
+static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
+ int status)
+{
+ struct gr_udc *dev;
+
+ list_del_init(&req->queue);
+
+ if (likely(req->req.status == -EINPROGRESS))
+ req->req.status = status;
+ else
+ status = req->req.status;
+
+ dev = ep->dev;
+ usb_gadget_unmap_request(&dev->gadget, &req->req, ep->is_in);
+ gr_free_dma_desc_chain(dev, req);
+
+ if (ep->is_in) /* For OUT, actual gets updated bit by bit */
+ req->req.actual = req->req.length;
+
+ if (!status) {
+ if (ep->is_in)
+ gr_dbgprint_request("SENT", ep, req);
+ else
+ gr_dbgprint_request("RECV", ep, req);
+ }
+
+ /* Prevent changes to ep->queue during callback */
+ ep->callback = 1;
+ if (req == dev->ep0reqo && !status) {
+ if (req->setup)
+ gr_ep0_setup(dev, req);
+ else
+ dev_err(dev->dev,
+ "Unexpected non setup packet on ep0in\n");
+ } else if (req->req.complete) {
+ unsigned long flags;
+
+ /*
+* Complete should be called with interrupts disabled according
+* to the contract of struct usb_request
+*/
+ local_irq_save(flags);
sorry but this driver isn't ready for inclusion. local_irq_save() is a
pretty good hint that there's something wrong in the driver. Consider
the fact that local_irq_save() will disable preemption even when
CONFIG_PREEMPT_FULL is enabled and you have a bit a problem.
This connection between local_irq_save and CONFIG_PREEMPT_RT_FULL was
unknown to me. Sure, I can disable interrupts right at spin lock
time.
that's better.
Alright, I'll put this in v4.
Also, the way you're using thread IRQs is quite wrong. I can't let that
pass and get merged upstream, sorry.
What is quite wrong? What is it that I need to fix?
Ideally the hardirq handler should be usually to actually check if
$this_device generated the IRQ, that should involve reading a IRQSTATUS
register of some sort.
Sure, check that IRQs are actually enabled, but you also need to read
STATUS register before waking the thread up.
I agree that that would be preferable. Unfortunately, the hardware lacks
status register bits that indicates whether interrupts has been
generated or not. That is why the only check is if interrupts are
disabled, because that is the only sure way to know that the softirq
handler does not need to go through everything.
Best regards,
Andreas Larsson
--
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 v6 08/15] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume
On Fri, Dec 13, 2013 at 02:09:24PM -0600, Felipe Balbi wrote:
> On Fri, Dec 13, 2013 at 02:31:42PM +0800, Peter Chen wrote:
> > On Fri, Dec 13, 2013 at 12:32 PM, Felipe Balbi wrote:
> > > On Fri, Dec 13, 2013 at 09:23:38AM +0800, Peter Chen wrote:
> > >> Implementation of notify_suspend and notify_resume will be different
> > >> according to mxs_phy_data->flags.
> > >>
> > >> Signed-off-by: Peter Chen
> > >> ---
> > >> drivers/usb/phy/phy-mxs-usb.c | 55
> > >> ++---
> > >> 1 files changed, 51 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/phy/phy-mxs-usb.c
> > >> b/drivers/usb/phy/phy-mxs-usb.c
> > >> index 0ef930a..e3df53f 100644
> > >> --- a/drivers/usb/phy/phy-mxs-usb.c
> > >> +++ b/drivers/usb/phy/phy-mxs-usb.c
> > >> @@ -166,8 +166,8 @@ static int mxs_phy_suspend(struct usb_phy *x, int
> > >> suspend)
> > >> static int mxs_phy_on_connect(struct usb_phy *phy,
> > >> enum usb_device_speed speed)
> > >> {
> > >> - dev_dbg(phy->dev, "%s speed device has connected\n",
> > >> - (speed == USB_SPEED_HIGH) ? "high" : "non-high");
> > >> + dev_dbg(phy->dev, "%s device has connected\n",
> > >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
> > >
> > > unrelated.
> > >
> > >> @@ -179,8 +179,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy,
> > >> static int mxs_phy_on_disconnect(struct usb_phy *phy,
> > >> enum usb_device_speed speed)
> > >> {
> > >> - dev_dbg(phy->dev, "%s speed device has disconnected\n",
> > >> - (speed == USB_SPEED_HIGH) ? "high" : "non-high");
> > >> + dev_dbg(phy->dev, "%s device has disconnected\n",
> > >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
> > >
> > > unrelated.
> > >
> >
> > Marek suggested using that string, I will added it at another patch.
> >
> > >> @@ -189,6 +189,48 @@ static int mxs_phy_on_disconnect(struct usb_phy
> > >> *phy,
> > >> return 0;
> > >> }
> > >>
> > >> +static int mxs_phy_on_suspend(struct usb_phy *phy,
> > >> + enum usb_device_speed speed)
> > >> +{
> > >> + struct mxs_phy *mxs_phy = to_mxs_phy(phy);
> > >> +
> > >> + dev_dbg(phy->dev, "%s device has suspended\n",
> > >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
> > >> +
> > >> + /* delay 4ms to wait bus entering idle */
> > >> + usleep_range(4000, 5000);
> > >> +
> > >> + if (mxs_phy->data->flags & MXS_PHY_ABNORMAL_IN_SUSPEND) {
> > >> + writel_relaxed(0x, phy->io_priv + HW_USBPHY_PWD);
> > >> + writel_relaxed(0, phy->io_priv + HW_USBPHY_PWD);
> > >> + }
> > >> +
> > >> + if (speed == USB_SPEED_HIGH)
> > >> + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
> > >> + phy->io_priv + HW_USBPHY_CTRL_CLR);
> > >
> > > why only on HS ? So if !HS and !ABNORMAL, this is no-op.
> > >
> > >> +static int mxs_phy_on_resume(struct usb_phy *phy,
> > >> + enum usb_device_speed speed)
> > >> +{
> > >> + dev_dbg(phy->dev, "%s device has resumed\n",
> > >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
> > >> +
> > >> + if (speed == USB_SPEED_HIGH) {
> > >> + /* Make sure the device has switched to High-Speed mode */
> > >> + udelay(500);
> > >> + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
> > >> + phy->io_priv + HW_USBPHY_CTRL_SET);
> > >> + }
> > >
> > > likewise, if !HS it's a no-op.
> > >
> >
> > Correct, this operation is only needed for HS.
> >
> > >> @@ -235,6 +277,11 @@ static int mxs_phy_probe(struct platform_device
> > >> *pdev)
> > >>
> > >> platform_set_drvdata(pdev, mxs_phy);
> > >>
> > >> + if (mxs_phy->data->flags & MXS_PHY_SENDING_SOF_TOO_FAST) {
> > >> + mxs_phy->phy.notify_suspend = mxs_phy_on_suspend;
> > >> + mxs_phy->phy.notify_resume = mxs_phy_on_resume;
> > >> + }
> > >
> > > hmm, and seems like you only need notify_* on a buggy device. Sorry
> > > Peter but you don't have enough arguments to make me agree with this
> > > (and previous) patch.
> > >
> > > You gotta find a better way to handle this using normal phy
> > > suspend/resume calls.
> > >
> >
> > Like I explained at previous patch, it needs to be notified during
> > ehci suspend/resume.
> > I admit it is a SoC bug, but all SoCs have bugs, hmm.
> > Software needs the solution to workaround it which breaks the standard USB
> > spec.
>
> Then I think what you need is a real notification mechanism. usbcore
> already notifies about buses and devices being added and removed,
> perhaps you can convince Greg to accept suspend/resume notifications.
>
> With that, you can (conditionally) make this driver listen to usbcore
> notifications. That'll be more work, but I guess it's best in the long
> run as we won't need to keep on adding callbacks to the USB PHY
> structure just because another buggy
Re: [PATCH 1/3] usb: chipidea: add support for USB OTG controller on LSI Zevio SoCs
On Sun, Dec 15, 2013 at 02:35:07PM +1100, dt.ta...@gmail.com wrote:
> From: Daniel Tang
>
> The USB controller in TI-NSPIRE calculators (LSI Zevio SoC) are based off
> either
> Freescale's USB OTG controller or the USB controller found in the IMX233, both
> of which are Chipidea compatible.
>
> This patch adds a device tree binding for the controller.
>
> Signed-off-by: Daniel Tang
> ---
> drivers/usb/chipidea/Makefile|1 +
> drivers/usb/chipidea/ci_hdrc_zevio.c | 72
> ++
> 2 files changed, 73 insertions(+)
> create mode 100644 drivers/usb/chipidea/ci_hdrc_zevio.c
>
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index a99d980..7635407 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -10,6 +10,7 @@ ci_hdrc-$(CONFIG_USB_CHIPIDEA_DEBUG)+= debug.o
> # Glue/Bridge layers go here
>
> obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_msm.o
> +obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o
>
> # PCI doesn't provide stubs, need to check
> ifneq ($(CONFIG_PCI),)
> diff --git a/drivers/usb/chipidea/ci_hdrc_zevio.c
> b/drivers/usb/chipidea/ci_hdrc_zevio.c
> new file mode 100644
> index 000..3bf6489
> --- /dev/null
> +++ b/drivers/usb/chipidea/ci_hdrc_zevio.c
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright (C) 2013 Daniel Tang
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * Based off drivers/usb/chipidea/ci_hdrc_msm.c
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include "ci.h"
> +
> +static struct ci_hdrc_platform_data ci_hdrc_zevio_platdata = {
> + .name = "ci_hdrc_zevio",
> + .flags = CI_HDRC_REGS_SHARED,
> + .capoffset = DEF_CAPOFFSET,
> +};
> +
> +static int ci_hdrc_zevio_probe(struct platform_device *pdev)
> +{
> + struct platform_device *ci_pdev;
> +
> + dev_dbg(&pdev->dev, "ci_hdrc_zevio_probe\n");
> +
> + ci_pdev = ci_hdrc_add_device(&pdev->dev,
> + pdev->resource, pdev->num_resources,
> + &ci_hdrc_zevio_platdata);
> +
> + if (IS_ERR(ci_pdev)) {
> + dev_err(&pdev->dev, "ci_hdrc_add_device failed!\n");
> + return PTR_ERR(ci_pdev);
> + }
> +
> + platform_set_drvdata(pdev, ci_pdev);
> +
> + return 0;
> +}
> +
> +static int ci_hdrc_zevio_remove(struct platform_device *pdev)
> +{
> + struct platform_device *ci_pdev = platform_get_drvdata(pdev);
> +
> + ci_hdrc_remove_device(ci_pdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ci_hdrc_zevio_dt_ids[] = {
> + { .compatible = "lsi,zevio-usb", },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver ci_hdrc_zevio_driver = {
> + .probe = ci_hdrc_zevio_probe,
> + .remove = ci_hdrc_zevio_remove,
> + .driver = {
> + .name = "zevio_usb",
> + .owner = THIS_MODULE,
> + .of_match_table = ci_hdrc_zevio_dt_ids,
> + },
> +};
> +
> +MODULE_DEVICE_TABLE(of, ci_hdrc_zevio_dt_ids);
> +module_platform_driver(ci_hdrc_zevio_driver);
> +
> +MODULE_LICENSE("GPL v2");
> --
The driver patch is okay for me, please cc me your dt patch next
time, I will apply it when your dt patches are applied by
dt maintainer (or any of your dt strings "zevio_xxx" are applied).
--
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 1/3] usb: chipidea: add support for USB OTG controller on LSI Zevio SoCs
Hi, On 16/12/2013, at 12:19 PM, Peter Chen wrote: > > The driver patch is okay for me, please cc me your dt patch next > time, I will apply it when your dt patches are applied by > dt maintainer (or any of your dt strings "zevio_xxx" are applied). > > -- > > Best Regards, > Peter Chen > Thank you very much Cheers, Daniel Tang -- 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 7/7] usb: dwc3: exynos: add pm_runtime support
Hi Felipe,
> On Fri, Dec 13, 2013 at 02:01:32PM +0900, Anton Tikhomirov wrote:
> > Hi Felipe,
> >
> > > -static int dwc3_exynos_suspend(struct device *dev)
> > > +static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
> > > {
> > > - struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > -
> > > clk_disable(exynos->clk);
> > >
> > > return 0;
> > > }
> > >
> > > +static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
> > > +{
> > > + return clk_enable(exynos->clk);
> > > +}
> > > +
> > > +static int dwc3_exynos_suspend(struct device *dev)
> > > +{
> > > + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > +
> > > + return __dwc3_exynos_suspend(exynos);
> >
> > If dwc3-exynos is runtime suspended, the clock will be disabled
> > second time here (unbalanced clk_enable/clk_disable).
>
> I don't get what you mean but there is something that probably needs
> fixing, I guess below makes it better:
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> exynos.c
> index c93919a..1e5720a 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -218,6 +218,9 @@ static int dwc3_exynos_suspend(struct device *dev)
> {
> struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>
> + if (pm_runtime_suspended(dev))
> + return 0;
> +
> return __dwc3_exynos_suspend(exynos);
> }
>
>
> Is that what you meant ?
Yes, this is exactly what I meant.
Thanks.
--
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] fix our current target reap infrastructure.
On Sun, 15 Dec 2013, James Bottomley wrote: > No, I was thinking of the two thread scan bug (i.e. two scan threads) > not one scan and one remove, which is a bug in the old code. This is a > race between put and get when the kref is incremented from zero (an > illegal operation which triggers a warn on). > > The way to mediate this is to check for the kref already being zero > condition, like below. Yes, that seems reasonable. Consider now: Having done this, to what extent do starget->reap_ref and starget->state really need to be protected by the host_lock? Maybe only the linked lists require protection. (I haven't checked.) Can you post a single, combined patch incorporating all your proposed changes? It's little hard to review them in pieces... Alan Stern P.S.: Would you agree that the phrase "pretty astonishing cockup" did indeed turn out to be appropriate? :-) -- 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 7/7] usb: dwc3: exynos: add pm_runtime support
Hi Felipe,
> On Fri, Dec 13, 2013 at 01:56:18PM -0600, Felipe Balbi wrote:
> > On Fri, Dec 13, 2013 at 02:01:32PM +0900, Anton Tikhomirov wrote:
> > > Hi Felipe,
> > >
> > > > -static int dwc3_exynos_suspend(struct device *dev)
> > > > +static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
> > > > {
> > > > - struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > > -
> > > > clk_disable(exynos->clk);
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > +static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
> > > > +{
> > > > + return clk_enable(exynos->clk);
> > > > +}
> > > > +
> > > > +static int dwc3_exynos_suspend(struct device *dev)
> > > > +{
> > > > + struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> > > > +
> > > > + return __dwc3_exynos_suspend(exynos);
> > >
> > > If dwc3-exynos is runtime suspended, the clock will be disabled
> > > second time here (unbalanced clk_enable/clk_disable).
> >
> > I don't get what you mean but there is something that probably needs
> > fixing, I guess below makes it better:
> >
> > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-
> exynos.c
> > index c93919a..1e5720a 100644
> > --- a/drivers/usb/dwc3/dwc3-exynos.c
> > +++ b/drivers/usb/dwc3/dwc3-exynos.c
> > @@ -218,6 +218,9 @@ static int dwc3_exynos_suspend(struct device *dev)
> > {
> > struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> >
> > + if (pm_runtime_suspended(dev))
> > + return 0;
> > +
> > return __dwc3_exynos_suspend(exynos);
> > }
> >
> >
> > Is that what you meant ?
>
> note, however, that this is *not* a case where we would fall today. See
> that we pm_runtime_get() in probe and only pm_runtime_put() during
> remove. So there would never be a case where we would try system
> suspend
> while device was already runtime suspended.
You are right, while runtime PM is blocked by get_sync() in probe, this
check
doesn't matter.
>
> I have fixed all patches in my testing/next branch anyway, just to make
> sure we're "idiot-proof" when it comes to implementing real runtime pm
> later on :-)
>
> cheers
>
> --
> balbi
Thank you
--
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] fix our current target reap infrastructure.
On Sun, 15 Dec 2013, James Bottomley wrote: > No, I was thinking of the two thread scan bug (i.e. two scan threads) > not one scan and one remove, which is a bug in the old code. By the way, the existing code doesn't allow two threads to scan a target at the same time. They would both have to hold the host's scan_mutex. On the other hand, as far as I can see there's nothing to prevent two threads from removing a device at the same time. But that wouldn't cause any 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: [RFC] fix our current target reap infrastructure.
On Sun, 2013-12-15 at 21:44 -0500, Alan Stern wrote: > On Sun, 15 Dec 2013, James Bottomley wrote: > > > No, I was thinking of the two thread scan bug (i.e. two scan threads) > > not one scan and one remove, which is a bug in the old code. This is a > > race between put and get when the kref is incremented from zero (an > > illegal operation which triggers a warn on). > > > > The way to mediate this is to check for the kref already being zero > > condition, like below. > > Yes, that seems reasonable. Consider now: Having done this, to what > extent do starget->reap_ref and starget->state really need to be > protected by the host_lock? Maybe only the linked lists require > protection. (I haven't checked.) Yes, I think so, but that can be done as an enhancement patch after the fact. > Can you post a single, combined patch incorporating all your proposed > changes? It's little hard to review them in pieces... Sure, I'll repost what I have. > Alan Stern > > P.S.: Would you agree that the phrase "pretty astonishing cockup" did > indeed turn out to be appropriate? :-) Objection, m'lud, my learned friend is leading the witness ... James -- 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] fix our current target reap infrastructure.
On Sun, 2013-12-15 at 21:49 -0500, Alan Stern wrote: > On Sun, 15 Dec 2013, James Bottomley wrote: > > > No, I was thinking of the two thread scan bug (i.e. two scan threads) > > not one scan and one remove, which is a bug in the old code. > > By the way, the existing code doesn't allow two threads to scan a > target at the same time. They would both have to hold the host's > scan_mutex. I thought of that, but it's dropped too early. That makes the race almost untriggerable because the racing thread starts way behind, but it's not theoretically impossible. > On the other hand, as far as I can see there's nothing to prevent two > threads from removing a device at the same time. But that wouldn't > cause any problems. Right. Thanks, James -- 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: BBB gadgetfs freeze patch
palesius . writes: > > In working on a project (http://github.com/dominicgs/USBProxy) we ran > across an issue where io on endpoint 0 would cause the system to hang. > > We made the following change which resolved the problem for us. I'm > not sure if this is the "correct" fix, or if the problem is specific > to our platform BeagleBone Black with a musb-hrdc controller. I seem to be experiencing the same issue. Could you advise on where you made the change? Thank you! -- 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/2] usb: gadget: atmel_usba: Use devm_*() functions
Use devm_*() functions to make cleanup paths simpler.
Signed-off-by: Jingoo Han
---
drivers/usb/gadget/atmel_usba_udc.c | 64 ++-
1 file changed, 17 insertions(+), 47 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..409a055 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1995,14 +1995,12 @@ static int __init usba_udc_probe(struct platform_device
*pdev)
if (irq < 0)
return irq;
- pclk = clk_get(&pdev->dev, "pclk");
+ pclk = devm_clk_get(&pdev->dev, "pclk");
if (IS_ERR(pclk))
return PTR_ERR(pclk);
- hclk = clk_get(&pdev->dev, "hclk");
- if (IS_ERR(hclk)) {
- ret = PTR_ERR(hclk);
- goto err_get_hclk;
- }
+ hclk = devm_clk_get(&pdev->dev, "hclk");
+ if (IS_ERR(hclk))
+ return PTR_ERR(hclk);
spin_lock_init(&udc->lock);
udc->pdev = pdev;
@@ -2011,17 +2009,17 @@ static int __init usba_udc_probe(struct platform_device
*pdev)
udc->vbus_pin = -ENODEV;
ret = -ENOMEM;
- udc->regs = ioremap(regs->start, resource_size(regs));
+ udc->regs = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
if (!udc->regs) {
dev_err(&pdev->dev, "Unable to map I/O memory, aborting.\n");
- goto err_map_regs;
+ return ret;
}
dev_info(&pdev->dev, "MMIO registers at 0x%08lx mapped at %p\n",
(unsigned long)regs->start, udc->regs);
- udc->fifo = ioremap(fifo->start, resource_size(fifo));
+ udc->fifo = devm_ioremap(&pdev->dev, fifo->start, resource_size(fifo));
if (!udc->fifo) {
dev_err(&pdev->dev, "Unable to map FIFO, aborting.\n");
- goto err_map_fifo;
+ return ret;
}
dev_info(&pdev->dev, "FIFO at 0x%08lx mapped at %p\n",
(unsigned long)fifo->start, udc->fifo);
@@ -2032,7 +2030,7 @@ static int __init usba_udc_probe(struct platform_device
*pdev)
ret = clk_prepare_enable(pclk);
if (ret) {
dev_err(&pdev->dev, "Unable to enable pclk, aborting.\n");
- goto err_clk_enable;
+ return ret;
}
toggle_bias(0);
usba_writel(udc, CTRL, USBA_DISABLE_MASK);
@@ -2043,22 +2041,22 @@ static int __init usba_udc_probe(struct platform_device
*pdev)
else
udc->usba_ep = usba_udc_pdata(pdev, udc);
- if (IS_ERR(udc->usba_ep)) {
- ret = PTR_ERR(udc->usba_ep);
- goto err_alloc_ep;
- }
+ if (IS_ERR(udc->usba_ep))
+ return PTR_ERR(udc->usba_ep);
- ret = request_irq(irq, usba_udc_irq, 0, "atmel_usba_udc", udc);
+ ret = devm_request_irq(&pdev->dev, irq, usba_udc_irq, 0,
+ "atmel_usba_udc", udc);
if (ret) {
dev_err(&pdev->dev, "Cannot request irq %d (error %d)\n",
irq, ret);
- goto err_request_irq;
+ return ret;
}
udc->irq = irq;
if (gpio_is_valid(udc->vbus_pin)) {
if (!devm_gpio_request(&pdev->dev, udc->vbus_pin,
"atmel_usba_udc")) {
- ret = request_irq(gpio_to_irq(udc->vbus_pin),
+ ret = devm_request_irq(&pdev->dev,
+ gpio_to_irq(udc->vbus_pin),
usba_vbus_irq, 0,
"atmel_usba_udc", udc);
if (ret) {
@@ -2077,31 +2075,13 @@ static int __init usba_udc_probe(struct platform_device
*pdev)
ret = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
if (ret)
- goto err_add_udc;
+ return ret;
usba_init_debugfs(udc);
for (i = 1; i < udc->num_ep; i++)
usba_ep_init_debugfs(udc, &udc->usba_ep[i]);
return 0;
-
-err_add_udc:
- if (gpio_is_valid(udc->vbus_pin))
- free_irq(gpio_to_irq(udc->vbus_pin), udc);
-
- free_irq(irq, udc);
-err_request_irq:
-err_alloc_ep:
-err_clk_enable:
- iounmap(udc->fifo);
-err_map_fifo:
- iounmap(udc->regs);
-err_map_regs:
- clk_put(hclk);
-err_get_hclk:
- clk_put(pclk);
-
- return ret;
}
static int __exit usba_udc_remove(struct platform_device *pdev)
@@ -2117,16 +2097,6 @@ static int __exit usba_udc_remove(struct platform_device
*pdev)
usba_ep_cleanup_debugfs(&udc->usba_ep[i]);
usba_cleanup_debugfs(udc);
- if (gpio_is_valid(udc->vbus_pin)) {
- free_irq(gpio_to_irq(udc->vbus_pin), udc);
- }
-
- free_irq(udc->irq, udc);
- iounmap(udc->fifo);
- iounmap(udc->regs);
- clk_put(udc->hclk);
- clk_put(udc->pclk);
-
[PATCH 2/2] usb: gadget: atmel_usba: Fix sparse warning
'usba_gadget_template' is used only in this file. Thus, make
'usba_gadget_template' static, in order to fix the following
sparse warning.
warning: symbol 'usba_gadget_template' was not declared. Should it be static?
Signed-off-by: Jingoo Han
---
drivers/usb/gadget/atmel_usba_udc.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c
b/drivers/usb/gadget/atmel_usba_udc.c
index 409a055..55ae5ef 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1012,7 +1012,7 @@ static void nop_release(struct device *dev)
}
-struct usb_gadget usba_gadget_template = {
+static struct usb_gadget usba_gadget_template = {
.ops= &usba_udc_ops,
.max_speed = USB_SPEED_HIGH,
.name = "atmel_usba_udc",
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Remove double initialization of msg_namelen variable
This removes the double initialization of the msg_namelen variable. Signed-off-by: Sankha Narayan Guria --- linux/drivers/staging/usbip/usbip_common.c.orig 2013-12-16 11:24:32.930512781 +0530 +++ linux/drivers/staging/usbip/usbip_common.c 2013-12-16 11:25:03.190513334 +0530 @@ -366,7 +366,6 @@ int usbip_recv(struct socket *sock, void msg.msg_namelen = 0; msg.msg_control = NULL; msg.msg_controllen = 0; - msg.msg_namelen= 0; msg.msg_flags = MSG_NOSIGNAL; result = kernel_recvmsg(sock, &msg, &iov, 1, size, MSG_WAITALL); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] AX88179_178A: Add FLAG_HW_IPALIGN to determine whether reserving NET_IP_ALIGN bytes for an SKB.
On 2013年12月13日 18:36, David Laight wrote:
From: fre...@asix.com.tw
...
- skb = __netdev_alloc_skb_ip_align(dev->net, size, flags);
+ if (dev->driver_info->flags & FLAG_HW_IPALIGN)
+ skb = __netdev_alloc_skb(dev->net, size, flags);
+ else
+ skb = __netdev_alloc_skb_ip_align(dev->net, size, flags);
Given the definition:
static inline struct sk_buff *__netdev_alloc_skb_ip_align(struct net_device
*dev,
unsigned int length, gfp_t gfp)
{
struct sk_buff *skb = __netdev_alloc_skb(dev, length + NET_IP_ALIGN,
gfp);
if (NET_IP_ALIGN && skb)
skb_reserve(skb, NET_IP_ALIGN);
return skb;
}
It really ought to be possible to code that without an extra conditional.
David
The AX88179_178A driver do need to know the value of NET_IP_ALIGN
to determine whether enabling the feature that makes IP header align
on a dword-aligned address, but according to the comments from David
Miller, I need to consider all situations, not just for the case that
NET_IP_ALIGN is zero, so the condition added in rx_submit is just used to
determine whether reserving NET_IP_ALIGN bytes for each SKB.
Freddy
--
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
