Em Tue, 11 Aug 2015 18:18:39 +0300
Antti Palosaari <crope@xxxxxx> escreveu:
> 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.
Yes, (2) is more elegant.
> 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.
If only few interactions is needed, then OK. Please add a comment then,
explaining that.
>
> 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
>
--
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
