linux-kernel - Re: [PATCH net-next v3 3/3] net: stmmac: Add PCI driver support for BCM8958x

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: <c2e2f11a-89d8-42fa-a655-972a4ab372da@lunn.ch>
Date: Sat, 3 Aug 2024 01:08:00 +0200
From: Andrew Lunn <andrew@...n.ch>
To: jitendra.vegiraju@...adcom.com
Cc: netdev@...r.kernel.org, alexandre.torgue@...s.st.com,
	joabreu@...opsys.com, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, mcoquelin.stm32@...il.com,
	bcm-kernel-feedback-list@...adcom.com, richardcochran@...il.com,
	ast@...nel.org, daniel@...earbox.net, hawk@...nel.org,
	john.fastabend@...il.com, linux-kernel@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, bpf@...r.kernel.org,
	linux@...linux.org.uk, horms@...nel.org,
	florian.fainelli@...adcom.com
Subject: Re: [PATCH net-next v3 3/3] net: stmmac: Add PCI driver support for
 BCM8958x

> Management of integrated ethernet switch on this SoC is not handled by
> the PCIe interface.

MDIO? SPI? I2C?

> +#define XGMAC_PCIE_MISC_MII_CTRL			0x4
> +#define XGMAC_PCIE_MISC_MII_CTRL_VALUE			0x7

Could you replace these magic values with actual definitions. What
does 7 mean?

> +#define XGMAC_PCIE_MISC_PCIESS_CTRL			0x8
> +#define XGMAC_PCIE_MISC_PCIESS_CTRL_VALUE		0x200

> +static int num_instances;

> +	/* This device is directly attached to the switch chip internal to the
> +	 * SoC using XGMII interface. Since no MDIO is present, register
> +	 * fixed-link software_node to create phylink.
> +	 */
> +	if (num_instances == 0) {
> +		ret = software_node_register_node_group(fixed_link_node_group);
> +		if (ret) {
> +			dev_err(&pdev->dev,
> +				"%s: failed to register software node\n",
> +				__func__);
> +			return ret;
> +		}
> +	}
> +	num_instances++;

So all the instances of the MAC share one fixed link? That is pretty
unusual. In DT, each would have its own. Have you reviewed the
implications of this?

	Andrew