On Mon, 11 Mar 2024, Luke D. Jones wrote: > Laptops with any of the ppt or nv tunables default to the minimum setting > on boot so we can safely assume a stored value is correct. > > This patch adds storing of those values in the local struct, and enables > reading of those values back. > > Secondary to the above it renames some internal variables to be more > consistent (which makes code grepping show all related parts) > > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx> > --- > drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++------- > 1 file changed, 111 insertions(+), 30 deletions(-) > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index e4341abb71e0..482e23b55e1e 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -272,12 +272,19 @@ struct asus_wmi { > > /* Tunables provided by ASUS for gaming laptops */ > bool ppt_pl2_sppt_available; > + u32 ppt_pl2_sppt; > bool ppt_pl1_spl_available; > + u32 ppt_pl1_spl; > bool ppt_apu_sppt_available; > - bool ppt_plat_sppt_available; > + u32 ppt_apu_sppt; > + bool ppt_platform_sppt_available; > + u32 ppt_platform_sppt; > bool ppt_fppt_available; > - bool nv_dyn_boost_available; > - bool nv_temp_tgt_available; > + u32 ppt_fppt; > + bool nv_dynamic_boost_available; > + u32 nv_dynamic_boost; > + bool nv_temp_target_available; > + u32 nv_temp_target; > > bool kbd_rgb_mode_available; > u32 kbd_rgb_dev; Can you check with pahole if this structure is now full of 31-bit holes? The benefit of keeping bool & u32 doesn't seem that big to begin with (in visual sense because of the 1 char variation in column). > @@ -999,11 +1006,10 @@ static ssize_t ppt_pl2_sppt_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t count) > { > + struct asus_wmi *asus = dev_get_drvdata(dev); > int result, err; > u32 value; > > - struct asus_wmi *asus = dev_get_drvdata(dev); > - Please put this into own patch, it's entirely unrelated (but still useful change!). > result = kstrtou32(buf, 10, &value); > if (result) > return result; > @@ -1022,22 +1028,31 @@ static ssize_t ppt_pl2_sppt_store(struct device *dev, > return -EIO; > } > > + asus->ppt_pl2_sppt = value; > sysfs_notify(&asus->platform_device->dev.kobj, NULL, "ppt_pl2_sppt"); > > return count; > } > -static DEVICE_ATTR_WO(ppt_pl2_sppt); > + > +static ssize_t ppt_pl2_sppt_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) Alignment is not correct? > +{ > + struct asus_wmi *asus = dev_get_drvdata(dev); > + > + return sysfs_emit(buf, "%d\n", asus->ppt_pl2_sppt); > +} > +static DEVICE_ATTR_RW(ppt_pl2_sppt); > > /* Tunable: PPT, Intel=PL1, AMD=SPL ******************************************/ > static ssize_t ppt_pl1_spl_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t count) > { > + struct asus_wmi *asus = dev_get_drvdata(dev); > int result, err; > u32 value; > > - struct asus_wmi *asus = dev_get_drvdata(dev); > - Unrelated, put to the same move patch as the other change please. I won't mark all thse from this point on but please do them all. -- i.