platform/x86: thinkpad_acpi: Convert platform driver to use dev_groups

Platform drivers have the option of having the platform core create and
remove any needed sysfs attribute files. So take advantage of that and
refactor the attributes management to avoid to register them "by hand".

Also, due to some attributes are optionals, refactor the code and move
the logic inside the "is_visible" callbacks of the attribute_group
structures.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Len Baker <len.baker@gmx.com>
Link: https://lore.kernel.org/r/20211023154036.6800-1-len.baker@gmx.comReviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>

drivers/platform/x86/thinkpad_acpi.c

@@ -332,12 +332,11 @@ static struct {
 	u32 battery_force_primary:1;
 	u32 input_device_registered:1;
 	u32 platform_drv_registered:1;
-	u32 platform_drv_attrs_registered:1;
 	u32 sensors_pdrv_registered:1;
-	u32 sensors_pdrv_attrs_registered:1;
 	u32 sensors_pdev_attrs_registered:1;
 	u32 hotkey_poll_active:1;
 	u32 has_adaptive_kbd:1;
+	u32 kbd_lang:1;
 } tp_features;
 
 static struct {
@@ -983,20 +982,6 @@ static void tpacpi_shutdown_handler(struct platform_device *pdev)
 	}
 }
 
-static struct platform_driver tpacpi_pdriver = {
-	.driver = {
-		.name = TPACPI_DRVR_NAME,
-		.pm = &tpacpi_pm,
-	},
-	.shutdown = tpacpi_shutdown_handler,
-};
-
-static struct platform_driver tpacpi_hwmon_pdriver = {
-	.driver = {
-		.name = TPACPI_HWMON_DRVR_NAME,
-	},
-};
-
 /*************************************************************************
  * sysfs support helpers
  */
@@ -1479,53 +1464,6 @@ static ssize_t uwb_emulstate_store(struct device_driver *drv, const char *buf,
 static DRIVER_ATTR_RW(uwb_emulstate);
 #endif
 
-/* --------------------------------------------------------------------- */
-
-static struct driver_attribute *tpacpi_driver_attributes[] = {
-	&driver_attr_debug_level, &driver_attr_version,
-	&driver_attr_interface_version,
-};
-
-static int __init tpacpi_create_driver_attributes(struct device_driver *drv)
-{
-	int i, res;
-
-	i = 0;
-	res = 0;
-	while (!res && i < ARRAY_SIZE(tpacpi_driver_attributes)) {
-		res = driver_create_file(drv, tpacpi_driver_attributes[i]);
-		i++;
-	}
-
-#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
-	if (!res && dbg_wlswemul)
-		res = driver_create_file(drv, &driver_attr_wlsw_emulstate);
-	if (!res && dbg_bluetoothemul)
-		res = driver_create_file(drv, &driver_attr_bluetooth_emulstate);
-	if (!res && dbg_wwanemul)
-		res = driver_create_file(drv, &driver_attr_wwan_emulstate);
-	if (!res && dbg_uwbemul)
-		res = driver_create_file(drv, &driver_attr_uwb_emulstate);
-#endif
-
-	return res;
-}
-
-static void tpacpi_remove_driver_attributes(struct device_driver *drv)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(tpacpi_driver_attributes); i++)
-		driver_remove_file(drv, tpacpi_driver_attributes[i]);
-
-#ifdef THINKPAD_ACPI_DEBUGFACILITIES
-	driver_remove_file(drv, &driver_attr_wlsw_emulstate);
-	driver_remove_file(drv, &driver_attr_bluetooth_emulstate);
-	driver_remove_file(drv, &driver_attr_wwan_emulstate);
-	driver_remove_file(drv, &driver_attr_uwb_emulstate);
-#endif
-}
-
 /*************************************************************************
  * Firmware Data
  */
@@ -2999,7 +2937,14 @@ static struct attribute *adaptive_kbd_attributes[] = {
 	NULL
 };
 
