Re: [PATCH v5 4/4] net: phy: leds: add support for led triggers on phy link state change

2016-10-18 Thread Zach Brown 
On Tue, Oct 18, 2016 at 09:13:50AM +0200, Andrew Lunn wrote:
> On Mon, Oct 17, 2016 at 10:49:55AM -0500, Zach Brown wrote:
> > Create an option CONFIG_LED_TRIGGER_PHY (default n), which will create a
> > set of led triggers for each instantiated PHY device. There is one LED
> > trigger per link-speed, per-phy.
> > The triggers are registered during phy_attach and unregistered during
> > phy_detach.
> >
> > This allows for a user to configure their system to allow a set of LEDs
> > not controlled by the phy to represent link state changes on the phy.
> > LEDS controlled by the phy are unaffected.
> >
> > For example, we have a board where some of the leds in the
> > RJ45 socket are controlled by the phy, but others are not. Using the
> > triggers provided by this patch the leds not controlled by the phy can
> > be configured to show the current speed of the ethernet connection. The
> > leds controlled by the phy are unaffected.
>
> Is there a clear path how we generalise this, so it could also be used
> to control PHY LEDs?
>

No, this patch set is only concerned with generic LEDs not PHY LEDs.

> The idea i had a while ago was that the PHY LEDS would also have a
> list of triggers which mapped to what the PHY controller could do. So
> to enable the PHY LED to show RxTx activity, you would enable the RxTx
> trigger on the PHY LED.
>
> This needs an extension to the PHY code, in that these triggers are
> specific to the LED, not general across all LEDs. But this is not too
> big an issue.
>
> We could end up that if a PHY LED can be used as a generic LED, your
> 'manual' trigger could be used on it. But it could also support the
> PHY driven 'automatic' link status indication trigger. Do we want two
> triggers for the same or similar information?
>

If generic LEDs can only use the 'manual' triggers then having two triggers
for the same or similar information would be okay. The focus of this patch set
was to allow generic LEDs to represent the current speed of the phy, which is
very useful when the PHY LEDs don't for whatever reason i.e they don't exist or
are busy representing another aspect.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 0/4] Add support for led triggers on phy link state change

2016-10-17 Thread Zach Brown 
Fix skge driver that declared enum contants that conflicted with enum
constants in linux/leds.h

Create function that encapsulates actions taken during the adjust phy link step
of phy state changes.

Create function that provides list of speeds currently supported by the phy.

Add support for led triggers on phy link state changes by adding
a config option. When set the config option will create a set of led triggers
for each phy device. Users can use the led triggers to represent link state
changes on the phy.

v2:
 * New patch that creates phy_adjust_link function to encapsulate actions taken
   when adjusting phy link during phy state changes
 * led trigger speed strings changed to match existing phy speed strings
 * New function that maps speeds to led triggers
 * Replace magic constants with definitions when declaring trigger name
   buffer and number of triggers.
v3:
 * Changed LED_ON to LED_REG_ON in skge driver to avoid possible future
   conflict and improve consistency.
 * Dropped rtl8712 patch that was accepted separately.
v4:
 * tweaked commit message
v5
 * Changed commit message to explain relationship between the new triggers and
   leds driven by phys.
 * Added new patch that creates phy_supported_speeds function.
 * Moved phy_leds_triggers_register and phy_leds_triggers_unregister to
   phy_attach and phy_detach respectively. This change is so the
   phydev->supported field will be filled by the time the triggers are
   registered.
 * Changed hardcoded list of triggers to dynamic list determined by speeds
   return by phy_supported_speeds.

Zach Brown (4):
  skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid
conflicts with leds namespace
  net: phy: Encapsulate actions performed during link state changes into
function phy_adjust_link
  net: phy: Create phy_supported_speeds function which lists speeds
currently supported by a phydevice
  net: phy: leds: add support for led triggers on phy link state change

 drivers/net/ethernet/marvell/skge.c |   6 +-
 drivers/net/ethernet/marvell/skge.h |   4 +-
 drivers/net/phy/Kconfig |  13 
 drivers/net/phy/Makefile|   1 +
 drivers/net/phy/phy.c   |  57 ---
 drivers/net/phy/phy_device.c|   5 ++
 drivers/net/phy/phy_led_triggers.c  | 136 
 include/linux/phy.h |  22 ++
 include/linux/phy_led_triggers.h|  51 ++
 9 files changed, 282 insertions(+), 13 deletions(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 2/4] net: phy: Encapsulate actions performed during link state changes into function phy_adjust_link

2016-10-17 Thread Zach Brown 
During phy state machine state transitions some set of actions should
occur whenever the link state changes. These actions should be
encapsulated into a single function

This patch adds the phy_adjust_link function, which is called whenever
phydev->adjust_link would have been called before. Actions that should
occur whenever the phy link is adjusted can now be added to the
phy_adjust_link function.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/phy.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..f5721db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -893,6 +893,11 @@ void phy_start(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_start);
 
+static void phy_adjust_link(struct phy_device *phydev)
+{
+   phydev->adjust_link(phydev->attached_dev);
+}
+
 /**
  * phy_state_machine - Handle the state machine
  * @work: work_struct that describes the work to be done
@@ -935,7 +940,7 @@ void phy_state_machine(struct work_struct *work)
if (!phydev->link) {
phydev->state = PHY_NOLINK;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
}
 
@@ -948,7 +953,7 @@ void phy_state_machine(struct work_struct *work)
if (err > 0) {
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
} else if (0 == phydev->link_timeout--)
needs_aneg = true;
@@ -975,7 +980,7 @@ void phy_state_machine(struct work_struct *work)
}
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
case PHY_FORCING:
@@ -991,7 +996,7 @@ void phy_state_machine(struct work_struct *work)
needs_aneg = true;
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
case PHY_RUNNING:
/* Only register a CHANGE if we are polling and link changed
@@ -1020,7 +1025,7 @@ void phy_state_machine(struct work_struct *work)
netif_carrier_off(phydev->attached_dev);
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
if (phy_interrupt_is_valid(phydev))
err = phy_config_interrupt(phydev,
@@ -1030,7 +1035,7 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
do_suspend = true;
}
break;
@@ -1054,7 +1059,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
} else {
phydev->state = PHY_AN;
phydev->link_timeout = PHY_AN_TIMEOUT;
@@ -1070,7 +1075,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
}
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 1/4] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace

2016-10-17 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF
Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
for consistency.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/ethernet/marvell/skge.c | 6 +++---
 drivers/net/ethernet/marvell/skge.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c 
b/drivers/net/ethernet/marvell/skge.c
index 7173836..783df01 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1048,7 +1048,7 @@ static const char *skge_pause(enum pause_status status)
 static void skge_link_up(struct skge_port *skge)
 {
skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG),
-   LED_BLK_OFF|LED_SYNC_OFF|LED_ON);
+   LED_BLK_OFF|LED_SYNC_OFF|LED_REG_ON);
 
netif_carrier_on(skge->netdev);
netif_wake_queue(skge->netdev);
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
 
 static void skge_link_down(struct skge_port *skge)
 {
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
netif_carrier_off(skge->netdev);
netif_stop_queue(skge->netdev);
 
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
if (hw->ports == 1)
free_irq(hw->pdev->irq, hw);
 
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
if (is_genesis(hw))
genesis_stop(skge);
else
diff --git a/drivers/net/ethernet/marvell/skge.h 
b/drivers/net/ethernet/marvell/skge.h
index a2eb341..3ea151f 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -662,8 +662,8 @@ enum {
LED_BLK_OFF = 1<<4, /* Link LED Blinking Off */
LED_SYNC_ON = 1<<3, /* Use Sync Wire to switch LED */
LED_SYNC_OFF= 1<<2, /* Disable Sync Wire Input */
-   LED_ON  = 1<<1, /* switch LED on */
-   LED_OFF = 1<<0, /* switch LED off */
+   LED_REG_ON  = 1<<1, /* switch LED on */
+   LED_REG_OFF = 1<<0, /* switch LED off */
 };
 
 /* Receive GMAC FIFO (YUKON) */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 4/4] net: phy: leds: add support for led triggers on phy link state change

