Re: [PATCH v3 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

On Wed, Dec 5, 2018 at 5:57 AM PETER CHEN  wrote:

> So, there are two examples at binding-doc, one for normal, one for HSIC? 
> Fabio, do you
> mean that? If DT maintainer agrees it too, I will add it.

Yes, I think it will makes things clearer. Thanks

Re: [PATCH v3 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

Hi Frieder,

On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder
 wrote:

> There are many other optional properties for this driver and a lot of
> them are not in the given example. Maybe we should just keep the
> pinctrls for HSIC-mode out of the example, too?

I am just trying to make life easier for those who want to use HSIC
support with chipidea.

Can we just add a real dts snippet example of your board into the
binding document?

Re: [PATCH] usb: chipidea: imx: unify over-current polarity handling

[Fabio Estevam]

Hi Uwe,

[Adding Matt]

On Fri, Nov 30, 2018 at 6:53 PM Uwe Kleine-König
 wrote:
>
> The status quo is:
>  - on i.MX25 the overcurrent polarity is always explicitly configured as
>active high which matches the reset default.
>  - on i.MX6 and i.MX7 the overcurrent polarity is active high after
>reset. usbmisc_imx6q_init() and usbmisc_imx7d_init() keep the current
>state (probably as setup by the bootloader or still the reset
>default) unless the polarity is explicitly configured as active high
>which then is configured. (So if the pin is active low and the
>bootloader didn't set this up the configuration is wrong.)
>
> To improve the situation always configure the reset default value unless
> the device tree configures the polarity (and then use this one).
>
> Note that as the reset default is active high on all platforms there is
> no need to check for the presence of "over-current-active-high". In the
> absence of "over-current-active-low" it doesn't matter if active high is
> configured because that's the default or because it is configured
> explicitly.
>
> Signed-off-by: Uwe Kleine-König 

It seems that Matt is also interested in adding the
'over-current-active-low' property:
https://patchwork.kernel.org/patch/10701311/

Re: [PATCH v3 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

Hi Peter,

On Tue, Nov 27, 2018 at 7:31 AM PETER CHEN  wrote:
>
> For USB HSIC, the data and strobe pin needs to be pulled down
> at default, we consider it as "idle" state. When the USB host
> is ready to be used, the strobe pin needs to be pulled up,
> we consider it as "active" state.
>
> Signed-off-by: Peter Chen 
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt 
> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 529e51879fb2..1e5e7ddfb1a5 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -80,7 +80,10 @@ Optional properties:
>controller. It's expected that a mux state of 0 indicates device mode and a
>mux state of 1 indicates host mode.
>  - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
> -- pinctrl-names: Names for optional pin modes in "default", "host", "device"
> +- pinctrl-names: Names for optional pin modes in "default", "host", "device".
> +  In case of HSIC-mode, "idle" and "active" pin modes are mandatory. In this
> +  case, the "idle" state needs to pull down the data and strobe pin
> +  and the "active" state needs to pull up the strobe pin.
>  - pinctrl-n: alternate pin modes
>
>  i.mx specific properties
> @@ -110,4 +113,7 @@ Example:
> phy-clkgate-delay-us = <400>;
> mux-controls = <_switch>;
> mux-control-names = "usb_switch";
> +   pinctrl-names = "idle", "active";
> +   pinctrl-0 = <_usbh2_1>;
> +   pinctrl-1 = <_usbh2_2>;

I would put the "idle" and "active" entries inside a explicit section for HSIC.

Otherwise, it may confuse people using non-HSIC bindings.

Re: Set reference clock for onboard hub in DT

[Fabio Estevam]

Hi Frieder,

On Thu, Sep 20, 2018 at 10:52 AM Frieder Schrempf
 wrote:
>
> Hi,
>
> I have a question concerning the setup for a board with an onboard USB
> hub. The SoC (i.MX6S) is expected to provide a 12 MHz clock on one of
> the clock output pins as a reference for the USB hub.
>
> Now I was looking for a way to configure this in the DT, which works
> fine up to the point, that there doesn't seem to be a way to set a
> reference clock for a generic USB device.
>
> Would it make sense to implement the enabling/disabling of the clock in
> the generic USB device driver?
> It doesn't seem right to write a separate driver for the hub only to
> turn the clock on and off.
>
> My intended DT setup would look something like this:
>
>  {
> vbus-supply = <_usb_h1_vbus>;
> pinctrl-names = "default";
> pinctrl-0 = <_usbh1>;
> dr_mode = "host";
> status = "okay";
> #address-cells = <1>;
> #size-cells = <0>;
>
> hub@1 {
> compatible = "usb4b4,6570";
> reg = <1>;
> clocks = < IMX6QDL_CLK_CKO>;
> assigned-clocks = < IMX6QDL_CLK_CKO>,
>   < IMX6QDL_CLK_CKO2_SEL>,
>   < IMX6QDL_CLK_CKO2>;
> assigned-clock-parents = < IMX6QDL_CLK_CKO2>,
>  < IMX6QDL_CLK_OSC>,
>  < IMX6QDL_CLK_CKO2_PODF>;
> assigned-clock-rates = <0 0 1200>;
> }
> };

Peter Chen has tried to implement such support:
https://lkml.org/lkml/2017/6/21/88

However it still needs some work so that it can be accepted in mainline.

Re: [PATCH v2 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

On Mon, Oct 22, 2018 at 11:55 AM Schrempf Frieder
 wrote:

> I think you forgot to improve this description. Maybe something like this:
>
> pinctrl-names: Names for optional pin modes for "default", "host" or
> "device". In case of HSIC-mode "idle" and "active" pin
> modes are mandatory. In this case the "idle" state needs
> to pull down the data and strobe pin and the "active"
> state needs to pull up the strobe pin.

Correct, and the dts example is also missing.

Re: [PATCH 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups

[Fabio Estevam]

Hi Peter,

On Tue, Oct 16, 2018 at 2:02 AM Peter Chen  wrote:
>
> For USB HSIC, the data and strobe pin needs to be pulled down
> at default, we consider it as "idle" state. When the USB host
> is ready to be used, the strobe pin needs to be pulled up,
> we consider it as "active" state.
>
> Signed-off-by: Peter Chen 
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt 
> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 529e51879fb2..10c8d793ea49 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -81,6 +81,7 @@ Optional properties:
>mux state of 1 indicates host mode.
>  - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
>  - pinctrl-names: Names for optional pin modes in "default", "host", "device"
> +  Or names for HSIC "idle" and "active" pin modes.

I don't think this description is clear enough.

Could you please add a real dts snippet for the HSIC case instead?

Re: [PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

Hi Peter,

On Wed, Jul 4, 2018 at 5:24 AM, Peter Chen  wrote:

> It seems there is no harm if we always include drivers/usb/chipidea/ulpi.c 
> except increasing
> a little code size, how about always build ulpi.c for the whole chipidea at 
> Makefile, delete
> CONFIG_USB_CHIPIDEA_ULPI as well, meanwhile, we need to select USB_ULPI_BUS
> at Kconfig?
>
> Besides, at function ci_ulpi_resume for ulpi.c, we need to add condition to 
> quit for non-ulpi interface.

Yes, I sent a v2 letting ulpi.c to be always built-in.
--
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

[PATCH v2] usb: chipidea: Always build ULPI code

[Fabio Estevam]

From: Fabio Estevam 

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 
---
Changes since v1:
- Instead of warning when CONFIG_USB_CHIPIDEA_ULPI is not selected,
let the chipidea ULPI code always to be built-in. (Peter)

 drivers/usb/chipidea/Kconfig  | 9 +
 drivers/usb/chipidea/Makefile | 3 +--
 drivers/usb/chipidea/ci.h | 8 
 drivers/usb/chipidea/ulpi.c   | 6 ++
 4 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
index 785f0ed..ee34e90 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 0bf244d..6a2cc5c 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -240,10 +240,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;
@@ -426,15 +424,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 6da42dc..bf57f33 100644
--- a/drivers/usb/chipidea/ulpi.c
+++ b/drivers/usb/chipidea/ulpi.c
@@ -85,6 +85,9 @@ int ci_ulpi_init(struct ci_hdrc *ci)
 
 void ci_ulpi_exit(struct ci_hdrc *ci)
 {
+   if (ci->platdata->phy_mode != USBPHY_INTERFACE_MODE_ULPI)
+   return;
+
if (ci->ulpi) {
ulpi_unregister_interface(ci->ulpi);
ci->ulpi = NULL;
@@ -95,6 +98,9 @@ int ci_ulpi_resume(struct ci_hdrc *ci)
 {
int cnt = 10;
 
+   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

--
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

Re: [PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

Hi Shawn,

On Tue, Jul 3, 2018 at 12:40 AM, Shawn Guo  wrote:

> We can have the options in defconfig, but they can still be turned off
> for whatever reason and we get the hang.  Really, missing a user
> selectable option in defconfig shouldn't result in a system hang.

Yes, 100% agree and this is exactly what this USB patch is all about :-)

The whole point of this USB patch is not to let the the system to hang
when the defconfig options are turned off.

It will warn the user that CONFIG_USB_CHIPIDEA_ULPI needs to be
selected, will propagate an error, but at least the system will not
hang.

The user will certainly notice the big warning which says "Select
CONFIG_USB_CHIPIDEA_ULPI in order to use ULPI mode"

Please check the original patch of this thread:
https://patchwork.kernel.org/patch/10486371/
--
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

Re: [PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

Hi Shawn,

On Mon, Jul 2, 2018 at 11:06 PM, Shawn Guo  wrote:

> A second thought on this - shouldn't such dependency be solved by
> Kconfig select clause?

I suspect we are not able to fix it via Kconfig as we really don't
know if a system uses ULPI interface or not via Kconfig perspective.

If we always select the chipidea ULPI driver, then it will be dead
code for boards that do not use ULPI.

Currently there are some defconfigs that explicitly select the
chipidea ulpi driver:

arch/arm/configs/qcom_defconfig:CONFIG_USB_CHIPIDEA_ULPI=y
arch/arm64/configs/defconfig:CONFIG_USB_CHIPIDEA_ULPI=y

I would be interested to get feedback whether it would be possible to
fix it on a different way.

Thanks
--
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

Re: [PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

On Mon, Jul 2, 2018 at 10:48 PM, Shawn Guo  wrote:

> I will make sure both defconfig patches go into 4.18-rc.

Thanks, Shawn!

Peter,

Could you please then queue this chipidea patch for 4.18-rc?

Thanks
--
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

Re: [PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

Hi Peter,

On Mon, Jul 2, 2018 at 12:08 AM, Peter Chen  wrote:

> Fabio, since this function has dependency with defconfig, is the defconfig 
> change a fix for v4.18-rc
> or for v4.19-rc1?

I have sent the defconfig changes for imx_v6_v7_defconfig and
imx_v4_v5_defconfig and hope that they could be applied for 4.18-rc.

Maybe they could go to the same tree?
--
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

Re: [PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

Hi Peter,

On Mon, Jun 25, 2018 at 11:51 PM, Peter Chen  wrote:

> Fabio, I wonder it may cause the USB not work at imx27 which
> do not use this configuration now. Any possibilities to test and verify it?

I don't have access to a mx27 board, but I can send a patch that
selects CONFIG_USB_CHIPIDEA_ULPI in imx_v4_v5_defconfig.

I have already sent a patch that selects CONFIG_USB_CHIPIDEA_ULPI in
imx_v6_v7_defconfig.
--
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

[PATCH v2] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

From: Fabio Estevam 

Since commit 03e6275ae381087bd8 ("usb: chipidea: Fix ULPI on imx51") the
kernel hangs on a imx51-babbage board, when using the ULPI interface with
the CONFIG_USB_CHIPIDEA_ULPI option unselected.

Instead of hanging the kernel, let's warn the user that
CONFIG_USB_CHIPIDEA_ULPI needs to be selected, propagate an error
and continue with booting the kernel.

The user can then react to the warning and select
CONFIG_USB_CHIPIDEA_ULPI as needed.

Fixes: 03e6275ae381087bd8 ("usb: chipidea: Fix ULPI on imx51")
Suggested-by: Lucas Stach 
Signed-off-by: Fabio Estevam 
---
Changes since v1:
- Only propagate the error when CONFIG_USB_CHIPIDEA_ULPI is unselected

 drivers/usb/chipidea/core.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 85fc6db..e7018a1 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -608,6 +608,17 @@ static int ci_get_platdata(struct device *dev,
if (!platdata->phy_mode)
platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
 
+   if (platdata->phy_mode == USBPHY_INTERFACE_MODE_ULPI) {
+   /*
+* CONFIG_USB_CHIPIDEA_ULPI needs to be selected
+* for proper usage of the ULPI mode
+*/
+   if (!IS_ENABLED(CONFIG_USB_CHIPIDEA_ULPI)) {
+   WARN_ONCE(1, "Select CONFIG_USB_CHIPIDEA_ULPI in order 
to use ULPI mode\n");
+   return -EINVAL;
+   }
+   }
+
if (!platdata->dr_mode)
platdata->dr_mode = usb_get_dr_mode(dev);
 
-- 
2.7.4

--
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

Re: [PATCH] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

On Mon, Jun 25, 2018 at 11:06 AM, Fabio Estevam  wrote:
> From: Fabio Estevam 
>
> Since commit 03e6275ae381087bd8 ("usb: chipidea: Fix ULPI on imx51") the
> kernel hangs on a imx51-babbage board, when using the ULPI interface with
> the CONFIG_USB_CHIPIDEA_ULPI option unselected.
>
> Instead of hanging the kernel, let's warn the user that
> CONFIG_USB_CHIPIDEA_ULPI needs to be selected, propagate an error
> and continue with booting the kernel.
>
> The user can then react to the warning and select
> CONFIG_USB_CHIPIDEA_ULPI as needed.
>
> Fixes: 03e6275ae381087bd8 ("usb: chipidea: Fix ULPI on imx51")
> Suggested-by: Lucas Stach 
> Signed-off-by: Fabio Estevam 
> ---
>  drivers/usb/chipidea/core.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 85fc6db..5ab9db1 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -608,6 +608,16 @@ static int ci_get_platdata(struct device *dev,
> if (!platdata->phy_mode)
> platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
>
> +   if (platdata->phy_mode == USBPHY_INTERFACE_MODE_ULPI) {
> +   /*
> +* CONFIG_USB_CHIPIDEA_ULPI needs to be selected
> +* for proper usage of the ULPI mode
> +*/
> +   WARN_ONCE(!IS_ENABLED(CONFIG_USB_CHIPIDEA_ULPI),
> +   "Select CONFIG_USB_CHIPIDEA_ULPI in order to use ULPI 
> mode\n");
> +   return -EINVAL;

Oops, sorry. Please discard this one.

Logic is wrong here. Will fix and submit a v2.
--
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

[PATCH] USB: chipidea: Do not hang when CONFIG_USB_CHIPIDEA_ULPI is not selected

[Fabio Estevam]

From: Fabio Estevam 

Since commit 03e6275ae381087bd8 ("usb: chipidea: Fix ULPI on imx51") the
kernel hangs on a imx51-babbage board, when using the ULPI interface with
the CONFIG_USB_CHIPIDEA_ULPI option unselected.

Instead of hanging the kernel, let's warn the user that
CONFIG_USB_CHIPIDEA_ULPI needs to be selected, propagate an error
and continue with booting the kernel.

The user can then react to the warning and select
CONFIG_USB_CHIPIDEA_ULPI as needed.

Fixes: 03e6275ae381087bd8 ("usb: chipidea: Fix ULPI on imx51")
Suggested-by: Lucas Stach 
Signed-off-by: Fabio Estevam 
---
 drivers/usb/chipidea/core.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 85fc6db..5ab9db1 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -608,6 +608,16 @@ static int ci_get_platdata(struct device *dev,
if (!platdata->phy_mode)
platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
 
+   if (platdata->phy_mode == USBPHY_INTERFACE_MODE_ULPI) {
+   /*
+* CONFIG_USB_CHIPIDEA_ULPI needs to be selected
+* for proper usage of the ULPI mode
+*/
+   WARN_ONCE(!IS_ENABLED(CONFIG_USB_CHIPIDEA_ULPI),
+   "Select CONFIG_USB_CHIPIDEA_ULPI in order to use ULPI mode\n");
+   return -EINVAL;
+   }
+
if (!platdata->dr_mode)
platdata->dr_mode = usb_get_dr_mode(dev);
 
-- 
2.7.4

--
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

Re: Continuous USB resets on NXP i.MX 6ULL device

[Fabio Estevam]

Hi Stefan,

On Wed, Feb 28, 2018 at 2:08 PM, Stefan Wahren  wrote:
> Hi,
>
> i'm not sure this is related, but i noticed something ugly in
> driver/usb/chipidea/usbmisc_imx.c.
>
> The compatible "fsl,imx6ul-usbmisc" uses imx6sx_usbmisc_ops, which uses
> usbmisc_imx6sx_init (which also calls usbmisc_imx6q_init).
> Within usbmisc_imx6q_init the flag MX6_BM_NON_BURST_SETTING is enabled.
>
> But according to the i.MX6 ULL reference manual this bit is marked as
> reserved and set to zero at reset.
>
> So i think it would be better to have separate imx6ul_usbmisc_ops.

Yes, good catch.

Peter, do you agree with this proposed change?
--
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

Re: [PATCH v2] usb: phy: mxs: Fix NULL pointer dereference on i.MX23/28

[Fabio Estevam]

Hi Felipe,

On Mon, Jan 22, 2018 at 10:28 AM, Fabio Estevam <feste...@gmail.com> wrote:
> Hi Felipe,
>
> On Thu, Jan 18, 2018 at 12:22 AM, Fabio Estevam <feste...@gmail.com> wrote:
>> From: Fabio Estevam <fabio.este...@nxp.com>
>>
>> Commit e93650994a95 ("usb: phy: mxs: add usb charger type detection")
>> causes the following kernel hang on i.MX28:
>>
>> [2.207973] usbcore: registered new interface driver usb-storage
>> [2.235659] Unable to handle kernel NULL pointer dereference at virtual 
>> address 0188
>> [2.244195] pgd = (ptrval)
>> [2.246994] [0188] *pgd=
>> [2.250676] Internal error: Oops: 5 [#1] ARM
>> [2.254979] Modules linked in:
>> [2.258089] CPU: 0 PID: 1 Comm: swapper Not tainted 
>> 4.15.0-rc8-next-20180117-2-g75d5f21 #7
>> [2.266724] Hardware name: Freescale MXS (Device Tree)
>> [2.271921] PC is at regmap_read+0x0/0x5c
>> [2.275977] LR is at mxs_phy_charger_detect+0x34/0x1dc
>>
>> mxs_phy_charger_detect() makes accesses to the anatop registers via regmap,
>> however i.MX23/28 do not have such registers, which causes a NULL pointer
>> dereference.
>>
>> Fix the issue by doing a NULL check on the 'regmap' pointer.
>>
>> Fixes: e93650994a95 ("usb: phy: mxs: add usb charger type detection")
>> Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
>
> Could this one be applied to 4.15 final so that we avoid a boot
> regression on i.MX23/28?

A gentle ping.
--
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

[PATCH v2] usb: phy: mxs: Staticize mxs_charger_secondary_detection()

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

mxs_charger_secondary_detection() is only used in this file, so make
it static.

This fixes the following sparse warning:

drivers/usb/phy/phy-mxs-usb.c:581:23: warning: symbol 
'mxs_charger_secondary_detection' was not declared. Should it be static?

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- Fix typo in teh Subject (Staticize)

 drivers/usb/phy/phy-mxs-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index da031c4..67371a2 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -578,7 +578,7 @@ static enum usb_charger_type 
mxs_charger_primary_detection(struct mxs_phy *x)
  * It must be called after DP is pulled up, which is used to
  * differentiate DCP and CDP.
  */
-enum usb_charger_type mxs_charger_secondary_detection(struct mxs_phy *x)
+static enum usb_charger_type mxs_charger_secondary_detection(struct mxs_phy *x)
 {
struct regmap *regmap = x->regmap_anatop;
int val;
-- 
2.7.4

--
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

[PATCH] usb: phy: mxs: Stactize mxs_charger_secondary_detection()

[Fabio Estevam]

mxs_charger_secondary_detection() is only used in this file, so make
it static.

This fixes the following sparse warning:

drivers/usb/phy/phy-mxs-usb.c:581:23: warning: symbol 
'mxs_charger_secondary_detection' was not declared. Should it be static?

Signed-off-by: Fabio Estevam <feste...@gmail.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index da031c4..67371a2 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -578,7 +578,7 @@ static enum usb_charger_type 
mxs_charger_primary_detection(struct mxs_phy *x)
  * It must be called after DP is pulled up, which is used to
  * differentiate DCP and CDP.
  */
-enum usb_charger_type mxs_charger_secondary_detection(struct mxs_phy *x)
+static enum usb_charger_type mxs_charger_secondary_detection(struct mxs_phy *x)
 {
struct regmap *regmap = x->regmap_anatop;
int val;
-- 
2.7.4

--
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

Re: [U-Boot] [PATCH] imx: mx25: Remove SION bit in all pin-mux

[Fabio Estevam]

Hi Michael,

On Wed, Jan 24, 2018 at 12:56 PM, Michael Trimarchi
 wrote:
> SION bit should be used in the situation that we need
> to read back the value of a pin and should be set by
> default. This can generate any kind of random malfunction
> as described in this thread.
>
> According to this thread:
> https://www.spinics.net/lists/linux-usb/msg162574.html
>
> We consider this an early bug so all the boards running imx25
> with a minimimal set of functionalities can be affected.
>
> As reported by this application note:
> https://www.nxp.com/docs/en/application-note/AN5078.pdf
>
> The software input on (SION) bit is an option to force an input
> path to be active regardless of the value driven by the
> corresponding module. It is used when the nature direction
> of a pin depending on selected alternative function is an output,
> but it is needed to read the real logic value on a pin.
>
> The SION bit can be used in:
> • Loopback: the module of a selected alternative function drives
> the pad and also receives the pad value as an input
> • GPIO capture: the module of a selected alternative function
> drives the pin and the value is captured by the GPIO
>
> Signed-off-by: Michael Trimarchi 
> ---
> Refer-to:
> MX25 USB timeout on ID 0951:1665 Kingston Technology Digital
> DataTraveler SE9 64GB

Glad you found a fix for the issue!


> ---
>  arch/arm/include/asm/arch-mx25/iomux-mx25.h | 680 
> ++--
>  1 file changed, 340 insertions(+), 340 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-mx25/iomux-mx25.h 
> b/arch/arm/include/asm/arch-mx25/iomux-mx25.h
> index 5b2863e..2fcaf60 100644
> --- a/arch/arm/include/asm/arch-mx25/iomux-mx25.h
> +++ b/arch/arm/include/asm/arch-mx25/iomux-mx25.h
> @@ -30,481 +30,481 @@
>
>  /* PADMUXALT 
> INPSE PATH PADCTRL */
>  enum {
> -   MX25_PAD_A10__A10   = IOMUX_PAD(0x000, 0x008, 
> 0x00, 0, 0, NO_PAD_CTRL),
> -   MX25_PAD_A10__GPIO_4_0  = IOMUX_PAD(0x000, 0x008, 
> 0x05, 0, 0, NO_PAD_CTRL),
> +   MX25_PAD_A10__A10   = IOMUX_PAD(0x000, 0x008, 0, 
> 0, 0, NO_PAD_CTRL),
> +   MX25_PAD_A10__GPIO_4_0  = IOMUX_PAD(0x000, 0x008, 5, 
> 0, 0, NO_PAD_CTRL),

In many places in this patch you are only changing things like 0x00
--> 0 or 0x05--> 5, which just makes it harder to review.

Please send a new version that only removes the SION bit.
--
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

Re: [PATCH v2] usb: phy: mxs: Fix NULL pointer dereference on i.MX23/28

[Fabio Estevam]

Hi Felipe,

On Thu, Jan 18, 2018 at 12:22 AM, Fabio Estevam <feste...@gmail.com> wrote:
> From: Fabio Estevam <fabio.este...@nxp.com>
>
> Commit e93650994a95 ("usb: phy: mxs: add usb charger type detection")
> causes the following kernel hang on i.MX28:
>
> [2.207973] usbcore: registered new interface driver usb-storage
> [2.235659] Unable to handle kernel NULL pointer dereference at virtual 
> address 0188
> [2.244195] pgd = (ptrval)
> [2.246994] [0188] *pgd=
> [2.250676] Internal error: Oops: 5 [#1] ARM
> [2.254979] Modules linked in:
> [2.258089] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.15.0-rc8-next-20180117-2-g75d5f21 #7
> [2.266724] Hardware name: Freescale MXS (Device Tree)
> [2.271921] PC is at regmap_read+0x0/0x5c
> [2.275977] LR is at mxs_phy_charger_detect+0x34/0x1dc
>
> mxs_phy_charger_detect() makes accesses to the anatop registers via regmap,
> however i.MX23/28 do not have such registers, which causes a NULL pointer
> dereference.
>
> Fix the issue by doing a NULL check on the 'regmap' pointer.
>
> Fixes: e93650994a95 ("usb: phy: mxs: add usb charger type detection")
> Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>

Could this one be applied to 4.15 final so that we avoid a boot
regression on i.MX23/28?

Thanks
--
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

[PATCH v2] usb: phy: mxs: Fix NULL pointer dereference on i.MX23/28

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Commit e93650994a95 ("usb: phy: mxs: add usb charger type detection")
causes the following kernel hang on i.MX28:

[2.207973] usbcore: registered new interface driver usb-storage
[2.235659] Unable to handle kernel NULL pointer dereference at virtual 
address 0188
[2.244195] pgd = (ptrval)
[2.246994] [0188] *pgd=
[2.250676] Internal error: Oops: 5 [#1] ARM
[2.254979] Modules linked in:
[2.258089] CPU: 0 PID: 1 Comm: swapper Not tainted 
4.15.0-rc8-next-20180117-2-g75d5f21 #7
[2.266724] Hardware name: Freescale MXS (Device Tree)
[2.271921] PC is at regmap_read+0x0/0x5c
[2.275977] LR is at mxs_phy_charger_detect+0x34/0x1dc

mxs_phy_charger_detect() makes accesses to the anatop registers via regmap,
however i.MX23/28 do not have such registers, which causes a NULL pointer
dereference.

Fix the issue by doing a NULL check on the 'regmap' pointer.

Fixes: e93650994a95 ("usb: phy: mxs: add usb charger type detection")
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- Perform a NULL check on regmap (Jun Li)

 drivers/usb/phy/phy-mxs-usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index da031c4..4e2eb9d 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -602,6 +602,9 @@ static enum usb_charger_type mxs_phy_charger_detect(struct 
usb_phy *phy)
void __iomem *base = phy->io_priv;
enum usb_charger_type chgr_type = UNKNOWN_TYPE;
 
+   if (!regmap)
+   return UNKNOWN_TYPE;
+
if (mxs_charger_data_contact_detect(mxs_phy))
return chgr_type;
 
-- 
2.7.4

--
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

[PATCH] usb: phy: mxs: Do not run charger detection on i.MX23/28

[Fabio Estevam]

Commit e93650994a95 ("usb: phy: mxs: add usb charger type detection")
causes the following kernel hang on i.MX28:

[2.207973] usbcore: registered new interface driver usb-storage
[2.235659] Unable to handle kernel NULL pointer dereference at virtual 
address 0188
[2.244195] pgd = (ptrval)
[2.246994] [0188] *pgd=
[2.250676] Internal error: Oops: 5 [#1] ARM
[2.254979] Modules linked in:
[2.258089] CPU: 0 PID: 1 Comm: swapper Not tainted 
4.15.0-rc8-next-20180117-2-g75d5f21 #7
[2.266724] Hardware name: Freescale MXS (Device Tree)
[2.271921] PC is at regmap_read+0x0/0x5c
[2.275977] LR is at mxs_phy_charger_detect+0x34/0x1dc

The USB PHY present on i.MX23/28 does not support charger
detection so do not attempt to run it on these SoCs.

Fixes: e93650994a95 ("usb: phy: mxs: add usb charger type detection")
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index da031c4..54a887c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -129,6 +129,9 @@
  */
 #define MXS_PHY_NEED_IP_FIXBIT(3)
 
+/* MX23 and MX28 USB PHY does not support charger detecttion */
+#define MXS_PHY_NO_CHARGER BIT(4)
+
 /* Minimum and maximum values for device tree entries */
 #define MXS_PHY_TX_CAL45_MIN   30
 #define MXS_PHY_TX_CAL45_MAX   55
@@ -140,7 +143,8 @@ struct mxs_phy_data {
 };
 
 static const struct mxs_phy_data imx23_phy_data = {
-   .flags = MXS_PHY_ABNORMAL_IN_SUSPEND | MXS_PHY_SENDING_SOF_TOO_FAST,
+   .flags = MXS_PHY_ABNORMAL_IN_SUSPEND | MXS_PHY_SENDING_SOF_TOO_FAST |
+MXS_PHY_NO_CHARGER,
 };
 
 static const struct mxs_phy_data imx6q_phy_data = {
@@ -602,6 +606,9 @@ static enum usb_charger_type mxs_phy_charger_detect(struct 
usb_phy *phy)
void __iomem *base = phy->io_priv;
enum usb_charger_type chgr_type = UNKNOWN_TYPE;
 
+   if (mxs_phy->data->flags & MXS_PHY_NO_CHARGER)
+   return UNKNOWN_TYPE;
+
if (mxs_charger_data_contact_detect(mxs_phy))
return chgr_type;
 
-- 
2.7.4

--
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

[PATCH] doc: usb: chipidea: Fix typo in 'enumerate'

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Fix the spelling of 'enumerate' in this document.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 Documentation/usb/chipidea.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index edf7cdf..d1eedc0 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -23,13 +23,13 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
 2) Connect 2 boards with usb cable with one end is micro A plug, the other end
is micro B plug.
 
-   The A-device(with micro A plug inserted) should enumrate B-device.
+   The A-device(with micro A plug inserted) should enumerate B-device.
 
 3) Role switch
On B-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
 
-   B-device should take host role and enumrate A-device.
+   B-device should take host role and enumerate A-device.
 
 4) A-device switch back to host.
On B-device:
@@ -40,13 +40,13 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
side by answering the polling from B-Host, this can be done on A-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/a_bus_req
 
-   A-device should switch back to host and enumrate B-device.
+   A-device should switch back to host and enumerate B-device.
 
 5) Remove B-device(unplug micro B plug) and insert again in 10 seconds,
-   A-device should enumrate B-device again.
+   A-device should enumerate B-device again.
 
 6) Remove B-device(unplug micro B plug) and insert again after 10 seconds,
-   A-device should NOT enumrate B-device.
+   A-device should NOT enumerate B-device.
 
if A-device wants to use bus:
On A-device:
@@ -67,7 +67,7 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
On B-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
 
-   A-device should resume usb bus and enumrate B-device.
+   A-device should resume usb bus and enumerate B-device.
 
 1.3 Reference document
 --
-- 
2.7.4

--
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

Re: MX25 USB timeout on ID 0951:1665 Kingston Technology Digital DataTraveler SE9 64GB

[Fabio Estevam]

Hi Michael,

Adding Peter Chen.

On Wed, Nov 8, 2017 at 8:41 AM, Michael Nazzareno Trimarchi
 wrote:
> Hi Alan
>
> I'm working on IMX25 platform where I have a USB251xBi family
> connected to the ehci full speed serial port.
> IMX25 is capable to manage only full speed port using internal
> transceiver. Now we found a usb key where
> I get :
>
> [2.457831] usb 2-1.2: new full-speed USB device number 3 using ci_hdrc
> [8.593153] usb 2-1-port2: cannot reset (err = -110)
> [9.633234] usb 2-1-port2: cannot reset (err = -110)
> [   10.673155] usb 2-1-port2: cannot reset (err = -110)
> [   11.732003] usb 2-1-port2: cannot reset (err = -110)
> [   12.833154] usb 2-1-port2: cannot reset (err = -110)
> [   12.836859] usb 2-1-port2: Cannot enable. Maybe the USB cable is bad?

Does it help if you pass 'usbcore.autosuspend=-1' in the kernel command line?
--
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

Re: [PATCH] USB: EHCI: don't reread PORTSC after disabling port power

[Fabio Estevam]

Hi Uwe,

On Fri, Oct 20, 2017 at 5:20 PM, Uwe Kleine-König
 wrote:

> It also works. However I wonder if it's right that I'm spammed by
> over-current messages now (independent of which fix I choose) as long as
> there is something connected to the port that draws too much power:
>
> [   53.406833] usb usb1-port1: over-current condition
> [   53.631749] usb usb1-port1: over-current condition
> [   53.856720] usb usb1-port1: over-current condition
> [   54.081732] usb usb1-port1: over-current condition
> [   54.306727] usb usb1-port1: over-current condition
> [   54.531722] usb usb1-port1: over-current condition
> [   54.756722] usb usb1-port1: over-current condition
>
> It seems to be intended or am I missing something?

Does it help if you pass 'disable-over-current' property?
--
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

Re: g_hid loading unloading crash

[Fabio Estevam]

On Mon, Sep 11, 2017 at 11:16 AM, prabhu kalyan  wrote:
> Dear Fabio,
>
> My main objective is to switch between HID, VCP, Mass storage mode.
> But as the g_hid.ko crashes i am not able to switch mode.
>
> I am using nxp kernel Linux cpu49-ub 4.4.0-31-generic
> #50~14.04.1-Ubuntu SMP Wed Jul 13 01:06:37 UTC 2016 i686 i686 i686
> GNU/Linux

This seems to be the kernel version running on your PC.

>
> Can you please check if it happens to you on your architecture and
> also check that the g_hid.c file i have attached is there anything
> wrong.

Just try it with a mainline kernel instead and see if the problem persists.

Thanks
--
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

Re: g_hid loading unloading crash

[Fabio Estevam]

On Mon, Sep 11, 2017 at 10:32 AM, PRABHU  wrote:
> Dear all,
> I was trying to do a stress test on module insertion and removal. 1st time
> modprobe g_hid and rmmod g_hid works. second time when doing insmod g_hid
> crashes the core.
>
> crash trace
> 
>
> root@imx6ulevk:~# modprobe ci_hdrc ci_hdrc.0: Device No Response
> g_hid
> Unable to handle kernel paging request at virtual address 7f09b664
> pgd = 88a78000
> [7f09b664] *pgd=88576811, *pte=, *ppte=
> Internal error: Oops: 807 [#1] PREEMPT SMP ARM
> Modules linked in: g_hid(+) usb_f_hid libcomposite configfs ci_hdrc_imx
> usbmisc_imx ci_hdrc evbug uio_pdrv_genirq uio [last unloaded: g_hid]
> CPU: 0 PID: 679 Comm: modprobe Not tainted 4.1.15-2.1.0+g30278ab #31

Looks like you are using a vendor kernel.

Could you try this on a mainline 4.13.1 kernel instead?
--
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

[PATCH] usb: phy: qcom: Use devm_ioremap_resource()

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

Using devm_ioremap_resource() can make the code simpler, as it
already does the resource NULL check.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/phy/phy-qcom-8x16-usb.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/phy/phy-qcom-8x16-usb.c 
b/drivers/usb/phy/phy-qcom-8x16-usb.c
index b6a83a5..679afea 100644
--- a/drivers/usb/phy/phy-qcom-8x16-usb.c
+++ b/drivers/usb/phy/phy-qcom-8x16-usb.c
@@ -270,12 +270,9 @@ static int phy_8x16_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, qphy);
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res)
-   return -EINVAL;
-
-   qphy->regs = devm_ioremap(>dev, res->start, resource_size(res));
-   if (!qphy->regs)
-   return -ENOMEM;
+   qphy->regs = devm_ioremap_resource(>dev, res);
+   if (IS_ERR(qphy->regs))
+   return PTR_ERR(qphy->regs);
 
phy = >phy;
phy->dev= >dev;
-- 
2.7.4

--
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

[PATCH 2/2] usb: phy-generic: Remove unneeded gpiod NULL check

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

The gpiod API checks for NULL descriptors, so there is no need to
duplicate the check in the driver.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/phy/phy-generic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 89d6e7a..675a716 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -263,8 +263,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct 
usb_phy_generic *nop,
dev_err(dev, "Error requesting RESET or VBUS GPIO\n");
return err;
}
-   if (nop->gpiod_reset)
-   gpiod_direction_output(nop->gpiod_reset, 1);
+   gpiod_direction_output(nop->gpiod_reset, 1);
 
nop->phy.otg = devm_kzalloc(dev, sizeof(*nop->phy.otg),
GFP_KERNEL);
-- 
2.7.4

--
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

[PATCH 1/2] usb: pxa27x_udc: Remove unneeded gpiod NULL check

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

The gpiod API checks for NULL descriptors, so there is no need to
duplicate the check in the driver.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/gadget/udc/pxa27x_udc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c 
b/drivers/usb/gadget/udc/pxa27x_udc.c
index d48e239..404ae6d 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -2475,8 +2475,7 @@ static int pxa_udc_probe(struct platform_device *pdev)
PTR_ERR(udc->gpiod));
return PTR_ERR(udc->gpiod);
}
-   if (udc->gpiod)
-   gpiod_direction_output(udc->gpiod, 0);
+   gpiod_direction_output(udc->gpiod, 0);
 
udc->clk = devm_clk_get(>dev, NULL);
if (IS_ERR(udc->clk))
-- 
2.7.4

--
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

Re: Patch for drivers/usb/serial/ch341.c enabling parity

[Fabio Estevam]

On Sun, Apr 2, 2017 at 3:00 PM, Leif Neland  wrote:
> There is a patch
>
> https://github.com/karlp/ch341-linux/blob/master/0001-usb-serial-ch341-Add-parity-support.patch
>
> which enables parity selection for the ch341 USB-RS485 adapter.
>
> From: Karl Palsson 
> Date: Tue, 18 Mar 2014 23:33:27 +
>
> This adapter is available for cheap on eBay.
>
> The patch made me able to use this adapter for polling my modbus
> power-meter with raspberry pi.
>
> Would it be possible to get this into the official distribution?

Kernel 4.10 has commit ba781bdf866 ("USB: serial: ch341: add support
for parity, frame length, stop bits") that adds parity support for
this driver.

Have you tried using 4.10.x?
--
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

Re: Choppy USB audio on i.MX53 chipidea with 4.4 kernel. Ok in 4.10

[Fabio Estevam]

Hi Martin,

On Tue, Feb 21, 2017 at 1:06 PM, Martin Fuzzey  wrote:
> Hi,
>
> I ran into a problem of "choppy" audio when using a USB codec on a i.MX53
> (with the  chipidea driver)
> The problem occurs on 4.4.47 but not on an old 3.19
> It also does not occur on the latest 4.10.
>
> The codec is :
>
> Bus 001 Device 003: ID 08bb:29c0 Texas Instruments Japan PCM2900C Audio
> CODEC
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   2.00
>   bDeviceClass0 (Defined at Interface level)
>   bDeviceSubClass 0
>   bDeviceProtocol 0
>   bMaxPacketSize0 8
>   idVendor   0x08bb Texas Instruments Japan
>   idProduct  0x29c0 PCM2900C Audio CODEC
>   bcdDevice1.00
>   iManufacturer   1 BurrBrown from Texas Instruments
>   iProduct2 USB AUDIO  CODEC
>   iSerial 0
>
>
> Bisecting between 3.19 and 4.4 narrows it down to this commit:
>
> commit e765bfb73ff711b1d139a4438af9ecedabb07d34
> Author: Peter Chen 
> Date:   Thu Jun 18 11:43:34 2015 +0800
>
> usb: chipidea: imx: enable CI_HDRC_SET_NON_ZERO_TTHA
>
> For i.mx platform, set ttctrl.ttha with non-zero value only affects
> sitd, and ehci core makes sure the schedule is not full when accepts
> new request, so it will not occur the transaction which will acorss
> the SoF.
>
>
> And indeed reverting that fixes the problem on 4.4
>
> I see that this has already been reverted in 4.5 by:
>
> commit 1bc7da87c7410c6990c3251589e3854e64c55af2
> Author: Peter Chen 
> Date:   Fri Jan 29 16:47:24 2016 +0800
>
> Revert "usb: chipidea: imx: enable CI_HDRC_SET_NON_ZERO_TTHA"
>
> This reverts commit e765bfb73ff7.
>
> In the most of cases, we only use one transaction per frame and the
> frame rate may be high, If the platforms want to support multiple
> transactions but less frame rate cases like [1] and [2], it can set
> "non-zero-ttctrl-ttha" at dts.
>
> [1] http://www.spinics.net/lists/linux-usb/msg123125.html
> [2] http://www.spinics.net/lists/linux-usb/msg118679.html
>
> Signed-off-by: Peter Chen 
>
>
> Which explains why 4.10 is ok.
>
> But, maybe this should be backported to 4.4 (old)stable?

Yes, please submit the revert patch to stable.
--
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

Re: [PATCH] ARM: imx25: set default phy_type and dr_mode for usbotg port

[Fabio Estevam]

On Thu, Feb 16, 2017 at 7:18 PM, Uwe Kleine-König
<u.kleine-koe...@pengutronix.de> wrote:
> All currently supported i.MX25-based machines use phy_type = "utmi" and
> dr_mode = "otg".  So this seems to be a sensible default.
>
> This also doesn't hurt out-of-tree machines because up to now they had
> to specify these two properties in the machine.dts which still takes
> precedence by just overwriting the defaults added here.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koe...@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>
--
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

Re: [PATCH 1/3] usb: chipidea: udc: Use dma_pool_zalloc()

[Fabio Estevam]

Hi Sergei,

On Fri, Sep 9, 2016 at 9:47 AM, Sergei Shtylyov
 wrote:

>> We can make the code simpler by using dma_pool_zalloc() instead
>> of calling dma_pool_zalloc() and then a memset().
>
>
>dma_pool_alloc().

Thanks, fixed in v2.
--
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

[PATCH v2 2/3] usb: chipidea: udc: Fit into a single line

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

No need to split the dma_pool_zalloc() line in two lines as it can
perfectly fit into a single one.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- Add a missing 'line' in the commit log.

 drivers/usb/chipidea/udc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index f24d922..336793f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -350,8 +350,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct 
ci_hw_req *hwreq,
if (node == NULL)
return -ENOMEM;
 
-   node->ptr = dma_pool_zalloc(hwep->td_pool, GFP_ATOMIC,
-  >dma);
+   node->ptr = dma_pool_zalloc(hwep->td_pool, GFP_ATOMIC, >dma);
if (node->ptr == NULL) {
kfree(node);
return -ENOMEM;
-- 
1.9.1

--
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

[PATCH v2 1/3] usb: chipidea: udc: Use dma_pool_zalloc()

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

We can make the code simpler by using dma_pool_zalloc() instead
of calling dma_pool_alloc() followed by memset().

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- Fix a typo in commit log (Sergei)

 drivers/usb/chipidea/udc.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 0f692fc..f24d922 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1668,12 +1668,10 @@ static int init_eps(struct ci_hdrc *ci)
usb_ep_set_maxpacket_limit(>ep, (unsigned 
short)~0);
 
INIT_LIST_HEAD(>qh.queue);
-   hwep->qh.ptr = dma_pool_alloc(ci->qh_pool, GFP_KERNEL,
->qh.dma);
+   hwep->qh.ptr = dma_pool_zalloc(ci->qh_pool, GFP_KERNEL,
+  >qh.dma);
if (hwep->qh.ptr == NULL)
retval = -ENOMEM;
-   else
-   memset(hwep->qh.ptr, 0, sizeof(*hwep->qh.ptr));
 
/*
 * set up shorthands for ep0 out and in endpoints,
-- 
1.9.1

--
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

[PATCH v2 3/3] usb: chipidea: udc: Use the preferred form for passing a size of a struct

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

According to Documentation/CodingStyle:

"The preferred form for passing a size of a struct is the following:

p = kmalloc(sizeof(*p), ...);
"
, so do as suggested to improve readability.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
Changes since v1:
- None

 drivers/usb/chipidea/udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 336793f..16c251a 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1984,7 +1984,7 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
if (!hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC))
return -ENXIO;
 
-   rdrv = devm_kzalloc(ci->dev, sizeof(struct ci_role_driver), GFP_KERNEL);
+   rdrv = devm_kzalloc(ci->dev, sizeof(*rdrv), GFP_KERNEL);
if (!rdrv)
return -ENOMEM;
 
-- 
1.9.1

--
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

[PATCH 3/3] usb: chipidea: udc: Use the preferred form for passing a size of a struct

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

According to Documentation/CodingStyle:

"The preferred form for passing a size of a struct is the following:

p = kmalloc(sizeof(*p), ...);
"
, so do as suggested to improve readability.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/chipidea/udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 336793f..16c251a 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1984,7 +1984,7 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
if (!hw_read(ci, CAP_DCCPARAMS, DCCPARAMS_DC))
return -ENXIO;
 
-   rdrv = devm_kzalloc(ci->dev, sizeof(struct ci_role_driver), GFP_KERNEL);
+   rdrv = devm_kzalloc(ci->dev, sizeof(*rdrv), GFP_KERNEL);
if (!rdrv)
return -ENOMEM;
 
-- 
1.9.1

--
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

[PATCH 2/3] usb: chipidea: udc: Fit into a single line

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

No need to split the dma_pool_zalloc() line into two as it can
perfectly fit into a single line.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/chipidea/udc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index f24d922..336793f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -350,8 +350,7 @@ static int add_td_to_list(struct ci_hw_ep *hwep, struct 
ci_hw_req *hwreq,
if (node == NULL)
return -ENOMEM;
 
-   node->ptr = dma_pool_zalloc(hwep->td_pool, GFP_ATOMIC,
-  >dma);
+   node->ptr = dma_pool_zalloc(hwep->td_pool, GFP_ATOMIC, >dma);
if (node->ptr == NULL) {
kfree(node);
return -ENOMEM;
-- 
1.9.1

--
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

[PATCH 1/3] usb: chipidea: udc: Use dma_pool_zalloc()

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

We can make the code simpler by using dma_pool_zalloc() instead
of calling dma_pool_zalloc() and then a memset().

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/chipidea/udc.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 0f692fc..f24d922 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1668,12 +1668,10 @@ static int init_eps(struct ci_hdrc *ci)
usb_ep_set_maxpacket_limit(>ep, (unsigned 
short)~0);
 
INIT_LIST_HEAD(>qh.queue);
-   hwep->qh.ptr = dma_pool_alloc(ci->qh_pool, GFP_KERNEL,
->qh.dma);
+   hwep->qh.ptr = dma_pool_zalloc(ci->qh_pool, GFP_KERNEL,
+  >qh.dma);
if (hwep->qh.ptr == NULL)
retval = -ENOMEM;
-   else
-   memset(hwep->qh.ptr, 0, sizeof(*hwep->qh.ptr));
 
/*
 * set up shorthands for ep0 out and in endpoints,
-- 
1.9.1

--
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

Re: [PATCH] HID: usbkbd: return proper error code

[Fabio Estevam]

On Wed, Aug 31, 2016 at 1:28 PM, Sudip Mukherjee
 wrote:
> Use proper error code instead of using -1 on failure to allocate
> memory. We may use the error code later in the caller.
>
> Signed-off-by: Sudip Mukherjee 
> ---
>  drivers/hid/usbhid/usbkbd.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
> index 9a332e6..ee53359 100644
> --- a/drivers/hid/usbhid/usbkbd.c
> +++ b/drivers/hid/usbhid/usbkbd.c
> @@ -249,15 +249,15 @@ static void usb_kbd_close(struct input_dev *dev)
>  static int usb_kbd_alloc_mem(struct usb_device *dev, struct usb_kbd *kbd)
>  {
> if (!(kbd->irq = usb_alloc_urb(0, GFP_KERNEL)))
> -   return -1;
> +   return -ENOMEM;

While you are it, the code would look better like this:

kbd->irq = usb_alloc_urb(0, GFP_KERNEL)
if (!kbd->irq)
return -ENOMEM;
--
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

[PATCH] usb: phy: phy-generic: Check clk_prepare_enable() error

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

clk_prepare_enable() may fail, so we should better check its return
value and propagate it in the case of failure.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/phy/phy-generic.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 980c9de..427efb5 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -144,14 +144,18 @@ static irqreturn_t nop_gpio_vbus_thread(int irq, void 
*data)
 int usb_gen_phy_init(struct usb_phy *phy)
 {
struct usb_phy_generic *nop = dev_get_drvdata(phy->dev);
+   int ret;
 
if (!IS_ERR(nop->vcc)) {
if (regulator_enable(nop->vcc))
dev_err(phy->dev, "Failed to enable power\n");
}
 
-   if (!IS_ERR(nop->clk))
-   clk_prepare_enable(nop->clk);
+   if (!IS_ERR(nop->clk)) {
+   ret = clk_prepare_enable(nop->clk);
+   if (ret)
+   return ret;
+   }
 
nop_reset(nop);
 
-- 
1.9.1

--
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

Re: [PATCH v3 0/6] power: add power sequence library

[Fabio Estevam]

On Mon, Aug 1, 2016 at 11:55 AM, Joshua Clayton
 wrote:

> Thanks a million, Fabio!
>
> 'usbcore.autosuspend=-1' quiets the errors.
>
> ~joshua
>
> P.S. I guess this technically is a bug in chipidea usb.
> I'll give it a quick once over, though I am not very familiar
> with USB core or the CI driver.

Yes, it would be really nice if we can get a proper fix for this bug.
--
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

Re: [PATCH v3 0/6] power: add power sequence library

[Fabio Estevam]

Hi Joshua,

On Thu, Jul 28, 2016 at 12:56 PM, Joshua Clayton
<stillcompil...@gmail.com> wrote:

> I assume there is a v4 coming due to rmk's comments on patch 5.
> I couldn't figure out where to null the of_node in error paths, but in testing
> we did put a line of code to null the of_node on release.
>
> but...
> As an aside,
> I was hoping this series would magically fix a problem
> with the imx6q-evi which has forced us to disable
> runtime power management. But it did not. :(

I also see a similar problem on a mx28 board with a hub.

Does it help in your case if you pass 'usbcore.autosuspend=-1' in the
kernel command line?

Regards,

Fabio Estevam
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Wed, Feb 24, 2016 at 10:20 PM, Peter Chen  wrote:

> Would you get any chances to test i.mx28 evk with an external HUB?
> I just would like to know it is not a HUB issue which can't be
> suspended.

Just connected an external USB hub on a mx28evk and did not see any issue.
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Wed, Feb 24, 2016 at 10:29 AM, Felipe Balbi  wrote:

>> [2.814449] usb 1-1: new high-speed USB device number 2 using ci_hdrc
>> [2.857129] fec 800f.ethernet eth0: Freescale FEC PHY driver
>> [SMSC LAN8710/LAN8720] (mii_bus:phy_addr=800f.etherne:00, irq)
>> [2.993879] hub 1-1:1.0: USB hub found
>> [2.998414] hub 1-1:1.0: 2 ports detected
>> [3.525108] usb 1-1: USB disconnect, device number 2
>
> Why is the device disconnecting ? This is quite odd. Returning -EBUSY on

Not sure either.

> your runtime suspend should be the same thing as disabling runtime pm.
>
> Wonder what it is that we're missing.

Peter, any ideas?
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Wed, Feb 24, 2016 at 8:48 AM, Fabio Estevam <feste...@gmail.com> wrote:

> and this is the result:

I missed to post the first print. Here is the complete log:

[2.375588] usbcore: registered new interface driver usb-storage
[2.405944] ** Calling ci_hdrc_imx_runtime_suspend
[2.412608] 8009.usb supply vbus not found, using dummy regulator
[2.424703] ci_hdrc ci_hdrc.1: EHCI Host Controller
[2.430689] ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 1
[2.454341] ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
[2.475947] hub 1-0:1.0: USB hub found
[2.480321] hub 1-0:1.0: 1 port detected
[2.499691] mousedev: PS/2 mouse device common for all mice

.

[2.814449] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[2.857129] fec 800f.ethernet eth0: Freescale FEC PHY driver
[SMSC LAN8710/LAN8720] (mii_bus:phy_addr=800f.etherne:00, irq)
[2.993879] hub 1-1:1.0: USB hub found
[2.998414] hub 1-1:1.0: 2 ports detected
[3.525108] usb 1-1: USB disconnect, device number 2
[3.571144] usb usb1-port1: cannot reset (err = -32)
[3.576562] usb usb1-port1: cannot reset (err = -32)
[3.581823] usb usb1-port1: cannot reset (err = -32)
[3.587321] usb usb1-port1: cannot reset (err = -32)
[3.592577] usb usb1-port1: cannot reset (err = -32)
[3.597701] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.605924] usb usb1-port1: cannot reset (err = -32)
[3.611184] usb usb1-port1: cannot reset (err = -32)
[3.616577] usb usb1-port1: cannot reset (err = -32)
[3.621830] usb usb1-port1: cannot reset (err = -32)
[3.627321] usb usb1-port1: cannot reset (err = -32)
[3.632332] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.639697] usb usb1-port1: cannot reset (err = -32)
[3.645176] usb usb1-port1: cannot reset (err = -32)
[3.650430] usb usb1-port1: cannot reset (err = -32)
[3.655813] usb usb1-port1: cannot reset (err = -32)
[3.661064] usb usb1-port1: cannot reset (err = -32)
[3.666213] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.673547] usb usb1-port1: cannot reset (err = -32)
[3.678929] usb usb1-port1: cannot reset (err = -32)
[3.684440] usb usb1-port1: cannot reset (err = -32)
[3.689699] usb usb1-port1: cannot reset (err = -32)
[3.695077] usb usb1-port1: cannot reset (err = -32)
[3.700087] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.707183] usb usb1-port1: unable to enumerate USB device
[5.195598] ** Calling ci_hdrc_imx_runtime_suspend
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Wed, Feb 24, 2016 at 8:32 AM, Felipe Balbi  wrote:

> Then that's the problem. You should _always_ implement your runtime_pm
> callbacks otherwise driver model will assume you don't need to do
> ANYTHING for runtime pm and runtime suspend you unconditionally.
>
> As a test, try setting the flag but just returning -EBUSY from the top
> of runtime_suspend in chipidea.

Here are the changes as suggested:

--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -33,7 +33,8 @@ static const struct ci_hdrc_imx_platform_flag imx27_usb_data =

 static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
.flags = CI_HDRC_IMX28_WRITE_FIX |
-   CI_HDRC_TURN_VBUS_EARLY_ON,
+   CI_HDRC_TURN_VBUS_EARLY_ON |
+   CI_HDRC_SUPPORTS_RUNTIME_PM,
 };

 static const struct ci_hdrc_imx_platform_flag imx6q_usb_data = {
@@ -317,6 +318,10 @@ static int ci_hdrc_imx_runtime_suspend(struct device *dev)
struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
int ret;

+   pr_err("** Calling ci_hdrc_imx_runtime_suspend\n");
+   /* Quick test */
+   return -EBUSY;
+
if (data->in_lpm) {
WARN_ON(1);
return 0;

and this is the result:

[3.104883] hub 2-1:1.0: USB hub found
[3.109226] hub 2-1:1.0: 2 ports detected
[3.635138] usb 2-1: USB disconnect, device number 2
[3.656231] usb usb2-port1: cannot reset (err = -32)
[3.661500] usb usb2-port1: cannot reset (err = -32)
[3.667074] usb usb2-port1: cannot reset (err = -32)
[3.672329] usb usb2-port1: cannot reset (err = -32)
[3.677712] usb usb2-port1: cannot reset (err = -32)
[3.682722] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[3.690996] usb usb2-port1: cannot reset (err = -32)
[3.696495] usb usb2-port1: cannot reset (err = -32)
[3.701745] usb usb2-port1: cannot reset (err = -32)
[3.707242] usb usb2-port1: cannot reset (err = -32)
[3.712495] usb usb2-port1: cannot reset (err = -32)
[3.717618] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[3.725122] usb usb2-port1: cannot reset (err = -32)
[3.730380] usb usb2-port1: cannot reset (err = -32)
[3.735762] usb usb2-port1: cannot reset (err = -32)
[3.741015] usb usb2-port1: cannot reset (err = -32)
[3.746501] usb usb2-port1: cannot reset (err = -32)
[3.751510] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[3.758981] usb usb2-port1: cannot reset (err = -32)
[3.764468] usb usb2-port1: cannot reset (err = -32)
[3.769730] usb usb2-port1: cannot reset (err = -32)
[3.775100] usb usb2-port1: cannot reset (err = -32)
[3.780351] usb usb2-port1: cannot reset (err = -32)
[3.785509] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
[3.792544] usb usb2-port1: unable to enumerate USB device
[5.195776] ** Calling ci_hdrc_imx_runtime_suspend
[6.057095] fec 800f.ethernet eth0: Link is Up - 100Mbps/Full - flow conx
[6.084367] Sending DHCP requests ., OK

.

[7.156524] uart-pl011 80074000.serial: no DMA platform data
[7.228624] VFS: Mounted root (nfs filesystem) readonly on device 0:14.
[7.240462] devtmpfs: mounted
[7.245817] Freeing unused kernel memory: 248K (c070a000 - c0748000)
[7.586576] ci_hdrc ci_hdrc.0: timeout waiting for 0800 in 11
[7.593451] ** Calling ci_hdrc_imx_runtime_suspend
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

Hi Felipe,

On Wed, Feb 24, 2016 at 7:47 AM, Felipe Balbi  wrote:

> Then what DOES get called ? If we don't reach runtime_pm of the PHY, we
> must reach runtime_pm of chipidea. In that case, detect _there_ if
> you're running on one of the known broken ones and return -EBUSY or
> something like that.

We don't reach runtime_pm in chipidea either.

The reason is that mx23/mx28 do not turn on the
CI_HDRC_SUPPORTS_RUNTIME_PM flag in
drivers/usb/chipidea/ci_hdrc_imx.c.

The errors are:

hub 1-1:1.0: USB hub found
hub 1-1:1.0: 2 ports detected
usb 1-1: USB disconnect, device number 2
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: Cannot enable. Maybe the USB cable is bad?

which come from drivers/usb/core/hub.c.

Passing "usbcore.autosuspend=-1" make things to work fine.
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Tue, Feb 23, 2016 at 11:44 PM, Peter Chen  wrote:

> Just plug in an external USB HUB at i.mx28 evk host port without pass
> ''usbcore.autosuspend=-1' at bootargs to see if it works.

Well, the board I have is based on mx28 and has a USB hub onboard. It
does not work without ''usbcore.autosuspend=-1".
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Tue, Feb 23, 2016 at 10:20 PM, Peter Chen  wrote:

> Oh, you said it is called before.
>
> http://www.spinics.net/lists/linux-usb/msg136714.html

Yes, I have enabled runtime pm previously. Sorry for the confusion.

> If you make sure the neither .runtime_suspend at chipidea driver
> nor mxs_phy_suspend at mxs driver are called, it means the USB HUB
> can never be suspended, then the kernel command line is the only way.

Yes, passing 'usbcore.autosuspend=-1' is the only reliable method I see so far.

> Care to test external USB HUB with i.mx28 evk? I would like to know if it
> is the HUB's issue. Afaik, i.mx28 has no such bug when the bus is
> suspended but the PHY is still active.

How do I test this?
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Tue, Feb 23, 2016 at 4:47 PM, Fabio Estevam <feste...@gmail.com> wrote:
> Hi Peter,
>
> On Sun, Feb 21, 2016 at 10:59 PM, Peter Chen <hzpeterc...@gmail.com> wrote:
>
>> Fabio, Felipe is correct. The mx23 and mx28 should NOT call mxs phy's
>> suspend API
>> since the runtime suspend is not enabled for mx23 and mx28 at chipidea 
>> driver.
>> Would you please check if CI_HDRC_SUPPORTS_RUNTIME_PM is set wrongly
>> for mx23/mx28 at ci_hdrc_imx.c? If it does not been set, would please add
>> WARN_ON(1) at mxs_phy_suspend to show call stack?
>
> CI_HDRC_SUPPORTS_RUNTIME_PM is not set for mx23/mx28. I may have
> enabled in earlier tests, but now I confirm it is not being set.
>
> I am running 4.1.13 with only this patch applied:
>
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -368,6 +368,9 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
> low_speed_connection = mxs_phy_is_low_speed_connection(mxs_phy);
> vbus_is_on = mxs_phy_get_vbus_status(mxs_phy);
>
> +   pr_err("** entering mxs_phy_suspend\n");
> +   WARN_ON(1);
> +
> if (suspend) {
> /*
>  * FIXME: Do not power down RXPWD1PT1 bit for low speed
>
> .but I never see mxs_phy_suspend() getting called.

Actually the runtime suspend funcion is mxs_phy_system_suspend().

Also added a  WARN_ON(1) inside mxs_phy_system_suspend() and it is
also never called.

This means runtime pm functions are not being called for mx28 as expected.

Is there a way for getting USB to work on this board with a USB hub
without passing "usbcore.autosuspend=-1" in the kernel command line?
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

Hi Peter,

On Sun, Feb 21, 2016 at 10:59 PM, Peter Chen  wrote:

> Fabio, Felipe is correct. The mx23 and mx28 should NOT call mxs phy's
> suspend API
> since the runtime suspend is not enabled for mx23 and mx28 at chipidea driver.
> Would you please check if CI_HDRC_SUPPORTS_RUNTIME_PM is set wrongly
> for mx23/mx28 at ci_hdrc_imx.c? If it does not been set, would please add
> WARN_ON(1) at mxs_phy_suspend to show call stack?

CI_HDRC_SUPPORTS_RUNTIME_PM is not set for mx23/mx28. I may have
enabled in earlier tests, but now I confirm it is not being set.

I am running 4.1.13 with only this patch applied:

--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -368,6 +368,9 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
low_speed_connection = mxs_phy_is_low_speed_connection(mxs_phy);
vbus_is_on = mxs_phy_get_vbus_status(mxs_phy);

+   pr_err("** entering mxs_phy_suspend\n");
+   WARN_ON(1);
+
if (suspend) {
/*
 * FIXME: Do not power down RXPWD1PT1 bit for low speed

.but I never see mxs_phy_suspend() getting called.
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Fri, Feb 19, 2016 at 12:13 PM, Felipe Balbi  wrote:

> h, okay. So you have another problem, actually. Seems like ci_hdrc
> just shouldn't call your phy->suspend(), or your phy->suspend()
> shouldn't do anything. How about below?
>
> @@ -370,6 +370,9 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
> struct mxs_phy *mxs_phy = to_mxs_phy(x);
> bool low_speed_connection, vbus_is_on;
>
> +   if (mxs_phy->data->flags & MXS_PHY_ABNORMAL_IN_SUSPEND)
> +   return 0; /* or should we return -EBUSY ? */
> +

Tested with 0 and also -EBUSY. This code is called now, but it does
not fix the problem. Same log:

[3.005881] hub 1-1:1.0: USB hub found
[3.010341] hub 1-1:1.0: 2 ports detected
[3.536362] usb 1-1: USB disconnect, device number 2
[3.582732] usb usb1-port1: cannot reset (err = -32)
[3.588289] usb usb1-port1: cannot reset (err = -32)
[3.593546] usb usb1-port1: cannot reset (err = -32)
[3.598929] usb usb1-port1: cannot reset (err = -32)
[3.604188] usb usb1-port1: cannot reset (err = -32)
[3.609339] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

On Fri, Feb 19, 2016 at 11:33 AM, Felipe Balbi  wrote:

> alright, in which sense doesn't it help ? Which platform did you use ?

USB devices are not recognized with your patch applied on a mx28 board
with an on-board USB hub and if I remove 'usbcore.autosuspend=-1' from
the kernel command line.

> mx23 or mx28 ? Did you check that mxs_phy_runtime_idle() got called ?

mxs_phy_runtime_idle() is not being called on my mx28 board.

> Did you have any splats on dmesg ?

[2.788614] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[2.837487] fec 800f.ethernet eth0: Freescale FEC PHY driver
[SMSC LAN8710/LAN8720] (mii_bus:phy_addr=800f.etherne:00, irq=
-1)
[2.968109] hub 1-1:1.0: USB hub found
[2.972876] hub 1-1:1.0: 2 ports detected
[3.499166] usb 1-1: USB disconnect, device number 2
[3.544724] usb usb1-port1: cannot reset (err = -32)
[3.550257] usb usb1-port1: cannot reset (err = -32)
[3.16] usb usb1-port1: cannot reset (err = -32)
[3.560903] usb usb1-port1: cannot reset (err = -32)
[3.566159] usb usb1-port1: cannot reset (err = -32)
[3.571306] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.579584] usb usb1-port1: cannot reset (err = -32)
[3.584850] usb usb1-port1: cannot reset (err = -32)
[3.590368] usb usb1-port1: cannot reset (err = -32)
[3.595628] usb usb1-port1: cannot reset (err = -32)
[3.601014] usb usb1-port1: cannot reset (err = -32)
[3.606027] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.613481] usb usb1-port1: cannot reset (err = -32)
[3.618868] usb usb1-port1: cannot reset (err = -32)
[3.624125] usb usb1-port1: cannot reset (err = -32)
[3.629615] usb usb1-port1: cannot reset (err = -32)
[3.634867] usb usb1-port1: cannot reset (err = -32)
[3.639988] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.647249] usb usb1-port1: cannot reset (err = -32)
[3.652744] usb usb1-port1: cannot reset (err = -32)
[3.657999] usb usb1-port1: cannot reset (err = -32)
[3.663369] usb usb1-port1: cannot reset (err = -32)
[3.668856] usb usb1-port1: cannot reset (err = -32)
[3.673866] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.680862] usb usb1-port1: unable to enumerate USB device
--
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

Re: [PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

Hi Felipe,

On Fri, Feb 19, 2016 at 10:05 AM, Felipe Balbi  wrote:

> how about detecting that you're running on mx23/mx28 and returning
> -EBUSY on your runtime_idle implementation ? You already have the basics
> done for it. Care to test below and tell me whether it helps ?

Thanks for the suggestion. I tested it and unfortunately it does not help.
--
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

[PATCH RESEND] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

On a mx28 board with a USB hub the following error is observed:

hub 1-1:1.0: USB hub found
hub 1-1:1.0: 2 ports detected
usb 1-1: USB disconnect, device number 2
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: Cannot enable. Maybe the USB cable is bad?

,which is caused by a problem described by the MXS_PHY_ABNORMAL_IN_SUSPEND
flag.

As we currently do not have a proper implementation for this issue, one
possible workaround is to pass "usbcore.autosuspend=-1" in the kernel
command line, so add this suggestion into the MXS_PHY_ABNORMAL_IN_SUSPEND
flag description.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
Acked-by: Peter Chen <peter.c...@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 00bfea0..be46ab3 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -98,6 +98,10 @@
  * The PHY will be in messy if there is a wakeup after putting
  * bus to suspend (set portsc.suspendM) but before setting PHY to low
  * power mode (set portsc.phcd).
+ *
+ * To work around this problem on mx23/mx28 user should pass
+ * "usbcore.autosuspend=-1" in the kernel command line for now, as
+ * we do not have a proper fix for this flag in the kernel yet.
  */
 #define MXS_PHY_ABNORMAL_IN_SUSPENDBIT(1)
 
-- 
1.9.1

--
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

Wakeup from USB on mx6

[Fabio Estevam]

Hi Peter,

I am running imx6sl-evk with linux-next 20160203 and I am trying to
make it wake up via a USB mouse click.

These are the steps I am doing:

- Insert the USB mouse in the USB host port:

usb 1-1: new low-speed USB device number 7 using ci_hdrc
input: Logitech USB Optical Mouse as
/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1:1.0/0003:046
D:C077.0006/input/input7
hid-generic 0003:046D:C077.0006: input: USB HID v1.11 Mouse [Logitech
USB Optical Mouse] on usb-ci_hdrc.1-1/input0

- Configure USB1 to be a wakeup source:

$ echo enabled > /sys/bus/usb/devices/usb1/power/wakeup
$ echo enabled > /sys/bus/usb/devices/1-1/power/wakeup

- Put the system in suspend

$ echo mem > /sys/power/state

- Clicking in the mouse button does not wakeup the system.

Any ideas?

Thanks,

Fabio Estevam
--
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

Re: Wakeup from USB on mx6

[Fabio Estevam]

On Wed, Feb 3, 2016 at 1:21 PM, Fabio Estevam <feste...@gmail.com> wrote:
> Hi Peter,
>
> I am running imx6sl-evk with linux-next 20160203 and I am trying to
> make it wake up via a USB mouse click.
>
> These are the steps I am doing:
>
> - Insert the USB mouse in the USB host port:
>
> usb 1-1: new low-speed USB device number 7 using ci_hdrc
> input: Logitech USB Optical Mouse as
> /devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1:1.0/0003:046
> D:C077.0006/input/input7
> hid-generic 0003:046D:C077.0006: input: USB HID v1.11 Mouse [Logitech
> USB Optical Mouse] on usb-ci_hdrc.1-1/input0
>
> - Configure USB1 to be a wakeup source:
>
> $ echo enabled > /sys/bus/usb/devices/usb1/power/wakeup
> $ echo enabled > /sys/bus/usb/devices/1-1/power/wakeup
>
> - Put the system in suspend
>
> $ echo mem > /sys/power/state
>
> - Clicking in the mouse button does not wakeup the system.
>
> Any ideas?

I have also tried this:

--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -279,7 +279,7 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)

spin_unlock_irqrestore(>lock, flags);

-   usbmisc_imx6q_set_wakeup(data, false);
+   usbmisc_imx6q_set_wakeup(data, true);

return 0;

,but it does not help.
--
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

Re: Wakeup from USB on mx6

[Fabio Estevam]

On Wed, Feb 3, 2016 at 1:21 PM, Fabio Estevam <feste...@gmail.com> wrote:
> Hi Peter,
>
> I am running imx6sl-evk with linux-next 20160203 and I am trying to
> make it wake up via a USB mouse click.
>
> These are the steps I am doing:
>
> - Insert the USB mouse in the USB host port:
>
> usb 1-1: new low-speed USB device number 7 using ci_hdrc
> input: Logitech USB Optical Mouse as
> /devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/1-1:1.0/0003:046
> D:C077.0006/input/input7
> hid-generic 0003:046D:C077.0006: input: USB HID v1.11 Mouse [Logitech
> USB Optical Mouse] on usb-ci_hdrc.1-1/input0
>
> - Configure USB1 to be a wakeup source:
>
> $ echo enabled > /sys/bus/usb/devices/usb1/power/wakeup
> $ echo enabled > /sys/bus/usb/devices/1-1/power/wakeup
>
> - Put the system in suspend
>
> $ echo mem > /sys/power/state
>
> - Clicking in the mouse button does not wakeup the system.
>
> Any ideas?

Got it running after going through Documentation/usb/chipidea.txt,
where it has a useful script:

for i in $(find /sys -name wakeup | grep usb);do echo enabled > $i;done

Now clicking the mouse can wakeup the system :-)
--
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

Re: Wakeup from USB on mx6

[Fabio Estevam]

Hi Alan,

On Wed, Feb 3, 2016 at 2:29 PM, Alan Stern <st...@rowland.harvard.edu> wrote:

> It seems likely that this script does exactly the same thing that you
> did before by hand.  Why does one work but not the other?

The script enables all of these entries:

$ find /sys -name wakeup | grep usb
/sys/devices/soc0/soc/200.aips-bus/20c9000.usbphy/power/wakeup
/sys/devices/soc0/soc/200.aips-bus/20ca000.usbphy/power/wakeup
/sys/devices/soc0/soc/210.aips-bus/2184000.usb/power/wakeup
/sys/devices/soc0/soc/210.aips-bus/2184000.usb/ci_hdrc.0/power/wakeup
/sys/devices/soc0/soc/210.aips-bus/2184200.usb/power/wakeup
/sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/power/wakeup
/sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/power/wakeup
/sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/power/wakeup

Regards,

Fabio Estevam
--
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

Re: Wakeup from USB on mx6

[Fabio Estevam]

On Wed, Feb 3, 2016 at 2:58 PM, Alan Stern <st...@rowland.harvard.edu> wrote:
> On Wed, 3 Feb 2016, Fabio Estevam wrote:
>
>> Hi Alan,
>>
>> On Wed, Feb 3, 2016 at 2:29 PM, Alan Stern <st...@rowland.harvard.edu> wrote:
>>
>> > It seems likely that this script does exactly the same thing that you
>> > did before by hand.  Why does one work but not the other?
>>
>> The script enables all of these entries:
>>
>> $ find /sys -name wakeup | grep usb
>> /sys/devices/soc0/soc/200.aips-bus/20c9000.usbphy/power/wakeup
>> /sys/devices/soc0/soc/200.aips-bus/20ca000.usbphy/power/wakeup
>> /sys/devices/soc0/soc/210.aips-bus/2184000.usb/power/wakeup
>> /sys/devices/soc0/soc/210.aips-bus/2184000.usb/ci_hdrc.0/power/wakeup
>> /sys/devices/soc0/soc/210.aips-bus/2184200.usb/power/wakeup
>
> Okay.  As far as I know, those files should have been set to "enabled"
> by default.
>
>> /sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/1-1/power/wakeup
>> /sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/usb1/power/wakeup
>> /sys/devices/soc0/soc/210.aips-bus/2184200.usb/ci_hdrc.1/power/wakeup
>
> And so should that one.  So maybe the problem is that the default
> settings are wrong.

Yes, you are right. I checked all of these files and they come up with
"disabled".

I will try to change the code so that they come as "enabled" by default.

Thanks
--
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

[PATCH] usb: phy: mxs: Suggest passing "usbcore.autosuspend=-1" for mx23/mx28

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

On a mx28 board with a USB hub the following error is observed:

hub 1-1:1.0: USB hub found
hub 1-1:1.0: 2 ports detected
usb 1-1: USB disconnect, device number 2
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: Cannot enable. Maybe the USB cable is bad?

,which is caused by a problem described by the MXS_PHY_ABNORMAL_IN_SUSPEND
flag.

As we currently do not have a proper implementation for this issue, one
possible workaround is to pass "usbcore.autosuspend=-1" in the kernel
command line, so add this suggestion into the MXS_PHY_ABNORMAL_IN_SUSPEND
flag description.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index c2936dc..453a5ab 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -98,6 +98,10 @@
  * The PHY will be in messy if there is a wakeup after putting
  * bus to suspend (set portsc.suspendM) but before setting PHY to low
  * power mode (set portsc.phcd).
+ *
+ * To work around this problem on mx23/mx28 user should pass
+ * "usbcore.autosuspend=-1" in the kernel command line for now, as
+ * we do not have a proper fix for this flag in the kernel yet.
  */
 #define MXS_PHY_ABNORMAL_IN_SUSPENDBIT(1)
 
-- 
1.9.1

--
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

Re: MX28 with hub cannot reset

[Fabio Estevam]

Hi Stefan,

On Tue, Dec 29, 2015 at 9:44 AM, Stefan Wahren <stefan.wah...@i2se.com> wrote:

> i did some tests with a IMX233-Olinuxino Maxi and external 4 port usb hub, 
> but i
> can't reproduce the problem.
>
> Could you provide more information about the hub?

It is a USB2512.

> I think we don't know that the issue comes really from the mx28 instead of the
> hub.

It seems there are other users that reported this same issue at
community.freescale.com.

Regards,

Fabio Estevam
--
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

Re: [PATCH] usb: phy: mxs: Remove unused flags

[Fabio Estevam]

On Tue, Dec 29, 2015 at 6:15 AM, Peter Chen  wrote:

> I prefer to keep it, and it let us know the existing issues although
> we have not implemented workaround for them. In future, we may have
> solutions for how to implement them.

Ok, what about adding the recommendation below?

--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -98,6 +98,9 @@
  * The PHY will be in messy if there is a wakeup after putting
  * bus to suspend (set portsc.suspendM) but before setting PHY to low
  * power mode (set portsc.phcd).
+ *
+ * To work around this problem on mx23/mx28 user should pass
+ * 'usbcore.autosuspend=-1' in the kernel command line for now.
  */
 #define MXS_PHY_ABNORMAL_IN_SUSPENDBIT(1)
--
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

[PATCH] usb: phy: mxs: Remove unused flags

[Fabio Estevam]

From: Fabio Estevam <fabio.este...@nxp.com>

MXS_PHY_ABNORMAL_IN_SUSPEND and MXS_PHY_SENDING_SOF_TOO_FAST flags
are never used, so let's get rid of them.

Reported-by: Stefan Wahren <stefan.wah...@i2se.com> 
Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/phy/phy-mxs-usb.c | 31 +++
 1 file changed, 3 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index c2936dc..cbcc899 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -93,39 +93,14 @@
 
 /* Do disconnection between PHY and controller without vbus */
 #define MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS   BIT(0)
-
-/*
- * The PHY will be in messy if there is a wakeup after putting
- * bus to suspend (set portsc.suspendM) but before setting PHY to low
- * power mode (set portsc.phcd).
- */
-#define MXS_PHY_ABNORMAL_IN_SUSPENDBIT(1)
-
-/*
- * The SOF sends too fast after resuming, it will cause disconnection
- * between host and high speed device.
- */
-#define MXS_PHY_SENDING_SOF_TOO_FAST   BIT(2)
-
-/*
- * IC has bug fixes logic, they include
- * MXS_PHY_ABNORMAL_IN_SUSPEND and MXS_PHY_SENDING_SOF_TOO_FAST
- * which are described at above flags, the RTL will handle it
- * according to different versions.
- */
-#define MXS_PHY_NEED_IP_FIXBIT(3)
+#define MXS_PHY_NEED_IP_FIXBIT(1)
 
 struct mxs_phy_data {
unsigned int flags;
 };
 
-static const struct mxs_phy_data imx23_phy_data = {
-   .flags = MXS_PHY_ABNORMAL_IN_SUSPEND | MXS_PHY_SENDING_SOF_TOO_FAST,
-};
-
 static const struct mxs_phy_data imx6q_phy_data = {
-   .flags = MXS_PHY_SENDING_SOF_TOO_FAST |
-   MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
+   .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
MXS_PHY_NEED_IP_FIX,
 };
 
@@ -151,7 +126,7 @@ static const struct of_device_id mxs_phy_dt_ids[] = {
{ .compatible = "fsl,imx6sx-usbphy", .data = _phy_data, },
{ .compatible = "fsl,imx6sl-usbphy", .data = _phy_data, },
{ .compatible = "fsl,imx6q-usbphy", .data = _phy_data, },
-   { .compatible = "fsl,imx23-usbphy", .data = _phy_data, },
+   { .compatible = "fsl,imx23-usbphy", .data = NULL, },
{ .compatible = "fsl,vf610-usbphy", .data = _phy_data, },
{ .compatible = "fsl,imx6ul-usbphy", .data = _phy_data, },
{ /* sentinel */ }
-- 
1.9.1

--
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

Re: MX28 with hub cannot reset

[Fabio Estevam]

Hi Stefan,

On Thu, Dec 24, 2015 at 6:42 AM, Stefan Wahren <stefan.wah...@i2se.com> wrote:

> i want to know more about this issue:
>
> Are there any unaffected Mainline Kernel versions?

It affects all kernel versions.

> Are there special hardware requirements to reproduce this issue?

I can only see this problem on a mx28 board that has a USB hub. Saw
other users reporting similar issues on mx28 boards with hub.

> Is this issue documented in a chip errata?

I do not see this issue in the mx28 errata document.

> Is it reproducable with an i.MX28 EVK?

No, it is not as mx28evk does not have a hub.

> Is i.MX23 affected, too?

As far as I know mx23 olinuxino maxi has a USB hub and the issue is
not present there.

Maybe Peter can confirm.

Regards,

Fabio Estevam
--
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

Re: MX28 with hub cannot reset

[Fabio Estevam]

Hi Peter,

On Wed, Dec 23, 2015 at 7:49 AM, Peter Chen  wrote:

> Fabio, there is a bug for imx28 usb low power mode, and we have no
> good way to implement
> workaround using current USB PHY framework, would you please test by
> add "usbcore.autosuspend = -1"
> at bootargs (no CI_HDRC_SUPPORTS_RUNTIME_PM needed to set for imx28) to
> see if this problem can fix?

Excellent, passing 'usbcore.autosuspend=-1' fixes the issue!

Do you think we could have a fix for mx28 so that we don't need to
pass this option?

Thanks
--
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

MX28 with hub cannot reset

[Fabio Estevam]

Hi,

On a mx28 custom board with a USB hub I can get a USB stick to get
detected after applying this change:
http://marc.info/?l=linux-usb=145079935324339=2

Things work fine if I boot without the USB stick connected. Then I can
insert/remove the USB device and it always gets detected.

However, if I boot with the USB stick connected I get:

[2.968513] hub 1-1:1.0: USB hub found
[2.973096] hub 1-1:1.0: 2 ports detected
[3.479731] usb 1-1: USB disconnect, device number 2
[3.525403] usb usb1-port1: cannot reset (err = -32)
[3.530828] usb usb1-port1: cannot reset (err = -32)
[3.536089] usb usb1-port1: cannot reset (err = -32)
[3.541600] usb usb1-port1: cannot reset (err = -32)
[3.546857] usb usb1-port1: cannot reset (err = -32)
[3.551982] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.560302] usb usb1-port1: cannot reset (err = -32)
[3.565566] usb usb1-port1: cannot reset (err = -32)
[3.571011] usb usb1-port1: cannot reset (err = -32)
[3.576266] usb usb1-port1: cannot reset (err = -32)
[3.581769] usb usb1-port1: cannot reset (err = -32)
[3.586780] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.594138] usb usb1-port1: cannot reset (err = -32)
[3.599615] usb usb1-port1: cannot reset (err = -32)
[3.604871] usb usb1-port1: cannot reset (err = -32)
[3.610244] usb usb1-port1: cannot reset (err = -32)
[3.615499] usb usb1-port1: cannot reset (err = -32)
[3.620649] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.627989] usb usb1-port1: cannot reset (err = -32)
[3.633370] usb usb1-port1: cannot reset (err = -32)
[3.638617] usb usb1-port1: cannot reset (err = -32)
[3.644171] usb usb1-port1: cannot reset (err = -32)
[3.649546] usb usb1-port1: cannot reset (err = -32)
[3.654557] usb usb1-port1: Cannot enable. Maybe the USB cable is bad?
[3.661650] usb usb1-port1: unable to enumerate USB device

And the only way to get the USB port working again is to remove the
USB stick and reboot the board.

Commit d0ee68b59e6aa33221 ("usb: phy: mxs: Add VF610 USB PHY support")
shows the same error messages in the commit log, but trying the same
approach of adding the MXS_PHY_NEED_IP_FIX flag does not help on mx28.

Any suggestions?

Thanks,

Fabio Estevam
--
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

[PATCH] chipidea: imx: Enable PM runtime for mx28

[Fabio Estevam]

On a custom mx28 board that has a USB hub we see the following
error:

hub 1-1:1.0: USB hub found
hub 1-1:1.0: 2 ports detected
usb 1-1: USB disconnect, device number 2
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: cannot reset (err = -32)
usb usb1-port1: Cannot enable. Maybe the USB cable is bad?

Passing CI_HDRC_SUPPORTS_RUNTIME_PM flag fixes the problem.

Tested on a mx28evk and also on a mx28 custom board.

Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index f14f4ab..0e7bcbb 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -34,6 +34,7 @@ static const struct ci_hdrc_imx_platform_flag imx27_usb_data 
= {
 
 static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
.flags = CI_HDRC_IMX28_WRITE_FIX |
+   CI_HDRC_SUPPORTS_RUNTIME_PM |
CI_HDRC_TURN_VBUS_EARLY_ON |
CI_HDRC_DISABLE_STREAMING,
 };
-- 
1.9.1

--
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

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

Hi Maciej,

On Tue, Dec 15, 2015 at 6:19 PM, Maciej S. Szmigiero
 wrote:

> Can we now use this change for repairing the USB support on UDOO board?
>
> This seems to work fine if not 100% correct:
> --- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
> @@ -122,7 +122,10 @@
> pinctrl-names = "default";
> pinctrl-0 = <_usbh>;
> vbus-supply = <_usb_h1_vbus>;
> -   clocks = < 201>;
> +   clocks = < IMX6QDL_CLK_USBOH3>,
> +< IMX6QDL_CLK_USBOH3>,
> +< 201>;
> +   clock-names = "ipg", "ahb", "per";
> status = "okay";

Yes, just tested it and this makes USB to work again on Udoo board.

As Peter is working on a solution for providing reset and clock to the
USB hub, could you please help us debugging his proposal at
http://www.spinics.net/lists/linux-usb/msg134171.html ?

Thanks
--
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

Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver

[Fabio Estevam]

Hi Peter,

On Wed, Dec 16, 2015 at 2:11 AM, Peter Chen  wrote:

> Thanks, Fabio, but I am curious how things like that? The USBOH3 clock
> is not opened, the usb driver will hang when it tries to access
> registers. If this clock is always on, then, why the system will
> hang later?

I found the issue with your patch. You missed to add the pinctrl node.

With the change below USB is functional in Udoo:

--- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi
@@ -22,6 +22,8 @@

usb_hub1 {
compatible = "generic-onboard-hub";
+   pinctrl-names = "default";
+   pinctrl-0 = <_usbh>;
clocks = < IMX6QDL_CLK_CKO>;
reset-gpios = < 12 GPIO_ACTIVE_LOW>;
reset-duration-us = <2>;
--
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

Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver

[Fabio Estevam]

On Tue, Dec 15, 2015 at 4:28 AM, Peter Chen  wrote:

> Thanks, Fabio.
>
> I am afraid I forget to set gpio as output, would you please apply
> below patch against my original ones:

Same error happens with these changes applied.

Here are more details: if I run a pure 4.3.2 then I do see the USB
stick to get detected if I boot it with the USB stick connected to
Udoo board:

[2.170178] usb 1-1.1: new high-speed USB device number 3 using ci_hdrc
[2.305840] usb-storage 1-1.1:1.0: USB Mass Storage device detected
[2.314803] scsi host1: usb-storage 1-1.1:1.0
[2.400283] usb 1-1.3: new high-speed USB device number 4 using ci_hdrc
[3.327925] scsi 1:0:0:0: Direct-Access General  USB Flash Disk   1.0  P2
[3.347070] sd 1:0:0:0: [sda] 7831552 512-byte logical blocks: (4.01 GB/3.73)
[3.356181] sd 1:0:0:0: [sda] Write Protect is off
[3.362550] sd 1:0:0:0: [sda] No Caching mode page found
[3.367899] sd 1:0:0:0: [sda] Assuming drive cache: write through
[3.387401]  sda: sda1
[3.400238] sd 1:0:0:0: [sda] Attached SCSI removable disk
[4.931847] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow contx
[4.941414] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[4.961400] Sending DHCP requests ., OK
[5.380247] IP-Config: Got DHCP answer from 10.29.244.251, my address is 10.1
[5.390461] IP-Config: Complete:
[5.393731]  device=eth0, hwaddr=00:c0:08:88:0c:b5, ipaddr=10.29.244.61,4
[5.404074]  host=10.29.244.61, domain=am.freescale.net, nis-domain=(non)
[5.411362]  bootserver=0.0.0.0, rootserver=10.29.244.24, rootpath= 0
[5.423964] ALSA device list:
[5.426969]   No soundcards found.
[5.469374] VFS: Mounted root (nfs filesystem) readonly on device 0:14.
[5.478119] devtmpfs: mounted
[5.482376] Freeing unused kernel memory: 456K (c0a7e000 - c0af)
starting pid 160, tty '': '/etc/rc.d/rcS'
Mounting /proc and /sys
Starting the hotplug events dispatcher udevd
Synthesizing initial hotplug even[6.085842] udevd (173): /proc/173/oom_adj .

,but the system hangs here.

If I boot it with the USB stick disconnected, then the system boots
until the prompt, but the insertion of the USB stick is never detected
afterwards.

With your patch applied, the error message (usb 1-1: device descriptor
read/64, error -7) is shown with USB stick connected or disconnected
during boot.
--
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

Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver

[Fabio Estevam]

Hi Peter,

On Mon, Dec 14, 2015 at 5:26 AM, Peter Chen  wrote:

> Hi all,
>
> There is a known issue that the USB code can't handle USB HUB's
> external pins well, in that case, it may cause some onboard
> USB HUBs can't work since their PHY's clock or reset pin needs to
> operate.
>
> The user reported this issue at below:
> http://www.spinics.net/lists/linux-usb/msg131502.html
>
> In this patch set, I add a generic onboard USB HUB driver to
> handle this problem, the external signals will be configured
> before usb controller's initialization, it much likes we did
> it at board code before.
>
> The user needs to add this generic hub node at dts to support it.
>
> @The udoo users, help to test please.

This is what I get with your series applied:

[2.288300] usb 1-1: device descriptor read/64, error -71
[2.518083] usb 1-1: new full-speed USB device number 3 using ci_hdrc
[2.738078] usb 1-1: device descriptor read/64, error -71
[3.058078] usb 1-1: device descriptor read/64, error -71
[3.288079] usb 1-1: new full-speed USB device number 4 using ci_hdrc
[3.768069] usb 1-1: device not accepting address 4, error -71
[3.888084] usb 1-1: new full-speed USB device number 5 using ci_hdrc
[4.368067] usb 1-1: device not accepting address 5, error -71
[4.374117] usb usb1-port1: unable to enumerate USB device
--
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

Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB

[Fabio Estevam]

On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen  wrote:
> Add dt-binding documentation for generic onboard USB HUB.
>
> Signed-off-by: Peter Chen 
> ---
>  .../bindings/usb/generic-onboard-hub.txt   | 28 
> ++
>  1 file changed, 28 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt 
> b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> new file mode 100644
> index 000..ea92205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> @@ -0,0 +1,28 @@
> +Generic Onboard USB HUB
> +
> +Required properties:
> +- compatible: should be "generic-onboard-hub"
> +
> +Optional properties:
> +- clocks: the input clock for HUB.
> +
> +- clock-names: Should be "external_clk"
> +
> +- hub-reset-gpios: Should specify the GPIO for reset.
> +
> +- hub-reset-active-high: the active reset signal is high, if this property is
> +  not set, the active reset signal is low.
> +
> +- hub-reset-duration-us: the duration for assert reset signal, the time unit
> +  is microsecond.
> +
> +Example:
> +
> +   usb_hub1 {
> +   compatible = "generic-onboard-hub";
> +   clocks = < IMX6QDL_CLK_CKO>;
> +   clock-names = "external_clk";
> +   hub-reset-gpios = < 12 0>;
> +   hub-reset-active-high;

I think you could drop the 'hub-reset-active-high' property and do
like this instead:

hub-reset-gpios = < 12 GPIO_ACTIVE_HIGH>;

or

hub-reset-gpios = < 12 GPIO_ACTIVE_LOW>;
--
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

Re: Udoo support for chipidea

[Fabio Estevam]

Hi Peter,

On Mon, Oct 19, 2015 at 12:50 AM, Peter Chen  wrote:

> Add linux-usb.
>
> Patryk, your problem is you may need to open 24M OSC for HUB 2514
> manually, and you have used IMX6QDL_CLK_CKO for it in the design,
> but this clock is not controller's clock, controller's clock has
> already decided at SoC dts file (imx6qdl.dtsi), you don't need to
> override it at board's dts file.
>
> You can try delete clock property at imx6qdl-udoo.dtsi, if it still
> can't work, try to open IMX6QDL_CLK_CKO at one place to test.

What is the appropriate place to acquire and enable the USB hub clock?

This issue has appeared several times and it seems we don't have a
solution for this yet.

Any suggestions?
--
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

Re: iperf UDP packet loss with Chipidea HDRC

[Fabio Estevam]

On Mon, Oct 5, 2015 at 10:57 AM, Jayan John  wrote:
> We are developing a custom USB device on a iMX6q platform with a Chipidea
> HDRC. The device uses a single NCM interface for communication with another
> OTG device i.e. Chipidea HDRC again. I see very poor iperf UDP performance
> after role reversal with iperf server running on gadget.
>
> Kernel: 3.10.17 using Wandboard config

Could you test this using kernel 4.2.3 or 4.3-rc4?
--
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

Re: [PATCH 0/7][v4] Add OTG support for FSL socs

[Fabio Estevam]

On Thu, Aug 27, 2015 at 1:43 PM, Ramneek Mehresh
 wrote:
> Add support for otg for all freescale socs having internal
> usb phy.
>
> Ramneek Mehresh (7):
>   usb:fsl:otg: Make fsl otg driver as tristate
>   usb:fsl:otg: Add controller version based ULPI and UTMI phy
>   usb:fsl:otg: Add support to add/remove usb host driver
>   usb:fsl:otg: Signal host drv when host is otg
>   usb:fsl:otg: Modify otg_event to start host drv
>   usb:fsl:otg: Combine host/gadget start/resume for ID change
>   usb:fsl:otg: Add host-gadget drv sync delay
>
>  drivers/usb/host/ehci-fsl.c   | 73 
> +++
>  drivers/usb/host/ehci-fsl.h   | 16 ++
>  drivers/usb/phy/Kconfig   |  2 +-
>  drivers/usb/phy/phy-fsl-usb.c | 58 +-
>  drivers/usb/phy/phy-fsl-usb.h |  7 +
>  include/linux/usb.h   |  1 +
>  6 files changed, 122 insertions(+), 35 deletions(-)

Isn't this all Chipidea IP? If so, why don't you use the existing
chipidea driver instead?
--
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

Re: [PATCH 2/2] Doc: usb: ci-hdrc-usb2: Add phy-clkgate-delay-us entry

[Fabio Estevam]

Hi Peter,

On Tue, Sep 8, 2015 at 11:27 PM, Peter Chen  wrote:

> I will queue both for next rc, thanks.

It would be nice if these patches could appear in linux-next soon.
--
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

Re: [PATCH v3 0/5] usb: change clock information for chipidea

[Fabio Estevam]

On Sun, Sep 20, 2015 at 9:43 PM, Peter Chen  wrote:
> This patch set changes usb clock information for legacy i.mx platforms.
> At these platforms, they needs three clocks to let controller work.
>
> Hi Fabio,
>
> Would you please have a test at imx27 and imx25 boards, thanks.

Works fine on both boards, thanks
--
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

Re: Applying Patch to Change Max Packet Size for Device

[Fabio Estevam]

On Wed, Sep 16, 2015 at 8:27 PM, Todd Efflam  wrote:
> We were using EHCI with the 2.6.35 kernel.  When disabling XHCI in
> 3.14 and using EHCI the device still doesn't work however.
>
> The reason we are stuck with 3.14 is we use the Yocto build system and
> their latest support is for 3.14.

You could just build 4.2 or 4.3-rc1 manually and deploy it.

We use Yocto with kernel 4.1 without issues.

Which CPU are you using?
--
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

Re: [PATCH] USB: EHCI: fix dereference of ERR_PTR

[Fabio Estevam]

On Wed, Sep 16, 2015 at 11:08 AM, Sudip Mukherjee
 wrote:
> On error find_tt() returns either a NULL pointer or the error value in
> ERR_PTR. But we were dereferencing it directly without even checking if
> find_tt() returned a valid pointer or not.
>
> Signed-off-by: Sudip Mukherjee 
> ---
>  drivers/usb/host/ehci-sched.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index f9a3327..27bced7 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -257,6 +257,8 @@ static void reserve_release_intr_bandwidth(struct 
> ehci_hcd *ehci,
> /* FS/LS bus bandwidth */
> if (tt_usecs) {
> tt = find_tt(qh->ps.udev);
> +   if (!tt || IS_ERR(tt))
> +   return;

Could you use IS_ERR_OR_NULL(tt)?
--
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

Re: MIDI gadget allocating too much from coherent pool

[Fabio Estevam]

Hi Clemens,

On Mon, Sep 14, 2015 at 4:24 AM, Clemens Ladisch <clem...@ladisch.de> wrote:

> The "fsl_usb2_udc" driver (if that is what you're using) uses a coherent
> DMA pool for "TD management".  I see no obvious problem with how it
> calls dma_pool_alloc()/dma_pool_free().  Are there any messages in the
> system log?  You might want to modify the kernel to check how often
> it calls these functions.

The udc driver used by mx6 is drivers/usb/chipidea/udc.c.

Regards,

Fabio Estevam
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.c...@freescale.com> wrote:
> For imx27, it needs three clocks to let the controller work,
> the old code is wrong, and will cause below data abort when
> accass usbmisc registers.
>
> usbcore: registered new interface driver usb-storage
> Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
> pgd = c0004000
> [f4424600] *pgd=1452(bad)
> Internal error: : 8 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
> Hardware name: Freescale i.MX27 (Device Tree Support)
> task: c7832b60 ti: c783e000 task.ti: c783e000
> PC is at usbmisc_imx27_init+0x4c/0xbc
> LR is at usbmisc_imx27_init+0x40/0xbc
> pc : []lr : []psr: 6093
> sp : c783fe08  ip :   fp : 
> r10: c0576434  r9 : 009c  r8 : c7a773a0
> r7 : 0100  r6 : 6013  r5 : c7a776f0  r4 : c7a773f0
> r3 : f4424600  r2 :   r1 : 0001  r0 : 0001
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 0005317f  Table: a0004000  DAC: 0017
> Process swapper (pid: 1, stack limit = 0xc783e190)
> Stack: (0xc783fe08 to 0xc784)
>
> Signed-off-by: Peter Chen <peter.c...@freescale.com>
> Reported-by: Fabio Estevam <fabio.este...@freescale.com>
> Cc: <sta...@vger.kernel.org>
> ---
>  arch/arm/boot/dts/imx27.dtsi | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
> index feb9d34..93e3e14 100644
> --- a/arch/arm/boot/dts/imx27.dtsi
> +++ b/arch/arm/boot/dts/imx27.dtsi

mx25.dtsi and mx35.dtsi also need to be changed.

> @@ -486,7 +486,11 @@
> compatible = "fsl,imx27-usb";
> reg = <0x10024000 0x200>;
> interrupts = <56>;
> -   clocks = < IMX27_CLK_USB_IPG_GATE>;
> +   need-three-clocks;
> +   clocks = < IMX27_CLK_USB_IPG_GATE>,
> +   < IMX27_CLK_USB_AHB_GATE>,
> +   < IMX27_CLK_USB_DIV>;
> +   clock-names = "ipg", "ahb", "per";

'ipg' and 'ahb' are enough.
--
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

Re: [PATCH 4/4] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.c...@freescale.com> wrote:
> Some i.mx platforms need three clocks to let controller work, but
> others only need one, refine clock operation to adapt for all
> platforms.
>
> Cc: Fabio Estevam <fabio.este...@freescale.com>
> Signed-off-by: Peter Chen <peter.c...@freescale.com>

Please Cc stable on this patch as it fixes a regression.
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Fabio Estevam]

On Tue, Sep 15, 2015 at 11:31 PM, Peter Chen  wrote:

>> mx25.dtsi and mx35.dtsi also need to be changed.
>
> If you can help to test, I will update.

Sure, I can test it on mx25pdk tomorrow.
--
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

Re: [PATCH 3/4] ARM: dts: imx27.dtsi: change the clock information for usb

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.c...@freescale.com> wrote:
> For imx27, it needs three clocks to let the controller work,
> the old code is wrong, and will cause below data abort when
> accass usbmisc registers.

s/acass/accessing

>
> usbcore: registered new interface driver usb-storage
> Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
> pgd = c0004000
> [f4424600] *pgd=1452(bad)
> Internal error: : 8 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
> Hardware name: Freescale i.MX27 (Device Tree Support)
> task: c7832b60 ti: c783e000 task.ti: c783e000
> PC is at usbmisc_imx27_init+0x4c/0xbc
> LR is at usbmisc_imx27_init+0x40/0xbc
> pc : []lr : []psr: 6093
> sp : c783fe08  ip :   fp : 
> r10: c0576434  r9 : 009c  r8 : c7a773a0
> r7 : 0100  r6 : 6013  r5 : c7a776f0  r4 : c7a773f0
> r3 : f4424600  r2 :   r1 : 0001  r0 : 0001
> Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> Control: 0005317f  Table: a0004000  DAC: 0017
> Process swapper (pid: 1, stack limit = 0xc783e190)
> Stack: (0xc783fe08 to 0xc784)
>
> Signed-off-by: Peter Chen <peter.c...@freescale.com>
> Reported-by: Fabio Estevam <fabio.este...@freescale.com>
> Cc: <sta...@vger.kernel.org>

Tested-by: Fabio Estevam <fabio.este...@freescale.com>
--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Fabio Estevam]

On Tue, Sep 15, 2015 at 11:24 PM, Peter Chen  wrote:

> but drivers/usb/gadget/udc/fsl_mxc_udc.c has three clocks

Ok, looking at the clock driver I see that they used to associate
three clocks with mxc-ehci:

clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.0");
clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.0");

So your approach looks good.

I only think that mx25 and mx35 dtsi should be also updated to have
the 'three-clock' property.

Thanks
--
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

Re: [PATCH 4/4] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen <peter.c...@freescale.com> wrote:
> Some i.mx platforms need three clocks to let controller work, but
> others only need one, refine clock operation to adapt for all
> platforms.

It would be better to mention that this is fixing a regression on mx27.

>
> Cc: Fabio Estevam <fabio.este...@freescale.com>
> Signed-off-by: Peter Chen <peter.c...@freescale.com>

Tested-by: Fabio Estevam <fabio.este...@freescale.com>
--
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

Re: [PATCH 2/4] doc: dt-binding: ci-hdrc-usb2: add i.mx specific binding "need-three-clocks"

[Fabio Estevam]

On Tue, Sep 15, 2015 at 10:49 PM, Peter Chen  wrote:
> Some SoCs needs three clock to let controller work, but others only
> need one, add one property to differentiate this.
>
> Signed-off-by: Peter Chen 
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt 
> b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index f15a317..4900092 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -54,6 +54,9 @@ i.mx specific properties
>argument that indicate usb controller index
>  - disable-over-current: disable over current detect
>  - external-vbus-divider: enables off-chip resistor divider for Vbus
> +- need-three-clocks: the SoC before imx6 series (except for imx23/imx28)
> +  needs three clcoks for controller, others only need one. Without this
> +  property, the driver will consider this controller only need one clock.

Looking at mx31, mx35, mx25 reference manuals we see that they don't
really need three usb clocks.

Also, if we look at the old drivers/usb/host/ehci-mxc.c we only
require 'ipg' and 'ahb' clocks.

So it would be better to just require these two clocks and if they are
not found on dts, then we fall back to requesting clk_get(NULL),
without the need of an extra property.
--
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

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

Hi Peter,

On Mon, Sep 14, 2015 at 11:32 PM, Fabio Estevam <feste...@gmail.com> wrote:

> This did not help.
>
> It is getting late here, so I will be able to try more things tomorrow.

I was able to fix it. Your initial patch had a missing 'return 0' in
imx_prepare_enable_clks(), causing:

clk_disable_unprepare(data->clk_ahb);
clk_disable_unprepare(data->clk_ipg);

to always be called.

Doing like this on top of your original patch:

--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -212,6 +212,8 @@ static int imx_prepare_enable_clks(struct device *dev)
}
}

+   return 0;
+
 err2:
clk_disable_unprepare(data->clk_ahb);
 err1:

, fixes the crash.

Would you like to split your patch into dts and usb parts and then
resend it formally?

Thanks,

Fabio Estevam
--
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

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

On Mon, Sep 14, 2015 at 1:54 AM, Peter Chen <peter.c...@freescale.com> wrote:
> On Fri, Sep 11, 2015 at 10:17:20AM -0300, Fabio Estevam wrote:

> Sorry, a bug at former patch, it the clk_ahb has been override.
> Mind to try below one?

Applied the patch and still see the issue:

Booting Linux on physical CPU 0x0
Linux version 4.3.0-rc1-next-20150914-dirty (fabio@fabio-Latitude-E6410) (gcc v5
CPU: ARM926EJ-S [41069264] revision 4 (ARMv5TEJ), cr=0005317f
CPU: VIVT data cache, VIVT instruction cache
Machine model: Freescale i.MX27 Product Development Kit
Memory policy: Data cache writeback
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
Kernel command line: noinitrd console=ttymxc0,115200 root=/dev/nfs nfsroot=192.p
PID hash table entries: 512 (order: -1, 2048 bytes)
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
Memory: 121952K/131072K available (5428K kernel code, 370K rwdata, 1636K rodata)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xc880 - 0xff00   ( 872 MB)
lowmem  : 0xc000 - 0xc800   ( 128 MB)
modules : 0xbf00 - 0xc000   (  16 MB)
  .text : 0xc0008000 - 0xc06ee5f8   (7066 kB)
  .init : 0xc06ef000 - 0xc073a000   ( 300 kB)
  .data : 0xc073a000 - 0xc0796aac   ( 371 kB)
   .bss : 0xc0796aac - 0xc07b6a18   ( 128 kB)
Preemptible hierarchical RCU implementation.
Build-time adjustment of leaf fanout to 32.
NR_IRQS:16 nr_irqs:16 16
MXC IRQ initialized
CPU identified as i.MX27, silicon rev 2.1
Switching to timer-based delay loop, resolution 75ns
sched_clock: 32 bits at 13MHz, resolution 75ns, wraps every 161464936410ns
clocksource: mxc_timer1: mask: 0x max_cycles: 0x, max_idle_ns: s
Console: colour dummy device 80x30
Calibrating delay loop (skipped), value calculated using timer frequency.. 26.6)
pid_max: default: 32768 minimum: 301
Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
CPU: Testing write buffer coherency: ok
Setting up static identity map for 0xa0008400 - 0xa000847c
devtmpfs: initialized
clocksource: jiffies: mask: 0x max_cycles: 0x, max_idle_ns: 191s
pinctrl core: initialized pinctrl subsystem
NET: Registered protocol family 16
DMA: preallocated 256 KiB pool for atomic coherent allocations
imx27-pinctrl 10015000.iomuxc: initialized IMX pinctrl driver
SCSI subsystem initialized
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
usbphy:usbphy@0 supply vcc not found, using dummy regulator
Linux video capture interface: v2.00
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@>
PTP clock support registered
Advanced Linux Sound Architecture Driver Initialized.
clocksource: Switched to clocksource mxc_timer1
NET: Registered protocol family 2
TCP established hash table entries: 1024 (order: 0, 4096 bytes)
TCP bind hash table entries: 1024 (order: 0, 4096 bytes)
TCP: Hash tables configured (established 1024 bind 1024)
UDP hash table entries: 256 (order: 0, 4096 bytes)
UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
futex hash table entries: 256 (order: -1, 3072 bytes)
jffs2: version 2.2. (NAND) �© 2001-2006 Red Hat, Inc.
io scheduler noop registered (default)
1000a000.serial: ttymxc0 at MMIO 0x1000a000 (irq = 36, base_baud = 831250) is aX
console [ttymxc0] enabled
nand: device found, Manufacturer ID: 0xec, Chip ID: 0xaa
nand: Samsung NAND 256MiB 1,8V 8-bit
nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
Bad block table found at page 131008, version 0x01
Bad block table found at page 130944, version 0x01
mc13xxx spi1.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
spi_imx 1000f000.cspi: probed
1002b000.ethernet supply phy not found, using dummy regulator
libphy: fec_enet_mii_bus: probed
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
ehci-mxc: Freescale On-Chip EHCI Host driver
usbcore: registered new interface driver usb-storage
imx_usb 10024000.usb: three clock have enabled
Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
pgd = c0004000
[f4424600] *pgd=1452(bad)
Internal error: : 8 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0-rc1-next-20150914-dirty #56
Hardware name: Freescale i.MX27 (Device Tree Support)
task: c7832b60 ti: c783e000 task.ti: c783e000
PC is at usbmisc_imx27_init+0x4c/0xbc
LR is at usbmisc_imx27_init+0x40/0xbc
pc : []lr : []psr: 6

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

On Mon, Sep 14, 2015 at 10:03 PM, Peter Chen <peter.c...@freescale.com> wrote:

> Would you try to insert udelay(100) at every clk_prepare_enable at
> imx_prepare_enable_clks at ci_hdrc_imx.c?

This did not help.

It is getting late here, so I will be able to try more things tomorrow.

Regards,

Fabio Estevam
--
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

Re: MIDI gadget allocating too much from coherent pool

[Fabio Estevam]

On Thu, Sep 10, 2015 at 6:47 AM, Felipe Tonello  wrote:
> Hi,
>
> I have the following setup:
>
> DSP (read sensors and read/send MIDI) <- UART -> SOC (imx6) <- USB
> MIDI GADGET -> HOST
>
> When the throughput from the DSP is high, thus causing the throughput
> on the USB to be high as well, I get a Kernel Panic saying to increase
> the coherent_pool. I've used some crazy sizes like 1M, 4M and even 8M.
> Obviously when I increase, it takes longer to crash but it still
> crashes.
>
> I am using linux-fsl 3.14.28.

Please test it with 4.2 or 4.3-rc1 kernel.
--
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

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

On Mon, Sep 14, 2015 at 9:19 PM, Peter Chen  wrote:

> That's so strange, do we need to enable clk_ahb before than clk_ipg?
> Would you please try below diff based on my patch?

Same kernel crash with this one applied on top of the previous one.

>
> Meanwhile, you may need below changes
>
> http://marc.info/?l=linux-usb=143640061512265=2

This proposed change is not correct. It passes IMX27_CLK_USB_AHB_GATE
as if it were the clock driving the ULPI PHY, which is clearly wrong.
--
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

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

On Mon, Sep 14, 2015 at 9:50 PM, Peter Chen  wrote:

>
> Would you try below?
>
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 4decb12..34e9f6e 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -16,4 +16,4 @@ obj-$(CONFIG_USB_CHIPIDEA)+= ci_hdrc_zevio.o
>
>  obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o
>
> -obj-$(CONFIG_USB_CHIPIDEA_OF)  += usbmisc_imx.o ci_hdrc_imx.o
> +obj-$(CONFIG_USB_CHIPIDEA_OF)  += ci_hdrc_imx.o usbmisc_imx.o

This one gives kernel dump in loop, with a different pattern:

b9c0:    c073f110 c78bd800 c0743c38 c0522e3c c78bda3c
b9e0: c0033b02 c78eba44 c0743c38 c78eba08 c002f2f8 c0033b00 2093 
[] (__dabt_svc) from [] (kthread_data+0x4/0xc)
[] (kthread_data) from [] (wq_worker_sleeping+0xc/0xbc)
[] (wq_worker_sleeping) from [] (__schedule+0x2fc/0x654)
[] (__schedule) from [] (schedule+0x3c/0xa8)
[] (schedule) from [] (do_exit+0x7ac/0x9a0)
[] (do_exit) from [] (die+0x164/0x3b0)
[] (die) from [] (__do_kernel_fault.part.0+0x54/0x1e4)
[] (__do_kernel_fault.part.0) from [] (do_page_fault+0x338/)
[] (do_page_fault) from [] (do_DataAbort+0x34/0xb4)
[] (do_DataAbort) from [] (__dabt_svc+0x3c/0x60)
Exception stack(0xc78ebbe8 to 0xc78ebc30)
bbe0:   c78bd800     c7832b60
bc00: c78bd800 c0743c38 c0522e3c c78bda3c 0001 c78ebc74 c0743c38 c78ebc38
bc20: c002f2f8 c0033b00 2093 
[] (__dabt_svc) from [] (kthread_data+0x4/0xc)
[] (kthread_data) from [] (wq_worker_sleeping+0xc/0xbc)
[] (wq_worker_sleeping) from [] (__schedule+0x2fc/0x654)
[] (__schedule) from [] (schedule+0x3c/0xa8)
[] (schedule) from [] (do_exit+0x5a4/0x9a0)
[] (do_exit) from [] (die+0x164/0x3b0)
[] (die) from [] (do_DataAbort+0xa4/0xb4)
[] (do_DataAbort) from [] (__dabt_svc+0x3c/0x60)
Exception stack(0xc78ebdc8 to 0xc78ebe10)
bdc0:   0001 0001  f4424600 c7a24790 c7a8c410
bde0: 6013 0100 c055a2a4 c07943fc  0002  c78ebe18
be00: c03af778 c03af784 6093 
[] (__dabt_svc) from [] (usbmisc_imx27_init+0x4c/0xbc)
[] (usbmisc_imx27_init) from [] (imx_usbmisc_init+0x2c/0x38)
[] (imx_usbmisc_init) from [] (ci_hdrc_imx_probe+0x22c/0x38)
[] (ci_hdrc_imx_probe) from [] (platform_drv_probe+0x48/0xa)
[] (platform_drv_probe) from [] (driver_probe_device+0x1e0/)
[] (driver_probe_device) from [] (bus_for_each_drv+0x48/0x9)
[] (bus_for_each_drv) from [] (__device_attach+0x9c/0x100)
[] (__device_attach) from [] (bus_probe_device+0x88/0x90)
[] (bus_probe_device) from [] (deferred_probe_work_func+0x5)
[] (deferred_probe_work_func) from [] (process_one_work+0x1)
[] (process_one_work) from [] (worker_thread+0x28/0x524)
[] (worker_thread) from [] (kthread+0xc0/0xdc)
[] (kthread) from [] (ret_from_fork+0x14/0x24)
Code: c0032f64 c0621d18 c07976a0 e5903210 (e5130010)

> But the error log looks similar, it seems we does not open 
> IMX27_CLK_USB_AHB_GATE
> correctly.

Yes, but we cannot pass IMX27_CLK_USB_AHB_GATE as the clock for the
ULPI PHY as it is not correct.
--
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

Re: [PATCH] Revert "usb: chipidea: usbmisc_imx: delete clock information"

[Fabio Estevam]

On Fri, Sep 11, 2015 at 4:52 AM, Peter Chen <peter.c...@freescale.com> wrote:

> Hi Fabio,
>
> Would you please help to test build patches for this issue?

Tested the patch on a mx27pdk running 4.2 and I got the same crash:

usbcore: registered new interface driver usb-storage
Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
pgd = c0004000
[f4424600] *pgd=1452(bad)
Internal error: : 8 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-dirty #93
Hardware name: Freescale i.MX27 (Device Tree Support)
task: c7832b60 ti: c783e000 task.ti: c783e000
PC is at usbmisc_imx27_init+0x4c/0xbc
LR is at usbmisc_imx27_init+0x40/0xbc
pc : []lr : []psr: 6093
sp : c783fe10  ip :   fp : 
r10:   r9 : 009c  r8 : c056d488
r7 : 0100  r6 : 6013  r5 : c7a785d0  r4 : c7a782f0
r3 : f4424600  r2 :   r1 : 0001  r0 : 0001
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005317f  Table: a0004000  DAC: 0017
Process swapper (pid: 1, stack limit = 0xc783e190)
Stack: (0xc783fe10 to 0xc784)
fe00: c03c4a50 c7946410 c7946400 
fe20: c79b8290 c03c46c4  c03c5254 c7a77538 c79553c0 0100 
fe40:  c7989c30  0008    
fe60: ffed c7946410 fdfb c0792418  c02fdb94 c02fdb4c c7946410
fe80: c07c19d0  c0792418 c02fc1f0 c7946410 c0792418 c7946444 
fea0: c072c0c8 c02fc324  c0792418 c02fc298 c02fa978 c7895d2c c78d8f10
fec0: c0792418 c7a5fb20 c077e268 c02fb948 c067f7f4 c0284118 c0792418 c0792418
fee0: c07502b8 c7a78620 c07ab460 c02fcdc0  c07502b8 c07502b8 c00095c8
ff00: c07b372c c78bc980 c053c958 004f    c013b43c
ff20: c7ffc890 c0552e20 009c c0032448  c067ff7c c06d5d94 0006
ff40: c7ffc897 0006 c07552f8 c7ffc840 c074b090 0006 c073f8dc c07ab460
ff60: c0704594 009c c073f8e8 c0704d20 0006 0006  c0704594
ff80: c78327a0 c052f55c  c052f55c    
ffa0:  c052f564  c000a320    
ffc0:        
ffe0:     0013  bfff efff
[] (usbmisc_imx27_init) from [] (imx_usbmisc_init+0x2c/0x38)
[] (imx_usbmisc_init) from []
(ci_hdrc_imx_probe+0x208/0x35c)
[] (ci_hdrc_imx_probe) from []
(platform_drv_probe+0x48/0xa4)
[] (platform_drv_probe) from []
(driver_probe_device+0x1d0/0x278)
[] (driver_probe_device) from [] (__driver_attach+0x8c/0x90)
[] (__driver_attach) from [] (bus_for_each_dev+0x5c/0x8c)
[] (bus_for_each_dev) from [] (bus_add_driver+0xe8/0x1f8)
[] (bus_add_driver) from [] (driver_register+0x78/0xf4)
[] (driver_register) from [] (do_one_initcall+0x84/0x1f0)
[] (do_one_initcall) from []
(kernel_init_freeable+0xfc/0x1c0)
[] (kernel_init_freeable) from [] (kernel_init+0x8/0xec)
[] (kernel_init) from [] (ret_from_fork+0x14/0x34)
Code: ebf1cfb7 e5d43008 e3130001 e5953000 (e5932000)
---[ end trace 69bc01e63a4b5221 ]---
note: swapper[1] exited with preempt_count 1
Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b

---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b

If I revert 73dea4a912b2("usb: chipidea: usbmisc_imx: delete clock
information") then the board boots fine.

Regards,

Fabio Estevam
--
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