diff mbox series

[net-next,v2,2/2] net: phy: dp83822: Add support for GPIO2 clock output

Message ID 20241211-dp83822-gpio2-clk-out-v2-2-614a54f6acab@liebherr.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: phy: dp83822: Add support for GPIO2 clock output | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 82 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-12-12--00-00 (tests: 795)

Commit Message

Dimitri Fedrau via B4 Relay Dec. 11, 2024, 8:04 a.m. UTC 
From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

The GPIO2 pin on the DP83822 can be configured as clock output. Add support
for configuration via DT.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Krzysztof Kozlowski Dec. 11, 2024, 9:45 a.m. UTC | #1 
On Wed, Dec 11, 2024 at 09:04:40AM +0100, Dimitri Fedrau wrote:
> The GPIO2 pin on the DP83822 can be configured as clock output. Add support
> for configuration via DT.
> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index 25ee09c48027c86b7d8f4acb5cbe2e157c56a85a..dc5595eae6cc74e5c77914d53772c5fad64c3e70 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -14,6 +14,8 @@
>  #include <linux/netdevice.h>
>  #include <linux/bitfield.h>
>  
> +#include <dt-bindings/net/ti-dp83822.h>
> +
>  #define DP83822_PHY_ID	        0x2000a240
>  #define DP83825S_PHY_ID		0x2000a140
>  #define DP83825I_PHY_ID		0x2000a150
> @@ -30,6 +32,7 @@
>  #define MII_DP83822_FCSCR	0x14
>  #define MII_DP83822_RCSR	0x17
>  #define MII_DP83822_RESET_CTRL	0x1f
> +#define MII_DP83822_IOCTRL2	0x463
>  #define MII_DP83822_GENCFG	0x465
>  #define MII_DP83822_SOR1	0x467
>  
> @@ -104,6 +107,11 @@
>  #define DP83822_RX_CLK_SHIFT	BIT(12)
>  #define DP83822_TX_CLK_SHIFT	BIT(11)
>  
> +/* IOCTRL2 bits */
> +#define DP83822_IOCTRL2_GPIO2_CLK_SRC		GENMASK(6, 4)
> +#define DP83822_IOCTRL2_GPIO2_CTRL		GENMASK(2, 0)
> +#define DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF	GENMASK(1, 0)
> +
>  /* SOR1 mode */
>  #define DP83822_STRAP_MODE1	0
>  #define DP83822_STRAP_MODE2	BIT(0)
> @@ -139,6 +147,8 @@ struct dp83822_private {
>  	u8 cfg_dac_minus;
>  	u8 cfg_dac_plus;
>  	struct ethtool_wolinfo wol;
> +	bool set_gpio2_clk_out;
> +	u32 gpio2_clk_out;
>  };
>  
>  static int dp83822_config_wol(struct phy_device *phydev,
> @@ -413,6 +423,15 @@ static int dp83822_config_init(struct phy_device *phydev)
>  	int err = 0;
>  	int bmcr;
>  
> +	if (dp83822->set_gpio2_clk_out)
> +		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL2,
> +			       DP83822_IOCTRL2_GPIO2_CTRL |
> +			       DP83822_IOCTRL2_GPIO2_CLK_SRC,
> +			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CTRL,
> +					  DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF) |
> +			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
> +					  dp83822->gpio2_clk_out));

You include the header but you do not use the defines, so it's a proof
these are register values. Register values are not bindings, they do not
bind anything. Bindings are imaginary numbers starting from 0 or 1 which
are used between drivers and DTS, serving as abstraction layer (or
abstraction values) between these two.

You do not have here abstraction. Drop the bindings header entirely.

Best regards,
Krzysztof
Dimitri Fedrau Dec. 11, 2024, 10:52 a.m. UTC | #2 
Am Wed, Dec 11, 2024 at 10:45:53AM +0100 schrieb Krzysztof Kozlowski:
> On Wed, Dec 11, 2024 at 09:04:40AM +0100, Dimitri Fedrau wrote:
> > The GPIO2 pin on the DP83822 can be configured as clock output. Add support
> > for configuration via DT.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  drivers/net/phy/dp83822.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > index 25ee09c48027c86b7d8f4acb5cbe2e157c56a85a..dc5595eae6cc74e5c77914d53772c5fad64c3e70 100644
> > --- a/drivers/net/phy/dp83822.c
> > +++ b/drivers/net/phy/dp83822.c
> > @@ -14,6 +14,8 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/bitfield.h>
> >  
> > +#include <dt-bindings/net/ti-dp83822.h>
> > +
> >  #define DP83822_PHY_ID	        0x2000a240
> >  #define DP83825S_PHY_ID		0x2000a140
> >  #define DP83825I_PHY_ID		0x2000a150
> > @@ -30,6 +32,7 @@
> >  #define MII_DP83822_FCSCR	0x14
> >  #define MII_DP83822_RCSR	0x17
> >  #define MII_DP83822_RESET_CTRL	0x1f
> > +#define MII_DP83822_IOCTRL2	0x463
> >  #define MII_DP83822_GENCFG	0x465
> >  #define MII_DP83822_SOR1	0x467
> >  
> > @@ -104,6 +107,11 @@
> >  #define DP83822_RX_CLK_SHIFT	BIT(12)
> >  #define DP83822_TX_CLK_SHIFT	BIT(11)
> >  
> > +/* IOCTRL2 bits */
> > +#define DP83822_IOCTRL2_GPIO2_CLK_SRC		GENMASK(6, 4)
> > +#define DP83822_IOCTRL2_GPIO2_CTRL		GENMASK(2, 0)
> > +#define DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF	GENMASK(1, 0)
> > +
> >  /* SOR1 mode */
> >  #define DP83822_STRAP_MODE1	0
> >  #define DP83822_STRAP_MODE2	BIT(0)
> > @@ -139,6 +147,8 @@ struct dp83822_private {
> >  	u8 cfg_dac_minus;
> >  	u8 cfg_dac_plus;
> >  	struct ethtool_wolinfo wol;
> > +	bool set_gpio2_clk_out;
> > +	u32 gpio2_clk_out;
> >  };
> >  
> >  static int dp83822_config_wol(struct phy_device *phydev,
> > @@ -413,6 +423,15 @@ static int dp83822_config_init(struct phy_device *phydev)
> >  	int err = 0;
> >  	int bmcr;
> >  
> > +	if (dp83822->set_gpio2_clk_out)
> > +		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL2,
> > +			       DP83822_IOCTRL2_GPIO2_CTRL |
> > +			       DP83822_IOCTRL2_GPIO2_CLK_SRC,
> > +			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CTRL,
> > +					  DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF) |
> > +			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
> > +					  dp83822->gpio2_clk_out));
> 
> You include the header but you do not use the defines, so it's a proof
> these are register values. Register values are not bindings, they do not
> bind anything. Bindings are imaginary numbers starting from 0 or 1 which
> are used between drivers and DTS, serving as abstraction layer (or
> abstraction values) between these two.
> 
> You do not have here abstraction. Drop the bindings header entirely.
>
Ok, thanks for the explanation.

Best regards,
Dimitri Fedrau
diff mbox series

Patch

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 25ee09c48027c86b7d8f4acb5cbe2e157c56a85a..dc5595eae6cc74e5c77914d53772c5fad64c3e70 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -14,6 +14,8 @@ 
 #include <linux/netdevice.h>
 #include <linux/bitfield.h>
 
+#include <dt-bindings/net/ti-dp83822.h>
+
 #define DP83822_PHY_ID	        0x2000a240
 #define DP83825S_PHY_ID		0x2000a140
 #define DP83825I_PHY_ID		0x2000a150
@@ -30,6 +32,7 @@ 
 #define MII_DP83822_FCSCR	0x14
 #define MII_DP83822_RCSR	0x17
 #define MII_DP83822_RESET_CTRL	0x1f
+#define MII_DP83822_IOCTRL2	0x463
 #define MII_DP83822_GENCFG	0x465
 #define MII_DP83822_SOR1	0x467
 
@@ -104,6 +107,11 @@ 
 #define DP83822_RX_CLK_SHIFT	BIT(12)
 #define DP83822_TX_CLK_SHIFT	BIT(11)
 
+/* IOCTRL2 bits */
+#define DP83822_IOCTRL2_GPIO2_CLK_SRC		GENMASK(6, 4)
+#define DP83822_IOCTRL2_GPIO2_CTRL		GENMASK(2, 0)
+#define DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF	GENMASK(1, 0)
+
 /* SOR1 mode */
 #define DP83822_STRAP_MODE1	0
 #define DP83822_STRAP_MODE2	BIT(0)
@@ -139,6 +147,8 @@  struct dp83822_private {
 	u8 cfg_dac_minus;
 	u8 cfg_dac_plus;
 	struct ethtool_wolinfo wol;
+	bool set_gpio2_clk_out;
+	u32 gpio2_clk_out;
 };
 
 static int dp83822_config_wol(struct phy_device *phydev,
@@ -413,6 +423,15 @@  static int dp83822_config_init(struct phy_device *phydev)
 	int err = 0;
 	int bmcr;
 
+	if (dp83822->set_gpio2_clk_out)
+		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_IOCTRL2,
+			       DP83822_IOCTRL2_GPIO2_CTRL |
+			       DP83822_IOCTRL2_GPIO2_CLK_SRC,
+			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CTRL,
+					  DP83822_IOCTRL2_GPIO2_CTRL_CLK_REF) |
+			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
+					  dp83822->gpio2_clk_out));
+
 	if (phy_interface_is_rgmii(phydev)) {
 		rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
 						      true);
@@ -611,6 +630,7 @@  static int dp83822_of_init(struct phy_device *phydev)
 {
 	struct dp83822_private *dp83822 = phydev->priv;
 	struct device *dev = &phydev->mdio.dev;
+	int ret;
 
 	/* Signal detection for the PHY is only enabled if the FX_EN and the
 	 * SD_EN pins are strapped. Signal detection can only enabled if FX_EN
@@ -623,6 +643,26 @@  static int dp83822_of_init(struct phy_device *phydev)
 		dp83822->fx_enabled = device_property_present(dev,
 							      "ti,fiber-mode");
 
+	ret = of_property_read_u32(dev->of_node, "ti,gpio2-clk-out",
+				   &dp83822->gpio2_clk_out);
+	if (!ret) {
+		switch (dp83822->gpio2_clk_out) {
+		case DP83822_CLK_SRC_MAC_IF:
+		case DP83822_CLK_SRC_XI:
+		case DP83822_CLK_SRC_INT_REF:
+		case DP83822_CLK_SRC_RMII_MASTER_MODE_REF:
+		case DP83822_CLK_SRC_FREE_RUNNING:
+		case DP83822_CLK_SRC_RECOVERED:
+			break;
+		default:
+			phydev_err(phydev, "ti,gpio2-clk-out value %u not valid\n",
+				   dp83822->gpio2_clk_out);
+			return -EINVAL;
+		}
+
+		dp83822->set_gpio2_clk_out = true;
+	}
+
 	return 0;
 }