On 09/05/11 22:34, Alan Stern wrote:
> On Mon, 9 May 2011, Jan Andersson wrote:
>
>> This patch adds support for big endian mmio to the UHCI HCD. Big endian
>> mmio is supported by adding a flag bit to the UHCI HCD replicating the
>> solution used in the EHCI HCD.
>>
>> When adding big endian support this patch also adds a check to see if we
>> need to support HCs with PCI I/O registers when we support HCs with MMIO.
>>
>> This patch also adds 'const' to the register access functions' uhci_hcd
>> argument.
>>
>> Signed-off-by: Jan Andersson <jan@xxxxxxxxxxx>
>
>> diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
>> index a4e64d0..31f110f 100644
>> --- a/drivers/usb/host/uhci-hcd.h
>> +++ b/drivers/usb/host/uhci-hcd.h
>
> ...
>
>> +#if defined(CONFIG_USB_UHCI_BIG_ENDIAN_MMIO)
>
> Isn't "#ifdef" more common?
>
>> +/* Support (non-PCI) big endian host controllers */
>> +#define uhci_big_endian_mmio(u) ((u)->big_endian_mmio)
>> +#else
>> +#define uhci_big_endian_mmio(u) 0
>> +#endif
>>
>> -static inline u32 uhci_readl(struct uhci_hcd *uhci, int reg)
>> +static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
>> {
>> if (uhci_has_pci_registers(uhci))
>> return inl(uhci->io_addr + reg);
>> +#if defined(CONFIG_USB_UHCI_BIG_ENDIAN_MMIO)
>> + else if (uhci_big_endian_mmio(uhci))
>> + return readl_be(uhci->regs + reg);
>> +#endif
>> else
>> return readl(uhci->regs + reg);
>> }
>
> I have to admit, this whole business is kind of confusing. The same is
> true of the implementation in ehci.h (obviously, since this is more or
> less a direct copy).
>
> In principle, the device can use either big- or little-endian mmio, and
> the CPU can be either big- or little-endian. If the two agree then no
> change should be needed, otherwise the values should have to be
> byte-swapped. But that doesn't appear to be what this code does.
> Probably I'm just misunderstanding something.
>
> Alan Stern
I have never seen it formally documented but after looking at a couple
of read{l,w} implementations I concluded that memory accessed by
read/write is assumed to be little endian. For SPARC32, where the
GRUSBHC driver is used, read/write comes with a byteswap and __raw_readl
is just "return *(__force volatile u32 *)addr". My uneducated guess is
that read/write is assumed to access PCI which is typically implemented
as little endian. Perhaps it would be more suitable to use ioread32()
and ioread32be() (again I assume that ioread here is assumed to work
with LE memory since the is a *be variant). But I do not know if it is
available on all archs. The EHCI HCD used read/write and defined
read_be/write_be so I did the same thing.
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
