This implements the led_hw_* hooks to support hardware blinking LEDs on
the DP83867 phy. The driver supports all LED modes that have a
corresponding TRIGGER_NETDEV_* define. Error and collision do not have
a TRIGGER_NETDEV_* define, so these modes are currently not supported.
Signed-off-by: Sascha Hauer <[email protected]>
---
drivers/net/phy/dp83867.c | 137 ++++++++++++++++++++++++++++++++++++++
1 file changed, 137 insertions(+)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index e397e7d642d92..5f08f9d38bd7a 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -159,6 +159,23 @@
#define DP83867_LED_DRV_EN(x) BIT((x) * 4)
#define DP83867_LED_DRV_VAL(x) BIT((x) * 4 + 1)
+#define DP83867_LED_FN(idx, val) (((val) & 0xf) << ((idx) * 4))
+#define DP83867_LED_FN_MASK(idx) (0xf << ((idx) * 4))
+#define DP83867_LED_FN_RX_ERR 0xe /* Receive Error */
+#define DP83867_LED_FN_RX_TX_ERR 0xd /* Receive Error or Transmit Error */
+#define DP83867_LED_FN_LINK_RX_TX 0xb /* Link established, blink for rx or tx activity */
+#define DP83867_LED_FN_FULL_DUPLEX 0xa /* Full duplex */
+#define DP83867_LED_FN_LINK_100_1000_BT 0x9 /* 100/1000BT link established */
+#define DP83867_LED_FN_LINK_10_100_BT 0x8 /* 10/100BT link established */
+#define DP83867_LED_FN_LINK_10_BT 0x7 /* 10BT link established */
+#define DP83867_LED_FN_LINK_100_BTX 0x6 /* 100 BTX link established */
+#define DP83867_LED_FN_LINK_1000_BT 0x5 /* 1000 BT link established */
+#define DP83867_LED_FN_COLLISION 0x4 /* Collision detected */
+#define DP83867_LED_FN_RX 0x3 /* Receive activity */
+#define DP83867_LED_FN_TX 0x2 /* Transmit activity */
+#define DP83867_LED_FN_RX_TX 0x1 /* Receive or Transmit activity */
+#define DP83867_LED_FN_LINK 0x0 /* Link established */
+
enum {
DP83867_PORT_MIRROING_KEEP,
DP83867_PORT_MIRROING_EN,
@@ -1018,6 +1035,123 @@ dp83867_led_brightness_set(struct phy_device *phydev,
val);
}
+static int dp83867_led_mode(u8 index, unsigned long rules)
+{
+ if (index >= DP83867_LED_COUNT)
+ return -EINVAL;
+
+ switch (rules) {
+ case BIT(TRIGGER_NETDEV_LINK):
+ return DP83867_LED_FN_LINK;
+ case BIT(TRIGGER_NETDEV_LINK_10):
+ return DP83867_LED_FN_LINK_10_BT;
+ case BIT(TRIGGER_NETDEV_LINK_100):
+ return DP83867_LED_FN_LINK_100_BTX;
+ case BIT(TRIGGER_NETDEV_FULL_DUPLEX):
+ return DP83867_LED_FN_FULL_DUPLEX;
+ case BIT(TRIGGER_NETDEV_TX):
+ return DP83867_LED_FN_TX;
+ case BIT(TRIGGER_NETDEV_RX):
+ return DP83867_LED_FN_RX;
+ case BIT(TRIGGER_NETDEV_LINK_1000):
+ return DP83867_LED_FN_LINK_1000_BT;
+ case BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_RX):
+ return DP83867_LED_FN_RX_TX;
+ case BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_LINK_1000):
+ return DP83867_LED_FN_LINK_100_1000_BT;
+ case BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_LINK_100):
+ return DP83867_LED_FN_LINK_10_100_BT;
+ case BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_RX):
+ return DP83867_LED_FN_LINK_RX_TX;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int dp83867_led_hw_is_supported(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ int ret;
+
+ ret = dp83867_led_mode(index, rules);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int dp83867_led_hw_control_set(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ int mode, ret;
+
+ mode = dp83867_led_mode(index, rules);
+ if (mode < 0)
+ return mode;
+
+ ret = phy_modify(phydev, DP83867_LEDCR1, DP83867_LED_FN_MASK(index),
+ DP83867_LED_FN(index, mode));
+ if (ret)
+ return ret;
+
+ return phy_modify(phydev, DP83867_LEDCR2, DP83867_LED_DRV_EN(index), 0);
+}
+
+static int dp83867_led_hw_control_get(struct phy_device *phydev, u8 index,
+ unsigned long *rules)
+{
+ int val;
+
+ val = phy_read(phydev, DP83867_LEDCR1);
+ if (val < 0)
+ return val;
+
+ val &= DP83867_LED_FN_MASK(index);
+ val >>= index * 4;
+
+ switch (val) {
+ case DP83867_LED_FN_LINK:
+ *rules = BIT(TRIGGER_NETDEV_LINK);
+ break;
+ case DP83867_LED_FN_LINK_10_BT:
+ *rules = BIT(TRIGGER_NETDEV_LINK_10);
+ break;
+ case DP83867_LED_FN_LINK_100_BTX:
+ *rules = BIT(TRIGGER_NETDEV_LINK_100);
+ break;
+ case DP83867_LED_FN_FULL_DUPLEX:
+ *rules = BIT(TRIGGER_NETDEV_FULL_DUPLEX);
+ break;
+ case DP83867_LED_FN_TX:
+ *rules = BIT(TRIGGER_NETDEV_TX);
+ break;
+ case DP83867_LED_FN_RX:
+ *rules = BIT(TRIGGER_NETDEV_RX);
+ break;
+ case DP83867_LED_FN_LINK_1000_BT:
+ *rules = BIT(TRIGGER_NETDEV_LINK_1000);
+ break;
+ case DP83867_LED_FN_RX_TX:
+ *rules = BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_RX);
+ break;
+ case DP83867_LED_FN_LINK_100_1000_BT:
+ *rules = BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_LINK_1000);
+ break;
+ case DP83867_LED_FN_LINK_10_100_BT:
+ *rules = BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_LINK_100);
+ break;
+ case DP83867_LED_FN_LINK_RX_TX:
+ *rules = BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_TX) |
+ BIT(TRIGGER_NETDEV_RX);
+ break;
+ default:
+ *rules = 0;
+ break;
+ }
+
+ return 0;
+}
+
static struct phy_driver dp83867_driver[] = {
{
.phy_id = DP83867_PHY_ID,
@@ -1047,6 +1181,9 @@ static struct phy_driver dp83867_driver[] = {
.set_loopback = dp83867_loopback,
.led_brightness_set = dp83867_led_brightness_set,
+ .led_hw_is_supported = dp83867_led_hw_is_supported,
+ .led_hw_control_set = dp83867_led_hw_control_set,
+ .led_hw_control_get = dp83867_led_hw_control_get,
},
};
module_phy_driver(dp83867_driver);
--
2.39.2
Hi Sascha,
thanks for the patch.
Am Donnerstag, 7. September 2023, 10:47:31 CEST schrieb Sascha Hauer:
> This implements the led_hw_* hooks to support hardware blinking LEDs on
> the DP83867 phy. The driver supports all LED modes that have a
> corresponding TRIGGER_NETDEV_* define. Error and collision do not have
> a TRIGGER_NETDEV_* define, so these modes are currently not supported.
>
> Signed-off-by: Sascha Hauer <[email protected]>
This works as intended so far. Unfortunately this driver and the PHY LED
framework do not support active-low LEDs (yet).
Tested-by: Alexander Stein <[email protected]> #TQMa8MxML/MBa8Mx
> ---
> drivers/net/phy/dp83867.c | 137 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 137 insertions(+)
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index e397e7d642d92..5f08f9d38bd7a 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -159,6 +159,23 @@
> #define DP83867_LED_DRV_EN(x) BIT((x) * 4)
> #define DP83867_LED_DRV_VAL(x) BIT((x) * 4 + 1)
>
> +#define DP83867_LED_FN(idx, val) (((val) & 0xf) << ((idx) * 4))
> +#define DP83867_LED_FN_MASK(idx) (0xf << ((idx) * 4))
> +#define DP83867_LED_FN_RX_ERR 0xe /* Receive Error */
> +#define DP83867_LED_FN_RX_TX_ERR 0xd /* Receive Error or Transmit
Error */
> +#define DP83867_LED_FN_LINK_RX_TX 0xb /* Link established, blink for rx
or
> tx activity */ +#define DP83867_LED_FN_FULL_DUPLEX 0xa /* Full
duplex */
> +#define DP83867_LED_FN_LINK_100_1000_BT 0x9 /* 100/1000BT link
established
> */ +#define DP83867_LED_FN_LINK_10_100_BT 0x8 /* 10/100BT link
established
> */ +#define DP83867_LED_FN_LINK_10_BT 0x7 /* 10BT link established */
> +#define DP83867_LED_FN_LINK_100_BTX 0x6 /* 100 BTX link established */
> +#define DP83867_LED_FN_LINK_1000_BT 0x5 /* 1000 BT link established */
> +#define DP83867_LED_FN_COLLISION 0x4 /* Collision detected */
> +#define DP83867_LED_FN_RX 0x3 /* Receive activity */
> +#define DP83867_LED_FN_TX 0x2 /* Transmit activity */
> +#define DP83867_LED_FN_RX_TX 0x1 /* Receive or Transmit
activity */
> +#define DP83867_LED_FN_LINK 0x0 /* Link established */
> +
> enum {
> DP83867_PORT_MIRROING_KEEP,
> DP83867_PORT_MIRROING_EN,
> @@ -1018,6 +1035,123 @@ dp83867_led_brightness_set(struct phy_device
> *phydev, val);
> }
>
> +static int dp83867_led_mode(u8 index, unsigned long rules)
> +{
> + if (index >= DP83867_LED_COUNT)
> + return -EINVAL;
> +
> + switch (rules) {
> + case BIT(TRIGGER_NETDEV_LINK):
> + return DP83867_LED_FN_LINK;
> + case BIT(TRIGGER_NETDEV_LINK_10):
> + return DP83867_LED_FN_LINK_10_BT;
> + case BIT(TRIGGER_NETDEV_LINK_100):
> + return DP83867_LED_FN_LINK_100_BTX;
> + case BIT(TRIGGER_NETDEV_FULL_DUPLEX):
> + return DP83867_LED_FN_FULL_DUPLEX;
> + case BIT(TRIGGER_NETDEV_TX):
> + return DP83867_LED_FN_TX;
> + case BIT(TRIGGER_NETDEV_RX):
> + return DP83867_LED_FN_RX;
> + case BIT(TRIGGER_NETDEV_LINK_1000):
> + return DP83867_LED_FN_LINK_1000_BT;
> + case BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_RX):
> + return DP83867_LED_FN_RX_TX;
> + case BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_LINK_1000):
> + return DP83867_LED_FN_LINK_100_1000_BT;
> + case BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_LINK_100):
> + return DP83867_LED_FN_LINK_10_100_BT;
> + case BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_TX) |
> BIT(TRIGGER_NETDEV_RX): + return DP83867_LED_FN_LINK_RX_TX;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int dp83867_led_hw_is_supported(struct phy_device *phydev, u8 index,
> + unsigned long rules)
> +{
> + int ret;
> +
> + ret = dp83867_led_mode(index, rules);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int dp83867_led_hw_control_set(struct phy_device *phydev, u8 index,
> + unsigned long rules)
> +{
> + int mode, ret;
> +
> + mode = dp83867_led_mode(index, rules);
> + if (mode < 0)
> + return mode;
> +
> + ret = phy_modify(phydev, DP83867_LEDCR1, DP83867_LED_FN_MASK(index),
> + DP83867_LED_FN(index, mode));
> + if (ret)
> + return ret;
> +
> + return phy_modify(phydev, DP83867_LEDCR2, DP83867_LED_DRV_EN(index),
0);
> +}
> +
> +static int dp83867_led_hw_control_get(struct phy_device *phydev, u8 index,
> + unsigned long *rules)
> +{
> + int val;
> +
> + val = phy_read(phydev, DP83867_LEDCR1);
> + if (val < 0)
> + return val;
> +
> + val &= DP83867_LED_FN_MASK(index);
> + val >>= index * 4;
> +
> + switch (val) {
> + case DP83867_LED_FN_LINK:
> + *rules = BIT(TRIGGER_NETDEV_LINK);
> + break;
> + case DP83867_LED_FN_LINK_10_BT:
> + *rules = BIT(TRIGGER_NETDEV_LINK_10);
> + break;
> + case DP83867_LED_FN_LINK_100_BTX:
> + *rules = BIT(TRIGGER_NETDEV_LINK_100);
> + break;
> + case DP83867_LED_FN_FULL_DUPLEX:
> + *rules = BIT(TRIGGER_NETDEV_FULL_DUPLEX);
> + break;
> + case DP83867_LED_FN_TX:
> + *rules = BIT(TRIGGER_NETDEV_TX);
> + break;
> + case DP83867_LED_FN_RX:
> + *rules = BIT(TRIGGER_NETDEV_RX);
> + break;
> + case DP83867_LED_FN_LINK_1000_BT:
> + *rules = BIT(TRIGGER_NETDEV_LINK_1000);
> + break;
> + case DP83867_LED_FN_RX_TX:
> + *rules = BIT(TRIGGER_NETDEV_TX) | BIT(TRIGGER_NETDEV_RX);
> + break;
> + case DP83867_LED_FN_LINK_100_1000_BT:
> + *rules = BIT(TRIGGER_NETDEV_LINK_100) |
BIT(TRIGGER_NETDEV_LINK_1000);
> + break;
> + case DP83867_LED_FN_LINK_10_100_BT:
> + *rules = BIT(TRIGGER_NETDEV_LINK_10) |
BIT(TRIGGER_NETDEV_LINK_100);
> + break;
> + case DP83867_LED_FN_LINK_RX_TX:
> + *rules = BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_TX)
|
> + BIT(TRIGGER_NETDEV_RX);
> + break;
> + default:
> + *rules = 0;
> + break;
> + }
> +
> + return 0;
> +}
> +
> static struct phy_driver dp83867_driver[] = {
> {
> .phy_id = DP83867_PHY_ID,
> @@ -1047,6 +1181,9 @@ static struct phy_driver dp83867_driver[] = {
> .set_loopback = dp83867_loopback,
>
> .led_brightness_set = dp83867_led_brightness_set,
> + .led_hw_is_supported = dp83867_led_hw_is_supported,
> + .led_hw_control_set = dp83867_led_hw_control_set,
> + .led_hw_control_get = dp83867_led_hw_control_get,
> },
> };
> module_phy_driver(dp83867_driver);
--
TQ-Systems GmbH | M?hlstra?e 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht M?nchen, HRB 105018
Gesch?ftsf?hrer: Detlef Schneider, R?diger Stahl, Stefan Schneider
http://www.tq-group.com/
On Sat, Sep 09, 2023 at 04:27:31AM +0200, Andrew Lunn wrote:
> > This works as intended so far. Unfortunately this driver and the PHY LED
> > framework do not support active-low LEDs (yet).
>
> Polarity is something which i've seen a few PHY devices have. It also
> seems like a core LED concept, not something specific to PHY LEDs. So
> i think this needs to be partially addressed in the LED core.
However, doesn't the LED layer deals with LED brightness, not by logic
state? It certainly looks that way, and it's left up to the drivers
themselves to deal with any polarity inversion - which makes sense if
the core is just concerned about brightness.
Introducing inversion in the core means drivers will be passed a
brightness of "100" for off and "0" for on which, do you not think,
starts to get rather silly?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, 7 Sep 2023 10:47:31 +0200 Sascha Hauer wrote:
> This implements the led_hw_* hooks to support hardware blinking LEDs on
> the DP83867 phy. The driver supports all LED modes that have a
> corresponding TRIGGER_NETDEV_* define. Error and collision do not have
> a TRIGGER_NETDEV_* define, so these modes are currently not supported.
FWIW I think this patch got marked as Deferred in patchwork because it
was posted mid-merge window. Could you repost?