+static umode_t hadaptive_kbd_attr_is_visible(struct kobject *kobj,
+					     struct attribute *attr, int n)
+{
+	return tp_features.has_adaptive_kbd ? attr->mode : 0;
+}
+
 static const struct attribute_group adaptive_kbd_attr_group = {
+	.is_visible = hadaptive_kbd_attr_is_visible,
 	.attrs = adaptive_kbd_attributes,
 };
 
@@ -3094,8 +3039,6 @@ static void hotkey_exit(void)
 	hotkey_poll_stop_sync();
 	mutex_unlock(&hotkey_mutex);
 #endif
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &hotkey_attr_group);
-
 	dbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
 		   "restoring original HKEY status and mask\n");
 	/* yes, there is a bitwise or below, we want the
@@ -3490,14 +3433,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 			 */
 			if (acpi_evalf(hkey_handle, &hotkey_adaptive_all_mask,
 				       "MHKA", "dd", 2)) {
-				if (hotkey_adaptive_all_mask != 0) {
+				if (hotkey_adaptive_all_mask != 0)
 					tp_features.has_adaptive_kbd = true;
-					res = sysfs_create_group(
-						&tpacpi_pdev->dev.kobj,
-						&adaptive_kbd_attr_group);
-					if (res)
-						goto err_exit;
-				}
 			} else {
 				tp_features.has_adaptive_kbd = false;
 				hotkey_adaptive_all_mask = 0x0U;
@@ -3551,9 +3488,6 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 	}
 
 	tabletsw_state = hotkey_init_tablet_mode();
-	res = sysfs_create_group(&tpacpi_pdev->dev.kobj, &hotkey_attr_group);
-	if (res)
-		goto err_exit;
 
 	/* Set up key map */
 	keymap_id = tpacpi_check_quirks(tpacpi_keymap_qtable,
@@ -3650,9 +3584,6 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 	return 0;
 
 err_exit:
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &hotkey_attr_group);
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &adaptive_kbd_attr_group);
-
 	return (res < 0) ? res : 1;
 }
 
@@ -4384,7 +4315,14 @@ static struct attribute *bluetooth_attributes[] = {
 	NULL
 };
 
+static umode_t bluetooth_attr_is_visible(struct kobject *kobj,
+					 struct attribute *attr, int n)
+{
+	return tp_features.bluetooth ? attr->mode : 0;
+}
+
 static const struct attribute_group bluetooth_attr_group = {
+	.is_visible = bluetooth_attr_is_visible,
 	.attrs = bluetooth_attributes,
 };
 
@@ -4406,11 +4344,7 @@ static void bluetooth_shutdown(void)
 
 static void bluetooth_exit(void)
 {
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj,
-			&bluetooth_attr_group);
-
 	tpacpi_destroy_rfkill(TPACPI_RFK_BLUETOOTH_SW_ID);
-
 	bluetooth_shutdown();
 }
 
@@ -4524,17 +4458,7 @@ static int __init bluetooth_init(struct ibm_init_struct *iibm)
 				RFKILL_TYPE_BLUETOOTH,
 				TPACPI_RFK_BLUETOOTH_SW_NAME,
 				true);
-	if (res)
-		return res;
-
-	res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
-				&bluetooth_attr_group);
-	if (res) {
-		tpacpi_destroy_rfkill(TPACPI_RFK_BLUETOOTH_SW_ID);
-		return res;
-	}
-
-	return 0;
+	return res;
 }
 
 /* procfs -------------------------------------------------------------- */
@@ -4641,7 +4565,14 @@ static struct attribute *wan_attributes[] = {
 	NULL
 };
 
+static umode_t wan_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+				   int n)
+{
+	return tp_features.wan ? attr->mode : 0;
+}
+
 static const struct attribute_group wan_attr_group = {
+	.is_visible = wan_attr_is_visible,
 	.attrs = wan_attributes,
 };
 
@@ -4663,11 +4594,7 @@ static void wan_shutdown(void)
 
 static void wan_exit(void)
 {
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj,
-		&wan_attr_group);
-
 	tpacpi_destroy_rfkill(TPACPI_RFK_WWAN_SW_ID);
-
 	wan_shutdown();
 }
 
@@ -4711,18 +4638,7 @@ static int __init wan_init(struct ibm_init_struct *iibm)
 				RFKILL_TYPE_WWAN,
 				TPACPI_RFK_WWAN_SW_NAME,
 				true);
