From: ShuFan Lee <[email protected]>
Handle vendor defined behavior in tcpci_init, tcpci_set_vconn, tcpci_start_drp_toggling
and export tcpci_irq. More operations can be extended in tcpci_data if needed.
According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
TCPC shall not start DRP toggling until subsequently the TCPM
writes to the COMMAND register to start DRP toggling.
DRP toggling flow is changed as following:
- Write DRP = 1, Rp level and RC.CCx to Rd/Rd or Rp/Rp
- Set LOOK4CONNECTION command
Signed-off-by: ShuFan Lee <[email protected]>
---
drivers/staging/typec/tcpci.c | 127 +++++++++++++++++++++++++++++++++---------
drivers/staging/typec/tcpci.h | 15 +++++
2 files changed, 116 insertions(+), 26 deletions(-)
patch changelogs between v1 & v2
- Remove unnecessary i2c_client in the structure of tcpci.
- Rename structure of tcpci_vendor_data to tcpci_data.
- Not exporting tcpci read/write wrappers but register i2c regmap in glue driver.
- Add set_vconn ops in tcpci_data.
(It is necessary for RT1711H to enable/disable idle mode before disabling/enabling vconn)
- Export tcpci_irq so that vendor can call it in their own IRQ handler.
patch changelogs between v2 & v3
- Change the return type of tcpci_irq from int to irqreturn_t.
patch changelogs between v3 & v4
- Directly return regmap_write at the end of drp_toggling function.
- Because tcpci_irq is called in _tcpci_irq, move _tcpci_irq to the place after tcpci_irq.
patch changelogs between v4 & v5
- Add a space between my first & last name, from ShuFanLee to ShuFan Lee.
patch changelogs between v5 & v6
- Add start_drp_toggling in tcpci_data and call it at the beginning of tcpci_start_drp_toggling.
- Modify the flow of tcpci_start_drp_toggling as following:
- Set Rp level, RC.CCx to Rd/Rd or Rp/Rp and DRP = 1
- Set LOOK4CONNECTION command
patch changelogs between v6 & v7
- Remove checking whether tcpci->data is NULL because it is guaranteed to be available.
diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 9bd4412..8043740 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -21,7 +21,6 @@
struct tcpci {
struct device *dev;
- struct i2c_client *client;
struct tcpm_port *port;
@@ -30,6 +29,12 @@ struct tcpci {
bool controls_vbus;
struct tcpc_dev tcpc;
+ struct tcpci_data *data;
+};
+
+struct tcpci_chip {
+ struct tcpci *tcpci;
+ struct tcpci_data data;
};
static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
@@ -37,8 +42,7 @@ 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,
- u16 *val)
+static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
{
return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
}
@@ -98,9 +102,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
enum typec_cc_status cc)
{
+ int ret;
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
unsigned int reg = TCPC_ROLE_CTRL_DRP;
+ /* Handle vendor drp toggling */
+ if (tcpci->data->start_drp_toggling) {
+ ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
+ if (ret < 0)
+ return ret;
+ }
+
switch (cc) {
default:
case TYPEC_CC_RP_DEF:
@@ -117,7 +129,17 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
break;
}
- return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+ if (cc == TYPEC_CC_RD)
+ reg |= (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
+ (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
+ else
+ reg |= (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
+ (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT);
+ ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+ if (ret < 0)
+ return ret;
+ return regmap_write(tcpci->regmap, TCPC_COMMAND,
+ TCPC_CMD_LOOK4CONNECTION);
}
static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
@@ -178,6 +200,13 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
int ret;
+ /* Handle vendor set vconn */
+ if (tcpci->data->set_vconn) {
+ ret = tcpci->data->set_vconn(tcpci, tcpci->data, enable);
+ if (ret < 0)
+ return ret;
+ }
+
ret = regmap_write(tcpci->regmap, TCPC_POWER_CTRL,
enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
if (ret < 0)
@@ -323,6 +352,13 @@ static int tcpci_init(struct tcpc_dev *tcpc)
if (time_after(jiffies, timeout))
return -ETIMEDOUT;
+ /* Handle vendor init */
+ if (tcpci->data->init) {
+ ret = tcpci->data->init(tcpci, tcpci->data);
+ if (ret < 0)
+ return ret;
+ }
+
/* Clear all events */
ret = tcpci_write16(tcpci, TCPC_ALERT, 0xffff);
if (ret < 0)
@@ -344,9 +380,8 @@ static int tcpci_init(struct tcpc_dev *tcpc)
return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
}
-static irqreturn_t tcpci_irq(int irq, void *dev_id)
+irqreturn_t tcpci_irq(struct tcpci *tcpci)
{
- struct tcpci *tcpci = dev_id;
u16 status;
tcpci_read16(tcpci, TCPC_ALERT, &status);
@@ -412,6 +447,14 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
+EXPORT_SYMBOL_GPL(tcpci_irq);
+
+static irqreturn_t _tcpci_irq(int irq, void *dev_id)
+{
+ struct tcpci *tcpci = dev_id;
+
+ return tcpci_irq(tcpci);
+}
static const struct regmap_config tcpci_regmap_config = {
.reg_bits = 8,
@@ -435,22 +478,18 @@ static int tcpci_parse_config(struct tcpci *tcpci)
return 0;
}
-static int tcpci_probe(struct i2c_client *client,
- const struct i2c_device_id *i2c_id)
+struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
{
struct tcpci *tcpci;
int err;
- tcpci = devm_kzalloc(&client->dev, sizeof(*tcpci), GFP_KERNEL);
+ tcpci = devm_kzalloc(dev, sizeof(*tcpci), GFP_KERNEL);
if (!tcpci)
- return -ENOMEM;
+ return ERR_PTR(-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->dev = dev;
+ tcpci->data = data;
+ tcpci->regmap = data->regmap;
tcpci->tcpc.init = tcpci_init;
tcpci->tcpc.get_vbus = tcpci_get_vbus;
@@ -467,27 +506,63 @@ static int tcpci_probe(struct i2c_client *client,
err = tcpci_parse_config(tcpci);
if (err < 0)
- return err;
+ return ERR_PTR(err);
+
+ tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
+ if (PTR_ERR_OR_ZERO(tcpci->port))
+ return ERR_CAST(tcpci->port);
+
+ return tcpci;
+}
+EXPORT_SYMBOL_GPL(tcpci_register_port);
+
+void tcpci_unregister_port(struct tcpci *tcpci)
+{
+ tcpm_unregister_port(tcpci->port);
+}
+EXPORT_SYMBOL_GPL(tcpci_unregister_port);
+
+static int tcpci_probe(struct i2c_client *client,
+ const struct i2c_device_id *i2c_id)
+{
+ struct tcpci_chip *chip;
+ int err;
+ u16 val = 0;
- /* Disable chip interrupts */
- tcpci_write16(tcpci, TCPC_ALERT_MASK, 0);
+ chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->data.regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
+ if (IS_ERR(chip->data.regmap))
+ return PTR_ERR(chip->data.regmap);
- err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL,
- tcpci_irq,
+ /* Disable chip interrupts before requesting irq */
+ err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
+ sizeof(u16));
+ if (err < 0)
+ return err;
+
+ err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ _tcpci_irq,
IRQF_ONESHOT | IRQF_TRIGGER_LOW,
- dev_name(tcpci->dev), tcpci);
+ dev_name(&client->dev), chip);
if (err < 0)
return err;
- tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
- return PTR_ERR_OR_ZERO(tcpci->port);
+ chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
+ if (PTR_ERR_OR_ZERO(chip->tcpci))
+ return PTR_ERR(chip->tcpci);
+
+ i2c_set_clientdata(client, chip);
+ return 0;
}
static int tcpci_remove(struct i2c_client *client)
{
- struct tcpci *tcpci = i2c_get_clientdata(client);
+ struct tcpci_chip *chip = i2c_get_clientdata(client);
- tcpm_unregister_port(tcpci->port);
+ tcpci_unregister_port(chip->tcpci);
return 0;
}
diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h
index fdfb06c..a2c1754 100644
--- a/drivers/staging/typec/tcpci.h
+++ b/drivers/staging/typec/tcpci.h
@@ -59,6 +59,7 @@
#define TCPC_POWER_CTRL_VCONN_ENABLE BIT(0)
#define TCPC_CC_STATUS 0x1d
+#define TCPC_CC_STATUS_DRPRST BIT(5)
#define TCPC_CC_STATUS_TERM BIT(4)
#define TCPC_CC_STATUS_CC2_SHIFT 2
#define TCPC_CC_STATUS_CC2_MASK 0x3
@@ -121,4 +122,18 @@
#define TCPC_VBUS_VOLTAGE_ALARM_HI_CFG 0x76
#define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG 0x78
+struct tcpci;
+struct tcpci_data {
+ struct regmap *regmap;
+ int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
+ int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
+ bool enable);
+ int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *data,
+ enum typec_cc_status cc);
+};
+
+struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data);
+void tcpci_unregister_port(struct tcpci *tcpci);
+irqreturn_t tcpci_irq(struct tcpci *tcpci);
+
#endif /* __LINUX_USB_TCPCI_H */
--
1.9.1
On Tue, Mar 06, 2018 at 09:50:32AM +0800, ShuFan Lee wrote:
> From: ShuFan Lee <[email protected]>
>
> Handle vendor defined behavior in tcpci_init, tcpci_set_vconn, tcpci_start_drp_toggling
> and export tcpci_irq. More operations can be extended in tcpci_data if needed.
> According to TCPCI specification, 4.4.5.2 ROLE_CONTROL,
> TCPC shall not start DRP toggling until subsequently the TCPM
> writes to the COMMAND register to start DRP toggling.
> DRP toggling flow is changed as following:
> - Write DRP = 1, Rp level and RC.CCx to Rd/Rd or Rp/Rp
> - Set LOOK4CONNECTION command
>
> Signed-off-by: ShuFan Lee <[email protected]>
I am fine with this version.
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/staging/typec/tcpci.c | 127 +++++++++++++++++++++++++++++++++---------
> drivers/staging/typec/tcpci.h | 15 +++++
> 2 files changed, 116 insertions(+), 26 deletions(-)
>
> patch changelogs between v1 & v2
> - Remove unnecessary i2c_client in the structure of tcpci.
> - Rename structure of tcpci_vendor_data to tcpci_data.
> - Not exporting tcpci read/write wrappers but register i2c regmap in glue driver.
> - Add set_vconn ops in tcpci_data.
> (It is necessary for RT1711H to enable/disable idle mode before disabling/enabling vconn)
> - Export tcpci_irq so that vendor can call it in their own IRQ handler.
>
> patch changelogs between v2 & v3
> - Change the return type of tcpci_irq from int to irqreturn_t.
>
> patch changelogs between v3 & v4
> - Directly return regmap_write at the end of drp_toggling function.
> - Because tcpci_irq is called in _tcpci_irq, move _tcpci_irq to the place after tcpci_irq.
>
> patch changelogs between v4 & v5
> - Add a space between my first & last name, from ShuFanLee to ShuFan Lee.
>
> patch changelogs between v5 & v6
> - Add start_drp_toggling in tcpci_data and call it at the beginning of tcpci_start_drp_toggling.
> - Modify the flow of tcpci_start_drp_toggling as following:
> - Set Rp level, RC.CCx to Rd/Rd or Rp/Rp and DRP = 1
> - Set LOOK4CONNECTION command
>
> patch changelogs between v6 & v7
> - Remove checking whether tcpci->data is NULL because it is guaranteed to be available.
>
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index 9bd4412..8043740 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -21,7 +21,6 @@
>
> struct tcpci {
> struct device *dev;
> - struct i2c_client *client;
>
> struct tcpm_port *port;
>
> @@ -30,6 +29,12 @@ struct tcpci {
> bool controls_vbus;
>
> struct tcpc_dev tcpc;
> + struct tcpci_data *data;
> +};
> +
> +struct tcpci_chip {
> + struct tcpci *tcpci;
> + struct tcpci_data data;
> };
>
> static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc)
> @@ -37,8 +42,7 @@ 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,
> - u16 *val)
> +static int tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val)
> {
> return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u16));
> }
> @@ -98,9 +102,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> enum typec_cc_status cc)
> {
> + int ret;
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> unsigned int reg = TCPC_ROLE_CTRL_DRP;
>
> + /* Handle vendor drp toggling */
> + if (tcpci->data->start_drp_toggling) {
> + ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
> + if (ret < 0)
> + return ret;
> + }
> +
> switch (cc) {
> default:
> case TYPEC_CC_RP_DEF:
> @@ -117,7 +129,17 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> break;
> }
>
> - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (cc == TYPEC_CC_RD)
> + reg |= (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) |
> + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT);
> + else
> + reg |= (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC1_SHIFT) |
> + (TCPC_ROLE_CTRL_CC_RP << TCPC_ROLE_CTRL_CC2_SHIFT);
> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> + if (ret < 0)
> + return ret;
> + return regmap_write(tcpci->regmap, TCPC_COMMAND,
> + TCPC_CMD_LOOK4CONNECTION);
> }
>
> static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
> @@ -178,6 +200,13 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> int ret;
>
> + /* Handle vendor set vconn */
> + if (tcpci->data->set_vconn) {
> + ret = tcpci->data->set_vconn(tcpci, tcpci->data, enable);
> + if (ret < 0)
> + return ret;
> + }
> +
> ret = regmap_write(tcpci->regmap, TCPC_POWER_CTRL,
> enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
> if (ret < 0)
> @@ -323,6 +352,13 @@ static int tcpci_init(struct tcpc_dev *tcpc)
> if (time_after(jiffies, timeout))
> return -ETIMEDOUT;
>
> + /* Handle vendor init */
> + if (tcpci->data->init) {
> + ret = tcpci->data->init(tcpci, tcpci->data);
> + if (ret < 0)
> + return ret;
> + }
> +
> /* Clear all events */
> ret = tcpci_write16(tcpci, TCPC_ALERT, 0xffff);
> if (ret < 0)
> @@ -344,9 +380,8 @@ static int tcpci_init(struct tcpc_dev *tcpc)
> return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
> }
>
> -static irqreturn_t tcpci_irq(int irq, void *dev_id)
> +irqreturn_t tcpci_irq(struct tcpci *tcpci)
> {
> - struct tcpci *tcpci = dev_id;
> u16 status;
>
> tcpci_read16(tcpci, TCPC_ALERT, &status);
> @@ -412,6 +447,14 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id)
>
> return IRQ_HANDLED;
> }
> +EXPORT_SYMBOL_GPL(tcpci_irq);
> +
> +static irqreturn_t _tcpci_irq(int irq, void *dev_id)
> +{
> + struct tcpci *tcpci = dev_id;
> +
> + return tcpci_irq(tcpci);
> +}
>
> static const struct regmap_config tcpci_regmap_config = {
> .reg_bits = 8,
> @@ -435,22 +478,18 @@ static int tcpci_parse_config(struct tcpci *tcpci)
> return 0;
> }
>
> -static int tcpci_probe(struct i2c_client *client,
> - const struct i2c_device_id *i2c_id)
> +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> {
> struct tcpci *tcpci;
> int err;
>
> - tcpci = devm_kzalloc(&client->dev, sizeof(*tcpci), GFP_KERNEL);
> + tcpci = devm_kzalloc(dev, sizeof(*tcpci), GFP_KERNEL);
> if (!tcpci)
> - return -ENOMEM;
> + return ERR_PTR(-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->dev = dev;
> + tcpci->data = data;
> + tcpci->regmap = data->regmap;
>
> tcpci->tcpc.init = tcpci_init;
> tcpci->tcpc.get_vbus = tcpci_get_vbus;
> @@ -467,27 +506,63 @@ static int tcpci_probe(struct i2c_client *client,
>
> err = tcpci_parse_config(tcpci);
> if (err < 0)
> - return err;
> + return ERR_PTR(err);
> +
> + tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
> + if (PTR_ERR_OR_ZERO(tcpci->port))
> + return ERR_CAST(tcpci->port);
> +
> + return tcpci;
> +}
> +EXPORT_SYMBOL_GPL(tcpci_register_port);
> +
> +void tcpci_unregister_port(struct tcpci *tcpci)
> +{
> + tcpm_unregister_port(tcpci->port);
> +}
> +EXPORT_SYMBOL_GPL(tcpci_unregister_port);
> +
> +static int tcpci_probe(struct i2c_client *client,
> + const struct i2c_device_id *i2c_id)
> +{
> + struct tcpci_chip *chip;
> + int err;
> + u16 val = 0;
>
> - /* Disable chip interrupts */
> - tcpci_write16(tcpci, TCPC_ALERT_MASK, 0);
> + chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->data.regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config);
> + if (IS_ERR(chip->data.regmap))
> + return PTR_ERR(chip->data.regmap);
>
> - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL,
> - tcpci_irq,
> + /* Disable chip interrupts before requesting irq */
> + err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
> + sizeof(u16));
> + if (err < 0)
> + return err;
> +
> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + _tcpci_irq,
> IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> - dev_name(tcpci->dev), tcpci);
> + dev_name(&client->dev), chip);
> if (err < 0)
> return err;
>
> - tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc);
> - return PTR_ERR_OR_ZERO(tcpci->port);
> + chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> + if (PTR_ERR_OR_ZERO(chip->tcpci))
> + return PTR_ERR(chip->tcpci);
> +
> + i2c_set_clientdata(client, chip);
> + return 0;
> }
>
> static int tcpci_remove(struct i2c_client *client)
> {
> - struct tcpci *tcpci = i2c_get_clientdata(client);
> + struct tcpci_chip *chip = i2c_get_clientdata(client);
>
> - tcpm_unregister_port(tcpci->port);
> + tcpci_unregister_port(chip->tcpci);
>
> return 0;
> }
> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h
> index fdfb06c..a2c1754 100644
> --- a/drivers/staging/typec/tcpci.h
> +++ b/drivers/staging/typec/tcpci.h
> @@ -59,6 +59,7 @@
> #define TCPC_POWER_CTRL_VCONN_ENABLE BIT(0)
>
> #define TCPC_CC_STATUS 0x1d
> +#define TCPC_CC_STATUS_DRPRST BIT(5)
> #define TCPC_CC_STATUS_TERM BIT(4)
> #define TCPC_CC_STATUS_CC2_SHIFT 2
> #define TCPC_CC_STATUS_CC2_MASK 0x3
> @@ -121,4 +122,18 @@
> #define TCPC_VBUS_VOLTAGE_ALARM_HI_CFG 0x76
> #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG 0x78
>
> +struct tcpci;
> +struct tcpci_data {
> + struct regmap *regmap;
> + int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> + int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> + bool enable);
> + int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *data,
> + enum typec_cc_status cc);
> +};
> +
> +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data);
> +void tcpci_unregister_port(struct tcpci *tcpci);
> +irqreturn_t tcpci_irq(struct tcpci *tcpci);
> +
> #endif /* __LINUX_USB_TCPCI_H */
> --
> 1.9.1
>
Hi,
> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
> + struct tcpci *tcpci = dev_id;
> +
> + return tcpci_irq(tcpci);
> +}
>
...
> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + _tcpci_irq,
> IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> - dev_name(tcpci->dev), tcpci);
> + dev_name(&client->dev), chip);
- dev_name(&client->dev), chip);
+ dev_name(&client->dev), chip->tcpci);
Did you ever test this patch?
I noticed Greg already picked this patch[1]:
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-testing&id=8f94390226487bcb86c554ddc12eef0d27bdec3b
One more minor comment below.
Jun Li
> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h index
> fdfb06c..a2c1754 100644
> --- a/drivers/staging/typec/tcpci.h
> +++ b/drivers/staging/typec/tcpci.h
> @@ -59,6 +59,7 @@
> #define TCPC_POWER_CTRL_VCONN_ENABLE BIT(0)
>
> #define TCPC_CC_STATUS 0x1d
> +#define TCPC_CC_STATUS_DRPRST BIT(5)
Defined but not used.
> #define TCPC_CC_STATUS_TERM BIT(4)
> #define TCPC_CC_STATUS_CC2_SHIFT 2
> #define TCPC_CC_STATUS_CC2_MASK 0x3
> @@ -121,4 +122,18 @@
> #define TCPC_VBUS_VOLTAGE_ALARM_HI_CFG 0x76
> #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG 0x78
>
> +struct tcpci;
> +struct tcpci_data {
> + struct regmap *regmap;
> + int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> + int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> + bool enable);
> + int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *data,
> + enum typec_cc_status cc);
> +};
> +
> +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data
> +*data); void tcpci_unregister_port(struct tcpci *tcpci); irqreturn_t
> +tcpci_irq(struct tcpci *tcpci);
> +
> #endif /* __LINUX_USB_TCPCI_H */
> --
> 1.9.1
Hi Jun,
Thank you.
2018-03-12 12:33 GMT+08:00 Jun Li <[email protected]>:
> Hi,
>
>> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
>> + struct tcpci *tcpci = dev_id;
>> +
>> + return tcpci_irq(tcpci);
>> +}
>>
> ...
>
>> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>> + _tcpci_irq,
>> IRQF_ONESHOT | IRQF_TRIGGER_LOW,
>> - dev_name(tcpci->dev), tcpci);
>> + dev_name(&client->dev), chip);
>
> - dev_name(&client->dev), chip);
> + dev_name(&client->dev), chip->tcpci);
>
> Did you ever test this patch?
I've tested this patch with tcpci_rt1711h.c that will be sent out for
reviewing in the next patch after tcpci's modification is passed.
Because interrupt handler is registered in tcpci_rt1711h.c, here is
the place I didn't notice.
The interrupt handler for tcpci.c should be modified as following:
static irqreturn_t _tcpci_irq(int irq, void *dev_id)
{
- struct tcpci *tcpci = dev_id;
+ struct tcpci_chip *chip = dev_id;
- return tcpci_irq(tcpci);
+ return tcpci_irq(chip->tcpci);
}
>
> I noticed Greg already picked this patch[1]:
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-testing&id=8f94390226487bcb86c554ddc12eef0d27bdec3b
>
> One more minor comment below.
>
> Jun Li
>
>> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h index
>> fdfb06c..a2c1754 100644
>> --- a/drivers/staging/typec/tcpci.h
>> +++ b/drivers/staging/typec/tcpci.h
>> @@ -59,6 +59,7 @@
>> #define TCPC_POWER_CTRL_VCONN_ENABLE BIT(0)
>>
>> #define TCPC_CC_STATUS 0x1d
>> +#define TCPC_CC_STATUS_DRPRST BIT(5)
>
> Defined but not used.
This definition can be removed for now.
>
>> #define TCPC_CC_STATUS_TERM BIT(4)
>> #define TCPC_CC_STATUS_CC2_SHIFT 2
>> #define TCPC_CC_STATUS_CC2_MASK 0x3
>> @@ -121,4 +122,18 @@
>> #define TCPC_VBUS_VOLTAGE_ALARM_HI_CFG 0x76
>> #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG 0x78
>>
>> +struct tcpci;
>> +struct tcpci_data {
>> + struct regmap *regmap;
>> + int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
>> + int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
>> + bool enable);
>> + int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *data,
>> + enum typec_cc_status cc);
>> +};
>> +
>> +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data
>> +*data); void tcpci_unregister_port(struct tcpci *tcpci); irqreturn_t
>> +tcpci_irq(struct tcpci *tcpci);
>> +
>> #endif /* __LINUX_USB_TCPCI_H */
>> --
>> 1.9.1
>
--
Best Regards,
書帆
Hi
> -----Original Message-----
> From: 李書帆 [mailto:[email protected]]
> Sent: 2018年3月12日 13:22
> To: Jun Li <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v7] staging: typec: handle vendor defined part and modify
> drp toggling flow
>
> Hi Jun,
>
> Thank you.
>
> 2018-03-12 12:33 GMT+08:00 Jun Li <[email protected]>:
> > Hi,
> >
> >> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
> >> + struct tcpci *tcpci = dev_id;
> >> +
> >> + return tcpci_irq(tcpci);
> >> +}
> >>
> > ...
> >
> >> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> >> + _tcpci_irq,
> >> IRQF_ONESHOT |
> IRQF_TRIGGER_LOW,
> >> - dev_name(tcpci->dev), tcpci);
> >> + dev_name(&client->dev), chip);
> >
> > - dev_name(&client->dev), chip);
> > + dev_name(&client->dev), chip->tcpci);
> >
> > Did you ever test this patch?
> I've tested this patch with tcpci_rt1711h.c that will be sent out for reviewing in
> the next patch after tcpci's modification is passed.
> Because interrupt handler is registered in tcpci_rt1711h.c, here is the place I
> didn't notice.
Understood.
> The interrupt handler for tcpci.c should be modified as following:
> static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
> - struct tcpci *tcpci = dev_id;
> + struct tcpci_chip *chip = dev_id;
>
> - return tcpci_irq(tcpci);
> + return tcpci_irq(chip->tcpci);
> }
>
Either way is OK to fix it.
You may send out your v8 and notify Greg to drop your v7 version.
Jun Li
Hi Jun,
2018-03-12 13:58 GMT+08:00 Jun Li <[email protected]>:
> Hi
>> -----Original Message-----
>> From: 李書帆 [mailto:[email protected]]
>> Sent: 2018年3月12日 13:22
>> To: Jun Li <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>;
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH v7] staging: typec: handle vendor defined part and modify
>> drp toggling flow
>>
>> Hi Jun,
>>
>> Thank you.
>>
>> 2018-03-12 12:33 GMT+08:00 Jun Li <[email protected]>:
>> > Hi,
>> >
>> >> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
>> >> + struct tcpci *tcpci = dev_id;
>> >> +
>> >> + return tcpci_irq(tcpci);
>> >> +}
>> >>
>> > ...
>> >
>> >> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>> >> + _tcpci_irq,
>> >> IRQF_ONESHOT |
>> IRQF_TRIGGER_LOW,
>> >> - dev_name(tcpci->dev), tcpci);
>> >> + dev_name(&client->dev), chip);
>> >
>> > - dev_name(&client->dev), chip);
>> > + dev_name(&client->dev), chip->tcpci);
>> >
>> > Did you ever test this patch?
>> I've tested this patch with tcpci_rt1711h.c that will be sent out for reviewing in
>> the next patch after tcpci's modification is passed.
>> Because interrupt handler is registered in tcpci_rt1711h.c, here is the place I
>> didn't notice.
>
> Understood.
>
>> The interrupt handler for tcpci.c should be modified as following:
>> static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
>> - struct tcpci *tcpci = dev_id;
>> + struct tcpci_chip *chip = dev_id;
>>
>> - return tcpci_irq(tcpci);
>> + return tcpci_irq(chip->tcpci);
>> }
>>
>
> Either way is OK to fix it.
> You may send out your v8 and notify Greg to drop your v7 version.
>
> Jun Li
May I add you in the Reported-by list?
--
Best Regards,
書帆
> -----Original Message-----
> From: 李書帆 [mailto:[email protected]]
> Sent: 2018年3月12日 14:57
> To: Jun Li <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v7] staging: typec: handle vendor defined part and modify
> drp toggling flow
>
> Hi Jun,
>
> 2018-03-12 13:58 GMT+08:00 Jun Li <[email protected]>:
> > Hi
> >> -----Original Message-----
> >> From: 李書帆 [mailto:[email protected]]
> >> Sent: 2018年3月12日 13:22
> >> To: Jun Li <[email protected]>
> >> Cc: Greg Kroah-Hartman <[email protected]>;
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected];
> >> [email protected]; [email protected]
> >> Subject: Re: [PATCH v7] staging: typec: handle vendor defined part
> >> and modify drp toggling flow
> >>
> >> Hi Jun,
> >>
> >> Thank you.
> >>
> >> 2018-03-12 12:33 GMT+08:00 Jun Li <[email protected]>:
> >> > Hi,
> >> >
> >> >> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
> >> >> + struct tcpci *tcpci = dev_id;
> >> >> +
> >> >> + return tcpci_irq(tcpci);
> >> >> +}
> >> >>
> >> > ...
> >> >
> >> >> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> >> >> + _tcpci_irq,
> >> >> IRQF_ONESHOT |
> >> IRQF_TRIGGER_LOW,
> >> >> - dev_name(tcpci->dev),
> tcpci);
> >> >> + dev_name(&client->dev),
> >> >> + chip);
> >> >
> >> > - dev_name(&client->dev), chip);
> >> > + dev_name(&client->dev), chip->tcpci);
> >> >
> >> > Did you ever test this patch?
> >> I've tested this patch with tcpci_rt1711h.c that will be sent out for
> >> reviewing in the next patch after tcpci's modification is passed.
> >> Because interrupt handler is registered in tcpci_rt1711h.c, here is
> >> the place I didn't notice.
> >
> > Understood.
> >
> >> The interrupt handler for tcpci.c should be modified as following:
> >> static irqreturn_t _tcpci_irq(int irq, void *dev_id) {
> >> - struct tcpci *tcpci = dev_id;
> >> + struct tcpci_chip *chip = dev_id;
> >>
> >> - return tcpci_irq(tcpci);
> >> + return tcpci_irq(chip->tcpci);
> >> }
> >>
> >
> > Either way is OK to fix it.
> > You may send out your v8 and notify Greg to drop your v7 version.
> >
> > Jun Li
>
> May I add you in the Reported-by list?
I just gave a run with your patch on my HW, so for your new version,
you can directly add:
Reviewed-by: Li Jun <[email protected]>
Tested-by: Li Jun <[email protected]>
>
> --
> Best Regards,
> 書帆