diff mbox series

[v1,2/3] usb: typec: tcpci: Add support to report vSafe0V

Message ID 20201201013246.32034-2-badhri@google.com
State Superseded
Headers show
Series None | expand

Commit Message

Badhri Jagan Sridharan Dec. 1, 2020, 1:32 a.m. UTC
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(-)

Comments

Badhri Jagan Sridharan Dec. 2, 2020, 3:34 a.m. UTC | #1
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, &reg);
> > +     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, &reg);
> > -
> > -             /*
> > -              * 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 mbox series

Patch

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, &reg);
+	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, &reg);
-
-		/*
-		 * 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,