mbox series

[net-next,v3,00/14] net: Add basic LED support for switch/phy

Message ID 20230314101516.20427-1-ansuelsmth@gmail.com
Headers show
Series net: Add basic LED support for switch/phy | expand

Message

Christian Marangi March 14, 2023, 10:15 a.m. UTC 
This is a continue of [1]. It was decided to take a more gradual
approach to implement LEDs support for switch and phy starting with
basic support and then implementing the hw control part when we have all
the prereq done.

This series implements only the brightness_set() and blink_set() ops.
An example of switch implementation is done with qca8k.

For PHY a more generic approach is used with implementing the LED
support in PHY core and with the user (in this case marvell) adding all
the required functions.

Currently we set the default-state as "keep" to not change the default
configuration of the declared LEDs since almost every switch have a
default configuration.

[1] https://lore.kernel.org/lkml/20230216013230.22978-1-ansuelsmth@gmail.com/

Changes in new series v3:
- Move QCA8K_LEDS Kconfig option from tristate to bool
- Use new helper led_init_default_state_get for default-state in qca8k
- Drop cled_qca8k_brightness_get() as there isn't a good way to describe
  the mode the led is currently in
- Rework qca8k_led_brightness_get() to return true only when LED is set
  to always ON
Changes in new series v2:
- Add LEDs node for rb3011
- Fix rb3011 switch node unevaluated properties while running 
  make dtbs_check
- Fix a copypaste error in qca8k-leds.c for port 4 required shift
- Drop phy-handle usage for qca8k and use qca8k_port_to_phy()
- Add review tag from Andrew
- Add Christian Marangi SOB in each Andrew patch
- Add extra description for dsa-port stressing that PHY have no access
  and LED are controlled by the related MAC
- Add missing additionalProperties for dsa-port.yaml and ethernet-phy.yaml

Changes from the old v8 series:
- Drop linux,default-trigger set to netdev.
- Dropped every hw control related patch and implement only
  blink_set and brightness_set
- Add default-state to "keep" for each LED node example

Andrew Lunn (6):
  net: phy: Add a binding for PHY LEDs
  net: phy: phy_device: Call into the PHY driver to set LED brightness.
  net: phy: marvell: Add software control of the LEDs
  net: phy: phy_device: Call into the PHY driver to set LED blinking.
  net: phy: marvell: Implement led_blink_set()
  arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

Christian Marangi (8):
  net: dsa: qca8k: move qca8k_port_to_phy() to header
  net: dsa: qca8k: add LEDs basic support
  net: dsa: qca8k: add LEDs blink_set() support
  dt-bindings: net: dsa: dsa-port: Document support for LEDs node
  dt-bindings: net: dsa: qca8k: add LEDs definition example
  arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011
  arm: qcom: dt: Add Switch LED for each port for rb3011
  dt-bindings: net: phy: Document support for LEDs node

 .../devicetree/bindings/net/dsa/dsa-port.yaml |  21 ++
 .../devicetree/bindings/net/dsa/qca8k.yaml    |  24 ++
 .../devicetree/bindings/net/ethernet-phy.yaml |  31 +++
 arch/arm/boot/dts/armada-370-rd.dts           |  14 ++
 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts     | 124 +++++++++-
 drivers/net/dsa/qca/Kconfig                   |   7 +
 drivers/net/dsa/qca/Makefile                  |   1 +
 drivers/net/dsa/qca/qca8k-8xxx.c              |  19 +-
 drivers/net/dsa/qca/qca8k-leds.c              | 229 ++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h                   |  83 +++++++
 drivers/net/phy/marvell.c                     |  81 ++++++-
 drivers/net/phy/phy_device.c                  | 115 +++++++++
 include/linux/phy.h                           |  33 +++
 13 files changed, 758 insertions(+), 24 deletions(-)
 create mode 100644 drivers/net/dsa/qca/qca8k-leds.c

Comments

Vladimir Oltean March 15, 2023, 12:50 a.m. UTC | #1 
On Tue, Mar 14, 2023 at 11:15:11AM +0100, Christian Marangi wrote:
> Document support for LEDs node in dsa port.
> Switch may support different LEDs that can be configured for different
> operation like blinking on traffic event or port link.
> 
> Also add some Documentation to describe the difference of these nodes
> compared to PHY LEDs, since dsa-port LEDs are controllable by the switch
> regs and the possible intergated PHY doesn't have control on them.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/net/dsa/dsa-port.yaml | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)

Of all schemas, why did you choose dsa-port.yaml? Why not either something
hardware specific (qca8k.yaml) or more generic (ethernet-controller.yaml)?
Andrew Lunn March 15, 2023, 1:58 a.m. UTC | #2 
On Wed, Mar 15, 2023 at 02:50:00AM +0200, Vladimir Oltean wrote:
> On Tue, Mar 14, 2023 at 11:15:11AM +0100, Christian Marangi wrote:
> > Document support for LEDs node in dsa port.
> > Switch may support different LEDs that can be configured for different
> > operation like blinking on traffic event or port link.
> > 
> > Also add some Documentation to describe the difference of these nodes
> > compared to PHY LEDs, since dsa-port LEDs are controllable by the switch
> > regs and the possible intergated PHY doesn't have control on them.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/net/dsa/dsa-port.yaml | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> 
> Of all schemas, why did you choose dsa-port.yaml? Why not either something
> hardware specific (qca8k.yaml) or more generic (ethernet-controller.yaml)?

The binding should be generic. So qca8k.yaml is way to specific. The
Marvell switch should re-use it at some point.

Looking at the hierarchy, ethernet-controller.yaml would work since
dsa-port includes ethernet-switch-port, which includes
ethernet-controller.

These are MAC LEDs, and there is no reason why a standalone MAC in a
NIC could not implement such LEDs. So yes,
ethernet-controller.yaml.

Is there actually anything above ethernet-controller.yaml? This is
about a netdev really, so a wifi MAC, or even a CAN MAC could also use
the binding....

    Andrew
Christian Marangi March 15, 2023, 3:28 a.m. UTC | #3 
On Wed, Mar 15, 2023 at 02:58:23AM +0100, Andrew Lunn wrote:
> On Wed, Mar 15, 2023 at 02:50:00AM +0200, Vladimir Oltean wrote:
> > On Tue, Mar 14, 2023 at 11:15:11AM +0100, Christian Marangi wrote:
> > > Document support for LEDs node in dsa port.
> > > Switch may support different LEDs that can be configured for different
> > > operation like blinking on traffic event or port link.
> > > 
> > > Also add some Documentation to describe the difference of these nodes
> > > compared to PHY LEDs, since dsa-port LEDs are controllable by the switch
> > > regs and the possible intergated PHY doesn't have control on them.
> > > 
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > >  .../devicetree/bindings/net/dsa/dsa-port.yaml | 21 +++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > 
> > Of all schemas, why did you choose dsa-port.yaml? Why not either something
> > hardware specific (qca8k.yaml) or more generic (ethernet-controller.yaml)?
> 
> The binding should be generic. So qca8k.yaml is way to specific. The
> Marvell switch should re-use it at some point.
> 
> Looking at the hierarchy, ethernet-controller.yaml would work since
> dsa-port includes ethernet-switch-port, which includes
> ethernet-controller.
> 
> These are MAC LEDs, and there is no reason why a standalone MAC in a
> NIC could not implement such LEDs. So yes,
> ethernet-controller.yaml.
> 
> Is there actually anything above ethernet-controller.yaml? This is
> about a netdev really, so a wifi MAC, or even a CAN MAC could also use
> the binding....
>

Yes ideally when we manage to do all the things, ath10k would benefits
from this since it does have leds that blink on tx/rx traffic and are
specially controlled...

Don't think there is something above ethernet-controller tho...
Rob Herring (Arm) March 17, 2023, 9:13 p.m. UTC | #4 
On Wed, Mar 15, 2023 at 02:58:23AM +0100, Andrew Lunn wrote:
> On Wed, Mar 15, 2023 at 02:50:00AM +0200, Vladimir Oltean wrote:
> > On Tue, Mar 14, 2023 at 11:15:11AM +0100, Christian Marangi wrote:
> > > Document support for LEDs node in dsa port.
> > > Switch may support different LEDs that can be configured for different
> > > operation like blinking on traffic event or port link.
> > > 
> > > Also add some Documentation to describe the difference of these nodes
> > > compared to PHY LEDs, since dsa-port LEDs are controllable by the switch
> > > regs and the possible intergated PHY doesn't have control on them.
> > > 
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > >  .../devicetree/bindings/net/dsa/dsa-port.yaml | 21 +++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > 
> > Of all schemas, why did you choose dsa-port.yaml? Why not either something
> > hardware specific (qca8k.yaml) or more generic (ethernet-controller.yaml)?
> 
> The binding should be generic. So qca8k.yaml is way to specific. The
> Marvell switch should re-use it at some point.
> 
> Looking at the hierarchy, ethernet-controller.yaml would work since
> dsa-port includes ethernet-switch-port, which includes
> ethernet-controller.
> 
> These are MAC LEDs, and there is no reason why a standalone MAC in a
> NIC could not implement such LEDs. So yes,
> ethernet-controller.yaml.
> 
> Is there actually anything above ethernet-controller.yaml?

Yes, the one under review[1].

Rob
 
[1] https://lore.kernel.org/all/20230203-dt-bindings-network-class-v2-0-499686795073@jannau.net/