Re: thunderx sgmii interface hang

[Tim Harvey]

On Fri, Dec 22, 2017 at 4:30 PM, David Daney <dda...@caviumnetworks.com> wrote:
> On 12/22/2017 04:22 PM, Tim Harvey wrote:

>>
>> BGXX_GMP_GMI_TXX_INT[UNDFLW] is getting set when the issue is
>> triggered. From CN80XX-HM-1.2P this is caused by:
>>
>> "In the unlikely event that P2X data cannot keep the GMP TX FIFO full,
>> the SGMII/1000BASE-X/ QSGMII packet transfer will underflow. This
>> should be detected by the receiving device as an FCS error.
>> Internally, the packet is drained and lost"
>>
>
> Yikes!
>
>> Perhaps this needs to be caught and handled in some way. There's some
>> interrupt handlers in nicvf_main.c yet I'm not clear where to hook up
>> this one.
>
>
> This would be an interrupt generated by the BGX device, not the NIC device
> It will have an MSI-X index of (6 + LMAC * 7).  See BGX_INT_VEC_E in the
> HRM.
>
> Note that I am telling you which interrupt it is, but not recommending that
> catching it and doing something is necessarily the best thing to do.
>

David,

The following patch registers the BGX_GMP_GMI_TXX interrupt, enables
the UNDFLW interrupt, and toggles the MAC/PCS enable and does indeed
catch/resolve the issue which never occurs again.

I would agree this isn't a 'fix' but works around whatever the issue
is. Any ideas what could cause an UNDFLW like this?

I suspect it might have something to do with the fact we have a 100MHz
external ref clock for the DLM which is configured for
GSERx_LANE_MODE[LMODE] of 0x6 (R_125G_REFCLK15625_SGMII 1.25Gbd
156.25MHz ref clock SGMII) which seems just wrong (100MHz clock yet we
tell CN80XX 165MHz) but according to the reference manual and BDK code
this is an allowed configuration.

Do you have access to a CN80XX reference board that has an SGMII PHY
on it to test for this issue? I've been trying to get hold of one but
have not been successful.

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
index 2bba9d1..be9148f9 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
@@ -179,6 +179,15 @@
 #define BGX_GMP_GMI_TXX_BURST  0x38228
 #define BGX_GMP_GMI_TXX_MIN_PKT0x38240
 #define BGX_GMP_GMI_TXX_SGMII_CTL  0x38300
+#define BGX_GMP_GMI_TXX_INT0x38500
+#define BGX_GMP_GMI_TXX_INT_W1S0x38508
+#define BGX_GMP_GMI_TXX_INT_ENA_W1C0x38510
+#define BGX_GMP_GMI_TXX_INT_ENA_W1S0x38518
+#define  GMI_TXX_INT_PTP_LOST  BIT_ULL(4)
+#define  GMI_TXX_INT_LATE_COL  BIT_ULL(3)
+#define  GMI_TXX_INT_XSDEF BIT_ULL(2)
+#define  GMI_TXX_INT_XSCOL BIT_ULL(1)
+#define  GMI_TXX_INT_UNDFLWBIT_ULL(0)

 #define BGX_MSIX_VEC_0_29_ADDR 0x40 /* +(0..29) << 4 */
 #define BGX_MSIX_VEC_0_29_CTL  0x48

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 805c02a..0690966 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1344,6 +1344,54 @@ static int bgx_init_phy(struct bgx *bgx)
return bgx_init_of_phy(bgx);
 }

+static irqreturn_t bgx_intr_handler(int irq, void *data)
+{
+   struct bgx *bgx = (struct bgx *)data;
+   struct device *dev = >pdev->dev;
+   u64 status, val;
+   int lmac;
+
+   for (lmac = 0; lmac < bgx->lmac_count; lmac++) {
+   status = bgx_reg_read(bgx, lmac, BGX_GMP_GMI_TXX_INT);
+   if (status & GMI_TXX_INT_UNDFLW) {
+   dev_err(dev, "BGX%d lmac%d UNDFLW\n", bgx->bgx_id,
+   lmac);
+   val = bgx_reg_read(bgx, lmac, BGX_CMRX_CFG);
+   val &= ~CMR_EN;
+   bgx_reg_write(bgx, lmac, BGX_CMRX_CFG, val);
+   val |= CMR_EN;
+   bgx_reg_write(bgx, lmac, BGX_CMRX_CFG, val);
+   }
+   /* clear interrupts */
+   bgx_reg_write(bgx, lmac, BGX_GMP_GMI_TXX_INT, status);
+   }
+
+   return IRQ_HANDLED;
+}
+
+static int bgx_register_intr(struct pci_dev *pdev)
+{
+   struct bgx *bgx = pci_get_drvdata(pdev);
+   struct device *dev = >dev;
+   int num_vec, ret;
+   char irq_name[32];
+
+   /* Enable MSI-X */
+   num_vec = pci_msix_vec_count(pdev);
+   ret = pci_alloc_irq_vectors(pdev, num_vec, num_vec, PCI_IRQ_MSIX);
+   if (ret < 0) {
+   dev_err(dev, "Req for #%d msix vectors failed\n", num_vec);
+   return 1;
+   }
+   sprintf(irq_name, "BGX%d", bgx->bgx_id);
+   ret = request_irq(pci_irq_vector(pdev, GMPX_GMI_TX_INT),
+

Re: thunderx sgmii interface hang

[Tim Harvey]

On Fri, Dec 22, 2017 at 3:00 PM, David Daney <dda...@caviumnetworks.com> wrote:
> On 12/22/2017 02:19 PM, Tim Harvey wrote:
>>
>> On Tue, Dec 19, 2017 at 12:52 PM, Andrew Lunn <and...@lunn.ch> wrote:
>>>
>>> On Mon, Dec 18, 2017 at 01:53:47PM -0800, Tim Harvey wrote:
>>>>
>>>> On Wed, Dec 13, 2017 at 11:43 AM, Andrew Lunn <and...@lunn.ch> wrote:
>>>>>>
>>>>>> The nic appears to work fine (pings, TCP etc) up until a performance
>>>>>> test is attempted.
>>>>>> When an iperf bandwidth test is attempted the nic ends up in a state
>>>>>> where truncated-ip packets are being sent out (per a tcpdump from
>>>>>> another board):
>>>>>
>>>>>
>>>>> Hi Tim
>>>>>
>>>>> Are pause frames supported? Have you tried turning them off?
>>>>>
>>>>> Can you reproduce the issue with UDP? Or is it TCP only?
>>>>>
>>>>
>>>> Andrew,
>>>>
>>>> Pause frames don't appear to be supported yet and the issue occurs
>>>> when using UDP as well as TCP. I'm not clear what the best way to
>>>> troubleshoot this is.
>>>
>>>
>>> Hi Tim
>>>
>>> Is pause being negotiated? In theory, it should not be. The PHY should
>>> not offer it, if the MAC has not enabled it. But some PHY drivers are
>>> probably broken and offer pause when they should not.
>>>
>>> Also, can you trigger the issue using UDP at say 75% the maximum
>>> bandwidth. That should be low enough that the peer never even tries to
>>> use pause.
>>>
>>> All this pause stuff is just a stab in the dark. Something else to try
>>> is to turn off various forms off acceleration, ethtook -K, and see if
>>> that makes a difference.
>>>
>>
>> Andrew,
>>
>> Currently I'm not using the DP83867_PHY driver (after verifying the
>> issue occurs with or without that driver).
>>
>> It does not occur if I limit UDP (ie 950mbps). I disabled all offloads
>> and the issue still occurs.
>>
>> I have found that once the issue occurs I can recover to a working
>> state by clearing/setting BGX_CMRX_CFG[BGX_EN] and once I encounter
>> the issue and recover with that, I can never trigger the issue again.
>> If toggle that register bit upon power-up before the issue occurs it
>> will still occur.
>>
>> The CN80XX reference manual describes BGX_CMRX_CFG[BGX_EN] as:
>> - when cleared all dedicated BGX context state for LMAC (state
>> machine, FIFOs, counters etc) are reset and LMAC access to shared BGX
>> resources (data path, serdes lanes) is disabled
>> - when set LMAC operation is enabled (link bring-up, sync, and tx/rx
>> of idles and fault sequences)
>
>
> You could try looking at
> BGXX_GMP_PCS_INTX
> BGXX_GMP_GMI_RXX_INT
> BGXX_GMP_GMI_TXX_INT
>
> Those are all W1C registers that should contain all zeros.  If they don't,
> just write back to them to clear before running a test.
>
> If there are bits asserting in these when the thing gets wedged up, it might
> point to a possible cause.

David,

BGXX_GMP_GMI_TXX_INT[UNDFLW] is getting set when the issue is
triggered. From CN80XX-HM-1.2P this is caused by:

"In the unlikely event that P2X data cannot keep the GMP TX FIFO full,
the SGMII/1000BASE-X/ QSGMII packet transfer will underflow. This
should be detected by the receiving device as an FCS error.
Internally, the packet is drained and lost"

Perhaps this needs to be caught and handled in some way. There's some
interrupt handlers in nicvf_main.c yet I'm not clear where to hook up
this one.

>
> You could also look at these RO registers:
> BGXX_GMP_PCS_TXX_STATES
> BGXX_GMP_PCS_RXX_STATES
>

These show the same before/after triggering the issue and
RX_BAD/TX_BAD are still 0.

Tim

Re: thunderx sgmii interface hang

[Tim Harvey]

On Fri, Dec 22, 2017 at 2:45 PM, Andrew Lunn  wrote:
>> Currently I'm not using the DP83867_PHY driver (after verifying the
>> issue occurs with or without that driver).
>>
>> It does not occur if I limit UDP (ie 950mbps). I disabled all offloads
>> and the issue still occurs.
>
>> I'm told that the particular Cavium reference board with an SGMII phy
>> doesn't show this issue (I don't have that specific board to do my own
>> testing or comparisons against our board) so I'm inclined to think it
>> has something to do with an interaction with the DP83867 PHY. I would
>> like to start poking at PHY registers to see if I can find anything
>> unusual. The best way to do that from userspace is via
>> SIOCGMIIREG/SIOCSMIIREG right? The thunderx nic doesn't currently
>> support ioctl's so I guess I'll have to add that support unless
>> there's a way to get at phy registers from userspace through a phy
>> driver?
>
> phy_mii_ioctl() does what you need, and is simple to use.
>
> mii-tool will then give you access to the standard PHY registers.
>
>  Andre

I didn't think mii-tool or ethtool (its replacement right?) could
read/write specific phy registers via cmdline? I wrote my own tool
some time ago to do that [1].

Tim

[1] http://trac.gateworks.com/attachment/wiki/conformance_testing/mii-reg.c

Re: thunderx sgmii interface hang

[Tim Harvey]

On Tue, Dec 19, 2017 at 12:52 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Mon, Dec 18, 2017 at 01:53:47PM -0800, Tim Harvey wrote:
>> On Wed, Dec 13, 2017 at 11:43 AM, Andrew Lunn <and...@lunn.ch> wrote:
>> >> The nic appears to work fine (pings, TCP etc) up until a performance
>> >> test is attempted.
>> >> When an iperf bandwidth test is attempted the nic ends up in a state
>> >> where truncated-ip packets are being sent out (per a tcpdump from
>> >> another board):
>> >
>> > Hi Tim
>> >
>> > Are pause frames supported? Have you tried turning them off?
>> >
>> > Can you reproduce the issue with UDP? Or is it TCP only?
>> >
>>
>> Andrew,
>>
>> Pause frames don't appear to be supported yet and the issue occurs
>> when using UDP as well as TCP. I'm not clear what the best way to
>> troubleshoot this is.
>
> Hi Tim
>
> Is pause being negotiated? In theory, it should not be. The PHY should
> not offer it, if the MAC has not enabled it. But some PHY drivers are
> probably broken and offer pause when they should not.
>
> Also, can you trigger the issue using UDP at say 75% the maximum
> bandwidth. That should be low enough that the peer never even tries to
> use pause.
>
> All this pause stuff is just a stab in the dark. Something else to try
> is to turn off various forms off acceleration, ethtook -K, and see if
> that makes a difference.
>

Andrew,

Currently I'm not using the DP83867_PHY driver (after verifying the
issue occurs with or without that driver).

It does not occur if I limit UDP (ie 950mbps). I disabled all offloads
and the issue still occurs.

I have found that once the issue occurs I can recover to a working
state by clearing/setting BGX_CMRX_CFG[BGX_EN] and once I encounter
the issue and recover with that, I can never trigger the issue again.
If toggle that register bit upon power-up before the issue occurs it
will still occur.

The CN80XX reference manual describes BGX_CMRX_CFG[BGX_EN] as:
- when cleared all dedicated BGX context state for LMAC (state
machine, FIFOs, counters etc) are reset and LMAC access to shared BGX
resources (data path, serdes lanes) is disabled
- when set LMAC operation is enabled (link bring-up, sync, and tx/rx
of idles and fault sequences)

I'm told that the particular Cavium reference board with an SGMII phy
doesn't show this issue (I don't have that specific board to do my own
testing or comparisons against our board) so I'm inclined to think it
has something to do with an interaction with the DP83867 PHY. I would
like to start poking at PHY registers to see if I can find anything
unusual. The best way to do that from userspace is via
SIOCGMIIREG/SIOCSMIIREG right? The thunderx nic doesn't currently
support ioctl's so I guess I'll have to add that support unless
there's a way to get at phy registers from userspace through a phy
driver?

Regards,

Tim

Re: [PATCH] net: thunderx: add support for rgmii internal delay

[Tim Harvey]

On Thu, Dec 14, 2017 at 12:45 AM, Andrew Lunn <and...@lunn.ch> wrote:
> On Wed, Dec 13, 2017 at 03:28:33PM -0800, Tim Harvey wrote:
>> On Wed, Dec 13, 2017 at 3:10 AM, Andrew Lunn <and...@lunn.ch> wrote:
>> >> +void xcv_init_hw(int phy_mode)
>> >>  {
>> >>   u64  cfg;
>> >>
>> >> @@ -81,12 +81,31 @@ void xcv_init_hw(void)
>> >>   /* Wait for DLL to lock */
>> >>   msleep(1);
>> >>
>> >> - /* Configure DLL - enable or bypass
>> >> -  * TX no bypass, RX bypass
>> >> -  */
>> >> + /* enable/bypass DLL providing MAC based internal TX/RX delays */
>> >>   cfg = readq_relaxed(xcv->reg_base + XCV_DLL_CTL);
>> >> - cfg &= ~0xFF03;
>> >> - cfg |= CLKRX_BYP;
>> >> + cfg &= ~0x00;
>> >> + switch (phy_mode) {
>> >> + /* RX and TX delays are added by the MAC */
>> >> + case PHY_INTERFACE_MODE_RGMII:
>> >> + break;
>> >> + /* internal RX and TX delays provided by the PHY */
>> >> + case PHY_INTERFACE_MODE_RGMII_ID:
>> >> + cfg |= CLKRX_BYP;
>> >> + cfg |= CLKTX_BYP;
>> >> + break;
>> >> + /* internal RX delay provided by the PHY, the MAC
>> >> +  * should not add an RX delay in this case
>> >> +  */
>> >> + case PHY_INTERFACE_MODE_RGMII_RXID:
>> >> + cfg |= CLKRX_BYP;
>> >> + break;
>> >> + /* internal TX delay provided by the PHY, the MAC
>> >> +  * should not add an TX delay in this case
>> >> +  */
>> >> + case PHY_INTERFACE_MODE_RGMII_TXID:
>> >> + cfg |= CLKRX_BYP;
>> >> + break;
>> >> + }
>> >
>> > Hi Tim
>> >
>> > This i don't get. Normally, you leave the PHY to handle delays, if
>> > needed. The MAC should not add any. Here you seem to assume a delay is
>> > always needed, and if the PHY is not providing it, the MAC should.
>> >
>> >Andrew
>>
>> Andrew,
>>
>> The thunder RGX inserts a delay via an on-board DLL. The 'bypass'
>> register will bypass this DLL and not insert a delay from the MAC
>> side. By default out of reset CLKTX_BYP=1 causing the RGX transmit
>> interface to not introduce a delay and CLKRX_BYP=0 causing the RGX
>> receive interface to introduce a delay.
>
> Hi Tim
>
> So the MAC by default is doing PHY_INTERFACE_MODE_RGMII_RXID.  And it
> calls phy_connect_direct() passing PHY_INTERFACE_MODE_RGMII. It does
> not get anything from device tree. So it looks like we have a chance
> to clean this up.
>
> So the correct thing to do is set the MAC to PHY_INTERFACE_MODE_RGMII,
> i.e. no delays. By default call phy_connect_direct()
> PHY_INTERFACE_MODE_RGMII_RXID. That should give you the same behaviour
> as today.

I don't understand - PHY_INTERFACE_MODE_RGMII means delays are added by the MAC

The way I see it today the driver is making an assumption that is not
always correct. What is the right way to configure a MAC when no
phy-mode is present in the dts? I assumed it would be RGMII_ID such
that the MAC introduces no delay.

>
> Then add code to look in device tree, to find a per board setting. In
> your case, you want PHY_INTERFACE_MODE_RGMII_ID. And make sure the PHY
> driver respects the value passed.
>
>Andrew
>

Should I be attempting to make the default if no phy-mode is in the
dts be PHY_INTERFACE_MODE_RGMII_RXID so that existing boards do not
break (as I assume they configure the phy's that way in firmware).

My original goal was to make the bgx driver flexible for different
delay configurations as well as allow phy drivers to be used. However
I found that the dp83867 driver doesn't work with my board anyway as
it issues a soft reset that disables CLKOUT which I setup in firmware
and require. Is it standard for phy drivers to issue hard or soft
resets during init and if so how do boards deal with custom LED or
CLKOUT configs as those don't seem to be supported by phy drivers? I
only have experience with phy drivers that support an optional hard
reset and if you don't want to reset any custom regs you simply don't
expose the phy_rst gpio (assuming there is on) to the driver by not
defining it in device-tree.

Tim

Re: thunderx sgmii interface hang

[Tim Harvey]

On Wed, Dec 13, 2017 at 11:43 AM, Andrew Lunn  wrote:
>> The nic appears to work fine (pings, TCP etc) up until a performance
>> test is attempted.
>> When an iperf bandwidth test is attempted the nic ends up in a state
>> where truncated-ip packets are being sent out (per a tcpdump from
>> another board):
>
> Hi Tim
>
> Are pause frames supported? Have you tried turning them off?
>
> Can you reproduce the issue with UDP? Or is it TCP only?
>

Andrew,

Pause frames don't appear to be supported yet and the issue occurs
when using UDP as well as TCP. I'm not clear what the best way to
troubleshoot this is.

Sunil, have any others reported this issue? I do not have a Cavium
CN80xx/CN81xx reference board that has SGMII.

Regards,

Tim

Re: [PATCH] net: thunderx: add support for rgmii internal delay

[Tim Harvey]

On Wed, Dec 13, 2017 at 3:10 AM, Andrew Lunn  wrote:
>> +void xcv_init_hw(int phy_mode)
>>  {
>>   u64  cfg;
>>
>> @@ -81,12 +81,31 @@ void xcv_init_hw(void)
>>   /* Wait for DLL to lock */
>>   msleep(1);
>>
>> - /* Configure DLL - enable or bypass
>> -  * TX no bypass, RX bypass
>> -  */
>> + /* enable/bypass DLL providing MAC based internal TX/RX delays */
>>   cfg = readq_relaxed(xcv->reg_base + XCV_DLL_CTL);
>> - cfg &= ~0xFF03;
>> - cfg |= CLKRX_BYP;
>> + cfg &= ~0x00;
>> + switch (phy_mode) {
>> + /* RX and TX delays are added by the MAC */
>> + case PHY_INTERFACE_MODE_RGMII:
>> + break;
>> + /* internal RX and TX delays provided by the PHY */
>> + case PHY_INTERFACE_MODE_RGMII_ID:
>> + cfg |= CLKRX_BYP;
>> + cfg |= CLKTX_BYP;
>> + break;
>> + /* internal RX delay provided by the PHY, the MAC
>> +  * should not add an RX delay in this case
>> +  */
>> + case PHY_INTERFACE_MODE_RGMII_RXID:
>> + cfg |= CLKRX_BYP;
>> + break;
>> + /* internal TX delay provided by the PHY, the MAC
>> +  * should not add an TX delay in this case
>> +  */
>> + case PHY_INTERFACE_MODE_RGMII_TXID:
>> + cfg |= CLKRX_BYP;
>> + break;
>> + }
>
> Hi Tim
>
> This i don't get. Normally, you leave the PHY to handle delays, if
> needed. The MAC should not add any. Here you seem to assume a delay is
> always needed, and if the PHY is not providing it, the MAC should.
>
>Andrew

Andrew,

The thunder RGX inserts a delay via an on-board DLL. The 'bypass'
register will bypass this DLL and not insert a delay from the MAC
side. By default out of reset CLKTX_BYP=1 causing the RGX transmit
interface to not introduce a delay and CLKRX_BYP=0 causing the RGX
receive interface to introduce a delay.

The current code assumes the opposite setting CLKRX_BYP and clearing
CLKTX_BYP such that the RGX interface introduces a TX delay but not RX
which would be appropriate for
PHY_INTERFACE_MODE_RGMII_RXID/rgmii-txid (right, or is my logic there
backwards?). I may have my commit msg wrong in this case.

At any rate, I've got a board where the phy provides both TX/RX delay
and thus I don't want the RGX to insert any delays which means I need
to set both CLKRX_BYP and CLKTX_BYP which the driver currently doesn't
support.

Tim

thunderx sgmii interface hang

[Tim Harvey]

Greetings,

We are experiencing an issue on a CN80XX with an SGMII interface
coupled to a TI DP83867IS phy. We have the same PHY connected to the
RGMII interface on the same board design and everything is working as
expected on that nic both before and after triggering the hang.

The nic appears to work fine (pings, TCP etc) up until a performance
test is attempted.
When an iperf bandwidth test is attempted the nic ends up in a state
where truncated-ip packets are being sent out (per a tcpdump from
another board):

2016-02-11 16:40:23.996660 IP truncated-ip - 1454 bytes missing! (tos
0x0, ttl 64, id 39570, offset 0, flags [DF], proto TCP (6), length
1500, bad cksum 172a (->7033)!)
192.168.1.5.0 > 192.168.168.0.0:  tcp 1480 [bad hdr length 0 - too
short, < 20]

Prior to 'net: thunderx: Fix BGX transmit stall due to underflow'
unplugging the cable and re-plugging would resolve the issue to a
point where it could no longer be created. Prior to this patch a link
status change would disable and re-enable the BGX so perhaps that
helps shed some light on what's going on.

I'm using 4.14.4 with the following patches (although the issue
existed with 4.14.0 as well):
2615c91 net: thunderx: Fix TCP/UDP checksum offload for IPv4 pkts
763d8b3 net: thunderx: Fix TCP/UDP checksum offload for IPv6 pkts
93b2e67 net: thunderx: fix double free error

The issue persists regardless of the DP83867 PHY driver being in the
kernel. Any ideas or details that can help troubleshoot this?

Best Regards,

Tim

[PATCH] net: thunderx: add support for rgmii internal delay

[Tim Harvey]

The XCV_DLL_CTL is being configured with the assumption that
phy-mode is rgmii-txid (PHY_INTERFACE_MODE_RGMII_TXID) which is not always
the case.

This patch parses the phy-mode property and uses it to configure CXV_DLL_CTL
properly.

Signed-off-by: Tim Harvey <thar...@gateworks.com>
---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 13 +++---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.h |  2 +-
 drivers/net/ethernet/cavium/thunder/thunder_xcv.c | 31 ++-
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c 
b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 5e5c4d7..805c02a 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -55,6 +55,7 @@ struct bgx {
struct pci_dev  *pdev;
boolis_dlm;
boolis_rgx;
+   int phy_mode;
 };
 
 static struct bgx *bgx_vnic[MAX_BGX_THUNDER];
@@ -841,12 +842,12 @@ static void bgx_poll_for_link(struct work_struct *work)
queue_delayed_work(lmac->check_link, >dwork, HZ * 2);
 }
 
-static int phy_interface_mode(u8 lmac_type)
+static int phy_interface_mode(struct bgx *bgx, u8 lmac_type)
 {
if (lmac_type == BGX_MODE_QSGMII)
return PHY_INTERFACE_MODE_QSGMII;
if (lmac_type == BGX_MODE_RGMII)
-   return PHY_INTERFACE_MODE_RGMII;
+   return bgx->phy_mode;
 
return PHY_INTERFACE_MODE_SGMII;
 }
@@ -912,7 +913,8 @@ static int bgx_lmac_enable(struct bgx *bgx, u8 lmacid)
 
if (phy_connect_direct(>netdev, lmac->phydev,
   bgx_lmac_handler,
-  phy_interface_mode(lmac->lmac_type)))
+  phy_interface_mode(bgx,
+ lmac->lmac_type)))
return -ENODEV;
 
phy_start_aneg(lmac->phydev);
@@ -1287,6 +1289,8 @@ static int bgx_init_of_phy(struct bgx *bgx)
bgx->lmac[lmac].lmacid = lmac;
 
phy_np = of_parse_phandle(node, "phy-handle", 0);
+   if (phy_np)
+   bgx->phy_mode = of_get_phy_mode(phy_np);
/* If there is no phy or defective firmware presents
 * this cortina phy, for which there is no driver
 * support, ignore it.
@@ -1390,7 +1394,6 @@ static int bgx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
bgx->max_lmac = 1;
bgx->bgx_id = MAX_BGX_PER_CN81XX - 1;
bgx_vnic[bgx->bgx_id] = bgx;
-   xcv_init_hw();
}
 
/* On 81xx all are DLMs and on 83xx there are 3 BGX QLMs and one
@@ -1407,6 +1410,8 @@ static int bgx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
if (err)
goto err_enable;
 
+   if (bgx->is_rgx)
+   xcv_init_hw(bgx->phy_mode);
bgx_init_hw(bgx);
 
/* Enable all LMACs */
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h 
b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
index 23acdc5..2bba9d1 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
@@ -226,7 +226,7 @@ void bgx_lmac_internal_loopback(int node, int bgx_idx,
 void bgx_lmac_get_pfc(int node, int bgx_idx, int lmacid, void *pause);
 void bgx_lmac_set_pfc(int node, int bgx_idx, int lmacid, void *pause);
 
-void xcv_init_hw(void);
+void xcv_init_hw(int phy_mode);
 void xcv_setup_link(bool link_up, int link_speed);
 
 u64 bgx_get_rx_stats(int node, int bgx_idx, int lmac, int idx);
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_xcv.c 
b/drivers/net/ethernet/cavium/thunder/thunder_xcv.c
index 578c7f8..7e0c4cb 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_xcv.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_xcv.c
@@ -65,7 +65,7 @@ MODULE_LICENSE("GPL v2");
 MODULE_VERSION(DRV_VERSION);
 MODULE_DEVICE_TABLE(pci, xcv_id_table);
 
-void xcv_init_hw(void)
+void xcv_init_hw(int phy_mode)
 {
u64  cfg;
 
@@ -81,12 +81,31 @@ void xcv_init_hw(void)
/* Wait for DLL to lock */
msleep(1);
 
-   /* Configure DLL - enable or bypass
-* TX no bypass, RX bypass
-*/
+   /* enable/bypass DLL providing MAC based internal TX/RX delays */
cfg = readq_relaxed(xcv->reg_base + XCV_DLL_CTL);
-   cfg &= ~0xFF03;
-   cfg |= CLKRX_BYP;
+   cfg &= ~0x00;
+   switch (phy_mode) {
+   /* RX and TX delays are added by the MAC */
+   case PHY_INTERFACE_MODE_RGMII:
+   break;
+   /* internal RX and TX delays provided by the PHY */
+   case PHY_INTERFACE_MODE_RGMII_ID:
+  

Re: DSA mv88e6xxx RX frame errors and TCP/IP RX failure

[Tim Harvey]

On Wed, Aug 30, 2017 at 6:46 PM, Andrew Lunn  wrote:
>> >/* Report late collisions as a frame error. */
>> > if (status & (BD_ENET_RX_NO | BD_ENET_RX_CL))
>> > ndev->stats.rx_frame_errors++;
>> >
>> > I don't see anywhere else frame errors are counted, but it would be
>> > good to prove we are looking in the right place.
>> >
>>
>> Andrew,
>>
>> (adding IMX FEC driver maintainer to CC)
>>
>> Yes, that's one of them being hit. It looks like ifconfig reports
>> 'frame' as the accumulation of a few stats so here are some more
>> specifics from /sys/class/net/eth0/statistics:
>>
>> root@xenial:/sys/devices/soc0/soc/210.aips-bus/2188000.ethernet/net/eth0/statistics#
>> for i in `ls rx_*`; do echo $i:$(cat $i); done
>> rx_bytes:103229
>> rx_compressed:0
>> rx_crc_errors:22
>> rx_dropped:0
>> rx_errors:22
>> rx_fifo_errors:0
>> rx_frame_errors:22
>> rx_length_errors:22
>> rx_missed_errors:0
>> rx_nohandler:0
>> rx_over_errors:0
>> rx_packets:1174
>> root@xenial:/sys/devices/soc0/soc/210.aips-bus/2188000.ethernet/net/eth0/statistics#
>> ifconfig eth0
>> eth0  Link encap:Ethernet  HWaddr 00:D0:12:41:F3:E7
>>   inet6 addr: fe80::2d0:12ff:fe41:f3e7/64 Scope:Link
>>   UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>>   RX packets:1207 errors:22 dropped:0 overruns:0 frame:66
>>   TX packets:42 errors:0 dropped:0 overruns:0 carrier:0
>>   collisions:0 txqueuelen:1000
>>   RX bytes:106009 (103.5 KiB)  TX bytes:4604 (4.4 KiB)
>>
>> Instrumenting fec driver I see the following getting hit:
>>
>> status & BD_ENET_RX_LG /* rx_length_errors: Frame too long */
>> status & BD_ENET_RX_CR  /* rx_crc_errors: CRC Error */
>> status & BD_ENET_RX_CL /* rx_frame_errors: Collision? */
>>
>> Is this a frame size issue where the MV88E6176 is sending frames down
>> that exceed the MTU because of headers added?
>
> I did fix an issue recently with that. See
>
> commit fbbeefdd21049fcf9437c809da3828b210577f36
> Author: Andrew Lunn 
> Date:   Sun Jul 30 19:36:05 2017 +0200
>
> net: fec: Allow reception of frames bigger than 1522 bytes
>
> The FEC Receive Control Register has a 14 bit field indicating the
> longest frame that may be received. It is being set to 1522. Frames
> longer than this are discarded, but counted as being in error.
>
> When using DSA, frames from the switch has an additional header,
> either 4 or 8 bytes if a Marvell switch is used. Thus a full MTU frame
> of 1522 bytes received by the switch on a port becomes 1530 bytes when
> passed to the host via the FEC interface.
>
> Change the maximum receive size to 2048 - 64, where 64 is the maximum
> rx_alignment applied on the receive buffer for AVB capable FEC
> cores. Use this value also for the maximum receive buffer size. The
> driver is already allocating a receive SKB of 2048 bytes, so this
> change should not have any significant effects.
>
> Tested on imx51, imx6, vf610.
>
> Signed-off-by: Andrew Lunn 
> Signed-off-by: David S. Miller 
>
>
> However, this is was of an all/nothing problem. All frames with the
> full MTU were getting dropped, where as i think you are only seeing a
> few dropped?
>

Andrew,

Indeed this does resolve the issue. I see a burst of FIFO overruns
initially when receiving an iperf bandwidth test but that would be
caused by the IMX6 errata and should be mitigated via pause frames.
After that short burst I see no other errors and iperf works fine.

Should we get this patch in the linux-stable tree for the 4.9 kernel?

Thanks!

Tim

Re: DSA mv88e6xxx RX frame errors and TCP/IP RX failure

[Tim Harvey]

On Wed, Aug 30, 2017 at 3:06 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Wed, Aug 30, 2017 at 12:53:56PM -0700, Tim Harvey wrote:
>> Greetings,
>>
>> I'm seeing RX frame errors when using the mv88e6xxx DSA driver on
>> 4.13-rc7. The board I'm using is a GW5904 [1] which has an IMX6 FEC
>> MAC (eth0) connected via RGMII to a MV88E6176 with its downstream
>> P0/P1/P2/P3 to front panel RJ45's (lan1-lan4).
>
> Hi Tim
>
> Can you confirm the counter is this one:
>
>/* Report late collisions as a frame error. */
> if (status & (BD_ENET_RX_NO | BD_ENET_RX_CL))
> ndev->stats.rx_frame_errors++;
>
> I don't see anywhere else frame errors are counted, but it would be
> good to prove we are looking in the right place.
>

Andrew,

(adding IMX FEC driver maintainer to CC)

Yes, that's one of them being hit. It looks like ifconfig reports
'frame' as the accumulation of a few stats so here are some more
specifics from /sys/class/net/eth0/statistics:

root@xenial:/sys/devices/soc0/soc/210.aips-bus/2188000.ethernet/net/eth0/statistics#
for i in `ls rx_*`; do echo $i:$(cat $i); done
rx_bytes:103229
rx_compressed:0
rx_crc_errors:22
rx_dropped:0
rx_errors:22
rx_fifo_errors:0
rx_frame_errors:22
rx_length_errors:22
rx_missed_errors:0
rx_nohandler:0
rx_over_errors:0
rx_packets:1174
root@xenial:/sys/devices/soc0/soc/210.aips-bus/2188000.ethernet/net/eth0/statistics#
ifconfig eth0
eth0  Link encap:Ethernet  HWaddr 00:D0:12:41:F3:E7
  inet6 addr: fe80::2d0:12ff:fe41:f3e7/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:1207 errors:22 dropped:0 overruns:0 frame:66
  TX packets:42 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000
  RX bytes:106009 (103.5 KiB)  TX bytes:4604 (4.4 KiB)

Instrumenting fec driver I see the following getting hit:

status & BD_ENET_RX_LG /* rx_length_errors: Frame too long */
status & BD_ENET_RX_CR  /* rx_crc_errors: CRC Error */
status & BD_ENET_RX_CL /* rx_frame_errors: Collision? */

Is this a frame size issue where the MV88E6176 is sending frames down
that exceed the MTU because of headers added?

Tim

DSA mv88e6xxx RX frame errors and TCP/IP RX failure

[Tim Harvey]

Greetings,

I'm seeing RX frame errors when using the mv88e6xxx DSA driver on
4.13-rc7. The board I'm using is a GW5904 [1] which has an IMX6 FEC
MAC (eth0) connected via RGMII to a MV88E6176 with its downstream
P0/P1/P2/P3 to front panel RJ45's (lan1-lan4).

What I see is the following:
- bring up eth0/lan1
- DHCP ipv4 on lan1
- iperf client to server on network connected to lan1 shows ~150mbps
TX without any errors/overruns/frame but 10 or so dropped
- iperf server with a 100mbps TCP client test shows
- iperf server will hang when connected to from iperf client on lan1
network and I see frame errors from ifconfig:

root@xenial:/# ifconfig lan1
lan1  Link encap:Ethernet  HWaddr 00:D0:12:41:F3:E7
  inet addr:172.24.22.125  Bcast:172.24.255.255  Mask:255.240.0.0
  inet6 addr: fe80::2d0:12ff:fe41:f3e7/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:148 errors:0 dropped:30 overruns:0 frame:0
  TX packets:15 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000
  RX bytes:8780 (8.5 KiB)  TX bytes:1762 (1.7 KiB)

root@xenial:/# ifconfig eth0
eth0  Link encap:Ethernet  HWaddr 00:D0:12:41:F3:E7
  inet6 addr: fe80::2d0:12ff:fe41:f3e7/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:386 errors:19 dropped:39 overruns:0 frame:57
  TX packets:24 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000
  RX bytes:39484 (38.5 KiB)  TX bytes:2880 (2.8 KiB)

It doesn't appear that this is a new issue as it exists on also on
older kernels.

Note that the IMX6 has an errata (ERR004512) [2] that limits the
theoretical max performance of the FEC to 470mbps (total TX+RX) and if
the TX and RK peak datarate is higher than ~400mps there is a risk of
ENET RX FIFO overrun but I don't think this is the issue here. It
would be the cause of the relatively low throughput of ~150 TX though
I would assume.

Best Regards,

Tim

[1] - 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/imx6qdl-gw5904.dtsi
[2] - http://cache.nxp.com/docs/en/errata/IMX6DQCE.pdf - ERR004512

Re: IMX6 FEC connection drops occasionally with 'MDIO read timeout'

[Tim Harvey]

On Wed, Apr 12, 2017 at 9:26 AM, Fabio Estevam <feste...@gmail.com> wrote:
> Hi Tim,
>
> On Wed, Apr 12, 2017 at 1:15 PM, Tim Harvey <thar...@gateworks.com> wrote:
>>
>> Andrew,
>>
>> Thanks for the reply. Your talking about suspend/resume power
>> management right? The users reporting this were not using
>> suspend/resume.
>>
>> With regards to clock are you talking about the IPG clock? Is there
>> any other way that would get turned off other than fec suspend/resume?
>
> Yes, through pm_runtime.
>
> Can you check if this quick debug change help?
>
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3606,8 +3606,6 @@ static int __maybe_unused
> fec_runtime_suspend(struct device *dev)
> struct net_device *ndev = dev_get_drvdata(dev);
> struct fec_enet_private *fep = netdev_priv(ndev);
>
> -   clk_disable_unprepare(fep->clk_ipg);
> -
> return 0;
>  }
>
>
> If you don't see the problem with it, then it means we need to fix the
> pm runtime support in this driver.
>
> Most likely pm runtime is turning off the clocks when it should not.

Fabio,

Ok, I understand now. We will disable the IPG clock disable and see if
that makes a difference.

Tim

Re: IMX6 FEC connection drops occasionally with 'MDIO read timeout'

[Tim Harvey]

On Sun, Apr 9, 2017 at 6:42 PM, Andy Duan <fugang.d...@nxp.com> wrote:
> From: Tim Harvey <thar...@gateworks.com> Sent: Friday, April 07, 2017 10:47 PM
>>To: netdev <netdev@vger.kernel.org>
>>Cc: Fabio Estevam <fabio.este...@nxp.com>; Lucas Stach
>><l.st...@pengutronix.de>; Andy Duan <fugang.d...@nxp.com>; Koen
>>Vandeputte <koen.vandepu...@ncentric.com>
>>Subject: IMX6 FEC connection drops occasionally with 'MDIO read timeout'
>>
>>Greetings,
>>
>>I've had a couple of users report that they are getting occasional link drops
>>when using IMX6 FEC on our boards which are using a Marvell
>>88E1510 PHY:
>>
>>[ 9348.474418] fec 2188000.ethernet eth0: MDIO read timeout [ 9350.478804]
>>fec 2188000.ethernet eth0: Link is Down
>>
>>If they bring the interface back up it works fine.
>>
>>The most recent version of Linux they have tested and seen this on is
>>v4.9 with CONFIG_MARVELL_PHY=y
>>
>>Has anyone else run into this or know what the issue could be? Should there
>>be any retries at the MDIO level or above?
>>
>>Regards,
>>
>>Tim
>
> Pls keep ipg clock always on and try it.
> In general, MDIO timeout maybe caused by ipg clock off and mii interrupt lost.
>

Andy,

Sounds like what Andrew was talking about. In this case suspend/resume
is not being used. Is there any other way IPG can get turned off that
I should be looking at?

Tim

Re: IMX6 FEC connection drops occasionally with 'MDIO read timeout'

[Tim Harvey]

On Fri, Apr 7, 2017 at 9:09 AM, Andrew Lunn <and...@lunn.ch> wrote:
> On Fri, Apr 07, 2017 at 07:47:17AM -0700, Tim Harvey wrote:
>> Greetings,
>>
>> I've had a couple of users report that they are getting occasional
>> link drops when using IMX6 FEC on our boards which are using a Marvell
>> 88E1510 PHY:
>>
>> [ 9348.474418] fec 2188000.ethernet eth0: MDIO read timeout
>> [ 9350.478804] fec 2188000.ethernet eth0: Link is Down
>>
>> If they bring the interface back up it works fine.
>>
>> The most recent version of Linux they have tested and seen this on is
>> v4.9 with CONFIG_MARVELL_PHY=y
>>
>> Has anyone else run into this or know what the issue could be? Should
>> there be any retries at the MDIO level or above?
>
> Hi Tim
>
> MDIO is assumed to be reliable. It failing is bad.
>
> However, ETIMEOUT is interesting. The MDIO protocol has no flow
> control. There is no way for the PHY to delay replying.
>
> I would suggest you look at the power management code. What i have
> seen with the FEC, when using switches attached to the FEC, is that if
> the clocks are turned off, you get ETIMEOUT from MDIO.
>
> Andrew

Andrew,

Thanks for the reply. Your talking about suspend/resume power
management right? The users reporting this were not using
suspend/resume.

With regards to clock are you talking about the IPG clock? Is there
any other way that would get turned off other than fec suspend/resume?

Tim

IMX6 FEC connection drops occasionally with 'MDIO read timeout'

[Tim Harvey]

Greetings,

I've had a couple of users report that they are getting occasional
link drops when using IMX6 FEC on our boards which are using a Marvell
88E1510 PHY:

[ 9348.474418] fec 2188000.ethernet eth0: MDIO read timeout
[ 9350.478804] fec 2188000.ethernet eth0: Link is Down

If they bring the interface back up it works fine.

The most recent version of Linux they have tested and seen this on is
v4.9 with CONFIG_MARVELL_PHY=y

Has anyone else run into this or know what the issue could be? Should
there be any retries at the MDIO level or above?

Regards,

Tim

Re: device-tree support for writing to phy registers?

[Tim Harvey]

On Fri, Sep 23, 2016 at 9:29 AM, Florian Fainelli <f.faine...@gmail.com> wrote:
> On 09/23/2016 08:40 AM, Tim Harvey wrote:
>> Greetings,
>>
>> I've got a TI DP83867 GbE phy that requires some register writes to
>> configure its refclock output. Is there a generic device-tree API for
>> writing to raw registers or is that something that would be need to be
>> added to a specific phy driver with a device-tree binding?
>
> There are no standard properties that indicate how to write to register
> from Device Tree (unfortunately there are non standard that allow this
> to happen, e.g: marvell,reg-init), because that would mean that Device
> Tree acts as some kind of firmware/binary interface, which is a bit of
> stretch. Some bindings may indicate how to write to registers in a way
> that accepts a address = value pair, but quite frankly, this is
> absolutely horrible and not controllable nor easily transferable from
> one model of device to the other, strongly discouraged.
>
>> There is a
>> DP83867 phy driver but it doesn't contain anything related to
>> configuring its CLKOUT via register 0x170.
>
> Then, I guess you should add a set of properties and corresponding code
> reading these properties that would result in getting the register
> programmed with the values you need.
>

Florian,

agreed - this seems like the right thing to do and takes care of the
important detail about power-management you mention below.

Are there any phy drivers you know of that do and CLKOUT configuration
that I could use as inspiration for dt prop names?

Thanks,

Tim

>>
>> Alternatively, is it generally considered 'ok' to take care of this in
>> the bootloader and not provide the MAC driver the gpio for phy-reset
>> so that bootloader configuration persists through the kernel?
>
> It depends on what your platform does, punting on the bootloader is
> usually fine, but also breaks nicely when you start implementing power
> management in the kernel properly (e.g: deep sleep states) and you are
> not calling back into the bootloader, yet your hardware lost its state
> between power transitions.
>
> --
> Florian

device-tree support for writing to phy registers?

[Tim Harvey]

Greetings,

I've got a TI DP83867 GbE phy that requires some register writes to
configure its refclock output. Is there a generic device-tree API for
writing to raw registers or is that something that would be need to be
added to a specific phy driver with a device-tree binding? There is a
DP83867 phy driver but it doesn't contain anything related to
configuring its CLKOUT via register 0x170.

Alternatively, is it generally considered 'ok' to take care of this in
the bootloader and not provide the MAC driver the gpio for phy-reset
so that bootloader configuration persists through the kernel?

Regards,

Tim

Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards

[Tim Harvey]

On Tue, Dec 8, 2015 at 9:50 PM, Krzysztof Hałasa <khal...@piap.pl> wrote:
> David Miller <da...@davemloft.net> writes:
>
>> Since this only touched DT files in the ARM platform code, maybe
>> the ARM tree is the best path for this patch rather than mine?
>
> I think so.
>
> Tim, would you like to handle this patch yourself, or should I send it
> to the ARM contacts?

Krzysztof,

Go ahead and send it. You can add my:

Acked-by: Tim Harvey <thar...@gateworks.com>

I know Shawn would be happy to pick this up as well as he has an
imx-fixes tree:
http://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/log/?h=imx/soc

Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards

[Tim Harvey]

On Tue, Dec 8, 2015 at 6:13 AM, Krzysztof Hałasa <khal...@piap.pl> wrote:
> Tim Harvey <thar...@gateworks.com> writes:
>
>> It sounds like your saying this controls whether the phy is in charge
>> of delay vs the MAC. I have never needed to set this and haven't found
>> where its actually used (in at least 4.3). Is this caused by something
>> new in the kernel I haven't seen yet or is it possible you have board
>> that has an Ethernet issue?
>
> I think you aren't using the Marvell PHY driver :-)
> The existing DTS files work fine with the "default" PHY driver.
> It's only the Marvell driver (drivers/net/phy/marvell.c, aka
> CONFIG_MARVELL_PHY) which have the issue.
>

Kryzsztof,

True - I have not used CONFIG_MARVELL_PHY and didn't know of its existance.

> The problem is caused by the following code in m88e1121_config_aneg():
>
> if (phy_interface_is_rgmii(phydev)) {
>
> mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
> MII_88E1121_PHY_MSCR_DELAY_MASK;
>
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
>  MII_88E1121_PHY_MSCR_TX_DELAY);
> else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
> else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
>
> err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
> if (err < 0)
> return err;
> }
>
> mscr initially holds the correct value (one with RX_DELAY and TX_DELAY
> bits set). Since the _*ID are not specified, these bits are reset and
> (now incorrect) value is written back to the register.
> OTOH the generic PHY driver knows nothing about the MSCR and thus
> doesn't "corrupt" it.
>
> The whole -ID thing is about signal trace lengths (on PCB), where the
> non-ID setup requires special, longer traces for certain signals to
> compensate for propagation times etc. The -ID version does it internally
> (when enabled) and the PCB layout can be a bit simpler.

right - I understand what the delay lines are, it just struck me as
strange that I've never needed this.

>
> To be honest I only checked it on a single GW5400 rev. C, but I'd expect
> it to be exactly the same across all GW5[234]*. Can test on GW5[23] if
> there is any doubt (in few days I guess).
>

no doubt now. I re-created your config and see the problem as well.

The marvell phy driver (drivers/net/phy/marvell.c) has a very lacking
Kconfig stating its a PHY driver for the 88E1011S when in fact it has
id's for 12 ID's including the 88E1510 we use on the GW51xx, GW52xx,
GW53xx, GW54xx.

This may be a stupid question but what does the Marvell phy driver
offer that the generic phy driver does not?

Thanks for finding this and submitting a patch!

Acked-by: Tim Harvey <thar...@gateworks.com>

Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] I.MX6: Fix Ethernet PHY mode on Ventana boards

[Tim Harvey]

On Mon, Dec 7, 2015 at 4:56 AM, Krzysztof Hałasa <khal...@piap.pl> wrote:
> Gateworks Ventana boards seem to need "RGMII-ID" (internal delay)
> PHY mode, instead of simple "RGMII", for their Marvell 88E1510
> transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY
> driver (TX doesn't seem to work correctly).
>
> Tested on GW5400 rev. C.
>
> This bug affects ARM Fedora 23.
>
> Signed-off-by: Krzysztof Hałasa <khal...@piap.pl>
>
> diff --git a/arch/arm/boot/dts/imx6q-gw5400-a.dts 
> b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> index 822ffb2..6c168dc 100644
> --- a/arch/arm/boot/dts/imx6q-gw5400-a.dts
> +++ b/arch/arm/boot/dts/imx6q-gw5400-a.dts
> @@ -154,7 +154,7 @@
>   {
> pinctrl-names = "default";
> pinctrl-0 = <_enet>;
> -   phy-mode = "rgmii";
> +   phy-mode = "rgmii-id";
> phy-reset-gpios = < 30 GPIO_ACTIVE_HIGH>;
> status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi 
> b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> index f2867c4..90496aa 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
> @@ -94,7 +94,7 @@
>   {
> pinctrl-names = "default";
> pinctrl-0 = <_enet>;
> -   phy-mode = "rgmii";
> +   phy-mode = "rgmii-id";
> phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
> status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi 
> b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> index 4493f6e..0a6730b 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> @@ -154,7 +154,7 @@
>   {
> pinctrl-names = "default";
> pinctrl-0 = <_enet>;
> -   phy-mode = "rgmii";
> +   phy-mode = "rgmii-id";
> phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
> status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi 
> b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> index cfad214..2c549ad 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
> @@ -174,7 +174,7 @@
>   {
> pinctrl-names = "default";
> pinctrl-0 = <_enet>;
> -   phy-mode = "rgmii";
> +   phy-mode = "rgmii-id";
> phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
> status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi 
> b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> index 535b536..b4ea087 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
> @@ -164,7 +164,7 @@
>   {
> pinctrl-names = "default";
> pinctrl-0 = <_enet>;
> -   phy-mode = "rgmii";
> +   phy-mode = "rgmii-id";
> phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
>     status = "okay";
>  };
>
> --
> Krzysztof Halasa
>
> Industrial Research Institute for Automation and Measurements PIAP
> Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Krzysztof,

It sounds like your saying this controls whether the phy is in charge
of delay vs the MAC. I have never needed to set this and haven't found
where its actually used (in at least 4.3). Is this caused by something
new in the kernel I haven't seen yet or is it possible you have board
that has an Ethernet issue?

Regards,

Tim

Tim Harvey - Principal Software Engineer
Gateworks Corporation - http://www.gateworks.com/
3026 S. Higuera St. San Luis Obispo CA 93401
805-781-2000
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: DSA driver - how to glue to a PCI based NIC's mdio?

[Tim Harvey]

On Wed, Sep 30, 2015 at 2:40 PM, Andrew Lunn  wrote:
>> > information to the NIC's device driver. Better would be to have a
>> > small shim driver which is loaded on your PCI_ID/DEVICE_ID. That would
>> > instantiate the NIC driver, and insert a DSA platform device.
>>
>> I was thinking of this as well, but then I would still need that shim
>> to know the netdevice that the driver I'm shimming creates so I can't
>> figure a way to do it without touching the PCI driver.
>
> You can probably get that from the PCI information you have:
>
> # /sys/devices/pci:00/:00:1c.2/:06:00.0 is the PCIe device. And 
> it has
>
> # /sys/devices/pci:00/:00:1c.2/:06:00.0/net$ ls -l
> total 0
> drwxr-xr-x 5 root root 0 Aug 19 18:23 eth0
>
> So the linking is there.
>
>Andrew

Andrew,

I read over the thread in April (https://lkml.org/lkml/2015/4/21/195)
regarding a USB Ethernet device with a switch attached as it discusses
a similar issue to mine where you do not have device-tree handles to
the mii bus or netdev and the device is enumerated. It does not seem
that any resolution was found there.

Do you know of any example of a 'shim' for a PCI device driver? I was
thinking you were talking about a PCI fixup or something that operated
on the device at enumeration time but in order to get hold of the
associated netdev I would need to instantiate the original NIC drive
then somehow get its mii_bus and netdev after its probe completed. I'm
not clear how to do that. How can I from my own PCI device driver, get
another PCI device driver's probe to fire, and following that, how
could I find that devices netdev and mdio_bus structures?

Another possibility discussed in that thread, that I have thought of
as well is doing this through a phylib driver. My device also has some
phy's hanging off two of the MV88E6xxx switch ports that need some
configuration so I 'already' have a phylib driver. However I haven't
figured out how to get hold of the netdev in order to register a DSA
platform device from there. The phylib probe function passes a
phy_device struct which gives me the mdio bus, but at that point the
attached_dev is NULL and remains that way until the NIC is brought up.
Currently I have a workqueue firing at 1Hz that checks for
fiber/copper/SFP state changes and in there I can check to see if the
phy_device has an attached_dev and when it does I can register a DSA
platform device and that works with the caviot that you have to bring
up the NIC that is attached to the switch in order for DSA to probe.

Regards,

Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

DSA driver - how to glue to a PCI based NIC's mdio?

[Tim Harvey]

Greetings,

I'm working on adding DSA support for a PCIe expansion card (designed
by us) that has common PCIe NIC connected via its mii-bus to a Marvell
MV88E6171. Because the NIC is a PCIe device, it has no device-tree
representation of its NIC or its mdio bus, but does register its mdio
bus with Linux.

Does anyone know how I would be able to specify the Ethernet device
and MDIO bus for this add-in card if it has no device-tree handle?

I can write a phy driver that gets probed by matching the MV88E6171
PHY ID when the NIC's mdio bus get's registered yet even then I'm not
clear how to get hold of the netdev pointer given the mdio bus so as
to build a dsa_chip_data struct to register as a platform device. I'm
also not clear in this case how to verify in this case if the
MV88E6171 is from 'my' add-in card vs another card.

Perhaps the right approach is to program the NIC's EEPROM on our board
with a PCI_ID/DEVICE_ID of ours, add support for those ID's to the
NIC's driver, and within the NIC's driver create and register dsa
platform device when our ID is encountered?

Thanks for any advise,

Tim

Tim Harvey - Principal Software Engineer
Gateworks Corporation - http://www.gateworks.com/
3026 S. Higuera St. San Luis Obispo CA 93401
805-781-2000
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: DSA driver - how to glue to a PCI based NIC's mdio?

[Tim Harvey]

On Wed, Sep 30, 2015 at 2:12 PM, Andrew Lunn <and...@lunn.ch> wrote:
> On Wed, Sep 30, 2015 at 01:44:52PM -0700, Tim Harvey wrote:
>> Greetings,
>>
>> I'm working on adding DSA support for a PCIe expansion card (designed
>> by us) that has common PCIe NIC connected via its mii-bus to a Marvell
>> MV88E6171. Because the NIC is a PCIe device, it has no device-tree
>> representation of its NIC or its mdio bus, but does register its mdio
>> bus with Linux.
>
> It is possible to represent PCIe devices in device tree. Take a look
> at ePAPR. Is the PCIe host in DT?

It is possible to represent PCI devices in device-tree however not in
a dynamic or plug-able fashion - they have to be nested per bus/slot
which defeats the purpose of dynamic enumeration.

>
>> Perhaps the right approach is to program the NIC's EEPROM on our board
>> with a PCI_ID/DEVICE_ID of ours, add support for those ID's to the
>> NIC's driver, and within the NIC's driver create and register dsa
>> platform device when our ID is encountered?
>
> This sounds sensible. But i doubt you can add your DSA platform
> information to the NIC's device driver. Better would be to have a
> small shim driver which is loaded on your PCI_ID/DEVICE_ID. That would
> instantiate the NIC driver, and insert a DSA platform device.

I was thinking of this as well, but then I would still need that shim
to know the netdevice that the driver I'm shimming creates so I can't
figure a way to do it without touching the PCI driver.

Tim
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html