Re: [PATCH resend 2/6] toshiba_acpi: Add fan entry to sysfs

2015-02-09 Thread Azael Avalos 
Hi Darren,

2015-02-09 21:02 GMT-07:00 Darren Hart :
> On Mon, Feb 09, 2015 at 08:34:50PM -0700, Azael Avalos wrote:
>> This patch adds a fan entry to sysfs, enabling the user to get and
>> set the fan status.
>>
>
> Hi Azael,
>
> I was finally getting around to these when you resent them. Apologies for the
> delay. Travel and still fighting a cold/flu/bug. Sigh. Anyway... on to patch
> review :-)

Sorry for the rushing of the patches, hope you're better now, in case you're
still in recovery, take my motto "la cerveza cura todo" into account XD

>
>> Signed-off-by: Azael Avalos 
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 51 
>> -
>>  1 file changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c 
>> b/drivers/platform/x86/toshiba_acpi.c
>> index 334b65e..413af60 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -1516,6 +1516,11 @@ static const struct backlight_ops 
>> toshiba_backlight_data = {
>>   */
>>  static ssize_t toshiba_version_show(struct device *dev,
>>   struct device_attribute *attr, char *buf);
>> +static ssize_t toshiba_fan_store(struct device *dev,
>> +  struct device_attribute *attr,
>> +  const char *buf, size_t count);
>> +static ssize_t toshiba_fan_show(struct device *dev,
>> + struct device_attribute *attr, char *buf);
>>  static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
>>struct device_attribute *attr,
>>const char *buf, size_t count);
>> @@ -1569,6 +1574,8 @@ static ssize_t toshiba_usb_sleep_music_store(struct 
>> device *dev,
>>const char *buf, size_t count);
>>
>>  static DEVICE_ATTR(version, S_IRUGO, toshiba_version_show, NULL);
>> +static DEVICE_ATTR(fan, S_IRUGO | S_IWUSR,
>> +toshiba_fan_show, toshiba_fan_store);
>>  static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
>>  toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
>>  static DEVICE_ATTR(kbd_type, S_IRUGO, toshiba_kbd_type_show, NULL);
>
> At some point, before we add too much more, it would be nice to convert these
> over to DEVICE_ATTR_RW and DEVICE_ATTR_RO. Any reason not to do this sooner
> rather than later?

I have patches ready for all the functions in sysfs regarding this change,
I just wanted the patches to land in you tree (or next) to send them for 3.21,
I'll be in janitor mode after these patches, cleaning the driver according to
coding style, add files to Documentation/ABI, etc..

>
>> @@ -1594,6 +1601,7 @@ static DEVICE_ATTR(usb_sleep_music, S_IRUGO | S_IWUSR,
>>
>>  static struct attribute *toshiba_attributes[] = {
>>   _attr_version.attr,
>> + _attr_fan.attr,
>>   _attr_kbd_backlight_mode.attr,
>>   _attr_kbd_type.attr,
>>   _attr_available_kbd_modes.attr,
>> @@ -1621,6 +1629,45 @@ static ssize_t toshiba_version_show(struct device 
>> *dev,
>>   return sprintf(buf, "%s\n", TOSHIBA_ACPI_VERSION);
>>  }
>>
>> +static ssize_t toshiba_fan_store(struct device *dev,
>> +  struct device_attribute *attr,
>> +  const char *buf, size_t count)
>> +{
>> + struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
>> + u32 result;
>> + int state;
>> + int ret;
>> +
>> + ret = kstrtoint(buf, 0, );
>> + if (ret)
>> + return ret;
>> +
>> + if (state != 0 && state != 1)
>> + return -EINVAL;
>> +
>> + result = hci_write1(toshiba, HCI_FAN, state);
>> + if (result == TOS_FAILURE)
>> + return -EIO;
>> + else if (result == TOS_NOT_SUPPORTED)
>> + return -ENODEV;
>> +
>
> A quick scan of hci_write1 makes me wonder if there are more than two possible
> failures. Should we also have an "else if (result)" or "else if (result !=
> WHATEVER_SUCCESS_IS)" before we assume success and return count?

Can't really tell you, I just ported the code "as-is" from the procfs files,
and to be honest, I haven't seen this function in any recent Toshiba
laptop out there,
I just wanted to port this in case some old laptop/app/script/etc. is
still using this.

>
> --
> Darren Hart
> Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH resend 2/6] toshiba_acpi: Add fan entry to sysfs

2015-02-09 Thread Darren Hart 
On Mon, Feb 09, 2015 at 08:34:50PM -0700, Azael Avalos wrote:
> This patch adds a fan entry to sysfs, enabling the user to get and
> set the fan status.
> 

Hi Azael,

I was finally getting around to these when you resent them. Apologies for the
delay. Travel and still fighting a cold/flu/bug. Sigh. Anyway... on to patch
review :-)

> Signed-off-by: Azael Avalos 
> ---
>  drivers/platform/x86/toshiba_acpi.c | 51 
> -
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c 
> b/drivers/platform/x86/toshiba_acpi.c
> index 334b65e..413af60 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1516,6 +1516,11 @@ static const struct backlight_ops 
> toshiba_backlight_data = {
>   */
>  static ssize_t toshiba_version_show(struct device *dev,
>   struct device_attribute *attr, char *buf);
> +static ssize_t toshiba_fan_store(struct device *dev,
> +  struct device_attribute *attr,
> +  const char *buf, size_t count);
> +static ssize_t toshiba_fan_show(struct device *dev,
> + struct device_attribute *attr, char *buf);
>  static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
>struct device_attribute *attr,
>const char *buf, size_t count);
> @@ -1569,6 +1574,8 @@ static ssize_t toshiba_usb_sleep_music_store(struct 
> device *dev,
>const char *buf, size_t count);
>  
>  static DEVICE_ATTR(version, S_IRUGO, toshiba_version_show, NULL);
> +static DEVICE_ATTR(fan, S_IRUGO | S_IWUSR,
> +toshiba_fan_show, toshiba_fan_store);
>  static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
>  toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
>  static DEVICE_ATTR(kbd_type, S_IRUGO, toshiba_kbd_type_show, NULL);

At some point, before we add too much more, it would be nice to convert these
over to DEVICE_ATTR_RW and DEVICE_ATTR_RO. Any reason not to do this sooner
rather than later?

> @@ -1594,6 +1601,7 @@ static DEVICE_ATTR(usb_sleep_music, S_IRUGO | S_IWUSR,
>  
>  static struct attribute *toshiba_attributes[] = {
>   _attr_version.attr,
> + _attr_fan.attr,
>   _attr_kbd_backlight_mode.attr,
>   _attr_kbd_type.attr,
>   _attr_available_kbd_modes.attr,
> @@ -1621,6 +1629,45 @@ static ssize_t toshiba_version_show(struct device *dev,
>   return sprintf(buf, "%s\n", TOSHIBA_ACPI_VERSION);
>  }
>  
> +static ssize_t toshiba_fan_store(struct device *dev,
> +  struct device_attribute *attr,
> +  const char *buf, size_t count)
> +{
> + struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
> + u32 result;
> + int state;
> + int ret;
> +
> + ret = kstrtoint(buf, 0, );
> + if (ret)
> + return ret;
> +
> + if (state != 0 && state != 1)
> + return -EINVAL;
> +
> + result = hci_write1(toshiba, HCI_FAN, state);
> + if (result == TOS_FAILURE)
> + return -EIO;
> + else if (result == TOS_NOT_SUPPORTED)
> + return -ENODEV;
> +

A quick scan of hci_write1 makes me wonder if there are more than two possible
failures. Should we also have an "else if (result)" or "else if (result !=
WHATEVER_SUCCESS_IS)" before we assume success and return count?

-- 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH resend 2/6] toshiba_acpi: Add fan entry to sysfs

2015-02-09 Thread Azael Avalos 
This patch adds a fan entry to sysfs, enabling the user to get and
set the fan status.

Signed-off-by: Azael Avalos 
---
 drivers/platform/x86/toshiba_acpi.c | 51 -
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c 
b/drivers/platform/x86/toshiba_acpi.c
index 334b65e..413af60 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1516,6 +1516,11 @@ static const struct backlight_ops toshiba_backlight_data 
= {
  */
 static ssize_t toshiba_version_show(struct device *dev,
struct device_attribute *attr, char *buf);
+static ssize_t toshiba_fan_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count);
+static ssize_t toshiba_fan_show(struct device *dev,
+   struct device_attribute *attr, char *buf);
 static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
 struct device_attribute *attr,
 const char *buf, size_t count);
