| Message ID | 20240220160406.3363002-2-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | doc: sfp-phylink: update the porting guide | expand |
On Tue, Feb 20, 2024 at 05:04:03PM +0100, Maxime Chevallier wrote: > commit 64b4a0f8b51b ("net: phylink: remove phylink_config's pcs_poll") > dropped the phylink_config->pcs_poll field, which is no longer required > to be set by MAC drivers. Remove that mention in the phylink porting > guide. The porting guide needs to be updated with the PCS, and the details for that moved over rather than being deleted. While it's true that this member is gone from phylink_config, it was only removed after the introduction of the similarly named member in phylink_pcs. In other words, point 10 should probably read: 10. Arrange for PCS link state interrupts to be forwarded into phylink, via: .. code-block:: c phylink_pcs_change(pcs, link_is_up); where ``link_is_up`` is true if the link is currently up or false otherwise. If a PCS is unable to provide these interrupts, then it should set ``pcs->pcs_poll = true;`` when creating the PCS. However, for that to make sense, we then need the guide to provide details about creating the PCS, and also the mac_select_pcs() method. Thus my comment about a much bigger update being required.
Hi Russell, On Mon, 26 Feb 2024 11:41:07 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Tue, Feb 20, 2024 at 05:04:03PM +0100, Maxime Chevallier wrote: > > commit 64b4a0f8b51b ("net: phylink: remove phylink_config's pcs_poll") > > dropped the phylink_config->pcs_poll field, which is no longer required > > to be set by MAC drivers. Remove that mention in the phylink porting > > guide. > > The porting guide needs to be updated with the PCS, and the details for > that moved over rather than being deleted. While it's true that this > member is gone from phylink_config, it was only removed after the > introduction of the similarly named member in phylink_pcs. > > In other words, point 10 should probably read: > > 10. Arrange for PCS link state interrupts to be forwarded into > phylink, via: > > .. code-block:: c > > phylink_pcs_change(pcs, link_is_up); > > where ``link_is_up`` is true if the link is currently up or false > otherwise. If a PCS is unable to provide these interrupts, then > it should set ``pcs->pcs_poll = true;`` when creating the PCS. > > However, for that to make sense, we then need the guide to provide > details about creating the PCS, and also the mac_select_pcs() method. > Thus my comment about a much bigger update being required. > Makes sense. If that's OK I'll send a V2 with a PCS handling addition to it, and see how that looks. Thanks for the review, Maxime
diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst index 8054d33f449f..bc3365bbf096 100644 --- a/Documentation/networking/sfp-phylink.rst +++ b/Documentation/networking/sfp-phylink.rst @@ -264,8 +264,7 @@ this documentation. phylink_mac_change(priv->phylink, link_is_up); where ``link_is_up`` is true if the link is currently up or false - otherwise. If a MAC is unable to provide these interrupts, then - it should set ``priv->phylink_config.pcs_poll = true;`` in step 9. + otherwise. 11. Verify that the driver does not call::
commit 64b4a0f8b51b ("net: phylink: remove phylink_config's pcs_poll") dropped the phylink_config->pcs_poll field, which is no longer required to be set by MAC drivers. Remove that mention in the phylink porting guide. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- Documentation/networking/sfp-phylink.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)