LinuxLists.cc - [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers

2023-12-07 09:08:18

Subject: [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers

Add enum tcpm_altmode_transmit_type that Alternate Mode drivers can use
to communicate which SOP type to send a SVDM on to the tcpm, and that the
tcpm can use to communicate a received SVDM' SOP type to the Alternate Mode
drivers.

Update all typec_altmode_ops users to use tcpm_altmode_transmit_type, and
drop all messages that are not TYPEC_ALTMODE_SOP. Default all calls that
require sop_type as input to TYPEC_ALTMODE_SOP.

Signed-off-by: RD Babiera <[email protected]>
---
drivers/platform/chrome/cros_typec_vdm.c | 12 +++++++++--
drivers/usb/typec/altmodes/displayport.c | 15 +++++++-------
drivers/usb/typec/bus.c | 17 ++++++++++------
drivers/usb/typec/class.c | 2 +-
drivers/usb/typec/tcpm/tcpm.c | 15 ++++++++------
drivers/usb/typec/ucsi/displayport.c | 18 +++++++++++++---
include/linux/usb/typec_altmode.h | 26 ++++++++++++++++++------
7 files changed, 74 insertions(+), 31 deletions(-)

diff --git a/drivers/platform/chrome/cros_typec_vdm.c b/drivers/platform/chrome/cros_typec_vdm.c
index 3f632fd35000..ff33e5305866 100644
--- a/drivers/platform/chrome/cros_typec_vdm.c
+++ b/drivers/platform/chrome/cros_typec_vdm.c
@@ -92,7 +92,8 @@ void cros_typec_handle_vdm_response(struct cros_typec_data *typec, int port_num)
svid, port_num);
}

-static int cros_typec_port_amode_enter(struct typec_altmode *amode, u32 *vdo)
+static int cros_typec_port_amode_enter(struct typec_altmode *amode, u32 *vdo,
+ enum typec_altmode_transmit_type sop_type)
{
struct cros_typec_port *port = typec_altmode_get_drvdata(amode);
struct ec_params_typec_control req = {
@@ -102,6 +103,9 @@ static int cros_typec_port_amode_enter(struct typec_altmode *amode, u32 *vdo)
struct typec_vdm_req vdm_req = {};
u32 hdr;

+ if (sop_type != TYPEC_ALTMODE_SOP)
+ return 0;
+
hdr = VDO(amode->svid, 1, SVDM_VER_2_0, CMD_ENTER_MODE);
hdr |= VDO_OPOS(amode->mode);

@@ -118,7 +122,8 @@ static int cros_typec_port_amode_enter(struct typec_altmode *amode, u32 *vdo)
}

static int cros_typec_port_amode_vdm(struct typec_altmode *amode, const u32 hdr,
- const u32 *vdo, int cnt)
+ const u32 *vdo, int cnt,
+ enum typec_altmode_transmit_type sop_type)
{
struct cros_typec_port *port = typec_altmode_get_drvdata(amode);
struct ec_params_typec_control req = {
@@ -128,6 +133,9 @@ static int cros_typec_port_amode_vdm(struct typec_altmode *amode, const u32 hdr,
struct typec_vdm_req vdm_req = {};
int i;

+ if (sop_type != TYPEC_ALTMODE_SOP)
+ return 0;
+
vdm_req.vdm_data[0] = hdr;
vdm_req.vdm_data_objects = cnt;
for (i = 1; i < cnt; i++)
diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index f81bec0c7b86..5ed470069c3e 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -202,7 +202,7 @@ static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
return ret;
}

- ret = typec_altmode_vdm(dp->alt, header, &conf, 2);
+ ret = typec_altmode_vdm(dp->alt, header, &conf, 2, TYPEC_ALTMODE_SOP);
if (ret)
dp_altmode_notify(dp);

@@ -221,7 +221,7 @@ static void dp_altmode_work(struct work_struct *work)

switch (dp->state) {
case DP_STATE_ENTER:
- ret = typec_altmode_enter(dp->alt, NULL);
+ ret = typec_altmode_enter(dp->alt, NULL, TYPEC_ALTMODE_SOP);
if (ret && ret != -EBUSY)
dev_err(&dp->alt->dev, "failed to enter mode\n");
break;
@@ -231,7 +231,7 @@ static void dp_altmode_work(struct work_struct *work)
break;
header = DP_HEADER(dp, svdm_version, DP_CMD_STATUS_UPDATE);
vdo = 1;
- ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
+ ret = typec_altmode_vdm(dp->alt, header, &vdo, 2, TYPEC_ALTMODE_SOP);
if (ret)
dev_err(&dp->alt->dev,
"unable to send Status Update command (%d)\n",
@@ -244,7 +244,7 @@ static void dp_altmode_work(struct work_struct *work)
"unable to send Configure command (%d)\n", ret);
break;
case DP_STATE_EXIT:
- if (typec_altmode_exit(dp->alt))
+ if (typec_altmode_exit(dp->alt, TYPEC_ALTMODE_SOP))
dev_err(&dp->alt->dev, "Exit Mode Failed!\n");
break;
default:
@@ -283,7 +283,8 @@ static void dp_altmode_attention(struct typec_altmode *alt, const u32 vdo)
}

static int dp_altmode_vdm(struct typec_altmode *alt,
- const u32 hdr, const u32 *vdo, int count)
+ const u32 hdr, const u32 *vdo, int count,
+ enum typec_altmode_transmit_type sop_type)
{
struct dp_altmode *dp = typec_altmode_get_drvdata(alt);
int cmd_type = PD_VDO_CMDT(hdr);
@@ -350,8 +351,8 @@ static int dp_altmode_vdm(struct typec_altmode *alt,

static int dp_altmode_activate(struct typec_altmode *alt, int activate)
{
- return activate ? typec_altmode_enter(alt, NULL) :
- typec_altmode_exit(alt);
+ return activate ? typec_altmode_enter(alt, NULL, TYPEC_ALTMODE_SOP) :
+ typec_altmode_exit(alt, TYPEC_ALTMODE_SOP);
}

static const struct typec_altmode_ops dp_altmode_ops = {
diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index e95ec7e382bb..c9c6e55bed9b 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -117,13 +117,15 @@ EXPORT_SYMBOL_GPL(typec_altmode_notify);
* typec_altmode_enter - Enter Mode
* @adev: The alternate mode
* @vdo: VDO for the Enter Mode command
+ * @sop_type: SOP* target for the Enter Mode command
*
* The alternate mode drivers use this function to enter mode. The port drivers
* use this to inform the alternate mode drivers that the partner has initiated
* Enter Mode command. If the alternate mode does not require VDO, @vdo must be
* NULL.
*/
-int typec_altmode_enter(struct typec_altmode *adev, u32 *vdo)
+int typec_altmode_enter(struct typec_altmode *adev, u32 *vdo,
+ enum typec_altmode_transmit_type sop_type)
{
struct altmode *partner = to_altmode(adev)->partner;
struct typec_altmode *pdev = &partner->adev;
@@ -144,17 +146,18 @@ int typec_altmode_enter(struct typec_altmode *adev, u32 *vdo)
return ret;

/* Enter Mode */
- return pdev->ops->enter(pdev, vdo);
+ return pdev->ops->enter(pdev, vdo, sop_type);
}
EXPORT_SYMBOL_GPL(typec_altmode_enter);

/**
* typec_altmode_exit - Exit Mode
* @adev: The alternate mode
+ * @sop_type: SOP* target for the Exit Mode command
*
* The partner of @adev has initiated Exit Mode command.
*/
-int typec_altmode_exit(struct typec_altmode *adev)
+int typec_altmode_exit(struct typec_altmode *adev, enum typec_altmode_transmit_type sop_type)
{
struct altmode *partner = to_altmode(adev)->partner;
struct typec_altmode *pdev = &partner->adev;
@@ -172,7 +175,7 @@ int typec_altmode_exit(struct typec_altmode *adev)
return ret;

/* Exit Mode command */
- return pdev->ops->exit(pdev);
+ return pdev->ops->exit(pdev, sop_type);
}
EXPORT_SYMBOL_GPL(typec_altmode_exit);

@@ -206,13 +209,15 @@ EXPORT_SYMBOL_GPL(typec_altmode_attention);
* @header: VDM Header
* @vdo: Array of Vendor Defined Data Objects
* @count: Number of Data Objects
+ * @sop_type: SOP* target for the VDM
*
* The alternate mode drivers use this function for SVID specific communication
* with the partner. The port drivers use it to deliver the Structured VDMs
* received from the partners to the alternate mode drivers.
*/
int typec_altmode_vdm(struct typec_altmode *adev,
- const u32 header, const u32 *vdo, int count)
+ const u32 header, const u32 *vdo, int count,
+ enum typec_altmode_transmit_type sop_type)
{
struct typec_altmode *pdev;
struct altmode *altmode;
@@ -230,7 +235,7 @@ int typec_altmode_vdm(struct typec_altmode *adev,
if (!pdev->ops || !pdev->ops->vdm)
return -EOPNOTSUPP;

- return pdev->ops->vdm(pdev, header, vdo, count);
+ return pdev->ops->vdm(pdev, header, vdo, count, sop_type);
}
EXPORT_SYMBOL_GPL(typec_altmode_vdm);

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 2e0451bd336e..7514766df195 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -375,7 +375,7 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,

/* Make sure that the partner exits the mode before disabling */
if (altmode->partner && !enter && altmode->partner->adev.active)
- typec_altmode_exit(&altmode->partner->adev);
+ typec_altmode_exit(&altmode->partner->adev, TYPEC_ALTMODE_SOP);
} else if (altmode->partner) {
if (enter && !altmode->partner->adev.active) {
dev_warn(dev, "port has the mode disabled\n");
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index ff67553b6932..795e3145b0c2 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1862,13 +1862,13 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
break;
case ADEV_NOTIFY_USB_AND_QUEUE_VDM:
WARN_ON(typec_altmode_notify(adev, TYPEC_STATE_USB, NULL));
- typec_altmode_vdm(adev, p[0], &p[1], cnt);
+ typec_altmode_vdm(adev, p[0], &p[1], cnt, TYPEC_ALTMODE_SOP);
break;
case ADEV_QUEUE_VDM:
- typec_altmode_vdm(adev, p[0], &p[1], cnt);
+ typec_altmode_vdm(adev, p[0], &p[1], cnt, TYPEC_ALTMODE_SOP);
break;
case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
- if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
+ if (typec_altmode_vdm(adev, p[0], &p[1], cnt, TYPEC_ALTMODE_SOP)) {
int svdm_version = typec_get_negotiated_svdm_version(
port->typec_port);
if (svdm_version < 0)
@@ -2219,7 +2219,8 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
return 0;
}

-static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
+static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo,
+ enum typec_altmode_transmit_type tx_sop_type)
{
struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
int svdm_version;
@@ -2236,7 +2237,8 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
return 0;
}

-static int tcpm_altmode_exit(struct typec_altmode *altmode)
+static int tcpm_altmode_exit(struct typec_altmode *altmode,
+ enum typec_altmode_transmit_type tx_sop_type)
{
struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
int svdm_version;
@@ -2254,7 +2256,8 @@ static int tcpm_altmode_exit(struct typec_altmode *altmode)
}

static int tcpm_altmode_vdm(struct typec_altmode *altmode,
- u32 header, const u32 *data, int count)
+ u32 header, const u32 *data, int count,
+ enum typec_altmode_transmit_type tx_sop_type)
{
struct tcpm_port *port = typec_altmode_get_drvdata(altmode);

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index d9d3c91125ca..9043defbb86c 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -45,7 +45,8 @@ struct ucsi_dp {
* -EOPNOTSUPP.
*/

-static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
+static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo,
+ enum typec_altmode_transmit_type sop_type)
{
struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
struct ucsi *ucsi = dp->con->ucsi;
@@ -54,6 +55,9 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
u8 cur = 0;
int ret;

+ if (sop_type != TYPEC_ALTMODE_SOP)
+ return 0;
+
mutex_lock(&dp->con->lock);

if (!dp->override && dp->initialized) {
@@ -105,13 +109,17 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
return ret;
}

-static int ucsi_displayport_exit(struct typec_altmode *alt)
+static int ucsi_displayport_exit(struct typec_altmode *alt,
+ enum typec_altmode_transmit_type sop_type)
{
struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
int svdm_version;
u64 command;
int ret = 0;

+ if (sop_type != TYPEC_ALTMODE_SOP)
+ return 0;
+
mutex_lock(&dp->con->lock);

if (!dp->override) {
@@ -195,13 +203,17 @@ static int ucsi_displayport_configure(struct ucsi_dp *dp)
}

static int ucsi_displayport_vdm(struct typec_altmode *alt,
- u32 header, const u32 *data, int count)
+ u32 header, const u32 *data, int count,
+ enum typec_altmode_transmit_type sop_type)
{
struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
int cmd_type = PD_VDO_CMDT(header);
int cmd = PD_VDO_CMD(header);
int svdm_version;

+ if (sop_type != TYPEC_ALTMODE_SOP)
+ return 0;
+
mutex_lock(&dp->con->lock);

if (!dp->override && dp->initialized) {
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 28aeef8f9e7b..4d527d92457d 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -34,6 +34,16 @@ struct typec_altmode {

#define to_typec_altmode(d) container_of(d, struct typec_altmode, dev)

+/**
+ * These are used by the Alternate Mode drivers to tell the tcpm to transmit
+ * over the selected SOP type, and are used by the tcpm to communicate the
+ * received VDM SOP type to the Alternate Mode drivers.
+ */
+enum typec_altmode_transmit_type {
+ TYPEC_ALTMODE_SOP,
+ TYPEC_ALTMODE_SOP_PRIME,
+};
+
static inline void typec_altmode_set_drvdata(struct typec_altmode *altmode,
void *data)
{
@@ -55,21 +65,25 @@ static inline void *typec_altmode_get_drvdata(struct typec_altmode *altmode)
* @activate: User callback for Enter/Exit Mode
*/
struct typec_altmode_ops {
- int (*enter)(struct typec_altmode *altmode, u32 *vdo);
- int (*exit)(struct typec_altmode *altmode);
+ int (*enter)(struct typec_altmode *altmode, u32 *vdo,
+ enum typec_altmode_transmit_type sop_type);
+ int (*exit)(struct typec_altmode *altmode,
+ enum typec_altmode_transmit_type sop_type);
void (*attention)(struct typec_altmode *altmode, u32 vdo);
int (*vdm)(struct typec_altmode *altmode, const u32 hdr,
- const u32 *vdo, int cnt);
+ const u32 *vdo, int cnt, enum typec_altmode_transmit_type sop_type);
int (*notify)(struct typec_altmode *altmode, unsigned long conf,
void *data);
int (*activate)(struct typec_altmode *altmode, int activate);
};

-int typec_altmode_enter(struct typec_altmode *altmode, u32 *vdo);
-int typec_altmode_exit(struct typec_altmode *altmode);
+int typec_altmode_enter(struct typec_altmode *altmode, u32 *vdo,
+ enum typec_altmode_transmit_type sop_type);
+int typec_altmode_exit(struct typec_altmode *altmode, enum typec_altmode_transmit_type sop_type);
int typec_altmode_attention(struct typec_altmode *altmode, u32 vdo);
int typec_altmode_vdm(struct typec_altmode *altmode,
- const u32 header, const u32 *vdo, int count);
+ const u32 header, const u32 *vdo, int count,
+ enum typec_altmode_transmit_type sop_type);
int typec_altmode_notify(struct typec_altmode *altmode, unsigned long conf,
void *data);
const struct typec_altmode *
--
2.43.0.rc2.451.g8631bc7472-goog

2023-12-07 11:35:13

by Heikki Krogerus

[permalink] [raw]

Subject: Re: [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers

Hi,

On Thu, Dec 07, 2023 at 09:07:32AM +0000, RD Babiera wrote:
> Add enum tcpm_altmode_transmit_type that Alternate Mode drivers can use
> to communicate which SOP type to send a SVDM on to the tcpm, and that the
> tcpm can use to communicate a received SVDM' SOP type to the Alternate Mode
> drivers.
>
> Update all typec_altmode_ops users to use tcpm_altmode_transmit_type, and
> drop all messages that are not TYPEC_ALTMODE_SOP. Default all calls that
> require sop_type as input to TYPEC_ALTMODE_SOP.
>
> Signed-off-by: RD Babiera <[email protected]>
> ---
> drivers/platform/chrome/cros_typec_vdm.c | 12 +++++++++--
> drivers/usb/typec/altmodes/displayport.c | 15 +++++++-------
> drivers/usb/typec/bus.c | 17 ++++++++++------
> drivers/usb/typec/class.c | 2 +-
> drivers/usb/typec/tcpm/tcpm.c | 15 ++++++++------
> drivers/usb/typec/ucsi/displayport.c | 18 +++++++++++++---
> include/linux/usb/typec_altmode.h | 26 ++++++++++++++++++------
> 7 files changed, 74 insertions(+), 31 deletions(-)

<snip>

> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index 28aeef8f9e7b..4d527d92457d 100644
> --- a/include/linux/usb/typec_altmode.h
> +++ b/include/linux/usb/typec_altmode.h
> @@ -34,6 +34,16 @@ struct typec_altmode {
>
> #define to_typec_altmode(d) container_of(d, struct typec_altmode, dev)
>
> +/**
> + * These are used by the Alternate Mode drivers to tell the tcpm to transmit
> + * over the selected SOP type, and are used by the tcpm to communicate the
> + * received VDM SOP type to the Alternate Mode drivers.
> + */
> +enum typec_altmode_transmit_type {
> + TYPEC_ALTMODE_SOP,
> + TYPEC_ALTMODE_SOP_PRIME,
> +};
> +
> static inline void typec_altmode_set_drvdata(struct typec_altmode *altmode,
> void *data)
> {
> @@ -55,21 +65,25 @@ static inline void *typec_altmode_get_drvdata(struct typec_altmode *altmode)
> * @activate: User callback for Enter/Exit Mode
> */
> struct typec_altmode_ops {
> - int (*enter)(struct typec_altmode *altmode, u32 *vdo);
> - int (*exit)(struct typec_altmode *altmode);
> + int (*enter)(struct typec_altmode *altmode, u32 *vdo,
> + enum typec_altmode_transmit_type sop_type);
> + int (*exit)(struct typec_altmode *altmode,
> + enum typec_altmode_transmit_type sop_type);
> void (*attention)(struct typec_altmode *altmode, u32 vdo);
> int (*vdm)(struct typec_altmode *altmode, const u32 hdr,
> - const u32 *vdo, int cnt);
> + const u32 *vdo, int cnt, enum typec_altmode_transmit_type sop_type);
> int (*notify)(struct typec_altmode *altmode, unsigned long conf,
> void *data);
> int (*activate)(struct typec_altmode *altmode, int activate);
> };
>
> -int typec_altmode_enter(struct typec_altmode *altmode, u32 *vdo);
> -int typec_altmode_exit(struct typec_altmode *altmode);
> +int typec_altmode_enter(struct typec_altmode *altmode, u32 *vdo,
> + enum typec_altmode_transmit_type sop_type);
> +int typec_altmode_exit(struct typec_altmode *altmode, enum typec_altmode_transmit_type sop_type);
> int typec_altmode_attention(struct typec_altmode *altmode, u32 vdo);
> int typec_altmode_vdm(struct typec_altmode *altmode,
> - const u32 header, const u32 *vdo, int count);
> + const u32 header, const u32 *vdo, int count,
> + enum typec_altmode_transmit_type sop_type);
> int typec_altmode_notify(struct typec_altmode *altmode, unsigned long conf,
> void *data);
> const struct typec_altmode *

Instead of forcing this change immediately on every existing user of
that API, why not supply separate API for the cable alt modes?

Although the SOP* communication is the same in most parts, at least
Attention (and probable some other messages too) is not valid with
cable plugs. So maybe it would be more clear to just separate SOP
communication from SOP Prime/Double Prime in the API?

So it would look something like this:

diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index bd41bc362ab0..2f7ae50585b1 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -75,6 +75,24 @@ int typec_altmode_notify(struct typec_altmode *altmode, unsigned long conf,
const struct typec_altmode *
typec_altmode_get_partner(struct typec_altmode *altmode);

+/**
+ * struct typec_cable_ops - Cable alternate mode operations vector
+ * @enter: Operations to be executed with Enter Mode Command
+ * @exit: Operations to be executed with Exit Mode Command
+ * @vdm: Callback for SVID specific commands
+ */
+struct typec_cable_ops {
+ int (*enter)(struct typec_altmode *altmode, enum typec_plug_index sop, u32 *vdo);
+ int (*exit)(struct typec_altmode *altmode, enum typec_plug_index sop);
+ int (*vdm)(struct typec_altmode *altmode, enum typec_plug_index sop,
+ const u32 hdr, const u32 *vdo, int cnt);
+};
+
+int typec_cable_altmode_enter(struct typec_altmode *altmode, enum typec_plug_index sop, u32 *vdo);
+int typec_cable_altmode_exit(struct typec_altmode *altmode, enum typec_plug_index sop);
+int typec_cable_altmode_vdm(struct typec_altmode *altmode, enum typec_plug_index sop,
+ const u32 header, const u32 *vdo, int count);
+
/*
* These are the connector states (USB, Safe and Alt Mode) defined in USB Type-C
* Specification. SVID specific connector states are expected to follow and

thanks,

--
heikki

2023-12-07 20:34:19

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers

2023-12-07 21:07:52

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers

Hi RD,

kernel test robot noticed the following build errors:

[auto build test ERROR on 5e4c8814a431d21bfaf20b464134f40f2f81e152]

url: https://github.com/intel-lab-lkp/linux/commits/RD-Babiera/usb-typec-bus-provide-transmit-type-for-alternate-mode-drivers/20231207-171114
base: 5e4c8814a431d21bfaf20b464134f40f2f81e152
patch link: https://lore.kernel.org/r/20231207090738.15721-13-rdbabiera%40google.com
patch subject: [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers
config: arm-defconfig (https://download.01.org/0day-ci/archive/20231208/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231208/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/platform/chrome/cros_typec_vdm.c:89:82: error: too few arguments to function call, expected 5, have 4
ret = typec_altmode_vdm(amode, hdr, &resp.vdm_response[1], resp.vdm_data_objects);
~~~~~~~~~~~~~~~~~ ^
include/linux/usb/typec_altmode.h:84:5: note: 'typec_altmode_vdm' declared here
int typec_altmode_vdm(struct typec_altmode *altmode,
^
1 error generated.

vim +89 drivers/platform/chrome/cros_typec_vdm.c

f54c013e7eef29 Prashant Malani 2023-01-26 55
50ed638bbc47ba Prashant Malani 2022-12-28 56 /*
50ed638bbc47ba Prashant Malani 2022-12-28 57 * Retrieves a VDM response from the EC and forwards it to the altmode driver based on SVID.
50ed638bbc47ba Prashant Malani 2022-12-28 58 */
50ed638bbc47ba Prashant Malani 2022-12-28 59 void cros_typec_handle_vdm_response(struct cros_typec_data *typec, int port_num)
50ed638bbc47ba Prashant Malani 2022-12-28 60 {
50ed638bbc47ba Prashant Malani 2022-12-28 61 struct ec_response_typec_vdm_response resp;
50ed638bbc47ba Prashant Malani 2022-12-28 62 struct ec_params_typec_vdm_response req = {
50ed638bbc47ba Prashant Malani 2022-12-28 63 .port = port_num,
50ed638bbc47ba Prashant Malani 2022-12-28 64 };
50ed638bbc47ba Prashant Malani 2022-12-28 65 struct typec_altmode *amode;
50ed638bbc47ba Prashant Malani 2022-12-28 66 u16 svid;
50ed638bbc47ba Prashant Malani 2022-12-28 67 u32 hdr;
50ed638bbc47ba Prashant Malani 2022-12-28 68 int ret;
50ed638bbc47ba Prashant Malani 2022-12-28 69
50ed638bbc47ba Prashant Malani 2022-12-28 70 ret = cros_ec_cmd(typec->ec, 0, EC_CMD_TYPEC_VDM_RESPONSE, &req,
50ed638bbc47ba Prashant Malani 2022-12-28 71 sizeof(req), &resp, sizeof(resp));
50ed638bbc47ba Prashant Malani 2022-12-28 72 if (ret < 0) {
50ed638bbc47ba Prashant Malani 2022-12-28 73 dev_warn(typec->dev, "Failed VDM response fetch, port: %d\n", port_num);
50ed638bbc47ba Prashant Malani 2022-12-28 74 return;
50ed638bbc47ba Prashant Malani 2022-12-28 75 }
50ed638bbc47ba Prashant Malani 2022-12-28 76
50ed638bbc47ba Prashant Malani 2022-12-28 77 hdr = resp.vdm_response[0];
50ed638bbc47ba Prashant Malani 2022-12-28 78 svid = PD_VDO_VID(hdr);
50ed638bbc47ba Prashant Malani 2022-12-28 79 dev_dbg(typec->dev, "Received VDM header: %x, port: %d\n", hdr, port_num);
50ed638bbc47ba Prashant Malani 2022-12-28 80
50ed638bbc47ba Prashant Malani 2022-12-28 81 amode = typec_match_altmode(typec->ports[port_num]->port_altmode, CROS_EC_ALTMODE_MAX,
50ed638bbc47ba Prashant Malani 2022-12-28 82 svid, PD_VDO_OPOS(hdr));
50ed638bbc47ba Prashant Malani 2022-12-28 83 if (!amode) {
50ed638bbc47ba Prashant Malani 2022-12-28 84 dev_err(typec->dev, "Received VDM for unregistered altmode (SVID:%x), port: %d\n",
50ed638bbc47ba Prashant Malani 2022-12-28 85 svid, port_num);
50ed638bbc47ba Prashant Malani 2022-12-28 86 return;
50ed638bbc47ba Prashant Malani 2022-12-28 87 }
50ed638bbc47ba Prashant Malani 2022-12-28 88
50ed638bbc47ba Prashant Malani 2022-12-28 @89 ret = typec_altmode_vdm(amode, hdr, &resp.vdm_response[1], resp.vdm_data_objects);
50ed638bbc47ba Prashant Malani 2022-12-28 90 if (ret)
50ed638bbc47ba Prashant Malani 2022-12-28 91 dev_err(typec->dev, "Failed to forward VDM to altmode (SVID:%x), port: %d\n",
50ed638bbc47ba Prashant Malani 2022-12-28 92 svid, port_num);
50ed638bbc47ba Prashant Malani 2022-12-28 93 }
50ed638bbc47ba Prashant Malani 2022-12-28 94

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-14 23:03:15

by RD Babiera

[permalink] [raw]

Subject: Re: [PATCH v1 01/10] usb: typec: bus: provide transmit type for alternate mode drivers

Hi Heikki,

On Thu, Dec 7, 2023 at 3:34 AM Heikki Krogerus
<[email protected]> wrote:
> Instead of forcing this change immediately on every existing user of
> that API, why not supply separate API for the cable alt modes?
>
> Although the SOP* communication is the same in most parts, at least
> Attention (and probable some other messages too) is not valid with
> cable plugs. So maybe it would be more clear to just separate SOP
> communication from SOP Prime/Double Prime in the API?

Your idea makes much more sense and simplified implementation, so
I added it to the next version of the patch set.

Thanks a lot!
---
RD