Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 24 Jan 2023 15:22:41 +0100 | Subject | Re: [PATCH v3] thermal: int340x_thermal: Add production mode attribute |
| |
On Mon, Jan 23, 2023 at 5:31 PM Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > It is possible that the system manufacturer locks down thermal tuning > beyond what is usually done on the given platform. In that case user > space calibration tools should not try to adjust the thermal > configuration of the system. > > To allow user space to check if that is the case, add a new sysfs > attribute "production_mode" that will be present when the ACPI DCFG > method is present under the INT3400 device object in the ACPI Namespace. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > v3: > Build warning reported by for missing static > Reported-by: kernel test robot <lkp@intel.com> > > v2 > Addressed comments from Rafael: > - Updated commit excatly same as Rafael wrote > - Removed production_mode_support bool > - Use sysfs_emit > - Update documentation > > .../driver-api/thermal/intel_dptf.rst | 3 ++ > .../intel/int340x_thermal/int3400_thermal.c | 48 +++++++++++++++++++ > 2 files changed, 51 insertions(+) > > diff --git a/Documentation/driver-api/thermal/intel_dptf.rst b/Documentation/driver-api/thermal/intel_dptf.rst > index 372bdb4d04c6..f5c193cccbda 100644 > --- a/Documentation/driver-api/thermal/intel_dptf.rst > +++ b/Documentation/driver-api/thermal/intel_dptf.rst > @@ -84,6 +84,9 @@ DPTF ACPI Drivers interface > https:/github.com/intel/thermal_daemon for decoding > thermal table. > > +``production_mode`` (RO) > + When different from zero, manufacturer locked thermal configuration > + from further changes. > > ACPI Thermal Relationship table interface > ------------------------------------------ > diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > index db8a6f63657d..23ea21238bbd 100644 > --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > @@ -60,6 +60,7 @@ struct int3400_thermal_priv { > int odvp_count; > int *odvp; > u32 os_uuid_mask; > + int production_mode; > struct odvp_attr *odvp_attrs; > }; > > @@ -315,6 +316,44 @@ static int int3400_thermal_get_uuids(struct int3400_thermal_priv *priv) > return result; > } > > +static ssize_t production_mode_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct int3400_thermal_priv *priv = dev_get_drvdata(dev); > + > + return sysfs_emit(buf, "%d\n", priv->production_mode); > +} > + > +static DEVICE_ATTR_RO(production_mode); > + > +static int production_mode_init(struct int3400_thermal_priv *priv) > +{ > + unsigned long long mode; > + acpi_status status; > + int ret; > + > + priv->production_mode = -1; > + > + status = acpi_evaluate_integer(priv->adev->handle, "DCFG", NULL, &mode); > + /* If the method is not present, this is not an error */ > + if (ACPI_FAILURE(status)) > + return 0; > + > + ret = sysfs_create_file(&priv->pdev->dev.kobj, &dev_attr_production_mode.attr); > + if (ret) > + return ret; > + > + priv->production_mode = mode; > + > + return 0; > +} > + > +static void production_mode_exit(struct int3400_thermal_priv *priv) > +{ > + if (priv->production_mode >= 0) > + sysfs_remove_file(&priv->pdev->dev.kobj, &dev_attr_production_mode.attr);
Isn't it OK to call sysfs_remove_file() if the given attribute is not there?
If so, the above check is unnecessary and the assignment to -1 above too (as this is the only place where the value is tested).
> +} > + > static ssize_t odvp_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > @@ -610,8 +649,15 @@ static int int3400_thermal_probe(struct platform_device *pdev) > if (result) > goto free_sysfs; > > + result = production_mode_init(priv); > + if (result) > + goto free_notify; > + > return 0; > > +free_notify: > + acpi_remove_notify_handler(priv->adev->handle, ACPI_DEVICE_NOTIFY, > + int3400_notify); > free_sysfs: > cleanup_odvp(priv); > if (!ZERO_OR_NULL_PTR(priv->data_vault)) { > @@ -638,6 +684,8 @@ static int int3400_thermal_remove(struct platform_device *pdev) > { > struct int3400_thermal_priv *priv = platform_get_drvdata(pdev); > > + production_mode_exit(priv); > + > acpi_remove_notify_handler( > priv->adev->handle, ACPI_DEVICE_NOTIFY, > int3400_notify); > --
| |