| Message ID | 20200227104212.12562-1-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | USB IMX Chipidea fix gpio vbus control | expand |
On 20-02-27 11:42:09, Marco Felsch wrote: > Hi, > > the gpio-based port power is broken since commit [1,2]. I changed the > core behaviour to to cleanup the code and avoid local workaround fixes. Many i.mx series evk boards work well for gpio-based port power control, what problem you have found, would you please list it detail? Peter > > With this series and [3] I'am now able to disable the port completely > after I disbaled the pm_qos_no_power_off flag. > > [1] 6adb9b7b5fb6 ("usb: chipidea: add a flag for turn on vbus early for host") > [2] 659459174188 ("usb: chipidea: host: turn on vbus before add hcd if early vbus on is required") > [3] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F2%2F27%2F198&data=02%7C01%7Cpeter.chen%40nxp.com%7Cc1b3094e826b49b572cb08d7bb71b6bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637183969379126557&sdata=i7O7vBqiVCb2opVNva7ffxIdqFb%2BvGzV5Qsy5wwarbQ%3D&reserved=0 > > Regards, > Marco > > Marco Felsch (3): > USB: ehci-hub: let port_power() override the ehci_port_power() > Partially Revert "usb: chipidea: host: turn on vbus before add hcd if > early vbus on is required" > Revert "usb: chipidea: add a flag for turn on vbus early for host" > > drivers/usb/chipidea/ci_hdrc_imx.c | 10 ++-------- > drivers/usb/chipidea/host.c | 31 ++++++++++-------------------- > drivers/usb/host/ehci-hub.c | 6 +++--- > include/linux/usb/chipidea.h | 17 ++++++++-------- > 4 files changed, 23 insertions(+), 41 deletions(-) > > -- > 2.20.1 >
Hi Peter, thanks for your fast reply :) On 20-02-27 11:18, Peter Chen wrote: > On 20-02-27 11:42:09, Marco Felsch wrote: > > Hi, > > > > the gpio-based port power is broken since commit [1,2]. I changed the > > core behaviour to to cleanup the code and avoid local workaround fixes. > > Many i.mx series evk boards work well for gpio-based port power control, > what problem you have found, would you please list it detail? Hm.. who could that work? I picked the important parts: static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) { ... if (priv->reg_vbus && enable != priv->enabled) { ... if (enable) ret = regulator_enable(priv->reg_vbus); else ret = regulator_disable(priv->reg_vbus); ... } ... } static int host_start(struct ci_hdrc *ci) { ... priv->reg_vbus = NULL; if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci)) { if (ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON) { ret = regulator_enable(ci->platdata->reg_vbus); if (ret) { dev_err(ci->dev, "Failed to enable vbus regulator, ret=%d\n", ret); goto put_hcd; } } else { priv->reg_vbus = ci->platdata->reg_vbus; } } ... } Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set. Regards, Marco > Peter > > > > With this series and [3] I'am now able to disable the port completely > > after I disbaled the pm_qos_no_power_off flag. > > > > [1] 6adb9b7b5fb6 ("usb: chipidea: add a flag for turn on vbus early for host") > > [2] 659459174188 ("usb: chipidea: host: turn on vbus before add hcd if early vbus on is required") > > [3] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F2%2F27%2F198&data=02%7C01%7Cpeter.chen%40nxp.com%7Cc1b3094e826b49b572cb08d7bb71b6bc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637183969379126557&sdata=i7O7vBqiVCb2opVNva7ffxIdqFb%2BvGzV5Qsy5wwarbQ%3D&reserved=0 > > > > Regards, > > Marco > > > > Marco Felsch (3): > > USB: ehci-hub: let port_power() override the ehci_port_power() > > Partially Revert "usb: chipidea: host: turn on vbus before add hcd if > > early vbus on is required" > > Revert "usb: chipidea: add a flag for turn on vbus early for host" > > > > drivers/usb/chipidea/ci_hdrc_imx.c | 10 ++-------- > > drivers/usb/chipidea/host.c | 31 ++++++++++-------------------- > > drivers/usb/host/ehci-hub.c | 6 +++--- > > include/linux/usb/chipidea.h | 17 ++++++++-------- > > 4 files changed, 23 insertions(+), 41 deletions(-) > > > > -- > > 2.20.1 > > > > -- > > Thanks, > Peter Chen > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 20-02-27 12:35:39, Marco Felsch wrote: > Hi Peter, > > thanks for your fast reply :) > > On 20-02-27 11:18, Peter Chen wrote: > > On 20-02-27 11:42:09, Marco Felsch wrote: > > > Hi, > > > > > > the gpio-based port power is broken since commit [1,2]. I changed the > > > core behaviour to to cleanup the code and avoid local workaround fixes. > > > > Many i.mx series evk boards work well for gpio-based port power control, > > what problem you have found, would you please list it detail? > > Hm.. who could that work? I picked the important parts: > > static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) > { > ... > > if (priv->reg_vbus && enable != priv->enabled) { > > ... > > if (enable) > ret = regulator_enable(priv->reg_vbus); > else > ret = regulator_disable(priv->reg_vbus); > > ... > } > > ... > } > > static int host_start(struct ci_hdrc *ci) > { > ... > > priv->reg_vbus = NULL; > > if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci)) { > if (ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON) { > ret = regulator_enable(ci->platdata->reg_vbus); > if (ret) { > dev_err(ci->dev, > "Failed to enable vbus regulator, ret=%d\n", > ret); > goto put_hcd; > } > } else { > priv->reg_vbus = ci->platdata->reg_vbus; > } > } > > ... > } > > Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set. > Do you have a VBUS regulator at your dts, and add it at controller's node? See: arch/arm/boot/dts/imx6qdl-sabresd.dtsi as an example please? If you have set CI_HDRC_TURN_VBUS_EARLY_ON, the VBUS is controlled by chipidea driver, not by USB core through PORTSC.PP (ehci_ci_portpower).
On 20-02-27 12:20, Peter Chen wrote: > On 20-02-27 12:35:39, Marco Felsch wrote: > > Hi Peter, > > > > thanks for your fast reply :) > > > > On 20-02-27 11:18, Peter Chen wrote: > > > On 20-02-27 11:42:09, Marco Felsch wrote: > > > > Hi, > > > > > > > > the gpio-based port power is broken since commit [1,2]. I changed the > > > > core behaviour to to cleanup the code and avoid local workaround fixes. > > > > > > Many i.mx series evk boards work well for gpio-based port power control, > > > what problem you have found, would you please list it detail? > > > > Hm.. who could that work? I picked the important parts: > > > > static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) > > { > > ... > > > > if (priv->reg_vbus && enable != priv->enabled) { > > > > ... > > > > if (enable) > > ret = regulator_enable(priv->reg_vbus); > > else > > ret = regulator_disable(priv->reg_vbus); > > > > ... > > } > > > > ... > > } > > > > static int host_start(struct ci_hdrc *ci) > > { > > ... > > > > priv->reg_vbus = NULL; > > > > if (ci->platdata->reg_vbus && !ci_otg_is_fsm_mode(ci)) { > > if (ci->platdata->flags & CI_HDRC_TURN_VBUS_EARLY_ON) { > > ret = regulator_enable(ci->platdata->reg_vbus); > > if (ret) { > > dev_err(ci->dev, > > "Failed to enable vbus regulator, ret=%d\n", > > ret); > > goto put_hcd; > > } > > } else { > > priv->reg_vbus = ci->platdata->reg_vbus; > > } > > } > > > > ... > > } > > > > Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set. > > > > Do you have a VBUS regulator at your dts, and add it at controller's > node? See: arch/arm/boot/dts/imx6qdl-sabresd.dtsi as an example please? Yes, that's my use case too. > If you have set CI_HDRC_TURN_VBUS_EARLY_ON, the VBUS is controlled by > chipidea driver, not by USB core through PORTSC.PP (ehci_ci_portpower). I know, pls have a look my the patches. I had the problem that the vbus regulator wasn't turned off during a suspend/resume logic. The first issue within the usb-core should be fixed by [1] (v2 RFC is on the way). You never run in that case if you don't fix this. After I fixed it the port-power is called during suspend but obviously the regulator didn't get turned off because we don't add it to the priv->reg_vbus. This patchset should fix this and get rid of the CI_HDRC_TURN_VBUS_EARLY_ON flag. Regards, Marco > -- > > Thanks, > Peter Chen
> > > > > > Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set. > > > > > > > Do you have a VBUS regulator at your dts, and add it at controller's > > node? See: arch/arm/boot/dts/imx6qdl-sabresd.dtsi as an example please? > > Yes, that's my use case too. > > > If you have set CI_HDRC_TURN_VBUS_EARLY_ON, the VBUS is controlled by > > chipidea driver, not by USB core through PORTSC.PP (ehci_ci_portpower). > > I know, pls have a look my the patches. > > I had the problem that the vbus regulator wasn't turned off during a > suspend/resume logic. The first issue within the usb-core should be fixed by [1] (v2 > RFC is on the way). You never run in that case if you don't fix this. After I fixed it > the port-power is called during suspend but obviously the regulator didn't get turned > off because we don't add it to the priv->reg_vbus. > > This patchset should fix this and get rid of the CI_HDRC_TURN_VBUS_EARLY_ON > flag. > Hi Marco, I may understand your case now. At old USB port design, the VBUS is never allowed to turned off to response the USB wakeup event. But the expected behavior has changed after pm_qos_no_power_off is introduced for USB port, it is allowed the port is powered off. PORTSC.PP could be controlled by USB core, but USB VBUS's power is not controlled by this bit if the VBUS power enable pin is configured as GPIO function, that is your case. CI_HDRC_TURN_VBUS_EARLY_ON is introduced by fixing a bug that some i.mx USB controllers PHY's power is sourced from VBUS, the PHY's power need to be on before touch some ehci registers, otherwise, the USB signal will be wrong at some low speed devices use case. So, this flag can't be deleted, it may cause regression. The solution I see is your may need to implement chipidea VBUS control flow by considering pm_qos_no_power_off at suspend situation. You may add .suspend API for ci_role_driver, and called by ci_controller_suspend/ci_controller_resume, of cos, better solution is welcome. Peter
> > I had the problem that the vbus regulator wasn't turned off during a > > suspend/resume logic. The first issue within the usb-core should be > > fixed by [1] (v2 RFC is on the way). You never run in that case if you > > don't fix this. After I fixed it the port-power is called during > > suspend but obviously the regulator didn't get turned off because we don't add it > to the priv->reg_vbus. > > > > This patchset should fix this and get rid of the > > CI_HDRC_TURN_VBUS_EARLY_ON flag. > > > > Hi Marco, > > I may understand your case now. At old USB port design, the VBUS is never > allowed to turned off to response the USB wakeup event. But the expected behavior > has changed after pm_qos_no_power_off is introduced for USB port, it is allowed > the port is powered off. > > PORTSC.PP could be controlled by USB core, but USB VBUS's power is not > controlled by this bit if the VBUS power enable pin is configured as GPIO function, > that is your case. > > CI_HDRC_TURN_VBUS_EARLY_ON is introduced by fixing a bug that some i.mx > USB controllers PHY's power is sourced from VBUS, the PHY's power need to be > on before touch some ehci registers, otherwise, the USB signal will be wrong at > some low speed devices use case. So, this flag can't be deleted, it may cause > regression. > > The solution I see is your may need to implement chipidea VBUS control flow by > considering pm_qos_no_power_off at suspend situation. You may add .suspend > API for ci_role_driver, and called by ci_controller_suspend/ci_controller_resume, of > cos, better solution is welcome. > Or you keep refcount for VBUS regulator on/off, if VBUS is already on, don't re-turn on again. Do not consider CI_HDRC_TURN_VBUS_EARLY_ON after probe, and let the USB core handle pm_qos_no_power_off, and call .ehci_ci_portpower accordingly. Peter
Hi Peter, On 20-02-27 13:30, Peter Chen wrote: > > > > > > > > > Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set. > > > > > > > > > > Do you have a VBUS regulator at your dts, and add it at controller's > > > node? See: arch/arm/boot/dts/imx6qdl-sabresd.dtsi as an example please? > > > > Yes, that's my use case too. > > > > > If you have set CI_HDRC_TURN_VBUS_EARLY_ON, the VBUS is controlled by > > > chipidea driver, not by USB core through PORTSC.PP (ehci_ci_portpower). > > > > I know, pls have a look my the patches. > > > > I had the problem that the vbus regulator wasn't turned off during a > > suspend/resume logic. The first issue within the usb-core should be fixed by [1] (v2 > > RFC is on the way). You never run in that case if you don't fix this. After I fixed it > > the port-power is called during suspend but obviously the regulator didn't get turned > > off because we don't add it to the priv->reg_vbus. > > > > This patchset should fix this and get rid of the CI_HDRC_TURN_VBUS_EARLY_ON > > flag. > > > > Hi Marco, > > I may understand your case now. At old USB port design, the VBUS is never allowed to > turned off to response the USB wakeup event. But the expected behavior has changed > after pm_qos_no_power_off is introduced for USB port, it is allowed the port is powered off. Luckily we have git :) and I my archeological findings are: 0ero Patch 2012-07-07) 1530280084c3 usb: chipidea: add imx platform driver 1st Patch 2012-10-23) ae0fb4b72c8d PM / QoS: Introduce PM QoS device flags support 2nd Patch 2013-01-23) ad493e5e5805 usb: add usb port auto power off mechanism 3th Patch 2014-10-13) 11a7e5940514 usb: ehci: add ehci_port_power interface 4th Patch 2014-10-13) c8679a2fb8de usb: chipidea: host: add portpower override 5th Patch 2015-02-11) 6adb9b7b5fb6 usb: chipidea: add a flag for turn on vbus early for host 6th Patch 2015-02-11) 659459174188 usb: chipidea: host: turn on vbus before add hcd if early vbus on is required A few more details: - Since day 0 the imx chipidea driver supports the regulator but it was only used to turn it on (0ero Patch). Later the regulator support was shifted to the core and optional. - 1-2 Patch added the pm_qos_no_power_off support - 3-4 Patch adds the support to disable the regulator - 5-6 Patch adding a workaround for patches 3-4 which breaks the regulator power-off support. So as you can see the pm_qos_no_power_off was introduced before the gpio regulator vbus power-off support was added. > PORTSC.PP could be controlled by USB core, but USB VBUS's power is not controlled > by this bit if the VBUS power enable pin is configured as GPIO function, that is your case. Yep I know :) > CI_HDRC_TURN_VBUS_EARLY_ON is introduced by fixing a bug that some i.mx USB > controllers PHY's power is sourced from VBUS, the PHY's power need to be on before > touch some ehci registers, otherwise, the USB signal will be wrong at some low speed > devices use case. So, this flag can't be deleted, it may cause regression. Pls check my archeological findings and again pls check my patches. I deleted the flag because isn't required anymore afterwards. > The solution I see is your may need to implement chipidea VBUS control flow by considering > pm_qos_no_power_off at suspend situation. You may add .suspend API for ci_role_driver, > and called by ci_controller_suspend/ci_controller_resume, of cos, better solution is welcome. I fixed it within the core [1] and here at the chipidea side. [1] https://lkml.org/lkml/2020/2/27/669 You will never enter the ehci_ci_portpower() during suspend without [1] if you are using a vanilla kernel. So IMHO this case can't be tested, sorry. Kind regards, Marco > Peter > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 20-02-27 15:39:14, Marco Felsch wrote: > Hi Peter, > > On 20-02-27 13:30, Peter Chen wrote: > > > > > > > > > > > > Note, I'm using a imx6q which has the CI_HDRC_TURN_VBUS_EARLY_ON set. > > > > > > > > > > > > > Do you have a VBUS regulator at your dts, and add it at controller's > > > > node? See: arch/arm/boot/dts/imx6qdl-sabresd.dtsi as an example please? > > > > > > Yes, that's my use case too. > > > > > > > If you have set CI_HDRC_TURN_VBUS_EARLY_ON, the VBUS is controlled by > > > > chipidea driver, not by USB core through PORTSC.PP (ehci_ci_portpower). > > > > > > I know, pls have a look my the patches. > > > > > > I had the problem that the vbus regulator wasn't turned off during a > > > suspend/resume logic. The first issue within the usb-core should be fixed by [1] (v2 > > > RFC is on the way). You never run in that case if you don't fix this. After I fixed it > > > the port-power is called during suspend but obviously the regulator didn't get turned > > > off because we don't add it to the priv->reg_vbus. > > > > > > This patchset should fix this and get rid of the CI_HDRC_TURN_VBUS_EARLY_ON > > > flag. > > > > > > > Hi Marco, > > > > I may understand your case now. At old USB port design, the VBUS is never allowed to > > turned off to response the USB wakeup event. But the expected behavior has changed > > after pm_qos_no_power_off is introduced for USB port, it is allowed the port is powered off. > > Luckily we have git :) and I my archeological findings are: > > 0ero Patch 2012-07-07) 1530280084c3 usb: chipidea: add imx platform driver > 1st Patch 2012-10-23) ae0fb4b72c8d PM / QoS: Introduce PM QoS device flags support > 2nd Patch 2013-01-23) ad493e5e5805 usb: add usb port auto power off mechanism > 3th Patch 2014-10-13) 11a7e5940514 usb: ehci: add ehci_port_power interface > 4th Patch 2014-10-13) c8679a2fb8de usb: chipidea: host: add portpower override > 5th Patch 2015-02-11) 6adb9b7b5fb6 usb: chipidea: add a flag for turn on vbus early for host > 6th Patch 2015-02-11) 659459174188 usb: chipidea: host: turn on vbus before add hcd if early vbus on is required > > A few more details: > - Since day 0 the imx chipidea driver supports the regulator but it was > only used to turn it on (0ero Patch). Later the regulator support was > shifted to the core and optional. > - 1-2 Patch added the pm_qos_no_power_off support > - 3-4 Patch adds the support to disable the regulator > - 5-6 Patch adding a workaround for patches 3-4 which breaks the > regulator power-off support. > Thanks for tracking it so detail. 1-2: pm_qos_no_power_off is default on, and it is a standard USB working way. In fact, no customer mentioned that before, and even for me, it is the first time to try it. 3-4: It is not adding support for gpio-base VBUS control for chipidea, it lets the USB core control the VBUS instead of controller driver. 5-6: With patch 3-4 introduced, we found issues at imx6 series SoCs (see detail at the last reply), so we add quirk for imx6. > So as you can see the pm_qos_no_power_off was introduced before the gpio > regulator vbus power-off support was added. > Like I said it is default off, we don't know there is such feature at kernel. In my mind, the VBUS is never off for host. > > PORTSC.PP could be controlled by USB core, but USB VBUS's power is not controlled > > by this bit if the VBUS power enable pin is configured as GPIO function, that is your case. > > Yep I know :) > > > CI_HDRC_TURN_VBUS_EARLY_ON is introduced by fixing a bug that some i.mx USB > > controllers PHY's power is sourced from VBUS, the PHY's power need to be on before > > touch some ehci registers, otherwise, the USB signal will be wrong at some low speed > > devices use case. So, this flag can't be deleted, it may cause regression. > > Pls check my archeological findings and again pls check my patches. I > deleted the flag because isn't required anymore afterwards. I have already checked your patch, your patch deletes CI_HDRC_TURN_VBUS_EARLY_ON quirk, and it may cause regression. > > > The solution I see is your may need to implement chipidea VBUS control flow by considering > > pm_qos_no_power_off at suspend situation. You may add .suspend API for ci_role_driver, > > and called by ci_controller_suspend/ci_controller_resume, of cos, better solution is welcome. > > I fixed it within the core [1] and here at the chipidea side. > > [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F2%2F27%2F669&data=02%7C01%7Cpeter.chen%40nxp.com%7Cad9b3833ae2f433d93ef08d7bb92d4a0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637184111614326500&sdata=SPwwBEGBco6IdP8ufmAnJeeRxuAXGLa0xzYlzFA%2FAvg%3D&reserved=0 > > You will never enter the ehci_ci_portpower() during suspend without [1] > if you are using a vanilla kernel. So IMHO this case can't be tested, > sorry. > Through set pm_qos_no_power_off as 0, the VBUS will be off. You never need to call .ehci_ci_portpower again. You may try my second suggestion for fix chipidea issue. I will reply your RFC patch for USB core.
Hi Peter, On 20-02-28 02:51, Peter Chen wrote: ... > > > CI_HDRC_TURN_VBUS_EARLY_ON is introduced by fixing a bug that some i.mx USB > > > controllers PHY's power is sourced from VBUS, the PHY's power need to be on before > > > touch some ehci registers, otherwise, the USB signal will be wrong at some low speed > > > devices use case. So, this flag can't be deleted, it may cause regression. > > > > Pls check my archeological findings and again pls check my patches. I > > deleted the flag because isn't required anymore afterwards. > > I have already checked your patch, your patch deletes CI_HDRC_TURN_VBUS_EARLY_ON > quirk, and it may cause regression. Arg, sorry now I see what you mean. Thanks for your explanation :) Since the 'struct ehci_ci_priv' contains now an enabled state we can git rid of the flag. To get it right, the writing the ehci PORT_POWER must be done before or after we enabled the VBUS? I'm asking because we can drop the 1st patch of this series. > > > The solution I see is your may need to implement chipidea VBUS control flow by considering > > > pm_qos_no_power_off at suspend situation. You may add .suspend API for ci_role_driver, > > > and called by ci_controller_suspend/ci_controller_resume, of cos, better solution is welcome. > > > > I fixed it within the core [1] and here at the chipidea side. > > > > [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2020%2F2%2F27%2F669&data=02%7C01%7Cpeter.chen%40nxp.com%7Cad9b3833ae2f433d93ef08d7bb92d4a0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637184111614326500&sdata=SPwwBEGBco6IdP8ufmAnJeeRxuAXGLa0xzYlzFA%2FAvg%3D&reserved=0 > > > > You will never enter the ehci_ci_portpower() during suspend without [1] > > if you are using a vanilla kernel. So IMHO this case can't be tested, > > sorry. > > > > Through set pm_qos_no_power_off as 0, the VBUS will be off. You > never need to call .ehci_ci_portpower again. You may try my second > suggestion for fix chipidea issue. I will reply your RFC patch for > USB core. Many thanks for testing =) Regards, Marco > -- > > Thanks, > Peter Chen
On 20-02-28 08:48:56, Marco Felsch wrote: > Hi Peter, > > On 20-02-28 02:51, Peter Chen wrote: > > ... > > > > > CI_HDRC_TURN_VBUS_EARLY_ON is introduced by fixing a bug that some i.mx USB > > > > controllers PHY's power is sourced from VBUS, the PHY's power need to be on before > > > > touch some ehci registers, otherwise, the USB signal will be wrong at some low speed > > > > devices use case. So, this flag can't be deleted, it may cause regression. > > > > > > Pls check my archeological findings and again pls check my patches. I > > > deleted the flag because isn't required anymore afterwards. > > > > I have already checked your patch, your patch deletes CI_HDRC_TURN_VBUS_EARLY_ON > > quirk, and it may cause regression. > > Arg, sorry now I see what you mean. Thanks for your explanation :) > Since the 'struct ehci_ci_priv' contains now an enabled state we can > git rid of the flag. To get it right, the writing the ehci PORT_POWER > must be done before or after we enabled the VBUS? I'm asking because > we can drop the 1st patch of this series. Both are OK.
Hi, the gpio-based port power is broken since commit [1,2]. I changed the core behaviour to to cleanup the code and avoid local workaround fixes. With this series and [3] I'am now able to disable the port completely after I disbaled the pm_qos_no_power_off flag. [1] 6adb9b7b5fb6 ("usb: chipidea: add a flag for turn on vbus early for host") [2] 659459174188 ("usb: chipidea: host: turn on vbus before add hcd if early vbus on is required") [3] https://lkml.org/lkml/2020/2/27/198 Regards, Marco Marco Felsch (3): USB: ehci-hub: let port_power() override the ehci_port_power() Partially Revert "usb: chipidea: host: turn on vbus before add hcd if early vbus on is required" Revert "usb: chipidea: add a flag for turn on vbus early for host" drivers/usb/chipidea/ci_hdrc_imx.c | 10 ++-------- drivers/usb/chipidea/host.c | 31 ++++++++++-------------------- drivers/usb/host/ehci-hub.c | 6 +++--- include/linux/usb/chipidea.h | 17 ++++++++-------- 4 files changed, 23 insertions(+), 41 deletions(-)