igb: Add BCM54616 initialization code for I354 mgmt on Netberg Aurora 420

[sergeypopovich-ord]

Add BCM54616 initialization and "link UP" code for I354 used as management network interface on Netberg Aurora 420 and other devices.

This change has to be merged before sonic-net/sonic-buildimage#2400.

[lguohan]

we already have patch for support bcm phy 5461 for igb driver. why it does not work for you?

https://github.com/Azure/sonic-linux-kernel/blob/master/patch/0011-support-Broadcom-54616-Phy-for-Intel-igb-driver.patch

[sergeypopovich-ord]

Link just wont come up without this specific initialization, linkup code.

Yes, this potentially can broke things for others and if you consider not to merge this:
I can take copy of igb (either intel-out-of-tree or directly from kernel) and maintain own
copy of the driver as platform specific module.

[lguohan]

your patch does not seem to be aligned with linux kernel since there is already 5461S phy support in 4.9 kernel.

https://elixir.bootlin.com/linux/v4.9.147/source/drivers/net/phy/broadcom.c#L490

can you align you patch with the linux kernel approach to support broadcom phy.

we cannot accept you patch in current format since it creates lots of overhead to maintain this patch.

[sergeypopovich-ord]

Will try upstream patch then. Thank you for pointing.

[sergeypopovich-ord]

I have compared upstream kernel for common part between it and patch within this request
and found bcm54xx_config_init() is only common bit. Link setup process is unique.

Also igb seems implement phy code on it's own, not using libphy from kernel.

I agree that this change

1. makes overhead for maintenance
2. can potentially broke things for others

and I'm fine to decline this pull request, but I would like to ask how bad is to maintain own copy of igb in
platform specific module? I'm planning to use out-of-tree igb driver that is most portable across kernel versions.

I understand that on kernel version change we could get igb in our platform code failing to build
requiring maintenance.

Looking for better approach...

[lguohan]

patch 0011 is taken from a linux upstream commit https://patchwork.ozlabs.org/patch/799983/

we do have the same setup with igb mac and bcm 5461 phy in other platform, and the patch 0011 works for us. Wonder what is the difference that makes the patch 0011 not work for you?

[sergeypopovich-ord]

It seems we need special initialization here:

• case e1000_phy_bcm54616:

•   ret_val = 0;
  

•   break;
  

Upstream patch just silence error and does not initialize PHY like done for others.

[sergeypopovich-ord]

It seems I found root case of the problem: if there is no EEPROM (like our case) we need to manually initialize PHY.

I ported drivers/net/phy/broadcom.c code to initialize BCM54616S PHY among with routines required
to setup link and force speed/duplex as done for other PHYs (e.g. IGP3 from Marvell).

Now link comes up and becomes operational. I think this patch is subject for upstreaming to Linux Kernel.

[paulmenzel]

Did you ever sent this to Linux upstream?

[jarias-lfx]

/easycla

[linux-foundation-easycla]

• ❌ - login: @sergeypopovich-ord / name: Sergey Popovich . The commit (34b5061, eae4b44) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.