netdev - Re: [net-next PATCH 8/8] fbnic: Add phydev representing PMD to phylink setup

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: <CAKgT0UdqH0swVcQFypY8tbDpL58ZDNLpkmQMPNzQep1=eb1hQQ@mail.gmail.com>
Date: Tue, 28 Oct 2025 15:32:10 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, kuba@...nel.org, kernel-team@...a.com, 
	andrew+netdev@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk, 
	pabeni@...hat.com, davem@...emloft.net
Subject: Re: [net-next PATCH 8/8] fbnic: Add phydev representing PMD to
 phylink setup

On Tue, Oct 28, 2025 at 2:02 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > +/**
> > + * fbnic_phylink_connect - Connect phylink structure to IRQ, PHY, and enable it
> > + * @fbn: FBNIC Netdev private data struct phylink device attached to
> > + *
> > + * This function connects the phylink structure to the PHY and IRQ and then
> > + * enables it to resuem operations. With this function completed the PHY will
>
> resume
>
> > + * be able to obtain link and notify the netdev of its current state.
> > + **/
> > +int fbnic_phylink_connect(struct fbnic_net *fbn)
> > +{
> > +     struct fbnic_dev *fbd = fbn->fbd;
> > +     struct phy_device *phydev;
> > +     int err;
> > +
> > +     phydev = phy_find_first(fbd->mii_bus);
>
> phy_find_first() is generally used when you have no idea what address
> the PHY is using. It can cause future surprises when additional
> devices appear on the bus.
>
> In this case, you know what address the device is on the bus, so
> mdiobus_get_phy() would be better.

I'll make the switch then.

> > +     if (!phydev) {
> > +             dev_err(fbd->dev, "No PHY found\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     /* We don't need to poll, the MAC will notify us of events */
> > +     phydev->irq = PHY_MAC_INTERRUPT;
> > +
> > +     phy_attached_info(phydev);
> > +
> > +     err = phylink_connect_phy(fbn->phylink, phydev);
> > +     if (err) {
> > +             dev_err(fbd->dev, "Error connecting phy, err: %d\n", err);
> > +             return err;
> > +     }
> > +
> > +     err = fbnic_mac_request_irq(fbd);
> > +     if (err) {
> > +             phylink_disconnect_phy(fbn->phylink);
> > +             dev_err(fbd->dev, "Error requesting MAC IRQ, err: %d", err);
> > +             return err;
> > +     }
> > +
> > +     phylink_resume(fbn->phylink);
>
> When was is suspended?

We don't use the start/stop calls. Instead we use the resume/suspend
calls in order to deal with the fact that we normally aren't fully
resetting the link. The first call automatically gets converted to a
phylink_start because the bit isn't set for the MAC_WOL, however all
subsequent setups it becomes a resume so that we aren't tearing the
link down fully in order to avoid blocking the BMC which is sharing
the link similar to how a WOL connection would.