Date:   Wed, 12 Oct 2016 15:17:15 -0700
From:   Markus Mayer <markus.mayer@...adcom.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>
Cc:     Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>,
        Device Tree List <devicetree@...r.kernel.org>,
        Power Management List <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Markus Mayer <code@...yer.net>,
        Markus Mayer <mmayer@...adcom.com>
Subject: Re: [PATCH v4 3/3] cpufreq: brcmstb-avs-cpufreq: add debugfs support

On 12 October 2016 at 15:12, Markus Mayer <mmayer@...adcom.com> wrote:
> From: Markus Mayer <code@...yer.net>

Ugh. Missed that. In case this version gets applied, would you mind
fixing the author address while applying (so they all say Broadcom)?

> In order to aid debugging, we add a debugfs interface to the driver
> that allows direct interaction with the AVS co-processor.
>
> The debugfs interface provides a means for reading all and writing some
> of the mailbox registers directly from the shell prompt and enables a
> user to execute the communications protocol between ARM CPU and AVS CPU
> step-by-step.
>
> This interface should be used for debugging purposes only.
>
> Signed-off-by: Markus Mayer <mmayer@...adcom.com>
> Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
>  drivers/cpufreq/Kconfig.arm           |  10 ++
>  drivers/cpufreq/brcmstb-avs-cpufreq.c | 323 +++++++++++++++++++++++++++++++++-
>  2 files changed, 332 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index d4e3795..43f261e 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -23,6 +23,16 @@ config ARM_BRCMSTB_AVS_CPUFREQ
>
>           Say Y, if you have a Broadcom SoC with AVS support for DFS or DVFS.
>
> +config ARM_BRCMSTB_AVS_CPUFREQ_DEBUG
> +       bool "Broadcom STB AVS CPUfreq driver sysfs debug capability"
> +       depends on ARM_BRCMSTB_AVS_CPUFREQ
> +       help
> +         Enabling this option turns on debug support via sysfs under
> +         /sys/kernel/debug/brcmstb-avs-cpufreq. It is possible to read all and
> +         write some AVS mailbox registers through sysfs entries.
> +
> +         If in doubt, say N.
> +
>  config ARM_DT_BL_CPUFREQ
>         tristate "Generic probing via DT for ARM big LITTLE CPUfreq driver"
>         depends on ARM_BIG_LITTLE_CPUFREQ && OF
> diff --git a/drivers/cpufreq/brcmstb-avs-cpufreq.c b/drivers/cpufreq/brcmstb-avs-cpufreq.c
> index 4415fa0..b761d54 100644
> --- a/drivers/cpufreq/brcmstb-avs-cpufreq.c
> +++ b/drivers/cpufreq/brcmstb-avs-cpufreq.c
> @@ -49,6 +49,13 @@
>  #include <linux/platform_device.h>
>  #include <linux/semaphore.h>
>
> +#ifdef CONFIG_ARM_BRCMSTB_AVS_CPUFREQ_DEBUG
> +#include <linux/ctype.h>
> +#include <linux/debugfs.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#endif
> +
>  /* Max number of arguments AVS calls take */
>  #define AVS_MAX_CMD_ARGS       4
>  /*
> @@ -175,11 +182,88 @@ struct private_data {
>         void __iomem *base;
>         void __iomem *avs_intr_base;
>         struct device *dev;
> +#ifdef CONFIG_ARM_BRCMSTB_AVS_CPUFREQ_DEBUG
> +       struct dentry *debugfs;
> +#endif
>         struct completion done;
>         struct semaphore sem;
>         struct pmap pmap;
>  };
>
> +#ifdef CONFIG_ARM_BRCMSTB_AVS_CPUFREQ_DEBUG
> +
> +enum debugfs_format {
> +       DEBUGFS_NORMAL,
> +       DEBUGFS_FLOAT,
> +       DEBUGFS_REV,
> +};
> +
> +struct debugfs_data {
> +       struct debugfs_entry *entry;
> +       struct private_data *priv;
> +};
> +
> +struct debugfs_entry {
> +       char *name;
> +       u32 offset;
> +       fmode_t mode;
> +       enum debugfs_format format;
> +};
> +
> +#define DEBUGFS_ENTRY(name, mode, format)      { \
> +       #name, AVS_MBOX_##name, mode, format \
> +}
> +
> +/*
> + * These are used for debugfs only. Otherwise we use AVS_MBOX_PARAM() directly.
> + */
> +#define AVS_MBOX_PARAM1                AVS_MBOX_PARAM(0)
> +#define AVS_MBOX_PARAM2                AVS_MBOX_PARAM(1)
> +#define AVS_MBOX_PARAM3                AVS_MBOX_PARAM(2)
> +#define AVS_MBOX_PARAM4                AVS_MBOX_PARAM(3)
> +
> +/*
> + * This table stores the name, access permissions and offset for each hardware
> + * register and is used to generate debugfs entries.
> + */
> +static struct debugfs_entry debugfs_entries[] = {
> +       DEBUGFS_ENTRY(COMMAND, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(STATUS, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(VOLTAGE0, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(TEMP0, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(PV0, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(MV0, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(PARAM1, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(PARAM2, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(PARAM3, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(PARAM4, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(REVISION, 0, DEBUGFS_REV),
> +       DEBUGFS_ENTRY(PSTATE, 0, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(HEARTBEAT, 0, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(MAGIC, S_IWUSR, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(SIGMA_HVT, 0, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(SIGMA_SVT, 0, DEBUGFS_NORMAL),
> +       DEBUGFS_ENTRY(VOLTAGE1, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(TEMP1, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(PV1, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(MV1, 0, DEBUGFS_FLOAT),
> +       DEBUGFS_ENTRY(FREQUENCY, 0, DEBUGFS_NORMAL),
> +};
> +
> +static int brcm_avs_target_index(struct cpufreq_policy *, unsigned int);
> +
> +static char *__strtolower(char *s)
> +{
> +       char *p;
> +
> +       for (p = s; *p; p++)
> +               *p = tolower(*p);
> +
> +       return s;
> +}
> +
> +#endif /* CONFIG_ARM_BRCMSTB_AVS_CPUFREQ_DEBUG */
> +
>  static void __iomem *__map_region(const char *name)
>  {
>         struct device_node *np;
> @@ -432,6 +516,238 @@ brcm_avs_get_freq_table(struct device *dev, struct private_data *priv)
>         return table;
>  }
>
> +#ifdef CONFIG_ARM_BRCMSTB_AVS_CPUFREQ_DEBUG
> +
> +#define MANT(x)        (unsigned int)(abs((x)) / 1000)
> +#define FRAC(x)        (unsigned int)(abs((x)) - abs((x)) / 1000 * 1000)
> +
> +static int brcm_avs_debug_show(struct seq_file *s, void *data)
> +{
> +       struct debugfs_data *dbgfs = s->private;
> +       void __iomem *base;
> +       u32 val, offset;
> +
> +       if (!dbgfs) {
> +               seq_puts(s, "No device pointer\n");
> +               return 0;
> +       }
> +
> +       base = dbgfs->priv->base;
> +       offset = dbgfs->entry->offset;
> +       val = readl(base + offset);
> +       switch (dbgfs->entry->format) {
> +       case DEBUGFS_NORMAL:
> +               seq_printf(s, "%u\n", val);
> +               break;
> +       case DEBUGFS_FLOAT:
> +               seq_printf(s, "%d.%03d\n", MANT(val), FRAC(val));
> +               break;
> +       case DEBUGFS_REV:
> +               seq_printf(s, "%c.%c.%c.%c\n", (val >> 24 & 0xff),
> +                          (val >> 16 & 0xff), (val >> 8 & 0xff),
> +                          val & 0xff);
> +               break;
> +       }
> +       seq_printf(s, "0x%08x\n", val);
> +
> +       return 0;
> +}
> +
> +#undef MANT
> +#undef FRAC
> +
> +static ssize_t brcm_avs_seq_write(struct file *file, const char __user *buf,
> +                                 size_t size, loff_t *ppos)
> +{
> +       struct seq_file *s = file->private_data;
> +       struct debugfs_data *dbgfs = s->private;
> +       struct private_data *priv = dbgfs->priv;
> +       void __iomem *base, *avs_intr_base;
> +       bool use_issue_command = false;
> +       unsigned long val, offset;
> +       char str[128];
> +       int ret;
> +       char *str_ptr = str;
> +
> +       if (size >= sizeof(str))
> +               return -E2BIG;
> +
> +       memset(str, 0, sizeof(str));
> +       ret = copy_from_user(str, buf, size);
> +       if (ret)
> +               return ret;
> +
> +       base = priv->base;
> +       avs_intr_base = priv->avs_intr_base;
> +       offset = dbgfs->entry->offset;
> +       /*
> +        * Special case writing to "command" entry only: if the string starts
> +        * with a 'c', we use the driver's __issue_avs_command() function.
> +        * Otherwise, we perform a raw write. This should allow testing of raw
> +        * access as well as using the higher level function. (Raw access
> +        * doesn't clear the firmware return status after issuing the command.)
> +        */
> +       if (str_ptr[0] == 'c' && offset == AVS_MBOX_COMMAND) {
> +               use_issue_command = true;
> +               str_ptr++;
> +       }
> +       if (kstrtoul(str_ptr, 0, &val) != 0)
> +               return -EINVAL;
> +
> +       /*
> +        * Setting the P-state is a special case. We need to update the CPU
> +        * frequency we report.
> +        */
> +       if (val == AVS_CMD_SET_PSTATE) {
> +               struct cpufreq_policy *policy;
> +               unsigned int pstate;
> +
> +               policy = cpufreq_cpu_get(smp_processor_id());
> +               /* Read back the P-state we are about to set */
> +               pstate = readl(base + AVS_MBOX_PARAM(0));
> +               if (use_issue_command) {
> +                       ret = brcm_avs_target_index(policy, pstate);
> +                       return ret ? ret : size;
> +               }
> +               policy->cur = policy->freq_table[pstate].frequency;
> +       }
> +
> +       if (use_issue_command) {
> +               ret = __issue_avs_command(priv, val, false, NULL);
> +       } else {
> +               /* Locking here is not perfect, but is only for debug. */
> +               ret = down_interruptible(&priv->sem);
> +               if (ret)
> +                       return ret;
> +
> +               writel(val, base + offset);
> +               /* We have to wake up the firmware to process a command. */
> +               if (offset == AVS_MBOX_COMMAND)
> +                       writel(AVS_CPU_L2_INT_MASK,
> +                              avs_intr_base + AVS_CPU_L2_SET0);
> +               up(&priv->sem);
> +       }
> +
> +       return ret ? ret : size;
> +}
> +
> +static struct debugfs_entry *__find_debugfs_entry(const char *name)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(debugfs_entries); i++)
> +               if (strcasecmp(debugfs_entries[i].name, name) == 0)
> +                       return &debugfs_entries[i];
> +
> +       return NULL;
> +}
> +
> +static int brcm_avs_debug_open(struct inode *inode, struct file *file)
> +{
> +       struct debugfs_data *data;
> +       fmode_t fmode;
> +       int ret;
> +
> +       /*
> +        * seq_open(), which is called by single_open(), clears "write" access.
> +        * We need write access to some files, so we preserve our access mode
> +        * and restore it.
> +        */
> +       fmode = file->f_mode;
> +       /*
> +        * Check access permissions even for root. We don't want to be writing
> +        * to read-only registers. Access for regular users has already been
> +        * checked by the VFS layer.
> +        */
> +       if ((fmode & FMODE_WRITER) && !(inode->i_mode & S_IWUSR))
> +               return -EACCES;
> +
> +       data = kmalloc(sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +       /*
> +        * We use the same file system operations for all our debug files. To
> +        * produce specific output, we look up the file name upon opening a
> +        * debugfs entry and map it to a memory offset. This offset is then used
> +        * in the generic "show" function to read a specific register.
> +        */
> +       data->entry = __find_debugfs_entry(file->f_path.dentry->d_iname);
> +       data->priv = inode->i_private;
> +
> +       ret = single_open(file, brcm_avs_debug_show, data);
> +       if (ret)
> +               kfree(data);
> +       file->f_mode = fmode;
> +
> +       return ret;
> +}
> +
> +static int brcm_avs_debug_release(struct inode *inode, struct file *file)
> +{
> +       struct seq_file *seq_priv = file->private_data;
> +       struct debugfs_data *data = seq_priv->private;
> +
> +       kfree(data);
> +       return single_release(inode, file);
> +}
> +
> +static const struct file_operations brcm_avs_debug_ops = {
> +       .open           = brcm_avs_debug_open,
> +       .read           = seq_read,
> +       .write          = brcm_avs_seq_write,
> +       .llseek         = seq_lseek,
> +       .release        = brcm_avs_debug_release,
> +};
> +
> +static void brcm_avs_cpufreq_debug_init(struct platform_device *pdev)
> +{
> +       struct private_data *priv = platform_get_drvdata(pdev);
> +       struct dentry *dir;
> +       int i;
> +
> +       if (!priv)
> +               return;
> +
> +       dir = debugfs_create_dir(BRCM_AVS_CPUFREQ_NAME, NULL);
> +       if (IS_ERR_OR_NULL(dir))
> +               return;
> +       priv->debugfs = dir;
> +
> +       for (i = 0; i < ARRAY_SIZE(debugfs_entries); i++) {
> +               /*
> +                * The DEBUGFS_ENTRY macro generates uppercase strings. We
> +                * convert them to lowercase before creating the debugfs
> +                * entries.
> +                */
> +               char *entry = __strtolower(debugfs_entries[i].name);
> +               fmode_t mode = debugfs_entries[i].mode;
> +
> +               if (!debugfs_create_file(entry, S_IFREG | S_IRUGO | mode,
> +                                        dir, priv, &brcm_avs_debug_ops)) {
> +                       priv->debugfs = NULL;
> +                       debugfs_remove_recursive(dir);
> +                       break;
> +               }
> +       }
> +}
> +
> +static void brcm_avs_cpufreq_debug_exit(struct platform_device *pdev)
> +{
> +       struct private_data *priv = platform_get_drvdata(pdev);
> +
> +       if (priv && priv->debugfs) {
> +               debugfs_remove_recursive(priv->debugfs);
> +               priv->debugfs = NULL;
> +       }
> +}
> +
> +#else
> +
> +static void brcm_avs_cpufreq_debug_init(struct platform_device *pdev) {}
> +static void brcm_avs_cpufreq_debug_exit(struct platform_device *pdev) {}
> +
> +#endif /* CONFIG_ARM_BRCMSTB_AVS_CPUFREQ_DEBUG */
> +
>  /*
>   * To ensure the right firmware is running we need to
>   *    - check the MAGIC matches what we expect
> @@ -694,8 +1010,11 @@ static int brcm_avs_cpufreq_probe(struct platform_device *pdev)
>                 return ret;
>
>         brcm_avs_driver.driver_data = pdev;
> +       ret = cpufreq_register_driver(&brcm_avs_driver);
> +       if (!ret)
> +               brcm_avs_cpufreq_debug_init(pdev);
>
> -       return cpufreq_register_driver(&brcm_avs_driver);
> +       return ret;
>  }
>
>  static int brcm_avs_cpufreq_remove(struct platform_device *pdev)
> @@ -707,6 +1026,8 @@ static int brcm_avs_cpufreq_remove(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> +       brcm_avs_cpufreq_debug_exit(pdev);
> +
>         priv = platform_get_drvdata(pdev);
>         iounmap(priv->base);
>         iounmap(priv->avs_intr_base);
> --
> 2.7.4
>

Powered by blists - more mailing lists