On 19.08.19 21:51, Heiner Kallweit wrote:
> 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".
>
You are correct, I fixed the tag and annotation.
>> Signed-off-by: Marco Hartmann <marco.hartm...@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.
>
ok.
>> [...]
>
> Overall looks good to me. For a single patch you don't have to provide
> a cover letter.
>
Thank you for your feedback,
I will provide a v2 of the patch with the above fixes.
Regards,
Marco
