Re: [PATCH v3 2/2] drivers: net: ethernet: 3com: fix return value
On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote: > In some cases the return value of a failing function is not being used > and the function typhoon_init_one() returns another negative error > code instead. I'm not sure these changes are especially valuable, since we'll need to look at the dmesg log anyways to figure out what went wrong, but again I don't feel strongly. Fix up the subject issues and I'm happy to ack them.
Re: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
On Sun, 2016-12-25 at 01:30 +0100, Thomas Preisner wrote: > Those spaces were actually left out purposely: The file in question > (typhoon.c) > is missing those spaces between the statements (if, for, while) and the > following opening bracket pretty much always (except 2-3 times) and we figured > that it might be better to keep the coding style consistent since this might > aswell have been intended by the original author. I'm not sure if we had the rule back then, or if I just missed it. Either way, we should follow the rules for new code if we can. I'm not sure it's worth fixing all of the instances -- usually formatting-only changes are not worth the churn -- but I don't have a strong opinion on the matter.
Re: [PATCH] drivers: net: ethernet: 3com: fix return value
On Sat, 2016-12-24 at 00:00 +0100, Thomas Preisner wrote:
> diff --git a/drivers/net/ethernet/3com/typhoon.c
> b/drivers/net/ethernet/3com/typhoon.c
> index a0cacbe..9a3ab58 100644
> --- a/drivers/net/ethernet/3com/typhoon.c
> +++ b/drivers/net/ethernet/3com/typhoon.c
> @@ -2404,6 +2404,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>
> if(!is_valid_ether_addr(dev->dev_addr)) {
> err_msg = "Could not obtain valid ethernet address, aborting";
> + err = -EIO;
> goto error_out_reset;
The change above is fine, but the other two should use the return value
from the failing function call.
> @@ -2413,6 +2414,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> INIT_COMMAND_WITH_RESPONSE(_cmd, TYPHOON_CMD_READ_VERSIONS);
> if(typhoon_issue_command(tp, 1, _cmd, 3, xp_resp) < 0) {
> err_msg = "Could not get Sleep Image version";
> + err = -EIO;
> goto error_out_reset;
> }
>
> @@ -2455,6 +2457,7 @@ typhoon_init_one(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>
> if(register_netdev(dev) < 0) {
> err_msg = "unable to register netdev";
> + err = -EIO;
> goto error_out_reset;
> }
>
Re: [PATCH] net: 3com: typhoon: use new api ethtool_{get|set}_link_ksettings
On Wed, 2016-11-02 at 00:11 +0100, Philippe Reynes wrote:
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
>
> Signed-off-by: Philippe Reynes <trem...@gmail.com>
LGTM.
Reviewed-by: David Dillow <d...@thedillows.org>
Re: [patch 1/7] typhoon section fix
On Fri, 2008-02-08 at 03:11 -0800, [EMAIL PROTECTED] wrote: From: Andrew Morton [EMAIL PROTECTED] gcc-3.4.4 on powerpc: drivers/net/typhoon.c:137: error: version causes a section type conflict Cc: Jeff Garzik [EMAIL PROTECTED] Cc: Sam Ravnborg [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] -static const char version[] __devinitdata = +static char version[] __devinitdata = typhoon.c: version DRV_MODULE_VERSION ( DRV_MODULE_RELDATE )\n; Wouldn't going to __devinitconst be better? I'll try to whip up a patch and test-compile it. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] typhoon section fix
On Fri, 2008-02-08 at 09:52 -0800, Andrew Morton wrote: On Fri, 08 Feb 2008 08:59:10 -0500 David Dillow [EMAIL PROTECTED] wrote: On Fri, 2008-02-08 at 03:11 -0800, [EMAIL PROTECTED] wrote: From: Andrew Morton [EMAIL PROTECTED] gcc-3.4.4 on powerpc: drivers/net/typhoon.c:137: error: version causes a section type conflict Cc: Jeff Garzik [EMAIL PROTECTED] Cc: Sam Ravnborg [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] -static const char version[] __devinitdata = +static char version[] __devinitdata = typhoon.c: version DRV_MODULE_VERSION ( DRV_MODULE_RELDATE )\n; Wouldn't going to __devinitconst be better? I'll try to whip up a patch and test-compile it. Sam told me that doesn't work right, that this approach is the one to use and, iirc, that __devinitcont and friends will be removed. I'm not sure why, actually. I think I missed that dicussion. Thanks for the searchable terms -- this is the thread, I think: http://lkml.org/lkml/2008/2/3/99 It looks like Jan had an idea to fold the const into the __devinitconst marker, and if that seems to be viable, then I'd prefer that route to keep the const'ness where it is possible. Otherwise, your patch is fine as-is. Acked-by: David Dillow [EMAIL PROTECTED] -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
