2020-11-12 01:44:21

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

This patch series add plug registration support to the cros-ec-typec
driver. It also adds support for registering alternate modes for the
registered plug. These features utilize the API provided by the Type C
connector class framework.

The first patch adds support to the connector class framework for the
number_of_alternate_modes attribute (along with the relevant ABI
documentation).

The next two patches add plug registration, and then altmode
registration for the plugs. The latter of these two patches utilizes the
new function for plug number_of_alternate_modes introduced in the first patch.

This series is based on top of the following branch and other patch
series (applied in the order specified):
- Branch: chrome-platform for-next [1], which is currently set to the
"Linux 5.10-rc1" tag.
- cros-ec-typec: Patch series to register PD identity information + partner altmodes[2]
- cros-ec-typec: Patch series to register cable[3]
- cros-ec-typec: Patch series to add partner number_of_altmodes[4]

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
[2]: https://lore.kernel.org/lkml/[email protected]/
[3]: https://lore.kernel.org/lkml/[email protected]/
[4]: https://lore.kernel.org/lkml/[email protected]/

Prashant Malani (3):
usb: typec: Add plug num_altmodes sysfs attr
platform/chrome: cros_ec_typec: Register SOP' cable plug
platform/chrome: cros_ec_typec: Register plug altmodes

Documentation/ABI/testing/sysfs-class-typec | 9 +++
drivers/platform/chrome/cros_ec_typec.c | 85 ++++++++++++++++-----
drivers/usb/typec/class.c | 77 ++++++++++++++++++-
include/linux/usb/typec.h | 1 +
4 files changed, 151 insertions(+), 21 deletions(-)

--
2.29.2.222.g5d2a92d10f8-goog


2020-11-12 01:45:09

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 1/3] usb: typec: Add plug num_altmodes sysfs attr

Add a field to the typec_plug struct to record the number of available
altmodes as well as the corresponding sysfs attribute to expose this to
userspace.

This allows userspace to determine whether there are any
remaining alternate modes left to be registered by the kernel driver. It
can begin executing any policy state machine after all available
alternate modes have been registered with the connector class framework.

This value is set to "-1" initially, signifying that a valid number of
alternate modes haven't been set for the plug. The sysfs file remains
hidden as long as the attribute value is -1.

We re-use the partner attribute for number_of_alternate_modes since the
usage and name is similar, and update the corresponding *_show() command
to support both partner and plugs.

Signed-off-by: Prashant Malani <[email protected]>
---
Documentation/ABI/testing/sysfs-class-typec | 9 +++
drivers/usb/typec/class.c | 77 ++++++++++++++++++++-
include/linux/usb/typec.h | 1 +
3 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 73ac7b461ae5..29eccf5fb8ed 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -204,6 +204,15 @@ Description:
- type-c
- captive

+What: /sys/class/typec/<port>-<plug>/number_of_alternate_modes
+Date: November 2020
+Contact: Prashant Malani <[email protected]>
+Description:
+ Shows the number of alternate modes which are advertised by the plug
+ associated with a particular cable during Power Delivery discovery.
+ This file remains hidden until a value greater than or equal to 0
+ is set by Type C port driver.
+
What: /sys/class/typec/<port>-cable/identity/
Date: April 2017
Contact: Heikki Krogerus <[email protected]>
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c7412ddbd311..e68798599ca8 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -18,6 +18,7 @@ struct typec_plug {
struct device dev;
enum typec_plug_index index;
struct ida mode_ids;
+ int num_altmodes;
};

struct typec_cable {
@@ -536,9 +537,21 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
static ssize_t number_of_alternate_modes_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct typec_partner *p = to_typec_partner(dev);
+ struct typec_partner *partner;
+ struct typec_plug *plug;
+ int num_altmodes;
+
+ if (is_typec_partner(dev)) {
+ partner = to_typec_partner(dev);
+ num_altmodes = partner->num_altmodes;
+ } else if (is_typec_plug(dev)) {
+ plug = to_typec_plug(dev);
+ num_altmodes = plug->num_altmodes;
+ } else {
+ return 0;
+ }

- return sysfs_emit(buf, "%d\n", p->num_altmodes);
+ return sysfs_emit(buf, "%d\n", num_altmodes);
}
static DEVICE_ATTR_RO(number_of_alternate_modes);

@@ -726,11 +739,70 @@ static void typec_plug_release(struct device *dev)
kfree(plug);
}

+static struct attribute *typec_plug_attrs[] = {
+ &dev_attr_number_of_alternate_modes.attr,
+ NULL
+};
+
+static umode_t typec_plug_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+ struct typec_plug *plug = to_typec_plug(kobj_to_dev(kobj));
+
+ if (attr == &dev_attr_number_of_alternate_modes.attr) {
+ if (plug->num_altmodes < 0)
+ return 0;
+ }
+
+ return attr->mode;
+}
+
+static struct attribute_group typec_plug_group = {
+ .is_visible = typec_plug_attr_is_visible,
+ .attrs = typec_plug_attrs
+};
+
+static const struct attribute_group *typec_plug_groups[] = {
+ &typec_plug_group,
+ NULL
+};
+
static const struct device_type typec_plug_dev_type = {
.name = "typec_plug",
+ .groups = typec_plug_groups,
.release = typec_plug_release,
};

+/**
+ * typec_plug_set_num_altmodes - Set the number of available plug altmodes
+ * @plug: The plug to be updated.
+ * @num_altmodes: The number of altmodes we want to specify as available.
+ *
+ * This routine is used to report the number of alternate modes supported by the
+ * plug. This value is *not* enforced in alternate mode registration routines.
+ *
+ * @plug.num_altmodes is set to -1 on plug registration, denoting that
+ * a valid value has not been set for it yet.
+ *
+ * Returns 0 on success or negative error number on failure.
+ */
+int typec_plug_set_num_altmodes(struct typec_plug *plug, int num_altmodes)
+{
+ int ret;
+
+ if (num_altmodes < 0)
+ return -EINVAL;
+
+ plug->num_altmodes = num_altmodes;
+ ret = sysfs_update_group(&plug->dev.kobj, &typec_plug_group);
+ if (ret < 0)
+ return ret;
+
+ sysfs_notify(&plug->dev.kobj, NULL, "number_of_alternate_modes");
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(typec_plug_set_num_altmodes);
+
/**
* typec_plug_register_altmode - Register USB Type-C Cable Plug Alternate Mode
* @plug: USB Type-C Cable Plug that supports the alternate mode
@@ -776,6 +848,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
sprintf(name, "plug%d", desc->index);

ida_init(&plug->mode_ids);
+ plug->num_altmodes = -1;
plug->index = desc->index;
plug->dev.class = typec_class;
plug->dev.parent = &cable->dev;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index bc6b1a71cb8a..54475323f83b 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -130,6 +130,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod
struct typec_altmode
*typec_partner_register_altmode(struct typec_partner *partner,
const struct typec_altmode_desc *desc);
+int typec_plug_set_num_altmodes(struct typec_plug *plug, int num_altmodes);
struct typec_altmode
*typec_plug_register_altmode(struct typec_plug *plug,
const struct typec_altmode_desc *desc);
--
2.29.2.222.g5d2a92d10f8-goog

2020-11-12 01:46:00

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 3/3] platform/chrome: cros_ec_typec: Register plug altmodes

Modify the altmode registration (and unregistration) code so that it
can be used by both partners and plugs.

Then, add code to register plug altmodes using the newly parameterized
function. Also set the number of alternate modes for the plug using the
associated Type C connector class function
typec_plug_set_num_altmodes().

Signed-off-by: Prashant Malani <[email protected]>
---
drivers/platform/chrome/cros_ec_typec.c | 50 ++++++++++++++++++++-----
1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index d2e154ae2362..65c5d0090ccd 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -67,6 +67,7 @@ struct cros_typec_port {
bool sop_prime_disc_done;
struct ec_response_typec_discovery *disc_data;
struct list_head partner_mode_list;
+ struct list_head plug_mode_list;
};

/* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -186,12 +187,15 @@ static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
return ret;
}

-static void cros_typec_unregister_altmodes(struct cros_typec_data *typec, int port_num)
+static void cros_typec_unregister_altmodes(struct cros_typec_data *typec, int port_num,
+ bool is_partner)
{
struct cros_typec_port *port = typec->ports[port_num];
struct cros_typec_altmode_node *node, *tmp;
+ struct list_head *head;

- list_for_each_entry_safe(node, tmp, &port->partner_mode_list, list) {
+ head = is_partner ? &port->partner_mode_list : &port->plug_mode_list;
+ list_for_each_entry_safe(node, tmp, head, list) {
list_del(&node->list);
typec_unregister_altmode(node->amode);
devm_kfree(typec->dev, node);
@@ -203,7 +207,7 @@ static void cros_typec_remove_partner(struct cros_typec_data *typec,
{
struct cros_typec_port *port = typec->ports[port_num];

- cros_typec_unregister_altmodes(typec, port_num);
+ cros_typec_unregister_altmodes(typec, port_num, true);

port->state.alt = NULL;
port->state.mode = TYPEC_STATE_USB;
@@ -224,6 +228,8 @@ static void cros_typec_remove_cable(struct cros_typec_data *typec,
{
struct cros_typec_port *port = typec->ports[port_num];

+ cros_typec_unregister_altmodes(typec, port_num, false);
+
typec_unregister_plug(port->plug);
port->plug = NULL;
typec_unregister_cable(port->cable);
@@ -352,6 +358,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
}

INIT_LIST_HEAD(&cros_port->partner_mode_list);
+ INIT_LIST_HEAD(&cros_port->plug_mode_list);
}

return 0;
@@ -639,7 +646,11 @@ static int cros_typec_get_mux_info(struct cros_typec_data *typec, int port_num,
sizeof(req), resp, sizeof(*resp));
}

-static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_num)
+/*
+ * Helper function to register partner/plug altmodes.
+ */
+static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_num,
+ bool is_partner)
{
struct cros_typec_port *port = typec->ports[port_num];
struct ec_response_typec_discovery *sop_disc = port->disc_data;
@@ -657,7 +668,11 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
desc.mode = j;
desc.vdo = sop_disc->svids[i].mode_vdo[j];

- amode = typec_partner_register_altmode(port->partner, &desc);
+ if (is_partner)
+ amode = typec_partner_register_altmode(port->partner, &desc);
+ else
+ amode = typec_plug_register_altmode(port->plug, &desc);
+
if (IS_ERR(amode)) {
ret = PTR_ERR(amode);
goto err_cleanup;
@@ -672,21 +687,30 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
}

node->amode = amode;
- list_add_tail(&node->list, &port->partner_mode_list);
+
+ if (is_partner)
+ list_add_tail(&node->list, &port->partner_mode_list);
+ else
+ list_add_tail(&node->list, &port->plug_mode_list);
num_altmodes++;
}
}

- ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+ if (is_partner)
+ ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+ else
+ ret = typec_plug_set_num_altmodes(port->plug, num_altmodes);
+
if (ret < 0) {
- dev_err(typec->dev, "Unable to set partner num_altmodes for port: %d\n", port_num);
+ dev_err(typec->dev, "Unable to set %s num_altmodes for port: %d\n",
+ is_partner ? "partner" : "plug", port_num);
goto err_cleanup;
}

return 0;

err_cleanup:
- cros_typec_unregister_altmodes(typec, port_num);
+ cros_typec_unregister_altmodes(typec, port_num, is_partner);
return ret;
}

@@ -774,6 +798,12 @@ static int cros_typec_handle_sop_prime_disc(struct cros_typec_data *typec, int p
goto sop_prime_disc_exit;
}

+ ret = cros_typec_register_altmodes(typec, port_num, false);
+ if (ret < 0) {
+ dev_err(typec->dev, "Failed to register plug altmodes, port: %d\n", port_num);
+ goto sop_prime_disc_exit;
+ }
+
return 0;

sop_prime_disc_exit:
@@ -815,7 +845,7 @@ static int cros_typec_handle_sop_disc(struct cros_typec_data *typec, int port_nu
goto disc_exit;
}

- ret = cros_typec_register_altmodes(typec, port_num);
+ ret = cros_typec_register_altmodes(typec, port_num, true);
if (ret < 0) {
dev_err(typec->dev, "Failed to register partner altmodes, port: %d\n", port_num);
goto disc_exit;
--
2.29.2.222.g5d2a92d10f8-goog

2020-11-13 14:17:06

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

On Wed, Nov 11, 2020 at 05:23:25PM -0800, Prashant Malani wrote:
> This patch series add plug registration support to the cros-ec-typec
> driver. It also adds support for registering alternate modes for the
> registered plug. These features utilize the API provided by the Type C
> connector class framework.
>
> The first patch adds support to the connector class framework for the
> number_of_alternate_modes attribute (along with the relevant ABI
> documentation).
>
> The next two patches add plug registration, and then altmode
> registration for the plugs. The latter of these two patches utilizes the
> new function for plug number_of_alternate_modes introduced in the first patch.
>
> This series is based on top of the following branch and other patch
> series (applied in the order specified):
> - Branch: chrome-platform for-next [1], which is currently set to the
> "Linux 5.10-rc1" tag.
> - cros-ec-typec: Patch series to register PD identity information + partner altmodes[2]
> - cros-ec-typec: Patch series to register cable[3]
> - cros-ec-typec: Patch series to add partner number_of_altmodes[4]
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
> [2]: https://lore.kernel.org/lkml/[email protected]/
> [3]: https://lore.kernel.org/lkml/[email protected]/
> [4]: https://lore.kernel.org/lkml/[email protected]/

Ok, I'm confused. This is not the first submission of this series, as
you sent out a v2 a few days before this one.

And am I supposed to suck in the chrome-platform branch into the
usb-next tree?

What should I do here, ignore these? Merge them?

I see the USB change lost the reviewer's ack as well, why?

I'm going to delete all of these patches from my review queue now and
wait for a resend with some clarity as to what I should do with it :)

