Support Runtime PM and host mode by Tegra ChipIdea USB driver
Subsystems (5)
v2 cover Support Runtime PM and host mode by Tegra ChipIdea USB driver
This series implements Runtime PM support for the Tegra ChipIdea USB driver. It also squashes the older ehci-tegra driver into the ChipIdea driver, hence the RPM is supported by both UDC and host controllers, secondly this opens opportunity for implementing OTG support in the future.
Patchset was tested on various Tegra20, Tegra30 and Tegra124 devices. Thanks to Peter Geis, Matt Merhar, Nicolas Chauvet and Ion Agorria for helping with the extensive and productive testing!
Changelog:
v2: - Improved comments in the code as it was suggested by Peter Chen and
Sergei Shtylyov for v1.
- Replaced mdelay() with fsleep() and made ci->hdc to reset to NULL in a error code path, like it was suggested by Peter Chen.
- Redirected deprecated USB_EHCI_TEGRA Kconfig entry to USB_CHIPIDEA_TEGRA as was suggested by Alan Stern.
- Improved commit message and added ACK from Thierry Reding to the patch that removes MODULE_ALIAS.
- Fixed UDC PHY waking up on ASUS TF201 tablet device by utilizing additional VBUS sensor. This was reported and tested by Ion Agorria.
- Added t-b from Ion Agorria.
Dmitry Osipenko (7):
usb: phy: tegra: Add delay after power up
usb: phy: tegra: Support waking up from a low power mode
usb: chipidea: tegra: Remove MODULE_ALIAS
usb: chipidea: tegra: Rename UDC to USB
usb: chipidea: tegra: Support runtime PM
usb: host: ehci-tegra: Remove the driver
ARM: tegra_defconfig: Enable USB_CHIPIDEA and remove USB_EHCI_TEGRA
Peter Geis (1):
usb: chipidea: tegra: Support host mode
arch/arm/configs/tegra_defconfig | 3 +- drivers/usb/chipidea/Kconfig | 3 +- drivers/usb/chipidea/ci_hdrc_tegra.c | 327 +++++++++++++-- drivers/usb/chipidea/core.c | 10 +- drivers/usb/chipidea/host.c | 104 ++++- drivers/usb/host/Kconfig | 8 +- drivers/usb/host/Makefile | 1 - drivers/usb/host/ehci-tegra.c | 604 --------------------------- drivers/usb/phy/phy-tegra-usb.c | 97 ++++- include/linux/usb/chipidea.h | 6 + include/linux/usb/tegra_usb_phy.h | 2 + 11 files changed, 503 insertions(+), 662 deletions(-) delete mode 100644 drivers/usb/host/ehci-tegra.c
Signature · 1 line
2.29.2
PATCH v2 1/8 usb: phy: tegra: Add delay after power up
The PHY hardware needs the delay of 2ms after power up, otherwise initial interrupt may be lost if USB controller is accessed before PHY is settled down. Previously this issue was masked by implicit delays, but now it pops up after squashing the older ehci-tegra driver into the ChipIdea driver.
--- drivers/usb/phy/phy-tegra-usb.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/usb/phy/phy-tegra-usb.c +3 -0
@@ -784,6 +784,9 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy) phy->powered_on = true; +/* Let PHY settle down */ +usleep_range (2000 ,2500 ); +return 0 ; }
Signature · 1 line
2.29.2
Re: [PATCH v2 1/8] usb: phy: tegra: Add delay after power up
Quoted 5 lines · Dmitry Osipenko
The PHY hardware needs the delay of 2ms after power up, otherwise initial interrupt may be lost if USB controller is accessed before PHY is settled down. Previously this issue was masked by implicit delays, but now it pops up after squashing the older ehci-tegra driver into the ChipIdea driver.
Quoted 8 lines
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/phy/phy-tegra-usb.c | 3 +++ 1 file changed, 3 insertions(+)
PATCH v2 2/8 usb: phy: tegra: Support waking up from a low power mode
In reply to Dmitry OsipenkoSupport programming of waking up from a low power mode by implementing the generic set_wakeup() callback of the USB PHY API.
--- drivers/usb/phy/phy-tegra-usb.c | 94 +++++++++++++++++++++++++++++-- include/linux/usb/tegra_usb_phy.h | 2 + 2 files changed, 90 insertions(+), 6 deletions(-)
drivers/usb/phy/phy-tegra-usb.c +88 -6
@@ -45,6 +45,7 @@#define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC)#define USB_SUSP_CTRL 0x400 +#define USB_WAKE_ON_RESUME_EN BIT(2)#define USB_WAKE_ON_CNNT_EN_DEV BIT(3)#define USB_WAKE_ON_DISCON_EN_DEV BIT(4)#define USB_SUSP_CLR BIT(5)
@@ -56,6 +57,15 @@#define USB_SUSP_SET BIT(14)#define USB_WAKEUP_DEBOUNCE_COUNT (x) (((x) & 0x7) << 16) +#define USB_PHY_VBUS_SENSORS 0x404 +#define B_SESS_VLD_WAKEUP_EN BIT(6) +#define B_VBUS_VLD_WAKEUP_EN BIT(14) +#define A_SESS_VLD_WAKEUP_EN BIT(22) +#define A_VBUS_VLD_WAKEUP_EN BIT(30) + +#define USB_PHY_VBUS_WAKEUP_ID 0x408 +#define VBUS_WAKEUP_WAKEUP_EN BIT(30) +#define USB1_LEGACY_CTRL 0x410#define USB1_NO_LEGACY_MODE BIT(0)#define USB1_VBUS_SENSE_CTL_MASK (3 << 1)
@@ -334,6 +344,11 @@ static int utmip_pad_power_on(struct tegra_usb_phy *phy)writel_relaxed (val, base +UTMIP_BIAS_CFG0 ); } +if (phy->pad_wakeup) { + phy->pad_wakeup = false; + utmip_pad_count--; + } +spin_unlock (&utmip_pad_lock);clk_disable_unprepare (phy->pad_clk);
@@ -359,6 +374,17 @@ static int utmip_pad_power_off(struct tegra_usb_phy *phy) goto ulock; } +/* +* In accordance to TRM, OTG and Bias pad circuits could be turned off +* to save power if wake is enabled, but the VBUS-change detection +* method is board-specific and these circuits may need to be enabled +* to generate wakeup event, hence we will just keep them both enabled. +*/ +if (phy->wakeup_enabled) { + phy->pad_wakeup = true; + utmip_pad_count++; + } +if (--utmip_pad_count ==0 ) { val =readl_relaxed (base +UTMIP_BIAS_CFG0 ); val |=UTMIP_OTGPD |UTMIP_BIASPD ;
@@ -503,11 +529,24 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)writel_relaxed (val, base +UTMIP_PLL_CFG1 ); } + val =readl_relaxed (base +USB_SUSP_CTRL ); + val &= ~USB_WAKE_ON_RESUME_EN ; +writel_relaxed (val, base +USB_SUSP_CTRL ); +if (phy->mode ==USB_DR_MODE_PERIPHERAL ) { val =readl_relaxed (base +USB_SUSP_CTRL ); val &= ~(USB_WAKE_ON_CNNT_EN_DEV |USB_WAKE_ON_DISCON_EN_DEV );writel_relaxed (val, base +USB_SUSP_CTRL ); + val =readl_relaxed (base +USB_PHY_VBUS_WAKEUP_ID ); + val &= ~VBUS_WAKEUP_WAKEUP_EN ; +writel_relaxed (val, base +USB_PHY_VBUS_WAKEUP_ID ); + + val =readl_relaxed (base +USB_PHY_VBUS_SENSORS ); + val &= ~(A_VBUS_VLD_WAKEUP_EN |A_SESS_VLD_WAKEUP_EN ); + val &= ~(B_VBUS_VLD_WAKEUP_EN |B_SESS_VLD_WAKEUP_EN ); +writel_relaxed (val, base +USB_PHY_VBUS_SENSORS ); + val =readl_relaxed (base +UTMIP_BAT_CHRG_CFG0 ); val &= ~UTMIP_PD_CHRG ;writel_relaxed (val, base +UTMIP_BAT_CHRG_CFG0 );
@@ -605,31 +644,55 @@ static int utmi_phy_power_off(struct tegra_usb_phy *phy)utmi_phy_clk_disable (phy); -if (phy->mode ==USB_DR_MODE_PERIPHERAL ) { - val =readl_relaxed (base +USB_SUSP_CTRL ); - val &= ~USB_WAKEUP_DEBOUNCE_COUNT (~0 ); - val |=USB_WAKE_ON_CNNT_EN_DEV |USB_WAKEUP_DEBOUNCE_COUNT (5 ); -writel_relaxed (val, base +USB_SUSP_CTRL ); - } +/* PHY won't resume if reset is asserted */ +if (phy->wakeup_enabled) + goto chrg_cfg0; val =readl_relaxed (base +USB_SUSP_CTRL ); val |=UTMIP_RESET ;writel_relaxed (val, base +USB_SUSP_CTRL ); +chrg_cfg0: val =readl_relaxed (base +UTMIP_BAT_CHRG_CFG0 ); val |=UTMIP_PD_CHRG ;writel_relaxed (val, base +UTMIP_BAT_CHRG_CFG0 ); +if (phy->wakeup_enabled) + goto xcvr_cfg1; + val =readl_relaxed (base +UTMIP_XCVR_CFG0 ); val |=UTMIP_FORCE_PD_POWERDOWN |UTMIP_FORCE_PD2_POWERDOWN |UTMIP_FORCE_PDZI_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG0 ); +xcvr_cfg1: val =readl_relaxed (base +UTMIP_XCVR_CFG1 ); val |=UTMIP_FORCE_PDDISC_POWERDOWN |UTMIP_FORCE_PDCHRP_POWERDOWN |UTMIP_FORCE_PDDR_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG1 ); +if (phy->wakeup_enabled) { + val =readl_relaxed (base +USB_SUSP_CTRL ); + val &= ~USB_WAKEUP_DEBOUNCE_COUNT (~0 ); + val |=USB_WAKEUP_DEBOUNCE_COUNT (5 ); + val |=USB_WAKE_ON_RESUME_EN ; +writel_relaxed (val, base +USB_SUSP_CTRL ); + +/* +* Ask VBUS sensor to generate wake event once cable is +* connected. +*/ +if (phy->mode ==USB_DR_MODE_PERIPHERAL ) { + val =readl_relaxed (base +USB_PHY_VBUS_WAKEUP_ID ); + val |=VBUS_WAKEUP_WAKEUP_EN ; +writel_relaxed (val, base +USB_PHY_VBUS_WAKEUP_ID ); + + val =readl_relaxed (base +USB_PHY_VBUS_SENSORS ); + val |=A_VBUS_VLD_WAKEUP_EN ; +writel_relaxed (val, base +USB_PHY_VBUS_SENSORS ); + } + } +return utmip_pad_power_off (phy); }
@@ -765,6 +828,15 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)usleep_range (5000 ,6000 );clk_disable_unprepare (phy->clk); +/* +* Wakeup currently unimplemented for ULPI, thus PHY needs to be +* force-resumed. +*/ +if (WARN_ON_ONCE (phy->wakeup_enabled)) { +ulpi_phy_power_on (phy); +return -EOPNOTSUPP ; + } +return 0 ; }
@@ -827,6 +899,15 @@ static void tegra_usb_phy_shutdown(struct usb_phy *u_phy) phy->freq =NULL ; } +static int tegra_usb_phy_set_wakeup (struct usb_phy *u_phy,bool enable) +{ +struct tegra_usb_phy *phy =to_tegra_usb_phy (u_phy); + + phy->wakeup_enabled = enable; + +return 0 ; +} +static int tegra_usb_phy_set_suspend (struct usb_phy *u_phy,int suspend) {struct tegra_usb_phy *phy =to_tegra_usb_phy (u_phy);
@@ -1198,6 +1279,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev) tegra_phy->u_phy.dev = &pdev->dev; tegra_phy->u_phy.init = tegra_usb_phy_init; tegra_phy->u_phy.shutdown = tegra_usb_phy_shutdown; + tegra_phy->u_phy.set_wakeup = tegra_usb_phy_set_wakeup; tegra_phy->u_phy.set_suspend = tegra_usb_phy_set_suspend;platform_set_drvdata (pdev, tegra_phy);
include/linux/usb/tegra_usb_phy.h +2 -0
@@ -79,6 +79,8 @@ struct tegra_usb_phy {bool is_ulpi_phy;struct gpio_desc *reset_gpio;struct reset_control *pad_rst; +bool wakeup_enabled; +bool pad_wakeup;bool powered_on; };
Signature · 1 line
2.29.2
Re: [PATCH v2 2/8] usb: phy: tegra: Support waking up from a low power mode
Quoted 105 lines · Dmitry Osipenko
Support programming of waking up from a low power mode by implementing the generic
set_wakeup()callback of the USB PHY API.Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/phy/phy-tegra-usb.c | 94 +++++++++++++++++++++++++++++-- include/linux/usb/tegra_usb_phy.h | 2 + 2 files changed, 90 insertions(+), 6 deletions(-)drivers/usb/phy/phy-tegra-usb.c +39
-5@@ -45,6 +45,7 @@#define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC)#define USB_SUSP_CTRL 0x400 +#define USB_WAKE_ON_RESUME_EN BIT(2)#define USB_WAKE_ON_CNNT_EN_DEV BIT(3)#define USB_WAKE_ON_DISCON_EN_DEV BIT(4)#define USB_SUSP_CLR BIT(5)@@ -56,6 +57,15 @@#define USB_SUSP_SET BIT(14)#define USB_WAKEUP_DEBOUNCE_COUNT (x) (((x) & 0x7) << 16) +#define USB_PHY_VBUS_SENSORS 0x404 +#define B_SESS_VLD_WAKEUP_EN BIT(6) +#define B_VBUS_VLD_WAKEUP_EN BIT(14) +#define A_SESS_VLD_WAKEUP_EN BIT(22) +#define A_VBUS_VLD_WAKEUP_EN BIT(30) + +#define USB_PHY_VBUS_WAKEUP_ID 0x408 +#define VBUS_WAKEUP_WAKEUP_EN BIT(30) +#define USB1_LEGACY_CTRL 0x410#define USB1_NO_LEGACY_MODE BIT(0)#define USB1_VBUS_SENSE_CTL_MASK (3 << 1)@@ -334,6 +344,11 @@ static int utmip_pad_power_on(struct tegra_usb_phy *phy)writel_relaxed (val, base +UTMIP_BIAS_CFG0 ); } +if (phy->pad_wakeup) { + phy->pad_wakeup = false; + utmip_pad_count--; + } +spin_unlock (&utmip_pad_lock);clk_disable_unprepare (phy->pad_clk);@@ -359,6 +374,17 @@ static int utmip_pad_power_off(struct tegra_usb_phy *phy) goto ulock; } +/* +* In accordance to TRM, OTG and Bias pad circuits could be turned off +* to save power if wake is enabled, but the VBUS-change detection +* method is board-specific and these circuits may need to be enabled +* to generate wakeup event, hence we will just keep them both enabled. +*/ +if (phy->wakeup_enabled) { + phy->pad_wakeup = true; + utmip_pad_count++; + } +if (--utmip_pad_count ==0 ) { val =readl_relaxed (base +UTMIP_BIAS_CFG0 ); val |=UTMIP_OTGPD |UTMIP_BIASPD ;@@ -503,11 +529,24 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)writel_relaxed (val, base +UTMIP_PLL_CFG1 ); } + val =readl_relaxed (base +USB_SUSP_CTRL ); + val &= ~USB_WAKE_ON_RESUME_EN ; +writel_relaxed (val, base +USB_SUSP_CTRL ); +if (phy->mode ==USB_DR_MODE_PERIPHERAL ) { val =readl_relaxed (base +USB_SUSP_CTRL ); val &= ~(USB_WAKE_ON_CNNT_EN_DEV |USB_WAKE_ON_DISCON_EN_DEV );writel_relaxed (val, base +USB_SUSP_CTRL ); + val =readl_relaxed (base +USB_PHY_VBUS_WAKEUP_ID ); + val &= ~VBUS_WAKEUP_WAKEUP_EN ; +writel_relaxed (val, base +USB_PHY_VBUS_WAKEUP_ID ); + + val =readl_relaxed (base +USB_PHY_VBUS_SENSORS ); + val &= ~(A_VBUS_VLD_WAKEUP_EN |A_SESS_VLD_WAKEUP_EN ); + val &= ~(B_VBUS_VLD_WAKEUP_EN |B_SESS_VLD_WAKEUP_EN ); +writel_relaxed (val, base +USB_PHY_VBUS_SENSORS ); + val =readl_relaxed (base +UTMIP_BAT_CHRG_CFG0 ); val &= ~UTMIP_PD_CHRG ;writel_relaxed (val, base +UTMIP_BAT_CHRG_CFG0 );@@ -605,31 +644,55 @@ static int utmi_phy_power_off(struct tegra_usb_phy *phy)utmi_phy_clk_disable (phy); -if (phy->mode ==USB_DR_MODE_PERIPHERAL ) { - val =readl_relaxed (base +USB_SUSP_CTRL ); - val &= ~USB_WAKEUP_DEBOUNCE_COUNT (~0 ); - val |=USB_WAKE_ON_CNNT_EN_DEV |USB_WAKEUP_DEBOUNCE_COUNT (5 ); -writel_relaxed (val, base +USB_SUSP_CTRL );
drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 655
Quoted 10 lines
drivers/usb/phy/phy-tegra-usb.c +4
-1- } +/* PHY won't resume if reset is asserted */ +if (phy->wakeup_enabled) + goto chrg_cfg0; val =readl_relaxed (base +USB_SUSP_CTRL ); val |=UTMIP_RESET ;writel_relaxed (val, base +USB_SUSP_CTRL ); +chrg_cfg0:
I found this diffcult to read until I realized that it was basically just the equivalent of this:
if (!phy->wakeup_enabled) {
val = readl_relaxed(base + USB_SUSP_CTRL);
val |= UTMIP_RESET;
writel_relaxed(val, base + USB_SUSP_CTRL);
}drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 668
Quoted 13 lines
drivers/usb/phy/phy-tegra-usb.c +4
-0val =readl_relaxed (base +UTMIP_BAT_CHRG_CFG0 ); val |=UTMIP_PD_CHRG ;writel_relaxed (val, base +UTMIP_BAT_CHRG_CFG0 ); +if (phy->wakeup_enabled) + goto xcvr_cfg1; + val =readl_relaxed (base +UTMIP_XCVR_CFG0 ); val |=UTMIP_FORCE_PD_POWERDOWN |UTMIP_FORCE_PD2_POWERDOWN |UTMIP_FORCE_PDZI_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG0 ); +xcvr_cfg1:
Similarly, I think this is more readable as:
if (!phy->wakeup_enabled) {
val = readl_relaxed(base + UTMIP_XCVR_CFG0);
val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN |
UTMIP_FORCE_PDZI_POWERDOWN;
writel_relaxed(val, base + UTMIP_XCVR_CFG0);
}drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 674
Quoted 6 lines
drivers/usb/phy/phy-tegra-usb.c +1
-0val =readl_relaxed (base +UTMIP_XCVR_CFG1 ); val |=UTMIP_FORCE_PDDISC_POWERDOWN |UTMIP_FORCE_PDCHRP_POWERDOWN |UTMIP_FORCE_PDDR_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG1 ); +if (phy->wakeup_enabled) {
Which then also matches the style of this conditional here.
Quoted 26 lines
drivers/usb/phy/phy-tegra-usb.c +21
-0+ val =readl_relaxed (base +USB_SUSP_CTRL ); + val &= ~USB_WAKEUP_DEBOUNCE_COUNT (~0 ); + val |=USB_WAKEUP_DEBOUNCE_COUNT (5 ); + val |=USB_WAKE_ON_RESUME_EN ; +writel_relaxed (val, base +USB_SUSP_CTRL ); + +/* +* Ask VBUS sensor to generate wake event once cable is +* connected. +*/ +if (phy->mode ==USB_DR_MODE_PERIPHERAL ) { + val =readl_relaxed (base +USB_PHY_VBUS_WAKEUP_ID ); + val |=VBUS_WAKEUP_WAKEUP_EN ; +writel_relaxed (val, base +USB_PHY_VBUS_WAKEUP_ID ); + + val =readl_relaxed (base +USB_PHY_VBUS_SENSORS ); + val |=A_VBUS_VLD_WAKEUP_EN ; +writel_relaxed (val, base +USB_PHY_VBUS_SENSORS ); + } + } +return utmip_pad_power_off (phy); }@@ -765,6 +828,15 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)usleep_range (5000 ,6000 );
drivers/usb/phy/phy-tegra-usb.c static int ulpi_phy_power_off(struct tegra_usb_phy *phy) · line 838
Quoted 10 lines
drivers/usb/phy/phy-tegra-usb.c +8
-0clk_disable_unprepare (phy->clk); +/* +* Wakeup currently unimplemented for ULPI, thus PHY needs to be +* force-resumed. +*/ +if (WARN_ON_ONCE (phy->wakeup_enabled)) { +ulpi_phy_power_on (phy); +return -EOPNOTSUPP ; + }
How do we control phy->wakeup_enabled? Is this something that we can set in device tree, for example? I hope so, because otherwise this would cause a nasty splat every time we try to power-off a ULPI PHY.
Thierry
Re: [PATCH v2 2/8] usb: phy: tegra: Support waking up from a low power mode
17.12.2020 16:33, Thierry Reding пишет:
Quoted 40 lines
drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 655
Quoted 9 lines
drivers/usb/phy/phy-tegra-usb.c +4
-0+/* PHY won't resume if reset is asserted */ +if (phy->wakeup_enabled) + goto chrg_cfg0; val =readl_relaxed (base +USB_SUSP_CTRL ); val |=UTMIP_RESET ;writel_relaxed (val, base +USB_SUSP_CTRL ); +chrg_cfg0:I found this diffcult to read until I realized that it was basically just the equivalent of this:
if (!phy->wakeup_enabled) { val = readl_relaxed(base + USB_SUSP_CTRL); val |= UTMIP_RESET; writel_relaxed(val, base + USB_SUSP_CTRL); }drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 668
Quoted 13 lines
drivers/usb/phy/phy-tegra-usb.c +4
-0val =readl_relaxed (base +UTMIP_BAT_CHRG_CFG0 ); val |=UTMIP_PD_CHRG ;writel_relaxed (val, base +UTMIP_BAT_CHRG_CFG0 ); +if (phy->wakeup_enabled) + goto xcvr_cfg1; + val =readl_relaxed (base +UTMIP_XCVR_CFG0 ); val |=UTMIP_FORCE_PD_POWERDOWN |UTMIP_FORCE_PD2_POWERDOWN |UTMIP_FORCE_PDZI_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG0 ); +xcvr_cfg1:Similarly, I think this is more readable as:
if (!phy->wakeup_enabled) { val = readl_relaxed(base + UTMIP_XCVR_CFG0); val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN | UTMIP_FORCE_PDZI_POWERDOWN; writel_relaxed(val, base + UTMIP_XCVR_CFG0); }
Quoted 7 lines
drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 674
Quoted 6 lines
drivers/usb/phy/phy-tegra-usb.c +1
-0val =readl_relaxed (base +UTMIP_XCVR_CFG1 ); val |=UTMIP_FORCE_PDDISC_POWERDOWN |UTMIP_FORCE_PDCHRP_POWERDOWN |UTMIP_FORCE_PDDR_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG1 ); +if (phy->wakeup_enabled) {Which then also matches the style of this conditional here.
I'll change it in v3, thanks.
Thierry Reding
Acked-byRe: [PATCH v2 2/8] usb: phy: tegra: Support waking up from a low power mode
Quoted 46 lines · Dmitry Osipenko
17.12.2020 16:33, Thierry Reding пишет:
Quoted 45 lines
drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 655
Quoted 9 lines
drivers/usb/phy/phy-tegra-usb.c +4
-0+/* PHY won't resume if reset is asserted */ +if (phy->wakeup_enabled) + goto chrg_cfg0; val =readl_relaxed (base +USB_SUSP_CTRL ); val |=UTMIP_RESET ;writel_relaxed (val, base +USB_SUSP_CTRL ); +chrg_cfg0:I found this diffcult to read until I realized that it was basically just the equivalent of this:
if (!phy->wakeup_enabled) { val = readl_relaxed(base + USB_SUSP_CTRL); val |= UTMIP_RESET; writel_relaxed(val, base + USB_SUSP_CTRL); }drivers/usb/phy/phy-tegra-usb.c static int utmi_phy_power_off(struct tegra_usb_phy *phy) · line 668
Quoted 13 lines
drivers/usb/phy/phy-tegra-usb.c +4
-0val =readl_relaxed (base +UTMIP_BAT_CHRG_CFG0 ); val |=UTMIP_PD_CHRG ;writel_relaxed (val, base +UTMIP_BAT_CHRG_CFG0 ); +if (phy->wakeup_enabled) + goto xcvr_cfg1; + val =readl_relaxed (base +UTMIP_XCVR_CFG0 ); val |=UTMIP_FORCE_PD_POWERDOWN |UTMIP_FORCE_PD2_POWERDOWN |UTMIP_FORCE_PDZI_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG0 ); +xcvr_cfg1:Similarly, I think this is more readable as:
if (!phy->wakeup_enabled) { val = readl_relaxed(base + UTMIP_XCVR_CFG0); val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN | UTMIP_FORCE_PDZI_POWERDOWN; writel_relaxed(val, base + UTMIP_XCVR_CFG0); }Quoted 5 lines
drivers/usb/phy/phy-tegra-usb.c +0
-0val =readl_relaxed (base +UTMIP_XCVR_CFG1 ); val |=UTMIP_FORCE_PDDISC_POWERDOWN |UTMIP_FORCE_PDCHRP_POWERDOWN |UTMIP_FORCE_PDDR_POWERDOWN ;writel_relaxed (val, base +UTMIP_XCVR_CFG1 );
Quoted 4 lines
Quoted 2 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ if (phy->wakeup_enabled) {Which then also matches the style of this conditional here.
I'll change it in v3, thanks.
Given that my other comment about the WARN_ONCE seems to have been resolved, with the above gotos replaced by conditionals as I suggested:
PATCH v2 3/8 usb: chipidea: tegra: Remove MODULE_ALIAS
In reply to Dmitry OsipenkoThe OF core adds an alias based on the OF device ID table, which is enough to have the driver autoloaded. The legacy MODULE_ALIAS macro was relevant to a pre-OF board files which manually created platform devices, this is irrelevant to the modern ARM kernels since devices are created by the OF core. Remove the unnecessary macro in order to keep the driver's code cleaner.
--- drivers/usb/chipidea/ci_hdrc_tegra.c | 1 - 1 file changed, 1 deletion(-)
drivers/usb/chipidea/ci_hdrc_tegra.c +0 -1
@@ -128,5 +128,4 @@ module_platform_driver(tegra_udc_driver);MODULE_DESCRIPTION ("NVIDIA Tegra USB device mode driver" );MODULE_AUTHOR ("Thierry Reding <[email protected]>" ); -MODULE_ALIAS ("platform:tegra-udc" );MODULE_LICENSE ("GPL v2" );
Signature · 1 line
2.29.2
PATCH v2 4/8 usb: chipidea: tegra: Rename UDC to USB
In reply to Dmitry OsipenkoRename all occurrences in the code from "udc" to "usb" and change the Kconfig entry in order to show that this driver supports USB modes other than device-only mode. The follow up patch will add host-mode support and it will be cleaner to perform the renaming separately, i.e. in this patch.
--- drivers/usb/chipidea/Kconfig | 2 +- drivers/usb/chipidea/ci_hdrc_tegra.c | 78 ++++++++++++++-------------- 2 files changed, 40 insertions(+), 40 deletions(-)
drivers/usb/chipidea/Kconfig +1 -1
@@ -53,7 +53,7 @@ config USB_CHIPIDEA_GENERIC default USB_CHIPIDEA config USB_CHIPIDEA_TEGRA - tristate "Enable Tegra UDC glue driver" if EMBEDDED + tristate "Enable Tegra USB glue driver" if EMBEDDED depends on OF depends on USB_CHIPIDEA_UDC default USB_CHIPIDEA
drivers/usb/chipidea/ci_hdrc_tegra.c +39 -39
146 lines not on this page · raw message
Signature · 1 line
2.29.2
Re: [PATCH v2 4/8] usb: chipidea: tegra: Rename UDC to USB
Quoted 49 lines · Dmitry Osipenko
Rename all occurrences in the code from "udc" to "usb" and change the
Kconfigentry in order to show that this driver supports USB modes other than device-only mode. The follow up patch will add host-mode support and it will be cleaner to perform the renaming separately, i.e. in this patch.Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/Kconfig | 2 +- drivers/usb/chipidea/ci_hdrc_tegra.c | 78 ++++++++++++++-------------- 2 files changed, 40 insertions(+), 40 deletions(-)drivers/usb/chipidea/Kconfig +1
-1@@ -53,7 +53,7 @@ config USB_CHIPIDEA_GENERIC default USB_CHIPIDEA config USB_CHIPIDEA_TEGRA - tristate "Enable Tegra UDC glue driver" if EMBEDDED + tristate "Enable Tegra USB glue driver" if EMBEDDED depends on OF depends on USB_CHIPIDEA_UDC default USB_CHIPIDEAdrivers/usb/chipidea/ci_hdrc_tegra.c +2
-2@@ -12,7 +12,7 @@#include "ci.h" -struct tegra_udc { +struct tegra_usb {struct ci_hdrc_platform_data data;struct platform_device *dev;@@ -20,15 +20,15 @@ struct tegra_udc {struct clk *clk; }; -struct tegra_udc_soc_info { +struct tegra_usb_soc_info {unsigned long flags; };
drivers/usb/chipidea/ci_hdrc_tegra.c struct tegra_udc { · line 33
Quoted 10 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +2
-2-static const struct tegra_udc_soc_info tegra_udc_soc_info = { +static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA , }; -static const struct of_device_id tegra_udc_of_match[] = { +static const struct of_device_id tegra_usb_of_match[] = { { .compatible ="nvidia,tegra20-udc" ,
Do we perhaps also want to add a new tegra20-usb compatible string here and deprecate the old one since this now no longer properly describes the device.
In either case, this looks fine:
Re: [PATCH v2 4/8] usb: chipidea: tegra: Rename UDC to USB
17.12.2020 16:36, Thierry Reding пишет:
Quoted 56 lines · Dmitry Osipenko
Quoted 55 lines
Rename all occurrences in the code from "udc" to "usb" and change the
Kconfigentry in order to show that this driver supports USB modes other than device-only mode. The follow up patch will add host-mode support and it will be cleaner to perform the renaming separately, i.e. in this patch.Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/Kconfig | 2 +- drivers/usb/chipidea/ci_hdrc_tegra.c | 78 ++++++++++++++-------------- 2 files changed, 40 insertions(+), 40 deletions(-)drivers/usb/chipidea/Kconfig +1
-1@@ -53,7 +53,7 @@ config USB_CHIPIDEA_GENERIC default USB_CHIPIDEA config USB_CHIPIDEA_TEGRA - tristate "Enable Tegra UDC glue driver" if EMBEDDED + tristate "Enable Tegra USB glue driver" if EMBEDDED depends on OF depends on USB_CHIPIDEA_UDC default USB_CHIPIDEAdrivers/usb/chipidea/ci_hdrc_tegra.c +3
-3@@ -12,7 +12,7 @@#include "ci.h" -struct tegra_udc { +struct tegra_usb {struct ci_hdrc_platform_data data;struct platform_device *dev;@@ -20,15 +20,15 @@ struct tegra_udc {struct clk *clk; }; -struct tegra_udc_soc_info { +struct tegra_usb_soc_info {unsigned long flags; }; -static const struct tegra_udc_soc_info tegra_udc_soc_info = { +static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA , };
Quoted 8 lines
drivers/usb/chipidea/ci_hdrc_tegra.c struct tegra_udc { · line 33
Quoted 4 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +1
-1-static const struct of_device_id tegra_udc_of_match[] = { +static const struct of_device_id tegra_usb_of_match[] = { { .compatible ="nvidia,tegra20-udc" ,Do we perhaps also want to add a new tegra20-usb compatible string here and deprecate the old one since this now no longer properly describes the device.
Ideally it should have been "tegra20-otg" to match TRM, but UDC is also okay since it's a part of OTG and kinda presumes the OTG support of USB1 controller for anyone who read the TRM. Hence there is no need to change the compatible, IMO.
Quoted 4 lines
In either case, this looks fine:
Acked-by Thierry Reding <[email protected]>
thanks
PATCH v2 5/8 usb: chipidea: tegra: Support host mode
In reply to Dmitry OsipenkoFrom: Peter Geis <[email protected]>
Add USB host mode to the Tegra HDRC driver. This allows us to benefit from support provided by the generic ChipIdea driver instead of duplicating the effort in a separate ehci-tegra driver.
--- drivers/usb/chipidea/Kconfig | 1 - drivers/usb/chipidea/ci_hdrc_tegra.c | 243 ++++++++++++++++++++++++++- drivers/usb/chipidea/core.c | 10 +- drivers/usb/chipidea/host.c | 104 +++++++++++- include/linux/usb/chipidea.h | 6 + 5 files changed, 356 insertions(+), 8 deletions(-)
drivers/usb/chipidea/Kconfig +0 -1
@@ -55,7 +55,6 @@ config USB_CHIPIDEA_GENERIC config USB_CHIPIDEA_TEGRA tristate "Enable Tegra USB glue driver" if EMBEDDED depends on OF - depends on USB_CHIPIDEA_UDC default USB_CHIPIDEA endif
drivers/usb/chipidea/ci_hdrc_tegra.c +239 -4
306 lines not on this page · raw message
drivers/usb/chipidea/core.c +9 -1
@@ -195,7 +195,7 @@ static void hw_wait_phy_stable(void) }/* The PHY enters/leaves low power mode */ -static void ci_hdrc_enter_lpm (struct ci_hdrc *ci,bool enable) +static void ci_hdrc_enter_lpm_common (struct ci_hdrc *ci,bool enable) {enum ci_hw_regs reg = ci->hw_bank.lpm ?OP_DEVLC :OP_PORTSC ;bool lpm = !!(hw_read (ci, reg,PORTSC_PHCD (ci->hw_bank.lpm)));
@@ -208,6 +208,11 @@ static void ci_hdrc_enter_lpm(struct ci_hdrc *ci, bool enable) 0); } +static void ci_hdrc_enter_lpm (struct ci_hdrc *ci,bool enable) +{ +return ci->platdata->enter_lpm (ci, enable); +} +static int hw_device_init (struct ci_hdrc *ci,void __iomem *base) {u32 reg;
@@ -790,6 +795,9 @@ static int ci_get_platdata(struct device *dev, platdata->pins_device = p; } +if (!platdata->enter_lpm) + platdata->enter_lpm = ci_hdrc_enter_lpm_common; +return 0 ; }
drivers/usb/chipidea/host.c +102 -2
146 lines not on this page · raw message
include/linux/usb/chipidea.h +6 -0
@@ -88,6 +88,12 @@ struct ci_hdrc_platform_data {struct pinctrl_state *pins_default;struct pinctrl_state *pins_host;struct pinctrl_state *pins_device; + +/* platform-specific hooks */ +int (*hub_control)(struct ci_hdrc *ci,u16 typeReq,u16 wValue, +u16 wIndex,char *buf,u16 wLength, +bool *done,unsigned long *flags); +void (*enter_lpm)(struct ci_hdrc *ci,bool enable); };/* Default offset of capability registers */
Signature · 1 line
2.29.2
Re: [PATCH v2 5/8] usb: chipidea: tegra: Support host mode
Quoted 11 lines · Dmitry Osipenko
From: Peter Geis <[email protected]>
Add USB host mode to the Tegra HDRC driver. This allows us to benefit from support provided by the generic ChipIdea driver instead of duplicating the effort in a separate ehci-tegra driver.
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Peter Geis <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>
Quoted 7 lines
--- drivers/usb/chipidea/Kconfig | 1 - drivers/usb/chipidea/ci_hdrc_tegra.c | 243 ++++++++++++++++++++++++++- drivers/usb/chipidea/core.c | 10 +- drivers/usb/chipidea/host.c | 104 +++++++++++- include/linux/usb/chipidea.h | 6 + 5 files changed, 356 insertions(+), 8 deletions(-)
Looks good:
PATCH v2 6/8 usb: chipidea: tegra: Support runtime PM
In reply to Dmitry OsipenkoTegra PHY driver now supports waking up controller from a low power mode. Enable runtime PM in order to put controller into the LPM during idle.
--- drivers/usb/chipidea/ci_hdrc_tegra.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
drivers/usb/chipidea/ci_hdrc_tegra.c +10 -3
@@ -38,21 +38,24 @@ struct tegra_usb_soc_info {static const struct tegra_usb_soc_info tegra20_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =10 , };static const struct tegra_usb_soc_info tegra30_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =16 , };static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_UNKNOWN , };
@@ -323,6 +326,10 @@ static int tegra_usb_probe(struct platform_device *pdev) usb->data.hub_control = tegra_ehci_hub_control; usb->data.notify_event = tegra_usb_notify_event; +/* Tegra PHY driver currently doesn't support LPM for ULPI */ +if (of_usb_get_phy_mode (pdev->dev.of_node) ==USBPHY_INTERFACE_MODE_ULPI ) + usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM ; + usb->dev =ci_hdrc_add_device (&pdev->dev, pdev->resource, pdev->num_resources, &usb->data);if (IS_ERR (usb->dev)) {
Signature · 1 line
2.29.2
Re: [PATCH v2 6/8] usb: chipidea: tegra: Support runtime PM
Quoted 45 lines · Dmitry Osipenko
Tegra PHY driver now supports waking up controller from a low power mode. Enable runtime PM in order to put controller into the LPM during idle.
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/ci_hdrc_tegra.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)drivers/usb/chipidea/ci_hdrc_tegra.c +6
-3@@ -38,21 +38,24 @@ struct tegra_usb_soc_info {static const struct tegra_usb_soc_info tegra20_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =10 , };static const struct tegra_usb_soc_info tegra30_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =16 , };static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_UNKNOWN , };
drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_usb_probe(struct platform_device *pdev) · line 332
Quoted 7 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +4
-0@@ -323,6 +326,10 @@ static int tegra_usb_probe(struct platform_device *pdev) usb->data.hub_control = tegra_ehci_hub_control; usb->data.notify_event = tegra_usb_notify_event; +/* Tegra PHY driver currently doesn't support LPM for ULPI */ +if (of_usb_get_phy_mode (pdev->dev.of_node) ==USBPHY_INTERFACE_MODE_ULPI ) + usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM ; +
Does this prevent the wakeup_enabled from being set for ULPI PHYs?
Thierry
Re: [PATCH v2 6/8] usb: chipidea: tegra: Support runtime PM
17.12.2020 16:41, Thierry Reding пишет:
Quoted 49 lines · Dmitry Osipenko
Quoted 48 lines
Tegra PHY driver now supports waking up controller from a low power mode. Enable runtime PM in order to put controller into the LPM during idle.
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/ci_hdrc_tegra.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)drivers/usb/chipidea/ci_hdrc_tegra.c +6
-3@@ -38,21 +38,24 @@ struct tegra_usb_soc_info {static const struct tegra_usb_soc_info tegra20_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =10 , };static const struct tegra_usb_soc_info tegra30_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =16 , };static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_UNKNOWN , };@@ -323,6 +326,10 @@ static int tegra_usb_probe(struct platform_device *pdev) usb->data.hub_control = tegra_ehci_hub_control; usb->data.notify_event = tegra_usb_notify_event;
Quoted 6 lines
drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_usb_probe(struct platform_device *pdev) · line 332
Quoted 4 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +4
-0+/* Tegra PHY driver currently doesn't support LPM for ULPI */ +if (of_usb_get_phy_mode (pdev->dev.of_node) ==USBPHY_INTERFACE_MODE_ULPI ) + usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM ; +Does this prevent the
wakeup_enabledfrom being set for ULPI PHYs?
Yes
Re: [PATCH v2 6/8] usb: chipidea: tegra: Support runtime PM
Quoted 50 lines · Dmitry Osipenko
17.12.2020 16:41, Thierry Reding пишет:
Quoted 49 lines · Dmitry Osipenko
Quoted 48 lines
Tegra PHY driver now supports waking up controller from a low power mode. Enable runtime PM in order to put controller into the LPM during idle.
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/ci_hdrc_tegra.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)drivers/usb/chipidea/ci_hdrc_tegra.c +6
-3@@ -38,21 +38,24 @@ struct tegra_usb_soc_info {static const struct tegra_usb_soc_info tegra20_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =10 , };static const struct tegra_usb_soc_info tegra30_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =16 , };static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_UNKNOWN , };@@ -323,6 +326,10 @@ static int tegra_usb_probe(struct platform_device *pdev) usb->data.hub_control = tegra_ehci_hub_control; usb->data.notify_event = tegra_usb_notify_event;
Quoted 8 lines
Quoted 6 lines
drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_usb_probe(struct platform_device *pdev) · line 332
Quoted 4 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +4
-0+/* Tegra PHY driver currently doesn't support LPM for ULPI */ +if (of_usb_get_phy_mode (pdev->dev.of_node) ==USBPHY_INTERFACE_MODE_ULPI ) + usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM ; +Does this prevent the
wakeup_enabledfrom being set for ULPI PHYs?Yes
Okay, it should be fine then to keep that WARN_ONCE in that prior patch since we should really only get there if there's something seriously wrong.
Thierry
Re: [PATCH v2 6/8] usb: chipidea: tegra: Support runtime PM
17.12.2020 18:02, Thierry Reding пишет:
Quoted 58 lines · Dmitry Osipenko
Quoted 57 lines
17.12.2020 16:41, Thierry Reding пишет:
Quoted 55 lines · Dmitry Osipenko
Quoted 45 lines
Tegra PHY driver now supports waking up controller from a low power mode. Enable runtime PM in order to put controller into the LPM during idle.
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/ci_hdrc_tegra.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)drivers/usb/chipidea/ci_hdrc_tegra.c +6
-3@@ -38,21 +38,24 @@ struct tegra_usb_soc_info {static const struct tegra_usb_soc_info tegra20_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =10 , };static const struct tegra_usb_soc_info tegra30_ehci_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_HOST , .txfifothresh =16 , };static const struct tegra_usb_soc_info tegra_udc_soc_info = { .flags =CI_HDRC_REQUIRES_ALIGNED_DMA | -CI_HDRC_OVERRIDE_PHY_CONTROL , +CI_HDRC_OVERRIDE_PHY_CONTROL | +CI_HDRC_SUPPORTS_RUNTIME_PM , .dr_mode =USB_DR_MODE_UNKNOWN , };drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_usb_probe(struct platform_device *pdev) · line 332
Quoted 7 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +4
-0@@ -323,6 +326,10 @@ static int tegra_usb_probe(struct platform_device *pdev) usb->data.hub_control = tegra_ehci_hub_control; usb->data.notify_event = tegra_usb_notify_event; +/* Tegra PHY driver currently doesn't support LPM for ULPI */ +if (of_usb_get_phy_mode (pdev->dev.of_node) ==USBPHY_INTERFACE_MODE_ULPI ) + usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM ; +Does this prevent the
wakeup_enabledfrom being set for ULPI PHYs?
Quoted 5 lines
Quoted 1 line
Yes
Okay, it should be fine then to keep that
WARN_ONCEin that prior patch since we should really only get there if there's something seriously wrong.
Correct, that is the actual intention of the warning.
Re: [PATCH v2 6/8] usb: chipidea: tegra: Support runtime PM
In reply to Dmitry OsipenkoQuoted 11 lines · Dmitry Osipenko
Tegra PHY driver now supports waking up controller from a low power mode. Enable runtime PM in order to put controller into the LPM during idle.
Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/chipidea/ci_hdrc_tegra.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
PATCH v2 7/8 usb: host: ehci-tegra: Remove the driver
In reply to Dmitry OsipenkoThe ChipIdea driver now provides USB2 host mode support for NVIDIA Tegra SoCs. The ehci-tegra driver is obsolete now, remove it and redirect the older Kconfig entry to the CI driver.
--- drivers/usb/host/Kconfig | 8 +- drivers/usb/host/Makefile | 1 - drivers/usb/host/ehci-tegra.c | 604 ---------------------------------- 3 files changed, 6 insertions(+), 607 deletions(-) delete mode 100644 drivers/usb/host/ehci-tegra.c
drivers/usb/host/Kconfig +6 -2
@@ -269,9 +269,13 @@ config USB_EHCI_HCD_AT91 config USB_EHCI_TEGRA tristate "NVIDIA Tegra HCD support" depends on ARCH_TEGRA - select USB_EHCI_ROOT_HUB_TT - select USB_TEGRA_PHY + select USB_CHIPIDEA + select USB_CHIPIDEA_HOST + select USB_CHIPIDEA_TEGRA help + This option is deprecated now and the driver was removed, use + USB_CHIPIDEA_TEGRA instead. + This driver enables support for the internal USB Host Controllers found in NVIDIA Tegra SoCs. The controllers are EHCI compliant.
drivers/usb/host/Makefile +0 -1
@@ -47,7 +47,6 @@ obj-$(CONFIG_USB_EHCI_HCD_SPEAR) += ehci-spear.o obj-$(CONFIG_USB_EHCI_HCD_STI) += ehci-st.o obj-$(CONFIG_USB_EHCI_EXYNOS) += ehci-exynos.o obj-$(CONFIG_USB_EHCI_HCD_AT91) += ehci-atmel.o -obj-$(CONFIG_USB_EHCI_TEGRA) += ehci-tegra.o obj-$(CONFIG_USB_EHCI_BRCMSTB) += ehci-brcm.o obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
drivers/usb/host/ehci-tegra.c deleted +0 -604
604 lines not on this page · raw message
Signature · 1 line
2.29.2
Re: [PATCH v2 7/8] usb: host: ehci-tegra: Remove the driver
Quoted 9 lines · Dmitry Osipenko
The ChipIdea driver now provides USB2 host mode support for NVIDIA Tegra SoCs. The ehci-tegra driver is obsolete now, remove it and redirect the older
Kconfigentry to the CI driver.Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>
Quoted 6 lines
--- drivers/usb/host/Kconfig | 8 +- drivers/usb/host/Makefile | 1 - drivers/usb/host/ehci-tegra.c | 604 ---------------------------------- 3 files changed, 6 insertions(+), 607 deletions(-) delete mode 100644 drivers/usb/host/ehci-tegra.c
Re: [PATCH v2 7/8] usb: host: ehci-tegra: Remove the driver
In reply to Dmitry OsipenkoQuoted 25 lines · Dmitry Osipenko
The ChipIdea driver now provides USB2 host mode support for NVIDIA Tegra SoCs. The ehci-tegra driver is obsolete now, remove it and redirect the older
Kconfigentry to the CI driver.Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/host/Kconfig | 8 +- drivers/usb/host/Makefile | 1 - drivers/usb/host/ehci-tegra.c | 604 ---------------------------------- 3 files changed, 6 insertions(+), 607 deletions(-) delete mode 100644 drivers/usb/host/ehci-tegra.cdrivers/usb/host/Kconfig +0
-1@@ -269,9 +269,13 @@ config USB_EHCI_HCD_AT91 config USB_EHCI_TEGRA tristate "NVIDIA Tegra HCD support" depends on ARCH_TEGRA - select USB_EHCI_ROOT_HUB_TT
drivers/usb/host/Kconfig config USB_EHCI_HCD_AT91 · line 280
Quoted 10 lines
drivers/usb/host/Kconfig +6
-1- select USB_TEGRA_PHY + select USB_CHIPIDEA + select USB_CHIPIDEA_HOST + select USB_CHIPIDEA_TEGRA help + This option is deprecated now and the driver was removed, use + USB_CHIPIDEA_TEGRA instead. + This driver enables support for the internal USB Host Controllers found in NVIDIA Tegra SoCs. The controllers are EHCI compliant.
It doesn't really make sense to say "... the driver was removed..." and then in the next paragraph say "This driver enables...". You should change the second paragraph to begin: "Enable support for...".
That's a minor matter, though, and you can easily fix it in the next patch version. Everything else is okay.
Alan Stern
Re: [PATCH v2 7/8] usb: host: ehci-tegra: Remove the driver
17.12.2020 18:50, Alan Stern пишет:
Quoted 41 lines · Dmitry Osipenko
Quoted 25 lines
The ChipIdea driver now provides USB2 host mode support for NVIDIA Tegra SoCs. The ehci-tegra driver is obsolete now, remove it and redirect the older
Kconfigentry to the CI driver.Tested-by Matt Merhar <[email protected]>Tested-by Nicolas Chauvet <[email protected]>Tested-by Peter Geis <[email protected]>Tested-by Ion Agorria <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/usb/host/Kconfig | 8 +- drivers/usb/host/Makefile | 1 - drivers/usb/host/ehci-tegra.c | 604 ---------------------------------- 3 files changed, 6 insertions(+), 607 deletions(-) delete mode 100644 drivers/usb/host/ehci-tegra.cdrivers/usb/host/Kconfig +0
-1@@ -269,9 +269,13 @@ config USB_EHCI_HCD_AT91 config USB_EHCI_TEGRA tristate "NVIDIA Tegra HCD support" depends on ARCH_TEGRA - select USB_EHCI_ROOT_HUB_TTdrivers/usb/host/Kconfig config USB_EHCI_HCD_AT91 · line 280
Quoted 10 lines
drivers/usb/host/Kconfig +6
-1- select USB_TEGRA_PHY + select USB_CHIPIDEA + select USB_CHIPIDEA_HOST + select USB_CHIPIDEA_TEGRA help + This option is deprecated now and the driver was removed, use + USB_CHIPIDEA_TEGRA instead. + This driver enables support for the internal USB Host Controllers found in NVIDIA Tegra SoCs. The controllers are EHCI compliant.It doesn't really make sense to say "... the driver was removed..." and then in the next paragraph say "This driver enables...". You should change the second paragraph to begin: "Enable support for...".
Quoted 4 lines
That's a minor matter, though, and you can easily fix it in the next patch version. Everything else is okay.
Acked-by Alan Stern <[email protected]>
Okay, thanks.
PATCH v2 8/8 ARM: tegra_defconfig: Enable USB_CHIPIDEA and remove USB_EHCI_TEGRA
In reply to Dmitry OsipenkoThe ehci-tegra driver was superseded by the generic ChipIdea USB driver, update the tegra's defconfig accordingly.
--- arch/arm/configs/tegra_defconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm/configs/tegra_defconfig +2 -1
@@ -237,12 +237,13 @@ CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_TEGRA=y CONFIG_USB_EHCI_HCD=y -CONFIG_USB_EHCI_TEGRA=y CONFIG_USB_ACM=y CONFIG_USB_WDM=y CONFIG_USB_STORAGE=y CONFIG_USB_CHIPIDEA=y CONFIG_USB_CHIPIDEA_UDC=y +CONFIG_USB_CHIPIDEA_HOST=y +CONFIG_USB_CHIPIDEA_TEGRA=y CONFIG_USB_GADGET=y CONFIG_MMC=y CONFIG_MMC_BLOCK_MINORS=16
Signature · 1 line
2.29.2
Re: [PATCH v2 8/8] ARM: tegra_defconfig: Enable USB_CHIPIDEA and remove USB_EHCI_TEGRA
Quoted 7 lines · Dmitry Osipenko
The ehci-tegra driver was superseded by the generic ChipIdea USB driver, update the tegra's defconfig accordingly.
Signed-off-by Dmitry Osipenko <[email protected]>--- arch/arm/configs/tegra_defconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Greg, Peter, it might make sense for this to go through the USB tree along with the rest of the series to make sure it goes in at the right time. This file doesn't see many changes, so I expect there to be no conflicts with the ARM SoC tree.
If you'd rather not pick this up, I'll just wait until the rest are applied and take this through the Tegra tree.
Thierry