mbox series

[0/4] usb: chipidea: imx: add HSIC support

Message ID 20181016045846.2345-1-peter.chen@nxp.com (mailing list archive)
Headers show
Series usb: chipidea: imx: add HSIC support | expand

Message

Peter Chen Oct. 16, 2018, 5:01 a.m. UTC
Most of NXP (Freescale) i.mx USB part has HSIC support, in this series,
we add support for them, it should cover all imx6 and imx7d. I have
no HSIC interface board which is supported by upstream kernel, so this
patches are only compiled ok, Frieder Schrempf, would you please
help me test it on your board? Thanks.

Peter Chen (4):
  usb: chipidea: add flag for imx hsic implementation
  usb: chipidea: imx: add HSIC support
  usb: chipidea: host: override ehci->hub_control
  doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

 .../devicetree/bindings/usb/ci-hdrc-usb2.txt       |   1 +
 drivers/usb/chipidea/ci_hdrc_imx.c                 | 153 ++++++++++++++++++---
 drivers/usb/chipidea/ci_hdrc_imx.h                 |   9 +-
 drivers/usb/chipidea/host.c                        |  98 +++++++++++++
 drivers/usb/chipidea/usbmisc_imx.c                 | 131 ++++++++++++++++++
 include/linux/usb/chipidea.h                       |   3 +
 6 files changed, 376 insertions(+), 19 deletions(-)

Comments

Frieder Schrempf Oct. 17, 2018, 7:02 a.m. UTC | #1
Hi Peter,

On 16.10.18 07:01, Peter Chen wrote:
> Most of NXP (Freescale) i.mx USB part has HSIC support, in this series,
> we add support for them, it should cover all imx6 and imx7d. I have
> no HSIC interface board which is supported by upstream kernel, so this
> patches are only compiled ok, Frieder Schrempf, would you please
> help me test it on your board? Thanks.

Thank you for providing the patch!
I applied it to v4.19-rc8 and tested and the LAN9730 comes up and works 
just fine.

Here is how my devicetree looks like:

usbphy_dummy: usbphy_dummy@1 {
	compatible = "usb-nop-xceiv";
};

[...]

&usbh2 {
	vbus-supply = <&reg_usb_h2_vbus>;
	pinctrl-names = "idle", "active";
	pinctrl-0 = <&pinctrl_usbh2_idle>;
	pinctrl-1 = <&pinctrl_usbh2_active>;
	fsl,usbphy = <&usbphy_dummy>;
	phy_type = "hsic";
	status = "okay";
	#address-cells = <1>;
	#size-cells = <0>;

	usbnet: smsc@1 {
		compatible = "usb424,9730";
		/* Filled in by U-Boot */
		mac-address = [00 00 00 00 00 00];
		reg = <1>;
	};
};

[...]

pinctrl_usbh2_idle: usbh2grp-idle {
   fsl,pins = <
     MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
     MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40013030
   >;
};

pinctrl_usbh2_active: usbh2grp-active {
   fsl,pins = <
     MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
     MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40017030
   >;
};


Are there any test cases I should try?
How can I test suspend/resume?

I also have some suggestions for your patch. Please see the separate email.

Thanks,
Frieder

> 
> Peter Chen (4):
>    usb: chipidea: add flag for imx hsic implementation
>    usb: chipidea: imx: add HSIC support
>    usb: chipidea: host: override ehci->hub_control
>    doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups
> 
>   .../devicetree/bindings/usb/ci-hdrc-usb2.txt       |   1 +
>   drivers/usb/chipidea/ci_hdrc_imx.c                 | 153 ++++++++++++++++++---
>   drivers/usb/chipidea/ci_hdrc_imx.h                 |   9 +-
>   drivers/usb/chipidea/host.c                        |  98 +++++++++++++
>   drivers/usb/chipidea/usbmisc_imx.c                 | 131 ++++++++++++++++++
>   include/linux/usb/chipidea.h                       |   3 +
>   6 files changed, 376 insertions(+), 19 deletions(-)
>
Peter Chen Oct. 17, 2018, 7:23 a.m. UTC | #2
> On 16.10.18 07:01, Peter Chen wrote:
> > Most of NXP (Freescale) i.mx USB part has HSIC support, in this
> > series, we add support for them, it should cover all imx6 and imx7d. I
> > have no HSIC interface board which is supported by upstream kernel, so
> > this patches are only compiled ok, Frieder Schrempf, would you please
> > help me test it on your board? Thanks.
> 
> Thank you for providing the patch!
> I applied it to v4.19-rc8 and tested and the LAN9730 comes up and works just fine.
> 
> Here is how my devicetree looks like:
> 
> usbphy_dummy: usbphy_dummy@1 {
> 	compatible = "usb-nop-xceiv";
> };
> 
> [...]
> 
> &usbh2 {
> 	vbus-supply = <&reg_usb_h2_vbus>;
> 	pinctrl-names = "idle", "active";
> 	pinctrl-0 = <&pinctrl_usbh2_idle>;
> 	pinctrl-1 = <&pinctrl_usbh2_active>;
> 	fsl,usbphy = <&usbphy_dummy>;
> 	phy_type = "hsic";
> 	status = "okay";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	usbnet: smsc@1 {
> 		compatible = "usb424,9730";
> 		/* Filled in by U-Boot */
> 		mac-address = [00 00 00 00 00 00];
> 		reg = <1>;
> 	};
> };
> 
> [...]
> 
> pinctrl_usbh2_idle: usbh2grp-idle {
>    fsl,pins = <
>      MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
>      MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40013030
>    >;
> };
> 
> pinctrl_usbh2_active: usbh2grp-active {
>    fsl,pins = <
>      MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
>      MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40017030
>    >;
> };
> 
> 
> Are there any test cases I should try?
> How can I test suspend/resume?
> 

- System suspend/resume
1. Enable USB wakeup
for i in $(find /sys -name wakeup | grep usb);do echo enabled  > $i;echo "echo enabled > $i";done;
2. Let the system enter suspend using below command 
echo mem > /sys/power/state
3. And see if there is a wakeup block system entering suspend, and check if USB HSIC works ok
after system resume

- Runtime suspend
1. Enable auto suspend for all USB devices, and check if USBOH3 clock is closed,
make sure do not plug any ethernet cable on the RJ45 port.

/* Enable auto suspend */
for i in $(find /sys -name control | grep usb);do echo auto  > $i;echo "echo auto > $i";done;

/* Scripts to see USB clock */
root@imx8qxpmek:~# cat uclk.sh 
/home/root/dump-clocks.sh | grep usb
root@imx8qxpmek:~# cat /home/root/dump-clocks.sh 
#!/bin/bash

if ! mount|grep -sq '/sys/kernel/debug'; then
        mount -t debugfs none /sys/kernel/debug
fi

cat /sys/kernel/debug/clk/clk_summary

2. If USB OH3 clock can close successfully, it means USB HSIC can enter suspend successfully.

3. Plug ethernet cable to see if the link can be on.

The purpose of above two tests is to see if USB HSIC can be suspended.

Thanks.

Peter


> I also have some suggestions for your patch. Please see the separate email.
> 
> Thanks,
> Frieder
> 
> >
> > Peter Chen (4):
> >    usb: chipidea: add flag for imx hsic implementation
> >    usb: chipidea: imx: add HSIC support
> >    usb: chipidea: host: override ehci->hub_control
> >    doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups
> >
> >   .../devicetree/bindings/usb/ci-hdrc-usb2.txt       |   1 +
> >   drivers/usb/chipidea/ci_hdrc_imx.c                 | 153 ++++++++++++++++++---
> >   drivers/usb/chipidea/ci_hdrc_imx.h                 |   9 +-
> >   drivers/usb/chipidea/host.c                        |  98 +++++++++++++
> >   drivers/usb/chipidea/usbmisc_imx.c                 | 131 ++++++++++++++++++
> >   include/linux/usb/chipidea.h                       |   3 +
> >   6 files changed, 376 insertions(+), 19 deletions(-)
> >
Frieder Schrempf Oct. 17, 2018, 9:56 a.m. UTC | #3
Hi Peter,

On 17.10.18 09:23, Peter Chen wrote:
>   
>> On 16.10.18 07:01, Peter Chen wrote:
>>> Most of NXP (Freescale) i.mx USB part has HSIC support, in this
>>> series, we add support for them, it should cover all imx6 and imx7d. I
>>> have no HSIC interface board which is supported by upstream kernel, so
>>> this patches are only compiled ok, Frieder Schrempf, would you please
>>> help me test it on your board? Thanks.
>>
>> Thank you for providing the patch!
>> I applied it to v4.19-rc8 and tested and the LAN9730 comes up and works just fine.
>>
>> Here is how my devicetree looks like:
>>
>> usbphy_dummy: usbphy_dummy@1 {
>> 	compatible = "usb-nop-xceiv";
>> };
>>
>> [...]
>>
>> &usbh2 {
>> 	vbus-supply = <&reg_usb_h2_vbus>;
>> 	pinctrl-names = "idle", "active";
>> 	pinctrl-0 = <&pinctrl_usbh2_idle>;
>> 	pinctrl-1 = <&pinctrl_usbh2_active>;
>> 	fsl,usbphy = <&usbphy_dummy>;
>> 	phy_type = "hsic";
>> 	status = "okay";
>> 	#address-cells = <1>;
>> 	#size-cells = <0>;
>>
>> 	usbnet: smsc@1 {
>> 		compatible = "usb424,9730";
>> 		/* Filled in by U-Boot */
>> 		mac-address = [00 00 00 00 00 00];
>> 		reg = <1>;
>> 	};
>> };
>>
>> [...]
>>
>> pinctrl_usbh2_idle: usbh2grp-idle {
>>     fsl,pins = <
>>       MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
>>       MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40013030
>>     >;
>> };
>>
>> pinctrl_usbh2_active: usbh2grp-active {
>>     fsl,pins = <
>>       MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
>>       MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40017030
>>     >;
>> };
>>
>>
>> Are there any test cases I should try?
>> How can I test suspend/resume?
>>
> 
> - System suspend/resume
> 1. Enable USB wakeup
> for i in $(find /sys -name wakeup | grep usb);do echo enabled  > $i;echo "echo enabled > $i";done;
> 2. Let the system enter suspend using below command
> echo mem > /sys/power/state
> 3. And see if there is a wakeup block system entering suspend, and check if USB HSIC works ok
> after system resume

System suspend/resume seems to work fine. After resume the ethernet 
controller works.

root@imxceet-solo-s-43:~# echo mem > /sys/power/state
PM: suspend entry (deep)
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.001 seconds) done.
OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
smsc95xx 3-1:1.0 eth1: entering SUSPEND2 mode
PM: suspend devices took 0.050 seconds
Disabling non-boot CPUs ...
usb 3-1: reset high-speed USB device number 2 using ci_hdrc
PM: resume devices took 0.590 seconds
OOM killer enabled.
Restarting tasks ... done.
PM: suspend exit
smsc95xx 3-1:1.0 eth1: link up, 100Mbps, full-duplex, lpa 0x4DE1
fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx

> 
> - Runtime suspend
> 1. Enable auto suspend for all USB devices, and check if USBOH3 clock is closed,
> make sure do not plug any ethernet cable on the RJ45 port.
> 
> /* Enable auto suspend */
> for i in $(find /sys -name control | grep usb);do echo auto  > $i;echo "echo auto > $i";done;

This doesn't work. When the port is suspended it gets into a loop of 
suspending/resuming endlessly. If i put two logs in 
ci_hdrc_imx_runtime_suspend() and ci_hdrc_imx_runtime_resume(), I get this:

[...]
ci_hdrc_imx_runtime_resume:603
ci_hdrc_imx_runtime_suspend:574
ci_hdrc_imx_runtime_resume:603
ci_hdrc_imx_runtime_suspend:574
ci_hdrc_imx_runtime_resume:603
ci_hdrc_imx_runtime_suspend:574
ci_hdrc_imx_runtime_resume:603
ci_hdrc_imx_runtime_suspend:574
[...]

Any ideas how to debug?

Thanks,
Frieder

