[OpenBIOS] [PATCH 2/3] fw_cfg: add fw_cfg_find_file() and fw_cfg_read_file() functions

Segher Boessenkool segher at kernel.crashing.org
Mon Mar 20 10:21:10 CET 2017


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



More information about the OpenBIOS mailing list