On Tue, May 10, 2022 at 8:22 PM Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
> With the new OS handshake introduced with the commit: "c7ff29763989
> ("thermal: int340x: Update OS policy capability handshake")",
> thermal zone mode "enabled" doesn't work in the same way as the legacy
> handshake. The mode "enabled" fails with -EINVAL using new handshake.
>
> To address this issue, when the new OS UUID mask is set:
> - When mode is "enabled", return 0 as the firmware already has the
> latest policy mask.
> - When mode is "disabled", update the firmware with UUID mask of zero.
> In this way firmware can take control of the thermal control. Also
> reset the OS UUID mask. This allows user space to update with new
> set of policies.
>
> Fixes: c7ff29763989 ("thermal: int340x: Update OS policy capability handshake")
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
This is not -stable material yet.
> ---
> update:
> Added Fixes tag
>
> .../intel/int340x_thermal/int3400_thermal.c | 48 ++++++++++++-------
> 1 file changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index d97f496bab9b..1061728ad5a9 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -194,12 +194,31 @@ static int int3400_thermal_run_osc(acpi_handle handle, char *uuid_str, int *enab
> return result;
> }
>
> +static int set_os_uuid_mask(struct int3400_thermal_priv *priv, u32 mask)
> +{
> + int cap = 0;
> +
> + /*
> + * Capability bits:
> + * Bit 0: set to 1 to indicate DPTF is active
> + * Bi1 1: set to 1 to active cooling is supported by user space daemon
> + * Bit 2: set to 1 to passive cooling is supported by user space daemon
> + * Bit 3: set to 1 to critical trip is handled by user space daemon
> + */
> + if (mask)
> + cap = ((priv->os_uuid_mask << 1) | 0x01);
> +
> + return int3400_thermal_run_osc(priv->adev->handle,
> + "b23ba85d-c8b7-3542-88de-8de2ffcfd698",
> + &cap);
> +}
> +
> static ssize_t current_uuid_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct int3400_thermal_priv *priv = dev_get_drvdata(dev);
> - int i;
> + int ret, i;
>
> for (i = 0; i < INT3400_THERMAL_MAXIMUM_UUID; ++i) {
> if (!strncmp(buf, int3400_thermal_uuids[i],
> @@ -231,19 +250,7 @@ static ssize_t current_uuid_store(struct device *dev,
> }
>
> if (priv->os_uuid_mask) {
> - int cap, ret;
> -
> - /*
> - * Capability bits:
> - * Bit 0: set to 1 to indicate DPTF is active
> - * Bi1 1: set to 1 to active cooling is supported by user space daemon
> - * Bit 2: set to 1 to passive cooling is supported by user space daemon
> - * Bit 3: set to 1 to critical trip is handled by user space daemon
> - */
> - cap = ((priv->os_uuid_mask << 1) | 0x01);
> - ret = int3400_thermal_run_osc(priv->adev->handle,
> - "b23ba85d-c8b7-3542-88de-8de2ffcfd698",
> - &cap);
> + ret = set_os_uuid_mask(priv, priv->os_uuid_mask);
> if (ret)
> return ret;
> }
> @@ -469,17 +476,26 @@ static int int3400_thermal_change_mode(struct thermal_zone_device *thermal,
> if (mode != thermal->mode) {
> int enabled;
>
> + enabled = (mode == THERMAL_DEVICE_ENABLED);
> +
> + if (priv->os_uuid_mask) {
> + if (!enabled) {
> + priv->os_uuid_mask = 0;
> + result = set_os_uuid_mask(priv, priv->os_uuid_mask);
This change worries me a bit, because it means replaying an already
established _OSC handshake which shouldn't be done by the spec.
But I suppose you have tested this?
> + }
> + goto eval_odvp;
> + }
> +
> if (priv->current_uuid_index < 0 ||
> priv->current_uuid_index >= INT3400_THERMAL_MAXIMUM_UUID)
> return -EINVAL;
>
> - enabled = (mode == THERMAL_DEVICE_ENABLED);
> result = int3400_thermal_run_osc(priv->adev->handle,
> int3400_thermal_uuids[priv->current_uuid_index],
> &enabled);
> }
>
> -
> +eval_odvp:
> evaluate_odvp(priv);
>
> return result;
> --
Patch applied as 5.18-rc material, but I've removed some unneeded
parens from the new code, so please double check the result in
bleeding-edge.
Thanks!
