diff mbox series

cpufreq: imx6q: Only disabling 792MHz OPP for i.MX6ULL types below 792MHz

Message ID 20231107163136.63440-1-cniedermaier@dh-electronics.com (mailing list archive)
State New
Delegated to: viresh kumar
Headers show
Series cpufreq: imx6q: Only disabling 792MHz OPP for i.MX6ULL types below 792MHz | expand

Commit Message

Christoph Niedermaier Nov. 7, 2023, 4:31 p.m. UTC 
For a 900MHz i.MX6ULL CPU the 792MHz OPP is disabled. There is no
convincing reason to disable this OPP. If a CPU can run at 900MHz,
it should also be able to cope with 792MHz. Looking at the voltage
level of 792MHz in [1] (page 24, table 10. "Operating Ranges") the
current defined OPP is above the minimum. So the voltage level
shouldn't be a problem. Although in [2] (page 24, table 10.
"Operating Ranges") 792MHz isn't mentioned there isn't note that
792MHz OPP isn't allowed. Change it to only disable 792MHz OPP for
i.MX6ULL types below 792 MHz.

[1] https://www.nxp.com/docs/en/data-sheet/IMX6ULLIEC.pdf
[2] https://www.nxp.com/docs/en/data-sheet/IMX6ULLCEC.pdf

Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Reviewed-by: Marek Vasut <marex@denx.de>
---
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@denx.de>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
To: linux-arm-kernel@lists.infradead.org
---
 drivers/cpufreq/imx6q-cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Fabio Estevam Nov. 7, 2023, 5:47 p.m. UTC | #1 
Hi Christoph,

Thanks for your patch.

On 07/11/2023 13:31, Christoph Niedermaier wrote:
> For a 900MHz i.MX6ULL CPU the 792MHz OPP is disabled. There is no
> convincing reason to disable this OPP. If a CPU can run at 900MHz,
> it should also be able to cope with 792MHz. Looking at the voltage
> level of 792MHz in [1] (page 24, table 10. "Operating Ranges") the
> current defined OPP is above the minimum. So the voltage level
> shouldn't be a problem. Although in [2] (page 24, table 10.
> "Operating Ranges") 792MHz isn't mentioned there isn't note that
> 792MHz OPP isn't allowed. Change it to only disable 792MHz OPP for

I find this part:

"792MHz isn't mentioned there isn't note that 792MHz OPP isn't allowed."

a bit confusing.

Maybe:

"However in [2] (page 24, table 10. "Operating Ranges"), it is not 
mentioned that 792MHz OPP isn't allowed."


> [1] https://www.nxp.com/docs/en/data-sheet/IMX6ULLIEC.pdf
> [2] https://www.nxp.com/docs/en/data-sheet/IMX6ULLCEC.pdf
> 
> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Reviewed-by: Marek Vasut <marex@denx.de>

I think it is worth adding a Fixes tag so that this fix could be 
backported
to stable kernels.

Reviewed-by: Fabio Estevam <festevam@denx.de>

Thanks
Christoph Niedermaier Nov. 8, 2023, 10:04 a.m. UTC | #2 
From: Fabio Estevam
Sent: Tuesday, November 7, 2023 6:48 PM
> 
> Hi Christoph,
> 
> Thanks for your patch.
> 
> On 07/11/2023 13:31, Christoph Niedermaier wrote:
>> For a 900MHz i.MX6ULL CPU the 792MHz OPP is disabled. There is no
>> convincing reason to disable this OPP. If a CPU can run at 900MHz,
>> it should also be able to cope with 792MHz. Looking at the voltage
>> level of 792MHz in [1] (page 24, table 10. "Operating Ranges") the
>> current defined OPP is above the minimum. So the voltage level
>> shouldn't be a problem. Although in [2] (page 24, table 10.
>> "Operating Ranges") 792MHz isn't mentioned there isn't note that
>> 792MHz OPP isn't allowed. Change it to only disable 792MHz OPP for
> 
> I find this part:
> 
> "792MHz isn't mentioned there isn't note that 792MHz OPP isn't allowed."
> 
> a bit confusing.
> 
> Maybe:
> 
> "However in [2] (page 24, table 10. "Operating Ranges"), it is not
> mentioned that 792MHz OPP isn't allowed."

You are right, your suggestion expresses it more clearly.
I will change it in V2.

> 
> 
>> [1] https://www.nxp.com/docs/en/data-sheet/IMX6ULLIEC.pdf
>> [2] https://www.nxp.com/docs/en/data-sheet/IMX6ULLCEC.pdf
>>
>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
>> Reviewed-by: Marek Vasut <marex@denx.de>
> 
> I think it is worth adding a Fixes tag so that this fix could be
> backported
> to stable kernels.

You mean the following fixes tag when the change was introduced?
Fixes: 0aa9abd4c212 ("cpufreq: imx6q: check speed grades for i.MX6ULL")
But if I'm right, then it goes down to version 4.19 and maybe the
commit 11a3b0ac33d9 ("cpufreq: imx6q: don't warn for disabling a non-existing frequency")
is also needed to easily apply.

Regards
Christoph
diff mbox series

Patch

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 494d044b9e72..33728c242f66 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -327,7 +327,7 @@  static int imx6ul_opp_check_speed_grading(struct device *dev)
 			imx6x_disable_freq_in_opp(dev, 696000000);
 
 	if (of_machine_is_compatible("fsl,imx6ull")) {
-		if (val != OCOTP_CFG3_6ULL_SPEED_792MHZ)
+		if (val < OCOTP_CFG3_6ULL_SPEED_792MHZ)
 			imx6x_disable_freq_in_opp(dev, 792000000);
 
 		if (val != OCOTP_CFG3_6ULL_SPEED_900MHZ)