Alan Stern wrote:
On Fri, 22 Jan 2010, Jan Andersson wrote:
Hi,
I am writing bus glue to add support for a USB host controller that,
depending on the implementation, may have big or little endian registers
(and descriptors). Everything is working fine when running on a system
that has the controller in little endian mode. However, when running
with a big endian host controller I get into problems with the following
definitions in include/linux/usb/ehci-def.h:
#define HC_LENGTH(p) (((p)>>00)&0x00ff) /* bits 7:0 */
#define HC_VERSION(p) (((p)>>16)&0xffff) /* bits 31:16 */
The EHCI specification defines CAPLENGTH as an 8-bit register at offset
0. The byte at offset 0x01 is reserved and HCIVERSION is a 2 byte
register at offset 0x02. Since the macros above are used on a word they
make sense for little endian. However, in a big endian system a 32-bit
read would return the registers in the following order:
CAPLENGTH : RESERVED : HCIVERSION
as opposed to the little endian arrangement of:
HCIVERSION : RESERVED : CAPLENGTH
On a big endian system this means that HC_LENGTH will select the LSB of
HCIVERSION and HC_VERSION will select CAPLENGTH plus the reserved field.
Since there is support for other big endian host controllers I assume
that they have considered HCIVERSION and CAPLENGTH to not be individual
registers and instead that they are part of a 32-bit register. This
works fine since the capability registers are always read using a 32-bit
read.
I would like to fix this so that a host controller that is (truly) big
endian can be used. The best way would have been to access CAPLENGTH and
HCIVERSION using byte and half-word accesses. But according to the
comment in ehci-def.h this is bad since some hosts can't perform these
types of accesses and it would also break for current "big" endian host
controllers.
The only decent way of fixing this, that I can come up with, is to let
the initialization code read out the two values and store them in struct
ehci_hcd. Would this be an acceptable solution?
Why is this a problem? With the exception of some old code in
ehci-au1xxx.c, this register is always read via ehci_readl(), which
automatically takes the endianness into account.
Yes, and EHCI readl() works for the other registers that are defined as
32-bit and should be read with DWORD accesses only. However here we have
the case where CAPLENGTH is specified to be at byte offset 0 (as opposed
to a bit position within a 32-bit word). On a big endian system byte 0
is 31:24 in a 32-bit word and on a little endian system it is 7:0 (in a
32-bit word).
For the big endian version of the host controller I need to set
ehci->big_endian_mmio to 1 so that the reads and writes to registers are
not byte swapped by software (I run on SPARC that is big endian).
ehci_readl will then call readl_be() that will return a 32 bit word
organized as CAPLENGTH : RESERVED : HCIVERSION.
Since CAPLENGTH is defined to be a 8-bit register at offset base + 0x0
this is the expected position of that register (on a big endian system)
when doing a 32-bit read access. But it does not match with the
definition of HC_LENGTH(p).
This issue would not have come up if CAPLENGTH and HCIVERSION were
specified as members of a 32-bit register (as most of the other
registers are). If you do not agree with me think of the case where
CAPLENGTH, as specified, is read out using a byte access. The
specification says that it is located at offset 0. Should a 32-bit read
on a big endian system return CAPLENGTH on any other position that 31:24
on a 32-bit read to offset 0 then that byte will not be located at the
specified address.
Best regards,
Jan
--
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
