Add build and install instructions

[ Upstream commit ddd4e9d78057383704a84cbe462bb63598c9baef ]

Statically allocated array of pointers to hwmon_channel_info can be made
const for safety.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/r/20230511175627.282246-3-krzysztof.kozlowski@linaro.org
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Tim Crawford <tcrawford@system76.com>

README.md

@@ -1,3 +1,18 @@
 # System76 ACPI Driver (DKMS)
 
 This provides the system76_acpi in-tree driver for systems missing it.
+
+## Building .deb from source
+```bash
+sudo dpkg-buildpackage  -b -uc -us
+```
+
+## Installing from built .deb
+```bash
+sudo dpkg -i ../system76-acpi-dkms_1.0.2_amd64.deb
+```
+
+## build, install and reload kernel module (for developing)
+```bash
+sudo dpkg-buildpackage  -b -uc -us && sudo dpkg -i ../system76-acpi-dkms_1.0.2_amd64.deb && sudo modprobe -r system76_acpi && sleep 5 && sudo modprobe system76_acpi
+```

debian/control

@@ -4,7 +4,8 @@ Priority: optional
 Maintainer: Jeremy Soller <jeremy@system76.com>
 Build-Depends:
   debhelper (>=9),
-  dkms
+  dh-sequence-dkms,
+  dkms,
 Standards-Version: 4.1.1
 Homepage: https://github.com/pop-os/system76-acpi-dkms
 

system76_acpi.c

@@ -25,6 +25,12 @@
 
 #include <acpi/battery.h>
 
