netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] net: phy: Support master-slave config via device tree
@ 2024-09-09 12:43 Oleksij Rempel
  2024-09-09 12:43 ` [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs Oleksij Rempel
  2024-09-09 12:43 ` [PATCH net-next v2 2/2] net: phy: Add support for master-slave role configuration via device tree Oleksij Rempel
  0 siblings, 2 replies; 9+ messages in thread
From: Oleksij Rempel @ 2024-09-09 12:43 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Russell King,
	devicetree

This patch series adds support for configuring the master/slave role of
PHYs via the device tree. A new `master-slave` property is introduced in
the device tree bindings, allowing PHYs to be forced into either master
or slave mode. This is particularly necessary for Single Pair Ethernet
(SPE) PHYs (1000/100/10Base-T1), where hardware strap pins may not be
available or correctly configured, but it is applicable to all PHY
types.

Oleksij Rempel (2):
  dt-bindings: net: ethernet-phy: Add master-slave role property for SPE
    PHYs
  net: phy: Add support for master-slave role configuration via device
    tree

 .../devicetree/bindings/net/ethernet-phy.yaml | 14 +++++++++
 drivers/net/phy/phy-core.c                    | 29 +++++++++++++++++++
 drivers/net/phy/phy_device.c                  |  3 ++
 include/linux/phy.h                           |  1 +
 4 files changed, 47 insertions(+)

--
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-09 12:43 [PATCH net-next v2 0/2] net: phy: Support master-slave config via device tree Oleksij Rempel
@ 2024-09-09 12:43 ` Oleksij Rempel
  2024-09-09 15:56   ` Florian Fainelli
  2024-09-09 16:20   ` Rob Herring
  2024-09-09 12:43 ` [PATCH net-next v2 2/2] net: phy: Add support for master-slave role configuration via device tree Oleksij Rempel
  1 sibling, 2 replies; 9+ messages in thread
From: Oleksij Rempel @ 2024-09-09 12:43 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Russell King,
	devicetree

Introduce a new `master-slave` string property in the ethernet-phy
binding to specify the link role for Single Pair Ethernet
(1000/100/10Base-T1) PHYs. This property supports the values
`forced-master` and `forced-slave`, which allow the PHY to operate in a
predefined role, necessary when hardware strap pins are unavailable or
wrongly set.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- use string property instead of multiple flags
---
 .../devicetree/bindings/net/ethernet-phy.yaml      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index d9b62741a2259..025e59f6be6f3 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -158,6 +158,20 @@ properties:
       Mark the corresponding energy efficient ethernet mode as
       broken and request the ethernet to stop advertising it.
 
+  master-slave:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - forced-master
+      - forced-slave
+    description: |
+      Specifies the predefined link role for the PHY in Single Pair Ethernet
+      (1000/100/10Base-T1).  This property is required for setups where the link
+      role must be assigned by the device tree due to limitations in using
+      hardware strap pins.
+
+      - 'forced-master': The PHY is forced to operate as a master.
+      - 'forced-slave': The PHY is forced to operate as a slave.
+
   pses:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next v2 2/2] net: phy: Add support for master-slave role configuration via device tree
  2024-09-09 12:43 [PATCH net-next v2 0/2] net: phy: Support master-slave config via device tree Oleksij Rempel
  2024-09-09 12:43 ` [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs Oleksij Rempel
@ 2024-09-09 12:43 ` Oleksij Rempel
  1 sibling, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2024-09-09 12:43 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Russell King,
	devicetree

Introduce support for configuring the master/slave role of PHYs based on
the `master-slave` property in the device tree. While this functionality
is necessary for Single Pair Ethernet (SPE) PHYs (1000/100/10Base-T1)
where hardware strap pins may be unavailable or incorrectly set, it
works for any PHY type. The property supports `forced-master` and
`forced-slave` values, allowing for predefined link role assignment.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy-core.c   | 29 +++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |  3 +++
 include/linux/phy.h          |  1 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 1f98b6a96c153..296c446037144 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -412,6 +412,35 @@ void of_set_phy_eee_broken(struct phy_device *phydev)
 	phydev->eee_broken_modes = broken;
 }
 
