Hi Mauro, Some comments below: On 01/03/2021 12:46, Mauro Carvalho Chehab wrote: > Document the basic policies of the media subsystem profile. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > --- > > v4: address issues pointed by Laurent and Sakari > v3: address a few nitpicks from Randy and Lukas > v2: fix the Documentation/*/media directories > > Documentation/driver-api/media/index.rst | 2 + > .../media/maintainer-entry-profile.rst | 201 ++++++++++++++++++ > .../maintainer/maintainer-entry-profile.rst | 1 + > 3 files changed, 204 insertions(+) > create mode 100644 Documentation/driver-api/media/maintainer-entry-profile.rst > > diff --git a/Documentation/driver-api/media/index.rst b/Documentation/driver-api/media/index.rst > index c140692454b1..2ad71dfa8828 100644 > --- a/Documentation/driver-api/media/index.rst > +++ b/Documentation/driver-api/media/index.rst > @@ -28,6 +28,8 @@ Please see: > :maxdepth: 5 > :numbered: > > + maintainer-entry-profile > + > v4l2-core > dtv-core > rc-core > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst > new file mode 100644 > index 000000000000..a460e2278ffc > --- /dev/null > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst > @@ -0,0 +1,201 @@ > +Media Subsystem Profile > +======================= > + > +Overview > +-------- > + > +The media subsystem covers support for a variety of devices: stream > +capture, analog and digital TV streams, cameras, remote controllers, HDMI CEC > +and media pipeline control. > + > +It covers, mainly, the contents of those directories: > + > + - drivers/media > + - drivers/staging/media > + - Documentation/admin-guide/media > + - Documentation/driver-api/media > + - Documentation/userspace-api/media > + - Documentation/devicetree/bindings/media/\ [1]_ > + - include/media > + > +.. [1] Device tree bindings are maintained by the > + OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS maintainers > + (see the MAINTAINERS file). So, changes there should be reviewed should -> must > + by them before being merged via the media subsystem's development > + tree. > + > +Both media userspace and Kernel APIs are documented and the documentation > +should be kept in sync with the API changes. It means that all patches that should -> must > +add new features to the subsystem should also bring changes to the should -> must > +corresponding API 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 > +following the subsystem rules and are properly using the media kernel and > +userspace APIs. > + > +Patches for the media subsystem should be sent to the media mailing list > +at linux-media@xxxxxxxxxxxxxxx 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'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 > +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 > +probably something got wrong on your submission. Please check if the got wrong on your -> went wrong with your > +email is in plain text\ [2]_ only and if your emailer is not mangling with Drop 'with'. > +whitespaces before complaining or submitting them again. > + > +You can check if the mailing list server accepted your patch, by looking at: > + > + - https://lore.kernel.org/linux-media/ > + > +.. [2] If your email contains HTML, the mailing list server will simply > + drop it, without any further notice. > + > + > +Sub-maintainers > ++++++++++++++++ > + > +At the media subsystem, we have a group of experienced developers that > +are responsible for doing the code reviews at the drivers (called > +sub-maintainers), and another senior developer responsible for the > +subsystem as a hole. For core changes, whenever possible, multiple hole -> whole > +media (sub-)maintainers do the review. > + > +The sub-maintainers work on specific areas of the subsystem, as > +described below: > + > +Digital TV: > + Sean Young <sean@xxxxxxxx> > + > +HDMI CEC: > + Hans Verkuil <hverkuil@xxxxxxxxx> > + > +Media controller drivers: > + Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > + > +Remote Controllers: > + Sean Young <sean@xxxxxxxx> > + > +Sensor drivers: > + Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > + > +V4L2 drivers: V4L2 non-sensor drivers and core V4L2 frameworks: > + Hans Verkuil <hverkuil@xxxxxxxxx> > + > +Submit Checklist Addendum > +------------------------- > + > +Patches that change the Open Firmware/Device Tree bindings should be > +reviewed by the Device Tree maintainers. So, DT maintainers should be > +Cc:ed when those are submitted. Mention the devicetree@xxxxxxxxxxxxxxx mailinglist here. > + > +There is a set of compliance tools at https://git.linuxtv.org/v4l-utils.git/ > +that should be used in order to check if the drivers are properly > +implementing the media APIs: > + > +================== ======================================================= > +Type Tool > +================== ======================================================= > +V4L2 drivers\ [3]_ `v4l2-compliance`, `contrib/test/test-media/` > +CEC drivers `cec-compliance` > +================== ======================================================= > + > +.. [3] 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:: > + > + make CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y C=1 W=1 CHECK=check_script > + > +Where the check script is:: > + > + #!/bin/bash > + /devel/smatch/smatch -p=kernel $@ >&2 > + /devel/sparse/sparse $@ >&2 > + > +Be sure to not introduce new warnings on your patches without a > +very good reason. > + > +Style Cleanup Patches > ++++++++++++++++++++++ > + > +Style cleanups are welcome when they come together with other changes > +at the files where the style changes will affect. > + > +We may accept pure standalone style cleanups, but they should ideally > +be one patch for the whole subsystem (if the cleanup is low volume), > +or at least be grouped per directory. So, for example, if you're doing a > +big cleanup change set at drivers under drivers/media, please send a single > +patch for all drivers under drivers/media/pci, another one for > +drivers/media/usb and so on. > + > +Coding Style Addendum > ++++++++++++++++++++++ > + > +Media development uses checkpatch on strict mode to verify the code style, > +e.g.:: > + > + $ ./scripts/checkpatch.pl --strict --max-line-length=80 > + > +In principle, patches should follow the coding style rules, but exceptions > +are allowed if there are good reasons. On such case, maintainers and reviewers > +may question about the rationale for not addressing the `checkpatch.pl`. > + > +Please notice that the goal here is to improve code readability. On > +a few cases, `checkpatch.pl` may actually point to something that would > +look worse. So, you should use good sense. > + > +Note that addressing one `checkpatch.pl` issue (of any kind) alone may lead > +to having longer lines than 80 characters per line. While this is not > +strictly prohibited, efforts should be made towards staying within 80 > +characters per line. This could include using refactoring code that leads > +to less indentation, shorter variable or function names and last but not > +least, simply wrapping the lines. > + > +In particular, we accept lines with more than 80 columns: > + > + - on strings, as they shouldn't be broken due to line length limits; > + - when a function or variable name need to have a big identifier name, > + which keeps hard to honor the 80 columns limit; > + - on arithmetic expressions, when breaking lines makes them harder to > + read; > + - when they avoid a line to end with an open parenthesis or an open > + bracket. > + > +Key Cycle Dates > +--------------- > + > +New submissions can be sent at any time, but if they intend to hit the > +next merge window they should be sent before -rc5, and ideally stabilized > +in the linux-media branch by -rc6. > + > +Review Cadence > +-------------- > + > +Provided that your patch is at https://patchwork.linuxtv.org, it should > +be sooner or later handled, so you don't need to re-submit a patch. > + > +Except for bug fixes, we don't usually add new patches to the development > +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, > +Tested-by tags. What I have noticed is that sometimes important bug fixes are missed due to the high traffic volume. I would like to see something along the lines of: "If important bug fixes are not reviewed or picked up within a week of posting, then do not hesitate to ping." Regards, Hans > + > +Please note that we expect a detailed description for Tested-by, > +identifying what boards were used at the test and what it was tested. > diff --git a/Documentation/maintainer/maintainer-entry-profile.rst b/Documentation/maintainer/maintainer-entry-profile.rst > index b7a627d6c97d..5d5cc3acdf85 100644 > --- a/Documentation/maintainer/maintainer-entry-profile.rst > +++ b/Documentation/maintainer/maintainer-entry-profile.rst > @@ -102,3 +102,4 @@ to do something different in the near future. > ../doc-guide/maintainer-profile > ../nvdimm/maintainer-entry-profile > ../riscv/patch-acceptance > + ../driver-api/media/maintainer-entry-profile >
