Marek
On 7/15/20 2:03 PM, Marek Behún wrote:
On Wed, 15 Jul 2020 08:27:51 -0500
Dan Murphy <dmurphy@xxxxxx> wrote:
Can this be built as a module?
Yes. But only if MC framework is compiled in. If MC framework is
compiled as module as well, this results in
FATAL: modpost: GPL-incompatible module led-class-multicolor.ko uses
GPL-only symbol 'led_set_brightness'
Hmm. That is interesting
<snip>
+ led->subled_info[0].color_index = LED_COLOR_ID_RED;
+ led->subled_info[0].channel = 0;
+ led->subled_info[1].color_index = LED_COLOR_ID_GREEN;
+ led->subled_info[1].channel = 1;
+ led->subled_info[2].color_index = LED_COLOR_ID_BLUE;
+ led->subled_info[2].channel = 2;
OK this is why you may want to have sub-nodes. Where reg is the
channel and color is the color. Then you can do a for_each_child.
Rob says that it doesn't need to be in DT if the controller only
supports RGB LEDs. This controller is only in Turris Omnia which was
never made with another kind of LEDs. So I think it is pointless and
would only grow the DT and complicate the driver.
OK I know there was a discussion on this
+ cdev->max_brightness = 255;
This is not needed. It is defaulted to LED_FULL in led_class
This was discussed last year and resulted in LED_FULL being
declared obsolete in the header file.
No I am referring to setting the max_brightness to 255 the LED class
sets this to 255 if the value is not set.
+/*
+ * On the front panel of the Turris Omnia router there is also a
button which can be used to control
+ * the intensity of all the LEDs at once, so that if they are too
bright, user can dim them.
+ * The microcontroller cycles between 8 levels of this global
brightness (from 100% to 0%), but this
+ * setting can have any integer value between 0 and 100.
+ * It is usable to be able to change this value from software, so
that it does not start at 100%
This does not make sense.
It does. The user changes the brightness of all 12 LEDs with the button
to his liking and wants to have the same setting after powering
the router on again.
No the english does not make sense
" It is usable to be able to change this value from software, so
that it does not start at 100%"
"It is usable" is not really clear.
+ * after every power on and annoy the user.
+ * We expose this setting via a sysfs attribute file called
"brightness". This file lives in the
+ * device directory of the LED controller, not an individual LED,
so it should not confuse users.
+ */
Sorry if this has been discussed already
This seems a bit wonky. You are overriding the brightness set by the
LED class.
I am not. Pressing the button does not change the brightness read from
the /sys/class/leds/<LED>/brightness file. This is different brightness,
it is above the classic brightnes in the PWM hierarchy in the
microcontroller. I discussed this with Pavel and he said we can call
this file brightness as well (since it is brightness of the whole
panel), and the file does not reside in /sys/class/leds/<LED> directory.
OK then there needs to be some ABI documentation no?
Dan