@@ -1569,6 +1574,8 @@ static ssize_t toshiba_usb_sleep_music_store(struct 
device *dev,
 const char *buf, size_t count);
 
 static DEVICE_ATTR(version, S_IRUGO, toshiba_version_show, NULL);
+static DEVICE_ATTR(fan, S_IRUGO | S_IWUSR,
+  toshiba_fan_show, toshiba_fan_store);
 static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
   toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
 static DEVICE_ATTR(kbd_type, S_IRUGO, toshiba_kbd_type_show, NULL);
@@ -1594,6 +1601,7 @@ static DEVICE_ATTR(usb_sleep_music, S_IRUGO | S_IWUSR,
 
 static struct attribute *toshiba_attributes[] = {
_attr_version.attr,
+   _attr_fan.attr,
_attr_kbd_backlight_mode.attr,
_attr_kbd_type.attr,
_attr_available_kbd_modes.attr,
@@ -1621,6 +1629,45 @@ static ssize_t toshiba_version_show(struct device *dev,
return sprintf(buf, "%s\n", TOSHIBA_ACPI_VERSION);
 }
 
+static ssize_t toshiba_fan_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
+   u32 result;
+   int state;
+   int ret;
+
+   ret = kstrtoint(buf, 0, );
+   if (ret)
+   return ret;
+
+   if (state != 0 && state != 1)
+   return -EINVAL;
+
+   result = hci_write1(toshiba, HCI_FAN, state);
+   if (result == TOS_FAILURE)
+   return -EIO;
+   else if (result == TOS_NOT_SUPPORTED)
+   return -ENODEV;
+
+   return count;
+}
+
+static ssize_t toshiba_fan_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
+   u32 value;
+   int ret;
+
+   ret = get_fan_status(toshiba, );
+   if (ret)
+   return ret;
+
+   return sprintf(buf, "%d\n", value);
+}
+
 static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
 struct device_attribute *attr,
 const char *buf, size_t count)
@@ -2017,7 +2064,9 @@ static umode_t toshiba_sysfs_is_visible(struct kobject 
*kobj,
struct toshiba_acpi_dev *drv = dev_get_drvdata(dev);
bool exists = true;
 
-   if (attr == _attr_kbd_backlight_mode.attr)
+   if (attr == _attr_fan.attr)
+   exists = (drv->fan_supported) ? true : false;
+   else if (attr == _attr_kbd_backlight_mode.attr)
exists = (drv->kbd_illum_supported) ? true : false;
else if (attr == _attr_kbd_backlight_timeout.attr)
exists = (drv->kbd_mode == SCI_KBD_MODE_AUTO) ? true : false;
-- 
2.2.2

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

Re: [PATCH resend 2/6] toshiba_acpi: Add fan entry to sysfs

2015-02-09 Thread Darren Hart 
On Mon, Feb 09, 2015 at 08:34:50PM -0700, Azael Avalos wrote:
 This patch adds a fan entry to sysfs, enabling the user to get and
 set the fan status.
 

Hi Azael,

I was finally getting around to these when you resent them. Apologies for the
delay. Travel and still fighting a cold/flu/bug. Sigh. Anyway... on to patch
review :-)

 Signed-off-by: Azael Avalos coproscef...@gmail.com
 ---
  drivers/platform/x86/toshiba_acpi.c | 51 
 -
  1 file changed, 50 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/platform/x86/toshiba_acpi.c 
 b/drivers/platform/x86/toshiba_acpi.c
 index 334b65e..413af60 100644
 --- a/drivers/platform/x86/toshiba_acpi.c
 +++ b/drivers/platform/x86/toshiba_acpi.c
 @@ -1516,6 +1516,11 @@ static const struct backlight_ops 
 toshiba_backlight_data = {
   */
  static ssize_t toshiba_version_show(struct device *dev,
   struct device_attribute *attr, char *buf);
 +static ssize_t toshiba_fan_store(struct device *dev,
 +  struct device_attribute *attr,
 +  const char *buf, size_t count);
 +static ssize_t toshiba_fan_show(struct device *dev,
 + struct device_attribute *attr, char *buf);
  static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count);
 @@ -1569,6 +1574,8 @@ static ssize_t toshiba_usb_sleep_music_store(struct 
 device *dev,
const char *buf, size_t count);
  
  static DEVICE_ATTR(version, S_IRUGO, toshiba_version_show, NULL);
 +static DEVICE_ATTR(fan, S_IRUGO | S_IWUSR,
 +toshiba_fan_show, toshiba_fan_store);
  static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
  toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
  static DEVICE_ATTR(kbd_type, S_IRUGO, toshiba_kbd_type_show, NULL);

