| Message ID | 20191031142112.12431-1-frieder.schrempf@kontron.de |
|---|---|
| Headers | show |
| Series | Add support for more Kontron i.MX6UL/ULL SoMs and boards | expand |
On Thu, Oct 31, 2019 at 02:24:18PM +0000, Schrempf Frieder wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > Both, the SD card and the eMMC are connected to the usdhc controller > by four data lines. Therefore we set 'bus-width = <4>' for both > interfaces. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards") > --- > arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > index 7c98a1a46fb1..2299cad900af 100644 > --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > @@ -209,6 +209,7 @@ > wakeup-source; > vmmc-supply = <®_3v3>; > voltage-ranges = <3300 3300>; > + bus-width = <4>; Isn't it already set in arch/arm/boot/dts/imx6ul.dtsi as the default? Shawn > no-1-8-v; > status = "okay"; > }; > @@ -223,6 +224,7 @@ > wakeup-source; > vmmc-supply = <®_3v3>; > voltage-ranges = <3300 3300>; > + bus-width = <4>; > no-1-8-v; > status = "okay"; > }; > -- > 2.17.1
On Thu, Oct 31, 2019 at 02:24:21PM +0000, Schrempf Frieder wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > To silence the warnings shown by the driver at boot time, we add a > fixed regulator for the 5V supply of usbotg2 and specify the polarity > of the overcurrent signal for usbotg1. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards") I do not think it's a bug fix, so the Fixes tag doesn't really apply. Shawn > --- > arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > index 2299cad900af..d3eb21aa9014 100644 > --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > @@ -45,6 +45,13 @@ > regulator-max-microvolt = <3300000>; > }; > > + reg_5v: regulator-5v { > + compatible = "regulator-fixed"; > + regulator-name = "5v"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + }; > + > reg_usb_otg1_vbus: regulator-usb-otg1-vbus { > compatible = "regulator-fixed"; > regulator-name = "usb_otg1_vbus"; > @@ -191,6 +198,7 @@ > srp-disable; > hnp-disable; > adp-disable; > + over-current-active-low; > vbus-supply = <®_usb_otg1_vbus>; > status = "okay"; > }; > @@ -198,6 +206,7 @@ > &usbotg2 { > dr_mode = "host"; > disable-over-current; > + vbus-supply = <®_5v>; > status = "okay"; > }; > > -- > 2.17.1
On Thu, Oct 31, 2019 at 02:24:24PM +0000, Schrempf Frieder wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > The ECSPI1 is not used for a FRAM chip, so remove the comment. > While at it, also change some whitespaces to tabs to comply with the > indentation style of the rest of the file. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards") It's not a bug fix. Shawn > --- > arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > index d3eb21aa9014..e18a8bd239be 100644 > --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi > @@ -256,7 +256,6 @@ > >; > }; > > - /* FRAM */ > pinctrl_ecspi1: ecspi1grp { > fsl,pins = < > MX6UL_PAD_CSI_DATA07__ECSPI1_MISO 0x100b1 > @@ -281,8 +280,8 @@ > > pinctrl_enet2_mdio: enet2mdiogrp { > fsl,pins = < > - MX6UL_PAD_GPIO1_IO07__ENET2_MDC 0x1b0b0 > - MX6UL_PAD_GPIO1_IO06__ENET2_MDIO 0x1b0b0 > + MX6UL_PAD_GPIO1_IO07__ENET2_MDC 0x1b0b0 > + MX6UL_PAD_GPIO1_IO06__ENET2_MDIO 0x1b0b0 > >; > }; > > @@ -295,10 +294,10 @@ > > pinctrl_gpio: gpiogrp { > fsl,pins = < > - MX6UL_PAD_SNVS_TAMPER5__GPIO5_IO05 0x1b0b0 /* DOUT1 */ > - MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04 0x1b0b0 /* DIN1 */ > - MX6UL_PAD_SNVS_TAMPER1__GPIO5_IO01 0x1b0b0 /* DOUT2 */ > - MX6UL_PAD_SNVS_TAMPER0__GPIO5_IO00 0x1b0b0 /* DIN2 */ > + MX6UL_PAD_SNVS_TAMPER5__GPIO5_IO05 0x1b0b0 /* DOUT1 */ > + MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04 0x1b0b0 /* DIN1 */ > + MX6UL_PAD_SNVS_TAMPER1__GPIO5_IO01 0x1b0b0 /* DOUT2 */ > + MX6UL_PAD_SNVS_TAMPER0__GPIO5_IO00 0x1b0b0 /* DIN2 */ > >; > }; > > -- > 2.17.1
On 04.11.19 08:29, Shawn Guo wrote: > On Thu, Oct 31, 2019 at 02:24:18PM +0000, Schrempf Frieder wrote: >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> Both, the SD card and the eMMC are connected to the usdhc controller >> by four data lines. Therefore we set 'bus-width = <4>' for both >> interfaces. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> >> Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards") >> --- >> arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> index 7c98a1a46fb1..2299cad900af 100644 >> --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> @@ -209,6 +209,7 @@ >> wakeup-source; >> vmmc-supply = <®_3v3>; >> voltage-ranges = <3300 3300>; >> + bus-width = <4>; > > Isn't it already set in arch/arm/boot/dts/imx6ul.dtsi as the default? Right, I somehow missed this. So this patch can be ignored. > > Shawn > >> no-1-8-v; >> status = "okay"; >> }; >> @@ -223,6 +224,7 @@ >> wakeup-source; >> vmmc-supply = <®_3v3>; >> voltage-ranges = <3300 3300>; >> + bus-width = <4>; >> no-1-8-v; >> status = "okay"; >> }; >> -- >> 2.17.1 > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Thu, Oct 31, 2019 at 02:24:27PM +0000, Schrempf Frieder wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > 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. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > 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. 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
On Thu, Oct 31, 2019 at 02:24:34PM +0000, Schrempf Frieder wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > Kontron Electronics GmbH produces several ARM boards, that are > planned to be upstreamed eventually. For now we have some > i.MX6UL/ULL based SoMs and boards, that are already available > in the kernel. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> We usually do not need MAINTAINERS entry for individual DTS files. Shawn > --- > MAINTAINERS | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 296de2b51c83..a461d31ee98d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9103,6 +9103,12 @@ F: include/linux/kmod.h > F: lib/test_kmod.c > F: tools/testing/selftests/kmod/ > > +KONTRON ELECTRONICS ARM BOARDS SUPPORT > +M: Frieder Schrempf <frieder.schrempf@kontron.de> > +S: Maintained > +F: arch/arm/boot/dts/imx6ul-kontron-* > +F: arch/arm/boot/dts/imx6ull-kontron-* > + > KPROBES > M: Naveen N. Rao <naveen.n.rao@linux.ibm.com> > M: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> > -- > 2.17.1
On 04.11.19 08:31, Shawn Guo wrote: > On Thu, Oct 31, 2019 at 02:24:21PM +0000, Schrempf Frieder wrote: >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> To silence the warnings shown by the driver at boot time, we add a >> fixed regulator for the 5V supply of usbotg2 and specify the polarity >> of the overcurrent signal for usbotg1. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> >> Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards") > > I do not think it's a bug fix, so the Fixes tag doesn't really apply. I guess you're right. It only prevents warnings at boot time and functionality is not broken. I think I had a wrong understanding of the Fixes tag and need to reread the docs... > > Shawn > >> --- >> arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> index 2299cad900af..d3eb21aa9014 100644 >> --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> @@ -45,6 +45,13 @@ >> regulator-max-microvolt = <3300000>; >> }; >> >> + reg_5v: regulator-5v { >> + compatible = "regulator-fixed"; >> + regulator-name = "5v"; >> + regulator-min-microvolt = <5000000>; >> + regulator-max-microvolt = <5000000>; >> + }; >> + >> reg_usb_otg1_vbus: regulator-usb-otg1-vbus { >> compatible = "regulator-fixed"; >> regulator-name = "usb_otg1_vbus"; >> @@ -191,6 +198,7 @@ >> srp-disable; >> hnp-disable; >> adp-disable; >> + over-current-active-low; >> vbus-supply = <®_usb_otg1_vbus>; >> status = "okay"; >> }; >> @@ -198,6 +206,7 @@ >> &usbotg2 { >> dr_mode = "host"; >> disable-over-current; >> + vbus-supply = <®_5v>; >> status = "okay"; >> }; >> >> -- >> 2.17.1 > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 04.11.19 08:33, Shawn Guo wrote: > On Thu, Oct 31, 2019 at 02:24:24PM +0000, Schrempf Frieder wrote: >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> The ECSPI1 is not used for a FRAM chip, so remove the comment. >> While at it, also change some whitespaces to tabs to comply with the >> indentation style of the rest of the file. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> >> Fixes: 1ea4b76cdfde ("ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards") > > It's not a bug fix. Right. > > Shawn > >> --- >> arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> index d3eb21aa9014..e18a8bd239be 100644 >> --- a/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> +++ b/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi >> @@ -256,7 +256,6 @@ >> >; >> }; >> >> - /* FRAM */ >> pinctrl_ecspi1: ecspi1grp { >> fsl,pins = < >> MX6UL_PAD_CSI_DATA07__ECSPI1_MISO 0x100b1 >> @@ -281,8 +280,8 @@ >> >> pinctrl_enet2_mdio: enet2mdiogrp { >> fsl,pins = < >> - MX6UL_PAD_GPIO1_IO07__ENET2_MDC 0x1b0b0 >> - MX6UL_PAD_GPIO1_IO06__ENET2_MDIO 0x1b0b0 >> + MX6UL_PAD_GPIO1_IO07__ENET2_MDC 0x1b0b0 >> + MX6UL_PAD_GPIO1_IO06__ENET2_MDIO 0x1b0b0 >> >; >> }; >> >> @@ -295,10 +294,10 @@ >> >> pinctrl_gpio: gpiogrp { >> fsl,pins = < >> - MX6UL_PAD_SNVS_TAMPER5__GPIO5_IO05 0x1b0b0 /* DOUT1 */ >> - MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04 0x1b0b0 /* DIN1 */ >> - MX6UL_PAD_SNVS_TAMPER1__GPIO5_IO01 0x1b0b0 /* DOUT2 */ >> - MX6UL_PAD_SNVS_TAMPER0__GPIO5_IO00 0x1b0b0 /* DIN2 */ >> + MX6UL_PAD_SNVS_TAMPER5__GPIO5_IO05 0x1b0b0 /* DOUT1 */ >> + MX6UL_PAD_SNVS_TAMPER4__GPIO5_IO04 0x1b0b0 /* DIN1 */ >> + MX6UL_PAD_SNVS_TAMPER1__GPIO5_IO01 0x1b0b0 /* DOUT2 */ >> + MX6UL_PAD_SNVS_TAMPER0__GPIO5_IO00 0x1b0b0 /* DIN2 */ >> >; >> }; >> >> -- >> 2.17.1
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@kontron.de> >> >> 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@kontron.de> >> 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. 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
On 04.11.19 08:45, Shawn Guo wrote: > On Thu, Oct 31, 2019 at 02:24:34PM +0000, Schrempf Frieder wrote: >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> Kontron Electronics GmbH produces several ARM boards, that are >> planned to be upstreamed eventually. For now we have some >> i.MX6UL/ULL based SoMs and boards, that are already available >> in the kernel. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > > We usually do not need MAINTAINERS entry for individual DTS files. Ok, I just thought it would be nice for things like get_maintainer.pl, to make sure I receive notifications, when someone sends patches for theses DTS files. But if this is against the usual conventions, I will drop this. > > Shawn > >> --- >> MAINTAINERS | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 296de2b51c83..a461d31ee98d 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -9103,6 +9103,12 @@ F: include/linux/kmod.h >> F: lib/test_kmod.c >> F: tools/testing/selftests/kmod/ >> >> +KONTRON ELECTRONICS ARM BOARDS SUPPORT >> +M: Frieder Schrempf <frieder.schrempf@kontron.de> >> +S: Maintained >> +F: arch/arm/boot/dts/imx6ul-kontron-* >> +F: arch/arm/boot/dts/imx6ull-kontron-* >> + >> KPROBES >> M: Naveen N. Rao <naveen.n.rao@linux.ibm.com> >> M: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> >> -- >> 2.17.1
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@kontron.de> >>> >>> 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@kontron.de> >>> 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
On Mon, Nov 04, 2019 at 11:18:19AM +0000, Schrempf Frieder wrote: > 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@kontron.de> > >>> > >>> 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@kontron.de> > >>> 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. I still do not get it. Are you saying that with snvs-poweroff being disabled, your system is now able to be shut down properly by PMIC? Shawn > > 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.
On 04.11.19 13:45, Shawn Guo wrote: > On Mon, Nov 04, 2019 at 11:18:19AM +0000, Schrempf Frieder wrote: >> 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@kontron.de> >>>>> >>>>> 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@kontron.de> >>>>> 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. > > I still do not get it. Are you saying that with snvs-poweroff being > disabled, your system is now able to be shut down properly by PMIC? Sorry for the confusion. The board does not have a PMIC and the PMIC_ON_REQ signal is not connected. So the snvs-poweroff driver should definitely be disabled because there is no reason for it to be enabled. But indeed, with snvs-poweroff disabled the board does now at least halt correctly (it is still powered of course). For some reason, probably by some copy and paste mistake, the snvs-poweroff driver is enabled in the dts which causes the issue described above when trying to halt/shutdown the system. Please also see the logs below for comparison of both cases. With snvs-poweroff enabled: ############ ~# poweroff The system is going down for system halt NOW!ymxc3) (Thu Oct 31 18:57:55 2019 [...] reboot: Power down Unable to poweroff system ==================================== WARNING: halt/869 still has locks held! 5.4.0-rc5-ktn #1 Not tainted ------------------------------------ 1 lock held by halt/869: #0: c11138f8 (system_transition_mutex){+.+.}, at: __do_sys_reboot+0x174/0x20c stack backtrace: CPU: 0 PID: 869 Comm: halt Not tainted 5.4.0-rc5-ktn #1 Hardware name: Freescale i.MX6 Ultralite (Device Tree) [<c0112bb8>] (unwind_backtrace) from [<c010ce14>] (show_stack+0x10/0x14) [<c010ce14>] (show_stack) from [<c0ae18dc>] (dump_stack+0xd8/0x110) [<c0ae18dc>] (dump_stack) from [<c012a5c0>] (do_exit+0x7a4/0xb94) [<c012a5c0>] (do_exit) from [<c014fe64>] (__do_sys_reboot+0x180/0x20c) [<c014fe64>] (__do_sys_reboot) from [<c0101000>] (ret_fast_syscall+0x0/0x28) Exception stack(0xd9e15fa8 to 0xd9e15ff0) 5fa0: bed53e14 00000005 fee1dead 28121969 4321fedc fffffff6 5fc0: bed53e14 00000005 0040cf04 00000058 0041def8 00000000 00000001 00000000 5fe0: 00000058 bed53c74 b6f03ad5 b6e89c46 ############ With snvs-poweroff disabled: ############ The system is going down for system halt NOW!ymxc3) (Thu Oct 31 18:56:33 2019 [...] reboot: System halted ############ > > Shawn > >> >> 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.
From: Frieder Schrempf <frieder.schrempf@kontron.de> In order to support more of the i.MX6UL/ULL-based SoMs and boards by Kontron Electronics GmbH, we restructure the devicetrees to share common parts and add new devicetrees for the missing boards. Currently there are the following SoM flavors: * N6310: SoM with i.MX6UL-2, 256MB RAM, 256MB SPI NAND * N6311: SoM with i.MX6UL-2, 512MB RAM, 512MB SPI NAND (new) * N6411: SoM with i.MX6ULL, 512MB RAM, 512MB SPI NAND (new) Each of the SoMs also features 1MB SPI NOR and an Ethernet PHY. The carrier board for the evalkit is the same for all SoMs. This series also includes some small fixes for the shared parts of the devicetrees. --- Changes for v3: =============== * Add a patch that removes the snvs-poweroff node as it is not used * Drop patch to add missing includes for gpio.h and irq.h as it is not needed * Add Fixes tags to those patches that fix an issue in the existing devicetrees * Add Reviewed-by tags from Krzysztof * Minor improvements to commit messages and cover letter Changes for v2: =============== * Patch 1: Move the spi-flash node instead of duplicating it * Patch 3: Add a missing include for dt-bindings/gpio/gpio.h * Patch 11: Add patch to add missing includes for gpio.h and irq.h * Add Reviewed-by tags from Krzysztof and Rob --- Frieder Schrempf (11): ARM: dts: imx6ul-kontron-n6310: Move common SoM nodes to a separate file ARM: dts: Add support for two more Kontron SoMs N6311 and N6411 ARM: dts: imx6ul-kontron-n6310-s: Move common nodes to a separate file ARM: dts: Add support for two more Kontron evalkit boards 'N6311 S' and 'N6411 S' ARM: dts: imx6ul-kontron-n6x1x: Add 'chosen' node with 'stdout-path' ARM: dts: imx6ul-kontron-n6x1x-s: Specify bus-width for SD card and eMMC ARM: dts: imx6ul-kontron-n6x1x-s: Add vbus-supply and overcurrent polarity to usb nodes ARM: dts: imx6ul-kontron-n6x1x-s: Remove an obsolete comment and fix indentation ARM: dts: imx6ul-kontron-n6x1x-s: Disable the snvs-poweroff driver dt-bindings: arm: fsl: Add more Kontron i.MX6UL/ULL compatibles MAINTAINERS: Add an entry for Kontron Electronics ARM board support .../devicetree/bindings/arm/fsl.yaml | 14 + MAINTAINERS | 6 + arch/arm/boot/dts/imx6ul-kontron-n6310-s.dts | 405 +---------------- .../boot/dts/imx6ul-kontron-n6310-som.dtsi | 95 +--- arch/arm/boot/dts/imx6ul-kontron-n6311-s.dts | 16 + .../boot/dts/imx6ul-kontron-n6311-som.dtsi | 40 ++ arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi | 420 ++++++++++++++++++ .../dts/imx6ul-kontron-n6x1x-som-common.dtsi | 109 +++++ arch/arm/boot/dts/imx6ull-kontron-n6411-s.dts | 16 + .../boot/dts/imx6ull-kontron-n6411-som.dtsi | 40 ++ 10 files changed, 663 insertions(+), 498 deletions(-) create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6311-s.dts create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6311-som.dtsi create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi create mode 100644 arch/arm/boot/dts/imx6ul-kontron-n6x1x-som-common.dtsi create mode 100644 arch/arm/boot/dts/imx6ull-kontron-n6411-s.dts create mode 100644 arch/arm/boot/dts/imx6ull-kontron-n6411-som.dtsi