* [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 12:45 ` ShuFanLee 0 siblings, 0 replies; 11+ messages in thread From: ShuFanLee @ 2018-02-14 12:45 UTC (permalink / raw) To: heikki.krogerus, linux; +Cc: linux-kernel, linux-usb, cy_huang, shufan_lee From: ShuFanLee <shufan_lee@richtek.com> Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: - Write DRP = 0 & Rd/Rd - Write DRP = 1 - Set LOOK4CONNECTION command Signed-off-by: ShuFanLee <shufan_lee@richtek.com> --- drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++++++-------- drivers/staging/typec/tcpci.h | 13 +++++ 2 files changed, 103 insertions(+), 21 deletions(-) diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c index 9bd4412..461fbd48 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,7 @@ struct tcpci { bool controls_vbus; struct tcpc_dev tcpc; + struct tcpci_data *data; }; static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) @@ -98,8 +98,10 @@ 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; + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) | + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT); switch (cc) { default: @@ -116,8 +118,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc, TCPC_ROLE_CTRL_RP_VAL_SHIFT); break; } - - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); + if (ret < 0) + return ret; + usleep_range(500, 1000); + reg |= TCPC_ROLE_CTRL_DRP; + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); + if (ret < 0) + return ret; + ret = regmap_write(tcpci->regmap, TCPC_COMMAND, + TCPC_CMD_LOOK4CONNECTION); + if (ret < 0) + return ret; + return 0; } static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink) @@ -178,6 +191,16 @@ 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) { + 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 +346,15 @@ static int tcpci_init(struct tcpc_dev *tcpc) if (time_after(jiffies, timeout)) return -ETIMEDOUT; + /* Handle vendor init */ + if (tcpci->data) { + 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 +376,16 @@ 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) +static irqreturn_t _tcpci_irq(int irq, void *dev_id) { struct tcpci *tcpci = dev_id; + + tcpci_irq(tcpci); + return IRQ_HANDLED; +} + +int tcpci_irq(struct tcpci *tcpci) +{ u16 status; tcpci_read16(tcpci, TCPC_ALERT, &status); @@ -412,6 +451,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(tcpci_irq); static const struct regmap_config tcpci_regmap_config = { .reg_bits = 8, @@ -435,22 +475,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 +503,60 @@ static int tcpci_probe(struct i2c_client *client, err = tcpci_parse_config(tcpci); if (err < 0) - return err; + return ERR_PTR(err); /* Disable chip interrupts */ tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, - tcpci_irq, + 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 *tcpci; + struct tcpci_data *data; + int err; + + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + + tcpci = tcpci_register_port(&client->dev, data); + if (PTR_ERR_OR_ZERO(tcpci)) + return PTR_ERR(tcpci); + + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, + _tcpci_irq, IRQF_ONESHOT | IRQF_TRIGGER_LOW, dev_name(tcpci->dev), tcpci); if (err < 0) return err; - tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc); - return PTR_ERR_OR_ZERO(tcpci->port); + i2c_set_clientdata(client, tcpci); + return 0; } static int tcpci_remove(struct i2c_client *client) { struct tcpci *tcpci = i2c_get_clientdata(client); - tcpm_unregister_port(tcpci->port); + tcpci_unregister_port(tcpci); return 0; } diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h index fdfb06c..c838723 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,16 @@ #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); +}; + +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data); +void tcpci_unregister_port(struct tcpci *tcpci); +int tcpci_irq(struct tcpci *tcpci); + #endif /* __LINUX_USB_TCPCI_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 12:45 ` ShuFanLee 0 siblings, 0 replies; 11+ messages in thread From: ShuFanLee @ 2018-02-14 12:45 UTC (permalink / raw) To: heikki.krogerus, linux; +Cc: linux-kernel, linux-usb, cy_huang, shufan_lee From: ShuFanLee <shufan_lee@richtek.com> Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: - Write DRP = 0 & Rd/Rd - Write DRP = 1 - Set LOOK4CONNECTION command Signed-off-by: ShuFanLee <shufan_lee@richtek.com> --- drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++++++-------- drivers/staging/typec/tcpci.h | 13 +++++ 2 files changed, 103 insertions(+), 21 deletions(-) diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c index 9bd4412..461fbd48 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,7 @@ struct tcpci { bool controls_vbus; struct tcpc_dev tcpc; + struct tcpci_data *data; }; static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) @@ -98,8 +98,10 @@ 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; + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) | + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT); switch (cc) { default: @@ -116,8 +118,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc, TCPC_ROLE_CTRL_RP_VAL_SHIFT); break; } - - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); + if (ret < 0) + return ret; + usleep_range(500, 1000); + reg |= TCPC_ROLE_CTRL_DRP; + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); + if (ret < 0) + return ret; + ret = regmap_write(tcpci->regmap, TCPC_COMMAND, + TCPC_CMD_LOOK4CONNECTION); + if (ret < 0) + return ret; + return 0; } static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink) @@ -178,6 +191,16 @@ 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) { + 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 +346,15 @@ static int tcpci_init(struct tcpc_dev *tcpc) if (time_after(jiffies, timeout)) return -ETIMEDOUT; + /* Handle vendor init */ + if (tcpci->data) { + 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 +376,16 @@ 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) +static irqreturn_t _tcpci_irq(int irq, void *dev_id) { struct tcpci *tcpci = dev_id; + + tcpci_irq(tcpci); + return IRQ_HANDLED; +} + +int tcpci_irq(struct tcpci *tcpci) +{ u16 status; tcpci_read16(tcpci, TCPC_ALERT, &status); @@ -412,6 +451,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(tcpci_irq); static const struct regmap_config tcpci_regmap_config = { .reg_bits = 8, @@ -435,22 +475,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 +503,60 @@ static int tcpci_probe(struct i2c_client *client, err = tcpci_parse_config(tcpci); if (err < 0) - return err; + return ERR_PTR(err); /* Disable chip interrupts */ tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, - tcpci_irq, + 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 *tcpci; + struct tcpci_data *data; + int err; + + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + + tcpci = tcpci_register_port(&client->dev, data); + if (PTR_ERR_OR_ZERO(tcpci)) + return PTR_ERR(tcpci); + + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, + _tcpci_irq, IRQF_ONESHOT | IRQF_TRIGGER_LOW, dev_name(tcpci->dev), tcpci); if (err < 0) return err; - tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc); - return PTR_ERR_OR_ZERO(tcpci->port); + i2c_set_clientdata(client, tcpci); + return 0; } static int tcpci_remove(struct i2c_client *client) { struct tcpci *tcpci = i2c_get_clientdata(client); - tcpm_unregister_port(tcpci->port); + tcpci_unregister_port(tcpci); return 0; } diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h index fdfb06c..c838723 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,16 @@ #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); +}; + +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data); +void tcpci_unregister_port(struct tcpci *tcpci); +int tcpci_irq(struct tcpci *tcpci); + #endif /* __LINUX_USB_TCPCI_H */ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 12:57 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2018-02-14 12:57 UTC (permalink / raw) To: ShuFanLee Cc: heikki.krogerus, linux, linux-kernel, linux-usb, cy_huang, shufan_lee On Wed, Feb 14, 2018 at 08:45:34PM +0800, ShuFanLee wrote: > From: ShuFanLee <shufan_lee@richtek.com> > > Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: > - Write DRP = 0 & Rd/Rd > - Write DRP = 1 > - Set LOOK4CONNECTION command > > Signed-off-by: ShuFanLee <shufan_lee@richtek.com> > --- > drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++++++-------- > drivers/staging/typec/tcpci.h | 13 +++++ > 2 files changed, 103 insertions(+), 21 deletions(-) What changed from v1? And I have to ask, are there no ' ' characters in your name? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 12:57 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2018-02-14 12:57 UTC (permalink / raw) To: ShuFanLee Cc: heikki.krogerus, linux, linux-kernel, linux-usb, cy_huang, shufan_lee On Wed, Feb 14, 2018 at 08:45:34PM +0800, ShuFanLee wrote: > From: ShuFanLee <shufan_lee@richtek.com> > > Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: > - Write DRP = 0 & Rd/Rd > - Write DRP = 1 > - Set LOOK4CONNECTION command > > Signed-off-by: ShuFanLee <shufan_lee@richtek.com> > --- > drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++++++-------- > drivers/staging/typec/tcpci.h | 13 +++++ > 2 files changed, 103 insertions(+), 21 deletions(-) What changed from v1? And I have to ask, are there no ' ' characters in your name? thanks, greg k-h --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow 2018-02-14 12:57 ` [v2] " Greg KH (?) @ 2018-02-14 14:43 ` 李書帆 2018-02-14 15:37 ` [v2] " Greg KH -1 siblings, 1 reply; 11+ messages in thread From: 李書帆 @ 2018-02-14 14:43 UTC (permalink / raw) To: Greg KH Cc: heikki.krogerus, Guenter Roeck, linux-kernel, linux-usb, cy_huang, shufan_lee [-- Attachment #1: Type: text/plain, Size: 9731 bytes --] Hi Gerg, What changed from v1? The diff between v1 and v2 is as following: diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c index 305a75d..8bccaa8 100644 --- a/drivers/staging/typec/tcpci.c +++ b/drivers/staging/typec/tcpci.c @@ -30,7 +30,7 @@ struct tcpci { bool controls_vbus; struct tcpc_dev tcpc; - struct tcpci_vendor_data *vdata; + struct tcpci_data *data; }; static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) @@ -38,29 +38,16 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) return container_of(tcpc, struct tcpci, tcpc); } -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)); } -EXPORT_SYMBOL_GPL(tcpci_read16); -int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val) +static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val) { return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16)); } -EXPORT_SYMBOL_GPL(tcpci_write16); - -int tcpci_read8(struct tcpci *tcpci, unsigned int reg, u8 *val) -{ - return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u8)); -} -EXPORT_SYMBOL_GPL(tcpci_read8); - -int tcpci_write8(struct tcpci *tcpci, unsigned int reg, u8 val) -{ - return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u8)); -} -EXPORT_SYMBOL_GPL(tcpci_write8); static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc) { @@ -206,6 +193,16 @@ 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) { + 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) @@ -352,9 +349,9 @@ static int tcpci_init(struct tcpc_dev *tcpc) return -ETIMEDOUT; /* Handle vendor init */ - if (tcpci->vdata) { - if (tcpci->vdata->init) { - ret = (*tcpci->vdata->init)(tcpci, tcpci->vdata); + if (tcpci->data) { + if (tcpci->data->init) { + ret = tcpci->data->init(tcpci, tcpci->data); if (ret < 0) return ret; } @@ -381,20 +378,20 @@ 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) +static irqreturn_t _tcpci_irq(int irq, void *dev_id) { struct tcpci *tcpci = dev_id; + + tcpci_irq(tcpci); + return IRQ_HANDLED; +} + +int tcpci_irq(struct tcpci *tcpci) +{ u16 status; tcpci_read16(tcpci, TCPC_ALERT, &status); - /* Handle vendor defined interrupt */ - if (tcpci->vdata) { - if (tcpci->vdata->irq_handler) - (*tcpci->vdata->irq_handler)(tcpci, tcpci->vdata, - &status); - } - /* * Clear alert status for everything except RX_STATUS, which shouldn't * be cleared until we have successfully retrieved message. @@ -456,6 +453,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(tcpci_irq); static const struct regmap_config tcpci_regmap_config = { .reg_bits = 8, @@ -479,22 +477,18 @@ static int tcpci_parse_config(struct tcpci *tcpci) return 0; } -struct tcpci *tcpci_register_port(struct i2c_client *client, - struct tcpci_vendor_data *vdata) +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 ERR_PTR(-ENOMEM); - tcpci->client = client; - tcpci->dev = &client->dev; - tcpci->vdata = vdata; - tcpci->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); - if (IS_ERR(tcpci->regmap)) - return ERR_CAST(tcpci->regmap); + tcpci->dev = dev; + tcpci->data = data; + tcpci->regmap = data->regmap; tcpci->tcpc.init = tcpci_init; tcpci->tcpc.get_vbus = tcpci_get_vbus; @@ -516,13 +510,6 @@ struct tcpci *tcpci_register_port(struct i2c_client *client, /* Disable chip interrupts */ tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, - tcpci_irq, - IRQF_ONESHOT | IRQF_TRIGGER_LOW, - dev_name(tcpci->dev), tcpci); - if (err < 0) - 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); @@ -541,11 +528,28 @@ static int tcpci_probe(struct i2c_client *client, const struct i2c_device_id *i2c_id) { struct tcpci *tcpci; + struct tcpci_data *data; + int err; + + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; - tcpci = tcpci_register_port(client, NULL); + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + + tcpci = tcpci_register_port(&client->dev, data); if (PTR_ERR_OR_ZERO(tcpci)) return PTR_ERR(tcpci); + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, + _tcpci_irq, + IRQF_ONESHOT | IRQF_TRIGGER_LOW, + dev_name(tcpci->dev), tcpci); + if (err < 0) + return err; + i2c_set_clientdata(client, tcpci); return 0; } diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h index b1fcb4a..c838723 100644 --- a/drivers/staging/typec/tcpci.h +++ b/drivers/staging/typec/tcpci.h @@ -123,17 +123,15 @@ #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG 0x78 struct tcpci; -struct tcpci_vendor_data { - int (*init)(struct tcpci *tcpci, struct tcpci_vendor_data *data); - int (*irq_handler)(struct tcpci *tcpci, struct tcpci_vendor_data *data, - u16 *status); +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 tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val); -int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val); -int tcpci_read8(struct tcpci *tcpci, unsigned int reg, u8 *val); -int tcpci_write8(struct tcpci *tcpci, unsigned int reg, u8 val); -struct tcpci *tcpci_register_port(struct i2c_client *client, - struct tcpci_vendor_data *data); +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data); void tcpci_unregister_port(struct tcpci *tcpci); +int tcpci_irq(struct tcpci *tcpci); + #endif /* __LINUX_USB_TCPCI_H */ And I have to ask, are there no ' ' characters in your name? Yes, there should be a ' '. So the name is ShuFan Lee. 2018-02-14 20:57 GMT+08:00 Greg KH <greg@kroah.com>: > On Wed, Feb 14, 2018 at 08:45:34PM +0800, ShuFanLee wrote: > > From: ShuFanLee <shufan_lee@richtek.com> > > > > Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: > > - Write DRP = 0 & Rd/Rd > > - Write DRP = 1 > > - Set LOOK4CONNECTION command > > > > Signed-off-by: ShuFanLee <shufan_lee@richtek.com> > > --- > > drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++ > ++++-------- > > drivers/staging/typec/tcpci.h | 13 +++++ > > 2 files changed, 103 insertions(+), 21 deletions(-) > > What changed from v1? > > And I have to ask, are there no ' ' characters in your name? > > thanks, > > greg k-h > -- Best Regards, 書帆 [-- Attachment #2: Type: text/html, Size: 33455 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 15:37 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2018-02-14 15:37 UTC (permalink / raw) To: 李書帆 Cc: heikki.krogerus, Guenter Roeck, linux-kernel, linux-usb, cy_huang, shufan_lee On Wed, Feb 14, 2018 at 10:43:45PM +0800, 李書帆 wrote: > Hi Gerg, > > > What changed from v1? > > The diff between v1 and v2 is as following: <snip> You need to explain what changed below the --- line, as is described in Documentation/SubmittingPatches. Please fix that up, and the text for your name, when you resend version 3 of this patch. thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 15:37 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2018-02-14 15:37 UTC (permalink / raw) To: 李書帆 Cc: heikki.krogerus, Guenter Roeck, linux-kernel, linux-usb, cy_huang, shufan_lee On Wed, Feb 14, 2018 at 10:43:45PM +0800, 李書帆 wrote: > Hi Gerg, > > > What changed from v1? > > The diff between v1 and v2 is as following: <snip> You need to explain what changed below the --- line, as is described in Documentation/SubmittingPatches. Please fix that up, and the text for your name, when you resend version 3 of this patch. thanks, greg k-h --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow 2018-02-14 15:37 ` [v2] " Greg KH (?) @ 2018-02-14 16:55 ` 李書帆 -1 siblings, 0 replies; 11+ messages in thread From: 李書帆 @ 2018-02-14 16:55 UTC (permalink / raw) To: Greg KH Cc: heikki.krogerus, Guenter Roeck, linux-kernel, linux-usb, cy_huang, shufan_lee [-- Attachment #1: Type: text/plain, Size: 10470 bytes --] Hi Gerg, Thank you for your time! I'll summarize the difference between v1, v2 and v3 below the --- line and modify the format of my name in PATCH v3. For your reference, the detailed explanation of the difference between v1 and v2 is as following: diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c index 305a75d..8bccaa8 100644 --- a/drivers/staging/typec/tcpci.c +++ b/drivers/staging/typec/tcpci.c @@ -30,7 +30,7 @@ struct tcpci { bool controls_vbus; struct tcpc_dev tcpc; - struct tcpci_vendor_data *vdata; + struct tcpci_data *data; }; Change naming of structure of tcpci_vendor_data to tcpci_data ============================================================= static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) @@ -38,29 +38,16 @@ static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) return container_of(tcpc, struct tcpci, tcpc); } -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)); } -EXPORT_SYMBOL_GPL(tcpci_read16); -int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val) +static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val) { return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16)); } -EXPORT_SYMBOL_GPL(tcpci_write16); - -int tcpci_read8(struct tcpci *tcpci, unsigned int reg, u8 *val) -{ - return regmap_raw_read(tcpci->regmap, reg, val, sizeof(u8)); -} -EXPORT_SYMBOL_GPL(tcpci_read8); - -int tcpci_write8(struct tcpci *tcpci, unsigned int reg, u8 val) -{ - return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u8)); -} -EXPORT_SYMBOL_GPL(tcpci_write8); Remove exporting the above wrappers like Heikki suggested. ============================================================= static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc) { @@ -206,6 +193,16 @@ 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) { + 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) Add set vconn ops for vendor to do things before standard set vconn operation e.x. It is necessary for RT1711H to disable idle mode before enabling vconn ============================================================= @@ -352,9 +349,9 @@ static int tcpci_init(struct tcpc_dev *tcpc) return -ETIMEDOUT; /* Handle vendor init */ - if (tcpci->vdata) { - if (tcpci->vdata->init) { - ret = (*tcpci->vdata->init)(tcpci, tcpci->vdata); + if (tcpci->data) { + if (tcpci->data->init) { + ret = tcpci->data->init(tcpci, tcpci->data); if (ret < 0) return ret; } Fix the type error ============================================================= @@ -381,20 +378,20 @@ 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) +static irqreturn_t _tcpci_irq(int irq, void *dev_id) { struct tcpci *tcpci = dev_id; + + tcpci_irq(tcpci); + return IRQ_HANDLED; +} + +int tcpci_irq(struct tcpci *tcpci) +{ u16 status; tcpci_read16(tcpci, TCPC_ALERT, &status); - /* Handle vendor defined interrupt */ - if (tcpci->vdata) { - if (tcpci->vdata->irq_handler) - (*tcpci->vdata->irq_handler)(tcpci, tcpci->vdata, - &status); - } - /* * Clear alert status for everything except RX_STATUS, which shouldn't * be cleared until we have successfully retrieved message. @@ -456,6 +453,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(tcpci_irq); As Heikki suggested, I export tcpci_irq so that vendor can register their own IRQ handler and call tcpci_irq in their IRQ handler ============================================================= static const struct regmap_config tcpci_regmap_config = { .reg_bits = 8, @@ -479,22 +477,18 @@ static int tcpci_parse_config(struct tcpci *tcpci) return 0; } -struct tcpci *tcpci_register_port(struct i2c_client *client, - struct tcpci_vendor_data *vdata) +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 ERR_PTR(-ENOMEM); - tcpci->client = client; - tcpci->dev = &client->dev; - tcpci->vdata = vdata; - tcpci->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); - if (IS_ERR(tcpci->regmap)) - return ERR_CAST(tcpci->regmap); + tcpci->dev = dev; + tcpci->data = data; + tcpci->regmap = data->regmap; tcpci->tcpc.init = tcpci_init; tcpci->tcpc.get_vbus = tcpci_get_vbus; @@ -516,13 +510,6 @@ struct tcpci *tcpci_register_port(struct i2c_client *client, /* Disable chip interrupts */ tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, - tcpci_irq, - IRQF_ONESHOT | IRQF_TRIGGER_LOW, - dev_name(tcpci->dev), tcpci); - if (err < 0) - 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); @@ -541,11 +528,28 @@ static int tcpci_probe(struct i2c_client *client, const struct i2c_device_id *i2c_id) { struct tcpci *tcpci; + struct tcpci_data *data; + int err; + + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; - tcpci = tcpci_register_port(client, NULL); + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + + tcpci = tcpci_register_port(&client->dev, data); if (PTR_ERR_OR_ZERO(tcpci)) return PTR_ERR(tcpci); + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, + _tcpci_irq, + IRQF_ONESHOT | IRQF_TRIGGER_LOW, + dev_name(tcpci->dev), tcpci); + if (err < 0) + return err; + i2c_set_clientdata(client, tcpci); return 0; } Remove i2c_client from tcpci Move regmap_init & request_threaded_irq to the glue driver Note: request_threaded_irq may need to be done earlier, prior to tcpci_register_port. This is still under discussion. ============================================================= diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h index b1fcb4a..c838723 100644 --- a/drivers/staging/typec/tcpci.h +++ b/drivers/staging/typec/tcpci.h @@ -123,17 +123,15 @@ #define TCPC_VBUS_VOLTAGE_ALARM_LO_CFG 0x78 struct tcpci; -struct tcpci_vendor_data { - int (*init)(struct tcpci *tcpci, struct tcpci_vendor_data *data); - int (*irq_handler)(struct tcpci *tcpci, struct tcpci_vendor_data *data, - u16 *status); +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 tcpci_read16(struct tcpci *tcpci, unsigned int reg, u16 *val); -int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val); -int tcpci_read8(struct tcpci *tcpci, unsigned int reg, u8 *val); -int tcpci_write8(struct tcpci *tcpci, unsigned int reg, u8 val); -struct tcpci *tcpci_register_port(struct i2c_client *client, - struct tcpci_vendor_data *data); +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data); void tcpci_unregister_port(struct tcpci *tcpci); +int tcpci_irq(struct tcpci *tcpci); + #endif /* __LINUX_USB_TCPCI_H */ Modify structure of tcpci_vendor_data to tcpci_data Only declare tcpci_register_port & tcpci_unregister_port & tcpci_irq in header file ============================================================= 2018-02-14 23:37 GMT+08:00 Greg KH <greg@kroah.com>: > On Wed, Feb 14, 2018 at 10:43:45PM +0800, 李書帆 wrote: > > Hi Gerg, > > > > > > What changed from v1? > > > > The diff between v1 and v2 is as following: > > <snip> > > You need to explain what changed below the --- line, as is described in > Documentation/SubmittingPatches. > > Please fix that up, and the text for your name, when you resend > version 3 of this patch. > > thanks, > > greg k-h > -- Best Regards, 書帆 [-- Attachment #2: Type: text/html, Size: 32797 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 14:46 ` Guenter Roeck 0 siblings, 0 replies; 11+ messages in thread From: Guenter Roeck @ 2018-02-14 14:46 UTC (permalink / raw) To: ShuFanLee, heikki.krogerus; +Cc: linux-kernel, linux-usb, cy_huang, shufan_lee On 02/14/2018 04:45 AM, ShuFanLee wrote: > From: ShuFanLee <shufan_lee@richtek.com> > > Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: > - Write DRP = 0 & Rd/Rd > - Write DRP = 1 > - Set LOOK4CONNECTION command > > Signed-off-by: ShuFanLee <shufan_lee@richtek.com> > --- > drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++++++-------- > drivers/staging/typec/tcpci.h | 13 +++++ > 2 files changed, 103 insertions(+), 21 deletions(-) > > diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c > index 9bd4412..461fbd48 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,7 @@ struct tcpci { > bool controls_vbus; > > struct tcpc_dev tcpc; > + struct tcpci_data *data; > }; > > static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) > @@ -98,8 +98,10 @@ 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; > + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) | > + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT); > > switch (cc) { > default: > @@ -116,8 +118,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc, > TCPC_ROLE_CTRL_RP_VAL_SHIFT); > break; > } > - > - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + if (ret < 0) > + return ret; > + usleep_range(500, 1000); > + reg |= TCPC_ROLE_CTRL_DRP; > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + if (ret < 0) > + return ret; > + ret = regmap_write(tcpci->regmap, TCPC_COMMAND, > + TCPC_CMD_LOOK4CONNECTION); > + if (ret < 0) > + return ret; > + return 0; > } > > static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink) > @@ -178,6 +191,16 @@ 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) { > + 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 +346,15 @@ static int tcpci_init(struct tcpc_dev *tcpc) > if (time_after(jiffies, timeout)) > return -ETIMEDOUT; > > + /* Handle vendor init */ > + if (tcpci->data) { > + 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 +376,16 @@ 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) > +static irqreturn_t _tcpci_irq(int irq, void *dev_id) > { > struct tcpci *tcpci = dev_id; > + > + tcpci_irq(tcpci); > + return IRQ_HANDLED; This should probably be return tcpci_irq(tcpci); > +} > + > +int tcpci_irq(struct tcpci *tcpci) and this should be irqreturn_t tcpci_irq(struct tcpci *tcpci) Otherwise it doesn't add value to have tcpci_irq() return anything. Alternative would be to make it void tcpci_irq(struct tcpci *tcpci) if it will always only return IRQ_HANDLED. Guenter > +{ > u16 status; > > tcpci_read16(tcpci, TCPC_ALERT, &status); > @@ -412,6 +451,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) > > return IRQ_HANDLED; > } > +EXPORT_SYMBOL_GPL(tcpci_irq); > > static const struct regmap_config tcpci_regmap_config = { > .reg_bits = 8, > @@ -435,22 +475,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 +503,60 @@ static int tcpci_probe(struct i2c_client *client, > > err = tcpci_parse_config(tcpci); > if (err < 0) > - return err; > + return ERR_PTR(err); > > /* Disable chip interrupts */ > tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); > > - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, > - tcpci_irq, > + 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 *tcpci; > + struct tcpci_data *data; > + int err; > + > + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); > + if (IS_ERR(data->regmap)) > + return PTR_ERR(data->regmap); > + > + tcpci = tcpci_register_port(&client->dev, data); > + if (PTR_ERR_OR_ZERO(tcpci)) > + return PTR_ERR(tcpci); > + > + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, > + _tcpci_irq, > IRQF_ONESHOT | IRQF_TRIGGER_LOW, > dev_name(tcpci->dev), tcpci); I am a bit concerned that this may cause problems, since tcpm_register_port() will send messages to the driver, which may trigger interrupts. It may be necessary to register interrupts early, prior to calling tcpci_register_port(). > if (err < 0) > return err; You'll need to call tcpm_unregister_port() here (or register the interrupt first). Guenter > > - tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc); > - return PTR_ERR_OR_ZERO(tcpci->port); > + i2c_set_clientdata(client, tcpci); > + return 0; > } > > static int tcpci_remove(struct i2c_client *client) > { > struct tcpci *tcpci = i2c_get_clientdata(client); > > - tcpm_unregister_port(tcpci->port); > + tcpci_unregister_port(tcpci); > > return 0; > } > diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h > index fdfb06c..c838723 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,16 @@ > #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); > +}; > + > +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data); > +void tcpci_unregister_port(struct tcpci *tcpci); > +int tcpci_irq(struct tcpci *tcpci); > + > #endif /* __LINUX_USB_TCPCI_H */ > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2] staging: typec: handle vendor defined part and modify drp toggling flow @ 2018-02-14 14:46 ` Guenter Roeck 0 siblings, 0 replies; 11+ messages in thread From: Guenter Roeck @ 2018-02-14 14:46 UTC (permalink / raw) To: ShuFanLee, heikki.krogerus; +Cc: linux-kernel, linux-usb, cy_huang, shufan_lee On 02/14/2018 04:45 AM, ShuFanLee wrote: > From: ShuFanLee <shufan_lee@richtek.com> > > Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: > - Write DRP = 0 & Rd/Rd > - Write DRP = 1 > - Set LOOK4CONNECTION command > > Signed-off-by: ShuFanLee <shufan_lee@richtek.com> > --- > drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++++++-------- > drivers/staging/typec/tcpci.h | 13 +++++ > 2 files changed, 103 insertions(+), 21 deletions(-) > > diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c > index 9bd4412..461fbd48 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,7 @@ struct tcpci { > bool controls_vbus; > > struct tcpc_dev tcpc; > + struct tcpci_data *data; > }; > > static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) > @@ -98,8 +98,10 @@ 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; > + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC1_SHIFT) | > + (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_CC2_SHIFT); > > switch (cc) { > default: > @@ -116,8 +118,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc, > TCPC_ROLE_CTRL_RP_VAL_SHIFT); > break; > } > - > - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + if (ret < 0) > + return ret; > + usleep_range(500, 1000); > + reg |= TCPC_ROLE_CTRL_DRP; > + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); > + if (ret < 0) > + return ret; > + ret = regmap_write(tcpci->regmap, TCPC_COMMAND, > + TCPC_CMD_LOOK4CONNECTION); > + if (ret < 0) > + return ret; > + return 0; > } > > static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink) > @@ -178,6 +191,16 @@ 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) { > + 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 +346,15 @@ static int tcpci_init(struct tcpc_dev *tcpc) > if (time_after(jiffies, timeout)) > return -ETIMEDOUT; > > + /* Handle vendor init */ > + if (tcpci->data) { > + 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 +376,16 @@ 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) > +static irqreturn_t _tcpci_irq(int irq, void *dev_id) > { > struct tcpci *tcpci = dev_id; > + > + tcpci_irq(tcpci); > + return IRQ_HANDLED; This should probably be return tcpci_irq(tcpci); > +} > + > +int tcpci_irq(struct tcpci *tcpci) and this should be irqreturn_t tcpci_irq(struct tcpci *tcpci) Otherwise it doesn't add value to have tcpci_irq() return anything. Alternative would be to make it void tcpci_irq(struct tcpci *tcpci) if it will always only return IRQ_HANDLED. Guenter > +{ > u16 status; > > tcpci_read16(tcpci, TCPC_ALERT, &status); > @@ -412,6 +451,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) > > return IRQ_HANDLED; > } > +EXPORT_SYMBOL_GPL(tcpci_irq); > > static const struct regmap_config tcpci_regmap_config = { > .reg_bits = 8, > @@ -435,22 +475,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 +503,60 @@ static int tcpci_probe(struct i2c_client *client, > > err = tcpci_parse_config(tcpci); > if (err < 0) > - return err; > + return ERR_PTR(err); > > /* Disable chip interrupts */ > tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); > > - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, > - tcpci_irq, > + 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 *tcpci; > + struct tcpci_data *data; > + int err; > + > + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); > + if (IS_ERR(data->regmap)) > + return PTR_ERR(data->regmap); > + > + tcpci = tcpci_register_port(&client->dev, data); > + if (PTR_ERR_OR_ZERO(tcpci)) > + return PTR_ERR(tcpci); > + > + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, > + _tcpci_irq, > IRQF_ONESHOT | IRQF_TRIGGER_LOW, > dev_name(tcpci->dev), tcpci); I am a bit concerned that this may cause problems, since tcpm_register_port() will send messages to the driver, which may trigger interrupts. It may be necessary to register interrupts early, prior to calling tcpci_register_port(). > if (err < 0) > return err; You'll need to call tcpm_unregister_port() here (or register the interrupt first). Guenter > > - tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc); > - return PTR_ERR_OR_ZERO(tcpci->port); > + i2c_set_clientdata(client, tcpci); > + return 0; > } > > static int tcpci_remove(struct i2c_client *client) > { > struct tcpci *tcpci = i2c_get_clientdata(client); > > - tcpm_unregister_port(tcpci->port); > + tcpci_unregister_port(tcpci); > > return 0; > } > diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h > index fdfb06c..c838723 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,16 @@ > #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); > +}; > + > +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data); > +void tcpci_unregister_port(struct tcpci *tcpci); > +int tcpci_irq(struct tcpci *tcpci); > + > #endif /* __LINUX_USB_TCPCI_H */ > --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow 2018-02-14 14:46 ` [v2] " Guenter Roeck (?) @ 2018-02-14 15:32 ` 李書帆 -1 siblings, 0 replies; 11+ messages in thread From: 李書帆 @ 2018-02-14 15:32 UTC (permalink / raw) To: Guenter Roeck Cc: heikki.krogerus, linux-kernel, linux-usb, cy_huang, shufan_lee, Greg KH [-- Attachment #1: Type: text/plain, Size: 11855 bytes --] HI Guenter, I've thought about registering interrupt early, prior to tcpci_register_port. Just like you said. But to make sure interrupt handler will not be invoked, we'll also need to mask all alerts early. And we'll need another structure used as dev_id of devm_request_threaded_irq() because tcpci is not allocated yet at that time. For example: struct tcpci_chip { struct tcpci *tcpci; struct tcpci_data; ... }; static int tcpci_probe(struct i2c_client *client, const struct i2c_device_id *i2c_id) { u16 val = 0; struct tcpci_chip *chip; chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); .... .... .... /* Disable chip interrupts */ regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val, sizeof(u16)); err = devm_request_threaded_irq(&client->dev, client->irq, NULL, _tcpci_irq, IRQF_ONESHOT | IRQF_TRIGGER_LOW, dev_name(&client->dev), chip); if (err < 0) return err; chip->tcpci = tcpci_register_port(&client->dev, &chip->data); if (PTR_ERR_OR_ZERO(chip->tcpci)) return PTR_ERR(chip->tcpci); If you agree with this flow, I think this one will be better. 2018-02-14 22:46 GMT+08:00 Guenter Roeck <linux@roeck-us.net>: > On 02/14/2018 04:45 AM, ShuFanLee wrote: > >> From: ShuFanLee <shufan_lee@richtek.com> >> >> Handle vendor defined behavior in tcpci_init, tcpci_set_vconn 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 chagned as following: >> - Write DRP = 0 & Rd/Rd >> - Write DRP = 1 >> - Set LOOK4CONNECTION command >> >> Signed-off-by: ShuFanLee <shufan_lee@richtek.com> >> --- >> drivers/staging/typec/tcpci.c | 111 ++++++++++++++++++++++++++++++ >> ++++-------- >> drivers/staging/typec/tcpci.h | 13 +++++ >> 2 files changed, 103 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci. >> c >> index 9bd4412..461fbd48 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,7 @@ struct tcpci { >> bool controls_vbus; >> struct tcpc_dev tcpc; >> + struct tcpci_data *data; >> }; >> static inline struct tcpci *tcpc_to_tcpci(struct tcpc_dev *tcpc) >> @@ -98,8 +98,10 @@ 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; >> + unsigned int reg = (TCPC_ROLE_CTRL_CC_RD << >> TCPC_ROLE_CTRL_CC1_SHIFT) | >> + (TCPC_ROLE_CTRL_CC_RD << >> TCPC_ROLE_CTRL_CC2_SHIFT); >> switch (cc) { >> default: >> @@ -116,8 +118,19 @@ static int tcpci_start_drp_toggling(struct tcpc_dev >> *tcpc, >> TCPC_ROLE_CTRL_RP_VAL_SHIFT); >> break; >> } >> - >> - return regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); >> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); >> + if (ret < 0) >> + return ret; >> + usleep_range(500, 1000); >> + reg |= TCPC_ROLE_CTRL_DRP; >> + ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg); >> + if (ret < 0) >> + return ret; >> + ret = regmap_write(tcpci->regmap, TCPC_COMMAND, >> + TCPC_CMD_LOOK4CONNECTION); >> + if (ret < 0) >> + return ret; >> + return 0; >> } >> static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool >> sink) >> @@ -178,6 +191,16 @@ 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) { >> + 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 +346,15 @@ static int tcpci_init(struct tcpc_dev *tcpc) >> if (time_after(jiffies, timeout)) >> return -ETIMEDOUT; >> + /* Handle vendor init */ >> + if (tcpci->data) { >> + 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 +376,16 @@ 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) >> +static irqreturn_t _tcpci_irq(int irq, void *dev_id) >> { >> struct tcpci *tcpci = dev_id; >> + >> + tcpci_irq(tcpci); >> + return IRQ_HANDLED; >> > > This should probably be > return tcpci_irq(tcpci); > > +} >> + >> +int tcpci_irq(struct tcpci *tcpci) >> > > and this should be > > irqreturn_t tcpci_irq(struct tcpci *tcpci) > > Otherwise it doesn't add value to have tcpci_irq() return anything. > Alternative would be to make it > > void tcpci_irq(struct tcpci *tcpci) > > if it will always only return IRQ_HANDLED. > > Guenter > > > +{ >> u16 status; >> tcpci_read16(tcpci, TCPC_ALERT, &status); >> @@ -412,6 +451,7 @@ static irqreturn_t tcpci_irq(int irq, void *dev_id) >> return IRQ_HANDLED; >> } >> +EXPORT_SYMBOL_GPL(tcpci_irq); >> static const struct regmap_config tcpci_regmap_config = { >> .reg_bits = 8, >> @@ -435,22 +475,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 +503,60 @@ static int tcpci_probe(struct i2c_client *client, >> err = tcpci_parse_config(tcpci); >> if (err < 0) >> - return err; >> + return ERR_PTR(err); >> /* Disable chip interrupts */ >> tcpci_write16(tcpci, TCPC_ALERT_MASK, 0); >> - err = devm_request_threaded_irq(tcpci->dev, client->irq, NULL, >> - tcpci_irq, >> + 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 *tcpci; >> + struct tcpci_data *data; >> + int err; >> + >> + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + data->regmap = devm_regmap_init_i2c(client, &tcpci_regmap_config); >> + if (IS_ERR(data->regmap)) >> + return PTR_ERR(data->regmap); >> + >> + tcpci = tcpci_register_port(&client->dev, data); >> + if (PTR_ERR_OR_ZERO(tcpci)) >> + return PTR_ERR(tcpci); >> + >> + err = devm_request_threaded_irq(&client->dev, client->irq, NULL, >> + _tcpci_irq, >> IRQF_ONESHOT | IRQF_TRIGGER_LOW, >> dev_name(tcpci->dev), tcpci); >> > > I am a bit concerned that this may cause problems, since > tcpm_register_port() > will send messages to the driver, which may trigger interrupts. It may be > necessary to register interrupts early, prior to calling > tcpci_register_port(). > > if (err < 0) >> return err; >> > > You'll need to call tcpm_unregister_port() here (or register the interrupt > first). > > Guenter > > > - tcpci->port = tcpm_register_port(tcpci->dev, &tcpci->tcpc); >> - return PTR_ERR_OR_ZERO(tcpci->port); >> + i2c_set_clientdata(client, tcpci); >> + return 0; >> } >> static int tcpci_remove(struct i2c_client *client) >> { >> struct tcpci *tcpci = i2c_get_clientdata(client); >> - tcpm_unregister_port(tcpci->port); >> + tcpci_unregister_port(tcpci); >> return 0; >> } >> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci. >> h >> index fdfb06c..c838723 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,16 @@ >> #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); >> +}; >> + >> +struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data >> *data); >> +void tcpci_unregister_port(struct tcpci *tcpci); >> +int tcpci_irq(struct tcpci *tcpci); >> + >> #endif /* __LINUX_USB_TCPCI_H */ >> >> > -- Best Regards, 書帆 [-- Attachment #2: Type: text/html, Size: 17458 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-02-14 16:55 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-14 12:45 [PATCH v2] staging: typec: handle vendor defined part and modify drp toggling flow ShuFanLee 2018-02-14 12:45 ` [v2] " ShuFanLee 2018-02-14 12:57 ` [PATCH v2] " Greg KH 2018-02-14 12:57 ` [v2] " Greg KH 2018-02-14 14:43 ` [PATCH v2] " 李書帆 2018-02-14 15:37 ` Greg KH 2018-02-14 15:37 ` [v2] " Greg KH 2018-02-14 16:55 ` [PATCH v2] " 李書帆 2018-02-14 14:46 ` Guenter Roeck 2018-02-14 14:46 ` [v2] " Guenter Roeck 2018-02-14 15:32 ` [PATCH v2] " 李書帆
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.