netdev - Re: [PATCH net-next v3 5/7] net: dsa: Add helpers to convert netdev to ds or port index

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]

Date: Tue, 2 Apr 2024 23:51:21 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, 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>,
	Gregory Clement <gregory.clement@...tlin.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 5/7] net: dsa: Add helpers to convert netdev
 to ds or port index

On Tue, Apr 02, 2024 at 02:31:34PM +0100, Russell King (Oracle) wrote:
> Yes, I would tend to agree having done this for my experimental phylink
> changes. struct dsa_port seems to make the most sense:
> 
> static inline struct dsa_port *
> dsa_phylink_to_port(struct phylink_config *config)
> {
>         return container_of(config, struct dsa_port, pl_config);
> }
> 
> which then means e.g.:
> 
> static void mv88e6xxx_mac_config(struct phylink_config *config,
>                                  unsigned int mode,
>                                  const struct phylink_link_state *state)
> {
>         struct dsa_port *dp = dsa_phylink_to_port(config);
>         struct mv88e6xxx_chip *chip = dp->ds->priv;
>         int port = dp->index;
>         int err = 0;
> 
>         mv88e6xxx_reg_lock(chip);
> 
>         if (mode != MLO_AN_PHY || !mv88e6xxx_phy_is_internal(chip, port)) {
>                 err = mv88e6xxx_port_config_interface(chip, port,
>                                                       state->interface);
>                 if (err && err != -EOPNOTSUPP)
>                         goto err_unlock;
>         }
> 
> err_unlock:
>         mv88e6xxx_reg_unlock(chip);
> 
>         if (err && err != -EOPNOTSUPP)
>                 dev_err(dp->ds->dev, "p%d: failed to configure MAC/PCS\n",
>                         port);
> }

Looks ok, looking forward to seeing more of it. Maybe a slight
preference to keeping "struct dsa_switch *ds" as a local variable of its
own, even if this means declaring and initializing "chip" on separate
lines.