Hi Utkarsh,
On Thu, Jul 6, 2023 at 11:51 PM Utkarsh Patel <utkarsh.h.patel@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
> ---
> 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(-)
>
> 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.)
> +{
> + 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);
> 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.
> +
> /* Cable Speed */
> data.eudo |= pd_ctrl->cable_speed << EUDO_CABLE_SPEED_SHIFT;
>
> /* Cable Type */
> if (pd_ctrl->control_flags & USB_PD_CTRL_OPTICAL_CABLE)
> data.eudo |= EUDO_CABLE_TYPE_OPTICAL << EUDO_CABLE_TYPE_SHIFT;
> - else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
> + else if (cable_vdo & TBT_CABLE_RETIMER)
> data.eudo |= EUDO_CABLE_TYPE_RE_TIMER << EUDO_CABLE_TYPE_SHIFT;
> + else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
> + data.eudo |= EUDO_CABLE_TYPE_RE_DRIVER << EUDO_CABLE_TYPE_SHIFT;
>
> data.active_link_training = !!(pd_ctrl->control_flags &
> USB_PD_CTRL_ACTIVE_LINK_UNIDIR);
> --
> 2.25.1
>
