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] [day] [month] [year] [list]

Message-ID: <c05002cf-3aeb-96b1-7da0-bf2c35b9c6c8@rempel-privat.de>
Date:   Sun, 1 Mar 2020 14:24:16 +0100
From:   Oleksij Rempel <linux@...pel-privat.de>
To:     Hauke Mehrtens <hauke@...ke-m.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

Am 01.03.20 um 13:19 schrieb Hauke Mehrtens:
> 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.

Thank you!

> 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?

Not now. Currently I work on AR9331 which has no external RGMII
interface. If you have HW capable to do RGMII, patches are welcome :)

> 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
>>
>
>


--
Regards,
Oleksij