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;
MIME-Version: 1.0
References: <20220805081909.10962-1-luke@ljones.dev>
In-Reply-To: <20220805081909.10962-1-luke@ljones.dev>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Sat, 6 Aug 2022 11:10:37 +0200
Message-ID: <CAHp75Vd4iEGkAE1Ds_Zyqi0P+gQbOyqvGoJctpOHNmMD7cUG0g@mail.gmail.com>
Subject: Re: [PATCH 0/5] asus-wmi: Add support for RGB keyboards
To:     "Luke D. Jones" <luke@ljones.dev>
Cc:     Hans de Goede <hdegoede@redhat.com>,
        Mark Gross <markgross@kernel.org>,
        Platform Driver <platform-driver-x86@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Aug 5, 2022 at 10:20 AM Luke D. Jones <luke@ljones.dev> wrote:
>
> This is a patch series to add RGB support for ASUS laptops.
> The laptops with this RGB tend to be the TUF series of gamer laptops.
>
> The first step is initial bringup of support using the multicolor LED API.
>
> These types of keyboards implement a slightly more complex interface than
> just RGB control however - they also have modes with can be static LED,
> blinking, rainbow, color cycles, and more. They also have some custom
> animations that can play depending on device state, such as suspended
> playing a fancy colour cycle, or playing a "wave" animation.
>
> Two of the patches add support for these features.
>
> The last patch adds documentation in:
> Documentation/ABI/testing/sysfs-platform-asus-wmi
>
> Some notes:
>
> - this patch series obsoletes the previous RGB patches by myself
>
> - it is not possible to add attribute groups to multicolor LED as
>   they get overwritten by `led_multicolor_groups` in
>   `led_classdev_multicolor_register_ext`.
>
> - the methods for RGB control do not provide a way to fetch exisiting
>   state, so these methods are WO.
>
> - There is an existing `asus::kbd_backlight`, this provides a 4-step
>   brightness to the RGB (off,low,med,high) individually to multicolor.
>   I was unsure of the effect of adding a similar path so have used the
>   `asus::multicolour::kbd_backlight` name to be clear about purpose.
>   If the `asus::kbd_backlight` is off, then no RGB is shown at all.\
>
> I'm hopeful that this patch series addresses all previous feedback related
> to the obsoleted patches.

There are so many patches and versioning of all of this is completely
broken. You really have to clean up the mess and realize what version
of this is. To me it looks like this series is v5 or so of the
previously sent patch(es). Also you missed the changelog between
versions so we can see what you have done from vX to vX+1 for the
whole range (1 ... X+1).

-- 
With Best Regards,
Andy Shevchenko