diff mbox series

[v6,02/14] usb: typec: tcpci: Add support when hidden tx registers are inaccessible

Message ID 20200901025927.3596190-3-badhri@google.com
State New
Headers show
Series TCPM support for FRS and AutoDischarge Disconnect | expand

Commit Message

Badhri Jagan Sridharan Sept. 1, 2020, 2:59 a.m. UTC
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>
---
Changes since v1:
 - Refactored the code to check for TX_BUF_BYTE_x_hidden as suggested by
   Heikki.
 - Not formatting the tcpci_pd_transmit to follow the 100 character/line
   limit as suggested by Heikki. (Should be a separate change).
 - Changing patch version to v6 to fix version number confusion.
---
 drivers/usb/typec/tcpm/tcpci.c | 46 ++++++++++++++++++++++++++--------
 drivers/usb/typec/tcpm/tcpci.h |  8 ++++++
 2 files changed, 43 insertions(+), 11 deletions(-)

Comments

Heikki Krogerus Sept. 15, 2020, 12:11 p.m. UTC | #1
On Mon, Aug 31, 2020 at 07:59:15PM -0700, Badhri Jagan Sridharan wrote:
> 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>


Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


> ---

> Changes since v1:

>  - Refactored the code to check for TX_BUF_BYTE_x_hidden as suggested by

>    Heikki.

>  - Not formatting the tcpci_pd_transmit to follow the 100 character/line

>    limit as suggested by Heikki. (Should be a separate change).

>  - Changing patch version to v6 to fix version number confusion.

> ---

>  drivers/usb/typec/tcpm/tcpci.c | 46 ++++++++++++++++++++++++++--------

>  drivers/usb/typec/tcpm/tcpci.h |  8 ++++++

>  2 files changed, 43 insertions(+), 11 deletions(-)

> 

> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c

> index bd80e03b2b6f..7d36d5e2d3f7 100644

> --- a/drivers/usb/typec/tcpm/tcpci.c

> +++ b/drivers/usb/typec/tcpm/tcpci.c

> @@ -330,23 +330,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) {

> +		u8 buf[TCPC_TRANSMIT_BUFFER_MAX_LEN] = {0,};

> +		u8 pos = 0;

>  

> -	ret = tcpci_write16(tcpci, TCPC_TX_HDR, header);

> -	if (ret < 0)

> -		return ret;

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

> -		ret = regmap_raw_write(tcpci->regmap, TCPC_TX_DATA,

> -				       &msg->payload, cnt);

> +		if (cnt > 0)

> +			memcpy(&buf[pos], msg->payload, cnt);

> +

> +		pos += cnt;

> +		ret = regmap_raw_write(tcpci->regmap, TCPC_TX_BYTE_CNT, buf, pos);

> +		if (ret < 0)

> +			return ret;

> +	} else {

> +		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;

> +

> +		if (cnt > 0) {

> +			ret = regmap_raw_write(tcpci->regmap, TCPC_TX_DATA, &msg->payload, cnt);

> +			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.402.g5ffc5be6b7-goog


-- 
heikki
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index bd80e03b2b6f..7d36d5e2d3f7 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -330,23 +330,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) {
+		u8 buf[TCPC_TRANSMIT_BUFFER_MAX_LEN] = {0,};
+		u8 pos = 0;
 
-	ret = tcpci_write16(tcpci, TCPC_TX_HDR, header);
-	if (ret < 0)
-		return ret;
+		/* 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) {
-		ret = regmap_raw_write(tcpci->regmap, TCPC_TX_DATA,
-				       &msg->payload, cnt);
+		if (cnt > 0)
+			memcpy(&buf[pos], msg->payload, cnt);
+
+		pos += cnt;
+		ret = regmap_raw_write(tcpci->regmap, TCPC_TX_BYTE_CNT, buf, pos);
+		if (ret < 0)
+			return ret;
+	} else {
+		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;
+
+		if (cnt > 0) {
+			ret = regmap_raw_write(tcpci->regmap, TCPC_TX_DATA, &msg->payload, cnt);
+			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);