2016-10-17 Thread Zach Brown 
Create an option CONFIG_LED_TRIGGER_PHY (default n), which will create a
set of led triggers for each instantiated PHY device. There is one LED
trigger per link-speed, per-phy.
The triggers are registered during phy_attach and unregistered during
phy_detach.

This allows for a user to configure their system to allow a set of LEDs
not controlled by the phy to represent link state changes on the phy.
LEDS controlled by the phy are unaffected.

For example, we have a board where some of the leds in the
RJ45 socket are controlled by the phy, but others are not. Using the
triggers provided by this patch the leds not controlled by the phy can
be configured to show the current speed of the ethernet connection. The
leds controlled by the phy are unaffected.

Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/Kconfig|  13 
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/phy.c  |   1 +
 drivers/net/phy/phy_device.c   |   5 ++
 drivers/net/phy/phy_led_triggers.c | 136 +
 include/linux/phy.h|   7 ++
 include/linux/phy_led_triggers.h   |  51 ++
 7 files changed, 214 insertions(+)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 5078a0d..54c8eb8 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -15,6 +15,19 @@ if PHYLIB
 config SWPHY
bool
 
+config LED_TRIGGER_PHY
+   bool "Support LED triggers for tracking link state"
+   depends on LEDS_TRIGGERS
+   ---help---
+ Adds support for a set of LED trigger events per-PHY.  Link
+ state change will trigger the events, for consumption by an
+ LED class driver.  There are triggers for each link speed currently
+ supported by the phy, and are of the form:
+  ::
+
+ Where speed is in the form:
+   Mbps or Gbps
+
 comment "MDIO bus device drivers"
 
 config MDIO_BCM_IPROC
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
 
 libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY) += swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)   += libphy.o
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 82ee233..ef0e3d0 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -931,6 +931,7 @@ EXPORT_SYMBOL(phy_start);
 static void phy_adjust_link(struct phy_device *phydev)
 {
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
 }
 
 /**
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba9..b41ebd5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -916,6 +917,8 @@ int phy_attach_direct(struct net_device *dev, struct 
phy_device *phydev,
else
phy_resume(phydev);
 
+   phy_led_triggers_register(phydev);
+
return err;
 
 error:
@@ -989,6 +992,8 @@ void phy_detach(struct phy_device *phydev)
}
}
 
+   phy_led_triggers_unregister(phydev);
+
/*
 * The phydev might go away on the put_device() below, so avoid
 * a use-after-free bug by reading the underlying bus first.
diff --git a/drivers/net/phy/phy_led_triggers.c 
b/drivers/net/phy/phy_led_triggers.c
new file mode 100644
index 000..cda600a
--- /dev/null
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -0,0 +1,136 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+  * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include 
+#include 
+#include 
+
+static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy,
+   unsigned int speed)
+{
+   unsigned int i;
+
+   for (i = 0; i < phy->phy_num_led_triggers; i++) {
+   if (phy->phy_led_triggers[i].speed == speed)
+   return >phy_led_triggers[i];
+   }
+   return NULL;
+

[PATCH v5 3/4] net: phy: Create phy_supported_speeds function which lists speeds currently supported by a phydevice

2016-10-17 Thread Zach Brown 
phy_supported_speeds provides a means to get a list of all the speeds a
phy device currently supports.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/phy.c | 35 +++
 include/linux/phy.h   | 15 +++
 2 files changed, 50 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f5721db..82ee233 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -261,6 +261,41 @@ static inline unsigned int phy_find_valid(unsigned int 
idx, u32 features)
 }
 
 /**
+ * phy_supported_speeds - return all speeds currently supported by a phy device
+ * @phy: The phy device to return supported speeds of.
+ * @speeds: buffer to store supported speeds in.
+ * @size:   size of speeds buffer.
+ *
+ * Description: Returns the number of supported speeds, and fills the speeds
+ * buffer with the supported speeds. If speeds buffer is too small to contain
+ * all currently supported speeds, will return as many speeds as can fit.
+ */
+unsigned int phy_supported_speeds(struct phy_device *phy,
+ unsigned int *speeds,
+ unsigned int size)
+{
+   unsigned int count = 0;
+   unsigned int idx = 0;
+
+   while (idx < MAX_NUM_SETTINGS && count < size) {
+   idx = phy_find_valid(idx, phy->supported);
+
+   if (!(settings[idx].setting & phy->supported))
+   break;
+
+   /* Assumes settings are grouped by speed */
+   if ((count == 0) ||
+   (speeds[count - 1] != settings[idx].speed)) {
+   speeds[count] = settings[idx].speed;
+   count++;
+   }
+   idx++;
+   }
+
+   return count;
+}
+
+/**
  * phy_check_valid - check if there is a valid PHY setting which matches
  *  speed, duplex, and feature mask
  * @speed: speed to match
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e25f183..8761f30 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -85,6 +85,21 @@ typedef enum {
 } phy_interface_t;
 
 /**
+ * phy_supported_speeds - return all speeds currently supported by a phy device
+ * @phy: The phy device to return supported speeds of.
+ * @speeds: buffer to store supported speeds in.
+ * @size: size of speeds buffer.
+ *
+ * Description: Returns the number of supported speeds, and
+ * fills the speeds * buffer with the supported speeds. If speeds buffer is
+ * too small to contain * all currently supported speeds, will return as
+ * many speeds as can fit.
+ */
+unsigned int phy_supported_speeds(struct phy_device *phy,
+ unsigned int *speeds,
+ unsigned int size);
+
+/**
  * It maps 'enum phy_interface_t' found in include/linux/phy.h
  * into the device tree binding of 'phy-mode', so that Ethernet
  * device driver can get phy interface from device tree.
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 3/3] net: phy: leds: add support for led triggers on phy link state change

2016-10-13 Thread Zach Brown 
On Thu, Oct 13, 2016 at 10:46:34AM -0400, David Miller wrote:
> From: Zach Brown <zach.br...@ni.com>
> Date: Tue, 11 Oct 2016 15:26:20 -0500
> 
> > From: Josh Cartwright <josh.cartwri...@ni.com>
> > 
> > Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
> > create a set of led triggers for each instantiated PHY device.  There is
> > one LED trigger per link-speed, per-phy.
> > 
> > This allows for a user to configure their system to allow a set of LEDs
> > to represent link state changes on the phy.
> > 
> > Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
> > Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
> > Signed-off-by: Zach Brown <zach.br...@ni.com>
>  ...
> > +   static const char * const name_suffix[] = {
> > +   "10Mbps",
> > +   "100Mbps",
> > +   "1Gbps",
> > +   "2.5Gbps",
> > +   "10Gbps",
> 
> This choice of both the array size and the speeds to support seems
> entirely arbitrary and is inappropriate for a generic driver of this
> kind.
> 
> This seems to be hard coding this to support the list of speeds
> supported by whatever driver you want to use with this new LED
> facility, and sorry that's not how we build nice generic pieces of
> infrastructure.
> 
> Thanks.

The speeds listed are the speeds found in the phy_speed_to_str function in 
phy.c.
They are also the speeds found in the struct phy_setting settings array,
which is commented with
"/* A mapping of all SUPPORTED settings to speed/duplex */"
We believed they represented the commonly supported speeds of phys.

Do you have suggestions on how to better handle the choice of the array size
and the speeds?

Thanks.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC v3 3/3] phy,leds: add support for led triggers on phy link state change

2016-10-12 Thread Zach Brown 
On Mon, Oct 10, 2016 at 02:03:32AM -0700, Florian Fainelli wrote:
> > +
> > +#ifdef CONFIG_LED_TRIGGER_PHY
> > +
> > +#include 
> > +#include 
> > +
> > +#define PHY_LINK_LED_MAX_TRIGGERS  5
> > +#define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE  7
> > +#define PHY_MII_BUS_ID_SIZE(20 - 3)
> 
> This particular constant may be something worth moving to
> include/linux/phy.h eventually.
> -- 
> Florian

MII_BUS_ID_SIZE is defined in include/linux/phy.h but it's defined after
phy_led_triggers.h is included so phy_led_triggers.h doesn't have access.
I could move the definition of MII_BUS_ID_SIZE above the include, but that
seemed ugly. Do you have any suggestions?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 1/3] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace

2016-10-11 Thread Zach Brown 
On Tue, Oct 11, 2016 at 02:14:07PM -0700, Stephen Hemminger wrote:
> On Tue, 11 Oct 2016 15:26:18 -0500
> Zach Brown <zach.br...@ni.com> wrote:
>
> > Adding led support for phy causes namespace conflicts for some
> > phy drivers.
> >
> > The marvel skge driver declared an enum for representing the states of
> > Link LED Register. The enum contained constant LED_OFF which conflicted
> > with declartation found in linux/leds.h.
> > LED_OFF changed to LED_REG_OFF
> > Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
> > for consistency.
> >
> > Signed-off-by: Zach Brown <zach.br...@ni.com>
>
> Sure, that's fine but not sure why skge would be including linux/leds.h
> anyway.

It's pretty convoluted. Here's the chain of includes.
skge -> netdevice -> dsa -> phy -> phy_led_triggers -> leds

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 2/3] net: phy: Encapsulate actions performed during link state changes into function phy_adjust_link

2016-10-11 Thread Zach Brown 
During phy state machine state transitions some set of actions should
occur whenever the link state changes. These actions should be
encapsulated into a single function

This patch adds the phy_adjust_link function, which is called whenever
phydev->adjust_link would have been called before. Actions that should
occur whenever the phy link is adjusted can now be added to the
phy_adjust_link function.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/phy.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..f5721db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -893,6 +893,11 @@ void phy_start(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_start);
 
+static void phy_adjust_link(struct phy_device *phydev)
+{
+   phydev->adjust_link(phydev->attached_dev);
+}
+
 /**
  * phy_state_machine - Handle the state machine
  * @work: work_struct that describes the work to be done
@@ -935,7 +940,7 @@ void phy_state_machine(struct work_struct *work)
if (!phydev->link) {
phydev->state = PHY_NOLINK;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
}
 
@@ -948,7 +953,7 @@ void phy_state_machine(struct work_struct *work)
if (err > 0) {
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
} else if (0 == phydev->link_timeout--)
needs_aneg = true;
@@ -975,7 +980,7 @@ void phy_state_machine(struct work_struct *work)
}
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
case PHY_FORCING:
@@ -991,7 +996,7 @@ void phy_state_machine(struct work_struct *work)
needs_aneg = true;
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
case PHY_RUNNING:
/* Only register a CHANGE if we are polling and link changed
@@ -1020,7 +1025,7 @@ void phy_state_machine(struct work_struct *work)
netif_carrier_off(phydev->attached_dev);
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
if (phy_interrupt_is_valid(phydev))
err = phy_config_interrupt(phydev,
@@ -1030,7 +1035,7 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
do_suspend = true;
}
break;
@@ -1054,7 +1059,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
} else {
phydev->state = PHY_AN;
phydev->link_timeout = PHY_AN_TIMEOUT;
@@ -1070,7 +1075,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
}
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 0/3] Add support for led triggers on phy link state change

2016-10-11 Thread Zach Brown 
Fix skge driver that declared enum contants that conflicted with enum
constants in linux/leds.h

Create function that encapsulates actions taken during the adjust phy link step
of phy state changes.

Add support for led triggers on phy link state changes by adding
a config option. When set the config option will create a set of led triggers
for each phy device. Users can use the led triggers to represent link state
changes on the phy.

v2:
 * New patch that creates phy_adjust_link function to encapsulate actions taken
   when adjusting phy link during phy state changes
 * led trigger speed strings changed to match existing phy speed strings
 * New function that maps speeds to led triggers
 * Replace magic constants with definitions when declaring trigger name
   buffer and number of triggers.
v3:
 * Changed LED_ON to LED_REG_ON in skge driver to avoid possible future
   conflict and improve consistency.
 * Dropped rtl8712 patch that was accepted separately.
v4:
 * tweaked commit message

Josh Cartwright (1):
  net: phy: leds: add support for led triggers on phy link state change

Zach Brown (2):
  skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid
conflicts with leds namespace
  net: phy: Encapsulate actions performed during link state changes into
function phy_adjust_link

 drivers/net/ethernet/marvell/skge.c |   6 +-
 drivers/net/ethernet/marvell/skge.h |   4 +-
 drivers/net/phy/Kconfig |  13 +++-
 drivers/net/phy/Makefile|   1 +
 drivers/net/phy/phy.c   |  22 ---
 drivers/net/phy/phy_device.c|   4 ++
 drivers/net/phy/phy_led_triggers.c  | 121 
 include/linux/phy.h |   9 +++
 include/linux/phy_led_triggers.h|  52 
 9 files changed, 218 insertions(+), 14 deletions(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 3/3] net: phy: leds: add support for led triggers on phy link state change

2016-10-11 Thread Zach Brown 
From: Josh Cartwright <josh.cartwri...@ni.com>

Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
create a set of led triggers for each instantiated PHY device.  There is
one LED trigger per link-speed, per-phy.

This allows for a user to configure their system to allow a set of LEDs
to represent link state changes on the phy.

Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/Kconfig|  13 +++-
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/phy.c  |   1 +
 drivers/net/phy/phy_device.c   |   4 ++
 drivers/net/phy/phy_led_triggers.c | 121 +
 include/linux/phy.h|   9 +++
 include/linux/phy_led_triggers.h   |  52 
 7 files changed, 200 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 5078a0d..4fd912d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -25,6 +25,18 @@ config MDIO_BCM_IPROC
  This module provides a driver for the MDIO busses found in the
  Broadcom iProc SoC's.
 
+config LED_TRIGGER_PHY
+   bool "Support LED triggers for tracking link state"
+   depends on LEDS_TRIGGERS
+   ---help---
+ Adds support for a set of LED trigger events per-PHY.  Link
+ state change will trigger the events, for consumption by an
+ LED class driver.  There are triggers for each link speed,
+ and are of the form:
+  ::
+
+ Where speed is one of: 10Mbps, 100Mbps, 1Gbps, 2.5Gbps, or 10Gbps.
+
 config MDIO_BCM_UNIMAC
tristate "Broadcom UniMAC MDIO bus controller"
depends on HAS_IOMEM
@@ -40,7 +52,6 @@ config MDIO_BITBANG
  This module implements the MDIO bus protocol in software,
  for use by low level drivers that export the ability to
  drive the relevant pins.
-
  If in doubt, say N.
 
 config MDIO_BUS_MUX
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
 
 libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY) += swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)   += libphy.o
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f5721db..e5f9fee7 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -896,6 +896,7 @@ EXPORT_SYMBOL(phy_start);
 static void phy_adjust_link(struct phy_device *phydev)
 {
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
 }
 
 /**
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba9..4671c13 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -57,6 +58,7 @@ static void phy_mdio_device_free(struct mdio_device *mdiodev)
 
 static void phy_device_release(struct device *dev)
 {
+   phy_led_triggers_unregister(to_phy_device(dev));
kfree(to_phy_device(dev));
 }
 
@@ -345,6 +347,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, 
int addr, int phy_id,
 
dev->state = PHY_DOWN;
 
+   phy_led_triggers_register(dev);
+
mutex_init(>lock);
INIT_DELAYED_WORK(>state_queue, phy_state_machine);
INIT_WORK(>phy_queue, phy_change);
diff --git a/drivers/net/phy/phy_led_triggers.c 
b/drivers/net/phy/phy_led_triggers.c
new file mode 100644
index 000..32326d7
--- /dev/null
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -0,0 +1,121 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+  * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include 
+#include 
+#include 
+
+static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy,
+   unsigned int speed)
+{
+   switch (speed) {
+   case SPEED_10:
+   return >phy_led_trigger[0];
+   case SPEED_100:
+   return >phy_led_trigger[1];
+   case SPEED_1000:
+   return

[PATCH v4 1/3] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace

2016-10-11 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF
Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
for consistency.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/ethernet/marvell/skge.c | 6 +++---
 drivers/net/ethernet/marvell/skge.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c 
b/drivers/net/ethernet/marvell/skge.c
index 7173836..783df01 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1048,7 +1048,7 @@ static const char *skge_pause(enum pause_status status)
 static void skge_link_up(struct skge_port *skge)
 {
skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG),
-   LED_BLK_OFF|LED_SYNC_OFF|LED_ON);
+   LED_BLK_OFF|LED_SYNC_OFF|LED_REG_ON);
 
netif_carrier_on(skge->netdev);
netif_wake_queue(skge->netdev);
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
 
 static void skge_link_down(struct skge_port *skge)
 {
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
netif_carrier_off(skge->netdev);
netif_stop_queue(skge->netdev);
 
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
if (hw->ports == 1)
free_irq(hw->pdev->irq, hw);
 
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
if (is_genesis(hw))
genesis_stop(skge);
else
diff --git a/drivers/net/ethernet/marvell/skge.h 
b/drivers/net/ethernet/marvell/skge.h
index a2eb341..3ea151f 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -662,8 +662,8 @@ enum {
LED_BLK_OFF = 1<<4, /* Link LED Blinking Off */
LED_SYNC_ON = 1<<3, /* Use Sync Wire to switch LED */
LED_SYNC_OFF= 1<<2, /* Disable Sync Wire Input */
-   LED_ON  = 1<<1, /* switch LED on */
-   LED_OFF = 1<<0, /* switch LED off */
+   LED_REG_ON  = 1<<1, /* switch LED on */
+   LED_REG_OFF = 1<<0, /* switch LED off */
 };
 
 /* Receive GMAC FIFO (YUKON) */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC v3 2/3] phy: Encapsulate actions performed during link state changes into function phy_adjust_link

2016-10-07 Thread Zach Brown 
During phy state machine state transitions some set of actions should
occur whenever the link state changes. These actions should be
encapsulated into a single function

This patch adds the phy_adjust_link function, which is called whenever
phydev->adjust_link would have been called before. Actions that should
occur whenever the phy link is adjusted can now be added to the
phy_adjust_link function.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/phy.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..f5721db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -893,6 +893,11 @@ void phy_start(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_start);
 
+static void phy_adjust_link(struct phy_device *phydev)
+{
+   phydev->adjust_link(phydev->attached_dev);
+}
+
 /**
  * phy_state_machine - Handle the state machine
  * @work: work_struct that describes the work to be done
@@ -935,7 +940,7 @@ void phy_state_machine(struct work_struct *work)
if (!phydev->link) {
phydev->state = PHY_NOLINK;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
}
 
@@ -948,7 +953,7 @@ void phy_state_machine(struct work_struct *work)
if (err > 0) {
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
} else if (0 == phydev->link_timeout--)
needs_aneg = true;
@@ -975,7 +980,7 @@ void phy_state_machine(struct work_struct *work)
}
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
case PHY_FORCING:
@@ -991,7 +996,7 @@ void phy_state_machine(struct work_struct *work)
needs_aneg = true;
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
case PHY_RUNNING:
/* Only register a CHANGE if we are polling and link changed
@@ -1020,7 +1025,7 @@ void phy_state_machine(struct work_struct *work)
netif_carrier_off(phydev->attached_dev);
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
if (phy_interrupt_is_valid(phydev))
err = phy_config_interrupt(phydev,
@@ -1030,7 +1035,7 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
do_suspend = true;
}
break;
@@ -1054,7 +1059,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
} else {
phydev->state = PHY_AN;
phydev->link_timeout = PHY_AN_TIMEOUT;
@@ -1070,7 +1075,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
}
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC v3 3/3] phy, leds: add support for led triggers on phy link state change

2016-10-07 Thread Zach Brown 
From: Josh Cartwright <josh.cartwri...@ni.com>

Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
create a set of led triggers for each instantiated PHY device.  There is
one LED trigger per link-speed, per-phy.

This allows for a user to configure their system to allow a set of LEDs
to represent link state changes on the phy.

Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/Kconfig|  13 +++-
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/phy.c  |   1 +
 drivers/net/phy/phy_device.c   |   4 ++
 drivers/net/phy/phy_led_triggers.c | 121 +
 include/linux/phy.h|   9 +++
 include/linux/phy_led_triggers.h   |  52 
 7 files changed, 200 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 5078a0d..4fd912d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -25,6 +25,18 @@ config MDIO_BCM_IPROC
  This module provides a driver for the MDIO busses found in the
  Broadcom iProc SoC's.
 
+config LED_TRIGGER_PHY
+   bool "Support LED triggers for tracking link state"
+   depends on LEDS_TRIGGERS
+   ---help---
+ Adds support for a set of LED trigger events per-PHY.  Link
+ state change will trigger the events, for consumption by an
+ LED class driver.  There are triggers for each link speed,
+ and are of the form:
+  ::
+
+ Where speed is one of: 10Mbps, 100Mbps, 1Gbps, 2.5Gbps, or 10Gbps.
+
 config MDIO_BCM_UNIMAC
tristate "Broadcom UniMAC MDIO bus controller"
depends on HAS_IOMEM
@@ -40,7 +52,6 @@ config MDIO_BITBANG
  This module implements the MDIO bus protocol in software,
  for use by low level drivers that export the ability to
  drive the relevant pins.
-
  If in doubt, say N.
 
 config MDIO_BUS_MUX
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
 
 libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY) += swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)   += libphy.o
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f5721db..e5f9fee7 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -896,6 +896,7 @@ EXPORT_SYMBOL(phy_start);
 static void phy_adjust_link(struct phy_device *phydev)
 {
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
 }
 
 /**
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba9..4671c13 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -57,6 +58,7 @@ static void phy_mdio_device_free(struct mdio_device *mdiodev)
 
 static void phy_device_release(struct device *dev)
 {
+   phy_led_triggers_unregister(to_phy_device(dev));
kfree(to_phy_device(dev));
 }
 
@@ -345,6 +347,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, 
int addr, int phy_id,
 
dev->state = PHY_DOWN;
 
+   phy_led_triggers_register(dev);
+
mutex_init(>lock);
INIT_DELAYED_WORK(>state_queue, phy_state_machine);
INIT_WORK(>phy_queue, phy_change);
diff --git a/drivers/net/phy/phy_led_triggers.c 
b/drivers/net/phy/phy_led_triggers.c
new file mode 100644
index 000..32326d7
--- /dev/null
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -0,0 +1,121 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+  * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include 
+#include 
+#include 
+
+static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy,
+   unsigned int speed)
+{
+   switch (speed) {
+   case SPEED_10:
+   return >phy_led_trigger[0];
+   case SPEED_100:
+   return >phy_led_trigger[1];
+   case SPEED_1000:
+   return

[RFC v3 1/3] skge: Rename LED_OFF and LED_ON in marvel skge driver to avoid conflicts with leds namespace

2016-10-07 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF
Also changed LED_ON to LED_REG_ON to avoid possible future conflict and
for consistency.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/ethernet/marvell/skge.c | 6 +++---
 drivers/net/ethernet/marvell/skge.h | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c 
b/drivers/net/ethernet/marvell/skge.c
index 7173836..783df01 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1048,7 +1048,7 @@ static const char *skge_pause(enum pause_status status)
 static void skge_link_up(struct skge_port *skge)
 {
skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG),
-   LED_BLK_OFF|LED_SYNC_OFF|LED_ON);
+   LED_BLK_OFF|LED_SYNC_OFF|LED_REG_ON);
 
netif_carrier_on(skge->netdev);
netif_wake_queue(skge->netdev);
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
 
 static void skge_link_down(struct skge_port *skge)
 {
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
netif_carrier_off(skge->netdev);
netif_stop_queue(skge->netdev);
 
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
if (hw->ports == 1)
free_irq(hw->pdev->irq, hw);
 
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
if (is_genesis(hw))
genesis_stop(skge);
else
diff --git a/drivers/net/ethernet/marvell/skge.h 
b/drivers/net/ethernet/marvell/skge.h
index a2eb341..3ea151f 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -662,8 +662,8 @@ enum {
LED_BLK_OFF = 1<<4, /* Link LED Blinking Off */
LED_SYNC_ON = 1<<3, /* Use Sync Wire to switch LED */
LED_SYNC_OFF= 1<<2, /* Disable Sync Wire Input */
-   LED_ON  = 1<<1, /* switch LED on */
-   LED_OFF = 1<<0, /* switch LED off */
+   LED_REG_ON  = 1<<1, /* switch LED on */
+   LED_REG_OFF = 1<<0, /* switch LED off */
 };
 
 /* Receive GMAC FIFO (YUKON) */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/3] Add support for led triggers on phy link state change

2016-10-07 Thread Zach Brown 
Fix skge driver that declared enum contants that conflicted with enum
constants in linux/leds.h

Create function that encapsulates actions taken during the adjust phy link step
of phy state changes.

Add support for led triggers on phy link state changes by adding
a config option. When set the config option will create a set of led triggers
for each phy device. Users can use the led triggers to represent link state
changes on the phy.

v2:
 * New patch that creates phy_adjust_link function to encapsulate actions taken
   when adjusting phy link during phy state changes
 * led trigger speed strings changed to match existing phy speed strings
 * New function that maps speeds to led triggers
 * Replace magic constants with definitions when declaring trigger name
   buffer and number of triggers.
v3:
 * Changed LED_ON to LED_REG_ON in skge driver to avoid possible future
   conflict and improve consistency.
 * Dropped rtl8712 patch that was accepted separately.

Josh Cartwright (1):
  phy,leds: add support for led triggers on phy link state change

Zach Brown (2):
  skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid
conflicts with leds namespace
  phy: Encapsulate actions performed during link state changes into
function phy_adjust_link

 drivers/net/ethernet/marvell/skge.c |   6 +-
 drivers/net/ethernet/marvell/skge.h |   4 +-
 drivers/net/phy/Kconfig |  13 +++-
 drivers/net/phy/Makefile|   1 +
 drivers/net/phy/phy.c   |  22 ---
 drivers/net/phy/phy_device.c|   4 ++
 drivers/net/phy/phy_led_triggers.c  | 121 
 include/linux/phy.h |   9 +++
 include/linux/phy_led_triggers.h|  52 
 9 files changed, 218 insertions(+), 14 deletions(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

--
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC v2 1/4] skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid conflicts with leds namespace

2016-09-27 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/ethernet/marvell/skge.c | 4 ++--
 drivers/net/ethernet/marvell/skge.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c 
b/drivers/net/ethernet/marvell/skge.c
index 7173836..ff5a087 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
 
 static void skge_link_down(struct skge_port *skge)
 {
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
netif_carrier_off(skge->netdev);
netif_stop_queue(skge->netdev);
 
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
if (hw->ports == 1)
free_irq(hw->pdev->irq, hw);
 
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
if (is_genesis(hw))
genesis_stop(skge);
else
diff --git a/drivers/net/ethernet/marvell/skge.h 
b/drivers/net/ethernet/marvell/skge.h
index a2eb341..ec054d3 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -663,7 +663,7 @@ enum {
LED_SYNC_ON = 1<<3, /* Use Sync Wire to switch LED */
LED_SYNC_OFF= 1<<2, /* Disable Sync Wire Input */
LED_ON  = 1<<1, /* switch LED on */
-   LED_OFF = 1<<0, /* switch LED off */
+   LED_REG_OFF = 1<<0, /* switch LED off */
 };
 
 /* Receive GMAC FIFO (YUKON) */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC v2 2/4] staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid conflicts with LED namespace

2016-09-27 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The rtl871 driver declared an enum for representing LED states. The enum
contains constant LED_OFF which conflicted with declaration found in
linux/leds.h. LED_OFF changed to LED_STATE_OFF
In order to avoid a possible future collision LED_ON was changed to
LED_STATE_ON as well.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/staging/rtl8712/rtl8712_led.c | 388 +-
 1 file changed, 194 insertions(+), 194 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl8712_led.c 
b/drivers/staging/rtl8712/rtl8712_led.c
index 9055827..8a524ac 100644
--- a/drivers/staging/rtl8712/rtl8712_led.c
+++ b/drivers/staging/rtl8712/rtl8712_led.c
@@ -51,8 +51,8 @@
  */
 enum _LED_STATE_871x {
LED_UNKNOWN = 0,
-   LED_ON = 1,
-   LED_OFF = 2,
+   LED_STATE_ON = 1,
+   LED_STATE_OFF = 2,
LED_BLINK_NORMAL = 3,
LED_BLINK_SLOWLY = 4,
LED_POWER_ON_BLINK = 5,
@@ -92,7 +92,7 @@ static void InitLed871x(struct _adapter *padapter, struct 
LED_871x *pLed,
nic = padapter->pnetdev;
pLed->padapter = padapter;
pLed->LedPin = LedPin;
-   pLed->CurrLedState = LED_OFF;
+   pLed->CurrLedState = LED_STATE_OFF;
pLed->bLedOn = false;
pLed->bLedBlinkInProgress = false;
pLed->BlinkTimes = 0;
@@ -208,7 +208,7 @@ static void SwLedBlink(struct LED_871x *pLed)
u8 bStopBlinking = false;
 
/* Change LED according to BlinkingLedState specified. */
-   if (pLed->BlinkingLedState == LED_ON)
+   if (pLed->BlinkingLedState == LED_STATE_ON)
SwLedOn(padapter, pLed);
else
SwLedOff(padapter, pLed);
@@ -248,10 +248,10 @@ static void SwLedBlink(struct LED_871x *pLed)
pLed->bLedBlinkInProgress = false;
} else {
/* Assign LED state to toggle. */
-   if (pLed->BlinkingLedState == LED_ON)
-   pLed->BlinkingLedState = LED_OFF;
+   if (pLed->BlinkingLedState == LED_STATE_ON)
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
-   pLed->BlinkingLedState = LED_ON;
+   pLed->BlinkingLedState = LED_STATE_ON;
 
/* Schedule a timer to toggle LED state. */
switch (pLed->CurrLedState) {
@@ -288,7 +288,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
if (peeprompriv->CustomerID == RT_CID_819x_CAMEO)
pLed = &(ledpriv->SwLed1);
/* Change LED according to BlinkingLedState specified. */
-   if (pLed->BlinkingLedState == LED_ON)
+   if (pLed->BlinkingLedState == LED_STATE_ON)
SwLedOn(padapter, pLed);
else
SwLedOff(padapter, pLed);
@@ -312,17 +312,17 @@ static void SwLedBlink1(struct LED_871x *pLed)
switch (pLed->CurrLedState) {
case LED_BLINK_SLOWLY:
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
-   pLed->BlinkingLedState = LED_ON;
+   pLed->BlinkingLedState = LED_STATE_ON;
mod_timer(>BlinkTimer, jiffies +
  msecs_to_jiffies(LED_BLINK_NO_LINK_INTERVAL_ALPHA));
break;
case LED_BLINK_NORMAL:
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
-   pLed->BlinkingLedState = LED_ON;
+   pLed->BlinkingLedState = LED_STATE_ON;
mod_timer(>BlinkTimer, jiffies +
  msecs_to_jiffies(LED_BLINK_LINK_INTERVAL_ALPHA));
break;
@@ -335,27 +335,27 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLed->bLedLinkBlinkInProgress = true;
pLed->CurrLedState = LED_BLINK_NORMAL;
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
-   pLed->BlinkingLedState = LED_ON;
+   pLed->BlinkingLedState = LED_STATE_ON;
mod_timer(>BlinkTimer, jiffies +
  
msecs_to_jiffies(LED_BLINK_LINK_INTERVAL_ALPHA));
} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
pLed->bLedNoLinkBlinkInProgress = true;

[RFC v2 4/4] phy, leds: add support for led triggers on phy link state change

2016-09-27 Thread Zach Brown 
From: Josh Cartwright <josh.cartwri...@ni.com>

Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
create a set of led triggers for each instantiated PHY device.  There is
one LED trigger per link-speed, per-phy.

This allows for a user to configure their system to allow a set of LEDs
to represent link state changes on the phy.

Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/Kconfig|  13 +++-
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/phy.c  |   1 +
 drivers/net/phy/phy_device.c   |   4 ++
 drivers/net/phy/phy_led_triggers.c | 121 +
 include/linux/phy.h|   9 +++
 include/linux/phy_led_triggers.h   |  52 
 7 files changed, 200 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1c3e07c..f233625 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -25,6 +25,18 @@ config MDIO_BCM_IPROC
  This module provides a driver for the MDIO busses found in the
  Broadcom iProc SoC's.
 
+config LED_TRIGGER_PHY
+   bool "Support LED triggers for tracking link state"
+   depends on LEDS_TRIGGERS
+   ---help---
+ Adds support for a set of LED trigger events per-PHY.  Link
+ state change will trigger the events, for consumption by an
+ LED class driver.  There are triggers for each link speed,
+ and are of the form:
+  ::
+
+ Where speed is one of: 10Mbps, 100Mbps, 1Gbps, 2.5Gbps, or 10Gbps.
+
 config MDIO_BCM_UNIMAC
tristate "Broadcom UniMAC MDIO bus controller"
depends on HAS_IOMEM
@@ -40,7 +52,6 @@ config MDIO_BITBANG
  This module implements the MDIO bus protocol in software,
  for use by low level drivers that export the ability to
  drive the relevant pins.
-
  If in doubt, say N.
 
 config MDIO_BUS_MUX
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
 
 libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY) += swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)   += libphy.o
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f5721db..e5f9fee7 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -896,6 +896,7 @@ EXPORT_SYMBOL(phy_start);
 static void phy_adjust_link(struct phy_device *phydev)
 {
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
 }
 
 /**
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba9..4671c13 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -57,6 +58,7 @@ static void phy_mdio_device_free(struct mdio_device *mdiodev)
 
 static void phy_device_release(struct device *dev)
 {
+   phy_led_triggers_unregister(to_phy_device(dev));
kfree(to_phy_device(dev));
 }
 
@@ -345,6 +347,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, 
int addr, int phy_id,
 
dev->state = PHY_DOWN;
 
+   phy_led_triggers_register(dev);
+
mutex_init(>lock);
INIT_DELAYED_WORK(>state_queue, phy_state_machine);
INIT_WORK(>phy_queue, phy_change);
diff --git a/drivers/net/phy/phy_led_triggers.c 
b/drivers/net/phy/phy_led_triggers.c
new file mode 100644
index 000..32326d7
--- /dev/null
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -0,0 +1,121 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+  * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include 
+#include 
+#include 
+
+static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy,
+   unsigned int speed)
+{
+   switch (speed) {
+   case SPEED_10:
+   return >phy_led_trigger[0];
+   case SPEED_100:
+   return >phy_led_trigger[1];
+   case SPEED_1000:
+   return

[RFC v2 0/4] Add support for led triggers on phy link state change

2016-09-27 Thread Zach Brown 
Fix two net drivers that declared enum constants that conflict with enum
constants in linux/leds.h

Create function that encapsulates actions taken during the adjust phy link step
of phy state changes.

Add support for led triggers on phy link state changes by adding
a config option. When set the config option will create a set of led triggers
for each phy device. Users can use the led triggers to represent link state
changes on the phy.

v2:
 * New patch that creates phy_adjust_link function to encapsulate actions taken
   when adjusting phy link during phy state changes
 * led trigger speed strings changed to match existing phy speed strings
 * New function that maps speeds to led triggers
 * Replace magic constants with definitions when declaring trigger name buffer
   and number of triggers.

Josh Cartwright (1):
  phy,leds: add support for led triggers on phy link state change

Zach Brown (3):
  skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid
conflicts with leds namespace
  staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid
conflicts with LED namespace
  phy: Encapsulate actions performed during link state changes into
function phy_adjust_link

 drivers/net/ethernet/marvell/skge.c   |   4 +-
 drivers/net/ethernet/marvell/skge.h   |   2 +-
 drivers/net/phy/Kconfig   |  13 +-
 drivers/net/phy/Makefile  |   1 +
 drivers/net/phy/phy.c |  22 +-
 drivers/net/phy/phy_device.c  |   4 +
 drivers/net/phy/phy_led_triggers.c| 121 +++
 drivers/staging/rtl8712/rtl8712_led.c | 388 +-
 include/linux/phy.h   |   9 +
 include/linux/phy_led_triggers.h  |  52 +
 10 files changed, 410 insertions(+), 206 deletions(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

--
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC v2 3/4] phy: Encapsulate actions performed during link state changes into function phy_adjust_link

2016-09-27 Thread Zach Brown 
During phy state machine state transitions some set of actions should
occur whenever the link state changes. These actions should be
encapsulated into a single function.

This patch adds the phy_adjust_link function, which is called whenever
phydev->adjust_link would have been called before. Actions that should
occur whenever the phy link is adjusted can now be added to the
phy_adjust_link function.

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/phy.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..f5721db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -893,6 +893,11 @@ void phy_start(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_start);
 
+static void phy_adjust_link(struct phy_device *phydev)
+{
+   phydev->adjust_link(phydev->attached_dev);
+}
+
 /**
  * phy_state_machine - Handle the state machine
  * @work: work_struct that describes the work to be done
@@ -935,7 +940,7 @@ void phy_state_machine(struct work_struct *work)
if (!phydev->link) {
phydev->state = PHY_NOLINK;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
}
 
@@ -948,7 +953,7 @@ void phy_state_machine(struct work_struct *work)
if (err > 0) {
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
} else if (0 == phydev->link_timeout--)
needs_aneg = true;
@@ -975,7 +980,7 @@ void phy_state_machine(struct work_struct *work)
}
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
case PHY_FORCING:
@@ -991,7 +996,7 @@ void phy_state_machine(struct work_struct *work)
needs_aneg = true;
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
break;
case PHY_RUNNING:
/* Only register a CHANGE if we are polling and link changed
@@ -1020,7 +1025,7 @@ void phy_state_machine(struct work_struct *work)
netif_carrier_off(phydev->attached_dev);
}
 
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
 
if (phy_interrupt_is_valid(phydev))
err = phy_config_interrupt(phydev,
@@ -1030,7 +1035,7 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
do_suspend = true;
}
break;
@@ -1054,7 +1059,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
} else {
phydev->state = PHY_AN;
phydev->link_timeout = PHY_AN_TIMEOUT;
@@ -1070,7 +1075,7 @@ void phy_state_machine(struct work_struct *work)
} else  {
phydev->state = PHY_NOLINK;
}
-   phydev->adjust_link(phydev->attached_dev);
+   phy_adjust_link(phydev);
}
break;
}
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC 3/3] phy,leds: add support for led triggers on phy link state change

2016-09-19 Thread Zach Brown 
On Wed, Sep 14, 2016 at 04:16:15PM -0700, Florian Fainelli wrote:
> On 09/14/2016 02:55 PM, Zach Brown wrote:
> > From: Josh Cartwright <josh.cartwri...@ni.com>
> > 
> > Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
> > create a set of led triggers for each instantiated PHY device.  There is
> > one LED trigger per link-speed, per-phy.
> > 
> > This allows for a user to configure their system to allow a set of LEDs
> > to represent link state changes on the phy.
> 
> The part that seems to be missing from this changeset (not an issue) is
> how you can "accelerate" the triggers, or rather make sure that the
> trigger configuration potentially calls back into the PHY driver when
> the requested trigger is actually supported by the on-PHY LEDs.
> 
> Other than that, just minor nits here and there.
> 
> > 
> > Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
> > Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
> > Signed-off-by: Zach Brown <zach.br...@ni.com>
> > ---
> 
> > +config LED_TRIGGER_PHY
> > +   bool "Support LED triggers for tracking link state"
> > +   depends on LEDS_TRIGGERS
> > +   ---help---
> > + Adds support for a set of LED trigger events per-PHY.  Link
> > + state change will trigger the events, for consumption by an
> > + LED class driver.  There are triggers for each link speed,
> > + and are of the form:
> > +  ::
> > +
> > + Where speed is one of: 10Mb, 100Mb, Gb, 2.5Gb, or 10GbE.
> 
> I would do s/10GbE/10Gb/ just to be consistent with the other speeds, or
> maybe, to avoid too much duplicationg of how we represent speeds, use
> the same set of strings that drivers/net/phy/phy.c::phy_speed_to_str uses.
> 
> 
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index c6f6683..3345767 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -936,6 +936,7 @@ void phy_state_machine(struct work_struct *work)
> > phydev->state = PHY_NOLINK;
> > netif_carrier_off(phydev->attached_dev);
> > phydev->adjust_link(phydev->attached_dev);
> > +   phy_led_trigger_change_speed(phydev);
> 
> Since we have essentially two actions to take when performing link state
> changes:
> 
> - call the adjust_link callback
> - call into the LED trigger
> 
> we might consider encapsulating this into a function that could be named
> phy_adjust_link() and does both of these. That would be a preliminary
> patch to this this one.
> 
> >  
> > @@ -345,6 +347,8 @@ struct phy_device *phy_device_create(struct mii_bus 
> > *bus, int addr, int phy_id,
> >  
> > dev->state = PHY_DOWN;
> >  
> > +   phy_led_triggers_register(dev);
> > +
> > mutex_init(>lock);
> > INIT_DELAYED_WORK(>state_queue, phy_state_machine);
> > INIT_WORK(>phy_queue, phy_change);
> 
> Humm, should it be before the PHY state machine workqueue creation or
> after? Just wondering if there is a remote chance we could get an user
> to immediately program a trigger and this could create a problem, maybe
> not so much.
> 
I'm not sure, but I don't think there would be an issue since the interaction
between the triggers and the phy system only goes in one direction. The
triggers don't influence the phy system to my understanding.

> > diff --git a/drivers/net/phy/phy_led_triggers.c 
> > b/drivers/net/phy/phy_led_triggers.c
> > new file mode 100644
> > index 000..6c40414
> > --- /dev/null
> > +++ b/drivers/net/phy/phy_led_triggers.c
> > @@ -0,0 +1,109 @@
> > +/* Copyright (C) 2016 National Instruments Corp.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +#include 
> > +#include 
> > +
> > +void phy_led_trigger_change_speed(struct phy_device *phy)
> > +{
> > +   struct phy_led_trigger *plt;
> > +
> > +   if (!phy->link) {
> > +   if (phy->last_tri

[RFC 2/3] staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid conflicts with LED namespace

2016-09-14 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The rtl871 driver declared an enum for representing LED states. The enum
contains constant LED_OFF which conflicted with declartation found in
linux/leds.h. LED_OFF changed to LED_STATE_OFF

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/staging/rtl8712/rtl8712_led.c | 192 +-
 1 file changed, 96 insertions(+), 96 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl8712_led.c 
b/drivers/staging/rtl8712/rtl8712_led.c
index 9055827..d53ad76 100644
--- a/drivers/staging/rtl8712/rtl8712_led.c
+++ b/drivers/staging/rtl8712/rtl8712_led.c
@@ -52,7 +52,7 @@
 enum _LED_STATE_871x {
LED_UNKNOWN = 0,
LED_ON = 1,
-   LED_OFF = 2,
+   LED_STATE_OFF = 2,
LED_BLINK_NORMAL = 3,
LED_BLINK_SLOWLY = 4,
LED_POWER_ON_BLINK = 5,
@@ -92,7 +92,7 @@ static void InitLed871x(struct _adapter *padapter, struct 
LED_871x *pLed,
nic = padapter->pnetdev;
pLed->padapter = padapter;
pLed->LedPin = LedPin;
-   pLed->CurrLedState = LED_OFF;
+   pLed->CurrLedState = LED_STATE_OFF;
pLed->bLedOn = false;
pLed->bLedBlinkInProgress = false;
pLed->BlinkTimes = 0;
@@ -249,7 +249,7 @@ static void SwLedBlink(struct LED_871x *pLed)
} else {
/* Assign LED state to toggle. */
if (pLed->BlinkingLedState == LED_ON)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
 
@@ -312,7 +312,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
switch (pLed->CurrLedState) {
case LED_BLINK_SLOWLY:
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
mod_timer(>BlinkTimer, jiffies +
@@ -320,7 +320,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
break;
case LED_BLINK_NORMAL:
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
mod_timer(>BlinkTimer, jiffies +
@@ -335,7 +335,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLed->bLedLinkBlinkInProgress = true;
pLed->CurrLedState = LED_BLINK_NORMAL;
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
mod_timer(>BlinkTimer, jiffies +
@@ -344,7 +344,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLed->bLedNoLinkBlinkInProgress = true;
pLed->CurrLedState = LED_BLINK_SLOWLY;
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
mod_timer(>BlinkTimer, jiffies +
@@ -353,7 +353,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLed->bLedScanBlinkInProgress = false;
} else {
 if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
mod_timer(>BlinkTimer, jiffies +
@@ -369,7 +369,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLed->bLedLinkBlinkInProgress = true;
pLed->CurrLedState = LED_BLINK_NORMAL;
if (pLed->bLedOn)
-   pLed->BlinkingLedState = LED_OFF;
+   pLed->BlinkingLedState = LED_STATE_OFF;
else
pLed->BlinkingLedState = LED_ON;
mod_timer(>BlinkTimer, jiffies +
@@ -378,7 +378,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
pLe

[RFC 3/3] phy, leds: add support for led triggers on phy link state change

2016-09-14 Thread Zach Brown 
From: Josh Cartwright <josh.cartwri...@ni.com>

Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
create a set of led triggers for each instantiated PHY device.  There is
one LED trigger per link-speed, per-phy.

This allows for a user to configure their system to allow a set of LEDs
to represent link state changes on the phy.

Signed-off-by: Josh Cartwright <josh.cartwri...@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sulli...@ni.com>
Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/phy/Kconfig|  12 
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/phy.c  |   8 +++
 drivers/net/phy/phy_device.c   |   4 ++
 drivers/net/phy/phy_led_triggers.c | 109 +
 include/linux/phy.h|   9 +++
 include/linux/phy_led_triggers.h   |  42 ++
 7 files changed, 185 insertions(+)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1c3e07c..4aeaba4 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -15,6 +15,18 @@ if PHYLIB
 config SWPHY
bool
 
+config LED_TRIGGER_PHY
+   bool "Support LED triggers for tracking link state"
+   depends on LEDS_TRIGGERS
+   ---help---
+ Adds support for a set of LED trigger events per-PHY.  Link
+ state change will trigger the events, for consumption by an
+ LED class driver.  There are triggers for each link speed,
+ and are of the form:
+  ::
+
+ Where speed is one of: 10Mb, 100Mb, Gb, 2.5Gb, or 10GbE.
+
 comment "MDIO bus device drivers"
 
 config MDIO_BCM_IPROC
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
 
 libphy-y   := phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY) += swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY)   += phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)   += libphy.o
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..3345767 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -936,6 +936,7 @@ void phy_state_machine(struct work_struct *work)
phydev->state = PHY_NOLINK;
netif_carrier_off(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
break;
}
 
@@ -949,6 +950,7 @@ void phy_state_machine(struct work_struct *work)
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
 
} else if (0 == phydev->link_timeout--)
needs_aneg = true;
@@ -976,6 +978,7 @@ void phy_state_machine(struct work_struct *work)
phydev->state = PHY_RUNNING;
netif_carrier_on(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
}
break;
case PHY_FORCING:
@@ -992,6 +995,7 @@ void phy_state_machine(struct work_struct *work)
}
 
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
break;
case PHY_RUNNING:
/* Only register a CHANGE if we are polling and link changed
@@ -1021,6 +1025,7 @@ void phy_state_machine(struct work_struct *work)
}
 
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
 
if (phy_interrupt_is_valid(phydev))
err = phy_config_interrupt(phydev,
@@ -1031,6 +1036,7 @@ void phy_state_machine(struct work_struct *work)
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
do_suspend = true;
}
break;
@@ -1055,6 +1061,7 @@ void phy_state_machine(struct work_struct *work)
phydev->state = PHY_NOLINK;
}
phydev->adjust_link(phydev->attached_dev);
+   phy_led_trigger_change_speed(phydev);
} else {

[RFC 1/3] skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid conflicts with leds namespace

2016-09-14 Thread Zach Brown 
Adding led support for phy causes namespace conflicts for some
phy drivers.

The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF

Signed-off-by: Zach Brown <zach.br...@ni.com>
---
 drivers/net/ethernet/marvell/skge.c | 4 ++--
 drivers/net/ethernet/marvell/skge.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c 
b/drivers/net/ethernet/marvell/skge.c
index 7173836..ff5a087 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
 
 static void skge_link_down(struct skge_port *skge)
 {
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
netif_carrier_off(skge->netdev);
netif_stop_queue(skge->netdev);
 
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
if (hw->ports == 1)
free_irq(hw->pdev->irq, hw);
 
-   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+   skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
if (is_genesis(hw))
genesis_stop(skge);
else
diff --git a/drivers/net/ethernet/marvell/skge.h 
b/drivers/net/ethernet/marvell/skge.h
index a2eb341..ec054d3 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -663,7 +663,7 @@ enum {
LED_SYNC_ON = 1<<3, /* Use Sync Wire to switch LED */
LED_SYNC_OFF= 1<<2, /* Disable Sync Wire Input */
LED_ON  = 1<<1, /* switch LED on */
-   LED_OFF = 1<<0, /* switch LED off */
+   LED_REG_OFF = 1<<0, /* switch LED off */
 };
 
 /* Receive GMAC FIFO (YUKON) */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC 0/3] Add support for led triggers on phy link state change

2016-09-14 Thread Zach Brown 
Some drivers that include phy.h defined LED_OFF which conflicts with
definition in leds.h. phy led support uses leds.h so the two namespaces are no
longer isolated.
The first two patches fix the two net drivers that declared enum constants that
conflict with enum constants in linux/leds.h.

The final patch adds support for led triggers on phy link state changes by
adding a config option. When set the config option will create a set of led
triggers for each phy device. Users can use the led triggers to represent link
state changes on the phy.
The changes assumes that there are 5 speed options
10Mb,100Mb,1Gb,2.5Gb,10Gb
The assumption makes mapping a phy_device's current speed to a trigger easy,
but means there are triggers made that aren't used if the phy doesn't support
the corresponding speeds.
Thoughts on how to better manage the triggers created would be appreciated
if is important to do so.

Josh Cartwright (1):
  phy,leds: add support for led triggers on phy link state change

Zach Brown (2):
  skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid
conflicts with leds namespace
  staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid
conflicts with LED namespace

 drivers/net/ethernet/marvell/skge.c   |   4 +-
 drivers/net/ethernet/marvell/skge.h   |   2 +-
 drivers/net/phy/Kconfig   |  12 +++
 drivers/net/phy/Makefile  |   1 +
 drivers/net/phy/phy.c |   8 ++
 drivers/net/phy/phy_device.c  |   4 +
 drivers/net/phy/phy_led_triggers.c| 109 +++
 drivers/staging/rtl8712/rtl8712_led.c | 192 +-
 include/linux/phy.h   |   9 ++
 include/linux/phy_led_triggers.h  |  42 
 10 files changed, 284 insertions(+), 99 deletions(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

--
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel