From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Denis Benato <denis.benato@linux.dev>
Cc: platform-driver-x86@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
Hans de Goede <hansg@kernel.org>,
Corentin Chary <corentin.chary@gmail.com>,
Luke Jones <luke@ljones.dev>, busybox11 <busybox11th@gmail.com>,
Denis Benato <benato.denis96@gmail.com>
Subject: Re: [PATCH v1 1/2] platform/x86: asus-armoury: let attribute groups decide their own visibility
Date: Thu, 17 Sep 2026 12:23:49 +0300 (EEST) [thread overview]
Message-ID: <328b1420-374a-3c77-19ea-f9f7892070eb@linux.intel.com> (raw)
In-Reply-To: <20260916154210.181441-2-denis.benato@linux.dev>
On Wed, 16 Sep 2026, Denis Benato wrote:
> mini_led_mode, gpu_mux_mode and dgpu_disable are created by ad-hoc
> conditional blocks in asus_fw_attr_add() which must be manually tracked
> by the error and exit paths; those paths also re-probe WMI to know which
> groups were actually created.
>
> Give attribute groups whose support depends on a resolved device ID
> their own .is_visible() callback: the group is created unconditionally
> and sysfs hides it entirely (SYSFS_GROUP_INVISIBLE) when the backing WMI
> device is not present. The new ASUS_ATTR_GROUP_BOOL_VIS() and
> ASUS_ATTR_GROUP_ENUM_VIS() macros declare such attribute groups.
> Creation and removal become symmetric, groups can be removed
> unconditionally since sysfs_remove_group() is a no-op for groups that
> were never created, and no WMI probe is needed outside of
> initialization.
>
> The dgpu_disable attribute now goes through the same resolved device ID
> scheme as mini_led_mode and gpu_mux_mode, storing the device ID to use
> in asus_armoury.dgpu_disable_dev_id instead of always operating on
> ASUS_WMI_DEVID_DGPU.
>
> No functional change is intended.
>
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> ---
> drivers/platform/x86/asus-armoury.c | 111 +++++++++++++++++-----------
> drivers/platform/x86/asus-armoury.h | 55 ++++++++++++++
> 2 files changed, 123 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> index 2d5ca75bc727..8639902f084d 100644
> --- a/drivers/platform/x86/asus-armoury.c
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -94,6 +94,7 @@ struct asus_armoury_priv {
>
> u32 mini_led_dev_id;
> u32 gpu_mux_dev_id;
> + u32 dgpu_disable_dev_id;
>
> bool requires_fan_curve;
> };
> @@ -458,7 +459,13 @@ static ssize_t mini_led_mode_possible_values_show(struct kobject *kobj,
> return -ENODEV;
> }
> }
> -ASUS_ATTR_GROUP_ENUM(mini_led_mode, "mini_led_mode", "Set the mini-LED backlight mode");
> +
> +static bool mini_led_mode_group_visible(struct kobject *kobj)
> +{
> + return asus_armoury.mini_led_dev_id;
> +}
> +
> +ASUS_ATTR_GROUP_ENUM_VIS(mini_led_mode, "mini_led_mode", "Set the mini-LED backlight mode");
>
> static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
> struct kobj_attribute *attr,
> @@ -471,8 +478,8 @@ static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
> if (err)
> return err;
>
> - if (armoury_has_devstate(ASUS_WMI_DEVID_DGPU)) {
> - err = armoury_get_devstate(NULL, &result, ASUS_WMI_DEVID_DGPU);
> + if (asus_armoury.dgpu_disable_dev_id) {
> + err = armoury_get_devstate(NULL, &result, asus_armoury.dgpu_disable_dev_id);
> if (err)
> return err;
> if (result && !optimus) {
> @@ -502,7 +509,13 @@ static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
> return count;
> }
> ASUS_WMI_SHOW_INT(gpu_mux_mode_current_value, asus_armoury.gpu_mux_dev_id);
> -ASUS_ATTR_GROUP_BOOL(gpu_mux_mode, "gpu_mux_mode", "Set the GPU display MUX mode");
> +
> +static bool gpu_mux_mode_group_visible(struct kobject *kobj)
> +{
> + return asus_armoury.gpu_mux_dev_id;
> +}
> +
> +ASUS_ATTR_GROUP_BOOL_VIS(gpu_mux_mode, "gpu_mux_mode", "Set the GPU display MUX mode");
>
> static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
> struct kobj_attribute *attr, const char *buf,
> @@ -538,7 +551,8 @@ static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
> }
>
> scoped_guard(mutex, &asus_armoury.egpu_mutex) {
> - err = armoury_set_devstate(attr, disable ? 1 : 0, NULL, ASUS_WMI_DEVID_DGPU);
> + err = armoury_set_devstate(attr, disable ? 1 : 0, NULL,
> + asus_armoury.dgpu_disable_dev_id);
> if (err)
> return err;
> }
> @@ -547,8 +561,14 @@ static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
>
> return count;
> }
> -ASUS_WMI_SHOW_INT(dgpu_disable_current_value, ASUS_WMI_DEVID_DGPU);
> -ASUS_ATTR_GROUP_BOOL(dgpu_disable, "dgpu_disable", "Disable the dGPU");
> +
> +static bool dgpu_disable_group_visible(struct kobject *kobj)
> +{
> + return asus_armoury.dgpu_disable_dev_id;
> +}
> +
> +ASUS_WMI_SHOW_INT(dgpu_disable_current_value, asus_armoury.dgpu_disable_dev_id);
> +ASUS_ATTR_GROUP_BOOL_VIS(dgpu_disable, "dgpu_disable", "Disable the dGPU");
>
> /* Values map for eGPU activation requests. */
> static u32 egpu_status_map[] = {
> @@ -823,7 +843,6 @@ ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_
> static const struct asus_attr_group armoury_attr_groups[] = {
> { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
> { &egpu_enable_attr_group, ASUS_WMI_DEVID_EGPU },
> - { &dgpu_disable_attr_group, ASUS_WMI_DEVID_DGPU },
The point with .is_visible is that you don't need to remove these from
this array because they'll appear selectively under sysfs even if you list
all of them here.
> { &dgpu_power_state_attr_group, ASUS_WMI_DEVID_DGPU_POWER_STATE },
> { &apu_mem_attr_group, ASUS_WMI_DEVID_APU_MEM },
>
> @@ -938,34 +957,46 @@ static int asus_fw_attr_add(void)
> goto err_destroy_kset;
> }
>
> + /*
> + * Device IDs with model-dependent alternatives are resolved once
> + * here: their attribute groups then decide visibility themselves
> + * through .is_visible.
> + */
> asus_armoury.mini_led_dev_id = 0;
> if (armoury_has_devstate(ASUS_WMI_DEVID_MINI_LED_MODE))
> asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE;
> else if (armoury_has_devstate(ASUS_WMI_DEVID_MINI_LED_MODE2))
> asus_armoury.mini_led_dev_id = ASUS_WMI_DEVID_MINI_LED_MODE2;
>
> - if (asus_armoury.mini_led_dev_id) {
> - err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> - &mini_led_mode_attr_group);
> - if (err) {
> - pr_err("Failed to create sysfs-group for mini_led\n");
> - goto err_remove_file;
> - }
> - }
> -
> asus_armoury.gpu_mux_dev_id = 0;
> if (armoury_has_devstate(ASUS_WMI_DEVID_GPU_MUX))
> asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX;
> else if (armoury_has_devstate(ASUS_WMI_DEVID_GPU_MUX_VIVO))
> asus_armoury.gpu_mux_dev_id = ASUS_WMI_DEVID_GPU_MUX_VIVO;
>
> - if (asus_armoury.gpu_mux_dev_id) {
> - err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> - &gpu_mux_mode_attr_group);
> - if (err) {
> - pr_err("Failed to create sysfs-group for gpu_mux\n");
> - goto err_remove_mini_led_group;
> - }
> + asus_armoury.dgpu_disable_dev_id = 0;
> + if (armoury_has_devstate(ASUS_WMI_DEVID_DGPU))
> + asus_armoury.dgpu_disable_dev_id = ASUS_WMI_DEVID_DGPU;
> +
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + &mini_led_mode_attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for mini_led\n");
> + goto err_remove_file;
> + }
> +
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + &gpu_mux_mode_attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for gpu_mux\n");
> + goto err_remove_mini_led_group;
> + }
> +
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + &dgpu_disable_attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for dgpu_disable\n");
> + goto err_remove_gpu_mux_group;
> }
>
> for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
> @@ -1000,16 +1031,14 @@ static int asus_fw_attr_add(void)
> return 0;
>
> err_remove_groups:
> - while (i--) {
> - if (armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
> - sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
> - armoury_attr_groups[i].attr_group);
> - }
> - if (asus_armoury.gpu_mux_dev_id)
> - sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + while (i--)
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
> + armoury_attr_groups[i].attr_group);
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &dgpu_disable_attr_group);
> +err_remove_gpu_mux_group:
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> err_remove_mini_led_group:
> - if (asus_armoury.mini_led_dev_id)
> - sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> err_remove_file:
> sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> err_destroy_kset:
> @@ -1182,17 +1211,13 @@ static void __exit asus_fw_exit(void)
> {
> int i;
>
> - for (i = ARRAY_SIZE(armoury_attr_groups) - 1; i >= 0; i--) {
> - if (armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
> - sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
> - armoury_attr_groups[i].attr_group);
> - }
> -
> - if (asus_armoury.gpu_mux_dev_id)
> - sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + for (i = ARRAY_SIZE(armoury_attr_groups) - 1; i >= 0; i--)
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
> + armoury_attr_groups[i].attr_group);
>
> - if (asus_armoury.mini_led_dev_id)
> - sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &dgpu_disable_attr_group);
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
>
> sysfs_remove_file(&asus_armoury.fw_attr_kset->kobj, &pending_reboot.attr);
> kset_unregister(asus_armoury.fw_attr_kset);
> diff --git a/drivers/platform/x86/asus-armoury.h b/drivers/platform/x86/asus-armoury.h
> index 6dfe5ddfe4ac..44d12f20cd3c 100644
> --- a/drivers/platform/x86/asus-armoury.h
> +++ b/drivers/platform/x86/asus-armoury.h
> @@ -198,6 +198,61 @@ ssize_t armoury_attr_uint_show(struct kobject *kobj, struct kobj_attribute *attr
> .name = _fsname, .attrs = _attrname##_attrs \
> }
>
> +/*
> + * Same as ASUS_ATTR_GROUP_BOOL() but the whole attribute group is
> + * created only when <name>_group_visible() returns true.
> + * Requires <name>_current_value_show(), <name>_current_value_store()
> + * and <name>_group_visible()
> + */
> +#define ASUS_ATTR_GROUP_BOOL_VIS(_attrname, _fsname, _dispname) \
> + DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(_attrname) \
> + static struct kobj_attribute attr_##_attrname##_current_value = \
> + __ASUS_ATTR_RW(_attrname, current_value); \
> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
> + __ATTR_SHOW_FMT(possible_values, _attrname, "%s\n", "0;1"); \
> + static struct kobj_attribute attr_##_attrname##_type = \
> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
> + static struct attribute *_attrname##_attrs[] = { \
> + &attr_##_attrname##_current_value.attr, \
> + &attr_##_attrname##_display_name.attr, \
> + &attr_##_attrname##_possible_values.attr, \
> + &attr_##_attrname##_type.attr, \
> + NULL \
> + }; \
> + static const struct attribute_group _attrname##_attr_group = { \
> + .name = _fsname, \
> + .is_visible = SYSFS_GROUP_VISIBLE(_attrname), \
> + .attrs = _attrname##_attrs \
> + }
> +
> +/*
> + * Same as ASUS_ATTR_GROUP_ENUM() but the whole attribute group is
> + * created only when <name>_group_visible() returns true.
> + * Requires <name>_current_value_show(), <name>_current_value_store(),
> + * <name>_possible_values_show() and <name>_group_visible()
> + */
> +#define ASUS_ATTR_GROUP_ENUM_VIS(_attrname, _fsname, _dispname) \
> + DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(_attrname) \
> + static struct kobj_attribute attr_##_attrname##_current_value = \
> + __ASUS_ATTR_RW(_attrname, current_value); \
> + __ATTR_SHOW_FMT(display_name, _attrname, "%s\n", _dispname); \
> + static struct kobj_attribute attr_##_attrname##_possible_values =\
> + __ASUS_ATTR_RO(_attrname, possible_values); \
> + static struct kobj_attribute attr_##_attrname##_type = \
> + __ASUS_ATTR_RO_AS(type, enum_type_show); \
> + static struct attribute *_attrname##_attrs[] = { \
> + &attr_##_attrname##_current_value.attr, \
> + &attr_##_attrname##_display_name.attr, \
> + &attr_##_attrname##_possible_values.attr, \
> + &attr_##_attrname##_type.attr, \
> + NULL \
> + }; \
> + static const struct attribute_group _attrname##_attr_group = { \
> + .name = _fsname, \
> + .is_visible = SYSFS_GROUP_VISIBLE(_attrname), \
> + .attrs = _attrname##_attrs \
> + }
> +
> #define ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(_attrname, _fsname, _wmi, _dispname) \
> ASUS_WMI_SHOW_INT(_attrname##_current_value, _wmi); \
> static struct kobj_attribute attr_##_attrname##_current_value = \
>
--
i.
next prev parent reply other threads:[~2026-09-17 9:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-16 15:42 [PATCH v1 0/2] platform/x86: asus-armoury: reorganize visibility and extend dgpu_disable Denis Benato
2026-09-16 15:42 ` [PATCH v1 1/2] platform/x86: asus-armoury: let attribute groups decide their own visibility Denis Benato
2026-09-17 9:23 ` Ilpo Järvinen [this message]
2026-09-16 15:42 ` [PATCH v1 2/2] platform/x86: asus-armoury: add dGPU disable fallback DEVID for ProArt H7606 series Denis Benato
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=328b1420-374a-3c77-19ea-f9f7892070eb@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=benato.denis96@gmail.com \
--cc=busybox11th@gmail.com \
--cc=corentin.chary@gmail.com \
--cc=denis.benato@linux.dev \
--cc=hansg@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luke@ljones.dev \
--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®