ipts: Address comments by qzed and grayhatter

* Removed some of the gotos in the simpler companion function
* Made the companion driver the first parameter in the firmware handler
* Changed ipts-surface to warn when adding the companion driver failed
* Made ipts_alloc_fw_list static

Signed-off-by: Dorian Stoll <dorian.stoll@tmsp.io>

Author: StollD

drivers/misc/ipts/companion/ipts-surface.c

@@ -13,8 +13,9 @@
 	MODULE_FIRMWARE("intel/ipts/" X "/vendor_desc.bin");		\
 	MODULE_FIRMWARE("intel/ipts/" X "/vendor_kernel.bin");		\
 
-int ipts_surface_request_firmware(const struct firmware **fw, const char *name,
-		struct device *device, ipts_companion_t *companion)
+int ipts_surface_request_firmware(ipts_companion_t *companion,
+		const struct firmware **fw, const char *name,
+		struct device *device)
 {
 	char fw_path[MAX_IOCL_FILE_PATH_LEN];
 
@@ -77,7 +78,7 @@ static int ipts_surface_probe(struct platform_device *pdev)
 	ipts_surface_companion.data = (void *)acpi_device_hid(adev);
 	ret = ipts_add_companion(&ipts_surface_companion);
 	if (ret) {
-		dev_info(&pdev->dev, "Adding IPTS companion failed, "
+		dev_warn(&pdev->dev, "Adding IPTS companion failed, "
 				"error: %d\n", ret);
 		return ret;
 	}
@@ -89,7 +90,7 @@ static int ipts_surface_remove(struct platform_device *pdev)
 {
 	int ret = ipts_remove_companion(&ipts_surface_companion);
 	if (ret) {
-		dev_info(&pdev->dev, "Removing IPTS companion failed, "
+		dev_warn(&pdev->dev, "Removing IPTS companion failed, "
 				"error: %d\n", ret);
 	}
 

drivers/misc/ipts/ipts-companion.c

@@ -35,42 +35,34 @@ bool ipts_companion_available(void)
 
 int ipts_add_companion(ipts_companion_t *companion)
 {
-	int ret = 0;
+	int ret;
 	mutex_lock(&ipts_companion_lock);
 
-	if (ipts_companion != NULL) {
+	if (ipts_companion == NULL) {
+		ret = 0;
+		ipts_companion = companion;
+	} else {
 		ret = -EBUSY;
-		goto add_companion_return;
 	}
 
-	ipts_companion = companion;
-
-add_companion_return:
-
 	mutex_unlock(&ipts_companion_lock);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(ipts_add_companion);
 
 int ipts_remove_companion(ipts_companion_t *companion)
 {
-	int ret = 0;
+	int ret;
 	mutex_lock(&ipts_companion_lock);
 
-	if (ipts_companion == NULL || companion == NULL) {
-		ret = 0;
-		goto remove_companion_return;
-	}
-
-	if (ipts_companion->name != companion->name) {
+	if (ipts_companion != NULL && companion != NULL &&
+			ipts_companion->name != companion->name) {
 		ret = -EPERM;
-		goto remove_companion_return;
+	} else {
+		ret = 0;
+		ipts_companion = NULL;
 	}
 
-	ipts_companion = NULL;
-
-remove_companion_return:
-
 	mutex_unlock(&ipts_companion_lock);
 	return ret;
 }
@@ -95,7 +87,7 @@ int ipts_request_firmware(const struct firmware **fw, const char *name,
 		goto request_firmware_fallback;
 	}
 
-	ret = ipts_companion->firmware_request(fw, name, device, ipts_companion);
+	ret = ipts_companion->firmware_request(ipts_companion, fw, name, device);
 	if (!ret) {
 		goto request_firmware_return;
 	}
@@ -119,7 +111,7 @@ int ipts_request_firmware(const struct firmware **fw, const char *name,
 	return ret;
 }
 
-ipts_bin_fw_list_t *ipts_alloc_fw_list(ipts_bin_fw_info_t **fw)
+static ipts_bin_fw_list_t *ipts_alloc_fw_list(ipts_bin_fw_info_t **fw)
 {
 	int size, len, i, j;
 	ipts_bin_fw_list_t *fw_list;

include/linux/ipts-companion.h

@@ -6,8 +6,8 @@
 
 typedef struct ipts_companion ipts_companion_t;
 
-typedef int (*ipts_fw_handler_t)(const struct firmware **, const char *,
-		struct device *, ipts_companion_t *companion);
+typedef int (*ipts_fw_handler_t)(ipts_companion_t *, const struct firmware **,
+		const char *, struct device *);
 
 struct ipts_companion {
 	ipts_fw_handler_t firmware_request;