Message-ID: <b4fc5099-81c9-4873-b5f0-0c8a5d77571e@kernel.org>
Date: Thu, 5 Feb 2026 14:58:17 +0100
From: Hans Verkuil <hverkuil+cisco@...nel.org>
To: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc: Linux Doc Mailing List <linux-doc@...r.kernel.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, linux-kernel@...r.kernel.org,
 linux-media@...r.kernel.org, Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Jonathan Corbet <corbet@....net>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Ricardo Ribalda <ribalda@...omium.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, Sean Young <sean@...s.org>
Subject: Re: [PATCH 2/2] docs: media: media-committer: do some editorial
 changes

On 2/5/26 14:51, Mauro Carvalho Chehab wrote:
> On Thu, Feb 05, 2026 at 12:52:35PM +0100, Hans Verkuil wrote:
>> Hi Mauro,
>>
>> For the most part I agree, but I have some suggestions regarding the point 4
>> you added, since I think it just restates what is already mentioned in
>> maintainer-entry-profile.rst.
>>
>> Let me know what you think of my suggestions.
>>
>> On 2/4/26 15:37, Mauro Carvalho Chehab wrote:
>>> Do some editorial changes to make it look clearer:
>>>
>>>   - media-committers tree references corrected from singular to plural;
>>>   - updated commit rights wording and responsibilities;
>>>   - fixed various typographical errors;
>>>   - corrected “mailing list” and “Kernel” references;
>>>   - improved core committer description;
>>>   - updated documentation paths and URLs;
>>>   - added missing “for” and improved sentence flow.
>>>
>>> Perhaps the most relevant change is that i removed a word
>>> that was requiring granting Patchwork rights some time before
>>> adding commit rights (we may grant them altogether if makes
>>> sense for us), and I added a 4th note to committer notes
>>> list to let it clear that about what it is expected from a
>>> committer with regards to updating Patchwork.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
>>> ---
>>>  .../driver-api/media/media-committer.rst      | 97 ++++++++++---------
>>>  1 file changed, 51 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/Documentation/driver-api/media/media-committer.rst b/Documentation/driver-api/media/media-committer.rst
>>> index 18cce6e06a2b..c83e94750e57 100644
>>> --- a/Documentation/driver-api/media/media-committer.rst
>>> +++ b/Documentation/driver-api/media/media-committer.rst
>>> @@ -20,8 +20,8 @@ and the Linux Media community.
>>>  
>>>  .. Note::
>>
>> Re-reading this I don't really think this should be a note at all. This just
>> lists the additional responsibilities of a media committer, no need to
>> present this as a 'note'.
> 
> Agreed.
> 
>>>  
>>> -   1. Patches you authored must have a Signed-off-by, Reviewed-by or Acked-by
>>> -      of another Media Maintainer;
>>> +   1. Patches you authored must have a ``Signed-off-by``, ``Reviewed-by``
>>> +      or ``Acked-by`` from another Media Maintainer;
>>>     2. If a patch introduces a regression, then it is the Media Committer's
>>>        responsibility to correct that as soon as possible. Typically the
>>>        patch is either reverted, or an additional patch is committed to
>>> @@ -29,14 +29,18 @@ and the Linux Media community.
>>>     3. If patches are fixing bugs against already released Kernels, including
>>>        the reverts above mentioned, the Media Committer shall add the needed
>>>        tags. Please see :ref:`Media development workflow` for more details.
>>> +   4. All Media Committers are responsible for maintaining
>>> +      `Patchwork <https://patchwork.linuxtv.org/project/linux-media/list/>`_,
>>> +      updating the state of the patches they review or merge.
>>> +
>>
>> I don't really agree with this. Not that it hurts, but maintaining patchwork
>> is a job of media maintainers.
> 
> Not really: "normal" media driver maintainers don't need to do that, and, even
> if we write they should, I doubt most would.

This applies to maintainers with patchwork rights. If you have patchwork access,
then I expect them to keep it up to date for those areas that they maintain.

> 
> In practice, I expect only core maintainers, subsystem maitnainers and a couple
> of driver maintainers to actually update it.

I.e., those with patchwork access.

