On 24-07-08 07:26, Krzysztof Helt wrote:
diff --git a/sound/isa/ad1848/ad1848.c b/sound/isa/ad1848/ad1848.c index 5f5271e..3500548 100644 --- a/sound/isa/ad1848/ad1848.c +++ b/sound/isa/ad1848/ad1848.c @@ -70,15 +70,15 @@ static int __devinit snd_ad1848_match(struct device *dev, unsigned int n) return 0;
if (port[n] == SNDRV_AUTO_PORT) {
snd_printk(KERN_ERR "%s: please specify port\n", dev->bus_id);
snd_printk(KERN_ERR "%s: please specify port\n", dev_name(dev)); return 0; }The comment in dev_name() makes me wonder a bit but I guess we'll deal with it then if there's anything to deal with.
This is not my change. It is probably diff between some -mm kernel version and alsa-driver. It should not be there as the ad1848.c is going to be deleted.
I see. It's definitely from the patch you sent though:
http://mailman.alsa-project.org/pipermail/alsa-devel/2008-July/008978.html
-CS4236_DOUBLE("Master Digital Playback Switch", 0, CS4236_LEFT_MASTER, CS4236_RIGHT_MASTER, 7, 7, 1, 1), -CS4236_DOUBLE("Master Digital Capture Switch", 0, CS4236_DAC_MUTE, CS4236_DAC_MUTE, 7, 6, 1, 1), +CS4236_DOUBLE("Master Digital Playback Switch", 0,
CS4236_LEFT_MASTER, CS4236_RIGHT_MASTER, 7, 7, 1, 1),+CS4236_DOUBLE("Master Digital Capture Switch", 0,
CS4236_DAC_MUTE, CS4236_DAC_MUTE, 7, 6, 1, 1),I can't say I'm personally a fan of these kinds of changes. The point of them would supposedly be to make the code more readable but as far as I am concerned it does the reverse.
I know that Takashi can be an 80-column fundamentalist so I'll not object I guess. I'd personally like these (all) restored to a single line but if he doesn't, so be it.
Exactly. It was done for Takashi.
Yes, he overrides. I'd try to get away with just saying no though. That checkpatch thing desperately needs a clue.
+static void snd_wss_playback_format(struct snd_wss *chip,
[ ... ]
snd_cs4231_out(chip, CS4231_PLAYBK_FORMAT, chip->image[CS4231_PLAYBK_FORMAT] = pdfr);
chip->image[CS4231_PLAYBK_FORMAT] = pdfr;snd_wss_out(chip, CS4231_ALT_FEATURE_1,chip->image[CS4231_ALT_FEATURE_1] &= ~0x10);
Oh, missed commenting on this assignment-in-argument due to commenting on the other one...
As long as you're at it... could you split this assignment same as you did above?
Ok. I did it above because otherwise the line was too long ...
Okay. Style-consistency really is fairly important. Otherwise you each time have to "retrain" while reading code. In that sense, even leaving them all in would/can be better than some in, some out although in this case "all out" is quite preffered.
(I only looked at the patch itself though -- if there are many more of these than leaving them in and (perhaps) fixing it in a follow-up might be preferred).
Right-o. That was a 4000+ line patch and I ran out of evening. Rest will have to wait for tomorrow then. If you make changes, could you very specifically indicate those changes so that I might be able to get away with not reading the entire thing again? :-/
Yes.
Thank you :-)
Rene.