-	if (res)
-		return res;
-
-	res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
-				&wan_attr_group);
-
-	if (res) {
-		tpacpi_destroy_rfkill(TPACPI_RFK_WWAN_SW_ID);
-		return res;
-	}
-
-	return 0;
+	return res;
 }
 
 /* procfs -------------------------------------------------------------- */
@@ -5623,30 +5539,35 @@ static ssize_t cmos_command_store(struct device *dev,
 
 static DEVICE_ATTR_WO(cmos_command);
 
+static struct attribute *cmos_attributes[] = {
+	&dev_attr_cmos_command.attr,
+	NULL
+};
+
+static umode_t cmos_attr_is_visible(struct kobject *kobj,
+				    struct attribute *attr, int n)
+{
+	return cmos_handle ? attr->mode : 0;
+}
+
+static const struct attribute_group cmos_attr_group = {
+	.is_visible = cmos_attr_is_visible,
+	.attrs = cmos_attributes,
+};
+
 /* --------------------------------------------------------------------- */
 
 static int __init cmos_init(struct ibm_init_struct *iibm)
 {
-	int res;
-
 	vdbg_printk(TPACPI_DBG_INIT,
-		"initializing cmos commands subdriver\n");
+		    "initializing cmos commands subdriver\n");
 
 	TPACPI_ACPIHANDLE_INIT(cmos);
 
 	vdbg_printk(TPACPI_DBG_INIT, "cmos commands are %s\n",
-		str_supported(cmos_handle != NULL));
+		    str_supported(cmos_handle != NULL));
 
-	res = device_create_file(&tpacpi_pdev->dev, &dev_attr_cmos_command);
-	if (res)
-		return res;
-
-	return (cmos_handle) ? 0 : 1;
-}
-
-static void cmos_exit(void)
-{
-	device_remove_file(&tpacpi_pdev->dev, &dev_attr_cmos_command);
+	return cmos_handle ? 0 : 1;
 }
 
 static int cmos_read(struct seq_file *m)
@@ -5687,7 +5608,6 @@ static struct ibm_struct cmos_driver_data = {
 	.name = "cmos",
 	.read = cmos_read,
 	.write = cmos_write,
-	.exit = cmos_exit,
 };
 
 /*************************************************************************
@@ -6198,7 +6118,6 @@ struct ibm_thermal_sensors_struct {
 };
 
 static enum thermal_access_mode thermal_read_mode;
-static const struct attribute_group *thermal_attr_group;
 static bool thermal_use_labels;
 
 /* idx is zero-based */
@@ -6371,12 +6290,26 @@ static struct attribute *thermal_temp_input_attr[] = {
 	NULL
 };
 
-static const struct attribute_group thermal_temp_input16_group = {
-	.attrs = thermal_temp_input_attr
-};
+static umode_t thermal_attr_is_visible(struct kobject *kobj,
+				       struct attribute *attr, int n)
+{
+	if (thermal_read_mode == TPACPI_THERMAL_NONE)
+		return 0;
 
-static const struct attribute_group thermal_temp_input8_group = {
-	.attrs = &thermal_temp_input_attr[8]
+	if (attr == THERMAL_ATTRS(8) || attr == THERMAL_ATTRS(9) ||
+	    attr == THERMAL_ATTRS(10) || attr == THERMAL_ATTRS(11) ||
+	    attr == THERMAL_ATTRS(12) || attr == THERMAL_ATTRS(13) ||
+	    attr == THERMAL_ATTRS(14) || attr == THERMAL_ATTRS(15)) {
+		if (thermal_read_mode != TPACPI_THERMAL_TPEC_16)
+			return 0;
+	}
+
+	return attr->mode;
+}
+
+static const struct attribute_group thermal_attr_group = {
+	.is_visible = thermal_attr_is_visible,
+	.attrs = thermal_temp_input_attr,
 };
 
 #undef THERMAL_SENSOR_ATTR_TEMP
@@ -6400,7 +6333,14 @@ static struct attribute *temp_label_attributes[] = {
 	NULL
 };
 
+static umode_t temp_label_attr_is_visible(struct kobject *kobj,
+					  struct attribute *attr, int n)
+{
+	return thermal_use_labels ? attr->mode : 0;
+}
+
 static const struct attribute_group temp_label_attr_group = {
+	.is_visible = temp_label_attr_is_visible,
 	.attrs = temp_label_attributes,
 };
 
@@ -6411,7 +6351,6 @@ static int __init thermal_init(struct ibm_init_struct *iibm)
 	u8 t, ta1, ta2, ver = 0;
 	int i;
 	int acpi_tmp7;
-	int res;
 
 	vdbg_printk(TPACPI_DBG_INIT, "initializing thermal subdriver\n");
 
@@ -6486,42 +6425,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm)
 		str_supported(thermal_read_mode != TPACPI_THERMAL_NONE),
 		thermal_read_mode);
 
-	switch (thermal_read_mode) {
-	case TPACPI_THERMAL_TPEC_16:
-		thermal_attr_group = &thermal_temp_input16_group;
-		break;
-	case TPACPI_THERMAL_TPEC_8:
-	case TPACPI_THERMAL_ACPI_TMP07:
-	case TPACPI_THERMAL_ACPI_UPDT:
-		thermal_attr_group = &thermal_temp_input8_group;
-		break;
-	case TPACPI_THERMAL_NONE:
-	default:
-		return 1;
-	}
-
-	res = sysfs_create_group(&tpacpi_hwmon->kobj, thermal_attr_group);
-	if (res)
-		return res;
-
-	if (thermal_use_labels) {
-		res = sysfs_create_group(&tpacpi_hwmon->kobj, &temp_label_attr_group);
-		if (res) {
-			sysfs_remove_group(&tpacpi_hwmon->kobj, thermal_attr_group);
-			return res;
-		}
-	}
-
-	return 0;
-}
-
-static void thermal_exit(void)
-{
-	if (thermal_attr_group)
-		sysfs_remove_group(&tpacpi_hwmon->kobj, thermal_attr_group);
-
-	if (thermal_use_labels)
-		sysfs_remove_group(&tpacpi_hwmon->kobj, &temp_label_attr_group);
+	return thermal_read_mode == TPACPI_THERMAL_NONE ? 1 : 0;
 }
 
 static int thermal_read(struct seq_file *m)
@@ -6548,7 +6452,6 @@ static int thermal_read(struct seq_file *m)
 static struct ibm_struct thermal_driver_data = {
 	.name = "thermal",
 	.read = thermal_read,
-	.exit = thermal_exit,
 };
 
 /*************************************************************************
@@ -8723,14 +8626,33 @@ static ssize_t fan_watchdog_store(struct device_driver *drv, const char *buf,
 static DRIVER_ATTR_RW(fan_watchdog);
 
 /* --------------------------------------------------------------------- */
+
 static struct attribute *fan_attributes[] = {
-	&dev_attr_pwm1_enable.attr, &dev_attr_pwm1.attr,
+	&dev_attr_pwm1_enable.attr,
+	&dev_attr_pwm1.attr,
 	&dev_attr_fan1_input.attr,
-	NULL, /* for fan2_input */
+	&dev_attr_fan2_input.attr,
+	&driver_attr_fan_watchdog.attr,
 	NULL
 };
 
+static umode_t fan_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+				   int n)
+{
+	if (fan_status_access_mode == TPACPI_FAN_NONE &&
+	    fan_control_access_mode == TPACPI_FAN_WR_NONE)
+		return 0;
+
+	if (attr == &dev_attr_fan2_input.attr) {
+		if (!tp_features.second_fan)
+			return 0;
+	}
+
+	return attr->mode;
+}
+
 static const struct attribute_group fan_attr_group = {
+	.is_visible = fan_attr_is_visible,
 	.attrs = fan_attributes,
 };
 
