RE: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
> > Admittedly without really understanding everything that is going on, I put > back the > deleted lines from this patch chunk and it started working again: > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c index > 19d60ed..af45aa32 100644 > --- a/drivers/usb/chipidea/host.c > +++ b/drivers/usb/chipidea/host.c > @@ -124,10 +124,8 @@ static int host_start(struct ci_hdrc *ci) > > hcd->power_budget = ci->platdata->power_budget; > hcd->tpl_support = ci->platdata->tpl_support; > - if (ci->phy) > - hcd->phy = ci->phy; > - else > - hcd->usb_phy = ci->usb_phy; > + if (ci->phy || ci->usb_phy) > + hcd->skip_phy_initialization = 1; > > ehci = hcd_to_ehci(hcd); > ehci->caps = ci->hw_bank.cap; > > Without a value in hcd->usb_phy, the call to usb_phy_notify_disconnect() in > hub_port_connect() (usb/core/hub.c) never fires but that is only part of the > problem. > Hope this helps. > Thanks, Mat. I posted a patch for this fix, help to test please. Peter
Re: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
On 2018-06-05 02:54, Peter Chen wrote:
And this is what the "decompiled" device tree entry for the USB
controller and phy look like:
usb@2184200 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x2184200 0x200>;
interrupts = <0x0 0x28 0x4>;
clocks = <0x4 0xa2>;
fsl,usbphy = <0x2c>;
fsl,usbmisc = <0x29 0x1>;
dr_mode = "host";
ahb-burst-config = <0x0>;
tx-burst-size-dword = <0x10>;
rx-burst-size-dword = <0x10>;
status = "okay";
disable-over-current;
vbus-supply = <0x2d>;
};
usbphy@20ca000 {
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
reg = <0x20ca000 0x1000>;
interrupts = <0x0 0x2d 0x4>;
clocks = <0x4 0xb7>;
fsl,anatop = <0x2>;
phandle = <0x2c>;
};
So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
It is ok.
Check two things:
- ci->usb_phy is non-NULL, and ci->phy is NULL
That is correct
- phy_roothub is NULL at the functions of drivers/usb/core/phy.c
I put a trace at the beginning of each of the functions of that file but none
of them is
ever called.
It is so strange. Please double confirm your git bisect is correct, if it is,
try to find which
line causes your regression.
Bisect confirmed.
Admittedly without really understanding everything that is going on, I put
back the deleted lines from this patch chunk and it started working again:
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 19d60ed..af45aa32 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -124,10 +124,8 @@ static int host_start(struct ci_hdrc *ci)
hcd->power_budget = ci->platdata->power_budget;
hcd->tpl_support = ci->platdata->tpl_support;
- if (ci->phy)
- hcd->phy = ci->phy;
- else
- hcd->usb_phy = ci->usb_phy;
+ if (ci->phy || ci->usb_phy)
+ hcd->skip_phy_initialization = 1;
ehci = hcd_to_ehci(hcd);
ehci->caps = ci->hw_bank.cap;
Without a value in hcd->usb_phy, the call to usb_phy_notify_disconnect()
in hub_port_connect() (usb/core/hub.c) never fires but that is only part of the
problem. Hope this helps.
BR // Mats
RE: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
> >>>
> >> And this is what the "decompiled" device tree entry for the USB
> >> controller and phy look like:
> >>
> >> usb@2184200 {
> >> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> >> reg = <0x2184200 0x200>;
> >> interrupts = <0x0 0x28 0x4>;
> >> clocks = <0x4 0xa2>;
> >> fsl,usbphy = <0x2c>;
> >> fsl,usbmisc = <0x29 0x1>;
> >> dr_mode = "host";
> >> ahb-burst-config = <0x0>;
> >> tx-burst-size-dword = <0x10>;
> >> rx-burst-size-dword = <0x10>;
> >> status = "okay";
> >> disable-over-current;
> >> vbus-supply = <0x2d>;
> >> };
> >>
> >> usbphy@20ca000 {
> >> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> >> reg = <0x20ca000 0x1000>;
> >> interrupts = <0x0 0x2d 0x4>;
> >> clocks = <0x4 0xb7>;
> >> fsl,anatop = <0x2>;
> >> phandle = <0x2c>;
> >> };
> >>
> >> So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
> >>
> > It is ok.
> >
> > Check two things:
> > - ci->usb_phy is non-NULL, and ci->phy is NULL
>
> That is correct
>
> > - phy_roothub is NULL at the functions of drivers/usb/core/phy.c
>
> I put a trace at the beginning of each of the functions of that file but none
> of them is
> ever called.
>
It is so strange. Please double confirm your git bisect is correct, if it is,
try to find which
line causes your regression.
Peter
Re: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
On 2018-06-04 03:14, Peter Chen wrote:
causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL
SoC with chipidea controller).
Example:
-- Cold boot of system.
-- Plug in USB memory stick --> Attached OK.
-- Mount disk --> OK, "ls" works
-- Unmount disk --> OK
-- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events.
-- Plug in/unplug stick again --> Still nothing.
-- Plug in stick and try mounting --> Somehow causes both detach and
attach
of the stick (mounting fails but mounting again now works).
-- All tested USB devices are working in a similar way. I.e first
attach
after boot works and then trouble starts.
-- Tested using default device tree from kernel tree
(arch/arm/boot/dts/imx6dl-hummingboard.dts).
Please ask if you think I can help in any way to track down the issue.
Hi Mats,
I have tested the latest usb-next tree, the hot plug works at my imx6sx-sdb
board.
Would you please check the value of hcd->skip_phy_initialization at the end of
function host_start?
hcd->skip_phy_initialization=1
BR // Mats
And this is what the "decompiled" device tree entry for the USB controller and
phy
look like:
usb@2184200 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x2184200 0x200>;
interrupts = <0x0 0x28 0x4>;
clocks = <0x4 0xa2>;
fsl,usbphy = <0x2c>;
fsl,usbmisc = <0x29 0x1>;
dr_mode = "host";
ahb-burst-config = <0x0>;
tx-burst-size-dword = <0x10>;
rx-burst-size-dword = <0x10>;
status = "okay";
disable-over-current;
vbus-supply = <0x2d>;
};
usbphy@20ca000 {
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
reg = <0x20ca000 0x1000>;
interrupts = <0x0 0x2d 0x4>;
clocks = <0x4 0xb7>;
fsl,anatop = <0x2>;
phandle = <0x2c>;
};
So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
It is ok.
Check two things:
- ci->usb_phy is non-NULL, and ci->phy is NULL
That is correct
- phy_roothub is NULL at the functions of drivers/usb/core/phy.c
I put a trace at the beginning of each of the functions of that file but none
of them is ever called.
// Mats
RE: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
> >>> causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL
> >>> SoC with chipidea controller).
> >>>
> >>> Example:
> >>> -- Cold boot of system.
> >>> -- Plug in USB memory stick --> Attached OK.
> >>> -- Mount disk --> OK, "ls" works
> >>> -- Unmount disk --> OK
> >>> -- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events.
> >>> -- Plug in/unplug stick again --> Still nothing.
> >>> -- Plug in stick and try mounting --> Somehow causes both detach and
> >>> attach
> >>> of the stick (mounting fails but mounting again now works).
> >>>
> >>> -- All tested USB devices are working in a similar way. I.e first
> >>> attach
> >>> after boot works and then trouble starts.
> >>> -- Tested using default device tree from kernel tree
> >>> (arch/arm/boot/dts/imx6dl-hummingboard.dts).
> >>>
> >>> Please ask if you think I can help in any way to track down the issue.
> >>>
> >> Hi Mats,
> >>
> >> I have tested the latest usb-next tree, the hot plug works at my
> >> imx6sx-sdb board.
> >> Would you please check the value of hcd->skip_phy_initialization at the
> >> end of
> function host_start?
> > hcd->skip_phy_initialization=1
> >
> > BR // Mats
> >
> And this is what the "decompiled" device tree entry for the USB controller
> and phy
> look like:
>
> usb@2184200 {
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x2184200 0x200>;
> interrupts = <0x0 0x28 0x4>;
> clocks = <0x4 0xa2>;
> fsl,usbphy = <0x2c>;
> fsl,usbmisc = <0x29 0x1>;
> dr_mode = "host";
> ahb-burst-config = <0x0>;
> tx-burst-size-dword = <0x10>;
> rx-burst-size-dword = <0x10>;
> status = "okay";
> disable-over-current;
> vbus-supply = <0x2d>;
> };
>
> usbphy@20ca000 {
> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> reg = <0x20ca000 0x1000>;
> interrupts = <0x0 0x2d 0x4>;
> clocks = <0x4 0xb7>;
> fsl,anatop = <0x2>;
> phandle = <0x2c>;
> };
>
> So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
>
It is ok.
Check two things:
- ci->usb_phy is non-NULL, and ci->phy is NULL
- phy_roothub is NULL at the functions of drivers/usb/core/phy.c
Peter
Re: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
Hi,
On 2018-06-01 16:08, Mats Karrman wrote:
Hi Peter,
On 2018-06-01 03:30, Peter Chen wrote:
After bisecting usb-next starting from tag 'v4.16' to current head I found
that the commit:
commit 4e88d4c083016454f179686529ae65d70b933b58
Author: Martin Blumenstingl
Date: Sat Mar 3 22:43:03 2018 +0100
usb: add a flag to skip PHY initialization to struct usb_hcd
causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL
SoC with chipidea controller).
Example:
-- Cold boot of system.
-- Plug in USB memory stick --> Attached OK.
-- Mount disk --> OK, "ls" works
-- Unmount disk --> OK
-- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events.
-- Plug in/unplug stick again --> Still nothing.
-- Plug in stick and try mounting --> Somehow causes both detach and attach
of the stick (mounting fails but mounting again now works).
-- All tested USB devices are working in a similar way. I.e first attach
after boot works and then trouble starts.
-- Tested using default device tree from kernel tree
(arch/arm/boot/dts/imx6dl-hummingboard.dts).
Please ask if you think I can help in any way to track down the issue.
Hi Mats,
I have tested the latest usb-next tree, the hot plug works at my imx6sx-sdb
board.
Would you please check the value of hcd->skip_phy_initialization at the end of
function host_start?
hcd->skip_phy_initialization=1
BR // Mats
And this is what the "decompiled" device tree entry for the USB controller and
phy look like:
usb@2184200 {
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x2184200 0x200>;
interrupts = <0x0 0x28 0x4>;
clocks = <0x4 0xa2>;
fsl,usbphy = <0x2c>;
fsl,usbmisc = <0x29 0x1>;
dr_mode = "host";
ahb-burst-config = <0x0>;
tx-burst-size-dword = <0x10>;
rx-burst-size-dword = <0x10>;
status = "okay";
disable-over-current;
vbus-supply = <0x2d>;
};
usbphy@20ca000 {
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
reg = <0x20ca000 0x1000>;
interrupts = <0x0 0x2d 0x4>;
clocks = <0x4 0xb7>;
fsl,anatop = <0x2>;
phandle = <0x2c>;
};
So, using deprecated? "fsl,usbphy" instead of "phys", in case that matters.
// Mats
Re: Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
Hi Peter, On 2018-06-01 03:30, Peter Chen wrote: After bisecting usb-next starting from tag 'v4.16' to current head I found that the commit: commit 4e88d4c083016454f179686529ae65d70b933b58 Author: Martin Blumenstingl Date: Sat Mar 3 22:43:03 2018 +0100 usb: add a flag to skip PHY initialization to struct usb_hcd causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL SoC with chipidea controller). Example: -- Cold boot of system. -- Plug in USB memory stick --> Attached OK. -- Mount disk --> OK, "ls" works -- Unmount disk --> OK -- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events. -- Plug in/unplug stick again --> Still nothing. -- Plug in stick and try mounting --> Somehow causes both detach and attach of the stick (mounting fails but mounting again now works). -- All tested USB devices are working in a similar way. I.e first attach after boot works and then trouble starts. -- Tested using default device tree from kernel tree (arch/arm/boot/dts/imx6dl-hummingboard.dts). Please ask if you think I can help in any way to track down the issue. Hi Mats, I have tested the latest usb-next tree, the hot plug works at my imx6sx-sdb board. Would you please check the value of hcd->skip_phy_initialization at the end of function host_start? hcd->skip_phy_initialization=1 BR // Mats -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Regression caused by: usb: add a flag to skip PHY initialization to struct usb_hcd
Hi, After bisecting usb-next starting from tag 'v4.16' to current head I found that the commit: commit 4e88d4c083016454f179686529ae65d70b933b58 Author: Martin Blumenstingl Date: Sat Mar 3 22:43:03 2018 +0100 usb: add a flag to skip PHY initialization to struct usb_hcd causes a regression for me on my SolidRun Hummingboard (NXP i.MX6 DL SoC with chipidea controller). Example: -- Cold boot of system. -- Plug in USB memory stick --> Attached OK. -- Mount disk --> OK, "ls" works -- Unmount disk --> OK -- Unplug USB stick --> Nothing happens. "udevadm monitor" see no events. -- Plug in/unplug stick again --> Still nothing. -- Plug in stick and try mounting --> Somehow causes both detach and attach of the stick (mounting fails but mounting again now works). -- All tested USB devices are working in a similar way. I.e first attach after boot works and then trouble starts. -- Tested using default device tree from kernel tree (arch/arm/boot/dts/imx6dl-hummingboard.dts). Please ask if you think I can help in any way to track down the issue. BR // Mats Kernel config: # # Automatically generated file; DO NOT EDIT. # Linux/arm 4.17.0-rc4 Kernel Configuration # CONFIG_ARM=y CONFIG_ARM_HAS_SG_CHAIN=y CONFIG_MIGHT_HAVE_PCI=y CONFIG_SYS_SUPPORTS_APM_EMULATION=y CONFIG_HAVE_PROC_CPU=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_LOCKDEP_SUPPORT=y CONFIG_TRACE_IRQFLAGS_SUPPORT=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_GENERIC_HWEIGHT=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_NEED_DMA_MAP_STATE=y CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_ARM_PATCH_PHYS_VIRT=y CONFIG_GENERIC_BUG=y CONFIG_PGTABLE_LEVELS=2 CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y # CONFIG_KERNEL_GZIP is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set CONFIG_KERNEL_LZO=y # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_CROSS_MEMORY_ATTACH=y CONFIG_USELIB=y CONFIG_AUDIT=y CONFIG_HAVE_ARCH_AUDITSYSCALL=y CONFIG_AUDITSYSCALL=y CONFIG_AUDIT_WATCH=y CONFIG_AUDIT_TREE=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_IRQ_SHOW_LEVEL=y CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK=y CONFIG_HARDIRQS_SW_RESEND=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_HANDLE_DOMAIN_IRQ=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y # CONFIG_GENERIC_IRQ_DEBUGFS is not set CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_ARCH_HAS_TICK_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y # CONFIG_NO_HZ_FULL is not set CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set # CONFIG_IRQ_TIME_ACCOUNTING is not set CONFIG_BSD_PROCESS_ACCT=y CONFIG_BSD_PROCESS_ACCT_V3=y CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y CONFIG_TASK_XACCT=y CONFIG_TASK_IO_ACCOUNTING=y CONFIG_CPU_ISOLATION=y # # RCU Subsystem # CONFIG_TREE_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y CONFIG_TREE_SRCU=y CONFIG_RCU_STALL_COMMON=y CONFIG_RCU_NEED_SEGCBLIST=y # CONFIG_IKCONFIG is not set CONFIG_LOG_BUF_SHIFT=18 CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=13 CONFIG_GENERIC_SCHED_CLOCK=y CONFIG_CGROUPS=y CONFIG_PAGE_COUNTER=y CONFIG_MEMCG=y CONFIG_MEMCG_SWAP=y CONFIG_MEMCG_SWAP_ENABLED=y CONFIG_BLK_CGROUP=y # CONFIG_DEBUG_BLK_CGROUP is not set CONFIG_CGROUP_WRITEBACK=y CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y CONFIG_CFS_BANDWIDTH=y CONFIG_RT_GROUP_SCHED=y # CONFIG_CGROUP_PIDS is not set # CONFIG_CGROUP_RDMA is not set CONFIG_CGROUP_FREEZER=y CONFIG_CPUSETS=y CONFIG_PROC_PID_CPUSET=y CONFIG_CGROUP_DEVICE=y CONFIG_CGROUP_CPUACCT=y CONFIG_CGROUP_PERF=y # CONFIG_CGROUP_DEBUG is not set CONFIG_SOCK_CGROUP_DATA=y CONFIG_NAMESPACES=y CONFIG_UTS_NS=y CONFIG_IPC_NS=y CONFIG_USER_NS=y CONFIG_PID_NS=y CONFIG_NET_NS=y CONFIG_SCHED_AUTOGROUP=y # CONFIG_SYSFS_DEPRECATED is not set CONFIG_RELAY=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE="" CONFIG_RD_GZIP=y # CONFIG_RD_BZIP2 is not set # CONFIG_RD_LZMA is not set # CONFIG_RD_XZ is not set # CONFIG_RD_LZO is not set # CONFIG_RD_LZ4 is not set CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_SYSCTL=y CONFIG_ANON_INODES=y CONFIG_HAVE_UID16=y CONFIG_BPF=y CONFIG_EXPERT=y CONFIG_UID16=y CONFIG_MULTIUSER=y # CONFIG_SGETMASK_SYSCALL is not set CONFIG_SYSFS_SYSCALL=y #
