Message ID | 20230704080304.816942-3-andrej.picej@norik.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f45:0:b0:3ea:f831:8777 with SMTP id v5csp1078339vqx; Tue, 4 Jul 2023 02:10:07 -0700 (PDT) X-Google-Smtp-Source: APBJJlHAy0caFChOv2qcTqwLoL67LWL3ObmAL0FuZHGNLXPw7yqiG1fTFJtfB5bsvPjpDSeBtv4M X-Received: by 2002:a17:903:24e:b0:1b7:f73d:524 with SMTP id j14-20020a170903024e00b001b7f73d0524mr16540841plh.43.1688461807540; Tue, 04 Jul 2023 02:10:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688461807; cv=none; d=google.com; s=arc-20160816; b=KR8aQAJuwTc84XhsQQ0+Aq0pqOj7Ops667KDlzlTWCK0eQyGGVh4YsPQO80ylF4Nb9 9siF765xBaGV8EuCInBgHpLx/E9qkxKPAshTmWGIgFxFi/gtZ4/BYY3MjysUOveaMaRG SW58xToLG6e0qWJmKSjrzhPOdeLIeXiLksmcRCXsOJGWEy21396hytIAYSNzONXnST0x S/AVtZUb5Q2IL5oEm4MfKPvtMXWMdGpWZWRbBSjKdRUqP3f6xJSulEtmUxjd67YnSeaB nUFz++9Gfjgw6P6NXmlh1uLM+kRzDVj7I4e+esh1PA2njoneZkofEb/3vBnQVIMkmu6F 5d4w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=jM+TtMP9aKuqji/7LgTWiurmTk0GfsL2Jv2TGbk6/bU=; fh=SpVPJ8yG5KftALQcal7tALlED7QvFLpG4O8BQkHGaPU=; b=IAVC5FBPAYhNM7fhesYdnslu54dFd4E3uXEF6nq6z1xIJIOdHyBxZF5P3cYcSCZBk2 3SJmI9jcz/rIy3Icszq9zUuZcS7SGEDrP6AxQLAMje/r/VGUMc4ngtCRdpHYXDiSAKbf 1v7rUw25sR43TBE7YFcAI50K+u+yShILnrxkYgIZWajfA4aOZyhdzLzFOxNFt7UD5yuz ckqKwwoTyQ2rQLhJiQyghLJ9JeHPahsGUUoShZSG4XEPa7+lBIyuZikZOcKpjGXu/Kgg HWREFD8OGfgqgA6OS93CTbrGE8xd+pDJmmqNxs+TtIpp9kYIQJ/voCCL79BcIWS1l8a5 eVvw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@norik.com header.s=default header.b=PZNwwdZO; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s2-20020a170902ea0200b001b89ce303e4si3508966plg.195.2023.07.04.02.09.52; Tue, 04 Jul 2023 02:10:07 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@norik.com header.s=default header.b=PZNwwdZO; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231577AbjGDJAH (ORCPT <rfc822;ybw1215001957@gmail.com> + 99 others); Tue, 4 Jul 2023 05:00:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33986 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230414AbjGDJAG (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 4 Jul 2023 05:00:06 -0400 X-Greylist: delayed 3349 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 04 Jul 2023 02:00:03 PDT Received: from cpanel.siel.si (cpanel.siel.si [46.19.9.99]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6F752127; Tue, 4 Jul 2023 02:00:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=norik.com; s=default; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=jM+TtMP9aKuqji/7LgTWiurmTk0GfsL2Jv2TGbk6/bU=; b=PZNwwdZO7AB9Wp+MmHl7kWIWmW 9fvQRGFrIzF2mRupJNfp4ag+TIBZqrZ/bcf1WqBURdEmK7GBfkwXLQ/d1cT5UWohTC2wcSmWSUs9w 3PV5fSBnUOPj5ZejUYMHUhvwZT2gS+vu680ICVZbPcgbJATB4Wgy3aGQ5P9IVMhgK4OHiBVvKxY6E JtSvdA7F8howtiuUwm4nlYzqUjZvtQBQ1679UFznfZfvSUPd2Br2yXv4hE/EFtoa1U1/nqKGUQ3es RGChHez9ofgNhh5Ze4Ud6lkHl3eZ8++l8lb1q2Tr9LrjlMCkgLMrkH/KHpSE9PNNK53kMSt0oN8rb WPK1m9YQ==; Received: from [89.212.21.243] (port=50488 helo=localhost.localdomain) by cpanel.siel.si with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from <andrej.picej@norik.com>) id 1qGb1A-009KMj-01; Tue, 04 Jul 2023 10:04:12 +0200 From: Andrej Picej <andrej.picej@norik.com> To: robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, upstream@phytec.de Subject: [PATCH 2/2] ARM: dts: imx6: pfla02: Fix SD card reboot problem Date: Tue, 4 Jul 2023 10:03:04 +0200 Message-Id: <20230704080304.816942-3-andrej.picej@norik.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230704080304.816942-1-andrej.picej@norik.com> References: <20230704080304.816942-1-andrej.picej@norik.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - cpanel.siel.si X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - norik.com X-Get-Message-Sender-Via: cpanel.siel.si: authenticated_id: andrej.picej@norik.com X-Authenticated-Sender: cpanel.siel.si: andrej.picej@norik.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_NONE, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1770478123767310027?= X-GMAIL-MSGID: =?utf-8?q?1770480528024477577?= |
Series |
PHYTEC i.MX6 device-tree fixes
|
|
Commit Message
Andrej Picej
July 4, 2023, 8:03 a.m. UTC
If regulator is not marked as always-on the regulator gets disabled on
reboot breaking the next boot.
Signed-off-by: Andrej Picej <andrej.picej@norik.com>
---
arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 +
1 file changed, 1 insertion(+)
Comments
Hi Andrej, [Restoring Cc-list] On 05.07.23 10:18, Andrej Picej wrote: > Hi Ahmad, > > On 4. 07. 23 10:15, Ahmad Fatoum wrote: >> Hello Andrej, >> >> On 04.07.23 10:03, Andrej Picej wrote: >>> If regulator is not marked as always-on the regulator gets disabled on >>> reboot breaking the next boot. >> >> While this is ok as a fix, the real issue is that your system reset doesn't >> restore PMIC rails to a good state. Are you perhaps doing a SoC-internal >> reset only or have the PMIC misconfigured? > > yes I would agree, this is just a fix since system reset doesn't reset the PMIC, leaving the configuration as it is in a misconfigured state. The i.MX6 reset is done with imx watchdog. If PMIC doesn't have own watchdog, i.MX watchdog can be configured to toggle WDOG_B line, which may be routed to PMIC. But fix is ok for now: Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de> Cheers, Ahmad > > Best regards, > Andrej > >> >> Cheers, >> Ahmad >> >>> >>> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >>> --- >>> arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>> index 80adb2a02cc9..25d6a036d5b8 100644 >>> --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>> +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>> @@ -192,6 +192,7 @@ vdd_3v3_pmic_io_reg: ldo6 { >>> vdd_sd0_reg: ldo9 { >>> regulator-min-microvolt = <3300000>; >>> regulator-max-microvolt = <3300000>; >>> + regulator-always-on; >>> }; >>> vdd_sd1_reg: ldo10 { >> >
Hi Marco, On 4. 07. 23 10:17, Marco Felsch wrote: > On 23-07-04, Andrej Picej wrote: >> If regulator is not marked as always-on the regulator gets disabled on >> reboot breaking the next boot. >> >> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >> --- >> arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >> index 80adb2a02cc9..25d6a036d5b8 100644 >> --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >> +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >> @@ -192,6 +192,7 @@ vdd_3v3_pmic_io_reg: ldo6 { >> vdd_sd0_reg: ldo9 { >> regulator-min-microvolt = <3300000>; >> regulator-max-microvolt = <3300000>; >> + regulator-always-on; > > I think this is the supply for the sd-card, so you can make use of > 'vmmc-supply'. This is already the case: > &usdhc3 { > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_usdhc3 > &pinctrl_usdhc3_cdwp>; > cd-gpios = <&gpio1 27 GPIO_ACTIVE_LOW>; > wp-gpios = <&gpio1 29 GPIO_ACTIVE_HIGH>; > vmmc-supply = <&vdd_sd0_reg>; > status = "disabled"; > }; I think the main reason for a failed boot is that the PMIC doesn't get reset and that the bootloader doesn't specifically enable the SD card regulator. Could this patch still be applied or should we put the fix in reset routine/bootloader? Best regards, Andrej > > Regards, > Marco > >> }; >> >> vdd_sd1_reg: ldo10 { >> -- >> 2.25.1 >> >> >>
On 05.07.23 10:28, Andrej Picej wrote: > Hi Marco, > > On 4. 07. 23 10:17, Marco Felsch wrote: >> On 23-07-04, Andrej Picej wrote: >>> If regulator is not marked as always-on the regulator gets disabled on >>> reboot breaking the next boot. >>> >>> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >>> --- >>> arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>> index 80adb2a02cc9..25d6a036d5b8 100644 >>> --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>> +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>> @@ -192,6 +192,7 @@ vdd_3v3_pmic_io_reg: ldo6 { >>> vdd_sd0_reg: ldo9 { >>> regulator-min-microvolt = <3300000>; >>> regulator-max-microvolt = <3300000>; >>> + regulator-always-on; >> >> I think this is the supply for the sd-card, so you can make use of >> 'vmmc-supply'. > > This is already the case: > >> &usdhc3 { >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_usdhc3 >> &pinctrl_usdhc3_cdwp>; >> cd-gpios = <&gpio1 27 GPIO_ACTIVE_LOW>; >> wp-gpios = <&gpio1 29 GPIO_ACTIVE_HIGH>; >> vmmc-supply = <&vdd_sd0_reg>; >> status = "disabled"; >> }; > > I think the main reason for a failed boot is that the PMIC doesn't get reset and that the bootloader doesn't specifically enable the SD card regulator. > > Could this patch still be applied or should we put the fix in reset routine/bootloader? Is SD-Card not main boot medium? From your description, I thought BootROM will fail to boot before bootloader has a chance to do anything about it. > > Best regards, > Andrej > >> >> Regards, >> Marco >> >>> }; >>> vdd_sd1_reg: ldo10 { >>> -- >>> 2.25.1 >>> >>> >>> > >
On 5. 07. 23 10:30, Ahmad Fatoum wrote: > On 05.07.23 10:28, Andrej Picej wrote: >> Hi Marco, >> >> On 4. 07. 23 10:17, Marco Felsch wrote: >>> On 23-07-04, Andrej Picej wrote: >>>> If regulator is not marked as always-on the regulator gets disabled on >>>> reboot breaking the next boot. >>>> >>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >>>> --- >>>> arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>> index 80adb2a02cc9..25d6a036d5b8 100644 >>>> --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>> +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>> @@ -192,6 +192,7 @@ vdd_3v3_pmic_io_reg: ldo6 { >>>> vdd_sd0_reg: ldo9 { >>>> regulator-min-microvolt = <3300000>; >>>> regulator-max-microvolt = <3300000>; >>>> + regulator-always-on; >>> >>> I think this is the supply for the sd-card, so you can make use of >>> 'vmmc-supply'. >> >> This is already the case: >> >>> &usdhc3 { >>> pinctrl-names = "default"; >>> pinctrl-0 = <&pinctrl_usdhc3 >>> &pinctrl_usdhc3_cdwp>; >>> cd-gpios = <&gpio1 27 GPIO_ACTIVE_LOW>; >>> wp-gpios = <&gpio1 29 GPIO_ACTIVE_HIGH>; >>> vmmc-supply = <&vdd_sd0_reg>; >>> status = "disabled"; >>> }; >> >> I think the main reason for a failed boot is that the PMIC doesn't get reset and that the bootloader doesn't specifically enable the SD card regulator. >> >> Could this patch still be applied or should we put the fix in reset routine/bootloader? > > Is SD-Card not main boot medium? From your description, I thought BootROM > will fail to boot before bootloader has a chance to do anything about it. > Yes sorry, you are absolutly right, the BootROM fails. It confused me because I could see the booloader booting, but it was from one of the fallback mediums. So I guess fixing the bootloader is not really an option. Sorry for the confusion. >> >> Best regards, >> Andrej >> >>> >>> Regards, >>> Marco >>> >>>> }; >>>> vdd_sd1_reg: ldo10 { >>>> -- >>>> 2.25.1 >>>> >>>> >>>> >> >> >
On 5. 07. 23 10:40, Andrej Picej wrote: > > > On 5. 07. 23 10:30, Ahmad Fatoum wrote: >> On 05.07.23 10:28, Andrej Picej wrote: >>> Hi Marco, >>> >>> On 4. 07. 23 10:17, Marco Felsch wrote: >>>> On 23-07-04, Andrej Picej wrote: >>>>> If regulator is not marked as always-on the regulator gets disabled on >>>>> reboot breaking the next boot. >>>>> >>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >>>>> --- >>>>> arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>>> b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>>> index 80adb2a02cc9..25d6a036d5b8 100644 >>>>> --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>>> +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi >>>>> @@ -192,6 +192,7 @@ vdd_3v3_pmic_io_reg: ldo6 { >>>>> vdd_sd0_reg: ldo9 { >>>>> regulator-min-microvolt = <3300000>; >>>>> regulator-max-microvolt = <3300000>; >>>>> + regulator-always-on; >>>> >>>> I think this is the supply for the sd-card, so you can make use of >>>> 'vmmc-supply'. >>> >>> This is already the case: >>> >>>> &usdhc3 { >>>> pinctrl-names = "default"; >>>> pinctrl-0 = <&pinctrl_usdhc3 >>>> &pinctrl_usdhc3_cdwp>; >>>> cd-gpios = <&gpio1 27 GPIO_ACTIVE_LOW>; >>>> wp-gpios = <&gpio1 29 GPIO_ACTIVE_HIGH>; >>>> vmmc-supply = <&vdd_sd0_reg>; >>>> status = "disabled"; >>>> }; >>> >>> I think the main reason for a failed boot is that the PMIC doesn't >>> get reset and that the bootloader doesn't specifically enable the SD >>> card regulator. >>> >>> Could this patch still be applied or should we put the fix in reset >>> routine/bootloader? >> >> Is SD-Card not main boot medium? From your description, I thought BootROM >> will fail to boot before bootloader has a chance to do anything about it. >> > > Yes sorry, you are absolutly right, the BootROM fails. It confused me > because I could see the booloader booting, but it was from one of the > fallback mediums. So I guess fixing the bootloader is not really an option. > Sorry for the confusion. > Ok, the main problem is well known, that's why PHYTEC disables the imx watchdog and uses a PMIC one for the reboot handler. This one resets the board completely. The SD card regulator problem is really just the manifestation of that bug. Unfortunately I didn't noticed that. :( I will create a v2 with a proper fix, where imx watchdog gets disabled. Thanks for your help, Andrej > >>> >>> Best regards, >>> Andrej >>> >>>> >>>> Regards, >>>> Marco >>>> >>>>> }; >>>>> vdd_sd1_reg: ldo10 { >>>>> -- >>>>> 2.25.1 >>>>> >>>>> >>>>> >>> >>> >>
On 05.07.23 12:39, Andrej Picej wrote:> On 5. 07. 23 10:40, Andrej Picej wrote: >> On 5. 07. 23 10:30, Ahmad Fatoum wrote: >>> On 05.07.23 10:28, Andrej Picej wrote: >>>> I think the main reason for a failed boot is that the PMIC doesn't get reset and that the bootloader doesn't specifically enable the SD card regulator. >>>> >>>> Could this patch still be applied or should we put the fix in reset routine/bootloader? >>> >>> Is SD-Card not main boot medium? From your description, I thought BootROM >>> will fail to boot before bootloader has a chance to do anything about it. >>> >> >> Yes sorry, you are absolutly right, the BootROM fails. It confused me because I could see the booloader booting, but it was from one of the fallback mediums. So I guess fixing the bootloader is not really an option. >> Sorry for the confusion. >> > > Ok, the main problem is well known, that's why PHYTEC disables the imx watchdog and uses a PMIC one for the reboot handler. This one resets the board completely. The SD card regulator problem is really just the manifestation of that bug. Unfortunately I didn't noticed that. :( > > I will create a v2 with a proper fix, where imx watchdog gets disabled. I'd be wary about solving it this way at the DTSI level, because it can break existing users: - Boot flow depends on reading boot reason, but with PMIC reset, everything is power-on reset - Bootloader starts i.MX watchdog, but new kernel will service only PMIC watchdog leading to system reset - Even if updating bootloader and kernel together, fallback of kernel may end up that bootloader uses PMIC watchdog, but kernel uses i.MX watchdog - There can be valid reasons to use both watchdogs and disabling one at the SoM level breaks that I had a similar issue once (Board controller reset to be used instead of SoC reset) and settled on using the barebox watchdog-priority/restart-priority[1] binding to select the "better" watchdog and then fixed up this choice into the kernel command line (barebox CONFIG_SYSTEMD_OF_WATCHDOG). If you decide to fix it for the evaluation kits, please add some text into the commit message that this fix should not be backported to older kernels. While it's ultimately the correct thing to do, changing this is IMO not stable backport material. [1]: FWIW, there was past discussion about adding restart priorities to Linux, e.g. https://lore.kernel.org/all/20201006102949.dbw6b2mrgt2ltgpw@pengutronix.de/ Cheers, Ahmad > > Thanks for your help, > Andrej > > >> >>>> >>>> Best regards, >>>> Andrej >>>> >>>>> >>>>> Regards, >>>>> Marco >>>>> >>>>>> }; >>>>>> vdd_sd1_reg: ldo10 { >>>>>> -- >>>>>> 2.25.1 >>>>>> >>>>>> >>>>>> >>>> >>>> >>> >
On 5. 07. 23 14:06, Ahmad Fatoum wrote: > On 05.07.23 12:39, Andrej Picej wrote:> On 5. 07. 23 10:40, Andrej Picej wrote: >>> On 5. 07. 23 10:30, Ahmad Fatoum wrote: >>>> On 05.07.23 10:28, Andrej Picej wrote: >>>>> I think the main reason for a failed boot is that the PMIC doesn't get reset and that the bootloader doesn't specifically enable the SD card regulator. >>>>> >>>>> Could this patch still be applied or should we put the fix in reset routine/bootloader? >>>> >>>> Is SD-Card not main boot medium? From your description, I thought BootROM >>>> will fail to boot before bootloader has a chance to do anything about it. >>>> >>> >>> Yes sorry, you are absolutly right, the BootROM fails. It confused me because I could see the booloader booting, but it was from one of the fallback mediums. So I guess fixing the bootloader is not really an option. >>> Sorry for the confusion. >>> >> >> Ok, the main problem is well known, that's why PHYTEC disables the imx watchdog and uses a PMIC one for the reboot handler. This one resets the board completely. The SD card regulator problem is really just the manifestation of that bug. Unfortunately I didn't noticed that. :( >> >> I will create a v2 with a proper fix, where imx watchdog gets disabled. > > I'd be wary about solving it this way at the DTSI level, because it can > break existing users: > > - Boot flow depends on reading boot reason, but with PMIC reset, everything > is power-on reset > > - Bootloader starts i.MX watchdog, but new kernel will service only > PMIC watchdog leading to system reset > > - Even if updating bootloader and kernel together, fallback of kernel > may end up that bootloader uses PMIC watchdog, but kernel uses i.MX > watchdog > > - There can be valid reasons to use both watchdogs and disabling > one at the SoM level breaks that > > I had a similar issue once (Board controller reset to be used instead of SoC > reset) and settled on using the barebox watchdog-priority/restart-priority[1] > binding to select the "better" watchdog and then fixed up this choice into > the kernel command line (barebox CONFIG_SYSTEMD_OF_WATCHDOG). > > If you decide to fix it for the evaluation kits, please add some text > into the commit message that this fix should not be backported to older kernels. > While it's ultimately the correct thing to do, changing this is IMO not stable > backport material. > > [1]: FWIW, there was past discussion about adding restart priorities to Linux, e.g. > https://lore.kernel.org/all/20201006102949.dbw6b2mrgt2ltgpw@pengutronix.de/ Ok I see the problems this might raise. Nevertheless I think it would be good to sync upstream and PHYTEC downstream dts with this fix. Since by default imx watchdog reset is really a no-go with phyFLEX. We could add the aliases for the watchdogs, marking the PMIC watchdog as watchdog0, and imx watchdog as watchdog1 which I think should tell the kernel to use the PMIC reboot handler, but not really sure how is that any better then disabling the imx watchdog. Why do you think this shouldn't be backported to older kernels? Because someone might use this with "not compatible" bootloader? Thanks for your help, Andrej > > Cheers, > Ahmad > >> >> Thanks for your help, >> Andrej >> >> >>> >>>>> >>>>> Best regards, >>>>> Andrej >>>>> >>>>>> >>>>>> Regards, >>>>>> Marco >>>>>> >>>>>>> }; >>>>>>> vdd_sd1_reg: ldo10 { >>>>>>> -- >>>>>>> 2.25.1 >>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>>> >> >
Hello Andrej, On 07.07.23 11:28, Andrej Picej wrote: > > > On 5. 07. 23 14:06, Ahmad Fatoum wrote: >> On 05.07.23 12:39, Andrej Picej wrote:> On 5. 07. 23 10:40, Andrej Picej wrote: >>>> On 5. 07. 23 10:30, Ahmad Fatoum wrote: >>>>> On 05.07.23 10:28, Andrej Picej wrote: >>>>>> I think the main reason for a failed boot is that the PMIC doesn't get reset and that the bootloader doesn't specifically enable the SD card regulator. >>>>>> >>>>>> Could this patch still be applied or should we put the fix in reset routine/bootloader? >>>>> >>>>> Is SD-Card not main boot medium? From your description, I thought BootROM >>>>> will fail to boot before bootloader has a chance to do anything about it. >>>>> >>>> >>>> Yes sorry, you are absolutly right, the BootROM fails. It confused me because I could see the booloader booting, but it was from one of the fallback mediums. So I guess fixing the bootloader is not really an option. >>>> Sorry for the confusion. >>>> >>> >>> Ok, the main problem is well known, that's why PHYTEC disables the imx watchdog and uses a PMIC one for the reboot handler. This one resets the board completely. The SD card regulator problem is really just the manifestation of that bug. Unfortunately I didn't noticed that. :( >>> >>> I will create a v2 with a proper fix, where imx watchdog gets disabled. >> >> I'd be wary about solving it this way at the DTSI level, because it can >> break existing users: >> >> - Boot flow depends on reading boot reason, but with PMIC reset, everything >> is power-on reset >> >> - Bootloader starts i.MX watchdog, but new kernel will service only >> PMIC watchdog leading to system reset >> >> - Even if updating bootloader and kernel together, fallback of kernel >> may end up that bootloader uses PMIC watchdog, but kernel uses i.MX >> watchdog >> >> - There can be valid reasons to use both watchdogs and disabling >> one at the SoM level breaks that >> > I had a similar issue once (Board controller reset to be used instead > of SoC >> reset) and settled on using the barebox watchdog-priority/restart-priority[1] >> binding to select the "better" watchdog and then fixed up this choice into >> the kernel command line (barebox CONFIG_SYSTEMD_OF_WATCHDOG). >> >> If you decide to fix it for the evaluation kits, please add some text >> into the commit message that this fix should not be backported to older kernels. >> While it's ultimately the correct thing to do, changing this is IMO not stable >> backport material. >> >> [1]: FWIW, there was past discussion about adding restart priorities to Linux, e.g. >> https://lore.kernel.org/all/20201006102949.dbw6b2mrgt2ltgpw@pengutronix.de/ > > Ok I see the problems this might raise. Nevertheless I think it would be good to sync upstream and PHYTEC downstream dts with this fix. Since by default imx watchdog reset is really a no-go with phyFLEX. What does downstream do? Might've been useful to note in the commit message, especially if there's an erratum. > We could add the aliases for the watchdogs, marking the PMIC watchdog as watchdog0, and imx watchdog as watchdog1 which I think should tell the kernel to use the PMIC reboot handler, but not really sure how is that any better then disabling the imx watchdog. Restart priority is set by watchdog_set_restart_priority and both da906[23] and i.MX watchdog set the same value of 128 here. Might be worthwhile to increase PMIC over i.MX watchdog priority to ensure it's taken instead. > Why do you think this shouldn't be backported to older kernels? Because > someone might use this with "not compatible" bootloader? Yes. Selecting a different watchdog is IMO not something that should come in via a stable update. Cheers, Ahmad > > Thanks for your help, > Andrej > >> >> Cheers, >> Ahmad >> >>> >>> Thanks for your help, >>> Andrej >>> >>> >>>> >>>>>> >>>>>> Best regards, >>>>>> Andrej >>>>>> >>>>>>> >>>>>>> Regards, >>>>>>> Marco >>>>>>> >>>>>>>> }; >>>>>>>> vdd_sd1_reg: ldo10 { >>>>>>>> -- >>>>>>>> 2.25.1 >>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>>> >>> >> >
diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi index 80adb2a02cc9..25d6a036d5b8 100644 --- a/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi +++ b/arch/arm/boot/dts/imx6qdl-phytec-pfla02.dtsi @@ -192,6 +192,7 @@ vdd_3v3_pmic_io_reg: ldo6 { vdd_sd0_reg: ldo9 { regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; + regulator-always-on; }; vdd_sd1_reg: ldo10 {