@@ -8761,7 +8683,6 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
 
 static int __init fan_init(struct ibm_init_struct *iibm)
 {
-	int rc;
 	unsigned long quirks;
 
 	vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
@@ -8858,27 +8779,10 @@ static int __init fan_init(struct ibm_init_struct *iibm)
 		fan_get_status_safe(NULL);
 
 	if (fan_status_access_mode != TPACPI_FAN_NONE ||
-	    fan_control_access_mode != TPACPI_FAN_WR_NONE) {
-		if (tp_features.second_fan) {
-			/* attach second fan tachometer */
-			fan_attributes[ARRAY_SIZE(fan_attributes)-2] =
-					&dev_attr_fan2_input.attr;
-		}
-		rc = sysfs_create_group(&tpacpi_hwmon->kobj,
-					 &fan_attr_group);
-		if (rc < 0)
-			return rc;
-
-		rc = driver_create_file(&tpacpi_hwmon_pdriver.driver,
-					&driver_attr_fan_watchdog);
-		if (rc < 0) {
-			sysfs_remove_group(&tpacpi_hwmon->kobj,
-					&fan_attr_group);
-			return rc;
-		}
+	    fan_control_access_mode != TPACPI_FAN_WR_NONE)
 		return 0;
-	} else
-		return 1;
+
+	return 1;
 }
 
 static void fan_exit(void)
@@ -8886,11 +8790,6 @@ static void fan_exit(void)
 	vdbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_FAN,
 		    "cancelling any pending fan watchdog tasks\n");
 
-	/* FIXME: can we really do this unconditionally? */
-	sysfs_remove_group(&tpacpi_hwmon->kobj, &fan_attr_group);
-	driver_remove_file(&tpacpi_hwmon_pdriver.driver,
-			   &driver_attr_fan_watchdog);
-
 	cancel_delayed_work(&fan_watchdog_task);
 	flush_workqueue(tpacpi_wq);
 }
@@ -9952,6 +9851,35 @@ static ssize_t palmsensor_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(palmsensor);
 
+static struct attribute *proxsensor_attributes[] = {
+	&dev_attr_dytc_lapmode.attr,
+	&dev_attr_palmsensor.attr,
+	NULL
+};
+
+static umode_t proxsensor_attr_is_visible(struct kobject *kobj,
+					  struct attribute *attr, int n)
+{
+	if (attr == &dev_attr_dytc_lapmode.attr) {
+		/*
+		 * Platforms before DYTC version 5 claim to have a lap sensor,
+		 * but it doesn't work, so we ignore them.
+		 */
+		if (!has_lapsensor ||  dytc_version < 5)
+			return 0;
+	} else if (attr == &dev_attr_palmsensor.attr) {
+		if (!has_palmsensor)
+			return 0;
+	}
+
+	return attr->mode;
+}
+
+static const struct attribute_group proxsensor_attr_group = {
+	.is_visible = proxsensor_attr_is_visible,
+	.attrs = proxsensor_attributes,
+};
+
 static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm)
 {
 	int palm_err, lap_err, err;
@@ -9970,41 +9898,18 @@ static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm)
 	if (lap_err && (lap_err != -ENODEV))
 		return lap_err;
 
-	if (has_palmsensor) {
-		err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_palmsensor.attr);
-		if (err)
-			return err;
-	}
-
 	/* Check if we know the DYTC version, if we don't then get it */
 	if (!dytc_version) {
 		err = dytc_get_version();
 		if (err)
 			return err;
 	}
-	/*
-	 * Platforms before DYTC version 5 claim to have a lap sensor, but it doesn't work, so we
-	 * ignore them
-	 */
-	if (has_lapsensor && (dytc_version >= 5)) {
-		err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr);
-		if (err)
-			return err;
-	}
-	return 0;
-}
 
-static void proxsensor_exit(void)
-{
-	if (has_lapsensor)
-		sysfs_remove_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr);
-	if (has_palmsensor)
-		sysfs_remove_file(&tpacpi_pdev->dev.kobj, &dev_attr_palmsensor.attr);
+	return 0;
 }
 
 static struct ibm_struct proxsensor_driver_data = {
 	.name = "proximity-sensor",
-	.exit = proxsensor_exit,
 };
 
 /*************************************************************************
@@ -10421,7 +10326,14 @@ static struct attribute *kbdlang_attributes[] = {
 	NULL
 };
 
+static umode_t kbdlang_attr_is_visible(struct kobject *kobj,
+				       struct attribute *attr, int n)
+{
+	return tp_features.kbd_lang ? attr->mode : 0;
+}
+
 static const struct attribute_group kbdlang_attr_group = {
+	.is_visible = kbdlang_attr_is_visible,
 	.attrs = kbdlang_attributes,
 };
 
@@ -10430,28 +10342,12 @@ static int tpacpi_kbdlang_init(struct ibm_init_struct *iibm)
 	int err, output;
 
 	err = get_keyboard_lang(&output);
-	/*
-	 * If support isn't available (ENODEV) then don't return an error
-	 * just don't create the sysfs group.
-	 */
-	if (err == -ENODEV)
-		return 0;
-
-	if (err)
-		return err;
-
-	/* Platform supports this feature - create the sysfs file */
-	return sysfs_create_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group);
-}
-
-static void kbdlang_exit(void)
-{
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &kbdlang_attr_group);
+	tp_features.kbd_lang = !err;
+	return err;
 }
 
 static struct ibm_struct kbdlang_driver_data = {
 	.name = "kbdlang",
-	.exit = kbdlang_exit,
 };
 
 /*************************************************************************
@@ -10522,41 +10418,131 @@ static ssize_t wwan_antenna_type_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(wwan_antenna_type);
 
+static struct attribute *dprc_attributes[] = {
+	&dev_attr_wwan_antenna_type.attr,
+	NULL
+};
+
+static umode_t dprc_attr_is_visible(struct kobject *kobj,
+				    struct attribute *attr, int n)
+{
+	return has_antennatype ? attr->mode : 0;
+}
+
+static const struct attribute_group dprc_attr_group = {
+	.is_visible = dprc_attr_is_visible,
+	.attrs = dprc_attributes,
+};
+
 static int tpacpi_dprc_init(struct ibm_init_struct *iibm)
 {
-	int wwanantenna_err, err;
+	int err = get_wwan_antenna(&wwan_antennatype);
 
-	wwanantenna_err = get_wwan_antenna(&wwan_antennatype);
 	/*
 	 * If support isn't available (ENODEV) then quit, but don't
 	 * return an error.
 	 */
-	if (wwanantenna_err == -ENODEV)
+	if (err == -ENODEV)
 		return 0;
 
-	/* if there was an error return it */
-	if (wwanantenna_err && (wwanantenna_err != -ENODEV))
-		return wwanantenna_err;
-	else if (!wwanantenna_err)
-		has_antennatype = true;
+	/* If there was an error return it */
+	if (err)
+		return err;
 
-	if (has_antennatype) {
-		err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_wwan_antenna_type.attr);
-		if (err)
-			return err;
-	}
+	has_antennatype = true;
 	return 0;
 }
 
-static void dprc_exit(void)
+static struct ibm_struct dprc_driver_data = {
+	.name = "dprc",
+};
+
+/* --------------------------------------------------------------------- */
+
+static struct attribute *tpacpi_attributes[] = {
+	&driver_attr_debug_level.attr,
+	&driver_attr_version.attr,
+	&driver_attr_interface_version.attr,
+#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
+	&driver_attr_wlsw_emulstate.attr,
+	&driver_attr_bluetooth_emulstate.attr,
+	&driver_attr_wwan_emulstate.attr,
+	&driver_attr_uwb_emulstate.attr,
+#endif
+	NULL
+};
+
+#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
+static umode_t tpacpi_attr_is_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
 {
-	if (has_antennatype)
-		sysfs_remove_file(&tpacpi_pdev->dev.kobj, &dev_attr_wwan_antenna_type.attr);
+	if (attr == &driver_attr_wlsw_emulstate.attr) {
+		if (!dbg_wlswemul)
+			return 0;
+	} else if (attr == &driver_attr_bluetooth_emulstate.attr) {
+		if (!dbg_bluetoothemul)
+			return 0;
+	} else if (attr == &driver_attr_wwan_emulstate.attr) {
+		if (!dbg_wwanemul)
+			return 0;
+	} else if (attr == &driver_attr_uwb_emulstate.attr) {
+		if (!dbg_uwbemul)
+			return 0;
+	}
+
+	return attr->mode;
 }
