netdev - Re: [RFC PATCH 1/2] gianfar: convert to phylink

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: <87muh33mvi.fsf@kb.kras.ru>
Date:   Wed, 24 Jul 2019 14:36:17 +0700
From:   Arseny Solokha <asolokha@...kras.ru>
To:     Claudiu Manoil <claudiu.manoil@....com>
Cc:     Ioana Ciornei <ioana.ciornei@....com>,
        Russell King <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH 1/2] gianfar: convert to phylink

>>-----Original Message-----
>>From: Arseny Solokha <asolokha@...kras.ru>
>>Sent: Tuesday, July 23, 2019 6:17 PM
>>To: Claudiu Manoil <claudiu.manoil@....com>; Ioana Ciornei
>><ioana.ciornei@....com>; Russell King <linux@...linux.org.uk>; Andrew Lunn
>><andrew@...n.ch>
>>Cc: netdev@...r.kernel.org; Arseny Solokha <asolokha@...kras.ru>
>>Subject: [RFC PATCH 1/2] gianfar: convert to phylink
>>
>>Convert gianfar to use the phylink API for better SFP modules support.
>>
>>The driver still uses phylib for serdes configuration over the TBI
>>interface, as there seems to be no functionally equivalent API present
>>in phylink (yet). phylib usage is basically confined in two functions.
>>
>
> Thanks for your patch.  Phylink in gianfar... that would be something!
> At first glance a lot of code has changed with this patch or got relocated.
> To make it easier to swallow, I think a few cleanup patches could be
> separated before migrating to phylink.  Like for instance getting rid of the
> old* link state variables, which I think are an artifact from early phylib usage.
> Nonetheless good to see this implemented, I'll have a closer look asap.

Hi,

meanwhile I'll have to post v2 of this patch because it has some issues which
initially escaped my attention. For now I'm pasting the diff against v1 here for
reference:

--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1666,7 +1666,7 @@ static int gfar_suspend(struct device *dev)
 		gfar_start_wol_filer(priv);
 
 	} else {
-		phylink_stop(phy->phylink);
+		phylink_stop(priv->phylink);
 	}
 
 	priv->speed = SPEED_UNKNOWN;
@@ -3699,9 +3699,6 @@ static void gfar_mac_config(struct phylink_config *config, unsigned int mode,
 	if (unlikely(test_bit(GFAR_RESETTING, &priv->state)))
 		return;
 
-	if (unlikely(phylink_autoneg_inband(mode)))
-		return;
-
 	maccfg1 = gfar_read(&regs->maccfg1);
 	maccfg2 = gfar_read(&regs->maccfg2);
 	ecntrl = gfar_read(&regs->ecntrl);

The first hunk here fixes a typo which broke build with PM enabled. The second
one removes an early return from gfar_mac_config() which I believe is really
bogus and also breaks coalesce parameters calculation for SGMII and 1000Base-X
attached PHYs.

I'd like to submit a real v2 after the patches gets actual review, though.

Thanks,
Arseny