From: Chuan Liu <chuan.liu@amlogic.com>
To: Jerome Brunet <jbrunet@baylibre.com>,
Neil Armstrong <neil.armstrong@linaro.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
Kevin Hilman <khilman@baylibre.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 21/26] clk: amlogic: pclk explicitly use CLK_IGNORE_UNUSED
Date: Thu, 3 Jul 2025 15:05:46 +0800 [thread overview]
Message-ID: <10cc2b6e-fd23-4d77-9a86-5ea91bd68e3a@amlogic.com> (raw)
In-Reply-To: <20250702-meson-clk-cleanup-24-v1-21-e163c9a1fc21@baylibre.com>
Hi Jerome:
On 7/2/2025 11:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> Every usage of CLK_IGNORE_UNUSED should be explicitly motivated and
> documented. However, the PCLK macros used by most Amlogic platforms are
> adding that flag systematically. Because of this, all pclks are marked with
> CLK_IGNORE_UNUSED, without any form of distinction or motivation.
>
> This may have been fine in the early days of CCF but it is not anymore.
>
> Just removing the flag is not an option at this stage since it could cause
> regression on existing platforms.
>
> Instead, drop the flag from the macro definition and add it to the each
> clock definition, for the existing clocks. This makes quite a nasty change
> but it will make it a lot easier for people to contribute to fixing the
> problem, clock by clock. It will also prevent new platform from being added
> with a silent use of the flag.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> drivers/clk/meson/a1-peripherals.c | 125 ++++++++++++-----------
> drivers/clk/meson/axg-aoclk.c | 18 ++--
> drivers/clk/meson/axg.c | 110 ++++++++++----------
> drivers/clk/meson/g12a-aoclk.c | 49 ++++-----
> drivers/clk/meson/g12a.c | 176 ++++++++++++++++----------------
> drivers/clk/meson/gxbb-aoclk.c | 16 +--
> drivers/clk/meson/gxbb.c | 188 ++++++++++++++++++-----------------
> drivers/clk/meson/meson-clkc-utils.h | 12 +--
> drivers/clk/meson/meson8b.c | 183 ++++++++++++++++++----------------
> drivers/clk/meson/s4-peripherals.c | 119 ++++++++++++----------
for 'drivers/clk/meson/s4-peripherals.c'.
Reviewed-by: Chuan Liu <chuan.liu@amlogic.com>
> 10 files changed, 528 insertions(+), 468 deletions(-)
>
[...]
> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index fc1500df926d056ce17252987dd91095a8399b55..23b51d84d8de40aa540dbc6dd5db9fb627e579de 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
> @@ -3165,61 +3165,70 @@ static struct clk_regmap s4_gen_clk = {
> },
> };
>
> -#define S4_PCLK(_name, _reg, _bit) \
> - MESON_PCLK(_name, _reg, _bit, &s4_sys_clk.hw)
> -
> -static S4_PCLK(s4_ddr, CLKCTRL_SYS_CLK_EN0_REG0, 0);
> -static S4_PCLK(s4_dos, CLKCTRL_SYS_CLK_EN0_REG0, 1);
> -static S4_PCLK(s4_ethphy, CLKCTRL_SYS_CLK_EN0_REG0, 4);
> -static S4_PCLK(s4_mali, CLKCTRL_SYS_CLK_EN0_REG0, 6);
> -static S4_PCLK(s4_aocpu, CLKCTRL_SYS_CLK_EN0_REG0, 13);
> -static S4_PCLK(s4_aucpu, CLKCTRL_SYS_CLK_EN0_REG0, 14);
> -static S4_PCLK(s4_cec, CLKCTRL_SYS_CLK_EN0_REG0, 16);
> -static S4_PCLK(s4_sdemmca, CLKCTRL_SYS_CLK_EN0_REG0, 24);
> -static S4_PCLK(s4_sdemmcb, CLKCTRL_SYS_CLK_EN0_REG0, 25);
> -static S4_PCLK(s4_nand, CLKCTRL_SYS_CLK_EN0_REG0, 26);
> -static S4_PCLK(s4_smartcard, CLKCTRL_SYS_CLK_EN0_REG0, 27);
> -static S4_PCLK(s4_acodec, CLKCTRL_SYS_CLK_EN0_REG0, 28);
> -static S4_PCLK(s4_spifc, CLKCTRL_SYS_CLK_EN0_REG0, 29);
> -static S4_PCLK(s4_msr_clk, CLKCTRL_SYS_CLK_EN0_REG0, 30);
> -static S4_PCLK(s4_ir_ctrl, CLKCTRL_SYS_CLK_EN0_REG0, 31);
> -
> -static S4_PCLK(s4_audio, CLKCTRL_SYS_CLK_EN0_REG1, 0);
> -static S4_PCLK(s4_eth, CLKCTRL_SYS_CLK_EN0_REG1, 3);
> -static S4_PCLK(s4_uart_a, CLKCTRL_SYS_CLK_EN0_REG1, 5);
> -static S4_PCLK(s4_uart_b, CLKCTRL_SYS_CLK_EN0_REG1, 6);
> -static S4_PCLK(s4_uart_c, CLKCTRL_SYS_CLK_EN0_REG1, 7);
> -static S4_PCLK(s4_uart_d, CLKCTRL_SYS_CLK_EN0_REG1, 8);
> -static S4_PCLK(s4_uart_e, CLKCTRL_SYS_CLK_EN0_REG1, 9);
> -static S4_PCLK(s4_aififo, CLKCTRL_SYS_CLK_EN0_REG1, 11);
> -static S4_PCLK(s4_ts_ddr, CLKCTRL_SYS_CLK_EN0_REG1, 15);
> -static S4_PCLK(s4_ts_pll, CLKCTRL_SYS_CLK_EN0_REG1, 16);
> -static S4_PCLK(s4_g2d, CLKCTRL_SYS_CLK_EN0_REG1, 20);
> -static S4_PCLK(s4_spicc0, CLKCTRL_SYS_CLK_EN0_REG1, 21);
> -static S4_PCLK(s4_usb, CLKCTRL_SYS_CLK_EN0_REG1, 26);
> -static S4_PCLK(s4_i2c_m_a, CLKCTRL_SYS_CLK_EN0_REG1, 30);
> -static S4_PCLK(s4_i2c_m_b, CLKCTRL_SYS_CLK_EN0_REG1, 31);
> -
> -static S4_PCLK(s4_i2c_m_c, CLKCTRL_SYS_CLK_EN0_REG2, 0);
> -static S4_PCLK(s4_i2c_m_d, CLKCTRL_SYS_CLK_EN0_REG2, 1);
> -static S4_PCLK(s4_i2c_m_e, CLKCTRL_SYS_CLK_EN0_REG2, 2);
> -static S4_PCLK(s4_hdmitx_apb, CLKCTRL_SYS_CLK_EN0_REG2, 4);
> -static S4_PCLK(s4_i2c_s_a, CLKCTRL_SYS_CLK_EN0_REG2, 5);
> -static S4_PCLK(s4_usb1_to_ddr, CLKCTRL_SYS_CLK_EN0_REG2, 8);
> -static S4_PCLK(s4_hdcp22, CLKCTRL_SYS_CLK_EN0_REG2, 10);
> -static S4_PCLK(s4_mmc_apb, CLKCTRL_SYS_CLK_EN0_REG2, 11);
> -static S4_PCLK(s4_rsa, CLKCTRL_SYS_CLK_EN0_REG2, 18);
> -static S4_PCLK(s4_cpu_debug, CLKCTRL_SYS_CLK_EN0_REG2, 19);
> -static S4_PCLK(s4_vpu_intr, CLKCTRL_SYS_CLK_EN0_REG2, 25);
> -static S4_PCLK(s4_demod, CLKCTRL_SYS_CLK_EN0_REG2, 27);
> -static S4_PCLK(s4_sar_adc, CLKCTRL_SYS_CLK_EN0_REG2, 28);
> -static S4_PCLK(s4_gic, CLKCTRL_SYS_CLK_EN0_REG2, 30);
> -
> -static S4_PCLK(s4_pwm_ab, CLKCTRL_SYS_CLK_EN0_REG3, 7);
> -static S4_PCLK(s4_pwm_cd, CLKCTRL_SYS_CLK_EN0_REG3, 8);
> -static S4_PCLK(s4_pwm_ef, CLKCTRL_SYS_CLK_EN0_REG3, 9);
> -static S4_PCLK(s4_pwm_gh, CLKCTRL_SYS_CLK_EN0_REG3, 10);
> -static S4_PCLK(s4_pwm_ij, CLKCTRL_SYS_CLK_EN0_REG3, 11);
> +#define S4_PCLK(_name, _reg, _bit, _flags) \
> + MESON_PCLK(_name, _reg, _bit, &s4_sys_clk.hw, _flags)
> +
> +/*
> + * NOTE: The gates below are marked with CLK_IGNORE_UNUSED for historic reasons
> + * Users are encouraged to test without it and submit changes to:
> + * - remove the flag if not necessary
> + * - replace the flag with something more adequate, such as CLK_IS_CRITICAL,
> + * if appropriate.
> + * - add a comment explaining why the use of CLK_IGNORE_UNUSED is desirable
> + * for a particular clock.
> + */
> +static S4_PCLK(s4_ddr, CLKCTRL_SYS_CLK_EN0_REG0, 0, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_dos, CLKCTRL_SYS_CLK_EN0_REG0, 1, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_ethphy, CLKCTRL_SYS_CLK_EN0_REG0, 4, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_mali, CLKCTRL_SYS_CLK_EN0_REG0, 6, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_aocpu, CLKCTRL_SYS_CLK_EN0_REG0, 13, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_aucpu, CLKCTRL_SYS_CLK_EN0_REG0, 14, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_cec, CLKCTRL_SYS_CLK_EN0_REG0, 16, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_sdemmca, CLKCTRL_SYS_CLK_EN0_REG0, 24, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_sdemmcb, CLKCTRL_SYS_CLK_EN0_REG0, 25, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_nand, CLKCTRL_SYS_CLK_EN0_REG0, 26, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_smartcard, CLKCTRL_SYS_CLK_EN0_REG0, 27, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_acodec, CLKCTRL_SYS_CLK_EN0_REG0, 28, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_spifc, CLKCTRL_SYS_CLK_EN0_REG0, 29, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_msr_clk, CLKCTRL_SYS_CLK_EN0_REG0, 30, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_ir_ctrl, CLKCTRL_SYS_CLK_EN0_REG0, 31, CLK_IGNORE_UNUSED);
> +
> +static S4_PCLK(s4_audio, CLKCTRL_SYS_CLK_EN0_REG1, 0, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_eth, CLKCTRL_SYS_CLK_EN0_REG1, 3, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_uart_a, CLKCTRL_SYS_CLK_EN0_REG1, 5, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_uart_b, CLKCTRL_SYS_CLK_EN0_REG1, 6, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_uart_c, CLKCTRL_SYS_CLK_EN0_REG1, 7, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_uart_d, CLKCTRL_SYS_CLK_EN0_REG1, 8, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_uart_e, CLKCTRL_SYS_CLK_EN0_REG1, 9, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_aififo, CLKCTRL_SYS_CLK_EN0_REG1, 11, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_ts_ddr, CLKCTRL_SYS_CLK_EN0_REG1, 15, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_ts_pll, CLKCTRL_SYS_CLK_EN0_REG1, 16, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_g2d, CLKCTRL_SYS_CLK_EN0_REG1, 20, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_spicc0, CLKCTRL_SYS_CLK_EN0_REG1, 21, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_usb, CLKCTRL_SYS_CLK_EN0_REG1, 26, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_i2c_m_a, CLKCTRL_SYS_CLK_EN0_REG1, 30, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_i2c_m_b, CLKCTRL_SYS_CLK_EN0_REG1, 31, CLK_IGNORE_UNUSED);
> +
> +static S4_PCLK(s4_i2c_m_c, CLKCTRL_SYS_CLK_EN0_REG2, 0, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_i2c_m_d, CLKCTRL_SYS_CLK_EN0_REG2, 1, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_i2c_m_e, CLKCTRL_SYS_CLK_EN0_REG2, 2, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_hdmitx_apb, CLKCTRL_SYS_CLK_EN0_REG2, 4, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_i2c_s_a, CLKCTRL_SYS_CLK_EN0_REG2, 5, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_usb1_to_ddr, CLKCTRL_SYS_CLK_EN0_REG2, 8, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_hdcp22, CLKCTRL_SYS_CLK_EN0_REG2, 10, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_mmc_apb, CLKCTRL_SYS_CLK_EN0_REG2, 11, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_rsa, CLKCTRL_SYS_CLK_EN0_REG2, 18, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_cpu_debug, CLKCTRL_SYS_CLK_EN0_REG2, 19, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_vpu_intr, CLKCTRL_SYS_CLK_EN0_REG2, 25, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_demod, CLKCTRL_SYS_CLK_EN0_REG2, 27, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_sar_adc, CLKCTRL_SYS_CLK_EN0_REG2, 28, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_gic, CLKCTRL_SYS_CLK_EN0_REG2, 30, CLK_IGNORE_UNUSED);
> +
> +static S4_PCLK(s4_pwm_ab, CLKCTRL_SYS_CLK_EN0_REG3, 7, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_pwm_cd, CLKCTRL_SYS_CLK_EN0_REG3, 8, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_pwm_ef, CLKCTRL_SYS_CLK_EN0_REG3, 9, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_pwm_gh, CLKCTRL_SYS_CLK_EN0_REG3, 10, CLK_IGNORE_UNUSED);
> +static S4_PCLK(s4_pwm_ij, CLKCTRL_SYS_CLK_EN0_REG3, 11, CLK_IGNORE_UNUSED);
>
> /* Array of all clocks provided by this provider */
> static struct clk_hw *s4_peripherals_hw_clks[] = {
>
> --
> 2.47.2
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
next prev parent reply other threads:[~2025-07-03 7:06 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-02 15:25 [PATCH 00/26] clk: amlogic: clock controllers clean-up and factorisation Jerome Brunet
2025-07-02 15:25 ` [PATCH 01/26] clk: amlogic: a1-peripherals: naming consistency alignment Jerome Brunet
2025-07-02 15:26 ` [PATCH 02/26] clk: amlogic: a1-pll: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 03/26] clk: amlogic: axg-ao: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 04/26] clk: amlogic: axg: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 05/26] clk: amlogic: c3-peripherals: " Jerome Brunet
2025-07-03 2:51 ` Chuan Liu
2025-07-03 7:48 ` Jerome Brunet
2025-07-03 8:31 ` Chuan Liu
2025-07-03 9:02 ` Jerome Brunet
2025-07-03 9:23 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 06/26] clk: amlogic: c3-pll: " Jerome Brunet
2025-07-03 2:57 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 07/26] clk: amlogic: g12a-ao: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 08/26] clk: amlogic: g12a: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 09/26] clk: amlogic: gxbb-ao: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 10/26] clk: amlogic: gxbb: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 11/26] clk: amlogic: meson8b: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 12/26] clk: amlogic: s4-peripherals: " Jerome Brunet
2025-07-03 3:18 ` Chuan Liu
2025-07-03 7:54 ` Jerome Brunet
2025-07-03 8:00 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 13/26] clk: amlogic: s4-pll: " Jerome Brunet
2025-07-03 3:19 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 14/26] clk: amlogic: meson8-ddr: " Jerome Brunet
2025-07-02 15:26 ` [PATCH 15/26] clk: amlogic: drop meson-clkcee Jerome Brunet
2025-07-02 15:26 ` [PATCH 16/26] clk: amlogic: add probe helper for mmio based controllers Jerome Brunet
2025-07-03 3:29 ` Chuan Liu
2025-07-03 8:35 ` Jerome Brunet
2025-07-02 15:26 ` [PATCH 17/26] clk: amlogic: use probe helper in " Jerome Brunet
2025-07-03 6:29 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 18/26] clk: amlogic: aoclk: use clkc-utils syscon probe Jerome Brunet
2025-07-02 15:26 ` [PATCH 19/26] clk: amlogic: move PCLK definition to clkc-utils Jerome Brunet
2025-07-02 15:26 ` [PATCH 20/26] clk: amlogic: drop CLK_SET_RATE_PARENT from peripheral clocks Jerome Brunet
2025-07-02 15:26 ` [PATCH 21/26] clk: amlogic: pclk explicitly use CLK_IGNORE_UNUSED Jerome Brunet
2025-07-03 7:05 ` Chuan Liu [this message]
2025-07-02 15:26 ` [PATCH 22/26] clk: amlogic: introduce a common pclk definition Jerome Brunet
2025-07-03 7:10 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 23/26] clk: amlogic: use the " Jerome Brunet
2025-07-03 7:16 ` Chuan Liu
2025-07-03 8:39 ` Jerome Brunet
2025-07-02 15:26 ` [PATCH 24/26] clk: amlogic: add composite clock helpers Jerome Brunet
2025-07-03 7:24 ` Chuan Liu
2025-07-03 8:39 ` Jerome Brunet
2025-07-02 15:26 ` [PATCH 25/26] clk: amlogic: align s4 and c3 pwm clock descriptions Jerome Brunet
2025-07-03 7:27 ` Chuan Liu
2025-07-02 15:26 ` [PATCH 26/26] clk: amlogic: c3-peripherals: use helper for basic composite clocks Jerome Brunet
2025-07-03 7:56 ` Chuan Liu
2025-07-03 8:44 ` Jerome Brunet
2025-08-06 7:10 ` [PATCH 00/26] clk: amlogic: clock controllers clean-up and factorisation Chuan Liu
2025-08-25 14:24 ` Jerome Brunet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=10cc2b6e-fd23-4d77-9a86-5ea91bd68e3a@amlogic.com \
--to=chuan.liu@amlogic.com \
--cc=jbrunet@baylibre.com \
--cc=khilman@baylibre.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=mturquette@baylibre.com \
--cc=neil.armstrong@linaro.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).