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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
