netdev - Re: [PATCH 2.6.20 2/5] S2IO: Fixes for reset and link handling.

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] [day] [month] [year] [list]

Date:	Tue, 23 Jan 2007 00:45:22 -0500
From:	Jeff Garzik <jeff@...zik.org>
To:	Ananda Raju <Ananda.Raju@...erion.com>
CC:	netdev@...r.kernel.org, leonid.grossman@...erion.com,
	alicia.pena@...erion.com, ramkrishna.vepa@...erion.com,
	sivakumar.subramani@...erion.com, sreenivasa.honnur@...erion.com,
	sriram.rapuru@...erion.com
Subject: Re: [PATCH 2.6.20 2/5] S2IO: Fixes for reset and link handling.

Ananda Raju wrote:
> 1. Fix for reset and link handling.
> 2. Allow for promiscuos mode and multicast state be maintained through
>    ifconfig up and down.
> 3. Support to print adapter serial number.
> 
> Signed-off-by: Sivakumar Subramani <sivakumar.subramani@...erion.com>

overall, looks OK, but have small nits noted inline:


> +	/*
> +	 * Andrew: in PCI 33 mode, the P_PLL is not used, and therefore,
> +	 * the the P_PLL_LOCK bit in the adapter_status register will
> +	 * not be asserted.
> +	 */

don't address somebody in a code comment ("Andrew:")



> @@ -537,9 +539,9 @@ typedef struct _RxD_block {
>  
>  #define SIZE_OF_BLOCK	4096
>  
> -#define RXD_MODE_1	0
> -#define RXD_MODE_3A	1
> -#define RXD_MODE_3B	2
> +#define RXD_MODE_1	0 //One Buffer mode
> +#define RXD_MODE_3A	1 //Three Buffer mode
> +#define RXD_MODE_3B	2 //Two Buffer mode

C++ comments discouraged


> @@ -849,8 +851,9 @@ struct s2io_nic {
>  	spinlock_t	rx_lock;
>  	atomic_t	isr_cnt;
>  	u64 *ufo_in_band_v;
> -#define VPD_PRODUCT_NAME_LEN 50
> -	u8  product_name[VPD_PRODUCT_NAME_LEN];
> +#define VPD_STRING_LEN 80
> +	u8  product_name[VPD_STRING_LEN];
> +	u8  serial_num[VPD_STRING_LEN];
>  };
>  
>  #define RESET_ERROR 1;
> @@ -893,10 +896,10 @@ static inline void SPECIAL_REG_WRITE(u64
>  		writel((u32) (val), addr);
>  		ret = readl(addr);
>  		writel((u32) (val >> 32), (addr + 4));
> -		ret = readl(addr + 4);
> +		ret = readl((addr + 4));
>  	} else {
>  		writel((u32) (val >> 32), (addr + 4));
> -		ret = readl(addr + 4);
> +		ret = readl((addr + 4));
>  		writel((u32) (val), addr);
>  		ret = readl(addr);
>  	}

adds pointless parens


-
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