LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4] leds: add SGI IP30 led support
@ 2020-02-21 11:11 Thomas Bogendoerfer
  2020-02-21 20:31 ` Jacek Anaszewski
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Bogendoerfer @ 2020-02-21 11:11 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek; +Cc: Dan Murphy, linux-kernel, linux-leds

This patch implemenets a driver to support the front panel LEDs of
SGI Octane (IP30) workstations.

Reviewed-by: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
Changes in v4:
  - simplified ip30led_set by using gated value from led framework

Changes in v3:
  - rebased to 5.6-rc2

Changes in v2:
  - use led names conforming to include/dt-bindings/leds/common.h
  - read LED state from firmware
  - leave setting up to user

 drivers/leds/Kconfig     | 11 ++++++
 drivers/leds/Makefile    |  1 +
 drivers/leds/leds-ip30.c | 77 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 drivers/leds/leds-ip30.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index d82f1dea3711..c664d84e1667 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -846,6 +846,17 @@ config LEDS_TPS6105X
 	  It is a single boost converter primarily for white LEDs and
 	  audio amplifiers.
 
+config LEDS_IP30
+	tristate "LED support for SGI Octane machines"
+	depends on LEDS_CLASS
+	depends on SGI_MFD_IOC3
+	help
+	  This option enables support for the Red and White LEDs of
+	  SGI Octane machines.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-ip30.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index d7e1107753fb..46bd611a03a9 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
 obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
 obj-$(CONFIG_LEDS_TPS6105X)		+= leds-tps6105x.o
+obj-$(CONFIG_LEDS_IP30)			+= leds-ip30.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-ip30.c b/drivers/leds/leds-ip30.c
new file mode 100644
index 000000000000..c5853780a31a
--- /dev/null
+++ b/drivers/leds/leds-ip30.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LED Driver for SGI Octane machines
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/leds.h>
+
+struct ip30_led {
+	struct led_classdev cdev;
+	u32 __iomem *reg;
+};
+
+static void ip30led_set(struct led_classdev *led_cdev,
+			enum led_brightness value)
+{
+	struct ip30_led *led = container_of(led_cdev, struct ip30_led, cdev);
+
+	writel(value, led->reg);
+}
+
+static int ip30led_create(struct platform_device *pdev, int num)
+{
+	struct resource *res;
+	struct ip30_led *data;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, num);
+	if (!res)
+		return -EBUSY;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->reg))
+		return PTR_ERR(data->reg);
+
+
+	if (num == 0)
+		data->cdev.name = "white:indicator";
+	else
+		data->cdev.name = "red:indicator";
+
+	data->cdev.brightness = readl(data->reg);
+	data->cdev.max_brightness = 1;
+	data->cdev.brightness_set = ip30led_set;
+
+	return devm_led_classdev_register(&pdev->dev, &data->cdev);
+}
+
+static int ip30led_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = ip30led_create(pdev, 0);
+	if (ret < 0)
+		return ret;
+
+	return ip30led_create(pdev, 1);
+}
+
+static struct platform_driver ip30led_driver = {
+	.probe		= ip30led_probe,
+	.driver		= {
+		.name		= "ip30-leds",
+	},
+};
+
+module_platform_driver(ip30led_driver);
+
+MODULE_AUTHOR("Thomas Bogendoerfer <tbogendoerfer@suse.de>");
+MODULE_DESCRIPTION("SGI Octane LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ip30-leds");
-- 
2.25.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] leds: add SGI IP30 led support
  2020-02-21 11:11 [PATCH v4] leds: add SGI IP30 led support Thomas Bogendoerfer
@ 2020-02-21 20:31 ` Jacek Anaszewski
  2020-02-22 10:05   ` Thomas Bogendoerfer
  0 siblings, 1 reply; 4+ messages in thread
From: Jacek Anaszewski @ 2020-02-21 20:31 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Pavel Machek; +Cc: Dan Murphy, linux-kernel, linux-leds

Hi Thomas,

I have one nit below.

On 2/21/20 12:11 PM, Thomas Bogendoerfer wrote:
> This patch implemenets a driver to support the front panel LEDs of
> SGI Octane (IP30) workstations.
> 
> Reviewed-by: Dan Murphy <dmurphy@ti.com>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> Changes in v4:
>   - simplified ip30led_set by using gated value from led framework
> 
> Changes in v3:
>   - rebased to 5.6-rc2
> 
> Changes in v2:
>   - use led names conforming to include/dt-bindings/leds/common.h
>   - read LED state from firmware
>   - leave setting up to user
> 
>  drivers/leds/Kconfig     | 11 ++++++
>  drivers/leds/Makefile    |  1 +
>  drivers/leds/leds-ip30.c | 77 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 89 insertions(+)
>  create mode 100644 drivers/leds/leds-ip30.c
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
[...]
> +	data->reg = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(data->reg))
> +		return PTR_ERR(data->reg);
> +
> +
> +	if (num == 0)
> +		data->cdev.name = "white:indicator";
> +	else
> +		data->cdev.name = "red:indicator";

Why indicator? Whereas it sounds quite generic it is used in the LED
subsystem specifically for naming indicator LEDs, that are often
found on flash LED controllers and are designed to indicate camera
sensor activity.

If it is on the front panel of SGI Octane workstations then its
function is perhaps well known?

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] leds: add SGI IP30 led support
  2020-02-21 20:31 ` Jacek Anaszewski
@ 2020-02-22 10:05   ` Thomas Bogendoerfer
  2020-02-22 16:36     ` Jacek Anaszewski
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Bogendoerfer @ 2020-02-22 10:05 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: Pavel Machek, Dan Murphy, linux-kernel, linux-leds

On Fri, 21 Feb 2020 21:31:04 +0100
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:


> > +	if (num == 0)
> > +		data->cdev.name = "white:indicator";
> > +	else
> > +		data->cdev.name = "red:indicator";
> 
> Why indicator? Whereas it sounds quite generic it is used in the LED
> subsystem specifically for naming indicator LEDs, that are often
> found on flash LED controllers and are designed to indicate camera
> sensor activity.

ok, ic.

> If it is on the front panel of SGI Octane workstations then its
> function is perhaps well known?

the red LED is clearly a fault led. The white LED will be switch on
after system diagnostic was successfull. Nothing from common.h really
fits that, maybe status ?

Thomas.

-- 
SUSE Software Solutions Germany GmbH
HRB 36809 (AG Nürnberg)
Geschäftsführer: Felix Imendörffer

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] leds: add SGI IP30 led support
  2020-02-22 10:05   ` Thomas Bogendoerfer
@ 2020-02-22 16:36     ` Jacek Anaszewski
  0 siblings, 0 replies; 4+ messages in thread
From: Jacek Anaszewski @ 2020-02-22 16:36 UTC (permalink / raw)
  To: Thomas Bogendoerfer; +Cc: Pavel Machek, Dan Murphy, linux-kernel, linux-leds

On 2/22/20 11:05 AM, Thomas Bogendoerfer wrote:
> On Fri, 21 Feb 2020 21:31:04 +0100
> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> 
> 
>>> +	if (num == 0)
>>> +		data->cdev.name = "white:indicator";
>>> +	else
>>> +		data->cdev.name = "red:indicator";
>>
>> Why indicator? Whereas it sounds quite generic it is used in the LED
>> subsystem specifically for naming indicator LEDs, that are often
>> found on flash LED controllers and are designed to indicate camera
>> sensor activity.
> 
> ok, ic.
> 
>> If it is on the front panel of SGI Octane workstations then its
>> function is perhaps well known?
> 
> the red LED is clearly a fault led.

We have fault in common LED names, so you can go for it.

> The white LED will be switch on
> after system diagnostic was successfull. Nothing from common.h really
> fits that, maybe status ?

I think that "system" would be OK. There are some LED drivers
and DT bindings that use "system" in the LED names. I even proposed
that also in the initial versions of my patch set standardizing LED
naming but dropped it afterwards, since it its use was not clear.

But now we have good use case for it.

And one more thing: please add definitions for LEDs in your
driver like:

#define IP30_LED_SYSTEM 0
#define IP30_LED_FAULT  1

and use them instead of bare 0 and 1 numbers.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-02-22 16:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-21 11:11 [PATCH v4] leds: add SGI IP30 led support Thomas Bogendoerfer
2020-02-21 20:31 ` Jacek Anaszewski
2020-02-22 10:05   ` Thomas Bogendoerfer
2020-02-22 16:36     ` Jacek Anaszewski

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 NNTP newsgroup(s).