On Sat, Dec 14, 2019 at 12:48 AM Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
> When _FPS object indicates support of variable speed fan, thermal cooling
> devices for fan shows max performance state count using attribute
> "max_state" greater than or equal to 1.
>
> But the thermal cooling device doesn't display properties of each
> performance state. This is not enough for smart fan control user space
> software, which also considers speed, power and noise level.
>
> This change presents fan performance states attributes under acpi
> device for the fan. This will be under:
> /sys/bus/acpi/devices/devices/INT3404:00
> or
> /sys/bus/platform/devices/PNP0C0B:00.
>
> For example:
> $ ls /sys/bus/acpi/devices/INT3404\:00
> description path state0 state11 state4 state7 status
> hid physical_node state1 state2 state5 state8 subsystem
> modalias power state10 state3 state6 state9 uevent
> uid wakeup
>
> Here each state* attribute is representing a performance state.
>
> Contents of state* attribute are formatted using:
> control_percent:trip_point:speed_rpm:noise_level_mdb:power_mw
>
> $ cat /sys/bus/acpi/devices/INT3404\:00/state10
> 95:0:11600:47500:4500
>
> For more information refer to:
> Documentation/acpi/fan_performance_states.txt
>
> While here, return correct error from acpi_fan_probe() when
> acpi_fan_get_fps() or acpi_fan_get_fif() fails.
>
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
Folded the (rewritten) [1/2] into this one and queued it up for 5.6, thanks!
> ---
> drivers/acpi/fan.c | 96 ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 88 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index 816b0803f7fb..86d2417953b5 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -44,12 +44,16 @@ static const struct dev_pm_ops acpi_fan_pm = {
> #define FAN_PM_OPS_PTR NULL
> #endif
>
> +#define ACPI_FPS_NAME_LEN 20
> +
> struct acpi_fan_fps {
> u64 control;
> u64 trip_point;
> u64 speed;
> u64 noise_level;
> u64 power;
> + char name[ACPI_FPS_NAME_LEN];
> + struct device_attribute dev_attr;
> };
>
> struct acpi_fan_fif {
> @@ -265,6 +269,39 @@ static int acpi_fan_speed_cmp(const void *a, const void *b)
> return fps1->speed - fps2->speed;
> }
>
> +static ssize_t show_state(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct acpi_fan_fps *fps = container_of(attr, struct acpi_fan_fps, dev_attr);
> + int count;
> +
> + if (fps->control == 0xFFFFFFFF || fps->control > 100)
> + count = snprintf(buf, PAGE_SIZE, "not-defined:");
> + else
> + count = snprintf(buf, PAGE_SIZE, "%lld:", fps->control);
> +
> + if (fps->trip_point == 0xFFFFFFFF || fps->trip_point > 9)
> + count += snprintf(&buf[count], PAGE_SIZE, "not-defined:");
> + else
> + count += snprintf(&buf[count], PAGE_SIZE, "%lld:", fps->trip_point);
> +
> + if (fps->speed == 0xFFFFFFFF)
> + count += snprintf(&buf[count], PAGE_SIZE, "not-defined:");
> + else
> + count += snprintf(&buf[count], PAGE_SIZE, "%lld:", fps->speed);
> +
> + if (fps->noise_level == 0xFFFFFFFF)
> + count += snprintf(&buf[count], PAGE_SIZE, "not-defined:");
> + else
> + count += snprintf(&buf[count], PAGE_SIZE, "%lld:", fps->noise_level * 100);
> +
> + if (fps->power == 0xFFFFFFFF)
> + count += snprintf(&buf[count], PAGE_SIZE, "not-defined\n");
> + else
> + count += snprintf(&buf[count], PAGE_SIZE, "%lld\n", fps->power);
> +
> + return count;
> +}
> +
> static int acpi_fan_get_fps(struct acpi_device *device)
> {
> struct acpi_fan *fan = acpi_driver_data(device);
> @@ -295,12 +332,13 @@ static int acpi_fan_get_fps(struct acpi_device *device)
> }
> for (i = 0; i < fan->fps_count; i++) {
> struct acpi_buffer format = { sizeof("NNNNN"), "NNNNN" };
> - struct acpi_buffer fps = { sizeof(fan->fps[i]), &fan->fps[i] };
> + struct acpi_buffer fps = { offsetof(struct acpi_fan_fps, name),
> + &fan->fps[i] };
> status = acpi_extract_package(&obj->package.elements[i + 1],
> &format, &fps);
> if (ACPI_FAILURE(status)) {
> dev_err(&device->dev, "Invalid _FPS element\n");
> - break;
> + goto err;
> }
> }
>
> @@ -308,6 +346,24 @@ static int acpi_fan_get_fps(struct acpi_device *device)
> sort(fan->fps, fan->fps_count, sizeof(*fan->fps),
> acpi_fan_speed_cmp, NULL);
>
> + for (i = 0; i < fan->fps_count; ++i) {
> + struct acpi_fan_fps *fps = &fan->fps[i];
> +
> + snprintf(fps->name, ACPI_FPS_NAME_LEN, "state%d", i);
> + fps->dev_attr.show = show_state;
> + fps->dev_attr.store = NULL;
> + fps->dev_attr.attr.name = fps->name;
> + fps->dev_attr.attr.mode = 0444;
> + status = sysfs_create_file(&device->dev.kobj, &fps->dev_attr.attr);
> + if (status) {
> + int j;
> +
> + for (j = 0; j < i; ++j)
> + sysfs_remove_file(&device->dev.kobj, &fan->fps[j].dev_attr.attr);
> + break;
> + }
> + }
> +
> err:
> kfree(obj);
> return status;
> @@ -330,14 +386,20 @@ static int acpi_fan_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, fan);
>
> if (acpi_fan_is_acpi4(device)) {
> - if (acpi_fan_get_fif(device) || acpi_fan_get_fps(device))
> - goto end;
> + result = acpi_fan_get_fif(device);
> + if (result)
> + return result;
> +
> + result = acpi_fan_get_fps(device);
> + if (result)
> + return result;
> +
> fan->acpi4 = true;
> } else {
> result = acpi_device_update_power(device, NULL);
> if (result) {
> dev_err(&device->dev, "Failed to set initial power state\n");
> - goto end;
> + goto err_end;
> }
> }
>
> @@ -350,7 +412,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
> &fan_cooling_ops);
> if (IS_ERR(cdev)) {
> result = PTR_ERR(cdev);
> - goto end;
> + goto err_end;
> }
>
> dev_dbg(&pdev->dev, "registered as cooling_device%d\n", cdev->id);
> @@ -365,10 +427,21 @@ static int acpi_fan_probe(struct platform_device *pdev)
> result = sysfs_create_link(&cdev->device.kobj,
> &pdev->dev.kobj,
> "device");
> - if (result)
> + if (result) {
> dev_err(&pdev->dev, "Failed to create sysfs link 'device'\n");
> + goto err_end;
> + }
> +
> + return 0;
> +
> +err_end:
> + if (fan->acpi4) {
> + int i;
> +
> + for (i = 0; i < fan->fps_count; ++i)
> + sysfs_remove_file(&device->dev.kobj, &fan->fps[i].dev_attr.attr);
> + }
>
> -end:
> return result;
> }
>
> @@ -376,6 +449,13 @@ static int acpi_fan_remove(struct platform_device *pdev)
> {
> struct acpi_fan *fan = platform_get_drvdata(pdev);
>
> + if (fan->acpi4) {
> + struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> + int i;
> +
> + for (i = 0; i < fan->fps_count; ++i)
> + sysfs_remove_file(&device->dev.kobj, &fan->fps[i].dev_attr.attr);
> + }
> sysfs_remove_link(&pdev->dev.kobj, "thermal_cooling");
> sysfs_remove_link(&fan->cdev->device.kobj, "device");
> thermal_cooling_device_unregister(fan->cdev);
> --
> 2.17.2
>
