Messages in this thread | | | Subject | Re: [DRIVER UPDATE] aty128fb | From | Jes Sorensen <> | Date | 27 Jan 2000 11:53:39 +0100 |
| |
>>>>> "Brad" == Brad Douglas <Brad@NERUO.com> writes:
Brad> A new patch is online for aty128fb (ATI Rage128) with added Brad> eieio() for testing.
Brad> Find it here: http://www.linux-fbdev.org/drivers/aty128fb.html
Hmmm just had a quick glance at the code.
First of all you ought to use wmb() and rmb() which are the portable names for those instructions rather than the PPC specific eieio() macro.
Second, you are not adding any io barrier to the 32 bit store macro (aty_st_le32()) but you do for the 8 bit version, why?
You also add the io barriers before the operation you are about to perform instead of after. That means that if you are storing a value to register, you have no idea when it is actually being propagated out to the hardware. The safer way to do this would be to put a wmb() after each register write and an rmb() after each register read(), instead of before
Last, this is more of a question, why does the PPC version of the driver use it's own inline asm version for the 32 access instead of using the readl/writel macros? readl/writel are defined to access the PCI bus shared memory in the bus native byte order, ie. little endian. Is this because it is running the chip in big endian mode? If you want to access it in the hos CPU's native byte order you should use the __raw_{write,read}l() macros instead and then add the appropriate [rw]mb() macro calls youself.
Note that standard {read,write}[bwl]() are guaranteeing ordering themselves so when using those you should not need to add eieio() manually. (note this was not the case on the Alpha until some time during 2.3.x).
Jes
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |