Re: [PATCH v2 RESEND 05/11] platform/x86/amd/pmf: Add heartbeat signal support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 7/28/22 20:20, Shyam Sundar S K wrote:
> PMF driver can send periodic heartbeat signals to OEM BIOS. When BIOS does
> not receive the signal after a period of time, it can infer that AMDPMF
> has hung or failed to load.
> 
> In this situation, BIOS can fallback to legacy operations. OEM can modify
> the time interval of the signal or completely disable signals through
> ACPI Method.
> 
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>

Still looks good to me.

Regards,

Hans

> ---
>  drivers/platform/x86/amd/pmf/acpi.c | 42 +++++++++++++++++++++++++++--
>  drivers/platform/x86/amd/pmf/core.c |  1 +
>  drivers/platform/x86/amd/pmf/pmf.h  | 10 +++++++
>  3 files changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 41abd8680d8d..58e4893edea2 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -114,6 +114,26 @@ int apmf_get_static_slider_granular(struct apmf_if *apmf_if,
>  									data, sizeof(*data));
>  }
>  
> +static void apmf_sbios_heartbeat_notify(struct work_struct *work)
> +{
> +	struct amd_pmf_dev *dev = container_of(work, struct amd_pmf_dev, heart_beat.work);
> +	struct apmf_notification_cfg *n = &dev->apmf_if->notify_cfg;
> +	union acpi_object *info;
> +	int err = 0;
> +
> +	dev_dbg(dev->dev, "Sending heartbeat to SBIOS\n");
> +	info = apmf_if_call(dev->apmf_if, APMF_FUNC_SBIOS_HEARTBEAT, NULL);
> +	if (!info) {
> +		err = -EIO;
> +		goto out;
> +	}
> +
> +	schedule_delayed_work(&dev->heart_beat, msecs_to_jiffies(n->hb_interval * 1000));
> +
> +out:
> +	kfree(info);
> +}
> +
>  static int apmf_if_verify_interface(struct amd_pmf_dev *pdev, struct apmf_if *apmf_if)
>  {
>  	struct apmf_verify_interface output;
> @@ -134,6 +154,7 @@ static int apmf_if_verify_interface(struct amd_pmf_dev *pdev, struct apmf_if *ap
>  
>  static int apmf_get_system_params(struct apmf_if *apmf_if)
>  {
> +	struct apmf_notification_cfg *n = &apmf_if->notify_cfg;
>  	struct apmf_system_params params;
>  	int err;
>  
> @@ -144,17 +165,26 @@ static int apmf_get_system_params(struct apmf_if *apmf_if)
>  			return err;
>  	}
>  
> -	pr_debug("PMF: system params mask:0x%x flags:0x%x cmd_code:0x%x\n",
> +	pr_debug("PMF: system params mask:0x%x flags:0x%x cmd_code:0x%x heartbeat:%d\n",
>  		 params.valid_mask,
>  		 params.flags,
> -		 params.command_code);
> +		 params.command_code,
> +		 params.heartbeat_int);
>  	params.flags = params.flags & params.valid_mask;
> +	n->hb_interval = params.heartbeat_int;
>  
>  	return 0;
>  }
>  
> +void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
> +{
> +	if (pmf_dev->apmf_if->func.sbios_heartbeat)
> +		cancel_delayed_work_sync(&pmf_dev->heart_beat);
> +}
> +
>  int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
>  {
> +	struct apmf_notification_cfg *n;
>  	acpi_handle apmf_if_handle;
>  	struct apmf_if *apmf_if;
>  	acpi_status status;
> @@ -177,6 +207,7 @@ int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
>  		goto out;
>  	}
>  	pmf_dev->apmf_if = apmf_if;
> +	n = &apmf_if->notify_cfg;
>  
>  	ret = apmf_get_system_params(apmf_if);
>  	if (ret) {
> @@ -185,6 +216,13 @@ int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
>  		goto out;
>  	}
>  
> +	if (n->hb_interval) {
> +		apmf_if->func.sbios_heartbeat = true;
> +		/* send heartbeats only if the interval is not zero */
> +		INIT_DELAYED_WORK(&pmf_dev->heart_beat, apmf_sbios_heartbeat_notify);
> +		schedule_delayed_work(&pmf_dev->heart_beat, 0);
> +	}
> +
>  out:
>  	return ret;
>  }
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index 4ce69864879a..6c1c5a89fe71 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -279,6 +279,7 @@ static int amd_pmf_remove(struct platform_device *pdev)
>  
>  	mutex_destroy(&dev->lock);
>  	amd_pmf_deinit_features(dev);
> +	apmf_acpi_deinit(dev);
>  	amd_pmf_dbgfs_unregister(dev);
>  	kfree(dev->buf);
>  	return 0;
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index de8dbd5e04e8..f546062a10a7 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -17,6 +17,7 @@
>  /* APMF Functions */
>  #define APMF_FUNC_VERIFY_INTERFACE			0
>  #define APMF_FUNC_GET_SYS_PARAMS			1
> +#define APMF_FUNC_SBIOS_HEARTBEAT			4
>  #define APMF_FUNC_STATIC_SLIDER_GRANULAR       9
>  
>  /* Message Definitions */
> @@ -43,12 +44,18 @@
>  /* AMD PMF BIOS interfaces */
>  struct apmf_if_functions {
>  	bool system_params;
> +	bool sbios_heartbeat;
>  	bool static_slider_granular;
>  };
>  
> +struct apmf_notification_cfg {
> +	int hb_interval; /* SBIOS heartbeat interval */
> +};
> +
>  struct apmf_if {
>  	acpi_handle handle;
>  	struct apmf_if_functions func;
> +	struct apmf_notification_cfg notify_cfg;
>  };
>  
>  struct apmf_verify_interface {
> @@ -63,6 +70,7 @@ struct apmf_system_params {
>  	u32 valid_mask;
>  	u32 flags;
>  	u8 command_code;
> +	u32 heartbeat_int;
>  } __packed;
>  
>  enum amd_stt_skin_temp {
> @@ -99,6 +107,7 @@ struct amd_pmf_dev {
>  	struct apmf_if *apmf_if;
>  	enum platform_profile_option current_profile;
>  	struct platform_profile_handler pprof;
> +	struct delayed_work heart_beat;
>  	struct mutex lock; /* protects the PMF interface */
>  	struct dentry *dbgfs_dir;
>  };
> @@ -126,6 +135,7 @@ struct amd_pmf_static_slider_granular {
>  
>  /* Core Layer */
>  int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
> +void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
>  int is_apmf_func_supported(unsigned long index);
>  int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data);
>  int amd_pmf_get_power_source(void);




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux