Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 23 Mar 2022 20:21:26 -0600
From:   Manuel =?iso-8859-1?Q?Sch=F6nlaub?= <manuel.schoenlaub@gmail.com>
To:     Pavel Machek <pavel@ucw.cz>
Cc:     lains@riseup.net, jikos@kernel.org, benjamin.tissoires@redhat.com,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] HID: logitech-hidpp: support Color LED feature (8071).
Message-ID: <YjvVptj8exUCD7sx@hermes>
References: <Yifr4etBFPu1a2Ct@hermes>
 <20220323210423.GA10741@duo.ucw.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220323210423.GA10741@duo.ucw.cz>
Precedence: bulk

On Wed, Mar 23, 2022 at 10:04:23PM +0100, Pavel Machek wrote:
> Hi!
> 
> > The HID++ protocol allows to set multicolor (RGB) to a static color.
> > Multiple of such LED zones per device are supported.
> > This patch exports said LEDs so that they can be set from userspace.
> > 
> > Signed-off-by: Manuel Sch?nlaub <manuel.schoenlaub@gmail.com>
> 
> Please cc LEDs stuff to the LED lists.
> 

Will do. Though it seems like first we should discuss whether the kernel
in fact is the right place, no?

> > +static int hidpp_mc_led_register(struct hidpp_device *hidpp_dev,
> > +				 struct led_classdev_mc *mc_dev,
> > +				 int zone)
> > +{
> > +	struct hid_device *hdev = hidpp_dev->hid_dev;
> > +	struct mc_subled *mc_led_info;
> > +	struct led_classdev *cdev;
> > +	int ret;
> > +
> > +	mc_led_info = devm_kmalloc_array(&hdev->dev, 3,
> > +					 sizeof(*mc_led_info),
> > +					 GFP_KERNEL | __GFP_ZERO);
> > +	if (!mc_led_info)
> > +		return -ENOMEM;
> > +
> > +	mc_led_info[0].color_index = LED_COLOR_ID_RED;
> > +	mc_led_info[1].color_index = LED_COLOR_ID_GREEN;
> > +	mc_led_info[2].color_index = LED_COLOR_ID_BLUE;
> > +
> > +	mc_dev->subled_info = mc_led_info;
> > +	mc_dev->num_colors = 3;
> > +
> > +	cdev = &mc_dev->led_cdev;
> > +	cdev->name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
> > +				    "%s:rgb:indicator-%d", hdev->uniq, zone);
> 
> So this is keyboard backlight? We should add the documentation at the
> very least, so that other drivers use same name.
> 
> Best regards,
> 								Pavel
> 
> -- 
> People of Russia, stop Putin before his war on Ukraine escalates.

I do not own a Logitech keyboard, but some mice. There are RGB leds
that you can normally control with Windows software.

I'd suppose (but could not verify) that supported keyboards by Logitech
work with the same feature.

Best Regards,

Manuel