[PATCH v3 0/2] Add support to configure active retimer cable

[PATCH v3 0/2] Add support to configure active retimer cable

[Utkarsh Patel]

This change adds support to configure retimer cable type

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.

Utkarsh Patel (2):
  platform/chrome: cros_ec_typec: Configure Retimer cable type
  usb: typec: intel_pmc_mux: Configure Active and Retimer Cable type

 drivers/platform/chrome/cros_ec_typec.c | 26 ++++++++++++++++++++++-
 drivers/usb/typec/mux/intel_pmc_mux.c   | 28 +++++++++++++++++++++----
 2 files changed, 49 insertions(+), 5 deletions(-)

-- 
2.25.1

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

[Utkarsh Patel]

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(-)

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)
+{
+	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;
 	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);
+
 	/* 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

[PATCH v3 2/2] usb: typec: intel_pmc_mux: Configure Active and Retimer Cable type

[Utkarsh Patel]

Cable type such as active and retimer received as a part of Thunderbolt3
or Thunderbolt4 cable discover mode VDO needs to be configured in the
thunderbolt alternate mode.

Configuring the register bits for this cable type is changed with Intel
Meteor Lake platform. BIT2 for Retimer/Redriver cable and BIT22 for
Active/Passive cable.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
---
Changes in v2:
 - No changes.

Changes in v3:
 - No changes.
---
---
 drivers/usb/typec/mux/intel_pmc_mux.c | 28 +++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 5e8edf3881c0..888632847a74 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -59,7 +59,7 @@ enum {
 };
 
 /* Common Mode Data bits */
-#define PMC_USB_ALTMODE_ACTIVE_CABLE	BIT(2)
+#define PMC_USB_ALTMODE_RETIMER_CABLE	BIT(2)
 
 #define PMC_USB_ALTMODE_ORI_SHIFT	1
 #define PMC_USB_ALTMODE_UFP_SHIFT	3
@@ -71,6 +71,7 @@ enum {
 #define PMC_USB_ALTMODE_TBT_TYPE	BIT(17)
 #define PMC_USB_ALTMODE_CABLE_TYPE	BIT(18)
 #define PMC_USB_ALTMODE_ACTIVE_LINK	BIT(20)
+#define PMC_USB_ALTMODE_ACTIVE_CABLE	BIT(22)
 #define PMC_USB_ALTMODE_FORCE_LSR	BIT(23)
 #define PMC_USB_ALTMODE_CABLE_SPD(_s_)	(((_s_) & GENMASK(2, 0)) << 25)
 #define   PMC_USB_ALTMODE_CABLE_USB31	1
@@ -319,8 +320,18 @@ pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
 	if (data->cable_mode & TBT_CABLE_LINK_TRAINING)
 		req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
 
-	if (data->enter_vdo & TBT_ENTER_MODE_ACTIVE_CABLE)
-		req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
+	if (acpi_dev_hid_uid_match(port->pmc->iom_adev, "INTC1072", NULL) ||
+	    acpi_dev_hid_uid_match(port->pmc->iom_adev, "INTC1079", NULL)) {
+		if ((data->enter_vdo & TBT_ENTER_MODE_ACTIVE_CABLE) ||
+		    (data->cable_mode & TBT_CABLE_RETIMER))
+			req.mode_data |= PMC_USB_ALTMODE_RETIMER_CABLE;
+	} else {
+		if (data->enter_vdo & TBT_ENTER_MODE_ACTIVE_CABLE)
+			req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
+
+		if (data->cable_mode & TBT_CABLE_RETIMER)
+			req.mode_data |= PMC_USB_ALTMODE_RETIMER_CABLE;
+	}
 
 	req.mode_data |= PMC_USB_ALTMODE_CABLE_SPD(cable_speed);
 
@@ -359,8 +370,17 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
 	case EUDO_CABLE_TYPE_OPTICAL:
 		req.mode_data |= PMC_USB_ALTMODE_CABLE_TYPE;
 		fallthrough;
+	case EUDO_CABLE_TYPE_RE_TIMER:
+		if (!acpi_dev_hid_uid_match(port->pmc->iom_adev, "INTC1072", NULL) ||
+		    !acpi_dev_hid_uid_match(port->pmc->iom_adev, "INTC1079", NULL))
+			req.mode_data |= PMC_USB_ALTMODE_RETIMER_CABLE;
+		fallthrough;
 	default:
-		req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
+		if (acpi_dev_hid_uid_match(port->pmc->iom_adev, "INTC1072", NULL) ||
+		    acpi_dev_hid_uid_match(port->pmc->iom_adev, "INTC1079", NULL))
+			req.mode_data |= PMC_USB_ALTMODE_RETIMER_CABLE;
+		else
+			req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
 
 		/* Configure data rate to rounded in the case of Active TBT3
 		 * and USB4 cables.
-- 
2.25.1

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

[Prashant Malani]

Hi Utkarsh,

On Thu, Jul 6, 2023 at 11:51 PM Utkarsh Patel <utkarsh.h.patel@intel.com> 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@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(-)
>
> 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
>

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

[Patel, Utkarsh H]

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.