Hi Roger, On 30/01/19 4:18 PM, Roger Quadros wrote: > Hi Kishon, > > On 24/01/19 12:48, Kishon Vijay Abraham I wrote: >> DRA72 platform has the second instance of PHY shared between USB3 >> controller and PCIe controller with default as USB3 controller. >> Since it is used with USB3 controller by default, it uses the >> compatible specific to USB (ti,omap-usb3). >> >> Populate set_mode callback so that the USB3 PHY can be configured >> to be used with PCIe controller. > > What is the use case of this? At what point and who decides the > phy_set_mode() to be called with USB vs PCIe? The PHY by default is configured to be used with USB controller (That's why it has a compatible of ti,omap-usb3). However there is a special case where this PHY has to be used PCIe controller. So the set_mode() will help the PHY driver to configure the USB PHY to be used with PCIe controller. This is not a shared PHY, so the PHY as such can be used only with USB or PCIe and is determined at designtime. > > >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx> >> --- >> drivers/phy/ti/phy-ti-pipe3.c | 66 ++++++++++++++++++++++++++++------- >> 1 file changed, 54 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c >> index 68ce4a082b9b..8c98f366416d 100644 >> --- a/drivers/phy/ti/phy-ti-pipe3.c >> +++ b/drivers/phy/ti/phy-ti-pipe3.c >> @@ -56,6 +56,12 @@ >> >> #define SATA_PLL_SOFT_RESET BIT(18) >> >> +#define PHY_RX_ANA_PRGRAMMABILITY_REG 0xC >> +#define MEM_EN_PLLBYP BIT(7) >> + >> +#define PHY_TX_TEST_CONFIG 0x2C >> +#define MEM_ENTESTCLK BIT(31) >> + >> #define PIPE3_PHY_PWRCTL_CLK_CMD_MASK 0x003FC000 >> #define PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT 14 >> >> @@ -110,6 +116,8 @@ >> #define PLL_IDLE_TIME 100 /* in milliseconds */ >> #define PLL_LOCK_TIME 100 /* in milliseconds */ >> >> +#define PIPE3_PHY_DISABLE_SYNC_POWER BIT(4) >> + >> struct pipe3_dpll_params { >> u16 m; >> u8 n; >> @@ -141,6 +149,7 @@ struct ti_pipe3 { >> unsigned int power_reg; /* power reg. index within syscon */ >> unsigned int pcie_pcs_reg; /* pcs reg. index in syscon */ >> bool sata_refclk_enabled; >> + u32 mode; >> }; >> >> static struct pipe3_dpll_map dpll_map_usb[] = { >> @@ -233,7 +242,10 @@ static int ti_pipe3_power_on(struct phy *x) >> rate = rate / 1000000; >> mask = OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_MASK | >> OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_MASK; >> - val = PIPE3_PHY_TX_RX_POWERON << PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT; >> + val = PIPE3_PHY_TX_RX_POWERON; >> + if (phy->mode == PHY_MODE_PCIE) >> + val |= PIPE3_PHY_DISABLE_SYNC_POWER; >> + val <<= PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT; >> val |= rate << OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_SHIFT; >> >> ret = regmap_update_bits(phy->phy_power_syscon, phy->power_reg, >> @@ -328,13 +340,11 @@ static void ti_pipe3_calibrate(struct ti_pipe3 *phy) >> ti_pipe3_writel(phy->phy_rx, PCIEPHYRX_EQUALIZER, val); >> } >> >> -static int ti_pipe3_init(struct phy *x) >> +static int ti_pipe3_pcie_init(struct ti_pipe3 *phy) >> { >> - struct ti_pipe3 *phy = phy_get_drvdata(x); >> - u32 val; >> int ret = 0; >> + u32 val; >> >> - ti_pipe3_enable_clocks(phy); >> /* >> * Set pcie_pcs register to 0x96 for proper functioning of phy >> * as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table >> @@ -353,10 +363,31 @@ static int ti_pipe3_init(struct phy *x) >> return ret; >> >> ti_pipe3_calibrate(phy); >> - >> - return 0; >> + } else { >> + val = ti_pipe3_readl(phy->phy_rx, >> + PHY_RX_ANA_PRGRAMMABILITY_REG); >> + val |= MEM_EN_PLLBYP; >> + ti_pipe3_writel(phy->phy_rx, PHY_RX_ANA_PRGRAMMABILITY_REG, >> + val); >> + val = ti_pipe3_readl(phy->phy_tx, PHY_TX_TEST_CONFIG); >> + val |= MEM_ENTESTCLK; >> + ti_pipe3_writel(phy->phy_tx, PHY_TX_TEST_CONFIG, val); >> } >> >> + return 0; >> +} >> + >> +static int ti_pipe3_init(struct phy *x) >> +{ >> + struct ti_pipe3 *phy = phy_get_drvdata(x); >> + u32 val; >> + int ret = 0; >> + >> + ti_pipe3_enable_clocks(phy); >> + >> + if (phy->mode == PHY_MODE_PCIE) >> + return ti_pipe3_pcie_init(phy); >> + >> /* Bring it out of IDLE if it is IDLE */ >> val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); >> if (val & PLL_IDLE) { >> @@ -395,7 +426,7 @@ static int ti_pipe3_exit(struct phy *x) >> return 0; >> >> /* PCIe doesn't have internal DPLL */ >> - if (!of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) { >> + if (!(phy->mode == PHY_MODE_PCIE)) { >> /* Put DPLL in IDLE mode */ >> val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); >> val |= PLL_IDLE; >> @@ -429,11 +460,25 @@ static int ti_pipe3_exit(struct phy *x) >> >> return 0; >> } >> + >> +static int ti_pipe3_set_mode(struct phy *x, enum phy_mode mode, int submode) >> +{ >> + struct ti_pipe3 *phy = phy_get_drvdata(x); >> + >> + if (phy->mode != PHY_MODE_INVALID) >> + return -EBUSY; >> + >> + phy->mode = mode; > > You are only saving the mode. But not really switching modes here. > How is this intended to work? The PHY doesn't have a specific mode. Rather it is a set of configurations required to operate in a particular mode. This mode value will be used in other init, power_on callback functions. Thanks Kishon