[alsa-devel] [PATCH 10/10] wss_lib: use wss detection code instead of ad1848 one

[krzysztof.h1 at poczta.fm]

Rene Herman napisał(a):
> >>>> 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.
> 


I cannot be convincing as I am not convinced myself. I understand your reasoning.
My situation was that I saw the guard in the ad1848 which was missing in the cs4231
while they should look like twins (in the area when MODE_RUNNING existed). 
I had two choices:

1. Add MODE_RUNNING to the cs4231
2. Remove MODE_RUNNING

The first approach is a bloat to me as I tested on ad1848 and it was not needed.
I don't know the conditions you called the "condition foo or bar" and I don't know 
if it ever exists. I have checked git history on the file, but this one was added
earlier. Maybe there was a kind of approach in the old days then dropped/replaced
later (like mutex_open which is not needed now).
The second approach is more dangerous but with enough testing should be justified
and provides us with cleaner code.

Regards,
Krzysztof

----------------------------------------------------------------------
Te newsy nakreca Cie na caly dzien!
Sprawdz >>> http://link.interia.pl/f1e94