* [PATCH v2 0/1] asus-wmi: add support variant of TUF RGB
@ 2024-03-20 1:14 Luke D. Jones
2024-03-20 1:14 ` [PATCH v2 1/1] platform/x86: " Luke D. Jones
0 siblings, 1 reply; 4+ messages in thread
From: Luke D. Jones @ 2024-03-20 1:14 UTC (permalink / raw
To: hdegoede
Cc: corentin.chary, ilpo.jarvinen, platform-driver-x86, linux-kernel,
Luke D. Jones
Changelog:
- v2
- remove unrequired kbd_rgb_mode_available check
-v1
- add missing define for WMI method
Luke D. Jones (1):
platform/x86: asus-wmi: add support variant of TUF RGB
drivers/platform/x86/asus-wmi.c | 13 +++++++++++--
include/linux/platform_data/x86/asus-wmi.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] platform/x86: asus-wmi: add support variant of TUF RGB
2024-03-20 1:14 [PATCH v2 0/1] asus-wmi: add support variant of TUF RGB Luke D. Jones
@ 2024-03-20 1:14 ` Luke D. Jones
2024-03-20 11:36 ` Ilpo Järvinen
0 siblings, 1 reply; 4+ messages in thread
From: Luke D. Jones @ 2024-03-20 1:14 UTC (permalink / raw
To: hdegoede
Cc: corentin.chary, ilpo.jarvinen, platform-driver-x86, linux-kernel,
Luke D. Jones
Adds support for a second TUF RGB wmi call that some versions of the TUF
laptop come with. Also adjusts existing support to select whichever is
available.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/platform/x86/asus-wmi.c | 13 +++++++++++--
include/linux/platform_data/x86/asus-wmi.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index b9a2fb8007c0..0d8a2b82cc06 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -280,6 +280,7 @@ struct asus_wmi {
bool nv_temp_tgt_available;
bool kbd_rgb_mode_available;
+ u32 kbd_rgb_dev;
bool kbd_rgb_state_available;
bool throttle_thermal_policy_available;
@@ -870,6 +871,7 @@ static ssize_t kbd_rgb_mode_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct asus_wmi *asus = dev_get_drvdata(dev);
u32 cmd, mode, r, g, b, speed;
int err;
@@ -906,7 +908,7 @@ static ssize_t kbd_rgb_mode_store(struct device *dev,
speed = 0xeb;
}
- err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
+ err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, asus->kbd_rgb_dev,
cmd | (mode << 8) | (r << 16) | (g << 24), b | (speed << 8), NULL);
if (err)
return err;
@@ -4515,7 +4517,6 @@ static int asus_wmi_add(struct platform_device *pdev)
asus->egpu_enable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU);
asus->egpu_connect_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_EGPU_CONNECTED);
asus->dgpu_disable_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_DGPU);
- asus->kbd_rgb_mode_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE);
asus->kbd_rgb_state_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_STATE);
asus->ppt_pl2_sppt_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PPT_PL2_SPPT);
asus->ppt_pl1_spl_available = asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_PPT_PL1_SPL);
@@ -4544,6 +4545,14 @@ static int asus_wmi_add(struct platform_device *pdev)
asus->gpu_mux_dev = ASUS_WMI_DEVID_GPU_MUX_VIVO;
}
+ if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
+ asus->kbd_rgb_mode_available = true;
+ asus->kbd_rgb_dev = ASUS_WMI_DEVID_TUF_RGB_MODE;
+ } else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE2)) {
+ asus->kbd_rgb_mode_available = true;
+ asus->kbd_rgb_dev = ASUS_WMI_DEVID_TUF_RGB_MODE2;
+ }
+
err = fan_boost_mode_check_present(asus);
if (err)
goto fail_fan_boost_mode;
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index b48b024dd844..3e9a01467c67 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -132,6 +132,7 @@
/* TUF laptop RGB modes/colours */
#define ASUS_WMI_DEVID_TUF_RGB_MODE 0x00100056
+#define ASUS_WMI_DEVID_TUF_RGB_MODE2 0x0010005A
/* TUF laptop RGB power/state */
#define ASUS_WMI_DEVID_TUF_RGB_STATE 0x00100057
--
2.44.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] platform/x86: asus-wmi: add support variant of TUF RGB
2024-03-20 1:14 ` [PATCH v2 1/1] platform/x86: " Luke D. Jones
@ 2024-03-20 11:36 ` Ilpo Järvinen
2024-03-20 20:01 ` Luke Jones
0 siblings, 1 reply; 4+ messages in thread
From: Ilpo Järvinen @ 2024-03-20 11:36 UTC (permalink / raw
To: Luke D. Jones; +Cc: Hans de Goede, corentin.chary, platform-driver-x86, LKML
[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]
On Wed, 20 Mar 2024, Luke D. Jones wrote:
> Adds support for a second TUF RGB wmi call that some versions of the TUF
> laptop come with. Also adjusts existing support to select whichever is
> available.
>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
> drivers/platform/x86/asus-wmi.c | 13 +++++++++++--
> include/linux/platform_data/x86/asus-wmi.h | 1 +
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index b9a2fb8007c0..0d8a2b82cc06 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -4544,6 +4545,14 @@ static int asus_wmi_add(struct platform_device *pdev)
> asus->gpu_mux_dev = ASUS_WMI_DEVID_GPU_MUX_VIVO;
> }
>
> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
The patch itself is fine,
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
However,
There's a major problem in the way you're submitting these. This patch is
built on top of the GPU_MUX_VIVO patch as can be seen from the context
above. Yet, you're sending these independently instead of series. I
suspect there are other similar problems among these patches that there's
hidden dependency order in which these should be applied. This will cause
problems if maintainer applies the patches in wrong order (they may even
misapply with fuzz).
Only if the patches are truly independent, that is, focus on solving
entirely differently thing (functional independency) and do not have any
linewise conflicts (code locality independecy) either, it's fine to send
patches as independent ones without making a series out of them. But
clearly it's not the case here.
--
i.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] platform/x86: asus-wmi: add support variant of TUF RGB
2024-03-20 11:36 ` Ilpo Järvinen
@ 2024-03-20 20:01 ` Luke Jones
0 siblings, 0 replies; 4+ messages in thread
From: Luke Jones @ 2024-03-20 20:01 UTC (permalink / raw
To: Ilpo Järvinen
Cc: Hans de Goede, corentin.chary, platform-driver-x86, LKML
On Thu, 21 Mar 2024, at 12:36 AM, Ilpo Järvinen wrote:
> On Wed, 20 Mar 2024, Luke D. Jones wrote:
>
> > Adds support for a second TUF RGB wmi call that some versions of the TUF
> > laptop come with. Also adjusts existing support to select whichever is
> > available.
> >
> > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > ---
> > drivers/platform/x86/asus-wmi.c | 13 +++++++++++--
> > include/linux/platform_data/x86/asus-wmi.h | 1 +
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index b9a2fb8007c0..0d8a2b82cc06 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
>
> > @@ -4544,6 +4545,14 @@ static int asus_wmi_add(struct platform_device *pdev)
> > asus->gpu_mux_dev = ASUS_WMI_DEVID_GPU_MUX_VIVO;
> > }
> >
> > + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_TUF_RGB_MODE)) {
>
> The patch itself is fine,
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> However,
>
> There's a major problem in the way you're submitting these. This patch is
> built on top of the GPU_MUX_VIVO patch as can be seen from the context
> above. Yet, you're sending these independently instead of series. I
> suspect there are other similar problems among these patches that there's
> hidden dependency order in which these should be applied. This will cause
> problems if maintainer applies the patches in wrong order (they may even
> misapply with fuzz).
>
> Only if the patches are truly independent, that is, focus on solving
> entirely differently thing (functional independency) and do not have any
> linewise conflicts (code locality independecy) either, it's fine to send
> patches as independent ones without making a series out of them. But
> clearly it's not the case here.
Honestly, yeah I should have made them a series. I was sick at the time of submission and shouldn't have been near a computer at all but I have a long backlog.
I'll go through your other reviews and then turn the lot in as a series to prevent any mishaps.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-20 20:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-20 1:14 [PATCH v2 0/1] asus-wmi: add support variant of TUF RGB Luke D. Jones
2024-03-20 1:14 ` [PATCH v2 1/1] platform/x86: " Luke D. Jones
2024-03-20 11:36 ` Ilpo Järvinen
2024-03-20 20:01 ` Luke Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).