| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <C7073C0E-3D58-41C3-99B7-A0A5EE448700@gmail.com>
Date: Fri, 25 Jul 2025 19:48:28 -0700
From: "Derek J. Clark" <derekjohn.clark@...il.com>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Hans de Goede <hansg@...nel.org>
CC: Jean Delvare <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hwmon@...r.kernel.org
Subject: Re: [PATCH 1/4] platform/x86: (ayn-ec) Add PWM Fan HWMON Interface
On July 25, 2025 8:08:42 AM PDT, ALOK TIWARI <alok.a.tiwari@...cle.com> wrote:
>
>> +/*
>> + * Platform driver for Ayn x86 Handhelds.
>> + *
>> + * Implements multiple attributes provided by the EC. Fan reading and control,
>> + * as well as temperature sensor readings are exposed via hwmon sysfs. EC RGB
>> + * control is exposed via an led-class-multicolor interface.
>> + *
>> + * Fan control is provided via a pwm interface in the range [0-255]. Ayn use
>> + * [0-128] as the range in the EC, the written value is scaled to accommodate.
>> + * The EC also provides a configurable fan curve with five set points that
>> + * associate a temperature in Celcius [0-100] with a fan speed [0-128]. The
>> + * auto_point fan speeds are also scaled from the range [0-255]. Temperature
>> + * readings are scaled from degrees to millidegrees when read.
>> + *
>> + * RGB control is provided using 4 registers. One each for the colors red,
>> + * green, and blue are [0-255]. There is also a effect register that takes
>> + * switches between an EC controlled breathing that cycles through all colors
>> + * and fades in/out, and manual, which enables setting a user defined color.
>> + *
>> + * Copyright (C) 2025 Derek J. Clark <derekjohn.clark@...il.com>
>> + */
>> +
>[clip]
>> + if (val < 0 || val > 255)
>> + return -EINVAL;
>> + val = val >> 1; /* Max EC value is 128, scale from 255 */
>> + break;
>> + case 5:
>> + case 6:
>> + case 7:
>> + case 8:
>> + case 9:
>> + if (val < 0 || val > 100)
>> + return -EINVAL;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = write_to_ec(reg, val);
>> + if (ret)
>> + return ret;
>> + return count;
>> +}
>> +
>> +/**
>> + * pwm_curve_show() - Read a fan curve speed or temperature value.
>> + *
>> + * @dev: The attribute's parent device.
>> + * @attr: The attribute to read.
>> + * @buf: Buffer to read to.
>
>"to read to" is awkward
>Output buffer.
>
>> + *
>> + * Return: Number of bytes read, or an error.
>> + */
>> +static ssize_t pwm_curve_show(struct device *dev, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + int i, ret;
>> + long val;
>> + u8 reg;
>> +
>> + i = to_sensor_dev_attr(attr)->index;
>> + switch (i) {
>> + case 0:
>> + reg = AYN_SENSOR_PWM_FAN_SPEED_1_REG;
>> + break;
>> + case 1:
>> + reg = AYN_SENSOR_PWM_FAN_SPEED_2_REG;
>> + break;
>> + case 2:
>> + reg = AYN_SENSOR_PWM_FAN_SPEED_3_REG;
>> + break;
>> + case 3:
>> + reg = AYN_SENSOR_PWM_FAN_SPEED_4_REG;
>> + break;
>> + case 4:
>> + reg = AYN_SENSOR_PWM_FAN_SPEED_5_REG;
>> + break;
>> + case 5:
>> + reg = AYN_SENSOR_PWM_FAN_TEMP_1_REG;
>> + break;
>> + case 6:
>> + reg = AYN_SENSOR_PWM_FAN_TEMP_2_REG;
>> + break;
>> + case 7:
>> + reg = AYN_SENSOR_PWM_FAN_TEMP_3_REG;
>> + break;
>> + case 8:
>> + reg = AYN_SENSOR_PWM_FAN_TEMP_4_REG;
>> + break;
>> + case 9:
>> + reg = AYN_SENSOR_PWM_FAN_TEMP_5_REG;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = read_from_ec(reg, 1, &val);
>> + if (ret)
>> + return ret;
>> +
>> + switch (i) {
>> + case 0:
>> + case 1:
>> + case 2:
>> + case 3:
>> + case 4:
>> + val = val << 1; /* Max EC value is 128, scale to 255 */
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + return sysfs_emit(buf, "%ld\n", val);
>> +}
>> +
>> +/* Fan curve attributes */
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point1_pwm, pwm_curve, 0);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point2_pwm, pwm_curve, 1);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point3_pwm, pwm_curve, 2);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point4_pwm, pwm_curve, 3);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point5_pwm, pwm_curve, 4);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point1_temp, pwm_curve, 5);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point2_temp, pwm_curve, 6);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point3_temp, pwm_curve, 7);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point4_temp, pwm_curve, 8);
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_auto_point5_temp, pwm_curve, 9);
>> +
>> +static struct attribute *ayn_sensors_attrs[] = {
>> + &sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point1_temp.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point2_pwm.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point2_temp.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point3_pwm.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point3_temp.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point4_pwm.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point4_temp.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point5_pwm.dev_attr.attr,
>> + &sensor_dev_attr_pwm1_auto_point5_temp.dev_attr.attr,
>> + NULL,
>> +};
>> +
>> +ATTRIBUTE_GROUPS(ayn_sensors);
>> +
>> +static int ayn_ec_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device *hwdev;
>> + int ret;
>
>ret is unused.
>
Woops, that should have been part of patch 3.
Ack all for 1/4 and 2/4 reviews. Thanks.
Derek
>> +
>> + hwdev = devm_hwmon_device_register_with_info(dev, "aynec", NULL,
>> + &ayn_ec_chip_info,
>> + ayn_sensors_groups);
>> + return PTR_ERR_OR_ZERO(hwdev);
>> +}
>> +
>> +static struct platform_driver ayn_ec_driver = {
>> + .driver = {
>> + .name = "ayn-ec",
>> + },
>> + .probe = ayn_ec_probe,
>> +};
>
>Thanks,
>Alok
- Derek
Powered by blists - more mailing lists