Re: [PATCH net-next v2 3/9] net: phy: phylink: Poll link GPIOs
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
Re: [PATCH net-next v2 3/9] net: phy: phylink: Poll link GPIOs
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.
> ---
> 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
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
[PATCH net-next v2 3/9] net: phy: phylink: Poll link GPIOs
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
---
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
