linux-kernel - Re: [PATCH net-next v3 2/6] net: dsa: mt7530: Extend device data ready for adding a new hardware

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: <20200905164702.72edf26c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Sat, 5 Sep 2020 16:47:02 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Landen Chao <landen.chao@...iatek.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Russell King <linux@...linux.org.uk>, <netdev@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <frank-w@...lic-files.de>,
        <opensource@...rst.com>, <dqfext@...il.com>
Subject: Re: [PATCH net-next v3 2/6] net: dsa: mt7530: Extend device data
 ready for adding a new hardware

On Fri, 4 Sep 2020 22:21:57 +0800 Landen Chao wrote:
> +static bool
> +mt7530_phy_mode_supported(struct dsa_switch *ds, int port,
> +			  const struct phylink_link_state *state)
>  {
>  	struct mt7530_priv *priv = ds->priv;
> -	u32 mcr_cur, mcr_new;
>  
>  	switch (port) {
>  	case 0: /* Internal phy */
> @@ -1363,33 +1364,114 @@ static void mt7530_phylink_mac_config(struct dsa_switch *ds, int port,
>  	case 3:
>  	case 4:
>  		if (state->interface != PHY_INTERFACE_MODE_GMII)
> -			return;
> +			goto unsupported;

return false;

Jumping to a label which does nothing but returns makes the code less
readable.

>  		break;
>  	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
> -		if (priv->p5_interface == state->interface)
> -			break;
>  		if (!phy_interface_mode_is_rgmii(state->interface) &&
>  		    state->interface != PHY_INTERFACE_MODE_MII &&
>  		    state->interface != PHY_INTERFACE_MODE_GMII)
> -			return;
> +			goto unsupported;
> +		break;
> +	case 6: /* 1st cpu port */
> +		if (state->interface != PHY_INTERFACE_MODE_RGMII &&
> +		    state->interface != PHY_INTERFACE_MODE_TRGMII)
> +			goto unsupported;
> +		break;
> +	default:
> +		dev_err(priv->dev, "%s: unsupported port: %i\n", __func__,
> +			port);
> +		goto unsupported;
> +	}
> +
> +	return true;
> +
> +unsupported:
> +	return false;
> +}

> +static void
> +mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> +			  const struct phylink_link_state *state)
> +{
> +	struct mt7530_priv *priv = ds->priv;
> +	u32 mcr_cur, mcr_new;
> +
> +	if (!mt753x_phy_mode_supported(ds, port, state))
> +		goto unsupported;
> +
> +	switch (port) {
> +	case 0: /* Internal phy */
> +	case 1:
> +	case 2:
> +	case 3:
> +	case 4:

case 0 ... 4:

> +		if (state->interface != PHY_INTERFACE_MODE_GMII)
> +			goto unsupported;
> +		break;
> +	case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
> +		if (priv->p5_interface == state->interface)
> +			break;

> +static void
> +mt753x_phylink_validate(struct dsa_switch *ds, int port,
> +			unsigned long *supported,
> +			struct phylink_link_state *state)
> +{
> +	struct mt7530_priv *priv = ds->priv;
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };

Please keep the variables longest to shortest (reverse xmas tree).