Re: [PATCH v2] HID: lg-g15 - Add support for Logitech G13.

["Leo L. Schwab" <ewhac@ewhac.org>]

On Thu, Aug 14, 2025 at 05:09:09PM +0800, Kate Hsuan wrote:
> Thank you for your work.
>
	Thank you for your feedback.  And thank you for collecting all your
comments into one post.

> On Tue, Aug 12, 2025 at 2:57 PM Leo L. Schwab <ewhac@ewhac.org> wrote:
> The comment should in C comments, for example
>  struct input_dev *input_js;  /*joystick device for G13*/
>
	Will sweep all those up.

> > +static int lg_g13_kbd_led_set(struct led_classdev *led_cdev, enum led_brightness brightness)
> > +{
> > +       struct led_classdev_mc *mc = lcdev_to_mccdev(led_cdev);
> > +       struct lg_g15_led *g15_led =
> > +               container_of(mc, struct lg_g15_led, mcdev);
> > +       struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent);
> > +
> > +       /* Ignore LED off on unregister / keyboard unplug */
> > +       if (led_cdev->flags & LED_UNREGISTERING)
> > +               return 0;
> > +
> > +       guard(mutex)(&g15->mutex);
> guard() can be moved to lg_g13_kbd_led_write() to ensure the code is
> protected by a mutex lock when lg_g13_kbd_led_write() is called.
>
	I was mimicking the existing structure of the G15 and G510 code,
which I assumed was set up that way for a reason.  Will move this.

> > +static int lg_g13_event(struct lg_g15_data *g15, u8 const *data)
> > +{
> > +       struct g13_input_report const * const rep = (struct g13_input_report *) data;
> > +       int i, val;
> > +       bool hw_brightness_changed;
> Remove unused variable.
>
	I will be slightly restructuring this.

> >         switch (g15->model) {
> > +       case LG_G13:
> > +               /*
> > +                * Some usermode libraries tend to ignore devices that don't
> > +                * "look like" a joystick.  Create additional input device
> > +                * dedicated as joystick.
> > +                */
> Nit.
> Improve the comment and describe the hardware and the variable
> settings below in brief.

	I'll wordsmith this.  It'll get a bit wordier, though...

> Some style and comment style issues are pointed out, and I'll start to
> test this work after I receive my G13.
>
	If anything explodes, please let me know right away.

					Schwab