Hi everyone,
I'm currently in the process of porting fs_enet to phylink, and I'm
using this occasion to update the porting guide along the way. Let me
know if these changes are clear enough, and if there are any mistakes in
there.
Thanks,
Maxime
Maxime Chevallier (2):
doc: sfp-phylink: drop the mention to phylink_config->pcs_poll
doc: sfp-phylink: mention the mac_capabilities and
supported_interfaces
Documentation/networking/sfp-phylink.rst | 48 ++++++++++++++++++------
1 file changed, 37 insertions(+), 11 deletions(-)
--
2.43.2
When porting a driver from bare phylib to phylink, one of the mandatory
steps is to fill-in the phylink_config.mac_capabilities and the
supported_interfaces. Add a dedicated step in the porting guide, with
some examples.
Signed-off-by: Maxime Chevallier <[email protected]>
---
Documentation/networking/sfp-phylink.rst | 45 +++++++++++++++++++-----
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
index bc3365bbf096..30b1f2cf997f 100644
--- a/Documentation/networking/sfp-phylink.rst
+++ b/Documentation/networking/sfp-phylink.rst
@@ -231,16 +231,43 @@ this documentation.
For further information on these methods, please see the inline
documentation in :c:type:`struct phylink_mac_ops <phylink_mac_ops>`.
-9. Remove calls to of_parse_phandle() for the PHY,
- of_phy_register_fixed_link() for fixed links etc. from the probe
- function, and replace with:
+9. Fill-in the :c:type:`struct phylink_config <phylink_config>` fields with
+ a reference to the :c:type:`struct device <device>` associated to your
+ :c:type:`struct net_device <net_device>`:
.. code-block:: c
- struct phylink *phylink;
priv->phylink_config.dev = &dev.dev;
priv->phylink_config.type = PHYLINK_NETDEV;
+ Fill-in the various speeds, pause and duplex modes your MAC can handle:
+
+ .. code-block:: c
+
+ priv->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD;
+
+ Fill-in all the :c:type:`phy_interface_t <phy_interface_t>` (i.e. all MAC to
+ PHY link modes) that your MAC can output. The following example shows a
+ configuration for a MAC that can handle all RGMII modes, SGMII and 1000BaseX.
+ You must adjust these according to what your MAC is capable of, and not just
+ the interface you wish to use:
+
+ .. code-block:: c
+
+ phy_interface_set_rgmii(priv->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_SGMII,
+ priv->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_1000BASEX,
+ priv->phylink_config.supported_interfaces);
+
+10. Remove calls to of_parse_phandle() for the PHY,
+ of_phy_register_fixed_link() for fixed links etc. from the probe
+ function, and replace with:
+
+ .. code-block:: c
+
+ struct phylink *phylink;
+
phylink = phylink_create(&priv->phylink_config, node, phy_mode, &phylink_ops);
if (IS_ERR(phylink)) {
err = PTR_ERR(phylink);
@@ -249,14 +276,14 @@ this documentation.
priv->phylink = phylink;
- and arrange to destroy the phylink in the probe failure path as
- appropriate and the removal path too by calling:
+ and arrange to destroy the phylink in the probe failure path as
+ appropriate and the removal path too by calling:
- .. code-block:: c
+ .. code-block:: c
phylink_destroy(priv->phylink);
-10. Arrange for MAC link state interrupts to be forwarded into
+11. Arrange for MAC link state interrupts to be forwarded into
phylink, via:
.. code-block:: c
@@ -266,7 +293,7 @@ this documentation.
where ``link_is_up`` is true if the link is currently up or false
otherwise.
-11. Verify that the driver does not call::
+12. Verify that the driver does not call::
netif_carrier_on()
netif_carrier_off()
--
2.43.2
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 <[email protected]>
---
Documentation/networking/sfp-phylink.rst | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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::
--
2.43.2
Hello everyone,
Russell, did you get a chance to take a look at this series (maybe it
went under the radar, if not, sorry about the ping) ? I'd like to make
sure the examples are meaningful and correct, and you're clearly the
most qualified to comment on this :)
Best regards,
Maxime
On Tue, 20 Feb 2024 17:04:02 +0100
Maxime Chevallier <[email protected]> wrote:
> Hi everyone,
>
> I'm currently in the process of porting fs_enet to phylink, and I'm
> using this occasion to update the porting guide along the way. Let me
> know if these changes are clear enough, and if there are any mistakes in
> there.
>
> Thanks,
>
> Maxime
>
> Maxime Chevallier (2):
> doc: sfp-phylink: drop the mention to phylink_config->pcs_poll
> doc: sfp-phylink: mention the mac_capabilities and
> supported_interfaces
>
> Documentation/networking/sfp-phylink.rst | 48 ++++++++++++++++++------
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
On Mon, 26 Feb 2024 11:41:07 +0000
"Russell King (Oracle)" <[email protected]> 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
On Mon, 26 Feb 2024 11:45:03 +0000
"Russell King (Oracle)" <[email protected]> wrote:
> On Tue, Feb 20, 2024 at 05:04:04PM +0100, Maxime Chevallier wrote:
> > + Fill-in all the :c:type:`phy_interface_t <phy_interface_t>` (i.e. all MAC to
> > + PHY link modes) that your MAC can output. The following example shows a
>
> Technically, this should be "MAC and all PCS associated with this MAC".
>
Given that PCS should be covered in the doc, that's true :) I'll add
that in V2.
Thanks,
Maxime
On Tue, Feb 20, 2024 at 05:04:04PM +0100, Maxime Chevallier wrote:
> + Fill-in all the :c:type:`phy_interface_t <phy_interface_t>` (i.e. all MAC to
> + PHY link modes) that your MAC can output. The following example shows a
Technically, this should be "MAC and all PCS associated with this MAC".
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!