[PATCH v1 1/2] usb: typec: Introduce typec attributes for limiting source current

[PATCH v1 1/2] usb: typec: Introduce typec attributes for limiting source current

[Badhri Jagan Sridharan]

This change introduces the following two attributes to the
typec sysfs class which allows userspace to limit the power
advertised while acting as source. This is useful to mitigate
battery drain in portable devices when the battery SOC is low.

New attibutes introduced:
1. limit_src_current_active
2. limit_src_current_ma

The port while in PD contract and acting as source would
only advertise vSafe5V fixed PDO with current set through
limit_src_current_ma when limit_src_current_active is set
to 1. When limit_src_current_active is set to 0, the port
would publish the default source capabilities.
limit_src_current_ma would limit the current to the
Maximum current published by vSafe5V fixed pdo of the default
source capabilities of the port.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 Documentation/ABI/testing/sysfs-class-typec | 25 ++++++
 drivers/usb/typec/class.c                   | 99 +++++++++++++++++++++
 drivers/usb/typec/class.h                   |  5 ++
 include/linux/usb/typec.h                   |  4 +
 4 files changed, 133 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 75088ecad202..dd2240632172 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -141,6 +141,31 @@ Description:
 		- "reverse": CC2 orientation
 		- "unknown": Orientation cannot be determined.
 
+What:		/sys/class/typec/<port>/limit_src_current_active
+Date:		February 2022
+Contact:	Badhri Jagan Sridharan <badhri@google.com>
+Description:
+		This attribute can be used to make the port only publish
+		vSafe5V fixed pdo with Maximum current limited to the
+		current limit set by limit_src_current_ma when the port
+		is acting as source.
+		Valid values:
+		- write(2) "1" limits source capabilities to vSafe5V
+		  with max current specified by limit_src_current_ma
+		- write(2) "0" publishes the default source capabilities
+		  of the port.
+
+What:		/sys/class/typec/<port>/limit_src_current_ma
+Date:		February 2022
+Contact:	Badhri Jagan Sridharan <badhri@google.com>
+Description:
+		This attribute allows write(2) to set the Maximum
+		current published when limit_src_current_active is set
+		to 1. When limit_src_current_active is already set
+		to 1, if the port is already acting as source with
+		explicit contract in place, write(2) will make the port
+		renegotiate the pd contract.
+
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
 What:		/sys/class/typec/<port>-partner/accessory_mode
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 45a6f0c807cb..3b3c7b080ad1 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1403,6 +1403,102 @@ port_type_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RW(port_type);
 
