Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
From: Dan Murphy <dmurphy@ti.com>
To: Thomas Bogendoerfer <tbogendoerfer@suse.de>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Pavel Machek <pavel@ucw.cz>, <linux-kernel@vger.kernel.org>,
	<linux-leds@vger.kernel.org>
Subject: Re: [PATCH] leds: add SGI IP30 led support
Date: Wed, 15 Jan 2020 07:46:13 -0600
Message-ID: <b5bf7941-3fc1-641d-5482-509eeae34eac@ti.com> (raw)
In-Reply-To: <20200115130536.11453-1-tbogendoerfer@suse.de>

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@suse.de>
> ---
>   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?
> +	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 2da39e896ce8..89a527ac8ab6 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
>   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_IP30)			+= leds-ip30.o
>   

Extra tab


>   # 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..b0a83f78c439
> --- /dev/null
> +++ b/drivers/leds/leds-ip30.c
> @@ -0,0 +1,82 @@
> +// 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);
> +
> +	if (value)
> +		writel(1, led->reg);
> +	else
> +		writel(0, 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 = "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.

> +	} else {
> +		data->cdev.name = "ip30:red";
Same as above
> +		data->cdev.default_trigger = "panic";
> +		writel(0, data->reg);

Is the LED on by default?

Dan


  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-15 13:05 Thomas Bogendoerfer
2020-01-15 13:46 ` Dan Murphy [this message]
2020-01-16 11:05   ` Thomas Bogendoerfer
2020-01-16 15:21     ` Dan Murphy

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b5bf7941-3fc1-641d-5482-509eeae34eac@ti.com \
    --to=dmurphy@ti.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=tbogendoerfer@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git