* [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation;
@ 2021-08-17 14:06 Petko Manolov
2021-08-17 16:06 ` Greg KH
2021-08-18 9:19 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Petko Manolov @ 2021-08-17 14:06 UTC (permalink / raw)
To: netdev; +Cc: kuba, paskripkin, stable, davem, Petko Manolov
- restore the behavior in enable_net_traffic() to avoid regressions - Jakub
Kicinski;
- hurried up and removed redundant assignment in pegasus_open() before yet
another checker complains;
- explicitly check for negative value in pegasus_set_wol(), even if
usb_control_msg_send() never return positive number we'd still be in sync
with the rest of the driver style;
Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
drivers/net/usb/pegasus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 652e9fcf0b77..1ef93082c772 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -446,7 +446,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
write_mii_word(pegasus, 0, 0x1b, &auxmode);
}
- return 0;
+ return ret;
fail:
netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
return ret;
@@ -835,7 +835,7 @@ static int pegasus_open(struct net_device *net)
if (!pegasus->rx_skb)
goto exit;
- res = set_registers(pegasus, EthID, 6, net->dev_addr);
+ set_registers(pegasus, EthID, 6, net->dev_addr);
usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
usb_rcvbulkpipe(pegasus->usb, 1),
@@ -932,7 +932,7 @@ pegasus_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
pegasus->wolopts = wol->wolopts;
ret = set_register(pegasus, WakeupControl, reg78);
- if (!ret)
+ if (ret < 0)
ret = device_set_wakeup_enable(&pegasus->usb->dev,
wol->wolopts);
return ret;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation;
2021-08-17 14:06 [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation; Petko Manolov
@ 2021-08-17 16:06 ` Greg KH
2021-08-18 9:19 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-08-17 16:06 UTC (permalink / raw)
To: Petko Manolov; +Cc: netdev, kuba, paskripkin, stable, davem
On Tue, Aug 17, 2021 at 05:06:13PM +0300, Petko Manolov wrote:
> - restore the behavior in enable_net_traffic() to avoid regressions - Jakub
> Kicinski;
> - hurried up and removed redundant assignment in pegasus_open() before yet
> another checker complains;
> - explicitly check for negative value in pegasus_set_wol(), even if
> usb_control_msg_send() never return positive number we'd still be in sync
> with the rest of the driver style;
>
> Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> ---
> drivers/net/usb/pegasus.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation;
2021-08-17 14:06 [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation; Petko Manolov
2021-08-17 16:06 ` Greg KH
@ 2021-08-18 9:19 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2021-08-18 9:19 UTC (permalink / raw)
To: petko.manolov; +Cc: netdev, kuba, paskripkin, stable
Please resubmit with a proper Fixes: tag, thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation;
2021-08-19 19:34 ` Jakub Kicinski
@ 2021-08-20 7:08 ` Petko Manolov
0 siblings, 0 replies; 6+ messages in thread
From: Petko Manolov @ 2021-08-20 7:08 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, paskripkin, stable
On 21-08-19 12:34:29, Jakub Kicinski wrote:
> On Thu, 19 Aug 2021 12:05:39 +0300 Petko Manolov wrote:
> > - restore the behavior in enable_net_traffic() to avoid regressions - Jakub
> > Kicinski;
> > - hurried up and removed redundant assignment in pegasus_open() before yet
> > another checker complains;
> > - explicitly check for negative value in pegasus_set_wol(), even if
> > usb_control_msg_send() never return positive number we'd still be in sync
> > with the rest of the driver style;
> >
> > Fixes: 8a160e2e9aeb net: usb: pegasus: Check the return value of get_geristers() and friends;
>
> I guess this is fine but not exactly the preferred format, please see
> Submitting patches.
If the preferred format involves brackets and quotes - so be it.
> > Reported-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> > ---
> > drivers/net/usb/pegasus.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> > index 652e9fcf0b77..1ef93082c772 100644
> > --- a/drivers/net/usb/pegasus.c
> > +++ b/drivers/net/usb/pegasus.c
> > @@ -446,7 +446,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
> > write_mii_word(pegasus, 0, 0x1b, &auxmode);
> > }
> >
> > - return 0;
> > + return ret;
>
> yup
>
> > fail:
> > netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
> > return ret;
> > @@ -835,7 +835,7 @@ static int pegasus_open(struct net_device *net)
> > if (!pegasus->rx_skb)
> > goto exit;
> >
> > - res = set_registers(pegasus, EthID, 6, net->dev_addr);
> > + set_registers(pegasus, EthID, 6, net->dev_addr);
>
> yup
>
> > usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
> > usb_rcvbulkpipe(pegasus->usb, 1),
> > @@ -932,7 +932,7 @@ pegasus_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> > pegasus->wolopts = wol->wolopts;
> >
> > ret = set_register(pegasus, WakeupControl, reg78);
> > - if (!ret)
> > + if (ret < 0)
> > ret = device_set_wakeup_enable(&pegasus->usb->dev,
> > wol->wolopts);
>
> now this looks incorrect and unrelated to recent changes (IOW the
> commit under Fixes), please drop this chunk
It may not be related, but the change is:
a) obviously correct; and
b) in sync with the rest of 'ret' evaluations;
To be honest i do not envision usb_control_msg_send() returning positive value
anytime soon, but (ret < 0) looks "more" correct than (!ret).
Petko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation;
2021-08-19 9:05 Petko Manolov
@ 2021-08-19 19:34 ` Jakub Kicinski
2021-08-20 7:08 ` Petko Manolov
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2021-08-19 19:34 UTC (permalink / raw)
To: Petko Manolov; +Cc: davem, netdev, paskripkin, stable
On Thu, 19 Aug 2021 12:05:39 +0300 Petko Manolov wrote:
> - restore the behavior in enable_net_traffic() to avoid regressions - Jakub
> Kicinski;
> - hurried up and removed redundant assignment in pegasus_open() before yet
> another checker complains;
> - explicitly check for negative value in pegasus_set_wol(), even if
> usb_control_msg_send() never return positive number we'd still be in sync
> with the rest of the driver style;
>
> Fixes: 8a160e2e9aeb net: usb: pegasus: Check the return value of get_geristers() and friends;
I guess this is fine but not exactly the preferred format, please see
Submitting patches.
> Reported-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> ---
> drivers/net/usb/pegasus.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 652e9fcf0b77..1ef93082c772 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -446,7 +446,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
> write_mii_word(pegasus, 0, 0x1b, &auxmode);
> }
>
> - return 0;
> + return ret;
yup
> fail:
> netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
> return ret;
> @@ -835,7 +835,7 @@ static int pegasus_open(struct net_device *net)
> if (!pegasus->rx_skb)
> goto exit;
>
> - res = set_registers(pegasus, EthID, 6, net->dev_addr);
> + set_registers(pegasus, EthID, 6, net->dev_addr);
yup
> usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
> usb_rcvbulkpipe(pegasus->usb, 1),
> @@ -932,7 +932,7 @@ pegasus_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> pegasus->wolopts = wol->wolopts;
>
> ret = set_register(pegasus, WakeupControl, reg78);
> - if (!ret)
> + if (ret < 0)
> ret = device_set_wakeup_enable(&pegasus->usb->dev,
> wol->wolopts);
now this looks incorrect and unrelated to recent changes (IOW the
commit under Fixes), please drop this chunk
> return ret;
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation;
@ 2021-08-19 9:05 Petko Manolov
2021-08-19 19:34 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Petko Manolov @ 2021-08-19 9:05 UTC (permalink / raw)
To: davem; +Cc: netdev, kuba, paskripkin, stable, Petko Manolov
- restore the behavior in enable_net_traffic() to avoid regressions - Jakub
Kicinski;
- hurried up and removed redundant assignment in pegasus_open() before yet
another checker complains;
- explicitly check for negative value in pegasus_set_wol(), even if
usb_control_msg_send() never return positive number we'd still be in sync
with the rest of the driver style;
Fixes: 8a160e2e9aeb net: usb: pegasus: Check the return value of get_geristers() and friends;
Reported-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
---
drivers/net/usb/pegasus.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 652e9fcf0b77..1ef93082c772 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -446,7 +446,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
write_mii_word(pegasus, 0, 0x1b, &auxmode);
}
- return 0;
+ return ret;
fail:
netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
return ret;
@@ -835,7 +835,7 @@ static int pegasus_open(struct net_device *net)
if (!pegasus->rx_skb)
goto exit;
- res = set_registers(pegasus, EthID, 6, net->dev_addr);
+ set_registers(pegasus, EthID, 6, net->dev_addr);
usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
usb_rcvbulkpipe(pegasus->usb, 1),
@@ -932,7 +932,7 @@ pegasus_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
pegasus->wolopts = wol->wolopts;
ret = set_register(pegasus, WakeupControl, reg78);
- if (!ret)
+ if (ret < 0)
ret = device_set_wakeup_enable(&pegasus->usb->dev,
wol->wolopts);
return ret;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-20 7:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 14:06 [PATCH] net: usb: pegasus: fixes of set_register(s) return value evaluation; Petko Manolov
2021-08-17 16:06 ` Greg KH
2021-08-18 9:19 ` David Miller
2021-08-19 9:05 Petko Manolov
2021-08-19 19:34 ` Jakub Kicinski
2021-08-20 7:08 ` Petko Manolov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.