+#endif
 
-static struct ibm_struct dprc_driver_data = {
-	.name = "dprc",
-	.exit = dprc_exit,
+static const struct attribute_group tpacpi_attr_group = {
+#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
+	.is_visible = tpacpi_attr_is_visible,
+#endif
+	.attrs = tpacpi_attributes,
+};
+
+static const struct attribute_group *tpacpi_groups[] = {
+	&adaptive_kbd_attr_group,
+	&hotkey_attr_group,
+	&bluetooth_attr_group,
+	&wan_attr_group,
+	&cmos_attr_group,
+	&proxsensor_attr_group,
+	&kbdlang_attr_group,
+	&dprc_attr_group,
+	&tpacpi_attr_group,
+	NULL,
+};
+
+static const struct attribute_group *tpacpi_hwmon_groups[] = {
+	&thermal_attr_group,
+	&temp_label_attr_group,
+	&fan_attr_group,
+	NULL,
+};
+
+/****************************************************************************
+ ****************************************************************************
+ *
+ * Platform drivers
+ *
+ ****************************************************************************
+ ****************************************************************************/
+
+static struct platform_driver tpacpi_pdriver = {
+	.driver = {
+		.name = TPACPI_DRVR_NAME,
+		.pm = &tpacpi_pm,
+		.dev_groups = tpacpi_groups,
+	},
+	.shutdown = tpacpi_shutdown_handler,
+};
+
+static struct platform_driver tpacpi_hwmon_pdriver = {
+	.driver = {
+		.name = TPACPI_HWMON_DRVR_NAME,
+		.dev_groups = tpacpi_hwmon_groups,
+	},
 };
 
 /****************************************************************************
@@ -11079,8 +11065,6 @@ static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
 
 	for (i = 0; i < ARRAY_SIZE(ibms_init); i++) {
 		ibm = ibms_init[i].data;
-		WARN_ON(ibm == NULL);
-
 		if (!ibm || !ibm->name)
 			continue;
 
@@ -11210,26 +11194,16 @@ static void thinkpad_acpi_module_exit(void)
 
 	if (tpacpi_hwmon)
 		hwmon_device_unregister(tpacpi_hwmon);
-
 	if (tpacpi_sensors_pdev)
 		platform_device_unregister(tpacpi_sensors_pdev);
 	if (tpacpi_pdev)
 		platform_device_unregister(tpacpi_pdev);
-
-	if (tp_features.sensors_pdrv_attrs_registered)
-		tpacpi_remove_driver_attributes(&tpacpi_hwmon_pdriver.driver);
-	if (tp_features.platform_drv_attrs_registered)
-		tpacpi_remove_driver_attributes(&tpacpi_pdriver.driver);
-
 	if (tp_features.sensors_pdrv_registered)
 		platform_driver_unregister(&tpacpi_hwmon_pdriver);
-
 	if (tp_features.platform_drv_registered)
 		platform_driver_unregister(&tpacpi_pdriver);
-
 	if (proc_dir)
 		remove_proc_entry(TPACPI_PROC_DIR, acpi_root_dir);
-
 	if (tpacpi_wq)
 		destroy_workqueue(tpacpi_wq);
 
@@ -11297,20 +11271,6 @@ static int __init thinkpad_acpi_module_init(void)
 	}
 	tp_features.sensors_pdrv_registered = 1;
 
-	ret = tpacpi_create_driver_attributes(&tpacpi_pdriver.driver);
-	if (!ret) {
-		tp_features.platform_drv_attrs_registered = 1;
-		ret = tpacpi_create_driver_attributes(
-					&tpacpi_hwmon_pdriver.driver);
-	}
-	if (ret) {
-		pr_err("unable to create sysfs driver attributes\n");
-		thinkpad_acpi_module_exit();
-		return ret;
-	}
-	tp_features.sensors_pdrv_attrs_registered = 1;
-
-
 	/* Device initialization */
 	tpacpi_pdev = platform_device_register_simple(TPACPI_DRVR_NAME, -1,
 							NULL, 0);