netdev - Re: [PATCH 3/3] ag71xx: Run ag71xx_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b120575e-b129-63c5-962e-cb3a90d4c62b@hauke-m.de>
Date:   Sun, 1 Mar 2020 13:19:56 +0100
From:   Hauke Mehrtens <hauke@...ke-m.de>
To:     Oleksij Rempel <linux@...pel-privat.de>,
        Heiner Kallweit <hkallweit1@...il.com>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, jcliburn@...il.com, chris.snook@...il.com
Subject: Re: [PATCH 3/3] ag71xx: Run ag71xx_link_adjust() only when needed

On 2/18/20 11:30 AM, Oleksij Rempel wrote:
> Hi,
> 
> current kernel still missing following patch:
> https://github.com/olerem/linux-2.6/commit/9a9a531bbad6d00c6221f393fd85628475e1f121
> 
> @Hauke, can you please test, rebase your changes (if needed) on top of this patch?

I rebased my changes on top of this:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=892e09153fa3564fcbf9f422760b61eba48c123e
and my patch is not needed any more, I will send a V2 only containing
the first patch of this series, which should fix a potential bug.

You missed adding RGMII support in your patch, but the MAC supports
RGMII, I will also send a patch for that.

@Oleksij: Are you planning to add support for the GMAC configuration
like RGMII delays and so on?

Hauke


> 
> Am 18.02.20 um 08:02 schrieb Heiner Kallweit:
>> On 18.02.2020 00:35, Hauke Mehrtens wrote:
>>> My system printed this line every second:
>>>   ag71xx 19000000.eth eth0: Link is Up - 1Gbps/Full - flow control off
>>> The function ag71xx_phy_link_adjust() was called by the PHY layer every
>>> second even when nothing changed.
>>>
>> With current phylib code this shouldn't happen, see phy_check_link_status().
>> On which kernel version do you observe this behavior?
>>
>>> With this patch the old status is stored and the real the
>>> ag71xx_link_adjust() function is only called when something really
>>> changed. This way the update and also this print is only done once any
>>> more.
>>>
>>> Signed-off-by: Hauke Mehrtens <hauke@...ke-m.de>
>>> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
>>> ---
>>>  drivers/net/ethernet/atheros/ag71xx.c | 24 +++++++++++++++++++++++-
>>>  1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
>>> index 7d3fec009030..12eaf6d2518d 100644
>>> --- a/drivers/net/ethernet/atheros/ag71xx.c
>>> +++ b/drivers/net/ethernet/atheros/ag71xx.c
>>> @@ -307,6 +307,10 @@ struct ag71xx {
>>>  	u32 msg_enable;
>>>  	const struct ag71xx_dcfg *dcfg;
>>>
>>> +	unsigned int		link;
>>> +	unsigned int		speed;
>>> +	int			duplex;
>>> +
>>>  	/* From this point onwards we're not looking at per-packet fields. */
>>>  	void __iomem *mac_base;
>>>
>>> @@ -854,6 +858,7 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
>>>
>>>  	if (!phydev->link && update) {
>>>  		ag71xx_hw_stop(ag);
>>> +		phy_print_status(phydev);
>>>  		return;
>>>  	}
>>>
>>> @@ -907,8 +912,25 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
>>>  static void ag71xx_phy_link_adjust(struct net_device *ndev)
>>>  {
>>>  	struct ag71xx *ag = netdev_priv(ndev);
>>> +	struct phy_device *phydev = ndev->phydev;
>>> +	int status_change = 0;
>>> +
>>> +	if (phydev->link) {
>>> +		if (ag->duplex != phydev->duplex ||
>>> +		    ag->speed != phydev->speed) {
>>> +			status_change = 1;
>>> +		}
>>> +	}
>>> +
>>> +	if (phydev->link != ag->link)
>>> +		status_change = 1;
>>> +
>>> +	ag->link = phydev->link;
>>> +	ag->duplex = phydev->duplex;
>>> +	ag->speed = phydev->speed;
>>>
>>> -	ag71xx_link_adjust(ag, true);
>>> +	if (status_change)
>>> +		ag71xx_link_adjust(ag, true);
>>>  }
>>>
>>>  static int ag71xx_phy_connect(struct ag71xx *ag)
>>>
>>
> 
> 
> --
> Regards,
> Oleksij
> 




Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)