Re: [PATCH] Gallant SC-6000 driver (3rd version)

[Rene Herman <rene.herman@gmail.com>]

On 09/05/2007 06:42 AM, Krzysztof Helt wrote:

>> But also -- please no inlines. There's no point to them. Kernel policy is 
>> basically "no inlines other than for things that would definitely be macros 
>> if we weren't fond of the typesafety inline functions give". GCC will figure 
>> things out on its own...
>>
> 
> These things were actually macros in the 2.4 driver.

Okay, but while I myself don't terribly mind the inline keyword or anything, 
the kernel as a whole does -- the general meme is "inline is always wrong, 
unless...", where "unless" includes exceedingly trivial and on a fastpath. 
Opinions as to the "exceedingly" may vary, but this is one-time setup code, 
so dropping the inlines is suggested -- otherwise you'd just saddle up the 
next bunch of kernel janitors with more instances of inline removals ;-)

>> So what happens if now someone starts poking at port[dev]? You probably want 
>> to keep these requested no?
>>
> 
> Sgalaxy dejavu.

Absolutely. The existing sgalaxy driver blows donkeys. In the replacement 
driver I have it's kept reserved (in fact, it keeps the entire SB part live, 
but I may want to revisit that -- I believe some testing is showing that no, 
the SB and WSS part really cannot be simultaneously active on any model; a 
forum post somewhere said they could, so I've been trying that)

>>> +	if (mpu_port[dev] != SNDRV_AUTO_PORT &&
>>> +	    (mpu_port[dev] & ~0x30l) != 0x300) {
>> Eh, what?
>>
> 
> Checking if mpu_port[dev] is one 0x3X0 where X is from 0 to 3

That ain't working. -0x301 = 0xcff and ((foo & 0xcff) != 0x300) for all foo. 
I'd suggest to simply introduce another one of those switch() helpers for 
this -- it's again one-time code and it only pays to optimise for 
readability -- open-source code is read many more times that it's written.

>> In the above, IRQ 5 also seems to be allowed. This is all rather massive 
>> sgalaxy deja-vu...
>>
> 
> Do you think that this card can be handled by sgalaxy driver (with
> minimal changes to sgalaxy)?

sgalaxy itself is on its way out as far as I'm concerned, but yes, if you 
can give me a bit to look at whether or not the sound galaxy driver can be 
handling this card, great. Generally, I don't have anything against more 
top-level drivers (as they're mostly just wrappers around lib functionality 
anyway) though, so if it gets clumsy and/or ugly the seperate driver is fine 
by me at least.

I'll step up getting snd-galaxy into shape...

Rene.