On Wed, 2019-02-13 at 13:32 +0000, Anson Huang wrote:
> On NXP's i.MX SoCs with system controller inside, CPU frequency
> scaling can ONLY be done by system controller firmware, and it
> can ONLY be requested from secure mode, so Linux kernel has to
> call ARM SMC to trap to ARM-Trusted-Firmware to request system
> controller firmware to do CPU frequency scaling.
> +#ifdef CONFIG_CPUFREQ_DT
> +#define IMX_SIP_CPUFREQ 0xC2000001
> +#define IMX_SIP_SET_CPUFREQ 0x00
> +
> static struct imx_sc_ipc *ccm_ipc_handle;
Without CONFIG_CPUFREQ_DT the ccm_ipc_handle won't be defined and build
will break.
But is there a good reason for ifdef? In general clock function are
compiled even if no driver is calling them.
> +#ifdef CONFIG_CPUFREQ_DT
> + /* CPU clock can ONLY be done by TF-A */
> + if (clk->clk_type == IMX_SC_PM_CLK_CPU) {
> + struct arm_smccc_res res;
> + unsigned int cluster_id;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(imx_sc_cpufreq_data); i++) {
> + if (!strcmp(clk_hw_get_name(hw),
> + imx_sc_cpufreq_data[i].clk_name)) {
> + cluster_id = imx_sc_cpufreq_data[i].cluster_id;
> + break;
> + }
> + }
> +
> + /*
> + * As some other clock types have same value as
> + * IMX_SC_PM_CLK_CPU, so we need to double check
> + * the clock being scaled is indeed CPU clock which
> + * matches the table we define.
> + */
> + if (i < ARRAY_SIZE(imx_sc_cpufreq_data)) {
> + arm_smccc_smc(IMX_SIP_CPUFREQ, IMX_SIP_SET_CPUFREQ,
> + cluster_id, rate, 0, 0, 0, 0, &res);
> + return 0;
> + }
> + }
> +#endif
The code inside the ifdef would look better in a separate
imx_clk_atf_set_rate function. Maybe even separate clk_ops?
