[PATCH v5 0/3] adin: add support for clock output

[PATCH v5 0/3] adin: add support for clock output

[Josua Mayer]

This patch series adds support for configuring the two clock outputs of adin
1200 and 1300 PHYs. Certain network controllers require an external reference
clock which can be provided by the PHY.

One of the replies to v1 was asking why the common clock framework isn't used.
Currently no PHY driver has implemented providing a clock to the network
controller. Instead they rely on vendor extensions to make the appropriate
configuration. For example ar8035 uses qca,clk-out-frequency - this patchset
aimed to replicate the same functionality.

Finally the 125MHz free-running clock is enabled in the device-tree for
SolidRun i.MX6 SoMs, to support revisions 1.9 and later, where the original phy
has been replaced with an adin 1300.
To avoid introducing new warning messages during boot for SoMs before rev 1.9,
the status field of the new phy node is disabled by default, and will be
enabled by U-Boot on demand.

Changes since v4:
- removed recovered clock options

Changes since v3:
- fix coding style violations reported by Andrew and checkpatch
- changed type of adi,phy-output-reference-clock from flag to boolean

Changes since v2:
- set new phy node status to disabled
- fix integer-as-null-pointer compiler warning
  Reported-by: kernel test robot <lkp@intel.com>

Changes since v1:
- renamed device-tree property and changed to enum
- added device-tree property for second clock output
- implemented all bits from the clock configuration register

Josua Mayer (3):
  dt-bindings: net: adin: document phy clock output properties
  net: phy: adin: add support for clock output
  ARM: dts: imx6qdl-sr-som: update phy configuration for som revision
    1.9

 .../devicetree/bindings/net/adi,adin.yaml     | 15 +++++++
 arch/arm/boot/dts/imx6qdl-sr-som.dtsi         | 10 +++++
 drivers/net/phy/adin.c                        | 40 +++++++++++++++++++
 3 files changed, 65 insertions(+)

-- 
2.35.3

[PATCH v5 1/3] dt-bindings: net: adin: document phy clock output properties

[Josua Mayer]

The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
well as providing the reference clock on CLK25_REF.

Add DT properties to configure both pins.

Technically the phy also supports a recovered 125MHz clock for
synchronous ethernet. However SyncE should be configured dynamically at
runtime, so it is explicitly omitted in this binding.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
V4 -> V5: removed recovered clock options
V3 -> V4: changed type of adi,phy-output-reference-clock to boolean
V1 -> V2: changed clkout property to enum
V1 -> V2: added property for CLK25_REF pin

 .../devicetree/bindings/net/adi,adin.yaml         | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
index 1129f2b58e98..77750df0c2c4 100644
--- a/Documentation/devicetree/bindings/net/adi,adin.yaml
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -36,6 +36,21 @@ properties:
     enum: [ 4, 8, 12, 16, 20, 24 ]
     default: 8
 
+  adi,phy-output-clock:
+    description: Select clock output on GP_CLK pin. Two clocks are available:
+      A 25MHz reference and a free-running 125MHz.
+      The phy can alternatively automatically switch between the reference and
+      the 125MHz clocks based on its internal state.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - 25mhz-reference
+      - 125mhz-free-running
+      - adaptive-free-running
+
+  adi,phy-output-reference-clock:
+    description: Enable 25MHz reference clock output on CLK25_REF pin.
+    type: boolean
+
 unevaluatedProperties: false
 
 examples:
-- 
2.35.3

[PATCH v5 2/3] net: phy: adin: add support for clock output

[Josua Mayer]

The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
well as providing the reference clock on CLK25_REF.

Add support for selecting the clock via device-tree properties.

Technically the phy also supports a recovered 125MHz clock for
synchronous ethernet. SyncE should be configured dynamically at
runtime, however Linux does not currently have a toggle for this,
so support is explicitly omitted.

Co-developed-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
Signed-off-by: Josua Mayer<josua@solid-run.com>
---
V4 -> V5: removed recovered clock options
V3 -> V4: fix coding style violations reported by Andrew and checkpatch
V2 -> V3: fix integer-as-null-pointer compiler warning
V1 -> V2: revised dts property name for clock(s)
V1 -> V2: implemented all 6 bits in the clock configuration register

 drivers/net/phy/adin.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 5ce6da62cc8e..ee374a85544a 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -99,6 +99,15 @@
 #define ADIN1300_GE_SOFT_RESET_REG		0xff0c
 #define   ADIN1300_GE_SOFT_RESET		BIT(0)
 
+#define ADIN1300_GE_CLK_CFG_REG			0xff1f
+#define   ADIN1300_GE_CLK_CFG_MASK		GENMASK(5, 0)
+#define   ADIN1300_GE_CLK_CFG_RCVR_125		BIT(5)
+#define   ADIN1300_GE_CLK_CFG_FREE_125		BIT(4)
+#define   ADIN1300_GE_CLK_CFG_REF_EN		BIT(3)
+#define   ADIN1300_GE_CLK_CFG_HRT_RCVR		BIT(2)
+#define   ADIN1300_GE_CLK_CFG_HRT_FREE		BIT(1)
+#define   ADIN1300_GE_CLK_CFG_25		BIT(0)
+
 #define ADIN1300_GE_RGMII_CFG_REG		0xff23
 #define   ADIN1300_GE_RGMII_RX_MSK		GENMASK(8, 6)
 #define   ADIN1300_GE_RGMII_RX_SEL(x)		\
@@ -433,6 +442,33 @@ static int adin_set_tunable(struct phy_device *phydev,
 	}
 }
 
+static int adin_config_clk_out(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	const char *val = NULL;
+	u8 sel = 0;
+
+	device_property_read_string(dev, "adi,phy-output-clock", &val);
+	if (!val) {
+		/* property not present, do not enable GP_CLK pin */
+	} else if (strcmp(val, "25mhz-reference") == 0) {
+		sel |= ADIN1300_GE_CLK_CFG_25;
+	} else if (strcmp(val, "125mhz-free-running") == 0) {
+		sel |= ADIN1300_GE_CLK_CFG_FREE_125;
+	} else if (strcmp(val, "adaptive-free-running") == 0) {
+		sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
+	} else {
+		phydev_err(phydev, "invalid adi,phy-output-clock\n");
+		return -EINVAL;
+	}
+
+	if (device_property_read_bool(dev, "adi,phy-output-reference-clock"))
+		sel |= ADIN1300_GE_CLK_CFG_REF_EN;
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_CLK_CFG_REG,
+			      ADIN1300_GE_CLK_CFG_MASK, sel);
+}
+
 static int adin_config_init(struct phy_device *phydev)
 {
 	int rc;
@@ -455,6 +491,10 @@ static int adin_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+	rc = adin_config_clk_out(phydev);
+	if (rc < 0)
+		return rc;
+
 	phydev_dbg(phydev, "PHY is using mode '%s'\n",
 		   phy_modes(phydev->interface));
 
-- 
2.35.3

[PATCH v5 3/3] ARM: dts: imx6qdl-sr-som: update phy configuration for som revision 1.9

[Josua Mayer]

Since SoM revision 1.9 the PHY has been replaced with an ADIN1300,
add an entry for it next to the original.

As Russell King pointed out, additional phy nodes cause warnings like:
mdio_bus 2188000.ethernet-1: MDIO device at address 1 is missing
To avoid this the new node has its status set to disabled. U-Boot will
be modified to enable the appropriate phy node after probing.

The existing ar8035 nodes have to stay enabled by default to avoid
breaking existing systems when they update Linux only.

Co-developed-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
V2 -> V3: new phy node status set disabled
V1 -> V2: changed dts property name

 arch/arm/boot/dts/imx6qdl-sr-som.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
index f86efd0ccc40..ce543e325cd3 100644
--- a/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sr-som.dtsi
@@ -83,6 +83,16 @@ ethernet-phy@4 {
 			qca,clk-out-frequency = <125000000>;
 			qca,smarteee-tw-us-1g = <24>;
 		};
+
+		/*
+		 * ADIN1300 (som rev 1.9 or later) is always at address 1. It
+		 * will be enabled automatically by U-Boot if detected.
+		 */
+		ethernet-phy@1 {
+			reg = <1>;
+			adi,phy-output-clock = "125mhz-free-running";
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.35.3

Re: [PATCH v5 1/3] dt-bindings: net: adin: document phy clock output properties

[Krzysztof Kozlowski]

On 17/05/2022 10:54, Josua Mayer wrote:
> The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
> well as providing the reference clock on CLK25_REF.
> 
> Add DT properties to configure both pins.
> 
> Technically the phy also supports a recovered 125MHz clock for
> synchronous ethernet. However SyncE should be configured dynamically at
> runtime, so it is explicitly omitted in this binding.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

No need to remove my tag, please keep it.


Best regards,
Krzysztof

Re: [PATCH v5 0/3] adin: add support for clock output

[Jakub Kicinski]

On Tue, 17 May 2022 11:51:36 +0300 Josua Mayer wrote:
> This patch series adds support for configuring the two clock outputs of adin
> 1200 and 1300 PHYs. Certain network controllers require an external reference
> clock which can be provided by the PHY.
> 
> One of the replies to v1 was asking why the common clock framework isn't used.
> Currently no PHY driver has implemented providing a clock to the network
> controller. Instead they rely on vendor extensions to make the appropriate
> configuration. For example ar8035 uses qca,clk-out-frequency - this patchset
> aimed to replicate the same functionality.
> 
> Finally the 125MHz free-running clock is enabled in the device-tree for
> SolidRun i.MX6 SoMs, to support revisions 1.9 and later, where the original phy
> has been replaced with an adin 1300.
> To avoid introducing new warning messages during boot for SoMs before rev 1.9,
> the status field of the new phy node is disabled by default, and will be
> enabled by U-Boot on demand.
> 
> Changes since v4:
> - removed recovered clock options

Acked-by: Jakub Kicinski <kuba@kernel.org>

Thanks!

Re: [PATCH v5 0/3] adin: add support for clock output

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 17 May 2022 11:51:36 +0300 you wrote:
> This patch series adds support for configuring the two clock outputs of adin
> 1200 and 1300 PHYs. Certain network controllers require an external reference
> clock which can be provided by the PHY.
> 
> One of the replies to v1 was asking why the common clock framework isn't used.
> Currently no PHY driver has implemented providing a clock to the network
> controller. Instead they rely on vendor extensions to make the appropriate
> configuration. For example ar8035 uses qca,clk-out-frequency - this patchset
> aimed to replicate the same functionality.
> 
> [...]

Here is the summary with links:
  - [v5,1/3] dt-bindings: net: adin: document phy clock output properties
    https://git.kernel.org/netdev/net-next/c/1f77204e11f8
  - [v5,2/3] net: phy: adin: add support for clock output
    https://git.kernel.org/netdev/net-next/c/ce3342161edc
  - [v5,3/3] ARM: dts: imx6qdl-sr-som: update phy configuration for som revision 1.9
    https://git.kernel.org/netdev/net-next/c/654cd22227e6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v5 1/3] dt-bindings: net: adin: document phy clock output properties

[Rob Herring]

On Tue, May 17, 2022 at 3:54 AM Josua Mayer <josua@solid-run.com> wrote:
>
> The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
> well as providing the reference clock on CLK25_REF.
>
> Add DT properties to configure both pins.
>
> Technically the phy also supports a recovered 125MHz clock for
> synchronous ethernet. However SyncE should be configured dynamically at
> runtime, so it is explicitly omitted in this binding.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> V4 -> V5: removed recovered clock options
> V3 -> V4: changed type of adi,phy-output-reference-clock to boolean
> V1 -> V2: changed clkout property to enum
> V1 -> V2: added property for CLK25_REF pin
>
>  .../devicetree/bindings/net/adi,adin.yaml         | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
> index 1129f2b58e98..77750df0c2c4 100644
> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> @@ -36,6 +36,21 @@ properties:
>      enum: [ 4, 8, 12, 16, 20, 24 ]
>      default: 8
>
> +  adi,phy-output-clock:
> +    description: Select clock output on GP_CLK pin. Two clocks are available:

Not valid yaml and now failing in linux-next:

make[1]: *** Deleting file
'Documentation/devicetree/bindings/net/adi,adin.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 52, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/main.py",
line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py",
line 119, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in
_ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in
_ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 891, in
_ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context
  in "<unicode string>", line 40, column 77
make[1]: *** [Documentation/devicetree/bindings/Makefile:26:
Documentation/devicetree/bindings/net/adi,adin.example.dts] Error 1
./Documentation/devicetree/bindings/net/adi,adin.yaml:40:77: [error]
syntax error: mapping values are not allowed here (syntax)
./Documentation/devicetree/bindings/net/adi,adin.yaml:  mapping values
are not allowed in this context
  in "<unicode string>", line 40, column 77


You need a '|' for a literal block if you use a colon in the description.

Rob