On 08/11/2015 01:20 PM, Mauro Carvalho Chehab wrote:
Em Thu, 9 Jul 2015 07:06:29 +0300
Antti Palosaari <crope@xxxxxx> escreveu:
Use jiffies to set timeout for firmware command status polling.
It is more elegant solution than poll X times with sleep.
/* wait cmd execution terminate */
- for (i = 1000, uitmp = 1; i && uitmp; i--) {
+ #define CMD_EXECUTE_TIMEOUT 30
+ timeout = jiffies + msecs_to_jiffies(CMD_EXECUTE_TIMEOUT);
+ for (uitmp = 1; !time_after(jiffies, timeout) && uitmp;) {
ret = regmap_read(dev->regmap, 0x1f, &uitmp);
if (ret)
goto error;
-
- usleep_range(200, 5000);
Hmm... removing the usleep() doesn't sound a good idea. You'll be
flooding the I2C bus with read commands and spending CPU cycles
for 30ms spending more power than the previous code. That doesn't
sound more "elegant solution than poll X times with sleep" for me.
So, I would keep the usleep_range() here and add a better
comment on the patch description.
First of all, polling firmware ready status is very common for chips
having firmware. And there is 2 ways to implement it:
1) poll N times in a loop using X sleep, timeout = N * X
2) poll in a loop using jiffies as a timeout
IMHO 2 is more elegant solution and I have started using it recently.
What you now propose is add some throttle in order to slow down polling
interval to reduce I2C I/O. Yes sure less I/O is better, but downside is
that it makes some unneeded extra delay to code path. Usually these sort
firmware ready polling ends rather quickly, in a loop or two.
Sure it eats some extra CPU cycles, but I think extra control messages
are about nothing compared to I/O used for data streaming.
Which kind of throttle delay you think is suitable for polling command
status over I2C bus?
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
