netdev - Re: [PATCH] forcedeth: Addition of new device id (resend without pci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20090603.145902.185480919.davem@davemloft.net>
Date:	Wed, 03 Jun 2009 14:59:02 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	aabdulla@...dia.com
Cc:	manfred@...orfullife.com, akpm@...l.org, netdev@...r.kernel.org
Subject: Re: [PATCH] forcedeth: Addition of new device id (resend without
 pci_ids.h)

From: Ayaz Abdulla <aabdulla@...dia.com>
Date: Wed, 03 Jun 2009 12:24:37 -0400

> 
> 
> David Miller wrote:
>> From: Ayaz Abdulla <aabdulla@...dia.com>
>> Date: Wed, 03 Jun 2009 12:06:04 -0400
>> 
>>>Add support for new ethernet device in the MCP89 chipset.
>>>
>>>Signed-off-by: Ayaz Abdulla <aabdulla@...dia.com>
>> Just use the constant directly, please :-)
> 
> But there is place in the patch that refers to device id so it makes
> more sense to have a macro. i.e
> 
> +       if (np->device_id != PCI_DEVICE_ID_NVIDIA_NVENET_40)
> 
> other places in the code also refer to device ids for certain
> workarounds. i.e.
> 
> 		if ((id->device == PCI_DEVICE_ID_NVIDIA_NVENET_12 ||
> 		     id->device == PCI_DEVICE_ID_NVIDIA_NVENET_13) &&

And you can add the define when that is necessary.

Otherwise, it's being used in exactly one location, the ID table.
And for that purpose it's superfluous.

You could also change the driver to use a bitmask of HW workaround
cases, that gets set very early in the probe.  And the HW
workarounds are keyed on those bit masks (one test) compared
to the direct ID checks (potentially multiple compares).

It would be both cleaner and more efficient.  And you could still
use the direct constants without any real loss of clarity.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html