linux-kernel - Re: [PATCH net-next v2 2/2] net: phy: dp83822: Add support for GPIO2 clock output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <qqqwdzmcnkuga6qvvszgg7o2myb26sld5i37e4konhln2n4cgc@mwtropwj3ywv>
Date: Wed, 11 Dec 2024 10:45:53 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Dimitri Fedrau <dimitri.fedrau@...bherr.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Andrew Davis <afd@...com>, 
	Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
	Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Dimitri Fedrau <dima.fedrau@...il.com>
Subject: Re: [PATCH net-next v2 2/2] net: phy: dp83822: Add support for GPIO2
 clock output

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@...bherr.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