On 04.11.19 09:06, Frieder Schrempf wrote:
> Hi Shawn,
>
> On 04.11.19 08:43, Shawn Guo wrote:
>> On Thu, Oct 31, 2019 at 02:24:27PM +0000, Schrempf Frieder wrote:
>>> From: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
>>>
>>> The snvs-poweroff driver can power off the system by pulling the
>>> PMIC_ON_REQ signal low, to let the PMIC disable the power.
>>> The Kontron SoMs do not have this signal connected, so let's remove
>>> the node.
>>>
>>> This seems to fix a real issue when the signal is asserted at
>>> poweroff, but not actually causing the power to turn off. It was
>>> observed, that in this case the system would not shut down properly.
>>
>> I do not quite follow on this. How does disabling snvs_poweroff fix the
>> issue? The root cause of system not shut down properly seems to be that
>> PMIC doesn't shut down power. This looks like a clean-up rather than
>> bug fix.
>
> I don't know the exact reasons, but we had issues on these boards when
> doing a "poweroff". The kernel would print something like the log below.
> Disabling the snvs-poweroff solved this.
>
> But note that this has last been reproduced on v4.14. So I'm not sure if
> this is still a problem with the current kernel.
>
> #######
> reboot: Power down
> Unable to poweroff system
>
> ====================================
> WARNING: halt/675 still has locks held!
> 4.14.104-exceet #1 Not tainted
> ------------------------------------
> 1 lock held by halt/675:
> #0: (reboot_mutex){+.+.}, at: [<c0145a98>] SyS_reboot+0x14c/0x1dc
> #######
>
>>
>>>
>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
>>> Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron
>>> i.MX6UL N6310 SoM and boards")
>>
>> If you think this is really a bug fix, it should be applied to the file
>> before renaming rather than the one after renaming.
>
> I will try to reproduce the issue with the current kernel and depending
> on the results either drop the Fixes tag or move the patch before the
> renaming.
I just tried this with 5.4-rc5 and the issue persists. When
snvs-poweroff is enabled without the hardware actually being able to
power down via PMIC I get a locking WARNING from the kernel. Probably
because the system is still running, when it's already supposed to be
shut down by the PMIC.
So this fixes a real problem and therefore I will keep the Fixes tag and
move the patch to the proper place in this series.
>
> Thanks for reviewing,
> Frieder
>
>>
>> Shawn
>>
>>> ---
>>> arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 4 ----
>>> 1 file changed, 4 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi
>>> b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi
>>> index e18a8bd239be..4682a79f5b23 100644
>>> --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi
>>> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi
>>> @@ -158,10 +158,6 @@
>>> status = "okay";
>>> };
>>> -&snvs_poweroff {
>>> - status = "okay";
>>> -};
>>> -
>>> &uart1 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&pinctrl_uart1>;
>>> --
>>> 2.17.1
