Re: [PATCH 5/7] dt-bindings: clk: meson: Add Amlogic T7 sys pll support

From: Ferass El Hafidi <funderscore@postmarketos.org>
To: linux-amlogic@lists.infradead.org,
	Ronald Claveau <linux-kernel-dev@aliel.fr>
Cc: Ronald Claveau <linux-kernel-dev@aliel.fr>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/7] dt-bindings: clk: meson: Add Amlogic T7 sys pll support
Date: Wed, 18 Feb 2026 11:10:29 +0000	[thread overview]
Message-ID: <tanjoa.3m1sr95zhvsal@postmarketos.org> (raw)
In-Reply-To: <20260218110127.36394-1-linux-kernel-dev@aliel.fr>

Hi,

On Wed, 18 Feb 2026 11:01, Ronald Claveau <linux-kernel-dev@aliel.fr> wrote:
>Add SYS PLL for the clock controller of the Amlogic T7 SoC family.

Your commit description is almost the same on every patch in the series.
Probably having a more specific description per commit would be much more
useful, for reviewers to better understand your commit.

There are documentation pages on how to write informative commit messages
at [1] and [2], but in summary, your description should talk about *what*
is being done and (provided that makes sense), *why*.

You also have multiple commits with the exact same commit message
("dt-bindings: clk: meson: Add Amlogic T7 sys pll support"), did you
mean to squash them all into one? (You can do so with `git rebase -i`.)

It probably also would make sense to add a cover letter, which would describe
overall what you goal is with this patch series, and on a high level
(doesn't have to be too detailled, because you'd describe these in more
detail in the respective commit messages themselves) what each patch does.
This once again helps people reviewing your code to understand what problem
you're trying to solve.

[1]: https://docs.kernel.org/process/submitting-patches.html#explanation-body
[2]: https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

Best regards,
Ferass

[PS: sorry if this email was sent twice, my email client did something
     strange]

>
>Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
>---
> include/dt-bindings/clock/amlogic,t7-peripherals-clkc.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/include/dt-bindings/clock/amlogic,t7-peripherals-clkc.h b/include/dt-bindings/clock/amlogic,t7-peripherals-clkc.h
>index 32c4b62037de..0cda8edfa7cd 100644
>--- a/include/dt-bindings/clock/amlogic,t7-peripherals-clkc.h
>+++ b/include/dt-bindings/clock/amlogic,t7-peripherals-clkc.h
>@@ -224,5 +224,12 @@
> #define CLKID_SYS_PWM_AO_CD		215
> #define CLKID_SYS_PWM_AO_EF		216
> #define CLKID_SYS_PWM_AO_GH		217
>+#define CLKID_SYS_A_SEL			218
>+#define CLKID_SYS_A_DIV			219
>+#define CLKID_SYS_A			220
>+#define CLKID_SYS_B_SEL			221
>+#define CLKID_SYS_B_DIV			222
>+#define CLKID_SYS_B			223
>+#define CLKID_SYS			224
> 
> #endif /* __T7_PERIPHERALS_CLKC_H */
>-- 
>2.49.0
>
>
>_______________________________________________
>linux-amlogic mailing list
>linux-amlogic@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-amlogic