Re: [PATCH RFC v3 3/3] PCI/ASPM: add sysfs attribute for controlling ASPM

[Frederick Lawler <fred@fredlawl.com>]

Heiner,

Heiner Kallweit wrote on 5/12/19 8:54 AM:> [snip]
> +static ssize_t aspm_link_states_show(struct device *dev,
> +				     struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct pcie_link_state *link;
> +	int len = 0, i;
> +
> +	link = aspm_get_parent_link(pdev);
> +	if (!link)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&aspm_lock);
> +
> +	for (i = 0; i < ARRAY_SIZE(aspm_sysfs_states); i++) {
> +		const struct aspm_sysfs_state *st = aspm_sysfs_states + i;
> +
> +		if (link->aspm_enabled & st->mask)
> +			len += scnprintf(buf + len, PAGE_SIZE - len, "[%s] ",
> +					 st->name);
> +		else
> +			len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
> +					 st->name);
> +	}

   # echo "-L1" > aspm_link_states
   # cat aspm_link_states
   L0S L1 L1.1 L1.2 [CLKPM]

   # echo "+L1" > aspm_link_states
   # cat aspm_link_states
   L0S [L1] L1.1 L1.2 [CLKPM]

In v1 & v2 [STATE] was used to show that a state was disabled, now 
[STATE] is used to show enabled. Is that intended?

> +
> +	if (link->clkpm_enabled)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "[CLKPM] ");
> +	else
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "CLKPM ");
> +

Same as above...

> +	mutex_unlock(&aspm_lock);
> +
> +	len += scnprintf(buf + len, PAGE_SIZE - len, "\n");
> +
> +	return len;
> +}

Other than the above, v3 works as expected on my machine. Good work :)

Frederick Lawler