Messages in this thread | | | Date | Thu, 16 Dec 2021 17:05:15 -0500 | Subject | Re: [PATCH 1/2] ACPI/AEST: Initial AEST driver | From | Tyler Baicar <> |
| |
-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: >> Hi Darren, >> >> On Mon, 29 Nov 2021 20:39:23 +0000, >> Darren Hart <darren@os.amperecomputing.com> wrote: >>> On Wed, Nov 24, 2021 at 06:09:14PM +0000, Marc Zyngier wrote: >>>> On Wed, 24 Nov 2021 17:07:07 +0000, >>>>> 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. > >> 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
>> 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,
| |