diff mbox series

[net-next,v4,2/7] net: phylink: add rxc_always_on flag to phylink_pcs

Message ID 20240221-rxc_bugfix-v4-2-4883ee1cc7b1@bootlin.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Fix missing PHY-to-MAC RX clock | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1079 this patch: 1079
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 963 this patch: 963
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1107 this patch: 1107
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 94 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc fail Errors and warnings before: 0 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

Romain Gantois Feb. 21, 2024, 1:04 p.m. UTC
Some MAC drivers (e.g. stmmac) require a continuous receive clock signal to
be generated by a PCS that is handled by a standalone PCS driver.

Such a PCS driver does not have access to a PHY device, thus cannot check
the PHY_F_RXC_ALWAYS_ON flag. They cannot check max_requires_rxc in the
phylink config either, since it is a private member. Therefore, a new flag
is needed to signal to the PCS that it should keep the RX clock signal up
at all times.

Suggested-by: Russell King <linux@armlinux.org.uk>
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/net/phy/phylink.c | 14 ++++++++++++++
 include/linux/phylink.h   | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

Comments

Simon Horman Feb. 21, 2024, 7:07 p.m. UTC | #1
On Wed, Feb 21, 2024 at 02:04:19PM +0100, Romain Gantois wrote:
> Some MAC drivers (e.g. stmmac) require a continuous receive clock signal to
> be generated by a PCS that is handled by a standalone PCS driver.
> 
> Such a PCS driver does not have access to a PHY device, thus cannot check
> the PHY_F_RXC_ALWAYS_ON flag. They cannot check max_requires_rxc in the
> phylink config either, since it is a private member. Therefore, a new flag
> is needed to signal to the PCS that it should keep the RX clock signal up
> at all times.
> 
> Suggested-by: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>

...

> @@ -549,6 +557,34 @@ void pcs_an_restart(struct phylink_pcs *pcs);
>   */
>  void pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
>  		 phy_interface_t interface, int speed, int duplex);
> +
> +/**
> + * pcs_pre_init() - Configure PCS components necessary for MAC initialization
> + * @pcs: a pointer to a &struct phylink_pcs.
> + *
> + * This function can be called by MAC drivers through the
> + * phylink_pcs_pre_init() wrapper, before their hardware is initialized. It
> + * should not be called after the link is brought up, as reconfiguring the PCS
> + * at this point could break the link.
> + *
> + * Some MAC devices require specific hardware initialization to be performed by
> + * their associated PCS device before they can properly initialize their own
> + * hardware. An example of this is the initialization of stmmac controllers,
> + * which requires an active REF_CLK signal to be provided by the PHY/PCS.
> + *
> + * By calling phylink_pcs_pre_init(), MAC drivers can ensure that the PCS is
> + * setup in a way that allows for successful hardware initialization.
> + *
> + * The specific configuration performed by pcs_pre_init() is dependent on the
> + * model of PCS and the requirements of the MAC device attached to it. PCS
> + * driver authors should consider whether their target device is to be used in
> + * conjunction with a MAC device whose driver calls phylink_pcs_pre_init(). MAC
> + * driver authors should document their requirements for the PCS
> + * pre-initialization.
> + *
> + */
> +int pcs_config(struct phylink_pcs *pcs);

Hi Romain,

Should this be pcs_pre_init instead of pcs_config?

> +
>  #endif
>  
>  struct phylink *phylink_create(struct phylink_config *,

...
Romain Gantois Feb. 22, 2024, 8:31 a.m. UTC | #2
Hello Simon,

On Wed, 21 Feb 2024, Simon Horman wrote:

> > + * driver authors should consider whether their target device is to be used in
> > + * conjunction with a MAC device whose driver calls phylink_pcs_pre_init(). MAC
> > + * driver authors should document their requirements for the PCS
> > + * pre-initialization.
> > + *
> > + */
> > +int pcs_config(struct phylink_pcs *pcs);
Yes it should, thank you for pointing that out.

Best Regards,
diff mbox series

Patch

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 2bb583543dea..dad7462996ab 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1042,6 +1042,20 @@  static void phylink_pcs_poll_start(struct phylink *pl)
 		mod_timer(&pl->link_poll, jiffies + HZ);
 }
 
