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 <kis...@ti.com>
>> ---
>>  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

Reply via email to