Thomas
On 1/16/20 5:05 AM, Thomas Bogendoerfer wrote:
On Wed, 15 Jan 2020 07:46:13 -0600
Dan Murphy <dmurphy@xxxxxx> wrote:
Thomas
On 1/15/20 7:05 AM, Thomas Bogendoerfer wrote:
This patch implemenets a driver to support the front panel LEDs of
SGI Octane (IP30) workstations.
Thanks for the patch
Some nitpicks below
Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@xxxxxxx>
---
drivers/leds/Kconfig | 11 ++++++
drivers/leds/Makefile | 1 +
drivers/leds/leds-ip30.c | 82 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 94 insertions(+)
create mode 100644 drivers/leds/leds-ip30.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 4b68520ac251..8ef0fe900928 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -836,6 +836,17 @@ config LEDS_LM36274
Say Y to enable the LM36274 LED driver for TI LMU devices.
This supports the LED device LM36274.
+config LEDS_IP30
+ tristate "LED support for SGI Octane machines"
+ depends on LEDS_CLASS
+ depends on SGI_MFD_IOC3
What is the dependency on the MFD?
the gpio lines where the leds are connected are managed by the mfd ioc3 driver.
So without that driver this led driver will not be started.
Ack
+
+
+ if (num == 0) {
+ data->cdev.name = "ip30:white";
This also needs a function as defined in dt-bindings/common.h
+ data->cdev.default_trigger = "default-on";
The name, color, function and trigger can be pulled from the DT or Firmware.
The firmware should contain a node for each LED to be configured.
SGI Octanes don't have DT and the firmware just toggles some of the leds,
but doesn't provide informations about it. That's why this is hardcoded
in the driver. The MFD driver detects the ioc3 chip and knows it's a
SGI Octane mainboard and creates the led platform device.
How is the correct way to this without DT ?
Please refer to the leds-class document (1) under LED device naming
convention
If you don't have a DT then set the name accordingly. The functions are
defined in the common.h and the colors are exported in the leds-class driver
1 https://www.kernel.org/doc/Documentation/leds/leds-class.rst
+ } else {
+ data->cdev.name = "ip30:red";
Same as above
+ data->cdev.default_trigger = "panic";
+ writel(0, data->reg);
Is the LED on by default?
Depends on the hardware state. If PROM firmware detects some hardware issues,
it turns on the red LED. Otherwise it's off.
So why would we turn it off if the PROM FW turned it on?
Dan
