Message ID | 20220203144657.16527-4-heikki.krogerus@linux.intel.com |
---|---|
State | New |
Headers | show |
Series | usb: typec: Separate sysfs directory for all USB PD objects | expand |
Hi, On Wed, Feb 09, 2022 at 11:56:11PM -0800, Jack Pham wrote: > Hi Heikki, > > On Thu, Feb 03, 2022 at 05:46:57PM +0300, Heikki Krogerus wrote: > > UCSI allows the USB PD capabilities to be read with the > > GET_PDO command. This will register those capabilities, and > > that way make them visible to the user space. > > > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > --- > > drivers/usb/typec/ucsi/ucsi.c | 128 +++++++++++++++++++++++++++++++--- > > drivers/usb/typec/ucsi/ucsi.h | 8 +++ > > 2 files changed, 125 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > > index f0c2fa19f3e0f..5149001093c7f 100644 > > --- a/drivers/usb/typec/ucsi/ucsi.c > > +++ b/drivers/usb/typec/ucsi/ucsi.c > > @@ -568,8 +568,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) > > } > > } > > > > -static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, > > - u32 *pdos, int offset, int num_pdos) > > +static int ucsi_read_pdos(struct ucsi_connector *con, enum typec_role role, int is_partner, > > + u32 *pdos, int offset, int num_pdos) > > { > > struct ucsi *ucsi = con->ucsi; > > u64 command; > > @@ -579,7 +579,7 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, > > command |= UCSI_GET_PDOS_PARTNER_PDO(is_partner); > > command |= UCSI_GET_PDOS_PDO_OFFSET(offset); > > command |= UCSI_GET_PDOS_NUM_PDOS(num_pdos - 1); > > - command |= UCSI_GET_PDOS_SRC_PDOS; > > + command |= is_source(role) ? UCSI_GET_PDOS_SRC_PDOS : 0; > > ret = ucsi_send_command(ucsi, command, pdos + offset, > > num_pdos * sizeof(u32)); > > if (ret < 0 && ret != -ETIMEDOUT) > > @@ -590,26 +590,39 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, > > return ret; > > } > > > > -static int ucsi_get_src_pdos(struct ucsi_connector *con) > > +static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role, > > + int is_partner, u32 *pdos) > > { > > + u8 num_pdos; > > int ret; > > > > /* UCSI max payload means only getting at most 4 PDOs at a time */ > > - ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS); > > + ret = ucsi_read_pdos(con, role, is_partner, pdos, 0, UCSI_MAX_PDOS); > > if (ret < 0) > > return ret; > > > > - con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */ > > - if (con->num_pdos < UCSI_MAX_PDOS) > > - return 0; > > + num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */ > > + if (num_pdos < UCSI_MAX_PDOS) > > + return num_pdos; > > > > /* get the remaining PDOs, if any */ > > - ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS, > > - PDO_MAX_OBJECTS - UCSI_MAX_PDOS); > > + ret = ucsi_read_pdos(con, role, is_partner, pdos, UCSI_MAX_PDOS, > > + PDO_MAX_OBJECTS - UCSI_MAX_PDOS); > > + if (ret < 0) > > + return ret; > > + > > + return ret / sizeof(u32) + num_pdos; > > +} > > + > > +static int ucsi_get_src_pdos(struct ucsi_connector *con) > > +{ > > + int ret; > > + > > + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 1, con->src_pdos); > > This issues the GET_PDOS command to PPM to get the source PDOs of the > partner... > > > if (ret < 0) > > return ret; > > > > - con->num_pdos += ret / sizeof(u32); > > + con->num_pdos += ret; > > > > ucsi_port_psy_changed(con); > > > > @@ -638,6 +651,60 @@ static int ucsi_check_altmodes(struct ucsi_connector *con) > > return ret; > > } > > > > +static int ucsi_register_partner_pdos(struct ucsi_connector *con) > > +{ > > + struct pd_desc desc = { con->ucsi->cap.pd_version }; > > + struct pd_capabilities *cap; > > + struct pd_caps_desc caps; > > + int ret; > > + > > + con->partner_pd = typec_partner_register_pd(con->partner, &desc); > > + if (IS_ERR(con->partner_pd)) > > + return PTR_ERR(con->partner_pd); > > + > > + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 1, caps.pdo); > > ... and also here. > > > + if (ret < 0) > > + return ret; > > + > > + if (ret < PDO_MAX_OBJECTS) > > + caps.pdo[ret] = 0; > > + caps.role = TYPEC_SOURCE; > > + > > + cap = pd_register_capabilities(con->partner_pd, &caps); > > + if (IS_ERR(cap)) > > + return PTR_ERR(cap); > > + > > + ret = typec_partner_set_pd_capabilities(con->partner, cap); > > + if (ret) { > > + pd_unregister_capabilities(cap); > > + return ret; > > + } > > + > > + con->partner_source_caps = cap; > > + > > + ret = ucsi_get_pdos(con, TYPEC_SINK, 1, caps.pdo); > > + if (ret <= 0) > > + return ret; > > + > > + if (ret < PDO_MAX_OBJECTS) > > + caps.pdo[ret] = 0; > > + caps.role = TYPEC_SINK; > > + > > + cap = pd_register_capabilities(con->partner_pd, &caps); > > + if (IS_ERR(cap)) > > + return PTR_ERR(cap); > > + > > + ret = typec_partner_set_pd_capabilities(con->partner, cap); > > + if (ret) { > > + pd_unregister_capabilities(cap); > > + return ret; > > + } > > + > > + con->partner_sink_caps = cap; > > + > > + return 0; > > +} > > + > > static void ucsi_pwr_opmode_change(struct ucsi_connector *con) > > { > > switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { > > @@ -646,6 +713,7 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con) > > typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD); > > ucsi_partner_task(con, ucsi_get_src_pdos, 30, 0); > > ucsi_partner_task(con, ucsi_check_altmodes, 30, 0); > > + ucsi_partner_task(con, ucsi_register_partner_pdos, 1, HZ); > > And, both ucsi_get_src_pdos() and ucsi_register_partner_pdos() are > scheduled to run here... > > > break; > > case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5: > > con->rdo = 0; > > @@ -704,6 +772,17 @@ static void ucsi_unregister_partner(struct ucsi_connector *con) > > if (!con->partner) > > return; > > > > + typec_partner_unset_pd_capabilities(con->partner, TYPEC_SINK); > > + pd_unregister_capabilities(con->partner_sink_caps); > > + con->partner_sink_caps = NULL; > > + > > + typec_partner_unset_pd_capabilities(con->partner, TYPEC_SOURCE); > > + pd_unregister_capabilities(con->partner_source_caps); > > + con->partner_source_caps = NULL; > > + > > + typec_partner_unregister_pd(con->partner); > > + con->partner_pd = NULL; > > + > > ucsi_unregister_altmodes(con, UCSI_RECIPIENT_SOP); > > typec_unregister_partner(con->partner); > > con->partner = NULL; > > @@ -1037,6 +1116,8 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > > u64 command; > > char *name; > > int ret; > > + struct pd_desc desc = { ucsi->cap.pd_version }; > > + struct pd_caps_desc caps; > > > > name = kasprintf(GFP_KERNEL, "%s-con%d", dev_name(ucsi->dev), con->num); > > if (!name) > > @@ -1103,6 +1184,24 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > > goto out; > > } > > > > + con->pd = typec_port_register_pd(con->port, &desc); > > + > > + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 0, caps.pdo); > > + if (ret > 0) { > > + caps.pdo[ret] = 0; > > + caps.role = TYPEC_SOURCE; > > + con->source_caps = pd_register_capabilities(con->pd, &caps); > > + typec_port_set_pd_capabilities(con->port, con->source_caps); > > + } > > + > > + ret = ucsi_get_pdos(con, TYPEC_SINK, 0, caps.pdo); > > + if (ret > 0) { > > + caps.pdo[ret] = 0; > > + caps.role = TYPEC_SINK; > > + con->sink_caps = pd_register_capabilities(con->pd, &caps); > > + typec_port_set_pd_capabilities(con->port, con->sink_caps); > > + } > > + > > /* Alternate modes */ > > ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_CON); > > if (ret) { > > @@ -1169,6 +1268,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > > UCSI_CONSTAT_PWR_OPMODE_PD) { > > ucsi_get_src_pdos(con); > > ucsi_check_altmodes(con); > > + ucsi_register_partner_pdos(con); > > ... as well as here. > > So wouldn't this result in the PPM issuing the same PD Get_Source_Cap > message twice to the port partner (in either case of initial port > registration or op mode change)? Could we just consolidate them to just > issue GET_PDOS only once and take care of populating the partner's > Source Caps for both the pd_capabilties as well as power_supply purposes > from a single helper? Yes. I tried create as little impact on existing code with this as possible, so I just duplicated that part, but I probable should not have done that. > Another aside, thinking back to a previous patch [1] I had proposed a > few months ago, another question I had is whether it is proper to even > issue a Get_Source_Cap message to sink-only devices, as we did encounter > certain DisplayPort adapters that don't like when that happens. > Wondering if it could be possible to limit calling the GET_PDOS command > unless we know the partner is capable of operating in that particular > power role. e.g. don't call get_src_pdos() if partner is sink-only. > Or is this the kind of thing that the PPM is supposed to be able to > figure out and allow OPM to naively issue the command regardless, and > just get an error/empty return? Well, in many cases we simply can not rely on the PPM, but I think we should be able rely on it with the role. What you are proposing probable would make sense. Do you want to work on those improvements? I don't have to modify this driver now with this series. I also can first register the sysfs attributes from tcpm.c. thanks,
On Tue, Feb 15, 2022 at 02:33:38PM +0200, Heikki Krogerus wrote: > Hi, > > On Wed, Feb 09, 2022 at 11:56:11PM -0800, Jack Pham wrote: > > Hi Heikki, > > > > On Thu, Feb 03, 2022 at 05:46:57PM +0300, Heikki Krogerus wrote: > > > UCSI allows the USB PD capabilities to be read with the > > > GET_PDO command. This will register those capabilities, and > > > that way make them visible to the user space. > > > > > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > > --- > > > drivers/usb/typec/ucsi/ucsi.c | 128 +++++++++++++++++++++++++++++++--- > > > drivers/usb/typec/ucsi/ucsi.h | 8 +++ > > > 2 files changed, 125 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > > > index f0c2fa19f3e0f..5149001093c7f 100644 > > > --- a/drivers/usb/typec/ucsi/ucsi.c > > > +++ b/drivers/usb/typec/ucsi/ucsi.c > > > @@ -568,8 +568,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) > > > } > > > } > > > > > > -static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, > > > - u32 *pdos, int offset, int num_pdos) > > > +static int ucsi_read_pdos(struct ucsi_connector *con, enum typec_role role, int is_partner, > > > + u32 *pdos, int offset, int num_pdos) > > > { > > > struct ucsi *ucsi = con->ucsi; > > > u64 command; > > > @@ -579,7 +579,7 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, > > > command |= UCSI_GET_PDOS_PARTNER_PDO(is_partner); > > > command |= UCSI_GET_PDOS_PDO_OFFSET(offset); > > > command |= UCSI_GET_PDOS_NUM_PDOS(num_pdos - 1); > > > - command |= UCSI_GET_PDOS_SRC_PDOS; > > > + command |= is_source(role) ? UCSI_GET_PDOS_SRC_PDOS : 0; > > > ret = ucsi_send_command(ucsi, command, pdos + offset, > > > num_pdos * sizeof(u32)); > > > if (ret < 0 && ret != -ETIMEDOUT) > > > @@ -590,26 +590,39 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, > > > return ret; > > > } > > > > > > -static int ucsi_get_src_pdos(struct ucsi_connector *con) > > > +static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role, > > > + int is_partner, u32 *pdos) > > > { > > > + u8 num_pdos; > > > int ret; > > > > > > /* UCSI max payload means only getting at most 4 PDOs at a time */ > > > - ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS); > > > + ret = ucsi_read_pdos(con, role, is_partner, pdos, 0, UCSI_MAX_PDOS); > > > if (ret < 0) > > > return ret; > > > > > > - con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */ > > > - if (con->num_pdos < UCSI_MAX_PDOS) > > > - return 0; > > > + num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */ > > > + if (num_pdos < UCSI_MAX_PDOS) > > > + return num_pdos; > > > > > > /* get the remaining PDOs, if any */ > > > - ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS, > > > - PDO_MAX_OBJECTS - UCSI_MAX_PDOS); > > > + ret = ucsi_read_pdos(con, role, is_partner, pdos, UCSI_MAX_PDOS, > > > + PDO_MAX_OBJECTS - UCSI_MAX_PDOS); > > > + if (ret < 0) > > > + return ret; > > > + > > > + return ret / sizeof(u32) + num_pdos; > > > +} > > > + > > > +static int ucsi_get_src_pdos(struct ucsi_connector *con) > > > +{ > > > + int ret; > > > + > > > + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 1, con->src_pdos); > > > > This issues the GET_PDOS command to PPM to get the source PDOs of the > > partner... > > > > > if (ret < 0) > > > return ret; > > > > > > - con->num_pdos += ret / sizeof(u32); > > > + con->num_pdos += ret; > > > > > > ucsi_port_psy_changed(con); > > > > > > @@ -638,6 +651,60 @@ static int ucsi_check_altmodes(struct ucsi_connector *con) > > > return ret; > > > } > > > > > > +static int ucsi_register_partner_pdos(struct ucsi_connector *con) > > > +{ > > > + struct pd_desc desc = { con->ucsi->cap.pd_version }; > > > + struct pd_capabilities *cap; > > > + struct pd_caps_desc caps; > > > + int ret; > > > + > > > + con->partner_pd = typec_partner_register_pd(con->partner, &desc); > > > + if (IS_ERR(con->partner_pd)) > > > + return PTR_ERR(con->partner_pd); > > > + > > > + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 1, caps.pdo); > > > > ... and also here. > > > > > + if (ret < 0) > > > + return ret; > > > + > > > + if (ret < PDO_MAX_OBJECTS) > > > + caps.pdo[ret] = 0; > > > + caps.role = TYPEC_SOURCE; > > > + > > > + cap = pd_register_capabilities(con->partner_pd, &caps); > > > + if (IS_ERR(cap)) > > > + return PTR_ERR(cap); > > > + > > > + ret = typec_partner_set_pd_capabilities(con->partner, cap); > > > + if (ret) { > > > + pd_unregister_capabilities(cap); > > > + return ret; > > > + } > > > + > > > + con->partner_source_caps = cap; > > > + > > > + ret = ucsi_get_pdos(con, TYPEC_SINK, 1, caps.pdo); > > > + if (ret <= 0) > > > + return ret; > > > + > > > + if (ret < PDO_MAX_OBJECTS) > > > + caps.pdo[ret] = 0; > > > + caps.role = TYPEC_SINK; > > > + > > > + cap = pd_register_capabilities(con->partner_pd, &caps); > > > + if (IS_ERR(cap)) > > > + return PTR_ERR(cap); > > > + > > > + ret = typec_partner_set_pd_capabilities(con->partner, cap); > > > + if (ret) { > > > + pd_unregister_capabilities(cap); > > > + return ret; > > > + } > > > + > > > + con->partner_sink_caps = cap; > > > + > > > + return 0; > > > +} > > > + > > > static void ucsi_pwr_opmode_change(struct ucsi_connector *con) > > > { > > > switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { > > > @@ -646,6 +713,7 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con) > > > typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD); > > > ucsi_partner_task(con, ucsi_get_src_pdos, 30, 0); > > > ucsi_partner_task(con, ucsi_check_altmodes, 30, 0); > > > + ucsi_partner_task(con, ucsi_register_partner_pdos, 1, HZ); > > > > And, both ucsi_get_src_pdos() and ucsi_register_partner_pdos() are > > scheduled to run here... > > > > > break; > > > case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5: > > > con->rdo = 0; > > > @@ -704,6 +772,17 @@ static void ucsi_unregister_partner(struct ucsi_connector *con) > > > if (!con->partner) > > > return; > > > > > > + typec_partner_unset_pd_capabilities(con->partner, TYPEC_SINK); > > > + pd_unregister_capabilities(con->partner_sink_caps); > > > + con->partner_sink_caps = NULL; > > > + > > > + typec_partner_unset_pd_capabilities(con->partner, TYPEC_SOURCE); > > > + pd_unregister_capabilities(con->partner_source_caps); > > > + con->partner_source_caps = NULL; > > > + > > > + typec_partner_unregister_pd(con->partner); > > > + con->partner_pd = NULL; > > > + > > > ucsi_unregister_altmodes(con, UCSI_RECIPIENT_SOP); > > > typec_unregister_partner(con->partner); > > > con->partner = NULL; > > > @@ -1037,6 +1116,8 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > > > u64 command; > > > char *name; > > > int ret; > > > + struct pd_desc desc = { ucsi->cap.pd_version }; > > > + struct pd_caps_desc caps; > > > > > > name = kasprintf(GFP_KERNEL, "%s-con%d", dev_name(ucsi->dev), con->num); > > > if (!name) > > > @@ -1103,6 +1184,24 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > > > goto out; > > > } > > > > > > + con->pd = typec_port_register_pd(con->port, &desc); > > > + > > > + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 0, caps.pdo); > > > + if (ret > 0) { > > > + caps.pdo[ret] = 0; > > > + caps.role = TYPEC_SOURCE; > > > + con->source_caps = pd_register_capabilities(con->pd, &caps); > > > + typec_port_set_pd_capabilities(con->port, con->source_caps); > > > + } > > > + > > > + ret = ucsi_get_pdos(con, TYPEC_SINK, 0, caps.pdo); > > > + if (ret > 0) { > > > + caps.pdo[ret] = 0; > > > + caps.role = TYPEC_SINK; > > > + con->sink_caps = pd_register_capabilities(con->pd, &caps); > > > + typec_port_set_pd_capabilities(con->port, con->sink_caps); > > > + } > > > + > > > /* Alternate modes */ > > > ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_CON); > > > if (ret) { > > > @@ -1169,6 +1268,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > > > UCSI_CONSTAT_PWR_OPMODE_PD) { > > > ucsi_get_src_pdos(con); > > > ucsi_check_altmodes(con); > > > + ucsi_register_partner_pdos(con); > > > > ... as well as here. > > > > So wouldn't this result in the PPM issuing the same PD Get_Source_Cap > > message twice to the port partner (in either case of initial port > > registration or op mode change)? Could we just consolidate them to just > > issue GET_PDOS only once and take care of populating the partner's > > Source Caps for both the pd_capabilties as well as power_supply purposes > > from a single helper? > > Yes. I tried create as little impact on existing code with this as > possible, so I just duplicated that part, but I probable should not > have done that. > > > Another aside, thinking back to a previous patch [1] I had proposed a > > few months ago, another question I had is whether it is proper to even > > issue a Get_Source_Cap message to sink-only devices, as we did encounter > > certain DisplayPort adapters that don't like when that happens. > > Wondering if it could be possible to limit calling the GET_PDOS command > > unless we know the partner is capable of operating in that particular > > power role. e.g. don't call get_src_pdos() if partner is sink-only. > > Or is this the kind of thing that the PPM is supposed to be able to > > figure out and allow OPM to naively issue the command regardless, and > > just get an error/empty return? > > Well, in many cases we simply can not rely on the PPM, but I think we > should be able rely on it with the role. > > What you are proposing probable would make sense. Do you want to work > on those improvements? I don't have to modify this driver now with > this series. I also can first register the sysfs attributes from > tcpm.c. Sure Heikki. I'll revise the earlier patch I had sent previously and update it to determine whether the partner is source-capable before before allowing ucsi_get_src_pdos() to be called. Thanks, Jack
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index f0c2fa19f3e0f..5149001093c7f 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -568,8 +568,8 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient) } } -static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, - u32 *pdos, int offset, int num_pdos) +static int ucsi_read_pdos(struct ucsi_connector *con, enum typec_role role, int is_partner, + u32 *pdos, int offset, int num_pdos) { struct ucsi *ucsi = con->ucsi; u64 command; @@ -579,7 +579,7 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, command |= UCSI_GET_PDOS_PARTNER_PDO(is_partner); command |= UCSI_GET_PDOS_PDO_OFFSET(offset); command |= UCSI_GET_PDOS_NUM_PDOS(num_pdos - 1); - command |= UCSI_GET_PDOS_SRC_PDOS; + command |= is_source(role) ? UCSI_GET_PDOS_SRC_PDOS : 0; ret = ucsi_send_command(ucsi, command, pdos + offset, num_pdos * sizeof(u32)); if (ret < 0 && ret != -ETIMEDOUT) @@ -590,26 +590,39 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, return ret; } -static int ucsi_get_src_pdos(struct ucsi_connector *con) +static int ucsi_get_pdos(struct ucsi_connector *con, enum typec_role role, + int is_partner, u32 *pdos) { + u8 num_pdos; int ret; /* UCSI max payload means only getting at most 4 PDOs at a time */ - ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS); + ret = ucsi_read_pdos(con, role, is_partner, pdos, 0, UCSI_MAX_PDOS); if (ret < 0) return ret; - con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */ - if (con->num_pdos < UCSI_MAX_PDOS) - return 0; + num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */ + if (num_pdos < UCSI_MAX_PDOS) + return num_pdos; /* get the remaining PDOs, if any */ - ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS, - PDO_MAX_OBJECTS - UCSI_MAX_PDOS); + ret = ucsi_read_pdos(con, role, is_partner, pdos, UCSI_MAX_PDOS, + PDO_MAX_OBJECTS - UCSI_MAX_PDOS); + if (ret < 0) + return ret; + + return ret / sizeof(u32) + num_pdos; +} + +static int ucsi_get_src_pdos(struct ucsi_connector *con) +{ + int ret; + + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 1, con->src_pdos); if (ret < 0) return ret; - con->num_pdos += ret / sizeof(u32); + con->num_pdos += ret; ucsi_port_psy_changed(con); @@ -638,6 +651,60 @@ static int ucsi_check_altmodes(struct ucsi_connector *con) return ret; } +static int ucsi_register_partner_pdos(struct ucsi_connector *con) +{ + struct pd_desc desc = { con->ucsi->cap.pd_version }; + struct pd_capabilities *cap; + struct pd_caps_desc caps; + int ret; + + con->partner_pd = typec_partner_register_pd(con->partner, &desc); + if (IS_ERR(con->partner_pd)) + return PTR_ERR(con->partner_pd); + + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 1, caps.pdo); + if (ret < 0) + return ret; + + if (ret < PDO_MAX_OBJECTS) + caps.pdo[ret] = 0; + caps.role = TYPEC_SOURCE; + + cap = pd_register_capabilities(con->partner_pd, &caps); + if (IS_ERR(cap)) + return PTR_ERR(cap); + + ret = typec_partner_set_pd_capabilities(con->partner, cap); + if (ret) { + pd_unregister_capabilities(cap); + return ret; + } + + con->partner_source_caps = cap; + + ret = ucsi_get_pdos(con, TYPEC_SINK, 1, caps.pdo); + if (ret <= 0) + return ret; + + if (ret < PDO_MAX_OBJECTS) + caps.pdo[ret] = 0; + caps.role = TYPEC_SINK; + + cap = pd_register_capabilities(con->partner_pd, &caps); + if (IS_ERR(cap)) + return PTR_ERR(cap); + + ret = typec_partner_set_pd_capabilities(con->partner, cap); + if (ret) { + pd_unregister_capabilities(cap); + return ret; + } + + con->partner_sink_caps = cap; + + return 0; +} + static void ucsi_pwr_opmode_change(struct ucsi_connector *con) { switch (UCSI_CONSTAT_PWR_OPMODE(con->status.flags)) { @@ -646,6 +713,7 @@ static void ucsi_pwr_opmode_change(struct ucsi_connector *con) typec_set_pwr_opmode(con->port, TYPEC_PWR_MODE_PD); ucsi_partner_task(con, ucsi_get_src_pdos, 30, 0); ucsi_partner_task(con, ucsi_check_altmodes, 30, 0); + ucsi_partner_task(con, ucsi_register_partner_pdos, 1, HZ); break; case UCSI_CONSTAT_PWR_OPMODE_TYPEC1_5: con->rdo = 0; @@ -704,6 +772,17 @@ static void ucsi_unregister_partner(struct ucsi_connector *con) if (!con->partner) return; + typec_partner_unset_pd_capabilities(con->partner, TYPEC_SINK); + pd_unregister_capabilities(con->partner_sink_caps); + con->partner_sink_caps = NULL; + + typec_partner_unset_pd_capabilities(con->partner, TYPEC_SOURCE); + pd_unregister_capabilities(con->partner_source_caps); + con->partner_source_caps = NULL; + + typec_partner_unregister_pd(con->partner); + con->partner_pd = NULL; + ucsi_unregister_altmodes(con, UCSI_RECIPIENT_SOP); typec_unregister_partner(con->partner); con->partner = NULL; @@ -1037,6 +1116,8 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) u64 command; char *name; int ret; + struct pd_desc desc = { ucsi->cap.pd_version }; + struct pd_caps_desc caps; name = kasprintf(GFP_KERNEL, "%s-con%d", dev_name(ucsi->dev), con->num); if (!name) @@ -1103,6 +1184,24 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) goto out; } + con->pd = typec_port_register_pd(con->port, &desc); + + ret = ucsi_get_pdos(con, TYPEC_SOURCE, 0, caps.pdo); + if (ret > 0) { + caps.pdo[ret] = 0; + caps.role = TYPEC_SOURCE; + con->source_caps = pd_register_capabilities(con->pd, &caps); + typec_port_set_pd_capabilities(con->port, con->source_caps); + } + + ret = ucsi_get_pdos(con, TYPEC_SINK, 0, caps.pdo); + if (ret > 0) { + caps.pdo[ret] = 0; + caps.role = TYPEC_SINK; + con->sink_caps = pd_register_capabilities(con->pd, &caps); + typec_port_set_pd_capabilities(con->port, con->sink_caps); + } + /* Alternate modes */ ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_CON); if (ret) { @@ -1169,6 +1268,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) UCSI_CONSTAT_PWR_OPMODE_PD) { ucsi_get_src_pdos(con); ucsi_check_altmodes(con); + ucsi_register_partner_pdos(con); } trace_ucsi_register_port(con->num, &con->status); @@ -1379,6 +1479,12 @@ void ucsi_unregister(struct ucsi *ucsi) ucsi_unregister_port_psy(&ucsi->connector[i]); if (ucsi->connector[i].wq) destroy_workqueue(ucsi->connector[i].wq); + typec_port_unset_pd_capabilities(ucsi->connector[i].port, + ucsi->connector[i].source_caps); + pd_unregister_capabilities(ucsi->connector[i].source_caps); + typec_port_unset_pd_capabilities(ucsi->connector[i].port, + ucsi->connector[i].sink_caps); + pd_unregister_capabilities(ucsi->connector[i].sink_caps); typec_unregister_port(ucsi->connector[i].port); } diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h index 280f1e1bda2c9..aa23df98b7730 100644 --- a/drivers/usb/typec/ucsi/ucsi.h +++ b/drivers/usb/typec/ucsi/ucsi.h @@ -335,6 +335,14 @@ struct ucsi_connector { int num_pdos; struct usb_role_switch *usb_role_sw; + + struct pd *pd; + struct pd_capabilities *source_caps; + struct pd_capabilities *sink_caps; + + struct pd *partner_pd; + struct pd_capabilities *partner_source_caps; + struct pd_capabilities *partner_sink_caps; }; int ucsi_send_command(struct ucsi *ucsi, u64 command,
UCSI allows the USB PD capabilities to be read with the GET_PDO command. This will register those capabilities, and that way make them visible to the user space. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- drivers/usb/typec/ucsi/ucsi.c | 128 +++++++++++++++++++++++++++++++--- drivers/usb/typec/ucsi/ucsi.h | 8 +++ 2 files changed, 125 insertions(+), 11 deletions(-)