Hi,
On 7/12/22 16:58, Shyam Sundar S K wrote:
> Add debugfs support to the PMF driver so that using this interface the
> live counters from the PMFW can be queried to see if the power parameters
> are getting set properly when a certain power mode change happens.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> ---
> drivers/platform/x86/amd/pmf/core.c | 46 +++++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/pmf.h | 3 ++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index b6006e8ee1a1..3d2af6a8e5e4 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -8,6 +8,7 @@
> * Author: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> */
>
> +#include <linux/debugfs.h>
> #include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> @@ -46,6 +47,49 @@
> #define DELAY_MIN_US 2000
> #define DELAY_MAX_US 3000
>
> +#ifdef CONFIG_DEBUG_FS
linux/debugfs.h defines stubs for the used debugfs functions when
debugfs is not enabled, which will cause all the referred code
to also get optimized away. So the #ifdef here is not necessary
please drop it.
> +static int current_power_limits_show(struct seq_file *seq, void *unused)
> +{
> + struct amd_pmf_dev *dev = seq->private;
> + struct amd_pmf_static_slider_granular table;
> + int mode, src = 0;
> +
> + mode = amd_pmf_get_pprof_modes(dev);
> + src = amd_pmf_get_power_source();
> + amd_pmf_update_slider(dev, SLIDER_OP_GET, mode, &table);
> + seq_printf(seq, "spl:%u fppt:%u sppt:%u sppt_apu_only:%u stt_min:%u stt[APU]:%u stt[HS2]: %u\n",
> + table.prop[src][mode].spl,
> + table.prop[src][mode].fppt,
> + table.prop[src][mode].sppt,
> + table.prop[src][mode].sppt_apu_only,
> + table.prop[src][mode].stt_min,
> + table.prop[src][mode].stt_skin_temp[STT_TEMP_APU],
> + table.prop[src][mode].stt_skin_temp[STT_TEMP_HS2]);
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(current_power_limits);
> +
> +static void amd_pmf_dbgfs_unregister(struct amd_pmf_dev *dev)
> +{
> + debugfs_remove_recursive(dev->dbgfs_dir);
> +}
> +
> +static void amd_pmf_dbgfs_register(struct amd_pmf_dev *dev)
> +{
> + dev->dbgfs_dir = debugfs_create_dir("amd_pmf", NULL);
> + debugfs_create_file("current_power_limits", 0644, dev->dbgfs_dir, dev,
> + ¤t_power_limits_fops);
> +}
> +#else
> +static inline void amd_pmf_dbgfs_register(struct amd_pmf_dev *dev)
> +{
> +}
> +
> +static inline void amd_pmf_dbgfs_unregister(struct amd_pmf_dev *dev)
> +{
> +}
and also drop the entire #else block where you define your own stubs.
> +#endif /* CONFIG_DEBUG_FS */
> +
> int amd_pmf_get_power_source(void)
> {
> if (power_supply_is_system_supplied() > 0)
> @@ -231,6 +275,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
> apmf_acpi_init(dev);
> platform_set_drvdata(pdev, dev);
> amd_pmf_init_features(dev);
> + amd_pmf_dbgfs_register(dev);
>
> mutex_init(&dev->lock);
> dev_info(dev->dev, "registered PMF device successfully\n");
> @@ -244,6 +289,7 @@ static int amd_pmf_remove(struct platform_device *pdev)
>
> mutex_destroy(&dev->lock);
> amd_pmf_deinit_features(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 a405987ae653..77021ef4bfde 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -102,6 +102,9 @@ struct amd_pmf_dev {
> enum platform_profile_option current_profile;
> struct platform_profile_handler pprof;
> struct mutex lock; /* protects the PMF interface */
> +#if IS_ENABLED(CONFIG_DEBUG_FS)
> + struct dentry *dbgfs_dir;
> +#endif /* CONFIG_DEBUG_FS */
And don't forget to drop the #if here. Also note your #if-s are not consistent
you are using:
#ifdef CONFIG_DEBUG_FS
vs:
#if IS_ENABLED(CONFIG_DEBUG_FS)
But since both should be dropped anyways this is not a problem :)
Regards,
Hans
