| Message ID | 1560407871-5642-1-git-send-email-ioana.ciornei@nxp.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phylink: set the autoneg state in phylink_phy_change | expand |
From: Ioana Ciornei <ioana.ciornei@nxp.com> Date: Thu, 13 Jun 2019 09:37:51 +0300 > The phy_state field of phylink should carry only valid information > especially when this can be passed to the .mac_config callback. > Update the an_enabled field with the autoneg state in the > phylink_phy_change function. > > Fixes: 9525ae83959b ("phylink: add phylink infrastructure") > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> Applied and queued up for -stable, thanks.
On Sat, Jun 15, 2019 at 01:30:21PM -0700, David Miller wrote: > From: Ioana Ciornei <ioana.ciornei@nxp.com> > Date: Thu, 13 Jun 2019 09:37:51 +0300 > > > The phy_state field of phylink should carry only valid information > > especially when this can be passed to the .mac_config callback. > > Update the an_enabled field with the autoneg state in the > > phylink_phy_change function. > > > > Fixes: 9525ae83959b ("phylink: add phylink infrastructure") > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> > > Applied and queued up for -stable, thanks. This is not a fix; it is an attempt to make phylink work differently from how it's been designed for the dpaa2 driver. I've already stated that this field is completely meaningless, so I'm surprised you applied it.
From: Russell King - ARM Linux admin <linux@armlinux.org.uk> Date: Sat, 15 Jun 2019 23:13:28 +0100 > On Sat, Jun 15, 2019 at 01:30:21PM -0700, David Miller wrote: >> From: Ioana Ciornei <ioana.ciornei@nxp.com> >> Date: Thu, 13 Jun 2019 09:37:51 +0300 >> >> > The phy_state field of phylink should carry only valid information >> > especially when this can be passed to the .mac_config callback. >> > Update the an_enabled field with the autoneg state in the >> > phylink_phy_change function. >> > >> > Fixes: 9525ae83959b ("phylink: add phylink infrastructure") >> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> >> >> Applied and queued up for -stable, thanks. > > This is not a fix; it is an attempt to make phylink work differently > from how it's been designed for the dpaa2 driver. I've already stated > that this field is completely meaningless, so I'm surprised you > applied it. I'm sorry, I did wait a day or so to see any direct responses to this patch and I saw no feedback. I'll revert.
On Sat, Jun 15, 2019 at 06:08:54PM -0700, David Miller wrote: > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > Date: Sat, 15 Jun 2019 23:13:28 +0100 > > > On Sat, Jun 15, 2019 at 01:30:21PM -0700, David Miller wrote: > >> From: Ioana Ciornei <ioana.ciornei@nxp.com> > >> Date: Thu, 13 Jun 2019 09:37:51 +0300 > >> > >> > The phy_state field of phylink should carry only valid information > >> > especially when this can be passed to the .mac_config callback. > >> > Update the an_enabled field with the autoneg state in the > >> > phylink_phy_change function. > >> > > >> > Fixes: 9525ae83959b ("phylink: add phylink infrastructure") > >> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> > >> > >> Applied and queued up for -stable, thanks. > > > > This is not a fix; it is an attempt to make phylink work differently > > from how it's been designed for the dpaa2 driver. I've already stated > > that this field is completely meaningless, so I'm surprised you > > applied it. > > I'm sorry, I did wait a day or so to see any direct responses to this > patch and I saw no feedback. > > I'll revert. Hi Dave, Thanks for the revert. There was discussion surrounding this patch: https://www.mail-archive.com/netdev@vger.kernel.org/thrd2.html#302220 It was then re-posted as part of a later RFC series ("DPAA2 MAC Driver") which shows why the change was proposed, where the discussion continued on Friday. The patch ended up with a slightly different subject line. There is still further discussion required to try and work out a way forward. Thanks.
> Subject: Re: [PATCH] net: phylink: set the autoneg state in phylink_phy_change > > On Sat, Jun 15, 2019 at 06:08:54PM -0700, David Miller wrote: > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk> > > Date: Sat, 15 Jun 2019 23:13:28 +0100 > > > > > On Sat, Jun 15, 2019 at 01:30:21PM -0700, David Miller wrote: > > >> From: Ioana Ciornei <ioana.ciornei@nxp.com> > > >> Date: Thu, 13 Jun 2019 09:37:51 +0300 > > >> > > >> > The phy_state field of phylink should carry only valid > > >> > information especially when this can be passed to the .mac_config > callback. > > >> > Update the an_enabled field with the autoneg state in the > > >> > phylink_phy_change function. > > >> > > > >> > Fixes: 9525ae83959b ("phylink: add phylink infrastructure") > > >> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> > > >> > > >> Applied and queued up for -stable, thanks. > > > > > > This is not a fix; it is an attempt to make phylink work differently > > > from how it's been designed for the dpaa2 driver. I've already > > > stated that this field is completely meaningless, so I'm surprised > > > you applied it. > > > > I'm sorry, I did wait a day or so to see any direct responses to this > > patch and I saw no feedback. > > > > I'll revert. > > Hi Dave, > > Thanks for the revert. There was discussion surrounding this patch: > > https://www.mail-archive.com/netdev@vger.kernel.org/thrd2.html#302220 > > It was then re-posted as part of a later RFC series ("DPAA2 MAC > Driver") which shows why the change was proposed, where the discussion > continued on Friday. The patch ended up with a slightly different subject line. > > There is still further discussion required to try and work out a way forward. > > Thanks. > Hi all, Sorry for not commenting on this until now but last weekend I had an unfortunate accident and ended up with a broken ankle. I'll start responding to all the emails and get back into this. -- Ioana
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 5d0af041b8f9..dd1feb7b5472 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -688,6 +688,7 @@ static void phylink_phy_change(struct phy_device *phydev, bool up, pl->phy_state.pause |= MLO_PAUSE_ASYM; pl->phy_state.interface = phydev->interface; pl->phy_state.link = up; + pl->phy_state.an_enabled = phydev->autoneg; mutex_unlock(&pl->state_mutex); phylink_run_resolve(pl);
The phy_state field of phylink should carry only valid information especially when this can be passed to the .mac_config callback. Update the an_enabled field with the autoneg state in the phylink_phy_change function. Fixes: 9525ae83959b ("phylink: add phylink infrastructure") Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> --- drivers/net/phy/phylink.c | 1 + 1 file changed, 1 insertion(+)