Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver

-Moved ACPI for ARM64 maintainers to "to:"

Hi Marc, Darren,

On 11/30/2021 11:41 AM, Darren Hart wrote:

On Tue, Nov 30, 2021 at 09:45:46AM +0000, Marc Zyngier wrote:

quoted

Hi Darren,

On Mon, 29 Nov 2021 20:39:23 +0000,
Darren Hart [off-list ref] wrote:

quoted

On Wed, Nov 24, 2021 at 06:09:14PM +0000, Marc Zyngier wrote:

quoted

On Wed, 24 Nov 2021 17:07:07 +0000,

quoted

diff --git a/MAINTAINERS b/MAINTAINERS
index 5250298d2817..aa0483726606 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS

@@ -382,6 +382,7 @@ ACPI FOR ARM64 (ACPI/arm64)
  M:	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
  M:	Hanjun Guo <guohanjun@huawei.com>
  M:	Sudeep Holla <sudeep.holla@arm.com>
+R:	Tyler Baicar <baicar@os.amperecomputing.com>
  L:	linux-acpi@vger.kernel.org
  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
  S:	Maintained

Isn't this a bit premature? This isn't even mentioned in the commit
message, only in passing in the cover letter.

Hi Marc,

This was something I encouraged Tyler to add during internal review,
both in response to the checkpatch.pl warning about adding new drivers
as well as our interest in reviewing any future changes to the aest
driver. Since refactoring is common, this level made sense to me - but
would it be preferable to add a new entry for just the new driver Tyler
added?

Adding someone as the co-maintainer/co-reviewer of a whole subsystem
(ACPI/arm64 in this case) comes, IMO, with a number of pre-requisites:
has the proposed co-{maintainer,reviewer} contributed and/or reviewed
a significant number of patches to that subsystem and/or actively
participated in the public discussions on the design and the
maintenance of the subsystem, so that their reviewing is authoritative
enough? I won't be judge of this, but it is definitely something to
consider.

Hi Marc,

Agreed. I applied similar criteria when considering sub maintainers for
the platform/x86 subsystem while I maintained it.

quoted

I don't think preemptively adding someone to the MAINTAINERS entry to
indicate an interest in a whole subsystem is the right way to do it.
One could argue that this is what a mailing list is for! ;-) On the
other hand, an active participation to the review process is the
perfect way to engage with fellow developers and to grow a profile. It
is at this stage that adding oneself as an upstream reviewer makes a
lot of sense.

Also generally agree. In this specific case, our interest was in the
driver itself, and we had to decide between the whole subsystem or
adding another F: entry in MAINTAINERS for the specific driver. Since
drivers/acpi/arm64 only has 3 .c files in it, adding another entry
seemed premature and overly granular. Certainly a subjective thing and
we have no objection to adding the extra line if that's preferred. This
should have been noted in the commit message.

Thank you for the feedback here, I will make sure to add this to the 
commit message and cover letter in the next version.

Hi Lorenzo, Hanjun, Sudeep,

As for adding myself as a reviewer under ACPI for ARM64 or adding 
another F: entry, do you have a preference or guidance on what I should 
do here?

Thanks,

Tyler

quoted

Alternatively, adding a MAINTAINERS entry for a specific driver is
definitely helpful and will certainly result in the listed maintainer
to be Cc'd on changes affecting it. But I would really like this
maintainer to actively engage with upstream, rather than simply be on
the receiving end of a stream of changes.

Agree for subsystems. For individual drivers, I think having the author
as a reviewer is appropriate and should result in more patch reviews,
which moves us in the direction of more community participation which we
all want to see.

Thanks,

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel