From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Rong Zhang <i@rong.moe>
Cc: Mark Pearson <mpearson-lenovo@squebb.ca>,
"Derek J. Clark" <derekjohn.clark@gmail.com>,
Hans de Goede <hansg@kernel.org>, Armin Wolf <W_Armin@gmx.de>,
Charles <hanker007@gmail.com>,
platform-driver-x86@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/9] platform/x86: lenovo-wmi-{capdata,other}: Only allocate capdata list when necessary
Date: Mon, 5 Oct 2026 19:28:45 +0300 (EEST) [thread overview]
Message-ID: <0aba598c-fe01-ba32-5eb3-e900c33a07f3@linux.intel.com> (raw)
In-Reply-To: <20260914-lwmi-wmi-new-api-v1-4-7a400f2f69f8@rong.moe>
On Mon, 14 Sep 2026, Rong Zhang wrote:
> When no capability data is available, there is no need to allocate
> capability data list as it's basically unused except for the
> priv->list->count == 0 placeholder.
>
> Therefore, only allocate priv->list when necessary, otherwise its
> absence implies the absence of capability data. In this manner,
> lenovo-wmi-other can skip registering unavailable functionalities
> accordingly. Meanwhile, skip creating the debugfs directory as it
> provides nothing when there is no capability data.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> drivers/platform/x86/lenovo/wmi-capdata.c | 43 ++++++++++++++++++++++---------
> drivers/platform/x86/lenovo/wmi-other.c | 11 +++++---
> 2 files changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> index 0123ec8f7b53..793b5103d533 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> @@ -313,8 +313,8 @@ static const struct component_ops lwmi_cd_component_ops = {
> * @dev: The sub-master capdata basic device.
> *
> * Call component_bind_all to bind the sub-component device to the sub-master
> - * device. On success, collect the pointer to the sub-component list and try
> - * to call the master callback.
> + * device. On success, collect the pointer (or ERR_PTR(-ENODEV) if it's stubbed)
> + * to the sub-component list and try to call the master callback.
> *
> * Return: 0 on success, or an error code.
> */
> @@ -328,7 +328,7 @@ static int lwmi_cd_sub_master_bind(struct device *dev)
> if (ret)
> return ret;
>
> - priv->sub_master->sub_component_list = sub_component_list;
> + priv->sub_master->sub_component_list = sub_component_list ?: ERR_PTR(-ENODEV);
> lwmi_cd_call_master_cb(priv);
>
> return 0;
> @@ -460,6 +460,9 @@ static const struct component_ops lwmi_cd_sub_component_ops = {
> { \
> u8 idx; \
> \
> + if (WARN_ON(!list)) \
> + return -EINVAL; \
> + \
> guard(mutex)(&list->list_mutex); \
> for (idx = 0; idx < list->count; idx++) { \
> if (list->_cdxx[idx].id != attribute_id) \
> @@ -571,6 +574,9 @@ DEFINE_SHOW_ATTRIBUTE(lwmi_cd_debugfs);
> */
> static void lwmi_cd_debugfs_add(struct lwmi_cd_priv *priv)
> {
> + if (!priv->list)
> + return;
> +
> priv->debugfs_dir = lwmi_debugfs_create_dir(priv->wdev);
>
> debugfs_create_file("capdata", 0444, priv->debugfs_dir, priv, &lwmi_cd_debugfs_fops);
> @@ -582,6 +588,7 @@ static void lwmi_cd_debugfs_add(struct lwmi_cd_priv *priv)
> */
> static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
> {
> + /* Debugfs can handle NULL dir, no need to check. */
> debugfs_remove_recursive(priv->debugfs_dir);
> priv->debugfs_dir = NULL;
> }
> @@ -645,6 +652,9 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
> */
> static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> {
> + if (!priv->list)
> + return 0;
> +
> if (!priv->initialized)
> return __lwmi_cd_cache(priv);
>
> @@ -707,6 +717,9 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
> count = 0;
> }
>
> + if (!count)
> + return 0;
> +
> list = devm_kzalloc(&priv->wdev->dev, struct_size(list, cd_fan, count), GFP_KERNEL);
> if (!list)
> return -ENOMEM;
> @@ -742,6 +755,8 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
> int count;
>
> count = wmidev_instance_count(priv->wdev);
> + if (!count)
> + return 0;
>
> switch (priv->info->type) {
> case LENOVO_CAPABILITY_DATA_00:
> @@ -884,7 +899,9 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
> enum lwmi_cd_type sub_component_type = LENOVO_FAN_TEST_DATA;
> struct capdata00 capdata00;
>
> - ret = lwmi_cd00_get_data(priv->list, LWMI_ATTR_ID_FAN_TEST, &capdata00);
> + ret = priv->list
> + ? lwmi_cd00_get_data(priv->list, LWMI_ATTR_ID_FAN_TEST, &capdata00)
> + : -ENODATA;
It's only 89 chars if you have it one a single line so this looks pretty
unnecessary line split that doesn't even buy you that much extra space.
The code can go up to 100 chars as needed.
Alternatively, split the parameters to two lines instead.
> if (ret || !(capdata00.supported & LWMI_SUPP_VALID)) {
> dev_dbg(&wdev->dev, "capdata00 declares no fan test support\n");
> sub_component_type = CD_TYPE_NONE;
> @@ -905,14 +922,16 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
> case LENOVO_CAPABILITY_DATA_01:
> priv->acpi_nb.notifier_call = lwmi_cd01_notifier_call;
>
> - ret = register_acpi_notifier(&priv->acpi_nb);
> - if (ret)
> - goto out;
> + if (priv->list) {
> + ret = register_acpi_notifier(&priv->acpi_nb);
> + if (ret)
> + goto out;
>
> - ret = devm_add_action_or_reset(&wdev->dev, lwmi_cd01_unregister,
> - &priv->acpi_nb);
> - if (ret)
> - goto out;
> + ret = devm_add_action_or_reset(&wdev->dev, lwmi_cd01_unregister,
> + &priv->acpi_nb);
> + if (ret)
> + goto out;
> + }
>
> ret = component_add(&wdev->dev, &lwmi_cd_component_ops);
> goto out;
> @@ -930,7 +949,7 @@ static int lwmi_cd_probe(struct wmi_device *wdev, const void *context)
> lwmi_cd_debugfs_add(priv);
>
> dev_dbg(&wdev->dev, "registered %s with %u items\n",
> - info->name, priv->list->count);
> + info->name, priv->list ? priv->list->count : 0);
> }
> return ret;
> }
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index fbb32bf404f2..e6c8f6bcf050 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -1643,16 +1643,19 @@ static int lwmi_om_master_bind(struct device *dev)
>
> priv->cd00_list = binder.cd00_list;
> priv->cd01_list = binder.cd01_list;
> - if (!priv->cd00_list || !priv->cd01_list) {
> + if (!priv->cd00_list && !priv->cd01_list) {
> component_unbind_all(dev, NULL);
>
> return -ENODEV;
> }
>
> - lwmi_om_fan_info_collect_cd00(priv);
> - lwmi_om_psy_ext_init(priv);
> + if (priv->cd00_list) {
> + lwmi_om_fan_info_collect_cd00(priv);
> + lwmi_om_psy_ext_init(priv);
> + }
>
> - lwmi_om_fw_attr_add(priv);
> + if (priv->cd01_list)
> + lwmi_om_fw_attr_add(priv);
>
> return 0;
> }
>
>
--
i.
next prev parent reply other threads:[~2026-10-05 16:29 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-13 20:50 [PATCH 0/9] platform/x86: lenovo-wmi-{other,capdata,helpers}: Improve robustness on buggy firmware Rong Zhang
2026-09-13 20:50 ` [PATCH 1/9] platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary Rong Zhang
2026-09-13 20:50 ` [PATCH 2/9] platform/x86: lenovo-wmi-capdata: Store a pointer to component info Rong Zhang
2026-09-13 20:50 ` [PATCH 3/9] platform/x86: lenovo-wmi-capdata: Defer mutex initialization Rong Zhang
2026-10-05 16:25 ` Ilpo Järvinen
2026-10-07 18:11 ` Rong Zhang
2026-09-13 20:50 ` [PATCH 4/9] platform/x86: lenovo-wmi-{capdata,other}: Only allocate capdata list when necessary Rong Zhang
2026-10-05 16:28 ` Ilpo Järvinen [this message]
2026-09-13 20:50 ` [PATCH 5/9] platform/x86: lenovo-wmi-capdata: Adopt new WMI API Rong Zhang
2026-09-13 20:50 ` [PATCH 6/9] platform/x86: lenovo-wmi-capdata: Register component even on WMI error Rong Zhang
2026-10-05 16:36 ` Ilpo Järvinen
2026-09-13 20:50 ` [PATCH 7/9] platform/x86: lenovo-wmi-capdata: Detect stubbed capdata device Rong Zhang
2026-09-13 20:50 ` [PATCH 8/9] platform/x86: lenovo-wmi-helpers: Adopt new WMI API Rong Zhang
2026-09-13 20:50 ` [PATCH 9/9] platform/x86: Add myself as LENOVO drivers maintainer Rong Zhang
2026-09-26 21:04 ` [PATCH 0/9] platform/x86: lenovo-wmi-{other,capdata,helpers}: Improve robustness on buggy firmware Navon John Lukose
2026-09-27 3:01 ` Rong Zhang
2026-09-28 16:04 ` Mark Pearson
2026-09-28 17:53 ` Rong Zhang
2026-09-28 19:09 ` Navon John Lukose
2026-10-07 18:30 ` Rong Zhang
2026-10-07 19:25 ` Mark Pearson
2026-10-07 23:44 ` Armin Wolf
2026-10-07 20:37 ` Derek J. Clark
2026-10-07 20:54 ` Ilpo Järvinen
2026-10-08 11:38 ` Rong Zhang
2026-10-08 11:47 ` Ilpo Järvinen
2026-10-08 14:01 ` Mark Pearson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0aba598c-fe01-ba32-5eb3-e900c33a07f3@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=W_Armin@gmx.de \
--cc=derekjohn.clark@gmail.com \
--cc=hanker007@gmail.com \
--cc=hansg@kernel.org \
--cc=i@rong.moe \
--cc=linux-kernel@vger.kernel.org \
--cc=mpearson-lenovo@squebb.ca \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®