Re: [PATCH net-next v2 2/8] bng_en: query PHY capabilities and report link status

[Bhargava Chenna Marreddy]

On Fri, Feb 27, 2026 at 2:34 AM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> > +int bnge_hwrm_set_pause(struct bnge_net *bn)
> > +{
> > +     struct hwrm_port_phy_cfg_input *req;
> > +     struct bnge_dev *bd = bn->bd;
> > +     int rc;
> > +
> > +     rc = bnge_hwrm_req_init(bd, req, HWRM_PORT_PHY_CFG);
> > +     if (rc)
> > +             return rc;
> > +
> > +     bnge_hwrm_set_pause_common(bn, req);
> > +
> > +     if ((bn->eth_link_info.autoneg & BNGE_AUTONEG_FLOW_CTRL) ||
> > +         bn->eth_link_info.force_link_chng)
> > +             bnge_hwrm_set_link_common(bn, req);
> > +
> > +     rc = bnge_hwrm_req_send(bd, req);
> > +     if (!rc && !(bn->eth_link_info.autoneg & BNGE_AUTONEG_FLOW_CTRL)) {
> > +             /* Since changing of pause setting doesn't trigger any link
> > +              * change event, the driver needs to update the current pause
> > +              * result upon successful return of the phy_cfg command
> > +              */
>
> This commit is a little bit misleading.
>
> /* Since changing of pause setting, with pause auto negotiation off,
>    doesn't trigger any link change event, the driver needs to update
>    the current MAC pause upon successful return of the phy_cfg command
>    */
>
> The pause auto negotiation off is important here.
>
> The code is not the easiest to read because it mixes up PHY and MAC
> configuration.
>
> 1) If autoneg and autoneg pause is used, you only configure the PHY.
> When autoneg completes, you then configure the MAC with the results of
> the negotiation.
>
> 2) If autoneg is used, but not pause autoneg, you can optionally
> remove pause from being advertised by the PHY, but you need to
> immediately configure the MAC.
>
> 3) If autoneg is off, you you need to immediately configure the MAC.
>
>         Andrew

Thanks for the review, Andrew.

I will update the comment in the next spin and refactor bnge_hwrm_set_pause()
to improve readability and add more comments for better understanding.

Thanks,
Bhargava Marreddy

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature