Re: [PATCH v11 15/15] platform/x86: lenovo-wmi-capdata: Add debugfs file for dumping capdata

[Derek John Clark]

On Fri, May 8, 2026 at 8:08 AM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 7 May 2026, Derek J. Clark wrote:
>
> > From: Rong Zhang <i@xxxxxxxx>
> >
> > The Lenovo GameZone/Other interfaces have some delicate divergences
> > among different devices. When making a bug report or adding support for
> > new devices/interfaces, capdata is the most important information to
> > cross-check with.
> >
> > Add a debugfs file (lenovo_wmi/<device_name>/capdata), so that users can
> > dump capdata and include it in their reports.
> >
> > Since `struct capdata01' is just an extension to `struct capdata00',
> > also converts the former to include the latter anonymously
> > (-fms-extensions, since v6.19). In this manner type casting won't be
> > confusing.
> >
> > Tested-by: Kurt Borja <kuurtb@xxxxxxxxx>
> > Signed-off-by: Rong Zhang <i@xxxxxxxx>
> > Signed-off-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>
> > ---
> >  drivers/platform/x86/lenovo/Kconfig       |   1 +
> >  drivers/platform/x86/lenovo/wmi-capdata.c | 120 ++++++++++++++++++++++
> >  drivers/platform/x86/lenovo/wmi-capdata.h |   4 +-
> >  3 files changed, 122 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> > index b9a5d18caa1e..4443f40ef8aa 100644
> > --- a/drivers/platform/x86/lenovo/Kconfig
> > +++ b/drivers/platform/x86/lenovo/Kconfig
> > @@ -236,6 +236,7 @@ config YT2_1380
> >  config LENOVO_WMI_CAPDATA
> >       tristate
> >       depends on ACPI_WMI
> > +     depends on LENOVO_WMI_HELPERS
> >
> >  config LENOVO_WMI_EVENTS
> >       tristate
> > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> > index 169665be4dcf..b5e2c16d638f 100644
> > --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> > +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> > @@ -31,6 +31,7 @@
> >  #include <linux/cleanup.h>
> >  #include <linux/component.h>
> >  #include <linux/container_of.h>
> > +#include <linux/debugfs.h>
> >  #include <linux/device.h>
> >  #include <linux/dev_printk.h>
> >  #include <linux/err.h>
> > @@ -42,6 +43,7 @@
> >  #include <linux/mutex_types.h>
> >  #include <linux/notifier.h>
> >  #include <linux/overflow.h>
> > +#include <linux/seq_file.h>
> >  #include <linux/stddef.h>
> >  #include <linux/types.h>
> >  #include <linux/wmi.h>
> > @@ -88,6 +90,7 @@ struct lwmi_cd_priv {
> >       struct notifier_block acpi_nb; /* ACPI events */
> >       struct wmi_device *wdev;
> >       struct cd_list *list;
> > +     struct dentry *debugfs_dir;
> >
> >       /*
> >        * A capdata device may be a component master of another capdata device.
> > @@ -118,6 +121,8 @@ struct cd_list {
> >
> >  static struct wmi_driver lwmi_cd_driver;
> >
> > +/* ======== Device components ======== */
> > +
> >  /**
> >   * lwmi_cd_match() - Match rule for the master driver.
> >   * @dev: Pointer to the capability data parent device.
> > @@ -471,6 +476,116 @@ EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CAPDATA");
> >  DEF_LWMI_CDXX_GET_DATA(cd_fan, LENOVO_FAN_TEST_DATA, struct capdata_fan);
> >  EXPORT_SYMBOL_NS_GPL(lwmi_cd_fan_get_data, "LENOVO_WMI_CAPDATA");
> >
> > +/* ======== debugfs ======== */
> > +
> > +/**
> > + * lwmi_cd00_show() - Dump capdata00
> > + * @s: Pointer to seq_file where the capdata00 is dumped.
> > + * @cd00: Pointer to a capdata00 struct to be dumped.
> > + */
> > +static void lwmi_cd00_show(struct seq_file *s, struct capdata00 *cd00)
> > +{
> > +     u8 dev = FIELD_GET(LWMI_ATTR_DEV_ID_MASK, cd00->id);
> > +     u8 feat = FIELD_GET(LWMI_ATTR_FEAT_ID_MASK, cd00->id);
> > +     u8 mode = FIELD_GET(LWMI_ATTR_MODE_ID_MASK, cd00->id);
> > +     u8 type = FIELD_GET(LWMI_ATTR_TYPE_ID_MASK, cd00->id);
> > +     bool extra = cd00->supported & ~(LWMI_SUPP_GET | LWMI_SUPP_SET | LWMI_SUPP_VALID);
> > +     bool get = cd00->supported & LWMI_SUPP_GET;
> > +     bool set = cd00->supported & LWMI_SUPP_SET;
> > +     bool valid = cd00->supported & LWMI_SUPP_VALID;
> > +
> > +     seq_printf(s, "  id:             0x%08x [dev: %2u, feat: %2u, mode: %2u, type: %2u]\n",
> > +                cd00->id, dev, feat, mode, type);
> > +
> > +     seq_printf(s, "  supported:      0x%08x [%c%c%c%c]\n", cd00->supported,
> > +                extra ? '+' : ' ',
> > +                get   ? 'R' : ' ',
> > +                set   ? 'W' : ' ',
> > +                valid ? 'V' : ' ');
> > +
> > +     seq_printf(s, "  default_value:  %u\n", cd00->default_value);
> > +}
> > +
> > +/**
> > + * lwmi_cd01_show() - Dump capdata01
> > + * @s: Pointer to seq_file where the capdata01 is dumped.
> > + * @cd01: Pointer to a capdata01 struct to be dumped.
> > + */
> > +static void lwmi_cd01_show(struct seq_file *s, struct capdata01 *cd01)
> > +{
> > +     /* capdata01 is an extension to capdata00. */
> > +     lwmi_cd00_show(s, (struct capdata00 *)cd01);
>
> Hpmf, so anonymous struct leads to this now.
>
> I wonder if we could wrap it with an union to have both named access to it
> (to get the pointer without casting) while retaining anonymous access to
> its members:
>
> struct capdata01 {
>         union {
>                 struct capdata00;
>                 struct capdata00 cd00;
>         };
>         ...
>
> ...If it's not possible, then I'll have to accept the cast I guess.
>

With that, the call becomes:
lwmi_cd00_show(s, &cd01->cd00);

Everything seems to work as expected. This looks cleaner to me, and
doesn't need the comment to explain.

- Derek


> > +     seq_printf(s, "  step:           %u\n", cd01->step);
> > +     seq_printf(s, "  min_value:      %u\n", cd01->min_value);
> > +     seq_printf(s, "  max_value:      %u\n", cd01->max_value);
> > +}
> > +
> > +/**
> > + * lwmi_cd_fan_show() - Dump capdata_fan
> > + * @s: Pointer to seq_file where the capdata_fan is dumped.
> > + * @cd_fan: Pointer to a capdata_fan struct to be dumped.
> > + */
> > +static void lwmi_cd_fan_show(struct seq_file *s, struct capdata_fan *cd_fan)
> > +{
> > +     seq_printf(s, "  id:             %u\n", cd_fan->id);
> > +     seq_printf(s, "  min_rpm:        %u\n", cd_fan->min_rpm);
> > +     seq_printf(s, "  max_rpm:        %u\n", cd_fan->max_rpm);
> > +}
> > +
> > +/**
> > + * lwmi_cd_debugfs_show() - Dump capability data to debugfs
> > + * @s: Pointer to seq_file where the capability data is dumped.
> > + * @data: unused.
> > + *
> > + * Return: 0
> > + */
> > +static int lwmi_cd_debugfs_show(struct seq_file *s, void *data)
> > +{
> > +     struct lwmi_cd_priv *priv = s->private;
> > +     u8 idx;
> > +
> > +     guard(mutex)(&priv->list->list_mutex);
> > +
> > +     /* lwmi_cd_alloc() ensured priv->list->type must be a valid type. */
> > +     for (idx = 0; idx < priv->list->count; idx++) {
> > +             seq_printf(s, "%s[%u]:\n", lwmi_cd_table[priv->list->type].name, idx);
> > +
> > +             if (priv->list->type == LENOVO_CAPABILITY_DATA_00)
> > +                     lwmi_cd00_show(s, &priv->list->cd00[idx]);
> > +             else if (priv->list->type == LENOVO_CAPABILITY_DATA_01)
> > +                     lwmi_cd01_show(s, &priv->list->cd01[idx]);
> > +             else if (priv->list->type == LENOVO_FAN_TEST_DATA)
> > +                     lwmi_cd_fan_show(s, &priv->list->cd_fan[idx]);
> > +     }
> > +
> > +     return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(lwmi_cd_debugfs);
> > +
> > +/**
> > + * lwmi_cd_debugfs_add() - Create debugfs directory and files for a device
> > + * @priv: lenovo-wmi-capdata driver data.
> > + */
> > +static void lwmi_cd_debugfs_add(struct lwmi_cd_priv *priv)
> > +{
> > +     priv->debugfs_dir = lwmi_debugfs_create_dir(priv->wdev);
> > +
> > +     debugfs_create_file("capdata", 0444, priv->debugfs_dir, priv, &lwmi_cd_debugfs_fops);
> > +}
> > +
> > +/**
> > + * lwmi_cd_debugfs_remove() - Remove debugfs directory for a device
> > + * @priv: lenovo-wmi-capdata driver data.
> > + */
> > +static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
> > +{
> > +     debugfs_remove_recursive(priv->debugfs_dir);
> > +     priv->debugfs_dir = NULL;
> > +}
> > +
> > +/* ======== WMI interface ======== */
> > +
> >  /**
> >   * lwmi_cd_cache() - Cache all WMI data block information
> >   * @priv: lenovo-wmi-capdata driver data.
> > @@ -773,6 +888,8 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
> >               dev_err(&wdev->dev, "failed to register %s: %d\n",
> >                       info->name, ret);
> >       } else {
> > +             lwmi_cd_debugfs_add(priv);
> > +
> >               dev_dbg(&wdev->dev, "registered %s with %u items\n",
> >                       info->name, priv->list->count);
> >       }
> > @@ -783,6 +900,8 @@ static void lwmi_cd_remove(struct wmi_device *wdev)
> >  {
> >       struct lwmi_cd_priv *priv = dev_get_drvdata(&wdev->dev);
> >
> > +     lwmi_cd_debugfs_remove(priv);
> > +
> >       switch (priv->list->type) {
> >       case LENOVO_CAPABILITY_DATA_00:
> >               lwmi_cd_sub_master_del(priv);
> > @@ -822,6 +941,7 @@ static struct wmi_driver lwmi_cd_driver = {
> >
> >  module_wmi_driver(lwmi_cd_driver);
> >
> > +MODULE_IMPORT_NS("LENOVO_WMI_HELPERS");
> >  MODULE_DEVICE_TABLE(wmi, lwmi_cd_id_table);
> >  MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@xxxxxxxxx>");
> >  MODULE_AUTHOR("Rong Zhang <i@xxxxxxxx>");
> > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
> > index e0a30f2c0c87..0df3ae534ed5 100644
> > --- a/drivers/platform/x86/lenovo/wmi-capdata.h
> > +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
> > @@ -38,9 +38,7 @@ struct capdata00 {
> >  };
> >
> >  struct capdata01 {
> > -     u32 id;
> > -     u32 supported;
> > -     u32 default_value;
> > +     struct capdata00;
> >       u32 step;
> >       u32 min_value;
> >       u32 max_value;
> >
>
> --
>  i.
>