lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 30 Sep 2016 06:24:59 +0000
From:   Jun Li <jun.li@....com>
To:     Guenter Roeck <groeck@...omium.org>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>
CC:     Chandra Sekhar Anagani <chandra.sekhar.anagani@...el.com>,
        Bruce Ashfield <bruce.ashfield@...driver.com>,
        Bin Gao <bin.gao@...el.com>,
        Pranav Tipnis <pranav.tipnis@...el.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: RE: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface
 driver (tcpci)

Hi Guenter,

> -----Original Message-----
> From: linux-usb-owner@...r.kernel.org [mailto:linux-usb-
> owner@...r.kernel.org] On Behalf Of Guenter Roeck
> Sent: Wednesday, August 24, 2016 5:11 AM
> To: Felipe Balbi <felipe.balbi@...ux.intel.com>
> Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@...el.com>; Bruce
> Ashfield <bruce.ashfield@...driver.com>; Bin Gao <bin.gao@...el.com>;
> Pranav Tipnis <pranav.tipnis@...el.com>; Heikki Krogerus
> <heikki.krogerus@...ux.intel.com>; linux-kernel@...r.kernel.org; linux-
> usb@...r.kernel.org; Guenter Roeck <groeck@...omium.org>
> Subject: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface
> driver (tcpci)
> 
> The port controller interface driver interconnects the Type-C Port Manager
> with a Type-C Port Controller Interface (TCPCI) compliant port controller.
> 
> Signed-off-by: Guenter Roeck <groeck@...omium.org>
> ---
> v3:
> - No change
> 
> v2:
> - Adjust to modified callbacks into tcpm code
> 
>  drivers/usb/typec/Kconfig  |   9 +
>  drivers/usb/typec/Makefile |   1 +
>  drivers/usb/typec/tcpci.c  | 487
> +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/typec/tcpci.h  | 133 +++++++++++++
>  4 files changed, 630 insertions(+)
>  create mode 100644 drivers/usb/typec/tcpci.c  create mode 100644
> drivers/usb/typec/tcpci.h
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig index
> 113bb1b3589c..a92c9d1a3e00 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -25,4 +25,13 @@ config TYPEC_TCPM
>  	  The Type-C Port Controller Manager provides a USB PD and USB Type-C
>  	  state machine for use with Type-C Port Controllers.
> 
> +if TYPEC_TCPM
> +
> +config TYPEC_TCPCI
> +	tristate "Type-C Port Controller Interface driver"
> +	help
> +	  Type-C Port Controller driver for TCPCI-compliant controller.
> +
> +endif
> +
>  endmenu
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile index
> bbe45721cf52..7dbaf8c3911d 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_TYPEC)		+= typec.o
>  obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
>  obj-$(CONFIG_TYPEC_TCPM)	+= tcpm.o
> +obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
> diff --git a/drivers/usb/typec/tcpci.c b/drivers/usb/typec/tcpci.c new
> file mode 100644 index 000000000000..af338218a1f3
> --- /dev/null
> +++ b/drivers/usb/typec/tcpci.c
> @@ -0,0 +1,487 @@
> +/*
> + * Copyright 2015-2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * USB Type-C Port Controller Interface.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/regmap.h>
> +#include <linux/usb/pd.h>
> +#include <linux/usb/typec.h>
> +
> +#include "tcpci.h"
> +#include "tcpm.h"
> +
> +#define PD_RETRY_COUNT 3
> +
> +struct tcpci {
> +	struct device *dev;
> +	struct i2c_client *client;
> +
> +	struct tcpm_port *port;
> +
> +	struct regmap *regmap;
> +
> +	bool controls_vbus;
> +
> +	struct tcpc_dev tcpc;
> +};
> +
> +static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) {
> +	return container_of(tcpc, struct tcpci, tcpc); }
> +
> +static int tcpci_read16(struct tcpci *tcpci, unsigned int reg,
> +			unsigned int *val)
> +{
> +	return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16)); }
> +
> +static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16
> +val) {
> +	return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16)); }
> +
> +static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> +{
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg;
> +	int ret;
> +
> +	switch (cc) {
> +	case TYPEC_CC_RA:
> +		reg = (TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +			(TCPC_ROLE_CTRL_CC_RA << TCPC_ROLE_CTRL_CC2_SHIFT);
> +		break;
> +	case TYPEC_CC_RD:
> +		reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +			(TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
> +		break;
> +	case TYPEC_CC_RP_DEF:
> +		reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +			(TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
> +			(TCPC_ROLE_CTRL_RP_VAL_DEF <<
> +			 TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> +		break;
> +	case TYPEC_CC_RP_1_5:
> +		reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +			(TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
> +			(TCPC_ROLE_CTRL_RP_VAL_1_5 <<
> +			 TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> +		break;
> +	case TYPEC_CC_RP_3_0:
> +		reg = (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +			(TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT) |
> +			(TCPC_ROLE_CTRL_RP_VAL_1_5 <<
> +			 TCPC_ROLE_CTRL_RP_VAL_SHIFT);
> +		break;
> +	case TYPEC_CC_OPEN:
> +	default:
> +		reg = (TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT) |
> +			(TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT);
> +		break;
> +	}
> +
> +	ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool
> +sink) {
> +	switch (cc) {
> +	case 0x1:
> +		return sink ? TYPEC_CC_RP_DEF : TYPEC_CC_RA;
> +	case 0x2:
> +		return sink ? TYPEC_CC_RP_1_5 : TYPEC_CC_RD;
> +	case 0x3:
> +		if (sink)
> +			return TYPEC_CC_RP_3_0;
> +	case 0x0:
> +	default:
> +		return TYPEC_CC_OPEN;
> +	}
> +}
> +
> +static int tcpci_get_cc(struct tcpc_dev *tcpc,
> +			enum typec_cc_status *cc1, enum typec_cc_status *cc2) {
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(tcpci->regmap, TCPC_CC_STATUS, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	*cc1 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC1_SHIFT) &
> +				 TCPC_CC_STATUS_CC1_MASK,
> +				 reg & TCPC_CC_STATUS_TERM);
> +	*cc2 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC2_SHIFT) &
> +				 TCPC_CC_STATUS_CC2_MASK,
> +				 reg & TCPC_CC_STATUS_TERM);
> +
> +	return 0;
> +}
> +
> +static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> +			      enum typec_cc_polarity polarity) {
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	int ret;
> +
> +	ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> +			   (polarity == TYPEC_POLARITY_CC2) ?
> +			   TCPC_TCPC_CTRL_ORIENTATION : 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable) {
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	int ret;
> +
> +	ret = regmap_write(tcpci->regmap, TCPC_POWER_CTRL,
> +			   enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tcpci_set_pd_header(struct tcpc_dev *tcpc, enum typec_role
> role,
> +			       enum typec_data_role data)
> +{
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg;
> +	int ret;
> +
> +	reg = PD_REV20 << TCPC_MSG_HDR_INFO_REV_SHIFT;
> +	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;
> +}
> +
> +static int tcpci_set_pd_rx(struct tcpc_dev *tcpc, bool enable) {
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg = 0;
> +	int ret;
> +
> +	if (enable)
> +		reg = TCPC_RX_DETECT_SOP | TCPC_RX_DETECT_HARD_RESET;
> +	ret = regmap_write(tcpci->regmap, TCPC_RX_DETECT, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tcpci_get_vbus(struct tcpc_dev *tcpc) {
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return !!(reg & TCPC_POWER_STATUS_VBUS_PRES); }
> +
> +static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool
> +sink) {
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	int ret;
> +
> +	if (source) {
> +		ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> +				   TCPC_CMD_SRC_VBUS_DEFAULT);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> +				   TCPC_CMD_DISABLE_SRC_VBUS);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	if (sink) {
> +		ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> +				   TCPC_CMD_SINK_VBUS);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> +				   TCPC_CMD_DISABLE_SINK_VBUS);
> +		if (ret < 0)
> +			return ret;
> +	}
> +

Per tcpc spec, disable should be firstly done before enable,
so should it be done like this?
if(!source) {};
if(!sink) {};
if(source) {};
if(sink) {};

> +	return 0;
> +}
> +
> +static int tcpci_pd_transmit(struct tcpc_dev *tcpc,
> +			     enum tcpm_transmit_type type,
> +			     const struct pd_message *msg)
> +{
> +	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg, cnt, header;
> +	int ret;
> +
> +	cnt = msg ? pd_header_cnt(msg->header) * 4 : 0;
> +	ret = regmap_write(tcpci->regmap, TCPC_TX_BYTE_CNT, cnt);

Here miss the 2 bytes' header, so cnt should be cnt += 2;

Tcpc 4.4.13
TRANSMIT_BYTE_COUNT:
The number of bytes the TCPM will write
This is the number of bytes in the TX_BUFFER_DATA_OBJECTS plus two
(for the TX_BUF_HEADER)
 
> +	if (ret < 0)
> +		return ret;
> +
> +	header = msg ? msg->header : 0;
> +	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);
> +	ret = regmap_write(tcpci->regmap, TCPC_TRANSMIT, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +

...

> +
> +static int tcpci_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *i2c_id) {
> +	struct tcpci *tcpci;
> +	int err;
> +
> +	tcpci = devm_kzalloc(&client->dev, sizeof(*tcpci), GFP_KERNEL);
> +	if (!tcpci)
> +		return -ENOMEM;
> +
> +	tcpci->client = client;
> +	tcpci->dev = &client->dev;
> +	i2c_set_clientdata(client, tcpci);
> +	tcpci->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
> +	if (IS_ERR(tcpci->regmap))
> +		return PTR_ERR(tcpci->regmap);
> +
> +	tcpci->tcpc.init = tcpci_init;
> +	tcpci->tcpc.get_vbus = tcpci_get_vbus;
> +	tcpci->tcpc.set_vbus = tcpci_set_vbus;
> +	tcpci->tcpc.set_cc = tcpci_set_cc;
> +	tcpci->tcpc.get_cc = tcpci_get_cc;
> +	tcpci->tcpc.set_polarity = tcpci_set_polarity;
> +	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> +
> +	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
> +	tcpci->tcpc.set_pd_header = tcpci_set_pd_header;
> +	tcpci->tcpc.pd_transmit = tcpci_pd_transmit;
> +
> +	err = tcpci_parse_config(tcpci);
> +	if (err < 0)
> +		return err;
> +

As the alert mask reg is defaultly to be unmasked after power on,
It's better clear and mask all before request irq.

> +	err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL,
> +					tcpci_irq, IRQF_TRIGGER_FALLING,

IRQF_ONESHOT is required, and do we need the irq trigger to be
IRQF_TRIGGER_LOW?

Li Jun

Powered by blists - more mailing lists