2017-04-25 16:58:23

by Leonard Crestez

[permalink] [raw]
Subject: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

The board file for imx6sx-dbg overrides cpufreq operating points to use
higher voltages. This is done because the board has a shared rail for
VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
needs to be a value suitable for both ARM and SOC.

This was introduced in:

commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")

This only only applies to LDO bypass mode, a feature not present in
upstream. When LDOs are enabled the effect is to use higher voltages than
necesarry for no good reason.

Setting these higher voltages can make some boards fail to boot with ugly
semi-random crashes, reminiscent of memory corruption. These failures
happen the first time the lowest idle state is used. Remove the OPP
override in order to fix those crashes.

Signed-off-by: Leonard Crestez <[email protected]>

---
It's not clear exactly why the crashes happen. Perhaps waking up from idle
draws more power than is available? Removing this override is a correct
change anyway so maybe there is no need to investigate deeper.

arch/arm/boot/dts/imx6sx-sdb.dts | 17 -----------------
1 file changed, 17 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
index 5bb8fd5..d71da30 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -12,23 +12,6 @@
model = "Freescale i.MX6 SoloX SDB RevB Board";
};

-&cpu0 {
- operating-points = <
- /* kHz uV */
- 996000 1250000
- 792000 1175000
- 396000 1175000
- 198000 1175000
- >;
- fsl,soc-operating-points = <
- /* ARM kHz SOC uV */
- 996000 1250000
- 792000 1175000
- 396000 1175000
- 198000 1175000
- >;
-};
-
&i2c1 {
clock-frequency = <100000>;
pinctrl-names = "default";
--
2.7.4


2017-04-25 17:02:15

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

Hi Leonard,

On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
<[email protected]> wrote:
> The board file for imx6sx-dbg overrides cpufreq operating points to use
> higher voltages. This is done because the board has a shared rail for
> VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
> needs to be a value suitable for both ARM and SOC.
>
> This was introduced in:
>
> commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
>
> This only only applies to LDO bypass mode, a feature not present in
> upstream. When LDOs are enabled the effect is to use higher voltages than
> necesarry for no good reason.
>
> Setting these higher voltages can make some boards fail to boot with ugly
> semi-random crashes, reminiscent of memory corruption. These failures
> happen the first time the lowest idle state is used. Remove the OPP
> override in order to fix those crashes.
>
> Signed-off-by: Leonard Crestez <[email protected]>
>
> ---
> It's not clear exactly why the crashes happen. Perhaps waking up from idle
> draws more power than is available? Removing this override is a correct
> change anyway so maybe there is no need to investigate deeper.

Marek just sent a similar one a few minutes ago:
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html

2017-04-25 17:02:59

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

On Tue, Apr 25, 2017 at 2:02 PM, Fabio Estevam <[email protected]> wrote:
> Hi Leonard,
>
> On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
> <[email protected]> wrote:
>> The board file for imx6sx-dbg overrides cpufreq operating points to use
>> higher voltages. This is done because the board has a shared rail for
>> VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
>> needs to be a value suitable for both ARM and SOC.
>>
>> This was introduced in:
>>
>> commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
>>
>> This only only applies to LDO bypass mode, a feature not present in
>> upstream. When LDOs are enabled the effect is to use higher voltages than
>> necesarry for no good reason.
>>
>> Setting these higher voltages can make some boards fail to boot with ugly
>> semi-random crashes, reminiscent of memory corruption. These failures
>> happen the first time the lowest idle state is used. Remove the OPP
>> override in order to fix those crashes.
>>
>> Signed-off-by: Leonard Crestez <[email protected]>
>>
>> ---
>> It's not clear exactly why the crashes happen. Perhaps waking up from idle
>> draws more power than is available? Removing this override is a correct
>> change anyway so maybe there is no need to investigate deeper.
>
> Marek just sent a similar one a few minutes ago:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html

Forgot to add Marek.

2017-04-25 17:23:42

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

On Tue, 2017-04-25 at 14:02 -0300, Fabio Estevam wrote:
> On Tue, Apr 25, 2017 at 2:02 PM, Fabio Estevam <[email protected]> wrote:
> >
> > Hi Leonard,
> >
> > On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
> > <[email protected]> wrote:
> > >
> > > The board file for imx6sx-dbg overrides cpufreq operating points to use
> > > higher voltages. This is done because the board has a shared rail for
> > > VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
> > > needs to be a value suitable for both ARM and SOC.
> > >
> > > This was introduced in:
> > >
> > > commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
> > >
> > > This only only applies to LDO bypass mode, a feature not present in
> > > upstream. When LDOs are enabled the effect is to use higher voltages than
> > > necesarry for no good reason.
> > >
> > > Setting these higher voltages can make some boards fail to boot with ugly
> > > semi-random crashes, reminiscent of memory corruption. These failures
> > > happen the first time the lowest idle state is used. Remove the OPP
> > > override in order to fix those crashes.
> > >
> > > Signed-off-by: Leonard Crestez <[email protected]>
> > >
> > > ---
> > > It's not clear exactly why the crashes happen. Perhaps waking up from idle
> > > draws more power than is available? Removing this override is a correct
> > > change anyway so maybe there is no need to investigate deeper.

> > Marek just sent a similar one a few minutes ago:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html

> Forgot to add Marek.

Wow, that was literally 15 minutes before my patch. In my defense I did
search the archives before starting to format the patch but it had not
arrived yet.

Anyway, that version also sets the supply for reg_arm and reg_soc. It
is not necessary for fixing the crash I'm seeing but is good because it
will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
1375mv. I tested Marek's patch and it works fine on my rev B board
(which otherwise fails to boot upstream).

-- 
Regards,
Leonard

2017-04-25 17:26:22

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

On Tue, Apr 25, 2017 at 2:23 PM, Leonard Crestez
<[email protected]> wrote:

> Wow, that was literally 15 minutes before my patch. In my defense I did
> search the archives before starting to format the patch but it had not
> arrived yet.
>
> Anyway, that version also sets the supply for reg_arm and reg_soc. It
> is not necessary for fixing the crash I'm seeing but is good because it
> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
> 1375mv. I tested Marek's patch and it works fine on my rev B board
> (which otherwise fails to boot upstream).

Excellent! I only have revA board and do not get the crash with this revision.

If you can reply to Marek's patch with your Tested-by that would be
nice, thanks.

2017-04-25 17:28:20

by Marek Vasut

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

On 04/25/2017 07:23 PM, Leonard Crestez wrote:
> On Tue, 2017-04-25 at 14:02 -0300, Fabio Estevam wrote:
>> On Tue, Apr 25, 2017 at 2:02 PM, Fabio Estevam <[email protected]> wrote:
>>>
>>> Hi Leonard,
>>>
>>> On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
>>> <[email protected]> wrote:
>>>>
>>>> The board file for imx6sx-dbg overrides cpufreq operating points to use
>>>> higher voltages. This is done because the board has a shared rail for
>>>> VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
>>>> needs to be a value suitable for both ARM and SOC.
>>>>
>>>> This was introduced in:
>>>>
>>>> commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
>>>>
>>>> This only only applies to LDO bypass mode, a feature not present in
>>>> upstream. When LDOs are enabled the effect is to use higher voltages than
>>>> necesarry for no good reason.
>>>>
>>>> Setting these higher voltages can make some boards fail to boot with ugly
>>>> semi-random crashes, reminiscent of memory corruption. These failures
>>>> happen the first time the lowest idle state is used. Remove the OPP
>>>> override in order to fix those crashes.
>>>>
>>>> Signed-off-by: Leonard Crestez <[email protected]>
>>>>
>>>> ---
>>>> It's not clear exactly why the crashes happen. Perhaps waking up from idle
>>>> draws more power than is available? Removing this override is a correct
>>>> change anyway so maybe there is no need to investigate deeper.
>
>>> Marek just sent a similar one a few minutes ago:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html
>
>> Forgot to add Marek.
>
> Wow, that was literally 15 minutes before my patch. In my defense I did
> search the archives before starting to format the patch but it had not
> arrived yet.

Hehehe :-)