> 
> /* Scripts to see USB clock */
> root@imx8qxpmek:~# cat uclk.sh
> /home/root/dump-clocks.sh | grep usb
> root@imx8qxpmek:~# cat /home/root/dump-clocks.sh
> #!/bin/bash
> 
> if ! mount|grep -sq '/sys/kernel/debug'; then
>          mount -t debugfs none /sys/kernel/debug
> fi
> 
> cat /sys/kernel/debug/clk/clk_summary
> 
> 2. If USB OH3 clock can close successfully, it means USB HSIC can enter suspend successfully.
> 
> 3. Plug ethernet cable to see if the link can be on.
> 
> The purpose of above two tests is to see if USB HSIC can be suspended.
> 
> Thanks.
> 
> Peter
> 
> 
>> I also have some suggestions for your patch. Please see the separate email.
>>
>> Thanks,
>> Frieder
>>
>>>
>>> Peter Chen (4):
>>>     usb: chipidea: add flag for imx hsic implementation
>>>     usb: chipidea: imx: add HSIC support
>>>     usb: chipidea: host: override ehci->hub_control
>>>     doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups
>>>
>>>    .../devicetree/bindings/usb/ci-hdrc-usb2.txt       |   1 +
>>>    drivers/usb/chipidea/ci_hdrc_imx.c                 | 153 ++++++++++++++++++---
>>>    drivers/usb/chipidea/ci_hdrc_imx.h                 |   9 +-
>>>    drivers/usb/chipidea/host.c                        |  98 +++++++++++++
>>>    drivers/usb/chipidea/usbmisc_imx.c                 | 131 ++++++++++++++++++
>>>    include/linux/usb/chipidea.h                       |   3 +
>>>    6 files changed, 376 insertions(+), 19 deletions(-)
>>>
Frieder Schrempf Oct. 17, 2018, 11:04 a.m. UTC | #4
On 17.10.18 11:56, Frieder Schrempf wrote:
> Hi Peter,
> 
> On 17.10.18 09:23, Peter Chen wrote:
>>> On 16.10.18 07:01, Peter Chen wrote:
>>>> Most of NXP (Freescale) i.mx USB part has HSIC support, in this
>>>> series, we add support for them, it should cover all imx6 and imx7d. I
>>>> have no HSIC interface board which is supported by upstream kernel, so
>>>> this patches are only compiled ok, Frieder Schrempf, would you please
>>>> help me test it on your board? Thanks.
>>>
>>> Thank you for providing the patch!
>>> I applied it to v4.19-rc8 and tested and the LAN9730 comes up and 
>>> works just fine.
>>>
>>> Here is how my devicetree looks like:
>>>
>>> usbphy_dummy: usbphy_dummy@1 {
>>>     compatible = "usb-nop-xceiv";
>>> };
>>>
>>> [...]
>>>
>>> &usbh2 {
>>>     vbus-supply = <&reg_usb_h2_vbus>;
>>>     pinctrl-names = "idle", "active";
>>>     pinctrl-0 = <&pinctrl_usbh2_idle>;
>>>     pinctrl-1 = <&pinctrl_usbh2_active>;
>>>     fsl,usbphy = <&usbphy_dummy>;
>>>     phy_type = "hsic";
>>>     status = "okay";
>>>     #address-cells = <1>;
>>>     #size-cells = <0>;
>>>
>>>     usbnet: smsc@1 {
>>>         compatible = "usb424,9730";
>>>         /* Filled in by U-Boot */
>>>         mac-address = [00 00 00 00 00 00];
>>>         reg = <1>;
>>>     };
>>> };
>>>
>>> [...]
>>>
>>> pinctrl_usbh2_idle: usbh2grp-idle {
>>>     fsl,pins = <
>>>       MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
>>>       MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40013030
>>>     >;
>>> };
>>>
>>> pinctrl_usbh2_active: usbh2grp-active {
>>>     fsl,pins = <
>>>       MX6QDL_PAD_RGMII_TXC__USB_H2_DATA       0x40013030
>>>       MX6QDL_PAD_RGMII_TX_CTL__USB_H2_STROBE  0x40017030
>>>     >;
>>> };
>>>
>>>
>>> Are there any test cases I should try?
>>> How can I test suspend/resume?
>>>
>>
>> - System suspend/resume
>> 1. Enable USB wakeup
>> for i in $(find /sys -name wakeup | grep usb);do echo enabled  > 
>> $i;echo "echo enabled > $i";done;
>> 2. Let the system enter suspend using below command
>> echo mem > /sys/power/state
>> 3. And see if there is a wakeup block system entering suspend, and 
>> check if USB HSIC works ok
>> after system resume
> 
> System suspend/resume seems to work fine. After resume the ethernet 
> controller works.
> 
> root@imxceet-solo-s-43:~# echo mem > /sys/power/state
> PM: suspend entry (deep)
> PM: Syncing filesystems ... done.
> Freezing user space processes ... (elapsed 0.001 seconds) done.
> OOM killer disabled.
> Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> smsc95xx 3-1:1.0 eth1: entering SUSPEND2 mode
> PM: suspend devices took 0.050 seconds
> Disabling non-boot CPUs ...
> usb 3-1: reset high-speed USB device number 2 using ci_hdrc
> PM: resume devices took 0.590 seconds
> OOM killer enabled.
> Restarting tasks ... done.
> PM: suspend exit
> smsc95xx 3-1:1.0 eth1: link up, 100Mbps, full-duplex, lpa 0x4DE1
> fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> 
>>
>> - Runtime suspend
>> 1. Enable auto suspend for all USB devices, and check if USBOH3 clock 
>> is closed,
>> make sure do not plug any ethernet cable on the RJ45 port.
>>
>> /* Enable auto suspend */
>> for i in $(find /sys -name control | grep usb);do echo auto  > $i;echo 
>> "echo auto > $i";done;
> 
> This doesn't work. When the port is suspended it gets into a loop of 
> suspending/resuming endlessly. If i put two logs in 
> ci_hdrc_imx_runtime_suspend() and ci_hdrc_imx_runtime_resume(), I get this:
> 
> [...]
> ci_hdrc_imx_runtime_resume:603
> ci_hdrc_imx_runtime_suspend:574
> ci_hdrc_imx_runtime_resume:603
> ci_hdrc_imx_runtime_suspend:574
> ci_hdrc_imx_runtime_resume:603
> ci_hdrc_imx_runtime_suspend:574
> ci_hdrc_imx_runtime_resume:603
> ci_hdrc_imx_runtime_suspend:574
> [...]

