reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

[Rasmus Villemoes]

We're using imx_v6_v7_defconfig on our Wandboards. After upgrading to
v4.14.67, reboot no longer works (or, well, takes a very long time when
the watchdog is configured).

v4.14.66 works fine, the breakage bisects to
2059e527a659cf16d6bb709f1c8509f7a7623fc4 (ARM: imx_v6_v7_defconfig:
Select ULPI support), and reverting that on top of v4.14.67 again works.

Rasmus

Re: reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

[Rasmus Villemoes]

On 2018-08-29 11:55, Rasmus Villemoes wrote:
> We're using imx_v6_v7_defconfig on our Wandboards. After upgrading to
> v4.14.67, reboot no longer works (or, well, takes a very long time when
> the watchdog is configured).
> 
> v4.14.66 works fine, the breakage bisects to
> 2059e527a659cf16d6bb709f1c8509f7a7623fc4 (ARM: imx_v6_v7_defconfig:
> Select ULPI support), and reverting that on top of v4.14.67 again works.

FWIW, v4.18 (the mainline release containing the upstream commit
157bcc0609) works fine, while v4.17.19 which also has that commit
backported (as df06ca1f56) fails. So I'm guessing there must be some
other commits in v4.18 that happen to make imx_v6_v7_defconfig work.

Rasmus

Re: reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

[Rasmus Villemoes]

On 2018-08-29 12:55, Rasmus Villemoes wrote:
> On 2018-08-29 11:55, Rasmus Villemoes wrote:
>> We're using imx_v6_v7_defconfig on our Wandboards. After upgrading to
>> v4.14.67, reboot no longer works (or, well, takes a very long time when
>> the watchdog is configured).
>>
>> v4.14.66 works fine, the breakage bisects to
>> 2059e527a659cf16d6bb709f1c8509f7a7623fc4 (ARM: imx_v6_v7_defconfig:
>> Select ULPI support), and reverting that on top of v4.14.67 again works.
> 
> FWIW, v4.18 (the mainline release containing the upstream commit
> 157bcc0609) works fine, while v4.17.19 which also has that commit
> backported (as df06ca1f56) fails. So I'm guessing there must be some
> other commits in v4.18 that happen to make imx_v6_v7_defconfig work.

OK, so v4.18-rc4 is also broken. Bisecting between that and v4.18 points
to a930d8bd94d (usb: chipidea: Always build ULPI code) introduced in
-rc7. The Makefile/Kconfig parts of that commit should not make any
difference wrt. 2059e527a6, since that commit explicitly set
USB_CHIPIDEA_ULPI, so we compile the same TUs with or without
a930d8bd94d. However, a930d8bd94d also adds this hunk

