diff mbox series

[3/4] platform/x86: int3472: Fix skl_int3472_handle_gpio_resources() return value

Message ID 20241128154212.6216-3-hdegoede@redhat.com (mailing list archive)
State New
Headers show
Series [1/4] platform/x86: int3472: Check for adev == NULL | expand

Commit Message

Hans de Goede Nov. 28, 2024, 3:42 p.m. UTC 
The INT3472 code never wants a copy of the ACPI resource to be added
to the list-head passed to acpi_dev_get_resources().

Make skl_int3472_handle_gpio_resources() always return -errno or 1
and drop the now no longer acpi_dev_free_resource_list() call.

Also update the inaccurate comment about the return value.
skl_int3472_handle_gpio_resources() was already returning 1 in the case
of not a GPIO resource or invalid _DSM return and not -EINVAL / -ENODEV
as the comment claimed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Despite the "Fix" in the subject this is not really a bugfix, the old code
works fine too, so no Fixes tag
---
 drivers/platform/x86/intel/int3472/discrete.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Andy Shevchenko Nov. 28, 2024, 3:53 p.m. UTC | #1 
On Thu, Nov 28, 2024 at 04:42:11PM +0100, Hans de Goede wrote:
> The INT3472 code never wants a copy of the ACPI resource to be added
> to the list-head passed to acpi_dev_get_resources().
> 
> Make skl_int3472_handle_gpio_resources() always return -errno or 1
> and drop the now no longer acpi_dev_free_resource_list() call.
> 
> Also update the inaccurate comment about the return value.
> skl_int3472_handle_gpio_resources() was already returning 1 in the case
> of not a GPIO resource or invalid _DSM return and not -EINVAL / -ENODEV
> as the comment claimed.

...

> -	acpi_dev_free_resource_list(&resource_list);

Even though it's better to have this (no-op) call. As people may use the driver
as an example and then make the real leakage somewhere else.
diff mbox series

Patch

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 01da18b426ae..05e442078f8f 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -178,11 +178,11 @@  static void int3472_get_func_and_polarity(u8 type, const char **func, u32 *polar
  * to create clocks and regulators via the usual frameworks.
  *
  * Return:
- * * 1		- To continue the loop
- * * 0		- When all resources found are handled properly.
- * * -EINVAL	- If the resource is not a GPIO IO resource
- * * -ENODEV	- If the resource has no corresponding _DSM entry
- * * -Other	- Errors propagated from one of the sub-functions.
+ * * 1		- Continue the loop without adding a copy of the resource to
+ * *		  the list passed to acpi_dev_get_resources()
+ * * 0		- Continue the loop after adding a copy of the resource to
+ * *		  the list passed to acpi_dev_get_resources()
+ * * -errno	- Error, break loop
  */
 static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 					     void *data)
@@ -283,7 +283,8 @@  static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	if (ret < 0)
 		return dev_err_probe(int3472->dev, ret, err_msg);
 
-	return ret;
+	/* Tell acpi_dev_get_resources() to not make a copy of the resource */
+	return 1;
 }
 
 static int skl_int3472_parse_crs(struct int3472_discrete_device *int3472)
@@ -299,8 +300,6 @@  static int skl_int3472_parse_crs(struct int3472_discrete_device *int3472)
 	if (ret < 0)
 		return ret;
 
-	acpi_dev_free_resource_list(&resource_list);
-
 	/* Register _DSM based clock (no-op if a GPIO clock was already registered) */
 	ret = skl_int3472_register_dsm_clock(int3472);
 	if (ret < 0)