[v3,1/3] docs: media: update maintainer-entry-profile for multi-committers

Commit Message

Mauro Carvalho Chehab Dec. 2, 2024, 9:26 a.m. UTC

As the media subsystem will experiment with a multi-committers model,
update the Maintainer's entry profile to the new rules.

Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
Signed-off-by: Hans Verkuil <[e-mail]>
Reviewed-by: Ricardo Ribalda <[e-mail]>
---
 .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
 MAINTAINERS                                   |   1 +
 2 files changed, 163 insertions(+), 46 deletions(-)

Comments

Sakari Ailus Dec. 2, 2024, 12:40 p.m. UTC | #1

Hi Mauro,

Thanks for the set.

Looks good overall, please still see my comments below.

On Mon, Dec 02, 2024 at 10:26:19AM +0100, Mauro Carvalho Chehab wrote:
> As the media subsystem will experiment with a multi-committers model,
> update the Maintainer's entry profile to the new rules.
> 
> Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
> Signed-off-by: Hans Verkuil <[e-mail]>
> Reviewed-by: Ricardo Ribalda <[e-mail]>
> ---
>  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
>  MAINTAINERS                                   |   1 +
>  2 files changed, 163 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index ffc712a5f632..dc764163cf1c 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
>  Both media userspace and Kernel APIs are documented and the documentation
>  must be kept in sync with the API changes. It means that all patches that
>  add new features to the subsystem must also bring changes to the
> -corresponding API files.
> +corresponding API documentation files.
>  
> -Due to the size and wide scope of the media subsystem, media's
> -maintainership model is to have sub-maintainers that have a broad
> -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> -task to review the patches, providing feedback to users if the patches are
> +Due to the size and wide scope of the media subsystem, the media's
> +maintainership model is to have committers that have a broad knowledge of
> +a specific aspect of the subsystem. It is the committers' task to
> +review the patches, providing feedback to users if the patches are
>  following the subsystem rules and are properly using the media kernel and
>  userspace APIs.
>  
> -Patches for the media subsystem must be sent to the media mailing list
> -at [e-mail] as plain text only e-mail. Emails with
> -HTML will be automatically rejected by the mail server. It could be wise
> -to also copy the sub-maintainer(s).
> +Media committers
> +----------------
> +
> +In the media subsystem, there are experienced developers who can push

The media subsystem is generally understood to comprise of what's under
drivers/media, this should be referring to the community instead.

> +patches directly to the development tree. These developers are called
> +Media committers and are divided into the following categories:
> +
> +- Committers:
> +    contributors for one or more drivers within the media subsystem.
> +    They can push changes to the tree that do not affect the core or ABI.
> +
> +- Core committers:
> +    responsible for part of the media core. They are typically
> +    responsible for one or more drivers within the media subsystem, but, besides
> +    that, they can also merge patches that change the code common to multiple
> +    drivers, including the kernel internal API.
> +
> +- Subsystem maintainers:

s/Subsystem/Media tree/

?
> +    responsible for the subsystem as a whole, with access to the
> +    entire subsystem.
> +
> +    Only subsystem maintainers can push changes that affect the userspace
> +    API/ABI.

This is ambiguous. I think it should intend to say API/ABI changes require
approval from Media tree maintainers.

What constitutes a UAPI change is a topic of discussion on its own. Does it
require adding a new IOCTL? Taking into use a reserved field? Changing
little-used driver behaviour slightly? Fixing a bug in a driver?

The first two obviously yes, but the latter two probably not.

Also:

s/Only subsystem maintainers can push/Media tree maintainers' ack is
required for/

?

> +
> +All media committers shall explicitly agree with the Kernel development process
> +as described at Documentation/process/index.rst and to the Kernel
> +development rules inside the Kernel documentation, including its code of
> +conduct.
> +
> +Media development tree
> +----------------------
> +
> +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> +where we also maintain news about the subsystem, wiki pages and a patchwork
> +instance where we track patches though their lifetime.
> +
> +The main tree used by media developers is at:
> +
> +https://git.linuxtv.org/media.git/
> +
> +.. _Media development workflow:
> +
> +Media development workflow
> +++++++++++++++++++++++++++
> +
> +All changes for the media subsystem must be sent first as e-mails to the
> +media mailing list, following the process documented at

s/^/linux-/

Also I'd refer to it as "LMML".

> +Documentation/process/index.rst.
> +
> +It means that patches shall be submitted as plain text only via e-mail to
> [e-mail]. While subscription is not mandatory, you
> +can find details about how to subscribe to it and to see its archives at:
> +
> +  https://subspace.kernel.org/vger.kernel.org.html
> +
> +Emails with HTML will be automatically rejected by the mail server.
> +
> +It could be wise to also copy the media committer(s). You should use
> +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> +Please always copy driver's authors and maintainers.
> +
> +Such patches need to be based against a public branch or tag as follows:
> +
> +1. Patches for new features need to be based at the ``next`` branch of
> +   media.git tree;
> +
> +2. Fixes against an already released kernel should preferably be against
> +   the latest released Kernel. If they require a previously-applied
> +   change at media.git tree, they need to be against its ``fixes`` branch.

This would be a change to the current process that I don't think has been
discussed. If I understand correctly, generally this applies to patches
that have been merged to the development branch (formerly media stage tree
master) with Cc: stable and Fixes: tags.

> +
> +3. Fixes for issues not present at the latest released kernel shall
> +   be either against a -rc kernel for an upcoming release or
> +   against the ``fixes`` branch of the media.git tree.
> +
> +Patches with fixes shall have:
> +
> +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> +- when applicable, a ``Cc: [e-mail]``.
> +
> +Patches that were fixing bugs publicly reported by someone at the
> [e-mail] mailing list shall have:
> +
> +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> +
> +Patches that change API shall update documentation accordingly at the
> +same patch series.
> +
> +See Documentation/process/index.rst for more details about e-mail submission.
> +
> +Once a patch is submitted, it may follow either one of the following
> +workflows:
> +
> +a. Pull request workflow: patches are handled by subsystem maintainers::
> +
> +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|

s/e-mail/LMML/

(see earlier comment)? Same below.

> +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> +                +---------+   +-------+   +-----------------------+
> +
> +   For this workflow, pull requests can be generated by a committer,
> +   a previous committer, subsystem maintainers or by a trusted long-time

s/previous/former/

I'd also use plural in all cases here.

> +   contributor. If you are not in such group, please don't submit

> +   pull requests, as they will not be processed.
> +
> +b. Committers' workflow: patches are handled by media committers::
> +
> +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> +                +---------+   +--------------------+   +-----------+
> +
> +On both workflows, all patches shall be properly reviewed at
> [e-mail] before being merged at media-committers.git.
> +
> +When patches are picked by patchwork and when merged at media-committers,
> +CI bots will check for errors and may provide e-mail feedback about
> +patch problems. When this happens, the patch submitter must fix them, or
> +explain why the errors are false positives.
> +
> +Patches will only be moved to the next stage in those two workflows if they
> +don't fail on CI or if there are false-positives in the CI reports.

s/don't fail on/pass/

> +
> +Failures during e-mail submission
> ++++++++++++++++++++++++++++++++++
>  
>  Media's workflow is heavily based on Patchwork, meaning that, once a patch
>  is submitted, the e-mail will first be accepted by the mailing list
> @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
>  
>     - https://patchwork.linuxtv.org/project/linux-media/list/
>  
> -If it doesn't automatically appear there after a few minutes, then
> +If it doesn't automatically appear there after some time [2]_, then
>  probably something went wrong on your submission. Please check if the
> -email is in plain text\ [2]_ only and if your emailer is not mangling
> +email is in plain text\ [3]_ only and if your emailer is not mangling
>  whitespaces before complaining or submitting them again.
>  
> -You can check if the mailing list server accepted your patch, by looking at:
> +To troubleshoot problems, you should first check if the mailing list
> +server has accepted your patch, by looking at:
>  
>     - https://lore.kernel.org/linux-media/
>  
> -.. [2] If your email contains HTML, the mailing list server will simply
> +If the patch is there and not at patchwork, it is likely that your e-mailer
> +mangled the patch. Patchwork internally has a logic that checks if the
> +received e-mail contain a valid patch. Any whitespace and new line
> +breakages mangling the patch won't be recognized by patchwork, thus such
> +patch will be rejected.
> +
> +.. [2] It usually takes a few minutes for the patch to arrive, but
> +       the e-mail server may be busy, so it may take up to a few hours
> +       for a patch to be picked by patchwork.
> +
> +.. [3] If your email contains HTML, the mailing list server will simply
>         drop it, without any further notice.
>  
> +.. _media-developers-gpg:
>  
> -Media maintainers
> -+++++++++++++++++
> +Authentication for pull and merge requests
> +++++++++++++++++++++++++++++++++++++++++++
>  
> -At the media subsystem, we have a group of senior developers that
> -are responsible for doing the code reviews at the drivers (also known as
> -sub-maintainers), and another senior developer responsible for the
> -subsystem as a whole. For core changes, whenever possible, multiple
> -media maintainers do the review.
> +The authenticity of developers submitting pull requests and merge requests
> +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
>  
> -The media maintainers that work on specific areas of the subsystem are:
> +With the pull request workflow, pull requests shall use a PGP-signed tag.
>  
> -- Remote Controllers (infrared):
> -    Sean Young <[e-mail]>
> +For more details about PGP sign, please read
> +Documentation/process/maintainer-pgp-guide.rst.

s/.*/:ref:`the PGP guide <pgpguide>`/

>  
> -- HDMI CEC:
> -    Hans Verkuil <[e-mail]>
> +Subsystem maintainers
> +---------------------
>  
> -- Media controller drivers:
> -    Laurent Pinchart <[e-mail]>
> -
> -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> -    Sakari Ailus <[e-mail]>
> -
> -- V4L2 drivers and core V4L2 frameworks:
> -    Hans Verkuil <[e-mail]>
> -
> -The subsystem maintainer is:
> -  Mauro Carvalho Chehab <[e-mail]>
> -
> -Media maintainers may delegate a patch to other media maintainers as needed.
> -On such case, checkpatch's ``delegate`` field indicates who's currently
> -responsible for reviewing a patch.
> +The subsystem maintainers are:
> +  - Mauro Carvalho Chehab <[e-mail]> and
> +  - Hans Verkuil <[e-mail]>
>  
>  Submit Checklist Addendum
>  -------------------------
> @@ -108,17 +225,14 @@ implementing the media APIs:
>  ====================	=======================================================
>  Type			Tool
>  ====================	=======================================================
> -V4L2 drivers\ [3]_	``v4l2-compliance``
> +V4L2 drivers\ [4]_	``v4l2-compliance``
>  V4L2 virtual drivers	``contrib/test/test-media``
>  CEC drivers		``cec-compliance``
>  ====================	=======================================================
>  
> -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
>         V4L2 drivers.
>  
> -Other compilance tools are under development to check other parts of the
> -subsystem.
> -
>  Those tests need to pass before the patches go upstream.
>  
>  Also, please notice that we build the Kernel with::
> @@ -134,6 +248,8 @@ Where the check script is::
>  Be sure to not introduce new warnings on your patches without a
>  very good reason.
>  
> +Please see `Media development workflow`_ for e-mail submission rules.
> +
>  Style Cleanup Patches
>  +++++++++++++++++++++
>  
> @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
>  Please notice that the media subsystem is a high traffic one, so it
>  could take a while for us to be able to review your patches. Feel free
>  to ping if you don't get a feedback in a couple of weeks or to ask
> -other developers to publicly add Reviewed-by and, more importantly,
> +other developers to publicly add ``Reviewed-by:`` and, more importantly,
>  ``Tested-by:`` tags.
>  
>  Please note that we expect a detailed description for ``Tested-by:``,
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..c77f56a2e695 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
>  M:	Mauro Carvalho Chehab <[e-mail]>
>  L:	[e-mail]
>  S:	Maintained
> +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
>  W:	https://linuxtv.org
>  Q:	http://patchwork.kernel.org/project/linux-media/list/
>  T:	git git://linuxtv.org/media.git

Hans Verkuil Dec. 2, 2024, 1:17 p.m. UTC | #2

Hi Mauro,

On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> As the media subsystem will experiment with a multi-committers model,
> update the Maintainer's entry profile to the new rules.
> 
> Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
> Signed-off-by: Hans Verkuil <[e-mail]>

xs4ll.nl -> xs4all.nl

(it's probably my typo, but please fix this in the final version)

> Reviewed-by: Ricardo Ribalda <[e-mail]>
> ---
>  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
>  MAINTAINERS                                   |   1 +
>  2 files changed, 163 insertions(+), 46 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> index ffc712a5f632..dc764163cf1c 100644
> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
>  Both media userspace and Kernel APIs are documented and the documentation
>  must be kept in sync with the API changes. It means that all patches that
>  add new features to the subsystem must also bring changes to the
> -corresponding API files.
> +corresponding API documentation files.
>  
> -Due to the size and wide scope of the media subsystem, media's
> -maintainership model is to have sub-maintainers that have a broad
> -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> -task to review the patches, providing feedback to users if the patches are
> +Due to the size and wide scope of the media subsystem, the media's
> +maintainership model is to have committers that have a broad knowledge of
> +a specific aspect of the subsystem. It is the committers' task to
> +review the patches, providing feedback to users if the patches are
>  following the subsystem rules and are properly using the media kernel and
>  userspace APIs.
>  
> -Patches for the media subsystem must be sent to the media mailing list
> -at [e-mail] as plain text only e-mail. Emails with
> -HTML will be automatically rejected by the mail server. It could be wise
> -to also copy the sub-maintainer(s).
> +Media committers
> +----------------
> +
> +In the media subsystem, there are experienced developers who can push
> +patches directly to the development tree. These developers are called
> +Media committers and are divided into the following categories:
> +
> +- Committers:
> +    contributors for one or more drivers within the media subsystem.
> +    They can push changes to the tree that do not affect the core or ABI.
> +
> +- Core committers:
> +    responsible for part of the media core. They are typically
> +    responsible for one or more drivers within the media subsystem, but, besides
> +    that, they can also merge patches that change the code common to multiple
> +    drivers, including the kernel internal API.
> +
> +- Subsystem maintainers:
> +    responsible for the subsystem as a whole, with access to the
> +    entire subsystem.
> +
> +    Only subsystem maintainers can push changes that affect the userspace
> +    API/ABI.
> +
> +All media committers shall explicitly agree with the Kernel development process
> +as described at Documentation/process/index.rst and to the Kernel
> +development rules inside the Kernel documentation, including its code of
> +conduct.
> +
> +Media development tree
> +----------------------
> +
> +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> +where we also maintain news about the subsystem, wiki pages and a patchwork
> +instance where we track patches though their lifetime.
> +
> +The main tree used by media developers is at:
> +
> +https://git.linuxtv.org/media.git/
> +
> +.. _Media development workflow:
> +
> +Media development workflow
> +++++++++++++++++++++++++++
> +
> +All changes for the media subsystem must be sent first as e-mails to the
> +media mailing list, following the process documented at
> +Documentation/process/index.rst.
> +
> +It means that patches shall be submitted as plain text only via e-mail to
> [e-mail]. While subscription is not mandatory, you
> +can find details about how to subscribe to it and to see its archives at:
> +
> +  https://subspace.kernel.org/vger.kernel.org.html
> +
> +Emails with HTML will be automatically rejected by the mail server.
> +
> +It could be wise to also copy the media committer(s). You should use
> +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> +Please always copy driver's authors and maintainers.
> +
> +Such patches need to be based against a public branch or tag as follows:
> +
> +1. Patches for new features need to be based at the ``next`` branch of
> +   media.git tree;
> +
> +2. Fixes against an already released kernel should preferably be against
> +   the latest released Kernel. If they require a previously-applied
> +   change at media.git tree, they need to be against its ``fixes`` branch.

What is an "released kernel"? Does an -rcX kernel qualify?

> +
> +3. Fixes for issues not present at the latest released kernel shall
> +   be either against a -rc kernel for an upcoming release or
> +   against the ``fixes`` branch of the media.git tree.

2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
at some point.

> +
> +Patches with fixes shall have:
> +
> +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> +- when applicable, a ``Cc: [e-mail]``.
> +
> +Patches that were fixing bugs publicly reported by someone at the
> [e-mail] mailing list shall have:
> +
> +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> +
> +Patches that change API shall update documentation accordingly at the
> +same patch series.
> +
> +See Documentation/process/index.rst for more details about e-mail submission.
> +
> +Once a patch is submitted, it may follow either one of the following
> +workflows:
> +
> +a. Pull request workflow: patches are handled by subsystem maintainers::
> +
> +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
> +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> +                +---------+   +-------+   +-----------------------+
> +
> +   For this workflow, pull requests can be generated by a committer,
> +   a previous committer, subsystem maintainers or by a trusted long-time
> +   contributor. If you are not in such group, please don't submit
> +   pull requests, as they will not be processed.
> +
> +b. Committers' workflow: patches are handled by media committers::
> +
> +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> +                +---------+   +--------------------+   +-----------+
> +
> +On both workflows, all patches shall be properly reviewed at
> [e-mail] before being merged at media-committers.git.
> +
> +When patches are picked by patchwork and when merged at media-committers,
> +CI bots will check for errors and may provide e-mail feedback about
> +patch problems. When this happens, the patch submitter must fix them, or
> +explain why the errors are false positives.
> +
> +Patches will only be moved to the next stage in those two workflows if they
> +don't fail on CI or if there are false-positives in the CI reports.
> +
> +Failures during e-mail submission
> ++++++++++++++++++++++++++++++++++
>  
>  Media's workflow is heavily based on Patchwork, meaning that, once a patch
>  is submitted, the e-mail will first be accepted by the mailing list
> @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
>  
>     - https://patchwork.linuxtv.org/project/linux-media/list/
>  
> -If it doesn't automatically appear there after a few minutes, then
> +If it doesn't automatically appear there after some time [2]_, then
>  probably something went wrong on your submission. Please check if the
> -email is in plain text\ [2]_ only and if your emailer is not mangling
> +email is in plain text\ [3]_ only and if your emailer is not mangling
>  whitespaces before complaining or submitting them again.
>  
> -You can check if the mailing list server accepted your patch, by looking at:
> +To troubleshoot problems, you should first check if the mailing list
> +server has accepted your patch, by looking at:
>  
>     - https://lore.kernel.org/linux-media/
>  
> -.. [2] If your email contains HTML, the mailing list server will simply
> +If the patch is there and not at patchwork, it is likely that your e-mailer
> +mangled the patch. Patchwork internally has a logic that checks if the

a logic -> logic

> +received e-mail contain a valid patch. Any whitespace and new line

contain -> contains

> +breakages mangling the patch won't be recognized by patchwork, thus such
> +patch will be rejected.
> +
> +.. [2] It usually takes a few minutes for the patch to arrive, but
> +       the e-mail server may be busy, so it may take up to a few hours
> +       for a patch to be picked by patchwork.
> +
> +.. [3] If your email contains HTML, the mailing list server will simply
>         drop it, without any further notice.
>  
> +.. _media-developers-gpg:
>  
> -Media maintainers
> -+++++++++++++++++
> +Authentication for pull and merge requests
> +++++++++++++++++++++++++++++++++++++++++++
>  
> -At the media subsystem, we have a group of senior developers that
> -are responsible for doing the code reviews at the drivers (also known as
> -sub-maintainers), and another senior developer responsible for the
> -subsystem as a whole. For core changes, whenever possible, multiple
> -media maintainers do the review.
> +The authenticity of developers submitting pull requests and merge requests
> +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.

sign -> signing

Hmm, merge requests through gitlab are not signed. So this isn't correct, I
think.

>  
> -The media maintainers that work on specific areas of the subsystem are:
> +With the pull request workflow, pull requests shall use a PGP-signed tag.
>  
> -- Remote Controllers (infrared):
> -    Sean Young <[e-mail]>
> +For more details about PGP sign, please read
> +Documentation/process/maintainer-pgp-guide.rst.
>  
> -- HDMI CEC:
> -    Hans Verkuil <[e-mail]>
> +Subsystem maintainers
> +---------------------
>  
> -- Media controller drivers:
> -    Laurent Pinchart <[e-mail]>
> -
> -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> -    Sakari Ailus <[e-mail]>
> -
> -- V4L2 drivers and core V4L2 frameworks:
> -    Hans Verkuil <[e-mail]>
> -
> -The subsystem maintainer is:
> -  Mauro Carvalho Chehab <[e-mail]>
> -
> -Media maintainers may delegate a patch to other media maintainers as needed.
> -On such case, checkpatch's ``delegate`` field indicates who's currently
> -responsible for reviewing a patch.
> +The subsystem maintainers are:
> +  - Mauro Carvalho Chehab <[e-mail]> and
> +  - Hans Verkuil <[e-mail]>
>  
>  Submit Checklist Addendum
>  -------------------------
> @@ -108,17 +225,14 @@ implementing the media APIs:
>  ====================	=======================================================
>  Type			Tool
>  ====================	=======================================================
> -V4L2 drivers\ [3]_	``v4l2-compliance``
> +V4L2 drivers\ [4]_	``v4l2-compliance``
>  V4L2 virtual drivers	``contrib/test/test-media``
>  CEC drivers		``cec-compliance``
>  ====================	=======================================================
>  
> -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside

The ``v4l2-compliance`` utility

(add 'utility')

>         V4L2 drivers.
>  
> -Other compilance tools are under development to check other parts of the
> -subsystem.
> -
>  Those tests need to pass before the patches go upstream.
>  
>  Also, please notice that we build the Kernel with::
> @@ -134,6 +248,8 @@ Where the check script is::
>  Be sure to not introduce new warnings on your patches without a
>  very good reason.
>  
> +Please see `Media development workflow`_ for e-mail submission rules.
> +
>  Style Cleanup Patches
>  +++++++++++++++++++++
>  
> @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
>  Please notice that the media subsystem is a high traffic one, so it
>  could take a while for us to be able to review your patches. Feel free
>  to ping if you don't get a feedback in a couple of weeks or to ask
> -other developers to publicly add Reviewed-by and, more importantly,
> +other developers to publicly add ``Reviewed-by:`` and, more importantly,
>  ``Tested-by:`` tags.
>  
>  Please note that we expect a detailed description for ``Tested-by:``,
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..c77f56a2e695 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
>  M:	Mauro Carvalho Chehab <[e-mail]>
>  L:	[e-mail]
>  S:	Maintained
> +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
>  W:	https://linuxtv.org
>  Q:	http://patchwork.kernel.org/project/linux-media/list/

Shouldn't this point to our patchwork instance instead?

>  T:	git git://linuxtv.org/media.git

Regards,

	Hans

Mauro Carvalho Chehab Dec. 2, 2024, 1:23 p.m. UTC | #3

Em Mon, 2 Dec 2024 12:40:44 +0000
Sakari Ailus <[e-mail]> escreveu:

> Hi Mauro,
> 
> Thanks for the set.
> 
> Looks good overall, please still see my comments below.
> 
> On Mon, Dec 02, 2024 at 10:26:19AM +0100, Mauro Carvalho Chehab wrote:
> > As the media subsystem will experiment with a multi-committers model,
> > update the Maintainer's entry profile to the new rules.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
> > Signed-off-by: Hans Verkuil <[e-mail]>
> > Reviewed-by: Ricardo Ribalda <[e-mail]>
> > ---
> >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> >  MAINTAINERS                                   |   1 +
> >  2 files changed, 163 insertions(+), 46 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > index ffc712a5f632..dc764163cf1c 100644
> > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> >  Both media userspace and Kernel APIs are documented and the documentation
> >  must be kept in sync with the API changes. It means that all patches that
> >  add new features to the subsystem must also bring changes to the
> > -corresponding API files.
> > +corresponding API documentation files.
> >  
> > -Due to the size and wide scope of the media subsystem, media's
> > -maintainership model is to have sub-maintainers that have a broad
> > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > -task to review the patches, providing feedback to users if the patches are
> > +Due to the size and wide scope of the media subsystem, the media's
> > +maintainership model is to have committers that have a broad knowledge of
> > +a specific aspect of the subsystem. It is the committers' task to
> > +review the patches, providing feedback to users if the patches are
> >  following the subsystem rules and are properly using the media kernel and
> >  userspace APIs.
> >  
> > -Patches for the media subsystem must be sent to the media mailing list
> > -at [e-mail] as plain text only e-mail. Emails with
> > -HTML will be automatically rejected by the mail server. It could be wise
> > -to also copy the sub-maintainer(s).
> > +Media committers
> > +----------------
> > +
> > +In the media subsystem, there are experienced developers who can push  
> 
> The media subsystem is generally understood to comprise of what's under
> drivers/media, this should be referring to the community instead.

Do you have a proposal for a different text here?

> 
> > +patches directly to the development tree. These developers are called
> > +Media committers and are divided into the following categories:
> > +
> > +- Committers:
> > +    contributors for one or more drivers within the media subsystem.
> > +    They can push changes to the tree that do not affect the core or ABI.
> > +
> > +- Core committers:
> > +    responsible for part of the media core. They are typically
> > +    responsible for one or more drivers within the media subsystem, but, besides
> > +    that, they can also merge patches that change the code common to multiple
> > +    drivers, including the kernel internal API.
> > +
> > +- Subsystem maintainers:  
> 
> s/Subsystem/Media tree/

Here, we're talking specifically about my hole and Hans, which will
co-maintain the subsystem with me.. I guess subsystem maintainer is the 
best to describe it.

Besides that, we used "media maintainers" in the past with a different
meaning. Better to not re-use it here.
> 
> ?
> > +    responsible for the subsystem as a whole, with access to the
> > +    entire subsystem.
> > +
> > +    Only subsystem maintainers can push changes that affect the userspace
> > +    API/ABI.  
> 
> This is ambiguous. I think it should intend to say API/ABI changes require
> approval from Media tree maintainers.

At the first moment, the idea is to commit them via PRs. So, no such
commits will be merged by committers/core committers, but yeah, it
could also be merged directly by a committer if it has our approval.

In any case, such changes need a consensus from the subsystem maintainers,
which can just be based on a trust relationship between them with
regards to certain parts of the subsystem, or via explicit acks.

Maybe:

	API/ABI changes are done via consensus between subsystem
	maintainers.

	Only subsystem maintainers push changes that affect the userspace
	API/ABI. Committers may push directly if they have approvals
	from subsystem maintainers.

> What constitutes a UAPI change is a topic of discussion on its own. 

Everything that would break backward compatibility with existing non-kernel
code are API/ABI changes.

> Does it
> require adding a new IOCTL? Taking into use a reserved field? Changing
> little-used driver behaviour slightly? 

For all of those, yes: any changes affecting the behavior or fields/nodes
exported via ioctls and sysfs, including new V4L2 controls are API/ABI changes.

Some changes at OF are also API/ABI changes, but those can flow via
committers, provided that OF maintainers added their review or acked-by.

> Fixing a bug in a driver?

No, fixing a bug, even if related to ABI/API non-compliance aren't.

Yet, ABI/API behavior changes at drivers shall not cause regressions.

I don't think we need to let it clear at the text

> The first two obviously yes, but the latter two probably not.
> 
> Also:
> 
> s/Only subsystem maintainers can push/Media tree maintainers' ack is
> required for/
> 
> ?
> 
> > +
> > +All media committers shall explicitly agree with the Kernel development process
> > +as described at Documentation/process/index.rst and to the Kernel
> > +development rules inside the Kernel documentation, including its code of
> > +conduct.
> > +
> > +Media development tree
> > +----------------------
> > +
> > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > +instance where we track patches though their lifetime.
> > +
> > +The main tree used by media developers is at:
> > +
> > +https://git.linuxtv.org/media.git/
> > +
> > +.. _Media development workflow:
> > +
> > +Media development workflow
> > +++++++++++++++++++++++++++
> > +
> > +All changes for the media subsystem must be sent first as e-mails to the
> > +media mailing list, following the process documented at  
> 
> s/^/linux-/
> 
> Also I'd refer to it as "LMML".

We can add an alias there, but better to be explicit about what mailing
list we're referring to.

> 
> > +Documentation/process/index.rst.
> > +
> > +It means that patches shall be submitted as plain text only via e-mail to
> > [e-mail]. While subscription is not mandatory, you
> > +can find details about how to subscribe to it and to see its archives at:
> > +
> > +  https://subspace.kernel.org/vger.kernel.org.html
> > +
> > +Emails with HTML will be automatically rejected by the mail server.
> > +
> > +It could be wise to also copy the media committer(s). You should use
> > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > +Please always copy driver's authors and maintainers.
> > +
> > +Such patches need to be based against a public branch or tag as follows:
> > +
> > +1. Patches for new features need to be based at the ``next`` branch of
> > +   media.git tree;
> > +
> > +2. Fixes against an already released kernel should preferably be against
> > +   the latest released Kernel. If they require a previously-applied
> > +   change at media.git tree, they need to be against its ``fixes`` branch.  
> 
> This would be a change to the current process that I don't think has been
> discussed. If I understand correctly, generally this applies to patches
> that have been merged to the development branch (formerly media stage tree
> master) with Cc: stable and Fixes: tags.


Not really a change: this is what we do in practice (except that we renamed
master to next).

We did discuss that during the LPC week. There was a request there about
simplifying the trees during media summit with some suggestions. I did a 
followup meeting with Hans afterwards for us to check what would work best.

The change is, basically:
	media-tree master -> media.git next
	media-tree fixes   -> media.git fixes

	media-stage master -> media-committers next

Subsystem maintainers are also merging patches at media-committers fixes,
in order to let media-ci to test the fixes branch.

As agreed during the media summit, only subsystem maintainers will be
merging fixes patches.

> > +
> > +3. Fixes for issues not present at the latest released kernel shall
> > +   be either against a -rc kernel for an upcoming release or
> > +   against the ``fixes`` branch of the media.git tree.
> > +
> > +Patches with fixes shall have:
> > +
> > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > +- when applicable, a ``Cc: [e-mail]``.
> > +
> > +Patches that were fixing bugs publicly reported by someone at the
> > [e-mail] mailing list shall have:
> > +
> > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > +
> > +Patches that change API shall update documentation accordingly at the
> > +same patch series.
> > +
> > +See Documentation/process/index.rst for more details about e-mail submission.
> > +
> > +Once a patch is submitted, it may follow either one of the following
> > +workflows:
> > +
> > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > +
> > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|  
> 
> s/e-mail/LMML/

maybe, instead: e-mail to LMML.

> (see earlier comment)? Same below.
> 
> > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > +                +---------+   +-------+   +-----------------------+
> > +
> > +   For this workflow, pull requests can be generated by a committer,
> > +   a previous committer, subsystem maintainers or by a trusted long-time  
> 
> s/previous/former/
> 
> I'd also use plural in all cases here.

Ok.

> 
> > +   contributor. If you are not in such group, please don't submit  
> 
> > +   pull requests, as they will not be processed.
> > +
> > +b. Committers' workflow: patches are handled by media committers::
> > +
> > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > +                +---------+   +--------------------+   +-----------+
> > +
> > +On both workflows, all patches shall be properly reviewed at
> > [e-mail] before being merged at media-committers.git.
> > +
> > +When patches are picked by patchwork and when merged at media-committers,
> > +CI bots will check for errors and may provide e-mail feedback about
> > +patch problems. When this happens, the patch submitter must fix them, or
> > +explain why the errors are false positives.
> > +
> > +Patches will only be moved to the next stage in those two workflows if they
> > +don't fail on CI or if there are false-positives in the CI reports.  
> 
> s/don't fail on/pass/

Ok.

> 
> > +
> > +Failures during e-mail submission
> > ++++++++++++++++++++++++++++++++++
> >  
> >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> >  is submitted, the e-mail will first be accepted by the mailing list
> > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> >  
> >     - https://patchwork.linuxtv.org/project/linux-media/list/
> >  
> > -If it doesn't automatically appear there after a few minutes, then
> > +If it doesn't automatically appear there after some time [2]_, then
> >  probably something went wrong on your submission. Please check if the
> > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > +email is in plain text\ [3]_ only and if your emailer is not mangling
> >  whitespaces before complaining or submitting them again.
> >  
> > -You can check if the mailing list server accepted your patch, by looking at:
> > +To troubleshoot problems, you should first check if the mailing list
> > +server has accepted your patch, by looking at:
> >  
> >     - https://lore.kernel.org/linux-media/
> >  
> > -.. [2] If your email contains HTML, the mailing list server will simply
> > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > +mangled the patch. Patchwork internally has a logic that checks if the
> > +received e-mail contain a valid patch. Any whitespace and new line
> > +breakages mangling the patch won't be recognized by patchwork, thus such
> > +patch will be rejected.
> > +
> > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > +       the e-mail server may be busy, so it may take up to a few hours
> > +       for a patch to be picked by patchwork.
> > +
> > +.. [3] If your email contains HTML, the mailing list server will simply
> >         drop it, without any further notice.
> >  
> > +.. _media-developers-gpg:
> >  
> > -Media maintainers
> > -+++++++++++++++++
> > +Authentication for pull and merge requests
> > +++++++++++++++++++++++++++++++++++++++++++
> >  
> > -At the media subsystem, we have a group of senior developers that
> > -are responsible for doing the code reviews at the drivers (also known as
> > -sub-maintainers), and another senior developer responsible for the
> > -subsystem as a whole. For core changes, whenever possible, multiple
> > -media maintainers do the review.
> > +The authenticity of developers submitting pull requests and merge requests
> > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
> >  
> > -The media maintainers that work on specific areas of the subsystem are:
> > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> >  
> > -- Remote Controllers (infrared):
> > -    Sean Young <[e-mail]>
> > +For more details about PGP sign, please read
> > +Documentation/process/maintainer-pgp-guide.rst.  
> 
> s/.*/:ref:`the PGP guide <pgpguide>`/

No need. A Sphinx plugin does that automatically.

> 
> >  
> > -- HDMI CEC:
> > -    Hans Verkuil <[e-mail]>
> > +Subsystem maintainers
> > +---------------------
> >  
> > -- Media controller drivers:
> > -    Laurent Pinchart <[e-mail]>
> > -
> > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > -    Sakari Ailus <[e-mail]>
> > -
> > -- V4L2 drivers and core V4L2 frameworks:
> > -    Hans Verkuil <[e-mail]>
> > -
> > -The subsystem maintainer is:
> > -  Mauro Carvalho Chehab <[e-mail]>
> > -
> > -Media maintainers may delegate a patch to other media maintainers as needed.
> > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > -responsible for reviewing a patch.
> > +The subsystem maintainers are:
> > +  - Mauro Carvalho Chehab <[e-mail]> and
> > +  - Hans Verkuil <[e-mail]>
> >  
> >  Submit Checklist Addendum
> >  -------------------------
> > @@ -108,17 +225,14 @@ implementing the media APIs:
> >  ====================	=======================================================
> >  Type			Tool
> >  ====================	=======================================================
> > -V4L2 drivers\ [3]_	``v4l2-compliance``
> > +V4L2 drivers\ [4]_	``v4l2-compliance``
> >  V4L2 virtual drivers	``contrib/test/test-media``
> >  CEC drivers		``cec-compliance``
> >  ====================	=======================================================
> >  
> > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
> >         V4L2 drivers.
> >  
> > -Other compilance tools are under development to check other parts of the
> > -subsystem.
> > -
> >  Those tests need to pass before the patches go upstream.
> >  
> >  Also, please notice that we build the Kernel with::
> > @@ -134,6 +248,8 @@ Where the check script is::
> >  Be sure to not introduce new warnings on your patches without a
> >  very good reason.
> >  
> > +Please see `Media development workflow`_ for e-mail submission rules.
> > +
> >  Style Cleanup Patches
> >  +++++++++++++++++++++
> >  
> > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> >  Please notice that the media subsystem is a high traffic one, so it
> >  could take a while for us to be able to review your patches. Feel free
> >  to ping if you don't get a feedback in a couple of weeks or to ask
> > -other developers to publicly add Reviewed-by and, more importantly,
> > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> >  ``Tested-by:`` tags.
> >  
> >  Please note that we expect a detailed description for ``Tested-by:``,
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1e930c7a58b1..c77f56a2e695 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> >  M:	Mauro Carvalho Chehab <[e-mail]>
> >  L:	[e-mail]
> >  S:	Maintained
> > +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
> >  W:	https://linuxtv.org
> >  Q:	http://patchwork.kernel.org/project/linux-media/list/
> >  T:	git git://linuxtv.org/media.git  
> 



Thanks,
Mauro

Mauro Carvalho Chehab Dec. 2, 2024, 1:54 p.m. UTC | #4

Em Mon, 2 Dec 2024 14:17:48 +0100
Hans Verkuil <[e-mail]> escreveu:

> Hi Mauro,
> 
> On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> > As the media subsystem will experiment with a multi-committers model,
> > update the Maintainer's entry profile to the new rules.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
> > Signed-off-by: Hans Verkuil <[e-mail]>  
> 
> xs4ll.nl -> xs4all.nl
> 
> (it's probably my typo, but please fix this in the final version)

Yes, I copied and pasted it. Will fix at the next version.

> 
> > Reviewed-by: Ricardo Ribalda <[e-mail]>
> > ---
> >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> >  MAINTAINERS                                   |   1 +
> >  2 files changed, 163 insertions(+), 46 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > index ffc712a5f632..dc764163cf1c 100644
> > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> >  Both media userspace and Kernel APIs are documented and the documentation
> >  must be kept in sync with the API changes. It means that all patches that
> >  add new features to the subsystem must also bring changes to the
> > -corresponding API files.
> > +corresponding API documentation files.
> >  
> > -Due to the size and wide scope of the media subsystem, media's
> > -maintainership model is to have sub-maintainers that have a broad
> > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > -task to review the patches, providing feedback to users if the patches are
> > +Due to the size and wide scope of the media subsystem, the media's
> > +maintainership model is to have committers that have a broad knowledge of
> > +a specific aspect of the subsystem. It is the committers' task to
> > +review the patches, providing feedback to users if the patches are
> >  following the subsystem rules and are properly using the media kernel and
> >  userspace APIs.
> >  
> > -Patches for the media subsystem must be sent to the media mailing list
> > -at [e-mail] as plain text only e-mail. Emails with
> > -HTML will be automatically rejected by the mail server. It could be wise
> > -to also copy the sub-maintainer(s).
> > +Media committers
> > +----------------
> > +
> > +In the media subsystem, there are experienced developers who can push
> > +patches directly to the development tree. These developers are called
> > +Media committers and are divided into the following categories:
> > +
> > +- Committers:
> > +    contributors for one or more drivers within the media subsystem.
> > +    They can push changes to the tree that do not affect the core or ABI.
> > +
> > +- Core committers:
> > +    responsible for part of the media core. They are typically
> > +    responsible for one or more drivers within the media subsystem, but, besides
> > +    that, they can also merge patches that change the code common to multiple
> > +    drivers, including the kernel internal API.
> > +
> > +- Subsystem maintainers:
> > +    responsible for the subsystem as a whole, with access to the
> > +    entire subsystem.
> > +
> > +    Only subsystem maintainers can push changes that affect the userspace
> > +    API/ABI.
> > +
> > +All media committers shall explicitly agree with the Kernel development process
> > +as described at Documentation/process/index.rst and to the Kernel
> > +development rules inside the Kernel documentation, including its code of
> > +conduct.
> > +
> > +Media development tree
> > +----------------------
> > +
> > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > +instance where we track patches though their lifetime.
> > +
> > +The main tree used by media developers is at:
> > +
> > +https://git.linuxtv.org/media.git/
> > +
> > +.. _Media development workflow:
> > +
> > +Media development workflow
> > +++++++++++++++++++++++++++
> > +
> > +All changes for the media subsystem must be sent first as e-mails to the
> > +media mailing list, following the process documented at
> > +Documentation/process/index.rst.
> > +
> > +It means that patches shall be submitted as plain text only via e-mail to
> > [e-mail]. While subscription is not mandatory, you
> > +can find details about how to subscribe to it and to see its archives at:
> > +
> > +  https://subspace.kernel.org/vger.kernel.org.html
> > +
> > +Emails with HTML will be automatically rejected by the mail server.
> > +
> > +It could be wise to also copy the media committer(s). You should use
> > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > +Please always copy driver's authors and maintainers.
> > +
> > +Such patches need to be based against a public branch or tag as follows:
> > +
> > +1. Patches for new features need to be based at the ``next`` branch of
> > +   media.git tree;
> > +
> > +2. Fixes against an already released kernel should preferably be against
> > +   the latest released Kernel. If they require a previously-applied
> > +   change at media.git tree, they need to be against its ``fixes`` branch.  
> 
> What is an "released kernel"? Does an -rcX kernel qualify?
> > +
> > +3. Fixes for issues not present at the latest released kernel shall
> > +   be either against a -rc kernel for an upcoming release or
> > +   against the ``fixes`` branch of the media.git tree.  
> 
> 2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
> at some point.

Some Kernel documents use "mainline release" to refer to Kernel final
releases.

One option to make it clearer would be to add a link to:
	https://www.kernel.org/category/releases.html

And then use the terms:
	- "Mainline release" for v6.10, v6.11, v6.12...
	- "Prepatch release" for -rc1, -rc2, -rc3...

What 2 and means are:

	1. Features for the next mainline release:

	   - baseline shall be media.git ``next`` branch;

	2. Fixes bugs at mainline releases:

	   - baseline shall be the latest mainline release or media.git ``fixes``  
	     if changes depend on a fix already merged;

	3. Fixes for the next mainline release:

	   - baseline shall be a prepatch release or media.git ``fixes``  
	     if changes depend on a fix already merged;

	.. Note:

	   See https://www.kernel.org/category/releases.html for an overview
	   about Kernel release types.
  
Would the above text work best you? If not please propose another
text.

In practice, we'll likely accept other baselines for fixes as
well, for practical reasons. From my side, I don't have any issues
if I need to rebase a tested bugfix patch produced against an
older Kernel send by an occasional contributor, if it only has 
trivial merge conflicts.

> > +Patches with fixes shall have:
> > +
> > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > +- when applicable, a ``Cc: [e-mail]``.
> > +
> > +Patches that were fixing bugs publicly reported by someone at the
> > [e-mail] mailing list shall have:
> > +
> > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > +
> > +Patches that change API shall update documentation accordingly at the
> > +same patch series.
> > +
> > +See Documentation/process/index.rst for more details about e-mail submission.
> > +
> > +Once a patch is submitted, it may follow either one of the following
> > +workflows:
> > +
> > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > +
> > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
> > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > +                +---------+   +-------+   +-----------------------+
> > +
> > +   For this workflow, pull requests can be generated by a committer,
> > +   a previous committer, subsystem maintainers or by a trusted long-time
> > +   contributor. If you are not in such group, please don't submit
> > +   pull requests, as they will not be processed.
> > +
> > +b. Committers' workflow: patches are handled by media committers::
> > +
> > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > +                +---------+   +--------------------+   +-----------+
> > +
> > +On both workflows, all patches shall be properly reviewed at
> > [e-mail] before being merged at media-committers.git.
> > +
> > +When patches are picked by patchwork and when merged at media-committers,
> > +CI bots will check for errors and may provide e-mail feedback about
> > +patch problems. When this happens, the patch submitter must fix them, or
> > +explain why the errors are false positives.
> > +
> > +Patches will only be moved to the next stage in those two workflows if they
> > +don't fail on CI or if there are false-positives in the CI reports.
> > +
> > +Failures during e-mail submission
> > ++++++++++++++++++++++++++++++++++
> >  
> >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> >  is submitted, the e-mail will first be accepted by the mailing list
> > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> >  
> >     - https://patchwork.linuxtv.org/project/linux-media/list/
> >  
> > -If it doesn't automatically appear there after a few minutes, then
> > +If it doesn't automatically appear there after some time [2]_, then
> >  probably something went wrong on your submission. Please check if the
> > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > +email is in plain text\ [3]_ only and if your emailer is not mangling
> >  whitespaces before complaining or submitting them again.
> >  
> > -You can check if the mailing list server accepted your patch, by looking at:
> > +To troubleshoot problems, you should first check if the mailing list
> > +server has accepted your patch, by looking at:
> >  
> >     - https://lore.kernel.org/linux-media/
> >  
> > -.. [2] If your email contains HTML, the mailing list server will simply
> > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > +mangled the patch. Patchwork internally has a logic that checks if the  
> 
> a logic -> logic
> 
> > +received e-mail contain a valid patch. Any whitespace and new line  
> 
> contain -> contains
> 
> > +breakages mangling the patch won't be recognized by patchwork, thus such
> > +patch will be rejected.
> > +
> > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > +       the e-mail server may be busy, so it may take up to a few hours
> > +       for a patch to be picked by patchwork.
> > +
> > +.. [3] If your email contains HTML, the mailing list server will simply
> >         drop it, without any further notice.
> >  
> > +.. _media-developers-gpg:
> >  
> > -Media maintainers
> > -+++++++++++++++++
> > +Authentication for pull and merge requests
> > +++++++++++++++++++++++++++++++++++++++++++
> >  
> > -At the media subsystem, we have a group of senior developers that
> > -are responsible for doing the code reviews at the drivers (also known as
> > -sub-maintainers), and another senior developer responsible for the
> > -subsystem as a whole. For core changes, whenever possible, multiple
> > -media maintainers do the review.
> > +The authenticity of developers submitting pull requests and merge requests
> > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.  
> 
> sign -> signing
> 
> Hmm, merge requests through gitlab are not signed. So this isn't correct, I
> think.

No, but they are authenticated based on gitlab's user ID. The authentication
we'll have is when the new committer sends us an e-mail providing the
gitlab's user ID.

See patch 3, as it contains some changes aiming to better explain it. Once
it get the same people reviewing that also reviewed 1 and 2, I'll fold it.

> 
> >  
> > -The media maintainers that work on specific areas of the subsystem are:
> > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> >  
> > -- Remote Controllers (infrared):
> > -    Sean Young <[e-mail]>
> > +For more details about PGP sign, please read
> > +Documentation/process/maintainer-pgp-guide.rst.
> >  
> > -- HDMI CEC:
> > -    Hans Verkuil <[e-mail]>
> > +Subsystem maintainers
> > +---------------------
> >  
> > -- Media controller drivers:
> > -    Laurent Pinchart <[e-mail]>
> > -
> > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > -    Sakari Ailus <[e-mail]>
> > -
> > -- V4L2 drivers and core V4L2 frameworks:
> > -    Hans Verkuil <[e-mail]>
> > -
> > -The subsystem maintainer is:
> > -  Mauro Carvalho Chehab <[e-mail]>
> > -
> > -Media maintainers may delegate a patch to other media maintainers as needed.
> > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > -responsible for reviewing a patch.
> > +The subsystem maintainers are:
> > +  - Mauro Carvalho Chehab <[e-mail]> and
> > +  - Hans Verkuil <[e-mail]>
> >  
> >  Submit Checklist Addendum
> >  -------------------------
> > @@ -108,17 +225,14 @@ implementing the media APIs:
> >  ====================	=======================================================
> >  Type			Tool
> >  ====================	=======================================================
> > -V4L2 drivers\ [3]_	``v4l2-compliance``
> > +V4L2 drivers\ [4]_	``v4l2-compliance``
> >  V4L2 virtual drivers	``contrib/test/test-media``
> >  CEC drivers		``cec-compliance``
> >  ====================	=======================================================
> >  
> > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside  
> 
> The ``v4l2-compliance`` utility
> 
> (add 'utility')

Ok.

> 
> >         V4L2 drivers.
> >  
> > -Other compilance tools are under development to check other parts of the
> > -subsystem.
> > -
> >  Those tests need to pass before the patches go upstream.
> >  
> >  Also, please notice that we build the Kernel with::
> > @@ -134,6 +248,8 @@ Where the check script is::
> >  Be sure to not introduce new warnings on your patches without a
> >  very good reason.
> >  
> > +Please see `Media development workflow`_ for e-mail submission rules.
> > +
> >  Style Cleanup Patches
> >  +++++++++++++++++++++
> >  
> > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> >  Please notice that the media subsystem is a high traffic one, so it
> >  could take a while for us to be able to review your patches. Feel free
> >  to ping if you don't get a feedback in a couple of weeks or to ask
> > -other developers to publicly add Reviewed-by and, more importantly,
> > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> >  ``Tested-by:`` tags.
> >  
> >  Please note that we expect a detailed description for ``Tested-by:``,
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1e930c7a58b1..c77f56a2e695 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> >  M:	Mauro Carvalho Chehab <[e-mail]>
> >  L:	[e-mail]
> >  S:	Maintained
> > +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
> >  W:	https://linuxtv.org
> >  Q:	http://patchwork.kernel.org/project/linux-media/list/  
> 
> Shouldn't this point to our patchwork instance instead?

Good catch! I'll update it.

> 
> >  T:	git git://linuxtv.org/media.git  
> 
> Regards,
> 
> 	Hans



Thanks,
Mauro

Hans Verkuil Dec. 2, 2024, 2:49 p.m. UTC | #5

On 02/12/2024 14:54, Mauro Carvalho Chehab wrote:
> Em Mon, 2 Dec 2024 14:17:48 +0100
> Hans Verkuil <[e-mail]> escreveu:
> 
>> Hi Mauro,
>>
>> On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
>>> As the media subsystem will experiment with a multi-committers model,
>>> update the Maintainer's entry profile to the new rules.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
>>> Signed-off-by: Hans Verkuil <[e-mail]>  
>>
>> xs4ll.nl -> xs4all.nl
>>
>> (it's probably my typo, but please fix this in the final version)
> 
> Yes, I copied and pasted it. Will fix at the next version.
> 
>>
>>> Reviewed-by: Ricardo Ribalda <[e-mail]>
>>> ---
>>>  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
>>>  MAINTAINERS                                   |   1 +
>>>  2 files changed, 163 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
>>> index ffc712a5f632..dc764163cf1c 100644
>>> --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
>>> +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
>>> @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
>>>  Both media userspace and Kernel APIs are documented and the documentation
>>>  must be kept in sync with the API changes. It means that all patches that
>>>  add new features to the subsystem must also bring changes to the
>>> -corresponding API files.
>>> +corresponding API documentation files.
>>>  
>>> -Due to the size and wide scope of the media subsystem, media's
>>> -maintainership model is to have sub-maintainers that have a broad
>>> -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
>>> -task to review the patches, providing feedback to users if the patches are
>>> +Due to the size and wide scope of the media subsystem, the media's
>>> +maintainership model is to have committers that have a broad knowledge of
>>> +a specific aspect of the subsystem. It is the committers' task to
>>> +review the patches, providing feedback to users if the patches are
>>>  following the subsystem rules and are properly using the media kernel and
>>>  userspace APIs.
>>>  
>>> -Patches for the media subsystem must be sent to the media mailing list
>>> -at [e-mail] as plain text only e-mail. Emails with
>>> -HTML will be automatically rejected by the mail server. It could be wise
>>> -to also copy the sub-maintainer(s).
>>> +Media committers
>>> +----------------
>>> +
>>> +In the media subsystem, there are experienced developers who can push
>>> +patches directly to the development tree. These developers are called
>>> +Media committers and are divided into the following categories:
>>> +
>>> +- Committers:
>>> +    contributors for one or more drivers within the media subsystem.
>>> +    They can push changes to the tree that do not affect the core or ABI.
>>> +
>>> +- Core committers:
>>> +    responsible for part of the media core. They are typically
>>> +    responsible for one or more drivers within the media subsystem, but, besides
>>> +    that, they can also merge patches that change the code common to multiple
>>> +    drivers, including the kernel internal API.
>>> +
>>> +- Subsystem maintainers:
>>> +    responsible for the subsystem as a whole, with access to the
>>> +    entire subsystem.
>>> +
>>> +    Only subsystem maintainers can push changes that affect the userspace
>>> +    API/ABI.
>>> +
>>> +All media committers shall explicitly agree with the Kernel development process
>>> +as described at Documentation/process/index.rst and to the Kernel
>>> +development rules inside the Kernel documentation, including its code of
>>> +conduct.
>>> +
>>> +Media development tree
>>> +----------------------
>>> +
>>> +The main development tree used by the media subsystem is hosted at LinuxTV.org,
>>> +where we also maintain news about the subsystem, wiki pages and a patchwork
>>> +instance where we track patches though their lifetime.
>>> +
>>> +The main tree used by media developers is at:
>>> +
>>> +https://git.linuxtv.org/media.git/
>>> +
>>> +.. _Media development workflow:
>>> +
>>> +Media development workflow
>>> +++++++++++++++++++++++++++
>>> +
>>> +All changes for the media subsystem must be sent first as e-mails to the
>>> +media mailing list, following the process documented at
>>> +Documentation/process/index.rst.
>>> +
>>> +It means that patches shall be submitted as plain text only via e-mail to
>>> [e-mail]. While subscription is not mandatory, you
>>> +can find details about how to subscribe to it and to see its archives at:
>>> +
>>> +  https://subspace.kernel.org/vger.kernel.org.html
>>> +
>>> +Emails with HTML will be automatically rejected by the mail server.
>>> +
>>> +It could be wise to also copy the media committer(s). You should use
>>> +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
>>> +Please always copy driver's authors and maintainers.
>>> +
>>> +Such patches need to be based against a public branch or tag as follows:
>>> +
>>> +1. Patches for new features need to be based at the ``next`` branch of
>>> +   media.git tree;
>>> +
>>> +2. Fixes against an already released kernel should preferably be against
>>> +   the latest released Kernel. If they require a previously-applied
>>> +   change at media.git tree, they need to be against its ``fixes`` branch.  
>>
>> What is an "released kernel"? Does an -rcX kernel qualify?
>>> +
>>> +3. Fixes for issues not present at the latest released kernel shall
>>> +   be either against a -rc kernel for an upcoming release or
>>> +   against the ``fixes`` branch of the media.git tree.  
>>
>> 2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
>> at some point.
> 
> Some Kernel documents use "mainline release" to refer to Kernel final
> releases.
> 
> One option to make it clearer would be to add a link to:
> 	https://www.kernel.org/category/releases.html
> 
> And then use the terms:
> 	- "Mainline release" for v6.10, v6.11, v6.12...
> 	- "Prepatch release" for -rc1, -rc2, -rc3...
> 
> What 2 and means are:
> 
> 	1. Features for the next mainline release:
> 
> 	   - baseline shall be media.git ``next`` branch;
> 
> 	2. Fixes bugs at mainline releases:
> 
> 	   - baseline shall be the latest mainline release or media.git ``fixes``  
> 	     if changes depend on a fix already merged;
> 
> 	3. Fixes for the next mainline release:
> 
> 	   - baseline shall be a prepatch release or media.git ``fixes``  
> 	     if changes depend on a fix already merged;

If it is for the next mainline release, then I would also be fine with 'next'
as baseline. That avoids having to modify the patch if it conflicts with
new work that went into next. Having to base a fix on the 'fixes' branch is
rare in this case. You typically only need to do that in case 2.

> 
> 	.. Note:
> 
> 	   See https://www.kernel.org/category/releases.html for an overview
> 	   about Kernel release types.
>   
> Would the above text work best you? If not please propose another
> text.

Yes, that's much better.

> 
> In practice, we'll likely accept other baselines for fixes as
> well, for practical reasons. From my side, I don't have any issues
> if I need to rebase a tested bugfix patch produced against an
> older Kernel send by an occasional contributor, if it only has 
> trivial merge conflicts.

Sure, these three points are really about our preference since following
that reduces the chance of merge conflicts.

We can say something like this:

"To minimize the chance of merge conflicts for your patch series, we recommend
that you use the following baseline for your patch series:"

> 
>>> +Patches with fixes shall have:
>>> +
>>> +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
>>> +- when applicable, a ``Cc: [e-mail]``.
>>> +
>>> +Patches that were fixing bugs publicly reported by someone at the
>>> [e-mail] mailing list shall have:
>>> +
>>> +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
>>> +
>>> +Patches that change API shall update documentation accordingly at the
>>> +same patch series.
>>> +
>>> +See Documentation/process/index.rst for more details about e-mail submission.
>>> +
>>> +Once a patch is submitted, it may follow either one of the following
>>> +workflows:
>>> +
>>> +a. Pull request workflow: patches are handled by subsystem maintainers::
>>> +
>>> +     +------+   +---------+   +-------+   +-----------------------+   +---------+
>>> +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
>>> +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
>>> +                +---------+   +-------+   +-----------------------+
>>> +
>>> +   For this workflow, pull requests can be generated by a committer,
>>> +   a previous committer, subsystem maintainers or by a trusted long-time
>>> +   contributor. If you are not in such group, please don't submit
>>> +   pull requests, as they will not be processed.
>>> +
>>> +b. Committers' workflow: patches are handled by media committers::
>>> +
>>> +     +------+   +---------+   +--------------------+   +-----------+   +---------+
>>> +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
>>> +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
>>> +                +---------+   +--------------------+   +-----------+
>>> +
>>> +On both workflows, all patches shall be properly reviewed at
>>> [e-mail] before being merged at media-committers.git.
>>> +
>>> +When patches are picked by patchwork and when merged at media-committers,
>>> +CI bots will check for errors and may provide e-mail feedback about
>>> +patch problems. When this happens, the patch submitter must fix them, or
>>> +explain why the errors are false positives.
>>> +
>>> +Patches will only be moved to the next stage in those two workflows if they
>>> +don't fail on CI or if there are false-positives in the CI reports.
>>> +
>>> +Failures during e-mail submission
>>> ++++++++++++++++++++++++++++++++++
>>>  
>>>  Media's workflow is heavily based on Patchwork, meaning that, once a patch
>>>  is submitted, the e-mail will first be accepted by the mailing list
>>> @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
>>>  
>>>     - https://patchwork.linuxtv.org/project/linux-media/list/
>>>  
>>> -If it doesn't automatically appear there after a few minutes, then
>>> +If it doesn't automatically appear there after some time [2]_, then
>>>  probably something went wrong on your submission. Please check if the
>>> -email is in plain text\ [2]_ only and if your emailer is not mangling
>>> +email is in plain text\ [3]_ only and if your emailer is not mangling
>>>  whitespaces before complaining or submitting them again.
>>>  
>>> -You can check if the mailing list server accepted your patch, by looking at:
>>> +To troubleshoot problems, you should first check if the mailing list
>>> +server has accepted your patch, by looking at:
>>>  
>>>     - https://lore.kernel.org/linux-media/
>>>  
>>> -.. [2] If your email contains HTML, the mailing list server will simply
>>> +If the patch is there and not at patchwork, it is likely that your e-mailer
>>> +mangled the patch. Patchwork internally has a logic that checks if the  
>>
>> a logic -> logic
>>
>>> +received e-mail contain a valid patch. Any whitespace and new line  
>>
>> contain -> contains
>>
>>> +breakages mangling the patch won't be recognized by patchwork, thus such
>>> +patch will be rejected.
>>> +
>>> +.. [2] It usually takes a few minutes for the patch to arrive, but
>>> +       the e-mail server may be busy, so it may take up to a few hours
>>> +       for a patch to be picked by patchwork.
>>> +
>>> +.. [3] If your email contains HTML, the mailing list server will simply
>>>         drop it, without any further notice.
>>>  
>>> +.. _media-developers-gpg:
>>>  
>>> -Media maintainers
>>> -+++++++++++++++++
>>> +Authentication for pull and merge requests
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>  
>>> -At the media subsystem, we have a group of senior developers that
>>> -are responsible for doing the code reviews at the drivers (also known as
>>> -sub-maintainers), and another senior developer responsible for the
>>> -subsystem as a whole. For core changes, whenever possible, multiple
>>> -media maintainers do the review.
>>> +The authenticity of developers submitting pull requests and merge requests
>>> +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.  
>>
>> sign -> signing
>>
>> Hmm, merge requests through gitlab are not signed. So this isn't correct, I
>> think.
> 
> No, but they are authenticated based on gitlab's user ID. The authentication
> we'll have is when the new committer sends us an e-mail providing the
> gitlab's user ID.
> 
> See patch 3, as it contains some changes aiming to better explain it. Once
> it get the same people reviewing that also reviewed 1 and 2, I'll fold it.
> 
>>
>>>  
>>> -The media maintainers that work on specific areas of the subsystem are:
>>> +With the pull request workflow, pull requests shall use a PGP-signed tag.
>>>  
>>> -- Remote Controllers (infrared):
>>> -    Sean Young <[e-mail]>
>>> +For more details about PGP sign, please read
>>> +Documentation/process/maintainer-pgp-guide.rst.
>>>  
>>> -- HDMI CEC:
>>> -    Hans Verkuil <[e-mail]>
>>> +Subsystem maintainers
>>> +---------------------
>>>  
>>> -- Media controller drivers:
>>> -    Laurent Pinchart <[e-mail]>
>>> -
>>> -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
>>> -    Sakari Ailus <[e-mail]>
>>> -
>>> -- V4L2 drivers and core V4L2 frameworks:
>>> -    Hans Verkuil <[e-mail]>
>>> -
>>> -The subsystem maintainer is:
>>> -  Mauro Carvalho Chehab <[e-mail]>
>>> -
>>> -Media maintainers may delegate a patch to other media maintainers as needed.
>>> -On such case, checkpatch's ``delegate`` field indicates who's currently
>>> -responsible for reviewing a patch.
>>> +The subsystem maintainers are:
>>> +  - Mauro Carvalho Chehab <[e-mail]> and
>>> +  - Hans Verkuil <[e-mail]>
>>>  
>>>  Submit Checklist Addendum
>>>  -------------------------
>>> @@ -108,17 +225,14 @@ implementing the media APIs:
>>>  ====================	=======================================================
>>>  Type			Tool
>>>  ====================	=======================================================
>>> -V4L2 drivers\ [3]_	``v4l2-compliance``
>>> +V4L2 drivers\ [4]_	``v4l2-compliance``
>>>  V4L2 virtual drivers	``contrib/test/test-media``
>>>  CEC drivers		``cec-compliance``
>>>  ====================	=======================================================
>>>  
>>> -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
>>> +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside  
>>
>> The ``v4l2-compliance`` utility
>>
>> (add 'utility')
> 
> Ok.
> 
>>
>>>         V4L2 drivers.
>>>  
>>> -Other compilance tools are under development to check other parts of the
>>> -subsystem.
>>> -
>>>  Those tests need to pass before the patches go upstream.
>>>  
>>>  Also, please notice that we build the Kernel with::
>>> @@ -134,6 +248,8 @@ Where the check script is::
>>>  Be sure to not introduce new warnings on your patches without a
>>>  very good reason.
>>>  
>>> +Please see `Media development workflow`_ for e-mail submission rules.
>>> +
>>>  Style Cleanup Patches
>>>  +++++++++++++++++++++
>>>  
>>> @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
>>>  Please notice that the media subsystem is a high traffic one, so it
>>>  could take a while for us to be able to review your patches. Feel free
>>>  to ping if you don't get a feedback in a couple of weeks or to ask
>>> -other developers to publicly add Reviewed-by and, more importantly,
>>> +other developers to publicly add ``Reviewed-by:`` and, more importantly,
>>>  ``Tested-by:`` tags.
>>>  
>>>  Please note that we expect a detailed description for ``Tested-by:``,
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 1e930c7a58b1..c77f56a2e695 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
>>>  M:	Mauro Carvalho Chehab <[e-mail]>
>>>  L:	[e-mail]
>>>  S:	Maintained
>>> +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
>>>  W:	https://linuxtv.org
>>>  Q:	http://patchwork.kernel.org/project/linux-media/list/  
>>
>> Shouldn't this point to our patchwork instance instead?
> 
> Good catch! I'll update it.
> 
>>
>>>  T:	git git://linuxtv.org/media.git  

Regards,

	Hans

Ricardo Ribalda Dec. 2, 2024, 8:48 p.m. UTC | #6

Minor nits that can be ignored

On Mon, 2 Dec 2024 at 14:54, Mauro Carvalho Chehab
<[e-mail]> wrote:
>
> Em Mon, 2 Dec 2024 14:17:48 +0100
> Hans Verkuil <[e-mail]> escreveu:
>
> > Hi Mauro,
> >
> > On 02/12/2024 10:26, Mauro Carvalho Chehab wrote:
> > > As the media subsystem will experiment with a multi-committers model,
> > > update the Maintainer's entry profile to the new rules.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
> > > Signed-off-by: Hans Verkuil <[e-mail]>
> >
> > xs4ll.nl -> xs4all.nl
> >
> > (it's probably my typo, but please fix this in the final version)
>
> Yes, I copied and pasted it. Will fix at the next version.
>
> >
> > > Reviewed-by: Ricardo Ribalda <[e-mail]>
> > > ---
> > >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> > >  MAINTAINERS                                   |   1 +
> > >  2 files changed, 163 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > index ffc712a5f632..dc764163cf1c 100644
> > > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> > >  Both media userspace and Kernel APIs are documented and the documentation
> > >  must be kept in sync with the API changes. It means that all patches that
> > >  add new features to the subsystem must also bring changes to the
> > > -corresponding API files.
> > > +corresponding API documentation files.
> > >
> > > -Due to the size and wide scope of the media subsystem, media's
> > > -maintainership model is to have sub-maintainers that have a broad
> > > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > > -task to review the patches, providing feedback to users if the patches are
> > > +Due to the size and wide scope of the media subsystem, the media's
> > > +maintainership model is to have committers that have a broad knowledge of
> > > +a specific aspect of the subsystem. It is the committers' task to
> > > +review the patches, providing feedback to users if the patches are
> > >  following the subsystem rules and are properly using the media kernel and
> > >  userspace APIs.
> > >
> > > -Patches for the media subsystem must be sent to the media mailing list
> > > -at [e-mail] as plain text only e-mail. Emails with
> > > -HTML will be automatically rejected by the mail server. It could be wise
> > > -to also copy the sub-maintainer(s).
> > > +Media committers
> > > +----------------
> > > +
> > > +In the media subsystem, there are experienced developers who can push
> > > +patches directly to the development tree. These developers are called
> > > +Media committers and are divided into the following categories:
> > > +
> > > +- Committers:
> > > +    contributors for one or more drivers within the media subsystem.
> > > +    They can push changes to the tree that do not affect the core or ABI.
> > > +
> > > +- Core committers:
> > > +    responsible for part of the media core. They are typically
> > > +    responsible for one or more drivers within the media subsystem, but, besides
> > > +    that, they can also merge patches that change the code common to multiple
> > > +    drivers, including the kernel internal API.
> > > +
> > > +- Subsystem maintainers:
> > > +    responsible for the subsystem as a whole, with access to the
> > > +    entire subsystem.
> > > +
> > > +    Only subsystem maintainers can push changes that affect the userspace
> > > +    API/ABI.
> > > +
> > > +All media committers shall explicitly agree with the Kernel development process
> > > +as described at Documentation/process/index.rst and to the Kernel
> > > +development rules inside the Kernel documentation, including its code of
> > > +conduct.
> > > +
> > > +Media development tree
> > > +----------------------
> > > +
> > > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > > +instance where we track patches though their lifetime.
> > > +
> > > +The main tree used by media developers is at:
> > > +
> > > +https://git.linuxtv.org/media.git/
> > > +
> > > +.. _Media development workflow:
> > > +
> > > +Media development workflow
> > > +++++++++++++++++++++++++++
> > > +
> > > +All changes for the media subsystem must be sent first as e-mails to the
> > > +media mailing list, following the process documented at
> > > +Documentation/process/index.rst.
> > > +
> > > +It means that patches shall be submitted as plain text only via e-mail to
> > > [e-mail]. While subscription is not mandatory, you
> > > +can find details about how to subscribe to it and to see its archives at:
> > > +
> > > +  https://subspace.kernel.org/vger.kernel.org.html
> > > +
> > > +Emails with HTML will be automatically rejected by the mail server.
> > > +
> > > +It could be wise to also copy the media committer(s). You should use
> > > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > > +Please always copy driver's authors and maintainers.
> > > +
> > > +Such patches need to be based against a public branch or tag as follows:
> > > +
> > > +1. Patches for new features need to be based at the ``next`` branch of
> > > +   media.git tree;
> > > +
> > > +2. Fixes against an already released kernel should preferably be against
> > > +   the latest released Kernel. If they require a previously-applied
> > > +   change at media.git tree, they need to be against its ``fixes`` branch.
> >
> > What is an "released kernel"? Does an -rcX kernel qualify?
> > > +
> > > +3. Fixes for issues not present at the latest released kernel shall
> > > +   be either against a -rc kernel for an upcoming release or
> > > +   against the ``fixes`` branch of the media.git tree.
> >
> > 2 and 3 remain vague, IMO. Not a blocker, but I think this needs more work
> > at some point.
>
> Some Kernel documents use "mainline release" to refer to Kernel final
> releases.
>
> One option to make it clearer would be to add a link to:
>         https://www.kernel.org/category/releases.html
>
> And then use the terms:
>         - "Mainline release" for v6.10, v6.11, v6.12...
>         - "Prepatch release" for -rc1, -rc2, -rc3...
>
> What 2 and means are:
>
>         1. Features for the next mainline release:
>
>            - baseline shall be media.git ``next`` branch;
>
>         2. Fixes bugs at mainline releases:
         2. Bug fixes for the current mainline release

>
>            - baseline shall be the latest mainline release or media.git ``fixes``
>              if changes depend on a fix already merged;
>
>         3. Fixes for the next mainline release:
         3. Bug fixes for the next mainline release
>
>            - baseline shall be a prepatch release or media.git ``fixes``
            - baseline shall be a prepatch release (rcX) or media.git ``fixes``
>              if changes depend on a fix already merged;
>
>         .. Note:
>
>            See https://www.kernel.org/category/releases.html for an overview
>            about Kernel release types.
>
> Would the above text work best you? If not please propose another
> text.
>
> In practice, we'll likely accept other baselines for fixes as
> well, for practical reasons. From my side, I don't have any issues
> if I need to rebase a tested bugfix patch produced against an
> older Kernel send by an occasional contributor, if it only has
> trivial merge conflicts.
>
> > > +Patches with fixes shall have:
> > > +
> > > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > > +- when applicable, a ``Cc: [e-mail]``.
> > > +
> > > +Patches that were fixing bugs publicly reported by someone at the
> > > [e-mail] mailing list shall have:
> > > +
> > > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > > +
> > > +Patches that change API shall update documentation accordingly at the
> > > +same patch series.
> > > +
> > > +See Documentation/process/index.rst for more details about e-mail submission.
> > > +
> > > +Once a patch is submitted, it may follow either one of the following
> > > +workflows:
> > > +
> > > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > > +
> > > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
> > > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > > +                +---------+   +-------+   +-----------------------+
> > > +
> > > +   For this workflow, pull requests can be generated by a committer,
> > > +   a previous committer, subsystem maintainers or by a trusted long-time
> > > +   contributor. If you are not in such group, please don't submit
> > > +   pull requests, as they will not be processed.
> > > +
> > > +b. Committers' workflow: patches are handled by media committers::
> > > +
> > > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > > +                +---------+   +--------------------+   +-----------+
> > > +
> > > +On both workflows, all patches shall be properly reviewed at
> > > [e-mail] before being merged at media-committers.git.
> > > +
> > > +When patches are picked by patchwork and when merged at media-committers,
> > > +CI bots will check for errors and may provide e-mail feedback about
> > > +patch problems. When this happens, the patch submitter must fix them, or
> > > +explain why the errors are false positives.
> > > +
> > > +Patches will only be moved to the next stage in those two workflows if they
> > > +don't fail on CI or if there are false-positives in the CI reports.
> > > +
> > > +Failures during e-mail submission
> > > ++++++++++++++++++++++++++++++++++
> > >
> > >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> > >  is submitted, the e-mail will first be accepted by the mailing list
> > > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> > >
> > >     - https://patchwork.linuxtv.org/project/linux-media/list/
> > >
> > > -If it doesn't automatically appear there after a few minutes, then
> > > +If it doesn't automatically appear there after some time [2]_, then
> > >  probably something went wrong on your submission. Please check if the
> > > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > > +email is in plain text\ [3]_ only and if your emailer is not mangling
> > >  whitespaces before complaining or submitting them again.
> > >
> > > -You can check if the mailing list server accepted your patch, by looking at:
> > > +To troubleshoot problems, you should first check if the mailing list
> > > +server has accepted your patch, by looking at:
> > >
> > >     - https://lore.kernel.org/linux-media/
> > >
> > > -.. [2] If your email contains HTML, the mailing list server will simply
> > > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > > +mangled the patch. Patchwork internally has a logic that checks if the
> >
> > a logic -> logic
> >
> > > +received e-mail contain a valid patch. Any whitespace and new line
> >
> > contain -> contains
> >
> > > +breakages mangling the patch won't be recognized by patchwork, thus such
> > > +patch will be rejected.
> > > +
> > > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > > +       the e-mail server may be busy, so it may take up to a few hours
> > > +       for a patch to be picked by patchwork.
> > > +
> > > +.. [3] If your email contains HTML, the mailing list server will simply
> > >         drop it, without any further notice.
> > >
> > > +.. _media-developers-gpg:
> > >
> > > -Media maintainers
> > > -+++++++++++++++++
> > > +Authentication for pull and merge requests
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >
> > > -At the media subsystem, we have a group of senior developers that
> > > -are responsible for doing the code reviews at the drivers (also known as
> > > -sub-maintainers), and another senior developer responsible for the
> > > -subsystem as a whole. For core changes, whenever possible, multiple
> > > -media maintainers do the review.
> > > +The authenticity of developers submitting pull requests and merge requests
> > > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
> >
> > sign -> signing
> >
> > Hmm, merge requests through gitlab are not signed. So this isn't correct, I
> > think.
>
> No, but they are authenticated based on gitlab's user ID. The authentication
> we'll have is when the new committer sends us an e-mail providing the
> gitlab's user ID.
>
> See patch 3, as it contains some changes aiming to better explain it. Once
> it get the same people reviewing that also reviewed 1 and 2, I'll fold it.
>
> >
> > >
> > > -The media maintainers that work on specific areas of the subsystem are:
> > > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> > >
> > > -- Remote Controllers (infrared):
> > > -    Sean Young <[e-mail]>
> > > +For more details about PGP sign, please read
> > > +Documentation/process/maintainer-pgp-guide.rst.
> > >
> > > -- HDMI CEC:
> > > -    Hans Verkuil <[e-mail]>
> > > +Subsystem maintainers
> > > +---------------------
> > >
> > > -- Media controller drivers:
> > > -    Laurent Pinchart <[e-mail]>
> > > -
> > > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > > -    Sakari Ailus <[e-mail]>
> > > -
> > > -- V4L2 drivers and core V4L2 frameworks:
> > > -    Hans Verkuil <[e-mail]>
> > > -
> > > -The subsystem maintainer is:
> > > -  Mauro Carvalho Chehab <[e-mail]>
> > > -
> > > -Media maintainers may delegate a patch to other media maintainers as needed.
> > > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > > -responsible for reviewing a patch.
> > > +The subsystem maintainers are:
> > > +  - Mauro Carvalho Chehab <[e-mail]> and
> > > +  - Hans Verkuil <[e-mail]>
> > >
> > >  Submit Checklist Addendum
> > >  -------------------------
> > > @@ -108,17 +225,14 @@ implementing the media APIs:
> > >  ====================       =======================================================
> > >  Type                       Tool
> > >  ====================       =======================================================
> > > -V4L2 drivers\ [3]_ ``v4l2-compliance``
> > > +V4L2 drivers\ [4]_ ``v4l2-compliance``
> > >  V4L2 virtual drivers       ``contrib/test/test-media``
> > >  CEC drivers                ``cec-compliance``
> > >  ====================       =======================================================
> > >
> > > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
> >
> > The ``v4l2-compliance`` utility
> >
> > (add 'utility')
>
> Ok.
>
> >
> > >         V4L2 drivers.
> > >
> > > -Other compilance tools are under development to check other parts of the
> > > -subsystem.
> > > -
> > >  Those tests need to pass before the patches go upstream.
> > >
> > >  Also, please notice that we build the Kernel with::
> > > @@ -134,6 +248,8 @@ Where the check script is::
> > >  Be sure to not introduce new warnings on your patches without a
> > >  very good reason.
> > >
> > > +Please see `Media development workflow`_ for e-mail submission rules.
> > > +
> > >  Style Cleanup Patches
> > >  +++++++++++++++++++++
> > >
> > > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> > >  Please notice that the media subsystem is a high traffic one, so it
> > >  could take a while for us to be able to review your patches. Feel free
> > >  to ping if you don't get a feedback in a couple of weeks or to ask
> > > -other developers to publicly add Reviewed-by and, more importantly,
> > > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> > >  ``Tested-by:`` tags.
> > >
> > >  Please note that we expect a detailed description for ``Tested-by:``,
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 1e930c7a58b1..c77f56a2e695 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> > >  M: Mauro Carvalho Chehab <[e-mail]>
> > >  L: [e-mail]
> > >  S: Maintained
> > > +P: Documentation/driver-api/media/maintainer-entry-profile.rst
> > >  W: https://linuxtv.org
> > >  Q: http://patchwork.kernel.org/project/linux-media/list/
> >
> > Shouldn't this point to our patchwork instance instead?
>
> Good catch! I'll update it.
>
> >
> > >  T: git git://linuxtv.org/media.git
> >
> > Regards,
> >
> >       Hans
>
>
>
> Thanks,
> Mauro

Sakari Ailus Dec. 3, 2024, 11:52 a.m. UTC | #7

Hi Mauro,

On Mon, Dec 02, 2024 at 02:23:34PM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 2 Dec 2024 12:40:44 +0000
> Sakari Ailus <[e-mail]> escreveu:
> 
> > Hi Mauro,
> > 
> > Thanks for the set.
> > 
> > Looks good overall, please still see my comments below.
> > 
> > On Mon, Dec 02, 2024 at 10:26:19AM +0100, Mauro Carvalho Chehab wrote:
> > > As the media subsystem will experiment with a multi-committers model,
> > > update the Maintainer's entry profile to the new rules.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab <[e-mail]>
> > > Signed-off-by: Hans Verkuil <[e-mail]>
> > > Reviewed-by: Ricardo Ribalda <[e-mail]>
> > > ---
> > >  .../media/maintainer-entry-profile.rst        | 208 ++++++++++++++----
> > >  MAINTAINERS                                   |   1 +
> > >  2 files changed, 163 insertions(+), 46 deletions(-)
> > > 
> > > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > index ffc712a5f632..dc764163cf1c 100644
> > > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > > @@ -27,19 +27,139 @@ It covers, mainly, the contents of those directories:
> > >  Both media userspace and Kernel APIs are documented and the documentation
> > >  must be kept in sync with the API changes. It means that all patches that
> > >  add new features to the subsystem must also bring changes to the
> > > -corresponding API files.
> > > +corresponding API documentation files.
> > >  
> > > -Due to the size and wide scope of the media subsystem, media's
> > > -maintainership model is to have sub-maintainers that have a broad
> > > -knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
> > > -task to review the patches, providing feedback to users if the patches are
> > > +Due to the size and wide scope of the media subsystem, the media's
> > > +maintainership model is to have committers that have a broad knowledge of
> > > +a specific aspect of the subsystem. It is the committers' task to
> > > +review the patches, providing feedback to users if the patches are
> > >  following the subsystem rules and are properly using the media kernel and
> > >  userspace APIs.
> > >  
> > > -Patches for the media subsystem must be sent to the media mailing list
> > > -at [e-mail] as plain text only e-mail. Emails with
> > > -HTML will be automatically rejected by the mail server. It could be wise
> > > -to also copy the sub-maintainer(s).
> > > +Media committers
> > > +----------------
> > > +
> > > +In the media subsystem, there are experienced developers who can push  
> > 
> > The media subsystem is generally understood to comprise of what's under
> > drivers/media, this should be referring to the community instead.
> 
> Do you have a proposal for a different text here?

How about:

In the Linux Media community developers with formal status are classified as
follows.

> 
> > 
> > > +patches directly to the development tree. These developers are called
> > > +Media committers and are divided into the following categories:
> > > +
> > > +- Committers:
> > > +    contributors for one or more drivers within the media subsystem.
> > > +    They can push changes to the tree that do not affect the core or ABI.
> > > +
> > > +- Core committers:
> > > +    responsible for part of the media core. They are typically
> > > +    responsible for one or more drivers within the media subsystem, but, besides
> > > +    that, they can also merge patches that change the code common to multiple
> > > +    drivers, including the kernel internal API.
> > > +
> > > +- Subsystem maintainers:  
> > 
> > s/Subsystem/Media tree/
> 
> Here, we're talking specifically about my hole and Hans, which will
> co-maintain the subsystem with me.. I guess subsystem maintainer is the 
> best to describe it.
> 
> Besides that, we used "media maintainers" in the past with a different
> meaning. Better to not re-use it here.

Seems good, although...

> > 
> > ?
> > > +    responsible for the subsystem as a whole, with access to the

I'd do here:

s/subsystem/Media tree/ or
s/subsystem/Media subsystem/

> > > +    entire subsystem.
> > > +
> > > +    Only subsystem maintainers can push changes that affect the userspace
> > > +    API/ABI.  
> > 
> > This is ambiguous. I think it should intend to say API/ABI changes require
> > approval from Media tree maintainers.
> 
> At the first moment, the idea is to commit them via PRs. So, no such
> commits will be merged by committers/core committers, but yeah, it
> could also be merged directly by a committer if it has our approval.
> 
> In any case, such changes need a consensus from the subsystem maintainers,
> which can just be based on a trust relationship between them with
> regards to certain parts of the subsystem, or via explicit acks.
> 
> Maybe:
> 
> 	API/ABI changes are done via consensus between subsystem
> 	maintainers.
> 
> 	Only subsystem maintainers push changes that affect the userspace
> 	API/ABI. Committers may push directly if they have approvals
> 	from subsystem maintainers.

Seems good to me. These could be in the same paragraph.

> 
> > What constitutes a UAPI change is a topic of discussion on its own. 
> 
> Everything that would break backward compatibility with existing non-kernel
> code are API/ABI changes.
> 
> > Does it
> > require adding a new IOCTL? Taking into use a reserved field? Changing
> > little-used driver behaviour slightly? 
> 
> For all of those, yes: any changes affecting the behavior or fields/nodes
> exported via ioctls and sysfs, including new V4L2 controls are API/ABI changes.
> 
> Some changes at OF are also API/ABI changes, but those can flow via
> committers, provided that OF maintainers added their review or acked-by.
> 
> > Fixing a bug in a driver?
> 
> No, fixing a bug, even if related to ABI/API non-compliance aren't.
> 
> Yet, ABI/API behavior changes at drivers shall not cause regressions.
> 
> I don't think we need to let it clear at the text

Works for me.

> 
> > The first two obviously yes, but the latter two probably not.
> > 
> > Also:
> > 
> > s/Only subsystem maintainers can push/Media tree maintainers' ack is
> > required for/
> > 
> > ?
> > 
> > > +
> > > +All media committers shall explicitly agree with the Kernel development process
> > > +as described at Documentation/process/index.rst and to the Kernel
> > > +development rules inside the Kernel documentation, including its code of
> > > +conduct.
> > > +
> > > +Media development tree
> > > +----------------------
> > > +
> > > +The main development tree used by the media subsystem is hosted at LinuxTV.org,
> > > +where we also maintain news about the subsystem, wiki pages and a patchwork
> > > +instance where we track patches though their lifetime.
> > > +
> > > +The main tree used by media developers is at:
> > > +
> > > +https://git.linuxtv.org/media.git/
> > > +
> > > +.. _Media development workflow:
> > > +
> > > +Media development workflow
> > > +++++++++++++++++++++++++++
> > > +
> > > +All changes for the media subsystem must be sent first as e-mails to the
> > > +media mailing list, following the process documented at  
> > 
> > s/^/linux-/
> > 
> > Also I'd refer to it as "LMML".
> 
> We can add an alias there, but better to be explicit about what mailing
> list we're referring to.

The list is known as the Linux Media mailing list. We should refer it by
that name or an abbreviation (LMML).

> 
> > 
> > > +Documentation/process/index.rst.
> > > +
> > > +It means that patches shall be submitted as plain text only via e-mail to
> > > [e-mail]. While subscription is not mandatory, you
> > > +can find details about how to subscribe to it and to see its archives at:
> > > +
> > > +  https://subspace.kernel.org/vger.kernel.org.html
> > > +
> > > +Emails with HTML will be automatically rejected by the mail server.
> > > +
> > > +It could be wise to also copy the media committer(s). You should use
> > > +``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
> > > +Please always copy driver's authors and maintainers.
> > > +
> > > +Such patches need to be based against a public branch or tag as follows:
> > > +
> > > +1. Patches for new features need to be based at the ``next`` branch of
> > > +   media.git tree;
> > > +
> > > +2. Fixes against an already released kernel should preferably be against
> > > +   the latest released Kernel. If they require a previously-applied
> > > +   change at media.git tree, they need to be against its ``fixes`` branch.  
> > 
> > This would be a change to the current process that I don't think has been
> > discussed. If I understand correctly, generally this applies to patches
> > that have been merged to the development branch (formerly media stage tree
> > master) with Cc: stable and Fixes: tags.
> 
> 
> Not really a change: this is what we do in practice (except that we renamed
> master to next).

I mean fixes in particular. Fixes are generally meant for an *upcoming*
release, not a past release, and primarily fixing issues that have been
introduced in that cycle. Other bugfixes should go through the normal
process.

How about adding:

Patches with security implications should be handled using a different
process defined in Documentation/process/security-bugs.rst.

> 
> We did discuss that during the LPC week. There was a request there about
> simplifying the trees during media summit with some suggestions. I did a 
> followup meeting with Hans afterwards for us to check what would work best.
> 
> The change is, basically:
> 	media-tree master -> media.git next
> 	media-tree fixes   -> media.git fixes
> 
> 	media-stage master -> media-committers next
> 
> Subsystem maintainers are also merging patches at media-committers fixes,
> in order to let media-ci to test the fixes branch.
> 
> As agreed during the media summit, only subsystem maintainers will be
> merging fixes patches.
> 
> > > +
> > > +3. Fixes for issues not present at the latest released kernel shall
> > > +   be either against a -rc kernel for an upcoming release or
> > > +   against the ``fixes`` branch of the media.git tree.
> > > +
> > > +Patches with fixes shall have:
> > > +
> > > +- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
> > > +- when applicable, a ``Cc: [e-mail]``.
> > > +
> > > +Patches that were fixing bugs publicly reported by someone at the
> > > [e-mail] mailing list shall have:
> > > +
> > > +- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
> > > +
> > > +Patches that change API shall update documentation accordingly at the
> > > +same patch series.
> > > +
> > > +See Documentation/process/index.rst for more details about e-mail submission.
> > > +
> > > +Once a patch is submitted, it may follow either one of the following
> > > +workflows:
> > > +
> > > +a. Pull request workflow: patches are handled by subsystem maintainers::
> > > +
> > > +     +------+   +---------+   +-------+   +-----------------------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|  
> > 
> > s/e-mail/LMML/
> 
> maybe, instead: e-mail to LMML.

Sounds good.

> 
> > (see earlier comment)? Same below.
> > 
> > > +     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
> > > +                +---------+   +-------+   +-----------------------+
> > > +
> > > +   For this workflow, pull requests can be generated by a committer,
> > > +   a previous committer, subsystem maintainers or by a trusted long-time  
> > 
> > s/previous/former/
> > 
> > I'd also use plural in all cases here.
> 
> Ok.
> 
> > 
> > > +   contributor. If you are not in such group, please don't submit  
> > 
> > > +   pull requests, as they will not be processed.
> > > +
> > > +b. Committers' workflow: patches are handled by media committers::
> > > +
> > > +     +------+   +---------+   +--------------------+   +-----------+   +---------+
> > > +     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
> > > +     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
> > > +                +---------+   +--------------------+   +-----------+
> > > +
> > > +On both workflows, all patches shall be properly reviewed at
> > > [e-mail] before being merged at media-committers.git.
> > > +
> > > +When patches are picked by patchwork and when merged at media-committers,
> > > +CI bots will check for errors and may provide e-mail feedback about
> > > +patch problems. When this happens, the patch submitter must fix them, or
> > > +explain why the errors are false positives.
> > > +
> > > +Patches will only be moved to the next stage in those two workflows if they
> > > +don't fail on CI or if there are false-positives in the CI reports.  
> > 
> > s/don't fail on/pass/
> 
> Ok.
> 
> > 
> > > +
> > > +Failures during e-mail submission
> > > ++++++++++++++++++++++++++++++++++
> > >  
> > >  Media's workflow is heavily based on Patchwork, meaning that, once a patch
> > >  is submitted, the e-mail will first be accepted by the mailing list
> > > @@ -47,51 +167,48 @@ server, and, after a while, it should appear at:
> > >  
> > >     - https://patchwork.linuxtv.org/project/linux-media/list/
> > >  
> > > -If it doesn't automatically appear there after a few minutes, then
> > > +If it doesn't automatically appear there after some time [2]_, then
> > >  probably something went wrong on your submission. Please check if the
> > > -email is in plain text\ [2]_ only and if your emailer is not mangling
> > > +email is in plain text\ [3]_ only and if your emailer is not mangling
> > >  whitespaces before complaining or submitting them again.
> > >  
> > > -You can check if the mailing list server accepted your patch, by looking at:
> > > +To troubleshoot problems, you should first check if the mailing list
> > > +server has accepted your patch, by looking at:
> > >  
> > >     - https://lore.kernel.org/linux-media/
> > >  
> > > -.. [2] If your email contains HTML, the mailing list server will simply
> > > +If the patch is there and not at patchwork, it is likely that your e-mailer
> > > +mangled the patch. Patchwork internally has a logic that checks if the
> > > +received e-mail contain a valid patch. Any whitespace and new line
> > > +breakages mangling the patch won't be recognized by patchwork, thus such
> > > +patch will be rejected.
> > > +
> > > +.. [2] It usually takes a few minutes for the patch to arrive, but
> > > +       the e-mail server may be busy, so it may take up to a few hours
> > > +       for a patch to be picked by patchwork.
> > > +
> > > +.. [3] If your email contains HTML, the mailing list server will simply
> > >         drop it, without any further notice.
> > >  
> > > +.. _media-developers-gpg:
> > >  
> > > -Media maintainers
> > > -+++++++++++++++++
> > > +Authentication for pull and merge requests
> > > +++++++++++++++++++++++++++++++++++++++++++
> > >  
> > > -At the media subsystem, we have a group of senior developers that
> > > -are responsible for doing the code reviews at the drivers (also known as
> > > -sub-maintainers), and another senior developer responsible for the
> > > -subsystem as a whole. For core changes, whenever possible, multiple
> > > -media maintainers do the review.
> > > +The authenticity of developers submitting pull requests and merge requests
> > > +shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
> > >  
> > > -The media maintainers that work on specific areas of the subsystem are:
> > > +With the pull request workflow, pull requests shall use a PGP-signed tag.
> > >  
> > > -- Remote Controllers (infrared):
> > > -    Sean Young <[e-mail]>
> > > +For more details about PGP sign, please read
> > > +Documentation/process/maintainer-pgp-guide.rst.  
> > 
> > s/.*/:ref:`the PGP guide <pgpguide>`/
> 
> No need. A Sphinx plugin does that automatically.

Ack.

> 
> > 
> > >  
> > > -- HDMI CEC:
> > > -    Hans Verkuil <[e-mail]>
> > > +Subsystem maintainers
> > > +---------------------
> > >  
> > > -- Media controller drivers:
> > > -    Laurent Pinchart <[e-mail]>
> > > -
> > > -- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
> > > -    Sakari Ailus <[e-mail]>
> > > -
> > > -- V4L2 drivers and core V4L2 frameworks:
> > > -    Hans Verkuil <[e-mail]>
> > > -
> > > -The subsystem maintainer is:
> > > -  Mauro Carvalho Chehab <[e-mail]>
> > > -
> > > -Media maintainers may delegate a patch to other media maintainers as needed.
> > > -On such case, checkpatch's ``delegate`` field indicates who's currently
> > > -responsible for reviewing a patch.
> > > +The subsystem maintainers are:
> > > +  - Mauro Carvalho Chehab <[e-mail]> and
> > > +  - Hans Verkuil <[e-mail]>
> > >  
> > >  Submit Checklist Addendum
> > >  -------------------------
> > > @@ -108,17 +225,14 @@ implementing the media APIs:
> > >  ====================	=======================================================
> > >  Type			Tool
> > >  ====================	=======================================================
> > > -V4L2 drivers\ [3]_	``v4l2-compliance``
> > > +V4L2 drivers\ [4]_	``v4l2-compliance``
> > >  V4L2 virtual drivers	``contrib/test/test-media``
> > >  CEC drivers		``cec-compliance``
> > >  ====================	=======================================================
> > >  
> > > -.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
> > > +.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
> > >         V4L2 drivers.
> > >  
> > > -Other compilance tools are under development to check other parts of the
> > > -subsystem.
> > > -
> > >  Those tests need to pass before the patches go upstream.
> > >  
> > >  Also, please notice that we build the Kernel with::
> > > @@ -134,6 +248,8 @@ Where the check script is::
> > >  Be sure to not introduce new warnings on your patches without a
> > >  very good reason.
> > >  
> > > +Please see `Media development workflow`_ for e-mail submission rules.
> > > +
> > >  Style Cleanup Patches
> > >  +++++++++++++++++++++
> > >  
> > > @@ -199,7 +315,7 @@ tree between -rc6 and the next -rc1.
> > >  Please notice that the media subsystem is a high traffic one, so it
> > >  could take a while for us to be able to review your patches. Feel free
> > >  to ping if you don't get a feedback in a couple of weeks or to ask
> > > -other developers to publicly add Reviewed-by and, more importantly,
> > > +other developers to publicly add ``Reviewed-by:`` and, more importantly,
> > >  ``Tested-by:`` tags.
> > >  
> > >  Please note that we expect a detailed description for ``Tested-by:``,
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 1e930c7a58b1..c77f56a2e695 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -14510,6 +14510,7 @@ MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
> > >  M:	Mauro Carvalho Chehab <[e-mail]>
> > >  L:	[e-mail]
> > >  S:	Maintained
> > > +P:	Documentation/driver-api/media/maintainer-entry-profile.rst
> > >  W:	https://linuxtv.org
> > >  Q:	http://patchwork.kernel.org/project/linux-media/list/
> > >  T:	git git://linuxtv.org/media.git  
> > 
> 
>

Patch

diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
index ffc712a5f632..dc764163cf1c 100644
--- a/Documentation/driver-api/media/maintainer-entry-profile.rst
+++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
@@ -27,19 +27,139 @@  It covers, mainly, the contents of those directories:
 Both media userspace and Kernel APIs are documented and the documentation
 must be kept in sync with the API changes. It means that all patches that
 add new features to the subsystem must also bring changes to the
-corresponding API files.
+corresponding API documentation files.
 
-Due to the size and wide scope of the media subsystem, media's
-maintainership model is to have sub-maintainers that have a broad
-knowledge of a specific aspect of the subsystem. It is the sub-maintainers'
-task to review the patches, providing feedback to users if the patches are
+Due to the size and wide scope of the media subsystem, the media's
+maintainership model is to have committers that have a broad knowledge of
+a specific aspect of the subsystem. It is the committers' task to
+review the patches, providing feedback to users if the patches are
 following the subsystem rules and are properly using the media kernel and
 userspace APIs.
 
-Patches for the media subsystem must be sent to the media mailing list
-at [e-mail] as plain text only e-mail. Emails with
-HTML will be automatically rejected by the mail server. It could be wise
-to also copy the sub-maintainer(s).
+Media committers
+----------------
+
+In the media subsystem, there are experienced developers who can push
+patches directly to the development tree. These developers are called
+Media committers and are divided into the following categories:
+
+- Committers:
+    contributors for one or more drivers within the media subsystem.
+    They can push changes to the tree that do not affect the core or ABI.
+
+- Core committers:
+    responsible for part of the media core. They are typically
+    responsible for one or more drivers within the media subsystem, but, besides
+    that, they can also merge patches that change the code common to multiple
+    drivers, including the kernel internal API.
+
+- Subsystem maintainers:
+    responsible for the subsystem as a whole, with access to the
+    entire subsystem.
+
+    Only subsystem maintainers can push changes that affect the userspace
+    API/ABI.
+
+All media committers shall explicitly agree with the Kernel development process
+as described at Documentation/process/index.rst and to the Kernel
+development rules inside the Kernel documentation, including its code of
+conduct.
+
+Media development tree
+----------------------
+
+The main development tree used by the media subsystem is hosted at LinuxTV.org,
+where we also maintain news about the subsystem, wiki pages and a patchwork
+instance where we track patches though their lifetime.
+
+The main tree used by media developers is at:
+
+https://git.linuxtv.org/media.git/
+
+.. _Media development workflow:
+
+Media development workflow
+++++++++++++++++++++++++++
+
+All changes for the media subsystem must be sent first as e-mails to the
+media mailing list, following the process documented at
+Documentation/process/index.rst.
+
+It means that patches shall be submitted as plain text only via e-mail to
[e-mail]. While subscription is not mandatory, you
+can find details about how to subscribe to it and to see its archives at:
+
+  https://subspace.kernel.org/vger.kernel.org.html
+
+Emails with HTML will be automatically rejected by the mail server.
+
+It could be wise to also copy the media committer(s). You should use
+``scripts/get_maintainers.pl`` to identify whom else needs to be copied.
+Please always copy driver's authors and maintainers.
+
+Such patches need to be based against a public branch or tag as follows:
+
+1. Patches for new features need to be based at the ``next`` branch of
+   media.git tree;
+
+2. Fixes against an already released kernel should preferably be against
+   the latest released Kernel. If they require a previously-applied
+   change at media.git tree, they need to be against its ``fixes`` branch.
+
+3. Fixes for issues not present at the latest released kernel shall
+   be either against a -rc kernel for an upcoming release or
+   against the ``fixes`` branch of the media.git tree.
+
+Patches with fixes shall have:
+
+- a ``Fixes:`` tag pointing to the first commit that introduced the bug;
+- when applicable, a ``Cc: [e-mail]``.
+
+Patches that were fixing bugs publicly reported by someone at the
[e-mail] mailing list shall have:
+
+- a ``Reported-by:`` tag immediately followed by a ``Closes:`` tag.
+
+Patches that change API shall update documentation accordingly at the
+same patch series.
+
+See Documentation/process/index.rst for more details about e-mail submission.
+
+Once a patch is submitted, it may follow either one of the following
+workflows:
+
+a. Pull request workflow: patches are handled by subsystem maintainers::
+
+     +------+   +---------+   +-------+   +-----------------------+   +---------+
+     |e-mail|-->|patchwork|-->|pull   |-->|maintainers merge      |-->|media.git|
+     +------+   |picks it |   |request|   |in media-committers.git|   +---------+
+                +---------+   +-------+   +-----------------------+
+
+   For this workflow, pull requests can be generated by a committer,
+   a previous committer, subsystem maintainers or by a trusted long-time
+   contributor. If you are not in such group, please don't submit
+   pull requests, as they will not be processed.
+
+b. Committers' workflow: patches are handled by media committers::
+
+     +------+   +---------+   +--------------------+   +-----------+   +---------+
+     |e-mail|-->|patchwork|-->|committers merge at |-->|maintainers|-->|media.git|
+     +------+   |picks it |   |media-committers.git|   |approval   |   +---------+
+                +---------+   +--------------------+   +-----------+
+
+On both workflows, all patches shall be properly reviewed at
[e-mail] before being merged at media-committers.git.
+
+When patches are picked by patchwork and when merged at media-committers,
+CI bots will check for errors and may provide e-mail feedback about
+patch problems. When this happens, the patch submitter must fix them, or
+explain why the errors are false positives.
+
+Patches will only be moved to the next stage in those two workflows if they
+don't fail on CI or if there are false-positives in the CI reports.
+
+Failures during e-mail submission
++++++++++++++++++++++++++++++++++
 
 Media's workflow is heavily based on Patchwork, meaning that, once a patch
 is submitted, the e-mail will first be accepted by the mailing list
@@ -47,51 +167,48 @@  server, and, after a while, it should appear at:
 
    - https://patchwork.linuxtv.org/project/linux-media/list/
 
-If it doesn't automatically appear there after a few minutes, then
+If it doesn't automatically appear there after some time [2]_, then
 probably something went wrong on your submission. Please check if the
-email is in plain text\ [2]_ only and if your emailer is not mangling
+email is in plain text\ [3]_ only and if your emailer is not mangling
 whitespaces before complaining or submitting them again.
 
-You can check if the mailing list server accepted your patch, by looking at:
+To troubleshoot problems, you should first check if the mailing list
+server has accepted your patch, by looking at:
 
    - https://lore.kernel.org/linux-media/
 
-.. [2] If your email contains HTML, the mailing list server will simply
+If the patch is there and not at patchwork, it is likely that your e-mailer
+mangled the patch. Patchwork internally has a logic that checks if the
+received e-mail contain a valid patch. Any whitespace and new line
+breakages mangling the patch won't be recognized by patchwork, thus such
+patch will be rejected.
+
+.. [2] It usually takes a few minutes for the patch to arrive, but
+       the e-mail server may be busy, so it may take up to a few hours
+       for a patch to be picked by patchwork.
+
+.. [3] If your email contains HTML, the mailing list server will simply
        drop it, without any further notice.
 
+.. _media-developers-gpg:
 
-Media maintainers
-+++++++++++++++++
+Authentication for pull and merge requests
+++++++++++++++++++++++++++++++++++++++++++
 
-At the media subsystem, we have a group of senior developers that
-are responsible for doing the code reviews at the drivers (also known as
-sub-maintainers), and another senior developer responsible for the
-subsystem as a whole. For core changes, whenever possible, multiple
-media maintainers do the review.
+The authenticity of developers submitting pull requests and merge requests
+shall be validated by using PGP sign. See: :ref:`kernel_org_trust_repository`.
 
-The media maintainers that work on specific areas of the subsystem are:
+With the pull request workflow, pull requests shall use a PGP-signed tag.
 
-- Remote Controllers (infrared):
-    Sean Young <[e-mail]>
+For more details about PGP sign, please read
+Documentation/process/maintainer-pgp-guide.rst.
 
-- HDMI CEC:
-    Hans Verkuil <[e-mail]>
+Subsystem maintainers
+---------------------
 
-- Media controller drivers:
-    Laurent Pinchart <[e-mail]>
-
-- ISP, v4l2-async, v4l2-fwnode, v4l2-flash-led-class and Sensor drivers:
-    Sakari Ailus <[e-mail]>
-
-- V4L2 drivers and core V4L2 frameworks:
-    Hans Verkuil <[e-mail]>
-
-The subsystem maintainer is:
-  Mauro Carvalho Chehab <[e-mail]>
-
-Media maintainers may delegate a patch to other media maintainers as needed.
-On such case, checkpatch's ``delegate`` field indicates who's currently
-responsible for reviewing a patch.
+The subsystem maintainers are:
+  - Mauro Carvalho Chehab <[e-mail]> and
+  - Hans Verkuil <[e-mail]>
 
 Submit Checklist Addendum
 -------------------------
@@ -108,17 +225,14 @@  implementing the media APIs:
 ====================	=======================================================
 Type			Tool
 ====================	=======================================================
-V4L2 drivers\ [3]_	``v4l2-compliance``
+V4L2 drivers\ [4]_	``v4l2-compliance``
 V4L2 virtual drivers	``contrib/test/test-media``
 CEC drivers		``cec-compliance``
 ====================	=======================================================
 
-.. [3] The ``v4l2-compliance`` also covers the media controller usage inside
+.. [4] The ``v4l2-compliance`` also covers the media controller usage inside
        V4L2 drivers.
 
-Other compilance tools are under development to check other parts of the
-subsystem.
-
 Those tests need to pass before the patches go upstream.
 
 Also, please notice that we build the Kernel with::
@@ -134,6 +248,8 @@  Where the check script is::
 Be sure to not introduce new warnings on your patches without a
 very good reason.
 
+Please see `Media development workflow`_ for e-mail submission rules.
+
 Style Cleanup Patches
 +++++++++++++++++++++
 
@@ -199,7 +315,7 @@  tree between -rc6 and the next -rc1.
 Please notice that the media subsystem is a high traffic one, so it
 could take a while for us to be able to review your patches. Feel free
 to ping if you don't get a feedback in a couple of weeks or to ask
-other developers to publicly add Reviewed-by and, more importantly,
+other developers to publicly add ``Reviewed-by:`` and, more importantly,
 ``Tested-by:`` tags.
 
 Please note that we expect a detailed description for ``Tested-by:``,
diff --git a/MAINTAINERS b/MAINTAINERS
index 1e930c7a58b1..c77f56a2e695 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14510,6 +14510,7 @@  MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
 M:	Mauro Carvalho Chehab <[e-mail]>
 L:	[e-mail]
 S:	Maintained
+P:	Documentation/driver-api/media/maintainer-entry-profile.rst
 W:	https://linuxtv.org
 Q:	http://patchwork.kernel.org/project/linux-media/list/
 T:	git git://linuxtv.org/media.git