| Message ID | 20200909081041.3190157-1-jckuo@nvidia.com |
|---|---|
| Headers | show |
| Series | Tegra XHCI controller ELPG support | expand |
On Wed, Sep 09, 2020 at 04:10:27PM +0800, JC Kuo wrote: > PLLE has a hardware power sequencer logic which is a state machine > that can power on/off PLLE without any software intervention. The > sequencer has two inputs, one from XUSB UPHY PLL and the other from > SATA UPHY PLL. PLLE provides reference clock to XUSB and SATA UPHY > PLLs. When both of the downstream PLLs are powered-off, PLLE hardware > power sequencer will automatically power off PLLE for power saving. > > XUSB and SATA UPHY PLLs also have their own hardware power sequencer > logic. XUSB UPHY PLL is shared between XUSB SuperSpeed ports and PCIE > controllers. The XUSB UPHY PLL hardware power sequencer has inputs > from XUSB and PCIE. When all of the XUSB SuperSpeed ports and PCIE > controllers are in low power state, XUSB UPHY PLL hardware power > sequencer automatically power off PLL and flags idle to PLLE hardware > power sequencer. Similar applies to SATA UPHY PLL. > > PLLE hardware power sequencer has to be enabled after both downstream > sequencers are enabled. > > This commit adds two helper functions: > 1. tegra210_plle_hw_sequence_start() for XUSB PADCTL driver to enable > PLLE hardware sequencer at proper time. > > 2. tegra210_plle_hw_sequence_is_enabled() for XUSB PADCTL driver to > check whether PLLE hardware sequencer has been enabled or not. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > rename 'val' with 'value > > drivers/clk/tegra/clk-tegra210.c | 51 ++++++++++++++++++++++++++++++++ > include/linux/clk/tegra.h | 2 ++ > 2 files changed, 53 insertions(+) Acked-by: Thierry Reding <treding@nvidia.com>
On Wed, Sep 09, 2020 at 04:10:28PM +0800, JC Kuo wrote: > PLLE hardware power sequencer references PEX/SATA UPHY PLL hardware > power sequencers' output to enable/disable PLLE. PLLE hardware power > sequencer has to be enabled only after PEX/SATA UPHY PLL's sequencers > are enabled. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > no change > > drivers/clk/tegra/clk-pll.c | 12 ------------ > 1 file changed, 12 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com>
On Wed, Sep 09, 2020 at 04:10:26PM +0800, JC Kuo wrote: > Tegra XHCI controler can be placed in ELPG (Engine Level PowerGated) > state for power saving when all of the connected USB devices are in > suspended state. This patch series includes clk, phy and pmc changes > that are required for properly place controller in ELPG and bring > controller out of ELPG. > > JC Kuo (15): > clk: tegra: Add PLLE HW power sequencer control > clk: tegra: Don't enable PLLE HW sequencer at init Is it safe to apply this second patch before the others have applied? Since we now need to explicitly enable the HW sequencer, it won't be enabled before the corresponding patch does that. So applying patch 2 before the others sounds like it would break existing users of the HW sequencer. Thierry
On Wed, Sep 09, 2020 at 04:10:30PM +0800, JC Kuo wrote: > Once UPHY PLL hardware power sequencer is enabled, do not assert > reset to PEX/SATA PLLs, otherwise UPHY PLL operation will be broken. > This commit removes reset_control_assert(pcie->rst) and > reset_control_assert(sata->rst) from PEX/SATA UPHY disable procedure. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > new, was a part of "phy: tegra: xusb: Rearrange UPHY init on Tegra210" > > drivers/phy/tegra/xusb-tegra210.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c > index f06e7bc7a51b..ef4bbcbed60b 100644 > --- a/drivers/phy/tegra/xusb-tegra210.c > +++ b/drivers/phy/tegra/xusb-tegra210.c > @@ -504,7 +504,6 @@ static void tegra210_pex_uphy_disable(struct tegra_xusb_padctl *padctl) > if (--pcie->enable > 0) > goto unlock; > > - reset_control_assert(pcie->rst); > clk_disable_unprepare(pcie->pll); > > unlock: > @@ -746,7 +745,6 @@ static void tegra210_sata_uphy_disable(struct tegra_xusb_padctl *padctl) > if (--sata->enable > 0) > goto unlock; > > - reset_control_assert(sata->rst); > clk_disable_unprepare(sata->pll); > > unlock: Does this mean that we can no longer reset these PLLs anymore? Is that safe? Would we ever need to reset them for recovery or similar? For power saving, is disabling the clock enough, or could we save some extra power by putting the PLLs into reset? Thierry
On Wed, Sep 09, 2020 at 04:10:32PM +0800, JC Kuo wrote: > As per Tegra210 TRM, before changing lane assignments, driver should > keep lanes in IDDQ and sleep state; after changing lane assignments, > driver should bring lanes out of IDDQ. > This commit implements the required operations. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > add 'misc_ctl2' data member to UPHY lane for carrying MISC_PAD_PX_CTL2 offset > tegra210_uphy_lane_iddq_[enable/disable]() to access 'misc_ctl2' data member > > drivers/phy/tegra/xusb-tegra210.c | 82 ++++++++++++++++++++++++++++--- > drivers/phy/tegra/xusb.c | 6 +++ > drivers/phy/tegra/xusb.h | 6 +++ > 3 files changed, 86 insertions(+), 8 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com>
On Wed, Sep 09, 2020 at 04:10:34PM +0800, JC Kuo wrote: > This commit implements a register map which grants USB (UTMI and HSIC) > sleepwalk registers access to USB PHY drivers. The USB sleepwalk logic > is in PMC hardware block but USB PHY drivers have the best knowledge > of proper programming sequence. This approach prevents using custom > pmc APIs. I don't think this final sentence is useful. The commit message should explain what you're doing, but there's no need to enumerate any other inferior solution you didn't choose to implement. If you do want to keep it: s/pmc/PMC/. While at it, perhaps replace "usb" by "USB" in the subject as well. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > commit message improvement > drop regmap_reg() usage > rename 'reg' with 'offset' > rename 'val' with 'value' > drop '__force' when invokes devm_regmap_init() > print error code of devm_regmap_init() > move devm_regmap_init() a litter bit earlier > explicitly set '.has_usb_sleepwalk=false' > > drivers/soc/tegra/pmc.c | 95 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 95 insertions(+) > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > index d332e5d9abac..ff24891ce9ca 100644 > --- a/drivers/soc/tegra/pmc.c > +++ b/drivers/soc/tegra/pmc.c > @@ -43,6 +43,7 @@ > #include <linux/seq_file.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > +#include <linux/regmap.h> > > #include <soc/tegra/common.h> > #include <soc/tegra/fuse.h> > @@ -102,6 +103,9 @@ > > #define PMC_PWR_DET_VALUE 0xe4 > > +#define PMC_USB_DEBOUNCE_DEL 0xec > +#define PMC_USB_AO 0xf0 > + > #define PMC_SCRATCH41 0x140 > > #define PMC_WAKE2_MASK 0x160 > @@ -133,6 +137,13 @@ > #define IO_DPD2_STATUS 0x1c4 > #define SEL_DPD_TIM 0x1c8 > > +#define PMC_UTMIP_UHSIC_TRIGGERS 0x1ec > +#define PMC_UTMIP_UHSIC_SAVED_STATE 0x1f0 > + > +#define PMC_UTMIP_TERM_PAD_CFG 0x1f8 > +#define PMC_UTMIP_UHSIC_SLEEP_CFG 0x1fc > +#define PMC_UTMIP_UHSIC_FAKE 0x218 > + > #define PMC_SCRATCH54 0x258 > #define PMC_SCRATCH54_DATA_SHIFT 8 > #define PMC_SCRATCH54_ADDR_SHIFT 0 > @@ -145,8 +156,18 @@ > #define PMC_SCRATCH55_CHECKSUM_SHIFT 16 > #define PMC_SCRATCH55_I2CSLV1_SHIFT 0 > > +#define PMC_UTMIP_UHSIC_LINE_WAKEUP 0x26c > + > +#define PMC_UTMIP_BIAS_MASTER_CNTRL 0x270 > +#define PMC_UTMIP_MASTER_CONFIG 0x274 > +#define PMC_UTMIP_UHSIC2_TRIGGERS 0x27c > +#define PMC_UTMIP_MASTER2_CONFIG 0x29c > + > #define GPU_RG_CNTRL 0x2d4 > > +#define PMC_UTMIP_PAD_CFG0 0x4c0 > +#define PMC_UTMIP_UHSIC_SLEEP_CFG1 0x4d0 > +#define PMC_UTMIP_SLEEPWALK_P3 0x4e0 > /* Tegra186 and later */ > #define WAKE_AOWAKE_CNTRL(x) (0x000 + ((x) << 2)) > #define WAKE_AOWAKE_CNTRL_LEVEL (1 << 3) > @@ -334,6 +355,7 @@ struct tegra_pmc_soc { > const struct pmc_clk_init_data *pmc_clks_data; > unsigned int num_pmc_clks; > bool has_blink_output; > + bool has_usb_sleepwalk; > }; > > static const char * const tegra186_reset_sources[] = { > @@ -2495,6 +2517,68 @@ static void tegra_pmc_clock_register(struct tegra_pmc *pmc, > err); > } > > +static const struct regmap_range pmc_usb_sleepwalk_ranges[] = { > + regmap_reg_range(PMC_USB_DEBOUNCE_DEL, PMC_USB_AO), > + regmap_reg_range(PMC_UTMIP_UHSIC_TRIGGERS, PMC_UTMIP_UHSIC_SAVED_STATE), > + regmap_reg_range(PMC_UTMIP_TERM_PAD_CFG, PMC_UTMIP_UHSIC_FAKE), > + regmap_reg_range(PMC_UTMIP_UHSIC_LINE_WAKEUP, PMC_UTMIP_UHSIC_LINE_WAKEUP), > + regmap_reg_range(PMC_UTMIP_BIAS_MASTER_CNTRL, PMC_UTMIP_MASTER_CONFIG), > + regmap_reg_range(PMC_UTMIP_UHSIC2_TRIGGERS, PMC_UTMIP_MASTER2_CONFIG), > + regmap_reg_range(PMC_UTMIP_PAD_CFG0, PMC_UTMIP_UHSIC_SLEEP_CFG1), > + regmap_reg_range(PMC_UTMIP_SLEEPWALK_P3, PMC_UTMIP_SLEEPWALK_P3), > +}; > + > +static const struct regmap_access_table pmc_usb_sleepwalk_table = { > + .yes_ranges = pmc_usb_sleepwalk_ranges, > + .n_yes_ranges = ARRAY_SIZE(pmc_usb_sleepwalk_ranges), > +}; > + > +static int tegra_pmc_regmap_readl(void *context, unsigned int offset, unsigned int *value) > +{ > + struct tegra_pmc *pmc = context; > + > + *value = tegra_pmc_readl(pmc, offset); > + return 0; > +} > + > +static int tegra_pmc_regmap_writel(void *context, unsigned int offset, unsigned int value) > +{ > + struct tegra_pmc *pmc = context; > + > + tegra_pmc_writel(pmc, value, offset); > + return 0; > +} > + > +static const struct regmap_config usb_sleepwalk_regmap_config = { > + .name = "usb_sleepwalk", > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .fast_io = true, > + .rd_table = &pmc_usb_sleepwalk_table, > + .wr_table = &pmc_usb_sleepwalk_table, > + .reg_read = tegra_pmc_regmap_readl, > + .reg_write = tegra_pmc_regmap_writel, > +}; > + > +static int tegra_pmc_regmap_init(struct tegra_pmc *pmc) > +{ > + struct regmap *regmap; > + int err; > + > + if (pmc->soc->has_usb_sleepwalk) { > + regmap = devm_regmap_init(pmc->dev, NULL, (void *) pmc, I don't think you need that explicit cast there. With those minor comments addressed: Acked-by: Thierry Reding <treding@nvidia.com>
On Wed, Sep 09, 2020 at 04:10:37PM +0800, JC Kuo wrote: > To support XUSB host controller ELPG, this commit moves VBUS control > .phy_power_on()/.phy_power_off() to .phy_init()/.phy_exit(). > When XUSB host controller enters ELPG, host driver invokes > .phy_power_off(), VBUS should remain ON so that USB devices will not > disconnect. VBUS can be turned OFF when host driver invokes > .phy_exit() which indicates disabling a USB port. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > new, was a part of "phy: tegra: xusb: Add wake/sleepwalk for Tegra210" > > drivers/phy/tegra/xusb-tegra210.c | 52 ++++++++++++++++++++++++------- > 1 file changed, 40 insertions(+), 12 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com>
On Wed, Sep 09, 2020 at 04:10:40PM +0800, JC Kuo wrote: > This commit unlinks xhci-tegra platform device with ss/host power > domain devices. Reasons for this change is - at elpg entry, PHY s/elpg/ELPG/ > sleepwalk and wake configuration need to be done before powering > down ss/host partitions, and PHY need be powered off after powering s/ss/SS/ here (because it's an abbreviation) and in a few cases below. Otherwise this seems fine, though it'd be good if Jon could take a look since he's more familiar with the power domain setup here. Thierry
On Wed, Sep 09, 2020 at 04:10:41PM +0800, JC Kuo wrote: > This commit implements the complete programming sequence for ELPG > entry and exit. > > 1. At ELPG entry, invokes tegra_xusb_padctl_enable_phy_sleepwalk() > and tegra_xusb_padctl_enable_phy_wake() to configure XUSB PADCTL > sleepwalk and wake detection circuits to maintain USB lines level > and respond to wake events (wake-on-connect, wake-on-disconnect, > device-initiated-wake). > > 2. At ELPG exit, invokes tegra_xusb_padctl_disable_phy_sleepwalk() > and tegra_xusb_padctl_disable_phy_wake() to disarm sleepwalk and > wake detection circuits. > > At runtime suspend, XUSB host controller can enter ELPG to reduce > power consumption. When XUSB PADCTL wake detection circuit detects > a wake event, an interrupt will be raised. xhci-tegra driver then > will invoke pm_runtime_resume() for xhci-tegra. > > Runtime resume could also be triggered by protocol drivers, this is > the host-initiated-wake event. At runtime resume, xhci-tegra driver > brings XUSB host controller out of ELPG to handle the wake events. > > The same ELPG enter/exit procedure will be performed for system > suspend/resume path so USB devices can remain connected across SC7. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > v3: > use 'unsigned int' for PHY index > remove unnecessary 'else' > drop IRQF_TRIGGER_HIGH when invokes devm_request_threaded_irq() > > drivers/usb/host/xhci-tegra.c | 389 +++++++++++++++++++++++++++++++--- > 1 file changed, 360 insertions(+), 29 deletions(-) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index aabff8ee0bb3..ba3f40e78171 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -15,9 +15,11 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of_irq.h> > #include <linux/phy/phy.h> > #include <linux/phy/tegra/xusb.h> > #include <linux/platform_device.h> > +#include <linux/usb/ch9.h> > #include <linux/pm.h> > #include <linux/pm_domain.h> > #include <linux/pm_runtime.h> > @@ -224,6 +226,7 @@ struct tegra_xusb { > > int xhci_irq; > int mbox_irq; > + int padctl_irq; > > void __iomem *ipfs_base; > void __iomem *fpci_base; > @@ -269,10 +272,13 @@ struct tegra_xusb { > dma_addr_t phys; > } fw; > > + bool suspended; > struct tegra_xusb_context context; > }; > > static struct hc_driver __read_mostly tegra_xhci_hc_driver; > +static int tegra_xusb_exit_elpg(struct tegra_xusb *tegra, bool runtime); > +static int tegra_xusb_enter_elpg(struct tegra_xusb *tegra, bool runtime); Can we reshuffle the code to avoid these predeclarations? Looks like they're only used in tegra_xusb_runtime_{suspend,resume}(), so perhaps move the implementations right before those? Thierry
Yes, it's safe to apply "clk: tegra: Don't enable PLLE HW sequencer at init" before the others have applied. Disabling PLLE hardware power sequencer will not cause any functionality problem to XUSB/PCIE/SATA. The only thing changed is PLLE won't be powered off by hardware when all clients are in low power state, i.e., software has to explicitly power off PLLE. Thanks for review. JC On 9/28/20 8:54 PM, Thierry Reding wrote: > On Wed, Sep 09, 2020 at 04:10:26PM +0800, JC Kuo wrote: >> Tegra XHCI controler can be placed in ELPG (Engine Level PowerGated) >> state for power saving when all of the connected USB devices are in >> suspended state. This patch series includes clk, phy and pmc changes >> that are required for properly place controller in ELPG and bring >> controller out of ELPG. >> >> JC Kuo (15): >> clk: tegra: Add PLLE HW power sequencer control >> clk: tegra: Don't enable PLLE HW sequencer at init > > Is it safe to apply this second patch before the others have applied? > Since we now need to explicitly enable the HW sequencer, it won't be > enabled before the corresponding patch does that. So applying patch 2 > before the others sounds like it would break existing users of the HW > sequencer. > > Thierry >
Asserting reset to a PLL when it's managed by hardware power sequencer would break sequencer's state machine. Putting PLL in reset doesn't save some extra power. Thanks for review. JC On 9/28/20 9:06 PM, Thierry Reding wrote: > On Wed, Sep 09, 2020 at 04:10:30PM +0800, JC Kuo wrote: >> Once UPHY PLL hardware power sequencer is enabled, do not assert >> reset to PEX/SATA PLLs, otherwise UPHY PLL operation will be broken. >> This commit removes reset_control_assert(pcie->rst) and >> reset_control_assert(sata->rst) from PEX/SATA UPHY disable procedure. >> >> Signed-off-by: JC Kuo <jckuo@nvidia.com> >> --- >> v3: >> new, was a part of "phy: tegra: xusb: Rearrange UPHY init on Tegra210" >> >> drivers/phy/tegra/xusb-tegra210.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c >> index f06e7bc7a51b..ef4bbcbed60b 100644 >> --- a/drivers/phy/tegra/xusb-tegra210.c >> +++ b/drivers/phy/tegra/xusb-tegra210.c >> @@ -504,7 +504,6 @@ static void tegra210_pex_uphy_disable(struct tegra_xusb_padctl *padctl) >> if (--pcie->enable > 0) >> goto unlock; >> >> - reset_control_assert(pcie->rst); >> clk_disable_unprepare(pcie->pll); >> >> unlock: >> @@ -746,7 +745,6 @@ static void tegra210_sata_uphy_disable(struct tegra_xusb_padctl *padctl) >> if (--sata->enable > 0) >> goto unlock; >> >> - reset_control_assert(sata->rst); >> clk_disable_unprepare(sata->pll); >> >> unlock: > > Does this mean that we can no longer reset these PLLs anymore? Is that > safe? Would we ever need to reset them for recovery or similar? For > power saving, is disabling the clock enough, or could we save some extra > power by putting the PLLs into reset? > > Thierry >
I will amend commit accordingly and submit a new patch. Thanks for review. JC On 9/28/20 9:17 PM, Thierry Reding wrote: > On Wed, Sep 09, 2020 at 04:10:34PM +0800, JC Kuo wrote: >> This commit implements a register map which grants USB (UTMI and HSIC) >> sleepwalk registers access to USB PHY drivers. The USB sleepwalk logic >> is in PMC hardware block but USB PHY drivers have the best knowledge >> of proper programming sequence. This approach prevents using custom >> pmc APIs. > > I don't think this final sentence is useful. The commit message should > explain what you're doing, but there's no need to enumerate any other > inferior solution you didn't choose to implement. > > If you do want to keep it: s/pmc/PMC/. > > While at it, perhaps replace "usb" by "USB" in the subject as well. > >> >> Signed-off-by: JC Kuo <jckuo@nvidia.com> >> --- >> v3: >> commit message improvement >> drop regmap_reg() usage >> rename 'reg' with 'offset' >> rename 'val' with 'value' >> drop '__force' when invokes devm_regmap_init() >> print error code of devm_regmap_init() >> move devm_regmap_init() a litter bit earlier >> explicitly set '.has_usb_sleepwalk=false' >> >> drivers/soc/tegra/pmc.c | 95 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 95 insertions(+) >> >> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c >> index d332e5d9abac..ff24891ce9ca 100644 >> --- a/drivers/soc/tegra/pmc.c >> +++ b/drivers/soc/tegra/pmc.c >> @@ -43,6 +43,7 @@ >> #include <linux/seq_file.h> >> #include <linux/slab.h> >> #include <linux/spinlock.h> >> +#include <linux/regmap.h> >> >> #include <soc/tegra/common.h> >> #include <soc/tegra/fuse.h> >> @@ -102,6 +103,9 @@ >> >> #define PMC_PWR_DET_VALUE 0xe4 >> >> +#define PMC_USB_DEBOUNCE_DEL 0xec >> +#define PMC_USB_AO 0xf0 >> + >> #define PMC_SCRATCH41 0x140 >> >> #define PMC_WAKE2_MASK 0x160 >> @@ -133,6 +137,13 @@ >> #define IO_DPD2_STATUS 0x1c4 >> #define SEL_DPD_TIM 0x1c8 >> >> +#define PMC_UTMIP_UHSIC_TRIGGERS 0x1ec >> +#define PMC_UTMIP_UHSIC_SAVED_STATE 0x1f0 >> + >> +#define PMC_UTMIP_TERM_PAD_CFG 0x1f8 >> +#define PMC_UTMIP_UHSIC_SLEEP_CFG 0x1fc >> +#define PMC_UTMIP_UHSIC_FAKE 0x218 >> + >> #define PMC_SCRATCH54 0x258 >> #define PMC_SCRATCH54_DATA_SHIFT 8 >> #define PMC_SCRATCH54_ADDR_SHIFT 0 >> @@ -145,8 +156,18 @@ >> #define PMC_SCRATCH55_CHECKSUM_SHIFT 16 >> #define PMC_SCRATCH55_I2CSLV1_SHIFT 0 >> >> +#define PMC_UTMIP_UHSIC_LINE_WAKEUP 0x26c >> + >> +#define PMC_UTMIP_BIAS_MASTER_CNTRL 0x270 >> +#define PMC_UTMIP_MASTER_CONFIG 0x274 >> +#define PMC_UTMIP_UHSIC2_TRIGGERS 0x27c >> +#define PMC_UTMIP_MASTER2_CONFIG 0x29c >> + >> #define GPU_RG_CNTRL 0x2d4 >> >> +#define PMC_UTMIP_PAD_CFG0 0x4c0 >> +#define PMC_UTMIP_UHSIC_SLEEP_CFG1 0x4d0 >> +#define PMC_UTMIP_SLEEPWALK_P3 0x4e0 >> /* Tegra186 and later */ >> #define WAKE_AOWAKE_CNTRL(x) (0x000 + ((x) << 2)) >> #define WAKE_AOWAKE_CNTRL_LEVEL (1 << 3) >> @@ -334,6 +355,7 @@ struct tegra_pmc_soc { >> const struct pmc_clk_init_data *pmc_clks_data; >> unsigned int num_pmc_clks; >> bool has_blink_output; >> + bool has_usb_sleepwalk; >> }; >> >> static const char * const tegra186_reset_sources[] = { >> @@ -2495,6 +2517,68 @@ static void tegra_pmc_clock_register(struct tegra_pmc *pmc, >> err); >> } >> >> +static const struct regmap_range pmc_usb_sleepwalk_ranges[] = { >> + regmap_reg_range(PMC_USB_DEBOUNCE_DEL, PMC_USB_AO), >> + regmap_reg_range(PMC_UTMIP_UHSIC_TRIGGERS, PMC_UTMIP_UHSIC_SAVED_STATE), >> + regmap_reg_range(PMC_UTMIP_TERM_PAD_CFG, PMC_UTMIP_UHSIC_FAKE), >> + regmap_reg_range(PMC_UTMIP_UHSIC_LINE_WAKEUP, PMC_UTMIP_UHSIC_LINE_WAKEUP), >> + regmap_reg_range(PMC_UTMIP_BIAS_MASTER_CNTRL, PMC_UTMIP_MASTER_CONFIG), >> + regmap_reg_range(PMC_UTMIP_UHSIC2_TRIGGERS, PMC_UTMIP_MASTER2_CONFIG), >> + regmap_reg_range(PMC_UTMIP_PAD_CFG0, PMC_UTMIP_UHSIC_SLEEP_CFG1), >> + regmap_reg_range(PMC_UTMIP_SLEEPWALK_P3, PMC_UTMIP_SLEEPWALK_P3), >> +}; >> + >> +static const struct regmap_access_table pmc_usb_sleepwalk_table = { >> + .yes_ranges = pmc_usb_sleepwalk_ranges, >> + .n_yes_ranges = ARRAY_SIZE(pmc_usb_sleepwalk_ranges), >> +}; >> + >> +static int tegra_pmc_regmap_readl(void *context, unsigned int offset, unsigned int *value) >> +{ >> + struct tegra_pmc *pmc = context; >> + >> + *value = tegra_pmc_readl(pmc, offset); >> + return 0; >> +} >> + >> +static int tegra_pmc_regmap_writel(void *context, unsigned int offset, unsigned int value) >> +{ >> + struct tegra_pmc *pmc = context; >> + >> + tegra_pmc_writel(pmc, value, offset); >> + return 0; >> +} >> + >> +static const struct regmap_config usb_sleepwalk_regmap_config = { >> + .name = "usb_sleepwalk", >> + .reg_bits = 32, >> + .val_bits = 32, >> + .reg_stride = 4, >> + .fast_io = true, >> + .rd_table = &pmc_usb_sleepwalk_table, >> + .wr_table = &pmc_usb_sleepwalk_table, >> + .reg_read = tegra_pmc_regmap_readl, >> + .reg_write = tegra_pmc_regmap_writel, >> +}; >> + >> +static int tegra_pmc_regmap_init(struct tegra_pmc *pmc) >> +{ >> + struct regmap *regmap; >> + int err; >> + >> + if (pmc->soc->has_usb_sleepwalk) { >> + regmap = devm_regmap_init(pmc->dev, NULL, (void *) pmc, > > I don't think you need that explicit cast there. > > With those minor comments addressed: > > Acked-by: Thierry Reding <treding@nvidia.com> >
I will modify the commit message accordingly. Thanks for review. JC On 9/28/20 9:53 PM, Thierry Reding wrote: > On Wed, Sep 09, 2020 at 04:10:40PM +0800, JC Kuo wrote: >> This commit unlinks xhci-tegra platform device with ss/host power >> domain devices. Reasons for this change is - at elpg entry, PHY > > s/elpg/ELPG/ > >> sleepwalk and wake configuration need to be done before powering >> down ss/host partitions, and PHY need be powered off after powering > > s/ss/SS/ here (because it's an abbreviation) and in a few cases below. > > Otherwise this seems fine, though it'd be good if Jon could take a look > since he's more familiar with the power domain setup here. > > Thierry >
I will amend accordingly and submit new patch. Thanks for review. JC On 9/28/20 10:06 PM, Thierry Reding wrote: > On Wed, Sep 09, 2020 at 04:10:41PM +0800, JC Kuo wrote: >> This commit implements the complete programming sequence for ELPG >> entry and exit. >> >> 1. At ELPG entry, invokes tegra_xusb_padctl_enable_phy_sleepwalk() >> and tegra_xusb_padctl_enable_phy_wake() to configure XUSB PADCTL >> sleepwalk and wake detection circuits to maintain USB lines level >> and respond to wake events (wake-on-connect, wake-on-disconnect, >> device-initiated-wake). >> >> 2. At ELPG exit, invokes tegra_xusb_padctl_disable_phy_sleepwalk() >> and tegra_xusb_padctl_disable_phy_wake() to disarm sleepwalk and >> wake detection circuits. >> >> At runtime suspend, XUSB host controller can enter ELPG to reduce >> power consumption. When XUSB PADCTL wake detection circuit detects >> a wake event, an interrupt will be raised. xhci-tegra driver then >> will invoke pm_runtime_resume() for xhci-tegra. >> >> Runtime resume could also be triggered by protocol drivers, this is >> the host-initiated-wake event. At runtime resume, xhci-tegra driver >> brings XUSB host controller out of ELPG to handle the wake events. >> >> The same ELPG enter/exit procedure will be performed for system >> suspend/resume path so USB devices can remain connected across SC7. >> >> Signed-off-by: JC Kuo <jckuo@nvidia.com> >> --- >> v3: >> use 'unsigned int' for PHY index >> remove unnecessary 'else' >> drop IRQF_TRIGGER_HIGH when invokes devm_request_threaded_irq() >> >> drivers/usb/host/xhci-tegra.c | 389 +++++++++++++++++++++++++++++++--- >> 1 file changed, 360 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c >> index aabff8ee0bb3..ba3f40e78171 100644 >> --- a/drivers/usb/host/xhci-tegra.c >> +++ b/drivers/usb/host/xhci-tegra.c >> @@ -15,9 +15,11 @@ >> #include <linux/kernel.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> +#include <linux/of_irq.h> >> #include <linux/phy/phy.h> >> #include <linux/phy/tegra/xusb.h> >> #include <linux/platform_device.h> >> +#include <linux/usb/ch9.h> >> #include <linux/pm.h> >> #include <linux/pm_domain.h> >> #include <linux/pm_runtime.h> >> @@ -224,6 +226,7 @@ struct tegra_xusb { >> >> int xhci_irq; >> int mbox_irq; >> + int padctl_irq; >> >> void __iomem *ipfs_base; >> void __iomem *fpci_base; >> @@ -269,10 +272,13 @@ struct tegra_xusb { >> dma_addr_t phys; >> } fw; >> >> + bool suspended; >> struct tegra_xusb_context context; >> }; >> >> static struct hc_driver __read_mostly tegra_xhci_hc_driver; >> +static int tegra_xusb_exit_elpg(struct tegra_xusb *tegra, bool runtime); >> +static int tegra_xusb_enter_elpg(struct tegra_xusb *tegra, bool runtime); > > Can we reshuffle the code to avoid these predeclarations? Looks like > they're only used in tegra_xusb_runtime_{suspend,resume}(), so perhaps > move the implementations right before those? > > Thierry >