Re: [PATCH net-next 01/13] net: axienet: Fixed 64-bit compile, enable build on X86 and ARM

[Robert Hancock <hancock@sedsystems.ca>]

On 2019-05-31 3:10 p.m., Andrew Lunn wrote:
>>  static inline u32 axienet_ior(struct axienet_local *lp, off_t offset)
>>  {
>> -	return in_be32(lp->regs + offset);
>> +#ifdef CONFIG_MICROBLAZE
>> +	return __raw_readl(lp->regs + offset);
>> +#else
>> +	return ioread32(lp->regs + offset);
>> +#endif
>>  }
> 
> Please dig deeper into the available accessor functions. There should
> be a set which works without this #defery. 

This driver previously only compiled on MicroBlaze, and on that
platform, in_be32 is mapped to __raw_readl which reads with no byte
swapping. The confusing this is that MicroBlaze can apparently be set up
as either LE or BE, so I'm guessing that the hardware setup just
arranges that the reads are natively in the right byte order depending
on the mode. If I were to just use ioread32, there would be no change on
LE Microblaze, but BE Microblaze would start byte-swapping, which I
assume would break things.

The Xilinx version of this driver also supports Zynq (arm) and ZynqMP
(aarch64) platforms, and for those platforms it defines in_be32 to
__raw_readl as well. Since those are little-endian that ends up being
the same byte order as ioread32.

Finally, the setup we're using this hardware with on ARM over a PCIe to
AXI bridge exposes the device with the same byte order as any other PCIe
device, so the regular ioread32 accessors are correct.

I'm not quite sure what to make of that.. most platforms either would
need or work fine with the "regular" accessors, but I'm not sure that
wouldn't break big-endian MicroBlaze. It would be useful if one of the
Xilinx people could confirm that..

-- 
Robert Hancock
Senior Software Developer
SED Systems, a division of Calian Ltd.
Email: hancock@sedsystems.ca