Re: [PATCH net-next v5 1/9] net: phylink: provide mac_get_pcs_neg_mode() function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 23/2/2024 2:58 pm, Voon, Weifeng wrote:
For instance, if the interface switches from 2500baseX to SGMII mode,
and the current link mode is MLO_AN_PHY, calling
'phylink_pcs_neg_mode'
would yield PHYLINK_PCS_NEG_OUTBAND. Since the MAC and PCS driver
require PHYLINK_PCS_NEG_INBAND_ENABLED, the
'mac_get_pcs_neg_mode'
function will calculate the mode based on the interface, current link
negotiation mode, and advertising link mode, returning
PHYLINK_PCS_NEG_OUTBAND to enable the PCS to configure the correct
settings.

This paragraph doesn't make sense - at least to me. It first talks about
requiring PHYLINK_PCS_NEG_INBAND_ENABLED when in SGMII mode. On
this:

The example given here is a very specific condition and that probably why there are some confusions here. Basically, this patch provides an optional function for MAC driver to change the phy interface on-the-fly without the need of reinitialize the Ethernet driver. As we know that the 2500base-x is messy, in our case the 2500base-x does not support inband. To complete the picture, we are using SGMII c37 to handle speed 10/100/1000. Hence, to enable user to switch link speed from 2500 to 1000/100/10 and vice versa on-the-fly, the phy interface need to be configured to inband SGMII for speed 10/100/1000, and outband 2500base-x for speed 2500. Lastly, the newly introduced "mac_get_pcs_neg_mode"callback function enables MAC driver to reconfigure pcs negotiation mode to inband or outband based on the interface mode, current link negotiation mode, and advertising link mode.


1) are you sure that the hardware can't be programmed for the SGMII
symbol repititions?


No, the HW can be program for SGMII symbol repetitions.

2) what happens if you're paired with a PHY (e.g. on a SFP module) which
uses SGMII but has no capability of providing the inband data?
(They do exist.) If your hardware truly does require inband data, it is going to
be fundamentally inoperative with these modules.


Above explanation should have already cleared your doubts. Inband or outband capability is configured based on the phy interface.

Next, you then talk about returning PHYLINK_PCS_NEG_OUTBAND for the
"correct settings". How does this relate to the first part where you basically
describe the problem as SGMII requring inband? Basically the two don't
follow.

It should be a typo mistake. SGMII should return PHYLINK_PCS_NEG_INBAND_ENABLED.


How, from a design point of view, because this fundamentally allows drivers
to change how the system behaves, it will allow radically different behaviours
for the same parameters between different drivers.
I am opposed to that - I want to see a situation where we have uniform
behaviour for the same configuration, and where hardware doesn't support
something, we have some way to indicate that via some form of capabilities.


Hi Russell,
If I understand you correctly, MAC driver should not interfere with pcs negotiation mode and it should be standardized in the generic function, e.g., phylink_pcs_neg_mode()?

The issue of whether 2500base-X has inband or not is a long standing issue,
and there are arguments (and hardware) that take totally opposing views on
this. There is hardware where 2500base-X inband _must_ be used or the link
doesn't come up. There is also hardware where 2500base-X inband is not
"supported" in documentation but works in practice. There is also hardware
where 2500base-X inband doesn't work. The whole thing is a total mess
(thanks IEEE 802.3 for not getting on top of this early enough... and what's
now stated in 802.3 for 2500base-X is now irrelevant because they were too
late to the
party.)


Agreed. And I have also seen some of your comments regarding the 2500SGMII and 2500BASEX.

Hi Russell,

Did the previous reply clear your doubt?

If we understand you correctly that MAC driver should not interfere with pcs negotiation mode and it should be standardized in the generic function. If implement what you suggested earlier that during interface mode change then update the `cur_link_an_mode` but not cfg_link_an_mode: https://patchwork.kernel.org/project/netdevbpf/patch/20230921121946.3025771-4-yong.liang.choong@xxxxxxxxxxxxxxx/?

Would that be a better solution?
So that 'phylink_pcs_neg_mode' function still can serve as the generic for all the drivers.

Do you have anything in mind that we can handle better for this patch series?
or the solution can be aligned with what you are going to implement in the future?




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux