~lkcamp/patches

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH] staging: gdm724x: remove instances of functions returning -1

Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
Details
Message ID
<20241002023721.449600-1-rodrigo.gobbi.7@gmail.com>
DKIM signature
pass
Download raw message
2 months ago
Patch: +5 -6 
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.

[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
Gabriela Bittencourt <gbittencourt@lkcamp.dev>
Details
Message ID
<833d9676-ef35-4f03-9331-1b0d6aa543bf@lkcamp.dev>
In-Reply-To
<20241002023721.449600-1-rodrigo.gobbi.7@gmail.com> (view parent)
DKIM signature
pass
Download raw message
a month ago 
Hi Rodrigo,

For me the subject line could be clearer, something like this:
`staging: gdm724x: fix returning -1 with return equivalent errors`

On 10/1/24 11:37 PM, 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.
> 
> There is a RFC to delete this driver at [1], so I'm not
> sure if this change is worth it.
> 
> [1] https://lore.kernel.org/lkml/50020db0-3bad-41f5-8da3-c66bc0a90fe6@gmail.com/
> 

Yeh, maybe you had bad look :(
I suggest you send this patch in the mailing lists (as it is, no need to 
bother changing it for now). Just don't forget to add these last comments.
And see where the discussion on the mailing lists heads to.

>   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);

Thanks anyway, that was a good catch :)
Reply to thread Export thread (mbox)