netdev - Re: [PATCH]: Preliminary release of Sun Neptune driver

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: <46F19D25.2010403@garzik.org>
Date:	Wed, 19 Sep 2007 18:05:25 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	Stephen Hemminger <shemminger@...ux-foundation.org>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Ariel.Hendel@....com, greg.onufer@....com
Subject: Re: [PATCH]: Preliminary release of Sun Neptune driver

Stephen Hemminger wrote:
> O
>> +#define DRV_MODULE_NAME		"niu"
>> +#define PFX DRV_MODULE_NAME	": "
>> +#define DRV_MODULE_VERSION	"0.06"
>> +#define DRV_MODULE_RELDATE	"September 18, 2007"
>> +
>> +static char version[] __devinitdata =
>> +	DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
>> +
>> +MODULE_AUTHOR("David S. Miller (davem@...emloft.net)");
>> +MODULE_DESCRIPTION("NIU ethernet driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_VERSION(DRV_MODULE_VERSION);
>> +
>> +#ifndef DMA_44BIT_MASK
>> +#define DMA_44BIT_MASK	0x00000fffffffffffULL
>> +#endif
>> +
>> +#ifndef PCI_DEVICE_ID_SUN_NEPTUNE
>> +#define PCI_DEVICE_ID_SUN_NEPTUNE	0xabcd
>> +#endif
> 
> Why bother defining the ID

Yes, its pointless to use anything but the hex number for PCI IDs that 
are only used in a single place.


>> +static struct pci_device_id niu_pci_tbl[] = {
>> +	{PCI_DEVICE(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_NEPTUNE),
>> +	 .driver_data = 0xff},
>> +	{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(pci, niu_pci_tbl);
>> +
>> +#define NIU_TX_TIMEOUT			(5 * HZ)
>> +
>> +#define nr64(reg)		readq(np->regs + (reg))
>> +#define nw64(val, reg)		writeq((val), np->regs + (reg))
> 
> Macro's that make assumptions about context (ie variable name np)
> are evil and bad style.

No, that's a common and encouraged convenience that makes the code a lot 
easier to read.

That said, the 2-arg macro arg order is wrong.  It should be (reg,val) 
like tg3 and other drivers.

	Jeff



-
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