On Tue, Apr 16, 2019 at 10:07:54PM +0200, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <adam.thomson.opensou...@diasemi.com>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>

Acked-by: Heikki Krogerus <heikki.kroge...@linux.intel.com>

> ---
> Changes in v2:
> -No changes
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c 
> b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum 
> typec_cc_status cc)
>                           FUSB_REG_SWITCHES0_CC2_PU_EN |
>                           FUSB_REG_SWITCHES0_CC1_PD_EN |
>                           FUSB_REG_SWITCHES0_CC2_PD_EN;
> -     u8 switches0_data = 0x00;
> +     u8 rd_mda, switches0_data = 0x00;
>       int ret = 0;
> -     enum toggling_mode mode;
>  
>       mutex_lock(&chip->lock);
>       switch (cc) {
>       case TYPEC_CC_OPEN:
> -             mode = TOGGLING_MODE_OFF;
>               break;
>       case TYPEC_CC_RD:
>               switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>                                 FUSB_REG_SWITCHES0_CC2_PD_EN;
> -             mode = TOGGLING_MODE_SNK;
>               break;
>       case TYPEC_CC_RP_DEF:
>       case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum 
> typec_cc_status cc)
>               switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>                                 FUSB_REG_SWITCHES0_CC1_PU_EN :
>                                 FUSB_REG_SWITCHES0_CC2_PU_EN;
> -             mode = TOGGLING_MODE_SRC;
>               break;
>       default:
>               fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum 
> typec_cc_status cc)
>  
>       fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
>  
> +     ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +     if (ret < 0) {
> +             fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +             goto done;
> +     }
> +
>       ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>                                    switches0_mask, switches0_data);
>       if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum 
> typec_cc_status cc)
>               goto done;
>       }
>  
> -     ret = fusb302_set_toggling(chip, mode);
> -     if (ret < 0)
> -             fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +     /* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
> +     switch (cc) {
> +     case TYPEC_CC_RP_DEF:
> +     case TYPEC_CC_RP_1_5:
> +     case TYPEC_CC_RP_3_0:
> +             rd_mda = rd_mda_value[cc_src_current[cc]];
> +             ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +             if (ret < 0) {
> +                     fusb302_log(chip,
> +                                 "cannot set SRC measure value, ret=%d",
> +                                 ret);
> +                     goto done;
> +             }
> +             ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +                                          FUSB_REG_MASK_BC_LVL |
> +                                          FUSB_REG_MASK_COMP_CHNG,
> +                                          FUSB_REG_MASK_COMP_CHNG);
> +             if (ret < 0) {
> +                     fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +                                 ret);
> +                     goto done;
> +             }
> +             chip->intr_comp_chng = true;
> +             break;
> +     case TYPEC_CC_RD:
> +             ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +                                          FUSB_REG_MASK_BC_LVL |
> +                                          FUSB_REG_MASK_COMP_CHNG,
> +                                          FUSB_REG_MASK_BC_LVL);
> +             if (ret < 0) {
> +                     fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +                                 ret);
> +                     goto done;
> +             }
> +             chip->intr_bc_lvl = true;
> +             break;
> +     default:
> +             break;
> +     }
>  done:
>       mutex_unlock(&chip->lock);
>  
> -- 
> 2.21.0

thanks,

-- 
heikki

Reply via email to