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

[Jeff Garzik <jeff@garzik.org>]

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@neterion.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