[PATCH 1/2] dt-bindings: net: adin: document adi,clk_rcvr_125_en property

[PATCH 1/2] dt-bindings: net: adin: document adi,clk_rcvr_125_en property

[Nate Drude]

Document device tree property to set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG),
causing the 125 MHz PHY recovered clock (or PLL clock) to be driven at
the GP_CLK pin.

Signed-off-by: Nate Drude <nate.d@variscite.com>
---
 Documentation/devicetree/bindings/net/adi,adin.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
index 1129f2b58e98..5fdbbd5aff82 100644
--- a/Documentation/devicetree/bindings/net/adi,adin.yaml
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -36,6 +36,11 @@ properties:
     enum: [ 4, 8, 12, 16, 20, 24 ]
     default: 8
 
+  adi,clk_rcvr_125_en:
+    description: |
+      Set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG), causing the 125 MHz
+      PHY recovered clock (or PLL clock) to be driven at the GP_CLK pin.
+
 unevaluatedProperties: false
 
 examples:
-- 
2.25.1

[PATCH 2/2] net: phy: adin: add adi,clk_rcvr_125_en property

[Nate Drude]

Add device tree property to set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG),
causing the 125 MHz PHY recovered clock (or PLL clock) to be driven at
the GP_CLK pin.

Signed-off-by: Nate Drude <nate.d@variscite.com>
---
 drivers/net/phy/adin.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 5ce6da62cc8e..600472341cef 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -14,6 +14,7 @@
 #include <linux/mii.h>
 #include <linux/phy.h>
 #include <linux/property.h>
+#include <linux/of.h>
 
 #define PHY_ID_ADIN1200				0x0283bc20
 #define PHY_ID_ADIN1300				0x0283bc30
@@ -99,6 +100,9 @@
 #define ADIN1300_GE_SOFT_RESET_REG		0xff0c
 #define   ADIN1300_GE_SOFT_RESET		BIT(0)
 
+#define ADIN1300_GE_CLK_CFG			0xff1f
+#define   ADIN1300_GE_CLK_RCVR_125_EN		BIT(5)
+
 #define ADIN1300_GE_RGMII_CFG_REG		0xff23
 #define   ADIN1300_GE_RGMII_RX_MSK		GENMASK(8, 6)
 #define   ADIN1300_GE_RGMII_RX_SEL(x)		\
@@ -407,6 +411,27 @@ static int adin_set_edpd(struct phy_device *phydev, u16 tx_interval)
 			  val);
 }
 
+static int adin_set_clock_config(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+	int reg = 0;
+
+	if (of_property_read_bool(of_node, "adi,clk_rcvr_125_en")) {
+		reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_CLK_CFG);
+
+		reg |= ADIN1300_GE_CLK_RCVR_125_EN;
+
+		phydev_dbg(phydev, "%s: ADIN1300_GE_CLK_CFG = %x\n",
+		           __func__, reg);
+
+		reg = phy_write_mmd(phydev, MDIO_MMD_VEND1,
+			     ADIN1300_GE_CLK_CFG, reg);
+	}
+
+	return reg;
+}
+
 static int adin_get_tunable(struct phy_device *phydev,
 			    struct ethtool_tunable *tuna, void *data)
 {
@@ -455,6 +480,10 @@ static int adin_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+	rc = adin_set_clock_config(phydev);
+	if (rc < 0)
+		return rc;
+
 	phydev_dbg(phydev, "PHY is using mode '%s'\n",
 		   phy_modes(phydev->interface));
 
-- 
2.25.1

Re: [PATCH 2/2] net: phy: adin: add adi,clk_rcvr_125_en property

[Jakub Kicinski]

On Fri, 29 Apr 2022 13:44:32 -0500 Nate Drude wrote:
> +	if (of_property_read_bool(of_node, "adi,clk_rcvr_125_en")) {
> +		reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_CLK_CFG);
> +
> +		reg |= ADIN1300_GE_CLK_RCVR_125_EN;
> +
> +		phydev_dbg(phydev, "%s: ADIN1300_GE_CLK_CFG = %x\n",
> +		           __func__, reg);
> +
> +		reg = phy_write_mmd(phydev, MDIO_MMD_VEND1,
> +			     ADIN1300_GE_CLK_CFG, reg);

Looks like you could use phy_modify_mmd() instead.

The print could be removed IMHO, it adds little value. All it does is
says the register write was performed, not even old an new values.

Please CC the folks and list pointed out by Paolo and Josua from the
thread pointed out by Andrew.

Please tag v2 as [PATCH net-next v2] in the subject.

Thanks!

Re: [PATCH 1/2] dt-bindings: net: adin: document adi,clk_rcvr_125_en property

[Andrew Lunn]

On Fri, Apr 29, 2022 at 01:44:31PM -0500, Nate Drude wrote:
> Document device tree property to set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG),
> causing the 125 MHz PHY recovered clock (or PLL clock) to be driven at
> the GP_CLK pin.

Hi Nate

Have you seen:

https://lore.kernel.org/netdev/20220419102709.26432-1-josua@solid-run.com/

	Andrew

Re: [PATCH 1/2] dt-bindings: net: adin: document adi,clk_rcvr_125_en property

[Paolo Abeni]

Hello,

On Fri, 2022-04-29 at 13:44 -0500, Nate Drude wrote:
> Document device tree property to set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG),
> causing the 125 MHz PHY recovered clock (or PLL clock) to be driven at
> the GP_CLK pin.
> 
> Signed-off-by: Nate Drude <nate.d@variscite.com>
> ---
>  Documentation/devicetree/bindings/net/adi,adin.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
> index 1129f2b58e98..5fdbbd5aff82 100644
> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> @@ -36,6 +36,11 @@ properties:
>      enum: [ 4, 8, 12, 16, 20, 24 ]
>      default: 8
>  
> +  adi,clk_rcvr_125_en:
> +    description: |
> +      Set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG), causing the 125 MHz
> +      PHY recovered clock (or PLL clock) to be driven at the GP_CLK pin.
> +
>  unevaluatedProperties: false
>  
>  examples:

The recipients list does not contain a few required ones, adding for
awareness Rob, Krzysztof and the devicetree ML. If a new version should
be required, please include them.

Thanks!

Paolo

Re: [PATCH 1/2] dt-bindings: net: adin: document adi,clk_rcvr_125_en property

[Krzysztof Kozlowski]

On 02/05/2022 11:03, Paolo Abeni wrote:
> Hello,
> 
> On Fri, 2022-04-29 at 13:44 -0500, Nate Drude wrote:
>> Document device tree property to set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG),
>> causing the 125 MHz PHY recovered clock (or PLL clock) to be driven at
>> the GP_CLK pin.
>>
>> Signed-off-by: Nate Drude <nate.d@variscite.com>
>> ---
>>  Documentation/devicetree/bindings/net/adi,adin.yaml | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
>> index 1129f2b58e98..5fdbbd5aff82 100644
>> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
>> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
>> @@ -36,6 +36,11 @@ properties:
>>      enum: [ 4, 8, 12, 16, 20, 24 ]
>>      default: 8
>>  
>> +  adi,clk_rcvr_125_en:

No underscores in node names

>> +    description: |
>> +      Set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG), causing the 125 MHz
>> +      PHY recovered clock (or PLL clock) to be driven at the GP_CLK pin.

You are describing programming model but you should describe rather
hardware feature instead. This should be reflected in property name and
description. Focus on hardware and describe it.

>> +
>>  unevaluatedProperties: false
>>  
>>  examples:
> 
> The recipients list does not contain a few required ones, adding for
> awareness Rob, Krzysztof and the devicetree ML. If a new version should
> be required, please include them.

Thanks!

Nate,
Just please use scripts/get_maintainers.pl and all problems with
addressing are gone...


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: net: adin: document adi,clk_rcvr_125_en property

[Nate Drude]

On Tue, 2022-05-03 at 13:50 +0200, Krzysztof Kozlowski wrote:
> On 02/05/2022 11:03, Paolo Abeni wrote:
> > Hello,
> > 
> > On Fri, 2022-04-29 at 13:44 -0500, Nate Drude wrote:
> > > Document device tree property to set GE_CLK_RCVR_125_EN (bit 5 of
> > > GE_CLK_CFG),
> > > causing the 125 MHz PHY recovered clock (or PLL clock) to be
> > > driven at
> > > the GP_CLK pin.
> > > 
> > > Signed-off-by: Nate Drude <nate.d@variscite.com>
> > > ---
> > >  Documentation/devicetree/bindings/net/adi,adin.yaml | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml
> > > b/Documentation/devicetree/bindings/net/adi,adin.yaml
> > > index 1129f2b58e98..5fdbbd5aff82 100644
> > > --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
> > > +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> > > @@ -36,6 +36,11 @@ properties:
> > >      enum: [ 4, 8, 12, 16, 20, 24 ]
> > >      default: 8
> > >  
> > > +  adi,clk_rcvr_125_en:
> 
> No underscores in node names
> 
> > > +    description: |
> > > +      Set GE_CLK_RCVR_125_EN (bit 5 of GE_CLK_CFG), causing the
> > > 125 MHz
> > > +      PHY recovered clock (or PLL clock) to be driven at the
> > > GP_CLK pin.
> 
> You are describing programming model but you should describe rather
> hardware feature instead. This should be reflected in property name
> and
> description. Focus on hardware and describe it.
> 
> > > +
> > >  unevaluatedProperties: false
> > >  
> > >  examples:
> > 
> > The recipients list does not contain a few required ones, adding
> > for
> > awareness Rob, Krzysztof and the devicetree ML. If a new version
> > should
> > be required, please include them.
> 
> Thanks!
> 
> Nate,
> Just please use scripts/get_maintainers.pl and all problems with
> addressing are gone...
> 
> 
> Best regards,
> Krzysztof

Hi All,

Thanks for your feedback on the patch and proper addressing.

This patch is now duplicated by v3 of Josua's patch, which appears more
comprehensive:
https://lore.kernel.org/netdev/20220428082848.12191-3-josua@solid-run.com/

Regards,
Nate