[PATCH] staging: gdm724x: fix returning -1 with return equivalent errors

[PATCH] staging: gdm724x: fix returning -1 with return equivalent errors

[Rodrigo Gobbi]

As in the TODO file, use proper error codes from PM callbacks and init.

Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
---
Only one reference was left, regarding the packet_type_to_tty_index() but
I think it's reasonable to keep it that way since it's for tty index purpose.

There is a RFC to delete this driver at [1], so I'm not 
sure if this change is worth it. Anyway, I'm submitting 
it and I'll be waiting for an opinion about this.
Tks.

[1] https://lore.kernel.org/lkml/50020db0-3bad-41f5-8da3-c66bc0a90fe6@gmail.com/

 drivers/staging/gdm724x/TODO      | 1 -
 drivers/staging/gdm724x/gdm_mux.c | 4 ++--
 drivers/staging/gdm724x/gdm_usb.c | 6 +++---
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/gdm724x/TODO b/drivers/staging/gdm724x/TODO
index b2b571ecb063..56a415b9dcbe 100644
--- a/drivers/staging/gdm724x/TODO
+++ b/drivers/staging/gdm724x/TODO
@@ -2,7 +2,6 @@ TODO:
 - Clean up coding style to meet kernel standard. (80 line limit, netdev_err)
 - Remove test for host endian
 - Remove confusing macros (endian, hci_send, sdu_send, rcv_with_cb)
-- Fixes for every instances of function returning -1
 - Check for skb->len in gdm_lte_emulate_arp()
 - Use ALIGN() macro for dummy_cnt in up_to_host()
 - Error handling in init_usb()
diff --git a/drivers/staging/gdm724x/gdm_mux.c b/drivers/staging/gdm724x/gdm_mux.c
index 9b12619671a1..456e7cb66f03 100644
--- a/drivers/staging/gdm724x/gdm_mux.c
+++ b/drivers/staging/gdm724x/gdm_mux.c
@@ -594,7 +594,7 @@ static int gdm_mux_suspend(struct usb_interface *intf, pm_message_t pm_msg)
 
 	if (mux_dev->usb_state != PM_NORMAL) {
 		dev_err(intf->usb_dev, "usb suspend - invalid state\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	mux_dev->usb_state = PM_SUSPEND;
@@ -622,7 +622,7 @@ static int gdm_mux_resume(struct usb_interface *intf)
 
 	if (mux_dev->usb_state != PM_SUSPEND) {
 		dev_err(intf->usb_dev, "usb resume - invalid state\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	mux_dev->usb_state = PM_NORMAL;
diff --git a/drivers/staging/gdm724x/gdm_usb.c b/drivers/staging/gdm724x/gdm_usb.c
index 54bdb64f52e8..e4bbab6cb047 100644
--- a/drivers/staging/gdm724x/gdm_usb.c
+++ b/drivers/staging/gdm724x/gdm_usb.c
@@ -916,7 +916,7 @@ static int gdm_usb_suspend(struct usb_interface *intf, pm_message_t pm_msg)
 	rx = &udev->rx;
 	if (udev->usb_state != PM_NORMAL) {
 		dev_err(intf->usb_dev, "usb suspend - invalid state\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	udev->usb_state = PM_SUSPEND;
@@ -952,7 +952,7 @@ static int gdm_usb_resume(struct usb_interface *intf)
 
 	if (udev->usb_state != PM_SUSPEND) {
 		dev_err(intf->usb_dev, "usb resume - invalid state\n");
-		return -1;
+		return -EINVAL;
 	}
 	udev->usb_state = PM_NORMAL;
 
@@ -991,7 +991,7 @@ static int __init gdm_usb_lte_init(void)
 {
 	if (gdm_lte_event_init() < 0) {
 		pr_err("error creating event\n");
-		return -1;
+		return -ENODEV;
 	}
 
 	return usb_register(&gdm_usb_lte_driver);
-- 
2.34.1

Re: [PATCH] staging: gdm724x: fix returning -1 with return equivalent errors

[Dan Carpenter]

On Thu, Oct 03, 2024 at 11:20:25PM -0300, Rodrigo Gobbi wrote:
> As in the TODO file, use proper error codes from PM callbacks and init.
> 
> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
> ---
> Only one reference was left, regarding the packet_type_to_tty_index() but
> I think it's reasonable to keep it that way since it's for tty index purpose.

Better to return an error code there as well.  It doesn't change runtime
behavior at all, we're just having a discussoin about cleanliness.  I think
error codes are more clean.

> 
> There is a RFC to delete this driver at [1], so I'm not 
> sure if this change is worth it. Anyway, I'm submitting 
> it and I'll be waiting for an opinion about this.
> Tks.
> 
> [1] https://lore.kernel.org/lkml/50020db0-3bad-41f5-8da3-c66bc0a90fe6@gmail.com/

Most likely Greg will keep merging patches until the driver is removed.  It's
up to you to decide if it's worth your time to keeps sending the patches.

> @@ -991,7 +991,7 @@ static int __init gdm_usb_lte_init(void)
>  {
>  	if (gdm_lte_event_init() < 0) {
>  		pr_err("error creating event\n");
> -		return -1;
> +		return -ENODEV;
>  	}

This should be:

	ret = gdm_lte_event_init();
	if (ret < 0) {
		pr_err("error creating event\n");
		return ret;
	}

regards,
dan carpenter

Re: [PATCH] staging: gdm724x: fix returning -1 with return equivalent errors

[Rodrigo Gobbi]

First, Tks for the response and suggestion, Dan.

> Most likely Greg will keep merging patches until the driver is removed.  It's
> up to you to decide if it's worth your time to keeps sending the patches.

That's fine, I'll submit a v2 soon.
Tks and regards.