Re: [RFC v4 net-next 2/4] net: dsa: mt7530: add interrupt support

From: Marc Zyngier <maz@kernel.org>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "DENG Qingfang" <dqfext@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Russell King" <linux@armlinux.org.uk>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Vivien Didelot" <vivien.didelot@gmail.com>,
	"Vladimir Oltean" <olteanv@gmail.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Sergio Paracuellos" <sergio.paracuellos@gmail.com>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-staging@lists.linux.dev, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, "Weijie Gao" <weijie.gao@mediatek.com>,
	"Chuanhong Guo" <gch981213@gmail.com>,
	"René van Dorst" <opensource@vdorst.com>,
	"Frank Wunderlich" <frank-w@public-files.de>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Greg Ungerer" <gerg@kernel.org>
Subject: Re: [RFC v4 net-next 2/4] net: dsa: mt7530: add interrupt support
Date: Tue, 13 Apr 2021 09:06:51 +0100	[thread overview]
Message-ID: <8735vuobfo.wl-maz@kernel.org> (raw)
In-Reply-To: <YHTgu1+6GZFdFgWJ@lunn.ch>

On Tue, 13 Apr 2021 01:07:23 +0100,
Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > > > +static void
> > > > +mt7530_setup_mdio_irq(struct mt7530_priv *priv)
> > > > +{
> > > > +	struct dsa_switch *ds = priv->ds;
> > > > +	int p;
> > > > +
> > > > +	for (p = 0; p < MT7530_NUM_PHYS; p++) {
> > > > +		if (BIT(p) & ds->phys_mii_mask) {
> > > > +			unsigned int irq;
> > > > +
> > > > +			irq = irq_create_mapping(priv->irq_domain, p);
> > > 
> > > This seems odd. Why aren't the MDIO IRQs allocated on demand as
> > > endpoint attached to this interrupt controller are being probed
> > > individually? In general, doing this allocation upfront is an
> > > indication that there is some missing information in the DT to perform
> > > the discovery.
> > 
> > This is what Andrew's mv88e6xxx does, actually. In addition, I also check
> > the phys_mii_mask to avoid creating mappings for unused ports.
> 
> It can be done via DT, using the standard interrupt property, so long
> as you use of_mdiobus_register(np).
> 
> But when you have an 7 port switch, and a nice simple mapping, port 0
> PHY using interrupt 0, you can save a lot of device tree boilerplate
> by doing it in code. And when you have 4 of these switches, it gets
> very boring adding all the DT to just wire up the interrupts 28
> interrupts.

I guess this is depends whether the most usual case is to have all
these interrupts being actively in use or not. Most interrupts only
use a limited portion of their interrupt space at any given time.
Allocating all interrupts and creating mappings upfront is a waste of
memory.

If the use case here is that all these interrupts will be wired and
used in most cases, then upfront allocation is probably not a problem.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek