| Message ID | 20210311201813.3804249-3-robert.hancock@calian.com |
|---|---|
| State | New |
| Headers | show |
| Series | macb SGMII fixed-link fixes | expand |
On Thu, Mar 11, 2021 at 02:18:13PM -0600, Robert Hancock wrote: > When using a fixed-link configuration in SGMII mode, it's not really > sensible to have auto-negotiation enabled since the link settings are > fixed by definition. In other configurations, such as an SGMII > connection to a PHY, it should generally be enabled. So how do you tell the PCS it should be doing 10Mbps over the SGMII link? I'm assuming it is the PCS which does the bit replication, not the MAC? I'm surprised you are even using SGMII with a fixed link. 1000BaseX is the norm, and then you don't need to worry about the speed. Andrew
On Sat, 2021-03-13 at 02:45 +0100, Andrew Lunn wrote: > On Thu, Mar 11, 2021 at 02:18:13PM -0600, Robert Hancock wrote: > > When using a fixed-link configuration in SGMII mode, it's not really > > sensible to have auto-negotiation enabled since the link settings are > > fixed by definition. In other configurations, such as an SGMII > > connection to a PHY, it should generally be enabled. > > So how do you tell the PCS it should be doing 10Mbps over the SGMII > link? I'm assuming it is the PCS which does the bit replication, not > the MAC? I'm not sure if this is the same for all devices using this Cadence IP, but the register documentation I have for the Xilinx UltraScale+ MPSoC we are using indicates this PCS is only capable of 1000 Mbps speeds: https://www.xilinx.com/html_docs/registers/ug1087/gem___pcs_control.html So it doesn't actually seem applicable in this case. > > I'm surprised you are even using SGMII with a fixed link. 1000BaseX is > the norm, and then you don't need to worry about the speed. > That would be a bit simpler, yes - but it seems like this hardware is set up more for SGMII mode - it's not entirely clear to me that 1000BaseX is supported in the hardware, and it's not currently supported in the driver that I can see. -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
On Sun, Mar 14, 2021 at 11:22:03PM +0000, Robert Hancock wrote: > On Sat, 2021-03-13 at 02:45 +0100, Andrew Lunn wrote: > > On Thu, Mar 11, 2021 at 02:18:13PM -0600, Robert Hancock wrote: > > > When using a fixed-link configuration in SGMII mode, it's not really > > > sensible to have auto-negotiation enabled since the link settings are > > > fixed by definition. In other configurations, such as an SGMII > > > connection to a PHY, it should generally be enabled. > > > > So how do you tell the PCS it should be doing 10Mbps over the SGMII > > link? I'm assuming it is the PCS which does the bit replication, not > > the MAC? > > I'm not sure if this is the same for all devices using this Cadence IP, but the > register documentation I have for the Xilinx UltraScale+ MPSoC we are using > indicates this PCS is only capable of 1000 Mbps speeds: > > https://www.xilinx.com/html_docs/registers/ug1087/gem___pcs_control.html > > So it doesn't actually seem applicable in this case. > > > > > I'm surprised you are even using SGMII with a fixed link. 1000BaseX is > > the norm, and then you don't need to worry about the speed. > > > > That would be a bit simpler, yes - but it seems like this hardware is set up > more for SGMII mode - it's not entirely clear to me that 1000BaseX is supported > in the hardware, and it's not currently supported in the driver that I can see. This hardware just seems odd. If it was not for the fact the documentation say SGMII all over the place, i would be temped to say it is actually doing 1000BaseX. Assuming the documentation is not totally wrong, your code seems sensible for the hardware. Andrew
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h index d8c68906525a..d8d87213697c 100644 --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h @@ -159,6 +159,16 @@ #define GEM_PEFTN 0x01f4 /* PTP Peer Event Frame Tx Ns */ #define GEM_PEFRSL 0x01f8 /* PTP Peer Event Frame Rx Sec Low */ #define GEM_PEFRN 0x01fc /* PTP Peer Event Frame Rx Ns */ +#define GEM_PCSCNTRL 0x0200 /* PCS Control */ +#define GEM_PCSSTS 0x0204 /* PCS Status */ +#define GEM_PCSPHYTOPID 0x0208 /* PCS PHY Top ID */ +#define GEM_PCSPHYBOTID 0x020c /* PCS PHY Bottom ID */ +#define GEM_PCSANADV 0x0210 /* PCS AN Advertisement */ +#define GEM_PCSANLPBASE 0x0214 /* PCS AN Link Partner Base */ +#define GEM_PCSANEXP 0x0218 /* PCS AN Expansion */ +#define GEM_PCSANNPTX 0x021c /* PCS AN Next Page TX */ +#define GEM_PCSANNPLP 0x0220 /* PCS AN Next Page LP */ +#define GEM_PCSANEXTSTS 0x023c /* PCS AN Extended Status */ #define GEM_DCFG1 0x0280 /* Design Config 1 */ #define GEM_DCFG2 0x0284 /* Design Config 2 */ #define GEM_DCFG3 0x0288 /* Design Config 3 */ @@ -478,6 +488,10 @@ #define GEM_HS_MAC_SPEED_OFFSET 0 #define GEM_HS_MAC_SPEED_SIZE 3 +/* Bitfields in PCSCNTRL */ +#define GEM_PCSAUTONEG_OFFSET 12 +#define GEM_PCSAUTONEG_SIZE 1 + /* Bitfields in DCFG1. */ #define GEM_IRQCOR_OFFSET 23 #define GEM_IRQCOR_SIZE 1 diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index ca72a16c8da3..e7c123aadf56 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -694,6 +694,22 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode, if (old_ncr ^ ncr) macb_or_gem_writel(bp, NCR, ncr); + /* Disable AN for SGMII fixed link configuration, enable otherwise. + * Must be written after PCSSEL is set in NCFGR, + * otherwise writes will not take effect. + */ + if (macb_is_gem(bp) && state->interface == PHY_INTERFACE_MODE_SGMII) { + u32 pcsctrl, old_pcsctrl; + + old_pcsctrl = gem_readl(bp, PCSCNTRL); + if (mode == MLO_AN_FIXED) + pcsctrl = old_pcsctrl & ~GEM_BIT(PCSAUTONEG); + else + pcsctrl = old_pcsctrl | GEM_BIT(PCSAUTONEG); + if (old_pcsctrl != pcsctrl) + gem_writel(bp, PCSCNTRL, pcsctrl); + } + spin_unlock_irqrestore(&bp->lock, flags); }
When using a fixed-link configuration in SGMII mode, it's not really sensible to have auto-negotiation enabled since the link settings are fixed by definition. In other configurations, such as an SGMII connection to a PHY, it should generally be enabled. Signed-off-by: Robert Hancock <robert.hancock@calian.com> --- drivers/net/ethernet/cadence/macb.h | 14 ++++++++++++++ drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++++++++ 2 files changed, 30 insertions(+)