On 20/03/17 09:21, Segher Boessenkool wrote:
On Mon, Mar 20, 2017 at 09:04:49AM +0000, Mark
+ uint32_t buf, count;
+ fw_cfg_read(FW_CFG_FILE_DIR, (char *)&buf, sizeof(uint32_t));
+ count = __be32_to_cpu(buf);
It's a lot cleaner to use e.g.
count = fw_cfg_read32(FW_CFG_FILE_DIR);
Yeah I know what you mean here. The reason for the above was that the
existing registers are LE, except for the fw_cfg API which is BE. And
since this was the only BE I just added it straight in...
I'm not sure I'm too excited about splitting this into a separate
You can make it inline if you are worried about performance.
I'm not really worried about the performance, just really trying to
minimise the code churn for something that only has one caller. And if
someone else does need it at a later date, it's fairly easy to refactor out.
although I do
see that I'm missing a couple of __be*_to_cpu()
wrappers in this patch. I'll do a resend later.
Yeah, you cannot make such mistakes if you do all accesses with
explicit endianness ;-)
Indeed, although coincidentally both SPARC and PPC are BE which is why I
didn't notice this in my local tests :)
and ideally you wouldn't use byte-swapping things
uint32_t fw_cfg_read32(unsigned char *p)
uint32_t x = 0;
for (int i = 0; i < 4; i++_
x = (x << 8) | p[i];
This still wouldn't handle the case of mixed LE and BE accesses though?
Are there both LE and BE things in fw_cfg? read_be32 etc. then?
Oh yes - this is where it gets really fun. The original fw_cfg interface
(cmd < 0x20) is LE, while the fw_cfg file API uses BE. And to make this
even more fun, SPARC fw_cfg accesses are done via ioport accesses which
are LE, while PPC fw_cfg accesses are done via MMIO which is BE.