RE: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting usb role during disconnect

[Shaikh, Azhar]

Hi Prashant,


> -----Original Message-----
> From: Prashant Malani <pmalani@xxxxxxxxxxxx>
> Sent: Thursday, July 30, 2020 4:03 PM
> To: Shaikh, Azhar <azhar.shaikh@xxxxxxxxx>
> Cc: bleung@xxxxxxxxxxxx; enric.balletbo@xxxxxxxxxxxxx;
> groeck@xxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> heikki.krogerus@xxxxxxxxxxxxxxx; Patel, Utkarsh H
> <utkarsh.h.patel@xxxxxxxxx>; Bowman, Casey G
> <casey.g.bowman@xxxxxxxxx>; Mani, Rajmohan
> <rajmohan.mani@xxxxxxxxx>
> Subject: Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Avoid setting
> usb role during disconnect
> 
> Hi Azhar,
> 
> On Thu, Jul 30, 2020 at 03:56:09PM -0700, Azhar Shaikh wrote:
> > On disconnect port partner is removed and usb role is set to NONE.
> > But then in cros_typec_port_update() the role is set again.
> > Avoid this by moving usb_role_switch_set_role() to
> > cros_typec_configure_mux().
> >
> > Signed-off-by: Azhar Shaikh <azhar.shaikh@xxxxxxxxx>
> > Suggested-by: Prashant Malani <pmalani@xxxxxxxxxxxx>
> > ---
> >  drivers/platform/chrome/cros_ec_typec.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index eb4713b7ae14..df97431b2275 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -515,6 +515,12 @@ static int cros_typec_configure_mux(struct
> cros_typec_data *typec, int port_num,
> >  	if (ret)
> >  		return ret;
> >
> > +	ret = usb_role_switch_set_role(typec->ports[port_num]->role_sw,
> > +				       pd_ctrl->role &
> PD_CTRL_RESP_ROLE_DATA
> > +				       ? USB_ROLE_HOST : USB_ROLE_DEVICE);
> > +	if (ret)
> > +		return ret;
> 
> Since this was the last switch being configured, please maintain the same
> order and add this at the end of the function, after the if-else if block.
> 

Please correct if my understanding is not correct here:
Set the orientation , set the role, then configure the mux. Shouldn't this be the order?


> Best regards,
> 
> -Prashant