+static ssize_t
+limit_src_current_active_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			       size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	int ret;
+	u8 active;
+
+	if (port->cap->type == TYPEC_PORT_SNK || !port->ops || !port->ops->limit_src_current_set ||
+	    !port->cap->pd_revision) {
+		dev_dbg(dev, "Limiting source current not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (kstrtou8(buf, 0, &active))
+		return -EINVAL;
+
+	if (active != 1 && active != 0)
+		return -EINVAL;
+
+	mutex_lock(&port->limit_src_current_lock);
+
+	if (port->limit_src_current_active == (bool)active) {
+		ret = size;
+		goto unlock_and_ret;
+	}
+
+	ret = port->ops->limit_src_current_set(port, port->limit_src_current_ma, active);
+	if (ret)
+		goto unlock_and_ret;
+
+	port->limit_src_current_active = active;
+	ret = size;
+
+unlock_and_ret:
+	mutex_unlock(&port->limit_src_current_lock);
+	return ret;
+}
+
+static ssize_t
+limit_src_current_active_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+
+	return sysfs_emit(buf, "%d\n", port->limit_src_current_active ? 1 : 0);
+}
+static DEVICE_ATTR_RW(limit_src_current_active);
+
+static ssize_t
+limit_src_current_ma_store(struct device *dev, struct device_attribute *attr, const char *buf,
+			   size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	int ret;
+	u32 src_current_ma;
+
+	if (port->cap->type == TYPEC_PORT_SNK || !port->ops || !port->ops->limit_src_current_set ||
+	    !port->cap->pd_revision) {
+		dev_dbg(dev, "Limiting source current not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (kstrtou32(buf, 0, &src_current_ma))
+		return -EINVAL;
+
+	mutex_lock(&port->limit_src_current_lock);
+
+	if (port->limit_src_current_ma == src_current_ma) {
+		ret = size;
+		goto unlock_and_ret;
+	}
+
+	if (port->limit_src_current_active) {
+		ret = port->ops->limit_src_current_set(port, src_current_ma,
+						       port->limit_src_current_active);
+		if (ret)
+			goto unlock_and_ret;
+	}
+
+	port->limit_src_current_ma = src_current_ma;
+	ret = size;
+
+unlock_and_ret:
+	mutex_unlock(&port->limit_src_current_lock);
+	return ret;
+}
+
+static ssize_t
+limit_src_current_ma_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+
+	return sysfs_emit(buf, "%u\n", port->limit_src_current_ma);
+}
+static DEVICE_ATTR_RW(limit_src_current_ma);
+
 static const char * const typec_pwr_opmodes[] = {
 	[TYPEC_PWR_MODE_USB]	= "default",
 	[TYPEC_PWR_MODE_1_5A]	= "1.5A",
@@ -1536,6 +1632,8 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_vconn_source.attr,
 	&dev_attr_port_type.attr,
 	&dev_attr_orientation.attr,
+	&dev_attr_limit_src_current_active.attr,
+	&dev_attr_limit_src_current_ma.attr,
 	NULL,
 };
 
@@ -2039,6 +2137,7 @@ struct typec_port *typec_register_port(struct device *parent,
 
 	ida_init(&port->mode_ids);
 	mutex_init(&port->port_type_lock);
+	mutex_init(&port->limit_src_current_lock);
 
 	port->id = id;
 	port->ops = cap->ops;
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index 0f1bd6d19d67..3856bc058444 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -54,6 +54,11 @@ struct typec_port {
 
 	const struct typec_capability	*cap;
 	const struct typec_operations   *ops;
+
+	/* lock to protect limit_src_current_*_store operation */
+	struct mutex			limit_src_current_lock;
+	u32				limit_src_current_ma;
+	bool				limit_src_current_active;
 };
 
 #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 7ba45a97eeae..1b1958ae4c16 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -213,6 +213,8 @@ struct typec_partner_desc {
  * @pr_set: Set Power Role
  * @vconn_set: Source VCONN
  * @port_type_set: Set port type
+ * @limit_src_current_set: Used to limit source current advertisement while
+ *                         acting as source
  */
 struct typec_operations {
 	int (*try_role)(struct typec_port *port, int role);
@@ -221,6 +223,8 @@ struct typec_operations {
 	int (*vconn_set)(struct typec_port *port, enum typec_role role);
 	int (*port_type_set)(struct typec_port *port,
 			     enum typec_port_type type);
+	int (*limit_src_current_set)(struct typec_port *port, u32 limit_src_current_ma,
+				     bool enable);
 };
 
 enum usb_pd_svdm_ver {
-- 
2.35.0.263.gb82422642f-goog

[PATCH v1 2/2] usb: typec: tcpm: Enable limit_src_current_set callback

[Badhri Jagan Sridharan]

This change allows TCPM to support limit_src_current_set
callback. When limit_src_current_set is enabled, tcpm
updates the local source capabilities to only publish
vSafe5V fixed pdo with the current limit passed through
limit_src_current_set callback. When limit_src_current_set
is disabled, tcpm revert back to publishing the default
source caps.

This patch is co-authored with kyletso@google.com and
also uses some of parts of the code that was reverted
by c17c7cf147ac56312156eaaaf8b2e19c9a59a71a.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 108 ++++++++++++++++++++++++++++++----
 include/linux/usb/tcpm.h      |   4 ++
 2 files changed, 102 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 5fce795b69c7..491ad6621671 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -475,6 +475,15 @@ struct tcpm_port {
 	 * SNK_READY for non-pd link.
 	 */
 	bool slow_charger_loop;
+
+	/*
+	 * Max current published in vSafe5V fixed pdo when limit_src_current_enabled
+	 * is active.
+	 */
+	u32 limit_src_current_ma;
+	/* True when source current limiting is enabled */
+	bool limit_src_current_enabled;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -5907,12 +5916,99 @@ static int tcpm_port_type_set(struct typec_port *p, enum typec_port_type type)
 	return 0;
 }
 
+int tcpm_update_source_capabilities_locked(struct tcpm_port *port)
+{
+	int ret = 0;
+
+	switch (port->state) {
+	case SRC_UNATTACHED:
+	case SRC_ATTACH_WAIT:
+	case SRC_TRYWAIT:
+		tcpm_set_cc(port, tcpm_rp_cc(port));
+		break;
+	case SRC_SEND_CAPABILITIES:
+	case SRC_NEGOTIATE_CAPABILITIES:
+	case SRC_READY:
+	case SRC_WAIT_NEW_CAPABILITIES:
+		port->upcoming_state = SRC_SEND_CAPABILITIES;
+		ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+		if (ret == -EAGAIN) {
+			port->upcoming_state = INVALID_STATE;
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+	return ret;
+}
+
+static int tcpm_fw_get_src_pdo(struct tcpm_port *port, struct fwnode_handle *fwnode, u32 *src_pdo,
+			       unsigned int *nr_src_pdo)
+{
+	int ret;
+
+	ret = fwnode_property_count_u32(fwnode, "source-pdos");
+	if (ret == 0)
+		return -EINVAL;
+	else if (ret < 0)
+		return ret;
+
+	*nr_src_pdo = min(ret, PDO_MAX_OBJECTS);
+	ret = fwnode_property_read_u32_array(fwnode, "source-pdos", src_pdo, *nr_src_pdo);
+
+	return ret;
+}
+
+static int tcpm_limit_src_current_set(struct typec_port *p, u32 limit_src_current_ma, bool enable)
+{
+	struct tcpm_port *port = typec_get_drvdata(p);
+	int ret = 0;
+
+	mutex_lock(&port->lock);
+	if (limit_src_current_ma == port->limit_src_current_ma &&
+	    enable == port->limit_src_current_enabled)
+		goto port_unlock;
+
+	ret = tcpm_fw_get_src_pdo(port, port->tcpc->fwnode, port->src_pdo, &port->nr_src_pdo);
+	if (ret)
+		return ret;
+
+	if (enable) {
+		u32 current_vSafe5V_max_current_ma =
+			((port->src_pdo[0] & (PDO_CURR_MASK << PDO_FIXED_CURR_SHIFT)) >>
+			 PDO_FIXED_CURR_SHIFT) * 10;
+		/*
+		 * Check to see if limited source current does not exceed the
+		 * max current published in default source cap.
+		 */
+		if (limit_src_current_ma > current_vSafe5V_max_current_ma) {
+			ret = -EINVAL;
+			goto port_unlock;
+		}
+		port->src_pdo[0] &= ~(PDO_CURR_MASK << PDO_FIXED_CURR_SHIFT);
+		port->src_pdo[0] |= PDO_FIXED_CURR(limit_src_current_ma);
+		port->nr_src_pdo = 1;
+	}
+
+	ret = tcpm_update_source_capabilities_locked(port);
+	if (!ret) {
+		port->limit_src_current_ma = limit_src_current_ma;
+		port->limit_src_current_enabled = enable;
+	}
+
+port_unlock:
+	mutex_unlock(&port->lock);
+	return ret;
+}
+
 static const struct typec_operations tcpm_ops = {
 	.try_role = tcpm_try_role,
 	.dr_set = tcpm_dr_set,
 	.pr_set = tcpm_pr_set,
 	.vconn_set = tcpm_vconn_set,
-	.port_type_set = tcpm_port_type_set
+	.port_type_set = tcpm_port_type_set,
+	.limit_src_current_set = tcpm_limit_src_current_set
 };
 
 void tcpm_tcpc_reset(struct tcpm_port *port)
@@ -5970,15 +6066,7 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
 
 	/* Get Source PDOs for the PD port or Source Rp value for the non-PD port */
 	if (port->pd_supported) {
-		ret = fwnode_property_count_u32(fwnode, "source-pdos");
-		if (ret == 0)
-			return -EINVAL;
-		else if (ret < 0)
-			return ret;
-
-		port->nr_src_pdo = min(ret, PDO_MAX_OBJECTS);
-		ret = fwnode_property_read_u32_array(fwnode, "source-pdos",
-						     port->src_pdo, port->nr_src_pdo);
+		ret = tcpm_fw_get_src_pdo(port, fwnode, port->src_pdo, &port->nr_src_pdo);
 		if (ret)
 			return ret;
 		ret = tcpm_validate_caps(port, port->src_pdo, port->nr_src_pdo);
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index bffc8d3e14ad..18372d34a9f4 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -165,5 +165,9 @@ void tcpm_pd_transmit_complete(struct tcpm_port *port,
 			       enum tcpm_transmit_status status);
 void tcpm_pd_hard_reset(struct tcpm_port *port);
 void tcpm_tcpc_reset(struct tcpm_port *port);
+bool tcpm_is_debouncing(struct tcpm_port *tcpm);
+bool tcpm_is_toggling(struct tcpm_port *port);
+int tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
+				    unsigned int nr_pdo);
 
 #endif /* __LINUX_USB_TCPM_H */
-- 
2.35.0.263.gb82422642f-goog

Re: [PATCH v1 2/2] usb: typec: tcpm: Enable limit_src_current_set callback

[Greg Kroah-Hartman]

On Sun, Feb 06, 2022 at 08:39:07PM -0800, Badhri Jagan Sridharan wrote:
> This change allows TCPM to support limit_src_current_set
> callback. When limit_src_current_set is enabled, tcpm
> updates the local source capabilities to only publish
> vSafe5V fixed pdo with the current limit passed through
> limit_src_current_set callback. When limit_src_current_set
> is disabled, tcpm revert back to publishing the default
> source caps.
> 
> This patch is co-authored with kyletso@google.com and
> also uses some of parts of the code that was reverted
> by c17c7cf147ac56312156eaaaf8b2e19c9a59a71a.

Please use the standard format for commits as described in the
documentation.

thanks,

greg k-h

Re: [PATCH v1 1/2] usb: typec: Introduce typec attributes for limiting source current

[Heikki Krogerus]

+Benson

On Sun, Feb 06, 2022 at 08:39:06PM -0800, Badhri Jagan Sridharan wrote:
> This change introduces the following two attributes to the
> typec sysfs class which allows userspace to limit the power
> advertised while acting as source. This is useful to mitigate
> battery drain in portable devices when the battery SOC is low.
> 
> New attibutes introduced:
> 1. limit_src_current_active
> 2. limit_src_current_ma
> 
> The port while in PD contract and acting as source would
> only advertise vSafe5V fixed PDO with current set through
> limit_src_current_ma when limit_src_current_active is set
> to 1. When limit_src_current_active is set to 0, the port
> would publish the default source capabilities.
> limit_src_current_ma would limit the current to the
> Maximum current published by vSafe5V fixed pdo of the default
> source capabilities of the port.

This competes with Benson's idea of having "sets" of capabilites from
which to choose the ones that we advertise to the partner. You could
also use that idea to cover this case as well. You just have two
source capabilities sets defined - one where you only have the vSafe5V
and another for everything.

Benson's idea also seems to be something what we can support with UCSI
and some native USB PD controller host interfaces, but limiting the
source capabitites to only vSafe5V is something that we can't do. I
means, on some platforms we may have a source capabilities "set" that
we can choose that only exposes the vSafe5V, but there is no guarantee
that we always have it (and it's unlikely that we ever have it). It's
up to some firmware that we have no control over.

So this is the wrong way and Benson's idea is the right way IMO.

I already prepared a proposal for adding support for Benson's idea:
https://lore.kernel.org/linux-usb/20220203144657.16527-1-heikki.krogerus@linux.intel.com/

This patch adds the attributes that you can use to choose the
capabilities that are advertised to the partner:
https://lore.kernel.org/linux-usb/20220203144657.16527-3-heikki.krogerus@linux.intel.com/

> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>  Documentation/ABI/testing/sysfs-class-typec | 25 ++++++
>  drivers/usb/typec/class.c                   | 99 +++++++++++++++++++++
>  drivers/usb/typec/class.h                   |  5 ++
>  include/linux/usb/typec.h                   |  4 +
>  4 files changed, 133 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index 75088ecad202..dd2240632172 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -141,6 +141,31 @@ Description:
>  		- "reverse": CC2 orientation
>  		- "unknown": Orientation cannot be determined.
>  
> +What:		/sys/class/typec/<port>/limit_src_current_active
> +Date:		February 2022
> +Contact:	Badhri Jagan Sridharan <badhri@google.com>
> +Description:
> +		This attribute can be used to make the port only publish
> +		vSafe5V fixed pdo with Maximum current limited to the
> +		current limit set by limit_src_current_ma when the port
> +		is acting as source.
> +		Valid values:
> +		- write(2) "1" limits source capabilities to vSafe5V
> +		  with max current specified by limit_src_current_ma
> +		- write(2) "0" publishes the default source capabilities
> +		  of the port.
> +
> +What:		/sys/class/typec/<port>/limit_src_current_ma
> +Date:		February 2022
> +Contact:	Badhri Jagan Sridharan <badhri@google.com>
> +Description:
> +		This attribute allows write(2) to set the Maximum
> +		current published when limit_src_current_active is set
> +		to 1. When limit_src_current_active is already set
> +		to 1, if the port is already acting as source with
> +		explicit contract in place, write(2) will make the port
> +		renegotiate the pd contract.
> +
>  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
>  
>  What:		/sys/class/typec/<port>-partner/accessory_mode
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 45a6f0c807cb..3b3c7b080ad1 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1403,6 +1403,102 @@ port_type_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RW(port_type);
>  
> +static ssize_t
> +limit_src_current_active_store(struct device *dev, struct device_attribute *attr, const char *buf,
> +			       size_t size)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +	int ret;
> +	u8 active;
> +
> +	if (port->cap->type == TYPEC_PORT_SNK || !port->ops || !port->ops->limit_src_current_set ||
> +	    !port->cap->pd_revision) {
> +		dev_dbg(dev, "Limiting source current not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (kstrtou8(buf, 0, &active))
> +		return -EINVAL;
> +
> +	if (active != 1 && active != 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&port->limit_src_current_lock);
> +
> +	if (port->limit_src_current_active == (bool)active) {
> +		ret = size;
> +		goto unlock_and_ret;
> +	}
> +
> +	ret = port->ops->limit_src_current_set(port, port->limit_src_current_ma, active);
> +	if (ret)
> +		goto unlock_and_ret;
> +
> +	port->limit_src_current_active = active;
> +	ret = size;
> +
> +unlock_and_ret:
> +	mutex_unlock(&port->limit_src_current_lock);
> +	return ret;
> +}
> +
> +static ssize_t
> +limit_src_current_active_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +
> +	return sysfs_emit(buf, "%d\n", port->limit_src_current_active ? 1 : 0);
> +}
> +static DEVICE_ATTR_RW(limit_src_current_active);
> +
> +static ssize_t
> +limit_src_current_ma_store(struct device *dev, struct device_attribute *attr, const char *buf,
> +			   size_t size)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +	int ret;
> +	u32 src_current_ma;
> +
> +	if (port->cap->type == TYPEC_PORT_SNK || !port->ops || !port->ops->limit_src_current_set ||
> +	    !port->cap->pd_revision) {
> +		dev_dbg(dev, "Limiting source current not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (kstrtou32(buf, 0, &src_current_ma))
> +		return -EINVAL;
> +
> +	mutex_lock(&port->limit_src_current_lock);
> +
> +	if (port->limit_src_current_ma == src_current_ma) {
> +		ret = size;
> +		goto unlock_and_ret;
> +	}
> +
> +	if (port->limit_src_current_active) {
> +		ret = port->ops->limit_src_current_set(port, src_current_ma,
> +						       port->limit_src_current_active);
> +		if (ret)
> +			goto unlock_and_ret;
> +	}
> +
> +	port->limit_src_current_ma = src_current_ma;
> +	ret = size;
> +
> +unlock_and_ret:
> +	mutex_unlock(&port->limit_src_current_lock);
> +	return ret;
> +}
> +
> +static ssize_t
> +limit_src_current_ma_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct typec_port *port = to_typec_port(dev);
> +
> +	return sysfs_emit(buf, "%u\n", port->limit_src_current_ma);
> +}
> +static DEVICE_ATTR_RW(limit_src_current_ma);
> +
>  static const char * const typec_pwr_opmodes[] = {
>  	[TYPEC_PWR_MODE_USB]	= "default",
>  	[TYPEC_PWR_MODE_1_5A]	= "1.5A",
> @@ -1536,6 +1632,8 @@ static struct attribute *typec_attrs[] = {
>  	&dev_attr_vconn_source.attr,
>  	&dev_attr_port_type.attr,
>  	&dev_attr_orientation.attr,
> +	&dev_attr_limit_src_current_active.attr,
> +	&dev_attr_limit_src_current_ma.attr,
>  	NULL,
>  };
>  
> @@ -2039,6 +2137,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  
>  	ida_init(&port->mode_ids);
>  	mutex_init(&port->port_type_lock);
> +	mutex_init(&port->limit_src_current_lock);
>  
>  	port->id = id;
>  	port->ops = cap->ops;
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index 0f1bd6d19d67..3856bc058444 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -54,6 +54,11 @@ struct typec_port {
>  
>  	const struct typec_capability	*cap;
>  	const struct typec_operations   *ops;
> +
> +	/* lock to protect limit_src_current_*_store operation */
> +	struct mutex			limit_src_current_lock;
> +	u32				limit_src_current_ma;
> +	bool				limit_src_current_active;
>  };
>  
>  #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 7ba45a97eeae..1b1958ae4c16 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -213,6 +213,8 @@ struct typec_partner_desc {
>   * @pr_set: Set Power Role
>   * @vconn_set: Source VCONN
>   * @port_type_set: Set port type
> + * @limit_src_current_set: Used to limit source current advertisement while
> + *                         acting as source
>   */
>  struct typec_operations {
>  	int (*try_role)(struct typec_port *port, int role);
> @@ -221,6 +223,8 @@ struct typec_operations {
>  	int (*vconn_set)(struct typec_port *port, enum typec_role role);
>  	int (*port_type_set)(struct typec_port *port,
>  			     enum typec_port_type type);
> +	int (*limit_src_current_set)(struct typec_port *port, u32 limit_src_current_ma,
> +				     bool enable);
>  };
>  
>  enum usb_pd_svdm_ver {
> -- 
> 2.35.0.263.gb82422642f-goog

-- 
heikki

Re: [PATCH v1 1/2] usb: typec: Introduce typec attributes for limiting source current

[Badhri Jagan Sridharan]

On Mon, Feb 7, 2022 at 1:23 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> +Benson
>
> On Sun, Feb 06, 2022 at 08:39:06PM -0800, Badhri Jagan Sridharan wrote:
> > This change introduces the following two attributes to the
> > typec sysfs class which allows userspace to limit the power
> > advertised while acting as source. This is useful to mitigate
> > battery drain in portable devices when the battery SOC is low.
> >
> > New attibutes introduced:
> > 1. limit_src_current_active
> > 2. limit_src_current_ma
> >
> > The port while in PD contract and acting as source would
> > only advertise vSafe5V fixed PDO with current set through
> > limit_src_current_ma when limit_src_current_active is set
> > to 1. When limit_src_current_active is set to 0, the port
> > would publish the default source capabilities.
> > limit_src_current_ma would limit the current to the
> > Maximum current published by vSafe5V fixed pdo of the default
> > source capabilities of the port.
>
> This competes with Benson's idea of having "sets" of capabilites from
> which to choose the ones that we advertise to the partner. You could
> also use that idea to cover this case as well. You just have two
> source capabilities sets defined - one where you only have the vSafe5V
> and another for everything.

Agree. I was actually wondering whether there were any follow-ups for
the proposal.
Happy to make use of the interface additions that you are exposing through your
patchset.  Thanks for sending them out !

>
> Benson's idea also seems to be something what we can support with UCSI
> and some native USB PD controller host interfaces, but limiting the
> source capabitites to only vSafe5V is something that we can't do. I
> means, on some platforms we may have a source capabilities "set" that
> we can choose that only exposes the vSafe5V, but there is no guarantee
> that we always have it (and it's unlikely that we ever have it). It's
> up to some firmware that we have no control over.

Given that all pd sources should
mandatorily support vSafe5V and the patch only "reduces" the maximum
current exposed by default, there are no new additional capabilities
that the source
should support. So not sure why this would be not supportable though.

>
> So this is the wrong way and Benson's idea is the right way IMO.
>
> I already prepared a proposal for adding support for Benson's idea:
> https://lore.kernel.org/linux-usb/20220203144657.16527-1-heikki.krogerus@linux.intel.com/
>
> This patch adds the attributes that you can use to choose the
> capabilities that are advertised to the partner:
> https://lore.kernel.org/linux-usb/20220203144657.16527-3-heikki.krogerus@linux.intel.com/
>
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec | 25 ++++++
> >  drivers/usb/typec/class.c                   | 99 +++++++++++++++++++++
> >  drivers/usb/typec/class.h                   |  5 ++
> >  include/linux/usb/typec.h                   |  4 +
> >  4 files changed, 133 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index 75088ecad202..dd2240632172 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -141,6 +141,31 @@ Description:
> >               - "reverse": CC2 orientation
> >               - "unknown": Orientation cannot be determined.
> >
> > +What:                /sys/class/typec/<port>/limit_src_current_active
> > +Date:                February 2022
> > +Contact:     Badhri Jagan Sridharan <badhri@google.com>
> > +Description:
> > +             This attribute can be used to make the port only publish
> > +             vSafe5V fixed pdo with Maximum current limited to the
> > +             current limit set by limit_src_current_ma when the port
> > +             is acting as source.
> > +             Valid values:
> > +             - write(2) "1" limits source capabilities to vSafe5V
> > +               with max current specified by limit_src_current_ma
> > +             - write(2) "0" publishes the default source capabilities
> > +               of the port.
> > +
> > +What:                /sys/class/typec/<port>/limit_src_current_ma
> > +Date:                February 2022
> > +Contact:     Badhri Jagan Sridharan <badhri@google.com>
> > +Description:
> > +             This attribute allows write(2) to set the Maximum
> > +             current published when limit_src_current_active is set
> > +             to 1. When limit_src_current_active is already set
> > +             to 1, if the port is already acting as source with
> > +             explicit contract in place, write(2) will make the port
> > +             renegotiate the pd contract.
> > +
> >  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> >
> >  What:                /sys/class/typec/<port>-partner/accessory_mode
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 45a6f0c807cb..3b3c7b080ad1 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -1403,6 +1403,102 @@ port_type_show(struct device *dev, struct device_attribute *attr,
> >  }
> >  static DEVICE_ATTR_RW(port_type);
> >
> > +static ssize_t
> > +limit_src_current_active_store(struct device *dev, struct device_attribute *attr, const char *buf,
> > +                            size_t size)
> > +{
> > +     struct typec_port *port = to_typec_port(dev);
> > +     int ret;
> > +     u8 active;
> > +
> > +     if (port->cap->type == TYPEC_PORT_SNK || !port->ops || !port->ops->limit_src_current_set ||
> > +         !port->cap->pd_revision) {
> > +             dev_dbg(dev, "Limiting source current not supported\n");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     if (kstrtou8(buf, 0, &active))
> > +             return -EINVAL;
> > +
> > +     if (active != 1 && active != 0)
> > +             return -EINVAL;
> > +
> > +     mutex_lock(&port->limit_src_current_lock);
> > +
> > +     if (port->limit_src_current_active == (bool)active) {
> > +             ret = size;
> > +             goto unlock_and_ret;
> > +     }
> > +
> > +     ret = port->ops->limit_src_current_set(port, port->limit_src_current_ma, active);
> > +     if (ret)
> > +             goto unlock_and_ret;
> > +
> > +     port->limit_src_current_active = active;
> > +     ret = size;
> > +
> > +unlock_and_ret:
> > +     mutex_unlock(&port->limit_src_current_lock);
> > +     return ret;
> > +}
> > +
> > +static ssize_t
> > +limit_src_current_active_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +     struct typec_port *port = to_typec_port(dev);
> > +
> > +     return sysfs_emit(buf, "%d\n", port->limit_src_current_active ? 1 : 0);
> > +}
> > +static DEVICE_ATTR_RW(limit_src_current_active);
> > +
> > +static ssize_t
> > +limit_src_current_ma_store(struct device *dev, struct device_attribute *attr, const char *buf,
> > +                        size_t size)
> > +{
> > +     struct typec_port *port = to_typec_port(dev);
> > +     int ret;
> > +     u32 src_current_ma;
> > +
> > +     if (port->cap->type == TYPEC_PORT_SNK || !port->ops || !port->ops->limit_src_current_set ||
> > +         !port->cap->pd_revision) {
> > +             dev_dbg(dev, "Limiting source current not supported\n");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     if (kstrtou32(buf, 0, &src_current_ma))
> > +             return -EINVAL;
> > +
> > +     mutex_lock(&port->limit_src_current_lock);
> > +
> > +     if (port->limit_src_current_ma == src_current_ma) {
> > +             ret = size;
> > +             goto unlock_and_ret;
> > +     }
> > +
> > +     if (port->limit_src_current_active) {
> > +             ret = port->ops->limit_src_current_set(port, src_current_ma,
> > +                                                    port->limit_src_current_active);
> > +             if (ret)
> > +                     goto unlock_and_ret;
> > +     }
> > +
> > +     port->limit_src_current_ma = src_current_ma;
> > +     ret = size;
> > +
> > +unlock_and_ret:
> > +     mutex_unlock(&port->limit_src_current_lock);
> > +     return ret;
> > +}
> > +
> > +static ssize_t
> > +limit_src_current_ma_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +     struct typec_port *port = to_typec_port(dev);
> > +
> > +     return sysfs_emit(buf, "%u\n", port->limit_src_current_ma);
> > +}
> > +static DEVICE_ATTR_RW(limit_src_current_ma);
> > +
> >  static const char * const typec_pwr_opmodes[] = {
> >       [TYPEC_PWR_MODE_USB]    = "default",
> >       [TYPEC_PWR_MODE_1_5A]   = "1.5A",
> > @@ -1536,6 +1632,8 @@ static struct attribute *typec_attrs[] = {
> >       &dev_attr_vconn_source.attr,
> >       &dev_attr_port_type.attr,
> >       &dev_attr_orientation.attr,
> > +     &dev_attr_limit_src_current_active.attr,
> > +     &dev_attr_limit_src_current_ma.attr,
> >       NULL,
> >  };
> >
> > @@ -2039,6 +2137,7 @@ struct typec_port *typec_register_port(struct device *parent,
> >
> >       ida_init(&port->mode_ids);
> >       mutex_init(&port->port_type_lock);
> > +     mutex_init(&port->limit_src_current_lock);
> >
> >       port->id = id;
> >       port->ops = cap->ops;
> > diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> > index 0f1bd6d19d67..3856bc058444 100644
> > --- a/drivers/usb/typec/class.h
> > +++ b/drivers/usb/typec/class.h
> > @@ -54,6 +54,11 @@ struct typec_port {
> >
> >       const struct typec_capability   *cap;
> >       const struct typec_operations   *ops;
> > +
> > +     /* lock to protect limit_src_current_*_store operation */
> > +     struct mutex                    limit_src_current_lock;
> > +     u32                             limit_src_current_ma;
> > +     bool                            limit_src_current_active;
> >  };
> >
> >  #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> > diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> > index 7ba45a97eeae..1b1958ae4c16 100644
> > --- a/include/linux/usb/typec.h
> > +++ b/include/linux/usb/typec.h
> > @@ -213,6 +213,8 @@ struct typec_partner_desc {
> >   * @pr_set: Set Power Role
> >   * @vconn_set: Source VCONN
> >   * @port_type_set: Set port type
> > + * @limit_src_current_set: Used to limit source current advertisement while
> > + *                         acting as source
> >   */
> >  struct typec_operations {
> >       int (*try_role)(struct typec_port *port, int role);
> > @@ -221,6 +223,8 @@ struct typec_operations {
> >       int (*vconn_set)(struct typec_port *port, enum typec_role role);
> >       int (*port_type_set)(struct typec_port *port,
> >                            enum typec_port_type type);
> > +     int (*limit_src_current_set)(struct typec_port *port, u32 limit_src_current_ma,
> > +                                  bool enable);
> >  };
> >
> >  enum usb_pd_svdm_ver {
> > --
> > 2.35.0.263.gb82422642f-goog
>
> --
> heikki