> 
> I'd like to keep a mention here, as we expect media committers to actually
> read this file and understand what it is expected from them. As such,
> it doesn't hurt letting something explicit here.
> 
> The point is: if a committer forgets to update it, we may end having the
> same patch being reviewed by two people at different moments, wasting
> precious review time. Worse than that, if a rejected patch was kept
> as new on patchwork, another committer may end wrongly merging it.
> 
>> The only addition for committers is that they
>> have to update patches in patchwork to 'Accepted' when they have committed
>> them. That is certainly worth mentioning (including updating the maintainers
>> profile to clearly state that only committers can set it to Accepted.
> 
> With the "committers hat", yes: most of the time it will be just "Accepted",
> but, even so, if they pick a series with duplicated patches, other status
> needs to be updated, like "duplicated" and "superseeded". 
> 
>> So in maintainer-entry-profile.rst in 1.3 I would change the description for
>> the Accepted state to:
>>
>> "Accepted: Once a patch is merged in the media-committers tree. Only Media
>> Maintainers with commit rights can set this state."
> 
> Sounds good.
> 
>> And change point 4 to this:
>>
>> 4. After committing a patch, the Media Committer must also update the
>>    patch status to ``Accepted`` in
>>    `Patchwork <https://patchwork.linuxtv.org/project/linux-media/list/>`_.
> 
> I would avoid restricting it - or if you want to verbose what status
> type, those are the ones we currently have on patchwork(*):

I'll just keep your point 4. In the end it doesn't hurt.

Regards,

	Hans

> 
> 	Under Review
> 	Accepted
> 	Rejected
> 	RFC
> 	Not Applicable
> 	Changes Requested
> 	Awaiting Upstream
> 	Superseded
> 	Deferred
> 	Obsoleted	
> 	TODO
> 	driver maintainer
> 	Duplicated
> 
> (*) Heh, there are some that we only used for a very short period of time,
>     or maybe even never used, but we can't delete status there without
>     causing potential issues to the database.
> 
>>
>>>  
>>>  Becoming a Media Committer
>>>  --------------------------
>>>  
>>>  Existing Media Committers can nominate a Media Maintainer to be granted
>>> -commit rights. The Media Maintainer must already have patchwork access and
>>> -have been in that role for some time, and has demonstrated a good
>>> -understanding of the maintainer's duties and processes.
>>> +commit rights. The Media Maintainer must have patchwork access,
>>> +have been reviewing patches from third parties for some time, and has
>>> +demonstrated a good understanding of the maintainer's duties and processes.
>>>  
>>>  The ultimate responsibility for accepting a nominated committer is up to
>>>  the Media Subsystem Maintainers. The nominated committer must have earned a
>>> @@ -61,8 +65,8 @@ Media Committer's agreement
>>>  Once a nominated committer is accepted by all Media Subsystem Maintainers,
>>>  they will ask if the developer is interested in the nomination and discuss
>>>  what area(s) of the media subsystem the committer will be responsible for.
>>> -Those areas will typically be the same as the areas that are already
>>> -maintained by the nominated committer.
>>> +Those areas will typically be the same as the areas that the nominated
>>> +committer is already maintaining.
>>>  
>>>  When the developer accepts being a committer, the new committer shall
>>>  explicitly accept the Kernel development policies described under its
>>> @@ -77,7 +81,7 @@ following the model below::
>>>  
>>>     ...
>>>  
>>> -   For the purpose of committing patches to the media-committer's tree,
>>> +   For the purpose of committing patches to the media-committers tree,
>>>     I'll be using my user https://gitlab.freedesktop.org/users/<username>.
>>>  
>>>  Followed by a formal declaration of agreement with the Kernel development
>>> @@ -85,7 +89,7 @@ rules::
>>>  
>>>     I agree to follow the Kernel development rules described at:
>>>  
>>> -   https://www.kernel.org/doc/html/latest/driver-api/media/media-committer.rst
>>> +   https://www.kernel.org/doc/html/latest/driver-api/media/media-committers.rst
>>
>> BTW, I agree that this file should be renamed to media-committers.rst. That matches
>> the name of our git tree as well.
> 
> Good. Please check at the maintainers profile if we don't have a reference with
> the singular when submitting v8, as I guess we have.
> 
> Regards,
> Mauro

Powered by blists - more mailing lists