Re: [PATCH 1/4] platform/x86: (ayn-ec) Add PWM Fan HWMON Interface

[ALOK TIWARI <alok.a.tiwari@oracle.com>]

> +/*
> + * 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@gmail.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.

> +
> +	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