Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Rene Herman <rene.herman@keyaccess.nl>
To: Krzysztof Helt <krzysztof.h1@poczta.fm>
Cc: Takashi Iwai <tiwai@suse.de>, Alsa-devel <alsa-devel@alsa-project.org>
Subject: Re: [PATCH 10/10] wss_lib: use wss detection code instead of ad1848 one
Date: Fri, 25 Jul 2008 11:59:16 +0200	[thread overview]
Message-ID: <4889A3F4.5050903@keyaccess.nl> (raw)
In-Reply-To: <20080724234114.a8f6bcf3.krzysztof.h1@poczta.fm>

On 24-07-08 23:41, Krzysztof Helt wrote:

> On Thu, 24 Jul 2008 21:30:28 +0200
> Rene Herman <rene.herman@keyaccess.nl> wrote:

>>>> *** 0005-wss_lib-use-wss-constants-instead-of-ad1848-ones.patch
>>>>
>>>>> --- a/sound/isa/ad1848/ad1848_lib.c
>>>>> +++ b/sound/isa/ad1848/ad1848_lib.c
>>>>> @@ -283,14 +283,12 @@ static int snd_ad1848_trigger(struct snd_wss *chip, unsigned char what,
>>>>>                         return 0;
>>>>>                 }
>>>>>                 snd_ad1848_out(chip, AD1848_IFACE_CTRL, chip->image[AD1848_IFACE_CTRL] |= what);
>>>>> -               chip->mode |= AD1848_MODE_RUNNING;
>>>> Is this now done in generic code? Not necessary anymore? Was no comment 
>>>> in the changelog.
>>> The MODE_RUNNING is not used at all in the cs4231 code. I wonder what the purpose of it.
>> It was used by the AD1848 code though; snd_ad1848_trigger() set/reset it 
>> on start/stop and it was then tested by snd_ad1848_interrupt() to decide 
>> whether or not to call snd_pcm_period_elapsed().
> 
> It is not used by the cs4231. The only difference is that ad1848 does not 
> call the snd_pcm_period_elapsed() after calling the snd_ad1848_open()
> but before calling the snd_ad1848_trigger() (and similar restriction
> after the snd_ad1848_trigger() and snd_ad1848_close().
> 
> The cs4231 does not use such restriction. I decided it does not really matter.
> The interrupts are not enabled before and after the snd_ad1848_trigger().
> If the cs4231 driver needed this it would be already causing problems.

It seems we are talking at cross purposes. I'm not talking about cs4231. 
I see this MODE_RUNNING thing disappear from ad1848_lib and, unless I've 
missed it, not resurface in wss_lib -- that library that after these 
patches is used to drive AD1848 chips that used to be driven by ad1848_lib.

The MODE_RUNNING looks as something someone will have once added upon 
seeing spurious IRQs and as such, testing that it isn't needed on 
chip/card A, B and C doesn't tell us much. The problem may have been 
observed on type D, E or F and/or under condition foo or bar.

Looking at snd_wss_interrupt() I dom't seem to be seeing a similar guard 
after these patches.

If it's not needed, all for slashing it but please be convincing. Note 
that by now you should know a lot more about the innards of this code 
than I do, so please be as verbose as needed.

Rene.

  reply	other threads:[~2008-07-25  9:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-18 19:51 [PATCH 10/10] wss_lib: use wss detection code instead of ad1848 one Krzysztof Helt
2008-07-20 16:09 ` Rene Herman
2008-07-23 21:28   ` Rene Herman
2008-07-24  5:26     ` Krzysztof Helt
2008-07-24  9:31       ` Rene Herman
2008-07-28 15:37         ` Takashi Iwai
2008-07-28 18:39           ` Rene Herman
2008-07-29 13:02             ` Takashi Iwai
2008-07-29 14:15               ` Rene Herman
2008-07-29 14:31                 ` Takashi Iwai
2008-07-29 14:37                   ` Rene Herman
2008-07-29 18:53                   ` Krzysztof Helt
2008-07-29 19:00                     ` Rene Herman
2008-07-24 17:19       ` Rene Herman
2008-07-24 18:47         ` Krzysztof Helt
2008-07-24 19:30           ` Rene Herman
2008-07-24 20:05             ` Rene Herman
2008-07-24 21:41             ` Krzysztof Helt
2008-07-25  9:59               ` Rene Herman [this message]
2008-08-04  1:47             ` Rene Herman
2008-08-04  4:31               ` Krzysztof Helt
  -- strict thread matches above, loose matches on Subject: below --
2008-07-25 13:39 krzysztof.h1

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4889A3F4.5050903@keyaccess.nl \
    --to=rene.herman@keyaccess.nl \
    --cc=alsa-devel@alsa-project.org \
    --cc=krzysztof.h1@poczta.fm \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox