RE: [PATCH 1/4] EHCI: Add Intel Moorestown EHCI controller HOSTPCx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx]
>Sent: Thursday, June 11, 2009 11:44 AM
>To: Pan, Jacob jun
>Cc: linux-usb@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH 1/4] EHCI: Add Intel Moorestown EHCI controller HOSTPCx
>
>On Thu, 11 Jun 2009, Pan, Jacob jun wrote:
>
>> >> +		if (hcd->self.root_hub->do_remote_wakeup) {
>> >> +			if (t1 & PORT_CONNECT) {
>> >> +				t2 |= PORT_WKOC_E | PORT_WKDISC_E;
>> >> +				t2 &= ~PORT_WKCONN_E;
>> >> +			} else {
>> >> +				t2 |= PORT_WKOC_E | PORT_WKCONN_E;
>> >> +				t2 &= ~PORT_WKDISC_E;
>> >> +			}
>> >> +		} else
>>
>> > Why is this change needed?  Are your controllers non-compliant with the
>> > spec, causing them to generate wakeup notifications at the wrong time?
>>
>> > The spec says, for example, that if PORT_WKCONN_E is set then a
>> > suspended controller will generate a wakeup request when a connection
>> > event occurs.  But if the port was already connected then there is no
>> > connection event.
>>
>> The change is indeed needed due to new features in our host
>> controller HW which are beyond EHCI spec. Specifically, our host
>> controller can put the internal phy into low power mode based on the
>> state of the connection. With both wake on connect/disconnect set, it
>> can not go to phy low power suspend.
>
>Sounds like the hardware is badly designed.  It doesn't make sense that
>you can go to a low-power state when either one of the bits is set but
>not when both of them are set.
>
>> On the other side, I think it is also the right thing to do to enable
>> only the appropriate wake bits.
>
>No, it is not the right thing to do.  In fact, it is a race.  What
>happens if the connect state changes between the time when you read the
>port status and when you set the wakeup bits?  You'd end up with the
>wrong bit set.
>
>Alan Stern
[[JPAN]] in this case, would the port change event result in an interrupt and eventually set the right bits? Here is the flow I can think of in case of no device attached.
1. HCD read portsc, show not connected
2. PCD occurred due to a device connection
3. HCD write to portsc for wake on connect (wrong as you mentioned)
4. HCD process PCD then find out connected, then set wake on disconnect bit (corrected)

HCD=host controller driver
PCD=port change detection
Sorry for the acronyms, just try to be clear and lazy :).
--
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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux