Re: [PATCH 4/6] drivers: net: octeontx: fix QSGMII

On Fri, Mar 26, 2021 at 9:09 AM Suneel Garapati suneelglinux@gmail.com wrote:

This looks like a workaround than the root cause fix. As this patch just moves the bringup of link from probe stage to on-demand.

On Thu, Mar 25, 2021 at 11:46 PM Stefan Roese sr@denx.de wrote:

...

On 26.03.21 01:07, Tim Harvey wrote:

...

Revert a change that occured between the Marvell SDK-10.1.1.0 and SDK-10.3.1.1 which broke QSMII phy support.

Signed-off-by: Tim Harvey tharvey@gateworks.com

Thanks.

Suneel, do you have a comment on this? Is this revert the "best way" to handle this?

Thanks, Stefan

...

---

drivers/net/octeontx/bgx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index 2ea54be84d..a5c0c9fe2b 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -36,7 +36,6 @@ struct lmac { int dmac; u8 mac[6]; bool link_up;

• bool                    init_pend;
  int                     lmacid; /* ID within BGX */
  int                     phy_addr; /* ID on board */
  struct udevice          *dev;
  

@@ -849,6 +848,7 @@ static int bgx_lmac_enable(struct bgx *bgx, int8_t lmacid) u64 cfg;

  lmac = &bgx->lmac[lmacid];

• lmac->bgx = bgx;
  
  debug("%s: lmac: %p, lmacid = %d\n", __func__, lmac, lmacid);
  
  

@@ -895,16 +895,6 @@ int bgx_poll_for_link(int node, int bgx_idx, int lmacid) debug("%s: %d, lmac: %d/%d/%d %p\n", __FILE__, __LINE__, node, bgx_idx, lmacid, lmac);

• if (lmac->init_pend) {
  

•         ret = bgx_lmac_enable(lmac->bgx, lmacid);
  

•         if (ret < 0) {
  

•                 printf("BGX%d LMAC%d lmac_enable failed\n", bgx_idx,
  

•                        lmacid);
  

•                 return ret;
  

•         }
  

•         lmac->init_pend = 0;
  

•         mdelay(100);
  

• }
  if (lmac->qlm_mode == QLM_MODE_SGMII ||
      lmac->qlm_mode == QLM_MODE_RGMII ||
      lmac->qlm_mode == QLM_MODE_QSGMII) {
  

@@ -1461,6 +1451,7 @@ int octeontx_bgx_remove(struct udevice *dev)

int octeontx_bgx_probe(struct udevice *dev) {

• int err;
  struct bgx *bgx = dev_get_priv(dev);
  u8 lmac = 0;
  int qlm[4] = {-1, -1, -1, -1};
  

@@ -1540,8 +1531,11 @@ skip_qlm_config: struct lmac *tlmac = &bgx->lmac[lmac];

          tlmac->dev = dev;

•         tlmac->init_pend = 1;
  

•         tlmac->bgx = bgx;
  

•         err = bgx_lmac_enable(bgx, lmac);
  

•         if (err) {
  

•                 printf("BGX%d failed to enable lmac%d\n",
  

•                        bgx->bgx_id, lmac);
  

•         }
  }
  
  return 0;
  
  

Suneel,

I agree, it is a workaround and I don't quite understand the reason.

Can you look into the code history of the SDK BDK and see what the reason was for introducing this 'pending' state that defers the bgx_lmac_enable call that breaks QSGMII? That specific change was made between SDK-10.1.1.0 and SDK-10.3.1.1.

What boards and PHY's have you tested octeontx BGX with?

I have the following boards to test with that we designed and support: - GW630x: supports both RGMII and SGMII GbE PHY's - GW640x: supports both RGMII and QSGMII GbE PHy's

The only issue I have that is worked around by the above is QSGMII.

Best regards,

Tim