From: "Russell King (Oracle)" <linux@armlinux•org.uk>
To: Yanteng Si <siyanteng@loongson•cn>
Cc: Andrew Lunn <andrew@lunn•ch>,
Serge Semin <fancer.lancer@gmail•com>,
si.yanteng@linux•dev, Huacai Chen <chenhuacai@kernel•org>,
hkallweit1@gmail•com, peppe.cavallaro@st•com,
alexandre.torgue@foss•st.com, joabreu@synopsys•com,
Jose.Abreu@synopsys•com, guyinggang@loongson•cn,
netdev@vger•kernel.org, chris.chenfeiyang@gmail•com
Subject: Re: [PATCH net-next v13 12/15] net: stmmac: Fixed failure to set network speed to 1000.
Date: Fri, 5 Jul 2024 12:31:45 +0100 [thread overview]
Message-ID: <ZofZoRzfDNhl1vEP@shell.armlinux.org.uk> (raw)
In-Reply-To: <b8329de3-150a-4f71-bf53-cc52c513a620@loongson.cn>
On Fri, Jul 05, 2024 at 07:17:01PM +0800, Yanteng Si wrote:
> 在 2024/7/4 04:33, Russell King (Oracle) 写道:
> > I think we should "lie" to userspace rather than report how the
> > hardware was actually programmed - again, because that's what would
> > happen with Marvell Alaska.
> >
> > > What about other speeds? Is this limited to 1G? Since we have devices
> > > without auto-neg for 2500BaseX i assume it is not an issue there.
> > 1000base-X can have AN disabled - that's not an issue. Yes, there's
> > the ongoing issues with 2500base-X. 10Gbase-T wording is similar to
> > 1000base-T, so we probably need to do similar there. Likely also the
> > case for 2500base-T and 5000base-T as well.
> >
> > So I'm thinking of something like this (untested):
> >
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 6c6ec9475709..197c4d5ab55b 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -2094,22 +2094,20 @@ EXPORT_SYMBOL(phy_reset_after_clk_enable);
> > /**
> > * genphy_config_advert - sanitize and advertise auto-negotiation parameters
> > * @phydev: target phy_device struct
> > + * @advert: auto-negotiation parameters to advertise
> > *
> > * Description: Writes MII_ADVERTISE with the appropriate values,
> > * after sanitizing the values to make sure we only advertise
> > * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
> > * hasn't changed, and > 0 if it has changed.
> > */
> > -static int genphy_config_advert(struct phy_device *phydev)
> > +static int genphy_config_advert(struct phy_device *phydev,
> > + const unsigned long *advert)
> > {
> > int err, bmsr, changed = 0;
> > u32 adv;
> > - /* Only allow advertising what this PHY supports */
> > - linkmode_and(phydev->advertising, phydev->advertising,
> > - phydev->supported);
> > -
> > - adv = linkmode_adv_to_mii_adv_t(phydev->advertising);
> > + adv = linkmode_adv_to_mii_adv_t(advert);
> > /* Setup standard advertisement */
> > err = phy_modify_changed(phydev, MII_ADVERTISE,
> > @@ -2132,7 +2130,7 @@ static int genphy_config_advert(struct phy_device *phydev)
> > if (!(bmsr & BMSR_ESTATEN))
> > return changed;
> > - adv = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising);
> > + adv = linkmode_adv_to_mii_ctrl1000_t(advert);
> > err = phy_modify_changed(phydev, MII_CTRL1000,
> > ADVERTISE_1000FULL | ADVERTISE_1000HALF,
> > @@ -2356,6 +2354,9 @@ EXPORT_SYMBOL(genphy_check_and_restart_aneg);
> > */
> > int __genphy_config_aneg(struct phy_device *phydev, bool changed)
> > {
> > + __ETHTOOL_DECLARE_LINK_MODE_MASK(fixed_advert);
> > + const struct phy_setting *set;
> > + unsigned long *advert;
> > int err;
> > err = genphy_c45_an_config_eee_aneg(phydev);
> > @@ -2370,10 +2371,25 @@ int __genphy_config_aneg(struct phy_device *phydev, bool changed)
> > else if (err)
> > changed = true;
> > - if (AUTONEG_ENABLE != phydev->autoneg)
> > + if (phydev->autoneg == AUTONEG_ENABLE) {
> > + /* Only allow advertising what this PHY supports */
> > + linkmode_and(phydev->advertising, phydev->advertising,
> > + phydev->supported);
> > + advert = phydev->advertising;
> > + } else if (phydev->speed < SPEED_1000) {
> > return genphy_setup_forced(phydev);
> > + } else {
> > + linkmode_zero(fixed_advert);
> > +
> > + set = phy_lookup_setting(phydev->speed, phydev->duplex,
> > + phydev->supported, true);
> > + if (set)
> > + linkmode_set(set->bit, fixed_advert);
> > +
> > + advert = fixed_advert;
> > + }
> > - err = genphy_config_advert(phydev);
> > + err = genphy_config_advert(phydev, advert);
> > if (err < 0) /* error */
> > return err;
> > else if (err)
>
> It looks great, but I still want to follow Russell's earlier advice and drop
> this patch
>
> from v14, then submit it separately with the above code.
If the above change is made to phylib, then drivers do not need any
changes other than removing such workarounds detecting !AN with
speed = 1G.
The point of the above change is that drivers shouldn't be doing
anything and the issue should be handled entirely within phylib.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2024-07-05 11:32 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-29 10:17 [PATCH net-next v13 00/15] stmmac: Add Loongson platform support Yanteng Si
2024-05-29 10:18 ` [PATCH net-next v13 01/15] net: stmmac: Move the atds flag to the stmmac_dma_cfg structure Yanteng Si
2024-05-29 10:18 ` [PATCH net-next v13 02/15] net: stmmac: Add multi-channel support Yanteng Si
2024-06-14 13:31 ` Serge Semin
2024-06-15 9:18 ` Yanteng Si
2024-05-29 10:18 ` [PATCH net-next v13 03/15] net: stmmac: Export dwmac1000_dma_ops Yanteng Si
2024-05-29 10:19 ` [PATCH net-next v13 04/15] net: stmmac: dwmac-loongson: Drop duplicated hash-based filter size init Yanteng Si
2024-06-14 13:46 ` Serge Semin
2024-05-29 10:19 ` [PATCH net-next v13 05/15] net: stmmac: dwmac-loongson: Use PCI_DEVICE_DATA() macro for device identification Yanteng Si
2024-05-29 10:19 ` [PATCH net-next v13 06/15] net: stmmac: dwmac-loongson: Detach GMAC-specific platform data init Yanteng Si
2024-06-14 16:19 ` Serge Semin
2024-06-17 10:00 ` Yanteng Si
2024-06-24 1:47 ` Serge Semin
2024-06-25 12:31 ` Yanteng Si
2024-07-01 22:57 ` Serge Semin
2024-07-02 9:24 ` Yanteng Si
2024-07-02 8:28 ` Serge Semin
2024-07-02 13:14 ` Yanteng Si
2024-07-02 14:09 ` Serge Semin
2024-07-03 9:41 ` Yanteng Si
2024-07-03 16:19 ` Serge Semin
2024-07-04 8:56 ` Yanteng Si
2024-07-05 10:16 ` Serge Semin
2024-07-05 10:45 ` Yanteng Si
2024-07-05 10:59 ` Serge Semin
2024-07-05 11:29 ` Yanteng Si
2024-07-05 11:53 ` Serge Semin
2024-07-06 13:31 ` Yanteng Si
2024-07-07 10:40 ` Serge Semin
2024-07-08 7:00 ` Yanteng Si
2024-05-29 10:19 ` [PATCH net-next v13 07/15] net: stmmac: dwmac-loongson: Init ref and PTP clocks rate Yanteng Si
2024-05-29 10:19 ` [PATCH net-next v13 08/15] net: stmmac: dwmac-loongson: Add phy_interface for Loongson GMAC Yanteng Si
2024-07-02 8:43 ` Serge Semin
2024-07-04 9:18 ` Yanteng Si
2024-05-29 10:19 ` [PATCH net-next v13 09/15] net: stmmac: dwmac-loongson: Introduce PCI device info data Yanteng Si
2024-07-02 9:18 ` Serge Semin
2024-07-04 9:18 ` Yanteng Si
2024-05-29 10:20 ` [PATCH net-next v13 10/15] net: stmmac: dwmac-loongson: Add DT-less GMAC PCI-device support Yanteng Si
2024-07-02 9:35 ` Serge Semin
2024-07-04 9:17 ` Yanteng Si
2024-05-29 10:20 ` [PATCH net-next v13 11/15] net: stmmac: dwmac-loongson: Add loongson_dwmac_dt_config Yanteng Si
2024-07-02 9:46 ` Serge Semin
2024-07-04 9:15 ` Yanteng Si
2024-05-29 10:20 ` [PATCH net-next v13 12/15] net: stmmac: Fixed failure to set network speed to 1000 Yanteng Si
2024-05-30 2:25 ` Huacai Chen
2024-05-30 7:22 ` Russell King (Oracle)
2024-06-04 11:29 ` si.yanteng
2024-07-02 10:31 ` Serge Semin
2024-07-02 15:08 ` Russell King (Oracle)
2024-07-03 16:56 ` Serge Semin
2024-07-03 18:56 ` Russell King (Oracle)
2024-07-03 19:09 ` Andrew Lunn
2024-07-03 20:33 ` Russell King (Oracle)
2024-07-05 11:17 ` Yanteng Si
2024-07-05 11:31 ` Russell King (Oracle) [this message]
2024-07-05 11:38 ` Yanteng Si
2024-05-29 10:21 ` [PATCH net-next v13 13/15] net: stmmac: dwmac-loongson: Drop pci_enable/disable_msi temporarily Yanteng Si
2024-07-01 1:17 ` Serge Semin
2024-07-04 9:32 ` Yanteng Si
2024-05-29 10:21 ` [PATCH net-next v13 14/15] net: stmmac: dwmac-loongson: Add Loongson GNET support Yanteng Si
2024-05-30 2:46 ` Huacai Chen
2024-06-05 9:43 ` Yanteng Si
2024-06-10 12:12 ` Yanteng Si
2024-07-02 13:43 ` Serge Semin
2024-07-03 1:19 ` Huacai Chen
2024-07-05 10:40 ` Serge Semin
2024-07-05 12:06 ` Yanteng Si
2024-07-05 12:17 ` Serge Semin
2024-07-06 10:30 ` Yanteng Si
2024-07-06 10:36 ` Huacai Chen
2024-07-07 10:51 ` Serge Semin
2024-07-07 13:57 ` Huacai Chen
2024-07-08 7:31 ` Yanteng Si
2024-07-03 10:27 ` Yanteng Si
2024-05-29 10:21 ` [PATCH net-next v13 15/15] net: stmmac: dwmac-loongson: Add loongson module author Yanteng Si
2024-06-05 11:30 ` [PATCH net-next v13 00/15] stmmac: Add Loongson platform support Serge Semin
2024-06-06 12:27 ` Yanteng Si
2024-06-06 18:27 ` Russell King (Oracle)
2024-06-06 18:39 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZofZoRzfDNhl1vEP@shell.armlinux.org.uk \
--to=linux@armlinux$(echo .)org.uk \
--cc=Jose.Abreu@synopsys$(echo .)com \
--cc=alexandre.torgue@foss$(echo .)st.com \
--cc=andrew@lunn$(echo .)ch \
--cc=chenhuacai@kernel$(echo .)org \
--cc=chris.chenfeiyang@gmail$(echo .)com \
--cc=fancer.lancer@gmail$(echo .)com \
--cc=guyinggang@loongson$(echo .)cn \
--cc=hkallweit1@gmail$(echo .)com \
--cc=joabreu@synopsys$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=peppe.cavallaro@st$(echo .)com \
--cc=si.yanteng@linux$(echo .)dev \
--cc=siyanteng@loongson$(echo .)cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox