diff mbox series

[2/3] Documentation/process: maintainer-soc: add clean platforms profile

Message ID 20230714084725.27847-2-krzysztof.kozlowski@linaro.org (mailing list archive)
State Superseded
Headers show
Series [1/3] MAINTAINERS: soc: reference maintainer profile | expand

Commit Message

Krzysztof Kozlowski July 14, 2023, 8:47 a.m. UTC
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

Comments

Conor Dooley July 14, 2023, 12:50 p.m. UTC | #1
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
>
David Sterba July 14, 2023, 1:59 p.m. UTC | #2
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.
Krzysztof Kozlowski July 17, 2023, 7:51 a.m. UTC | #3
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
Krzysztof Kozlowski July 17, 2023, 7:58 a.m. UTC | #4
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
Krzysztof Kozlowski July 19, 2023, 2:26 p.m. UTC | #5
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 mbox series

Patch

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