From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey.Brodkin@synopsys.com (Alexey Brodkin) Date: Fri, 27 Jan 2017 10:23:06 +0000 Subject: stmmac: GMAC_RGSMIIIS reports bogus values In-Reply-To: <1485369563.7117.81.camel@synopsys.com> References: <1478189833.4072.65.camel@synopsys.com> <1485369563.7117.81.camel@synopsys.com> List-ID: Message-ID: <1485512586.2693.35.camel@synopsys.com> To: linux-snps-arc@lists.infradead.org Hi?Giuseppe, On Wed, 2017-01-25@21:39 +0300, Alexey Brodkin wrote: > Hi Giuseppe, > > On Mon, 2016-11-14@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. Hm, so I took a look at what am I reading from "Register 22 (HW Feature Register)" in?dwmac1000_get_hw_feature() and was really surprised to see the register value = 0x100509bf. See bit 6 "PCSSEL" is zeroed which stands for "PCS registers (TBI, SGMII, or RTBI PHY interface)". I.e. PCS doesn't exist in our GMAC so most of the previous comments below should be discarded. > > 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(). So it all really boils down to the following problem: Link state reported via "Register 54 (SGMII/RGMII/SMII Control and Status Register)" doesn't match status read via MDIO from the PHY. That's why my initial proposal was to ignore whatever we read from this register if we have MDIO bus instantiated already. -Alexey From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932558AbdA0KXl (ORCPT ); Fri, 27 Jan 2017 05:23:41 -0500 Received: from smtprelay.synopsys.com ([198.182.60.111]:56541 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932414AbdA0KXL (ORCPT ); Fri, 27 Jan 2017 05:23:11 -0500 From: Alexey Brodkin To: "peppe.cavallaro@st.com" CC: "manabian@gmail.com" , "linux-kernel@vger.kernel.org" , "fabrice.gasnier@st.com" , "linux-snps-arc@lists.infradead.org" , "alexandre.torgue@gmail.com" , "preid@electromag.com.au" , "netdev@vger.kernel.org" , Vineet Gupta , "davem@davemloft.net" Subject: Re: stmmac: GMAC_RGSMIIIS reports bogus values Thread-Topic: stmmac: GMAC_RGSMIIIS reports bogus values Thread-Index: AQHSNe3WHCWZ99cl6060z9b7QcHLU6DYIbIAgHHWYICAApoBAA== Date: Fri, 27 Jan 2017 10:23:06 +0000 Message-ID: <1485512586.2693.35.camel@synopsys.com> References: <1478189833.4072.65.camel@synopsys.com> <1485369563.7117.81.camel@synopsys.com> In-Reply-To: <1485369563.7117.81.camel@synopsys.com> Accept-Language: en-US, ru-RU Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.121.8.91] Content-Type: text/plain; charset="utf-8" Content-ID: <7875BE23A472B54294E8D0891DEA130E@internal.synopsys.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v0RAONru002923 Hi Giuseppe, On Wed, 2017-01-25 at 21:39 +0300, Alexey Brodkin wrote: > 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. Hm, so I took a look at what am I reading from "Register 22 (HW Feature Register)" in dwmac1000_get_hw_feature() and was really surprised to see the register value = 0x100509bf. See bit 6 "PCSSEL" is zeroed which stands for "PCS registers (TBI, SGMII, or RTBI PHY interface)". I.e. PCS doesn't exist in our GMAC so most of the previous comments below should be discarded. > > 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(). So it all really boils down to the following problem: Link state reported via "Register 54 (SGMII/RGMII/SMII Control and Status Register)" doesn't match status read via MDIO from the PHY. That's why my initial proposal was to ignore whatever we read from this register if we have MDIO bus instantiated already. -Alexey From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Brodkin Subject: Re: stmmac: GMAC_RGSMIIIS reports bogus values Date: Fri, 27 Jan 2017 10:23:06 +0000 Message-ID: <1485512586.2693.35.camel@synopsys.com> References: <1478189833.4072.65.camel@synopsys.com> <1485369563.7117.81.camel@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: "fabrice.gasnier@st.com" , "netdev@vger.kernel.org" , "manabian@gmail.com" , "linux-kernel@vger.kernel.org" , "preid@electromag.com.au" , "davem@davemloft.net" , "alexandre.torgue@gmail.com" , "linux-snps-arc@lists.infradead.org" , Vineet Gupta To: "peppe.cavallaro@st.com" Return-path: In-Reply-To: <1485369563.7117.81.camel@synopsys.com> Content-Language: en-US Content-ID: <7875BE23A472B54294E8D0891DEA130E@internal.synopsys.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-snps-arc" Errors-To: linux-snps-arc-bounces+gla-linux-snps-arc=m.gmane.org@lists.infradead.org List-Id: netdev.vger.kernel.org SGnCoEdpdXNlcHBlLA0KDQpPbiBXZWQsIDIwMTctMDEtMjUgYXQgMjE6MzkgKzAzMDAsIEFsZXhl eSBCcm9ka2luIHdyb3RlOg0KPiBIaSBHaXVzZXBwZSwNCj4gDQo+IE9uIE1vbiwgMjAxNi0xMS0x NCBhdCAwOToxNCArMDEwMCwgR2l1c2VwcGUgQ0FWQUxMQVJPIHdyb3RlOg0KPiA+IA0KPiA+IEhl bGxvIEFsZXhleQ0KPiA+IA0KPiA+IFNvcnJ5IGZvciBteSBsYXRlIHJlcGx5LiBJbiB0aGF0IGNh c2UsIEkgdGhpbmsgdGhhdCB0aGUgc3RtbWFjDQo+ID4gaXMgdXNpbmcgb3duIFJHTUlJL1NHTUlJ IHN1cHBvcnQgKFBDUykgdG8gZGlhbG9nIHdpdGggdGhlDQo+ID4gdHJhbnNjZWl2ZXIuIEkgdGhp bmssIGZyb20gdGhlIEhXIGNhcGFiaWxpdHkgcmVnaXN0ZXINCj4gPiB0aGlzIGZlYXR1cmUgaXMg cHJlc2VudCBhbmQgdGhlIGRyaXZlciBpcyB1c2luZyBpdCBhcyBkZWZhdWx0Lg0KPiANCj4gWWVw IGxvb2tzIGxpa2UgdGhhdC4NCg0KSG0sIHNvIEkgdG9vayBhIGxvb2sgYXQgd2hhdCBhbSBJIHJl YWRpbmcgZnJvbQ0KIlJlZ2lzdGVyIDIyIChIVyBGZWF0dXJlIFJlZ2lzdGVyKSIgaW7CoGR3bWFj MTAwMF9nZXRfaHdfZmVhdHVyZSgpDQphbmQgd2FzIHJlYWxseSBzdXJwcmlzZWQgdG8gc2VlIHRo ZSByZWdpc3RlciB2YWx1ZSA9IDB4MTAwNTA5YmYuDQoNClNlZSBiaXQgNiAiUENTU0VMIiBpcyB6 ZXJvZWQgd2hpY2ggc3RhbmRzIGZvcg0KIlBDUyByZWdpc3RlcnMgKFRCSSwgU0dNSUksIG9yIFJU QkkgUEhZIGludGVyZmFjZSkiLg0KDQpJLmUuIFBDUyBkb2Vzbid0IGV4aXN0IGluIG91ciBHTUFD IHNvIG1vc3Qgb2YgdGhlIHByZXZpb3VzIGNvbW1lbnRzDQpiZWxvdyBzaG91bGQgYmUgZGlzY2Fy ZGVkLg0KDQo+ID4gSSBraW5kbHkgYXNrIHlvdSB0byB2ZXJpZnkgaWYgdGhpcyBpcyB5b3VyIHNl dHVwIG9yIGlmIHlvdQ0KPiA+IGRvIG5vdCB3YW50IHRvIHVzZSBpdC4gSW4gdGhhdCBjYXNlLCBp dCBpcyBsaWtlbHkgd2UgbmVlZCB0bw0KPiA+IGFkZCBzb21lIGNvZGUgaW4gdGhlIGRyaXZlci4N Cj4gDQo+IFdlbGwgR01BQydzIGRhdGFib29rIHNheXM6DQo+IC0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLT44LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gVGhlIERXQ19nbWFjIHByb3Zp ZGVzIGFuIElFRUUgODAyLjN6IGNvbXBsaWFudCAxMC1iaXQNCj4gUGh5c2ljYWwgQ29kaW5nIFN1 YmxheWVyIChQQ1MpIGludGVyZmFjZSB0aGF0IHlvdSBjYW4gdXNlDQo+IHdoZW4gdGhlIE1BQyBp cyBjb25maWd1cmVkIGZvciB0aGUgVEJJLCBSVEJJLCBvciBTR01JSSBQSFkNCj4gaW50ZXJmYWNl Lg0KPiANCj4gLi4uDQo+IA0KPiBZb3UgY2FuIGluY2x1ZGUgdGhlIG9wdGlvbmFsIFBDUyBtb2R1 bGUgaW4gdGhlIERXQ19nbWFjIGJ5DQo+IHNlbGVjdGluZyB0aGUgVEJJLCBSVEJJLCBvciBTR01J SSBpbnRlcmZhY2UgaW4gY29yZUNvbnN1bHRhbnQNCj4gZHVyaW5nIGNvbmZpZ3VyYXRpb24uDQo+ IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLT44LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0N Cj4gDQo+IEJ1dCB3ZSB1c2UgUkdNSUkgZm9yIGNvbW11bmljYXRpb24gd2l0aCB0aGUgUEhZLg0K PiBBbmQgZnJvbSB0aGUgcXVvdGUgYWJvdmUgSSB3b3VsZCBjb25jbHVkZSB0aGF0IGZvciBSR01J SSB3ZSBzaG91bGQNCj4gaGF2ZSBQQ1MgZXhjbHVkZWQgZnJvbSBHTUFDLg0KPiANCj4gSSBqdXN0 IHNlbnQgZW1haWwgdG8gR01BQyBkZXZlbG9wZXJzIGhlcmUgaW4gU3lub3BzeXMgYW5kDQo+IGhv cGVmdWxseSB3aWxsIGdldCBzb21lIGNsYXJpZmljYXRpb25zIGZyb20gdGhlbSBzb29uLg0KPiAN Cj4gUHJvYmFibHkgY29ycmVjdCBzb2x1dGlvbiBpcyBhcyBzaW1wbGUgYXM6DQo+IC0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLT44LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvbmV0L2V0aGVybmV0L3N0bWljcm8vc3RtbWFjL3N0bW1hY19tYWluLmMg Yi9kcml2ZXJzL25ldC9ldGhlcm5ldC9zdG1pY3JvL3N0bW1hYy9zdG1tYWNfbWFpbi5jDQo+IGlu ZGV4IGEyNzZhMzJkNTdmMi4uZjRiNjdkYzdkNTMwIDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL25l dC9ldGhlcm5ldC9zdG1pY3JvL3N0bW1hYy9zdG1tYWNfbWFpbi5jDQo+ICsrKyBiL2RyaXZlcnMv bmV0L2V0aGVybmV0L3N0bWljcm8vc3RtbWFjL3N0bW1hY19tYWluLmMNCj4gQEAgLTgwMywxMyAr ODAzLDcgQEAgc3RhdGljIHZvaWQgc3RtbWFjX2NoZWNrX3Bjc19tb2RlKHN0cnVjdCBzdG1tYWNf cHJpdiAqcHJpdikNCj4gwqDCoMKgwqDCoMKgwqDCoGludCBpbnRlcmZhY2UgPSBwcml2LT5wbGF0 LT5pbnRlcmZhY2U7DQo+IMKgDQo+IMKgwqDCoMKgwqDCoMKgwqBpZiAocHJpdi0+ZG1hX2NhcC5w Y3MpIHsNCj4gLcKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGlmICgoaW50ZXJmYWNlID09 IFBIWV9JTlRFUkZBQ0VfTU9ERV9SR01JSSkgfHwNCj4gLcKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoMKgKGludGVyZmFjZSA9PSBQSFlfSU5URVJGQUNFX01PREVfUkdNSUlfSUQp IHx8DQo+IC3CoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoChpbnRlcmZhY2Ug PT0gUEhZX0lOVEVSRkFDRV9NT0RFX1JHTUlJX1JYSUQpIHx8DQo+IC3CoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoChpbnRlcmZhY2UgPT0gUEhZX0lOVEVSRkFDRV9NT0RFX1JH TUlJX1RYSUQpKSB7DQo+IC3CoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgbmV0ZGV2X2RiZyhwcml2LT5kZXYsICJQQ1MgUkdNSUkgc3VwcG9ydCBlbmFibGVkXG4i KTsNCj4gLcKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBwcml2 LT5ody0+cGNzID0gU1RNTUFDX1BDU19SR01JSTsNCj4gLcKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC oMKgwqDCoH0gZWxzZSBpZiAoaW50ZXJmYWNlID09IFBIWV9JTlRFUkZBQ0VfTU9ERV9TR01JSSkg ew0KPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgaWYgKGludGVyZmFjZSA9PSBQSFlf SU5URVJGQUNFX01PREVfU0dNSUkpIHsNCj4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgbmV0ZGV2X2RiZyhwcml2LT5kZXYsICJQQ1MgU0dNSUkgc3VwcG9y dCBlbmFibGVkXG4iKTsNCj4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC oMKgwqDCoMKgcHJpdi0+aHctPnBjcyA9IFNUTU1BQ19QQ1NfU0dNSUk7DQo+IMKgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgfQ0KPiAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0+OC0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQo+IA0KPiA+IA0KPiA+IEFsc28gSSB3b25kZXIgaWYs IG90aGVyIHZlcnNpb24gb2YgdGhlIHN0bW1hYyB3b3JrZWQgb24gdGhpcyBwbGF0Zm9ybQ0KPiA+ IGJlZm9yZS4NCj4gDQo+IEl0IGRpZCB3b3JrIGFuZCBzdGlsbCB3b3Jrcy4gVGhlIG9ubHkgcHJv YmxlbSBpcyB3ZSdyZSBnZXR0aW5nDQo+IGEgbG90IG9mIG5vaXNlIG5vdyBhYm91dCBib2d1cyBs aW5rIHN0YXR1cyBjaGFuZ2UuIFRoYXQncyBiZWNhdXNlDQo+IHRoaXMgaW5mbyBpcyBub3cgaW4g cHJfaW5mbygpIGNvbXBhcmVkIHRvIGJlaW5nIHByZXZpb3VzbHkgaW4gcHJfZGVidWcoKS4NCg0K U28gaXQgYWxsIHJlYWxseSBib2lscyBkb3duIHRvIHRoZSBmb2xsb3dpbmcgcHJvYmxlbToNCkxp bmsgc3RhdGUgcmVwb3J0ZWQgdmlhICJSZWdpc3RlciA1NCAoU0dNSUkvUkdNSUkvU01JSSBDb250 cm9sIGFuZCBTdGF0dXMgUmVnaXN0ZXIpIg0KZG9lc24ndCBtYXRjaCBzdGF0dXMgcmVhZCB2aWEg TURJTyBmcm9tIHRoZSBQSFkuDQoNClRoYXQncyB3aHkgbXkgaW5pdGlhbCBwcm9wb3NhbCB3YXMg dG8gaWdub3JlIHdoYXRldmVyIHdlIHJlYWQgZnJvbSB0aGlzIHJlZ2lzdGVyDQppZiB3ZSBoYXZl IE1ESU8gYnVzIGluc3RhbnRpYXRlZCBhbHJlYWR5Lg0KDQotQWxleGV5DQoNCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LXNucHMtYXJjIG1haWxp bmcgbGlzdApsaW51eC1zbnBzLWFyY0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5p bmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtc25wcy1hcmM=