Re: [PATCH 2/4] toshiba_acpi: Add support for USB Sleep functions under battery

[Darren Hart]

On Wed, Jan 14, 2015 at 02:40:19PM -0700, Azael Avalos wrote:
> Toshiba laptops supporting USB Sleep and Charge also come with a
> feature called "USB functions under battery", which what it does when
> enabled, is allows the USB Sleep functions when the computer is under
> battery power.
> 
> This patch adds support to that function, creating a sysfs entry
> named "sleep_functions_on_battery", accepting values from 0-100,
> where zero disables the function and 1-100 sets the battery level at
> which point the USB Sleep functions will be disabled, and printing
> the current state of the functon and also the battery level currently
> set.
> 
> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx>
> ---
>  drivers/platform/x86/toshiba_acpi.c | 133 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 133 insertions(+)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index b03129d..9e054c5 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -151,6 +151,10 @@ MODULE_LICENSE("GPL");
>  #define SCI_USB_CHARGE_DISABLED		0x30000
>  #define SCI_USB_CHARGE_ALTERNATE	0x30009
>  #define SCI_USB_CHARGE_AUTO		0x30021
> +#define SCI_USB_CHARGE_BAT_MASK		0x7
> +#define SCI_USB_CHARGE_BAT_LVL_OFF	0x1
> +#define SCI_USB_CHARGE_BAT_LVL_ON	0x4
> +#define SCI_USB_CHARGE_BAT_LVL		0x0200
>  
>  struct toshiba_acpi_dev {
>  	struct acpi_device *acpi_dev;
> @@ -169,6 +173,7 @@ struct toshiba_acpi_dev {
>  	int kbd_type;
>  	int kbd_mode;
>  	int kbd_time;
> +	int usbsc_bat_level;
>  
>  	unsigned int illumination_supported:1;
>  	unsigned int video_supported:1;
> @@ -814,6 +819,61 @@ static int toshiba_usb_sleep_charge_set(struct toshiba_acpi_dev *dev,
>  	return 0;
>  }
>  
> +static int toshiba_sleep_functions_status_get(struct toshiba_acpi_dev *dev,
> +					      u32 *mode)
> +{
> +	u32 in[TCI_WORDS] = { SCI_GET, SCI_USB_SLEEP_CHARGE, 0, 0, 0, 0 };
> +	u32 out[TCI_WORDS];
> +	acpi_status status;
> +
> +	if (!sci_open(dev))
> +		return -EIO;
> +
> +	in[5] = SCI_USB_CHARGE_BAT_LVL;
> +	status = tci_raw(dev, in, out);
> +	sci_close(dev);
> +	if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
> +		pr_err("ACPI call to get USB S&C battery level failed\n");
> +		return -EIO;
> +	} else if (out[0] == TOS_NOT_SUPPORTED) {
> +		pr_info("USB Sleep and Charge not supported\n");
> +		return -ENODEV;
> +	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
> +		return -EIO;
> +	}
> +
> +	*mode = out[2];
> +
> +	return 0;
> +}
> +
> +static int toshiba_sleep_functions_status_set(struct toshiba_acpi_dev *dev,
> +					      u32 mode)
> +{
> +	u32 in[TCI_WORDS] = { SCI_SET, SCI_USB_SLEEP_CHARGE, 0, 0, 0, 0 };
> +	u32 out[TCI_WORDS];
> +	acpi_status status;
> +
> +	if (!sci_open(dev))
> +		return -EIO;
> +
> +	in[2] = mode;
> +	in[5] = SCI_USB_CHARGE_BAT_LVL;
> +	status = tci_raw(dev, in, out);
> +	sci_close(dev);
> +	if (ACPI_FAILURE(status) || out[0] == TOS_FAILURE) {
> +		pr_err("ACPI call to set USB S&C battery level failed\n");
> +		return -EIO;
> +	} else if (out[0] == TOS_NOT_SUPPORTED) {
> +		pr_info("USB Sleep and Charge not supported\n");
> +		return -ENODEV;
> +	} else if (out[0] == TOS_INPUT_DATA_ERROR) {
> +		return -EIO;
> +	}
> +

See comments to 1/4 regarding error codes and input validation.

> +	return 0;
> +}
> +
>  /* Bluetooth rfkill handlers */
>  
>  static u32 hci_get_bt_present(struct toshiba_acpi_dev *dev, bool *present)
> @@ -1373,6 +1433,12 @@ static ssize_t toshiba_usb_sleep_charge_show(struct device *dev,
>  static ssize_t toshiba_usb_sleep_charge_store(struct device *dev,
>  					      struct device_attribute *attr,
>  					      const char *buf, size_t count);
> +static ssize_t sleep_functions_on_battery_show(struct device *dev,
> +					       struct device_attribute *attr,
> +					       char *buf);
> +static ssize_t sleep_functions_on_battery_store(struct device *dev,
> +						struct device_attribute *attr,
> +						const char *buf, size_t count);
>  
>  static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
>  		   toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
> @@ -1387,6 +1453,9 @@ static DEVICE_ATTR(position, S_IRUGO, toshiba_position_show, NULL);
>  static DEVICE_ATTR(usb_sleep_charge, S_IRUGO | S_IWUSR,
>  		   toshiba_usb_sleep_charge_show,
>  		   toshiba_usb_sleep_charge_store);
> +static DEVICE_ATTR(sleep_functions_on_battery, S_IRUGO | S_IWUSR,
> +		   sleep_functions_on_battery_show,
> +		   sleep_functions_on_battery_store);
>  
>  static struct attribute *toshiba_attributes[] = {
>  	&dev_attr_kbd_backlight_mode.attr,
> @@ -1396,6 +1465,7 @@ static struct attribute *toshiba_attributes[] = {
>  	&dev_attr_touchpad.attr,
>  	&dev_attr_position.attr,
>  	&dev_attr_usb_sleep_charge.attr,
> +	&dev_attr_sleep_functions_on_battery.attr,
>  	NULL,
>  };
>  
> @@ -1657,6 +1727,67 @@ static ssize_t toshiba_usb_sleep_charge_store(struct device *dev,
>  	return count;
>  }
>  
> +static ssize_t sleep_functions_on_battery_show(struct device *dev,
> +					       struct device_attribute *attr,
> +					       char *buf)
> +{
> +	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
> +	u32 state;
> +	int ret;
> +	int tmp;
> +	int status;
> +	int bat_lvl;

Order declarations in decreasing line length please.

int bat_lvl;
int status;
int state;
int tmp;
int ret;

> +
> +	ret = toshiba_sleep_functions_status_get(toshiba, &state);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Determine the status: 0x4 - Enabled | 0x1 - Disabled */
> +	tmp = state & SCI_USB_CHARGE_BAT_MASK;
> +	status = (tmp == 0x4) ? 1 : 0;
> +	/* Determine the battery level set */
> +	bat_lvl = state >> HCI_MISC_SHIFT;
> +
> +	return sprintf(buf, "%d %d\n", status, bat_lvl);
> +}
> +
> +static ssize_t sleep_functions_on_battery_store(struct device *dev,
> +						struct device_attribute *attr,
> +						const char *buf, size_t count)
> +{
> +	struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
> +	u32 status;
> +	int value;
> +	int ret;
> +	int tmp;
> +

Yeah, like that ;-)

> +	ret = kstrtoint(buf, 0, &value);
> +	if (ret)
> +		return ret;
> +
> +	/* Set the status of the function:
> +	 * 0 - Disabled
> +	 * 1-100 - Enabled
> +	 */
> +	if (value < 0 || value > 100)
> +		return -EINVAL;
> +

Ah, oops, you catch the input here. Good.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/