| Message ID | 20230714084725.27847-2-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [1/3] MAINTAINERS: soc: reference maintainer profile | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | warning | Series does not have a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 36 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Hey Krzysztof, On Fri, Jul 14, 2023 at 10:47:24AM +0200, Krzysztof Kozlowski wrote: > Some SoC platforms require that commits must not bring any new > dtbs_check warnings. Maintainers of such platforms usually have some > automation set, so any new warning will be spotted sooner or later. > Worst case: they run the tests themselves. Document requirements for > such platforms, so contributors can expect their patches being dropped > or ignored, if they bring new warnings for existing boards. Definitely a more scalable approach than your previous version! > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > .../process/maintainer-handbooks.rst | 1 + > .../process/maintainer-soc-clean-dts.rst | 22 +++++++++++++++++++ > MAINTAINERS | 2 +- > 3 files changed, 24 insertions(+), 1 deletion(-) > create mode 100644 Documentation/process/maintainer-soc-clean-dts.rst > > diff --git a/Documentation/process/maintainer-handbooks.rst b/Documentation/process/maintainer-handbooks.rst > index 9992bfd7eaa3..976391cec528 100644 > --- a/Documentation/process/maintainer-handbooks.rst > +++ b/Documentation/process/maintainer-handbooks.rst > @@ -17,5 +17,6 @@ Contents: > > maintainer-netdev > maintainer-soc > + maintainer-soc-clean-dts > maintainer-tip > maintainer-kvm-x86 > diff --git a/Documentation/process/maintainer-soc-clean-dts.rst b/Documentation/process/maintainer-soc-clean-dts.rst > new file mode 100644 > index 000000000000..87feeb5543ff > --- /dev/null > +++ b/Documentation/process/maintainer-soc-clean-dts.rst > @@ -0,0 +1,22 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +============================= > +SoC Platforms with Strict DTS I don't think that this title makes much sense, it feels like it has been truncated. Perhaps add "Requirements" to the end? > +============================= > + > +Overview > +-------- > + > +SoC platforms or subarchitectures follow all the rules from s/follow/should follow/? > +Documentation/process/maintainer-soc.rst. However platforms referencing this > +document impose additional requirements listed below. > + > +Strict DTS DT schema compliance > +------------------------------- Should there be a blank line here to match the other section headings? Also, to match the title case you used elsewhere, "Schema Compliance"? > +None of the changes to the SoC platform Devicetree sources (DTS files) can > +bring new ``make dtbs_check W=1`` warnings. The platform maintainers have Nitpickery again, but perhaps the first sentence here would read better as "No changes to the SoC platform Devicetree sources (DTS files) should introduce new ``make dtbs_check W=1`` warnings."? > +automation in place which should point out any new warnings. > + > +If a commit introducing new warning gets accepted somehow, the resulting issues > +shall be fixed in reasonable time (e.g. within one release) or the commit > +reverted. It is loosely related, but I was wondering if we should also try to push people that change the platform's bindings to update the DTS also, so that binding changes do not introduce W=1 complaints? For many bindings the platform entry in MAINTAINERS does not cover them, but things like the arm64 Apple stuff mention them specifically & others will get coverage due to regexes. Anyway, nitpickery aside I like this approach. Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > diff --git a/MAINTAINERS b/MAINTAINERS > index b61289fa7891..7405fb6e38c3 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1561,7 +1561,7 @@ S: Maintained > P: Documentation/process/maintainer-soc.rst > C: irc://irc.libera.chat/armlinux > T: git git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git > -F: Documentation/process/maintainer-soc.rst > +F: Documentation/process/maintainer-soc*.rst > F: arch/arm/boot/dts/Makefile > F: arch/arm64/boot/dts/Makefile > > -- > 2.34.1 >
On Fri, Jul 14, 2023 at 10:47:24AM +0200, Krzysztof Kozlowski wrote: > +Overview > +-------- > + > +SoC platforms or subarchitectures follow all the rules from > +Documentation/process/maintainer-soc.rst. However platforms referencing this Just a drive by comment, references to highly relevant documents should be clickable, so :doc:`Documentation/process/maintainer-soc` , with exceptions like if the document has been referenced already.
On 14/07/2023 15:59, David Sterba wrote: > On Fri, Jul 14, 2023 at 10:47:24AM +0200, Krzysztof Kozlowski wrote: >> +Overview >> +-------- >> + >> +SoC platforms or subarchitectures follow all the rules from >> +Documentation/process/maintainer-soc.rst. However platforms referencing this > > Just a drive by comment, references to highly relevant documents should > be clickable, so :doc:`Documentation/process/maintainer-soc` , with > exceptions like if the document has been referenced already. Is it needed though? The link is anyway detected by sphinx. Best regards, Krzysztof
On 14/07/2023 14:50, Conor Dooley wrote: > Hey Krzysztof, > > On Fri, Jul 14, 2023 at 10:47:24AM +0200, Krzysztof Kozlowski wrote: >> Some SoC platforms require that commits must not bring any new >> dtbs_check warnings. Maintainers of such platforms usually have some >> automation set, so any new warning will be spotted sooner or later. >> Worst case: they run the tests themselves. Document requirements for >> such platforms, so contributors can expect their patches being dropped >> or ignored, if they bring new warnings for existing boards. > > Definitely a more scalable approach than your previous version! > >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> .../process/maintainer-handbooks.rst | 1 + >> .../process/maintainer-soc-clean-dts.rst | 22 +++++++++++++++++++ >> MAINTAINERS | 2 +- >> 3 files changed, 24 insertions(+), 1 deletion(-) >> create mode 100644 Documentation/process/maintainer-soc-clean-dts.rst >> >> diff --git a/Documentation/process/maintainer-handbooks.rst b/Documentation/process/maintainer-handbooks.rst >> index 9992bfd7eaa3..976391cec528 100644 >> --- a/Documentation/process/maintainer-handbooks.rst >> +++ b/Documentation/process/maintainer-handbooks.rst >> @@ -17,5 +17,6 @@ Contents: >> >> maintainer-netdev >> maintainer-soc >> + maintainer-soc-clean-dts >> maintainer-tip >> maintainer-kvm-x86 >> diff --git a/Documentation/process/maintainer-soc-clean-dts.rst b/Documentation/process/maintainer-soc-clean-dts.rst >> new file mode 100644 >> index 000000000000..87feeb5543ff >> --- /dev/null >> +++ b/Documentation/process/maintainer-soc-clean-dts.rst >> @@ -0,0 +1,22 @@ >> +.. SPDX-License-Identifier: GPL-2.0 >> + >> +============================= >> +SoC Platforms with Strict DTS > > I don't think that this title makes much sense, it feels like it has > been truncated. Perhaps add "Requirements" to the end? OK, but maybe better then SoC Platforms with DTS Compliance Requirements ? > >> +============================= >> + >> +Overview >> +-------- >> + >> +SoC platforms or subarchitectures follow all the rules from > > s/follow/should follow/? Ack > >> +Documentation/process/maintainer-soc.rst. However platforms referencing this >> +document impose additional requirements listed below. >> + >> +Strict DTS DT schema compliance >> +------------------------------- > Should there be a blank line here to match the other section headings? Ack > Also, to match the title case you used elsewhere, "Schema Compliance"? Ack > >> +None of the changes to the SoC platform Devicetree sources (DTS files) can >> +bring new ``make dtbs_check W=1`` warnings. The platform maintainers have > > Nitpickery again, but perhaps the first sentence here would read better as > "No changes to the SoC platform Devicetree sources (DTS files) should > introduce new ``make dtbs_check W=1`` warnings."? Ack > >> +automation in place which should point out any new warnings. >> + >> +If a commit introducing new warning gets accepted somehow, the resulting issues >> +shall be fixed in reasonable time (e.g. within one release) or the commit >> +reverted. > > It is loosely related, but I was wondering if we should also try to push > people that change the platform's bindings to update the DTS also, so > that binding changes do not introduce W=1 complaints? Makes sense, we could add such rule to Devicetree maintainer profile. Anyway enforcing it relies on Rob's bot reporting the warnings, which seems silent recently. > For many bindings the platform entry in MAINTAINERS does not cover them, > but things like the arm64 Apple stuff mention them specifically & others > will get coverage due to regexes. > > Anyway, nitpickery aside I like this approach. > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > Best regards, Krzysztof
On 17/07/2023 09:51, Krzysztof Kozlowski wrote: > On 14/07/2023 15:59, David Sterba wrote: >> On Fri, Jul 14, 2023 at 10:47:24AM +0200, Krzysztof Kozlowski wrote: >>> +Overview >>> +-------- >>> + >>> +SoC platforms or subarchitectures follow all the rules from >>> +Documentation/process/maintainer-soc.rst. However platforms referencing this >> >> Just a drive by comment, references to highly relevant documents should >> be clickable, so :doc:`Documentation/process/maintainer-soc` , with >> exceptions like if the document has been referenced already. > > Is it needed though? The link is anyway detected by sphinx. And it does not work: maintainer-soc-clean-dts.rst:10: WARNING: unknown document: Documentation/process/maintainer-soc I also tried: maintainer-soc-clean-dts.rst:10: WARNING: unknown document: Documentation/process/maintainer-soc.rst Best regards, Krzysztof
diff --git a/Documentation/process/maintainer-handbooks.rst b/Documentation/process/maintainer-handbooks.rst index 9992bfd7eaa3..976391cec528 100644 --- a/Documentation/process/maintainer-handbooks.rst +++ b/Documentation/process/maintainer-handbooks.rst @@ -17,5 +17,6 @@ Contents: maintainer-netdev maintainer-soc + maintainer-soc-clean-dts maintainer-tip maintainer-kvm-x86 diff --git a/Documentation/process/maintainer-soc-clean-dts.rst b/Documentation/process/maintainer-soc-clean-dts.rst new file mode 100644 index 000000000000..87feeb5543ff --- /dev/null +++ b/Documentation/process/maintainer-soc-clean-dts.rst @@ -0,0 +1,22 @@ +.. SPDX-License-Identifier: GPL-2.0 + +============================= +SoC Platforms with Strict DTS +============================= + +Overview +-------- + +SoC platforms or subarchitectures follow all the rules from +Documentation/process/maintainer-soc.rst. However platforms referencing this +document impose additional requirements listed below. + +Strict DTS DT schema compliance +------------------------------- +None of the changes to the SoC platform Devicetree sources (DTS files) can +bring new ``make dtbs_check W=1`` warnings. The platform maintainers have +automation in place which should point out any new warnings. + +If a commit introducing new warning gets accepted somehow, the resulting issues +shall be fixed in reasonable time (e.g. within one release) or the commit +reverted. diff --git a/MAINTAINERS b/MAINTAINERS index b61289fa7891..7405fb6e38c3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1561,7 +1561,7 @@ S: Maintained P: Documentation/process/maintainer-soc.rst C: irc://irc.libera.chat/armlinux T: git git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git -F: Documentation/process/maintainer-soc.rst +F: Documentation/process/maintainer-soc*.rst F: arch/arm/boot/dts/Makefile F: arch/arm64/boot/dts/Makefile
Some SoC platforms require that commits must not bring any new dtbs_check warnings. Maintainers of such platforms usually have some automation set, so any new warning will be spotted sooner or later. Worst case: they run the tests themselves. Document requirements for such platforms, so contributors can expect their patches being dropped or ignored, if they bring new warnings for existing boards. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- .../process/maintainer-handbooks.rst | 1 + .../process/maintainer-soc-clean-dts.rst | 22 +++++++++++++++++++ MAINTAINERS | 2 +- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 Documentation/process/maintainer-soc-clean-dts.rst