linux-kernel - Re: [PATCH net-next v5 02/12] net: pse-pd: Add support for reporting events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250224120228.62531319@kmaincent-XPS-13-7390>
Date: Mon, 24 Feb 2025 12:02:28 +0100
From: Kory Maincent <kory.maincent@...tlin.com>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Andrew Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
 Abeni <pabeni@...hat.com>, Jonathan Corbet <corbet@....net>, Donald Hunter
 <donald.hunter@...il.com>, Rob Herring <robh@...nel.org>, Andrew Lunn
 <andrew+netdev@...n.ch>, Simon Horman <horms@...nel.org>, Heiner Kallweit
 <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Thomas
 Petazzoni <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
 linux-doc@...r.kernel.org, Kyle Swenson <kyle.swenson@....tech>, Dent
 Project <dentproject@...uxfoundation.org>, kernel@...gutronix.de, Maxime
 Chevallier <maxime.chevallier@...tlin.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v5 02/12] net: pse-pd: Add support for
 reporting events

Hello Oleksij,

On Fri, 21 Feb 2025 09:50:33 +0100
Oleksij Rempel <o.rempel@...gutronix.de> wrote:

> Hi Kory,
> 
> On Tue, Feb 18, 2025 at 05:19:06PM +0100, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@...tlin.com>
> > 
> > Add support for devm_pse_irq_helper() to register PSE interrupts. This aims
> > to report events such as over-current or over-temperature conditions
> > similarly to how the regulator API handles them but using a specific PSE
> > ethtool netlink socket.  
> 
> Thank you for your work. Here some comments.
> 
> ...
> 
> > --- a/drivers/net/mdio/fwnode_mdio.c
> > +++ b/drivers/net/mdio/fwnode_mdio.c
> > @@ -18,7 +18,8 @@ MODULE_LICENSE("GPL");
> >  MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
> >  
> >  static struct pse_control *
> > -fwnode_find_pse_control(struct fwnode_handle *fwnode)
> > +fwnode_find_pse_control(struct fwnode_handle *fwnode,
> > +			struct phy_device *phydev)
> >  {  
> 
> This change seems to be not directly related to the commit message.
> Is it the preparation for the multi-phy support?

I need to save the phy_device related to PSE control to use the right network
interface for the ethtool notification. (ethnl_pse_send_ntf())
Indeed I have not described this in the commit message.

> ...
> 
> > +/**
> > + * pse_to_regulator_notifs - Convert PSE notifications to Regulator
> > + *			     notifications
> > + * @notifs: PSE notifications
> > + *
> > + * Return: Regulator notifications
> > + */
> > +static unsigned long pse_to_regulator_notifs(unsigned long notifs)  
> 
> I prefer converting it the other way around to make it reusable for
> plain regulator-based PSEs. For example, the podl-pse-regulator driver
> won’t have its own interrupt handler but will instead use
> devm_regulator_register_notifier().

The driver PIs part send PSE notifications which will be converted to regulator
events from the core. It is posting events.
If you use devm_regulator_register_notifier() you will registers a listener for
the regulator events. It is two distinct things.

> Even full-fledged PSE controllers like the PD692x0 are just one part of
> a larger chain of regulators. An overcurrent event may originate from a
> downstream regulator that is not part of the PD692x0 itself. In this
> case, we need to process the event from the downstream regulator,
> convert it into an ethtool event, and forward it to the user.

If you want to do something in case of downstream regulator events you will deal
with regulator events not PSE events. I think you want to disable PIs in case of
event like downstream regulator over current.
What policy should we use? Should we disable all the PIs or only disabled the
low priority like the budget evaluation strategy of this series? As it is over
current event not related to budget we don't know how many PIs we should
disable.

Still as said before it is a distinct development that could be tackled later.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com