Re: [6/9][PATCH 2.6] Fix Tx engine race for good

Re: [6/9][PATCH 2.6] Fix Tx engine race for good

[Jeff Garzik]

Roger Luethi wrote:
> +#define RHINE_WAIT_FOR(condition) do {					\
> +	int i=1024;							\
> +	while (!(condition) && --i)					\
> +		;							\
> +	if (debug > 1 && i < 512)					\
> +		printk(KERN_INFO "%s: %4d cycles used @ %s:%d\n",	\
> +				DRV_NAME, 1024-i, __func__, __LINE__);	\
> +} while(0)

empty loops need at least a cpu_relax(), if not a true delay to 
guarantee the timing you desire.

Also, it would be nice to change the name, since there isn't anything 
rhine-specific about this macro.

	Jeff

Re: [6/9][PATCH 2.6] Fix Tx engine race for good

[Roger Luethi]

On Sat, 19 Jun 2004 17:22:44 -0400, Jeff Garzik wrote:
> Roger Luethi wrote:
> >+#define RHINE_WAIT_FOR(condition) do {				 \
> >+	int i=1024;							\
> >+	while (!(condition) && --i)					\
> >+		;							\
> >+	if (debug > 1 && i < 512)					\
> >+		printk(KERN_INFO "%s: %4d cycles used @ %s:%d\n",	\
> >+				DRV_NAME, 1024-i, __func__, __LINE__);	\
> >+} while(0)
> 
> empty loops need at least a cpu_relax(), if not a true delay to 
> guarantee the timing you desire.

Sure, I can add a cpu_relax(). FWIW, though, this macro is only used
for one purpose: Waiting for registers to reach a certain value. IOW:
Every evaluation of "condition" causes an I/O operation (inb or readb).

> Also, it would be nice to change the name, since there isn't anything 
> rhine-specific about this macro.

Hmm... It relies on DRV_NAME :-). It's trivial to write a more generic
version if there's interest. I'm just trying to keep the namespace
clean. ... I just checked: drivers/macintosh/via-cuda.c defines a macro
WAIT_FOR that does pretty much the same. Does that already make the
case for a generic function for everyone to use?

Roger