* [PATCH v3 0/2] usb: typec: ucsi_glink: Add support UCSI v2 @ 2025-08-27 20:12 Anjelique Melendez 2025-08-27 20:12 ` [PATCH v3 1/2] usb: typec: ucsi_glink: Update request/response buffers to be packed Anjelique Melendez 2025-08-27 20:12 ` [PATCH v3 2/2] usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 Anjelique Melendez 0 siblings, 2 replies; 5+ messages in thread From: Anjelique Melendez @ 2025-08-27 20:12 UTC (permalink / raw) To: heikki.krogerus, gregkh Cc: lumag, neil.armstrong, johan+linaro, quic_bjorande, linux-usb, linux-kernel, linux-arm-msm UCSI v2 specification has increased the MSG_IN and MSG_OUT size from 16 bytes to 256 bytes each for the message exchange between OPM and PPM This makes the total buffer size increase from 48 bytes to 528 bytes. Update the buffer size to support this increase. While at it also update the UCSI read/request buffers to be packed. Changes since v2: - Added "usb: typec: ucsi_glink: Update request/response buffers to be packed" patch - Added length checks - Updated version checks to use UCSI_VERSION_2_0 instead of UCSI_VERSION_2_1 - link: https://lore.kernel.org/all/20250716005224.312155-1-anjelique.melendez@oss.qualcomm.com/ Changes since v1: - Defined buf size in terms of other UCSI defines - Removed UCSI_BUF_SIZE and used the explicit v1 or v2 buffer size macros - Removed Qualcomm copyright - link: https://lore.kernel.org/all/20250624222922.2010820-1-anjelique.melendez@oss.qualcomm.com/ Anjelique Melendez (2): usb: typec: ucsi_glink: Update request/response buffers to be packed usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 drivers/usb/typec/ucsi/ucsi_glink.c | 93 ++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 14 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] usb: typec: ucsi_glink: Update request/response buffers to be packed 2025-08-27 20:12 [PATCH v3 0/2] usb: typec: ucsi_glink: Add support UCSI v2 Anjelique Melendez @ 2025-08-27 20:12 ` Anjelique Melendez 2025-08-28 10:16 ` Heikki Krogerus 2025-08-27 20:12 ` [PATCH v3 2/2] usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 Anjelique Melendez 1 sibling, 1 reply; 5+ messages in thread From: Anjelique Melendez @ 2025-08-27 20:12 UTC (permalink / raw) To: heikki.krogerus, gregkh Cc: lumag, neil.armstrong, johan+linaro, quic_bjorande, linux-usb, linux-kernel, linux-arm-msm Update the ucsi request/response buffers to be packed to ensure there are no "holes" in memory while we read/write these structs. Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com> --- drivers/usb/typec/ucsi/ucsi_glink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index 8af79101a2fc..1f9f0d942c1a 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -30,24 +30,24 @@ struct ucsi_read_buf_req_msg { struct pmic_glink_hdr hdr; }; -struct ucsi_read_buf_resp_msg { +struct __packed ucsi_read_buf_resp_msg { struct pmic_glink_hdr hdr; u8 buf[UCSI_BUF_SIZE]; u32 ret_code; }; -struct ucsi_write_buf_req_msg { +struct __packed ucsi_write_buf_req_msg { struct pmic_glink_hdr hdr; u8 buf[UCSI_BUF_SIZE]; u32 reserved; }; -struct ucsi_write_buf_resp_msg { +struct __packed ucsi_write_buf_resp_msg { struct pmic_glink_hdr hdr; u32 ret_code; }; -struct ucsi_notify_ind_msg { +struct __packed ucsi_notify_ind_msg { struct pmic_glink_hdr hdr; u32 notification; u32 receiver; -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] usb: typec: ucsi_glink: Update request/response buffers to be packed 2025-08-27 20:12 ` [PATCH v3 1/2] usb: typec: ucsi_glink: Update request/response buffers to be packed Anjelique Melendez @ 2025-08-28 10:16 ` Heikki Krogerus 0 siblings, 0 replies; 5+ messages in thread From: Heikki Krogerus @ 2025-08-28 10:16 UTC (permalink / raw) To: Anjelique Melendez Cc: gregkh, lumag, neil.armstrong, johan+linaro, quic_bjorande, linux-usb, linux-kernel, linux-arm-msm On Wed, Aug 27, 2025 at 01:12:40PM -0700, Anjelique Melendez wrote: > Update the ucsi request/response buffers to be packed to ensure there > are no "holes" in memory while we read/write these structs. > > Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/ucsi/ucsi_glink.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c > index 8af79101a2fc..1f9f0d942c1a 100644 > --- a/drivers/usb/typec/ucsi/ucsi_glink.c > +++ b/drivers/usb/typec/ucsi/ucsi_glink.c > @@ -30,24 +30,24 @@ struct ucsi_read_buf_req_msg { > struct pmic_glink_hdr hdr; > }; > > -struct ucsi_read_buf_resp_msg { > +struct __packed ucsi_read_buf_resp_msg { > struct pmic_glink_hdr hdr; > u8 buf[UCSI_BUF_SIZE]; > u32 ret_code; > }; > > -struct ucsi_write_buf_req_msg { > +struct __packed ucsi_write_buf_req_msg { > struct pmic_glink_hdr hdr; > u8 buf[UCSI_BUF_SIZE]; > u32 reserved; > }; > > -struct ucsi_write_buf_resp_msg { > +struct __packed ucsi_write_buf_resp_msg { > struct pmic_glink_hdr hdr; > u32 ret_code; > }; > > -struct ucsi_notify_ind_msg { > +struct __packed ucsi_notify_ind_msg { > struct pmic_glink_hdr hdr; > u32 notification; > u32 receiver; > -- > 2.34.1 -- heikki ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 2025-08-27 20:12 [PATCH v3 0/2] usb: typec: ucsi_glink: Add support UCSI v2 Anjelique Melendez 2025-08-27 20:12 ` [PATCH v3 1/2] usb: typec: ucsi_glink: Update request/response buffers to be packed Anjelique Melendez @ 2025-08-27 20:12 ` Anjelique Melendez 2025-08-28 11:05 ` Heikki Krogerus 1 sibling, 1 reply; 5+ messages in thread From: Anjelique Melendez @ 2025-08-27 20:12 UTC (permalink / raw) To: heikki.krogerus, gregkh Cc: lumag, neil.armstrong, johan+linaro, quic_bjorande, linux-usb, linux-kernel, linux-arm-msm UCSI v2 specification has increased the MSG_IN and MSG_OUT size from 16 bytes to 256 bytes each for the message exchange between OPM and PPM This makes the total buffer size increase from 48 bytes to 528 bytes. Update the buffer size to support this increase. Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com> --- drivers/usb/typec/ucsi/ucsi_glink.c | 85 +++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 10 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c index 1f9f0d942c1a..fc12569ec520 100644 --- a/drivers/usb/typec/ucsi/ucsi_glink.c +++ b/drivers/usb/typec/ucsi/ucsi_glink.c @@ -16,10 +16,10 @@ #define PMIC_GLINK_MAX_PORTS 3 -#define UCSI_BUF_SIZE 48 +#define UCSI_BUF_V1_SIZE (UCSI_MESSAGE_OUT + (UCSI_MESSAGE_OUT - UCSI_MESSAGE_IN)) +#define UCSI_BUF_V2_SIZE (UCSIv2_MESSAGE_OUT + (UCSIv2_MESSAGE_OUT - UCSI_MESSAGE_IN)) #define MSG_TYPE_REQ_RESP 1 -#define UCSI_BUF_SIZE 48 #define UC_NOTIFY_RECEIVER_UCSI 0x0 #define UC_UCSI_READ_BUF_REQ 0x11 @@ -32,13 +32,25 @@ struct ucsi_read_buf_req_msg { struct __packed ucsi_read_buf_resp_msg { struct pmic_glink_hdr hdr; - u8 buf[UCSI_BUF_SIZE]; + u8 buf[UCSI_BUF_V1_SIZE]; + u32 ret_code; +}; + +struct __packed ucsi_v2_read_buf_resp_msg { + struct pmic_glink_hdr hdr; + u8 buf[UCSI_BUF_V2_SIZE]; u32 ret_code; }; struct __packed ucsi_write_buf_req_msg { struct pmic_glink_hdr hdr; - u8 buf[UCSI_BUF_SIZE]; + u8 buf[UCSI_BUF_V1_SIZE]; + u32 reserved; +}; + +struct __packed ucsi_v2_write_buf_req_msg { + struct pmic_glink_hdr hdr; + u8 buf[UCSI_BUF_V2_SIZE]; u32 reserved; }; @@ -72,7 +84,7 @@ struct pmic_glink_ucsi { bool ucsi_registered; bool pd_running; - u8 read_buf[UCSI_BUF_SIZE]; + u8 read_buf[UCSI_BUF_V2_SIZE]; }; static int pmic_glink_ucsi_read(struct ucsi *__ucsi, unsigned int offset, @@ -131,18 +143,34 @@ static int pmic_glink_ucsi_read_message_in(struct ucsi *ucsi, void *val, size_t static int pmic_glink_ucsi_locked_write(struct pmic_glink_ucsi *ucsi, unsigned int offset, const void *val, size_t val_len) { - struct ucsi_write_buf_req_msg req = {}; + struct ucsi_v2_write_buf_req_msg req = {}; + size_t len, max_buf_len; unsigned long left; int ret; req.hdr.owner = PMIC_GLINK_OWNER_USBC; req.hdr.type = MSG_TYPE_REQ_RESP; req.hdr.opcode = UC_UCSI_WRITE_BUF_REQ; + + if (ucsi->ucsi->version >= UCSI_VERSION_2_0) { + max_buf_len = UCSI_BUF_V2_SIZE; + len = sizeof(req); + } else if (ucsi->ucsi->version) { + max_buf_len = UCSI_BUF_V1_SIZE; + len = sizeof(struct ucsi_write_buf_req_msg); + } else { + dev_err(ucsi->dev, "UCSI version not set\n"); + return -EINVAL; + } + + if (offset + val_len > max_buf_len) + return -EINVAL; + memcpy(&req.buf[offset], val, val_len); reinit_completion(&ucsi->write_ack); - ret = pmic_glink_send(ucsi->client, &req, sizeof(req)); + ret = pmic_glink_send(ucsi->client, &req, len); if (ret < 0) { dev_err(ucsi->dev, "failed to send UCSI write request: %d\n", ret); return ret; @@ -216,12 +244,49 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = { static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len) { - const struct ucsi_read_buf_resp_msg *resp = data; + u32 ret_code, buf_len, max_len; + u8 *buf; + + if (ucsi->ucsi->version) { + if (ucsi->ucsi->version >= UCSI_VERSION_2_0) { + max_len = sizeof(struct ucsi_v2_read_buf_resp_msg); + buf = ((struct ucsi_v2_read_buf_resp_msg *)data)->buf; + buf_len = UCSI_BUF_V2_SIZE; + } else { + max_len = sizeof(struct ucsi_read_buf_resp_msg); + buf = ((struct ucsi_read_buf_resp_msg *)data)->buf; + buf_len = UCSI_BUF_V1_SIZE; + } + } else if (!ucsi->ucsi->version && !ucsi->ucsi_registered) { + /* + * If UCSI version is not known yet because device is not registered, choose buffer + * size which best fits incoming data + */ + if (len > sizeof(struct pmic_glink_hdr) + UCSI_BUF_V2_SIZE) { + max_len = sizeof(struct ucsi_v2_read_buf_resp_msg); + buf = ((struct ucsi_v2_read_buf_resp_msg *)data)->buf; + buf_len = UCSI_BUF_V2_SIZE; + } else { + max_len = sizeof(struct ucsi_read_buf_resp_msg); + buf = ((struct ucsi_read_buf_resp_msg *)data)->buf; + buf_len = UCSI_BUF_V1_SIZE; + } + } else { + dev_err(ucsi->dev, "UCSI version not set\n"); + return; + } - if (resp->ret_code) + if (len > max_len) + return; + + if (buf_len > len - sizeof(struct pmic_glink_hdr) - sizeof(u32)) + buf_len = len - sizeof(struct pmic_glink_hdr) - sizeof(u32); + + memcpy(&ret_code, buf + sizeof(struct pmic_glink_hdr) + buf_len, sizeof(u32)); + if (ret_code) return; - memcpy(ucsi->read_buf, resp->buf, UCSI_BUF_SIZE); + memcpy(ucsi->read_buf, buf, buf_len); complete(&ucsi->read_ack); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 2025-08-27 20:12 ` [PATCH v3 2/2] usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 Anjelique Melendez @ 2025-08-28 11:05 ` Heikki Krogerus 0 siblings, 0 replies; 5+ messages in thread From: Heikki Krogerus @ 2025-08-28 11:05 UTC (permalink / raw) To: Anjelique Melendez Cc: gregkh, lumag, neil.armstrong, johan+linaro, quic_bjorande, linux-usb, linux-kernel, linux-arm-msm On Wed, Aug 27, 2025 at 01:12:41PM -0700, Anjelique Melendez wrote: > UCSI v2 specification has increased the MSG_IN and MSG_OUT size from > 16 bytes to 256 bytes each for the message exchange between OPM and PPM > This makes the total buffer size increase from 48 bytes to 528 bytes. > Update the buffer size to support this increase. > > Signed-off-by: Anjelique Melendez <anjelique.melendez@oss.qualcomm.com> > --- > drivers/usb/typec/ucsi/ucsi_glink.c | 85 +++++++++++++++++++++++++---- > 1 file changed, 75 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c > index 1f9f0d942c1a..fc12569ec520 100644 > --- a/drivers/usb/typec/ucsi/ucsi_glink.c > +++ b/drivers/usb/typec/ucsi/ucsi_glink.c > @@ -16,10 +16,10 @@ > > #define PMIC_GLINK_MAX_PORTS 3 > > -#define UCSI_BUF_SIZE 48 > +#define UCSI_BUF_V1_SIZE (UCSI_MESSAGE_OUT + (UCSI_MESSAGE_OUT - UCSI_MESSAGE_IN)) > +#define UCSI_BUF_V2_SIZE (UCSIv2_MESSAGE_OUT + (UCSIv2_MESSAGE_OUT - UCSI_MESSAGE_IN)) > > #define MSG_TYPE_REQ_RESP 1 > -#define UCSI_BUF_SIZE 48 > > #define UC_NOTIFY_RECEIVER_UCSI 0x0 > #define UC_UCSI_READ_BUF_REQ 0x11 > @@ -32,13 +32,25 @@ struct ucsi_read_buf_req_msg { > > struct __packed ucsi_read_buf_resp_msg { > struct pmic_glink_hdr hdr; > - u8 buf[UCSI_BUF_SIZE]; > + u8 buf[UCSI_BUF_V1_SIZE]; > + u32 ret_code; > +}; > + > +struct __packed ucsi_v2_read_buf_resp_msg { > + struct pmic_glink_hdr hdr; > + u8 buf[UCSI_BUF_V2_SIZE]; > u32 ret_code; > }; Couldn't you just make a union inside that original struct ucsi_read_buf_resp_msg? > struct __packed ucsi_write_buf_req_msg { > struct pmic_glink_hdr hdr; > - u8 buf[UCSI_BUF_SIZE]; > + u8 buf[UCSI_BUF_V1_SIZE]; > + u32 reserved; > +}; > + > +struct __packed ucsi_v2_write_buf_req_msg { > + struct pmic_glink_hdr hdr; > + u8 buf[UCSI_BUF_V2_SIZE]; > u32 reserved; > }; Ditto for the ucsi_write_buf_req_msg. > @@ -72,7 +84,7 @@ struct pmic_glink_ucsi { > bool ucsi_registered; > bool pd_running; > > - u8 read_buf[UCSI_BUF_SIZE]; > + u8 read_buf[UCSI_BUF_V2_SIZE]; > }; > > static int pmic_glink_ucsi_read(struct ucsi *__ucsi, unsigned int offset, > @@ -131,18 +143,34 @@ static int pmic_glink_ucsi_read_message_in(struct ucsi *ucsi, void *val, size_t > static int pmic_glink_ucsi_locked_write(struct pmic_glink_ucsi *ucsi, unsigned int offset, > const void *val, size_t val_len) > { > - struct ucsi_write_buf_req_msg req = {}; > + struct ucsi_v2_write_buf_req_msg req = {}; > + size_t len, max_buf_len; > unsigned long left; > int ret; > > req.hdr.owner = PMIC_GLINK_OWNER_USBC; > req.hdr.type = MSG_TYPE_REQ_RESP; > req.hdr.opcode = UC_UCSI_WRITE_BUF_REQ; > + > + if (ucsi->ucsi->version >= UCSI_VERSION_2_0) { > + max_buf_len = UCSI_BUF_V2_SIZE; > + len = sizeof(req); > + } else if (ucsi->ucsi->version) { > + max_buf_len = UCSI_BUF_V1_SIZE; > + len = sizeof(struct ucsi_write_buf_req_msg); > + } else { > + dev_err(ucsi->dev, "UCSI version not set\n"); > + return -EINVAL; > + } > + > + if (offset + val_len > max_buf_len) > + return -EINVAL; > + > memcpy(&req.buf[offset], val, val_len); > > reinit_completion(&ucsi->write_ack); > > - ret = pmic_glink_send(ucsi->client, &req, sizeof(req)); > + ret = pmic_glink_send(ucsi->client, &req, len); > if (ret < 0) { > dev_err(ucsi->dev, "failed to send UCSI write request: %d\n", ret); > return ret; > @@ -216,12 +244,49 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = { > > static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len) > { > - const struct ucsi_read_buf_resp_msg *resp = data; > + u32 ret_code, buf_len, max_len; > + u8 *buf; > + > + if (ucsi->ucsi->version) { > + if (ucsi->ucsi->version >= UCSI_VERSION_2_0) { > + max_len = sizeof(struct ucsi_v2_read_buf_resp_msg); > + buf = ((struct ucsi_v2_read_buf_resp_msg *)data)->buf; > + buf_len = UCSI_BUF_V2_SIZE; > + } else { > + max_len = sizeof(struct ucsi_read_buf_resp_msg); > + buf = ((struct ucsi_read_buf_resp_msg *)data)->buf; > + buf_len = UCSI_BUF_V1_SIZE; > + } > + } else if (!ucsi->ucsi->version && !ucsi->ucsi_registered) { ucsi->ucsi->version will never be set in this else condition, so no need to check it, no? > + /* > + * If UCSI version is not known yet because device is not registered, choose buffer > + * size which best fits incoming data > + */ > + if (len > sizeof(struct pmic_glink_hdr) + UCSI_BUF_V2_SIZE) { > + max_len = sizeof(struct ucsi_v2_read_buf_resp_msg); > + buf = ((struct ucsi_v2_read_buf_resp_msg *)data)->buf; > + buf_len = UCSI_BUF_V2_SIZE; > + } else { > + max_len = sizeof(struct ucsi_read_buf_resp_msg); > + buf = ((struct ucsi_read_buf_resp_msg *)data)->buf; > + buf_len = UCSI_BUF_V1_SIZE; > + } > + } else { > + dev_err(ucsi->dev, "UCSI version not set\n"); > + return; > + } I don't really see when you could enter that else statement? > - if (resp->ret_code) > + if (len > max_len) > + return; > + > + if (buf_len > len - sizeof(struct pmic_glink_hdr) - sizeof(u32)) > + buf_len = len - sizeof(struct pmic_glink_hdr) - sizeof(u32); > + > + memcpy(&ret_code, buf + sizeof(struct pmic_glink_hdr) + buf_len, sizeof(u32)); > + if (ret_code) > return; > > - memcpy(ucsi->read_buf, resp->buf, UCSI_BUF_SIZE); > + memcpy(ucsi->read_buf, buf, buf_len); > complete(&ucsi->read_ack); > } thanks, -- heikki ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-08-28 11:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-27 20:12 [PATCH v3 0/2] usb: typec: ucsi_glink: Add support UCSI v2 Anjelique Melendez 2025-08-27 20:12 ` [PATCH v3 1/2] usb: typec: ucsi_glink: Update request/response buffers to be packed Anjelique Melendez 2025-08-28 10:16 ` Heikki Krogerus 2025-08-27 20:12 ` [PATCH v3 2/2] usb: typec: ucsi_glink: Increase buffer size to support UCSI v2 Anjelique Melendez 2025-08-28 11:05 ` Heikki Krogerus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).