Re: [PATCH v2 6/6] clk: imx8mp: inform CCF of maximum frequency of clocks

[Stephen Boyd <sboyd@kernel.org>]

Quoting Ahmad Fatoum (2025-01-06 06:21:47)
> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index 516dbd170c8a356f293621b371b9ef9b9bec90a4..3b06990b73adcb2e4807f91a8983ad35068572a7 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -405,6 +406,145 @@ static const char * const imx8mp_clkout_sels[] = {"audio_pll1_out", "audio_pll2_
>  static struct clk_hw **hws;
>  static struct clk_hw_onecell_data *clk_hw_data;
>  
> +struct imx8mp_clock_constraints {
> +       int clkid;

Is it signed instead of unsigned because the terminal element of the
array has -1 as a clkid? And because clkid 0 is a valid one?

> +       u32 maxrate;
> +};
> +
> +/*
> + * Below tables are taken from IMX8MPCEC Rev. 2.1, 07/2023
> + * Table 13. Maximum frequency of modules.
> + * Probable typos fixed are marked with a comment.
> + */
> +static const struct imx8mp_clock_constraints imx8mp_clock_common_constraints[] = {
[...]
> +       { IMX8MP_CLK_MEDIA_LDB,            595 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_MEDIA_MIPI_TEST_BYTE, 200 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_ECSPI3,                80 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_PDM,                  200 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_SAI7,                  66666667 }, /* Datasheet claims 66MHz */
> +       { IMX8MP_CLK_MAIN_AXI,             400 * HZ_PER_MHZ },
> +       { /* sentinel */ },

See nitpick below.

> +};
> +
> +static const struct imx8mp_clock_constraints imx8mp_clock_nominal_constraints[] = {
[...]
> +       { IMX8MP_CLK_MEDIA_CAM1_PIX,    400 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_VPU_VC8000E,       400 * HZ_PER_MHZ }, /* Datasheet claims 500MHz */
> +       { IMX8MP_CLK_DRAM_CORE,         800 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_GIC,               400 * HZ_PER_MHZ },
> +       { /* sentinel */ },

See nitpick below.

> +};
> +
> +static const struct imx8mp_clock_constraints imx8mp_clock_overdrive_constraints[] = {
[...]
> +       { IMX8MP_CLK_VPU_VC8000E,        500 * HZ_PER_MHZ }, /* Datasheet claims 400MHz */
> +       { IMX8MP_CLK_DRAM_CORE,         1000 * HZ_PER_MHZ },
> +       { IMX8MP_CLK_GIC,                500 * HZ_PER_MHZ },
> +       { /* sentinel */ },

Please remove the comma because nothing can come after this and if
someone were to add something after they would get a compilation error.