Hi!OK
Introduce a multicolor class that groups colored LEDs? "This file contains array of integers".
within a LED node.
+What: /sys/class/leds/<led>/multi_intensity
+Date: March 2020
+KernelVersion: 5.8
+Contact: Dan Murphy <dmurphy@xxxxxx>
+Description: read/write
+ Intensity level for the LED color within an array of integers.
OK
+ The intensities for each color must be entered based on theThis does not make sense to me. "Order of components is described by
+ multi_index array.
the multi_index array"?
The max_intensity should not exceed"max_intensity" -> "maximum intensity"?
This is redundant and will be removed.
+ /sys/class/leds/<led>/max_brightness.?
+Multicolor Class Brightness Control
+===================================
+The multicolor class framework will calculate each monochrome LEDs intensity.
It is a new line the comment is incorrect I can remove the comment or update the comment to account for the new line
+static ssize_t multi_intensity_store(struct device *dev,space? I'd expect \n there. And it would be good to verify it is
+ struct device_attribute *intensity_attr,
+ const char *buf, size_t size)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
+ int nrchars, offset = 0;
+ int intensity_value[LED_COLOR_ID_MAX];
+ int i;
+ ssize_t ret;
+
+ mutex_lock(&led_cdev->led_access);
+
+ for (i = 0; i < mcled_cdev->num_colors; i++) {
+ ret = sscanf(buf + offset, "%i%n",
+ &intensity_value[i], &nrchars);
+ if (ret != 1) {
+ dev_dbg(led_cdev->dev,
+ "Incorrect number of LEDs expected %i values intensity was not applied\n",
+ mcled_cdev->num_colors);
+ ret = -EINVAL;
+ goto err_out;
+ }
+ offset += nrchars;
+ }
+
+ /* account for the space at the end of the buffer */
+ offset++;
indeed \n, so that for example "0 0 0b" is not accepted.
Please remove the dev_dbg()s that can be triggered by userspace. WeRemoved
don't want users spamming the logs.
+static ssize_t multi_intensity_show(struct device *dev,We should not really put " " before newline.
+ struct device_attribute *intensity_attr,
+ char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
+ int len = 0;
+ int i;
+
+ for (i = 0; i < mcled_cdev->num_colors; i++) {
+ len += sprintf(buf + len, "%d",
+ mcled_cdev->subled_info[i].intensity);
+ len += sprintf(buf + len, " ");
+static ssize_t multi_index_show(struct device *dev,We should not really put " " before newline.
+ struct device_attribute *multi_index_attr,
+ char *buf)
+{
+ for (i = 0; i < mcled_cdev->num_colors; i++) {
+ index = mcled_cdev->subled_info[i].color_index;
+ len += sprintf(buf + len, "%s", led_colors[index]);
+ len += sprintf(buf + len, " ");
+ }
+{It is plain int, so you may want to check for <= 0? Or maybe make it
+ struct led_classdev *led_cdev;
+
+ if (!mcled_cdev)
+ return -EINVAL;
+
+ if (!mcled_cdev->num_colors)
+ return -EINVAL;
unsigned?
+MODULE_LICENSE("GPL v2");If your legal department allows that, GPL v2+ would be preffered
(globally).
+struct mc_subled {Would some "unsigned"s make sense here to cut number of corner cases?
+ int color_index;
+ int brightness;
+ int intensity;
+ int channel;
+};
+
+struct led_classdev_mc {
+ /* led class device */
+ struct led_classdev led_cdev;
+ int num_colors;
+
+ struct mc_subled *subled_info;
+};
Best regards,
Pavel