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

From: Andrew Lunn <[email-masked]>
Date: Sat, 1 Jun 2019 05:04:00 +0200

On Fri, May 31, 2019 at 05:28:45PM -0600, Robert Hancock wrote:
> 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..

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