[PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

[PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

According to latest datasheet (Rev.1, 10/2018) from below links,
in the consumer datasheet, 1.5GHz is mentioned as highest opp but
depends on speed grading fuse, and in the industrial datasheet,
1.3GHz is mentioned as highest opp but depends on speed grading
fuse. 1.5GHz and 1.3GHz opp use same voltage, so no need for
consumer part to support 1.3GHz opp, with same voltage, CPU should
run at highest frequency in order to go into idle as quick as
possible, this can save power.

That means for consumer part, 1GHz/1.5GHz are supported, for
industrial part, 800MHz/1.3GHz are supported, and then check the
speed grading fuse to limit the highest CPU frequency further.
Correct the market segment bits in opp table to make them work
according to datasheets.

https://www.nxp.com/docs/en/data-sheet/IMX8MDQLQIEC.pdf
https://www.nxp.com/docs/en/data-sheet/IMX8MDQLQCEC.pdf

Fixes: 12629c5c3749 ("arm64: dts: imx8mq: Add cpu speed grading and all OPPs")
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 9d99191..bea53bc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -169,7 +169,8 @@
 		opp-1300000000 {
 			opp-hz = /bits/ 64 <1300000000>;
 			opp-microvolt = <1000000>;
-			opp-supported-hw = <0xc>, <0x7>;
+			/* Industrial only but rely on speed grading */
+			opp-supported-hw = <0xc>, <0x4>;
 			clock-latency-ns = <150000>;
 		};
 
@@ -177,7 +178,7 @@
 			opp-hz = /bits/ 64 <1500000000>;
 			opp-microvolt = <1000000>;
 			/* Consumer only but rely on speed grading */
-			opp-supported-hw = <0x8>, <0x7>;
+			opp-supported-hw = <0x8>, <0x3>;
 			clock-latency-ns = <150000>;
 		};
 	};
-- 
2.7.4

[PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to latest datasheet

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

According to latest datasheet (Rev.0.2, 04/2019) from below links,
1.8GHz is ONLY available for consumer part, so the market segment
bits for 1.8GHz opp should ONLY available for consumer part accordingly.

https://www.nxp.com/docs/en/data-sheet/IMX8MMIEC.pdf
https://www.nxp.com/docs/en/data-sheet/IMX8MMCEC.pdf

Fixes: f403a26c865b (arm64: dts: imx8mm: Add cpu speed grading and all OPPs)
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c38813d..ab0d135 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -135,7 +135,7 @@
 			opp-hz = /bits/ 64 <1800000000>;
 			opp-microvolt = <1000000>;
 			/* Consumer only but rely on speed grading */
-			opp-supported-hw = <0x8>, <0x7>;
+			opp-supported-hw = <0x8>, <0x3>;
 			clock-latency-ns = <150000>;
 		};
 	};
-- 
2.7.4

Re: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

[Leonard Crestez]

On 28.06.2019 06:37, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> According to latest datasheet (Rev.1, 10/2018) from below links,
> in the consumer datasheet, 1.5GHz is mentioned as highest opp but
> depends on speed grading fuse, and in the industrial datasheet,
> 1.3GHz is mentioned as highest opp but depends on speed grading
> fuse. 1.5GHz and 1.3GHz opp use same voltage, so no need for
> consumer part to support 1.3GHz opp, with same voltage, CPU should
> run at highest frequency in order to go into idle as quick as
> possible, this can save power.

I looked at the same datasheets and it's not clear to me that 1.3 Ghz 
should be disallowed for consumer parts. Power consumption increases 
with both voltage and frequency so having two OPPs with same voltage 
does make sense.

>   			opp-hz = /bits/ 64 <1300000000>;
>   			opp-microvolt = <1000000>;
> -			opp-supported-hw = <0xc>, <0x7>;
> +			/* Industrial only but rely on speed grading */
> +			opp-supported-hw = <0xc>, <0x4>;

Comment is false, you're explicitly excluding consumer parts via the 
second element.

>   			opp-hz = /bits/ 64 <1500000000>;
>   			opp-microvolt = <1000000>;
>   			/* Consumer only but rely on speed grading */
> -			opp-supported-hw = <0x8>, <0x7>;
> +			opp-supported-hw = <0x8>, <0x3>;

