[PATCH v4 0/3] add clkout support to mscc phys

[PATCH v4 0/3] add clkout support to mscc phys

[Heiko Stuebner]

The main part of this series is adding handling of the clkout
controls some of the mscc phys have and while at it Andrew
asked for some of the duplicated probe functionality to be
factored out into a common function.

changes in v4:
- fix missing variable initialization in one probe function
changes in v3:
- adapt to 5.8 merge-window results
- introduce a more generic enet-phy-property instead of
  using a vsc8531,* one - suggested by Andrew
changes in v2:
- new probe factoring patch as suggested by Andrew


Heiko Stuebner (3):
  net: phy: mscc: move shared probe code into a helper
  dt-bindings: net: ethernet-phy: add enet-phy-clock-out-frequency
  net: phy: mscc: handle the clkout control on some phy variants

 .../devicetree/bindings/net/ethernet-phy.yaml |   5 +
 drivers/net/phy/mscc/mscc.h                   |   9 +
 drivers/net/phy/mscc/mscc_main.c              | 211 ++++++++++++------
 3 files changed, 155 insertions(+), 70 deletions(-)

-- 
2.26.2

[PATCH v4 1/3] net: phy: mscc: move shared probe code into a helper

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

The different probe functions share a lot of code, so move the
common parts into a helper to reduce duplication.

This moves the devm_phy_package_join below the general allocation
but as all components just allocate things, this should be ok.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/net/phy/mscc/mscc_main.c | 124 +++++++++++++++----------------
 1 file changed, 61 insertions(+), 63 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 5ddc44f87eaf..5d2777522fb4 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1935,12 +1935,11 @@ static int vsc85xx_read_status(struct phy_device *phydev)
 	return genphy_read_status(phydev);
 }
 
-static int vsc8514_probe(struct phy_device *phydev)
+static int vsc85xx_probe_helper(struct phy_device *phydev,
+				u32 *leds, int num_leds, u16 led_modes,
+				const struct vsc85xx_hw_stat *stats, int nstats)
 {
 	struct vsc8531_private *vsc8531;
-	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
-	   VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
-	   VSC8531_DUPLEX_COLLISION};
 
 	vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
 	if (!vsc8531)
@@ -1948,54 +1947,66 @@ static int vsc8514_probe(struct phy_device *phydev)
 
 	phydev->priv = vsc8531;
 
-	vsc8584_get_base_addr(phydev);
-	devm_phy_package_join(&phydev->mdio.dev, phydev,
-			      vsc8531->base_addr, 0);
-
-	vsc8531->nleds = 4;
-	vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES;
-	vsc8531->hw_stats = vsc85xx_hw_stats;
-	vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats);
+	vsc8531->nleds = num_leds;
+	vsc8531->supp_led_modes = led_modes;
+	vsc8531->hw_stats = stats;
+	vsc8531->nstats = nstats;
 	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
 				      sizeof(u64), GFP_KERNEL);
 	if (!vsc8531->stats)
 		return -ENOMEM;
 
-	return vsc85xx_dt_led_modes_get(phydev, default_mode);
+	return vsc85xx_dt_led_modes_get(phydev, leds);
 }
 
-static int vsc8574_probe(struct phy_device *phydev)
+static int vsc8514_probe(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531;
+	int rc;
 	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
 	   VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
 	   VSC8531_DUPLEX_COLLISION};
 
-	vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
-	if (!vsc8531)
-		return -ENOMEM;
-
-	phydev->priv = vsc8531;
+	rc = vsc85xx_probe_helper(phydev, default_mode,
+				  ARRAY_SIZE(default_mode),
+				  VSC85XX_SUPP_LED_MODES,
+				  vsc85xx_hw_stats,
+				  ARRAY_SIZE(vsc85xx_hw_stats));
+	if (rc < 0)
+		return rc;
 
+	vsc8531 = phydev->priv;
 	vsc8584_get_base_addr(phydev);
-	devm_phy_package_join(&phydev->mdio.dev, phydev,
-			      vsc8531->base_addr, 0);
+	return devm_phy_package_join(&phydev->mdio.dev, phydev,
+				     vsc8531->base_addr, 0);
+}
 
-	vsc8531->nleds = 4;
-	vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
-	vsc8531->hw_stats = vsc8584_hw_stats;
-	vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats);
-	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
-				      sizeof(u64), GFP_KERNEL);
-	if (!vsc8531->stats)
-		return -ENOMEM;
+static int vsc8574_probe(struct phy_device *phydev)
+{
+	struct vsc8531_private *vsc8531;
+	int rc;
+	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
+	   VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
+	   VSC8531_DUPLEX_COLLISION};
+
+	rc = vsc85xx_probe_helper(phydev, default_mode,
+				  ARRAY_SIZE(default_mode),
+				  VSC8584_SUPP_LED_MODES,
+				  vsc8584_hw_stats,
+				  ARRAY_SIZE(vsc8584_hw_stats));
+	if (rc < 0)
+		return rc;
 
-	return vsc85xx_dt_led_modes_get(phydev, default_mode);
+	vsc8531 = phydev->priv;
+	vsc8584_get_base_addr(phydev);
+	return devm_phy_package_join(&phydev->mdio.dev, phydev,
+				     vsc8531->base_addr, 0);
 }
 
 static int vsc8584_probe(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531;
+	int rc;
 	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
 	   VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
 	   VSC8531_DUPLEX_COLLISION};
@@ -2005,32 +2016,24 @@ static int vsc8584_probe(struct phy_device *phydev)
 		return -ENOTSUPP;
 	}
 
-	vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
-	if (!vsc8531)
-		return -ENOMEM;
-
-	phydev->priv = vsc8531;
+	rc = vsc85xx_probe_helper(phydev, default_mode,
+				  ARRAY_SIZE(default_mode),
+				  VSC8584_SUPP_LED_MODES,
+				  vsc8584_hw_stats,
+				  ARRAY_SIZE(vsc8584_hw_stats));
+	if (rc < 0)
+		return rc;
 
+	vsc8531 = phydev->priv;
 	vsc8584_get_base_addr(phydev);
-	devm_phy_package_join(&phydev->mdio.dev, phydev,
-			      vsc8531->base_addr, 0);
-
-	vsc8531->nleds = 4;
-	vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
-	vsc8531->hw_stats = vsc8584_hw_stats;
-	vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats);
-	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
-				      sizeof(u64), GFP_KERNEL);
-	if (!vsc8531->stats)
-		return -ENOMEM;
-
-	return vsc85xx_dt_led_modes_get(phydev, default_mode);
+	return devm_phy_package_join(&phydev->mdio.dev, phydev,
+				     vsc8531->base_addr, 0);
 }
 
 static int vsc85xx_probe(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531;
-	int rate_magic;
+	int rate_magic, rc;
 	u32 default_mode[2] = {VSC8531_LINK_1000_ACTIVITY,
 	   VSC8531_LINK_100_ACTIVITY};
 
@@ -2038,23 +2041,18 @@ static int vsc85xx_probe(struct phy_device *phydev)
 	if (rate_magic < 0)
 		return rate_magic;
 
-	vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
-	if (!vsc8531)
-		return -ENOMEM;
-
-	phydev->priv = vsc8531;
+	rc = vsc85xx_probe_helper(phydev, default_mode,
+				  ARRAY_SIZE(default_mode),
+				  VSC85XX_SUPP_LED_MODES,
+				  vsc85xx_hw_stats,
+				  ARRAY_SIZE(vsc85xx_hw_stats));
+	if (rc < 0)
+		return rc;
 
+	vsc8531 = phydev->priv;
 	vsc8531->rate_magic = rate_magic;
-	vsc8531->nleds = 2;
-	vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES;
-	vsc8531->hw_stats = vsc85xx_hw_stats;
-	vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats);
-	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
-				      sizeof(u64), GFP_KERNEL);
-	if (!vsc8531->stats)
-		return -ENOMEM;
 
-	return vsc85xx_dt_led_modes_get(phydev, default_mode);
+	return 0;
 }
 
 /* Microsemi VSC85xx PHYs */
-- 
2.26.2

Re: [PATCH v4 1/3] net: phy: mscc: move shared probe code into a helper

[Andrew Lunn]

On Wed, Jun 17, 2020 at 11:33:24PM +0200, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> The different probe functions share a lot of code, so move the
> common parts into a helper to reduce duplication.
> 
> This moves the devm_phy_package_join below the general allocation
> but as all components just allocate things, this should be ok.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v4 2/3] dt-bindings: net: ethernet-phy: add enet-phy-clock-out-frequency

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

Some ethernet phys have a configurable clock output, so add a generic
property to describe its target rate.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 9b1f1147ca36..4dcf93f1c555 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -84,6 +84,11 @@ properties:
       the turn around line low at end of the control phase of the
       MDIO transaction.
 
+  enet-phy-clock-out-frequency:
+    $ref: /schemas/types.yaml#definitions/uint32
+    description:
+      Frequency in Hz to set an available clock output to.
+
   enet-phy-lane-swap:
     $ref: /schemas/types.yaml#definitions/flag
     description:
-- 
2.26.2

Re: [PATCH v4 2/3] dt-bindings: net: ethernet-phy: add enet-phy-clock-out-frequency

[Heiko Stübner]

Am Mittwoch, 17. Juni 2020, 23:33:25 CEST schrieb Heiko Stuebner:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> Some ethernet phys have a configurable clock output, so add a generic
> property to describe its target rate.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

just now Rob wrote for v3:

----- 8< ------
The correct thing to do here is make the phy a clock provider and then 
the client side use 'assigned-clock-rate' to set the rate. That has the 
advantage that it also describes the connection of the clock signal. You 
might not need that for a simple case, but I could imagine needing that 
in a more complex case.

Rob
----- 8< ------



>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 9b1f1147ca36..4dcf93f1c555 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -84,6 +84,11 @@ properties:
>        the turn around line low at end of the control phase of the
>        MDIO transaction.
>  
> +  enet-phy-clock-out-frequency:
> +    $ref: /schemas/types.yaml#definitions/uint32
> +    description:
> +      Frequency in Hz to set an available clock output to.
> +
>    enet-phy-lane-swap:
>      $ref: /schemas/types.yaml#definitions/flag
>      description:
> 

[PATCH v4 3/3] net: phy: mscc: handle the clkout control on some phy variants

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>

At least VSC8530/8531/8540/8541 contain a clock output that can emit
a predefined rate of 25, 50 or 125MHz.

