linux-kernel - Re: [PATCH V2 2/2] clk: imx: scu: add cpu frequency scaling support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <59b181b77605f516b0e3a896ddce0224b4504b30.camel@nxp.com>
Date:   Wed, 13 Feb 2019 13:53:04 +0000
From:   Leonard Crestez <leonard.crestez@....com>
To:     Anson Huang <anson.huang@....com>
CC:     dl-linux-imx <linux-imx@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        Aisheng Dong <aisheng.dong@....com>,
        Daniel Baluta <daniel.baluta@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "viresh.kumar@...aro.org" <viresh.kumar@...aro.org>
Subject: Re: [PATCH V2 2/2] clk: imx: scu: add cpu frequency scaling support

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?