[alsa-devel] [PATCH v2 2/3] ALSA: seq: Avoid open-code for getting timer resolution

[Takashi Iwai]

On Thu, 31 May 2018 19:18:53 +0200,
Ben Hutchings wrote:
> 
> On Thu, 2018-05-17 at 10:49 +0200, Takashi Iwai wrote:
> > Instead of open-coding for getting the timer resolution, use the
> > standard snd_timer_resolution() helper.
> > 
> > The original code falls back to the callback function when the
> > resolution is zero, but it must be always so when the callback
> > function is defined.  So this should be no functional change.
> 
> Maybe it *should* be so, but I can see three drivers where it isn't:
> 
> sound/isa/ad1816a/ad1816a_lib.c
> sound/isa/wss/wss_lib.c
> sound/sparc/cs4231.c
> 
> For ad1816a_lib.c, the c_resolution implementation always returns the
> same value that's in resolution, so this really doesn't make a
> functional change.
> 
> However, for the other two, the c_resolution implementation can return
> several different values.

A good point.  This implies that the patch fixes a long-standing issue
:)

The ALSA timer API is very rarely used, however, maybe the only real
application is the alsa-lib dmix/dsnoop.  And the h/w resolution
doesn't matter in that case (as it's deploying only the PCM slave
timer), I don't think we'd hit any issues, practically seen.


thanks,

Takashi

> 
> Ben.
> 
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> >  sound/core/seq/seq_timer.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c
> > index 23167578231f..f587d0e27476 100644
> > --- a/sound/core/seq/seq_timer.c
> > +++ b/sound/core/seq/seq_timer.c
> > @@ -371,9 +371,7 @@ static int initialize_timer(struct snd_seq_timer *tmr)
> >  
> >  	tmr->ticks = 1;
> >  	if (!(t->hw.flags & SNDRV_TIMER_HW_SLAVE)) {
> > -		unsigned long r = t->hw.resolution;
> > -		if (! r && t->hw.c_resolution)
> > -			r = t->hw.c_resolution(t);
> > +		unsigned long r = snd_timer_resolution(tmr->timeri);
> >  		if (r) {
> >  			tmr->ticks = (unsigned int)(1000000000uL / (r * freq));
> >  			if (! tmr->ticks)
> -- 
> Ben Hutchings, Software Developer                         Codethink Ltd
> https://www.codethink.co.uk/                 Dale House, 35 Dale Street
>                                      Manchester, M1 2HF, United Kingdom
>