netdev - Re: [PATCH net-next] net: stmmac: don't create the MDIO bus if there's no mdio node on DT

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: Tue, 8 Aug 2023 20:45:59 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Alex Elder <elder@...aro.org>,
	Srini Kandagatla <srinivas.kandagatla@...aro.org>,
	Andrew Halaney <ahalaney@...hat.com>, netdev@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH net-next] net: stmmac: don't create the MDIO bus if
 there's no mdio node on DT

On Tue, Aug 08, 2023 at 02:02:54PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> 
> The stmmac_dt_phy() function that parses the device-tree node of the MAC
> and allocates the MDIO and PHY resources misses one use-case: when the
> MAC doesn't have a fixed link but also doesn't define its own mdio bus
> on the device tree and instead shares the MDIO lines with a different
> MAC with its PHY phandle reaching over into a different node.

It does not share the MDIO lines. The other MDIO bus master happens to
have two PHYs and there are no PHYs on this MDIO bus, so no point
instantiating it.

>  static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
>  			 struct device_node *np, struct device *dev)
>  {
> -	bool mdio = !of_phy_is_fixed_link(np);
>  	static const struct of_device_id need_mdio_ids[] = {
>  		{ .compatible = "snps,dwc-qos-ethernet-4.10" },
>  		{},
>  	};
>  
> +	if (of_phy_is_fixed_link(np))
> +		return 0;
> +

		/**
		 * If snps,dwmac-mdio is passed from DT, always register
		 * the MDIO
		 */
		for_each_child_of_node(np, plat->mdio_node) {
			if (of_device_is_compatible(plat->mdio_node,
						    "snps,dwmac-mdio"))
				break;
		}

The comment suggests it should always be registered. This MAC might
have a fixed-phy, but that does not mean there is not an Ethernet
switch on the bus, or a PHY for some other MAC etc. MDIO busses
masters should be considered fully independent devices.

https://elixir.bootlin.com/linux/v6.5-rc5/source/arch/arm/boot/dts/nxp/vf/vf610-zii-ssmb-dtu.dts

&fec1 {
	phy-mode = "rmii";
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_fec1>;
	status = "okay";

	fixed-link {
		speed = <100>;
		full-duplex;
	};

	mdio1: mdio {
		#address-cells = <1>;
		#size-cells = <0>;
		clock-frequency = <12500000>;
		suppress-preamble;
		status = "okay";

		switch0: switch0@0 {
			compatible = "marvell,mv88e6190";
			pinctrl-0 = <&pinctrl_gpio_switch0>;
			pinctrl-names = "default";
			reg = <0>;
			eeprom-length = <65536>;
			interrupt-parent = <&gpio3>;
			interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
			interrupt-controller;
			#interrupt-cells = <2>;

Both a fixed link, and something on the MDIO bus....

     Andrew