This may then feed back into the network interface as source clock.
So follow the example the at803x already set and introduce a
vsc8531,clk-out-frequency property to set that output.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/net/phy/mscc/mscc.h      |  9 ++++
 drivers/net/phy/mscc/mscc_main.c | 87 +++++++++++++++++++++++++++++---
 2 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index fbcee5fce7b2..a3afc35c3eab 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -218,6 +218,13 @@ enum rgmii_clock_delay {
 #define INT_MEM_DATA_M			  0x00ff
 #define INT_MEM_DATA(x)			  (INT_MEM_DATA_M & (x))
 
+#define MSCC_CLKOUT_CNTL		  13
+#define CLKOUT_ENABLE			  BIT(15)
+#define CLKOUT_FREQ_MASK		  GENMASK(14, 13)
+#define CLKOUT_FREQ_25M			  (0x0 << 13)
+#define CLKOUT_FREQ_50M			  (0x1 << 13)
+#define CLKOUT_FREQ_125M		  (0x2 << 13)
+
 #define MSCC_PHY_PROC_CMD		  18
 #define PROC_CMD_NCOMPLETED		  0x8000
 #define PROC_CMD_FAILED			  0x4000
@@ -360,6 +367,8 @@ struct vsc8531_private {
 	 */
 	unsigned int base_addr;
 
+	u32 clkout_rate;
+
 #if IS_ENABLED(CONFIG_MACSEC)
 	/* MACsec fields:
 	 * - One SecY per device (enforced at the s/w implementation level)
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 5d2777522fb4..2f06cfa2f539 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -432,6 +432,18 @@ static int vsc85xx_dt_led_mode_get(struct phy_device *phydev,
 	return led_mode;
 }
 
+static void vsc8531_dt_clkout_rate_get(struct phy_device *phydev)
+{
+	struct vsc8531_private *priv = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+
+	if (!of_node)
+		return;
+
+	of_property_read_u32(of_node, "enet-phy-clock-out-frequency",
+			     &priv->clkout_rate);
+}
 #else
 static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
 {
@@ -444,6 +456,10 @@ static int vsc85xx_dt_led_mode_get(struct phy_device *phydev,
 {
 	return default_mode;
 }
+
+static void vsc8531_dt_clkout_rate_get(struct phy_device *phydev)
+{
+}
 #endif /* CONFIG_OF_MDIO */
 
 static int vsc85xx_dt_led_modes_get(struct phy_device *phydev,
@@ -1508,6 +1524,37 @@ static int vsc85xx_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int vsc8531_config_init(struct phy_device *phydev)
+{
+	struct vsc8531_private *vsc8531 = phydev->priv;
+	u16 val;
+	int rc;
+
+	rc = vsc85xx_config_init(phydev);
+	if (rc)
+		return rc;
+
+	switch (vsc8531->clkout_rate) {
+	case 0:
+		val = 0;
+		break;
+	case 25000000:
+		val = CLKOUT_FREQ_25M | CLKOUT_ENABLE;
+		break;
+	case 50000000:
+		val = CLKOUT_FREQ_50M | CLKOUT_ENABLE;
+		break;
+	case 125000000:
+		val = CLKOUT_FREQ_125M | CLKOUT_ENABLE;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return phy_write_paged(phydev, MSCC_PHY_PAGE_EXTENDED_GPIO,
+			       MSCC_CLKOUT_CNTL, val);
+}
+
 static int vsc8584_did_interrupt(struct phy_device *phydev)
 {
 	int rc = 0;
@@ -1981,6 +2028,32 @@ static int vsc8514_probe(struct phy_device *phydev)
 				     vsc8531->base_addr, 0);
 }
 
+static int vsc8531_probe(struct phy_device *phydev)
+{
+	struct vsc8531_private *vsc8531;
+	int rate_magic, rc;
+	u32 default_mode[2] = {VSC8531_LINK_1000_ACTIVITY,
+	   VSC8531_LINK_100_ACTIVITY};
+
+	rate_magic = vsc85xx_edge_rate_magic_get(phydev);
+	if (rate_magic < 0)
+		return rate_magic;
+
+	rc = vsc85xx_probe_helper(phydev, default_mode,
+				  ARRAY_SIZE(default_mode),
+				  VSC85XX_SUPP_LED_MODES,
+				  vsc85xx_hw_stats,
+				  ARRAY_SIZE(vsc85xx_hw_stats));
+	if (rc < 0)
+		return rc;
+
+	vsc8531 = phydev->priv;
+	vsc8531->rate_magic = rate_magic;
+	vsc8531_dt_clkout_rate_get(phydev);
+
+	return 0;
+}
+
 static int vsc8574_probe(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531;
@@ -2136,14 +2209,14 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id_mask	= 0xfffffff0,
 	/* PHY_BASIC_FEATURES */
 	.soft_reset	= &genphy_soft_reset,
-	.config_init	= &vsc85xx_config_init,
+	.config_init	= &vsc8531_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,
 	.read_status	= &vsc85xx_read_status,
 	.ack_interrupt	= &vsc85xx_ack_interrupt,
 	.config_intr	= &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
-	.probe		= &vsc85xx_probe,
+	.probe		= &vsc8531_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
 	.get_tunable	= &vsc85xx_get_tunable,
@@ -2160,14 +2233,14 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id_mask    = 0xfffffff0,
 	/* PHY_GBIT_FEATURES */
 	.soft_reset	= &genphy_soft_reset,
-	.config_init    = &vsc85xx_config_init,
+	.config_init    = &vsc8531_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,
 	.read_status	= &vsc85xx_read_status,
 	.ack_interrupt  = &vsc85xx_ack_interrupt,
 	.config_intr    = &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
-	.probe		= &vsc85xx_probe,
+	.probe		= &vsc8531_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
 	.get_tunable	= &vsc85xx_get_tunable,
@@ -2184,14 +2257,14 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id_mask	= 0xfffffff0,
 	/* PHY_BASIC_FEATURES */
 	.soft_reset	= &genphy_soft_reset,
-	.config_init	= &vsc85xx_config_init,
+	.config_init	= &vsc8531_config_init,
 	.config_aneg	= &vsc85xx_config_aneg,
 	.read_status	= &vsc85xx_read_status,
 	.ack_interrupt	= &vsc85xx_ack_interrupt,
 	.config_intr	= &vsc85xx_config_intr,
 	.suspend	= &genphy_suspend,
 	.resume		= &genphy_resume,
-	.probe		= &vsc85xx_probe,
+	.probe		= &vsc8531_probe,
 	.set_wol	= &vsc85xx_wol_set,
 	.get_wol	= &vsc85xx_wol_get,
 	.get_tunable	= &vsc85xx_get_tunable,
@@ -2208,7 +2281,7 @@ static struct phy_driver vsc85xx_driver[] = {
 	.phy_id_mask    = 0xfffffff0,
 	/* PHY_GBIT_FEATURES */
 	.soft_reset	= &genphy_soft_reset,
-	.config_init    = &vsc85xx_config_init,
+	.config_init    = &vsc8531_config_init,
 	.config_aneg    = &vsc85xx_config_aneg,
 	.read_status	= &vsc85xx_read_status,
 	.ack_interrupt  = &vsc85xx_ack_interrupt,
-- 
2.26.2