linux-kernel - Re: [PATCH v3] leds: add SGI IP30 led support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <93666bdf-dc53-e091-157e-e5b988ede97c@ti.com>
Date:   Wed, 19 Feb 2020 11:51:24 -0600
From:   Dan Murphy <dmurphy@...com>
To:     Thomas Bogendoerfer <tbogendoerfer@...e.de>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Pavel Machek <pavel@....cz>
CC:     <linux-kernel@...r.kernel.org>, <linux-leds@...r.kernel.org>
Subject: Re: [PATCH v3] leds: add SGI IP30 led support

Thomas

On 2/19/20 10:05 AM, Thomas Bogendoerfer wrote:
> This patch implemenets a driver to support the front panel LEDs of
> SGI Octane (IP30) workstations.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@...e.de>
> ---

Sorry for the delay in review I had to go back and review v1 and v2 
comments


> 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 | 80 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 92 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..82453a216f81
> --- /dev/null
> +++ b/drivers/leds/leds-ip30.c
> @@ -0,0 +1,80 @@
> +// 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);
> +

With max_brightness set to 1 then value will only be either 1 or 0.  The 
LED framework takes care of gating the values.

So this can be writel(value, led->reg);

Otherwise

Reviewed-by: Dan Murphy <dmurphy@...com>