At some point, before we add too much more, it would be nice to convert these
over to DEVICE_ATTR_RW and DEVICE_ATTR_RO. Any reason not to do this sooner
rather than later?

 @@ -1594,6 +1601,7 @@ static DEVICE_ATTR(usb_sleep_music, S_IRUGO | S_IWUSR,
  
  static struct attribute *toshiba_attributes[] = {
   dev_attr_version.attr,
 + dev_attr_fan.attr,
   dev_attr_kbd_backlight_mode.attr,
   dev_attr_kbd_type.attr,
   dev_attr_available_kbd_modes.attr,
 @@ -1621,6 +1629,45 @@ static ssize_t toshiba_version_show(struct device *dev,
   return sprintf(buf, %s\n, TOSHIBA_ACPI_VERSION);
  }
  
 +static ssize_t toshiba_fan_store(struct device *dev,
 +  struct device_attribute *attr,
 +  const char *buf, size_t count)
 +{
 + struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
 + u32 result;
 + int state;
 + int ret;
 +
 + ret = kstrtoint(buf, 0, state);
 + if (ret)
 + return ret;
 +
 + if (state != 0  state != 1)
 + return -EINVAL;
 +
 + result = hci_write1(toshiba, HCI_FAN, state);
 + if (result == TOS_FAILURE)
 + return -EIO;
 + else if (result == TOS_NOT_SUPPORTED)
 + return -ENODEV;
 +

A quick scan of hci_write1 makes me wonder if there are more than two possible
failures. Should we also have an else if (result) or else if (result !=
WHATEVER_SUCCESS_IS) before we assume success and return count?

-- 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH resend 2/6] toshiba_acpi: Add fan entry to sysfs

2015-02-09 Thread Azael Avalos 
This patch adds a fan entry to sysfs, enabling the user to get and
set the fan status.

Signed-off-by: Azael Avalos coproscef...@gmail.com
---
 drivers/platform/x86/toshiba_acpi.c | 51 -
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c 
b/drivers/platform/x86/toshiba_acpi.c
index 334b65e..413af60 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -1516,6 +1516,11 @@ static const struct backlight_ops toshiba_backlight_data 
= {
  */
 static ssize_t toshiba_version_show(struct device *dev,
struct device_attribute *attr, char *buf);
+static ssize_t toshiba_fan_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count);
+static ssize_t toshiba_fan_show(struct device *dev,
+   struct device_attribute *attr, char *buf);
 static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
 struct device_attribute *attr,
 const char *buf, size_t count);
@@ -1569,6 +1574,8 @@ static ssize_t toshiba_usb_sleep_music_store(struct 
device *dev,
 const char *buf, size_t count);
 
 static DEVICE_ATTR(version, S_IRUGO, toshiba_version_show, NULL);
+static DEVICE_ATTR(fan, S_IRUGO | S_IWUSR,
+  toshiba_fan_show, toshiba_fan_store);
 static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
   toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
 static DEVICE_ATTR(kbd_type, S_IRUGO, toshiba_kbd_type_show, NULL);
@@ -1594,6 +1601,7 @@ static DEVICE_ATTR(usb_sleep_music, S_IRUGO | S_IWUSR,
 
 static struct attribute *toshiba_attributes[] = {
dev_attr_version.attr,
+   dev_attr_fan.attr,
dev_attr_kbd_backlight_mode.attr,
dev_attr_kbd_type.attr,
dev_attr_available_kbd_modes.attr,
@@ -1621,6 +1629,45 @@ static ssize_t toshiba_version_show(struct device *dev,
return sprintf(buf, %s\n, TOSHIBA_ACPI_VERSION);
 }
 
+static ssize_t toshiba_fan_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
+   u32 result;
+   int state;
+   int ret;
+
+   ret = kstrtoint(buf, 0, state);
+   if (ret)
+   return ret;
+
+   if (state != 0  state != 1)
+   return -EINVAL;
+
+   result = hci_write1(toshiba, HCI_FAN, state);
+   if (result == TOS_FAILURE)
+   return -EIO;
+   else if (result == TOS_NOT_SUPPORTED)
+   return -ENODEV;
+
+   return count;
+}
+
+static ssize_t toshiba_fan_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
+   u32 value;
+   int ret;
+
+   ret = get_fan_status(toshiba, value);
+   if (ret)
+   return ret;
+
+   return sprintf(buf, %d\n, value);
+}
+
 static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
 struct device_attribute *attr,
 const char *buf, size_t count)
@@ -2017,7 +2064,9 @@ static umode_t toshiba_sysfs_is_visible(struct kobject 
*kobj,
struct toshiba_acpi_dev *drv = dev_get_drvdata(dev);
bool exists = true;
 
-   if (attr == dev_attr_kbd_backlight_mode.attr)
+   if (attr == dev_attr_fan.attr)
+   exists = (drv-fan_supported) ? true : false;
+   else if (attr == dev_attr_kbd_backlight_mode.attr)
exists = (drv-kbd_illum_supported) ? true : false;
else if (attr == dev_attr_kbd_backlight_timeout.attr)
exists = (drv-kbd_mode == SCI_KBD_MODE_AUTO) ? true : false;
-- 
2.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH resend 2/6] toshiba_acpi: Add fan entry to sysfs

2015-02-09 Thread Azael Avalos 
Hi Darren,

2015-02-09 21:02 GMT-07:00 Darren Hart dvh...@infradead.org:
 On Mon, Feb 09, 2015 at 08:34:50PM -0700, Azael Avalos wrote:
 This patch adds a fan entry to sysfs, enabling the user to get and
 set the fan status.


 Hi Azael,

 I was finally getting around to these when you resent them. Apologies for the
 delay. Travel and still fighting a cold/flu/bug. Sigh. Anyway... on to patch
 review :-)

