Re: [PATCH 02/14 v2] usb: typec: tcpci: Add support when hidden tx registers are inaccessible

On Fri, Aug 28, 2020 at 6:12 AM Heikki Krogerus
[off-list ref] wrote:

Looks like you forgot v1 somewhere :-)

Dumb me :P Yeah I was doing the patch numbers manually causing the confusion.
Greg was complaining as well.
These are all v1. I will re-upload everything as v2. Just noticed that
I can use the -v option.

On Mon, Aug 24, 2020 at 09:21:58PM -0700, Badhri Jagan Sridharan wrote:

quoted

TCPCI spec forbids direct access of TX_BUF_BYTE_x register.
The existing version of tcpci driver assumes that those registers
are directly addressible. Add support for tcpci chips which do
not support direct access to TX_BUF_BYTE_x registers. TX_BUF_BYTE_x
can only be accessed by I2C_WRITE_BYTE_COUNT.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpci.c | 49 +++++++++++++++++++++++++---------
 drivers/usb/typec/tcpm/tcpci.h |  8 ++++++
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index f57d91fd0e09..90d348caa6a8 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c

@@ -320,8 +320,7 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
      return 0;
 }

-static int tcpci_pd_transmit(struct tcpc_dev *tcpc,
-                          enum tcpm_transmit_type type,
+static int tcpci_pd_transmit(struct tcpc_dev *tcpc, enum tcpm_transmit_type type,
                           const struct pd_message *msg)

That does not look like a relevant change.

quoted

 {
      struct tcpci *tcpci = tcpc_to_tcpci(tcpc);

@@ -330,23 +329,47 @@ static int tcpci_pd_transmit(struct tcpc_dev *tcpc,
      int ret;

      cnt = msg ? pd_header_cnt(header) * 4 : 0;
-     ret = regmap_write(tcpci->regmap, TCPC_TX_BYTE_CNT, cnt + 2);
-     if (ret < 0)
-             return ret;
+     /**
+      * TCPCI spec forbids direct access of TCPC_TX_DATA.
+      * But, since some of the chipsets offer this capability,
+      * it's fair to support both.
+      */
+     if (!tcpci->data->TX_BUF_BYTE_x_hidden) {

Couldn't check if the flag is set first?

Yes will do !

        if (tcpci->data->TX_BUF_BYTE_x_hidden) {
                ...

quoted

+             ret = regmap_write(tcpci->regmap, TCPC_TX_BYTE_CNT, cnt + 2);
+             if (ret < 0)
+                     return ret;

-     ret = tcpci_write16(tcpci, TCPC_TX_HDR, header);
-     if (ret < 0)
-             return ret;
+             ret = tcpci_write16(tcpci, TCPC_TX_HDR, header);
+             if (ret < 0)
+                     return ret;
+
+             if (cnt > 0) {
+                     ret = regmap_raw_write(tcpci->regmap, TCPC_TX_DATA, &msg->payload, cnt);
+                     if (ret < 0)
+                             return ret;
+             }
+     } else {
+             u8 buf[TCPC_TRANSMIT_BUFFER_MAX_LEN] = {0,};
+             u8 pos = 0;
+
+             /* Payload + header + TCPC_TX_BYTE_CNT */
+             buf[pos++] = cnt + 2;
+
+             if (msg)
+                     memcpy(&buf[pos], &msg->header, sizeof(msg->header));
+
+             pos += sizeof(header);
+
+             if (cnt > 0)
+                     memcpy(&buf[pos], msg->payload, cnt);

-     if (cnt > 0) {
-             ret = regmap_raw_write(tcpci->regmap, TCPC_TX_DATA,
-                                    &msg->payload, cnt);
+             pos += cnt;
+             ret = regmap_raw_write(tcpci->regmap, TCPC_TX_BYTE_CNT, buf, pos);
              if (ret < 0)
                      return ret;
      }

-     reg = (PD_RETRY_COUNT << TCPC_TRANSMIT_RETRY_SHIFT) |
-             (type << TCPC_TRANSMIT_TYPE_SHIFT);
+     reg = (PD_RETRY_COUNT << TCPC_TRANSMIT_RETRY_SHIFT) | (type << TCPC_TRANSMIT_TYPE_SHIFT);
      ret = regmap_write(tcpci->regmap, TCPC_TRANSMIT, reg);
      if (ret < 0)
              return ret;

diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
index fd26ca35814c..cf9d8b63adcb 100644
--- a/drivers/usb/typec/tcpm/tcpci.h
+++ b/drivers/usb/typec/tcpm/tcpci.h

@@ -128,9 +128,17 @@
 #define TCPC_VBUS_VOLTAGE_ALARM_HI_CFG               0x76
 #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG               0x78

+/* I2C_WRITE_BYTE_COUNT + 1 when TX_BUF_BYTE_x is only accessible I2C_WRITE_BYTE_COUNT */
+#define TCPC_TRANSMIT_BUFFER_MAX_LEN         31
+
+/*
+ * @TX_BUF_BYTE_x_hidden
+ *           optional; Set when TX_BUF_BYTE_x can only be accessed through I2C_WRITE_BYTE_COUNT.
+ */
 struct tcpci;
 struct tcpci_data {
      struct regmap *regmap;
+     unsigned char TX_BUF_BYTE_x_hidden:1;
      int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
      int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
                       bool enable);
--

2.28.0.297.g1956fa8f8d-goog

thanks,

--
heikki