* [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x
@ 2024-01-25 20:36 Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 1/5] dt-bindings: net: phy: Make LED active-low property common Christian Marangi
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Christian Marangi @ 2024-01-25 20:36 UTC (permalink / raw
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, Christian Marangi,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
This small series add LEDs support for qca808x.
QCA808x apply on PHY reset a strange polarity settings and require
some tweak to apply a more common configuration found on devices.
On adding support for it, it was pointed out that a similar
feature is also being implemented for a marvell PHY where
LED polarity is set per LED (and not global) and also have
a special mode where the LED is tristated.
The first 3 patch are to generalize this as we expect more PHY
in the future to have a similar configuration.
The implementation is extensible to support additional special
mode in the future with minimal changes and don't create regression
on already implemented PHY drivers.
(changelog present in single patch)
Christian Marangi (5):
dt-bindings: net: phy: Make LED active-low property common
dt-bindings: net: phy: Document LED inactive high impedance mode
net: phy: add support for PHY LEDs polarity modes
dt-bindings: net: Document QCA808x PHYs
net: phy: at803x: add LED support for qca808x
.../devicetree/bindings/leds/common.yaml | 12 +
.../bindings/leds/leds-bcm63138.yaml | 4 -
.../bindings/leds/leds-bcm6328.yaml | 4 -
.../devicetree/bindings/leds/leds-bcm6358.txt | 2 -
.../bindings/leds/leds-pwm-multicolor.yaml | 4 -
.../devicetree/bindings/leds/leds-pwm.yaml | 5 -
.../devicetree/bindings/net/qca,qca808x.yaml | 54 +++
drivers/net/phy/at803x.c | 327 ++++++++++++++++++
drivers/net/phy/phy_device.c | 16 +
include/linux/phy.h | 22 ++
10 files changed, 431 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/qca,qca808x.yaml
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [net-next PATCH v10 1/5] dt-bindings: net: phy: Make LED active-low property common
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
@ 2024-01-25 20:36 ` Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 2/5] dt-bindings: net: phy: Document LED inactive high impedance mode Christian Marangi
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Marangi @ 2024-01-25 20:36 UTC (permalink / raw
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, Christian Marangi,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
Cc: Rob Herring
Move LED active-low property to common.yaml. This property is currently
defined multiple times by bcm LEDs. This property will now be supported
in a generic way for PHY LEDs with the use of a generic function.
With active-low bool property not defined, active-high is always
assumed.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Lee Jones <lee@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes v10:
- Add Review and Ack tag
Changes v5:
- Make active-low generic
Changes v4:
- Drop support for global active-low
- Rework to polarity option (for marvell10g series support)
Changes v3:
- Out of RFC
Changes v2:
- Add this patch
Documentation/devicetree/bindings/leds/common.yaml | 6 ++++++
Documentation/devicetree/bindings/leds/leds-bcm63138.yaml | 4 ----
Documentation/devicetree/bindings/leds/leds-bcm6328.yaml | 4 ----
Documentation/devicetree/bindings/leds/leds-bcm6358.txt | 2 --
.../devicetree/bindings/leds/leds-pwm-multicolor.yaml | 4 ----
Documentation/devicetree/bindings/leds/leds-pwm.yaml | 5 -----
6 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 55a8d1385e21..5633e0aa6bdf 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -200,6 +200,12 @@ properties:
#trigger-source-cells property in the source node.
$ref: /schemas/types.yaml#/definitions/phandle-array
+ active-low:
+ type: boolean
+ description:
+ Makes LED active low. To turn the LED ON, line needs to be
+ set to low voltage instead of high.
+
# Required properties for flash LED child nodes:
flash-max-microamp:
description:
diff --git a/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml b/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml
index 52252fb6bb32..bb20394fca5c 100644
--- a/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-bcm63138.yaml
@@ -52,10 +52,6 @@ patternProperties:
maxItems: 1
description: LED pin number
- active-low:
- type: boolean
- description: Makes LED active low
-
required:
- reg
diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6328.yaml b/Documentation/devicetree/bindings/leds/leds-bcm6328.yaml
index 51cc0d82c12e..f3a3ef992929 100644
--- a/Documentation/devicetree/bindings/leds/leds-bcm6328.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6328.yaml
@@ -78,10 +78,6 @@ patternProperties:
- maximum: 23
description: LED pin number (only LEDs 0 to 23 are valid).
- active-low:
- type: boolean
- description: Makes LED active low.
-
brcm,hardware-controlled:
type: boolean
description: Makes this LED hardware controlled.
diff --git a/Documentation/devicetree/bindings/leds/leds-bcm6358.txt b/Documentation/devicetree/bindings/leds/leds-bcm6358.txt
index 6e51c6b91ee5..211ffc3c4a20 100644
--- a/Documentation/devicetree/bindings/leds/leds-bcm6358.txt
+++ b/Documentation/devicetree/bindings/leds/leds-bcm6358.txt
@@ -25,8 +25,6 @@ LED sub-node required properties:
LED sub-node optional properties:
- label : see Documentation/devicetree/bindings/leds/common.txt
- - active-low : Boolean, makes LED active low.
- Default : false
- default-state : see
Documentation/devicetree/bindings/leds/common.txt
- linux,default-trigger : see
diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
index bd6ec04a8727..5edfbe347341 100644
--- a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
@@ -41,10 +41,6 @@ properties:
pwm-names: true
- active-low:
- description: For PWMs where the LED is wired to supply rather than ground.
- type: boolean
-
color: true
required:
diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.yaml b/Documentation/devicetree/bindings/leds/leds-pwm.yaml
index 7de6da58be3c..113b7c218303 100644
--- a/Documentation/devicetree/bindings/leds/leds-pwm.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-pwm.yaml
@@ -34,11 +34,6 @@ patternProperties:
Maximum brightness possible for the LED
$ref: /schemas/types.yaml#/definitions/uint32
- active-low:
- description:
- For PWMs where the LED is wired to supply rather than ground.
- type: boolean
-
required:
- pwms
- max-brightness
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-next PATCH v10 2/5] dt-bindings: net: phy: Document LED inactive high impedance mode
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 1/5] dt-bindings: net: phy: Make LED active-low property common Christian Marangi
@ 2024-01-25 20:36 ` Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes Christian Marangi
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Christian Marangi @ 2024-01-25 20:36 UTC (permalink / raw
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, Christian Marangi,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
Cc: Rob Herring
Document LED inactive high impedance mode to set the LED to require high
impedance configuration to be turned OFF.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Lee Jones <lee@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes v10:
- Add Review and Ack tags
Changes v5:
- Add this patch
Documentation/devicetree/bindings/leds/common.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 5633e0aa6bdf..8a3c2398b10c 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -206,6 +206,12 @@ properties:
Makes LED active low. To turn the LED ON, line needs to be
set to low voltage instead of high.
+ inactive-high-impedance:
+ type: boolean
+ description:
+ Set LED to high-impedance mode to turn the LED OFF. LED might also
+ describe this mode as tristate.
+
# Required properties for flash LED child nodes:
flash-max-microamp:
description:
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 1/5] dt-bindings: net: phy: Make LED active-low property common Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 2/5] dt-bindings: net: phy: Document LED inactive high impedance mode Christian Marangi
@ 2024-01-25 20:36 ` Christian Marangi
2024-05-10 23:14 ` Daniel Golle
2024-01-25 20:37 ` [net-next PATCH v10 4/5] dt-bindings: net: Document QCA808x PHYs Christian Marangi
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Christian Marangi @ 2024-01-25 20:36 UTC (permalink / raw
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, Christian Marangi,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
Add support for PHY LEDs polarity modes. Some PHY require LED to be set
to active low to be turned ON. Adds support for this by declaring
active-low property in DT.
PHY driver needs to declare .led_polarity_set() to configure LED
polarity modes. Function will pass the index with the LED index and a
bitmap with all the required modes to set.
Current supported modes are:
- active-low with the flag PHY_LED_ACTIVE_LOW. LED is set to active-low
to turn it ON.
- inactive-high-impedance with the flag PHY_LED_INACTIVE_HIGH_IMPEDANCE.
LED is set to high impedance to turn it OFF.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes v10:
- Add Review tag
Changes v9:
- Make LED probe fail if modes asked but not supported
- Fix wrong function description
Changes v5:
- Rework to LED modes bitmap
Changes v4:
- Drop for global active-low
- Rework to polarity option (for marvell10g series support)
Changes v3:
- Out of RFC
Changes v2:
- Add this patch
drivers/net/phy/phy_device.c | 16 ++++++++++++++++
include/linux/phy.h | 22 ++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3611ea64875e..dd778c7fde1d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3097,6 +3097,7 @@ static int of_phy_led(struct phy_device *phydev,
struct device *dev = &phydev->mdio.dev;
struct led_init_data init_data = {};
struct led_classdev *cdev;
+ unsigned long modes = 0;
struct phy_led *phyled;
u32 index;
int err;
@@ -3114,6 +3115,21 @@ static int of_phy_led(struct phy_device *phydev,
if (index > U8_MAX)
return -EINVAL;
+ if (of_property_read_bool(led, "active-low"))
+ set_bit(PHY_LED_ACTIVE_LOW, &modes);
+ if (of_property_read_bool(led, "inactive-high-impedance"))
+ set_bit(PHY_LED_INACTIVE_HIGH_IMPEDANCE, &modes);
+
+ if (modes) {
+ /* Return error if asked to set polarity modes but not supported */
+ if (!phydev->drv->led_polarity_set)
+ return -EINVAL;
+
+ err = phydev->drv->led_polarity_set(phydev, index, modes);
+ if (err)
+ return err;
+ }
+
phyled->index = index;
if (phydev->drv->led_brightness_set)
cdev->brightness_set_blocking = phy_led_set_brightness;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 684efaeca07c..c9994a59ca2e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -852,6 +852,15 @@ struct phy_plca_status {
bool pst;
};
+/* Modes for PHY LED configuration */
+enum phy_led_modes {
+ PHY_LED_ACTIVE_LOW = 0,
+ PHY_LED_INACTIVE_HIGH_IMPEDANCE = 1,
+
+ /* keep it last */
+ __PHY_LED_MODES_NUM,
+};
+
/**
* struct phy_led: An LED driven by the PHY
*
@@ -1145,6 +1154,19 @@ struct phy_driver {
int (*led_hw_control_get)(struct phy_device *dev, u8 index,
unsigned long *rules);
+ /**
+ * @led_polarity_set: Set the LED polarity modes
+ * @dev: PHY device which has the LED
+ * @index: Which LED of the PHY device
+ * @modes: bitmap of LED polarity modes
+ *
+ * Configure LED with all the required polarity modes in @modes
+ * to make it correctly turn ON or OFF.
+ *
+ * Returns 0, or an error code.
+ */
+ int (*led_polarity_set)(struct phy_device *dev, int index,
+ unsigned long modes);
};
#define to_phy_driver(d) container_of(to_mdio_common_driver(d), \
struct phy_driver, mdiodrv)
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-next PATCH v10 4/5] dt-bindings: net: Document QCA808x PHYs
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
` (2 preceding siblings ...)
2024-01-25 20:36 ` [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes Christian Marangi
@ 2024-01-25 20:37 ` Christian Marangi
2024-01-25 20:37 ` [net-next PATCH v10 5/5] net: phy: at803x: add LED support for qca808x Christian Marangi
2024-01-27 5:10 ` [net-next PATCH v10 0/5] net: phy: generic polarity + " patchwork-bot+netdevbpf
5 siblings, 0 replies; 10+ messages in thread
From: Christian Marangi @ 2024-01-25 20:37 UTC (permalink / raw
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, Christian Marangi,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
Cc: Rob Herring
Add Documentation for QCA808x PHYs for the additional LED configuration
for this PHY.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes v4:
- Add Reviewed-by tag
Changes v3:
- Use compatible instead of select
- Out of RFC
Changes v2:
- Fix License warning from checkpatch
- Drop redundant Description phrase
- Improve commit tile
- Drop special property (generalized)
.../devicetree/bindings/net/qca,qca808x.yaml | 54 +++++++++++++++++++
1 file changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/qca,qca808x.yaml
diff --git a/Documentation/devicetree/bindings/net/qca,qca808x.yaml b/Documentation/devicetree/bindings/net/qca,qca808x.yaml
new file mode 100644
index 000000000000..e2552655902a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qca,qca808x.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qca,qca808x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Atheros QCA808X PHY
+
+maintainers:
+ - Christian Marangi <ansuelsmth@gmail.com>
+
+description:
+ QCA808X PHYs can have up to 3 LEDs attached.
+ All 3 LEDs are disabled by default.
+ 2 LEDs have dedicated pins with the 3rd LED having the
+ double function of Interrupt LEDs/GPIO or additional LED.
+
+ By default this special PIN is set to LED function.
+
+allOf:
+ - $ref: ethernet-phy.yaml#
+
+properties:
+ compatible:
+ enum:
+ - ethernet-phy-id004d.d101
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-phy@0 {
+ compatible = "ethernet-phy-id004d.d101";
+ reg = <0>;
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_WAN;
+ default-state = "keep";
+ };
+ };
+ };
+ };
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-next PATCH v10 5/5] net: phy: at803x: add LED support for qca808x
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
` (3 preceding siblings ...)
2024-01-25 20:37 ` [net-next PATCH v10 4/5] dt-bindings: net: Document QCA808x PHYs Christian Marangi
@ 2024-01-25 20:37 ` Christian Marangi
2024-01-27 5:10 ` [net-next PATCH v10 0/5] net: phy: generic polarity + " patchwork-bot+netdevbpf
5 siblings, 0 replies; 10+ messages in thread
From: Christian Marangi @ 2024-01-25 20:37 UTC (permalink / raw
To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, Christian Marangi,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
Add LED support for QCA8081 PHY.
Documentation for this LEDs PHY is very scarce even with NDA access
to Documentation for OEMs. Only the blink pattern are documented and are
very confusing most of the time. No documentation is present about
forcing the LED on/off or to always blink.
Those settings were reversed by poking the regs and trying to find the
correct bits to trigger these modes. Some bits mode are not clear and
maybe the documentation option are not 100% correct. For the sake of LED
support the reversed option are enough to add support for current LED
APIs.
Supported HW control modes are:
- tx
- rx
- link_10
- link_100
- link_1000
- link_2500
- half_duplex
- full_duplex
Also add support for LED polarity set to set LED polarity to active
high or low. QSDK sets this value to high by default but PHY reset value
doesn't have this enabled by default.
QSDK also sets 2 additional bits but their usage is not clear, info about
this is added in the header. It was verified that for correct function
of the LED if active high is needed, only BIT 6 is needed.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes v10:
- Add Reviewed tag
- Add link_2500 speed (as the LED branch got merged in net-next)
Changes v9:
- Fix copy-paste error for blink settings
Changes v8:
- Drop unused ret variable
Changes v7:
- Improve set polarity settings (Suggested by Russell)
Changes v6:
- Rebase on top of net-next
Changes v5:
- Rework to polarity option bitmap
- Rafactor with new finding from further reverse
Changes v4:
- Rework to polarity option (for marvell10g series support)
- Rework logic to enforce single PHY polarity mode
Changes v3:
- Out of RFC
- Drop link_25000 and add TODO commends waiting for the
netdev trigger thing to be merged (I will take care of
sending a followup patch later)
Changes v2:
- Move to new led_polarity_set implementation
- Drop special probe
drivers/net/phy/at803x.c | 327 +++++++++++++++++++++++++++++++++++++++
1 file changed, 327 insertions(+)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index a62442a55774..9c07a6cc6e67 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -301,6 +301,87 @@
/* Added for reference of existence but should be handled by wait_for_completion already */
#define QCA808X_CDT_STATUS_STAT_BUSY (BIT(1) | BIT(3))
+#define QCA808X_MMD7_LED_GLOBAL 0x8073
+#define QCA808X_LED_BLINK_1 GENMASK(11, 6)
+#define QCA808X_LED_BLINK_2 GENMASK(5, 0)
+/* Values are the same for both BLINK_1 and BLINK_2 */
+#define QCA808X_LED_BLINK_FREQ_MASK GENMASK(5, 3)
+#define QCA808X_LED_BLINK_FREQ_2HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x0)
+#define QCA808X_LED_BLINK_FREQ_4HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x1)
+#define QCA808X_LED_BLINK_FREQ_8HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x2)
+#define QCA808X_LED_BLINK_FREQ_16HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x3)
+#define QCA808X_LED_BLINK_FREQ_32HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x4)
+#define QCA808X_LED_BLINK_FREQ_64HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x5)
+#define QCA808X_LED_BLINK_FREQ_128HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x6)
+#define QCA808X_LED_BLINK_FREQ_256HZ FIELD_PREP(QCA808X_LED_BLINK_FREQ_MASK, 0x7)
+#define QCA808X_LED_BLINK_DUTY_MASK GENMASK(2, 0)
+#define QCA808X_LED_BLINK_DUTY_50_50 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x0)
+#define QCA808X_LED_BLINK_DUTY_75_25 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x1)
+#define QCA808X_LED_BLINK_DUTY_25_75 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x2)
+#define QCA808X_LED_BLINK_DUTY_33_67 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x3)
+#define QCA808X_LED_BLINK_DUTY_67_33 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x4)
+#define QCA808X_LED_BLINK_DUTY_17_83 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x5)
+#define QCA808X_LED_BLINK_DUTY_83_17 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x6)
+#define QCA808X_LED_BLINK_DUTY_8_92 FIELD_PREP(QCA808X_LED_BLINK_DUTY_MASK, 0x7)
+
+#define QCA808X_MMD7_LED2_CTRL 0x8074
+#define QCA808X_MMD7_LED2_FORCE_CTRL 0x8075
+#define QCA808X_MMD7_LED1_CTRL 0x8076
+#define QCA808X_MMD7_LED1_FORCE_CTRL 0x8077
+#define QCA808X_MMD7_LED0_CTRL 0x8078
+#define QCA808X_MMD7_LED_CTRL(x) (0x8078 - ((x) * 2))
+
+/* LED hw control pattern is the same for every LED */
+#define QCA808X_LED_PATTERN_MASK GENMASK(15, 0)
+#define QCA808X_LED_SPEED2500_ON BIT(15)
+#define QCA808X_LED_SPEED2500_BLINK BIT(14)
+/* Follow blink trigger even if duplex or speed condition doesn't match */
+#define QCA808X_LED_BLINK_CHECK_BYPASS BIT(13)
+#define QCA808X_LED_FULL_DUPLEX_ON BIT(12)
+#define QCA808X_LED_HALF_DUPLEX_ON BIT(11)
+#define QCA808X_LED_TX_BLINK BIT(10)
+#define QCA808X_LED_RX_BLINK BIT(9)
+#define QCA808X_LED_TX_ON_10MS BIT(8)
+#define QCA808X_LED_RX_ON_10MS BIT(7)
+#define QCA808X_LED_SPEED1000_ON BIT(6)
+#define QCA808X_LED_SPEED100_ON BIT(5)
+#define QCA808X_LED_SPEED10_ON BIT(4)
+#define QCA808X_LED_COLLISION_BLINK BIT(3)
+#define QCA808X_LED_SPEED1000_BLINK BIT(2)
+#define QCA808X_LED_SPEED100_BLINK BIT(1)
+#define QCA808X_LED_SPEED10_BLINK BIT(0)
+
+#define QCA808X_MMD7_LED0_FORCE_CTRL 0x8079
+#define QCA808X_MMD7_LED_FORCE_CTRL(x) (0x8079 - ((x) * 2))
+
+/* LED force ctrl is the same for every LED
+ * No documentation exist for this, not even internal one
+ * with NDA as QCOM gives only info about configuring
+ * hw control pattern rules and doesn't indicate any way
+ * to force the LED to specific mode.
+ * These define comes from reverse and testing and maybe
+ * lack of some info or some info are not entirely correct.
+ * For the basic LED control and hw control these finding
+ * are enough to support LED control in all the required APIs.
+ *
+ * On doing some comparison with implementation with qca807x,
+ * it was found that it's 1:1 equal to it and confirms all the
+ * reverse done. It was also found further specification with the
+ * force mode and the blink modes.
+ */
+#define QCA808X_LED_FORCE_EN BIT(15)
+#define QCA808X_LED_FORCE_MODE_MASK GENMASK(14, 13)
+#define QCA808X_LED_FORCE_BLINK_1 FIELD_PREP(QCA808X_LED_FORCE_MODE_MASK, 0x3)
+#define QCA808X_LED_FORCE_BLINK_2 FIELD_PREP(QCA808X_LED_FORCE_MODE_MASK, 0x2)
+#define QCA808X_LED_FORCE_ON FIELD_PREP(QCA808X_LED_FORCE_MODE_MASK, 0x1)
+#define QCA808X_LED_FORCE_OFF FIELD_PREP(QCA808X_LED_FORCE_MODE_MASK, 0x0)
+
+#define QCA808X_MMD7_LED_POLARITY_CTRL 0x901a
+/* QSDK sets by default 0x46 to this reg that sets BIT 6 for
+ * LED to active high. It's not clear what BIT 3 and BIT 4 does.
+ */
+#define QCA808X_LED_ACTIVE_HIGH BIT(6)
+
/* QCA808X 1G chip type */
#define QCA808X_PHY_MMD7_CHIP_TYPE 0x901d
#define QCA808X_PHY_CHIP_TYPE_1G BIT(0)
@@ -346,6 +427,7 @@ struct at803x_priv {
struct regulator_dev *vddio_rdev;
struct regulator_dev *vddh_rdev;
u64 stats[ARRAY_SIZE(qca83xx_hw_stats)];
+ int led_polarity_mode;
};
struct at803x_context {
@@ -706,6 +788,9 @@ static int at803x_probe(struct phy_device *phydev)
if (!priv)
return -ENOMEM;
+ /* Init LED polarity mode to -1 */
+ priv->led_polarity_mode = -1;
+
phydev->priv = priv;
ret = at803x_parse_dt(phydev);
@@ -2235,6 +2320,242 @@ static void qca808x_link_change_notify(struct phy_device *phydev)
phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
}
+static int qca808x_led_parse_netdev(struct phy_device *phydev, unsigned long rules,
+ u16 *offload_trigger)
+{
+ /* Parsing specific to netdev trigger */
+ if (test_bit(TRIGGER_NETDEV_TX, &rules))
+ *offload_trigger |= QCA808X_LED_TX_BLINK;
+ if (test_bit(TRIGGER_NETDEV_RX, &rules))
+ *offload_trigger |= QCA808X_LED_RX_BLINK;
+ if (test_bit(TRIGGER_NETDEV_LINK_10, &rules))
+ *offload_trigger |= QCA808X_LED_SPEED10_ON;
+ if (test_bit(TRIGGER_NETDEV_LINK_100, &rules))
+ *offload_trigger |= QCA808X_LED_SPEED100_ON;
+ if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules))
+ *offload_trigger |= QCA808X_LED_SPEED1000_ON;
+ if (test_bit(TRIGGER_NETDEV_LINK_2500, &rules))
+ *offload_trigger |= QCA808X_LED_SPEED2500_ON;
+ if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
+ *offload_trigger |= QCA808X_LED_HALF_DUPLEX_ON;
+ if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
+ *offload_trigger |= QCA808X_LED_FULL_DUPLEX_ON;
+
+ if (rules && !*offload_trigger)
+ return -EOPNOTSUPP;
+
+ /* Enable BLINK_CHECK_BYPASS by default to make the LED
+ * blink even with duplex or speed mode not enabled.
+ */
+ *offload_trigger |= QCA808X_LED_BLINK_CHECK_BYPASS;
+
+ return 0;
+}
+
+static int qca808x_led_hw_control_enable(struct phy_device *phydev, u8 index)
+{
+ u16 reg;
+
+ if (index > 2)
+ return -EINVAL;
+
+ reg = QCA808X_MMD7_LED_FORCE_CTRL(index);
+
+ return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, reg,
+ QCA808X_LED_FORCE_EN);
+}
+
+static int qca808x_led_hw_is_supported(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ u16 offload_trigger = 0;
+
+ if (index > 2)
+ return -EINVAL;
+
+ return qca808x_led_parse_netdev(phydev, rules, &offload_trigger);
+}
+
+static int qca808x_led_hw_control_set(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ u16 reg, offload_trigger = 0;
+ int ret;
+
+ if (index > 2)
+ return -EINVAL;
+
+ reg = QCA808X_MMD7_LED_CTRL(index);
+
+ ret = qca808x_led_parse_netdev(phydev, rules, &offload_trigger);
+ if (ret)
+ return ret;
+
+ ret = qca808x_led_hw_control_enable(phydev, index);
+ if (ret)
+ return ret;
+
+ return phy_modify_mmd(phydev, MDIO_MMD_AN, reg,
+ QCA808X_LED_PATTERN_MASK,
+ offload_trigger);
+}
+
+static bool qca808x_led_hw_control_status(struct phy_device *phydev, u8 index)
+{
+ u16 reg;
+ int val;
+
+ if (index > 2)
+ return false;
+
+ reg = QCA808X_MMD7_LED_FORCE_CTRL(index);
+
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, reg);
+
+ return !(val & QCA808X_LED_FORCE_EN);
+}
+
+static int qca808x_led_hw_control_get(struct phy_device *phydev, u8 index,
+ unsigned long *rules)
+{
+ u16 reg;
+ int val;
+
+ if (index > 2)
+ return -EINVAL;
+
+ /* Check if we have hw control enabled */
+ if (qca808x_led_hw_control_status(phydev, index))
+ return -EINVAL;
+
+ reg = QCA808X_MMD7_LED_CTRL(index);
+
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, reg);
+ if (val & QCA808X_LED_TX_BLINK)
+ set_bit(TRIGGER_NETDEV_TX, rules);
+ if (val & QCA808X_LED_RX_BLINK)
+ set_bit(TRIGGER_NETDEV_RX, rules);
+ if (val & QCA808X_LED_SPEED10_ON)
+ set_bit(TRIGGER_NETDEV_LINK_10, rules);
+ if (val & QCA808X_LED_SPEED100_ON)
+ set_bit(TRIGGER_NETDEV_LINK_100, rules);
+ if (val & QCA808X_LED_SPEED1000_ON)
+ set_bit(TRIGGER_NETDEV_LINK_1000, rules);
+ if (val & QCA808X_LED_SPEED2500_ON)
+ set_bit(TRIGGER_NETDEV_LINK_2500, rules);
+ if (val & QCA808X_LED_HALF_DUPLEX_ON)
+ set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
+ if (val & QCA808X_LED_FULL_DUPLEX_ON)
+ set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
+
+ return 0;
+}
+
+static int qca808x_led_hw_control_reset(struct phy_device *phydev, u8 index)
+{
+ u16 reg;
+
+ if (index > 2)
+ return -EINVAL;
+
+ reg = QCA808X_MMD7_LED_CTRL(index);
+
+ return phy_clear_bits_mmd(phydev, MDIO_MMD_AN, reg,
+ QCA808X_LED_PATTERN_MASK);
+}
+
+static int qca808x_led_brightness_set(struct phy_device *phydev,
+ u8 index, enum led_brightness value)
+{
+ u16 reg;
+ int ret;
+
+ if (index > 2)
+ return -EINVAL;
+
+ if (!value) {
+ ret = qca808x_led_hw_control_reset(phydev, index);
+ if (ret)
+ return ret;
+ }
+
+ reg = QCA808X_MMD7_LED_FORCE_CTRL(index);
+
+ return phy_modify_mmd(phydev, MDIO_MMD_AN, reg,
+ QCA808X_LED_FORCE_EN | QCA808X_LED_FORCE_MODE_MASK,
+ QCA808X_LED_FORCE_EN | value ? QCA808X_LED_FORCE_ON :
+ QCA808X_LED_FORCE_OFF);
+}
+
+static int qca808x_led_blink_set(struct phy_device *phydev, u8 index,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ int ret;
+ u16 reg;
+
+ if (index > 2)
+ return -EINVAL;
+
+ reg = QCA808X_MMD7_LED_FORCE_CTRL(index);
+
+ /* Set blink to 50% off, 50% on at 4Hz by default */
+ ret = phy_modify_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_LED_GLOBAL,
+ QCA808X_LED_BLINK_FREQ_MASK | QCA808X_LED_BLINK_DUTY_MASK,
+ QCA808X_LED_BLINK_FREQ_4HZ | QCA808X_LED_BLINK_DUTY_50_50);
+ if (ret)
+ return ret;
+
+ /* We use BLINK_1 for normal blinking */
+ ret = phy_modify_mmd(phydev, MDIO_MMD_AN, reg,
+ QCA808X_LED_FORCE_EN | QCA808X_LED_FORCE_MODE_MASK,
+ QCA808X_LED_FORCE_EN | QCA808X_LED_FORCE_BLINK_1);
+ if (ret)
+ return ret;
+
+ /* We set blink to 4Hz, aka 250ms */
+ *delay_on = 250 / 2;
+ *delay_off = 250 / 2;
+
+ return 0;
+}
+
+static int qca808x_led_polarity_set(struct phy_device *phydev, int index,
+ unsigned long modes)
+{
+ struct at803x_priv *priv = phydev->priv;
+ bool active_low = false;
+ u32 mode;
+
+ for_each_set_bit(mode, &modes, __PHY_LED_MODES_NUM) {
+ switch (mode) {
+ case PHY_LED_ACTIVE_LOW:
+ active_low = true;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+
+ /* PHY polarity is global and can't be set per LED.
+ * To detect this, check if last requested polarity mode
+ * match the new one.
+ */
+ if (priv->led_polarity_mode >= 0 &&
+ priv->led_polarity_mode != active_low) {
+ phydev_err(phydev, "PHY polarity is global. Mismatched polarity on different LED\n");
+ return -EINVAL;
+ }
+
+ /* Save the last PHY polarity mode */
+ priv->led_polarity_mode = active_low;
+
+ return phy_modify_mmd(phydev, MDIO_MMD_AN,
+ QCA808X_MMD7_LED_POLARITY_CTRL,
+ QCA808X_LED_ACTIVE_HIGH,
+ active_low ? 0 : QCA808X_LED_ACTIVE_HIGH);
+}
+
static struct phy_driver at803x_driver[] = {
{
/* Qualcomm Atheros AR8035 */
@@ -2411,6 +2732,12 @@ static struct phy_driver at803x_driver[] = {
.cable_test_start = qca808x_cable_test_start,
.cable_test_get_status = qca808x_cable_test_get_status,
.link_change_notify = qca808x_link_change_notify,
+ .led_brightness_set = qca808x_led_brightness_set,
+ .led_blink_set = qca808x_led_blink_set,
+ .led_hw_is_supported = qca808x_led_hw_is_supported,
+ .led_hw_control_set = qca808x_led_hw_control_set,
+ .led_hw_control_get = qca808x_led_hw_control_get,
+ .led_polarity_set = qca808x_led_polarity_set,
}, };
module_phy_driver(at803x_driver);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
` (4 preceding siblings ...)
2024-01-25 20:37 ` [net-next PATCH v10 5/5] net: phy: at803x: add LED support for qca808x Christian Marangi
@ 2024-01-27 5:10 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-27 5:10 UTC (permalink / raw
To: Christian Marangi
Cc: pavel, lee, robh+dt, krzysztof.kozlowski+dt, conor+dt,
william.zhang, anand.gore, kursad.oney, florian.fainelli, rafal,
bcm-kernel-feedback-list, davem, edumazet, kuba, pabeni, andrew,
hkallweit1, linux, jacek.anaszewski, noltari, sven.schwermer,
linux-leds, devicetree, linux-kernel, linux-arm-kernel, netdev
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 25 Jan 2024 21:36:56 +0100 you wrote:
> This small series add LEDs support for qca808x.
>
> QCA808x apply on PHY reset a strange polarity settings and require
> some tweak to apply a more common configuration found on devices.
> On adding support for it, it was pointed out that a similar
> feature is also being implemented for a marvell PHY where
> LED polarity is set per LED (and not global) and also have
> a special mode where the LED is tristated.
>
> [...]
Here is the summary with links:
- [net-next,v10,1/5] dt-bindings: net: phy: Make LED active-low property common
https://git.kernel.org/netdev/net-next/c/c94d1783136e
- [net-next,v10,2/5] dt-bindings: net: phy: Document LED inactive high impedance mode
https://git.kernel.org/netdev/net-next/c/355c6dc37efa
- [net-next,v10,3/5] net: phy: add support for PHY LEDs polarity modes
https://git.kernel.org/netdev/net-next/c/7ae215ee7bb8
- [net-next,v10,4/5] dt-bindings: net: Document QCA808x PHYs
https://git.kernel.org/netdev/net-next/c/91e893b43d1c
- [net-next,v10,5/5] net: phy: at803x: add LED support for qca808x
https://git.kernel.org/netdev/net-next/c/7196062b64ee
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes
2024-01-25 20:36 ` [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes Christian Marangi
@ 2024-05-10 23:14 ` Daniel Golle
2024-05-10 23:58 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Golle @ 2024-05-10 23:14 UTC (permalink / raw
To: Christian Marangi
Cc: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, William Zhang, Anand Gore, Kursad Oney,
Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
Heiner Kallweit, Russell King, Jacek Anaszewski,
Fernández Rojas, Sven Schwermer, linux-leds, devicetree,
linux-kernel, linux-arm-kernel, netdev
Hi Christian,
Hi Andrew,
On Thu, Jan 25, 2024 at 09:36:59PM +0100, Christian Marangi wrote:
> Add support for PHY LEDs polarity modes. Some PHY require LED to be set
> to active low to be turned ON. Adds support for this by declaring
> active-low property in DT.
>
> PHY driver needs to declare .led_polarity_set() to configure LED
> polarity modes. Function will pass the index with the LED index and a
> bitmap with all the required modes to set.
>
> Current supported modes are:
> - active-low with the flag PHY_LED_ACTIVE_LOW. LED is set to active-low
> to turn it ON.
> - inactive-high-impedance with the flag PHY_LED_INACTIVE_HIGH_IMPEDANCE.
> LED is set to high impedance to turn it OFF.
Wanting to make use of this I noticed that polarity settings are only
applied once in of_phy_led(), which is not sufficient for my use-case:
I'm writing a LED driver for Aquantia PHYs and those PHYs reset the
polarity mode every time a PHY reset is triggered.
I ended up writing the patch below, but I'm not sure if phy_init_hw
should take care of this or if the polarity modes should be stored in
memory allocated by the PHY driver and re-applied by the driver after
reset (eg. in .config_init). Kinda depends on taste and on how common
this behavior is in practise, so I thought the best is to reach out to
discuss.
Let me know what you think.
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 616bd7ba46cb..1624884fd627 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1251,6 +1251,7 @@ static int phy_poll_reset(struct phy_device *phydev)
int phy_init_hw(struct phy_device *phydev)
{
int ret = 0;
+ struct phy_led *phyled;
/* Deassert the reset signal */
phy_device_reset(phydev, 0);
@@ -1285,6 +1286,17 @@ int phy_init_hw(struct phy_device *phydev)
return ret;
}
+ if (phydev->drv->led_polarity_set) {
+ list_for_each_entry(phyled, &phydev->leds, list) {
+ if (!phyled->polarity_modes)
+ continue;
+
+ ret = phydev->drv->led_polarity_set(phydev, phyled->index, phyled->polarity_modes);
+ if (ret)
+ return ret;
+ }
+ }
+
return 0;
}
EXPORT_SYMBOL(phy_init_hw);
@@ -3316,7 +3328,6 @@ static int of_phy_led(struct phy_device *phydev,
struct device *dev = &phydev->mdio.dev;
struct led_init_data init_data = {};
struct led_classdev *cdev;
- unsigned long modes = 0;
struct phy_led *phyled;
u32 index;
int err;
@@ -3335,18 +3346,14 @@ static int of_phy_led(struct phy_device *phydev,
return -EINVAL;
if (of_property_read_bool(led, "active-low"))
- set_bit(PHY_LED_ACTIVE_LOW, &modes);
+ set_bit(PHY_LED_ACTIVE_LOW, &phyled->polarity_modes);
if (of_property_read_bool(led, "inactive-high-impedance"))
- set_bit(PHY_LED_INACTIVE_HIGH_IMPEDANCE, &modes);
+ set_bit(PHY_LED_INACTIVE_HIGH_IMPEDANCE, &phyled->polarity_modes);
- if (modes) {
+ if (phyled->polarity_modes) {
/* Return error if asked to set polarity modes but not supported */
if (!phydev->drv->led_polarity_set)
return -EINVAL;
-
- err = phydev->drv->led_polarity_set(phydev, index, modes);
- if (err)
- return err;
}
phyled->index = index;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3ddfe7fe781a..7c8bd72e6fee 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -888,6 +888,7 @@ struct phy_led {
struct list_head list;
struct phy_device *phydev;
struct led_classdev led_cdev;
+ unsigned long polarity_modes;
u8 index;
};
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes
2024-05-10 23:14 ` Daniel Golle
@ 2024-05-10 23:58 ` Andrew Lunn
2024-05-11 9:35 ` Russell King (Oracle)
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2024-05-10 23:58 UTC (permalink / raw
To: Daniel Golle
Cc: Christian Marangi, Pavel Machek, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, William Zhang, Anand Gore,
Kursad Oney, Florian Fainelli, Rafał Miłecki,
Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Heiner Kallweit,
Russell King, Jacek Anaszewski, Fernández Rojas,
Sven Schwermer, linux-leds, devicetree, linux-kernel,
linux-arm-kernel, netdev
> Wanting to make use of this I noticed that polarity settings are only
> applied once in of_phy_led(), which is not sufficient for my use-case:
>
> I'm writing a LED driver for Aquantia PHYs and those PHYs reset the
> polarity mode every time a PHY reset is triggered.
>
> I ended up writing the patch below, but I'm not sure if phy_init_hw
> should take care of this or if the polarity modes should be stored in
> memory allocated by the PHY driver and re-applied by the driver after
> reset (eg. in .config_init). Kinda depends on taste and on how common
> this behavior is in practise, so I thought the best is to reach out to
> discuss.
There was a similar discussion recently about WoL settings getting
lost. The conclusion about that was the PHY should keep track of WoL
setting. So i would say the same applies there. Please store it in a
local priv structure.
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes
2024-05-10 23:58 ` Andrew Lunn
@ 2024-05-11 9:35 ` Russell King (Oracle)
0 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2024-05-11 9:35 UTC (permalink / raw
To: Andrew Lunn
Cc: Daniel Golle, Christian Marangi, Pavel Machek, Lee Jones,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, William Zhang,
Anand Gore, Kursad Oney, Florian Fainelli,
Rafał Miłecki, Broadcom internal kernel review list,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heiner Kallweit, Jacek Anaszewski, Fernández Rojas,
Sven Schwermer, linux-leds, devicetree, linux-kernel,
linux-arm-kernel, netdev
On Sat, May 11, 2024 at 01:58:13AM +0200, Andrew Lunn wrote:
> > Wanting to make use of this I noticed that polarity settings are only
> > applied once in of_phy_led(), which is not sufficient for my use-case:
> >
> > I'm writing a LED driver for Aquantia PHYs and those PHYs reset the
> > polarity mode every time a PHY reset is triggered.
What sort of reset? There are hard resets that set the registers back
to default, and soft resets that don't. I think you are referring to
a hard reset, but please be clear.
> > I ended up writing the patch below, but I'm not sure if phy_init_hw
> > should take care of this or if the polarity modes should be stored in
> > memory allocated by the PHY driver and re-applied by the driver after
> > reset (eg. in .config_init). Kinda depends on taste and on how common
> > this behavior is in practise, so I thought the best is to reach out to
> > discuss.
>
> There was a similar discussion recently about WoL settings getting
> lost. The conclusion about that was the PHY should keep track of WoL
> setting. So i would say the same applies there. Please store it in a
> local priv structure.
Agreed. If it turns out that it's something that many PHYs need to do,
that would be the tiem to move it into the core phylib code. If it only
affects a minority of drivers, then it's something drivers should do.
The reasoning here is: if its only a problem for a small amount of PHY
drivers, then we don't need to penalise everyone with additional
overhead. If it's the majority of drivers, then it makes sense to
remove this burden from drivers.
Also note that this is one of the reasons I don't particularly like
the kernel's approach to PHY hardware resets - if a platform firmware
decides to describe the PHy hardware reset to the kernel, then we end
up with the hardware reset being used at various points which will
clear all the registers back to the reset defaults including clearing
WoL settings and the like. That's fine for AN state which will get
reloaded, but other state does not, so describing the hardware reset
can make things much more complicated and introduce differing
behaviours compared to platforms that don't describe it.
A lot of platforms choose not to describe the PHY hardware reset, but
some people see that there's a way to describe it, so they do whether
or not there's a reason for the kernel to be manipulating that reset
signal.
What I'm saying is there's several issues here that all interact...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-05-11 9:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 1/5] dt-bindings: net: phy: Make LED active-low property common Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 2/5] dt-bindings: net: phy: Document LED inactive high impedance mode Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes Christian Marangi
2024-05-10 23:14 ` Daniel Golle
2024-05-10 23:58 ` Andrew Lunn
2024-05-11 9:35 ` Russell King (Oracle)
2024-01-25 20:37 ` [net-next PATCH v10 4/5] dt-bindings: net: Document QCA808x PHYs Christian Marangi
2024-01-25 20:37 ` [net-next PATCH v10 5/5] net: phy: at803x: add LED support for qca808x Christian Marangi
2024-01-27 5:10 ` [net-next PATCH v10 0/5] net: phy: generic polarity + " patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).