netdev - RE: [PATCH net-next 10/18] liquidio support for new device cn23xx

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]

Message-ID: <CO2PR11MB0088F3E8187DAC13B3FED31E97070@CO2PR11MB0088.namprd11.prod.outlook.com>
Date:	Thu, 4 Aug 2016 17:07:34 +0000
From:	Yuval Mintz <Yuval.Mintz@...gic.com>
To:	Raghu Vatsavayi <rvatsavayi@...iumnetworks.com>,
	David Miller <davem@...emloft.net>
CC:	netdev <netdev@...r.kernel.org>,
	Derek Chickles <derek.chickles@...iumnetworks.com>,
	Satanand Burla <satananda.burla@...iumnetworks.com>,
	Felix Manlunas <felix.manlunas@...iumnetworks.com>,
	Raghu Vatsavayi <raghu.vatsavayi@...iumnetworks.com>
Subject: RE: [PATCH net-next  10/18] liquidio support for new device cn23xx

> +	/** Complete DPI init is done at firmware level. */
> +	/*cn23xx_set_dpi_regs(oct);*/
Should be removed?

> +int cn23xx_fw_lock(struct octeon_device *oct)
> +{
> +	dev_dbg(&oct->pci_dev->dev, "%s : PF = %d\n", __func__, oct-
> >pf_num);
> +	return 0;
> +}
> +
> +int cn23xx_fw_unlock(struct octeon_device *oct)
> +{
> +	dev_dbg(&oct->pci_dev->dev, "%s : PF = %d\n", __func__, oct-
> >pf_num);
> +	return 0;
> +}
A bit odd that you have lock/unlock functions that only print.
Are you later extending them?

> +		if (!(OCTEON_CN23XX_PF(oct)) ||
> +		    (OCTEON_CN23XX_PF(oct) && (oct->octeon_id == 0)))
Can simply be
		If (!(OCTEON_CN23XX_PF(oct) || !oct->octeon_id)

> +		if (!cn23xx_fw_loaded(octeon_dev)) {
> +			fw_loaded = 0;
> +			/* Do a soft reset of the Octeon device. */
I can never remember - are empty lines prior to comments
nice-to-have or a style requirement?

> +			}
> +			/*Lock again soft reset equivalent to unlock */
Likewise

> +	if ((!OCTEON_CN23XX_PF(octeon_dev)) ||
> +	    (OCTEON_CN23XX_PF(octeon_dev) && !fw_loaded)) {
	If (!A || (A && !B)) == (!A || !B)

> +		if (ddr_timeout == 0) {
	If (!ddr_timeout)

> +		while (ddr_timeout == 0) {
Likewise

> +		if (octeon_wait_for_bootloader(octeon_dev, 1000) != 0) {
No need for `!= 0`