2023-12-18 10:30:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH] regulator: MAINTAINERS: add status for IRQ helpers

Each maintainer entry should have a status field:

$ ./scripts/get_maintainer.pl --self-test=sections
./MAINTAINERS:23368: warning: section without status

Fixes: d55444adedae ("MAINTAINERS: Add reviewer for regulator irq_helpers")
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 30322190a72f..6fd22db830f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23367,6 +23367,7 @@ K: regulator_get_optional

VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
R: Matti Vaittinen <[email protected]>
+S: Maintained
F: drivers/regulator/irq_helpers.c

VRF
--
2.34.1



2023-12-21 06:17:27

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH] regulator: MAINTAINERS: add status for IRQ helpers

Hi Krzysztof,

On 12/18/23 12:28, Krzysztof Kozlowski wrote:
> Each maintainer entry should have a status field:
>
> $ ./scripts/get_maintainer.pl --self-test=sections
> ./MAINTAINERS:23368: warning: section without status
>
> Fixes: d55444adedae ("MAINTAINERS: Add reviewer for regulator irq_helpers")
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 30322190a72f..6fd22db830f5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23367,6 +23367,7 @@ K: regulator_get_optional
>
> VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
> R: Matti Vaittinen <[email protected]>
> +S: Maintained

Isn't that a bit odd seeing the M: - entry is missing as well?

This entry falls under the drivers/regulator, and as such, is maintained
by Mark (and the "umbrella" entry VOLTAGE AND CURRENT REGULATOR
FRAMEWORK has all needed bits and pieces, like the M: and S:).

I think the current MAINTAINERS entries reflect the reality. Mark (and
Liam) are THE regulator guy(s). I am just doing bits and pieces here and
there, like reviewing the changes to these helpers.

I guess that from a technical POV duplicating the S: and M: here is a
bit pointless, and as all duplicates, adds overhead when changes are done.

I am happy with the existing entries, but seems like everyone else is
not. Still, having S: without M: can be a source of confusion. If S: is
required, maybe add Mark as M: here as well. (Or if this is not Ok with
Mark, switch my R to M - which in my opinion is still a bit pointless as
the changes to drivers/regulator/irq_helpers.c will flow through Mark's
hands in any case :] )

> F: drivers/regulator/irq_helpers.c
>
> VRF


Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2023-12-21 08:03:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] regulator: MAINTAINERS: add status for IRQ helpers

On 21/12/2023 07:17, Matti Vaittinen wrote:
> Hi Krzysztof,
>
> On 12/18/23 12:28, Krzysztof Kozlowski wrote:
>> Each maintainer entry should have a status field:
>>
>> $ ./scripts/get_maintainer.pl --self-test=sections
>> ./MAINTAINERS:23368: warning: section without status
>>
>> Fixes: d55444adedae ("MAINTAINERS: Add reviewer for regulator irq_helpers")
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> MAINTAINERS | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 30322190a72f..6fd22db830f5 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -23367,6 +23367,7 @@ K: regulator_get_optional
>>
>> VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
>> R: Matti Vaittinen <[email protected]>
>> +S: Maintained
>
> Isn't that a bit odd seeing the M: - entry is missing as well?
>
> This entry falls under the drivers/regulator, and as such, is maintained
> by Mark (and the "umbrella" entry VOLTAGE AND CURRENT REGULATOR
> FRAMEWORK has all needed bits and pieces, like the M: and S:).
>
> I think the current MAINTAINERS entries reflect the reality. Mark (and
> Liam) are THE regulator guy(s). I am just doing bits and pieces here and
> there, like reviewing the changes to these helpers.

And your piece needs S: to explain whether you do odd fixes, maintaining
or supporting. Although I understand questioning this with only R:, but
I would argue that it still applies - reviewing odd fixes, reviewing
unpaid or paid.

>
> I guess that from a technical POV duplicating the S: and M: here is a
> bit pointless, and as all duplicates, adds overhead when changes are done.

M: is optional, anyway the M: field from regulators count, but status
can be different than from the parent.

> I am happy with the existing entries, but seems like everyone else is
> not. Still, having S: without M: can be a source of confusion. If S: is
> required, maybe add Mark as M: here as well. (Or if this is not Ok with
> Mark, switch my R to M - which in my opinion is still a bit pointless as
> the changes to drivers/regulator/irq_helpers.c will flow through Mark's
> hands in any case :] )
>
Best regards,
Krzysztof


2023-12-22 06:57:22

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH] regulator: MAINTAINERS: add status for IRQ helpers

On 12/21/23 10:02, Krzysztof Kozlowski wrote:
> On 21/12/2023 07:17, Matti Vaittinen wrote:
>> Hi Krzysztof,
>>
>> On 12/18/23 12:28, Krzysztof Kozlowski wrote:
>>> Each maintainer entry should have a status field:
>>>
>>> $ ./scripts/get_maintainer.pl --self-test=sections
>>> ./MAINTAINERS:23368: warning: section without status
>>>
>>> Fixes: d55444adedae ("MAINTAINERS: Add reviewer for regulator irq_helpers")
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>> ---
>>> MAINTAINERS | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 30322190a72f..6fd22db830f5 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -23367,6 +23367,7 @@ K: regulator_get_optional
>>>
>>> VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
>>> R: Matti Vaittinen <[email protected]>
>>> +S: Maintained
>>
>> Isn't that a bit odd seeing the M: - entry is missing as well?
>>
>> This entry falls under the drivers/regulator, and as such, is maintained
>> by Mark (and the "umbrella" entry VOLTAGE AND CURRENT REGULATOR
>> FRAMEWORK has all needed bits and pieces, like the M: and S:).
>>
>> I think the current MAINTAINERS entries reflect the reality. Mark (and
>> Liam) are THE regulator guy(s). I am just doing bits and pieces here and
>> there, like reviewing the changes to these helpers.
>
> And your piece needs S: to explain whether you do odd fixes, maintaining
> or supporting. Although I understand questioning this with only R:, but
> I would argue that it still applies - reviewing odd fixes, reviewing
> unpaid or paid.

I still think this is a bit silly :) Reviewers are not expected to be
picking up the patches, nor are they really expected to be writing fixes
(although I'm somewhat committed to that). Hence, I'd claim the S: value
for a reviewer is very irrelevant for other developers.

What many of the developers care is getting their changes in, or getting
bugs they encounter fixed - and I bet the S: is used to get an
indication of the likelihood this happens.

>> I guess that from a technical POV duplicating the S: and M: here is a
>> bit pointless, and as all duplicates, adds overhead when changes are done.
>
> M: is optional, anyway the M: field from regulators count, but status
> can be different than from the parent.

I guess that in this case the status is not different as Mark is
maintaining both :)

TBH, Even if I think it's a bit silly to have S: without M:, I don't
care that much. From my POV, "Maintained" is the correct status. (It
might even be "Supported" for me - but this is kind of shady as there
may be extended periods when I am expected to fully focus on other stuff
during my paid time - so, "Supported" is "Supported" only when there is
bandwidth) :)

I can also be "maintaining" the regulator irq-helpers in a same fashion
I am "maintaining" the specific PMIC or IIO drivers (Eg, I do reviewing
but patches go via subsystem tree and the subsystem maintainer is the
real "gatekeeper") - it might make more sense if we add this "S:" line
(for the above reasons).

Well, I'll leave this to you and Mark to judge :)

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~