| Message ID | 20210617194154.2397-8-linux.amoon@gmail.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | Meson-8b and Meson-gxbb USB phy code re-structure | expand |
Hi Anand, On Thu, Jun 17, 2021 at 9:44 PM Anand Moon <linux.amoon@gmail.com> wrote: [...] > @@ -245,8 +250,6 @@ static int phy_meson8b_usb2_power_on(struct phy *phy) > regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_FSEL_MASK, > 0x5 << REG_CTRL_FSEL_SHIFT); > /* reset the PHY */ > - regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, > - REG_CTRL_POWER_ON_RESET); The vendor driver uses the following sequence for the power on reset: - set the power on reset bit - wait 500us - clear the power on reset bit - wait 500us With your change we now: - wait 500us - clear the power on reset bit - wait 500us I don't know if this is sufficient to bring the PHY into a well-defined state. Maybe it works, maybe it doesn't reset at all in this case - I don't know how to verify this though. Best regards, Martin
Hi Martin, On Fri, 18 Jun 2021 at 04:07, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Thu, Jun 17, 2021 at 9:44 PM Anand Moon <linux.amoon@gmail.com> wrote: > [...] > > @@ -245,8 +250,6 @@ static int phy_meson8b_usb2_power_on(struct phy *phy) > > regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_FSEL_MASK, > > 0x5 << REG_CTRL_FSEL_SHIFT); > > /* reset the PHY */ > > - regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, > > - REG_CTRL_POWER_ON_RESET); > The vendor driver uses the following sequence for the power on reset: > - set the power on reset bit > - wait 500us > - clear the power on reset bit > - wait 500us > > With your change we now: > - wait 500us > - clear the power on reset bit > - wait 500us > > I don't know if this is sufficient to bring the PHY into a well-defined state. > Maybe it works, maybe it doesn't reset at all in this case - I don't > know how to verify this though. > Initially, I tried to some bit mask code to resolve this but it failed, So no harm in keeping the original changes. There is another parameter REG_CTRL_PORT_RESET to be considered. > > Best regards, > Martin Thanks -Anand
Hi Anand, On Mon, Jun 21, 2021 at 9:15 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Martin, > > On Fri, 18 Jun 2021 at 04:07, Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > > > Hi Anand, > > > > On Thu, Jun 17, 2021 at 9:44 PM Anand Moon <linux.amoon@gmail.com> wrote: > > [...] > > > @@ -245,8 +250,6 @@ static int phy_meson8b_usb2_power_on(struct phy *phy) > > > regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_FSEL_MASK, > > > 0x5 << REG_CTRL_FSEL_SHIFT); > > > /* reset the PHY */ > > > - regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, > > > - REG_CTRL_POWER_ON_RESET); > > The vendor driver uses the following sequence for the power on reset: > > - set the power on reset bit > > - wait 500us > > - clear the power on reset bit > > - wait 500us > > > > With your change we now: > > - wait 500us > > - clear the power on reset bit > > - wait 500us > > > > I don't know if this is sufficient to bring the PHY into a well-defined state. > > Maybe it works, maybe it doesn't reset at all in this case - I don't > > know how to verify this though. > > > Initially, I tried to some bit mask code to resolve this but it failed, > So no harm in keeping the original changes. yes, I feel more comfortable with that > There is another parameter REG_CTRL_PORT_RESET to be considered. none of the vendor kernels that I have sets or clears this bit explicitly I agree that the name seems related, but due to lack of an example or documentation how/when to use this bit I suggest we don't touch it for now Best rergards, Martin
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c index c1ed2e5c80d8..d5edd31686bb 100644 --- a/drivers/phy/amlogic/phy-meson8b-usb2.c +++ b/drivers/phy/amlogic/phy-meson8b-usb2.c @@ -226,6 +226,11 @@ static int phy_meson8b_usb2_power_off(struct phy *phy) regmap_update_bits(priv->regmap, REG_DBG_UART, REG_DBG_UART_SET_IDDQ, REG_DBG_UART_SET_IDDQ); + + /* power off the PHY by putting it into reset mode */ + regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, + REG_CTRL_POWER_ON_RESET); + return 0; } @@ -245,8 +250,6 @@ static int phy_meson8b_usb2_power_on(struct phy *phy) regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_FSEL_MASK, 0x5 << REG_CTRL_FSEL_SHIFT); /* reset the PHY */ - regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, - REG_CTRL_POWER_ON_RESET); udelay(RESET_COMPLETE_TIME); regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET, 0); udelay(RESET_COMPLETE_TIME);
Power off the PHY by putting it into reset mode. Drop the phy power reset since we are doing reset of phy after we configure the phy. No functional change. Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- drivers/phy/amlogic/phy-meson8b-usb2.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)