> Hi, > > On 7/15/23 12:03, Luke Jones wrote: > > On Sat, 2023-07-15 at 11:40 +0200, Hans de Goede wrote: > > > Hi Kristian, > > > > > > On 7/14/23 22:44, Kristian Angelov wrote: > > > > This patch fixes setting the cmd values to 0xb3 and 0xb4. > > > > This is necessary on some TUF laptops in order to set the RGB > > > > mode. > > > > > > > > Closes: > > > > https://lore.kernel.org/platform-driver-x86/443078148.491022.1677576298133@xxxxxxxxxxx > > > > Signed-off-by: Kristian Angelov <kristiana2000@xxxxxx> > > > > --- > > > > V1 -> V2. Make setting 0xb3 and 0xb4 the default logic > > > > > > > > drivers/platform/x86/asus-wmi.c | 13 +++++++++++-- > > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/platform/x86/asus-wmi.c > > > > b/drivers/platform/x86/asus-wmi.c > > > > index 1038dfdcdd32..eb82ed723b42 100644 > > > > --- a/drivers/platform/x86/asus-wmi.c > > > > +++ b/drivers/platform/x86/asus-wmi.c > > > > @@ -738,13 +738,22 @@ static ssize_t kbd_rgb_mode_store(struct > > > > device *dev, > > > > struct device_attribute *attr, > > > > const char *buf, size_t count) > > > > { > > > > - u32 cmd, mode, r, g, b, speed; > > > > + u32 cmd, mode, r, g, b, speed; > > > > int err; > > > > > > > > if (sscanf(buf, "%d %d %d %d %d %d", &cmd, &mode, &r, > > > > &g, > > > > &b, &speed) != 6) > > > > return -EINVAL; > > > > > > > > - cmd = !!cmd; > > > > + /* B3 is set and B4 is save to BIOS. Only set by > > > > default*/ > > > > + switch (cmd) { > > > > + default: > > > > + case 0: > > > > + cmd = 0xb3; > > > > + break; > > > > + case 1: > > > > + cmd = 0xb4; > > > > + break; > > > > + } > > > > > > You are now leaving the value of cmd unmodified for values which > > > are > > > not 0 and 1. > > > > > > I think you need to add a: > > > > > > default: > > > return -EINVAL; > > > > > > here to catch cmd not being either 0 or 1. > > > > > > Luke, what do you think ? > > > > Looks fine to me. > > Fine with or without the default: return -EINVAL; added ? Sorry, i wasn't very clear at all. With -EINVAL. I don't see any reason it should continue with un-initialised cmd, or a default value as a default might give false expectations. > > > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx> > > I guess you mean Reviewed-by ? Yes sorry, I shouldn't have even added that. I will add review for V3. > > Regards, > > Hans > > > > > > > > /* These are the known usable modes across all TUF/ROG > > > > */ > > > > if (mode >= 12 || mode == 9) > > > > > >
