netdev - Re: [PATCH v2 1/1] alx: add a simple AR816x/AR817x device driver

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] [day] [month] [year] [list]

Date:	Tue, 25 Jun 2013 20:24:44 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Johannes Stezenbach <js@...21.net>
Cc:	Ben Hutchings <ben@...adent.org.uk>, netdev@...r.kernel.org,
	mcgrof@...not-panic.com, kvalo@...rom.com, adrian.chadd@...il.com
Subject: Re: [PATCH v2 1/1] alx: add a simple AR816x/AR817x device driver

On Sat, 2013-06-22 at 12:17 +0200, Johannes Stezenbach wrote:
> On Thu, Jun 20, 2013 at 11:40:38PM +0200, Johannes Berg wrote:
> > 
> > On Tue, 2013-06-18 at 02:18 +0100, Ben Hutchings wrote:
> 
> > > > +	/* Workaround for PCI problem when BIOS sets MMRBC incorrectly. */
> > > > +	pci_read_config_word(hw->pdev, PCI_COMMAND, &val16);
> > > > +	if (!(val16 & ALX_PCI_CMD) || (val16 & PCI_COMMAND_INTX_DISABLE)) {
> > > > +		val16 = (val16 | ALX_PCI_CMD) & ~PCI_COMMAND_INTX_DISABLE;
> > > > +		pci_write_config_word(hw->pdev, PCI_COMMAND, val16);
> > > > +	}
> > > [...]
> > > 
> > > I don't understand what this is trying to work around but it looks
> > > extremely dodgy.  The driver already appears to be doing
> > > pci_{save,restore}_state() in suspend/resume which is the right thing to
> > > do.
> > 
> > I have no idea, but ~PCI_COMMAND_INTX_DISABLE seems really just like
> > what we do with PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG... maybe this was
> > some sort of workaround for that.
> > 
> > Johannes, can you try to remove this chunk of code and see if your
> > device still works? If so I'd just kill it.
> 
> I removed it and did a full power cycle, alx still works.
> Also tried suspend-to-RAM and alx still works.  However,
> since the comment for this code is about BIOS bugs it might
> be that it is required on some other machine?  Not sure
> if it is wise to remove it...

I dunno ... it seems to me we should just try, absent other information?
It really does look a lot like what PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG
does ...

> Another thing: When testing suspend-to-RAM my machine immediately
> woke up.  ethtool said wol is enabled by default:
>         Supports Wake-on: pg
> 	Wake-on: pg
> but even after "ethtool -s eth0 wol d" it still woke up.

Hmm, yeah, I can reproduce that :-/ I'll try to fix it. Or maybe just
kill WoWLAN code ...

johannes

--
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