Hi,
On Mon, Dec 02, 2024 at 03:06:45AM +0000, Miao Zhu wrote:
> The tcpci driver doesn't fully follow the TCPCI spec even if
> it mentions this spec in its comments.
> - Add two flags into tcpci_data:
> RX_BUF_BYTE_x_hidden
> conn_present_capable
> - Following flags in tcpci_data are read from device tree in tcpci_probe.
> TX_BUF_BYTE_x_hidden
> RX_BUF_BYTE_x_hidden
> auto_discharge_disconnect
> vbus_vsafe0v
> The change makes the driver be compatible with the TCPCI spec and
> therefore won't impact existing HW.
>
> Signed-off-by: Miao Zhu <Miao.Zhu@xxxxxxxxxxxx>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 115 +++++++++++++++++++++++++++------
> include/linux/usb/tcpci.h | 11 ++++
> 2 files changed, 106 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ed32583829be..e4a885fa38b5 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -453,19 +453,26 @@ static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
> enum typec_role role, enum typec_data_role data)
> {
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> - unsigned int reg;
> + unsigned int reg = 0;
> int ret;
>
> - reg = FIELD_PREP(TCPC_MSG_HDR_INFO_REV, PD_REV20);
> - if (role == TYPEC_SOURCE)
> - reg |= TCPC_MSG_HDR_INFO_PWR_ROLE;
> - if (data == TYPEC_HOST)
> - reg |= TCPC_MSG_HDR_INFO_DATA_ROLE;
> + if (attached) {
> + reg = FIELD_PREP(TCPC_MSG_HDR_INFO_REV, PD_REV20);
> + if (role == TYPEC_SOURCE)
> + reg |= TCPC_MSG_HDR_INFO_PWR_ROLE;
> + if (data == TYPEC_HOST)
> + reg |= TCPC_MSG_HDR_INFO_DATA_ROLE;
> + }
> ret = regmap_write(tcpci->regmap, TCPC_MSG_HDR_INFO, reg);
> if (ret < 0)
> return ret;
>
> - return 0;
> + if (tcpci->data->conn_present_capable)
> + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> + TCPC_CONFIG_STD_OUTPUT_CON_PRES,
> + attached ? TCPC_CONFIG_STD_OUTPUT_CON_PRES : 0);
> + else
> + return 0;
> }
>
> static int tcpci_set_pd_rx(struct tcpc_dev *tcpc, bool enable)
> @@ -741,33 +748,86 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
> struct pd_message msg;
> unsigned int cnt, payload_cnt;
> u16 header;
> + unsigned int frame_type;
> + enum tcpm_transmit_type rx_type;
>
> regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> /*
> * 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
> * of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
> * defined in table 4-36 as one greater than the number of
> - * bytes received. And that number includes the header. So:
> + * bytes received. And that number includes the header.
> + * In Section 4.4.14 of the TCPCI spec [Rev 2.0 Ver 1.0 October, 2017],
> + * the RECEIVE_BUFFER comprises of three sets of registers:
> + * READABLE_BYTE_COUNT, RX_BUF_FRAME_TYPE and RX_BUF_BYTE_x.
> + * These registers can only be accessed by reading at a common
> + * register address 0x30h.
> */
> - if (cnt > 3)
> - payload_cnt = cnt - (1 + sizeof(msg.header));
> - else
> - payload_cnt = 0;
> + if (tcpci->data->TX_BUF_BYTE_x_hidden) {
use RX_BUF_BYTE_x_hidden for RX?
> + u8 buf[TCPC_RECEIVE_BUFFER_MAX_LEN] = {0,};
> + u8 pos = 0;
> +
> + /* Read the count and frame type in RECEIVE_BUFFER */
> + regmap_raw_read(tcpci->regmap, TCPC_RX_BYTE_CNT, buf, 2);
> + /* READABLE_BYTE_COUNT */
> + cnt = buf[0];
> + /* RX_BUF_FRAME_TYPE */
> + frame_type = buf[1];
> +
> + /* Read the content of the USB PD message in RECEIVE_BUFFER */
> + regmap_raw_read(tcpci->regmap, TCPC_RX_BYTE_CNT, buf, cnt + 1);
> +
> + pos += 2;
> + memcpy(&msg.header, &buf[pos], sizeof(msg.header));
> +
> + if (cnt > 3) {
> + pos += sizeof(msg.header);
> + payload_cnt = cnt - (1 + sizeof(msg.header));
> + if (WARN_ON(payload_cnt > sizeof(msg.payload)))
> + payload_cnt = sizeof(msg.payload);
> + memcpy(&msg.payload, &buf[pos], payload_cnt);
> + }
> + } else {
> + regmap_read(tcpci->regmap, TCPC_RX_BYTE_CNT, &cnt);
> + /*
> + * 'cnt' corresponds to READABLE_BYTE_COUNT in section 4.4.14
> + * of the TCPCI spec [Rev 2.0 Ver 1.0 October 2017] and is
> + * defined in table 4-36 as one greater than the number of
> + * bytes received. And that number includes the header. So:
> + */
> + if (cnt > 3)
> + payload_cnt = cnt - (1 + sizeof(msg.header));
> + else
> + payload_cnt = 0;
>
> - tcpci_read16(tcpci, TCPC_RX_HDR, &header);
> - msg.header = cpu_to_le16(header);
> + regmap_read(tcpci->regmap, TCPC_RX_BUF_FRAME_TYPE, &frame_type);
>
> - if (WARN_ON(payload_cnt > sizeof(msg.payload)))
> - payload_cnt = sizeof(msg.payload);
> + tcpci_read16(tcpci, TCPC_RX_HDR, &header);
> + msg.header = cpu_to_le16(header);
>
> - if (payload_cnt > 0)
> - regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
> - &msg.payload, payload_cnt);
> + if (WARN_ON(payload_cnt > sizeof(msg.payload)))
> + payload_cnt = sizeof(msg.payload);
> +
> + if (payload_cnt > 0)
> + regmap_raw_read(tcpci->regmap, TCPC_RX_DATA,
> + &msg.payload, payload_cnt);
> + }
>
> /* Read complete, clear RX status alert bit */
> tcpci_write16(tcpci, TCPC_ALERT, TCPC_ALERT_RX_STATUS);
>
> - tcpm_pd_receive(tcpci->port, &msg, TCPC_TX_SOP);
> + switch (frame_type) {
> + case TCPC_RX_BUF_FRAME_TYPE_SOP1:
> + rx_type = TCPC_TX_SOP_PRIME;
> + break;
> + case TCPC_RX_BUF_FRAME_TYPE_SOP:
> + rx_type = TCPC_TX_SOP;
> + break;
> + default:
> + rx_type = TCPC_TX_SOP;
> + break;
> + }
> + tcpm_pd_receive(tcpci->port, &msg, rx_type);
> }
>
> if (tcpci->data->vbus_vsafe0v && (status & TCPC_ALERT_EXTENDED_STATUS)) {
> @@ -916,6 +976,21 @@ static int tcpci_probe(struct i2c_client *client)
> if (err < 0)
> return err;
>
> + chip->data.TX_BUF_BYTE_x_hidden =
> + device_property_read_bool(&client->dev, "TX_BUF_BYTE_x_hidden");
> + chip->data.RX_BUF_BYTE_x_hidden =
> + device_property_read_bool(&client->dev, "RX_BUF_BYTE_x_hidden");
> + chip->data.auto_discharge_disconnect =
> + device_property_read_bool(&client->dev, "auto_discharge_disconnect");
> + chip->data.vbus_vsafe0v = device_property_read_bool(&client->dev, "vbus_vsafe0v");
Please also document these properties in dt-bindings.
> +
> + err = tcpci_check_std_output_cap(chip->data.regmap,
> + TCPC_STD_OUTPUT_CAP_CONN_PRESENT);
> + if (err < 0)
> + return err;
> +
> + chip->data.conn_present_capable = err;
> +
> err = tcpci_check_std_output_cap(chip->data.regmap,
> TCPC_STD_OUTPUT_CAP_ORIENTATION);
> if (err < 0)
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index f7f5cfbdef12..0760187ea4b5 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -50,6 +50,7 @@
> #define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> #define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> #define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
> +#define TCPC_CONFIG_STD_OUTPUT_CON_PRES BIT(1)
>
> #define TCPC_TCPC_CTRL 0x19
> #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> @@ -126,6 +127,7 @@
> #define TCPC_STD_INPUT_CAP 0x28
> #define TCPC_STD_OUTPUT_CAP 0x29
> #define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
> +#define TCPC_STD_OUTPUT_CAP_CONN_PRESENT BIT(1)
>
> #define TCPC_MSG_HDR_INFO 0x2e
> #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> @@ -167,6 +169,7 @@
>
> /* I2C_WRITE_BYTE_COUNT + 1 when TX_BUF_BYTE_x is only accessible I2C_WRITE_BYTE_COUNT */
> #define TCPC_TRANSMIT_BUFFER_MAX_LEN 31
> +#define TCPC_RECEIVE_BUFFER_MAX_LEN 32
>
> #define tcpc_presenting_rd(reg, cc) \
> (!(TCPC_ROLE_CTRL_DRP & (reg)) && \
> @@ -177,6 +180,9 @@ struct tcpci;
> /*
> * @TX_BUF_BYTE_x_hidden:
> * optional; Set when TX_BUF_BYTE_x can only be accessed through I2C_WRITE_BYTE_COUNT.
> + * @RX_BUF_BYTE_x_hidden:
> + * Optional; Set when READABLE_BYTE_COUNT, RX_BUF_FRAME_TYPE and RX_BUF_BYTE_x
> + * can only be accessed through READABLE_BYTE_COUNT.
> * @frs_sourcing_vbus:
> * Optional; Callback to perform chip specific operations when FRS
> * is sourcing vbus.
> @@ -204,6 +210,9 @@ struct tcpci;
> * swap following Discover Identity on SOP' occurs.
> * Return true when the TCPM is allowed to request a Vconn swap
> * after Discovery Identity on SOP.
> + * @conn_present_capable:
> + * Optional; Enable setting the connection present
> + * CONFIG_STANDARD_OUTPUT (0x18) bit1.
> * @set_orientation:
> * Optional; Enable setting the connector orientation
> * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> @@ -211,9 +220,11 @@ struct tcpci;
> struct tcpci_data {
> struct regmap *regmap;
> unsigned char TX_BUF_BYTE_x_hidden:1;
> + unsigned char RX_BUF_BYTE_x_hidden:1;
> unsigned char auto_discharge_disconnect:1;
> unsigned char vbus_vsafe0v:1;
> unsigned char cable_comm_capable:1;
> + unsigned char conn_present_capable:1;
> unsigned char set_orientation:1;
>
> int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> --
> 2.27.0
>
