This patch includes the source code of sysfs component of PCI
Express Advanced Error Reporting driver.
Signed-off-by: T. Long Nguyen <[email protected]>
--------------------------------------------------------------------
diff -urpN linux-2.6.11-rc5/drivers/pci/pcie/aer/aerdrv_sysfs.c patch-2.6.11-rc5-aerc3-split2/drivers/pci/pcie/aer/aerdrv_sysfs.c
--- linux-2.6.11-rc5/drivers/pci/pcie/aer/aerdrv_sysfs.c 1969-12-31 19:00:00.000000000 -0500
+++ patch-2.6.11-rc5-aerc3-split2/drivers/pci/pcie/aer/aerdrv_sysfs.c 2005-03-09 13:25:36.000000000 -0500
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 2005 Intel
+ * Copyright (C) Tom Long Nguyen ([email protected])
+ *
+ */
+
+#include "aerdrv_sysfs.h"
+
+static ssize_t aer_sysfs_consume_show(struct device_driver *dev, char *buf)
+{
+ return aer_fsprint_record(buf);
+}
+
+static ssize_t aer_sysfs_status_show(struct device_driver *dev, char *buf)
+{
+ return aer_fsprint_devices(buf);
+}
+
+static ssize_t aer_sysfs_verbose_show(struct device_driver *dev, char *buf)
+{
+ return sprintf(buf, "Verbose display set to %d\n",
+ aer_get_verbose());
+}
+
+static ssize_t aer_sysfs_verbose_store(struct device_driver *drv,
+ const char *buf, size_t count)
+{
+ aer_set_verbose(buf[0] - 0x30);
+ return count;
+}
+
+static ssize_t aer_sysfs_auto_show(struct device_driver *dev, char *buf)
+{
+ return sprintf(buf, "Automatic reporting is %s\n",
+ (aer_get_auto_mode()) ? "on" : "off");
+}
+
+static ssize_t aer_sysfs_auto_store(struct device_driver *drv,
+ const char *buf, size_t count)
+{
+ aer_set_auto_mode(buf[0] - 0x30);
+ return count;
+}
+
+/*
+ * Define AER Sysfs user interfaces
+ */
+static DRIVER_ATTR(consume, S_IRUGO, aer_sysfs_consume_show, NULL);
+static DRIVER_ATTR(status, S_IRUGO, aer_sysfs_status_show, NULL);
+static DRIVER_ATTR(verbose, S_IWUSR | S_IRUGO, aer_sysfs_verbose_show,
+ aer_sysfs_verbose_store);
+static DRIVER_ATTR(auto, S_IWUSR | S_IRUGO, aer_sysfs_auto_show,
+ aer_sysfs_auto_store);
+
+static struct attribute *aer_sysfs_driver_attrs[] = {
+ &driver_attr_status.attr,
+ &driver_attr_verbose.attr,
+ &driver_attr_consume.attr,
+ &driver_attr_auto.attr,
+ NULL
+};
+
+static struct attribute_group aer_sysfs_driver_attr_group = {
+ .attrs = aer_sysfs_driver_attrs,
+};
+
+/**
+ * aer_sysfs_init - AER user interface initialization
+ * @drv: pointer to device_driver data structure of AER service driver
+ *
+ * Invoked when AER service driver is loaded.
+ **/
+int aer_sysfs_init(struct device_driver *drv)
+{
+ return sysfs_create_group(&drv->kobj, &aer_sysfs_driver_attr_group);
+}
+
+/**
+ * aer_sysfs_cleanup - remove AER user interface
+ * @drv: pointer to device_driver data structure of AER service driver
+ *
+ * Invoked when AER service driver is unloaded.
+ **/
+void aer_sysfs_cleanup(struct device_driver *drv)
+{
+ sysfs_remove_group(&drv->kobj, &aer_sysfs_driver_attr_group);
+}
diff -urpN linux-2.6.11-rc5/drivers/pci/pcie/aer/aerdrv_sysfs.h patch-2.6.11-rc5-aerc3-split2/drivers/pci/pcie/aer/aerdrv_sysfs.h
--- linux-2.6.11-rc5/drivers/pci/pcie/aer/aerdrv_sysfs.h 1969-12-31 19:00:00.000000000 -0500
+++ patch-2.6.11-rc5-aerc3-split2/drivers/pci/pcie/aer/aerdrv_sysfs.h 2005-03-09 13:25:36.000000000 -0500
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2005 Intel
+ * Copyright (C) Tom Long Nguyen ([email protected])
+ *
+ */
+
+#ifndef _AERDRV_SYSFS_H_
+#define _AERDRV_SYSFS_H_
+
+#include <linux/device.h>
+
+extern int aer_fsprint_devices(char *page);
+extern int aer_fsprint_record(char *page);
+extern void aer_set_verbose(int value);
+extern int aer_get_verbose(void);
+extern void aer_set_auto_mode(int value);
+extern int aer_get_auto_mode(void);
+
+#endif //_AERDRV_SYSFS_H_
On Fri, Mar 11, 2005 at 04:13:33PM -0800, long wrote:
> +static ssize_t aer_sysfs_consume_show(struct device_driver *dev, char *buf)
> +{
> + return aer_fsprint_record(buf);
> +}
> +
> +static ssize_t aer_sysfs_status_show(struct device_driver *dev, char *buf)
> +{
> + return aer_fsprint_devices(buf);
> +}
> +
Why call wrapper functions that only do one thing? Why have this extra
layer of indirection that is not needed from what I can tell?
> +static ssize_t aer_sysfs_verbose_show(struct device_driver *dev, char *buf)
> +{
> + return sprintf(buf, "Verbose display set to %d\n",
> + aer_get_verbose());
> +}
Just echo the value, don't print out pretty strings :)
> +static ssize_t aer_sysfs_verbose_store(struct device_driver *drv,
> + const char *buf, size_t count)
> +{
> + aer_set_verbose(buf[0] - 0x30);
> + return count;
> +}
Oh, that's a problem waiting to happen... Please validate the user
provided value before acting on it.
> +static ssize_t aer_sysfs_auto_show(struct device_driver *dev, char *buf)
> +{
> + return sprintf(buf, "Automatic reporting is %s\n",
> + (aer_get_auto_mode()) ? "on" : "off");
> +}
Again, just print on/off.
> +static ssize_t aer_sysfs_auto_store(struct device_driver *drv,
> + const char *buf, size_t count)
> +{
> + aer_set_auto_mode(buf[0] - 0x30);
> + return count;
> +}
Also validate this.
thanks,
greg k-h
On Friday, March 11, 2005 11:25 PM Greg KH wrote:
>> +static ssize_t aer_sysfs_consume_show(struct device_driver *dev,
char >>*buf)
>> +{
>> + return aer_fsprint_record(buf);
>> +}
>> +
>> +static ssize_t aer_sysfs_status_show(struct device_driver *dev, char
>>*buf)
>> +{
>> + return aer_fsprint_devices(buf);
>> +}
>> +
>
>Why call wrapper functions that only do one thing? Why have this extra
>layer of indirection that is not needed from what I can tell?
Agree, will make changes appropriately. Thanks for your comment.
>> +static ssize_t aer_sysfs_verbose_show(struct device_driver *dev,
char >>*buf)
>> +{
>> + return sprintf(buf, "Verbose display set to %d\n",
>> + aer_get_verbose());
>> +}
>>
>Just echo the value, don't print out pretty strings :)
Agree, will make changes appropriately.
>> +static ssize_t aer_sysfs_verbose_store(struct device_driver *drv,
>> + const char *buf, size_t count)
>> +{
>> + aer_set_verbose(buf[0] - 0x30);
>> + return count;
>> +}
>>
>Oh, that's a problem waiting to happen... Please validate the user
>provided value before acting on it.
Agree, will make changes appropriately.
>> +static ssize_t aer_sysfs_auto_show(struct device_driver *dev, char
*buf)
>> +{
>> + return sprintf(buf, "Automatic reporting is %s\n",
>> + (aer_get_auto_mode()) ? "on" : "off");
>> +}
>>
>Again, just print on/off.
Agree, will make changes appropriately.
>> +static ssize_t aer_sysfs_auto_store(struct device_driver *drv,
>> + const char *buf, size_t count)
>> +{
>> + aer_set_auto_mode(buf[0] - 0x30);
>> + return count;
>> +}
>>
>Also validate this.
Agree, will make changes appropriately.
Thanks for all comments,
Long