Re: [RFC] net: stmmac: Problem with adding the native GPIOs support

[Serge Semin]

Hello Andrew,

On Mon, Dec 14, 2020 at 04:31:43PM +0100, Andrew Lunn wrote:
> On Mon, Dec 14, 2020 at 12:25:16PM +0300, Serge Semin wrote:
> > Hello folks,
> > 
> > I've got a problem, which has been blowing by head up for more than three
> > weeks now, and I'm desperately need your help in that matter. See our
> > Baikal-T1 SoC is created with two DW GMAC v3.73a IP-cores. Each core
> > has been synthesized with two GPIOs: one as GPI and another as GPO. There
> > are multiple Baikal-T1-based devices have been created so far with active
> > GMAC interface usage and each of them has been designed like this:
> > 
> >  +------------------------+
> >  | Baikal-T1 +------------+       +------------+
> >  |   SoC     | DW GMAC    |       |   Some PHY |
> >  |           |      Rx-clk+<------+Rx-clk      |
> >  |           |            |       |            |
> >  |           |         GPI+<------+#IRQ        |
> >  |           |            |       |            |
> >  |           |       RGMII+<----->+RGMII       |
> >  |           |        MDIO+<----->+MDIO        |
> >  |           |            |       |            |
> >  |           |         GPO+------>+#RST        |
> >  |           |            |       |            |
> >  |           |      Tx-clk+------>+Tx-clk      |
> >  |           |            |       |            |
> >  |           +------------+       +------------+
> >  +------------------------+
> > 
> > Each of such devices has got en external RGMII-PHY attached configured via the
> > MDIO bus with Rx-clock supplied by the PHY and Tx-clock consumed by it. The
> > main peculiarity of such configuration is that the DW GMAC GPIOs have been used
> > to catch the PHY IRQs and to reset the PHY. Seeing the GPIOs support hasn't
> > been added to the STMMAC driver it's the very first setup for now, which has
> > been using them.
> 

> It sounds like you need to cleanly implement a GPIO controller within
> the stmmac driver. But you probably want to make it conditional on a
> DT property. For example, look to see if there is the
> 'gpio-controller;'

Yeap, that's what I have already done. The problem is that the
GPOs state is getting reset together with the MAC reset. So we don't
have a full control over the GPOs state when the MAC gets reset.

> 
> > Anyway the hardware setup depicted above doesn't seem
> > problematic at the first glance, but in fact it is. See, the DW *MAC driver
> > (STMMAC ethernet driver) is doing the MAC reset each time it performs the
> > device open or resume by means of the call-chain:
> > 
> >   stmmac_open()---+
> >                   +->stmmac_hw_setup()->stmmac_init_dma_engine()->stmmac_reset().
> >   stmmac_resume()-+
> > 
> > Such reset causes the whole interface reset: MAC, DMA and, what is more
> > important, GPIOs as being exposed as part of the MAC registers. That
> > in our case automatically causes the external PHY reset, what neither
> > the STTMAC driver nor the PHY subsystem expect at all.
> 

> Is the reset of the GPIO sub block under software control? When you
> have a GPIO controller implemented, you would want to disable this.

Not sure I've fully understood your question. The GPIO sub-block of
the MAC is getting reset together with the MAC. So when we reset the
MAC, the GPOs state will get reset too. Seeing the STMMAC driver
performs the reset on open() and resume() callbacks the GPIOs gets to
reset synchronously there too. That's the main problem. We can't
somehow change the MAC reset behavior. So it's either to get rid of
the reset or somehow take the results of the reset into account in
software (like reinitialize the PHY too after it).

> 
> Once you have a GPIO controller, you can make use of the standard PHY
> DT properties to allow the PHY driver to make use of the interrupt,
> and to control the reset of the PHY.

Yeah, that's what I initially intended to implement. If only the
GPIO-control register wasn't reset on the MAC reset, I wouldn't even
asked the question.

-Sergey

> 
>      Andrew