linux-kernel - Re: [net-next PATCH v2 08/15] net: dsa: qca8k: move fast age/MTU/port enable/disable functions to common code

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: <20220719132239.ubxzxvkzlbfi7xli@skbuf>
Date:   Tue, 19 Jul 2022 16:22:39 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Christian Marangi <ansuelsmth@...il.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>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [net-next PATCH v2 08/15] net: dsa: qca8k: move fast
 age/MTU/port enable/disable functions to common code

On Tue, Jul 19, 2022 at 02:57:19AM +0200, Christian Marangi wrote:
> The same fast age, MTU and port enable/disable function are used by
> driver based on qca8k family switch.
> Move them to common code to make them accessible also by other drivers.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> ---
> +int
> +qca8k_port_enable(struct dsa_switch *ds, int port,
> +		  struct phy_device *phy)
> +{
> +	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;

I think you can make slight tweaks to the code being moved (and document
them in the commit message). For example, in C, there is no type casting
necessary for void pointers (as evidenced by all other places which seem
to do just fine with "priv = ds->priv").

> +
> +	qca8k_port_set_status(priv, port, 1);
> +	priv->port_enabled_map |= BIT(port);
> +
> +	if (dsa_is_user_port(ds, port))
> +		phy_support_asym_pause(phy);
> +
> +	return 0;
> +}
> +
> +void
> +qca8k_port_disable(struct dsa_switch *ds, int port)
> +{
> +	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
> +
> +	qca8k_port_set_status(priv, port, 0);
> +	priv->port_enabled_map &= ~BIT(port);
> +}
> +
> +int
> +qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> +{
> +	struct qca8k_priv *priv = ds->priv;
> +	int ret;
> +
> +	/* We have only have a general MTU setting.
> +	 * DSA always set the CPU port's MTU to the largest MTU of the slave
> +	 * ports.
> +	 * Setting MTU just for the CPU port is sufficient to correctly set a
> +	 * value for every port.
> +	 */
> +	if (!dsa_is_cpu_port(ds, port))
> +		return 0;
> +
> +	/* To change the MAX_FRAME_SIZE the cpu ports must be off or
> +	 * the switch panics.
> +	 * Turn off both cpu ports before applying the new value to prevent
> +	 * this.
> +	 */
> +	if (priv->port_enabled_map & BIT(0))
> +		qca8k_port_set_status(priv, 0, 0);
> +
> +	if (priv->port_enabled_map & BIT(6))
> +		qca8k_port_set_status(priv, 6, 0);
> +
> +	/* Include L2 header / FCS length */
> +	ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, new_mtu + ETH_HLEN + ETH_FCS_LEN);
> +
> +	if (priv->port_enabled_map & BIT(0))
> +		qca8k_port_set_status(priv, 0, 1);
> +
> +	if (priv->port_enabled_map & BIT(6))
> +		qca8k_port_set_status(priv, 6, 1);
> +
> +	return ret;
> +}
> +
> +int
> +qca8k_port_max_mtu(struct dsa_switch *ds, int port)
> +{
> +	return QCA8K_MAX_MTU;
> +}
> diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
> index dd3072e2f23c..bc9078ae2b70 100644
> --- a/drivers/net/dsa/qca/qca8k.h
> +++ b/drivers/net/dsa/qca/qca8k.h
> @@ -469,4 +469,17 @@ int qca8k_port_bridge_join(struct dsa_switch *ds, int port,
>  void qca8k_port_bridge_leave(struct dsa_switch *ds, int port,
>  			     struct dsa_bridge bridge);
>  
> +/* Common fast age function */
> +void qca8k_port_fast_age(struct dsa_switch *ds, int port);
> +int qca8k_set_ageing_time(struct dsa_switch *ds, unsigned int msecs);
> +
> +/* Common port enable/disable function */
> +int qca8k_port_enable(struct dsa_switch *ds, int port,
> +		      struct phy_device *phy);
> +void qca8k_port_disable(struct dsa_switch *ds, int port);
> +
> +/* Common MTU function */
> +int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu);
> +int qca8k_port_max_mtu(struct dsa_switch *ds, int port);
> +
>  #endif /* __QCA8K_H */
> -- 
> 2.36.1
>