Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 Aug 2022 21:11:40 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Arun.Ramadoss@microchip.com
Cc:     olteanv@gmail.com, linux-kernel@vger.kernel.org,
        UNGLinuxDriver@microchip.com, vivien.didelot@gmail.com,
        linux@armlinux.org.uk, Tristram.Ha@microchip.com,
        f.fainelli@gmail.com, kuba@kernel.org, edumazet@google.com,
        pabeni@redhat.com, netdev@vger.kernel.org,
        Woojung.Huh@microchip.com, davem@davemloft.net
Subject: Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable
 interrupt for internal phy link detection
Message-ID: <Ywka7MuutP93wIv2@lunn.ch>
References: <20220822092017.5671-1-arun.ramadoss@microchip.com>
 <YwOAxh7Bc12OornD@lunn.ch>
 <b1e66b49e8006bd7dcb3fd74d5185db807e5a9f6.camel@microchip.com>
 <YwTlKnpgMRp2Nugm@lunn.ch>
 <650478ca37c09c91566784db2b496838a15d567c.camel@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <650478ca37c09c91566784db2b496838a15d567c.camel@microchip.com>
Precedence: bulk

On Fri, Aug 26, 2022 at 04:21:39PM +0000, Arun.Ramadoss@microchip.com wrote:
> Hi Andrew,
> Thanks for the reply.
> 
> On Tue, 2022-08-23 at 16:33 +0200, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > 
> > 
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts#L93
> > 
> > Doing it this way is however very verbose. I later discovered a short
> > cut:
> > 
> > 
> https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1164
> > 
> > by setting mdiobus->irq[] to the interrupt number, phylib will
> > automatically use the correct interrupt without needing an DT.
> > 
> >         Andrew
> 
> In LAN937x, register REG_SW_PORT_INT_MASK__4 BIT7:0 used to
> enable/disable the interrupts for each port. It is the global interrupt
> enable for ports. In turn each port has REG_PORT_INT_MASK register,
> which enable/disable interrupts like PTP, PHY (BIT 1). And in turn for
> each phy it has different interrupts like link up, link down etc.
> 
> As per your suggestion, I enabled the global_irq domain for each port
> and port_irq domain for port 1 alone and the corresponding mdio irq is
> updated. The dts is updated with *only one user port*. And it is
> working as expected.
> 
> How can I extend the above procedure for remaining ports 2 -8. Do I
> need to create the array of port_irq[8] domain. But when I analyzed
> code flow, if the port_irq_thread is triggered it has function
> parameter as irq only. From this irq how can we derive the parent irq
> i.e from which port is triggered. Only if I know the port, then I can
> read the corresponding status register and check if the interrupt is
> from phy or ptp etc.
> 
> Can you suggest how to enable irq_domain for the each port and find out
> the phy interrupt. So that it can be extended further in our ptp
> interrupt based implementation. 

You need an interrupt controller per port.

Your top level interrupt controller should be pretty similar to the
mv88e6xxx global 1.

However, your need to create a second level interrupt controller per
port.

This is where mv88e6xxx creates its one second level interrupt
controller:

https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1135

Rather than finding the mapping for a fixed interrupt number, you need
to get the mapping for the specific port you are creating the
interrupt controller for.

In the end you will have a tree of interrupt controllers, where as
mv88e6xxx just has a chain.

Here is what mv88e6xxx looks like:

 47:          0  gpio-vf610  27 Level     mv88e6xxx-mdio_mux-0.1:00
 51:          0  mv88e6xxx-g1   3 Edge      mv88e6xxx-mdio_mux-0.1:00-g1-atu-prob
 53:          0  mv88e6xxx-g1   5 Edge      mv88e6xxx-mdio_mux-0.1:00-g1-vtu-prob
 55:          0  mv88e6xxx-g1   7 Edge      mv88e6xxx-mdio_mux-0.1:00-g2
 57:          0  mv88e6xxx-g2   0 Edge      !mdio-mux!mdio@1!switch@0!mdio:00
 58:          0  mv88e6xxx-g2   1 Edge      !mdio-mux!mdio@1!switch@0!mdio:01
 59:          0  mv88e6xxx-g2   2 Edge      !mdio-mux!mdio@1!switch@0!mdio:02
 72:          0  mv88e6xxx-g2  15 Edge      mv88e6xxx-mdio_mux-0.1:00-watchdog

Interrupt 47 is the GPIO line the switch it attached to. Interrupts 51
and 53 are handlers registered to the top level interrupt
controller. Interrupt 55 is the chain into the second level interrupt
handler. Interrupts 57-59 are PHY interrupts in the second level
interrupt controller, and interrupt 72 is also part of the second level.

For you, you will have a chain interrupt in the top level for each
port, rather than the single 55 interrupt here.

Build the tree and it should all work.

But make sure you turn on lockdep. I took me a while to get all the
locking correct with mv88e6xxx_reg_lock(). Maybe you wont face this
problem with the ksz driver, its locking could be different.

As to your question about knowing where the interrupt came from, you
can see in:

https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1026

that the interrupt handler gets called with a void * dev_id. For
mv88e6xxx this points to the chip structure. You can make it point to
the port structure. This is the last parameter you pass to
request_threaded_irq().

      Andrew