linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] net: phy: at803x device tree binding
@ 2019-10-30 22:42 Michael Walle
  2019-10-30 22:42 ` [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description Michael Walle
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-30 22:42 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev
  Cc: Michael Walle, David S. Miller, Rob Herring, Mark Rutland,
	Andrew Lunn, Florian Fainelli, Heiner Kallweit

Adds a device tree binding to configure the clock and the RGMII voltage.

I do have the following questions:
 - Who should be the maintainer of the atheros,at803x.yaml file?
 - Is the "atheros,rgmii-io-1v8" boolean property ok or should it be a
   "atheros,rgmii-io-microvolt = <1800000>"?
 - There is actually a typo throughout the whole at803x file. The actual
   name of the PHY is "Atheros AR803x". What should be the name of the yaml
   file and the dt-bindings header file? atheros,at803x.yaml or
   atheros,ar803x.yaml. Likewise for the header file.

Michael Walle (3):
  net: phy: at803x: fix Kconfig description
  dt-bindings: net: phy: Add support for AT803X
  net: phy: at803x: add device tree binding

 .../bindings/net/atheros,at803x.yaml          |  58 +++++++
 drivers/net/phy/Kconfig                       |   4 +-
 drivers/net/phy/at803x.c                      | 156 +++++++++++++++++-
 include/dt-bindings/net/atheros-at803x.h      |  13 ++
 4 files changed, 227 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/atheros,at803x.yaml
 create mode 100644 include/dt-bindings/net/atheros-at803x.h

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
-- 
2.20.1


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

* [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description
  2019-10-30 22:42 [RFC PATCH 0/3] net: phy: at803x device tree binding Michael Walle
@ 2019-10-30 22:42 ` Michael Walle
  2019-10-30 23:13   ` Andrew Lunn
  2019-10-30 23:16   ` Florian Fainelli
  2019-10-30 22:42 ` [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X Michael Walle
  2019-10-30 22:42 ` [RFC PATCH 3/3] net: phy: at803x: add device tree binding Michael Walle
  2 siblings, 2 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-30 22:42 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev; +Cc: Michael Walle

The name of the PHY is actually AR803x not AT803x. Additionally, add the
name of the vendor and mention the AR8031 support.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index fe602648b99f..38f180f9ca42 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -283,9 +283,9 @@ config AX88796B_PHY
 	  AX88796B package.
 
 config AT803X_PHY
-	tristate "AT803X PHYs"
+	tristate "Atheros AR803X PHYs"
 	---help---
-	  Currently supports the AT8030 and AT8035 model
+	  Currently supports the AR8030, AR8031 and AR8035 model
 
 config BCM63XX_PHY
 	tristate "Broadcom 63xx SOCs internal PHY"
-- 
2.20.1


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

* [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-30 22:42 [RFC PATCH 0/3] net: phy: at803x device tree binding Michael Walle
  2019-10-30 22:42 ` [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description Michael Walle
@ 2019-10-30 22:42 ` Michael Walle
  2019-10-30 23:17   ` Andrew Lunn
                     ` (2 more replies)
  2019-10-30 22:42 ` [RFC PATCH 3/3] net: phy: at803x: add device tree binding Michael Walle
  2 siblings, 3 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-30 22:42 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev; +Cc: Michael Walle

Document the Atheros AR803x PHY bindings.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../bindings/net/atheros,at803x.yaml          | 58 +++++++++++++++++++
 include/dt-bindings/net/atheros-at803x.h      | 13 +++++
 2 files changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/atheros,at803x.yaml
 create mode 100644 include/dt-bindings/net/atheros-at803x.h

diff --git a/Documentation/devicetree/bindings/net/atheros,at803x.yaml b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
new file mode 100644
index 000000000000..60500fd90fd8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/atheros,at803x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atheros AR803x PHY
+
+maintainers:
+  - TBD
+
+description: |
+  Bindings for Atheros AR803x PHYs
+
+allOf:
+  - $ref: ethernet-phy.yaml#
+
+properties:
+  atheros,clk-out-frequency:
+    description: Clock output frequency in Hertz.
+    enum: [ 25000000, 50000000, 62500000, 125000000 ]
+
+  atheros,clk-out-strength:
+    description: Clock output driver strength.
+    enum: [ 0, 1, 2 ]
+
+  atheros,keep-pll-enabled:
+    description: |
+      If set, keep the PLL enabled even if there is no link. Useful if you
+      want to use the clock output without an ethernet link.
+    type: boolean
+
+  atheros,rgmii-io-1v8:
+    description: |
+      The PHY supports RGMII I/O voltages of 2.5V, 1.8V and 1.5V. By default,
+      the PHY uses a voltage of 1.5V. If this is set, the voltage will changed
+      to 1.8V.
+      The 2.5V voltage is only supported with an external supply voltage.
+    type: boolean
+
+examples:
+  - |
+    #include <dt-bindings/net/atheros-at803x.h>
+
+    ethernet {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        phy-mode = "rgmii-id";
+
+        ethernet-phy@0 {
+            reg = <0>;
+
+            atheros,clk-out-frequency = <125000000>;
+            atheros,clk-out-strength = <AT803X_STRENGTH_FULL>;
+            atheros,rgmii-io-1v8;
+        };
+    };
diff --git a/include/dt-bindings/net/atheros-at803x.h b/include/dt-bindings/net/atheros-at803x.h
new file mode 100644
index 000000000000..63b4fd10b2c6
--- /dev/null
+++ b/include/dt-bindings/net/atheros-at803x.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Device Tree constants for the Atheros AR803x PHYs
+ */
+
+#ifndef _DT_BINDINGS_ATHEROS_AR803X_H
+#define _DT_BINDINGS_ATHEROS_AR803X_H
+
+#define AT803X_STRENGTH_FULL		0
+#define AT803X_STRENGTH_HALF		1
+#define AT803X_STRENGTH_QUARTER		2
+
+#endif
-- 
2.20.1


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

* [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-30 22:42 [RFC PATCH 0/3] net: phy: at803x device tree binding Michael Walle
  2019-10-30 22:42 ` [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description Michael Walle
  2019-10-30 22:42 ` [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X Michael Walle
@ 2019-10-30 22:42 ` Michael Walle
  2019-10-30 23:21   ` Andrew Lunn
  2019-10-30 23:28   ` Florian Fainelli
  2 siblings, 2 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-30 22:42 UTC (permalink / raw)
  To: linux-kernel, devicetree, netdev; +Cc: Michael Walle

Add support for configuring the CLK_25M pin as well as the RGMII I/O
voltage by the device tree.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/at803x.c | 156 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 154 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 1eb5d4fb8925..32be4c72cf4b 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -13,7 +13,9 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/of_gpio.h>
+#include <linux/bitfield.h>
 #include <linux/gpio/consumer.h>
+#include <dt-bindings/net/atheros-at803x.h>
 
 #define AT803X_SPECIFIC_STATUS			0x11
 #define AT803X_SS_SPEED_MASK			(3 << 14)
@@ -62,6 +64,37 @@
 #define AT803X_DEBUG_REG_5			0x05
 #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
 
+#define AT803X_DEBUG_REG_1F			0x1F
+#define AT803X_DEBUG_PLL_ON			BIT(2)
+#define AT803X_DEBUG_RGMII_1V8			BIT(3)
+
+/* AT803x supports either the XTAL input pad, an internal PLL or the
+ * DSP as clock reference for the clock output pad. The XTAL reference
+ * is only used for 25 MHz output, all other frequencies need the PLL.
+ * The DSP as a clock reference is used in synchronous ethernet
+ * applications.
+ *
+ * By default the PLL is only enabled if there is a link. Otherwise
+ * the PHY will go into low power state and disabled the PLL. You can
+ * set the PLL_ON bit (see debug register 0x1f) to keep the PLL always
+ * enabled.
+ */
+#define AT803X_MMD7_CLK25M			0x8016
+#define AT803X_CLK_OUT_MASK			GENMASK(4, 2)
+#define AT803X_CLK_OUT_25MHZ_XTAL		0
+#define AT803X_CLK_OUT_25MHZ_DSP		1
+#define AT803X_CLK_OUT_50MHZ_PLL		2
+#define AT803X_CLK_OUT_50MHZ_DSP		3
+#define AT803X_CLK_OUT_62_5MHZ_PLL		4
+#define AT803X_CLK_OUT_62_5MHZ_DSP		5
+#define AT803X_CLK_OUT_125MHZ_PLL		6
+#define AT803X_CLK_OUT_125MHZ_DSP		7
+
+#define AT803X_CLK_OUT_STRENGTH_MASK		GENMASK(8, 7)
+#define AT803X_CLK_OUT_STRENGTH_FULL		0
+#define AT803X_CLK_OUT_STRENGTH_HALF		1
+#define AT803X_CLK_OUT_STRENGTH_QUARTER		2
+
 #define ATH8030_PHY_ID 0x004dd076
 #define ATH8031_PHY_ID 0x004dd074
 #define ATH8035_PHY_ID 0x004dd072
@@ -73,6 +106,11 @@ MODULE_LICENSE("GPL");
 
 struct at803x_priv {
 	bool phy_reset:1;
+	int flags;
+#define AT803X_KEEP_PLL_ENABLED	BIT(0)	/* don't turn off internal PLL */
+#define AT803X_RGMII_1V8	BIT(1)	/* use 1.8V RGMII voltage */
+	u16 clk_25m_reg;
+	u16 clk_25m_mask;
 };
 
 struct at803x_context {
@@ -240,6 +278,74 @@ static int at803x_resume(struct phy_device *phydev)
 	return phy_modify(phydev, MII_BMCR, BMCR_PDOWN | BMCR_ISOLATE, 0);
 }
 
+static int at803x_parse_dt(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct at803x_priv *priv = phydev->priv;
+	u32 freq, strength;
+	unsigned int sel;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return 0;
+
+	if (!node)
+		return 0;
+
+	if (of_property_read_bool(node, "atheros,keep-pll-enabled"))
+		priv->flags |= AT803X_KEEP_PLL_ENABLED;
+
+	if (of_property_read_bool(node, "atheros,rgmii-io-1v8"))
+		priv->flags |= AT803X_RGMII_1V8;
+
+	ret = of_property_read_u32(node, "atheros,clk-out-frequency", &freq);
+	if (!ret) {
+		switch (freq) {
+		case 25000000:
+			sel = AT803X_CLK_OUT_25MHZ_XTAL;
+			break;
+		case 50000000:
+			sel = AT803X_CLK_OUT_50MHZ_PLL;
+			break;
+		case 62500000:
+			sel = AT803X_CLK_OUT_62_5MHZ_PLL;
+			break;
+		case 125000000:
+			sel = AT803X_CLK_OUT_125MHZ_PLL;
+			break;
+		default:
+			phydev_err(phydev,
+				   "invalid atheros,clk-out-frequency\n");
+			return -EINVAL;
+		}
+
+		priv->clk_25m_reg |= FIELD_PREP(AT803X_CLK_OUT_MASK, sel);
+		priv->clk_25m_mask |= AT803X_CLK_OUT_MASK;
+	}
+
+	ret = of_property_read_u32(node, "atheros,clk-out-strength", &strength);
+	if (!ret) {
+		priv->clk_25m_mask |= AT803X_CLK_OUT_STRENGTH_MASK;
+		switch (strength) {
+		case AT803X_STRENGTH_FULL:
+			priv->clk_25m_reg |= AT803X_CLK_OUT_STRENGTH_FULL;
+			break;
+		case AT803X_STRENGTH_HALF:
+			priv->clk_25m_reg |= AT803X_CLK_OUT_STRENGTH_HALF;
+			break;
+		case AT803X_STRENGTH_QUARTER:
+			priv->clk_25m_reg |= AT803X_CLK_OUT_STRENGTH_QUARTER;
+			break;
+		default:
+			phydev_err(phydev,
+				   "invalid atheros,clk-out-strength\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int at803x_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -251,11 +357,31 @@ static int at803x_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
-	return 0;
+	return at803x_parse_dt(phydev);
+}
+
+static int at803x_clk_out_config(struct phy_device *phydev)
+{
+	struct at803x_priv *priv = phydev->priv;
+	int val;
+
+	if (!priv->clk_25m_mask)
+		return 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M);
+	if (val < 0)
+		return val;
+
+	val &= ~priv->clk_25m_mask;
+	val |= priv->clk_25m_reg;
+
+	return phy_write_mmd(phydev, MDIO_MMD_AN, AT803X_MMD7_CLK25M, val);
 }
 
 static int at803x_config_init(struct phy_device *phydev)
 {
+	struct at803x_priv *priv = phydev->priv;
+	u16 set = 0, clear = 0;
 	int ret;
 
 	/* The RX and TX delay default is:
@@ -276,8 +402,34 @@ static int at803x_config_init(struct phy_device *phydev)
 		ret = at803x_enable_tx_delay(phydev);
 	else
 		ret = at803x_disable_tx_delay(phydev);
+	if (ret < 0)
+		return ret;
 
-	return ret;
+	ret = at803x_clk_out_config(phydev);
+	if (ret < 0)
+		return ret;
+
+	/* The default after hardware reset is:
+	 *   1.5V RGMII I/O voltage
+	 *   PLL OFF (which means it is off if there is no link)
+	 *
+	 * After a soft reset, the values are retained.
+	 */
+	if (priv->flags & AT803X_KEEP_PLL_ENABLED)
+		set |= AT803X_DEBUG_PLL_ON;
+	else
+		clear |= AT803X_DEBUG_PLL_ON;
+
+	if (priv->flags & AT803X_RGMII_1V8)
+		set |= AT803X_DEBUG_RGMII_1V8;
+	else
+		clear |= AT803X_DEBUG_RGMII_1V8;
+
+	ret = at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_1F, clear, set);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static int at803x_ack_interrupt(struct phy_device *phydev)
-- 
2.20.1


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

* Re: [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description
  2019-10-30 22:42 ` [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description Michael Walle
@ 2019-10-30 23:13   ` Andrew Lunn
  2019-10-30 23:16   ` Florian Fainelli
  1 sibling, 0 replies; 23+ messages in thread
From: Andrew Lunn @ 2019-10-30 23:13 UTC (permalink / raw)
  To: Michael Walle; +Cc: linux-kernel, devicetree, netdev

On Wed, Oct 30, 2019 at 11:42:49PM +0100, Michael Walle wrote:
> The name of the PHY is actually AR803x not AT803x. Additionally, add the
> name of the vendor and mention the AR8031 support.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>

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

    Andrew

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

* Re: [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description
  2019-10-30 22:42 ` [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description Michael Walle
  2019-10-30 23:13   ` Andrew Lunn
@ 2019-10-30 23:16   ` Florian Fainelli
  2019-10-30 23:18     ` Andrew Lunn
  1 sibling, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2019-10-30 23:16 UTC (permalink / raw)
  To: Michael Walle, linux-kernel, devicetree, netdev

On 10/30/19 3:42 PM, Michael Walle wrote:
> The name of the PHY is actually AR803x not AT803x. Additionally, add the
> name of the vendor and mention the AR8031 support.

Should not the vendor be QCA these days, or Qualcomm Atheros?
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/net/phy/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index fe602648b99f..38f180f9ca42 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -283,9 +283,9 @@ config AX88796B_PHY
>  	  AX88796B package.
>  
>  config AT803X_PHY
> -	tristate "AT803X PHYs"
> +	tristate "Atheros AR803X PHYs"
>  	---help---
> -	  Currently supports the AT8030 and AT8035 model
> +	  Currently supports the AR8030, AR8031 and AR8035 model
>  
>  config BCM63XX_PHY
>  	tristate "Broadcom 63xx SOCs internal PHY"
> 


-- 
Florian

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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-30 22:42 ` [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X Michael Walle
@ 2019-10-30 23:17   ` Andrew Lunn
  2019-10-31  0:14     ` Michael Walle
  2019-10-30 23:28   ` Florian Fainelli
  2019-11-01 15:03   ` Simon Horman
  2 siblings, 1 reply; 23+ messages in thread
From: Andrew Lunn @ 2019-10-30 23:17 UTC (permalink / raw)
  To: Michael Walle; +Cc: linux-kernel, devicetree, netdev

On Wed, Oct 30, 2019 at 11:42:50PM +0100, Michael Walle wrote:
> Document the Atheros AR803x PHY bindings.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  .../bindings/net/atheros,at803x.yaml          | 58 +++++++++++++++++++
>  include/dt-bindings/net/atheros-at803x.h      | 13 +++++
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/atheros,at803x.yaml
>  create mode 100644 include/dt-bindings/net/atheros-at803x.h
> 
> diff --git a/Documentation/devicetree/bindings/net/atheros,at803x.yaml b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
> new file mode 100644
> index 000000000000..60500fd90fd8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/atheros,at803x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atheros AR803x PHY
> +
> +maintainers:
> +  - TBD

Hi Michael

If you don't want to maintain it, then list the PHY maintainers.

> +
> +description: |
> +  Bindings for Atheros AR803x PHYs
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  atheros,clk-out-frequency:
> +    description: Clock output frequency in Hertz.
> +    enum: [ 25000000, 50000000, 62500000, 125000000 ]
> +
> +  atheros,clk-out-strength:
> +    description: Clock output driver strength.
> +    enum: [ 0, 1, 2 ]
> +
> +  atheros,keep-pll-enabled:
> +    description: |
> +      If set, keep the PLL enabled even if there is no link. Useful if you
> +      want to use the clock output without an ethernet link.
> +    type: boolean
> +
> +  atheros,rgmii-io-1v8:
> +    description: |
> +      The PHY supports RGMII I/O voltages of 2.5V, 1.8V and 1.5V. By default,
> +      the PHY uses a voltage of 1.5V. If this is set, the voltage will changed
> +      to 1.8V.
> +      The 2.5V voltage is only supported with an external supply voltage.

So we can later add atheros,rgmii-io-2v5. That might need a regulator
as well. Maybe add that 2.5V is currently not supported.

   Andrew

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

* Re: [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description
  2019-10-30 23:16   ` Florian Fainelli
@ 2019-10-30 23:18     ` Andrew Lunn
  2019-10-30 23:32       ` Florian Fainelli
  0 siblings, 1 reply; 23+ messages in thread
From: Andrew Lunn @ 2019-10-30 23:18 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Michael Walle, linux-kernel, devicetree, netdev

On Wed, Oct 30, 2019 at 04:16:01PM -0700, Florian Fainelli wrote:
> On 10/30/19 3:42 PM, Michael Walle wrote:
> > The name of the PHY is actually AR803x not AT803x. Additionally, add the
> > name of the vendor and mention the AR8031 support.
> 
> Should not the vendor be QCA these days, or Qualcomm Atheros?

Atheros Qualcomm would work best in terms of not upsetting the sort
order.

	Andrew

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

* Re: [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-30 22:42 ` [RFC PATCH 3/3] net: phy: at803x: add device tree binding Michael Walle
@ 2019-10-30 23:21   ` Andrew Lunn
  2019-10-30 23:28   ` Florian Fainelli
  1 sibling, 0 replies; 23+ messages in thread
From: Andrew Lunn @ 2019-10-30 23:21 UTC (permalink / raw)
  To: Michael Walle; +Cc: linux-kernel, devicetree, netdev

On Wed, Oct 30, 2019 at 11:42:51PM +0100, Michael Walle wrote:
> Add support for configuring the CLK_25M pin as well as the RGMII I/O
> voltage by the device tree.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>

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

    Andrew

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

* Re: [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-30 22:42 ` [RFC PATCH 3/3] net: phy: at803x: add device tree binding Michael Walle
  2019-10-30 23:21   ` Andrew Lunn
@ 2019-10-30 23:28   ` Florian Fainelli
  2019-10-30 23:59     ` Michael Walle
  1 sibling, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2019-10-30 23:28 UTC (permalink / raw)
  To: Michael Walle, linux-kernel, devicetree, netdev, Andrew Lunn,
	Heiner Kallweit

On 10/30/19 3:42 PM, Michael Walle wrote:
> Add support for configuring the CLK_25M pin as well as the RGMII I/O
> voltage by the device tree.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/net/phy/at803x.c | 156 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 154 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 1eb5d4fb8925..32be4c72cf4b 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -13,7 +13,9 @@
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h>
>  #include <linux/of_gpio.h>
> +#include <linux/bitfield.h>
>  #include <linux/gpio/consumer.h>
> +#include <dt-bindings/net/atheros-at803x.h>
>  
>  #define AT803X_SPECIFIC_STATUS			0x11
>  #define AT803X_SS_SPEED_MASK			(3 << 14)
> @@ -62,6 +64,37 @@
>  #define AT803X_DEBUG_REG_5			0x05
>  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
>  
> +#define AT803X_DEBUG_REG_1F			0x1F
> +#define AT803X_DEBUG_PLL_ON			BIT(2)
> +#define AT803X_DEBUG_RGMII_1V8			BIT(3)
> +
> +/* AT803x supports either the XTAL input pad, an internal PLL or the
> + * DSP as clock reference for the clock output pad. The XTAL reference
> + * is only used for 25 MHz output, all other frequencies need the PLL.
> + * The DSP as a clock reference is used in synchronous ethernet
> + * applications.

How does that tie in the mode in which the PHY is configured? In reverse
MII mode, the PHY provides the TX clock which can be either 25Mhz or
50Mhz AFAIR, in RGMII mode, the TXC provided by the MAC is internally
resynchronized and then fed back to the MAC as a 125Mhz clock.

Do you possibly need to cross check the clock output selection with the
PHY interface?

[snip]
> +static int at803x_parse_dt(struct phy_device *phydev)
> +{
> +	struct device_node *node = phydev->mdio.dev.of_node;
> +	struct at803x_priv *priv = phydev->priv;
> +	u32 freq, strength;
> +	unsigned int sel;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_OF_MDIO))
> +		return 0;
> +
> +	if (!node)
> +		return 0;

I don't think you need either of those two things, every of_* function
would check whether the node reference is non-NULL.

> +
> +	if (of_property_read_bool(node, "atheros,keep-pll-enabled"))
> +		priv->flags |= AT803X_KEEP_PLL_ENABLED;

This should probably be a PHY tunable rather than a Device Tree property
as this delves more into the policy than the pure hardware description.

> +
> +	if (of_property_read_bool(node, "atheros,rgmii-io-1v8"))
> +		priv->flags |= AT803X_RGMII_1V8;> +
> +	ret = of_property_read_u32(node, "atheros,clk-out-frequency", &freq);
> +	if (!ret) {
> +		switch (freq) {
> +		case 25000000:
> +			sel = AT803X_CLK_OUT_25MHZ_XTAL;
> +			break;
> +		case 50000000:
> +			sel = AT803X_CLK_OUT_50MHZ_PLL;
> +			break;
> +		case 62500000:
> +			sel = AT803X_CLK_OUT_62_5MHZ_PLL;
> +			break;
> +		case 125000000:
> +			sel = AT803X_CLK_OUT_125MHZ_PLL;
> +			break;
> +		default:
> +			phydev_err(phydev,
> +				   "invalid atheros,clk-out-frequency\n");
> +			return -EINVAL;
> +		}

Maybe the PHY should be a clock provider of some sort, this might be
especially important if the PHY supplies the Ethernet MAC's RXC (which
would be the case in a RGMII configuration).
-- 
Florian

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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-30 22:42 ` [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X Michael Walle
  2019-10-30 23:17   ` Andrew Lunn
@ 2019-10-30 23:28   ` Florian Fainelli
  2019-10-30 23:36     ` Michael Walle
  2019-11-01 15:03   ` Simon Horman
  2 siblings, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2019-10-30 23:28 UTC (permalink / raw)
  To: Michael Walle, linux-kernel, devicetree, netdev

On 10/30/19 3:42 PM, Michael Walle wrote:
> Document the Atheros AR803x PHY bindings.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  .../bindings/net/atheros,at803x.yaml          | 58 +++++++++++++++++++
>  include/dt-bindings/net/atheros-at803x.h      | 13 +++++
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/atheros,at803x.yaml
>  create mode 100644 include/dt-bindings/net/atheros-at803x.h
> 
> diff --git a/Documentation/devicetree/bindings/net/atheros,at803x.yaml b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
> new file mode 100644
> index 000000000000..60500fd90fd8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/atheros,at803x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atheros AR803x PHY
> +
> +maintainers:
> +  - TBD
> +
> +description: |
> +  Bindings for Atheros AR803x PHYs
> +
> +allOf:
> +  - $ref: ethernet-phy.yaml#
> +
> +properties:
> +  atheros,clk-out-frequency:
> +    description: Clock output frequency in Hertz.
> +    enum: [ 25000000, 50000000, 62500000, 125000000 ]
> +
> +  atheros,clk-out-strength:
> +    description: Clock output driver strength.
> +    enum: [ 0, 1, 2 ]
> +
> +  atheros,keep-pll-enabled:
> +    description: |
> +      If set, keep the PLL enabled even if there is no link. Useful if you
> +      want to use the clock output without an ethernet link.

This is more of a policy than a hardware description. Implementing this
has a PHY tunable, possibly as a form of auto-power down

> +    type: boolean
> +
> +  atheros,rgmii-io-1v8:
> +    description: |
> +      The PHY supports RGMII I/O voltages of 2.5V, 1.8V and 1.5V. By default,
> +      the PHY uses a voltage of 1.5V. If this is set, the voltage will changed
> +      to 1.8V.

will be changed?

This looks like a possibly dangerous configuration as it really can lead
to some good damage happening on the pins if there is an incompatible
voltage on the MAC and PHY side... of course, you have no way to tell
ahead of time other than by looking at the board schematics, lovely.

Does the PHY come up in some sort of super isolatation mode by default
at least?
-- 
Florian

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

* Re: [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description
  2019-10-30 23:18     ` Andrew Lunn
@ 2019-10-30 23:32       ` Florian Fainelli
  2019-10-31  0:05         ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2019-10-30 23:32 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Michael Walle, linux-kernel, devicetree, netdev

On 10/30/19 4:18 PM, Andrew Lunn wrote:
> On Wed, Oct 30, 2019 at 04:16:01PM -0700, Florian Fainelli wrote:
>> On 10/30/19 3:42 PM, Michael Walle wrote:
>>> The name of the PHY is actually AR803x not AT803x. Additionally, add the
>>> name of the vendor and mention the AR8031 support.
>>
>> Should not the vendor be QCA these days, or Qualcomm Atheros?
> 
> Atheros Qualcomm would work best in terms of not upsetting the sort
> order.

Yes except the company is actually named Qualcomm Atheros:

https://en.wikipedia.org/wiki/Qualcomm_Atheros
-- 
Florian

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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-30 23:28   ` Florian Fainelli
@ 2019-10-30 23:36     ` Michael Walle
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-30 23:36 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel, devicetree, netdev

Am 31. Oktober 2019 00:28:47 MEZ schrieb Florian Fainelli <f.fainelli@gmail.com>:
>On 10/30/19 3:42 PM, Michael Walle wrote:
>> Document the Atheros AR803x PHY bindings.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  .../bindings/net/atheros,at803x.yaml          | 58
>+++++++++++++++++++
>>  include/dt-bindings/net/atheros-at803x.h      | 13 +++++
>>  2 files changed, 71 insertions(+)
>>  create mode 100644
>Documentation/devicetree/bindings/net/atheros,at803x.yaml
>>  create mode 100644 include/dt-bindings/net/atheros-at803x.h
>> 
>> diff --git
>a/Documentation/devicetree/bindings/net/atheros,at803x.yaml
>b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
>> new file mode 100644
>> index 000000000000..60500fd90fd8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/atheros,at803x.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atheros AR803x PHY
>> +
>> +maintainers:
>> +  - TBD
>> +
>> +description: |
>> +  Bindings for Atheros AR803x PHYs
>> +
>> +allOf:
>> +  - $ref: ethernet-phy.yaml#
>> +
>> +properties:
>> +  atheros,clk-out-frequency:
>> +    description: Clock output frequency in Hertz.
>> +    enum: [ 25000000, 50000000, 62500000, 125000000 ]
>> +
>> +  atheros,clk-out-strength:
>> +    description: Clock output driver strength.
>> +    enum: [ 0, 1, 2 ]
>> +
>> +  atheros,keep-pll-enabled:
>> +    description: |
>> +      If set, keep the PLL enabled even if there is no link. Useful
>if you
>> +      want to use the clock output without an ethernet link.
>
>This is more of a policy than a hardware description. Implementing this
>has a PHY tunable, possibly as a form of auto-power down
>
>> +    type: boolean
>> +
>> +  atheros,rgmii-io-1v8:
>> +    description: |
>> +      The PHY supports RGMII I/O voltages of 2.5V, 1.8V and 1.5V. By
>default,
>> +      the PHY uses a voltage of 1.5V. If this is set, the voltage
>will changed
>> +      to 1.8V.
>
>will be changed?

oh.. yes of course. 

>This looks like a possibly dangerous configuration as it really can
>lead
>to some good damage happening on the pins if there is an incompatible
>voltage on the MAC and PHY side... of course, you have no way to tell
>ahead of time other than by looking at the board schematics, lovely.

correct.. although the standard mode of 1.5V has a max high voltage of 1.8V so this seems to be safe. But I guess no one has ever really though about how to really configure that safely.

>Does the PHY come up in some sort of super isolatation mode by default
>at least?

not that I'm aware of. also.. the rgmii mode just works without any setup (apart from the delay and voltage settings) 

-michael 


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

* Re: [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-30 23:28   ` Florian Fainelli
@ 2019-10-30 23:59     ` Michael Walle
  2019-10-31 17:22       ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2019-10-30 23:59 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel, devicetree, netdev, Andrew Lunn,
	Heiner Kallweit

Am 31. Oktober 2019 00:28:02 MEZ schrieb Florian Fainelli <f.fainelli@gmail.com>:
>On 10/30/19 3:42 PM, Michael Walle wrote:
>> Add support for configuring the CLK_25M pin as well as the RGMII I/O
>> voltage by the device tree.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  drivers/net/phy/at803x.c | 156
>++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 154 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>> index 1eb5d4fb8925..32be4c72cf4b 100644
>> --- a/drivers/net/phy/at803x.c
>> +++ b/drivers/net/phy/at803x.c
>> @@ -13,7 +13,9 @@
>>  #include <linux/netdevice.h>
>>  #include <linux/etherdevice.h>
>>  #include <linux/of_gpio.h>
>> +#include <linux/bitfield.h>
>>  #include <linux/gpio/consumer.h>
>> +#include <dt-bindings/net/atheros-at803x.h>
>>  
>>  #define AT803X_SPECIFIC_STATUS			0x11
>>  #define AT803X_SS_SPEED_MASK			(3 << 14)
>> @@ -62,6 +64,37 @@
>>  #define AT803X_DEBUG_REG_5			0x05
>>  #define AT803X_DEBUG_TX_CLK_DLY_EN		BIT(8)
>>  
>> +#define AT803X_DEBUG_REG_1F			0x1F
>> +#define AT803X_DEBUG_PLL_ON			BIT(2)
>> +#define AT803X_DEBUG_RGMII_1V8			BIT(3)
>> +
>> +/* AT803x supports either the XTAL input pad, an internal PLL or the
>> + * DSP as clock reference for the clock output pad. The XTAL
>reference
>> + * is only used for 25 MHz output, all other frequencies need the
>PLL.
>> + * The DSP as a clock reference is used in synchronous ethernet
>> + * applications.
>
>How does that tie in the mode in which the PHY is configured? In
>reverse
>MII mode, the PHY provides the TX clock which can be either 25Mhz or
>50Mhz AFAIR, in RGMII mode, the TXC provided by the MAC is internally
>resynchronized and then fed back to the MAC as a 125Mhz clock.
>
>Do you possibly need to cross check the clock output selection with the
>PHY interface?

what do you mean by mode? the "clock output pad" (maybe the term is wrong) is just an additional clock output. And I've ignored syncE mode for now. I don't think there is a real use case for now. because in almost all cases the clock out is used to generate 125MHz required by an RGMII core in the SoC. 


>
>[snip]
>> +static int at803x_parse_dt(struct phy_device *phydev)
>> +{
>> +	struct device_node *node = phydev->mdio.dev.of_node;
>> +	struct at803x_priv *priv = phydev->priv;
>> +	u32 freq, strength;
>> +	unsigned int sel;
>> +	int ret;
>> +
>> +	if (!IS_ENABLED(CONFIG_OF_MDIO))
>> +		return 0;
>> +
>> +	if (!node)
>> +		return 0;
>
>I don't think you need either of those two things, every of_* function
>would check whether the node reference is non-NULL.

The first is needed because otherwise the of_* return -ENOSYS IIRC. I guess it would make no difference here though. Although I don't know how clever the compiler is as it could optimize the whole function away if CONFIG_OF_MDIO is not enabled. 

>> +
>> +	if (of_property_read_bool(node, "atheros,keep-pll-enabled"))
>> +		priv->flags |= AT803X_KEEP_PLL_ENABLED;
>
>This should probably be a PHY tunable rather than a Device Tree
>property
>as this delves more into the policy than the pure hardware description.

To be frank. I'll first need to look into PHY tunables before answering ;) 
But keep in mind that this clock output might be used anywhere on the board. It must not have something to do with networking. The PHY has a crystal and it can generate these couple of frequencies regardless of its network operation. 

>> +
>> +	if (of_property_read_bool(node, "atheros,rgmii-io-1v8"))
>> +		priv->flags |= AT803X_RGMII_1V8;> +
>> +	ret = of_property_read_u32(node, "atheros,clk-out-frequency",
>&freq);
>> +	if (!ret) {
>> +		switch (freq) {
>> +		case 25000000:
>> +			sel = AT803X_CLK_OUT_25MHZ_XTAL;
>> +			break;
>> +		case 50000000:
>> +			sel = AT803X_CLK_OUT_50MHZ_PLL;
>> +			break;
>> +		case 62500000:
>> +			sel = AT803X_CLK_OUT_62_5MHZ_PLL;
>> +			break;
>> +		case 125000000:
>> +			sel = AT803X_CLK_OUT_125MHZ_PLL;
>> +			break;
>> +		default:
>> +			phydev_err(phydev,
>> +				   "invalid atheros,clk-out-frequency\n");
>> +			return -EINVAL;
>> +		}
>
>Maybe the PHY should be a clock provider of some sort, this might be
>especially important if the PHY supplies the Ethernet MAC's RXC (which
>would be the case in a RGMII configuration).

Could be the case, I don't know. I'm developing this on a NXP layerscape LS1028A and this SoC needs a fixed 125MHz clock for its RGMII interface (regardless if its 10/100 or 100 Mbit/s). I guess the same is true for the i.MX series. 

-michael 


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

* Re: [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description
  2019-10-30 23:32       ` Florian Fainelli
@ 2019-10-31  0:05         ` Michael Walle
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-31  0:05 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn; +Cc: linux-kernel, devicetree, netdev

Am 31. Oktober 2019 00:32:15 MEZ schrieb Florian Fainelli <f.fainelli@gmail.com>:
>On 10/30/19 4:18 PM, Andrew Lunn wrote:
>> On Wed, Oct 30, 2019 at 04:16:01PM -0700, Florian Fainelli wrote:
>>> On 10/30/19 3:42 PM, Michael Walle wrote:
>>>> The name of the PHY is actually AR803x not AT803x. Additionally,
>add the
>>>> name of the vendor and mention the AR8031 support.
>>>
>>> Should not the vendor be QCA these days, or Qualcomm Atheros?
>> 
>> Atheros Qualcomm would work best in terms of not upsetting the sort
>> order.
>
>Yes except the company is actually named Qualcomm Atheros:
>
>https://en.wikipedia.org/wiki/Qualcomm_Atheros

Correct.. my bad. also the atheros prefix is wrong then.. because there is already a qca prefix.

but.. should i also reorder the entry in the Kconfig then? 

-michael 


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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-30 23:17   ` Andrew Lunn
@ 2019-10-31  0:14     ` Michael Walle
  2019-10-31 16:45       ` Florian Fainelli
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2019-10-31  0:14 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: linux-kernel, devicetree, netdev

Am 31. Oktober 2019 00:17:06 MEZ schrieb Andrew Lunn <andrew@lunn.ch>:
>On Wed, Oct 30, 2019 at 11:42:50PM +0100, Michael Walle wrote:
>> Document the Atheros AR803x PHY bindings.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  .../bindings/net/atheros,at803x.yaml          | 58
>+++++++++++++++++++
>>  include/dt-bindings/net/atheros-at803x.h      | 13 +++++
>>  2 files changed, 71 insertions(+)
>>  create mode 100644
>Documentation/devicetree/bindings/net/atheros,at803x.yaml
>>  create mode 100644 include/dt-bindings/net/atheros-at803x.h
>> 
>> diff --git
>a/Documentation/devicetree/bindings/net/atheros,at803x.yaml
>b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
>> new file mode 100644
>> index 000000000000..60500fd90fd8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/atheros,at803x.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/atheros,at803x.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atheros AR803x PHY
>> +
>> +maintainers:
>> +  - TBD
>
>Hi Michael
>
>If you don't want to maintain it, then list the PHY maintainers.
>
>> +
>> +description: |
>> +  Bindings for Atheros AR803x PHYs
>> +
>> +allOf:
>> +  - $ref: ethernet-phy.yaml#
>> +
>> +properties:
>> +  atheros,clk-out-frequency:
>> +    description: Clock output frequency in Hertz.
>> +    enum: [ 25000000, 50000000, 62500000, 125000000 ]
>> +
>> +  atheros,clk-out-strength:
>> +    description: Clock output driver strength.
>> +    enum: [ 0, 1, 2 ]
>> +
>> +  atheros,keep-pll-enabled:
>> +    description: |
>> +      If set, keep the PLL enabled even if there is no link. Useful
>if you
>> +      want to use the clock output without an ethernet link.
>> +    type: boolean
>> +
>> +  atheros,rgmii-io-1v8:
>> +    description: |
>> +      The PHY supports RGMII I/O voltages of 2.5V, 1.8V and 1.5V. By
>default,
>> +      the PHY uses a voltage of 1.5V. If this is set, the voltage
>will changed
>> +      to 1.8V.
>> +      The 2.5V voltage is only supported with an external supply
>voltage.
>
>So we can later add atheros,rgmii-io-2v5. That might need a regulator
>as well. Maybe add that 2.5V is currently not supported.

There is no special setting for the 2.5V mode. This is how it works: there is one voltage pad for the RGMII interface. Either you connect this pad to a 2.5V voltage or you leave it open (well you would connect some decoupling Cs). If you leave it open the internal LDO, which seems to be enabled in any case takes over, supplying 1.5V. then there is a bit in the debug register which can switch the internal LDO to 1.8V. So if you'll use 2.5V the bit is irrelevant. 

Like I said maybe a "rgmii-io-microvolts" is a better property and only in the 1800000 setting would turn on this bit. but then both other setting would be a noop. 

-michael 


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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-31  0:14     ` Michael Walle
@ 2019-10-31 16:45       ` Florian Fainelli
  2019-10-31 17:14         ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2019-10-31 16:45 UTC (permalink / raw)
  To: Michael Walle, Andrew Lunn; +Cc: linux-kernel, devicetree, netdev

On 10/30/19 5:14 PM, Michael Walle wrote:
>> So we can later add atheros,rgmii-io-2v5. That might need a regulator
>> as well. Maybe add that 2.5V is currently not supported.
> 
> There is no special setting for the 2.5V mode. This is how it works: there is one voltage pad for the RGMII interface. Either you connect this pad to a 2.5V voltage or you leave it open (well you would connect some decoupling Cs). If you leave it open the internal LDO, which seems to be enabled in any case takes over, supplying 1.5V. then there is a bit in the debug register which can switch the internal LDO to 1.8V. So if you'll use 2.5V the bit is irrelevant. 
> 
> Like I said maybe a "rgmii-io-microvolts" is a better property and only in the 1800000 setting would turn on this bit. but then both other setting would be a noop. 

That would align with the regulator subsystem units, but maybe you
should have the PHY driver be a regulator provider for itself so you can
chose wether you want to operate at 1.5V or 1.8V, or you have an
external regulator providing I/O supplies. That would make the whole
thing consistent from the driver's perspective and would not necessarily
be too far fetched from a HW description perspective?
--
Florian

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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-31 16:45       ` Florian Fainelli
@ 2019-10-31 17:14         ` Michael Walle
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Walle @ 2019-10-31 17:14 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Andrew Lunn, linux-kernel, devicetree, netdev

Am 2019-10-31 17:45, schrieb Florian Fainelli:
> On 10/30/19 5:14 PM, Michael Walle wrote:
>>> So we can later add atheros,rgmii-io-2v5. That might need a regulator
>>> as well. Maybe add that 2.5V is currently not supported.
>> 
>> There is no special setting for the 2.5V mode. This is how it works: 
>> there is one voltage pad for the RGMII interface. Either you connect 
>> this pad to a 2.5V voltage or you leave it open (well you would 
>> connect some decoupling Cs). If you leave it open the internal LDO, 
>> which seems to be enabled in any case takes over, supplying 1.5V. then 
>> there is a bit in the debug register which can switch the internal LDO 
>> to 1.8V. So if you'll use 2.5V the bit is irrelevant.
>> 
>> Like I said maybe a "rgmii-io-microvolts" is a better property and 
>> only in the 1800000 setting would turn on this bit. but then both 
>> other setting would be a noop.
> 
> That would align with the regulator subsystem units, but maybe you
> should have the PHY driver be a regulator provider for itself so you 
> can
> chose wether you want to operate at 1.5V or 1.8V, or you have an
> external regulator providing I/O supplies. That would make the whole
> thing consistent from the driver's perspective and would not 
> necessarily
> be too far fetched from a HW description perspective?

Sounds good. But again, I'm not too familiar with that. Could you give 
an
example how the device tree would look like then? Maybe that way I can 
work
myself through that regulator stuff.

-- 
michael

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

* Re: [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-30 23:59     ` Michael Walle
@ 2019-10-31 17:22       ` Michael Walle
  2019-10-31 17:35         ` Florian Fainelli
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Walle @ 2019-10-31 17:22 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel, devicetree, netdev, Andrew Lunn,
	Heiner Kallweit

Am 2019-10-31 00:59, schrieb Michael Walle:
>>> +
>>> +	if (of_property_read_bool(node, "atheros,keep-pll-enabled"))
>>> +		priv->flags |= AT803X_KEEP_PLL_ENABLED;
>> 
>> This should probably be a PHY tunable rather than a Device Tree
>> property
>> as this delves more into the policy than the pure hardware 
>> description.
> 
> To be frank. I'll first need to look into PHY tunables before answering 
> ;)
> But keep in mind that this clock output might be used anywhere on the
> board. It must not have something to do with networking. The PHY has a
> crystal and it can generate these couple of frequencies regardless of
> its network operation.

Although it could be used to provide any clock on the board, I don't 
know
if that is possible at the moment, because the PHY is configured in
config_init() which is only called when someone brings the interface up,
correct?

Anyway, I don't know if that is worth the hassle because in almost all
cases the use case is to provide a fixed clock to the MAC for an RGMII
interface. I don't know if that really fits a PHY tunable, because in
the worst case the link won't work at all if the SoC expects an
always-on clock.

-- 
-michael

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

* Re: [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-31 17:22       ` Michael Walle
@ 2019-10-31 17:35         ` Florian Fainelli
  2019-11-02  1:18           ` Michael Walle
  0 siblings, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2019-10-31 17:35 UTC (permalink / raw)
  To: Michael Walle, linux-kernel, devicetree, netdev, Andrew Lunn,
	Heiner Kallweit

On 10/31/19 10:22 AM, Michael Walle wrote:
> Am 2019-10-31 00:59, schrieb Michael Walle:
>>>> +
>>>> +    if (of_property_read_bool(node, "atheros,keep-pll-enabled"))
>>>> +        priv->flags |= AT803X_KEEP_PLL_ENABLED;
>>>
>>> This should probably be a PHY tunable rather than a Device Tree
>>> property
>>> as this delves more into the policy than the pure hardware description.
>>
>> To be frank. I'll first need to look into PHY tunables before
>> answering ;)
>> But keep in mind that this clock output might be used anywhere on the
>> board. It must not have something to do with networking. The PHY has a
>> crystal and it can generate these couple of frequencies regardless of
>> its network operation.
> 
> Although it could be used to provide any clock on the board, I don't know
> if that is possible at the moment, because the PHY is configured in
> config_init() which is only called when someone brings the interface up,
> correct?
> 
> Anyway, I don't know if that is worth the hassle because in almost all
> cases the use case is to provide a fixed clock to the MAC for an RGMII
> interface. I don't know if that really fits a PHY tunable, because in
> the worst case the link won't work at all if the SoC expects an
> always-on clock.

Well, that was my question really, is the clock output being controlled
the actual RXC that will feed back to the MAC or is this is another
clock output pin (sorry if you indicated that before and I missed it)?

If this is the PHY's RXC, then does the configuration (DSP, PLL, XTAL)
matter at all on the generated output frequency, or is this just a
choice for the board designer, and whether the PHY is configured for
MII/RGMII, it outputs the appropriate clock at 25/125Mhz?
-- 
Florian

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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-10-30 22:42 ` [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X Michael Walle
  2019-10-30 23:17   ` Andrew Lunn
  2019-10-30 23:28   ` Florian Fainelli
@ 2019-11-01 15:03   ` Simon Horman
  2019-11-02  1:19     ` Michael Walle
  2 siblings, 1 reply; 23+ messages in thread
From: Simon Horman @ 2019-11-01 15:03 UTC (permalink / raw)
  To: Michael Walle; +Cc: linux-kernel, devicetree, netdev

On Wed, Oct 30, 2019 at 11:42:50PM +0100, Michael Walle wrote:
> Document the Atheros AR803x PHY bindings.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  .../bindings/net/atheros,at803x.yaml          | 58 +++++++++++++++++++
>  include/dt-bindings/net/atheros-at803x.h      | 13 +++++
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/atheros,at803x.yaml
>  create mode 100644 include/dt-bindings/net/atheros-at803x.h

Hi Michael,

please run the schema past dtbs_check as per the instructions in
Documentation/devicetree/writing-schema.rst

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

* Re: [RFC PATCH 3/3] net: phy: at803x: add device tree binding
  2019-10-31 17:35         ` Florian Fainelli
@ 2019-11-02  1:18           ` Michael Walle
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Walle @ 2019-11-02  1:18 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel, devicetree, netdev, Andrew Lunn, Heiner Kallweit

Am 2019-10-31 18:35, schrieb Florian Fainelli:
> On 10/31/19 10:22 AM, Michael Walle wrote:
>> Am 2019-10-31 00:59, schrieb Michael Walle:
>>>>> +
>>>>> +    if (of_property_read_bool(node, "atheros,keep-pll-enabled"))
>>>>> +        priv->flags |= AT803X_KEEP_PLL_ENABLED;
>>>> 
>>>> This should probably be a PHY tunable rather than a Device Tree
>>>> property
>>>> as this delves more into the policy than the pure hardware 
>>>> description.
>>> 
>>> To be frank. I'll first need to look into PHY tunables before
>>> answering ;)
>>> But keep in mind that this clock output might be used anywhere on the
>>> board. It must not have something to do with networking. The PHY has 
>>> a
>>> crystal and it can generate these couple of frequencies regardless of
>>> its network operation.
>> 
>> Although it could be used to provide any clock on the board, I don't 
>> know
>> if that is possible at the moment, because the PHY is configured in
>> config_init() which is only called when someone brings the interface 
>> up,
>> correct?
>> 
>> Anyway, I don't know if that is worth the hassle because in almost all
>> cases the use case is to provide a fixed clock to the MAC for an RGMII
>> interface. I don't know if that really fits a PHY tunable, because in
>> the worst case the link won't work at all if the SoC expects an
>> always-on clock.
> 
> Well, that was my question really, is the clock output being controlled
> the actual RXC that will feed back to the MAC or is this is another
> clock output pin (sorry if you indicated that before and I missed it)?

No it is not the RX clock. The PHY has three clock pins, RX clock, TX
clock and a general purpose CLK_25M pin.

> If this is the PHY's RXC, then does the configuration (DSP, PLL, XTAL)
> matter at all on the generated output frequency, or is this just a
> choice for the board designer, and whether the PHY is configured for
> MII/RGMII, it outputs the appropriate clock at 25/125Mhz?

The RXC changes the frequency according to the speed.

-- 
-michael

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

* Re: [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X
  2019-11-01 15:03   ` Simon Horman
@ 2019-11-02  1:19     ` Michael Walle
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Walle @ 2019-11-02  1:19 UTC (permalink / raw)
  To: Simon Horman; +Cc: linux-kernel, devicetree, netdev

Am 2019-11-01 16:03, schrieb Simon Horman:
> On Wed, Oct 30, 2019 at 11:42:50PM +0100, Michael Walle wrote:
>> Document the Atheros AR803x PHY bindings.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  .../bindings/net/atheros,at803x.yaml          | 58 
>> +++++++++++++++++++
>>  include/dt-bindings/net/atheros-at803x.h      | 13 +++++
>>  2 files changed, 71 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/net/atheros,at803x.yaml
>>  create mode 100644 include/dt-bindings/net/atheros-at803x.h
> 
> Hi Michael,
> 
> please run the schema past dtbs_check as per the instructions in
> Documentation/devicetree/writing-schema.rst

Hi Simon,

Thank you, I've run the tests and fixed the errors.

-- 
-michael

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

end of thread, other threads:[~2019-11-02  1:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 22:42 [RFC PATCH 0/3] net: phy: at803x device tree binding Michael Walle
2019-10-30 22:42 ` [RFC PATCH 1/3] net: phy: at803x: fix Kconfig description Michael Walle
2019-10-30 23:13   ` Andrew Lunn
2019-10-30 23:16   ` Florian Fainelli
2019-10-30 23:18     ` Andrew Lunn
2019-10-30 23:32       ` Florian Fainelli
2019-10-31  0:05         ` Michael Walle
2019-10-30 22:42 ` [RFC PATCH 2/3] dt-bindings: net: phy: Add support for AT803X Michael Walle
2019-10-30 23:17   ` Andrew Lunn
2019-10-31  0:14     ` Michael Walle
2019-10-31 16:45       ` Florian Fainelli
2019-10-31 17:14         ` Michael Walle
2019-10-30 23:28   ` Florian Fainelli
2019-10-30 23:36     ` Michael Walle
2019-11-01 15:03   ` Simon Horman
2019-11-02  1:19     ` Michael Walle
2019-10-30 22:42 ` [RFC PATCH 3/3] net: phy: at803x: add device tree binding Michael Walle
2019-10-30 23:21   ` Andrew Lunn
2019-10-30 23:28   ` Florian Fainelli
2019-10-30 23:59     ` Michael Walle
2019-10-31 17:22       ` Michael Walle
2019-10-31 17:35         ` Florian Fainelli
2019-11-02  1:18           ` Michael Walle

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).