Messages in this thread | | | Date | Wed, 28 Jan 2009 13:06:20 +0100 | From | Takashi Iwai <> | Subject | Re: [PATCH] ALSA: Don't cold reset AC97 codecs in some ICH chipsets |
| |
At Wed, 28 Jan 2009 08:02:36 -0200, Thadeu Lima de Souza Cascardo wrote: > > Check in a quirk list if it should do cold reset when AC97 power saving > is enabled. Some devices do not resume properly when cold reset, > although power saving works OK. > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
Thanks for the patch. We can go better further with this patch now :)
The logic is basically fine, but just a few comments...
> diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c > index 19d3391..69b3ed8 100644 > --- a/sound/pci/intel8x0.c > +++ b/sound/pci/intel8x0.c ... > +static int snd_intel8x0_ich_chip_cold_reset(struct intel8x0 *chip) > +{ > + unsigned int cnt; > /* ACLink on, 2 channels */ > + > + if (snd_pci_quirk_lookup(chip->pci, ich_chip_reset_mode)) > + return -EIO;
I feel it isn't intuitive to return -EIO here. It'd be more straightforward to call snd_intel8x0_ich_chip_reset() from here?
if (snd_pci_quirk_lookup(chip->pci, ich_chip_reset_mode)) return snd_intl8x0_ich_chip_reset(chip);
> + if (snd_intel8x0_ich_chip_cold_reset(chip) < 0) > + if ((err = snd_intel8x0_ich_chip_reset(chip)) < 0) > + return err;
Please make the change checkpatch-clean.
Could you fix and repost?
thanks,
Takashi
| |