[PATCH v2 1/2] usb: typec: Add number of altmodes partner attr

[PATCH v2 1/2] usb: typec: Add number of altmodes partner attr

[Prashant Malani]

Add a user-visible attribute for the number of alternate modes available
in a partner. 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 partner.

Also add a sysfs file which exposes this attribute. The file remains
hidden as long as the attribute value is -1.

Cc: Benson Leung <bleung@chromium.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- Added ABI/testing documentation entry for added sysfs file.
- Changed name of the sysfs file to "number_of_alternate_modes" based on
  review comments.
- Added is_visible() logic suggested by Heikki in the comments of v1.
- Updated commit message.

v1:
https://lore.kernel.org/lkml/20200701003149.3101219-1-pmalani@chromium.org/

 Documentation/ABI/testing/sysfs-class-typec |  8 +++
 drivers/usb/typec/class.c                   | 66 ++++++++++++++++++++-
 include/linux/usb/typec.h                   |  1 +
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index b834671522d6..73ac7b461ae5 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -134,6 +134,14 @@ Description:
 		Shows if the partner supports USB Power Delivery communication:
 		Valid values: yes, no
 
+What:		/sys/class/typec/<port>-partner/number_of_alternate_modes
+Date:		November 2020
+Contact:	Prashant Malani <pmalani@chromium.org>
+Description:
+		Shows the number of alternate modes which are advertised by the partner
+		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>-partner>/identity/
 Date:		April 2017
 Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 35eec707cb51..c7412ddbd311 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -33,6 +33,7 @@ struct typec_partner {
 	struct usb_pd_identity		*identity;
 	enum typec_accessory		accessory;
 	struct ida			mode_ids;
+	int				num_altmodes;
 };
 
 struct typec_port {
@@ -532,12 +533,43 @@ static ssize_t supports_usb_power_delivery_show(struct device *dev,
 }
 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);
+
+	return sysfs_emit(buf, "%d\n", p->num_altmodes);
+}
+static DEVICE_ATTR_RO(number_of_alternate_modes);
+
 static struct attribute *typec_partner_attrs[] = {
 	&dev_attr_accessory_mode.attr,
 	&dev_attr_supports_usb_power_delivery.attr,
+	&dev_attr_number_of_alternate_modes.attr,
+	NULL
+};
+
+static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	struct typec_partner *partner = to_typec_partner(kobj_to_dev(kobj));
+
+	if (attr == &dev_attr_number_of_alternate_modes.attr) {
+		if (partner->num_altmodes < 0)
+			return 0;
+	}
+
+	return attr->mode;
+}
+
+static struct attribute_group typec_partner_group = {
+	.is_visible = typec_partner_attr_is_visible,
+	.attrs = typec_partner_attrs
+};
+
+static const struct attribute_group *typec_partner_groups[] = {
+	&typec_partner_group,
 	NULL
 };
-ATTRIBUTE_GROUPS(typec_partner);
 
 static void typec_partner_release(struct device *dev)
 {
@@ -570,6 +602,37 @@ int typec_partner_set_identity(struct typec_partner *partner)
 }
 EXPORT_SYMBOL_GPL(typec_partner_set_identity);
 
+/**
+ * typec_partner_set_num_altmodes - Set the number of available partner altmodes
+ * @partner: The partner to be updated.
+ * @num_alt_modes: The number of altmodes we want to specify as available.
+ *
+ * This routine is used to report the number of alternate modes supported by the
+ * partner. This value is *not* enforced in alternate mode registration routines.
+ *
+ * @partner.num_altmodes is set to -1 on partner 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_partner_set_num_altmodes(struct typec_partner *partner, int num_altmodes)
+{
+	int ret;
+
+	if (num_altmodes < 0)
+		return -EINVAL;
+
+	partner->num_altmodes = num_altmodes;
+	ret = sysfs_update_group(&partner->dev.kobj, &typec_partner_group);
+	if (ret < 0)
+		return ret;
+
+	sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes");
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(typec_partner_set_num_altmodes);
+
 /**
  * typec_partner_register_altmode - Register USB Type-C Partner Alternate Mode
  * @partner: USB Type-C Partner that supports the alternate mode
@@ -612,6 +675,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 	ida_init(&partner->mode_ids);
 	partner->usb_pd = desc->usb_pd;
 	partner->accessory = desc->accessory;
+	partner->num_altmodes = -1;
 
 	if (desc->identity) {
 		/*
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 6be558045942..bc6b1a71cb8a 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -126,6 +126,7 @@ struct typec_altmode_desc {
 	enum typec_port_data	roles;
 };
 
+int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmodes);
 struct typec_altmode
 *typec_partner_register_altmode(struct typec_partner *partner,
 				const struct typec_altmode_desc *desc);
-- 
2.29.2.222.g5d2a92d10f8-goog

[PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Prashant Malani]

Set the number of altmodes available for a registered partner using the
Type C connector class framework routine.

Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v2:
- Patch introduced for the first time in v2.

 drivers/platform/chrome/cros_ec_typec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index ce031a10eb1b..743a28426f98 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -621,6 +621,7 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
 	struct cros_typec_altmode_node *node;
 	struct typec_altmode_desc desc;
 	struct typec_altmode *amode;
+	int num_altmodes = 0;
 	int ret = 0;
 	int i, j;
 
@@ -647,9 +648,16 @@ 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);
+			num_altmodes++;
 		}
 	}
 
+	ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
+	if (ret < 0) {
+		dev_err(typec->dev, "Unable to set partner num_altmodes for port: %d\n", port_num);
+		goto err_cleanup;
+	}
+
 	return 0;
 
 err_cleanup:
-- 
2.29.2.222.g5d2a92d10f8-goog

Re: [PATCH v2 1/2] usb: typec: Add number of altmodes partner attr

[Heikki Krogerus]

On Mon, Nov 09, 2020 at 10:15:34PM -0800, Prashant Malani wrote:
> Add a user-visible attribute for the number of alternate modes available
> in a partner. 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 partner.
> 
> Also add a sysfs file which exposes this attribute. The file remains
> hidden as long as the attribute value is -1.
> 
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - Added ABI/testing documentation entry for added sysfs file.
> - Changed name of the sysfs file to "number_of_alternate_modes" based on
>   review comments.
> - Added is_visible() logic suggested by Heikki in the comments of v1.
> - Updated commit message.
> 
> v1:
> https://lore.kernel.org/lkml/20200701003149.3101219-1-pmalani@chromium.org/
> 
>  Documentation/ABI/testing/sysfs-class-typec |  8 +++
>  drivers/usb/typec/class.c                   | 66 ++++++++++++++++++++-
>  include/linux/usb/typec.h                   |  1 +
>  3 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index b834671522d6..73ac7b461ae5 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -134,6 +134,14 @@ Description:
>  		Shows if the partner supports USB Power Delivery communication:
>  		Valid values: yes, no
>  
> +What:		/sys/class/typec/<port>-partner/number_of_alternate_modes
> +Date:		November 2020
> +Contact:	Prashant Malani <pmalani@chromium.org>
> +Description:
> +		Shows the number of alternate modes which are advertised by the partner
> +		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>-partner>/identity/
>  Date:		April 2017
>  Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 35eec707cb51..c7412ddbd311 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -33,6 +33,7 @@ struct typec_partner {
>  	struct usb_pd_identity		*identity;
>  	enum typec_accessory		accessory;
>  	struct ida			mode_ids;
> +	int				num_altmodes;
>  };
>  
>  struct typec_port {
> @@ -532,12 +533,43 @@ static ssize_t supports_usb_power_delivery_show(struct device *dev,
>  }
>  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);
> +
> +	return sysfs_emit(buf, "%d\n", p->num_altmodes);
> +}
> +static DEVICE_ATTR_RO(number_of_alternate_modes);
> +
>  static struct attribute *typec_partner_attrs[] = {
>  	&dev_attr_accessory_mode.attr,
>  	&dev_attr_supports_usb_power_delivery.attr,
> +	&dev_attr_number_of_alternate_modes.attr,
> +	NULL
> +};
> +
> +static umode_t typec_partner_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
> +{
> +	struct typec_partner *partner = to_typec_partner(kobj_to_dev(kobj));
> +
> +	if (attr == &dev_attr_number_of_alternate_modes.attr) {
> +		if (partner->num_altmodes < 0)
> +			return 0;
> +	}
> +
> +	return attr->mode;
> +}
> +
> +static struct attribute_group typec_partner_group = {
> +	.is_visible = typec_partner_attr_is_visible,
> +	.attrs = typec_partner_attrs
> +};
> +
> +static const struct attribute_group *typec_partner_groups[] = {
> +	&typec_partner_group,
>  	NULL
>  };
> -ATTRIBUTE_GROUPS(typec_partner);
>  
>  static void typec_partner_release(struct device *dev)
>  {
> @@ -570,6 +602,37 @@ int typec_partner_set_identity(struct typec_partner *partner)
>  }
>  EXPORT_SYMBOL_GPL(typec_partner_set_identity);
>  
> +/**
> + * typec_partner_set_num_altmodes - Set the number of available partner altmodes
> + * @partner: The partner to be updated.
> + * @num_alt_modes: The number of altmodes we want to specify as available.
> + *
> + * This routine is used to report the number of alternate modes supported by the
> + * partner. This value is *not* enforced in alternate mode registration routines.
> + *
> + * @partner.num_altmodes is set to -1 on partner 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_partner_set_num_altmodes(struct typec_partner *partner, int num_altmodes)
> +{
> +	int ret;
> +
> +	if (num_altmodes < 0)
> +		return -EINVAL;
> +
> +	partner->num_altmodes = num_altmodes;
> +	ret = sysfs_update_group(&partner->dev.kobj, &typec_partner_group);
> +	if (ret < 0)
> +		return ret;
> +
> +	sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes");
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(typec_partner_set_num_altmodes);
> +
>  /**
>   * typec_partner_register_altmode - Register USB Type-C Partner Alternate Mode
>   * @partner: USB Type-C Partner that supports the alternate mode
> @@ -612,6 +675,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
>  	ida_init(&partner->mode_ids);
>  	partner->usb_pd = desc->usb_pd;
>  	partner->accessory = desc->accessory;
> +	partner->num_altmodes = -1;
>  
>  	if (desc->identity) {
>  		/*
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 6be558045942..bc6b1a71cb8a 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -126,6 +126,7 @@ struct typec_altmode_desc {
>  	enum typec_port_data	roles;
>  };
>  
> +int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmodes);
>  struct typec_altmode
>  *typec_partner_register_altmode(struct typec_partner *partner,
>  				const struct typec_altmode_desc *desc);
> -- 
> 2.29.2.222.g5d2a92d10f8-goog

thanks,

-- 
heikki

Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Heikki Krogerus]

On Mon, Nov 09, 2020 at 10:15:36PM -0800, Prashant Malani wrote:
> Set the number of altmodes available for a registered partner using the
> Type C connector class framework routine.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v2:
> - Patch introduced for the first time in v2.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index ce031a10eb1b..743a28426f98 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -621,6 +621,7 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
>  	struct cros_typec_altmode_node *node;
>  	struct typec_altmode_desc desc;
>  	struct typec_altmode *amode;
> +	int num_altmodes = 0;
>  	int ret = 0;
>  	int i, j;
>  
> @@ -647,9 +648,16 @@ 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);
> +			num_altmodes++;
>  		}
>  	}
>  
> +	ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
> +	if (ret < 0) {
> +		dev_err(typec->dev, "Unable to set partner num_altmodes for port: %d\n", port_num);
> +		goto err_cleanup;
> +	}
> +
>  	return 0;
>  
>  err_cleanup:
> -- 
> 2.29.2.222.g5d2a92d10f8-goog

thanks,

-- 
heikki

Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Enric Balletbo i Serra]

Hi,

On 10/11/20 11:50, Heikki Krogerus wrote:
> On Mon, Nov 09, 2020 at 10:15:36PM -0800, Prashant Malani wrote:
>> Set the number of altmodes available for a registered partner using the
>> Type C connector class framework routine.
>>
>> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> 
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Heikki, would you like to take these two through your tree? It'd help if you can
create an inmutable branch so I can pick other cros_ec_typec patches on top of it.

Thanks,
 Enric

>> ---
>>
>> Changes in v2:
>> - Patch introduced for the first time in v2.
>>
>>  drivers/platform/chrome/cros_ec_typec.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
>> index ce031a10eb1b..743a28426f98 100644
>> --- a/drivers/platform/chrome/cros_ec_typec.c
>> +++ b/drivers/platform/chrome/cros_ec_typec.c
>> @@ -621,6 +621,7 @@ static int cros_typec_register_altmodes(struct cros_typec_data *typec, int port_
>>  	struct cros_typec_altmode_node *node;
>>  	struct typec_altmode_desc desc;
>>  	struct typec_altmode *amode;
>> +	int num_altmodes = 0;
>>  	int ret = 0;
>>  	int i, j;
>>  
>> @@ -647,9 +648,16 @@ 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);
>> +			num_altmodes++;
>>  		}
>>  	}
>>  
>> +	ret = typec_partner_set_num_altmodes(port->partner, num_altmodes);
>> +	if (ret < 0) {
>> +		dev_err(typec->dev, "Unable to set partner num_altmodes for port: %d\n", port_num);
>> +		goto err_cleanup;
>> +	}
>> +
>>  	return 0;
>>  
>>  err_cleanup:
>> -- 
>> 2.29.2.222.g5d2a92d10f8-goog
> 
> thanks,
> 

Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Heikki Krogerus]

On Tue, Nov 10, 2020 at 05:01:04PM +0100, Enric Balletbo i Serra wrote:
> Hi,
> 
> On 10/11/20 11:50, Heikki Krogerus wrote:
> > On Mon, Nov 09, 2020 at 10:15:36PM -0800, Prashant Malani wrote:
> >> Set the number of altmodes available for a registered partner using the
> >> Type C connector class framework routine.
> >>
> >> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > 
> > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > 
> 
> Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> Heikki, would you like to take these two through your tree? It'd help if you can
> create an inmutable branch so I can pick other cros_ec_typec patches on top of it.

I'm sorry, but I don't actually maintain a tree for the USB Type-C
stuff.

Greg, I'm sorry to bother you with this, but can you provide the
immutable branch for Enric?

Maybe I should set up a tree for the USB Type-C stuff?

thanks,

-- 
heikki

Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Greg KH]

On Wed, Nov 11, 2020 at 12:28:46PM +0200, Heikki Krogerus wrote:
> On Tue, Nov 10, 2020 at 05:01:04PM +0100, Enric Balletbo i Serra wrote:
> > Hi,
> > 
> > On 10/11/20 11:50, Heikki Krogerus wrote:
> > > On Mon, Nov 09, 2020 at 10:15:36PM -0800, Prashant Malani wrote:
> > >> Set the number of altmodes available for a registered partner using the
> > >> Type C connector class framework routine.
> > >>
> > >> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > > 
> > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > 
> > 
> > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > 
> > Heikki, would you like to take these two through your tree? It'd help if you can
> > create an inmutable branch so I can pick other cros_ec_typec patches on top of it.
> 
> I'm sorry, but I don't actually maintain a tree for the USB Type-C
> stuff.
> 
> Greg, I'm sorry to bother you with this, but can you provide the
> immutable branch for Enric?

My usb tree branches are always immutable :)

> Maybe I should set up a tree for the USB Type-C stuff?

Or I can just take all of these patches as they depend on USB stuff more
than any other specific platform driver changes, right?

thanks,

greg k-h

Re: [PATCH v2 2/2] platform/chrome: cros_ec_typec: Set partner num_altmodes

[Heikki Krogerus]

On Wed, Nov 11, 2020 at 11:56:37AM +0100, Greg KH wrote:
> On Wed, Nov 11, 2020 at 12:28:46PM +0200, Heikki Krogerus wrote:
> > On Tue, Nov 10, 2020 at 05:01:04PM +0100, Enric Balletbo i Serra wrote:
> > > Hi,
> > > 
> > > On 10/11/20 11:50, Heikki Krogerus wrote:
> > > > On Mon, Nov 09, 2020 at 10:15:36PM -0800, Prashant Malani wrote:
> > > >> Set the number of altmodes available for a registered partner using the
> > > >> Type C connector class framework routine.
> > > >>
> > > >> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > > > 
> > > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > > 
> > > 
> > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > 
> > > Heikki, would you like to take these two through your tree? It'd help if you can
> > > create an inmutable branch so I can pick other cros_ec_typec patches on top of it.
> > 
> > I'm sorry, but I don't actually maintain a tree for the USB Type-C
> > stuff.
> > 
> > Greg, I'm sorry to bother you with this, but can you provide the
> > immutable branch for Enric?
> 
> My usb tree branches are always immutable :)
> 
> > Maybe I should set up a tree for the USB Type-C stuff?
> 
> Or I can just take all of these patches as they depend on USB stuff more
> than any other specific platform driver changes, right?

Works for me.

Br,

-- 
heikki