linux-kernel - 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:51:01 +0200
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Andrew Lunn <andrew@...n.ch>
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 8, 2023 at 8:46 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> 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.

Yes, I sent it before we established that thanks to Andrew's input.

>
> >  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

Makes sense, we can drop all my stmmac patches from today, I need to
rethink it in detail.

Bart