Hi Jun, Thank you. 2018-03-12 12:33 GMT+08:00 Jun Li <jun.li@xxxxxxx>: > 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, 書帆 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html