Re: [RFC PATCH 08/12] e1000e: allow bad checksum

[Jiri Kosina]

On Mon, 29 Sep 2008, Jesse Brandeburg wrote:

>  	for (i = 0;; i++) {
> -		if (e1000_validate_nvm_checksum(&adapter->hw) >= 0)
> +		if (e1000_validate_nvm_checksum(hw) >= 0) {
> +			/* copy the MAC address out of the NVM */
> +			if (e1000e_read_mac_addr(&adapter->hw))
> +				e_err("NVM Read Error reading MAC address\n");
>  			break;
> +		}
>  		if (i == 2) {
>  			e_err("The NVM Checksum Is Not Valid\n");
> -			err = -EIO;
> -			goto err_eeprom;
> +			e1000e_dump_eeprom(adapter);
> +			/*
> +			 * set MAC address to all zeroes to invalidate and
> +			 * temporary disable this device for the user. This
> +			 * blocks regular traffic while still permitting
> +			 * ethtool ioctls from reaching the hardware as well as
> +			 * allowing the user to run the interface after
> +			 * manually setting a hw addr using
> +			 * `ip link set address`
> +			 */
> +			memset(hw->mac.addr, 0, netdev->addr_len);
> +			break;
>  		}
>  	}

BTW wouldn't something like

	if (e1000_validate_nvm_checksum(hw) >= 0 ||
	    e1000_validate_nvm_checksum(hw) >= 0) {
		/* copy the MAC address out of the NVM */
		if (e1000e_read_mac_addr(&adapter->hw))
		e_err("NVM Read Error reading MAC address\n");
	} else {
		e_err("The NVM Checksum Is Not Valid\n");
		e1000e_dump_eeprom(adapter);
		/*
		 * set MAC address to all zeroes to invalidate and
		 * temporary disable this device for the user. This
		 * blocks regular traffic while still permitting
		 * ethtool ioctls from reaching the hardware as well as
		 * allowing the user to run the interface after
		 * manually setting a hw addr using
		 * `ip link set address`
		 */
		memset(hw->mac.addr, 0, netdev->addr_len);
	}

just be much more readable? Having for(;;) loop which always performs 
three iterations, and having "if" inside that distinguishes two iterations 
from each other just looks peculiar to my eyes :)

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/