If you don't want to rely on the fact that only consumer parts should be 
fused for 1.5 Ghz then please delete the comment.

Re: [PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to latest datasheet

[Leonard Crestez]

On 28.06.2019 06:37, Anson.Huang@nxp.com wrote:

> According to latest datasheet (Rev.0.2, 04/2019) from below links,
> 1.8GHz is ONLY available for consumer part, so the market segment
> bits for 1.8GHz opp should ONLY available for consumer part accordingly.
> 
>   			opp-hz = /bits/ 64 <1800000000>;
>   			opp-microvolt = <1000000>;
>   			/* Consumer only but rely on speed grading */
> -			opp-supported-hw = <0x8>, <0x7>;
> +			opp-supported-hw = <0x8>, <0x3>;

Only consumer parts should be fused for this highest OPP. If you don't 
want to rely on this then maybe also delete the comment above?

--
Regards,
leonard

RE: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

[Anson Huang]

Hi, Leonard

> -----Original Message-----
> From: Leonard Crestez
> Sent: Friday, June 28, 2019 1:59 PM
> To: Anson Huang <anson.huang@nxp.com>; shawnguo@kernel.org; Jacky
> Bai <ping.bai@nxp.com>; l.stach@pengutronix.de
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; viresh.kumar@linaro.org;
> Daniel Baluta <daniel.baluta@nxp.com>; Abel Vesa <abel.vesa@nxp.com>;
> andrew.smirnov@gmail.com; ccaione@baylibre.com; angus@akkea.ca;
> agx@sigxcpu.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to
> latest datasheet
> 
> On 28.06.2019 06:37, Anson.Huang@nxp.com wrote:
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > According to latest datasheet (Rev.1, 10/2018) from below links, in
> > the consumer datasheet, 1.5GHz is mentioned as highest opp but depends
> > on speed grading fuse, and in the industrial datasheet, 1.3GHz is
> > mentioned as highest opp but depends on speed grading fuse. 1.5GHz and
> > 1.3GHz opp use same voltage, so no need for consumer part to support
> > 1.3GHz opp, with same voltage, CPU should run at highest frequency in
> > order to go into idle as quick as possible, this can save power.
> 
> I looked at the same datasheets and it's not clear to me that 1.3 Ghz should
> be disallowed for consumer parts. Power consumption increases with both
> voltage and frequency so having two OPPs with same voltage does make
> sense.

The consumer part datasheet does NOT mention 1.3GHz at all, so consumer part ONLY
support 1GHz/1.5GHz, and industrial part ONLY support 800MHz/1.3GHz, this is what
we did in our internal tree and NPI release, so better to make them aligned, otherwise,
we have to change it when kernel upgrade.

And normally, with same voltage, i.MX SoCs always run at highest frequency, so it is better
to keep the rule, otherwise customer may ask, how about using same voltage to run at 1.2GHz or
1.1GHz?

> 
> >   			opp-hz = /bits/ 64 <1300000000>;
> >   			opp-microvolt = <1000000>;
> > -			opp-supported-hw = <0xc>, <0x7>;
> > +			/* Industrial only but rely on speed grading */
> > +			opp-supported-hw = <0xc>, <0x4>;
> 
> Comment is false, you're explicitly excluding consumer parts via the second
> element.

Yes, that is what I meant to do, as we no need to support 1.3GHz for consumer
part, with 1.0V, consumer part can run up to 1.5GHz.

> 
> >   			opp-hz = /bits/ 64 <1500000000>;
> >   			opp-microvolt = <1000000>;
> >   			/* Consumer only but rely on speed grading */
> > -			opp-supported-hw = <0x8>, <0x7>;
> > +			opp-supported-hw = <0x8>, <0x3>;
> 
> If you don't want to rely on the fact that only consumer parts should be
> fused for 1.5 Ghz then please delete the comment.

Don't quite understand, 1.5GHz is indeed consumer ONLY, but if the consumer
part is fused to 1GHz, then 1.5GHz is also NOT available, so it also rely on speed
grading. So keep the comment there is OK?

Thanks,
Anson.

RE: [PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to latest datasheet

[Anson Huang]

Hi, Leonard

> -----Original Message-----
> From: Leonard Crestez
> Sent: Friday, June 28, 2019 2:01 PM
> To: Anson Huang <anson.huang@nxp.com>; Jacky Bai <ping.bai@nxp.com>;
> l.stach@pengutronix.de
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> viresh.kumar@linaro.org; Daniel Baluta <daniel.baluta@nxp.com>; Abel
> Vesa <abel.vesa@nxp.com>; andrew.smirnov@gmail.com;
> ccaione@baylibre.com; angus@akkea.ca; agx@sigxcpu.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 2/2] arm64: dts: imx8mm: Correct OPP table according to
> latest datasheet
> 
> On 28.06.2019 06:37, Anson.Huang@nxp.com wrote:
> 
> > According to latest datasheet (Rev.0.2, 04/2019) from below links,
> > 1.8GHz is ONLY available for consumer part, so the market segment bits
> > for 1.8GHz opp should ONLY available for consumer part accordingly.
> >
> >   			opp-hz = /bits/ 64 <1800000000>;
> >   			opp-microvolt = <1000000>;
> >   			/* Consumer only but rely on speed grading */
> > -			opp-supported-hw = <0x8>, <0x7>;
> > +			opp-supported-hw = <0x8>, <0x3>;
> 
> Only consumer parts should be fused for this highest OPP. If you don't want
> to rely on this then maybe also delete the comment above?

As I replied in previous i.MX8MQ patch, if the comments make reader confused,
should we just remove all those comments?

Thanks,
Anson.

> 
> --
> Regards,
> leonard

Re: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

[Leonard Crestez]

On 6/28/2019 9:16 AM, Anson Huang wrote:
>> From: Leonard Crestez
>>> From: Anson Huang <Anson.Huang@nxp.com>
>>>
>>> According to latest datasheet (Rev.1, 10/2018) from below links, in
>>> the consumer datasheet, 1.5GHz is mentioned as highest opp but depends
>>> on speed grading fuse, and in the industrial datasheet, 1.3GHz is
>>> mentioned as highest opp but depends on speed grading fuse. 1.5GHz and
>>> 1.3GHz opp use same voltage, so no need for consumer part to support
>>> 1.3GHz opp, with same voltage, CPU should run at highest frequency in
>>> order to go into idle as quick as possible, this can save power.
>>
>> I looked at the same datasheets and it's not clear to me that 1.3 Ghz should
>> be disallowed for consumer parts. Power consumption increases with both
>> voltage and frequency so having two OPPs with same voltage does make
>> sense.
> 
> The consumer part datasheet does NOT mention 1.3GHz at all, so consumer part ONLY
> support 1GHz/1.5GHz, and industrial part ONLY support 800MHz/1.3GHz, this is what
> we did in our internal tree and NPI release, so better to make them aligned, otherwise,
> we have to change it when kernel upgrade.

Datasheet seems ambiguous: it mentions "max freq for volt" so my 
understanding is that any lower freqs should also work at that voltage.

This also seems to be the understanding behind commit 8cfd813c7307 
("arm64: dts: imx8mq: fix higher CPU operating point") by Lucas.

On datasheet page 7 it mentions that product code can have "JZ" or "HZ" 
for 1.3Ghz or 1.5Ghz. Are you saying that only industrial parts can be "JZ"?

>>>    			opp-hz = /bits/ 64 <1500000000>;
>>>    			opp-microvolt = <1000000>;
>>>    			/* Consumer only but rely on speed grading */
>>> -			opp-supported-hw = <0x8>, <0x7>;
>>> +			opp-supported-hw = <0x8>, <0x3>;
>>
>> If you don't want to rely on the fact that only consumer parts should be
>> fused for 1.5 Ghz then please delete the comment.
> 
> Don't quite understand, 1.5GHz is indeed consumer ONLY, but if the consumer
> part is fused to 1GHz, then 1.5GHz is also NOT available, so it also rely on speed
> grading. So keep the comment there is OK?

What I meant with that comment is that 1.5Ghz is only mentioned for 
consumer parts but instead of explicitly banning it on industrial parts 
we rely on MFG never fusing industrial parts for 1.5Ghz.

Now you're explicitly banning it on industrial parts.

This comment is indeed confusing so better to just remove all instances.

RE: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to latest datasheet

[Anson Huang]

> -----Original Message-----
> From: Leonard Crestez
> Sent: Friday, June 28, 2019 2:45 PM
> To: Anson Huang <anson.huang@nxp.com>; Jacky Bai <ping.bai@nxp.com>;
> l.stach@pengutronix.de
> Cc: shawnguo@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> viresh.kumar@linaro.org; Daniel Baluta <daniel.baluta@nxp.com>; Abel
> Vesa <abel.vesa@nxp.com>; andrew.smirnov@gmail.com;
> ccaione@baylibre.com; angus@akkea.ca; agx@sigxcpu.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] arm64: dts: imx8mq: Correct OPP table according to
> latest datasheet
> 
> On 6/28/2019 9:16 AM, Anson Huang wrote:
> >> From: Leonard Crestez
> >>> From: Anson Huang <Anson.Huang@nxp.com>
> >>>
> >>> According to latest datasheet (Rev.1, 10/2018) from below links, in
> >>> the consumer datasheet, 1.5GHz is mentioned as highest opp but
> >>> depends on speed grading fuse, and in the industrial datasheet,
> >>> 1.3GHz is mentioned as highest opp but depends on speed grading
> >>> fuse. 1.5GHz and 1.3GHz opp use same voltage, so no need for
> >>> consumer part to support 1.3GHz opp, with same voltage, CPU should
> >>> run at highest frequency in order to go into idle as quick as possible, this
> can save power.
> >>
> >> I looked at the same datasheets and it's not clear to me that 1.3 Ghz
> >> should be disallowed for consumer parts. Power consumption increases
> >> with both voltage and frequency so having two OPPs with same voltage
> >> does make sense.
> >
> > The consumer part datasheet does NOT mention 1.3GHz at all, so
> > consumer part ONLY support 1GHz/1.5GHz, and industrial part ONLY
> > support 800MHz/1.3GHz, this is what we did in our internal tree and
> > NPI release, so better to make them aligned, otherwise, we have to change
> it when kernel upgrade.
> 
> Datasheet seems ambiguous: it mentions "max freq for volt" so my
> understanding is that any lower freqs should also work at that voltage.
> 
> This also seems to be the understanding behind commit 8cfd813c7307
> ("arm64: dts: imx8mq: fix higher CPU operating point") by Lucas. 
> 
> On datasheet page 7 it mentions that product code can have "JZ" or "HZ"
> for 1.3Ghz or 1.5Ghz. Are you saying that only industrial parts can be "JZ"?

If take a look at page 6 table2, industrial datasheet ONLY has "HZ", and consumer
Datasheet ONLY has "JZ". And yes, that is my understanding.

And considering our rule, I don't think is benefit for consumer part to run 1.3GHz.

> 
> >>>    			opp-hz = /bits/ 64 <1500000000>;
> >>>    			opp-microvolt = <1000000>;
> >>>    			/* Consumer only but rely on speed grading */
> >>> -			opp-supported-hw = <0x8>, <0x7>;
> >>> +			opp-supported-hw = <0x8>, <0x3>;
> >>
> >> If you don't want to rely on the fact that only consumer parts should
> >> be fused for 1.5 Ghz then please delete the comment.
> >
> > Don't quite understand, 1.5GHz is indeed consumer ONLY, but if the
> > consumer part is fused to 1GHz, then 1.5GHz is also NOT available, so
> > it also rely on speed grading. So keep the comment there is OK?
> 
> What I meant with that comment is that 1.5Ghz is only mentioned for
> consumer parts but instead of explicitly banning it on industrial parts we rely
> on MFG never fusing industrial parts for 1.5Ghz.
> 
> Now you're explicitly banning it on industrial parts.

Yes, industrial parts never support up to 1.5GHz, so explicitly banning it is just OK.
The speed grading fuse and market segment fuse are actually has some overlap,
we better to implement both of them.

> 
> This comment is indeed confusing so better to just remove all instances.

I agree to remove those comments, no need to let it introduce any confusion.

Anson.