Re: xhci regression since "xhci: replace xhci_write_64() with writeq()" - devices not detected

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

 



On 29/01/2014 01:43 μμ, David Laight wrote:
From: Xenia Ragiadakou
On 29/01/2014 12:08 μμ, Rafał Miłecki wrote:
2014-01-29 David Laight <David.Laight@xxxxxxxxxx>:
Maybe I misunderstand something, but won't that commit end up replacing
the previous pair of writel() with a single native writeq() on 64bit
platforms?

Judging by the comment in front of the xhci_write_64(), that might not
necessarily be supported on all xHCI implementations:

   * Some xHCI implementations may support 64-bit address pointers.  Registers
   * with 64-bit address pointers should be written to with dword accesses by
   * writing the low dword first (ptr[0]), then the high dword (ptr[1]) second.
   * xHCI implementations that do not support 64-bit address pointers will ignore
   * the high dword, and write order is irrelevant.

I wonder if (some of) these implementations actually depend on this
two-cycle write, making __raw_writeq() fail?
...
This is notebook with i7-2670QM, see cpuinfo.txt for details.
I use x86_64 kernel:
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
(see attached config for details).


So the xhci controller supports 64 bits addressing (etc) but the cpu doesn't
have a 64bit write, so writeq() will do two 32bit accesses.
ISTR that these are in the correct order.
I assume the required compiler barrier is present.
I've enabled some debugging in xhci-dbg.c, does it help?
xhci_hcd 0000:04:00.0: xHCI capability registers at ffffc90004e60000:
xhci_hcd 0000:04:00.0: CAPLENGTH AND HCIVERSION 0x960020:
xhci_hcd 0000:04:00.0: CAPLENGTH: 0x20
xhci_hcd 0000:04:00.0: HCIVERSION: 0x96
xhci_hcd 0000:04:00.0: HCSPARAMS 1: 0x4000820
...
Like Bjørn already pointed out, I think too the problem is that the
USB3.0 host controller does not support 64 bit addressing (this can be
seen from the first bit of HCC PARAMS that is 0) but the patch does not
take it into account and blindly tries to perform 64bit write accesses
just because your system is 64bit. My mistake. I will try to find a
solution for that and send a patch when I ll return home.
I'm suffering from 'brain-fade' again.
The USB3 support on recent intel systems is in the chipset, not the cpu.
For some reason I've been thinking that it is cpu related.

So if you have a 32-bit xhci controller you'd think it would be a 32bit
slave and the something would split the 64bit cycle into two 32bit ones.
That would always happen on earlier systems for (say) 8 bit peripherals.

However I believe PCIe is always 64bit. If you do a 32bit write the target
actually sees a 64bit write with 4 byte enables asserted.
(This has causes confusion on some fpga PCIe slaves that then generate
two 32bit writes (to the internal components) one of which has no asserted
byte enables.)
This probably means that a 64bit write is never split before reaching the
PCIe slave.
In reality, if the slave needs the transfer split by the host then it
is badly broken!

Due to the high latency of PCIe transfers, you probably don't want to be
doing two transfers on systems where a single transfer will work.

It might be that support for 64bit transfers can be determined at run time
(to avoid a big quirk table). A global flag is probably fine as well.

	David

Hi David,

I'm suffering from brain-fade, not you :) Sorry for being late.
I take back what I've said. Now I'm thinking that, performing 64bit transfers instead of ordered 32bit transfers, has nothing to do with whether the host controller supports 64bit addressing or not. The 64bit addressing capability enables the host controller to perform DMA transfers to higher memory addresses so some fields that correspond to DMA addresses are 64bit. These fields can be accesses either with 64bit transfers or with ordered 32bit transfers.

So the read/write order matters when we have a host controller capable of 64bit addressing but our system can perform only 32bit transfers. In that case, we need to read/write the 64bit values (which correspond to 64bit pointers) in 32bit chunks with specific order.

If the system is 64bit but the host controller can address only 32bit addresses, then the values to be written with writeq should be actually 32bit. And even if they are written as 64bit values, the host controller will ignore the higher order 32bit (that will be zeroes) anyway, so the order does not matter.

Since Rafał's system seems to fit to the second case, I am no sure yet why the patch caused problems. Maybe Sarah can figure out what s wrong.

Also, if the system is 64bit and the host controller is 64bit capable, the host controller will see the 64bit transfers as atomic, no? Is there still the need to impose a split transfer with specific order?

Rafał is it possible to send all the "bad" output with xhci debugging on? Since your config file shows that dynamic debugging is on, that would be easy to do by adding dyndbg='module xhci_hcd +p' or xhci_hcd.dyndbg=+p boot option to your linux command line. Also, I am interested in the lspci -v output related to your usb3 controllers.

regards,
ksenia

--
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