Re: [PATCH v7 2/5] usb: chipidea: msm: Configure phy for appropriate mode

From: Peter Chen
Date: Sat Jan 21 2017 - 21:11:49 EST


On Fri, Jan 20, 2017 at 10:50:54AM -0800, Stephen Boyd wrote:
> When the qcom chipidea controller is used with an extcon, we need
> to signal device mode or host mode to the phy so it can configure
> itself for the correct mode. This should be done after the phy is
> powered up, so that the register writes work correctly. Add in
> the appropriate phy_set_mode() call here.
>
> To signal the correct state to the qcom glue driver we need to
> change the ci->role before we do the role switch. Make sure to
> undo the change if the role switch fails, but otherwise update
> the state before calling the role start function so that the glue
> driver knows what state to configure for.
>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Stephen Boyd <stephen.boyd@xxxxxxxxxx>
> ---
>
> Made this msm specific because of how msm handles phy powerup.
>
> drivers/usb/chipidea/ci.h | 7 +++++--
> drivers/usb/chipidea/ci_hdrc_msm.c | 4 ++++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 59e22389c10b..18348b0529af 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -268,6 +268,7 @@ static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
> {
> int ret;
> + enum ci_role prev_role;
>
> if (role >= CI_ROLE_END)
> return -EINVAL;
> @@ -275,9 +276,11 @@ static inline int ci_role_start(struct ci_hdrc *ci, enum ci_role role)
> if (!ci->roles[role])
> return -ENXIO;
>
> + prev_role = ci->role;
> + ci->role = role;
> ret = ci->roles[role]->start(ci);
> - if (!ret)
> - ci->role = role;
> + if (ret)
> + ci->role = prev_role;
> return ret;

Sorry, this changes ci->role's life cycle. You may try to get coming
role at your glue layer code (eg, through usbmode), or add your
changes at ci_role_start directly if the sequence can make your requirement.

Peter
> }
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c
> index f1ede7909f54..9c58d13970ca 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -125,6 +125,10 @@ static int ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
> hw_write(ci, OP_USBCMD, HSPHY_SESS_VLD_CTRL,
> HSPHY_SESS_VLD_CTRL);
>
> + if (ci->role == CI_ROLE_GADGET)
> + phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
> + else if (ci->role == CI_ROLE_HOST)
> + phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
> }
> break;
> case CI_HDRC_CONTROLLER_STOPPED_EVENT:
> --
> 2.10.0.297.gf6727b0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--

Best Regards,
Peter Chen