Ok, forget about the loop, this was caused by one of the other ports, 
that had issues with overcurrent detection.

But still it doesn't work for the HSIC port.

The HSIC device is stuck in status "suspending" (note: "suspending" and 
not "suspended"):

~# cat /sys/bus/usb/devices/usb1/power/runtime_status
suspended
~# cat /sys/bus/usb/devices/usb2/power/runtime_status
suspended
~# cat /sys/bus/usb/devices/usb3/power/runtime_status
active
~# cat /sys/bus/usb/devices/usb3/3-1/power/runtime_status
suspending
Frieder Schrempf Oct. 17, 2018, 2:59 p.m. UTC | #5
On 17.10.18 13:04, Frieder Schrempf wrote:
> On 17.10.18 11:56, Frieder Schrempf wrote:
[...]
>>> - System suspend/resume
>>> 1. Enable USB wakeup
>>> for i in $(find /sys -name wakeup | grep usb);do echo enabled  > 
>>> $i;echo "echo enabled > $i";done;
>>> 2. Let the system enter suspend using below command
>>> echo mem > /sys/power/state
>>> 3. And see if there is a wakeup block system entering suspend, and 
>>> check if USB HSIC works ok
>>> after system resume
>>
>> System suspend/resume seems to work fine. After resume the ethernet 
>> controller works.
>>
>> root@imxceet-solo-s-43:~# echo mem > /sys/power/state
>> PM: suspend entry (deep)
>> PM: Syncing filesystems ... done.
>> Freezing user space processes ... (elapsed 0.001 seconds) done.
>> OOM killer disabled.
>> Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>> smsc95xx 3-1:1.0 eth1: entering SUSPEND2 mode
>> PM: suspend devices took 0.050 seconds
>> Disabling non-boot CPUs ...
>> usb 3-1: reset high-speed USB device number 2 using ci_hdrc
>> PM: resume devices took 0.590 seconds
>> OOM killer enabled.
>> Restarting tasks ... done.
>> PM: suspend exit
>> smsc95xx 3-1:1.0 eth1: link up, 100Mbps, full-duplex, lpa 0x4DE1
>> fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>
>>>
>>> - Runtime suspend
>>> 1. Enable auto suspend for all USB devices, and check if USBOH3 clock 
>>> is closed,
>>> make sure do not plug any ethernet cable on the RJ45 port.
>>>
>>> /* Enable auto suspend */
>>> for i in $(find /sys -name control | grep usb);do echo auto  > 
>>> $i;echo "echo auto > $i";done;
>>
>> This doesn't work. When the port is suspended it gets into a loop of 
>> suspending/resuming endlessly. If i put two logs in 
>> ci_hdrc_imx_runtime_suspend() and ci_hdrc_imx_runtime_resume(), I get 
>> this:
>>
>> [...]
>> ci_hdrc_imx_runtime_resume:603
>> ci_hdrc_imx_runtime_suspend:574
>> ci_hdrc_imx_runtime_resume:603
>> ci_hdrc_imx_runtime_suspend:574
>> ci_hdrc_imx_runtime_resume:603
>> ci_hdrc_imx_runtime_suspend:574
>> ci_hdrc_imx_runtime_resume:603
>> ci_hdrc_imx_runtime_suspend:574
>> [...]
> 
> Ok, forget about the loop, this was caused by one of the other ports, 
> that had issues with overcurrent detection.
> 
> But still it doesn't work for the HSIC port.
> 
> The HSIC device is stuck in status "suspending" (note: "suspending" and 
> not "suspended"):
> 
> ~# cat /sys/bus/usb/devices/usb1/power/runtime_status
> suspended
> ~# cat /sys/bus/usb/devices/usb2/power/runtime_status
> suspended
> ~# cat /sys/bus/usb/devices/usb3/power/runtime_status
> active
> ~# cat /sys/bus/usb/devices/usb3/3-1/power/runtime_status
> suspending

