bnxt: Correct the logic to report supported speeds

[siva.kallam_broadcom.com]

Summary

phy_type UNKNOWN is treated as valid type and report auto if phy_type is unknown and log a debug message for user attention.
On Link up & down, update media types again. If Link state or media changes, user will observe correct supported media and speeds.
Adding debug message in default case for user attention.

Test Plan

If cable is not plugged in while driver load, making default supported speed as auto select.
Once cable is plugged in and on Link up, depend on PHY type populating the real supported speeds.
If Cable already plugged in while loading driver, populating supported speeds(This support already present).

[siva.kallam_broadcom.com]

Added proper comment for default case while checking phy_type

[shurd]

sys/dev/bnxt/if_bnxt.c

2064

What action is a user expected to take when seeing these two messages? Should the default case print the phy_type value as well?

If new phy types are added in the future, old versions of the driver will report "ERROR: Invalid Phy type", but it should work aside from control over the media type... would that really be an error?

This looks like the only place in the kernel that uses a "devX: INFORMATION: message" format... I don't think it's necessary to tell users that a log message contains information (the ERROR: prefix is fine if that actually indicates an error).

[siva.kallam_broadcom.com]

sys/dev/bnxt/if_bnxt.c

2064

Logging always gives hint for user if link is not up because of unhandled PHY type. Without this logging, User may get clue.
Yes. I agree with you in logging phy_type in default/UNKNOWN phy_type case.
Do you think unknown phy_type and default case should be clubbed and log single message as "UNKNOWN/INVALID phy type ID"?
I am OK to club both UNKNOWN and default case with above message.

[shurd]

sys/dev/bnxt/if_bnxt.c

2064

I think in the default case, a "Reported PHY type %d not supported by driver" style message would be best. For the HWRM_PORT_PHY_QCFG_OUTPUT_PHY_TYPE_UNKNOWN case, it really depends on exactly what that message means. If it means there is *no* PHY, a message to that effect may make sense, but that's not what the message actually says, and it's the wrong field to pull that information from.

I think that errors should be based on the module_status field, not the phy_type one. There's no clear indication in the documentation for phy_type that UNKNOWN indicates a cable is not plugged in.