All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] power: supply: add input voltage limit property.
@ 2018-11-21 15:34 Enric Balletbo i Serra
  2018-11-21 15:34 ` [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit Enric Balletbo i Serra
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-21 15:34 UTC (permalink / raw)
  To: linux-pm, sre
  Cc: Sameer Nanda, gwendal, linux-kernel, groeck, kernel, bleung,
	Rafael J. Wysocki, Len Brown, Pavel Machek

We have a problem with USBPD chargers which under certain conditions
can result in system overheating if the voltage provided by the USBPD
port is too high. While the preferred means to control this would be
through devicetree or ACPI settings, this is not always possible, and
we need to have a means to set a voltage limit.

This patch exposes a new property, similar to input current limit, to
re-configure the maximum voltage from the external supply at runtime
based on system-level knowledge or user input.

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

 Documentation/power/power_supply_class.txt | 2 ++
 drivers/power/supply/power_supply_sysfs.c  | 1 +
 include/linux/power_supply.h               | 1 +
 3 files changed, 4 insertions(+)

diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
index 300d37896e51..7b4be615b4f8 100644
--- a/Documentation/power/power_supply_class.txt
+++ b/Documentation/power/power_supply_class.txt
@@ -137,6 +137,8 @@ power supply object.
 
 INPUT_CURRENT_LIMIT - input current limit programmed by charger. Indicates
 the current drawn from a charging source.
+INPUT_VOLTAGE_LIMIT - input voltage limit programmed by charger. Indicates
+the voltage limit from a charging source.
 
 CHARGE_CONTROL_LIMIT - current charge control limit setting
 CHARGE_CONTROL_LIMIT_MAX - maximum charge control limit setting
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index dce24f596160..5848742ebb59 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -275,6 +275,7 @@ static struct device_attribute power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(charge_control_limit),
 	POWER_SUPPLY_ATTR(charge_control_limit_max),
 	POWER_SUPPLY_ATTR(input_current_limit),
+	POWER_SUPPLY_ATTR(input_voltage_limit),
 	POWER_SUPPLY_ATTR(energy_full_design),
 	POWER_SUPPLY_ATTR(energy_empty_design),
 	POWER_SUPPLY_ATTR(energy_full),
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index f80769175c56..608ba88e32ee 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -122,6 +122,7 @@ enum power_supply_property {
 	POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT,
 	POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
 	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+	POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
 	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
 	POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
 	POWER_SUPPLY_PROP_ENERGY_FULL,
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit.
  2018-11-21 15:34 [PATCH 1/2] power: supply: add input voltage limit property Enric Balletbo i Serra
@ 2018-11-21 15:34 ` Enric Balletbo i Serra
  2018-11-21 19:03   ` Guenter Roeck
  2018-11-21 19:00 ` [PATCH 1/2] power: supply: add input voltage limit property Guenter Roeck
  2018-11-21 22:11 ` Adam Thomson
  2 siblings, 1 reply; 5+ messages in thread
From: Enric Balletbo i Serra @ 2018-11-21 15:34 UTC (permalink / raw)
  To: linux-pm, sre; +Cc: Sameer Nanda, gwendal, linux-kernel, groeck, kernel, bleung

This patch allows reading and writing the input voltage and current
limit through the POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT and
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT sysfs properties. This allows
userspace to see current values and to re-configure these values at
runtime based on system-level knowledge or user input.

By default there is no limit, this is reported as a -1 when reading from
userspace. Writing a value will set the current or voltage limit in uA
or uV, and writing any negative value will remove that limit.

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

 drivers/power/supply/cros_usbpd-charger.c | 117 ++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
index 7e9c3984ef6a..f011ce71b30c 100644
--- a/drivers/power/supply/cros_usbpd-charger.c
+++ b/drivers/power/supply/cros_usbpd-charger.c
@@ -53,6 +53,8 @@ struct charger_data {
 };
 
 static enum power_supply_property cros_usbpd_charger_props[] = {
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+	POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_CURRENT_MAX,
@@ -80,6 +82,10 @@ static enum power_supply_usb_type cros_usbpd_charger_usb_types[] = {
 	POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID
 };
 
+/* Input voltage/current limit in mV/mA. Default to none. */
+static u16 input_voltage_limit = EC_POWER_LIMIT_NONE;
+static u16 input_current_limit = EC_POWER_LIMIT_NONE;
+
 static bool cros_usbpd_charger_port_is_dedicated(struct port_data *port)
 {
 	return port->port_number >= port->charger->num_usbpd_ports;
@@ -324,6 +330,26 @@ static int cros_usbpd_charger_get_port_status(struct port_data *port,
 	return ret;
 }
 
+static int cros_usbpd_charger_set_ext_power_limit(struct charger_data *charger,
+						  u16 current_lim,
+						  u16 voltage_lim)
+{
+	struct ec_params_external_power_limit_v1 req;
+	int ret;
+
+	req.current_lim = current_lim;
+	req.voltage_lim = voltage_lim;
+
+	ret = cros_usbpd_charger_ec_command(charger, 0,
+					    EC_CMD_EXTERNAL_POWER_LIMIT,
+					    &req, sizeof(req), NULL, 0);
+	if (ret < 0)
+		dev_err(charger->dev,
+			"Unable to set the 'External Power Limit': %d\n", ret);
+
+	return ret;
+}
+
 static void cros_usbpd_charger_power_changed(struct power_supply *psy)
 {
 	struct port_data *port = power_supply_get_drvdata(psy);
@@ -396,6 +422,18 @@ static int cros_usbpd_charger_get_prop(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_USB_TYPE:
 		val->intval = port->psy_usb_type;
 		break;
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		if (input_current_limit == EC_POWER_LIMIT_NONE)
+			val->intval = -1;
+		else
+			val->intval = input_current_limit * 1000;
+		break;
+	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+		if (input_voltage_limit == EC_POWER_LIMIT_NONE)
+			val->intval = -1;
+		else
+			val->intval = input_voltage_limit * 1000;
+		break;
 	case POWER_SUPPLY_PROP_MODEL_NAME:
 		val->strval = port->model_name;
 		break;
@@ -409,6 +447,82 @@ static int cros_usbpd_charger_get_prop(struct power_supply *psy,
 	return 0;
 }
 
+static int cros_usbpd_charger_set_prop(struct power_supply *psy,
+				       enum power_supply_property psp,
+				       const union power_supply_propval *val)
+{
+	struct port_data *port = power_supply_get_drvdata(psy);
+	struct charger_data *charger = port->charger;
+	struct device *dev = charger->dev;
+	u16 intval;
+	int ret;
+
+	/* U16_MAX in mV/mA is the maximum supported value */
+	if (val->intval > U16_MAX * 1000)
+		return -EINVAL;
+	else if (val->intval < 0)
+		/* A negative number is used to clear the limit */
+		intval = EC_POWER_LIMIT_NONE;
+	else
+		/* Convert from uA/uV to mA/mV */
+		intval = val->intval / 1000;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = cros_usbpd_charger_set_ext_power_limit(charger, intval,
+							input_voltage_limit);
+		if (ret < 0)
+			break;
+
+		input_current_limit = intval;
+		if (input_current_limit == EC_POWER_LIMIT_NONE)
+			dev_info(dev,
+			  "External Current Limit cleared for all ports\n");
+		else
+			dev_info(dev,
+			  "External Current Limit set to %dmA for all ports\n",
+			  input_current_limit);
+		break;
+	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+		ret = cros_usbpd_charger_set_ext_power_limit(charger,
+							input_current_limit,
+							intval);
+		if (ret < 0)
+			break;
+
+		input_voltage_limit = intval;
+		if (input_voltage_limit == EC_POWER_LIMIT_NONE)
+			dev_info(dev,
+			  "External Voltage Limit cleared for all ports\n");
+		else
+			dev_info(dev,
+			  "External Voltage Limit set to %dmV for all ports\n",
+			  input_voltage_limit);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int cros_usbpd_charger_property_is_writeable(struct power_supply *psy,
+						enum power_supply_property psp)
+{
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+	case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+		ret = 1;
+		break;
+	default:
+		ret = 0;
+	}
+
+	return ret;
+}
+
 static int cros_usbpd_charger_ec_event(struct notifier_block *nb,
 				       unsigned long queued_during_suspend,
 				       void *_notify)
@@ -525,6 +639,9 @@ static int cros_usbpd_charger_probe(struct platform_device *pd)
 
 		psy_desc = &port->psy_desc;
 		psy_desc->get_property = cros_usbpd_charger_get_prop;
+		psy_desc->set_property = cros_usbpd_charger_set_prop;
+		psy_desc->property_is_writeable =
+				cros_usbpd_charger_property_is_writeable;
 		psy_desc->external_power_changed =
 					cros_usbpd_charger_power_changed;
 		psy_cfg.drv_data = port;
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] power: supply: add input voltage limit property.
  2018-11-21 15:34 [PATCH 1/2] power: supply: add input voltage limit property Enric Balletbo i Serra
  2018-11-21 15:34 ` [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit Enric Balletbo i Serra
@ 2018-11-21 19:00 ` Guenter Roeck
  2018-11-21 22:11 ` Adam Thomson
  2 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2018-11-21 19:00 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: linux-pm, Sebastian Reichel, Sameer Nanda, Gwendal Grignou,
	linux-kernel, Guenter Roeck, kernel, Benson Leung, rjw,
	len.brown, Pavel Machek

On Wed, Nov 21, 2018 at 7:34 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> We have a problem with USBPD chargers which under certain conditions
> can result in system overheating if the voltage provided by the USBPD
> port is too high. While the preferred means to control this would be
> through devicetree or ACPI settings, this is not always possible, and
> we need to have a means to set a voltage limit.
>
> This patch exposes a new property, similar to input current limit, to
> re-configure the maximum voltage from the external supply at runtime
> based on system-level knowledge or user input.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>


Reviewed-by: Guenter Roeck <groeck@chromium.org>

>
> ---
>
>  Documentation/power/power_supply_class.txt | 2 ++
>  drivers/power/supply/power_supply_sysfs.c  | 1 +
>  include/linux/power_supply.h               | 1 +
>  3 files changed, 4 insertions(+)
>
> diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
> index 300d37896e51..7b4be615b4f8 100644
> --- a/Documentation/power/power_supply_class.txt
> +++ b/Documentation/power/power_supply_class.txt
> @@ -137,6 +137,8 @@ power supply object.
>
>  INPUT_CURRENT_LIMIT - input current limit programmed by charger. Indicates
>  the current drawn from a charging source.
> +INPUT_VOLTAGE_LIMIT - input voltage limit programmed by charger. Indicates
> +the voltage limit from a charging source.
>
>  CHARGE_CONTROL_LIMIT - current charge control limit setting
>  CHARGE_CONTROL_LIMIT_MAX - maximum charge control limit setting
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index dce24f596160..5848742ebb59 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -275,6 +275,7 @@ static struct device_attribute power_supply_attrs[] = {
>         POWER_SUPPLY_ATTR(charge_control_limit),
>         POWER_SUPPLY_ATTR(charge_control_limit_max),
>         POWER_SUPPLY_ATTR(input_current_limit),
> +       POWER_SUPPLY_ATTR(input_voltage_limit),
>         POWER_SUPPLY_ATTR(energy_full_design),
>         POWER_SUPPLY_ATTR(energy_empty_design),
>         POWER_SUPPLY_ATTR(energy_full),
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index f80769175c56..608ba88e32ee 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -122,6 +122,7 @@ enum power_supply_property {
>         POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT,
>         POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
>         POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +       POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
>         POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
>         POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
>         POWER_SUPPLY_PROP_ENERGY_FULL,
> --
> 2.19.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit.
  2018-11-21 15:34 ` [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit Enric Balletbo i Serra
@ 2018-11-21 19:03   ` Guenter Roeck
  0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2018-11-21 19:03 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: linux-pm, Sebastian Reichel, Sameer Nanda, Gwendal Grignou,
	linux-kernel, Guenter Roeck, kernel, Benson Leung

Trying again, this time (hopefully if gmail lets me) in plaintext
mode. Sorry for the noise.

On Wed, Nov 21, 2018 at 7:34 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> This patch allows reading and writing the input voltage and current
> limit through the POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT and
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT sysfs properties. This allows
> userspace to see current values and to re-configure these values at
> runtime based on system-level knowledge or user input.
>
> By default there is no limit, this is reported as a -1 when reading from
> userspace. Writing a value will set the current or voltage limit in uA
> or uV, and writing any negative value will remove that limit.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
>  drivers/power/supply/cros_usbpd-charger.c | 117 ++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
>
> diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
> index 7e9c3984ef6a..f011ce71b30c 100644
> --- a/drivers/power/supply/cros_usbpd-charger.c
> +++ b/drivers/power/supply/cros_usbpd-charger.c
> @@ -53,6 +53,8 @@ struct charger_data {
>  };
>
>  static enum power_supply_property cros_usbpd_charger_props[] = {
> +       POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +       POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
>         POWER_SUPPLY_PROP_ONLINE,
>         POWER_SUPPLY_PROP_STATUS,
>         POWER_SUPPLY_PROP_CURRENT_MAX,
> @@ -80,6 +82,10 @@ static enum power_supply_usb_type cros_usbpd_charger_usb_types[] = {
>         POWER_SUPPLY_USB_TYPE_APPLE_BRICK_ID
>  };
>
> +/* Input voltage/current limit in mV/mA. Default to none. */
> +static u16 input_voltage_limit = EC_POWER_LIMIT_NONE;
> +static u16 input_current_limit = EC_POWER_LIMIT_NONE;
> +
>  static bool cros_usbpd_charger_port_is_dedicated(struct port_data *port)
>  {
>         return port->port_number >= port->charger->num_usbpd_ports;
> @@ -324,6 +330,26 @@ static int cros_usbpd_charger_get_port_status(struct port_data *port,
>         return ret;
>  }
>
> +static int cros_usbpd_charger_set_ext_power_limit(struct charger_data *charger,
> +                                                 u16 current_lim,
> +                                                 u16 voltage_lim)
> +{
> +       struct ec_params_external_power_limit_v1 req;
> +       int ret;
> +
> +       req.current_lim = current_lim;
> +       req.voltage_lim = voltage_lim;
> +
> +       ret = cros_usbpd_charger_ec_command(charger, 0,
> +                                           EC_CMD_EXTERNAL_POWER_LIMIT,
> +                                           &req, sizeof(req), NULL, 0);
> +       if (ret < 0)
> +               dev_err(charger->dev,
> +                       "Unable to set the 'External Power Limit': %d\n", ret);
> +
> +       return ret;
> +}
> +
>  static void cros_usbpd_charger_power_changed(struct power_supply *psy)
>  {
>         struct port_data *port = power_supply_get_drvdata(psy);
> @@ -396,6 +422,18 @@ static int cros_usbpd_charger_get_prop(struct power_supply *psy,
>         case POWER_SUPPLY_PROP_USB_TYPE:
>                 val->intval = port->psy_usb_type;
>                 break;
> +       case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +               if (input_current_limit == EC_POWER_LIMIT_NONE)
> +                       val->intval = -1;
> +               else
> +                       val->intval = input_current_limit * 1000;
> +               break;
> +       case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
> +               if (input_voltage_limit == EC_POWER_LIMIT_NONE)
> +                       val->intval = -1;
> +               else
> +                       val->intval = input_voltage_limit * 1000;
> +               break;
>         case POWER_SUPPLY_PROP_MODEL_NAME:
>                 val->strval = port->model_name;
>                 break;
> @@ -409,6 +447,82 @@ static int cros_usbpd_charger_get_prop(struct power_supply *psy,
>         return 0;
>  }
>
> +static int cros_usbpd_charger_set_prop(struct power_supply *psy,
> +                                      enum power_supply_property psp,
> +                                      const union power_supply_propval *val)
> +{
> +       struct port_data *port = power_supply_get_drvdata(psy);
> +       struct charger_data *charger = port->charger;
> +       struct device *dev = charger->dev;
> +       u16 intval;
> +       int ret;
> +
> +       /* U16_MAX in mV/mA is the maximum supported value */
> +       if (val->intval > U16_MAX * 1000)

The accepted upper limit is 0xffff * 1000, so this can result in an
intval of 0xffff after the divide
operation below. At the same time, EC_POWER_LIMIT_NONE is defined as
0xffff. This means
that setting a value of 65535000 will be interpreted as
EC_POWER_LIMIT_NONE and, when
read, will return -1. This is undesirable, but easy to fix by making
the above check ">="
instead of ">".

> +               return -EINVAL;
> +       else if (val->intval < 0)

Unnecessary else

> +               /* A negative number is used to clear the limit */
> +               intval = EC_POWER_LIMIT_NONE;
> +       else
> +               /* Convert from uA/uV to mA/mV */
> +               intval = val->intval / 1000;
> +
> +       switch (psp) {
> +       case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +               ret = cros_usbpd_charger_set_ext_power_limit(charger, intval,
> +                                                       input_voltage_limit);
> +               if (ret < 0)
> +                       break;
> +
> +               input_current_limit = intval;
> +               if (input_current_limit == EC_POWER_LIMIT_NONE)
> +                       dev_info(dev,
> +                         "External Current Limit cleared for all ports\n");
> +               else
> +                       dev_info(dev,
> +                         "External Current Limit set to %dmA for all ports\n",
> +                         input_current_limit);
> +               break;
> +       case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
> +               ret = cros_usbpd_charger_set_ext_power_limit(charger,
> +                                                       input_current_limit,
> +                                                       intval);
> +               if (ret < 0)
> +                       break;
> +
> +               input_voltage_limit = intval;
> +               if (input_voltage_limit == EC_POWER_LIMIT_NONE)
> +                       dev_info(dev,
> +                         "External Voltage Limit cleared for all ports\n");
> +               else
> +                       dev_info(dev,
> +                         "External Voltage Limit set to %dmV for all ports\n",
> +                         input_voltage_limit);
> +               break;
> +       default:
> +               ret = -EINVAL;
> +       }
> +
> +       return ret;
> +}
> +
> +static int cros_usbpd_charger_property_is_writeable(struct power_supply *psy,
> +                                               enum power_supply_property psp)
> +{
> +       int ret;
> +
> +       switch (psp) {
> +       case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +       case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
> +               ret = 1;
> +               break;
> +       default:
> +               ret = 0;
> +       }
> +
> +       return ret;
> +}
> +
>  static int cros_usbpd_charger_ec_event(struct notifier_block *nb,
>                                        unsigned long queued_during_suspend,
>                                        void *_notify)
> @@ -525,6 +639,9 @@ static int cros_usbpd_charger_probe(struct platform_device *pd)
>
>                 psy_desc = &port->psy_desc;
>                 psy_desc->get_property = cros_usbpd_charger_get_prop;
> +               psy_desc->set_property = cros_usbpd_charger_set_prop;
> +               psy_desc->property_is_writeable =
> +                               cros_usbpd_charger_property_is_writeable;
>                 psy_desc->external_power_changed =
>                                         cros_usbpd_charger_power_changed;
>                 psy_cfg.drv_data = port;
> --
> 2.19.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH 1/2] power: supply: add input voltage limit property.
  2018-11-21 15:34 [PATCH 1/2] power: supply: add input voltage limit property Enric Balletbo i Serra
  2018-11-21 15:34 ` [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit Enric Balletbo i Serra
  2018-11-21 19:00 ` [PATCH 1/2] power: supply: add input voltage limit property Guenter Roeck
@ 2018-11-21 22:11 ` Adam Thomson
  2 siblings, 0 replies; 5+ messages in thread
From: Adam Thomson @ 2018-11-21 22:11 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-pm, sre
  Cc: Sameer Nanda, gwendal, linux-kernel, groeck, kernel, bleung,
	Rafael J. Wysocki, Len Brown, Pavel Machek

On 21 November 2018 15:35, Enric Balletbo i Serra wrote:

> We have a problem with USBPD chargers which under certain conditions 
> can result in system overheating if the voltage provided by the USBPD 
> port is too high. While the preferred means to control this would be 
> through devicetree or ACPI settings, this is not always possible, and 
> we need to have a means to set a voltage limit.
> 
> This patch exposes a new property, similar to input current limit, to 
> re-configure the maximum voltage from the external supply at runtime 
> based on system-level knowledge or user input.

I think we should be adding new documentation to the following file for any property changes:

Documentation/ABI/testing/sysfs-class-power

Sebastian can maybe comment further though. 
 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>  Documentation/power/power_supply_class.txt | 2 ++ 
> drivers/power/supply/power_supply_sysfs.c  | 1 +
>  include/linux/power_supply.h               | 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/power/power_supply_class.txt
> b/Documentation/power/power_supply_class.txt
> index 300d37896e51..7b4be615b4f8 100644
> --- a/Documentation/power/power_supply_class.txt
> +++ b/Documentation/power/power_supply_class.txt
> @@ -137,6 +137,8 @@ power supply object.
> 
>  INPUT_CURRENT_LIMIT - input current limit programmed by charger. 
> Indicates the current drawn from a charging source.
> +INPUT_VOLTAGE_LIMIT - input voltage limit programmed by charger.
> +Indicates the voltage limit from a charging source.
> 
>  CHARGE_CONTROL_LIMIT - current charge control limit setting 
> CHARGE_CONTROL_LIMIT_MAX - maximum charge control limit setting diff 
> --git a/drivers/power/supply/power_supply_sysfs.c
> b/drivers/power/supply/power_supply_sysfs.c
> index dce24f596160..5848742ebb59 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -275,6 +275,7 @@ static struct device_attribute power_supply_attrs[] = {
>  	POWER_SUPPLY_ATTR(charge_control_limit),
>  	POWER_SUPPLY_ATTR(charge_control_limit_max),
>  	POWER_SUPPLY_ATTR(input_current_limit),
> +	POWER_SUPPLY_ATTR(input_voltage_limit),
>  	POWER_SUPPLY_ATTR(energy_full_design),
>  	POWER_SUPPLY_ATTR(energy_empty_design),
>  	POWER_SUPPLY_ATTR(energy_full),
> diff --git a/include/linux/power_supply.h 
> b/include/linux/power_supply.h index f80769175c56..608ba88e32ee 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -122,6 +122,7 @@ enum power_supply_property {
>  	POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT,
>  	POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
>  	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +	POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
>  	POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
>  	POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
>  	POWER_SUPPLY_PROP_ENERGY_FULL,
> --
> 2.19.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-11-21 22:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21 15:34 [PATCH 1/2] power: supply: add input voltage limit property Enric Balletbo i Serra
2018-11-21 15:34 ` [PATCH 2/2] power: supply: cros: allow to set input voltage and current limit Enric Balletbo i Serra
2018-11-21 19:03   ` Guenter Roeck
2018-11-21 19:00 ` [PATCH 1/2] power: supply: add input voltage limit property Guenter Roeck
2018-11-21 22:11 ` Adam Thomson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.