Re: [CFT] alc(4) QAC AR816x/AR817x ethernet controller support

On Fri, Oct 03, 2014 at 09:29:46PM +0200, Dariusz Wierzbicki wrote:
> Dnia 2014-10-02, o godz. 14:07:30
> Yonghyeon PYUN <pyunyh_at_gmail.com> napisał(a):
> 
> > On Wed, Oct 01, 2014 at 10:36:37AM +0900, Yonghyeon PYUN wrote:
> > > On Tue, Sep 30, 2014 at 10:57:41AM +0900, Yonghyeon PYUN wrote:
> > > > Hi,
> > > > I've added support for QAC AR816x/AR817x ethernet controllers.  It
> > > > passed my limited testing and I need more testers.  You can find
> > > > patches from the following URLs.
> > > > 
> > > > http://people.freebsd.org/~yongari/alc/pci.quirk.diff
> > > > and
> > > > http://people.freebsd.org/~yongari/alc/alc.diff.20140930
> > > > 
> > > > pci.qurik.diff is to workaround silicon bug of AR816x. Without it
> > > > MSI/MSIX interrupt wouldn't work.  If you just want to use
> > > > legacy INTx interrupt you don't have to apply it but you have to
> > > > tell alc(4) not to use MSI/MSIX interrupt with tunables(
> > > > hw.alc.msi.disable and hw.alc.msix_disable).
> > > > 
> > > > alc.diff.20140930 will add support for AR8161/AR8162/AR8171/AR8172
> > > > and E2200 controllers.  It supports all hardware features except
> > > > RSS.  If you have any QAC AR816x/AR817x or old AR813x/AR815x
> > > > controllers please test and report how the diff works for you.
> > > > Thanks.
> > > 
> > > http://people.freebsd.org/~yongari/alc/pci.quirk.diff
> > > http://people.freebsd.org/~yongari/alc/alc.diff.20141001
> > > 
> > > Patch updated to address link establishment issue.
> > 
> > http://people.freebsd.org/~yongari/alc/alc.diff.20141002
> > Patch updated again to correct wrong lock assertion.
> 
> Hi !
> 
> Thanks for your work !
> 
> Are your patches only for current ? I tried on 10 stable.
> 

No, it should be applied to stable/10 as well.  I intentionally
didn't include additional diff for MAC statistics which will not
work on stable/10 and stable/9 due to if_inc_counter changes made
in HEAD.

I tried to apply the diff again against stable/10 and it succeeded
with minor fuzz and offset differences.

> 
> My system:
> 
> dw_at_dw:~ % uname -a
> FreeBSD dw 10.1-RC1 FreeBSD 10.1-RC1 #1 r272477M: Fri Oct  3 20:48:05
> CEST 2014     dw_at_dw:/usr/obj/usr/src/sys/DW  amd64
> 

[...]

> I applied that part manually. Compiled and rebooted system.
> 
> 
> dmesg | grep alc :
> 
> alc0: could not disable Rx/Tx MAC(0x4000cb20)!
> alc0: reset timeout(0x4000cb20)!
> alc0: could not disable Rx/Tx MAC(0x4000cb20)!
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm more worried about MAC reset and master reset timeout shown
below.  The MAC reset timeout makes me wonder how this can happen
since driver just checks bit 0 and bit 1, the low nibble of the
register value can't be 0.

> alc0: link state changed to UP
> alc0: could not disable Rx/Tx MAC(0x4000cb20)!
> alc0: <Qualcomm Atheros AR8161 Gigabit Ethernet> port 0xd000-0xd07f mem
> 0xf7200000-0xf723ffff irq 18 at device 0.0 on pci3
> alc0: reset timeout(0x4000cd00)!
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I think this also can't happen since driver checks bit[0-3], the
low byte should be non-zero when the timeout triggers.

> alc0: 11776 Tx FIFO, 12032 Rx FIFO
> miibus0: <MII bus> on alc0
> alc0: Ethernet address: 74:d4:35:91:32:04

[...]

> 
> If you need other data or more testing, let me know.
> 

Do you have any local changes in alc(4)?  As I said, the diff
could be applied to stable/10 without any manual modification.

Thanks for testing!