2018-12-17 16:20:59

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

Several drivers didn't have a specific maintainer (other than the
subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
patterns to capture all of them and add exceptions to the couple of
drivers that contain 'msm' but are not related to qcom hardware.

Thanks to Marc for the idea to use the N regex.

Signed-off-by: Amit Kucheria <[email protected]>
---
MAINTAINERS | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3318f30903b2..c9376030f77a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
M: David Brown <[email protected]>
L: [email protected]
S: Maintained
-F: Documentation/devicetree/bindings/soc/qcom/
-F: arch/arm/boot/dts/qcom-*.dts
-F: arch/arm/boot/dts/qcom-*.dtsi
-F: arch/arm/mach-qcom/
-F: arch/arm64/boot/dts/qcom/*
+N: qcom
+N: msm
+X: drivers/rtc/rtc-msm6242.c
+X: drivers/net/wireless/broadcom/brcm80211/brcmsmac/
F: drivers/i2c/busses/i2c-qup.c
-F: drivers/clk/qcom/
-F: drivers/dma/qcom/
-F: drivers/soc/qcom/
F: drivers/spi/spi-qup.c
-F: drivers/tty/serial/msm_serial.c
F: drivers/*/pm8???-*
F: drivers/mfd/ssbi.c
-F: drivers/firmware/qcom_scm*
T: git git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git

ARM/RADISYS ENP2611 MACHINE SUPPORT
--
2.17.1



2018-12-17 20:30:47

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

On Mon, Dec 17, 2018 at 10:51 PM Joe Perches <[email protected]> wrote:
>
> On Mon, 2018-12-17 at 21:49 +0530, Amit Kucheria wrote:
> > Several drivers didn't have a specific maintainer (other than the
> > subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
> > patterns to capture all of them and add exceptions to the couple of
> > drivers that contain 'msm' but are not related to qcom hardware.
> >
> > Thanks to Marc for the idea to use the N regex.
> []
> > diff --git a/MAINTAINERS b/MAINTAINERS
> []
> > @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
> > M: David Brown <[email protected]>
> > L: [email protected]
> > S: Maintained
> > -F: Documentation/devicetree/bindings/soc/qcom/
> > -F: arch/arm/boot/dts/qcom-*.dts
> > -F: arch/arm/boot/dts/qcom-*.dtsi
> > -F: arch/arm/mach-qcom/
> > -F: arch/arm64/boot/dts/qcom/*
> > +N: qcom
> > +N: msm
>
> It is generally better to use a comprehensive list of F: patterns
> over N: patterns as N: patterns are not used as properly maintained
> entries for files matched by the get_maintainer.pl script.

I did start with something like the following before the suggestion to use N:

diff --git i/MAINTAINERS w/MAINTAINERS
index 3318f30903b2..a04479053570 100644
--- i/MAINTAINERS
+++ w/MAINTAINERS
@@ -1930,16 +1930,38 @@ M: David Brown <[email protected]>
L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/soc/qcom/
+F: Documentation/devicetree/bindings/*/qcom*
F: arch/arm/boot/dts/qcom-*.dts
F: arch/arm/boot/dts/qcom-*.dtsi
F: arch/arm/mach-qcom/
F: arch/arm64/boot/dts/qcom/*
+F: drivers/edac/qcom*
+F: drivers/firmware/qcom*
+F: drivers/iio/adc/qcom*
+F: drivers/iommu/qcom*
F: drivers/i2c/busses/i2c-qup.c
+F: drivers/irqchip/qcom*
+F: drivers/cpufreq/qcom*
F: drivers/clk/qcom/
+F: drivers/crypto/qcom-*
F: drivers/dma/qcom/
+F: drivers/hwspinlock/qcom_*
+F: drivers/media/platform/qcom/
+F: drivers/mfd/qcom-*
+F: drivers/mailbox/qcom-*
+F: drivers/misc/qcom-*
+F: drivers/mtd/*/qcom*
+F: drivers/perf/qcom*
+F: drivers/power/*/qcom*
+F: drivers/regulator/qcom*
+F: drivers/remoteproc/qcom*
+F: drivers/rpmsg/qcom*
+F: drivers/slimbus/qcom*
F: drivers/soc/qcom/
F: drivers/spi/spi-qup.c
F: drivers/tty/serial/msm_serial.c
+F: drivers/tty/serial/qcom*
+F: drivers/watchdog/qcom*
F: drivers/*/pm8???-*
F: drivers/mfd/ssbi.c
F: drivers/firmware/qcom_scm*

> git history is used by default for the N: entries and that can
> cause whitespace style commit authors to be cc'd on various changes
> to matched files.

Is that by design? Or can we fix get_maintainer.pl to be less
sensitive to whitespace authorship?

> Also msm is a pretty common filename pattern and the X: exclusions
> for various matching but not associated files might need to be added
> to quite often.
>
> From MAINTAINERS:
>
> N: Files and directories with regex patterns.
> N: [^a-z]tegra all files whose path contains the word tegra
> One pattern per line. Multiple N: lines acceptable.
> scripts/get_maintainer.pl has different behavior for files that
> match F: pattern and matches of N: patterns. By default,
> get_maintainer will not look at git log history when an F: pattern
> match occurs. When an N: match occurs, git log history is used
> to also notify the people that have git commit signatures.
>
>

2018-12-17 21:46:16

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

On 17/12/2018 17:19, Amit Kucheria wrote:

> Several drivers didn't have a specific maintainer (other than the
> subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
> patterns to capture all of them and add exceptions to the couple of
> drivers that contain 'msm' but are not related to qcom hardware.
>
> Thanks to Marc for the idea to use the N regex.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> MAINTAINERS | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3318f30903b2..c9376030f77a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
> M: David Brown <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/soc/qcom/
> -F: arch/arm/boot/dts/qcom-*.dts
> -F: arch/arm/boot/dts/qcom-*.dtsi
> -F: arch/arm/mach-qcom/
> -F: arch/arm64/boot/dts/qcom/*
> +N: qcom
> +N: msm
> +X: drivers/rtc/rtc-msm6242.c
> +X: drivers/net/wireless/broadcom/brcm80211/brcmsmac/

I would exclude all of drivers/net/wireless/broadcom

Aside from that trivial issue,
Reviewed-by: Marc Gonzalez <[email protected]>

Regards.

2018-12-17 22:44:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

On Mon, 2018-12-17 at 21:49 +0530, Amit Kucheria wrote:
> Several drivers didn't have a specific maintainer (other than the
> subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
> patterns to capture all of them and add exceptions to the couple of
> drivers that contain 'msm' but are not related to qcom hardware.
>
> Thanks to Marc for the idea to use the N regex.
[]
> diff --git a/MAINTAINERS b/MAINTAINERS
[]
> @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
> M: David Brown <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/soc/qcom/
> -F: arch/arm/boot/dts/qcom-*.dts
> -F: arch/arm/boot/dts/qcom-*.dtsi
> -F: arch/arm/mach-qcom/
> -F: arch/arm64/boot/dts/qcom/*
> +N: qcom
> +N: msm

It is generally better to use a comprehensive list of F: patterns
over N: patterns as N: patterns are not used as properly maintained
entries for files matched by the get_maintainer.pl script.

git history is used by default for the N: entries and that can
cause whitespace style commit authors to be cc'd on various changes
to matched files.

Also msm is a pretty common filename pattern and the X: exclusions
for various matching but not associated files might need to be added
to quite often.

From MAINTAINERS:

N: Files and directories with regex patterns.
N: [^a-z]tegra all files whose path contains the word tegra
One pattern per line. Multiple N: lines acceptable.
scripts/get_maintainer.pl has different behavior for files that
match F: pattern and matches of N: patterns. By default,
get_maintainer will not look at git log history when an F: pattern
match occurs. When an N: match occurs, git log history is used
to also notify the people that have git commit signatures.



2018-12-18 00:29:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

On Tue, 2018-12-18 at 00:20 +0530, Amit Kucheria wrote:
> On Mon, Dec 17, 2018 at 10:51 PM Joe Perches <[email protected]> wrote:
> > git history is used by default for the N: entries and that can
> > cause whitespace style commit authors to be cc'd on various changes
> > to matched files.
>
> Is that by design?

yes.

> Or can we fix get_maintainer.pl to be less
> sensitive to whitespace authorship?

Use get_maintainer.pl's --nogit-fallback option



2018-12-18 08:06:58

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

Amit Kucheria <[email protected]> writes:

> Several drivers didn't have a specific maintainer (other than the
> subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
> patterns to capture all of them and add exceptions to the couple of
> drivers that contain 'msm' but are not related to qcom hardware.
>
> Thanks to Marc for the idea to use the N regex.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> MAINTAINERS | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3318f30903b2..c9376030f77a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
> M: David Brown <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/soc/qcom/
> -F: arch/arm/boot/dts/qcom-*.dts
> -F: arch/arm/boot/dts/qcom-*.dtsi
> -F: arch/arm/mach-qcom/
> -F: arch/arm64/boot/dts/qcom/*
> +N: qcom
> +N: msm

IMHO this is pretty fragile in the long term. For example only due to
historical reasons qualcomm wireless drivers currently under ath
directory but who knows if at some point we switch using qcom (or
qualcomm) directory. Also the wireless drivers might easily have
filenames containing strings like "msm" or "qcom" (which I assume would
match with "N" rules above).

--
Kalle Valo

2018-12-18 09:23:22

by Marc Gonzalez

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

On 18/12/2018 08:42, Kalle Valo wrote:

> Amit Kucheria wrote:
>
>> Several drivers didn't have a specific maintainer (other than the
>> subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
>> patterns to capture all of them and add exceptions to the couple of
>> drivers that contain 'msm' but are not related to qcom hardware.
>>
>> Thanks to Marc for the idea to use the N regex.
>>
>> Signed-off-by: Amit Kucheria <[email protected]>
>> ---
>> MAINTAINERS | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3318f30903b2..c9376030f77a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
>> M: David Brown <[email protected]>
>> L: [email protected]
>> S: Maintained
>> -F: Documentation/devicetree/bindings/soc/qcom/
>> -F: arch/arm/boot/dts/qcom-*.dts
>> -F: arch/arm/boot/dts/qcom-*.dtsi
>> -F: arch/arm/mach-qcom/
>> -F: arch/arm64/boot/dts/qcom/*
>> +N: qcom
>> +N: msm
>
> IMHO this is pretty fragile in the long term. For example only due to
> historical reasons qualcomm wireless drivers currently under ath
> directory but who knows if at some point we switch using qcom (or
> qualcomm) directory.

I am failing to follow your logic.

(IIUC, you are talking about drivers/net/wireless/ath/ath10k)

The fact that the "qcom" or "msm" nomenclature is not used for this driver now
just means that an explicit F entry is required. The fact that it could be renamed
in the future just means that the entry would need to be updated or folded into a
more generic matching pattern. What am I missing?

> Also the wireless drivers might easily have filenames containing
> strings like "msm" or "qcom" (which I assume would match with "N"
> rules above).
Any driver (not just wireless) might match "msm" or "qcom". These could be excluded
with an X directive (as the proposed patch does, in fact).

Regards.

2018-12-18 10:02:35

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

On Tue, Dec 18, 2018 at 1:12 PM Kalle Valo <[email protected]> wrote:
>
> Amit Kucheria <[email protected]> writes:
>
> > Several drivers didn't have a specific maintainer (other than the
> > subsystem maintainer). Switch to using the 'qcom' and 'msm' regex
> > patterns to capture all of them and add exceptions to the couple of
> > drivers that contain 'msm' but are not related to qcom hardware.
> >
> > Thanks to Marc for the idea to use the N regex.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > ---
> > MAINTAINERS | 14 ++++----------
> > 1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3318f30903b2..c9376030f77a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
> > M: David Brown <[email protected]>
> > L: [email protected]
> > S: Maintained
> > -F: Documentation/devicetree/bindings/soc/qcom/
> > -F: arch/arm/boot/dts/qcom-*.dts
> > -F: arch/arm/boot/dts/qcom-*.dtsi
> > -F: arch/arm/mach-qcom/
> > -F: arch/arm64/boot/dts/qcom/*
> > +N: qcom
> > +N: msm
>
> IMHO this is pretty fragile in the long term. For example only due to
> historical reasons qualcomm wireless drivers currently under ath
> directory but who knows if at some point we switch using qcom (or
> qualcomm) directory. Also the wireless drivers might easily have
> filenames containing strings like "msm" or "qcom" (which I assume would
> match with "N" rules above).

I've now sent a v2 of the patch that tries to list all the drivers
explicitly. Let's see which one is better liked. :)

Regards,
Amit

2018-12-18 10:11:11

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] MAINTAINERS: update list of qcom drivers

Marc Gonzalez <[email protected]> writes:

> On 18/12/2018 08:42, Kalle Valo wrote:
>
>> Amit Kucheria wrote:
>>
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1929,20 +1929,14 @@ M: Andy Gross <[email protected]>
>>> M: David Brown <[email protected]>
>>> L: [email protected]
>>> S: Maintained
>>> -F: Documentation/devicetree/bindings/soc/qcom/
>>> -F: arch/arm/boot/dts/qcom-*.dts
>>> -F: arch/arm/boot/dts/qcom-*.dtsi
>>> -F: arch/arm/mach-qcom/
>>> -F: arch/arm64/boot/dts/qcom/*
>>> +N: qcom
>>> +N: msm
>>
>> IMHO this is pretty fragile in the long term. For example only due to
>> historical reasons qualcomm wireless drivers currently under ath
>> directory but who knows if at some point we switch using qcom (or
>> qualcomm) directory.
>
> I am failing to follow your logic.
>
> (IIUC, you are talking about drivers/net/wireless/ath/ath10k)

Yeah, my example was just about ath10k and wil6210 as they go through my
tree. But it can apply to any other driver and subsystem as well:
bluetooth, future drivers and what ever works with Qualcomm hardware.

> The fact that the "qcom" or "msm" nomenclature is not used for this driver now
> just means that an explicit F entry is required. The fact that it could be renamed
> in the future just means that the entry would need to be updated or folded into a
> more generic matching pattern. What am I missing?

Not sure, but maybe you are missing the point that keeping MAINTAINER's
file up-to-date is hard and having uncommon rules like Amit and you
propose makes it even harder. Yeah, it should be simple but in practise
it's not, people easily forget to update it.

>> Also the wireless drivers might easily have filenames containing
>> strings like "msm" or "qcom" (which I assume would match with "N"
>> rules above).
>
> Any driver (not just wireless) might match "msm" or "qcom". These could be excluded
> with an X directive (as the proposed patch does, in fact).

Nobody will remember, or even know (for example I saw Amit's patch by
accident), that when adding files with string "qcom" or "msm" in path
you also need to add an exclusion to "ARM/QUALCOMM SUPPORT". That won't
work so errors are likely. It's a much safer approach to use F: rules
just like Joe proposed, that way the risk of people submitting patches
to wrong lists is reduced.

--
Kalle Valo