diff mbox series

[v2,phy-next,24/24] MAINTAINERS: add regexes for linux-phy

Message ID 20260308114009.2546587-25-vladimir.oltean@nxp.com (mailing list archive)
State New
Headers
Series Split Generic PHY consumer and provider API |

Commit Message

Vladimir Oltean March 8, 2026, 11:40 a.m. UTC 
Some pragmatic shortcuts are being taken by PHY consumer driver authors,
which put a burden on the framework. A lot of these can be caught during
review.

Make sure the linux-phy list is copied on as many keywords that regexes
can reasonably catch.

Some considerations that led to this solution and not a simpler one:
- Consumers may be located anywhere, and their file naming provides no
  indication whatsoever that they are PHY API consumers.
- The network PHY API has similarly sounding API: phy_start(),
  phy_connect(), etc. Similarly, matching on "phy" would hit
  phys_addr_t, "cryptography", etc.
- The header files themselves need attention to avoid matching on
  include/linux/phy.h (network PHY), include/linux/usb/phy.h,
  drivers/net/vendor/device/phy.h, etc.
- At least for a transitional period, I suppose developers will still
  try to add PHY providers outside the subsystem (which is discouraged).

So I used \b to try to match on actual word boundaries and I went for
listing all markers of PHY API use as they may appear in patch contexts.

Bit rot is a valid concern. I will add a test to the build automation
that newly introduced struct and function names in include/linux/phy.h,
include/linux/phy-props.h and drivers/phy/phy-provider.h are matched by
the MAINTAINERS entry K: patterns.

The keyword patterns were written with great help from Joe Perches
<joe@perches.com>.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Cc: Joe Perches <joe@perches.com>

v1->v2:
- split into multiple regex patterns
- use matching-only (insted of capturing) regex patterns
- adjust commit message to reflect the Q&A from v1
---
 MAINTAINERS | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Joe Perches March 8, 2026, 6:40 p.m. UTC | #1 
On Sun, 2026-03-08 at 13:40 +0200, vladimir.oltean@nxp.com wrote:
> diff --git a/MAINTAINERS b/MAINTAINERS
[]
> @@ -10713,6 +10713,17 @@ F:	Documentation/devicetree/bindings/phy/
> +K:	(?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h)

phy-props.h and phy-provider.h don't seem to exist.
Are these going to be added later?
Maybe this should be phy-common-props.h ?

Perhaps if matching only the include uses:
(/ is escaped because get_maintainer is stupid)
something like:

K:	include\s*\<linux\/phy\/phy(?:-common-props|-provider)?\.h\>
Vladimir Oltean March 8, 2026, 7:10 p.m. UTC | #2 
On Sun, Mar 08, 2026 at 11:40:44AM -0700, Joe Perches wrote:
> On Sun, 2026-03-08 at 13:40 +0200, vladimir.oltean@nxp.com wrote:
> > diff --git a/MAINTAINERS b/MAINTAINERS
> []
> > @@ -10713,6 +10713,17 @@ F:	Documentation/devicetree/bindings/phy/
> > +K:	(?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h)
> 
> phy-props.h and phy-provider.h don't seem to exist.
> Are these going to be added later?

They're actually provided by patch 13/24:
https://lore.kernel.org/linux-phy/20260308114009.2546587-13-vladimir.oltean@nxp.com/T/#u
./include/linux/phy/phy-props.h
./drivers/phy/phy-provider.h

> Maybe this should be phy-common-props.h ?

No, that's unrelated.

> Perhaps if matching only the include uses:
> (/ is escaped because get_maintainer is stupid)
> something like:
> 
> K:	include\s*\<linux\/phy\/phy(?:-common-props|-provider)?\.h\>

Why is get_maintainer stupid? I ran git format-patch -1 0fc83bd79589b
(random patch which includes <linux/phy/phy.h> and I got:

GENERIC PHY FRAMEWORK status: Supported:Keyword:(?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h)
GENERIC PHY FRAMEWORK status: Supported:Keyword:\b(?:devm_)?(?:of_)?phy_(?:optional_)?(?:get|put)(?:_by_index)?\b
GENERIC PHY FRAMEWORK status: Supported:Keyword:\bphy_(?:init|exit|power_(?:on|off))\b
GENERIC PHY FRAMEWORK status: Supported:Keyword:\bphy_(?:get|set)_(?:mode(?:_ext)?|media|speed|bus_width|max_link_rate)\b
GENERIC PHY FRAMEWORK status: Supported:Keyword:\bstruct\s+phy(?:_ops|_attrs|_lookup|_provider)?\b

which seems to be OK, it matched the (?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h) regex.
Joe Perches March 8, 2026, 7:25 p.m. UTC | #3 
On Sun, 2026-03-08 at 21:10 +0200, Vladimir Oltean wrote:
> On Sun, Mar 08, 2026 at 11:40:44AM -0700, Joe Perches wrote:
> > Perhaps if matching only the include uses:
> > (/ is escaped because get_maintainer is stupid)
> > something like:
> > 
> > K:	include\s*\<linux\/phy\/phy(?:-common-props|-provider)?\.h\>

> Why is get_maintainer stupid?

The get_maintainer code used to match keywords is

	    foreach my $line (keys %keyword_hash) {
		if ($text =~ m/$keyword_hash{$line}/x) {

so it seems the first / in the K: <foo> regex would terminate
the match.

It might have been better to use a different delimiter.
Maybe:

		if ($text =~ m{$keyword_hash{$line}}/x
Vladimir Oltean March 8, 2026, 8:56 p.m. UTC | #4 
On Sun, Mar 08, 2026 at 12:25:32PM -0700, Joe Perches wrote:
> On Sun, 2026-03-08 at 21:10 +0200, Vladimir Oltean wrote:
> > On Sun, Mar 08, 2026 at 11:40:44AM -0700, Joe Perches wrote:
> > > Perhaps if matching only the include uses:
> > > (/ is escaped because get_maintainer is stupid)
> > > something like:
> > > 
> > > K:	include\s*\<linux\/phy\/phy(?:-common-props|-provider)?\.h\>
> 
> > Why is get_maintainer stupid?
> 
> The get_maintainer code used to match keywords is
> 
> 	    foreach my $line (keys %keyword_hash) {
> 		if ($text =~ m/$keyword_hash{$line}/x) {
> 
> so it seems the first / in the K: <foo> regex would terminate
> the match.
> 
> It might have been better to use a different delimiter.
> Maybe:
> 
> 		if ($text =~ m{$keyword_hash{$line}}/x

So why does it match in my example?
Joe Perches March 8, 2026, 10:22 p.m. UTC | #5 
On Sun, 2026-03-08 at 22:56 +0200, Vladimir Oltean wrote:
> On Sun, Mar 08, 2026 at 12:25:32PM -0700, Joe Perches wrote:
> > On Sun, 2026-03-08 at 21:10 +0200, Vladimir Oltean wrote:
> On Sun, Mar 08, 2026 at 11:40:44AM -0700, Joe Perches wrote:
> > > Perhaps if matching only the include uses:
> > > > (/ is escaped because get_maintainer is stupid)
> > > > something like:
> > > > 
> > > > K:	include\s*\<linux\/phy\/phy(?:-common-props|-provider)?\.h\>
> 
> > > Why is get_maintainer stupid?
> > 
> > The get_maintainer code used to match keywords is
> > 
> 	    foreach my $line (keys %keyword_hash) {
> 		if ($text =~ m/$keyword_hash{$line}/x) {
> > 
> > so it seems the first / in the K: <foo> regex would terminate
> > the match.
> > 
> > It might have been better to use a different delimiter.
> > Maybe:
> > 
> 		if ($text =~ m{$keyword_hash{$line}}/x
> 
> So why does it match in my example?

Not sure really.  But it does match exactly.
Maybe the regex code scans forward until the last /
as the / character is not a "real" metacharacter.

I added some test code and it produced:

line:	<+#include <linux/phy/phy.h>>
kw:	<(?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h)>
test:	<^[+-].*(?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h)>
match:	<+#include <linux/phy/phy.h>

>From https://perldoc.perl.org/perlrequick

Not all characters can be used 'as is' in a match.
Some characters, called metacharacters, are considered special,
and reserved for use in regex notation. The metacharacters are

{}[]()^$.|*+?\

A metacharacter can be matched literally by putting a backslash before it:

"2+2=4" =~ /2+2/;    # doesn't match, + is a metacharacter
"2+2=4" =~ /2\+2/;   # matches, \+ is treated like an ordinary +
'C:\WIN32' =~ /C:\\WIN/;                       # matches
"/usr/bin/perl" =~ /\/usr\/bin\/perl/;  # matches

In the last regex, the forward slash '/' is also backslashed, because it is used to delimit the regex.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 55af015174a5..29a1c2474bf2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10713,6 +10713,17 @@  F:	Documentation/devicetree/bindings/phy/
 F:	drivers/phy/
 F:	include/dt-bindings/phy/
 F:	include/linux/phy/
+K:	(?:linux/phy/phy\.h|phy-props\.h|phy-provider\.h)
+K:	\b(?:__)?(?:devm_)?(?:of_)?phy_(?:create|destroy|provider_(?:un)?register)\b
+K:	\bphy_(?:create|remove)_lookup\b
+K:	\bphy_(?:get|set)_drvdata\b
+K:	\b(?:devm_)?(?:of_)?phy_(?:optional_)?(?:get|put)(?:_by_index)?\b
+K:	\bphy_pm_runtime_(?:get|put)(?:_sync)?\b
+K:	\bphy_(?:init|exit|power_(?:on|off))\b
+K:	\bphy_(?:get|set)_(?:mode(?:_ext)?|media|speed|bus_width|max_link_rate)\b
+K:	\bphy_(?:reset|configure|validate|calibrate)\b
+K:	\bphy_notify_(?:connect|disconnect|state)\b
+K:	\bstruct\s+phy(?:_ops|_attrs|_lookup|_provider)?\b
 
 GENERIC PINCTRL I2C DEMULTIPLEXER DRIVER
 M:	Wolfram Sang <wsa+renesas@sang-engineering.com>