Messages in this thread | | | Subject | Re: [PATCH net-next 1/1] Add genphy_c45_config_aneg() function to phy-c45.c | From | Heiner Kallweit <> | Date | Mon, 19 Aug 2019 21:51:40 +0200 |
| |
On 19.08.2019 19:52, Marco Hartmann wrote: > and call it from phy_config_aneg(). > Here something went wrong.
> commit 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from > calling genphy_config_aneg") introduced a check that aborts > phy_config_aneg() if the phy is a C45 phy. > This causes phy_state_machine() to call phy_error() so that the phy > ends up in PHY_HALTED state. > > Instead of returning -EOPNOTSUPP, call genphy_c45_config_aneg() > (analogous to the C22 case) so that the state machine can run > correctly. > > genphy_c45_config_aneg() closely resembles mv3310_config_aneg() > in drivers/net/phy/marvell10g.c, excluding vendor specific > configurations for 1000BaseT. > > Fixes: 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from > calling genphy_config_aneg") > This tag seems to be the wrong one. This change was done before genphy_c45_driver was added. Most likely tag should be: 22b56e827093 ("net: phy: replace genphy_10g_driver with genphy_c45_driver") And because it's a fix applying to previous kernel versions it should be annotated "net", not "net-next".
> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com> > --- > drivers/net/phy/phy-c45.c | 26 ++++++++++++++++++++++++++ > drivers/net/phy/phy.c | 2 +- > include/linux/phy.h | 1 + > 3 files changed, 28 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index b9d4145781ca..fa9062fd9122 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -509,6 +509,32 @@ int genphy_c45_read_status(struct phy_device *phydev) > } > EXPORT_SYMBOL_GPL(genphy_c45_read_status); > > +/** > + * genphy_c45_config_aneg - restart auto-negotiation or forced setup > + * @phydev: target phy_device struct > + * > + * Description: If auto-negotiation is enabled, we configure the > + * advertising, and then restart auto-negotiation. If it is not > + * enabled, then we force a configuration. > + */ > +int genphy_c45_config_aneg(struct phy_device *phydev) > +{ > + int ret; > + bool changed = false;
Reverse xmas tree please.
> [...]
Overall looks good to me. For a single patch you don't have to provide a cover letter.
| |