linux-kernel - Re: [PATCH v14 2/2] leds: Add driver for Qualcomm LPG

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: <20220430145145.2zlxahmtjwyzezkg@SoMainline.org>
Date:   Sat, 30 Apr 2022 16:51:45 +0200
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Pavel Machek <pavel@....cz>,
        Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, Lee Jones <lee.jones@...aro.org>,
        Luca Weiss <luca@...tu.xyz>,
        Doug Anderson <dianders@...omium.org>,
        Rob Herring <robh+dt@...nel.org>,
        Jonathan Corbet <corbet@....net>, linux-leds@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-pwm@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v14 2/2] leds: Add driver for Qualcomm LPG

On 2022-04-30 14:35:04, Marijn Suijten wrote:
[..]
> > +static int lpg_add_led(struct lpg *lpg, struct device_node *np)
> > +{
> > +	struct led_init_data init_data = {};
> > +	struct led_classdev *cdev;
> > +	struct device_node *child;
> > +	struct mc_subled *info;
> > +	struct lpg_led *led;
> > +	const char *state;
> > +	int num_channels;
> > +	u32 color = 0;
> > +	int ret;
> > +	int i;
> > +
> > +	ret = of_property_read_u32(np, "color", &color);
> > +	if (ret < 0 && ret != -EINVAL) {
> > +		dev_err(lpg->dev, "failed to parse \"color\" of %pOF\n", np);
> > +		return ret;
> > +	}
> > +
> > +	if (color == LED_COLOR_ID_RGB)
> > +		num_channels = of_get_available_child_count(np);
> > +	else
> > +		num_channels = 1;
> > +
> > +	led = devm_kzalloc(lpg->dev, struct_size(led, channels, num_channels), GFP_KERNEL);
> > +	if (!led)
> > +		return -ENOMEM;
> > +
> > +	led->lpg = lpg;
> > +	led->num_channels = num_channels;
> > +
> > +	if (color == LED_COLOR_ID_RGB) {
> > +		info = devm_kcalloc(lpg->dev, num_channels, sizeof(*info), GFP_KERNEL);
> > +		if (!info)
> > +			return -ENOMEM;
> > +		i = 0;
> > +		for_each_available_child_of_node(np, child) {
> > +			ret = lpg_parse_channel(lpg, child, &led->channels[i]);
> > +			if (ret < 0)
> > +				return ret;
> > +
> > +			info[i].color_index = led->channels[i]->color;
> > +			info[i].intensity = 0;
> 
> This struct also has a "channel" field that doesn't seem to be used
> anywhere.  Should we still initialize it to something sensible, or is it
> better reported upstream for removal?

Never mind the second bit: lp552[13] uses the `channel` field internally
to store a custom index.  This LPG driver simply iterates over all the
led channels (stored in the LPG struct, with all necessary info) in the
same order as the subleds in the multicolor led device, so it's most
likely of no use for us.

- Marijn