+/**
+ * of_set_phy_master_slave - Set the master/slave mode of the PHY
+ *
+ * @phydev: The phy_device struct
+ *
+ * Set master/slave configuration of the PHY based on the device tree.
+ */
+void of_set_phy_master_slave(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	const char *master;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return;
+
+	if (!node)
+		return;
+
+	if (of_property_read_string(node, "master-slave", &master))
+		return;
+
+	if (strcmp(master, "forced-master") == 0)
+		phydev->master_slave_set = MASTER_SLAVE_CFG_MASTER_FORCE;
+	else if (strcmp(master, "forced-slave") == 0)
+		phydev->master_slave_set = MASTER_SLAVE_CFG_SLAVE_FORCE;
+	else
+		phydev_warn(phydev, "Unknown master-slave mode %s\n", master);
+}
+
 /**
  * phy_resolve_aneg_pause - Determine pause autoneg results
  *
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 560e338b307a4..8304f2781d5ae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3608,6 +3608,9 @@ static int phy_probe(struct device *dev)
 	 */
 	of_set_phy_eee_broken(phydev);
 
+	/* Get master/slave strap overrides */
+	of_set_phy_master_slave(phydev);
+
 	/* The Pause Frame bits indicate that the PHY can support passing
 	 * pause frames. During autonegotiation, the PHYs will determine if
 	 * they should allow pause frames to pass.  The MAC driver should then
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4a9a11749c554..81701bdac44c1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1260,6 +1260,7 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
 		  unsigned long *mask);
 void of_set_phy_supported(struct phy_device *phydev);
 void of_set_phy_eee_broken(struct phy_device *phydev);
+void of_set_phy_master_slave(struct phy_device *phydev);
 int phy_speed_down_core(struct phy_device *phydev);
 
 /**
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-09 12:43 ` [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs Oleksij Rempel
@ 2024-09-09 15:56   ` Florian Fainelli
  2024-09-09 16:20   ` Rob Herring
  1 sibling, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2024-09-09 15:56 UTC (permalink / raw)
  To: Oleksij Rempel, Andrew Lunn, Heiner Kallweit, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: kernel, linux-kernel, netdev, Russell King, devicetree

On 9/9/24 05:43, Oleksij Rempel wrote:
> Introduce a new `master-slave` string property in the ethernet-phy
> binding to specify the link role for Single Pair Ethernet
> (1000/100/10Base-T1) PHYs. This property supports the values
> `forced-master` and `forced-slave`, which allow the PHY to operate in a
> predefined role, necessary when hardware strap pins are unavailable or
> wrongly set.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v2:
> - use string property instead of multiple flags
> ---
>   .../devicetree/bindings/net/ethernet-phy.yaml      | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index d9b62741a2259..025e59f6be6f3 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -158,6 +158,20 @@ properties:
>         Mark the corresponding energy efficient ethernet mode as
>         broken and request the ethernet to stop advertising it.
>   
> +  master-slave:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - forced-master
> +      - forced-slave
> +    description: |
> +      Specifies the predefined link role for the PHY in Single Pair Ethernet
> +      (1000/100/10Base-T1).  This property is required for setups where the link
> +      role must be assigned by the device tree due to limitations in using
> +      hardware strap pins.

Nit: the way this is implemented right now, this is also applicable to 
1000BaseT which is fine, just needs to be called out explicitly IMHO.
-- 
Florian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-09 12:43 ` [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs Oleksij Rempel
  2024-09-09 15:56   ` Florian Fainelli
@ 2024-09-09 16:20   ` Rob Herring
  2024-09-09 17:00     ` Andrew Lunn
  1 sibling, 1 reply; 9+ messages in thread
From: Rob Herring @ 2024-09-09 16:20 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, kernel, linux-kernel, netdev, Russell King,
	devicetree

On Mon, Sep 09, 2024 at 02:43:40PM +0200, Oleksij Rempel wrote:
> Introduce a new `master-slave` string property in the ethernet-phy
> binding to specify the link role for Single Pair Ethernet
> (1000/100/10Base-T1) PHYs. This property supports the values
> `forced-master` and `forced-slave`, which allow the PHY to operate in a
> predefined role, necessary when hardware strap pins are unavailable or
> wrongly set.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v2:
> - use string property instead of multiple flags
> ---
>  .../devicetree/bindings/net/ethernet-phy.yaml      | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index d9b62741a2259..025e59f6be6f3 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -158,6 +158,20 @@ properties:
>        Mark the corresponding energy efficient ethernet mode as
>        broken and request the ethernet to stop advertising it.
>  
> +  master-slave:

Outdated terminology and kind of vague what it is for...

The usual transformation to 'controller-device' would not make much
sense though. I think a better name would be "spe-link-role" or
"spe-link-mode".

> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - forced-master
> +      - forced-slave
> +    description: |
> +      Specifies the predefined link role for the PHY in Single Pair Ethernet
> +      (1000/100/10Base-T1).  This property is required for setups where the link
> +      role must be assigned by the device tree due to limitations in using
> +      hardware strap pins.
> +
> +      - 'forced-master': The PHY is forced to operate as a master.
> +      - 'forced-slave': The PHY is forced to operate as a slave.
> +
>    pses:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
> -- 
> 2.39.2
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-09 16:20   ` Rob Herring
@ 2024-09-09 17:00     ` Andrew Lunn
  2024-09-10 16:54       ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2024-09-09 17:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Oleksij Rempel, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, kernel, linux-kernel, netdev, Russell King,
	devicetree

On Mon, Sep 09, 2024 at 11:20:09AM -0500, Rob Herring wrote:
> On Mon, Sep 09, 2024 at 02:43:40PM +0200, Oleksij Rempel wrote:
> > Introduce a new `master-slave` string property in the ethernet-phy
> > binding to specify the link role for Single Pair Ethernet
> > (1000/100/10Base-T1) PHYs. This property supports the values
> > `forced-master` and `forced-slave`, which allow the PHY to operate in a
> > predefined role, necessary when hardware strap pins are unavailable or
> > wrongly set.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > changes v2:
> > - use string property instead of multiple flags
> > ---
> >  .../devicetree/bindings/net/ethernet-phy.yaml      | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index d9b62741a2259..025e59f6be6f3 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -158,6 +158,20 @@ properties:
> >        Mark the corresponding energy efficient ethernet mode as
> >        broken and request the ethernet to stop advertising it.
> >  
> > +  master-slave:
> 
> Outdated terminology and kind of vague what it is for...
> 
> The usual transformation to 'controller-device' would not make much
> sense though. I think a better name would be "spe-link-role" or
> "spe-link-mode".

This applies to more than Single Pair Ethernet. This property could
also be used for 2 and 4 pair cables. So spe-link-mode would be wrong.

Also:

https://grouper.ieee.org/groups/802/3/dc/comments/P8023_D2p0_comments_final_by_cls.pdf

On 3 December 2020, the IEEE SA Standard Board passed the following resolution. (See
<https://standards.ieee.org/about/sasb/resolutions.html>.)

  "IEEE standards (including recommended practices and guides) shall
  be written in such a way as to unambiguously communicate the
  technical necessities, preferences, and options of the standard to
  best enable market adoption, conformity assessment,
  interoperability, and other technical aspirations of the developing
  standards committee. IEEE standards should be written in such a way
  as to avoid non-inclusive and insensitive terminology (see IEEE
  Policy 9.27) and other deprecated terminology (see clause 10 of the
  IEEE SA Style Manual) except when required by safety, legal,
  regulatory, and other similar considerations.  Terms such as
  master/slave, blacklist, and whitelist should be avoided."

  In IEEE Std 802.3, 1000BASE-T, 10BASE-T1L, 100BASE-T1, 1000BASE-T1,
  and MultiGBASE-T PHYs use the terms "master" and "slave" to indicate
  whether the clock is derived from an external source or from the
  received signal. In these cases, the terms appear in the text,
  figures, state names, variable names, register/bit names, etc. A
  direct substitution of terms will create disconnects between the
  standard and the documentation for devices in the field (e.g., the
  register interface) and also risks the introduction of technical
  errors. Note that "master" and "slave" are also occasionally used to
  describe the relationship between an ONT and an ONU for EPON and
  between a CNT and a CNU for EPoC.

  The approach that other IEEE standards are taking to address this
  issue have been considered. For example, IEEE P1588g proposes to
  define "optional alternative suitable and inclusive terminology" but
  not replace the original terms. (See
  <https://development.standards.ieee.org/myproject-web/public/view.html#pardetail/8858>.)
  It is understood that an annex to the IEEE 1588 standard has been
  proposed that defines the inclusive terminology. It is also
  understood that the inclusive terminology has been chosen to be
  "leader" and "follower".

  The IEEE P802.1ASdr project proposes to align to the IEEE P1588g
  inclusive terminology.  (See
  <https://development.standards.ieee.org/myprojectweb/public/view.html#pardetail/9009>.)
  Based on this, it seems reasonable to include an annex that defines
  optional alternative inclusive terminology and, for consistency, to
  use the terms "leader" and "follower" as the inclusive terminology

The 2022 revision of 802.3 still has master/slave when describing the
registers, but it does have Annex K as described above saying "leader"
and "follower" are optional substitutions.

The Linux code has not changed, and the uAPI has not changed. It seems
like the best compromise would be to allow both 'force-master' and
'force-leader', as well as 'force-slave' and 'force-follower', and a
reference to 802.3 Annex K.

As to you comment about it being unclear what it means i would suggest
a reference to 802.3 section 1.4.389:

  1.4.389 master Physical Layer device (PHY): Within IEEE 802.3, in a
  100BASE-T2, 1000BASE-T, 10BASE-T1L, 100BASE-T1, 1000BASE-T1, or any
  MultiGBASE-T link containing a pair of PHYs, the PHY that uses an
  external clock for generating its clock signals to determine the
  timing of transmitter and receiver operations. It also uses the
  master transmit scrambler generator polynomial for side-stream
  scrambling. Master and slave PHY status is determined during the
  Auto-Negotiation process that takes place prior to establishing the
  transmission link, or in the case of a PHY where Auto-Negotiation is
  optional and not used, master and slave PHY status

	Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-09 17:00     ` Andrew Lunn
@ 2024-09-10 16:54       ` Rob Herring
  2024-09-11  7:00         ` Oleksij Rempel
  2024-09-11 12:05         ` Andrew Lunn
  0 siblings, 2 replies; 9+ messages in thread
From: Rob Herring @ 2024-09-10 16:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Oleksij Rempel, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, kernel, linux-kernel, netdev, Russell King,
	devicetree

On Mon, Sep 9, 2024 at 12:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Sep 09, 2024 at 11:20:09AM -0500, Rob Herring wrote:
> > On Mon, Sep 09, 2024 at 02:43:40PM +0200, Oleksij Rempel wrote:
> > > Introduce a new `master-slave` string property in the ethernet-phy
> > > binding to specify the link role for Single Pair Ethernet
> > > (1000/100/10Base-T1) PHYs. This property supports the values
> > > `forced-master` and `forced-slave`, which allow the PHY to operate in a
> > > predefined role, necessary when hardware strap pins are unavailable or
> > > wrongly set.
> > >
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > > changes v2:
> > > - use string property instead of multiple flags
> > > ---
> > >  .../devicetree/bindings/net/ethernet-phy.yaml      | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > > index d9b62741a2259..025e59f6be6f3 100644
> > > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > > @@ -158,6 +158,20 @@ properties:
> > >        Mark the corresponding energy efficient ethernet mode as
> > >        broken and request the ethernet to stop advertising it.
> > >
> > > +  master-slave:
> >
> > Outdated terminology and kind of vague what it is for...
> >
> > The usual transformation to 'controller-device' would not make much
> > sense though. I think a better name would be "spe-link-role" or
> > "spe-link-mode".
>
> This applies to more than Single Pair Ethernet. This property could
> also be used for 2 and 4 pair cables. So spe-link-mode would be wrong.

I kind of figured that... Propose something that's not just
duplicating possible values.

>
> Also:
>
> https://grouper.ieee.org/groups/802/3/dc/comments/P8023_D2p0_comments_final_by_cls.pdf
>
> On 3 December 2020, the IEEE SA Standard Board passed the following resolution. (See
> <https://standards.ieee.org/about/sasb/resolutions.html>.)
>
>   "IEEE standards (including recommended practices and guides) shall
>   be written in such a way as to unambiguously communicate the
>   technical necessities, preferences, and options of the standard to
>   best enable market adoption, conformity assessment,
>   interoperability, and other technical aspirations of the developing
>   standards committee. IEEE standards should be written in such a way
>   as to avoid non-inclusive and insensitive terminology (see IEEE
>   Policy 9.27) and other deprecated terminology (see clause 10 of the
>   IEEE SA Style Manual) except when required by safety, legal,
>   regulatory, and other similar considerations.  Terms such as
>   master/slave, blacklist, and whitelist should be avoided."
>
>   In IEEE Std 802.3, 1000BASE-T, 10BASE-T1L, 100BASE-T1, 1000BASE-T1,
>   and MultiGBASE-T PHYs use the terms "master" and "slave" to indicate
>   whether the clock is derived from an external source or from the
>   received signal. In these cases, the terms appear in the text,
>   figures, state names, variable names, register/bit names, etc. A
>   direct substitution of terms will create disconnects between the
>   standard and the documentation for devices in the field (e.g., the
>   register interface) and also risks the introduction of technical
>   errors. Note that "master" and "slave" are also occasionally used to
>   describe the relationship between an ONT and an ONU for EPON and
>   between a CNT and a CNU for EPoC.
>
>   The approach that other IEEE standards are taking to address this
>   issue have been considered. For example, IEEE P1588g proposes to
>   define "optional alternative suitable and inclusive terminology" but
>   not replace the original terms. (See
>   <https://development.standards.ieee.org/myproject-web/public/view.html#pardetail/8858>.)
>   It is understood that an annex to the IEEE 1588 standard has been
>   proposed that defines the inclusive terminology. It is also
>   understood that the inclusive terminology has been chosen to be
>   "leader" and "follower".
>
>   The IEEE P802.1ASdr project proposes to align to the IEEE P1588g
>   inclusive terminology.  (See
>   <https://development.standards.ieee.org/myprojectweb/public/view.html#pardetail/9009>.)
>   Based on this, it seems reasonable to include an annex that defines
>   optional alternative inclusive terminology and, for consistency, to
>   use the terms "leader" and "follower" as the inclusive terminology
>
> The 2022 revision of 802.3 still has master/slave when describing the
> registers, but it does have Annex K as described above saying "leader"
> and "follower" are optional substitutions.
>
> The Linux code has not changed, and the uAPI has not changed. It seems
> like the best compromise would be to allow both 'force-master' and
> 'force-leader', as well as 'force-slave' and 'force-follower', and a
> reference to 802.3 Annex K.

It seems silly to maintain both forever. I'd rather have one or the
other than both.

> As to you comment about it being unclear what it means i would suggest
> a reference to 802.3 section 1.4.389:
>
>   1.4.389 master Physical Layer device (PHY): Within IEEE 802.3, in a
>   100BASE-T2, 1000BASE-T, 10BASE-T1L, 100BASE-T1, 1000BASE-T1, or any
>   MultiGBASE-T link containing a pair of PHYs, the PHY that uses an
>   external clock for generating its clock signals to determine the
>   timing of transmitter and receiver operations. It also uses the
>   master transmit scrambler generator polynomial for side-stream
>   scrambling. Master and slave PHY status is determined during the
>   Auto-Negotiation process that takes place prior to establishing the
>   transmission link, or in the case of a PHY where Auto-Negotiation is
>   optional and not used, master and slave PHY status

phy-status? Shrug.

Another thought. Is it possible that h/w strapping disables auto-neg,
but you actually want to override that and force auto-neg?

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-10 16:54       ` Rob Herring
@ 2024-09-11  7:00         ` Oleksij Rempel
  2024-09-11 12:05         ` Andrew Lunn
  1 sibling, 0 replies; 9+ messages in thread
From: Oleksij Rempel @ 2024-09-11  7:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, kernel, linux-kernel, netdev, Russell King,
	devicetree

On Tue, Sep 10, 2024 at 11:54:04AM -0500, Rob Herring wrote:
> On Mon, Sep 9, 2024 at 12:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
....
> > The 2022 revision of 802.3 still has master/slave when describing the
> > registers, but it does have Annex K as described above saying "leader"
> > and "follower" are optional substitutions.
> >
> > The Linux code has not changed, and the uAPI has not changed. It seems
> > like the best compromise would be to allow both 'force-master' and
> > 'force-leader', as well as 'force-slave' and 'force-follower', and a
> > reference to 802.3 Annex K.
> 
> It seems silly to maintain both forever. I'd rather have one or the
> other than both.

I'll accept what ever will be decided. Even if we will decide to use
words 'leader' and 'follower' for the properties, we still need to
document that they correspond to 'master' and 'slave' in the IEEE
specification and in the kernel UAPI.

I can imagine a mdi-link-clock-role = 'leader' or 'follower'

> > As to you comment about it being unclear what it means i would suggest
> > a reference to 802.3 section 1.4.389:
> >
> >   1.4.389 master Physical Layer device (PHY): Within IEEE 802.3, in a
> >   100BASE-T2, 1000BASE-T, 10BASE-T1L, 100BASE-T1, 1000BASE-T1, or any
> >   MultiGBASE-T link containing a pair of PHYs, the PHY that uses an
> >   external clock for generating its clock signals to determine the
> >   timing of transmitter and receiver operations. It also uses the
> >   master transmit scrambler generator polynomial for side-stream
> >   scrambling. Master and slave PHY status is determined during the
> >   Auto-Negotiation process that takes place prior to establishing the
> >   transmission link, or in the case of a PHY where Auto-Negotiation is
> >   optional and not used, master and slave PHY status
> 
> phy-status? Shrug.

In my understanding, the 'status' is result not actual configuration
request.

> Another thought. Is it possible that h/w strapping disables auto-neg,
> but you actually want to override that and force auto-neg?

Yes. If I would need it, I would recommend to have a different override
property, something like: autoneg-default = 'on' or 'off'

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs
  2024-09-10 16:54       ` Rob Herring
  2024-09-11  7:00         ` Oleksij Rempel
@ 2024-09-11 12:05         ` Andrew Lunn
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2024-09-11 12:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: Oleksij Rempel, Heiner Kallweit, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski, Conor Dooley,
	Florian Fainelli, kernel, linux-kernel, netdev, Russell King,
	devicetree

> It seems silly to maintain both forever. I'd rather have one or the
> other than both.

It currently seems like 802.3 is going to keep with master/slave in
the body of the text. And they don't even have to deal with breaking
backwards compatibility. So i suggest we keep with master/slave, but
comment that an annex of the standard proposes alternative names of
leader/follower. But don't actually accept them.

> 
> > As to you comment about it being unclear what it means i would suggest
> > a reference to 802.3 section 1.4.389:
> >
> >   1.4.389 master Physical Layer device (PHY): Within IEEE 802.3, in a
> >   100BASE-T2, 1000BASE-T, 10BASE-T1L, 100BASE-T1, 1000BASE-T1, or any
> >   MultiGBASE-T link containing a pair of PHYs, the PHY that uses an
> >   external clock for generating its clock signals to determine the
> >   timing of transmitter and receiver operations. It also uses the
> >   master transmit scrambler generator polynomial for side-stream
> >   scrambling. Master and slave PHY status is determined during the
> >   Auto-Negotiation process that takes place prior to establishing the
> >   transmission link, or in the case of a PHY where Auto-Negotiation is
> >   optional and not used, master and slave PHY status
> 
> phy-status? Shrug.

phy-status is too generic. Maybe 'timing-role' ?

> 
> Another thought. Is it possible that h/w strapping disables auto-neg,
> but you actually want to override that and force auto-neg?

Autoneg can be used for a bunch of parameters. In automotive
situations, it is generally disabled and those parameters are
forced. In more tradition settings those parameters are
negotiated. However, even with autoneg enabled, you can force each
individual parameter, rather than negotiate it.

So we would need a DT parameter about autoneg in general. And then a
DT parameter about 'timing-role', where force-master/force-slave means
don't negotiate, and prefer-master/prefer-slave means do negotiate
with the given preference.

	Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-09-11 12:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-09 12:43 [PATCH net-next v2 0/2] net: phy: Support master-slave config via device tree Oleksij Rempel
2024-09-09 12:43 ` [PATCH net-next v2 1/2] dt-bindings: net: ethernet-phy: Add master-slave role property for SPE PHYs Oleksij Rempel
2024-09-09 15:56   ` Florian Fainelli
2024-09-09 16:20   ` Rob Herring
2024-09-09 17:00     ` Andrew Lunn
2024-09-10 16:54       ` Rob Herring
2024-09-11  7:00         ` Oleksij Rempel
2024-09-11 12:05         ` Andrew Lunn
2024-09-09 12:43 ` [PATCH net-next v2 2/2] net: phy: Add support for master-slave role configuration via device tree Oleksij Rempel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).