2024-01-22 11:12:21

by Fabian Pfitzner

[permalink] [raw]
Subject: [PATCH] net: phy: adin: add missing clock option

The GP_CLK pin on Adin1300 PHY's offers three different output clocks.
This patch adds the missing 125MHz recovered clock option which is not
yet availible in the driver.

Signed-off-by: Fabian Pfitzner <[email protected]>
---
Documentation/devicetree/bindings/net/adi,adin.yaml | 7 +++++--
drivers/net/phy/adin.c | 2 ++
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
index 929cf8c0b0fd..cd1b4efa692b 100644
--- a/Documentation/devicetree/bindings/net/adi,adin.yaml
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -38,14 +38,17 @@ properties:

adi,phy-output-clock:
description: |
- Select clock output on GP_CLK pin. Two clocks are available:
- A 25MHz reference and a free-running 125MHz.
+ Select clock output on GP_CLK pin. Three clocks are available:
+ - 25MHz reference
+ - free-running 125MHz
+ - recovered 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
+ - 125mhz-recovered
- adaptive-free-running

adi,phy-output-reference-clock:
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 2e1a46e121d9..b1ed6fd24763 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -508,6 +508,8 @@ static int adin_config_clk_out(struct phy_device *phydev)
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, "125mhz-recovered") == 0) {
+ sel |= ADIN1300_GE_CLK_CFG_RCVR_125;
} else if (strcmp(val, "adaptive-free-running") == 0) {
sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
} else {

base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
--
2.39.2



2024-01-22 18:14:22

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] net: phy: adin: add missing clock option

On Mon, Jan 22, 2024 at 12:03:12PM +0100, Fabian Pfitzner wrote:
> The GP_CLK pin on Adin1300 PHY's offers three different output clocks.
> This patch adds the missing 125MHz recovered clock option which is not
> yet availible in the driver.
>
> Signed-off-by: Fabian Pfitzner <[email protected]>
> ---
> Documentation/devicetree/bindings/net/adi,adin.yaml | 7 +++++--

Binding patches should be split out from drivers please.

Thanks,
Conor.

> drivers/net/phy/adin.c | 2 ++
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
> index 929cf8c0b0fd..cd1b4efa692b 100644
> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> @@ -38,14 +38,17 @@ properties:
>
> adi,phy-output-clock:
> description: |
> - Select clock output on GP_CLK pin. Two clocks are available:
> - A 25MHz reference and a free-running 125MHz.
> + Select clock output on GP_CLK pin. Three clocks are available:
> + - 25MHz reference
> + - free-running 125MHz
> + - recovered 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
> + - 125mhz-recovered
> - adaptive-free-running
>
> adi,phy-output-reference-clock:
> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> index 2e1a46e121d9..b1ed6fd24763 100644
> --- a/drivers/net/phy/adin.c
> +++ b/drivers/net/phy/adin.c
> @@ -508,6 +508,8 @@ static int adin_config_clk_out(struct phy_device *phydev)
> 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, "125mhz-recovered") == 0) {
> + sel |= ADIN1300_GE_CLK_CFG_RCVR_125;
> } else if (strcmp(val, "adaptive-free-running") == 0) {
> sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
> } else {
>
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> --
> 2.39.2
>


Attachments:
(No filename) (2.27 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-23 09:21:01

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH] net: phy: adin: add missing clock option

On Mon, Jan 22, 2024 at 12:03:12PM +0100, Fabian Pfitzner wrote:
> The GP_CLK pin on Adin1300 PHY's offers three different output clocks.
> This patch adds the missing 125MHz recovered clock option which is not
> yet availible in the driver.

nit: available

..

2024-01-23 09:57:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] net: phy: adin: add missing clock option

On 22/01/2024 12:03, Fabian Pfitzner wrote:
> The GP_CLK pin on Adin1300 PHY's offers three different output clocks.
> This patch adds the missing 125MHz recovered clock option which is not
> yet availible in the driver.
>

Typo

> Signed-off-by: Fabian Pfitzner <[email protected]>
> ---
> Documentation/devicetree/bindings/net/adi,adin.yaml | 7 +++++--

Bindings are separate.

Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.



Best regards,
Krzysztof


2024-01-24 10:27:43

by Fabian Pfitzner

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: net: adin: add recovered clock output

The ADIN1300 offers three distinct output clocks which can be accessed
through the GP_CLK pin. The DT only offers two of the possible options
and thus the 125MHz-recovered output clock is missing.

As there is no other way to configure this pin than through the DT it
should be possible to do so for all available outputs.

Signed-off-by: Fabian Pfitzner <[email protected]>
---
Documentation/devicetree/bindings/net/adi,adin.yaml | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
index 929cf8c0b0fd..04059393b756 100644
--- a/Documentation/devicetree/bindings/net/adi,adin.yaml
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -38,14 +38,17 @@ properties:

adi,phy-output-clock:
description: |
- Select clock output on GP_CLK pin. Two clocks are available:
- A 25MHz reference and a free-running 125MHz.
+ Select clock output on GP_CLK pin. Three clocks are available:
+ - 25MHz reference
+ - free-running 125MHz
+ - recovered 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
+ - 125mhz-recovered
- adaptive-free-running

adi,phy-output-reference-clock:

base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
--
2.39.2


2024-01-24 10:28:01

by Fabian Pfitzner

[permalink] [raw]
Subject: [PATCH v2 2/2] net: phy: adin: add recovered clock output

The ADIN1300 offers three distinct output clocks which can be accessed
through the GP_CLK pin. The DT only offers two of the possible options
and thus the 125MHz-recovered output clock is missing.

As there is no other way to configure this pin than through the DT it
should be possible to do so for all available outputs.

Signed-off-by: Fabian Pfitzner <[email protected]>
---
drivers/net/phy/adin.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 2e1a46e121d9..b1ed6fd24763 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -508,6 +508,8 @@ static int adin_config_clk_out(struct phy_device *phydev)
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, "125mhz-recovered") == 0) {
+ sel |= ADIN1300_GE_CLK_CFG_RCVR_125;
} else if (strcmp(val, "adaptive-free-running") == 0) {
sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
} else {
--
2.39.2


2024-01-24 11:29:04

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] net: phy: adin: add recovered clock output

On Wed, 2024-01-24 at 11:25 +0100, Fabian Pfitzner wrote:
> The ADIN1300 offers three distinct output clocks which can be accessed
> through the GP_CLK pin. The DT only offers two of the possible options
> and thus the 125MHz-recovered output clock is missing.
>
> As there is no other way to configure this pin than through the DT it
> should be possible to do so for all available outputs.
>
> Signed-off-by: Fabian Pfitzner <[email protected]>
> ---

Reviewed-by: Nuno Sa <[email protected]>

>  drivers/net/phy/adin.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> index 2e1a46e121d9..b1ed6fd24763 100644
> --- a/drivers/net/phy/adin.c
> +++ b/drivers/net/phy/adin.c
> @@ -508,6 +508,8 @@ static int adin_config_clk_out(struct phy_device *phydev)
>   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, "125mhz-recovered") == 0) {
> + sel |= ADIN1300_GE_CLK_CFG_RCVR_125;
>   } else if (strcmp(val, "adaptive-free-running") == 0) {
>   sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
>   } else {


2024-01-24 16:42:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: net: adin: add recovered clock output

On Wed, Jan 24, 2024 at 11:25:54AM +0100, Fabian Pfitzner wrote:
> The ADIN1300 offers three distinct output clocks which can be accessed
> through the GP_CLK pin. The DT only offers two of the possible options
> and thus the 125MHz-recovered output clock is missing.
>
> As there is no other way to configure this pin than through the DT it
> should be possible to do so for all available outputs.
>
> Signed-off-by: Fabian Pfitzner <[email protected]>

Acked-by: Conor Dooley <[email protected]>

Cheers,
Conor.

> ---
> Documentation/devicetree/bindings/net/adi,adin.yaml | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
> index 929cf8c0b0fd..04059393b756 100644
> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> @@ -38,14 +38,17 @@ properties:
>
> adi,phy-output-clock:
> description: |
> - Select clock output on GP_CLK pin. Two clocks are available:
> - A 25MHz reference and a free-running 125MHz.
> + Select clock output on GP_CLK pin. Three clocks are available:
> + - 25MHz reference
> + - free-running 125MHz
> + - recovered 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
> + - 125mhz-recovered
> - adaptive-free-running
>
> adi,phy-output-reference-clock:
>
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> --
> 2.39.2
>


Attachments:
(No filename) (1.77 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-25 01:59:18

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: net: adin: add recovered clock output

On Wed, 24 Jan 2024 11:25:54 +0100 Fabian Pfitzner wrote:
> The ADIN1300 offers three distinct output clocks which can be accessed
> through the GP_CLK pin. The DT only offers two of the possible options
> and thus the 125MHz-recovered output clock is missing.
>
> As there is no other way to configure this pin than through the DT it
> should be possible to do so for all available outputs.

Hi Fabian!

If you want to use PHY-recovered clock you should really use the DPLL
subsystem. It will also allow you to configure other PHYs taking this
signal as an input, to forward the clock phase. Read lock state. Etc.

Even if the patches are good (which I'm not saying they are yet ;)) -
you'll have to repost this as a new thread, unfortunately. I'm not sure
why by the way this was posted made patchwork think that the patches
are separate series:

https://patchwork.kernel.org/project/netdevbpf/list/?series=819440
https://patchwork.kernel.org/project/netdevbpf/list/?series=818548

each of which is incomplete, since it only has one patch but subject
says "1/2" and "2/2".