Date:   Fri, 3 Sep 2021 21:53:51 +0200
From:   Gerhard Engleder <gerhard@...leder-embedded.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v2 3/3] tsnep: Add TSN endpoint Ethernet MAC driver

> > > > > > +static irqreturn_t tsnep_irq(int irq, void *arg)
> > > > > > +{
> > > > > > +     struct tsnep_adapter *adapter = arg;
> > > > > > +     u32 active = ioread32(adapter->addr + ECM_INT_ACTIVE);
> > > > > > +
> > > > > > +     /* acknowledge interrupt */
> > > > > > +     if (active != 0)
> > > > > > +             iowrite32(active, adapter->addr + ECM_INT_ACKNOWLEDGE);
> > > > > > +
> > > > > > +     /* handle management data interrupt */
> > > > > > +     if ((active & ECM_INT_MD) != 0) {
> > > > > > +             adapter->md_active = false;
> > > > > > +             wake_up_interruptible(&adapter->md_wait);
> > > > > > +     }
> > > > > > +
> > > > > > +     /* handle link interrupt */
> > > > > > +     if ((active & ECM_INT_LINK) != 0) {
> > > > > > +             if (adapter->netdev->phydev) {
> > > > > > +                     struct phy_device *phydev = adapter->netdev->phydev;
> > > > > > +                     u32 status = ioread32(adapter->addr + ECM_STATUS);
> > > > > > +                     int link = (status & ECM_NO_LINK) ? 0 : 1;
> > > > > > +                     u32 speed = status & ECM_SPEED_MASK;
> > > > >
> > > > > How does PHY link and speed get into this MAC register? Is the MAC
> > > > > polling the PHY over the MDIO bus? Is the PHY internal to the MAC and
> > > > > it has backdoor access to the PHY status?
> > > >
> > > > PHY is external. The MAC expects additional signals for link status. These
> > > > signals can be derived from RGMII in band signaling of the link status or by
> > > > using PHY link and speed LED outputs. The MAC is using the link status for
> > > > a quick no link reaction to minimize the impact to real time applications.
> > > > EtherCAT for example also uses the link LED output for a no link reaction
> > > > within a few microseconds.
> > >
> > > O.K. This is not the normal Linux way. You normally have the PHY
> > > driver tell the PHY core, which then tells the MAC driver. That always
> > > works. RGMII in band signaling is not supported by all PHY devices,
> > > and the board design would require the LED output are correctly
> > > connected, and i guess you need a hacked PHY driver to use the correct
> > > LED meanings? Plus i guess you have additional changes in the PHY
> > > driver to do fast link down detection?
> >
> > Yes, LED outputs must be correctly connected in the board design. LED
> > outputs are usually configured with strapping pins, which again require a
> > correct board design.
>
> Linux sometime, maybe soon, will be able the control the PHY LEDs, and
> probably export them to user space so root can change their meaning.
>
> > Fast link down detection is a hardware property of the selected
> > PHY. So far no PHY driver changes were necessary.
>
> Marvell PHYs for example follow 802.3 C40 and default to waiting 750ms
> before reporting the link down. You can configure them to only wait
> 10ms, 20ms or 40ms. So it sounds like you are using a PHY which does
> not conform to C40? In general, we probably need to be able to
> configure this, for those that do follow C40.
>
> > > I think this needs another DT property to enable using such short
> > > cuts, and you should use the Linux way by default.
> >
> > Isn't choosing PHY_MAC_INTERRUPT also the Linux way? I preferred it
> > over PHY_POLL, because I need the link information directly in the MAC
> > anyway. But maybe the speed information is too much and should be provided
> > to the MAC.
>
> PHY_MAC_INTERRUPT is just the first step. It means something happened
> in the PHY. You need to ask the PHY what? It could be link up or down,
> it could be cable diagnostics have finished, the temperature is
> getting too hot, whatever can cause the PHY to change state. The PHY
> driver will then determine what has actually happened. Some cases, the
> MAC does not needed to know. Others the MAC will be told, via the
> callback it registered. It gets to know the link speed, up down etc.
> That is the Linux way, the complete chain.

I will reduce the MAC knowledge to "something happened" and let the driver
tell it what. Driver and VHDL will be changed.

> > > Also, don't you need a property which tells you to either use RGMII
> > > inband, or LED signals?
> >
> > No, this decision is done in VHDL/FPGA. No need to consume precious FPGA
> > resources for runtime configuration.
>
> You mean you have two ways to synthesis the MAC. You have two
> bitstreams. One for LEDs and one of inband RGMII?

No, not two bitstreams. Just board specific glue logic around the IP core.
As mentioned above, it will be changed.

> > I'm afraid that relying on ACPI is not always an option. x86 CPU modules are
> > very often used in industrial automation and the BIOS of the CPU module is
> > usually not adapted to the carrier board.
>
> Yes, i've been there. I have managed to get the BIOS customised, but
> it is not easy. DT is much easier to use.

Yes, it is not easy and it makes you cry when you have to switch to a different
CPU module.

Gerhard

Powered by blists - more mailing lists