It seems like this is a problem with the smsc95xx driver. I fixed this 
and now the device is suspending properly, when the network interface is 
down and resumes when it comes up.

I also checked the USBOH3 clock and its properly switched on and off.

What didn't work is auto suspend and resume on ethernet link down/up, 
but this seems to be a restriction of the smsc95xx/usbnet driver.

So all in all this looks good to me. Please let me know if you need any 
more information or tests.

Thanks,
Frieder
Peter Chen Oct. 18, 2018, 1:22 a.m. UTC | #6
> >>> - System suspend/resume
> >>> 1. Enable USB wakeup
> >>> for i in $(find /sys -name wakeup | grep usb);do echo enabled  >
> >>> $i;echo "echo enabled > $i";done; 2. Let the system enter suspend
> >>> using below command echo mem > /sys/power/state 3. And see if there
> >>> is a wakeup block system entering suspend, and check if USB HSIC
> >>> works ok after system resume
> >>
> >> System suspend/resume seems to work fine. After resume the ethernet
> >> controller works.
> >>
> >> root@imxceet-solo-s-43:~# echo mem > /sys/power/state
> >> PM: suspend entry (deep)
> >> PM: Syncing filesystems ... done.
> >> Freezing user space processes ... (elapsed 0.001 seconds) done.
> >> OOM killer disabled.
> >> Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> >> smsc95xx 3-1:1.0 eth1: entering SUSPEND2 mode
> >> PM: suspend devices took 0.050 seconds Disabling non-boot CPUs ...
> >> usb 3-1: reset high-speed USB device number 2 using ci_hdrc
> >> PM: resume devices took 0.590 seconds OOM killer enabled.
> >> Restarting tasks ... done.
> >> PM: suspend exit
> >> smsc95xx 3-1:1.0 eth1: link up, 100Mbps, full-duplex, lpa 0x4DE1 fec
> >> 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> >>
> >>>
> >>> - Runtime suspend
> >>> 1. Enable auto suspend for all USB devices, and check if USBOH3
> >>> clock is closed, make sure do not plug any ethernet cable on the
> >>> RJ45 port.
> >>>
> >>> /* Enable auto suspend */
> >>> for i in $(find /sys -name control | grep usb);do echo auto  >
> >>> $i;echo "echo auto > $i";done;
> >>
> >> This doesn't work. When the port is suspended it gets into a loop of
> >> suspending/resuming endlessly. If i put two logs in
> >> ci_hdrc_imx_runtime_suspend() and ci_hdrc_imx_runtime_resume(), I get
> >> this:
> >>
> >> [...]
> >> ci_hdrc_imx_runtime_resume:603
> >> ci_hdrc_imx_runtime_suspend:574
> >> ci_hdrc_imx_runtime_resume:603
> >> ci_hdrc_imx_runtime_suspend:574
> >> ci_hdrc_imx_runtime_resume:603
> >> ci_hdrc_imx_runtime_suspend:574
> >> ci_hdrc_imx_runtime_resume:603
> >> ci_hdrc_imx_runtime_suspend:574
> >> [...]
> >
> > Ok, forget about the loop, this was caused by one of the other ports,
> > that had issues with overcurrent detection.
> >
> > But still it doesn't work for the HSIC port.
> >
> > The HSIC device is stuck in status "suspending" (note: "suspending"
> > and not "suspended"):
> >
> > ~# cat /sys/bus/usb/devices/usb1/power/runtime_status
> > suspended
> > ~# cat /sys/bus/usb/devices/usb2/power/runtime_status
> > suspended
> > ~# cat /sys/bus/usb/devices/usb3/power/runtime_status
> > active
> > ~# cat /sys/bus/usb/devices/usb3/3-1/power/runtime_status
> > suspending
> 
> It seems like this is a problem with the smsc95xx driver. I fixed this and now the
> device is suspending properly, when the network interface is down and resumes
> when it comes up.
> 
> I also checked the USBOH3 clock and its properly switched on and off.
> 
> What didn't work is auto suspend and resume on ethernet link down/up, but this
> seems to be a restriction of the smsc95xx/usbnet driver.
> 
> So all in all this looks good to me. Please let me know if you need any more
> information or tests.
> 

