On Wed, Oct 29, 2014 at 12:14 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> The host CPU is connected to the peripheral/register interface using a
>> 32-bit wide data bus. A simple 32-bit store originating from the host
>> CPU, targeted to an onchip SoC peripheral, will never need endian
>> swapping. i.e. this code works equally well on all supported systems
>> regardless of endianness:
>>
>> volatile u32 *foo = (void *)MY_REG_VA;
>> *foo = 0x12345678;
>>
>> 8-bit and 16-bit accesses may be another story, but only appear in a
>> few very special peripherals.
>
> Sorry, but this makes no sense. If you run a little-endian kernel
> on one of the MIPS systems that you marked as "always BE", or a
> big-endian kernel on the systems that are marked "always LE",
> then you have to byte swap.
If I ran an LE MIPS kernel on a BE system, it would hang on boot. I
know this through experience. ;-)
Setting aside the problem that the instruction words, pointers, and
bitfields in the image are all in the wrong byte order, there are many
other endian-specific assumptions baked into the executable.
Does this actually work on other architectures like ARM? I still see
compile-time checks for CONFIG_CPU_ENDIAN* in a couple of places under
arch/arm.
>> FWIW, several of the BCM7xxx peripherals default to "native" mode (no
>> swap for either LE/BE), but can be optionally reconfigured as LE in
>> order to preserve compatibility with the standard AHCI/SDHCI/...
>> drivers that use the PCI accessors.
>
> The reconfigurability is definitely the worst part.
That depends on who is doing the reconfiguring...
If the kernel has to support both options, then it's definitely a
hassle (and probably quite intrusive for some drivers).
In our case we just enable swapping unconditionally and then use the
stock driver code, with no changes to the I/O accessors.
>> Or, we could add IRQ_GC_NATIVE_IO and/or IRQ_GC_BE_IO to enum irq_gc_flags.
>>
>> Would either of these choices satisfy everyone's goals?
>
> This is what I meant with doing extra work in the case where we want to
> support both in the same kernel. We would only enable the runtime
> logic if both GENERIC_IRQ_CHIP and GENERIC_IRQ_CHIP_BE are set, and
> leave it up to the platform to select the right one. For MIPS BCM7xxx,
> you could use
>
> config BCM7xxx
> select GENERIC_IRQ_CHIP if CPU_LITTLE_ENDIAN
> select GENERIC_IRQ_CHIP_BE if CPU_BIG_ENDIAN
>
> so you would default to the hardwired big-endian accessor unless
> some other drivers selects GENERIC_IRQ_CHIP.
generic-chip.c already has a fair amount of indirection, with pointers
to saved masks, user-specified register offsets, and such. Is there a
concern that introducing, say, a pair of readl/writel function
pointers, would cause an unacceptable performance drop?
Backing up a little bit, do we have a consensus that defining
irq_reg_{readl,writel} at compile time from include/linux/irq.h is a
bad idea for multiplatform images, and it should be removed one way or
another?
