| Message ID | 20250428113517.426987-4-shin.son@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] dt-bindings: clock: exynosautov920: add cpucl1/2 clock definitions | expand |
On Mon, Apr 28, 2025 at 08:35:16PM GMT, Shin Son wrote: > The CLKS_NR_CPUCL0 macro was incorrectly defined based on a wrong clock ID. > It mistakenly referenced CLK_DOUT_CLUSTER0_PERIPHCLK, which corresponds to > a cluster peripheral clock, not the last clock ID for CPUCL0 as intended. > > This patch corrects the definition to use CLK_DOUT_CPUCL0_NOCP + 1, Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > properly matching the last clock ID for CPUCL0 as intended. > > This error was due to confusion with the hardware diagram, and this patch > ensures that the number of clocks for CPUCL0 is correctly defined. Fixes: 59636ec89c2c ("clk: samsung: exynosautov920: add cpucl0 clock support") And proper order - fixes are *ALWAYS* before new features. Best regards, Krzysztof
On Mon, 28 Apr 2025 20:35:16 +0900, Shin Son wrote: > The CLKS_NR_CPUCL0 macro was incorrectly defined based on a wrong clock ID. > It mistakenly referenced CLK_DOUT_CLUSTER0_PERIPHCLK, which corresponds to > a cluster peripheral clock, not the last clock ID for CPUCL0 as intended. > > This patch corrects the definition to use CLK_DOUT_CPUCL0_NOCP + 1, > properly matching the last clock ID for CPUCL0 as intended. > > [...] Applied, thanks! [3/4] clk: samsung: exynosautov920: Fix incorrect CLKS_NR_CPUCL0 definition https://git.kernel.org/krzk/linux/c/f00aef8e2ee0b642abdb91682bec5af38532faf7 Best regards,
Hello Krzysztof Kozlowski, > -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzk@kernel.org] > Sent: Wednesday, April 30, 2025 4:26 PM > To: Shin Son <shin.son@samsung.com> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>; Chanwoo Choi > <cw00.choi@samsung.com>; Alim Akhtar <alim.akhtar@samsung.com>; Michael > Turquette <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob > Herring <robh@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Sunyeal > Hong <sunyeal.hong@samsung.com>; linux-samsung-soc@vger.kernel.org; linux- > clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 3/4] clk: samsung: exynosautov920: Fix incorrect > CLKS_NR_CPUCL0 definition > > On Mon, Apr 28, 2025 at 08:35:16PM GMT, Shin Son wrote: > > The CLKS_NR_CPUCL0 macro was incorrectly defined based on a wrong clock > ID. > > It mistakenly referenced CLK_DOUT_CLUSTER0_PERIPHCLK, which > > corresponds to a cluster peripheral clock, not the last clock ID for > CPUCL0 as intended. > > > > This patch corrects the definition to use CLK_DOUT_CPUCL0_NOCP + 1, > > Please do not use "This commit/patch/change", but imperative mood. See > longer explanation here: > https://protect2.fireeye.com/v1/url?k=2e90faf7-4f1befd2-2e9171b8- > 74fe485cbff6-28bf0169b8fd9187&q=1&e=dfbde4e5-fe38-4140-aa45- > b3362eb2dc00&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.17.1%2Fsource > %2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L95 > > > properly matching the last clock ID for CPUCL0 as intended. > > > > This error was due to confusion with the hardware diagram, and this > > patch ensures that the number of clocks for CPUCL0 is correctly defined. > > Fixes: 59636ec89c2c ("clk: samsung: exynosautov920: add cpucl0 clock > support") > > > And proper order - fixes are *ALWAYS* before new features. > > Best regards, > Krzysztof Thank you for applying my patches. I appreciate your feedback, and I’ll make sure to improve those parts in the future. Have a great day. Best regards, Shin Son.
diff --git a/drivers/clk/samsung/clk-exynosautov920.c b/drivers/clk/samsung/clk-exynosautov920.c index c1b0203b8cb4..f8168eed4a66 100644 --- a/drivers/clk/samsung/clk-exynosautov920.c +++ b/drivers/clk/samsung/clk-exynosautov920.c @@ -18,7 +18,7 @@ /* NOTE: Must be equal to the last clock ID increased by one */ #define CLKS_NR_TOP (DOUT_CLKCMU_TAA_NOC + 1) -#define CLKS_NR_CPUCL0 (CLK_DOUT_CLUSTER0_PERIPHCLK + 1) +#define CLKS_NR_CPUCL0 (CLK_DOUT_CPUCL0_NOCP + 1) #define CLKS_NR_CPUCL1 (CLK_DOUT_CPUCL1_NOCP + 1) #define CLKS_NR_CPUCL2 (CLK_DOUT_CPUCL2_NOCP + 1) #define CLKS_NR_PERIC0 (CLK_DOUT_PERIC0_I3C + 1)
The CLKS_NR_CPUCL0 macro was incorrectly defined based on a wrong clock ID. It mistakenly referenced CLK_DOUT_CLUSTER0_PERIPHCLK, which corresponds to a cluster peripheral clock, not the last clock ID for CPUCL0 as intended. This patch corrects the definition to use CLK_DOUT_CPUCL0_NOCP + 1, properly matching the last clock ID for CPUCL0 as intended. This error was due to confusion with the hardware diagram, and this patch ensures that the number of clocks for CPUCL0 is correctly defined. Signed-off-by: Shin Son <shin.son@samsung.com> --- drivers/clk/samsung/clk-exynosautov920.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)