Skip to contentSkip to conversation

leds: add NCT6795D driver

PATCH In review Alexandre Courbot 3 participants 9 messages Jul 13, 2020 – Jul 20, 2020 LED

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 leds: add NCT6795D driver

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Also add self as maintainer.

---
 MAINTAINERS                  |   6 +
 drivers/leds/Kconfig         |   9 +
 drivers/leds/Makefile        |   1 +
 drivers/leds/leds-nct6795d.c | 447 +++++++++++++++++++++++++++++++++++
 4 files changed, 463 insertions(+)
 create mode 100644 drivers/leds/leds-nct6795d.c
MAINTAINERS +6 -0
index b4a43a9e7fbc1..118c347ec2990 100644
@@ -11792,6 +11792,12 @@ S:	Maintained
 F:	Documentation/hwmon/nct6775.rst
 F:	drivers/hwmon/nct6775.c
 
+NCT6795D LED CONTROLLER DRIVER
+M:	Alexandre Courbot <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/leds/leds-nct6795d.c
+
 NETDEVSIM
 M:	Jakub Kicinski <[email protected]>
 S:	Maintained
drivers/leds/Kconfig +9 -0
index ed943140e1fd4..aa41493377947 100644
@@ -886,6 +886,15 @@ config LEDS_SGM3140
 	  This option enables support for the SGM3140 500mA Buck/Boost Charge
 	  Pump LED Driver.
 
+config LEDS_NCT6795D
+	tristate "LED support for NCT6795D chipsets"
+	depends on LEDS_CLASS
+	help
+	  Enabled support for the leds feature of the NCT6795D chips.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-nct6795d.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
drivers/leds/Makefile +1 -0
index d6b8a792c9367..04fcb2bb1e3c6 100644
@@ -65,6 +65,7 @@ obj-$(CONFIG_LEDS_MIKROTIK_RB532)	+= leds-rb532.o
 obj-$(CONFIG_LEDS_MLXCPLD)		+= leds-mlxcpld.o
 obj-$(CONFIG_LEDS_MLXREG)		+= leds-mlxreg.o
 obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
+obj-$(CONFIG_LEDS_NCT6795D)		+= leds-nct6795d.o
 obj-$(CONFIG_LEDS_NET48XX)		+= leds-net48xx.o
 obj-$(CONFIG_LEDS_NETXBIG)		+= leds-netxbig.o
 obj-$(CONFIG_LEDS_NIC78BX)		+= leds-nic78bx.o
drivers/leds/leds-nct6795d.c new file +447 -0

447 lines not on this page · raw message

Signature · 1 line
2.27.0
#1 lore raw

Hi!

Quoted 4 lines

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Ok, let me take a look. What entries does it present in /sys?

We'll want you to switch to multicolor framework.

Quoted 1 line

Also add self as maintainer.

That will need to be separate patch, I'd be asking for trouble if I took that.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#2 lore raw

Hi Pavel,

Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Ok, let me take a look. What entries does it present in /sys?

Right now these 3 directories in /sys/class/leds:

nct6795d:blue:
nct6795d:green:
nct6795d:red:

with the usual suspects brightness and max_brightness in each. I am not 100% sure I got the names right so please let me know if that is not correct.

Quoted 2 lines

We'll want you to switch to multicolor framework.

Most definitely. Last time I checked it was not merged yet though?

Quoted 5 lines
Quoted 2 lines

Also add self as maintainer.

That will need to be separate patch, I'd be asking for trouble if I took that.

Note that I am also comfortable if you prefer to take care of this, just didn't want to look like I was throwing this code at you without following up on its maintenance.

Cheers,
Alex.

#3 lore raw

Hi Alexandre,

Quoted 15 lines · Alexandre Courbot

Hi Pavel,

Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Ok, let me take a look. What entries does it present in /sys?

Right now these 3 directories in /sys/class/leds:

Quoted 7 lines

nct6795d:blue:
nct6795d:green:
nct6795d:red:

with the usual suspects brightness and max_brightness in each. I am not 100% sure I got the names right so please let me know if that is not correct.

You miss LED function, that should be in the second section.

Signature · 2 lines
Best regards,
Jacek Anaszewski
#4 lore raw

Hi Jacek,

Quoted 23 lines · Jacek Anaszewski

Hi Alexandre,

Quoted 19 lines · Alexandre Courbot

Hi Pavel,

Quoted 9 lines · Pavel Machek

Hi!

Quoted 4 lines

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Ok, let me take a look. What entries does it present in /sys?

Right now these 3 directories in /sys/class/leds:

nct6795d:blue:
nct6795d:green:
nct6795d:red:

Quoted 5 lines
Quoted 3 lines

with the usual suspects brightness and max_brightness in each. I am not 100% sure I got the names right so please let me know if that is not correct.

You miss LED function, that should be in the second section.

The reason for not having a function at the moment is that I took a look at include/dt-bindings/leds/common.h and could not find any function that could reasonably apply. This basically controls a RGB connector on the motherboard which serves no particular function - you can plug a RGB fan or anything else you want and control it in any fashion. Is there a function that applies to this use-case?

#5 lore raw

Hi!

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 6 lines
Quoted 4 lines

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Ok, let me take a look. What entries does it present in /sys?

Right now these 3 directories in /sys/class/leds:

nct6795d:blue:
nct6795d:green:
nct6795d:red:

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

with the usual suspects brightness and max_brightness in each. I am not 100% sure I got the names right so please let me know if that is not correct.

You miss LED function, that should be in the second section.

third section?

Quoted 6 lines

The reason for not having a function at the moment is that I took a look at include/dt-bindings/leds/common.h and could not find any function that could reasonably apply. This basically controls a RGB connector on the motherboard which serves no particular function - you can plug a RGB fan or anything else you want and control it in any fashion. Is there a function that applies to this use-case?

This is normally used for motherboard lightning, right? I believe this is getting common on gaming boards, and we want common support for that.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#6 lore raw
Quoted 15 lines · Pavel Machek

Hi!

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 6 lines
Quoted 4 lines

Add support for the LED feature of the NCT6795D chip found on some motherboards, notably MSI ones. The LEDs are typically used using a RGB connector so this driver creates one LED device for each color component.

Ok, let me take a look. What entries does it present in /sys?

Right now these 3 directories in /sys/class/leds:

nct6795d:blue:
nct6795d:green:
nct6795d:red:

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

with the usual suspects brightness and max_brightness in each. I am not 100% sure I got the names right so please let me know if that is not correct.

You miss LED function, that should be in the second section.

third section?

Ekhm, right.

Quoted 7 lines
Quoted 7 lines

The reason for not having a function at the moment is that I took a look at include/dt-bindings/leds/common.h and could not find any function that could reasonably apply. This basically controls a RGB connector on the motherboard which serves no particular function - you can plug a RGB fan or anything else you want and control it in any fashion. Is there a function that applies to this use-case?

According to common LED bindings you should propose a new function if none of the existing ones fits your needs.

Quoted 3 lines

This is normally used for motherboard lightning, right? I believe this is getting common on gaming boards, and we want common support for that.

I agree.

Signature · 2 lines
Best regards,
Jacek Anaszewski
#7 lore raw
Quoted 8 lines · Jacek Anaszewski
Quoted 7 lines
Quoted 7 lines

The reason for not having a function at the moment is that I took a look at include/dt-bindings/leds/common.h and could not find any function that could reasonably apply. This basically controls a RGB connector on the motherboard which serves no particular function - you can plug a RGB fan or anything else you want and control it in any fashion. Is there a function that applies to this use-case?

Quoted 8 lines

According to common LED bindings you should propose a new function if none of the existing ones fits your needs.

Quoted 3 lines

This is normally used for motherboard lightning, right? I believe this is getting common on gaming boards, and we want common support for that.

I agree.

These boards are indeed far from being a rarity so having a function for them (maybe named LED_FUNCTION_RGB_HEADER?) makes sense IMHO. I'll submit a patch for that with the next revision.

Speaking of which, after looking at the multicolor patchset it is pretty obvious that it would be a much better way to expose this RGB header, so I think I will wait until it is merged and adapt the driver to use it.

Thanks for the feedback!
Alex.

#8 lore raw

Hi!

Quoted 7 lines
Quoted 7 lines

According to common LED bindings you should propose a new function if none of the existing ones fits your needs.

Quoted 3 lines

This is normally used for motherboard lightning, right? I believe this is getting common on gaming boards, and we want common support for that.

Quoted 5 lines
Quoted 1 line

I agree.

These boards are indeed far from being a rarity so having a function for them (maybe named LED_FUNCTION_RGB_HEADER?) makes sense IMHO. I'll submit a patch for that with the next revision.

I'd call it something LED_FUNCTION_INTERNALS, and make it clear this is for illuminating machine internals.

Quoted 4 lines

Speaking of which, after looking at the multicolor patchset it is pretty obvious that it would be a much better way to expose this RGB header, so I think I will wait until it is merged and adapt the driver to use it.

Take a look at LED tree today, or linux-next tommorow. Basics should be there.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html