Re: [PATCH 5/5] platform/x86: msi-ec: Add a cooler boost attribute

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 10 Oct 2023, Nikita Kravets wrote:

> Cooler boost increases the fan speed to improve the laptop cooling.
> This is a simple on/off feature that is easy to test: if it works
> you'll hear the fans spinning much faster. So far all supported models
> have this feature represented by a single bit at the 0x98 EC address.
> The attribute makes use of the previously added bit operation functions.
> 
> Cc: Aakash Singh <mail@singhaakash.dev>
> Cc: Jose Angel Pastrana <japp0005@red.ujaen.es>
> Signed-off-by: Nikita Kravets <teackot@gmail.com>
> ---
>  drivers/platform/x86/msi-ec.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/platform/x86/msi-ec.c b/drivers/platform/x86/msi-ec.c
> index ae73dcf01d09..f4e770b3dda1 100644
> --- a/drivers/platform/x86/msi-ec.c
> +++ b/drivers/platform/x86/msi-ec.c
> @@ -26,6 +26,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/seq_file.h>
>  #include <linux/string.h>
> +#include <linux/kstrtox.h>
>  
>  #define SM_ECO_NAME		"eco"
>  #define SM_COMFORT_NAME		"comfort"
> @@ -850,6 +851,35 @@ static struct acpi_battery_hook battery_hook = {
>   * Sysfs platform device attributes
>   */
>  
> +static ssize_t cooler_boost_show(struct device *device,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	int result;
> +	bool value;
> +
> +	result = ec_check_bit(conf.cooler_boost.address, conf.cooler_boost.bit, &value);

Missing error handling.

> +	return sysfs_emit(buf, "%s\n", value ? "on" : "off");

str_on_off() from linux/string_choices.h.

> +}
> +
> +static ssize_t cooler_boost_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	int result;
> +	bool value;
> +
> +	result = kstrtobool(buf, &value);
> +	if (result)
> +		return result;
> +
> +	result = ec_set_bit(conf.cooler_boost.address, conf.cooler_boost.bit, value);
> +	if (result < 0)
> +		return result;
> +
> +	return count;
> +}
> +
>  static ssize_t fw_version_show(struct device *device,
>  			       struct device_attribute *attr, char *buf)
>  {
> @@ -897,6 +927,7 @@ static ssize_t fw_release_date_show(struct device *device,
>  			  hour, minute, second);
>  }
>  
> +static DEVICE_ATTR_RW(cooler_boost);
>  static DEVICE_ATTR_RO(fw_version);
>  static DEVICE_ATTR_RO(fw_release_date);
>  
> @@ -922,6 +953,10 @@ static void filter_attributes(struct attribute_support *attributes,
>  static int msi_platform_probe(struct platform_device *pdev)
>  {
>  	struct attribute_support root_attrs_support[] = {
> +		{
> +			&dev_attr_cooler_boost.attr,
> +			conf.cooler_boost.address != MSI_EC_ADDR_UNSUPP,
> +		},
>  		{
>  			&dev_attr_fw_version.attr,
>  			true,
> 

-- 
 i.