| Message ID | 20190311104818.30216-6-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 702ad49fb8d2d5c8f3ddd22e4119d9e507e5c1a2 |
| Headers | show |
| Series | usb: typec: fusb302: Various fixes | expand |
On 3/11/19 3:48 AM, Hans de Goede wrote: > The FUSB302 will stop toggling with a FUSB_REG_STATUS1A_TOGSS_SRC? status, > as soon as it sees either Ra or Rd on a CC pin. > > Before this commit fusb302_handle_togdone_src would assume that the toggle- > engine always stopped at the CC pin indicating the polarity, IOW it assumed > that it stopped at the pin connected to Rd. It did check the CC-status of > that pin, but it did not expect to get a CC-status of Ra and therefore > treated this as CC-open. This lead to the following 2 problems: > > 1) If a powered cable/adapter gets plugged in with Ra on CC1 and Rd on CC2 > then 4 of 5 times when plugged in toggling will stop with a togdone_result > of FUSB_REG_STATUS1A_TOGSS_SRC1. 3/5th of the time the toggle-engine is > testing for being connected as a sink and after that it tests 1/5th of the > time for connected as a src through CC1 before finally testing the last > 1/5th of the time for being a src connected through CC2. > > This was a problem because we would only check the CC pin status for the > pin on which the toggling stopped which in this polarity 4 out of 5 > times would be the Ra pin. The code before this commit would treat Ra as > CC-open and then restart toggling. Once toggling is restarted we are > guaranteed to end with FUSB_REG_STATUS1A_TOGSS_SRC1 as CC1 is tested first, > leading to a CC-status of Ra again and an infinite restart toggling loop. > So 4 out of 5 times when plugged in in this polarity a powered adapter > will not work. > > 2) Even if we happen to have the right polarity or 1/5th of the time in > the polarity with problem 1), we would report the non Rd pin as CC-open > rather then as Ra, resulting in the tcpm.c code not enabling Vconn which > is a problem for some adapters. > > This commit fixes this by getting the CC-status of *both* pins and then > determining the polarity based on that, rather then on where the toggling > stopped. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > Changes in v3 > -Keep the delays for measuring Rd / Ra the same as before instead of > multiplying them by a factor 100 > --- > drivers/usb/typec/tcpm/fusb302.c | 149 ++++++++++++++++++++----------- > 1 file changed, 97 insertions(+), 52 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c > index a1256855eaa0..a4387ce5ac85 100644 > --- a/drivers/usb/typec/tcpm/fusb302.c > +++ b/drivers/usb/typec/tcpm/fusb302.c > @@ -1255,6 +1255,62 @@ static int fusb302_handle_togdone_snk(struct fusb302_chip *chip, > return ret; > } > > +/* On error returns < 0, otherwise a typec_cc_status value */ > +static int fusb302_get_src_cc_status(struct fusb302_chip *chip, > + enum typec_cc_polarity cc_polarity, > + enum typec_cc_status *cc) > +{ > + u8 ra_mda = ra_mda_value[chip->src_current_status]; > + u8 rd_mda = rd_mda_value[chip->src_current_status]; > + u8 switches0_data, status0; > + int ret; > + > + /* Step 1: Set switches so that we measure the right CC pin */ > + switches0_data = (cc_polarity == TYPEC_POLARITY_CC1) ? > + FUSB_REG_SWITCHES0_CC1_PU_EN | FUSB_REG_SWITCHES0_MEAS_CC1 : > + FUSB_REG_SWITCHES0_CC2_PU_EN | FUSB_REG_SWITCHES0_MEAS_CC2; > + ret = fusb302_i2c_write(chip, FUSB_REG_SWITCHES0, switches0_data); > + if (ret < 0) > + return ret; > + > + fusb302_i2c_read(chip, FUSB_REG_SWITCHES0, &status0); > + fusb302_log(chip, "get_src_cc_status switches: 0x%0x", status0); > + > + /* Step 2: Set compararator volt to differentiate between Open and Rd */ > + ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda); > + if (ret < 0) > + return ret; > + > + usleep_range(50, 100); > + ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); > + if (ret < 0) > + return ret; > + > + fusb302_log(chip, "get_src_cc_status rd_mda status0: 0x%0x", status0); > + if (status0 & FUSB_REG_STATUS0_COMP) { > + *cc = TYPEC_CC_OPEN; > + return 0; > + } > + > + /* Step 3: Set compararator input to differentiate between Rd and Ra. */ > + ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, ra_mda); > + if (ret < 0) > + return ret; > + > + usleep_range(50, 100); > + ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); > + if (ret < 0) > + return ret; > + > + fusb302_log(chip, "get_src_cc_status ra_mda status0: 0x%0x", status0); > + if (status0 & FUSB_REG_STATUS0_COMP) > + *cc = TYPEC_CC_RD; > + else > + *cc = TYPEC_CC_RA; > + > + return 0; > +} > + > static int fusb302_handle_togdone_src(struct fusb302_chip *chip, > u8 togdone_result) > { > @@ -1265,71 +1321,62 @@ static int fusb302_handle_togdone_src(struct fusb302_chip *chip, > * - set I_COMP interrupt on > */ > int ret = 0; > - u8 status0; > - u8 ra_mda = ra_mda_value[chip->src_current_status]; > u8 rd_mda = rd_mda_value[chip->src_current_status]; > - bool ra_comp, rd_comp; > + enum toggling_mode toggling_mode = chip->toggling_mode; > enum typec_cc_polarity cc_polarity; > - enum typec_cc_status cc_status_active, cc1, cc2; > + enum typec_cc_status cc1, cc2; > > - /* set polarity and pull_up, pull_down */ > - cc_polarity = (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) ? > - TYPEC_POLARITY_CC1 : TYPEC_POLARITY_CC2; > - ret = fusb302_set_cc_polarity_and_pull(chip, cc_polarity, true, false); > + /* > + * The toggle-engine will stop in a src state if it sees either Ra or > + * Rd. Determine the status for both CC pins, starting with the one > + * where toggling stopped, as that is where the switches point now. > + */ > + if (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) > + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC1, &cc1); > + else > + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC2, &cc2); > + if (ret < 0) > + return ret; > + /* we must turn off toggling before we can measure the other pin */ > + ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF); > if (ret < 0) { > - fusb302_log(chip, "cannot set cc polarity %s, ret=%d", > - cc_polarity_name[cc_polarity], ret); > + fusb302_log(chip, "cannot set toggling mode off, ret=%d", ret); > return ret; > } > - /* fusb302_set_cc_polarity() has set the correct measure block */ > - ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda); > - if (ret < 0) > - return ret; > - usleep_range(50, 100); > - ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); > + /* get the status of the other pin */ > + if (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) > + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC2, &cc2); > + else > + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC1, &cc1); > if (ret < 0) > return ret; > - rd_comp = !!(status0 & FUSB_REG_STATUS0_COMP); > - if (!rd_comp) { > - ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, ra_mda); > - if (ret < 0) > - return ret; > - usleep_range(50, 100); > - ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); > - if (ret < 0) > - return ret; > - ra_comp = !!(status0 & FUSB_REG_STATUS0_COMP); > + > + /* determine polarity based on the status of both pins */ > + if (cc1 == TYPEC_CC_RD && > + (cc2 == TYPEC_CC_OPEN || cc2 == TYPEC_CC_RA)) { > + cc_polarity = TYPEC_POLARITY_CC1; > + } else if (cc2 == TYPEC_CC_RD && > + (cc1 == TYPEC_CC_OPEN || cc1 == TYPEC_CC_RA)) { > + cc_polarity = TYPEC_POLARITY_CC2; > + } else { > + fusb302_log(chip, "unexpected CC status cc1=%s, cc2=%s, restarting toggling", > + typec_cc_status_name[cc1], > + typec_cc_status_name[cc2]); > + return fusb302_set_toggling(chip, toggling_mode); > } > - if (rd_comp) > - cc_status_active = TYPEC_CC_OPEN; > - else if (ra_comp) > - cc_status_active = TYPEC_CC_RD; > - else > - /* Ra is not supported, report as Open */ > - cc_status_active = TYPEC_CC_OPEN; > - /* restart toggling if the cc status on the active line is OPEN */ > - if (cc_status_active == TYPEC_CC_OPEN) { > - fusb302_log(chip, "restart toggling as CC_OPEN detected"); > - ret = fusb302_set_toggling(chip, chip->toggling_mode); > + /* set polarity and pull_up, pull_down */ > + ret = fusb302_set_cc_polarity_and_pull(chip, cc_polarity, true, false); > + if (ret < 0) { > + fusb302_log(chip, "cannot set cc polarity %s, ret=%d", > + cc_polarity_name[cc_polarity], ret); > return ret; > } > /* update tcpm with the new cc value */ > - cc1 = (cc_polarity == TYPEC_POLARITY_CC1) ? > - cc_status_active : TYPEC_CC_OPEN; > - cc2 = (cc_polarity == TYPEC_POLARITY_CC2) ? > - cc_status_active : TYPEC_CC_OPEN; > if ((chip->cc1 != cc1) || (chip->cc2 != cc2)) { > chip->cc1 = cc1; > chip->cc2 = cc2; > tcpm_cc_change(chip->tcpm_port); > } > - /* turn off toggling */ > - ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF); > - if (ret < 0) { > - fusb302_log(chip, > - "cannot set toggling mode off, ret=%d", ret); > - return ret; > - } > /* set MDAC to Rd threshold, and unmask I_COMP for unplug detection */ > ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda); > if (ret < 0) > @@ -1515,10 +1562,8 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id) > comp_result ? "true" : "false"); > if (comp_result) { > /* cc level > Rd_threashold, detach */ > - if (chip->cc_polarity == TYPEC_POLARITY_CC1) > - chip->cc1 = TYPEC_CC_OPEN; > - else > - chip->cc2 = TYPEC_CC_OPEN; > + chip->cc1 = TYPEC_CC_OPEN; > + chip->cc2 = TYPEC_CC_OPEN; > tcpm_cc_change(chip->tcpm_port); > } > } >
diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c index a1256855eaa0..a4387ce5ac85 100644 --- a/drivers/usb/typec/tcpm/fusb302.c +++ b/drivers/usb/typec/tcpm/fusb302.c @@ -1255,6 +1255,62 @@ static int fusb302_handle_togdone_snk(struct fusb302_chip *chip, return ret; } +/* On error returns < 0, otherwise a typec_cc_status value */ +static int fusb302_get_src_cc_status(struct fusb302_chip *chip, + enum typec_cc_polarity cc_polarity, + enum typec_cc_status *cc) +{ + u8 ra_mda = ra_mda_value[chip->src_current_status]; + u8 rd_mda = rd_mda_value[chip->src_current_status]; + u8 switches0_data, status0; + int ret; + + /* Step 1: Set switches so that we measure the right CC pin */ + switches0_data = (cc_polarity == TYPEC_POLARITY_CC1) ? + FUSB_REG_SWITCHES0_CC1_PU_EN | FUSB_REG_SWITCHES0_MEAS_CC1 : + FUSB_REG_SWITCHES0_CC2_PU_EN | FUSB_REG_SWITCHES0_MEAS_CC2; + ret = fusb302_i2c_write(chip, FUSB_REG_SWITCHES0, switches0_data); + if (ret < 0) + return ret; + + fusb302_i2c_read(chip, FUSB_REG_SWITCHES0, &status0); + fusb302_log(chip, "get_src_cc_status switches: 0x%0x", status0); + + /* Step 2: Set compararator volt to differentiate between Open and Rd */ + ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda); + if (ret < 0) + return ret; + + usleep_range(50, 100); + ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); + if (ret < 0) + return ret; + + fusb302_log(chip, "get_src_cc_status rd_mda status0: 0x%0x", status0); + if (status0 & FUSB_REG_STATUS0_COMP) { + *cc = TYPEC_CC_OPEN; + return 0; + } + + /* Step 3: Set compararator input to differentiate between Rd and Ra. */ + ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, ra_mda); + if (ret < 0) + return ret; + + usleep_range(50, 100); + ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); + if (ret < 0) + return ret; + + fusb302_log(chip, "get_src_cc_status ra_mda status0: 0x%0x", status0); + if (status0 & FUSB_REG_STATUS0_COMP) + *cc = TYPEC_CC_RD; + else + *cc = TYPEC_CC_RA; + + return 0; +} + static int fusb302_handle_togdone_src(struct fusb302_chip *chip, u8 togdone_result) { @@ -1265,71 +1321,62 @@ static int fusb302_handle_togdone_src(struct fusb302_chip *chip, * - set I_COMP interrupt on */ int ret = 0; - u8 status0; - u8 ra_mda = ra_mda_value[chip->src_current_status]; u8 rd_mda = rd_mda_value[chip->src_current_status]; - bool ra_comp, rd_comp; + enum toggling_mode toggling_mode = chip->toggling_mode; enum typec_cc_polarity cc_polarity; - enum typec_cc_status cc_status_active, cc1, cc2; + enum typec_cc_status cc1, cc2; - /* set polarity and pull_up, pull_down */ - cc_polarity = (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) ? - TYPEC_POLARITY_CC1 : TYPEC_POLARITY_CC2; - ret = fusb302_set_cc_polarity_and_pull(chip, cc_polarity, true, false); + /* + * The toggle-engine will stop in a src state if it sees either Ra or + * Rd. Determine the status for both CC pins, starting with the one + * where toggling stopped, as that is where the switches point now. + */ + if (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC1, &cc1); + else + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC2, &cc2); + if (ret < 0) + return ret; + /* we must turn off toggling before we can measure the other pin */ + ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF); if (ret < 0) { - fusb302_log(chip, "cannot set cc polarity %s, ret=%d", - cc_polarity_name[cc_polarity], ret); + fusb302_log(chip, "cannot set toggling mode off, ret=%d", ret); return ret; } - /* fusb302_set_cc_polarity() has set the correct measure block */ - ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda); - if (ret < 0) - return ret; - usleep_range(50, 100); - ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); + /* get the status of the other pin */ + if (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC2, &cc2); + else + ret = fusb302_get_src_cc_status(chip, TYPEC_POLARITY_CC1, &cc1); if (ret < 0) return ret; - rd_comp = !!(status0 & FUSB_REG_STATUS0_COMP); - if (!rd_comp) { - ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, ra_mda); - if (ret < 0) - return ret; - usleep_range(50, 100); - ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0); - if (ret < 0) - return ret; - ra_comp = !!(status0 & FUSB_REG_STATUS0_COMP); + + /* determine polarity based on the status of both pins */ + if (cc1 == TYPEC_CC_RD && + (cc2 == TYPEC_CC_OPEN || cc2 == TYPEC_CC_RA)) { + cc_polarity = TYPEC_POLARITY_CC1; + } else if (cc2 == TYPEC_CC_RD && + (cc1 == TYPEC_CC_OPEN || cc1 == TYPEC_CC_RA)) { + cc_polarity = TYPEC_POLARITY_CC2; + } else { + fusb302_log(chip, "unexpected CC status cc1=%s, cc2=%s, restarting toggling", + typec_cc_status_name[cc1], + typec_cc_status_name[cc2]); + return fusb302_set_toggling(chip, toggling_mode); } - if (rd_comp) - cc_status_active = TYPEC_CC_OPEN; - else if (ra_comp) - cc_status_active = TYPEC_CC_RD; - else - /* Ra is not supported, report as Open */ - cc_status_active = TYPEC_CC_OPEN; - /* restart toggling if the cc status on the active line is OPEN */ - if (cc_status_active == TYPEC_CC_OPEN) { - fusb302_log(chip, "restart toggling as CC_OPEN detected"); - ret = fusb302_set_toggling(chip, chip->toggling_mode); + /* set polarity and pull_up, pull_down */ + ret = fusb302_set_cc_polarity_and_pull(chip, cc_polarity, true, false); + if (ret < 0) { + fusb302_log(chip, "cannot set cc polarity %s, ret=%d", + cc_polarity_name[cc_polarity], ret); return ret; } /* update tcpm with the new cc value */ - cc1 = (cc_polarity == TYPEC_POLARITY_CC1) ? - cc_status_active : TYPEC_CC_OPEN; - cc2 = (cc_polarity == TYPEC_POLARITY_CC2) ? - cc_status_active : TYPEC_CC_OPEN; if ((chip->cc1 != cc1) || (chip->cc2 != cc2)) { chip->cc1 = cc1; chip->cc2 = cc2; tcpm_cc_change(chip->tcpm_port); } - /* turn off toggling */ - ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF); - if (ret < 0) { - fusb302_log(chip, - "cannot set toggling mode off, ret=%d", ret); - return ret; - } /* set MDAC to Rd threshold, and unmask I_COMP for unplug detection */ ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda); if (ret < 0) @@ -1515,10 +1562,8 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id) comp_result ? "true" : "false"); if (comp_result) { /* cc level > Rd_threashold, detach */ - if (chip->cc_polarity == TYPEC_POLARITY_CC1) - chip->cc1 = TYPEC_CC_OPEN; - else - chip->cc2 = TYPEC_CC_OPEN; + chip->cc1 = TYPEC_CC_OPEN; + chip->cc2 = TYPEC_CC_OPEN; tcpm_cc_change(chip->tcpm_port); } }
The FUSB302 will stop toggling with a FUSB_REG_STATUS1A_TOGSS_SRC? status, as soon as it sees either Ra or Rd on a CC pin. Before this commit fusb302_handle_togdone_src would assume that the toggle- engine always stopped at the CC pin indicating the polarity, IOW it assumed that it stopped at the pin connected to Rd. It did check the CC-status of that pin, but it did not expect to get a CC-status of Ra and therefore treated this as CC-open. This lead to the following 2 problems: 1) If a powered cable/adapter gets plugged in with Ra on CC1 and Rd on CC2 then 4 of 5 times when plugged in toggling will stop with a togdone_result of FUSB_REG_STATUS1A_TOGSS_SRC1. 3/5th of the time the toggle-engine is testing for being connected as a sink and after that it tests 1/5th of the time for connected as a src through CC1 before finally testing the last 1/5th of the time for being a src connected through CC2. This was a problem because we would only check the CC pin status for the pin on which the toggling stopped which in this polarity 4 out of 5 times would be the Ra pin. The code before this commit would treat Ra as CC-open and then restart toggling. Once toggling is restarted we are guaranteed to end with FUSB_REG_STATUS1A_TOGSS_SRC1 as CC1 is tested first, leading to a CC-status of Ra again and an infinite restart toggling loop. So 4 out of 5 times when plugged in in this polarity a powered adapter will not work. 2) Even if we happen to have the right polarity or 1/5th of the time in the polarity with problem 1), we would report the non Rd pin as CC-open rather then as Ra, resulting in the tcpm.c code not enabling Vconn which is a problem for some adapters. This commit fixes this by getting the CC-status of *both* pins and then determining the polarity based on that, rather then on where the toggling stopped. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v3 -Keep the delays for measuring Rd / Ra the same as before instead of multiplying them by a factor 100 --- drivers/usb/typec/tcpm/fusb302.c | 149 ++++++++++++++++++++----------- 1 file changed, 97 insertions(+), 52 deletions(-)