linux-kernel - Re: [PATCH] ALSA: hda: Only write ICH6_PCIREG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5h4o7bm13w.wl%tiwai@suse.de>
Date:	Thu, 10 Mar 2011 16:44:35 +0100
From:	Takashi Iwai <tiwai@...e.de>
To:	Adam Lackorzynski <adam@...inf.tu-dresden.de>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ALSA: hda: Only write ICH6_PCIREG_TCSEL on Intel systems.

At Thu, 10 Mar 2011 16:22:12 +0100,
Adam Lackorzynski wrote:
> 
> Hi,
> 
> azx_init_pci() always writes PCI config register ICH6_PCIREG_TCSEL
> although this looks to be only defined on Intel systems and has a
> different meaning on AMD systems. On AMD systems the PCI interrupt pin
> control register is modified instead. Since the register has 'ICH' in
> its name I pulled the call to update_pci_byte() into the switch block to
> only happen on Intel variants. I'm not sure on the other variants.

So, you did check only AMD systems, right?

Then it's safer to exclude this for AMD but apply to others for the
time being, since we haven't got any relevant bug reports by this.
Could you re-submit with that change?

The fact that it has ICH prefix doesn't mean much -- it's just like
the driver is called snd-hda-intel.  The first hardware, the first
set.  (At that time, we weren't sure at that time that HD-audio spec
would be followed widely by most vendors.)


thanks,

Takashi

> 
> Signed-off-by: Adam Lackorzynski <adam@...inf.tu-dresden.de>
> ---
>  sound/pci/hda/hda_intel.c |   15 ++++++++-------
>  1 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index fcedad9..41debbd 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -1049,13 +1049,6 @@ static void azx_init_pci(struct azx *chip)
>  {
>  	unsigned short snoop;
>  
> -	/* Clear bits 0-2 of PCI register TCSEL (at offset 0x44)
> -	 * TCSEL == Traffic Class Select Register, which sets PCI express QOS
> -	 * Ensuring these bits are 0 clears playback static on some HD Audio
> -	 * codecs
> -	 */
> -	update_pci_byte(chip->pci, ICH6_PCIREG_TCSEL, 0x07, 0);
> -
>  	switch (chip->driver_type) {
>  	case AZX_DRIVER_ATI:
>  		/* For ATI SB450 azalia HD audio, we need to enable snoop */
> @@ -1087,6 +1080,14 @@ static void azx_init_pci(struct azx *chip)
>  				(snoop & INTEL_SCH_HDA_DEVC_NOSNOOP)
>  				? "Failed" : "OK");
>  		}
> +		/* fall through */
> +	case AZX_DRIVER_ICH:
> +		/* Clear bits 0-2 of PCI register TCSEL (at offset 0x44)
> +		 * TCSEL == Traffic Class Select Register, which sets PCI
> +		 * express QOS Ensuring these bits are 0 clears playback
> +		 * static on some HD Audio codecs
> +		 */
> +		update_pci_byte(chip->pci, ICH6_PCIREG_TCSEL, 0x07, 0);
>  		break;
>  
>          }
> -- 
> 1.7.2.3
> 
> 
> Adam
> -- 
> Adam                 adam@...inf.tu-dresden.de
>   Lackorzynski         http://os.inf.tu-dresden.de/~adam/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/