Re: [PATCH net-next 01/13] net: axienet: Fixed 64-bit compile, enable build on X86 and ARM
From: Andrew Lunn <andrew@lunn.ch>
Date: 2019-06-01 03:04:05
On Fri, May 31, 2019 at 05:28:45PM -0600, Robert Hancock wrote:
On 2019-05-31 3:10 p.m., Andrew Lunn wrote:quoted
quoted
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..
What matter here is the endianness of the devices register. Once you know that, there should be macros which work independent of the endianness of the CPU and compile to the right thing. Assuming the endianness of the device is fixed and not a synthesis option? If it is synthesis option, i would hope there is a register you can read to determine its endianennes. Andrew