netdev - 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: <CAMdnO-JBznFpExduwCAm929N73Z_p4S4_nzRaowL9SzseqC6LA@mail.gmail.com>
Date: Mon, 5 Aug 2024 17:56:43 -0700
From: Jitendra Vegiraju <jitendra.vegiraju@...adcom.com>
To: Andrew Lunn <andrew@...n.ch>
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

On Fri, Aug 2, 2024 at 4:08 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > Management of integrated ethernet switch on this SoC is not handled by
> > the PCIe interface.
>
> MDIO? SPI? I2C?
>
The device uses SPI interface. The switch has internal ARM M7 for
controller firmware.

> > +#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?
>
Thanks, I will fix the macros.

> > +#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?
>
Our thinking was that since the software node is only used for static
node data to populate phylink config, a per device node is not
required.
We tested with multiple devices and repeated PCI remove/rescan operations.
However, It does make sense to be consistent with the DT usage model.
We will add the per device node entry in the next patch update.
>         Andrew