Message ID | 20201201013246.32034-2-badhri@google.com |
---|---|
State | Superseded |
Headers | show |
Series | None | expand |
On Tue, Dec 1, 2020 at 5:27 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On Mon, Nov 30, 2020 at 05:32:45PM -0800, Badhri Jagan Sridharan wrote: > > This change adds vbus_vsafe0v which when set, makes TCPM > > query for VSAFE0V by assigning the tcpc.is_vbus_vsafe0v callback. > > Also enables ALERT.ExtendedStatus which is triggered when > > status of EXTENDED_STATUS.vSafe0V changes. > > EXTENDED_STATUS.vSafe0V is set when vbus is at vSafe0V and > > cleared otherwise. > > > > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com> > > --- > > drivers/usb/typec/tcpm/tcpci.c | 55 ++++++++++++++++++++++++++-------- > > drivers/usb/typec/tcpm/tcpci.h | 6 ++++ > > 2 files changed, 49 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c > > index 12d983a75510..e281b8bee4db 100644 > > --- a/drivers/usb/typec/tcpm/tcpci.c > > +++ b/drivers/usb/typec/tcpm/tcpci.c > > @@ -402,6 +402,19 @@ static int tcpci_get_vbus(struct tcpc_dev *tcpc) > > return !!(reg & TCPC_POWER_STATUS_VBUS_PRES); > > } > > > > +static int tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc) > > +{ > > + struct tcpci *tcpci = tcpc_to_tcpci(tcpc); > > + unsigned int reg; > > + int ret; > > + > > + ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, ®); > > + if (ret < 0) > > + return ret; > > + > > + return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V); > > +} > > + > > static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink) > > { > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc); > > @@ -554,12 +567,22 @@ static int tcpci_init(struct tcpc_dev *tcpc) > > TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS; > > if (tcpci->controls_vbus) > > reg |= TCPC_ALERT_POWER_STATUS; > > + /* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */ > > + if (tcpci->data->vbus_vsafe0v) { > > + reg |= TCPC_ALERT_EXTENDED_STATUS; > > + ret = regmap_write(tcpci->regmap, TCPC_EXTENDED_STATUS_MASK, > > + TCPC_EXTENDED_STATUS_VSAFE0V); > > + if (ret < 0) > > + return ret; > > + } > > return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg); > > } > > > > irqreturn_t tcpci_irq(struct tcpci *tcpci) > > { > > u16 status; > > + int ret; > > + unsigned int raw; > > > > tcpci_read16(tcpci, TCPC_ALERT, &status); > > > > @@ -575,18 +598,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci) > > tcpm_cc_change(tcpci->port); > > > > if (status & TCPC_ALERT_POWER_STATUS) { > > - unsigned int reg; > > - > > - regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, ®); > > - > > - /* > > - * If power status mask has been reset, then the TCPC > > - * has reset. > > - */ > > - if (reg == 0xff) > > - tcpm_tcpc_reset(tcpci->port); > > - else > > - tcpm_vbus_change(tcpci->port); > > + ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw); > > + if (ret >= 0) { > > + /* > > + * If power status mask has been reset, then the TCPC > > + * has reset. > > + */ > > + if (raw == 0xff) > > + tcpm_tcpc_reset(tcpci->port); > > + else > > + tcpm_vbus_change(tcpci->port); > > + } > > This change seems unrelated to this patch. Besides that, are you sure that > ignoring an error from regmap_read() is sensible here ? Sorry should have split that into a separate patch. I was actually intending to do the following where tcpm calls are not made if TCPC_POWER_STATUS_MASK read returns error. The code was previously ignoring the error. if (!ret) { /* * If power status mask has been reset, then the TCPC * has reset. */ if (raw == 0xff) tcpm_tcpc_reset(tcpci->port); else tcpm_vbus_change(tcpci->port); } This is reasonable right ? } > > Overall, it may make sense to improve error handling in this driver, but I think > it should be done in a separate patch. > > > } > > > > if (status & TCPC_ALERT_RX_STATUS) { > > @@ -622,6 +644,12 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci) > > tcpm_pd_receive(tcpci->port, &msg); > > } > > > > + if (status & TCPC_ALERT_EXTENDED_STATUS) { > > + ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &raw); > > + if (ret >= 0 && (raw & TCPC_EXTENDED_STATUS_VSAFE0V)) > > + tcpm_vbus_change(tcpci->port); > > + } > > + > > if (status & TCPC_ALERT_RX_HARD_RST) > > tcpm_pd_hard_reset(tcpci->port); > > > > @@ -699,6 +727,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data) > > tcpci_set_auto_vbus_discharge_threshold; > > } > > > > + if (tcpci->data->vbus_vsafe0v) > > + tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v; > > + > > err = tcpci_parse_config(tcpci); > > if (err < 0) > > return ERR_PTR(err); > > diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h > > index 3fe313655f0c..116a69c85e38 100644 > > --- a/drivers/usb/typec/tcpm/tcpci.h > > +++ b/drivers/usb/typec/tcpm/tcpci.h > > @@ -49,6 +49,9 @@ > > #define TCPC_TCPC_CTRL_ORIENTATION BIT(0) > > #define TCPC_TCPC_CTRL_BIST_TM BIT(1) > > > > +#define TCPC_EXTENDED_STATUS 0x20 > > +#define TCPC_EXTENDED_STATUS_VSAFE0V BIT(0) > > + > > #define TCPC_ROLE_CTRL 0x1a > > #define TCPC_ROLE_CTRL_DRP BIT(6) > > #define TCPC_ROLE_CTRL_RP_VAL_SHIFT 4 > > @@ -155,11 +158,14 @@ struct tcpci; > > * is sourcing vbus. > > * @auto_discharge_disconnect: > > * Optional; Enables TCPC to autonously discharge vbus on disconnect. > > + * @vbus_vsafe0v: > > + * optional; Set when TCPC can detect whether vbus is at VSAFE0V. > > */ > > struct tcpci_data { > > struct regmap *regmap; > > unsigned char TX_BUF_BYTE_x_hidden:1; > > unsigned char auto_discharge_disconnect:1; > > + unsigned char vbus_vsafe0v:1; > > > > int (*init)(struct tcpci *tcpci, struct tcpci_data *data); > > int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data, > > -- > > 2.29.2.454.gaff20da3a2-goog > >
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c index 12d983a75510..e281b8bee4db 100644 --- a/drivers/usb/typec/tcpm/tcpci.c +++ b/drivers/usb/typec/tcpm/tcpci.c @@ -402,6 +402,19 @@ static int tcpci_get_vbus(struct tcpc_dev *tcpc) return !!(reg & TCPC_POWER_STATUS_VBUS_PRES); } +static int tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc) +{ + struct tcpci *tcpci = tcpc_to_tcpci(tcpc); + unsigned int reg; + int ret; + + ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, ®); + if (ret < 0) + return ret; + + return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V); +} + static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink) { struct tcpci *tcpci = tcpc_to_tcpci(tcpc); @@ -554,12 +567,22 @@ static int tcpci_init(struct tcpc_dev *tcpc) TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS; if (tcpci->controls_vbus) reg |= TCPC_ALERT_POWER_STATUS; + /* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */ + if (tcpci->data->vbus_vsafe0v) { + reg |= TCPC_ALERT_EXTENDED_STATUS; + ret = regmap_write(tcpci->regmap, TCPC_EXTENDED_STATUS_MASK, + TCPC_EXTENDED_STATUS_VSAFE0V); + if (ret < 0) + return ret; + } return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg); } irqreturn_t tcpci_irq(struct tcpci *tcpci) { u16 status; + int ret; + unsigned int raw; tcpci_read16(tcpci, TCPC_ALERT, &status); @@ -575,18 +598,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci) tcpm_cc_change(tcpci->port); if (status & TCPC_ALERT_POWER_STATUS) { - unsigned int reg; - - regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, ®); - - /* - * If power status mask has been reset, then the TCPC - * has reset. - */ - if (reg == 0xff) - tcpm_tcpc_reset(tcpci->port); - else - tcpm_vbus_change(tcpci->port); + ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw); + if (ret >= 0) { + /* + * If power status mask has been reset, then the TCPC + * has reset. + */ + if (raw == 0xff) + tcpm_tcpc_reset(tcpci->port); + else + tcpm_vbus_change(tcpci->port); + } } if (status & TCPC_ALERT_RX_STATUS) { @@ -622,6 +644,12 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci) tcpm_pd_receive(tcpci->port, &msg); } + if (status & TCPC_ALERT_EXTENDED_STATUS) { + ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &raw); + if (ret >= 0 && (raw & TCPC_EXTENDED_STATUS_VSAFE0V)) + tcpm_vbus_change(tcpci->port); + } + if (status & TCPC_ALERT_RX_HARD_RST) tcpm_pd_hard_reset(tcpci->port); @@ -699,6 +727,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data) tcpci_set_auto_vbus_discharge_threshold; } + if (tcpci->data->vbus_vsafe0v) + tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v; + err = tcpci_parse_config(tcpci); if (err < 0) return ERR_PTR(err); diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h index 3fe313655f0c..116a69c85e38 100644 --- a/drivers/usb/typec/tcpm/tcpci.h +++ b/drivers/usb/typec/tcpm/tcpci.h @@ -49,6 +49,9 @@ #define TCPC_TCPC_CTRL_ORIENTATION BIT(0) #define TCPC_TCPC_CTRL_BIST_TM BIT(1) +#define TCPC_EXTENDED_STATUS 0x20 +#define TCPC_EXTENDED_STATUS_VSAFE0V BIT(0) + #define TCPC_ROLE_CTRL 0x1a #define TCPC_ROLE_CTRL_DRP BIT(6) #define TCPC_ROLE_CTRL_RP_VAL_SHIFT 4 @@ -155,11 +158,14 @@ struct tcpci; * is sourcing vbus. * @auto_discharge_disconnect: * Optional; Enables TCPC to autonously discharge vbus on disconnect. + * @vbus_vsafe0v: + * optional; Set when TCPC can detect whether vbus is at VSAFE0V. */ struct tcpci_data { struct regmap *regmap; unsigned char TX_BUF_BYTE_x_hidden:1; unsigned char auto_discharge_disconnect:1; + unsigned char vbus_vsafe0v:1; int (*init)(struct tcpci *tcpci, struct tcpci_data *data); int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
This change adds vbus_vsafe0v which when set, makes TCPM query for VSAFE0V by assigning the tcpc.is_vbus_vsafe0v callback. Also enables ALERT.ExtendedStatus which is triggered when status of EXTENDED_STATUS.vSafe0V changes. EXTENDED_STATUS.vSafe0V is set when vbus is at vSafe0V and cleared otherwise. Signed-off-by: Badhri Jagan Sridharan <badhri@google.com> --- drivers/usb/typec/tcpm/tcpci.c | 55 ++++++++++++++++++++++++++-------- drivers/usb/typec/tcpm/tcpci.h | 6 ++++ 2 files changed, 49 insertions(+), 12 deletions(-)