| 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
| ||
|
Date: Wed, 9 Sep 2020 20:31:21 +0200
From: Marek Behún <marek.behun@....cz>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: netdev@...r.kernel.org, linux-leds@...r.kernel.org,
Pavel Machek <pavel@....cz>, Dan Murphy <dmurphy@...com>,
Ondřej Jirman <megous@...ous.com>,
Russell King <linux@...linux.org.uk>,
Andrew Lunn <andrew@...n.ch>, linux-kernel@...r.kernel.org,
Matthias Schiffer <matthias.schiffer@...tq-group.com>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next + leds v2 2/7] leds: add generic API for LEDs
that can be controlled by hardware
On Wed, 9 Sep 2020 11:20:00 -0700
Randy Dunlap <rdunlap@...radead.org> wrote:
> Hi,
>
> On 9/9/20 9:25 AM, Marek Behún wrote:
> > Many an ethernet PHY (and other chips) supports various HW control
> > modes for LEDs connected directly to them.
> >
> > This patch adds a generic API for registering such LEDs when
> > described in device tree. This API also exposes generic way to
> > select between these hardware control modes.
> >
> > This API registers a new private LED trigger called dev-hw-mode.
> > When this trigger is enabled for a LED, the various HW control
> > modes which are supported by the device for given LED can be
> > get/set via hw_mode sysfs file.
> >
> > Signed-off-by: Marek Behún <marek.behun@....cz>
> > ---
> > .../sysfs-class-led-trigger-dev-hw-mode | 8 +
> > drivers/leds/Kconfig | 10 +
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-hw-controlled.c | 227
> > ++++++++++++++++++ include/linux/leds-hw-controlled.h |
> > 74 ++++++ 5 files changed, 320 insertions(+)
> > create mode 100644
> > Documentation/ABI/testing/sysfs-class-led-trigger-dev-hw-mode
> > create mode 100644 drivers/leds/leds-hw-controlled.c create mode
> > 100644 include/linux/leds-hw-controlled.h
>
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 1c181df24eae4..5e47ab21aafb4 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -49,6 +49,16 @@ config LEDS_BRIGHTNESS_HW_CHANGED
> >
> > See Documentation/ABI/testing/sysfs-class-led for
> > details.
> > +config LEDS_HW_CONTROLLED
> > + bool "API for LEDs that can be controlled by hardware"
> > + depends on LEDS_CLASS
>
> depends on OF || COMPILE_TEST
> ?
>
I specifically did not add OF dependency so that this can be also used
on non-OF systems. A device driver may register such LED itself...
That's why hw_controlled_led_brightness_set symbol is exported.
Do you think I shouldn't do it?
> > + select LEDS_TRIGGERS
> > + help
> > + This option enables support for a generic API via which
> > other drivers
> > + can register LEDs that can be put into hardware
> > controlled mode, eg.
>
> e.g.
>
This will need to be changed on multiple places, thanks.
> > + a LED connected to an ethernet PHY can be configured to
> > blink on
>
> an LED
>
This as well
> > + network activity.
> > +
> > comment "LED drivers"
> >
> > config LEDS_88PM860X
>
>
> > diff --git a/include/linux/leds-hw-controlled.h
> > b/include/linux/leds-hw-controlled.h new file mode 100644
> > index 0000000000000..2c9b8a06def18
> > --- /dev/null
> > +++ b/include/linux/leds-hw-controlled.h
> > @@ -0,0 +1,74 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Generic API for LEDs that can be controlled by hardware (eg. by
> > an ethernet PHY chip)
> > + *
> > + * Copyright (C) 2020 Marek Behun <marek.behun@....cz>
> > + */
> > +#ifndef _LINUX_LEDS_HW_CONTROLLED_H_
> > +#define _LINUX_LEDS_HW_CONTROLLED_H_
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/leds.h>
> > +
> > +struct hw_controlled_led {
> > + struct led_classdev cdev;
> > + const struct hw_controlled_led_ops *ops;
> > + struct mutex lock;
> > +
> > + /* these members are filled in by OF if OF is enabled */
> > + int addr;
> > + bool active_low;
> > + bool tristate;
> > +
> > + /* also filled in by OF, but changed by led_set_hw_mode
> > operation */
> > + const char *hw_mode;
> > +
> > + void *priv;
> > +};
> > +#define led_cdev_to_hw_controlled_led(l) container_of(l, struct
> > hw_controlled_led, cdev) +
> > +/* struct hw_controlled_led_ops: Operations on LEDs that can be
> > controlled by HW
> > + *
> > + * All the following operations must be implemented:
> > + * @led_init: Should initialize the LED from OF data (and sanity
> > check whether they are correct).
> > + * This should also change led->cdev.max_brightness, if
> > the value differs from default,
> > + * which is 1.
> > + * @led_brightness_set: Sets brightness.
> > + * @led_iter_hw_mode: Iterates available HW control mode names for
> > this LED.
> > + * @led_set_hw_mode: Sets HW control mode to value specified by
> > given name.
> > + * @led_get_hw_mode: Returns current HW control mode name.
> > + */
>
> Convert that struct info to kernel-doc?
>
Will look into this.
Thanks.
> > +struct hw_controlled_led_ops {
> > + int (*led_init)(struct device *dev, struct
> > hw_controlled_led *led);
> > + int (*led_brightness_set)(struct device *dev, struct
> > hw_controlled_led *led,
> > + enum led_brightness brightness);
> > + const char *(*led_iter_hw_mode)(struct device *dev, struct
> > hw_controlled_led *led,
> > + void **iter);
> > + int (*led_set_hw_mode)(struct device *dev, struct
> > hw_controlled_led *led,
> > + const char *mode);
> > + const char *(*led_get_hw_mode)(struct device *dev, struct
> > hw_controlled_led *led); +};
> > +
>
> > +
> > +#endif /* _LINUX_LEDS_HW_CONTROLLED_H_ */
>
> thanks.
Powered by blists - more mailing lists