linux-kernel - Re: [PATCH v13 04/18] leds: multicolor: Introduce a multicolor class definition

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <141a58aa-654e-6edf-b84c-b451b7b2d96b@gmail.com>
Date:   Sat, 19 Oct 2019 20:33:10 +0200
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Dan Murphy <dmurphy@...com>, pavel@....cz
Cc:     linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v13 04/18] leds: multicolor: Introduce a multicolor class
 definition

Dan,

On 10/16/19 5:59 PM, Dan Murphy wrote:
> Introduce a multicolor class that groups colored LEDs
> within a LED node.
> 
> The multi color class groups monochrome LEDs and allows controlling two
> aspects of the final combined color: hue and lightness. The former is
> controlled via <color>_intensity files and the latter is controlled
> via brightness file.
> 
> Signed-off-by: Dan Murphy <dmurphy@...com>
> ---
[...]
> +static int led_multicolor_init_color(struct led_classdev_mc *mcled_cdev,
> +				     int color_id)
> +{
> +	struct led_classdev *led_cdev = mcled_cdev->led_cdev;
> +	struct led_mc_color_entry *mc_priv;
> +	char *intensity_file_name;
> +	char *max_intensity_file_name;
> +	size_t len;
> +	int ret;
> +
> +	mc_priv = devm_kzalloc(led_cdev->dev, sizeof(*mc_priv), GFP_KERNEL);
> +	if (!mc_priv)
> +		return -ENOMEM;
> +
> +	mc_priv->led_color_id = color_id;
> +	mc_priv->mcled_cdev = mcled_cdev;
> +
> +	sysfs_attr_init(&mc_priv->intensity_attr.attr);
> +	len = strlen(led_colors[color_id]) + strlen(INTENSITY_NAME) + 1;
> +	intensity_file_name = kzalloc(len, GFP_KERNEL);
> +	if (!intensity_file_name)
> +		return -ENOMEM;
> +
> +	snprintf(intensity_file_name, len, "%s%s",
> +		 led_colors[color_id], INTENSITY_NAME);
> +	mc_priv->intensity_attr.attr.name = intensity_file_name;
> +	mc_priv->intensity_attr.attr.mode = 0644;
> +	mc_priv->intensity_attr.store = intensity_store;
> +	mc_priv->intensity_attr.show = intensity_show;
> +	ret = sysfs_add_file_to_group(&led_cdev->dev->kobj,
> +				      &mc_priv->intensity_attr.attr,
> +				      led_color_group.name);
> +	if (ret)
> +		goto intensity_err_out;
> +
> +	sysfs_attr_init(&mc_priv->max_intensity_attr.attr);
> +	len = strlen(led_colors[color_id]) + strlen(MAX_INTENSITY_NAME) + 1;
> +	max_intensity_file_name = kzalloc(len, GFP_KERNEL);
> +	if (!max_intensity_file_name) {
> +		ret = -ENOMEM;
> +		goto intensity_err_out;
> +	}
> +
> +	snprintf(max_intensity_file_name, len, "%s%s",
> +		 led_colors[color_id], MAX_INTENSITY_NAME);
> +	mc_priv->max_intensity_attr.attr.name = max_intensity_file_name;
> +	mc_priv->max_intensity_attr.attr.mode = 0444;
> +	mc_priv->max_intensity_attr.show = max_intensity_show;
> +	ret = sysfs_add_file_to_group(&led_cdev->dev->kobj,
> +				      &mc_priv->max_intensity_attr.attr,
> +				      led_color_group.name);
> +	if (ret)
> +		goto max_intensity_err_out;
> +
> +	mc_priv->max_intensity = LED_FULL;
> +	list_add_tail(&mc_priv->list, &mcled_cdev->color_list);

We don't need the list here since our collection of color LEDs will
be fixed. Instead of the list we can do with a dynamically allocated
array of a size depending on available color LEDs.

It will allow also to get rid of lp55xx_map_channel() since random
access to array elements will be possible via lookup tables mapping
colors to array element id.

And regarding my amendments to the DT parser - attached is the
patch for your patch, that is compile-tested.

> +
> +max_intensity_err_out:
> +	kfree(max_intensity_file_name);
> +intensity_err_out:
> +	kfree(intensity_file_name);
> +	return ret;
> +}
> +
>
-- 
Best regards,
Jacek Anaszewski

View attachment "0001-DT-parser-amendments.patch" of type "text/x-patch" (5379 bytes)