Devices with hdmi_mux, amplifier or deepslp quirks create a sysfs group for each available feature. To accomplish this, helper create/remove functions were called on module init, but they had the following problems: - Create helpers called remove helpers on failure, which in turn tried to remove the sysfs group that failed to be created - If group creation failed mid way, previous successfully created groups were not cleaned up - Module exit only removed hdmi_mux group To improve this, drop all helpers in favor of two helpers that make use of sysfs_create_groups/sysfs_remove_groups to cleanly create/remove groups at module init/exit. Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx> --- I have a question. Do the created sysfs groups get removed when their kobj reference count goes to 0? I ask because I want to know if this is a bug fix. --- drivers/platform/x86/dell/alienware-wmi.c | 105 ++++++++-------------- 1 file changed, 36 insertions(+), 69 deletions(-) diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c index 44f1f7b57d0a..e9ed2089cba0 100644 --- a/drivers/platform/x86/dell/alienware-wmi.c +++ b/drivers/platform/x86/dell/alienware-wmi.c @@ -410,8 +410,10 @@ struct wmax_u32_args { u8 arg3; }; + static struct platform_device *platform_device; static struct platform_zone *zone_data; +const struct attribute_group *wmax_groups[4]; static struct platform_profile_handler pp_handler; static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST]; @@ -810,22 +812,6 @@ static const struct attribute_group hdmi_attribute_group = { .attrs = hdmi_attrs, }; -static void remove_hdmi(struct platform_device *dev) -{ - if (quirks->hdmi_mux > 0) - sysfs_remove_group(&dev->dev.kobj, &hdmi_attribute_group); -} - -static int create_hdmi(struct platform_device *dev) -{ - int ret; - - ret = sysfs_create_group(&dev->dev.kobj, &hdmi_attribute_group); - if (ret) - remove_hdmi(dev); - return ret; -} - /* * Alienware GFX amplifier support * - Currently supports reading cable status @@ -864,22 +850,6 @@ static const struct attribute_group amplifier_attribute_group = { .attrs = amplifier_attrs, }; -static void remove_amplifier(struct platform_device *dev) -{ - if (quirks->amplifier > 0) - sysfs_remove_group(&dev->dev.kobj, &lifier_attribute_group); -} - -static int create_amplifier(struct platform_device *dev) -{ - int ret; - - ret = sysfs_create_group(&dev->dev.kobj, &lifier_attribute_group); - if (ret) - remove_amplifier(dev); - return ret; -} - /* * Deep Sleep Control support * - Modifies BIOS setting for deep sleep control allowing extra wakeup events @@ -942,22 +912,6 @@ static const struct attribute_group deepsleep_attribute_group = { .attrs = deepsleep_attrs, }; -static void remove_deepsleep(struct platform_device *dev) -{ - if (quirks->deepslp > 0) - sysfs_remove_group(&dev->dev.kobj, &deepsleep_attribute_group); -} - -static int create_deepsleep(struct platform_device *dev) -{ - int ret; - - ret = sysfs_create_group(&dev->dev.kobj, &deepsleep_attribute_group); - if (ret) - remove_deepsleep(dev); - return ret; -} - /* * Thermal Profile control * - Provides thermal profile control through the Platform Profile API @@ -1165,6 +1119,34 @@ static void remove_thermal_profile(void) platform_profile_remove(); } +static int __init create_wmax_groups(struct platform_device *pdev) +{ + int no_groups = 0; + + if (quirks->hdmi_mux) { + wmax_groups[no_groups] = &hdmi_attribute_group; + no_groups++; + } + + if (quirks->amplifier) { + wmax_groups[no_groups] = &lifier_attribute_group; + no_groups++; + } + + if (quirks->deepslp) { + wmax_groups[no_groups] = &deepsleep_attribute_group; + no_groups++; + } + + return no_groups > 0 ? device_add_groups(&pdev->dev, wmax_groups) : 0; +} + +static void __exit remove_wmax_groups(struct platform_device *pdev) +{ + if (!wmax_groups[0]) + device_remove_groups(&pdev->dev, wmax_groups); +} + static int __init alienware_wmi_init(void) { int ret; @@ -1203,23 +1185,9 @@ static int __init alienware_wmi_init(void) goto fail_platform_device1; } - if (quirks->hdmi_mux > 0) { - ret = create_hdmi(platform_device); - if (ret) - goto fail_prep_hdmi; - } - - if (quirks->amplifier > 0) { - ret = create_amplifier(platform_device); - if (ret) - goto fail_prep_amplifier; - } - - if (quirks->deepslp > 0) { - ret = create_deepsleep(platform_device); - if (ret) - goto fail_prep_deepsleep; - } + ret = create_wmax_groups(platform_device); + if (ret) + goto fail_prep_groups; if (quirks->thermal) { ret = create_thermal_profile(); @@ -1236,9 +1204,8 @@ static int __init alienware_wmi_init(void) fail_prep_zones: remove_thermal_profile(); fail_prep_thermal_profile: -fail_prep_deepsleep: -fail_prep_amplifier: -fail_prep_hdmi: + remove_wmax_groups(platform_device); +fail_prep_groups: platform_device_unregister(platform_device); fail_platform_device1: platform_driver_unregister(&platform_driver); @@ -1251,7 +1218,7 @@ module_init(alienware_wmi_init); static void __exit alienware_wmi_exit(void) { alienware_zone_exit(platform_device); - remove_hdmi(platform_device); + remove_wmax_groups(platform_device); remove_thermal_profile(); platform_device_unregister(platform_device); platform_driver_unregister(&platform_driver); -- 2.47.0