Re: [DRIVER UPDATE] aty128fb

From: Jes Sorensen (Jes.Sorensen@cern.ch)
Date: Thu Jan 27 2000 - 05:53:39 EST


>>>>> "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/



This archive was generated by hypermail 2b29 : Mon Jan 31 2000 - 21:00:18 EST