netdev - Re: stmmac: GMAC_RGSMIIIS reports bogus values

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

Message-ID: <1485369563.7117.81.camel@synopsys.com>
Date:   Wed, 25 Jan 2017 18:39:24 +0000
From:   Alexey Brodkin <Alexey.Brodkin@...opsys.com>
To:     "peppe.cavallaro@...com" <peppe.cavallaro@...com>
CC:     "manabian@...il.com" <manabian@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "fabrice.gasnier@...com" <fabrice.gasnier@...com>,
        "linux-snps-arc@...ts.infradead.org" 
        <linux-snps-arc@...ts.infradead.org>,
        "alexandre.torgue@...il.com" <alexandre.torgue@...il.com>,
        "preid@...ctromag.com.au" <preid@...ctromag.com.au>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Vineet Gupta <Vineet.Gupta1@...opsys.com>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: stmmac: GMAC_RGSMIIIS reports bogus values

Hi Giuseppe,

On Mon, 2016-11-14 at 09:14 +0100, Giuseppe CAVALLARO wrote:
> Hello Alexey
> 
> Sorry for my late reply. In that case, I think that the stmmac
> is using own RGMII/SGMII support (PCS) to dialog with the
> transceiver. I think, from the HW capability register
> this feature is present and the driver is using it as default.

Yep looks like that.

> I kindly ask you to verify if this is your setup or if you
> do not want to use it. In that case, it is likely we need to
> add some code in the driver.

Well GMAC's databook says:
--------------------------->8--------------------------
The DWC_gmac provides an IEEE 802.3z compliant 10-bit
Physical Coding Sublayer (PCS) interface that you can use
when the MAC is configured for the TBI, RTBI, or SGMII PHY
interface.

...

You can include the optional PCS module in the DWC_gmac by
selecting the TBI, RTBI, or SGMII interface in coreConsultant
during configuration.
--------------------------->8--------------------------

But we use RGMII for communication with the PHY.
And from the quote above I would conclude that for RGMII we should
have PCS excluded from GMAC.

I just sent email to GMAC developers here in Synopsys and
hopefully will get some clarifications from them soon.

Probably correct solution is as simple as:
--------------------------->8--------------------------
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a276a32d57f2..f4b67dc7d530 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -803,13 +803,7 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv)
        int interface = priv->plat->interface;
 
        if (priv->dma_cap.pcs) {
-               if ((interface == PHY_INTERFACE_MODE_RGMII) ||
-                   (interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-                   (interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
-                   (interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-                       netdev_dbg(priv->dev, "PCS RGMII support enabled\n");
-                       priv->hw->pcs = STMMAC_PCS_RGMII;
-               } else if (interface == PHY_INTERFACE_MODE_SGMII) {
+               if (interface == PHY_INTERFACE_MODE_SGMII) {
                        netdev_dbg(priv->dev, "PCS SGMII support enabled\n");
                        priv->hw->pcs = STMMAC_PCS_SGMII;
                }
--------------------------->8--------------------------

> Also I wonder if, other version of the stmmac worked on this platform
> before.

It did work and still works. The only problem is we're getting
a lot of noise now about bogus link status change. That's because
this info is now in pr_info() compared to being previously in pr_debug().

-Alexey