From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH net-next v2 3/9] net: phy: phylink: Poll link GPIOs Date: Thu, 10 May 2018 13:32:51 -0700 Message-ID: <696b48ec-db9f-5e78-c6cb-e679eb18cb9b@gmail.com> References: <20180510201737.13887-1-f.fainelli@gmail.com> <20180510201737.13887-4-f.fainelli@gmail.com> <20180510202900.GZ16141@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, privat@egil-hjelmeland.no, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, davem@davemloft.net, sean.wang@mediatek.com, Woojung.Huh@microchip.com, john@phrozen.org, cphealy@gmail.com To: Russell King - ARM Linux Return-path: Received: from mail-qk0-f196.google.com ([209.85.220.196]:45374 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751924AbeEJUdB (ORCPT ); Thu, 10 May 2018 16:33:01 -0400 Received: by mail-qk0-f196.google.com with SMTP id c64-v6so2640825qkf.12 for ; Thu, 10 May 2018 13:33:01 -0700 (PDT) In-Reply-To: <20180510202900.GZ16141@n2100.armlinux.org.uk> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 05/10/2018 01:29 PM, Russell King - ARM Linux wrote: > On Thu, May 10, 2018 at 01:17:31PM -0700, Florian Fainelli wrote: >> From: Russell King >> >> When using a fixed link with a link GPIO, we need to poll that GPIO to >> determine link state changes. This is consistent with what fixed_phy.c does. >> >> Signed-off-by: Florian Fainelli > > I'd like this to use the GPIO interrupt where available, only falling back > to the timer approach when there's no interrupt. Unfortunately, I don't > have much time to devote to this at the moment, having recently been away > on vacation, and now having to work on ARM specific issues for probably > all of the remainder of this kernel cycle. > > That means I won't have time to test your series on any of the boards > I have available to me. No worries, thanks for looking at this. Andrew and I both tested this on the devel B and C boards where this is primarily useful. Can I still get your SoB for the portions you authored? I will follow up with a change that uses GPIO interrupts when they are available. > >> --- >> drivers/net/phy/phylink.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c >> index 6392b5248cf5..581ce93ecaf9 100644 >> --- a/drivers/net/phy/phylink.c >> +++ b/drivers/net/phy/phylink.c >> @@ -19,6 +19,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> #include "sfp.h" >> @@ -54,6 +55,7 @@ struct phylink { >> /* The link configuration settings */ >> struct phylink_link_state link_config; >> struct gpio_desc *link_gpio; >> + struct timer_list link_poll; >> void (*get_fixed_state)(struct net_device *dev, >> struct phylink_link_state *s); >> >> @@ -500,6 +502,15 @@ static void phylink_run_resolve(struct phylink *pl) >> queue_work(system_power_efficient_wq, &pl->resolve); >> } >> >> +static void phylink_fixed_poll(struct timer_list *t) >> +{ >> + struct phylink *pl = container_of(t, struct phylink, link_poll); >> + >> + mod_timer(t, jiffies + HZ); >> + >> + phylink_run_resolve(pl); >> +} >> + >> static const struct sfp_upstream_ops sfp_phylink_ops; >> >> static int phylink_register_sfp(struct phylink *pl, >> @@ -572,6 +583,7 @@ struct phylink *phylink_create(struct net_device *ndev, >> pl->link_config.an_enabled = true; >> pl->ops = ops; >> __set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state); >> + timer_setup(&pl->link_poll, phylink_fixed_poll, 0); >> >> bitmap_fill(pl->supported, __ETHTOOL_LINK_MODE_MASK_NBITS); >> linkmode_copy(pl->link_config.advertising, pl->supported); >> @@ -905,6 +917,8 @@ void phylink_start(struct phylink *pl) >> clear_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state); >> phylink_run_resolve(pl); >> >> + if (pl->link_an_mode == MLO_AN_FIXED && !IS_ERR(pl->link_gpio)) >> + mod_timer(&pl->link_poll, jiffies + HZ); >> if (pl->sfp_bus) >> sfp_upstream_start(pl->sfp_bus); >> if (pl->phydev) >> @@ -929,6 +943,8 @@ void phylink_stop(struct phylink *pl) >> phy_stop(pl->phydev); >> if (pl->sfp_bus) >> sfp_upstream_stop(pl->sfp_bus); >> + if (pl->link_an_mode == MLO_AN_FIXED && !IS_ERR(pl->link_gpio)) >> + del_timer_sync(&pl->link_poll); >> >> set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state); >> queue_work(system_power_efficient_wq, &pl->resolve); >> -- >> 2.14.1 >> > -- Florian