On Sun, 16 Dec 2012, Lan Tianyu wrote:
> On 2012/12/14 23:44, Alan Stern wrote:
> > On Fri, 14 Dec 2012, Lan Tianyu wrote:
> >
> >> Hi Alan:
> >> debounce is still needed. If connect status was not stable, resume
> >> operation will fail. So how about following?
> >
> > Actually, I'm not sure that a debounce is needed.
> >
> > Debouncing is used when a plug is physically inserted or removed,
> > because the electrical terminals make fleeting contact with one another
> > (they literally bounce on and off each other for a brief time). But in
> > this case there is no physical insertion. The terminals are in stable
> > physical contact with each other, so they can't bounce.
>
> I have done a test without debounce but resume failed due to port status
> bounce. So I add debounce again.
Okay. That's not a physical bounce but an electrical problem in the
device.
What you want here is sort of an alternate debounce routine. The
normal routine waits until the connect status has been stable for 100
ms. But you want to wait until the status has stable in the
"connected" state for 100 ms.
Maybe it would be better to do this by passing an extra argument to the
regular debounce routine.
> Is there a case that the port status bounced and its status maintained
> unconnected for a period which caused debounce returned un-connected
> status? And wait for more time, the port's status would become connected
> and stable.
That can happen. It doesn't cause any problems, because we will detect
the device when it connects the second time. It will then look like a
normal new device connection.
> > We're going to have to take a little more care with busy_bits in the
> > future. It's not protected by any lock, and there's not much to
> > prevent two different threads from using the same bit at the same time.
> > Only the fact that this would mean they were trying to control the same
> > port at the same time.
> Ok. So we need to add lock for busy_bits or a routine to change the
> busy_bits with a lock protecting. Something likes this.
>
> usb_hub_set_busy_bits(hub, port, set) {
>
> up(lock);
> if (set)
> set_bit(port1, hub->busy_bits);
> else
> clear_bit(port1, hub->busy_bits);
> down(lock);
> }
No, no, not at all. The set_bit and clear_bit operations don't need
any protection -- they are atomic. What we need to do is make sure
that two different threads don't try to manage the same port at the
same time. For example, it should not be possible for one thread to
reset the port while another thread is trying to suspend it.
I'm pretty sure that this can't happen with the existing code. The
only places where a port gets used are:
when a device is connected;
when a device is disconnected;
when a device is reset;
when a device is suspended or resumed.
We should check that these things really are mutually exclusive, and
that they remain mutually exclusive when your changes are added.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