> Anyway, that version also sets the supply for reg_arm and reg_soc. It
> is not necessary for fixing the crash I'm seeing but is good because it
> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
> 1375mv. I tested Marek's patch and it works fine on my rev B board
> (which otherwise fails to boot upstream).

Oh that's nice , thanks ! I don't have SDB and I hacked it up after a
brief discussion with Fabio without even compile-testing it, thus RFC.
Glad to hear it works and thanks for testing it ! Can you add a formal
Tested-by please ?

--
Best regards,
Marek Vasut

2017-04-27 01:17:29

by Peter Chen

[permalink] [raw]
Subject: RE: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override


>
>The board file for imx6sx-dbg overrides cpufreq operating points to use higher
>voltages. This is done because the board has a shared rail for VDD_ARM_IN and
>VDD_SOC_IN and when using LDO bypass the shared voltage needs to be a value
>suitable for both ARM and SOC.
>
>This was introduced in:
>
>commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
>
>This only only applies to LDO bypass mode, a feature not present in upstream. When
>LDOs are enabled the effect is to use higher voltages than necesarry for no good
>reason.
>
>Setting these higher voltages can make some boards fail to boot with ugly semi-
>random crashes, reminiscent of memory corruption. These failures happen the first
>time the lowest idle state is used. Remove the OPP override in order to fix those
>crashes.
>

Add Anson and Robin

This code has existed more than 2 years, it is strange why the bug has not reported both
for internal user and external user. I run upstream kernel using imx6sx-sdb revB very often
at recent years, but not meet this issue. How to trigger this unstable issue, anything needs
to change at u-boot?

Peter

>Signed-off-by: Leonard Crestez <[email protected]>
>
>---
>It's not clear exactly why the crashes happen. Perhaps waking up from idle draws
>more power than is available? Removing this override is a correct change anyway so
>maybe there is no need to investigate deeper.
>
> arch/arm/boot/dts/imx6sx-sdb.dts | 17 -----------------
> 1 file changed, 17 deletions(-)
>
>diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
>index 5bb8fd5..d71da30 100644
>--- a/arch/arm/boot/dts/imx6sx-sdb.dts
>+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
>@@ -12,23 +12,6 @@
> model = "Freescale i.MX6 SoloX SDB RevB Board"; };
>
>-&cpu0 {
>- operating-points = <
>- /* kHz uV */
>- 996000 1250000
>- 792000 1175000
>- 396000 1175000
>- 198000 1175000
>- >;
>- fsl,soc-operating-points = <
>- /* ARM kHz SOC uV */
>- 996000 1250000
>- 792000 1175000
>- 396000 1175000
>- 198000 1175000
>- >;
>-};
>-
> &i2c1 {
> clock-frequency = <100000>;
> pinctrl-names = "default";
>--
>2.7.4