Sorry for the rushing of the patches, hope you're better now, in case you're
still in recovery, take my motto la cerveza cura todo into account XD


 Signed-off-by: Azael Avalos coproscef...@gmail.com
 ---
  drivers/platform/x86/toshiba_acpi.c | 51 
 -
  1 file changed, 50 insertions(+), 1 deletion(-)

 diff --git a/drivers/platform/x86/toshiba_acpi.c 
 b/drivers/platform/x86/toshiba_acpi.c
 index 334b65e..413af60 100644
 --- a/drivers/platform/x86/toshiba_acpi.c
 +++ b/drivers/platform/x86/toshiba_acpi.c
 @@ -1516,6 +1516,11 @@ static const struct backlight_ops 
 toshiba_backlight_data = {
   */
  static ssize_t toshiba_version_show(struct device *dev,
   struct device_attribute *attr, char *buf);
 +static ssize_t toshiba_fan_store(struct device *dev,
 +  struct device_attribute *attr,
 +  const char *buf, size_t count);
 +static ssize_t toshiba_fan_show(struct device *dev,
 + struct device_attribute *attr, char *buf);
  static ssize_t toshiba_kbd_bl_mode_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count);
 @@ -1569,6 +1574,8 @@ static ssize_t toshiba_usb_sleep_music_store(struct 
 device *dev,
const char *buf, size_t count);

  static DEVICE_ATTR(version, S_IRUGO, toshiba_version_show, NULL);
 +static DEVICE_ATTR(fan, S_IRUGO | S_IWUSR,
 +toshiba_fan_show, toshiba_fan_store);
  static DEVICE_ATTR(kbd_backlight_mode, S_IRUGO | S_IWUSR,
  toshiba_kbd_bl_mode_show, toshiba_kbd_bl_mode_store);
  static DEVICE_ATTR(kbd_type, S_IRUGO, toshiba_kbd_type_show, NULL);

 At some point, before we add too much more, it would be nice to convert these
 over to DEVICE_ATTR_RW and DEVICE_ATTR_RO. Any reason not to do this sooner
 rather than later?

I have patches ready for all the functions in sysfs regarding this change,
I just wanted the patches to land in you tree (or next) to send them for 3.21,
I'll be in janitor mode after these patches, cleaning the driver according to
coding style, add files to Documentation/ABI, etc..


 @@ -1594,6 +1601,7 @@ static DEVICE_ATTR(usb_sleep_music, S_IRUGO | S_IWUSR,

  static struct attribute *toshiba_attributes[] = {
   dev_attr_version.attr,
 + dev_attr_fan.attr,
   dev_attr_kbd_backlight_mode.attr,
   dev_attr_kbd_type.attr,
   dev_attr_available_kbd_modes.attr,
 @@ -1621,6 +1629,45 @@ static ssize_t toshiba_version_show(struct device 
 *dev,
   return sprintf(buf, %s\n, TOSHIBA_ACPI_VERSION);
  }

 +static ssize_t toshiba_fan_store(struct device *dev,
 +  struct device_attribute *attr,
 +  const char *buf, size_t count)
 +{
 + struct toshiba_acpi_dev *toshiba = dev_get_drvdata(dev);
 + u32 result;
 + int state;
 + int ret;
 +
 + ret = kstrtoint(buf, 0, state);
 + if (ret)
 + return ret;
 +
 + if (state != 0  state != 1)
 + return -EINVAL;
 +
 + result = hci_write1(toshiba, HCI_FAN, state);
 + if (result == TOS_FAILURE)
 + return -EIO;
 + else if (result == TOS_NOT_SUPPORTED)
 + return -ENODEV;
 +

 A quick scan of hci_write1 makes me wonder if there are more than two possible
 failures. Should we also have an else if (result) or else if (result !=
 WHATEVER_SUCCESS_IS) before we assume success and return count?

Can't really tell you, I just ported the code as-is from the procfs files,
and to be honest, I haven't seen this function in any recent Toshiba
laptop out there,
I just wanted to port this in case some old laptop/app/script/etc. is
still using this.


 --
 Darren Hart
 Intel Open Source Technology Center


Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/