| Message ID | 1387318102-11070-1-git-send-email-b20788@freescale.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Dec 17, 2013 at 05:08:22PM -0500, Anson Huang wrote: > if (new_freq > old_freq) { > + if (regulator_is_enabled(pu_reg)) { > + ret = regulator_set_voltage_tol(pu_reg, imx6_soc_volt[index], 0); As I pointed out while reviewing the regulator driver this is racy, nothing stops the regulator state being changed after you read it and if you skip setting the voltage then when something comes along and enables the regulator later it will be enabled with a possibly incorrect voltage.
On Tue, Dec 17, 2013 at 05:08:21PM -0500, Anson Huang wrote: > i.MX6Q needs to update vddarm, vddsoc/pu regulators when cpu freq > is changed, each setpoint has different voltage, so we need to > pass vddarm, vddsoc/pu's freq-voltage info from dts together. > > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > .../devicetree/bindings/cpufreq/cpufreq-imx6.txt | 39 ++++++++++++++++++++ The binding doc changes should generally be part of driver changes or a separate patch, but never be part of dts patch. > arch/arm/boot/dts/imx6q.dtsi | 7 ++++ > 2 files changed, 46 insertions(+) > create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt > new file mode 100644 > index 0000000..0c71dbf > --- /dev/null > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt > @@ -0,0 +1,39 @@ > +i.MX6 cpufreq driver > +------------------- > + > +i.MX6 SoC cpufreq driver for CPU frequency scaling. > + I think you need to either copy the text from cpufreq-cpu0.txt or refer to it for some necessary info such as where the properties should be defined, what required properties should be defined, and what properties are optional etc. > +Optional properties: > +-fsl,soc-operating-points: Specify vddsoc/pu voltage settings that must > + go with cpu0's operating-points. > + > +Examples: > + > + cpu@0 { > + compatible = "arm,cortex-a9"; > + device_type = "cpu"; > + reg = <0>; > + next-level-cache = <&L2>; Above properties are not related to cpufreq driver, maybe we can save them. > + operating-points = < > + /* kHz uV */ > + 1200000 1275000 > + 996000 1250000 > + 792000 1150000 > + 396000 975000 > + >; > + fsl,soc-operating-points = < > + /* ARM kHz SOC-PU uV */ > + 1200000 1275000 > + 996000 1250000 > + 792000 1175000 > + 396000 1175000 > + >; > + clock-latency = <61036>; /* two CLK32 periods */ > + clocks = <&clks 104>, <&clks 6>, <&clks 16>, > + <&clks 17>, <&clks 170>; > + clock-names = "arm", "pll2_pfd2_396m", "step", > + "pll1_sw", "pll1_sys"; > + arm-supply = <®_arm>; > + pu-supply = <®_pu>; > + soc-supply = <®_soc>; Any properties in the example that are required for a successful driver probe should be documented in 'Required properties', and others are in 'Optional properties'. Shawn > + }; > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi > index e7e8332..021e0cb 100644 > --- a/arch/arm/boot/dts/imx6q.dtsi > +++ b/arch/arm/boot/dts/imx6q.dtsi > @@ -30,6 +30,13 @@ > 792000 1150000 > 396000 975000 > >; > + fsl,soc-operating-points = < > + /* ARM kHz SOC-PU uV */ > + 1200000 1275000 > + 996000 1250000 > + 792000 1175000 > + 396000 1175000 > + >; > clock-latency = <61036>; /* two CLK32 periods */ > clocks = <&clks 104>, <&clks 6>, <&clks 16>, > <&clks 17>, <&clks 170>; > -- > 1.7.9.5 > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 17, 2013 at 05:08:22PM -0500, Anson Huang wrote: > on i.MX6Q, cpu freq change need to follow below flows: > > 1. each setpoint has different VDDARM, VDDSOC/PU voltage, get the setpoint > table from dts; > 2. when cpu freq is scaling up, need to increase VDDSOC/PU voltage before > VDDARM, if VDDPU is off, no need to change it; > 3. when cpu freq is scaling down, need to decrease VDDARM voltage before > VDDSOC/PU, if VDDPU is off, no need to change it; > > normally dts will pass vddsoc/pu freq/volt info to kernel, if not, will > use fixed value for vddsoc/pu voltage setting. > > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > drivers/cpufreq/imx6q-cpufreq.c | 117 +++++++++++++++++++++++++++++---------- > 1 file changed, 88 insertions(+), 29 deletions(-) > > diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c > index 4b3f18e..65c1df1 100644 > --- a/drivers/cpufreq/imx6q-cpufreq.c > +++ b/drivers/cpufreq/imx6q-cpufreq.c > @@ -35,6 +35,9 @@ static struct device *cpu_dev; > static struct cpufreq_frequency_table *freq_table; > static unsigned int transition_latency; > > +static u32 *imx6_soc_volt; > +static u32 soc_opp_count; > + > static unsigned int imx6q_get_speed(unsigned int cpu) > { > return clk_get_rate(arm_clk) / 1000; > @@ -69,23 +72,24 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index) > > /* scaling up? scale voltage before frequency */ > if (new_freq > old_freq) { > + if (regulator_is_enabled(pu_reg)) { > + ret = regulator_set_voltage_tol(pu_reg, imx6_soc_volt[index], 0); > + if (ret) { > + dev_err(cpu_dev, "failed to scale vddpu up: %d\n", ret); > + return ret; > + } > + } > + ret = regulator_set_voltage_tol(soc_reg, imx6_soc_volt[index], 0); > + if (ret) { > + dev_err(cpu_dev, "failed to scale vddsoc up: %d\n", ret); > + return ret; > + } > ret = regulator_set_voltage_tol(arm_reg, volt, 0); > if (ret) { > dev_err(cpu_dev, > "failed to scale vddarm up: %d\n", ret); > return ret; > } > - > - /* > - * Need to increase vddpu and vddsoc for safety > - * if we are about to run at 1.2 GHz. > - */ > - if (new_freq == FREQ_1P2_GHZ / 1000) { > - regulator_set_voltage_tol(pu_reg, > - PU_SOC_VOLTAGE_HIGH, 0); > - regulator_set_voltage_tol(soc_reg, > - PU_SOC_VOLTAGE_HIGH, 0); > - } > } > > /* > @@ -120,12 +124,17 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index) > "failed to scale vddarm down: %d\n", ret); > ret = 0; > } > - > - if (old_freq == FREQ_1P2_GHZ / 1000) { > - regulator_set_voltage_tol(pu_reg, > - PU_SOC_VOLTAGE_NORMAL, 0); > - regulator_set_voltage_tol(soc_reg, > - PU_SOC_VOLTAGE_NORMAL, 0); > + ret = regulator_set_voltage_tol(soc_reg, imx6_soc_volt[index], 0); > + if (ret) { > + dev_warn(cpu_dev, "failed to scale vddsoc down: %d\n", ret); > + ret = 0; > + } > + if (regulator_is_enabled(pu_reg)) { > + ret = regulator_set_voltage_tol(pu_reg, imx6_soc_volt[index], 0); > + if (ret) { > + dev_warn(cpu_dev, "failed to scale vddpu down: %d\n", ret); > + ret = 0; > + } > } > } > > @@ -153,6 +162,9 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) > struct dev_pm_opp *opp; > unsigned long min_volt, max_volt; > int num, ret; > + const struct property *prop; > + const __be32 *val; > + u32 nr, i, j; > > cpu_dev = get_cpu_device(0); > if (!cpu_dev) { > @@ -201,10 +213,69 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) > goto put_node; > } > > + /* Make imx6_soc_volt array's size same as arm opp number */ > + imx6_soc_volt = devm_kzalloc(cpu_dev, sizeof(*imx6_soc_volt) * num, GFP_KERNEL); > + if (imx6_soc_volt == NULL) { > + dev_warn(cpu_dev, "No valid memory for imx6_soc_volt!\n"); Drop this message. Error code -ENOMEM is enough to tell what's going wrong. > + ret = -ENOMEM; > + goto free_freq_table; > + } > + > + prop = of_find_property(np, "fsl,soc-operating-points", NULL); > + if (!prop || !prop->value) { > + dev_warn(cpu_dev, "No valid fsl,soc-operating-points property is found!\n"); > + goto soc_opp_out; > + } > + > + /* > + * Each OPP is a set of tuples consisting of frequency and > + * voltage like <freq-kHz vol-uV>. > + */ > + nr = prop->length / sizeof(u32); > + if (nr % 2 || (nr / 2) < num) { > + dev_warn(cpu_dev, "Invalid fsl,soc-operating-points list!\n"); > + goto soc_opp_out; > + } > + > + rcu_read_lock(); You do not need this lock any more. > + for (j = 0; j < num; j++) { > + val = prop->value; > + for (i = 0; i < nr / 2; i++) { > + unsigned long freq = be32_to_cpup(val++); > + unsigned long volt = be32_to_cpup(val++); > + if (freq_table[j].frequency == freq) { > + imx6_soc_volt[soc_opp_count++] = volt; > + break; > + } > + } > + } > + rcu_read_unlock(); > + > +soc_opp_out: > + /* use fixed soc opp volt if no valid soc opp info found in dtb */ > + if (soc_opp_count != num) { > + dev_warn(cpu_dev, "can NOT find valid soc opp info in dtb, use default value!\n"); It will be a little noisy for old DTB case with print so many warnings. Maybe we should drop above two dev_warn() in fsl,soc-operating-points property check and do once here. > + for (j = 0; j < num; j++) > + imx6_soc_volt[j] = PU_SOC_VOLTAGE_NORMAL; > + if (freq_table[num - 1].frequency * 1000 == FREQ_1P2_GHZ) > + imx6_soc_volt[num - 1] = PU_SOC_VOLTAGE_HIGH; > + } > + > if (of_property_read_u32(np, "clock-latency", &transition_latency)) > transition_latency = CPUFREQ_ETERNAL; > > /* > + * Calculate the ramp time for max voltage change in the > + * VDDSOC and VDDPU regulators. > + */ > + ret = regulator_set_voltage_time(soc_reg, imx6_soc_volt[0], imx6_soc_volt[num - 1]); > + if (ret > 0) > + transition_latency += ret * 1000; > + ret = regulator_set_voltage_time(soc_reg, imx6_soc_volt[0], imx6_soc_volt[num - 1]); One of the regulator_set_voltage_time() should be for pu_reg. Shawn > + if (ret > 0) > + transition_latency += ret * 1000; > + > + /* > * OPP is maintained in order of increasing frequency, and > * freq_table initialised from OPP is therefore sorted in the > * same order. > @@ -221,18 +292,6 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) > if (ret > 0) > transition_latency += ret * 1000; > > - /* Count vddpu and vddsoc latency in for 1.2 GHz support */ > - if (freq_table[num].frequency == FREQ_1P2_GHZ / 1000) { > - ret = regulator_set_voltage_time(pu_reg, PU_SOC_VOLTAGE_NORMAL, > - PU_SOC_VOLTAGE_HIGH); > - if (ret > 0) > - transition_latency += ret * 1000; > - ret = regulator_set_voltage_time(soc_reg, PU_SOC_VOLTAGE_NORMAL, > - PU_SOC_VOLTAGE_HIGH); > - if (ret > 0) > - transition_latency += ret * 1000; > - } > - > ret = cpufreq_register_driver(&imx6q_cpufreq_driver); > if (ret) { > dev_err(cpu_dev, "failed register driver: %d\n", ret); > -- > 1.7.9.5 > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 18, 2013 at 03:17:42PM -0500, Anson Huang wrote: > > > + operating-points = < > > > + /* kHz uV */ > > > + 1200000 1275000 > > > + 996000 1250000 > > > + 792000 1150000 > > > + 396000 975000 > > > + >; > > > + fsl,soc-operating-points = < > > > + /* ARM kHz SOC-PU uV */ > > > + 1200000 1275000 > > > + 996000 1250000 > > > + 792000 1175000 > > > + 396000 1175000 > > > + >; > > > + clock-latency = <61036>; /* two CLK32 periods */ > > > + clocks = <&clks 104>, <&clks 6>, <&clks 16>, > > > + <&clks 17>, <&clks 170>; > > > + clock-names = "arm", "pll2_pfd2_396m", "step", > > > + "pll1_sw", "pll1_sys"; > > > + arm-supply = <®_arm>; > > > + pu-supply = <®_pu>; > > > + soc-supply = <®_soc>; > > > > Any properties in the example that are required for a successful driver > > probe should be documented in 'Required properties', and others are in > > 'Optional properties'. > > I will not list so many properties as example as long as I describe this > preoperties should be included in cpu@0's node? The example is good. But you also need to document properties clocks, clock-names, arm-supply etc. Shawn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 18, 2013 at 03:33:23PM +0800, Shawn Guo wrote: > On Tue, Dec 17, 2013 at 05:08:22PM -0500, Anson Huang wrote: > > on i.MX6Q, cpu freq change need to follow below flows: > > > > 1. each setpoint has different VDDARM, VDDSOC/PU voltage, get the setpoint > > table from dts; > > 2. when cpu freq is scaling up, need to increase VDDSOC/PU voltage before > > VDDARM, if VDDPU is off, no need to change it; > > 3. when cpu freq is scaling down, need to decrease VDDARM voltage before > > VDDSOC/PU, if VDDPU is off, no need to change it; > > > > normally dts will pass vddsoc/pu freq/volt info to kernel, if not, will > > use fixed value for vddsoc/pu voltage setting. > > > > Signed-off-by: Anson Huang <b20788@freescale.com> > > --- > > drivers/cpufreq/imx6q-cpufreq.c | 117 +++++++++++++++++++++++++++++---------- > > 1 file changed, 88 insertions(+), 29 deletions(-) > > > > diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c > > index 4b3f18e..65c1df1 100644 > > --- a/drivers/cpufreq/imx6q-cpufreq.c > > +++ b/drivers/cpufreq/imx6q-cpufreq.c > > @@ -35,6 +35,9 @@ static struct device *cpu_dev; > > static struct cpufreq_frequency_table *freq_table; > > static unsigned int transition_latency; > > > > +static u32 *imx6_soc_volt; > > +static u32 soc_opp_count; > > + > > static unsigned int imx6q_get_speed(unsigned int cpu) > > { > > return clk_get_rate(arm_clk) / 1000; > > @@ -69,23 +72,24 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index) > > > > /* scaling up? scale voltage before frequency */ > > if (new_freq > old_freq) { > > + if (regulator_is_enabled(pu_reg)) { > > + ret = regulator_set_voltage_tol(pu_reg, imx6_soc_volt[index], 0); > > + if (ret) { > > + dev_err(cpu_dev, "failed to scale vddpu up: %d\n", ret); > > + return ret; > > + } > > + } > > + ret = regulator_set_voltage_tol(soc_reg, imx6_soc_volt[index], 0); > > + if (ret) { > > + dev_err(cpu_dev, "failed to scale vddsoc up: %d\n", ret); > > + return ret; > > + } > > ret = regulator_set_voltage_tol(arm_reg, volt, 0); > > if (ret) { > > dev_err(cpu_dev, > > "failed to scale vddarm up: %d\n", ret); > > return ret; > > } > > - > > - /* > > - * Need to increase vddpu and vddsoc for safety > > - * if we are about to run at 1.2 GHz. > > - */ > > - if (new_freq == FREQ_1P2_GHZ / 1000) { > > - regulator_set_voltage_tol(pu_reg, > > - PU_SOC_VOLTAGE_HIGH, 0); > > - regulator_set_voltage_tol(soc_reg, > > - PU_SOC_VOLTAGE_HIGH, 0); > > - } > > } > > > > /* > > @@ -120,12 +124,17 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index) > > "failed to scale vddarm down: %d\n", ret); > > ret = 0; > > } > > - > > - if (old_freq == FREQ_1P2_GHZ / 1000) { > > - regulator_set_voltage_tol(pu_reg, > > - PU_SOC_VOLTAGE_NORMAL, 0); > > - regulator_set_voltage_tol(soc_reg, > > - PU_SOC_VOLTAGE_NORMAL, 0); > > + ret = regulator_set_voltage_tol(soc_reg, imx6_soc_volt[index], 0); > > + if (ret) { > > + dev_warn(cpu_dev, "failed to scale vddsoc down: %d\n", ret); > > + ret = 0; > > + } > > + if (regulator_is_enabled(pu_reg)) { > > + ret = regulator_set_voltage_tol(pu_reg, imx6_soc_volt[index], 0); > > + if (ret) { > > + dev_warn(cpu_dev, "failed to scale vddpu down: %d\n", ret); > > + ret = 0; > > + } > > } > > } > > > > @@ -153,6 +162,9 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) > > struct dev_pm_opp *opp; > > unsigned long min_volt, max_volt; > > int num, ret; > > + const struct property *prop; > > + const __be32 *val; > > + u32 nr, i, j; > > > > cpu_dev = get_cpu_device(0); > > if (!cpu_dev) { > > @@ -201,10 +213,69 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) > > goto put_node; > > } > > > > + /* Make imx6_soc_volt array's size same as arm opp number */ > > + imx6_soc_volt = devm_kzalloc(cpu_dev, sizeof(*imx6_soc_volt) * num, GFP_KERNEL); > > + if (imx6_soc_volt == NULL) { > > + dev_warn(cpu_dev, "No valid memory for imx6_soc_volt!\n"); > > Drop this message. Error code -ENOMEM is enough to tell what's going > wrong. Oh, sorry, I forget this one, you have told me in last patch's comment. Will drop it in V3. > > > + ret = -ENOMEM; > > + goto free_freq_table; > > + } > > + > > + prop = of_find_property(np, "fsl,soc-operating-points", NULL); > > + if (!prop || !prop->value) { > > + dev_warn(cpu_dev, "No valid fsl,soc-operating-points property is found!\n"); > > + goto soc_opp_out; > > + } > > + > > + /* > > + * Each OPP is a set of tuples consisting of frequency and > > + * voltage like <freq-kHz vol-uV>. > > + */ > > + nr = prop->length / sizeof(u32); > > + if (nr % 2 || (nr / 2) < num) { > > + dev_warn(cpu_dev, "Invalid fsl,soc-operating-points list!\n"); > > + goto soc_opp_out; > > + } > > + > > + rcu_read_lock(); > > You do not need this lock any more. Right, the lock is only required by opp_find_freq_exact routine, will remove it in V3. > > > + for (j = 0; j < num; j++) { > > + val = prop->value; > > + for (i = 0; i < nr / 2; i++) { > > + unsigned long freq = be32_to_cpup(val++); > > + unsigned long volt = be32_to_cpup(val++); > > + if (freq_table[j].frequency == freq) { > > + imx6_soc_volt[soc_opp_count++] = volt; > > + break; > > + } > > + } > > + } > > + rcu_read_unlock(); > > + > > +soc_opp_out: > > + /* use fixed soc opp volt if no valid soc opp info found in dtb */ > > + if (soc_opp_count != num) { > > + dev_warn(cpu_dev, "can NOT find valid soc opp info in dtb, use default value!\n"); > > It will be a little noisy for old DTB case with print so many warnings. > Maybe we should drop above two dev_warn() in fsl,soc-operating-points > property check and do once here. Agreed, will do that in V3. > > > + for (j = 0; j < num; j++) > > + imx6_soc_volt[j] = PU_SOC_VOLTAGE_NORMAL; > > + if (freq_table[num - 1].frequency * 1000 == FREQ_1P2_GHZ) > > + imx6_soc_volt[num - 1] = PU_SOC_VOLTAGE_HIGH; > > + } > > + > > if (of_property_read_u32(np, "clock-latency", &transition_latency)) > > transition_latency = CPUFREQ_ETERNAL; > > > > /* > > + * Calculate the ramp time for max voltage change in the > > + * VDDSOC and VDDPU regulators. > > + */ > > + ret = regulator_set_voltage_time(soc_reg, imx6_soc_volt[0], imx6_soc_volt[num - 1]); > > + if (ret > 0) > > + transition_latency += ret * 1000; > > + ret = regulator_set_voltage_time(soc_reg, imx6_soc_volt[0], imx6_soc_volt[num - 1]); > > One of the regulator_set_voltage_time() should be for pu_reg. Yes, I saw this right after sending out V2 patch set, will do that in V3. > > Shawn > > > + if (ret > 0) > > + transition_latency += ret * 1000; > > + > > + /* > > * OPP is maintained in order of increasing frequency, and > > * freq_table initialised from OPP is therefore sorted in the > > * same order. > > @@ -221,18 +292,6 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) > > if (ret > 0) > > transition_latency += ret * 1000; > > > > - /* Count vddpu and vddsoc latency in for 1.2 GHz support */ > > - if (freq_table[num].frequency == FREQ_1P2_GHZ / 1000) { > > - ret = regulator_set_voltage_time(pu_reg, PU_SOC_VOLTAGE_NORMAL, > > - PU_SOC_VOLTAGE_HIGH); > > - if (ret > 0) > > - transition_latency += ret * 1000; > > - ret = regulator_set_voltage_time(soc_reg, PU_SOC_VOLTAGE_NORMAL, > > - PU_SOC_VOLTAGE_HIGH); > > - if (ret > 0) > > - transition_latency += ret * 1000; > > - } > > - > > ret = cpufreq_register_driver(&imx6q_cpufreq_driver); > > if (ret) { > > dev_err(cpu_dev, "failed register driver: %d\n", ret); > > -- > > 1.7.9.5 > > > > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 18, 2013 at 03:05:39PM +0800, Shawn Guo wrote: > On Tue, Dec 17, 2013 at 05:08:21PM -0500, Anson Huang wrote: > > i.MX6Q needs to update vddarm, vddsoc/pu regulators when cpu freq > > is changed, each setpoint has different voltage, so we need to > > pass vddarm, vddsoc/pu's freq-voltage info from dts together. > > > > Signed-off-by: Anson Huang <b20788@freescale.com> > > --- > > .../devicetree/bindings/cpufreq/cpufreq-imx6.txt | 39 ++++++++++++++++++++ > > The binding doc changes should generally be part of driver changes or a > separate patch, but never be part of dts patch. > > > arch/arm/boot/dts/imx6q.dtsi | 7 ++++ > > 2 files changed, 46 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt > > > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt > > new file mode 100644 > > index 0000000..0c71dbf > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt > > @@ -0,0 +1,39 @@ > > +i.MX6 cpufreq driver > > +------------------- > > + > > +i.MX6 SoC cpufreq driver for CPU frequency scaling. > > + > > I think you need to either copy the text from cpufreq-cpu0.txt or refer > to it for some necessary info such as where the properties should be > defined, what required properties should be defined, and what properties > are optional etc. Right, I was confused before and not sure how to do that, so I copy the whole cpu@0's dts as example, will add info to let user know this is a sub property of cpu@0, and refer to cpufreq-cpu0.txt. > > > +Optional properties: > > +-fsl,soc-operating-points: Specify vddsoc/pu voltage settings that must > > + go with cpu0's operating-points. > > + > > +Examples: > > + > > + cpu@0 { > > + compatible = "arm,cortex-a9"; > > + device_type = "cpu"; > > + reg = <0>; > > + next-level-cache = <&L2>; > > Above properties are not related to cpufreq driver, maybe we can save > them. Yes, with above change, I can remove these properties. > > > + operating-points = < > > + /* kHz uV */ > > + 1200000 1275000 > > + 996000 1250000 > > + 792000 1150000 > > + 396000 975000 > > + >; > > + fsl,soc-operating-points = < > > + /* ARM kHz SOC-PU uV */ > > + 1200000 1275000 > > + 996000 1250000 > > + 792000 1175000 > > + 396000 1175000 > > + >; > > + clock-latency = <61036>; /* two CLK32 periods */ > > + clocks = <&clks 104>, <&clks 6>, <&clks 16>, > > + <&clks 17>, <&clks 170>; > > + clock-names = "arm", "pll2_pfd2_396m", "step", > > + "pll1_sw", "pll1_sys"; > > + arm-supply = <®_arm>; > > + pu-supply = <®_pu>; > > + soc-supply = <®_soc>; > > Any properties in the example that are required for a successful driver > probe should be documented in 'Required properties', and others are in > 'Optional properties'. I will not list so many properties as example as long as I describe this preoperties should be included in cpu@0's node? > > Shawn > > > + }; > > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi > > index e7e8332..021e0cb 100644 > > --- a/arch/arm/boot/dts/imx6q.dtsi > > +++ b/arch/arm/boot/dts/imx6q.dtsi > > @@ -30,6 +30,13 @@ > > 792000 1150000 > > 396000 975000 > > >; > > + fsl,soc-operating-points = < > > + /* ARM kHz SOC-PU uV */ > > + 1200000 1275000 > > + 996000 1250000 > > + 792000 1175000 > > + 396000 1175000 > > + >; > > clock-latency = <61036>; /* two CLK32 periods */ > > clocks = <&clks 104>, <&clks 6>, <&clks 16>, > > <&clks 17>, <&clks 170>; > > -- > > 1.7.9.5 > > > > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt new file mode 100644 index 0000000..0c71dbf --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt @@ -0,0 +1,39 @@ +i.MX6 cpufreq driver +------------------- + +i.MX6 SoC cpufreq driver for CPU frequency scaling. + +Optional properties: +-fsl,soc-operating-points: Specify vddsoc/pu voltage settings that must + go with cpu0's operating-points. + +Examples: + + cpu@0 { + compatible = "arm,cortex-a9"; + device_type = "cpu"; + reg = <0>; + next-level-cache = <&L2>; + operating-points = < + /* kHz uV */ + 1200000 1275000 + 996000 1250000 + 792000 1150000 + 396000 975000 + >; + fsl,soc-operating-points = < + /* ARM kHz SOC-PU uV */ + 1200000 1275000 + 996000 1250000 + 792000 1175000 + 396000 1175000 + >; + clock-latency = <61036>; /* two CLK32 periods */ + clocks = <&clks 104>, <&clks 6>, <&clks 16>, + <&clks 17>, <&clks 170>; + clock-names = "arm", "pll2_pfd2_396m", "step", + "pll1_sw", "pll1_sys"; + arm-supply = <®_arm>; + pu-supply = <®_pu>; + soc-supply = <®_soc>; + }; diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index e7e8332..021e0cb 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -30,6 +30,13 @@ 792000 1150000 396000 975000 >; + fsl,soc-operating-points = < + /* ARM kHz SOC-PU uV */ + 1200000 1275000 + 996000 1250000 + 792000 1175000 + 396000 1175000 + >; clock-latency = <61036>; /* two CLK32 periods */ clocks = <&clks 104>, <&clks 6>, <&clks 16>, <&clks 17>, <&clks 170>;
i.MX6Q needs to update vddarm, vddsoc/pu regulators when cpu freq is changed, each setpoint has different voltage, so we need to pass vddarm, vddsoc/pu's freq-voltage info from dts together. Signed-off-by: Anson Huang <b20788@freescale.com> --- .../devicetree/bindings/cpufreq/cpufreq-imx6.txt | 39 ++++++++++++++++++++ arch/arm/boot/dts/imx6q.dtsi | 7 ++++ 2 files changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-imx6.txt