Re: [PATCH] atl1c: Add support for Atheros AR8152 and AR8152

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 1669 bytes --]

Your commit 496c185c9495629ef1c65387cb2594578393cfe0 "atl1c: Add support
for Atheros AR8152 and AR8152" included the following changes:

> --- a/drivers/net/atl1c/atl1c_hw.c
> +++ b/drivers/net/atl1c/atl1c_hw.c
> @@ -70,17 +70,39 @@ static int atl1c_get_permanent_address(struct atl1c_hw *hw)
[...]
> -		/* Enable OTP CLK */
> -		if (!(otp_ctrl_data & OTP_CTRL_CLK_EN)) {
> -			otp_ctrl_data |= OTP_CTRL_CLK_EN;
> -			AT_WRITE_REG(hw, REG_OTP_CTRL, otp_ctrl_data);
> -			AT_WRITE_FLUSH(hw);
> -			msleep(1);
> +		if (hw->nic_type == athr_l1c || hw->nic_type == athr_l2c_b) {
> +			/* Enable OTP CLK */
> +			if (!(otp_ctrl_data & OTP_CTRL_CLK_EN)) {
> +				otp_ctrl_data |= OTP_CTRL_CLK_EN;
> +				AT_WRITE_REG(hw, REG_OTP_CTRL, otp_ctrl_data);
> +				AT_WRITE_FLUSH(hw);
> +				msleep(1);
> +			}
> +		}
[...]
> @@ -96,11 +118,31 @@ static int atl1c_get_permanent_address(struct atl1c_hw *hw)
>  			return -1;
>  	}
>  	/* Disable OTP_CLK */
> -	if (otp_ctrl_data & OTP_CTRL_CLK_EN) {
> -		otp_ctrl_data &= ~OTP_CTRL_CLK_EN;
> -		AT_WRITE_REG(hw, REG_OTP_CTRL, otp_ctrl_data);
> -		AT_WRITE_FLUSH(hw);
> -		msleep(1);
> +	if ((hw->nic_type == athr_l1c || hw->nic_type == athr_l2c)) {
> +		if (otp_ctrl_data & OTP_CTRL_CLK_EN) {
> +			otp_ctrl_data &= ~OTP_CTRL_CLK_EN;
> +			AT_WRITE_REG(hw, REG_OTP_CTRL, otp_ctrl_data);
> +			AT_WRITE_FLUSH(hw);
> +			msleep(1);
> +		}
> +	}

Shouldn't the first if-statement use the same condition as the second
i.e. matching the previously-defined hardware types athr_l1c and
athr_l2c?

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]