+enum kbled_type {
+	KBLED_NONE,
+	KBLED_WHITE,
+	KBLED_RGB,
+};
+
 struct system76_data {
 	struct acpi_device *acpi_dev;
 	struct led_classdev ap_led;
@@ -36,6 +42,8 @@ struct system76_data {
 	union acpi_object *nfan;
 	union acpi_object *ntmp;
 	struct input_dev *input;
+	bool has_open_ec;
+	enum kbled_type kbled_type;
 };
 
 static const struct acpi_device_id device_ids[] = {
@@ -148,8 +156,6 @@ static int system76_set(struct system76_data *data, char *method, int value)
 		return -1;
 }
 
-/* Battery */
-
 #define BATTERY_THRESHOLD_INVALID	0xFF
 
 enum {
@@ -260,7 +266,11 @@ static struct attribute *system76_battery_attrs[] = {
 
 ATTRIBUTE_GROUPS(system76_battery);
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,2,0)
+static int system76_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
+#else
 static int system76_battery_add(struct power_supply *battery)
+#endif
 {
 	// System76 EC only supports 1 battery
 	if (strcmp(battery->desc->name, "BAT0") != 0)
@@ -272,7 +282,11 @@ static int system76_battery_add(struct power_supply *battery)
 	return 0;
 }
 
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,2,0)
+static int system76_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
+#else
 static int system76_battery_remove(struct power_supply *battery)
+#endif
 {
 	device_remove_groups(&battery->dev, system76_battery_groups);
 	return 0;
@@ -286,24 +300,14 @@ static struct acpi_battery_hook system76_battery_hook = {
 
 static void system76_battery_init(void)
 {
-	acpi_handle handle;
+	battery_hook_register(&system76_battery_hook);
-
-	handle = ec_get_handle();
-	if (handle && acpi_has_method(handle, "GBCT"))
-		battery_hook_register(&system76_battery_hook);
 }
 
 static void system76_battery_exit(void)
 {
-	acpi_handle handle;
+	battery_hook_unregister(&system76_battery_hook);
-
-	handle = ec_get_handle();
-	if (handle && acpi_has_method(handle, "GBCT"))
-		battery_hook_unregister(&system76_battery_hook);
 }
 
-/* Keyboard */
-
 // Get the airplane mode LED brightness
 static enum led_brightness ap_led_get(struct led_classdev *led)
 {
@@ -343,7 +347,11 @@ static int kb_led_set(struct led_classdev *led, enum led_brightness value)
 
 	data = container_of(led, struct system76_data, kb_led);
 	data->kb_brightness = value;
-	return system76_set(data, "SKBL", (int)data->kb_brightness);
+	if (acpi_has_method(acpi_device_handle(data->acpi_dev), "GKBK")) {
+		return system76_set(data, "SKBB", (int)data->kb_brightness);
+	} else {
+		return system76_set(data, "SKBL", (int)data->kb_brightness);
+	}
 }
 
 // Get the last set keyboard LED color
@@ -355,7 +363,7 @@ static ssize_t kb_led_color_show(
 	struct led_classdev *led;
 	struct system76_data *data;
 
-	led = (struct led_classdev *)dev->driver_data;
+	led = dev_get_drvdata(dev);
 	data = container_of(led, struct system76_data, kb_led);
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 10, 0)
 	return sysfs_emit(buf, "%06X\n", data->kb_color);
@@ -376,7 +384,7 @@ static ssize_t kb_led_color_store(
 	unsigned int val;
 	int ret;
 
-	led = (struct led_classdev *)dev->driver_data;
+	led = dev_get_drvdata(dev);
 	data = container_of(led, struct system76_data, kb_led);
 	ret = kstrtouint(buf, 16, &val);
 	if (ret)
@@ -419,7 +427,12 @@ static void kb_led_hotkey_hardware(struct system76_data *data)
 {
 	int value;
 
-	value = system76_get(data, "GKBL");
+	if (acpi_has_method(acpi_device_handle(data->acpi_dev), "GKBK")) {
+		value = system76_get(data, "GKBB");
+	} else {
+		value = system76_get(data, "GKBL");
+	}
+
 	if (value < 0)
 		return;
 	data->kb_brightness = value;
@@ -479,8 +492,9 @@ static void kb_led_hotkey_color(struct system76_data *data)
 {
 	int i;
 
-	if (data->kb_color < 0)
+	if (data->kbled_type != KBLED_RGB)
 		return;
+
 	if (data->kb_brightness > 0) {
 		for (i = 0; i < ARRAY_SIZE(kb_colors); i++) {
 			if (kb_colors[i] == data->kb_color)
@@ -497,8 +511,6 @@ static void kb_led_hotkey_color(struct system76_data *data)
 	kb_led_notify(data);
 }
 
-/* hwmon */
-
 static umode_t thermal_is_visible(const void *drvdata, enum hwmon_sensor_types type,
 				  u32 attr, int channel)
 {
@@ -603,7 +615,7 @@ static const struct hwmon_ops thermal_ops = {
 };
 
 // Allocate up to 8 fans and temperatures
-static const struct hwmon_channel_info *thermal_channel_info[] = {
+static const struct hwmon_channel_info * const thermal_channel_info[] = {
 	HWMON_CHANNEL_INFO(fan,
 		HWMON_F_INPUT | HWMON_F_LABEL,
 		HWMON_F_INPUT | HWMON_F_LABEL,
@@ -639,8 +651,6 @@ static const struct hwmon_chip_info thermal_chip_info = {
 	.info = thermal_channel_info,
 };
 
-/* ACPI driver */
-
 static void input_key(struct system76_data *data, unsigned int code)
 {
 	input_report_key(data->input, code, 1);
@@ -690,6 +700,10 @@ static int system76_add(struct acpi_device *acpi_dev)
 	acpi_dev->driver_data = data;
 	data->acpi_dev = acpi_dev;
 
+	// Some models do not run open EC firmware. Check for an ACPI method
+	// that only exists on open EC to guard functionality specific to it.
+	data->has_open_ec = acpi_has_method(acpi_device_handle(data->acpi_dev), "NFAN");
+
 	err = system76_get(data, "INIT");
 	if (err)
 		return err;
@@ -707,19 +721,46 @@ static int system76_add(struct acpi_device *acpi_dev)
 	data->kb_led.flags = LED_BRIGHT_HW_CHANGED | LED_CORE_SUSPENDRESUME;
 	data->kb_led.brightness_get = kb_led_get;
 	data->kb_led.brightness_set_blocking = kb_led_set;
-	if (acpi_has_method(acpi_device_handle(data->acpi_dev), "SKBC")) {
+	if (acpi_has_method(acpi_device_handle(data->acpi_dev), "GKBK")) {
-		data->kb_led.max_brightness = 255;
+		// Use the new ACPI methods
-		data->kb_led.groups = system76_kb_led_color_groups;
+		data->kbled_type = system76_get(data, "GKBK");
-		data->kb_toggle_brightness = 72;
+
-		data->kb_color = 0xffffff;
+		switch (data->kbled_type) {
-		system76_set(data, "SKBC", data->kb_color);
+		case KBLED_NONE:
+			// Nothing to do: Device will not be registered.
+			break;
+		case KBLED_WHITE:
+			data->kb_led.max_brightness = 255;
+			data->kb_toggle_brightness = 72;
+			break;
+		case KBLED_RGB:
+			data->kb_led.max_brightness = 255;
+			data->kb_led.groups = system76_kb_led_color_groups;
+			data->kb_toggle_brightness = 72;
+			data->kb_color = 0xffffff;
+			system76_set(data, "SKBC", data->kb_color);
+			break;
+		}
 	} else {
-		data->kb_led.max_brightness = 5;
+		// Use the old ACPI methods
-		data->kb_color = -1;
+		if (acpi_has_method(acpi_device_handle(data->acpi_dev), "SKBC")) {
+			data->kbled_type = KBLED_RGB;
+			data->kb_led.max_brightness = 255;
+			data->kb_led.groups = system76_kb_led_color_groups;
+			data->kb_toggle_brightness = 72;
+			data->kb_color = 0xffffff;
+			system76_set(data, "SKBC", data->kb_color);
+		} else {
+			data->kbled_type = KBLED_WHITE;
+			data->kb_led.max_brightness = 5;
+		}
+	}
+
+	if (data->kbled_type != KBLED_NONE) {
+		err = devm_led_classdev_register(&acpi_dev->dev, &data->kb_led);
+		if (err)
+			return err;
 	}
-	err = devm_led_classdev_register(&acpi_dev->dev, &data->kb_led);
-	if (err)
-		return err;
 
 	data->input = devm_input_allocate_device(&acpi_dev->dev);
 	if (!data->input)
@@ -735,48 +776,59 @@ static int system76_add(struct acpi_device *acpi_dev)
 	if (err)
 		goto error;
 
-	err = system76_get_object(data, "NFAN", &data->nfan);
+	if (data->has_open_ec) {
-	if (err)
+		err = system76_get_object(data, "NFAN", &data->nfan);
-		goto error;
+		if (err)
+			goto error;
 
-	err = system76_get_object(data, "NTMP", &data->ntmp);
+		err = system76_get_object(data, "NTMP", &data->ntmp);
-	if (err)
+		if (err)
-		goto error;
+			goto error;
 
-	data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
+		data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
-		"system76_acpi", data, &thermal_chip_info, NULL);
+			"system76_acpi", data, &thermal_chip_info, NULL);
-	err = PTR_ERR_OR_ZERO(data->therm);
+		err = PTR_ERR_OR_ZERO(data->therm);
-	if (err)
+		if (err)
-		goto error;
+			goto error;
 
-	system76_battery_init();
+		system76_battery_init();
+	}
 
 	return 0;
 
 error:
-	kfree(data->ntmp);
+	if (data->has_open_ec) {
-	kfree(data->nfan);
+		kfree(data->ntmp);
+		kfree(data->nfan);
+	}
 	return err;
 }
 
 // Remove a System76 ACPI device
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,2,0)
+static void system76_remove(struct acpi_device *acpi_dev)
+#else
 static int system76_remove(struct acpi_device *acpi_dev)
+#endif
 {
 	struct system76_data *data;
 
 	data = acpi_driver_data(acpi_dev);
 
-	system76_battery_exit();
+	if (data->has_open_ec) {
+		system76_battery_exit();
+		kfree(data->nfan);
+		kfree(data->ntmp);
+	}
 
 	devm_led_classdev_unregister(&acpi_dev->dev, &data->ap_led);
 	devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led);
 
-	kfree(data->nfan);
-	kfree(data->ntmp);
-
 	system76_get(data, "FINI");
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(6,2,0)
 	return 0;
+#endif
 }
 
 static struct acpi_driver system76_driver = {