Re: [alsa-devel] [PATCH 11/11] sound/oxygen_io: take msecs_to_jiffies_min into use
At Fri, 10 May 2013 15:13:29 +0300, Imre Deak wrote:
Use msecs_to_jiffies_min instead of open-coding the same.
Signed-off-by: Imre Deak imre.deak@intel.com
sound/pci/oxygen/oxygen_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/oxygen/oxygen_io.c b/sound/pci/oxygen/oxygen_io.c index 521eae4..132ecbe 100644 --- a/sound/pci/oxygen/oxygen_io.c +++ b/sound/pci/oxygen/oxygen_io.c @@ -108,7 +108,7 @@ static int oxygen_ac97_wait(struct oxygen *chip, unsigned int mask) wait_event_timeout(chip->ac97_waitqueue, ({ status |= oxygen_read8(chip, OXYGEN_AC97_INTERRUPT_STATUS); status & mask; }),
msecs_to_jiffies(1) + 1);
msecs_to_jiffies_min(1));
This would change the behavior, I guess. (Though, I'm not sure whether the original code was intentional.)
And, isn't msecs_to_jiffies_min(1) identical with msecs_to_jiffies(1)?
thanks,
Takashi
On Mon, 2013-05-13 at 16:00 +0200, Takashi Iwai wrote:
At Fri, 10 May 2013 15:13:29 +0300, Imre Deak wrote:
Use msecs_to_jiffies_min instead of open-coding the same.
Signed-off-by: Imre Deak imre.deak@intel.com
sound/pci/oxygen/oxygen_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/oxygen/oxygen_io.c b/sound/pci/oxygen/oxygen_io.c index 521eae4..132ecbe 100644 --- a/sound/pci/oxygen/oxygen_io.c +++ b/sound/pci/oxygen/oxygen_io.c @@ -108,7 +108,7 @@ static int oxygen_ac97_wait(struct oxygen *chip, unsigned int mask) wait_event_timeout(chip->ac97_waitqueue, ({ status |= oxygen_read8(chip, OXYGEN_AC97_INTERRUPT_STATUS); status & mask; }),
msecs_to_jiffies(1) + 1);
msecs_to_jiffies_min(1));
This would change the behavior, I guess.
Not to my understanding, the new macro should end up doing the same thing.
(Though, I'm not sure whether the original code was intentional.)
Well, I only assumed that.. But using wait_event_timeout() without the +1 would make little sense to me. In that case we may not wait at all for the condition to become true, if we are close to the next scheduling clock tick.
And, isn't msecs_to_jiffies_min(1) identical with msecs_to_jiffies(1)?
No, it should be one more in value.
--Imre
At Mon, 13 May 2013 17:24:38 +0300, Imre Deak wrote:
On Mon, 2013-05-13 at 16:00 +0200, Takashi Iwai wrote:
At Fri, 10 May 2013 15:13:29 +0300, Imre Deak wrote:
Use msecs_to_jiffies_min instead of open-coding the same.
Signed-off-by: Imre Deak imre.deak@intel.com
sound/pci/oxygen/oxygen_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/oxygen/oxygen_io.c b/sound/pci/oxygen/oxygen_io.c index 521eae4..132ecbe 100644 --- a/sound/pci/oxygen/oxygen_io.c +++ b/sound/pci/oxygen/oxygen_io.c @@ -108,7 +108,7 @@ static int oxygen_ac97_wait(struct oxygen *chip, unsigned int mask) wait_event_timeout(chip->ac97_waitqueue, ({ status |= oxygen_read8(chip, OXYGEN_AC97_INTERRUPT_STATUS); status & mask; }),
msecs_to_jiffies(1) + 1);
msecs_to_jiffies_min(1));
This would change the behavior, I guess.
Not to my understanding, the new macro should end up doing the same thing.
Ah, OK, I just saw your patch 01/11.
But then msecs_to_jiffies_min() sounds confusing, if it plus one implicitly.
Takashi
(Though, I'm not sure whether the original code was intentional.)
Well, I only assumed that.. But using wait_event_timeout() without the +1 would make little sense to me. In that case we may not wait at all for the condition to become true, if we are close to the next scheduling clock tick.
And, isn't msecs_to_jiffies_min(1) identical with msecs_to_jiffies(1)?
No, it should be one more in value.
Takashi Iwai wrote:
Imre Deak wrote:
Use msecs_to_jiffies_min instead of open-coding the same.
+++ b/sound/pci/oxygen/oxygen_io.c @@ -108,7 +108,7 @@ static int oxygen_ac97_wait(struct oxygen *chip, unsigned int mask) wait_event_timeout(chip->ac97_waitqueue, ({ status |= oxygen_read8(chip, OXYGEN_AC97_INTERRUPT_STATUS); status & mask; }),
msecs_to_jiffies(1) + 1);
msecs_to_jiffies_min(1));
This would change the behavior, I guess.
No, that's exactly how msecs_to_jiffies_min would be implemented.
(Though, I'm not sure whether the original code was intentional.)
When I wrote this, I was not able to prove that msecs_to_jiffies always rounds up. (And I guess it doesn't; otherwise the _min variant would not be needed.)
Regards, Clemens
participants (3)
-
Clemens Ladisch
-
Imre Deak
-
Takashi Iwai