diff --git a/drivers/usb/chipidea/ulpi.c b/drivers/usb/chipidea/ulpi.c
index 6da42dcd2888..dfec07e8ae1d 100644
--- a/drivers/usb/chipidea/ulpi.c
+++ b/drivers/usb/chipidea/ulpi.c
@@ -95,6 +95,9 @@ int ci_ulpi_resume(struct ci_hdrc *ci)
 {
        int cnt = 100000;

+       if (ci->platdata->phy_mode != USBPHY_INTERFACE_MODE_ULPI)
+               return 0;
+
        while (cnt-- > 0) {
                if (hw_read(ci, OP_ULPI_VIEWPORT, ULPI_SYNC_STATE))
                        return 0;

and indeed just adding that on top of v4.14.67 makes it work again.

I don't know what the best fix for -stable is; the hunk above is the
minimal fix, but does not correspond to a mainline commit. a930d8bd94d
does cherrypick cleanly to v4.14.67, so might be the better option.

Rasmus

Re: reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

[Fabio Estevam]

Hi Rasmus,

> On 2018-08-29 11:55, Rasmus Villemoes wrote:
>> We're using imx_v6_v7_defconfig on our Wandboards. After upgrading to
>> v4.14.67, reboot no longer works (or, well, takes a very long time when
>> the watchdog is configured).
>>
>> v4.14.66 works fine, the breakage bisects to
>> 2059e527a659cf16d6bb709f1c8509f7a7623fc4 (ARM: imx_v6_v7_defconfig:
>> Select ULPI support), and reverting that on top of v4.14.67 again works.
>
> FWIW, v4.18 (the mainline release containing the upstream commit
> 157bcc0609) works fine, while v4.17.19 which also has that commit
> backported (as df06ca1f56) fails. So I'm guessing there must be some
> other commits in v4.18 that happen to make imx_v6_v7_defconfig work.

Thanks for reporting.

Commit 157bcc06094c3 ("ARM: imx_v6_v7_defconfig: Select ULPI support")
was meant to be applied for 4.18 only.

I think we should revert it from earlier stable versions.

Thanks

Re: reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

[Fabio Estevam]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

Hi Rasmus,

On Wed, Aug 29, 2018 at 10:57 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Rasmus,
>
>> On 2018-08-29 11:55, Rasmus Villemoes wrote:
>>> We're using imx_v6_v7_defconfig on our Wandboards. After upgrading to
>>> v4.14.67, reboot no longer works (or, well, takes a very long time when
>>> the watchdog is configured).
>>>
>>> v4.14.66 works fine, the breakage bisects to
>>> 2059e527a659cf16d6bb709f1c8509f7a7623fc4 (ARM: imx_v6_v7_defconfig:
>>> Select ULPI support), and reverting that on top of v4.14.67 again works.
>>
>> FWIW, v4.18 (the mainline release containing the upstream commit
>> 157bcc0609) works fine, while v4.17.19 which also has that commit
>> backported (as df06ca1f56) fails. So I'm guessing there must be some
>> other commits in v4.18 that happen to make imx_v6_v7_defconfig work.
>
> Thanks for reporting.
>
> Commit 157bcc06094c3 ("ARM: imx_v6_v7_defconfig: Select ULPI support")
> was meant to be applied for 4.18 only.
>
> I think we should revert it from earlier stable versions.

Or a better solution would be to port
a930d8bd94d8db7715d1af74299f710b1fb22fc8 from upstream.

Please test the attached patch adjusted for 4.14.67.

Thanks

[-- Attachment #2: 0001-usb-chipidea-Always-build-ULPI-code.patch --]
[-- Type: text/x-patch, Size: 3755 bytes --]

From 475a0301116c4853e4b37abf081ed25d6706c79e Mon Sep 17 00:00:00 2001
From: Fabio Estevam <fabio.estevam@nxp.com>
Date: Wed, 29 Aug 2018 17:13:33 -0300
Subject: [PATCH] usb: chipidea: Always build ULPI code

[commit a930d8bd94d8db7715d1af74299f710b1fb22fc8 upstream]

Commit 03e6275ae381 ("usb: chipidea: Fix ULPI on imx51") causes a kernel
hang on imx51 systems that use the ULPI interface and do not select the
CONFIG_USB_CHIPIDEA_ULPI option.

In order to avoid such potential misuse, let's always build the
chipidea ULPI code into the final ci_hdrc object.

Tested on a imx51-babbage board.

Fixes: 03e6275ae381 ("usb: chipidea: Fix ULPI on imx51")
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/usb/chipidea/Kconfig  | 9 +--------
 drivers/usb/chipidea/Makefile | 3 +--
 drivers/usb/chipidea/ci.h     | 8 --------
 drivers/usb/chipidea/ulpi.c   | 3 +++
 4 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
index 51f4157..6d990e4 100644
--- a/drivers/usb/chipidea/Kconfig
+++ b/drivers/usb/chipidea/Kconfig
@@ -3,6 +3,7 @@ config USB_CHIPIDEA
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
 	select EXTCON
 	select RESET_CONTROLLER
+	select USB_ULPI_BUS
 	help
 	  Say Y here if your system has a dual role high speed USB
 	  controller based on ChipIdea silicon IP. It supports:
@@ -38,12 +39,4 @@ config USB_CHIPIDEA_HOST
 	help
 	  Say Y here to enable host controller functionality of the
 	  ChipIdea driver.
-
-config USB_CHIPIDEA_ULPI
-	bool "ChipIdea ULPI PHY support"
-	depends on USB_ULPI_BUS=y || USB_ULPI_BUS=USB_CHIPIDEA
-	help
-	  Say Y here if you have a ULPI PHY attached to your ChipIdea
-	  controller.
-
 endif
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
index e3d5e72..12df94f 100644
--- a/drivers/usb/chipidea/Makefile
+++ b/drivers/usb/chipidea/Makefile
@@ -1,11 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_USB_CHIPIDEA)		+= ci_hdrc.o
 
-ci_hdrc-y				:= core.o otg.o debug.o
+ci_hdrc-y				:= core.o otg.o debug.o ulpi.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC)	+= udc.o
 ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)	+= host.o
 ci_hdrc-$(CONFIG_USB_OTG_FSM)		+= otg_fsm.o
