Re: usb/130230: [quirk] [usb67] [usb] [cam] [umass] Samsung Electronics YP-U3 does not attach in 7.1-RELEASE
On Saturday 06 November 2010 03:50:11 Boris Kochergin wrote: The following reply was made to PR usb/130230; it has been noted by GNATS. From: Boris Kochergin sp...@acm.poly.edu To: bug-follo...@freebsd.org Cc: Subject: Re: usb/130230: [quirk] [usb67] [usb] [cam] [umass] Samsung Electronics YP-U3 does not attach in 7.1-RELEASE Date: Fri, 05 Nov 2010 22:13:24 -0400 Here is the output of usbconfig dump_device_desc relevant to the device: ugen3.2: YP-U3 Samsung Electronics at usbus3, cfg=0 md=HOST spd=HIGH (480Mbps) pwr=ON bLength = 0x0012 bDescriptorType = 0x0001 bcdUSB = 0x0200 bDeviceClass = 0x bDeviceSubClass = 0x bDeviceProtocol = 0x bMaxPacketSize0 = 0x0040 idVendor = 0x04e8 idProduct = 0x507c bcdDevice = 0x0220 iManufacturer = 0x0001 Samsung Electronics iProduct = 0x0002 YP-U3 iSerialNumber = 0x0003 CEFBF7F26DFF bNumConfigurations = 0x0001 It still doesn't work out of the box on any version of FreeBSD, but I am running CURRENT now, so the following makes it work: usbconfig -d 3.2 add_quirk UQ_MSC_NO_INQUIRY usbconfig -d 3.2 add_quirk UQ_MSC_NO_SYNC_CACHE usbconfig -d 3.2 reset usbconfig -d 3.2 reset Hi, Can you create a quirk line for: /sys/dev/usb/quirk/usb_quirk.c ? And add any missing device ID's to usbdevs. Proably we should look at blacklisting using the idVendor and not just limit ourself to a single product. Thanks for reporting! --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: usb/130230: [quirk] [usb67] [usb] [cam] [umass] Samsung Electronics YP-U3 does not attach in 7.1-RELEASE
The following reply was made to PR usb/130230; it has been noted by GNATS. From: Hans Petter Selasky hsela...@c2i.net To: freebsd-usb@freebsd.org, Boris Kochergin sp...@acm.poly.edu, freebsd-gnats-sub...@freebsd.org Cc: Subject: Re: usb/130230: [quirk] [usb67] [usb] [cam] [umass] Samsung Electronics YP-U3 does not attach in 7.1-RELEASE Date: Sat, 6 Nov 2010 09:40:47 +0100 On Saturday 06 November 2010 03:50:11 Boris Kochergin wrote: The following reply was made to PR usb/130230; it has been noted by GNATS. From: Boris Kochergin sp...@acm.poly.edu To: bug-follo...@freebsd.org Cc: Subject: Re: usb/130230: [quirk] [usb67] [usb] [cam] [umass] Samsung Electronics YP-U3 does not attach in 7.1-RELEASE Date: Fri, 05 Nov 2010 22:13:24 -0400 Here is the output of usbconfig dump_device_desc relevant to the device: ugen3.2: YP-U3 Samsung Electronics at usbus3, cfg=0 md=HOST spd=HIGH (480Mbps) pwr=ON bLength = 0x0012 bDescriptorType = 0x0001 bcdUSB = 0x0200 bDeviceClass = 0x bDeviceSubClass = 0x bDeviceProtocol = 0x bMaxPacketSize0 = 0x0040 idVendor = 0x04e8 idProduct = 0x507c bcdDevice = 0x0220 iManufacturer = 0x0001 Samsung Electronics iProduct = 0x0002 YP-U3 iSerialNumber = 0x0003 CEFBF7F26DFF bNumConfigurations = 0x0001 It still doesn't work out of the box on any version of FreeBSD, but I am running CURRENT now, so the following makes it work: usbconfig -d 3.2 add_quirk UQ_MSC_NO_INQUIRY usbconfig -d 3.2 add_quirk UQ_MSC_NO_SYNC_CACHE usbconfig -d 3.2 reset usbconfig -d 3.2 reset Hi, Can you create a quirk line for: /sys/dev/usb/quirk/usb_quirk.c ? And add any missing device ID's to usbdevs. Proably we should look at blacklisting using the idVendor and not just limit ourself to a single product. Thanks for reporting! --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: USB 3.0 Fails To Attach Western Digital My Book 3.0
Hi,
Can you revert the last patch I sent an apply the attached one?
Then repeat the testing like last time.
--HPS
I captured several debug outputs here:
http://appliedtechnicalknowledge.com/freebsd/usb3-patch-214808/ . Maybe
something in these can help.
=== usb_hub.c
==
--- usb_hub.c (revision 214808)
+++ usb_hub.c (local)
@@ -126,9 +126,10 @@
static usb_callback_t uhub_intr_callback;
-static void usb_dev_resume_peer(struct usb_device *udev);
-static void usb_dev_suspend_peer(struct usb_device *udev);
-static uint8_t usb_peer_should_wakeup(struct usb_device *udev);
+static void usb_dev_resume_peer(struct usb_device *);
+static void usb_dev_suspend_peer(struct usb_device *);
+static uint8_t usb_peer_should_wakeup(struct usb_device *);
+static uint8_t usb_device_20_compatible(struct usb_device *);
static const struct usb_config uhub_config[UHUB_N_TRANSFER] = {
@@ -394,13 +395,29 @@
usb_pause_mtx(NULL,
USB_MS_TO_TICKS(USB_PORT_POWERUP_DELAY));
- /* reset port, which implies enabling it */
+ if (usb_device_20_compatible(udev)) {
+ /* reset port, which implies enabling it */
+ err = usbd_req_reset_port(udev, NULL, portno);
+ } else {
+ switch (UPS_PORT_LINK_STATE_GET(sc-sc_st.port_status))
{
+ case UPS_PORT_LS_U0:
+ case UPS_PORT_LS_U1:
+ case UPS_PORT_LS_U2:
+ case UPS_PORT_LS_U3:
+ /* no reset needed, link is already UP */
+ break;
+ case UPS_PORT_LS_SS_INA:
+ /* try to recover link using a warm reset */
+ goto error;
+ default:
+ /* reset port, which implies enabling it */
+ err = usbd_req_reset_port(udev, NULL, portno);
+ break;
+ }
+ }
- err = usbd_req_reset_port(udev, NULL, portno);
-
if (err) {
- DPRINTFN(0, port %d reset
- failed, error=%s\n,
+ DPRINTFN(0, port %d reset failed, error=%s\n,
portno, usbd_errstr(err));
goto error;
}
@@ -517,17 +534,43 @@
usb_free_device(child, 0);
child = NULL;
}
- if (err == 0) {
+ if (err) {
+ DPRINTFN(0, device problem (%s),
+ port %u\n, usbd_errstr(err), portno);
+ goto error_ret;
+ }
+ if (usb_device_20_compatible(udev)) {
+ /* disable the port, if enabled */
if (sc-sc_st.port_status UPS_PORT_ENABLED) {
err = usbd_req_clear_port_feature(
- sc-sc_udev, NULL,
- portno, UHF_PORT_ENABLE);
+ udev, NULL, portno, UHF_PORT_ENABLE);
}
+ } else {
+ /* get fresh status again */
+ err = uhub_read_port_status(sc, portno);
+ if (err)
+ goto error_ret;
+
+ switch (UPS_PORT_LINK_STATE_GET(sc-sc_st.port_status)) {
+ case UPS_PORT_LS_SS_INA:
+ /* Try a warm port reset to recover the port. */
+ err = usbd_req_warm_reset_port(udev, NULL, portno);
+ if (err) {
+ DPRINTF(warm port reset failed.\n);
+ goto error_ret;
+ }
+ break;
+ default:
+ /* disable the port, if enabled */
+ if (sc-sc_st.port_status UPS_PORT_ENABLED) {
+ err = usbd_req_clear_port_feature(
+ udev, NULL, portno, UHF_PORT_ENABLE);
+ }
+ break;
+ }
}
- if (err) {
- DPRINTFN(0, device problem (%s),
- disabling port %d\n, usbd_errstr(err), portno);
- }
+
+error_ret:
return (err);
}
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Sat, Nov 6, 2010 at 1:37 AM, Hans Petter Selasky hsela...@c2i.net wrote:
On Friday 05 November 2010 20:06:12 John Baldwin wrote:
On Friday, November 05, 2010 3:00:37 pm Hans Petter Selasky wrote:
On Friday 05 November 2010 19:48:05 Matthew Fleming wrote:
On Fri, Nov 5, 2010 at 11:45 AM, Hans Petter Selasky hsela...@c2i.net
wrote:
On Friday 05 November 2010 19:39:45 Matthew Fleming wrote:
True, but no taskqueue(9) code can handle that. Only the caller can
prevent a task from becoming enqueued again. The same issue exists
with taskqueue_drain().
I find that strange, because that means if I queue a task again while
it is running, then I doesn't get run? Are you really sure?
If a task is currently running when enqueued, the task struct will be
re-enqueued to the taskqueue. When that task comes up as the head of
the queue, it will be run again.
Right, and the taskqueue_cancel has to cancel in that state to, but it
doesn't because it only checks pending if !running() :-) ??
You can't close that race in taskqueue_cancel(). You have to manage that
race yourself in your task handler. For the callout(9) API we are only
able to close that race if you use callout_init_mtx() so that the code
managing the callout wheel can make use of your lock to resolve the races.
If you use callout_init() you have to explicitly manage these races in your
callout handler.
Hi,
I think you are getting me wrong! In the initial 0001-Implement-
taskqueue_cancel-9-to-cancel-a-task-from-a.patch you have the following code-
chunk:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ } else
+ rc = -EBUSY;
+ TQ_UNLOCK(queue);
+ return (rc);
+}
This code should be written like this, having the if (!task_is_running())
after checking the ta_pending variable.
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if ((rc = task-ta_pending) 0) {
+ STAILQ_REMOVE(queue-tq_queue, task, task, ta_link);
+ task-ta_pending = 0;
+ }
+ if (!task_is_running(queue, task))
+ rc = -EBUSY;
+ TQ_UNLOCK(queue);
+ return (rc);
+}
Or completely skip the !task_is_running() check. Else you are opening up a
race in this function! Do I need to explain that more? Isn't it obvious?
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
As John said, the taskqueue(9) implementation cannot protect consumers
of it from re-queueing a task; that kind of serialization needs to
happen at a higher level.
taskqueue(9) is not quite like callout(9); the taskqueue(9)
implementation drops all locks before calling the task's callback
function. So once the task is running, taskqueue(9) can do nothing
about it until the task stops running. This is why Jeff's
implementation of taskqueue_cancel(9) slept if the task was running,
and why mine returns an error code. The only way to know for sure
that a task is about to run is to ask taskqueue(9), because there's
a window where the TQ_LOCK is dropped before the callback is entered.
Thanks,
matthew
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
+ }
+ TQ_UNLOCK(queue);
+ return (rc);
+}
As John said, the taskqueue(9) implementation cannot protect consumers
of it from re-queueing a task; that kind of serialization needs to
happen at a higher level.
Agreed, but that is not what I'm pointing at. I'm pointing at what happens if
you re-queue a task and then cancel while it is actually running. Will the
task still be queued for execution after taskqueue_cancel()?
taskqueue(9) is not quite like callout(9); the taskqueue(9)
implementation drops all locks before calling the task's callback
function. So once the task is running, taskqueue(9) can do nothing
about it until the task stops running.
This is not the problem.
This is why Jeff's
implementation of taskqueue_cancel(9) slept if the task was running,
and why mine returns an error code. The only way to know for sure
that a task is about to run is to ask taskqueue(9), because there's
a window where the TQ_LOCK is dropped before the callback is entered.
And if you re-queue and cancel in this window, shouldn't this also be handled
like in the other cases?
Cut paste from kern/subr_taskqueue.c:
task-ta_pending = 0;
tb.tb_running = task;
TQ_UNLOCK(queue);
If a concurrent thread at exactly this point in time calls taskqueue_enqueue()
on this task, then we re-add the task to the queue-tq_queue. So far we
agree. Imagine now that for some reason a following call to taskqueue_cancel()
on this task at same point in time. Now, shouldn't taskqueue_cancel() also
remove the task from the queue-tq_queue in this case aswell? Because in
your implementation you only remove the task if we are not running, and that
is not true when we are at exactly this point in time.
task-ta_func(task-ta_context, pending);
TQ_LOCK(queue);
tb.tb_running = NULL;
wakeup(task);
Another issue I noticed is that the ta_pending counter should have a wrap
protector.
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: USB 3.0 Fails To Attach Western Digital My Book 3.0
On Saturday 06 November 2010 17:00:46 Michael Martin wrote: On 11/06/2010 03:31, Hans Petter Selasky wrote: Hi, Can you revert the last patch I sent an apply the attached one? Then repeat the testing like last time. --HPS Here are the results: http://appliedtechnicalknowledge.com/freebsd/usb_30_hub_patch/ I used one drive/port for the testing. da0 not recognized on first boot, but came up after an unplug cable/plug cable. Hi, I will have a closer look into this tomorrow. Does your device work when first recognized? What read/write speeds do you get? --HPS ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: USB 3.0 Fails To Attach Western Digital My Book 3.0
On 11/06/2010 10:26, Hans Petter Selasky wrote: On Saturday 06 November 2010 17:00:46 Michael Martin wrote: On 11/06/2010 03:31, Hans Petter Selasky wrote: Hi, Can you revert the last patch I sent an apply the attached one? Then repeat the testing like last time. --HPS Here are the results: http://appliedtechnicalknowledge.com/freebsd/usb_30_hub_patch/ I used one drive/port for the testing. da0 not recognized on first boot, but came up after an unplug cable/plug cable. Hi, I will have a closer look into this tomorrow. Does your device work when first recognized? What read/write speeds do you get? --HPS The device doesn't work when I try to import the zfs pool on the drive. The drive shows up in camcontrol ok. I initially exported the zfs file system to avoid attempts to mount it on boot. Once da0 is recognized by the kernel and camcontrol devlist shows it, I try a zpool import on the file system. The zpool import command hangs--sometimes the entire computer locks up, and I hard reboot the computer. I captured the usb logs during the zpool import for you there which captures some zfs i/o errors. mm ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Sat, Nov 6, 2010 at 7:22 AM, Hans Petter Selasky hsela...@c2i.net wrote:
Hi,
On Saturday 06 November 2010 14:57:50 Matthew Fleming wrote:
I think you're misunderstanding the existing taskqueue(9) implementation.
As long as TQ_LOCK is held, the state of ta-ta_pending cannot change,
nor can the set of running tasks. So the order of checks is
irrelevant.
I agree that the order of checks is not important. That is not the problem.
Cut paste from suggested taskqueue patch from Fleming:
+int
+taskqueue_cancel(struct taskqueue *queue, struct task *task)
+{
+ int rc;
+
+ TQ_LOCK(queue);
+ if (!task_is_running(queue, task)) {
+ if ((rc = task-ta_pending) 0)
+ STAILQ_REMOVE(queue-tq_queue, task, task,
ta_link); + task-ta_pending = 0;
+ } else {
+ rc = -EBUSY;
What happens in this case if ta_pending 0. Are you saying this is not
possible? If ta_pending 0, shouldn't we also do a STAILQ_REMOVE() ?
Ah! I see what you mean.
I'm not quite sure what the best thing to do here is; I agree it would
be nice if taskqueue_cancel(9) dequeued the task, but I believe it
also needs to indicate that the task is currently running. I guess
the best thing would be to return the old pending count by reference
parameter, and 0 or EBUSY to also indicate if there is a task
currently running.
Adding jhb@ to this mail since he has good thoughts on interfacing.
Thanks,
matthew
+ }
+ TQ_UNLOCK(queue);
+ return (rc);
+}
As John said, the taskqueue(9) implementation cannot protect consumers
of it from re-queueing a task; that kind of serialization needs to
happen at a higher level.
Agreed, but that is not what I'm pointing at. I'm pointing at what happens if
you re-queue a task and then cancel while it is actually running. Will the
task still be queued for execution after taskqueue_cancel()?
taskqueue(9) is not quite like callout(9); the taskqueue(9)
implementation drops all locks before calling the task's callback
function. So once the task is running, taskqueue(9) can do nothing
about it until the task stops running.
This is not the problem.
This is why Jeff's
implementation of taskqueue_cancel(9) slept if the task was running,
and why mine returns an error code. The only way to know for sure
that a task is about to run is to ask taskqueue(9), because there's
a window where the TQ_LOCK is dropped before the callback is entered.
And if you re-queue and cancel in this window, shouldn't this also be handled
like in the other cases?
Cut paste from kern/subr_taskqueue.c:
task-ta_pending = 0;
tb.tb_running = task;
TQ_UNLOCK(queue);
If a concurrent thread at exactly this point in time calls taskqueue_enqueue()
on this task, then we re-add the task to the queue-tq_queue. So far we
agree. Imagine now that for some reason a following call to taskqueue_cancel()
on this task at same point in time. Now, shouldn't taskqueue_cancel() also
remove the task from the queue-tq_queue in this case aswell? Because in
your implementation you only remove the task if we are not running, and that
is not true when we are at exactly this point in time.
task-ta_func(task-ta_context, pending);
TQ_LOCK(queue);
tb.tb_running = NULL;
wakeup(task);
Another issue I noticed is that the ta_pending counter should have a wrap
protector.
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
Re: [RFC] Outline of USB process integration in the kernel taskqueue system
On Fri, Nov 05, 2010 at 07:30:38PM +0100, Hans Petter Selasky wrote: Hi, In the patch attached to this e-mail I included Matthew Fleming's patch aswell. 1) I renamed taskqueue_cancel() into taskqueue_stop(), hence that resembles the words of the callout and USB API's terminology for doing the same. 2) I turns out I need to have code in subr_taskqueue.c to be able to make the operations atomic. 3) I did not update the manpage in this patch. Will do that before a commit. 4) My patch implements separate state keeping in struct task_pair, which avoids having to change any KPI's for now, like suggested by John Baldwin I think. 5) In my implementation I hard-coded the priority argument to zero, so that enqueuing is fast. Comments are welcome! The patch looks almost you moved usb_process.c code into taskqueue(9) that I means it still follows that a entry which enqueued at last should be executed at last. It seems that at least it's not a general for taskqueue(9). In my humble opinion it looks a trick. I think it'd better to find a general solution to solve it though I used sx(9) lock in my patches. regards, Weongyo Jeong ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org
