| Message ID | 20241125093415.21719-3-lihuisong@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (acpi_power_meter) Some trival optimizations | expand |
On 11/25/24 01:34, Huisong Li wrote: > The power trip points maintained in local should not be updated when '_PTP' > method fails to evaluate. > > Signed-off-by: Huisong Li <lihuisong@huawei.com> > --- > drivers/hwmon/acpi_power_meter.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c > index 4c3314e35d30..95da73858a0b 100644 > --- a/drivers/hwmon/acpi_power_meter.c > +++ b/drivers/hwmon/acpi_power_meter.c > @@ -292,8 +292,8 @@ static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > struct acpi_device *acpi_dev = to_acpi_device(dev); > struct acpi_power_meter_resource *resource = acpi_dev->driver_data; > + unsigned long temp, trip_bk; > int res; > - unsigned long temp; > > res = kstrtoul(buf, 10, &temp); > if (res) > @@ -302,8 +302,11 @@ static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, > temp = DIV_ROUND_CLOSEST(temp, 1000); > > mutex_lock(&resource->lock); > + trip_bk = resource->trip[attr->index - 7]; > resource->trip[attr->index - 7] = temp; > res = set_acpi_trip(resource); > + if (!res) > + resource->trip[attr->index - 7] = trip_bk; Unless I am missing something, this restores the old value if setting the new value succeeded. Please explain. Thanks, Guenter > mutex_unlock(&resource->lock); > > if (res)
在 2024/11/25 23:22, Guenter Roeck 写道: > On 11/25/24 01:34, Huisong Li wrote: >> The power trip points maintained in local should not be updated when >> '_PTP' >> method fails to evaluate. >> >> Signed-off-by: Huisong Li <lihuisong@huawei.com> >> --- >> drivers/hwmon/acpi_power_meter.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hwmon/acpi_power_meter.c >> b/drivers/hwmon/acpi_power_meter.c >> index 4c3314e35d30..95da73858a0b 100644 >> --- a/drivers/hwmon/acpi_power_meter.c >> +++ b/drivers/hwmon/acpi_power_meter.c >> @@ -292,8 +292,8 @@ static ssize_t set_trip(struct device *dev, >> struct device_attribute *devattr, >> struct sensor_device_attribute *attr = >> to_sensor_dev_attr(devattr); >> struct acpi_device *acpi_dev = to_acpi_device(dev); >> struct acpi_power_meter_resource *resource = >> acpi_dev->driver_data; >> + unsigned long temp, trip_bk; >> int res; >> - unsigned long temp; >> res = kstrtoul(buf, 10, &temp); >> if (res) >> @@ -302,8 +302,11 @@ static ssize_t set_trip(struct device *dev, >> struct device_attribute *devattr, >> temp = DIV_ROUND_CLOSEST(temp, 1000); >> mutex_lock(&resource->lock); >> + trip_bk = resource->trip[attr->index - 7]; >> resource->trip[attr->index - 7] = temp; >> res = set_acpi_trip(resource); >> + if (!res) >> + resource->trip[attr->index - 7] = trip_bk; > > Unless I am missing something, this restores the old value if setting > the new value succeeded. Please explain. > Yes, you are right. Restore the old value on faiure. will fix it. > >> mutex_unlock(&resource->lock); >> if (res) > > .
diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index 4c3314e35d30..95da73858a0b 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -292,8 +292,8 @@ static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct acpi_device *acpi_dev = to_acpi_device(dev); struct acpi_power_meter_resource *resource = acpi_dev->driver_data; + unsigned long temp, trip_bk; int res; - unsigned long temp; res = kstrtoul(buf, 10, &temp); if (res) @@ -302,8 +302,11 @@ static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, temp = DIV_ROUND_CLOSEST(temp, 1000); mutex_lock(&resource->lock); + trip_bk = resource->trip[attr->index - 7]; resource->trip[attr->index - 7] = temp; res = set_acpi_trip(resource); + if (!res) + resource->trip[attr->index - 7] = trip_bk; mutex_unlock(&resource->lock); if (res)
The power trip points maintained in local should not be updated when '_PTP' method fails to evaluate. Signed-off-by: Huisong Li <lihuisong@huawei.com> --- drivers/hwmon/acpi_power_meter.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)