On 2012-11-19 11:46, Russell King - ARM Linux wrote: > On Mon, Nov 19, 2012 at 11:36:24AM +0200, Tomi Valkeinen wrote: >> Probably not. I can't say anything to that matter, but I wonder if this >> patch is just going around the problem that we get sparse warnings when >> falling into the else ifdef block in fb.h. >> >> The macros in the else block are defined as: >> >> #define fb_readb(addr) (*(volatile u8 *) (addr)) >> >> And fb code passes a pointer to __iomem. So shouldn't the cast be to >> (volatile u8 __iomem *)? > > No, because sparse won't let you directly dereference an __iomem pointer. > > Directly dereferencing an __iomem pointer is wrong, always has been. It's > marked with __iomem _because_ it's a separate cookied address space from > the virtual address space. > > This is one of the situations which has been left because the warning is > correct - and this is one of those situations where silencing them via > casts et.al. is the wrong thing to do. The right thing to do is to leave > the warning in place. > > This is one of the hardest things that I seem to get people to understand: > if the code is wrong then we _should_ get a warning for it and we should > definitely not paper over the warning. Yes, you're right. Well, I'm not very experienced with handling different endiannesses, but my analysis: fb.h uses either __raw_read/write functions or (*(volatile u32 *) (addr)) and (*(volatile u32 *) (addr) = (b)) for read/write. __raw_read/write are defined in arch/arm/include/asm/io.h and are the same for BE and LE. I made a test c-file with two functions, one using raw_read style assembly, other using pointer dereference. I compiled it with -mlittle-endian and -mbig-endian, and the resulting assembly was identical for both, and the assembly for both functions were the same. So if I didn't make any mistakes above, using __raw_read/write instead of the direct pointer dereference results in identical operation for both big and little endian arms. So if big-endian fb reads/writes is correct now, it should be correct with raw_read/write also. Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
