Thread (22 messages) 22 messages, 4 authors, 2019-06-03

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help