Re: [PATCH] docs: sound: kernel-api: writing-an-alsa-driver.rst: polishing

[Takashi Iwai <tiwai@suse.de>]

On Thu, 06 Apr 2023 09:33:41 +0200,
Oswald Buddenhagen wrote:
> 
> On Thu, Apr 06, 2023 at 08:55:27AM +0200, Takashi Iwai wrote:
> > On Wed, 05 Apr 2023 22:12:20 +0200,
> > Oswald Buddenhagen wrote:
> >> @@ -2262,7 +2156,7 @@ Typical code would be like the following.
> >>                        /* over the period boundary? */
> >>                        if (chip->size >= runtime->period_size) {
> >>                                /* reset the accumulator */
> >> -                              chip->size %= runtime->period_size;
> >> +                              chip->size -= runtime->period_size;
> >>                                /* call updater */
> >>                                spin_unlock(&chip->lock);
> >>                                snd_pcm_period_elapsed(substream);
> > 
> > Hmm, this kind of change shouldn't be sneaked in.
> > That's more than the typo fixes etc,
> > 
> true, the patch grew too big and i missed this hunk.
> (i only kept it as one patch, because some pieces overlap and i didn't
> want to add churn.)

Maybe the changes could have been split from the first place, the
mechanical changes to drop empty lines before "::", typo fixes, and
text improvements, etc.  But I guess it's too late and I'm fine to
apply this whole change as a single patch with the correction.


> > and even worse, it's a wrong replacement.
> > 
> hmm, yeah, if the timer ints are configured to occur too rarely, this
> wouldn't do the right thing.
> but then, why would they be? that would basically defeat the point of
> using many periods in the first place. should i instead change the
> text to emphasize that the ints should occur at least once per period?
> (i've actually pondered the timer option in the context of the emu10k1
> driver as well, and concluded that there should be two timer ints per
> period, so snd_pcm_period_elapsed() is reliably called in the first
> half of the next period, which is critical when only two periods are
> configured.)

No, the point is that, if an irq handler misses the multiple period
updates, it has to handle the situation in a single shot, and the
offset gets corrected to the value within the period size; using "-="
instead of "%=" may leave the size over the period size in such a
case, and this may return an invalid PCM pointer value in the end.

The text about the irq handling could be improved, of course.


thanks,

Takashi