On Mon, Mar 20, 2017 at 09:04:49AM +0000, Mark Cave-Ayland wrote:
- 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 function
You can make it inline if you are worried about performance.
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 ;-)
and ideally you wouldn't use byte-swapping things at all:
uint32_t fw_cfg_read32(unsigned char *p) { uint32_t x = 0; for (int i = 0; i < 4; i++_ x = (x << 8) | p[i]; return x; }
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?
Segher