thanks,

greg k-h

2020-11-13 21:40:20

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 0/3] platform/chrome: cros_ec_typec: Add plug and plug altmodes

Hi Greg,

On Fri, Nov 13, 2020 at 6:13 AM Greg KH <[email protected]> wrote:
>
> On Wed, Nov 11, 2020 at 05:23:25PM -0800, Prashant Malani wrote:
> > This patch series add plug registration support to the cros-ec-typec
> > driver. It also adds support for registering alternate modes for the
> > registered plug. These features utilize the API provided by the Type C
> > connector class framework.
> >
> > The first patch adds support to the connector class framework for the
> > number_of_alternate_modes attribute (along with the relevant ABI
> > documentation).
> >
> > The next two patches add plug registration, and then altmode
> > registration for the plugs. The latter of these two patches utilizes the
> > new function for plug number_of_alternate_modes introduced in the first patch.
> >
> > This series is based on top of the following branch and other patch
> > series (applied in the order specified):
> > - Branch: chrome-platform for-next [1], which is currently set to the
> > "Linux 5.10-rc1" tag.
> > - cros-ec-typec: Patch series to register PD identity information + partner altmodes[2]
> > - cros-ec-typec: Patch series to register cable[3]
> > - cros-ec-typec: Patch series to add partner number_of_altmodes[4]
> >
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-next
> > [2]: https://lore.kernel.org/lkml/[email protected]/
> > [3]: https://lore.kernel.org/lkml/[email protected]/
> > [4]: https://lore.kernel.org/lkml/[email protected]/
>
> Ok, I'm confused. This is not the first submission of this series, as
> you sent out a v2 a few days before this one.

Sorry for confusing you. To clarify, this is the first version of this
series particular set
of patches. A few more related series of patches have been sent
regarding this earlier
(see [2], [3] & [4]) and I've not done a resend of those.

This series depends on those earlier series.

>
> And am I supposed to suck in the chrome-platform branch into the
> usb-next tree?
>
> What should I do here, ignore these? Merge them?

TBH I'm a little confused about how these get in myself. Across all
these patches, there are probably
3 patches (1 USB PD VDO header, two drivers/usb/typec/class.c patches)
which belong to usb-next
whereas everything else is chrome-platform.

>
> I see the USB change lost the reviewer's ack as well, why?

I'm sorry but I didn't follow. As I mentioned above this is the first
version of this
series (which deals with cable plug and it's altmodes) and AFAIK it
hasn't received any acks till now.

All the previous series have not had any re-uploads, so they should
still have all their review tags.

>
> I'm going to delete all of these patches from my review queue now and
> wait for a resend with some clarity as to what I should do with it :)

Enric, could you kindly help suggest a way I can upload things?
Should I just merge everything into one big set of patches (RIght from
[2] to this series) ?
Or just let all the series go through chrome-platform?

BR,

-Prashant