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

[PATCH v2 1/2] 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.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
changes in v2:
- new patch as suggested by Andrew

 drivers/net/phy/mscc/mscc_main.c | 97 +++++++++++++-------------------
 1 file changed, 40 insertions(+), 57 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index c8aa6d905d8e..54cac9c295ad 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1983,12 +1983,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)
@@ -1996,46 +1995,46 @@ static int vsc8514_probe(struct phy_device *phydev)
 
 	phydev->priv = vsc8531;
 
-	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;
 	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;
+	return vsc85xx_probe_helper(phydev, default_mode,
+				    ARRAY_SIZE(default_mode),
+				    VSC85XX_SUPP_LED_MODES,
+				    vsc85xx_hw_stats,
+				    ARRAY_SIZE(vsc85xx_hw_stats));
+}
 
-	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)
+{
+	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
+	   VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
+	   VSC8531_DUPLEX_COLLISION};
 
-	return vsc85xx_dt_led_modes_get(phydev, default_mode);
+	return vsc85xx_probe_helper(phydev, default_mode,
+				    ARRAY_SIZE(default_mode),
+				    VSC8584_SUPP_LED_MODES,
+				    vsc8584_hw_stats,
+				    ARRAY_SIZE(vsc8584_hw_stats));
 }
 
 static int vsc8584_probe(struct phy_device *phydev)
 {
-	struct vsc8531_private *vsc8531;
 	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
 	   VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
 	   VSC8531_DUPLEX_COLLISION};
@@ -2045,28 +2044,17 @@ 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;
-
-	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 vsc85xx_probe_helper(phydev, default_mode,
+				    ARRAY_SIZE(default_mode),
+				    VSC8584_SUPP_LED_MODES,
+				    vsc8584_hw_stats,
+				    ARRAY_SIZE(vsc8584_hw_stats));
 }
 
 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};
 
@@ -2074,23 +2062,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

[PATCH v2 2/2] 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>
---
Hi Andrew,

I didn't change the property yet, do you have a suggestion on
how to name it though? Going by the other examples in the
ethernet-phy.yamls, something like enet-phy-clock-out-frequency ?


 .../bindings/net/mscc-phy-vsc8531.txt         |  3 +
 drivers/net/phy/mscc/mscc.h                   |  9 ++
 drivers/net/phy/mscc/mscc_main.c              | 87 +++++++++++++++++--
 3 files changed, 92 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 5ff37c68c941..4a1f50ae48e1 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -1,6 +1,8 @@
 * Microsemi - vsc8531 Giga bit ethernet phy
 
 Optional properties:
+- vsc8531,clk-out-frequency: Clock output frequency in Hertz.
+			  Should be one of 25000000, 50000000, 125000000
 - vsc8531,vddmac	: The vddmac in mV. Allowed values is listed
 			  in the first row of Table 1 (below).
 			  This property is only used in combination
@@ -63,6 +65,7 @@ Example:
 
         vsc8531_0: ethernet-phy@0 {
                 compatible = "ethernet-phy-id0007.0570";
+                vsc8531,clk-out-frequency = <125000000>;
                 vsc8531,vddmac		= <3300>;
                 vsc8531,edge-slowdown	= <7>;
                 vsc8531,led-0-mode	= <LINK_1000_ACTIVITY>;
diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index 414e3b31bb1f..c8c395a041c2 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
@@ -361,6 +368,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 54cac9c295ad..012d5019d7c8 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, "vsc8531,clk-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,
@@ -1540,6 +1556,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;
@@ -2020,6 +2067,32 @@ static int vsc8514_probe(struct phy_device *phydev)
 				    ARRAY_SIZE(vsc85xx_hw_stats));
 }
 
+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)
 {
 	u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
@@ -2157,14 +2230,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,
@@ -2181,14 +2254,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,
@@ -2205,14 +2278,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,
@@ -2229,7 +2302,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

Re: [PATCH v2 2/2] net: phy: mscc: handle the clkout control on some phy variants

[David Miller]

From: Heiko Stuebner <heiko@sntech.de>
Date: Tue,  9 Jun 2020 15:31:40 +0200

> 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>
> ---
> Hi Andrew,
> 
> I didn't change the property yet, do you have a suggestion on
> how to name it though? Going by the other examples in the
> ethernet-phy.yamls, something like enet-phy-clock-out-frequency ?

Andrew, please help Heiko out with the naming here so we can move
forward on this patch series.

Thank you.

Re: [PATCH v2 2/2] net: phy: mscc: handle the clkout control on some phy variants

[Andrew Lunn]

On Tue, Jun 09, 2020 at 03:31:40PM +0200, Heiko Stuebner wrote:
> 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>
> ---
> Hi Andrew,
> 
> I didn't change the property yet, do you have a suggestion on
> how to name it though? Going by the other examples in the
> ethernet-phy.yamls, something like enet-phy-clock-out-frequency ?

Hi Heiko

Yes, that looks good.

Thanks
	Andrew

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

[David Miller]

From: Heiko Stuebner <heiko@sntech.de>
Date: Tue,  9 Jun 2020 15:31:39 +0200

> 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.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> changes in v2:
> - new patch as suggested by Andrew

This doesn't apply without rejects to the net GIT tree.

Re: [PATCH v2 2/2] net: phy: mscc: handle the clkout control on some phy variants

[Rob Herring]

On Tue, Jun 09, 2020 at 03:31:40PM +0200, Heiko Stuebner wrote:
> 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>
> ---
> Hi Andrew,
> 
> I didn't change the property yet, do you have a suggestion on
> how to name it though? Going by the other examples in the
> ethernet-phy.yamls, something like enet-phy-clock-out-frequency ?

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