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

[Arseny Solokha]

>>-----Original Message-----
>>From: Arseny Solokha <asolo...@kb.kras.ru>
>>Sent: Tuesday, July 23, 2019 6:17 PM
>>To: Claudiu Manoil <claudiu.man...@nxp.com>; Ioana Ciornei
>><ioana.cior...@nxp.com>; Russell King <li...@armlinux.org.uk>; Andrew Lunn
>><and...@lunn.ch>
>>Cc: netdev@vger.kernel.org; Arseny Solokha <asolo...@kb.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