Thanks, Frieder, no more tests are needed.
You could send me your dts changes as patches, I will append it at my v2 patch series.

Peter
Frieder Schrempf Oct. 18, 2018, 7:46 a.m. UTC | #7
On 18.10.18 03:22, Peter Chen wrote:
>   
>>>>> - System suspend/resume
>>>>> 1. Enable USB wakeup
>>>>> for i in $(find /sys -name wakeup | grep usb);do echo enabled  >
>>>>> $i;echo "echo enabled > $i";done; 2. Let the system enter suspend
>>>>> using below command echo mem > /sys/power/state 3. And see if there
>>>>> is a wakeup block system entering suspend, and check if USB HSIC
>>>>> works ok after system resume
>>>>
>>>> System suspend/resume seems to work fine. After resume the ethernet
>>>> controller works.
>>>>
>>>> root@imxceet-solo-s-43:~# echo mem > /sys/power/state
>>>> PM: suspend entry (deep)
>>>> PM: Syncing filesystems ... done.
>>>> Freezing user space processes ... (elapsed 0.001 seconds) done.
>>>> OOM killer disabled.
>>>> Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>>>> smsc95xx 3-1:1.0 eth1: entering SUSPEND2 mode
>>>> PM: suspend devices took 0.050 seconds Disabling non-boot CPUs ...
>>>> usb 3-1: reset high-speed USB device number 2 using ci_hdrc
>>>> PM: resume devices took 0.590 seconds OOM killer enabled.
>>>> Restarting tasks ... done.
>>>> PM: suspend exit
>>>> smsc95xx 3-1:1.0 eth1: link up, 100Mbps, full-duplex, lpa 0x4DE1 fec
>>>> 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>>
>>>>>
>>>>> - Runtime suspend
>>>>> 1. Enable auto suspend for all USB devices, and check if USBOH3
>>>>> clock is closed, make sure do not plug any ethernet cable on the
>>>>> RJ45 port.
>>>>>
>>>>> /* Enable auto suspend */
>>>>> for i in $(find /sys -name control | grep usb);do echo auto  >
>>>>> $i;echo "echo auto > $i";done;
>>>>
>>>> This doesn't work. When the port is suspended it gets into a loop of
>>>> suspending/resuming endlessly. If i put two logs in
>>>> ci_hdrc_imx_runtime_suspend() and ci_hdrc_imx_runtime_resume(), I get
>>>> this:
>>>>
>>>> [...]
>>>> ci_hdrc_imx_runtime_resume:603
>>>> ci_hdrc_imx_runtime_suspend:574
>>>> ci_hdrc_imx_runtime_resume:603
>>>> ci_hdrc_imx_runtime_suspend:574
>>>> ci_hdrc_imx_runtime_resume:603
>>>> ci_hdrc_imx_runtime_suspend:574
>>>> ci_hdrc_imx_runtime_resume:603
>>>> ci_hdrc_imx_runtime_suspend:574
>>>> [...]
>>>
>>> Ok, forget about the loop, this was caused by one of the other ports,
>>> that had issues with overcurrent detection.
>>>
>>> But still it doesn't work for the HSIC port.
>>>
>>> The HSIC device is stuck in status "suspending" (note: "suspending"
>>> and not "suspended"):
>>>
>>> ~# cat /sys/bus/usb/devices/usb1/power/runtime_status
>>> suspended
>>> ~# cat /sys/bus/usb/devices/usb2/power/runtime_status
>>> suspended
>>> ~# cat /sys/bus/usb/devices/usb3/power/runtime_status
>>> active
>>> ~# cat /sys/bus/usb/devices/usb3/3-1/power/runtime_status
>>> suspending
>>
>> It seems like this is a problem with the smsc95xx driver. I fixed this and now the
>> device is suspending properly, when the network interface is down and resumes
>> when it comes up.
>>
>> I also checked the USBOH3 clock and its properly switched on and off.
>>
>> What didn't work is auto suspend and resume on ethernet link down/up, but this
>> seems to be a restriction of the smsc95xx/usbnet driver.
>>
>> So all in all this looks good to me. Please let me know if you need any more
>> information or tests.
>>
> 
> Thanks, Frieder, no more tests are needed.
> You could send me your dts changes as patches, I will append it at my v2 patch series.

