Hello,
A few small comments.
> +/*
> + * Ratelimit enable toggle
> + * 0: disabled with ratelimit.interval = 0
> + * 1: enabled with ratelimit.interval = nonzero
> + */
Move the above comment so it hugs ratelimit_log_enable_store(). Also,
perhaps "Ratelimit enable toggle:", while at it, and feel free to indent
both choices for readability.
That sad, I am not sure if we really need to explain here how this
particular store() function works.
> +static ssize_t ratelimit_log_enable_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + bool enable = pdev->aer_report->cor_log_ratelimit.interval != 0;
> +
> + return sysfs_emit(buf, "%d\n", enable);
> +}
Perhaps "status" or "enabled" for the variable name above.
> +static ssize_t ratelimit_log_enable_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + bool enable;
> + int interval;
I would love if we could add the following here:
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
To ensure that only privileged user also sporting a proper set capabilities
can modify this attribute.
> + if (kstrtobool(buf, &enable) < 0)
> + return -EINVAL;
> +
> + if (enable)
> + interval = DEFAULT_RATELIMIT_INTERVAL;
> + else
> + interval = 0;
> +
> + pdev->aer_report->cor_log_ratelimit.interval = interval;
> + pdev->aer_report->uncor_log_ratelimit.interval = interval;
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(ratelimit_log_enable);
[...]
> + static ssize_t \
> + name##_store(struct device *dev, struct device_attribute *attr, \
> + const char *buf, size_t count) \
> +{ \
> + struct pci_dev *pdev = to_pci_dev(dev); \
> + int burst; \
Same as earlier comment. Add the following:
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
> + if (kstrtoint(buf, 0, &burst) < 0) \
> + return -EINVAL; \
> + \
> + pdev->aer_report->ratelimit.burst = burst; \
> + \
> + return count; \
> +} \
> +static DEVICE_ATTR_RW(name)
Thank you!
Krzysztof
