[v3,1/2] platform/chrome: cros_ec_typec: Configure Retimer cable type

Connector class driver only configure cable type active or passive.
Configure if the cable type is retimer or redriver with this change.
This detail will be provided as a part of cable discover mode VDO.

Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
---
Changes in v2:
- Implemented use of cable discover mode vdo.
- Removed adittional changes to host command.

Changes in v3:
- Changed the return method in cros_typec_get_cable_vdo.
- Changed passed parameters in cros_typec_get_cable_vdo.
- Corrected definition for unsigned integers as kerenl standard.
- Assigning cable_vdo values directly in to cable_mode.
- Removed unncessary checks for Retimer cable type.
---
---
 drivers/platform/chrome/cros_ec_typec.c | 26 ++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Hi Prashant,

Thank you for the review. 

> > diff --git a/drivers/platform/chrome/cros_ec_typec.c
> > b/drivers/platform/chrome/cros_ec_typec.c
> > index 25f9767c28e8..0ea085fec55a 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -406,6 +406,20 @@ static int cros_typec_usb_safe_state(struct
> cros_typec_port *port)
> >         return ret;
> >  }
> >
> > +static int cros_typec_get_cable_vdo(struct cros_typec_port *port, u16
> > +svid)
> Return type should be u32.
> Also, since you're not using common return patterns (-ve return value),
> please add a kernel doc comment specifying what the return value holds ("0
> if VDO is not found" etc.)

Ack.

> 
> > +{
> > +       struct list_head *head = &port->plug_mode_list;
> > +       struct cros_typec_altmode_node *node;
> > +       int ret = 0;
> > +
> > +       list_for_each_entry(node, head, list) {
> > +               if (node->amode->svid == svid)
> > +                       return node->amode->vdo;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >  /*
> >   * Spoof the VDOs that were likely communicated by the partner for TBT
> alt
> >   * mode.
> > @@ -416,6 +430,7 @@ static int cros_typec_enable_tbt(struct
> > cros_typec_data *typec,  {
> >         struct cros_typec_port *port = typec->ports[port_num];
> >         struct typec_thunderbolt_data data;
> > +       u32 cable_vdo;
> If we're using this in only 1 place, we can just inline it completely:
>           data.cable_mode |= cros_typec_get_cable_vdo(port,
> USB_TYPEC_TBT_SID);
> 

Ack.

> >         int ret;
> >
> >         if (typec->pd_ctrl_ver < 2) {
> > @@ -432,6 +447,10 @@ static int cros_typec_enable_tbt(struct
> > cros_typec_data *typec,
> >
> >         /* Cable Discover Mode VDO */
> >         data.cable_mode = TBT_MODE;
> > +
> > +       cable_vdo = cros_typec_get_cable_vdo(port, USB_TYPEC_TBT_SID);
> > +       data.cable_mode |= cable_vdo;
> > +
> >         data.cable_mode |= TBT_SET_CABLE_SPEED(pd_ctrl->cable_speed);
> >
> >         if (pd_ctrl->control_flags & USB_PD_CTRL_OPTICAL_CABLE) @@
> > -513,17 +532,22 @@ static int cros_typec_enable_usb4(struct
> > cros_typec_data *typec,  {
> >         struct cros_typec_port *port = typec->ports[port_num];
> >         struct enter_usb_data data;
> > +       u32 cable_vdo;
> >
> >         data.eudo = EUDO_USB_MODE_USB4 << EUDO_USB_MODE_SHIFT;
> >
> > +       cable_vdo = cros_typec_get_cable_vdo(port, USB_TYPEC_TBT_SID);
> 
> Same deal here, if we're only using this variable in one place, just inline the
> call inside the else if() statement.
> 

Ack. 


Sincerely,
Utkarsh Patel.