On 13 April 2019 21:40, 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.
>
> This commit adds a new start_srp_connection_detect callback to tcpc_dev and
> when this is implemented calls this in place of start_drp_toggling for SRP
> devices.
>
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 8 ++++++++
> include/linux/usb/tcpm.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..1af54af90b50 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2553,6 +2553,14 @@ static bool tcpm_start_drp_toggling(struct tcpm_port
> *port,
> return true;
> }
>
> + if (port->tcpc->start_srp_connection_detect &&
> + port->port_type != TYPEC_PORT_DRP) {
> + tcpm_log_force(port, "Start SRP connection detection");
> + ret = port->tcpc->start_srp_connection_detect(port->tcpc, cc);
> + if (!ret)
> + return true;
> + }
> +
Is it a little misleading when reading the state machine code that we're calling
tcpm_start_drp_toggling() to actually start SRP detection? Maybe we could rename
this function and the associated state to cover both SRP and DRP, or
alternatively add a new state for SRP_DETECTION, just to keep logging and code
readability clear and then move the above code to a different function just for
SRP?
Functionally though, the change makes sense to me.
> return false;
> }
>
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h index
> 0c532ca3f079..bf2bbbf2e2b2 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -125,6 +125,10 @@ struct tcpc_config {
> * Optional; if supported by hardware, called to start DRP
> * toggling. DRP toggling is stopped automatically if
> * a connection is established.
> + * @start_srp_connection_detect:
> + * Optional; if supported by hardware, called to start connection
> + * detection for single role ports. Connection detection is stopped
> + * automatically if a connection is established.
> * @try_role: Optional; called to set a preferred role
> * @pd_transmit:Called to transmit PD message
> * @mux: Pointer to multiplexer data
> @@ -149,6 +153,8 @@ struct tcpc_dev {
> enum typec_role role, enum typec_data_role data);
> int (*start_drp_toggling)(struct tcpc_dev *dev,
> enum typec_cc_status cc);
> + int (*start_srp_connection_detect)(struct tcpc_dev *dev,
> + enum typec_cc_status cc);
> int (*try_role)(struct tcpc_dev *dev, int role);
> int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type
> type,
> const struct pd_message *msg);
> --
> 2.21.0
