| 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
| ||
|
Date: Fri, 12 Jan 2007 15:17:40 +0100
From: Michael Buesch <mb@...sch.de>
To: Larry Finger <Larry.Finger@...inger.net>
Cc: John Linville <linville@...driver.com>, netdev@...r.kernel.org,
Bcm43xx-dev@...ts.berlios.de
Subject: Re: [PATCH] bcm43xx: Fix failure to deliver PCI-E interrupts
On Friday 12 January 2007 05:52, Larry Finger wrote:
> The PCI-E modifications to bcm43xx do not set up the interrupt vector
> correctly.
>
> Signed-off-by: Larry Finger <Larry.Finger@...inger.net>
> ---
>
> John,
>
> This fix should be applied to wireless-2.6 _AND_ pushed upstream to
> 2.6.20-rcX. Without this patch, none of the PCI-E interfaces will work.
>
> Larry
>
>
> Index: linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> +++ linux-2.6/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> @@ -2704,7 +2704,7 @@ static int bcm43xx_probe_cores(struct bc
> sb_id_hi = bcm43xx_read32(bcm, BCM43xx_CIR_SB_ID_HI);
>
> /* extract core_id, core_rev, core_vendor */
> - core_id = (sb_id_hi & 0xFFF0) >> 4;
> + core_id = (sb_id_hi & 0x8FF0) >> 4;
ACK. That one fixes a bug.
> core_rev = (sb_id_hi & 0xF);
This is also buggy. Should be something like:
core_rev = ((sb_id_hi & 0xF) | ((sb_id_hi & 0x7000) >> 8));
> core_vendor = (sb_id_hi & 0xFFFF0000) >> 16;
>
> @@ -2717,7 +2717,7 @@ static int bcm43xx_probe_cores(struct bc
> case BCM43xx_COREID_PCIE:
> core = &bcm->core_pci;
> if (core->available) {
> - printk(KERN_WARNING PFX "Multiple PCI cores found.\n");
> + printk(KERN_WARNING PFX "Multiple PCI/PCI-E cores found.\n");
> continue;
> }
> break;
> @@ -2872,11 +2872,14 @@ static int bcm43xx_wireless_core_init(st
> u32 sbimconfiglow;
> u8 limit;
>
> - if (bcm->core_pci.rev <= 5 && bcm->core_pci.id != BCM43xx_COREID_PCIE) {
> + if (bcm->core_pci.rev <= 5 && bcm->core_pci.id == BCM43xx_COREID_PCI) {
That's semantically equal.
> sbimconfiglow = bcm43xx_read32(bcm, BCM43xx_CIR_SBIMCONFIGLOW);
> sbimconfiglow &= ~ BCM43xx_SBIMCONFIGLOW_REQUEST_TOUT_MASK;
> sbimconfiglow &= ~ BCM43xx_SBIMCONFIGLOW_SERVICE_TOUT_MASK;
> - sbimconfiglow |= 0x32;
> + if (bcm->bustype == BCM43xx_BUSTYPE_PCI)
> + sbimconfiglow |= 0x32;
> + else
> + sbimconfiglow |= 0x53;
That used to be there until someone ripped it out (not me). Not sure why.
> bcm43xx_write32(bcm, BCM43xx_CIR_SBIMCONFIGLOW, sbimconfiglow);
> }
>
> @@ -3070,6 +3073,7 @@ static int bcm43xx_setup_backplane_pci_c
> u32 backplane_flag_nr;
> u32 value;
> struct bcm43xx_coreinfo *old_core;
> + struct bcm43xx_coreinfo *pci_core = &bcm->core_pci ;
> int err = 0;
>
> value = bcm43xx_read32(bcm, BCM43xx_CIR_SBTPSFLAG);
> @@ -3080,26 +3084,22 @@ static int bcm43xx_setup_backplane_pci_c
> if (err)
> goto out;
>
> - if (bcm->current_core->rev < 6 ||
> - bcm->current_core->id == BCM43xx_COREID_PCI) {
> - value = bcm43xx_read32(bcm, BCM43xx_CIR_SBINTVEC);
> - value |= (1 << backplane_flag_nr);
> - bcm43xx_write32(bcm, BCM43xx_CIR_SBINTVEC, value);
> - } else {
> + if ((pci_core->rev >= 6) || (pci_core->id == BCM43xx_COREID_PCIE)) {
> err = bcm43xx_pci_read_config32(bcm, BCM43xx_PCICFG_ICR, &value);
> - if (err) {
> - printk(KERN_ERR PFX "Error: ICR setup failure!\n");
> + if (err)
> goto out_switch_back;
> - }
> value |= core_mask << 8;
> err = bcm43xx_pci_write_config32(bcm, BCM43xx_PCICFG_ICR, value);
> - if (err) {
> - printk(KERN_ERR PFX "Error: ICR setup failure!\n");
> + if (err)
> goto out_switch_back;
> - }
> + } else {
> + err = -EINVAL;
> + value = bcm43xx_read32(bcm, BCM43xx_CIR_SBINTVEC);
> + value |= 1 << backplane_flag_nr;
> + bcm43xx_write32(bcm, BCM43xx_CIR_SBINTVEC, value);
> }
The above doesn't change semantics. Or am I simply not seeing it? :)
Seems that it just shuffles the code.
>
> - if (bcm->current_core->id == BCM43xx_COREID_PCI) {
> + if (pci_core->id == BCM43xx_COREID_PCI) {
That's semantically equal.
> value = bcm43xx_read32(bcm, BCM43xx_PCICORE_SBTOPCI2);
> value |= BCM43xx_SBTOPCI2_PREFETCH | BCM43xx_SBTOPCI2_BURST;
> bcm43xx_write32(bcm, BCM43xx_PCICORE_SBTOPCI2, value);
> @@ -3118,21 +3118,21 @@ static int bcm43xx_setup_backplane_pci_c
> value |= BCM43xx_SBTOPCI2_MEMREAD_MULTI;
> bcm43xx_write32(bcm, BCM43xx_PCICORE_SBTOPCI2, value);
> }
> - } else {
> - if (bcm->current_core->rev == 0 || bcm->current_core->rev == 1) {
> + } else if (pci_core->id == BCM43xx_COREID_PCIE) {
That's redundant. If it's not a PCI core, it must be a PCIE core.
> + if (pci_core->rev == 0 || pci_core->rev == 1) {
Semantically equal.
> value = bcm43xx_pcie_reg_read(bcm, BCM43xx_PCIE_TLP_WORKAROUND);
> value |= 0x8;
> bcm43xx_pcie_reg_write(bcm, BCM43xx_PCIE_TLP_WORKAROUND,
> value);
> }
> - if (bcm->current_core->rev == 0) {
> + if (pci_core->rev == 0) {
dito
> bcm43xx_pcie_mdio_write(bcm, BCM43xx_MDIO_SERDES_RX,
> BCM43xx_SERDES_RXTIMER, 0x8128);
> bcm43xx_pcie_mdio_write(bcm, BCM43xx_MDIO_SERDES_RX,
> BCM43xx_SERDES_CDR, 0x0100);
> bcm43xx_pcie_mdio_write(bcm, BCM43xx_MDIO_SERDES_RX,
> BCM43xx_SERDES_CDR_BW, 0x1466);
> - } else if (bcm->current_core->rev == 1) {
> + } else if (pci_core->rev == 1) {
dito
> value = bcm43xx_pcie_reg_read(bcm, BCM43xx_PCIE_DLLP_LINKCTL);
> value |= 0x40;
> bcm43xx_pcie_reg_write(bcm, BCM43xx_PCIE_DLLP_LINKCTL,
> @@ -3141,6 +3141,7 @@ static int bcm43xx_setup_backplane_pci_c
> }
> out_switch_back:
> err = bcm43xx_switch_core(bcm, old_core);
> + printk(KERN_ERR PFX "Error: ICR setup failure!\n");
> out:
> return err;
> }
>
> ---
>
>
--
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists