linux-kernel - Re: [PATCH net-next v9 3/3] net: dsa: yt921x: Add support for Motorcomm YT921x

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: <aMiWYWKzrFnwyTuz@shell.armlinux.org.uk>
Date: Mon, 15 Sep 2025 23:42:41 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: David Yang <mmyangfl@...il.com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
	Vladimir Oltean <olteanv@...il.com>,
	"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>, Simon Horman <horms@...nel.org>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v9 3/3] net: dsa: yt921x: Add support for
 Motorcomm YT921x

On Sat, Sep 13, 2025 at 12:44:01PM +0800, David Yang wrote:
> +static int
> +yt921x_port_config(struct yt921x_priv *priv, int port, unsigned int mode,
> +		   phy_interface_t interface)
> +{
> +	struct device *dev = to_device(priv);
> +	u32 mask;
> +	u32 ctrl;
> +	int res;
> +
> +	if (!yt921x_port_is_external(port)) {
> +		if (interface != PHY_INTERFACE_MODE_INTERNAL) {
> +			dev_err(dev, "Wrong mode %d on port %d\n",
> +				interface, port);
> +			return -EINVAL;
> +		}
> +		return 0;
> +	}
> +
> +	switch (interface) {
> +	/* SGMII */
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_100BASEX:
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		mask = YT921X_SGMII_CTRL_PORTn(port);
> +		res = yt921x_reg_set_bits(priv, YT921X_SGMII_CTRL, mask);
> +		if (res)
> +			return res;
> +
> +		mask = YT921X_XMII_CTRL_PORTn(port);
> +		res = yt921x_reg_clear_bits(priv, YT921X_XMII_CTRL, mask);
> +		if (res)
> +			return res;
> +
> +		mask = YT921X_SGMII_MODE_M;
> +		switch (interface) {
> +		case PHY_INTERFACE_MODE_SGMII:
> +			ctrl = YT921X_SGMII_MODE_SGMII_PHY;

Does this mean that "SGMII" here means you are sending SGMII
speed/duplex to a host MAC (in other words, immitating a PHY) ?

If yes, then I think we want PHY_INTERFACE_MODE_REVSGMII added.

> +static void
> +yt921x_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
> +			     phy_interface_t interface)
> +{
> +	struct dsa_port *dp = dsa_phylink_to_port(config);
> +	struct dsa_switch *ds = dp->ds;
> +	struct yt921x_priv *priv = to_yt921x_priv(ds);
> +	struct device *dev = to_device(priv);
> +	int port = dp->index;
> +	int res;

Reverse christmas tree ordering please, and do we need "struct device
*dev..." because ds->dev should be this same device. Maybe:

	struct dsa_port *dp = dsa_phylink_to_port(config);
	struct yt921x_priv *priv = to_yt921x_priv(dp->ds);
	int port = dp->index;
	int res;

> +
> +	cancel_delayed_work(&priv->ports[port].mib_read);
> +
> +	mutex_lock(&priv->reg_lock);
> +	res = yt921x_port_down(priv, port);
> +	mutex_unlock(&priv->reg_lock);
> +
> +	if (res)
> +		dev_err(dev, "Failed to %s port %d: %i\n", "bring down",
> +			port, res);

Then:
		dev_err(dp->ds->dev, ...

Same comment for the other phylink methods.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!