Re: [PATCH v3 01/16] ALSA: Oxygen: Add the separate SPI waiting function

[Roman Volkov <v1ron@mail.ru>]

В Sat, 18 Jan 2014 11:26:49 +0100
Clemens Ladisch <clemens@ladisch.de> пишет:

> > This function performs waiting when the SPI bus completes
> > a transaction. Timeout error checking introduced and
> > the timeout increased to 400 from 40.  
> 
> Why 400?  SPI does not allow the codec to delay reads.

Yes. AFAIK, every SPI transaction will take almost the same time,
depending on the SPI clock and the number of bits to transfer,
regardless of what we're performing, reading or writing.

400 us is just the maximum time, exceeding this will cause an error.
Okay, lets calculate, in worst case (3 bytes to transfer, 1280ns clock):

1280 * 8 * 3 = 30270ns, 30,72 us. Perhaps plus some time to initiate
the transfer.

400us is greater than 40 by 10 times, but the function never waits 400
us, it always returns earlier than in 40-50 us. I always prefer to
increase these timeouts much longer than needed, this guarantees
that I never get false-positives and this wait is safe. Even if the
error occured, 400us is very small amount of time.

> > +	for (count = 100; count > 0; count--) {
> > +		if ((oxygen_read8(chip, OXYGEN_SPI_CONTROL) &
> > +						OXYGEN_SPI_BUSY)
> > == 0)
> > +			return 0;
> > +		udelay(4);
> > +		--count;
> > +	}  
> 
> This loop waits for 200 µs.
> 

Ah... Thanks.
Also why I prefer higher timeouts:
http://stackoverflow.com/questions/8352812/linux-kernel-udelay-returns-too-early
Even if a mistake or a bug, this works as needed :) This driver
currently works on my PC, if the test succeeded, this decreases my
attention :)

> The SPI transaction will not be finished for the first 30 µs or so, so
> polling is not needed for that time.

We may switch the context and force CPU to
execute useful code, using something like usleep, msleep. However,
this may block the thread for too long time. Please suggest
something how to optimize this.

> With the wait coming after the transaction, it makes more sense to
> have the busy check not before but after the udelay.

Okay, this will be:

for (count = 100; count > 0; count--) {
	udelay(4);
	if ((oxygen_read8(chip, OXYGEN_SPI_CONTROL) &
					OXYGEN_SPI_BUSY) == 0)
		return 0;
}

-- 
Kind regards,
Roman Volkov,
v1ron@mail.ru
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel