Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Sat, 30 Apr 2022 16:51:45 +0200
From:   Marijn Suijten <marijn.suijten@somainline.org>
To:     Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     Pavel Machek <pavel@ucw.cz>,
        Thierry Reding <thierry.reding@gmail.com>,
        Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>, Lee Jones <lee.jones@linaro.org>,
        Luca Weiss <luca@z3ntu.xyz>,
        Doug Anderson <dianders@chromium.org>,
        Rob Herring <robh+dt@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>, linux-leds@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-pwm@vger.kernel.org,
        linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v14 2/2] leds: Add driver for Qualcomm LPG
Message-ID: <20220430145145.2zlxahmtjwyzezkg@SoMainline.org>
References: <20220303214300.59468-1-bjorn.andersson@linaro.org>
 <20220303214300.59468-2-bjorn.andersson@linaro.org>
 <20220430123502.7od55knvxkw3sv6s@SoMainline.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220430123502.7od55knvxkw3sv6s@SoMainline.org>
Precedence: bulk

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