Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Ioana Ciornei <ioana.ciornei@nxp.com>
To:     Andrew Lunn <andrew@lunn.ch>
CC:     Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Jakub Kicinski <kuba@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Alexandru Ardelean <alexandru.ardelean@analog.com>,
        Andre Edich <andre.edich@microchip.com>,
        Antoine Tenart <atenart@kernel.org>,
        Baruch Siach <baruch@tkos.co.il>,
        Christophe Leroy <christophe.leroy@c-s.fr>,
        Dan Murphy <dmurphy@ti.com>,
        Divya Koppera <Divya.Koppera@microchip.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Hauke Mehrtens <hauke@hauke-m.de>,
        Jerome Brunet <jbrunet@baylibre.com>,
        Kavya Sree Kotagiri <kavyasree.kotagiri@microchip.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Marco Felsch <m.felsch@pengutronix.de>,
        Marek Vasut <marex@denx.de>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Mathias Kresin <dev@kresin.me>,
        Maxim Kochetkov <fido_max@inbox.ru>,
        Michael Walle <michael@walle.cc>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Nisar Sayed <Nisar.Sayed@microchip.com>,
        Oleksij Rempel <o.rempel@pengutronix.de>,
        Philippe Schenker <philippe.schenker@toradex.com>,
        Willy Liu <willy.liu@realtek.com>,
        Yuiko Oshino <yuiko.oshino@microchip.com>
Subject: Re: [RFC net-next 0/5] net: phy: add support for shared interrupts
Thread-Topic: [RFC net-next 0/5] net: phy: add support for shared interrupts
Thread-Index: AQHWqf897amgBiGDU0S3g6psdVpuFqmm/v6AgAARg4A=
Date:   Sat, 24 Oct 2020 18:19:46 +0000
Message-ID: <20201024181945.pip6sdal6hpa6fns@skbuf>
References: <20201024121412.10070-1-ioana.ciornei@nxp.com>
 <20201024171705.GK745568@lunn.ch>
In-Reply-To: <20201024171705.GK745568@lunn.ch>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <34C67A79BAD7CF4E9A26C0FED74E9995@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 0d498649-7e5e-4fb1-6c41-08d878496392
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Oct 2020 18:19:46.9132
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: vkoVdVvfzu7nBz6rlgR6EmMCY0JIEpee0heR2W4EkLhKLBkuw9oxY1JxRzi5QFGFHkn7Japuq8Ola0Z75wMQJA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB7007
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Oct 24, 2020 at 07:17:05PM +0200, Andrew Lunn wrote:
> > - Every PHY driver gains a .handle_interrupt() implementation that, for
> >   the most part, would look like below:
> >=20
> > 	irq_status =3D phy_read(phydev, INTR_STATUS);
> > 	if (irq_status < 0) {
> > 		phy_error(phydev);
> > 		return IRQ_NONE;
> > 	}
> >=20
> > 	if (irq_status =3D=3D 0)
> > 		return IRQ_NONE;
> >=20
> > 	phy_trigger_machine(phydev);
> >=20
> > 	return IRQ_HANDLED;
>=20
> Hi Ioana
>=20
> It looks like phy_trigger_machine(phydev) could be left in the core,
> phy_interrupt(). It just needs to look at the return code, IRQ_HANDLED
> means trigger the state machine.

I tend to disagree that this would bring us any benefit.

Keeping the phy_trigger_machine() inside the phy_interrupt() would mean
that we are changing the convention of what the implementation of
.handle_interrupt() should do.

At the moment, there are drivers which use it to handle multiple
interrupt sources within the same PHY device (e.g. MACSEC, 1588, link
state). With your suggestion, when a MACSEC interrupt is received, the
PHY driver would be forced to return IRQ_NONE just so phylib does not
trigger the link state machine. I think this would eventually lead to
some "irq X: nobody cared".

Also, the vsc8584_handle_interrupt() already calls a wrapper over
phy_trigger_machine() called phy_mac_interrupt() which was intended for
MAC driver use only.

Ioana