| Message ID | 20190307163607.24016-5-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: fusb302: Various fixes | expand |
On Thu, Mar 07, 2019 at 05:36:03PM +0100, Hans de Goede wrote: > The datasheet says the vconn MUST be off when we start toggling. The > tcpm.c state-machine is responsible to make sure vconn is off, but > lets add a WARN_ON check to catch any cases where vconn is not off > for some reason. > Curious - have you seen this happening ? > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/usb/typec/tcpm/fusb302.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c > index cb6637e82f32..f43fe34b7a73 100644 > --- a/drivers/usb/typec/tcpm/fusb302.c > +++ b/drivers/usb/typec/tcpm/fusb302.c > @@ -607,6 +607,8 @@ static int fusb302_set_toggling(struct fusb302_chip *chip, > return ret; > chip->intr_togdone = false; > } else { > + /* Datasheet says vconn MUST be off when toggling */ > + WARN_ON(chip->vconn_on); Personally I am not a friend of WARN_ON. I prefer WARN statements telling me what is wrong. > /* unmask TOGDONE interrupt */ > ret = fusb302_i2c_clear_bits(chip, FUSB_REG_MASKA, > FUSB_REG_MASKA_TOGDONE); > -- > 2.20.1 >
Hi, On 07-03-19 19:11, Guenter Roeck wrote: > On Thu, Mar 07, 2019 at 05:36:03PM +0100, Hans de Goede wrote: >> The datasheet says the vconn MUST be off when we start toggling. The >> tcpm.c state-machine is responsible to make sure vconn is off, but >> lets add a WARN_ON check to catch any cases where vconn is not off >> for some reason. >> > > Curious - have you seen this happening ? No, just something which I noticed in the datasheet and it seems like a good idea to add a check for this. >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/usb/typec/tcpm/fusb302.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c >> index cb6637e82f32..f43fe34b7a73 100644 >> --- a/drivers/usb/typec/tcpm/fusb302.c >> +++ b/drivers/usb/typec/tcpm/fusb302.c >> @@ -607,6 +607,8 @@ static int fusb302_set_toggling(struct fusb302_chip *chip, >> return ret; >> chip->intr_togdone = false; >> } else { >> + /* Datasheet says vconn MUST be off when toggling */ >> + WARN_ON(chip->vconn_on); > > Personally I am not a friend of WARN_ON. I prefer WARN statements > telling me what is wrong. Ok, I will switch to WARN() for v3 of the series. Regards, Hans >> /* unmask TOGDONE interrupt */ >> ret = fusb302_i2c_clear_bits(chip, FUSB_REG_MASKA, >> FUSB_REG_MASKA_TOGDONE); >> -- >> 2.20.1 >>
diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c index cb6637e82f32..f43fe34b7a73 100644 --- a/drivers/usb/typec/tcpm/fusb302.c +++ b/drivers/usb/typec/tcpm/fusb302.c @@ -607,6 +607,8 @@ static int fusb302_set_toggling(struct fusb302_chip *chip, return ret; chip->intr_togdone = false; } else { + /* Datasheet says vconn MUST be off when toggling */ + WARN_ON(chip->vconn_on); /* unmask TOGDONE interrupt */ ret = fusb302_i2c_clear_bits(chip, FUSB_REG_MASKA, FUSB_REG_MASKA_TOGDONE);
The datasheet says the vconn MUST be off when we start toggling. The tcpm.c state-machine is responsible to make sure vconn is off, but lets add a WARN_ON check to catch any cases where vconn is not off for some reason. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/usb/typec/tcpm/fusb302.c | 2 ++ 1 file changed, 2 insertions(+)