My board is currently off-tree so I can't send any patch for the pinmux 
settings in devicetree. But I will send a patch with the changes that 
should go to imx6qdl.dtsi, imx6sl.dtsi and imx6sx.dtsi. Though I only 
tested on i.MX6S.

Thanks,
Frieder
Peter Chen Oct. 18, 2018, 8:48 a.m. UTC | #8
> >
> > Thanks, Frieder, no more tests are needed.
> > You could send me your dts changes as patches, I will append it at my v2 patch
> series.
> 
> My board is currently off-tree so I can't send any patch for the pinmux settings in
> devicetree. But I will send a patch with the changes that should go to imx6qdl.dtsi,
> imx6sl.dtsi and imx6sx.dtsi. Though I only tested on i.MX6S.
> 

No, the changes are board level specific, you could not add it into SoC file.

Peter
Frieder Schrempf Oct. 18, 2018, 8:56 a.m. UTC | #9
On 18.10.18 10:48, Peter Chen wrote:
>   
>>>
>>> Thanks, Frieder, no more tests are needed.
>>> You could send me your dts changes as patches, I will append it at my v2 patch
>> series.
>>
>> My board is currently off-tree so I can't send any patch for the pinmux settings in
>> devicetree. But I will send a patch with the changes that should go to imx6qdl.dtsi,
>> imx6sl.dtsi and imx6sx.dtsi. Though I only tested on i.MX6S.
>>
> 
> No, the changes are board level specific, you could not add it into SoC file.

Not all changes are board specific. The HSIC-only host controllers can 
only be used to interface with on-board chips via HSIC. So they need a 
"usb-nop-xceiv" dummy PHY in all use cases.

So if I understand this correctly this should be done in the SoCs dtsi 
files like in this patch I just sent: [1].

Please also note that for i.MX7S this is already added in imx7s.dtsi: [2]

Thanks,
Frieder

[1]: https://lkml.org/lkml/2018/10/18/414
[2]: 
https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/imx7s.dtsi#L88
Peter Chen Oct. 18, 2018, 9:04 a.m. UTC | #10
> >> My board is currently off-tree so I can't send any patch for the
> >> pinmux settings in devicetree. But I will send a patch with the
> >> changes that should go to imx6qdl.dtsi, imx6sl.dtsi and imx6sx.dtsi. Though I
> only tested on i.MX6S.
> >>
> >
> > No, the changes are board level specific, you could not add it into SoC file.
> 
> Not all changes are board specific. The HSIC-only host controllers can only be used
> to interface with on-board chips via HSIC. So they need a "usb-nop-xceiv" dummy
> PHY in all use cases.
> 
> So if I understand this correctly this should be done in the SoCs dtsi files like in this
> patch I just sent: [1].
> 
> Please also note that for i.MX7S this is already added in imx7s.dtsi: [2]
> 

Oh, I have misunderstood your meaning, the [1] is needed since it is for SoC.
I thought your board level changes (like pinctrl) would go into SoC dtsi file.
If there are no more comments, I will send v2 next week, feel free to add your
reviewed-by and tested-by tag, thanks.

Peter

> 
> [1]:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2
> Flkml%2F2018%2F10%2F18%2F414&amp;data=02%7C01%7Cpeter.chen%40nxp.
> com%7C3c73fdf4ec98477da49308d634d78b39%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C636754497687793991&amp;sdata=aHhMhaj%2FbdTFu0B
> qXi0loiBh%2BHzl%2F%2BTSw3txyjBSNLQ%3D&amp;reserved=0
> [2]:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com
> %2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Farch%2Farm%2Fboot%2Fdts%2Fim
> x7s.dtsi%23L88&amp;data=02%7C01%7Cpeter.chen%40nxp.com%7C3c73fdf4ec9
> 8477da49308d634d78b39%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636754497687793991&amp;sdata=7cVhAMQpWWYjGHjY8Xh2g0CG3jPaeG8X
> 9K368AooQcc%3D&amp;reserved=0