-ci_hdrc-$(CONFIG_USB_CHIPIDEA_ULPI)	+= ulpi.o
 
 # Glue/Bridge layers go here
 
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 6743f85..61aee2b 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -243,10 +243,8 @@ struct ci_hdrc {
 
 	struct ci_hdrc_platform_data	*platdata;
 	int				vbus_active;
-#ifdef CONFIG_USB_CHIPIDEA_ULPI
 	struct ulpi			*ulpi;
 	struct ulpi_ops 		ulpi_ops;
-#endif
 	struct phy			*phy;
 	/* old usb_phy interface */
 	struct usb_phy			*usb_phy;
@@ -429,15 +427,9 @@ static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
 #endif
 }
 
-#if IS_ENABLED(CONFIG_USB_CHIPIDEA_ULPI)
 int ci_ulpi_init(struct ci_hdrc *ci);
 void ci_ulpi_exit(struct ci_hdrc *ci);
 int ci_ulpi_resume(struct ci_hdrc *ci);
-#else
-static inline int ci_ulpi_init(struct ci_hdrc *ci) { return 0; }
-static inline void ci_ulpi_exit(struct ci_hdrc *ci) { }
-static inline int ci_ulpi_resume(struct ci_hdrc *ci) { return 0; }
-#endif
 
 u32 hw_read_intr_enable(struct ci_hdrc *ci);
 
diff --git a/drivers/usb/chipidea/ulpi.c b/drivers/usb/chipidea/ulpi.c
index 1219583..f652132 100644
--- a/drivers/usb/chipidea/ulpi.c
+++ b/drivers/usb/chipidea/ulpi.c
@@ -103,6 +103,9 @@ int ci_ulpi_resume(struct ci_hdrc *ci)
 {
 	int cnt = 100000;
 
+	if (ci->platdata->phy_mode != USBPHY_INTERFACE_MODE_ULPI)
+		return 0;
+
 	while (cnt-- > 0) {
 		if (hw_read(ci, OP_ULPI_VIEWPORT, ULPI_SYNC_STATE))
 			return 0;
-- 
2.7.4

Re: reboot on wandboard fails with v4.14.67 (bisected to 2059e527a6)

[Fabio Estevam]

Hi Rasmus and Greg,

On Wed, Aug 29, 2018 at 10:57 AM, Fabio Estevam <festevam@gmail.com> wrote:

> Thanks for reporting.
>
> Commit 157bcc06094c3 ("ARM: imx_v6_v7_defconfig: Select ULPI support")
> was meant to be applied for 4.18 only.
>
> I think we should revert it from earlier stable versions.

Yes, the simplest fix is indeed to revert this commit from the older
stable trees.

I have sent the revert patches for 4.4, 4.9, 4.14 and 4.17 stable trees.

Thanks