aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTrent Piepho <xyzzy@speakeasy.org>2008-11-30 17:10:12 -0800
committerJesse Barnes <jbarnes@virtuousgeek.org>2009-01-07 11:12:58 -0800
commit92425a405ea482209b43093a5e35be7de02acf18 (patch)
tree0cf7d494d8a743350f29a827a272287a7d99b5d1
parent1684f5ddd4c0c754f52c78eaa2c5c69ad09fb18c (diff)
PCI: Make settable sysfs attributes more consistent
PCI devices have three settable boolean attributes, enable, broken_parity_status, and msi_bus. The store functions for these would silently interpret "0x01" as false, "1llogical" as true, and "true" would be (silently!) ignored and do nothing. This is inconsistent with typical sysfs handling of settable attributes, and just plain doesn't make much sense. So, use strict_strtoul(), which was created for this purpose. The store functions will treat a value of 0 as false, non-zero as true, and return -EINVAL for a parse failure. Additionally, is_enabled_store() and msi_bus_store() return -EPERM if CAP_SYS_ADMIN is lacking, rather than silently doing nothing. This is more typical behavior for sysfs attributes that need a capability. And msi_bus_store() will only print the "forced subordinate bus ..." warning if the MSI flag was actually forced to a different value. Signed-off-by: Trent Piepho <xyzzy@speakeasy.org> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
-rw-r--r--drivers/pci/pci-sysfs.c48
1 files changed, 28 insertions, 20 deletions
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d5cdccf27a6..d2f1354fd18 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -58,13 +58,14 @@ static ssize_t broken_parity_status_store(struct device *dev,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
- ssize_t consumed = -EINVAL;
+ unsigned long val;
- if ((count > 0) && (*buf == '0' || *buf == '1')) {
- pdev->broken_parity_status = *buf == '1' ? 1 : 0;
- consumed = count;
- }
- return consumed;
+ if (strict_strtoul(buf, 0, &val) < 0)
+ return -EINVAL;
+
+ pdev->broken_parity_status = !!val;
+
+ return count;
}
static ssize_t local_cpus_show(struct device *dev,
@@ -133,19 +134,23 @@ static ssize_t is_enabled_store(struct device *dev,
struct device_attribute *attr, const char *buf,
size_t count)
{
- ssize_t result = -EINVAL;
struct pci_dev *pdev = to_pci_dev(dev);
+ unsigned long val;
+ ssize_t result = strict_strtoul(buf, 0, &val);
+
+ if (result < 0)
+ return result;
/* this can crash the machine when done on the "wrong" device */
if (!capable(CAP_SYS_ADMIN))
- return count;
+ return -EPERM;
- if (*buf == '0') {
+ if (!val) {
if (atomic_read(&pdev->enable_cnt) != 0)
pci_disable_device(pdev);
else
result = -EIO;
- } else if (*buf == '1')
+ } else
result = pci_enable_device(pdev);
return result < 0 ? result : count;
@@ -185,25 +190,28 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ unsigned long val;
+
+ if (strict_strtoul(buf, 0, &val) < 0)
+ return -EINVAL;
/* bad things may happen if the no_msi flag is changed
* while some drivers are loaded */
if (!capable(CAP_SYS_ADMIN))
- return count;
+ return -EPERM;
+ /* Maybe pci devices without subordinate busses shouldn't even have this
+ * attribute in the first place? */
if (!pdev->subordinate)
return count;
- if (*buf == '0') {
- pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
- dev_warn(&pdev->dev, "forced subordinate bus to not support MSI,"
- " bad things could happen.\n");
- }
+ /* Is the flag going to change, or keep the value it already had? */
+ if (!(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI) ^
+ !!val) {
+ pdev->subordinate->bus_flags ^= PCI_BUS_FLAGS_NO_MSI;
- if (*buf == '1') {
- pdev->subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
- dev_warn(&pdev->dev, "forced subordinate bus to support MSI,"
- " bad things could happen.\n");
+ dev_warn(&pdev->dev, "forced subordinate bus to%s support MSI,"
+ " bad things could happen\n", val ? "" : " not");
}
return count;