Re: [PATCH v1 10/15] platform/x86/amd/pmf: Add sysfs to toggle CnQF

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 7/12/22 16:58, Shyam Sundar S K wrote:
> Whether to turn CnQF on/off by default upon driver load would be decided
> by a BIOS flag. Add a sysfs node to provide a way to the user whether to
> use static slider or CnQF .
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/platform/x86/amd/pmf/cnqf.c | 52 +++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> index 8c6756faab25..2b03ae1ad37f 100644
> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> @@ -302,9 +302,59 @@ void amd_pmf_load_defaults_cnqf(struct amd_pmf_dev *dev)
>  		config_store.trans_prio[i] = i;
>  }
>  
> +static ssize_t feat_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> +	int result, src;
> +	bool enabled;
> +	u8 mode;
> +
> +	result = kstrtobool(buf, &enabled);
> +	if (result)
> +		return result;
> +
> +	src = amd_pmf_get_power_source();
> +	pdev->cnqf_feat = enabled;
> +	if (pdev->cnqf_feat) {
> +		amd_pmf_handle_cnqf(pdev, SLIDER_OP_SET, src, config_store.current_mode, NULL);
> +	} else {
> +		pdev->cnqf_running = false;
> +		mode = amd_pmf_get_pprof_modes(pdev);
> +		amd_pmf_update_slider(pdev, SLIDER_OP_SET, mode, NULL);
> +	}
> +
> +	dev_dbg(pdev->dev, "Received CnQF %s\n", enabled ? "on" : "off");
> +	return count;
> +}
> +
> +static ssize_t feat_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%s\n", pdev->cnqf_feat ? "on" : "off");
> +}
> +
> +static DEVICE_ATTR_RW(feat);
> +
> +static struct attribute *cnqf_feature_attrs[] = {
> +	&dev_attr_feat.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group cnqf_feature_attribute_group = {
> +	.attrs = cnqf_feature_attrs,
> +	.name = "cnqf"
> +};
> +
>  void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev)
>  {
>  	cancel_delayed_work_sync(&dev->work_buffer);
> +	sysfs_remove_group(&dev->dev->kobj, &cnqf_feature_attribute_group);
> +	kobject_uevent(&dev->dev->kobj, KOBJ_CHANGE);
>  }
>  
>  void amd_pmf_init_cnqf(struct amd_pmf_dev *dev)
> @@ -324,4 +374,6 @@ void amd_pmf_init_cnqf(struct amd_pmf_dev *dev)
>  	/* update the thermal for CnQF */
>  	src = amd_pmf_get_power_source();
>  	amd_pmf_handle_cnqf(dev, SLIDER_OP_SET, src, config_store.current_mode, NULL);
> +	ret = sysfs_create_group(&dev->dev->kobj, &cnqf_feature_attribute_group);
> +	kobject_uevent(&dev->dev->kobj, KOBJ_CHANGE);
>  }