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

Dnia 2014-10-06, o godz. 11:32:13
Yonghyeon PYUN <pyunyh_at_gmail.com> napisał(a):

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

Thanks for your answer.
I tried again and I succeeded. I used --ignore-whitespace option with
patch command and the alc.diff applied successfully.

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

You were right. I had a minor local change in if_alc.c file.
Now everything seems to work:

dw_at_dw:~ % dmesg | grep alc
Preloaded elf obj module "/boot/kernel/if_alc.ko" at 0xffffffff81bd6b38.
alc0: <Atheros AR8161 PCIe Gigabit Ethernet> port 0xd000-0xd07f mem
0xf7200000-0xf723ffff irq 18 at device 0.0 on pci3 alc0: PCI device
revision : 0x0010 alc0: Chip id/revision : 0xc002
alc0: AR816x revision : 0x2
alc0: 11776 Tx FIFO, 12032 Rx FIFO
alc0: Read request size : 512 bytes.
alc0: TLP payload size : 128 bytes.
alc0: MSIX count : 16
alc0: MSI count : 16
alc0: attempting to allocate 1 MSI-X vectors (16 supported)
alc0: using IRQ 268 for MSI-X
alc0: Using 1 MSIX message(s).
miibus0: <MII bus> on alc0
alc0: bpf attached
alc0: Ethernet address: 74:d4:35:91:32:04
alc0: link state changed to UP
alc0: <Atheros AR8161 PCIe Gigabit Ethernet> port 0xd000-0xd07f mem
0xf7200000-0xf723ffff irq 18 at device 0.0 on pci3 alc0: 11776 Tx FIFO,
12032 Rx FIFO alc0: Using 1 MSIX message(s).
miibus0: <MII bus> on alc0
alc0: Ethernet address: 74:d4:35:91:32:04
alc0: link state changed to UP

Thanks again for the patches and for your work !

Darek