search

openbmc / linux

OpenBMC Linux kernel source tree
Other
49 stars 132 forks source link

ftgmac100 spams PHYSTS_CHG #106

Closed shenki closed 7 years ago

shenki commented 8 years ago 
# ifconfig eth0 up
[   15.270000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.300000] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[   15.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.330000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.330000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.330000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   15.360000] random: fast init done
# [   15.440000] random: crng init done

# [   17.320000] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[   20.280000] net_ratelimit: 640497 callbacks suppressed
[   20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.330000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[   20.330000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
shenki commented 8 years ago

This is with the latest patchset as in dev-4.7 on the ast2500-evb (direct attached PHY, no NSCI).

@gwshan, this is what I was speaking about today.

To if I disable the interrupt (as we do for NSCI), the ast2500 with PHY still works.

shenki commented 8 years ago

The patches as I have tested them are here:

https://github.com/shenki/linux/tree/dev-4.8-ftgmac100

shenki commented 8 years ago

Some things to check out before we send this upstream:

MAC Control Register (MACCR, offset 0x50)

Bit Name Details
11 PHY link level "sensitiver" selection 0 high level sensitive, 1 low level sensitive
6 PHY link status detection 0 level sensitive, 1 Rising and falling edge trigger
gwshan commented 8 years ago

@shenki could you please dump registers (0x4, 0x50) when seeing the PHY link status interrupt? I did some experiment on AST2400 this morning. IER[9] should be set in order to see this interrupt. Besides, MACCR[11/6] should be set to 0b00. With all other values set to MACCR[11/6], I cannot see the bogus interrupt. It seems this PIN (GPIO-A0) is always in high level (voltage).

shenki commented 8 years ago 
# devmem 0x1e660050
0x00021F0F
# devmem 0x1e660004
0x00000000
$ bitfield MAC50 0x00021F0F
decoding as MAC Control Register (MACCR)
0x00021f0f [139023]
            SW_RST: Software reset: 0x0
             SPEED_100: Speed mode: 0x0 [10 Mbps]
                    DISCARD_CRCERR: 0x0
                 RX_BROADPACKET_EN: 0x1
                    RX_MULTIPKT_EN: 0x0
                          RX_HT_EN: 0x0
                        RX_ALLADDR: 0x0
                          JUMBO_LF: 0x0
                           RX_RUNT: 0x1
PHY link level senstiver selection: 0x1 [Low-level senstive]
                           CRC_APD: 0x1
                         GMAC_MODE: 0x1 [MAC is in 1000 Mbps mode]
                           FULLDUP: 0x1
                    ENRX_IN_HALFTX: 0x0
         PHY link status detection: 0x0 [Level sensitive]
                          HPTXR_EN: 0x0
                       REMOVE_VLAN: 0x0
                          RXMAC_EN: 0x1
                          TXMAC_EN: 0x1
                          RXDMA_EN: 0x1
                          TXDMA_EN: 0x1
bits set: 17, 12, 11, 10,  9,  8,  3,  2,  1,  0 (10)
shenki commented 8 years ago

@gwshan I cleared bit 11 and the interrupts went away. However, they came back after the cable was unplugged. I suspect this is due to the driver calling ftgmac100_adjust_link -> ftgmac100_start_hw which writes MACCR_ENABLE_ALL to MACCR.

I rebuilt my driver with bit 6 set (Phy link status detection: edge sensitive), and bit 11 cleared. The driver works as expected, including unplugging and replugging the cable.

I then rebuilt my driver with bit 6 cleared (Phy link status detection: level sensitive), and bit 11 set (low-level sensitive). The driver works as expected, including unplugging and replugging the cable.

Which approach do we prefer?

gwshan commented 8 years ago

@shenki, we prefer the later one: bit 6 cleared (Phy link status detection: level sensitive), and bit 11 set (low-level sensitive)

shenki commented 8 years ago 
15:01 < gavin> GPIO-A0 is the pin for this signal (PHYSTS_CHG)
15:02 < gavin> I think it's always in high level.
15:02 < gavin> This PIN is valid when we have a PHY attached.
15:02 < gavin> (It's invalid when we have NCSI enabled)
15:02 < gavin> However, this interrupt (PIN) isn't used even when PHY is attached.
15:03 < gavin> PHY driver works in polling mode.
15:03 < gavin> So we needn't this interrupt, but still nice to give correct configuration.
15:04 < gavin> MACCR[6/11] = 0b01    Is the correct configuration
15:04 < gavin> it means: High-level valid
15:04 < gavin> joel: we needn't a dt property, I think.
15:05 < joel> gavin: ok. but in that configuration (MACCR[6/11] = 0b01), my board continually generates interrupts
15:06 < gavin> joel: I think we just need: MACCR[6]=0, MACCR[11]=1
15:06 < joel> that's what we have: https://github.com/openbmc/linux/issues/106#issuecomment-248192507
15:06 < joel> and that is the broken configuration
15:06 < joel> at least on the ast2500evb
15:07 < gavin> joel: yeah, it's not broken on ast2400+ncsi
15:08 < joel> ok
15:08 < gavin> joel: I think MACCR[6]=0, MACCR[11]=0 worked on ast2500+PHY?
15:08 < joel> yes
15:08 < gavin> joel: ok, we need give different setting according to dt (2400/2500/ncsi)
15:09 < gavin> MACCR[6] = 0, always.
15:09 < joel> +1
15:09 < gavin> ncsi enabled: MACCR[11] = 1
15:10 < gavin> otherwise: MACCR[11]=0
15:10 < gavin> joel: is it elaborate? :)
15:10 < joel> nah, makes sense i think
15:11 < joel> once concern is that the phy interrupt depends on the phy
15:11 < joel> so if i had an ast2500 chip with a different phy (say, realtek instead of broadcom), it might use the other polarity
15:15 < gavin> joel: this interrupt isn't used so far, disregarding the PHY chip/type
15:15 < gavin> in ftgmac100_setup_mdio()
15:15 < gavin> priv->mii_bus->irq[i] = PHY_POLL;
shenki commented 8 years ago

Sent upstream: http://marc.info/?l=linux-netdev&m=147435308223085&w=2

shenki commented 7 years ago

These patches have been merged into net-next which will become 4.9