Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1493933 > unrolled thread
| Started by | Jun Li <jun.li@nxp.com> |
|---|---|
| First post | 2016-09-30 10:00 +0200 |
| Last post | 2016-10-01 18:30 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
RE: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface driver (tcpci) Jun Li <jun.li@nxp.com> - 2016-09-30 10:00 +0200
Re: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface driver (tcpci) Guenter Roeck <groeck@google.com> - 2016-09-30 20:50 +0200
RE: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface driver (tcpci) Jun Li <jun.li@nxp.com> - 2016-10-01 02:50 +0200
Re: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface driver (tcpci) Guenter Roeck <groeck@google.com> - 2016-10-01 18:30 +0200
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-09-30 10:00 +0200 |
| Subject | RE: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller Interface driver (tcpci) |
| Message-ID | <sn0TI-7jq-7@gated-at.bofh.it> |
Hi Guenter,
> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Guenter Roeck
> Sent: Wednesday, August 24, 2016 5:11 AM
> To: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>; Bruce
> Ashfield <bruce.ashfield@windriver.com>; Bin Gao <bin.gao@intel.com>;
> Pranav Tipnis <pranav.tipnis@intel.com>; Heikki Krogerus
> <heikki.krogerus@linux.intel.com>; linux-kernel@vger.kernel.org; linux-
> usb@vger.kernel.org; Guenter Roeck <groeck@chromium.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@chromium.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, ®);
> + 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, ®);
> + 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
[toc] | [next] | [standalone]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-09-30 20:50 +0200 |
| Message-ID | <snb2F-5Ed-15@gated-at.bofh.it> |
| In reply to | #1493933 |
On Thu, Sep 29, 2016 at 11:24 PM, Jun Li <jun.li@nxp.com> wrote:
> Hi Guenter,
>
>> -----Original Message-----
>> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
>> owner@vger.kernel.org] On Behalf Of Guenter Roeck
>> Sent: Wednesday, August 24, 2016 5:11 AM
>> To: Felipe Balbi <felipe.balbi@linux.intel.com>
>> Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>; Bruce
>> Ashfield <bruce.ashfield@windriver.com>; Bin Gao <bin.gao@intel.com>;
>> Pranav Tipnis <pranav.tipnis@intel.com>; Heikki Krogerus
>> <heikki.krogerus@linux.intel.com>; linux-kernel@vger.kernel.org; linux-
>> usb@vger.kernel.org; Guenter Roeck <groeck@chromium.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@chromium.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, ®);
>> + 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, ®);
>> + 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) {};
>
Makes sense. Changed accordingly.
>> + 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)
>
Yes, you are correct. Fixed.
>> + 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.
>
Agreed and done.
>> + 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?
>
Good question; I don't really know. IRQF_TRIGGER_FALLING definitely
seems to be wrong (and I don't recall where I got it from). Not sure
about IRQF_TRIGGER_LOW. Can you test on real hardware, by any chance
?
Thanks,
Guenter
> Li Jun
[toc] | [prev] | [next] | [standalone]
| From | Jun Li <jun.li@nxp.com> |
|---|---|
| Date | 2016-10-01 02:50 +0200 |
| Message-ID | <sngF3-Vh-9@gated-at.bofh.it> |
| In reply to | #1494234 |
Hi,
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck@google.com]
> Sent: Saturday, October 01, 2016 2:45 AM
> To: Jun Li <jun.li@nxp.com>
> Cc: Guenter Roeck <groeck@chromium.org>; Felipe Balbi
> <felipe.balbi@linux.intel.com>; Chandra Sekhar Anagani
> <chandra.sekhar.anagani@intel.com>; Bruce Ashfield
> <bruce.ashfield@windriver.com>; Bin Gao <bin.gao@intel.com>; Pranav Tipnis
> <pranav.tipnis@intel.com>; Heikki Krogerus
> <heikki.krogerus@linux.intel.com>; linux-kernel@vger.kernel.org; linux-
> usb@vger.kernel.org
> Subject: Re: [RFC PATCH v3 2/2] usb: typec: Type-C Port Controller
> Interface driver (tcpci)
>
> On Thu, Sep 29, 2016 at 11:24 PM, Jun Li <jun.li@nxp.com> wrote:
> > Hi Guenter,
> >
> >> -----Original Message-----
> >> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> >> owner@vger.kernel.org] On Behalf Of Guenter Roeck
> >> Sent: Wednesday, August 24, 2016 5:11 AM
> >> To: Felipe Balbi <felipe.balbi@linux.intel.com>
> >> Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>; Bruce
> >> Ashfield <bruce.ashfield@windriver.com>; Bin Gao <bin.gao@intel.com>;
> >> Pranav Tipnis <pranav.tipnis@intel.com>; Heikki Krogerus
> >> <heikki.krogerus@linux.intel.com>; linux-kernel@vger.kernel.org;
> >> linux- usb@vger.kernel.org; Guenter Roeck <groeck@chromium.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@chromium.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, ®);
> >> + 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, ®);
> >> + 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) {};
> >
> Makes sense. Changed accordingly.
>
> >> + 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)
> >
> Yes, you are correct. Fixed.
>
> >> + 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.
> >
> Agreed and done.
>
> >> + 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?
> >
>
> Good question; I don't really know. IRQF_TRIGGER_FALLING definitely seems
> to be wrong (and I don't recall where I got it from). Not sure about
> IRQF_TRIGGER_LOW. Can you test on real hardware, by any chance ?
>
I tested on real hardware, IRQF_TRIGGER_LOW can work fine, so I think
the right flag should be IRQF_TRIGGER_LOW | IRQF_ONESHOT.
Li Jun
> Thanks,
> Guenter
>
>
> > Li Jun
[toc] | [prev] | [next] | [standalone]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-10-01 18:30 +0200 |
| Message-ID | <snvkK-2zB-33@gated-at.bofh.it> |
| In reply to | #1494332 |
On Fri, Sep 30, 2016 at 5:12 PM, Jun Li <jun.li@nxp.com> wrote: [ ... ] >> >> >> + 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? >> > >> >> Good question; I don't really know. IRQF_TRIGGER_FALLING definitely seems >> to be wrong (and I don't recall where I got it from). Not sure about >> IRQF_TRIGGER_LOW. Can you test on real hardware, by any chance ? >> > > I tested on real hardware, IRQF_TRIGGER_LOW can work fine, so I think > the right flag should be IRQF_TRIGGER_LOW | IRQF_ONESHOT. > I had wondered if the trigger should be part of the devicetree file. However, TCPCI specifies (chapter 4, bullet point 6), that "The TCPC shall have an open drain output, active low Alert# Pin. This pin is used to indicate a change of state, where Alert# pin is asserted when a ny Alert Bits are set." Given that, I agree, and I'll add IRQF_TRIGGER_LOW. Thanks, Guenter
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web