netdev - Re: [PATCH]: Third (final?) 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]

Date:	Fri, 05 Oct 2007 15:18:37 -0700
From:	Matheos Worku <Matheos.Worku@....COM>
To:	Ingo Oeser <netdev@...eo.de>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Ariel.Hendel@....COM, Greg.Onufer@....COM, jeff@...zik.org,
	Ashley.Saulsbury@....COM
Subject: Re: [PATCH]: Third (final?) release of Sun Neptune driver

Ingo Oeser wrote:
> Hi David,

Repost since my last post ended up being html
Regards,

matheos

> 
> some minor nits.

Some  Neptune HW  info to explain some of the constants ...

> 
> Could this driver be split into more files. 8k lines per file

>> +	case 0:
>> +		ctrl_reg = ENET_SERDES_0_CTRL_CFG;
>> +		test_cfg_reg = ENET_SERDES_0_TEST_CFG;
>> +		break;
>> +	case 1:
>> +		ctrl_reg = ENET_SERDES_1_CTRL_CFG;
>> +		test_cfg_reg = ENET_SERDES_1_TEST_CFG;
>> +		break;
>> +
>> +	default:
>> +		return -EINVAL;
>> +	}
> 
> 
> Maybe small table?
> 
>> +	ctrl_val = (ENET_SERDES_CTRL_SDET_0 |
>> +		    ENET_SERDES_CTRL_SDET_1 |
>> +		    ENET_SERDES_CTRL_SDET_2 |
>> +		    ENET_SERDES_CTRL_SDET_3 |
>> +		    (0x5 << ENET_SERDES_CTRL_EMPH_0_SHIFT) |
>> +		    (0x5 << ENET_SERDES_CTRL_EMPH_1_SHIFT) |
>> +		    (0x5 << ENET_SERDES_CTRL_EMPH_2_SHIFT) |
>> +		    (0x5 << ENET_SERDES_CTRL_EMPH_3_SHIFT) |
>> +		    (0x1 << ENET_SERDES_CTRL_LADJ_0_SHIFT) |
>> +		    (0x1 << ENET_SERDES_CTRL_LADJ_1_SHIFT) |
>> +		    (0x1 << ENET_SERDES_CTRL_LADJ_2_SHIFT) |
>> +		    (0x1 << ENET_SERDES_CTRL_LADJ_3_SHIFT));
>> +	test_cfg_val = 0;
>> +
>> +	if (lp->loopback_mode == LOOPBACK_PHY) {
>> +		test_cfg_val |= ((ENET_TEST_MD_PAD_LOOPBACK <<
>> +				  ENET_SERDES_TEST_MD_0_SHIFT) |
>> +				 (ENET_TEST_MD_PAD_LOOPBACK <<
>> +				  ENET_SERDES_TEST_MD_1_SHIFT) |
>> +				 (ENET_TEST_MD_PAD_LOOPBACK <<
>> +				  ENET_SERDES_TEST_MD_2_SHIFT) |
>> +				 (ENET_TEST_MD_PAD_LOOPBACK <<
>> +				  ENET_SERDES_TEST_MD_3_SHIFT));
>> +	}
>> +
>> +	nw64(ctrl_reg, ctrl_val);
>> +	nw64(test_cfg_reg, test_cfg_val);
>> +
>> +	for (i = 0; i < 4; i++) {
> 
> why 4? (magic number)

The MAC HW has 4 SERDES lanes. Need to configure each lane.

> 
>> +		u32 rxtx_ctrl, glue0;
>> +		int err;
>> +
>> +		err = esr_read_rxtx_ctrl(np, i, &rxtx_ctrl);
>> +		if (err)
>> +			return err;
>> +		err = esr_read_glue0(np, i, &glue0);

>> +
> 

> 
> [...]
> 
>> +static void niu_set_max_burst(struct niu *np, struct tx_ring_info *rp)
>> +{
>> +	int mtu = np->dev->mtu;
>> +
>> +	rp->max_burst = mtu + 32;
>> +	if (rp->max_burst > 4096)
>> +		rp->max_burst = 4096;
> 
> Why 32 and 4096? (Magic values)

These values were recommended by the HW designers for fair utilization 
of DRR feature among TX/RX rings.

> 
>> +}
>> +
> 
> ... ok, I stop here, since this drivers is damn big :-)
> 
> Best Regards
> 
> Ingo Oeser

-
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