+int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
+{
+	int ret = 0;
+
+	/* Signal to PCS driver that MAC requires RX clock for init */
+	if (pl->config->mac_requires_rxc)
+		pcs->rxc_always_on = true;
+
+	if (pcs->ops->pcs_pre_init)
+		ret = pcs->ops->pcs_pre_init(pcs);
+
+	return ret;
+}
+
 static void phylink_mac_config(struct phylink *pl,
 			       const struct phylink_link_state *state)
 {
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 019993e5f570..f6ef5bc82179 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -396,6 +396,10 @@  struct phylink_pcs_ops;
  * @phylink: pointer to &struct phylink_config
  * @neg_mode: provide PCS neg mode via "mode" argument
  * @poll: poll the PCS for link changes
+ * @rxc_always_on: The MAC driver requires the reference clock
+ *                 to always be on. Standalone PCS drivers which
+ *                 do not have access to a PHY device can check
+ *                 this instead of PHY_F_RXC_ALWAYS_ON.
  *
  * This structure is designed to be embedded within the PCS private data,
  * and will be passed between phylink and the PCS.
@@ -408,6 +412,7 @@  struct phylink_pcs {
 	struct phylink *phylink;
 	bool neg_mode;
 	bool poll;
+	bool rxc_always_on;
 };
 
 /**
@@ -422,6 +427,8 @@  struct phylink_pcs {
  * @pcs_an_restart: restart 802.3z BaseX autonegotiation.
  * @pcs_link_up: program the PCS for the resolved link configuration
  *               (where necessary).
+ * @pcs_pre_init: configure PCS components necessary for MAC hardware
+ *                initialization e.g. RX clock for stmmac.
  */
 struct phylink_pcs_ops {
 	int (*pcs_validate)(struct phylink_pcs *pcs, unsigned long *supported,
@@ -441,6 +448,7 @@  struct phylink_pcs_ops {
 	void (*pcs_an_restart)(struct phylink_pcs *pcs);
 	void (*pcs_link_up)(struct phylink_pcs *pcs, unsigned int neg_mode,
 			    phy_interface_t interface, int speed, int duplex);
+	int (*pcs_pre_init)(struct phylink_pcs *pcs);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -549,6 +557,34 @@  void pcs_an_restart(struct phylink_pcs *pcs);
  */
 void pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
 		 phy_interface_t interface, int speed, int duplex);
+
+/**
+ * pcs_pre_init() - Configure PCS components necessary for MAC initialization
+ * @pcs: a pointer to a &struct phylink_pcs.
+ *
+ * This function can be called by MAC drivers through the
+ * phylink_pcs_pre_init() wrapper, before their hardware is initialized. It
+ * should not be called after the link is brought up, as reconfiguring the PCS
+ * at this point could break the link.
+ *
+ * Some MAC devices require specific hardware initialization to be performed by
+ * their associated PCS device before they can properly initialize their own
+ * hardware. An example of this is the initialization of stmmac controllers,
+ * which requires an active REF_CLK signal to be provided by the PHY/PCS.
+ *
+ * By calling phylink_pcs_pre_init(), MAC drivers can ensure that the PCS is
+ * setup in a way that allows for successful hardware initialization.
+ *
+ * The specific configuration performed by pcs_pre_init() is dependent on the
+ * model of PCS and the requirements of the MAC device attached to it. PCS
+ * driver authors should consider whether their target device is to be used in
+ * conjunction with a MAC device whose driver calls phylink_pcs_pre_init(). MAC
+ * driver authors should document their requirements for the PCS
+ * pre-initialization.
+ *
+ */
+int pcs_config(struct phylink_pcs *pcs);
+
 #endif
 
 struct phylink *phylink_create(struct phylink_config *,
@@ -568,6 +604,8 @@  void phylink_disconnect_phy(struct phylink *);
 void phylink_mac_change(struct phylink *, bool up);
 void phylink_pcs_change(struct phylink_pcs *, bool up);
 
+int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs);
+
 void phylink_start(struct phylink *);
 void phylink_stop(struct phylink *);