All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes
@ 2014-07-22 13:56 Benjamin Romer
  2014-07-22 13:56 ` [PATCH 1/8 v2] staging: unisys: add toolaction to sysfs Benjamin Romer
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

This patch set moves several interfaces in the visorchipset module from procfs
to sysfs, and removes the remaining proc interfaces from the module. It includes
documentation for the new interfaces in Documentation/ABI, and some code cleanup
in the new sysfs handler functions.

In version 2 of the set, extraneous checks for controlvm_channel pointer 
validity were removed, all newly created attributes use DEVICE_ATTR_RW or 
DEVICE_ATTR_WO, and logging was removed from the show and store functions to 
prevent creating a DOS attack possibility. 

Benjamin Romer (8):
  staging: unisys: add toolaction to sysfs
  staging: unisys: move boottotool out of proc to sysfs
  staging: unisys: move installer to sysfs and split fields
  staging: unisys: move chipsetready to sysfs
  staging: unisys: move parahotplug to sysfs
  staging: unisys: clean up diagdump proc entry code
  staging: unisys: remove partition information from proc
  staging: unisys: ABI documentation for new sysfs entries

 .../Documentation/ABI/sysfs-platform-visorchipset  |  74 +++
 .../unisys/visorchipset/visorchipset_main.c        | 730 +++++++--------------
 2 files changed, 300 insertions(+), 504 deletions(-)
 create mode 100644 drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset

-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/8 v2] staging: unisys: add toolaction to sysfs
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 13:56 ` [PATCH 2/8 v2] staging: unisys: move boottotool out of proc " Benjamin Romer
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

Move the proc entry for controlling the toolaction field to sysfs. The field
appears in /sys/devices/platform/visorchipset/install/toolaction.

This field is used to tell s-Par which type of recovery tool action to perform
on the next guest boot-up. The meaning of the value is dependent on the type
of installation software used to commission the guest.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed.

 .../unisys/visorchipset/visorchipset_main.c        | 142 ++++++++-------------
 1 file changed, 51 insertions(+), 91 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index a16d67e..a0d34eb 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -149,11 +149,6 @@ static ssize_t proc_read_installer(struct file *file, char __user *buf,
 static ssize_t proc_write_installer(struct file *file,
 				    const char __user *buffer,
 				    size_t count, loff_t *ppos);
-static ssize_t proc_read_toolaction(struct file *file, char __user *buf,
-				    size_t len, loff_t *offset);
-static ssize_t proc_write_toolaction(struct file *file,
-				     const char __user *buffer,
-				     size_t count, loff_t *ppos);
 static ssize_t proc_read_bootToTool(struct file *file, char __user *buf,
 				    size_t len, loff_t *offset);
 static ssize_t proc_write_bootToTool(struct file *file,
@@ -164,11 +159,6 @@ static const struct file_operations proc_installer_fops = {
 	.write = proc_write_installer,
 };
 
-static const struct file_operations proc_toolaction_fops = {
-	.read = proc_read_toolaction,
-	.write = proc_write_toolaction,
-};
-
 static const struct file_operations proc_bootToTool_fops = {
 	.read = proc_read_bootToTool,
 	.write = proc_write_bootToTool,
@@ -321,10 +311,33 @@ static VISORCHIPSET_BUSDEV_RESPONDERS BusDev_Responders = {
 /* info for /dev/visorchipset */
 static dev_t MajorDev = -1; /**< indicates major num for device */
 
+/* prototypes for attributes */
+static ssize_t toolaction_show(struct device *dev,
+	struct device_attribute *attr, char *buf);
+static ssize_t toolaction_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_RW(toolaction);
+
+static struct attribute *visorchipset_install_attrs[] = {
+	&dev_attr_toolaction.attr,
+	NULL
+};
+
+static struct attribute_group visorchipset_install_group = {
+	.name = "install",
+	.attrs = visorchipset_install_attrs
+};
+
+static const struct attribute_group *visorchipset_dev_groups[] = {
+	&visorchipset_install_group,
+	NULL
+};
+
 /* /sys/devices/platform/visorchipset */
 static struct platform_device Visorchipset_platform_device = {
 	.name = "visorchipset",
 	.id = -1,
+	.dev.groups = visorchipset_dev_groups,
 };
 
 /* Function prototypes */
@@ -336,6 +349,34 @@ static void controlvm_respond_physdev_changestate(CONTROLVM_MESSAGE_HEADER *
 						  msgHdr, int response,
 						  ULTRA_SEGMENT_STATE state);
 
+ssize_t toolaction_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	U8 toolAction;
+
+	visorchannel_read(ControlVm_channel,
+		offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+			   ToolAction), &toolAction, sizeof(U8));
+	return scnprintf(buf, PAGE_SIZE, "%u\n", toolAction);
+}
+
+ssize_t toolaction_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	U8 toolAction;
+
+	if (sscanf(buf, "%hhu\n", &toolAction) == 1) {
+		if (visorchannel_write(ControlVm_channel,
+			offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+				ToolAction),
+			&toolAction, sizeof(U8)) < 0)
+				return -EFAULT;
+			else
+				return count;
+	} else
+		return -EIO;
+}
+
 static void
 show_partition_property(struct seq_file *f, void *ctx, int property)
 {
@@ -2368,83 +2409,6 @@ proc_write_installer(struct file *file,
 }
 
 /**
- * Reads the ToolAction field of ControlVMChannel.
- */
-static ssize_t
-proc_read_toolaction(struct file *file, char __user *buf,
-		     size_t len, loff_t *offset)
-{
-	int length = 0;
-	U8 toolAction;
-	char *vbuf;
-	loff_t pos = *offset;
-
-	if (pos < 0)
-		return -EINVAL;
-
-	if (pos > 0 || !len)
-		return 0;
-
-	vbuf = kzalloc(len, GFP_KERNEL);
-	if (!vbuf)
-		return -ENOMEM;
-
-	visorchannel_read(ControlVm_channel,
-			  offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-				   ToolAction), &toolAction, sizeof(U8));
-
-	length = sprintf(vbuf, "%u\n", toolAction);
-	if (copy_to_user(buf, vbuf, length)) {
-		kfree(vbuf);
-		return -EFAULT;
-	}
-
-	kfree(vbuf);
-	*offset += length;
-	return length;
-}
-
-/**
- * Writes to the ToolAction field of ControlVMChannel.
- * Input: ToolAction
- * Limit 3 characters input
- */
-#define UINT8_MAX (255U)
-static ssize_t
-proc_write_toolaction(struct file *file,
-		      const char __user *buffer, size_t count, loff_t *ppos)
-{
-	char buf[3];
-	U8 toolAction;
-
-	/* Check to make sure there is no buffer overflow */
-	if (count > (sizeof(buf) - 1))
-		return -EINVAL;
-
-	if (copy_from_user(buf, buffer, count)) {
-		WARN(1, "Error copying from user space\n");
-		return -EFAULT;
-	}
-
-	if (sscanf(buf, "%hhd", &toolAction) != 1)
-		toolAction = UINT8_MAX;
-
-	if (toolAction != UINT8_MAX) {
-		if (visorchannel_write
-		    (ControlVm_channel,
-		     offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL, ToolAction),
-		     &toolAction, sizeof(U8)) < 0)
-			WARN(1, "Installation ToolAction Write Failed - ToolAction = %d\n",
-			     toolAction);
-	}
-
-	/* So this function isn't called multiple times, must return
-	 * size of buffer
-	 */
-	return count;
-}
-
-/**
  * Reads the EfiSparIndication.BootToTool field of ControlVMChannel.
  */
 static ssize_t
@@ -2534,7 +2498,6 @@ visorchipset_init(void)
 	int rc = 0, x = 0;
 	char s[64];
 	struct proc_dir_entry *installer_file;
-	struct proc_dir_entry *toolaction_file;
 	struct proc_dir_entry *bootToTool_file;
 	HOSTADDRESS addr;
 
@@ -2612,9 +2575,6 @@ visorchipset_init(void)
 	/* Setup Installation fields */
 	installer_file = proc_create("installer", 0644, ProcDir,
 				     &proc_installer_fops);
-	/* Setup the ToolAction field */
-	toolaction_file = proc_create("toolaction", 0644, ProcDir,
-				      &proc_toolaction_fops);
 	/* Setup the BootToTool field */
 	bootToTool_file = proc_create("boottotool", 0644, ProcDir,
 				      &proc_bootToTool_fops);
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/8 v2] staging: unisys: move boottotool out of proc to sysfs
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
  2014-07-22 13:56 ` [PATCH 1/8 v2] staging: unisys: add toolaction to sysfs Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 13:56 ` [PATCH 3/8 v2] staging: unisys: move installer to sysfs and split fields Benjamin Romer
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

Move the proc entry controlling the boottotool flag to procfs. The field
appears in /sys/devices/platform/visorchipset/install/boottotool.

The boottotool flag controls s-Par behavior on the next boot of this guest.
Setting the flag will cause the guest to boot from the utility and installation
image, which will use the value in the toolaction field to determine what
operation is being requested.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed.

 .../unisys/visorchipset/visorchipset_main.c        | 132 +++++++--------------
 1 file changed, 40 insertions(+), 92 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index a0d34eb..48db6ee 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -149,21 +149,12 @@ static ssize_t proc_read_installer(struct file *file, char __user *buf,
 static ssize_t proc_write_installer(struct file *file,
 				    const char __user *buffer,
 				    size_t count, loff_t *ppos);
-static ssize_t proc_read_bootToTool(struct file *file, char __user *buf,
-				    size_t len, loff_t *offset);
-static ssize_t proc_write_bootToTool(struct file *file,
-				     const char __user *buffer,
-				     size_t count, loff_t *ppos);
+
 static const struct file_operations proc_installer_fops = {
 	.read = proc_read_installer,
 	.write = proc_write_installer,
 };
 
-static const struct file_operations proc_bootToTool_fops = {
-	.read = proc_read_bootToTool,
-	.write = proc_write_bootToTool,
-};
-
 typedef struct {
 	U8 __iomem *ptr;	/* pointer to base address of payload pool */
 	U64 offset;		/* offset from beginning of controlvm
@@ -318,8 +309,15 @@ static ssize_t toolaction_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count);
 static DEVICE_ATTR_RW(toolaction);
 
+static ssize_t boottotool_show(struct device *dev,
+	struct device_attribute *attr, char *buf);
+static ssize_t boottotool_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_RW(boottotool);
+
 static struct attribute *visorchipset_install_attrs[] = {
 	&dev_attr_toolaction.attr,
+	&dev_attr_boottotool.attr,
 	NULL
 };
 
@@ -377,6 +375,38 @@ ssize_t toolaction_store(struct device *dev, struct device_attribute *attr,
 		return -EIO;
 }
 
+ssize_t boottotool_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	ULTRA_EFI_SPAR_INDICATION efiSparIndication;
+
+	visorchannel_read(ControlVm_channel,
+		offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+			EfiSparIndication), &efiSparIndication,
+		sizeof(ULTRA_EFI_SPAR_INDICATION));
+	return scnprintf(buf, PAGE_SIZE, "%u\n",
+			efiSparIndication.BootToTool);
+}
+
+ssize_t boottotool_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	int val;
+	ULTRA_EFI_SPAR_INDICATION efiSparIndication;
+
+	if (sscanf(buf, "%u\n", &val) == 1) {
+		efiSparIndication.BootToTool = val;
+		if (visorchannel_write(ControlVm_channel,
+			offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+				 EfiSparIndication),
+			&(efiSparIndication),
+		sizeof(ULTRA_EFI_SPAR_INDICATION)) < 0)
+				return -EFAULT;
+			else
+				return count;
+	} else
+		return -EIO;
+}
 static void
 show_partition_property(struct seq_file *f, void *ctx, int property)
 {
@@ -2408,84 +2438,6 @@ proc_write_installer(struct file *file,
 	return count;
 }
 
-/**
- * Reads the EfiSparIndication.BootToTool field of ControlVMChannel.
- */
-static ssize_t
-proc_read_bootToTool(struct file *file, char __user *buf,
-		     size_t len, loff_t *offset)
-{
-	int length = 0;
-	ULTRA_EFI_SPAR_INDICATION efiSparIndication;
-	char *vbuf;
-	loff_t pos = *offset;
-
-	if (pos < 0)
-		return -EINVAL;
-
-	if (pos > 0 || !len)
-		return 0;
-
-	vbuf = kzalloc(len, GFP_KERNEL);
-	if (!vbuf)
-		return -ENOMEM;
-
-	visorchannel_read(ControlVm_channel,
-			  offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-				   EfiSparIndication), &efiSparIndication,
-			  sizeof(ULTRA_EFI_SPAR_INDICATION));
-
-	length = sprintf(vbuf, "%d\n", (int) efiSparIndication.BootToTool);
-	if (copy_to_user(buf, vbuf, length)) {
-		kfree(vbuf);
-		return -EFAULT;
-	}
-
-	kfree(vbuf);
-	*offset += length;
-	return length;
-}
-
-/**
- * Writes to the EfiSparIndication.BootToTool field of ControlVMChannel.
- * Input: 1 or 0 (1 being on, 0 being off)
- */
-static ssize_t
-proc_write_bootToTool(struct file *file,
-		      const char __user *buffer, size_t count, loff_t *ppos)
-{
-	char buf[3];
-	int inputVal;
-	ULTRA_EFI_SPAR_INDICATION efiSparIndication;
-
-	/* Check to make sure there is no buffer overflow */
-	if (count > (sizeof(buf) - 1))
-		return -EINVAL;
-
-	if (copy_from_user(buf, buffer, count)) {
-		WARN(1, "Error copying from user space\n");
-		return -EFAULT;
-	}
-
-	if (sscanf(buf, "%i", &inputVal) != 1)
-		inputVal = 0;
-
-	efiSparIndication.BootToTool = (inputVal == 1 ? 1 : 0);
-
-	if (visorchannel_write
-	    (ControlVm_channel,
-	     offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL, EfiSparIndication),
-	     &efiSparIndication, sizeof(ULTRA_EFI_SPAR_INDICATION)) < 0)
-		printk
-		    ("Installation BootToTool Write Failed - BootToTool = %d\n",
-		     (int) efiSparIndication.BootToTool);
-
-	/* So this function isn't called multiple times, must return
-	 * size of buffer
-	 */
-	return count;
-}
-
 static const struct file_operations chipset_proc_fops = {
 	.owner = THIS_MODULE,
 	.read = visorchipset_proc_read_writeonly,
@@ -2498,7 +2450,6 @@ visorchipset_init(void)
 	int rc = 0, x = 0;
 	char s[64];
 	struct proc_dir_entry *installer_file;
-	struct proc_dir_entry *bootToTool_file;
 	HOSTADDRESS addr;
 
 	if (!unisys_spar_platform)
@@ -2575,9 +2526,6 @@ visorchipset_init(void)
 	/* Setup Installation fields */
 	installer_file = proc_create("installer", 0644, ProcDir,
 				     &proc_installer_fops);
-	/* Setup the BootToTool field */
-	bootToTool_file = proc_create("boottotool", 0644, ProcDir,
-				      &proc_bootToTool_fops);
 
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/8 v2] staging: unisys: move installer to sysfs and split fields
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
  2014-07-22 13:56 ` [PATCH 1/8 v2] staging: unisys: add toolaction to sysfs Benjamin Romer
  2014-07-22 13:56 ` [PATCH 2/8 v2] staging: unisys: move boottotool out of proc " Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 23:13   ` Greg KH
  2014-07-22 13:56 ` [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs Benjamin Romer
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

The installer entry in /proc/visorchipset/installer was composed of three
separate fields as one entry. This patch removes the proc entry and associated
functions, and creates new fields with distinct entries under sysfs in the
visorchipset/install directory. The fields are:

	textid: used to send the ID of a string that should be displayed on
		s-Par's automatic installation progress screen. Setting this
		field when not in installation mode (boottotool was set on
		the previous guest boot) has no effect.

	remaining_steps: used to set the value of the progress bar on the
		s-Par automatic installation progress screen. This field has
		no effect if not in installation mode.

	error: used to send the ID of a string that should be displayed on
		s-Par's automatic installation progress screen when an error
		is encountered during installation. This field has no effect
		if not in installation mode.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed.

 .../unisys/visorchipset/visorchipset_main.c        | 233 ++++++++++-----------
 1 file changed, 106 insertions(+), 127 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 48db6ee..a20e21b 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -144,16 +144,6 @@ static VISORCHANNEL *ControlVm_channel;
 static ssize_t visorchipset_proc_read_writeonly(struct file *file,
 						char __user *buf,
 						size_t len, loff_t *offset);
-static ssize_t proc_read_installer(struct file *file, char __user *buf,
-				   size_t len, loff_t *offset);
-static ssize_t proc_write_installer(struct file *file,
-				    const char __user *buffer,
-				    size_t count, loff_t *ppos);
-
-static const struct file_operations proc_installer_fops = {
-	.read = proc_read_installer,
-	.write = proc_write_installer,
-};
 
 typedef struct {
 	U8 __iomem *ptr;	/* pointer to base address of payload pool */
@@ -315,9 +305,30 @@ static ssize_t boottotool_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count);
 static DEVICE_ATTR_RW(boottotool);
 
+static ssize_t error_show(struct device *dev, struct device_attribute *attr,
+	char *buf);
+static ssize_t error_store(struct device *dev, struct device_attribute *attr,
+	const char *buf, size_t count);
+static DEVICE_ATTR_RW(error);
+
+static ssize_t textid_show(struct device *dev, struct device_attribute *attr,
+	char *buf);
+static ssize_t textid_store(struct device *dev,	struct device_attribute *attr,
+	const char *buf, size_t count);
+static DEVICE_ATTR_RW(textid);
+
+static ssize_t remaining_steps_show(struct device *dev,
+	struct device_attribute *attr, char *buf);
+static ssize_t remaining_steps_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_RW(remaining_steps);
+
 static struct attribute *visorchipset_install_attrs[] = {
 	&dev_attr_toolaction.attr,
 	&dev_attr_boottotool.attr,
+	&dev_attr_error.attr,
+	&dev_attr_textid.attr,
+	&dev_attr_remaining_steps.attr,
 	NULL
 };
 
@@ -407,6 +418,91 @@ ssize_t boottotool_store(struct device *dev, struct device_attribute *attr,
 	} else
 		return -EIO;
 }
+
+ssize_t error_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	U32 error;
+
+	visorchannel_read(ControlVm_channel, offsetof(
+		ULTRA_CONTROLVM_CHANNEL_PROTOCOL, InstallationError),
+		&error, sizeof(U32));
+	return scnprintf(buf, PAGE_SIZE, "%i\n", error);
+}
+
+ssize_t error_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	U32 error;
+
+	if (sscanf(buf, "%i\n", &error) == 1) {
+		if (visorchannel_write(ControlVm_channel,
+			offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+				InstallationError),
+			&error, sizeof(U32)) == 1) {
+				return count;
+		}
+	}
+	return -EIO;
+}
+
+ssize_t textid_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	U32 textId;
+
+	visorchannel_read(ControlVm_channel, offsetof(
+		ULTRA_CONTROLVM_CHANNEL_PROTOCOL, InstallationTextId),
+		&textId, sizeof(U32));
+	return scnprintf(buf, PAGE_SIZE, "%i\n", textId);
+}
+
+ssize_t textid_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	U32 textId;
+
+	if (sscanf(buf, "%i\n", &textId) == 1) {
+		if (visorchannel_write(ControlVm_channel,
+			offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+				InstallationTextId),
+			&textId, sizeof(U32)) == 1) {
+				return count;
+		}
+	}
+	return -EIO;
+}
+
+
+ssize_t remaining_steps_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	U16 remainingSteps;
+
+	visorchannel_read(ControlVm_channel,
+		offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+			InstallationRemainingSteps),
+		&remainingSteps,
+		sizeof(U16));
+	return scnprintf(buf, PAGE_SIZE, "%hu\n", remainingSteps);
+}
+
+ssize_t remaining_steps_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	U16 remainingSteps;
+
+	if (sscanf(buf, "%hu\n", &remainingSteps) == 1) {
+		if (visorchannel_write(ControlVm_channel,
+			offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
+				InstallationRemainingSteps),
+			&remainingSteps, sizeof(U16)) == 1) {
+				return count;
+		}
+	}
+	return -EIO;
+}
+
 static void
 show_partition_property(struct seq_file *f, void *ctx, int property)
 {
@@ -2326,118 +2422,6 @@ visorchipset_proc_read_writeonly(struct file *file, char __user *buf,
 	return 0;
 }
 
-/**
- * Reads the InstallationError, InstallationTextId,
- * InstallationRemainingSteps fields of ControlVMChannel.
- */
-static ssize_t
-proc_read_installer(struct file *file, char __user *buf,
-		    size_t len, loff_t *offset)
-{
-	int length = 0;
-	U16 remainingSteps;
-	U32 error, textId;
-	char *vbuf;
-	loff_t pos = *offset;
-
-	if (pos < 0)
-		return -EINVAL;
-
-	if (pos > 0 || !len)
-		return 0;
-
-	vbuf = kzalloc(len, GFP_KERNEL);
-	if (!vbuf)
-		return -ENOMEM;
-
-	visorchannel_read(ControlVm_channel,
-			  offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-				   InstallationRemainingSteps), &remainingSteps,
-			  sizeof(U16));
-	visorchannel_read(ControlVm_channel,
-			  offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-				   InstallationError), &error, sizeof(U32));
-	visorchannel_read(ControlVm_channel,
-			  offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-				   InstallationTextId), &textId, sizeof(U32));
-
-	length = sprintf(vbuf, "%u %u %u\n", remainingSteps, error, textId);
-	if (copy_to_user(buf, vbuf, length)) {
-		kfree(vbuf);
-		return -EFAULT;
-	}
-
-	kfree(vbuf);
-	*offset += length;
-	return length;
-}
-
-/**
- * Writes to the InstallationError, InstallationTextId,
- * InstallationRemainingSteps fields of
- * ControlVMChannel.
- * Input: RemainingSteps Error TextId
- * Limit 32 characters input
- */
-#define UINT16_MAX		(65535U)
-#define UINT32_MAX		(4294967295U)
-static ssize_t
-proc_write_installer(struct file *file,
-		     const char __user *buffer, size_t count, loff_t *ppos)
-{
-	char buf[32];
-	U16 remainingSteps;
-	U32 error, textId;
-
-	/* Check to make sure there is no buffer overflow */
-	if (count > (sizeof(buf) - 1))
-		return -EINVAL;
-
-	if (copy_from_user(buf, buffer, count)) {
-		WARN(1, "Error copying from user space\n");
-		return -EFAULT;
-	}
-
-	if (sscanf(buf, "%hu %i %i", &remainingSteps, &error, &textId) != 3) {
-		remainingSteps = UINT16_MAX;
-		error = UINT32_MAX;
-		textId = UINT32_MAX;
-	}
-
-	if (remainingSteps != UINT16_MAX) {
-		if (visorchannel_write
-		    (ControlVm_channel,
-		     offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-			      InstallationRemainingSteps), &remainingSteps,
-		     sizeof(U16)) < 0)
-			WARN(1, "Installation Status Write Failed - Write function error - RemainingSteps = %d\n",
-			     remainingSteps);
-	}
-
-	if (error != UINT32_MAX) {
-		if (visorchannel_write
-		    (ControlVm_channel,
-		     offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-			      InstallationError), &error, sizeof(U32)) < 0)
-			WARN(1, "Installation Status Write Failed - Write function error - Error = %d\n",
-			     error);
-	}
-
-	if (textId != UINT32_MAX) {
-		if (visorchannel_write
-		    (ControlVm_channel,
-		     offsetof(ULTRA_CONTROLVM_CHANNEL_PROTOCOL,
-			      InstallationTextId), &textId, sizeof(U32)) < 0)
-			WARN(1, "Installation Status Write Failed - Write function error - TextId = %d\n",
-			     textId);
-	}
-
-	/* So this function isn't called multiple times, must return
-	 * size of buffer
-	 */
-	return count;
-}
-
 static const struct file_operations chipset_proc_fops = {
 	.owner = THIS_MODULE,
 	.read = visorchipset_proc_read_writeonly,
@@ -2449,7 +2433,6 @@ visorchipset_init(void)
 {
 	int rc = 0, x = 0;
 	char s[64];
-	struct proc_dir_entry *installer_file;
 	HOSTADDRESS addr;
 
 	if (!unisys_spar_platform)
@@ -2523,10 +2506,6 @@ visorchipset_init(void)
 					      PartitionPropertyNames,
 					      &show_partition_property);
 
-	/* Setup Installation fields */
-	installer_file = proc_create("installer", 0644, ProcDir,
-				     &proc_installer_fops);
-
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	chipset_proc_dir = proc_create(VISORCHIPSET_CHIPSET_PROC_ENTRY_FN,
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
                   ` (2 preceding siblings ...)
  2014-07-22 13:56 ` [PATCH 3/8 v2] staging: unisys: move installer to sysfs and split fields Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 23:15   ` Greg KH
  2014-07-22 23:17   ` Greg KH
  2014-07-22 13:56 ` [PATCH 5/8 v2] staging: unisys: move parahotplug " Benjamin Romer
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: jkc, driverdev-devel, sparmaintainer, Benjamin Romer

Move the chipsetready proc entry to sysfs under a new directory guest. This
entry is used by Unisys application software on the guest to acknowledge
completion of specific events for integration purposes, but these
acknowledgements are not required for the guest to operate correctly.

The store function is simplified as well, to use scanf() instead of copying
the buffer and using strsep().

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed. The off-by-one error in the sscanf() was fixed. Error -1 that was 
being returned was changed to -EINVAL.

 .../unisys/visorchipset/visorchipset_main.c        | 88 ++++++++--------------
 1 file changed, 33 insertions(+), 55 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index a20e21b..74ab15b 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -129,9 +129,6 @@ static MYPROCTYPE *PartitionType;
 #define VISORCHIPSET_DIAG_PROC_ENTRY_FN "diagdump"
 static struct proc_dir_entry *diag_proc_dir;
 
-#define VISORCHIPSET_CHIPSET_PROC_ENTRY_FN "chipsetready"
-static struct proc_dir_entry *chipset_proc_dir;
-
 #define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN "parahotplug"
 static struct proc_dir_entry *parahotplug_proc_dir;
 
@@ -323,6 +320,10 @@ static ssize_t remaining_steps_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count);
 static DEVICE_ATTR_RW(remaining_steps);
 
+static ssize_t chipsetready_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_WO(chipsetready);
+
 static struct attribute *visorchipset_install_attrs[] = {
 	&dev_attr_toolaction.attr,
 	&dev_attr_boottotool.attr,
@@ -337,8 +338,19 @@ static struct attribute_group visorchipset_install_group = {
 	.attrs = visorchipset_install_attrs
 };
 
+static struct attribute *visorchipset_guest_attrs[] = {
+	&dev_attr_chipsetready.attr,
+	NULL
+};
+
+static struct attribute_group visorchipset_guest_group = {
+	.name = "guest",
+	.attrs = visorchipset_guest_attrs
+};
+
 static const struct attribute_group *visorchipset_dev_groups[] = {
 	&visorchipset_install_group,
+	&visorchipset_guest_group,
 	NULL
 };
 
@@ -2369,49 +2381,27 @@ visorchipset_cache_free(struct kmem_cache *pool, void *p, char *fn, int ln)
 	kmem_cache_free(pool, p);
 }
 
-#define gettoken(bufp) strsep(bufp, " -\t\n")
-
-static ssize_t
-chipset_proc_write(struct file *file, const char __user *buffer,
-		   size_t count, loff_t *ppos)
+ssize_t chipsetready_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
 {
-	char buf[512];
-	char *token, *p;
-
-	if (count > sizeof(buf) - 1) {
-		LOGERR("chipset_proc_write: count (%d) exceeds size of buffer (%d)",
-		     (int) count, (int) sizeof(buffer));
-		return -EINVAL;
-	}
-	if (copy_from_user(buf, buffer, count)) {
-		LOGERR("chipset_proc_write: copy_from_user failed");
-		return -EFAULT;
-	}
-	buf[count] = '\0';
-
-	p = buf;
-	token = gettoken(&p);
-
-	if (strcmp(token, "CALLHOMEDISK_MOUNTED") == 0) {
-		token = gettoken(&p);
-		/* The Call Home Disk has been mounted */
-		if (strcmp(token, "0") == 0)
-			chipset_events[0] = 1;
-	} else if (strcmp(token, "MODULES_LOADED") == 0) {
-		token = gettoken(&p);
-		/* All modules for the partition have been loaded */
-		if (strcmp(token, "0") == 0)
-			chipset_events[1] = 1;
-	} else if (token == NULL) {
-		/* No event specified */
-		LOGERR("No event was specified to send CHIPSET_READY response");
-		return -1;
+	char msgtype[64];
+	int msgparam;
+
+	if (sscanf(buf, "%63s %d", msgtype, &msgparam) == 2) {
+		if (strcmp(msgtype, "CALLHOMEDISK_MOUNTED") == 0) {
+			/* The Call Home Disk has been mounted */
+			if (msgparam == 0)
+				chipset_events[0] = 1;
+		} else if (strcmp(msgtype, "MODULES_LOADED") == 0) {
+			/* All modules for the partition have been loaded */
+			if (msgparam == 0)
+				chipset_events[1] = 1;
+		} else {
+			return -EINVAL;
+		}
 	} else {
-		/* Unsupported event specified */
-		LOGERR("%s is an invalid event for sending CHIPSET_READY response",		     token);
-		return -1;
+		return -EINVAL;
 	}
-
 	return count;
 }
 
@@ -2422,12 +2412,6 @@ visorchipset_proc_read_writeonly(struct file *file, char __user *buf,
 	return 0;
 }
 
-static const struct file_operations chipset_proc_fops = {
-	.owner = THIS_MODULE,
-	.read = visorchipset_proc_read_writeonly,
-	.write = chipset_proc_write,
-};
-
 static int __init
 visorchipset_init(void)
 {
@@ -2508,8 +2492,6 @@ visorchipset_init(void)
 
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
-	chipset_proc_dir = proc_create(VISORCHIPSET_CHIPSET_PROC_ENTRY_FN,
-				       0644, ProcDir, &chipset_proc_fops);
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	parahotplug_proc_dir =
@@ -2613,10 +2595,6 @@ visorchipset_exit(void)
 	}
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
-	if (chipset_proc_dir) {
-		remove_proc_entry(VISORCHIPSET_CHIPSET_PROC_ENTRY_FN, ProcDir);
-		chipset_proc_dir = NULL;
-	}
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	if (parahotplug_proc_dir) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8 v2] staging: unisys: move parahotplug to sysfs
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
                   ` (3 preceding siblings ...)
  2014-07-22 13:56 ` [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 23:18   ` Greg KH
  2014-07-22 13:56 ` [PATCH 6/8 v2] staging: unisys: clean up diagdump proc entry code Benjamin Romer
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

Move the /proc/visorchipset/parahotplug interface to sysfs under
/sys/devices/platform/visorchipset/guest/parahotplug.

The parahotplug interface is used to deal with recovery situations on s-Par
guest partitions. The command service partition will send a message to a guest
when a device that guest is using needs to be temporarily removed. The message
triggers a udev event that will cause a recovery script to run. When that
script has completed its work, it will write to the parahotplug interface to
send a message back to Command indicating that it is safe to remove the device.

Moving this interface to sysfs orphans the visorchipset_proc_read_writeonly()
function, so it is also removed.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: attribute creation was fixed and checks for controlvm_channel pointer were
removed.

 .../unisys/visorchipset/visorchipset_main.c        | 59 ++++------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 74ab15b..c88f95f 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -129,19 +129,12 @@ static MYPROCTYPE *PartitionType;
 #define VISORCHIPSET_DIAG_PROC_ENTRY_FN "diagdump"
 static struct proc_dir_entry *diag_proc_dir;
 
-#define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN "parahotplug"
-static struct proc_dir_entry *parahotplug_proc_dir;
-
 static LIST_HEAD(BusInfoList);
 static LIST_HEAD(DevInfoList);
 
 static struct proc_dir_entry *ProcDir;
 static VISORCHANNEL *ControlVm_channel;
 
-static ssize_t visorchipset_proc_read_writeonly(struct file *file,
-						char __user *buf,
-						size_t len, loff_t *offset);
-
 typedef struct {
 	U8 __iomem *ptr;	/* pointer to base address of payload pool */
 	U64 offset;		/* offset from beginning of controlvm
@@ -324,6 +317,10 @@ static ssize_t chipsetready_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count);
 static DEVICE_ATTR_WO(chipsetready);
 
+static ssize_t parahotplug_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count);
+static DEVICE_ATTR_WO(parahotplug);
+
 static struct attribute *visorchipset_install_attrs[] = {
 	&dev_attr_toolaction.attr,
 	&dev_attr_boottotool.attr,
@@ -340,6 +337,7 @@ static struct attribute_group visorchipset_install_group = {
 
 static struct attribute *visorchipset_guest_attrs[] = {
 	&dev_attr_chipsetready.attr,
+	&dev_attr_parahotplug.attr,
 	NULL
 };
 
@@ -1812,30 +1810,17 @@ parahotplug_process_message(CONTROLVM_MESSAGE *inmsg)
 
 /*
  * Gets called when the udev script writes to
- * /proc/visorchipset/parahotplug.  Expects input in the form of "<id>
- * <active>" where <id> is the identifier passed to the script that
- * matches a request on the request list, and <active> is 0 or 1
- * indicating whether the device is now enabled or not.
+ * /sys/devices/platform/visorchipset/guest/parahotplug.
+ * Expects input in the form of "<id> <active>" where <id> is the identifier
+ * passed to the script that matches a request on the request list, and <active>
+ * is 0 or 1 indicating whether the device is now enabled or not.
  */
-static ssize_t
-parahotplug_proc_write(struct file *file, const char __user *buffer,
-		       size_t count, loff_t *ppos)
+ssize_t parahotplug_store(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
 {
-	char buf[64];
 	uint id;
 	ushort active;
 
-	if (count > sizeof(buf) - 1) {
-		LOGERR("parahotplug_proc_write: count (%d) exceeds size of buffer (%d)",
-		     (int) count, (int) sizeof(buf));
-		return -EINVAL;
-	}
-	if (copy_from_user(buf, buffer, count)) {
-		LOGERR("parahotplug_proc_write: copy_from_user failed");
-		return -EFAULT;
-	}
-	buf[count] = '\0';
-
 	if (sscanf(buf, "%u %hu", &id, &active) != 2) {
 		id = 0;
 		active = 0;
@@ -1851,12 +1836,6 @@ parahotplug_proc_write(struct file *file, const char __user *buffer,
 	return count;
 }
 
-static const struct file_operations parahotplug_proc_fops = {
-	.owner = THIS_MODULE,
-	.read = visorchipset_proc_read_writeonly,
-	.write = parahotplug_proc_write,
-};
-
 /* Process a controlvm message.
  * Return result:
  *    FALSE - this function will return FALSE only in the case where the
@@ -2405,13 +2384,6 @@ ssize_t chipsetready_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static ssize_t
-visorchipset_proc_read_writeonly(struct file *file, char __user *buf,
-				 size_t len, loff_t *offset)
-{
-	return 0;
-}
-
 static int __init
 visorchipset_init(void)
 {
@@ -2494,9 +2466,6 @@ visorchipset_init(void)
 
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
-	parahotplug_proc_dir =
-	    proc_create(VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN, 0200,
-			ProcDir, &parahotplug_proc_fops);
 	memset(&g_DelDumpMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	Putfile_buffer_list_pool =
@@ -2597,12 +2566,6 @@ visorchipset_exit(void)
 
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
-	if (parahotplug_proc_dir) {
-		remove_proc_entry(VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN,
-				  ProcDir);
-		parahotplug_proc_dir = NULL;
-	}
-
 	memset(&g_DelDumpMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	proc_DeInit();
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8 v2] staging: unisys: clean up diagdump proc entry code
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
                   ` (4 preceding siblings ...)
  2014-07-22 13:56 ` [PATCH 5/8 v2] staging: unisys: move parahotplug " Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 13:56 ` [PATCH 7/8 v2] staging: unisys: remove partition information from proc Benjamin Romer
  2014-07-22 13:56 ` [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries Benjamin Romer
  7 siblings, 0 replies; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

Remove remnant code left over from the diagdump proc entry.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: patch locations changed due to prior patches being revised.

 drivers/staging/unisys/visorchipset/visorchipset_main.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index c88f95f..134f028 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -126,9 +126,6 @@ InitPartitionProperties(void)
 
 static MYPROCTYPE *PartitionType;
 
-#define VISORCHIPSET_DIAG_PROC_ENTRY_FN "diagdump"
-static struct proc_dir_entry *diag_proc_dir;
-
 static LIST_HEAD(BusInfoList);
 static LIST_HEAD(DevInfoList);
 
@@ -2558,10 +2555,7 @@ visorchipset_exit(void)
 		visor_proc_DestroyType(PartitionType);
 		PartitionType = NULL;
 	}
-	if (diag_proc_dir) {
-		remove_proc_entry(VISORCHIPSET_DIAG_PROC_ENTRY_FN, ProcDir);
-		diag_proc_dir = NULL;
-	}
+
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8 v2] staging: unisys: remove partition information from proc
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
                   ` (5 preceding siblings ...)
  2014-07-22 13:56 ` [PATCH 6/8 v2] staging: unisys: clean up diagdump proc entry code Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 13:56 ` [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries Benjamin Romer
  7 siblings, 0 replies; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

Debugging information for the guest's channels was being exposed in proc.
Remove the code that creates these entries, which are no longer needed.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: patch location changed due to prior patches being revised.

 .../unisys/visorchipset/visorchipset_main.c        | 100 ---------------------
 1 file changed, 100 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 134f028..1b6db5d 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -98,38 +98,9 @@ static CONTROLVM_MESSAGE_PACKET g_DeviceChangeStatePacket;
 #define is_diagpool_channel(channel_type_guid) \
 	 (uuid_le_cmp(channel_type_guid, UltraDiagPoolChannelProtocolGuid) == 0)
 
-typedef enum {
-	PARTPROP_invalid,
-	PARTPROP_name,
-	PARTPROP_description,
-	PARTPROP_handle,
-	PARTPROP_busNumber,
-	/* add new properties above, but don't forget to change
-	 * InitPartitionProperties() and show_partition_property() also...
-	 */
-	PARTPROP_last
-} PARTITION_property;
-static const char *PartitionTypeNames[] = { "partition", NULL };
-
-static char *PartitionPropertyNames[PARTPROP_last + 1];
-static void
-InitPartitionProperties(void)
-{
-	char **p = PartitionPropertyNames;
-	p[PARTPROP_invalid] = "";
-	p[PARTPROP_name] = "name";
-	p[PARTPROP_description] = "description";
-	p[PARTPROP_handle] = "handle";
-	p[PARTPROP_busNumber] = "busNumber";
-	p[PARTPROP_last] = NULL;
-}
-
-static MYPROCTYPE *PartitionType;
-
 static LIST_HEAD(BusInfoList);
 static LIST_HEAD(DevInfoList);
 
-static struct proc_dir_entry *ProcDir;
 static VISORCHANNEL *ControlVm_channel;
 
 typedef struct {
@@ -510,52 +481,6 @@ ssize_t remaining_steps_store(struct device *dev, struct device_attribute *attr,
 	return -EIO;
 }
 
-static void
-show_partition_property(struct seq_file *f, void *ctx, int property)
-{
-	VISORCHIPSET_BUS_INFO *info = (VISORCHIPSET_BUS_INFO *) (ctx);
-
-	switch (property) {
-	case PARTPROP_name:
-		seq_printf(f, "%s\n", NONULLSTR(info->name));
-		break;
-	case PARTPROP_description:
-		seq_printf(f, "%s\n", NONULLSTR(info->description));
-		break;
-	case PARTPROP_handle:
-		seq_printf(f, "0x%-16.16Lx\n", info->partitionHandle);
-		break;
-	case PARTPROP_busNumber:
-		seq_printf(f, "%d\n", info->busNo);
-		break;
-	default:
-		seq_printf(f, "(%d??)\n", property);
-		break;
-	}
-}
-
-static void
-proc_Init(void)
-{
-	if (ProcDir == NULL) {
-		ProcDir = proc_mkdir(MYDRVNAME, NULL);
-		if (ProcDir == NULL) {
-			LOGERR("failed to create /proc directory %s",
-			       MYDRVNAME);
-			POSTCODE_LINUX_2(CHIPSET_INIT_FAILURE_PC,
-					 POSTCODE_SEVERITY_ERR);
-		}
-	}
-}
-
-static void
-proc_DeInit(void)
-{
-	if (ProcDir != NULL)
-		remove_proc_entry(MYDRVNAME, NULL);
-	ProcDir = NULL;
-}
-
 #if 0
 static void
 testUnicode(void)
@@ -1258,16 +1183,6 @@ bus_configure(CONTROLVM_MESSAGE *inmsg, PARSER_CONTEXT *parser_ctx)
 	pBusInfo->name = parser_string_get(parser_ctx);
 
 	visorchannel_uuid_id(&pBusInfo->partitionGuid, s);
-	pBusInfo->procObject =
-	    visor_proc_CreateObject(PartitionType, s, (void *) (pBusInfo));
-	if (pBusInfo->procObject == NULL) {
-		LOGERR("CONTROLVM_BUS_CONFIGURE Failed: busNo=%lu failed to create /proc entry",
-		     busNo);
-		POSTCODE_LINUX_3(BUS_CONFIGURE_FAILURE_PC, busNo,
-				 POSTCODE_SEVERITY_ERR);
-		rc = -CONTROLVM_RESP_ERROR_KMALLOC_FAILED;
-		goto Away;
-	}
 	POSTCODE_LINUX_3(BUS_CONFIGURE_EXIT_PC, busNo, POSTCODE_SEVERITY_INFO);
 Away:
 	bus_epilog(busNo, CONTROLVM_BUS_CONFIGURE, &inmsg->hdr,
@@ -2450,15 +2365,6 @@ visorchipset_init(void)
 		goto Away;
 	}
 
-	proc_Init();
-	memset(PartitionPropertyNames, 0, sizeof(PartitionPropertyNames));
-	InitPartitionProperties();
-
-	PartitionType = visor_proc_CreateType(ProcDir, PartitionTypeNames,
-					      (const char **)
-					      PartitionPropertyNames,
-					      &show_partition_property);
-
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
@@ -2551,18 +2457,12 @@ visorchipset_exit(void)
 
 	cleanup_controlvm_structures();
 
-	if (PartitionType) {
-		visor_proc_DestroyType(PartitionType);
-		PartitionType = NULL;
-	}
-
 	memset(&g_DiagMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	memset(&g_ChipSetMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
 	memset(&g_DelDumpMsgHdr, 0, sizeof(CONTROLVM_MESSAGE_HEADER));
 
-	proc_DeInit();
 	LOGINF("Channel %s (ControlVm) disconnected",
 	       visorchannel_id(ControlVm_channel, s));
 	visorchannel_destroy(ControlVm_channel);
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries
  2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
                   ` (6 preceding siblings ...)
  2014-07-22 13:56 ` [PATCH 7/8 v2] staging: unisys: remove partition information from proc Benjamin Romer
@ 2014-07-22 13:56 ` Benjamin Romer
  2014-07-22 23:14   ` Greg KH
  7 siblings, 1 reply; 15+ messages in thread
From: Benjamin Romer @ 2014-07-22 13:56 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, sparmaintainer, Benjamin Romer

This patch adds a documentation file for all of the interfaces that were moved
to sysfs by the other patches in this set.

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
---
v2: whitespace errors were corrected.

 .../Documentation/ABI/sysfs-platform-visorchipset  | 74 ++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset

diff --git a/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset b/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset
new file mode 100644
index 0000000..b6cad24
--- /dev/null
+++ b/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset
@@ -0,0 +1,74 @@
+What:		install/error
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	used to send the ID of a string that should be displayed on
+		s-Par's automatic installation progress screen when an error
+		is encountered during installation. This field has no effect
+		if not in installation mode.
+Users:		sparmaintainer@unisys.com
+
+What:		install/remainingsteps
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	used to set the value of the progress bar on the s-Par automatic
+		installation progress screen. This field has no effect if not in
+		installation mode.
+Users:		sparmaintainer@unisys.com
+
+What:		install/textid
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	used to send the ID of a string that should be displayed on
+		s-Par's automatic installation progress screen. Setting this
+		field when not in installation mode (boottotool was set on
+		the previous guest boot) has no effect.
+Users:		sparmaintainer@unisys.com
+
+What:		install/boottotool
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	The boottotool flag controls s-Par behavior on the next boot of
+		this guest. Setting the flag will cause the guest to boot from
+		the utility and installation image, which will use the value in
+		the toolaction field to determine what operation is being
+		requested.
+Users:		sparmaintainer@unisys.com
+
+What:		install/toolaction
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	This field is used to tell s-Par which type of recovery tool
+		action to perform on the next guest boot-up. The meaning of the
+		value is dependent on the type of installation software used to
+		commission the guest.
+Users:		sparmaintainer@unisys.com
+
+What:		guest/chipsetready
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	This entry is used by Unisys application software on the guest
+		to acknowledge completion of specific events for integration
+		purposes, but these acknowledgements are not required for the
+		guest to operate correctly.
+Users:		sparmaintainer@unisys.com
+
+What:		guest/parahotplug
+Date:		7/18/2014
+KernelVersion: 	TBD
+Contact:	sparmaintainer@unisys.com
+Description:	The parahotplug interface is used to deal with recovery
+		situations on s-Par guest partitions. The command service
+		partition will send a message to a guest when a device that
+		guest is using needs to be temporarily removed. The message
+		triggers a udev event that will cause a recovery script to run.
+		When that script has completed its work, it will write to the
+		parahotplug interface to send a message back to Command
+		indicating that it is safe to remove the device.
+Users:		sparmaintainer@unisys.com
+
-- 
1.9.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/8 v2] staging: unisys: move installer to sysfs and split fields
  2014-07-22 13:56 ` [PATCH 3/8 v2] staging: unisys: move installer to sysfs and split fields Benjamin Romer
@ 2014-07-22 23:13   ` Greg KH
  0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2014-07-22 23:13 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: driverdev-devel, sparmaintainer

On Tue, Jul 22, 2014 at 09:56:27AM -0400, Benjamin Romer wrote:
> The installer entry in /proc/visorchipset/installer was composed of three
> separate fields as one entry. This patch removes the proc entry and associated
> functions, and creates new fields with distinct entries under sysfs in the
> visorchipset/install directory. The fields are:
> 
> 	textid: used to send the ID of a string that should be displayed on
> 		s-Par's automatic installation progress screen. Setting this
> 		field when not in installation mode (boottotool was set on
> 		the previous guest boot) has no effect.
> 
> 	remaining_steps: used to set the value of the progress bar on the
> 		s-Par automatic installation progress screen. This field has
> 		no effect if not in installation mode.
> 
> 	error: used to send the ID of a string that should be displayed on
> 		s-Par's automatic installation progress screen when an error
> 		is encountered during installation. This field has no effect
> 		if not in installation mode.
> 
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> ---
> v2: attribute creation was fixed and checks for controlvm_channel pointer were
> removed.
> 
>  .../unisys/visorchipset/visorchipset_main.c        | 233 ++++++++++-----------
>  1 file changed, 106 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> index 48db6ee..a20e21b 100644
> --- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
> +++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> @@ -144,16 +144,6 @@ static VISORCHANNEL *ControlVm_channel;
>  static ssize_t visorchipset_proc_read_writeonly(struct file *file,
>  						char __user *buf,
>  						size_t len, loff_t *offset);
> -static ssize_t proc_read_installer(struct file *file, char __user *buf,
> -				   size_t len, loff_t *offset);
> -static ssize_t proc_write_installer(struct file *file,
> -				    const char __user *buffer,
> -				    size_t count, loff_t *ppos);
> -
> -static const struct file_operations proc_installer_fops = {
> -	.read = proc_read_installer,
> -	.write = proc_write_installer,
> -};
>  
>  typedef struct {
>  	U8 __iomem *ptr;	/* pointer to base address of payload pool */
> @@ -315,9 +305,30 @@ static ssize_t boottotool_store(struct device *dev,
>  	struct device_attribute *attr, const char *buf, size_t count);
>  static DEVICE_ATTR_RW(boottotool);
>  
> +static ssize_t error_show(struct device *dev, struct device_attribute *attr,
> +	char *buf);
> +static ssize_t error_store(struct device *dev, struct device_attribute *attr,
> +	const char *buf, size_t count);
> +static DEVICE_ATTR_RW(error);
> +
> +static ssize_t textid_show(struct device *dev, struct device_attribute *attr,
> +	char *buf);
> +static ssize_t textid_store(struct device *dev,	struct device_attribute *attr,

You have an extra tab character in this line :(



> +ssize_t remaining_steps_show(struct device *dev, struct device_attribute *attr,
> +		char *buf)
> +{
> +	U16 remainingSteps;

Minor nit, I took your previous patch, but as this needs to be redone,
plese, for new functions, use the proper kernel variable types, like
u16 here, not made-up-ones-for-this-driver-alone.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries
  2014-07-22 13:56 ` [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries Benjamin Romer
@ 2014-07-22 23:14   ` Greg KH
  2014-07-23 13:46     ` Romer, Benjamin M
  0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2014-07-22 23:14 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: driverdev-devel, sparmaintainer

On Tue, Jul 22, 2014 at 09:56:32AM -0400, Benjamin Romer wrote:
> This patch adds a documentation file for all of the interfaces that were moved
> to sysfs by the other patches in this set.
> 
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> ---
> v2: whitespace errors were corrected.
> 
>  .../Documentation/ABI/sysfs-platform-visorchipset  | 74 ++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset
> 
> diff --git a/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset b/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset
> new file mode 100644
> index 0000000..b6cad24
> --- /dev/null
> +++ b/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset
> @@ -0,0 +1,74 @@
> +What:		install/error
> +Date:		7/18/2014
> +KernelVersion: 	TBD
> +Contact:	sparmaintainer@unisys.com
> +Description:	used to send the ID of a string that should be displayed on
> +		s-Par's automatic installation progress screen when an error
> +		is encountered during installation. This field has no effect
> +		if not in installation mode.
> +Users:		sparmaintainer@unisys.com
> +
> +What:		install/remainingsteps
> +Date:		7/18/2014
> +KernelVersion: 	TBD
> +Contact:	sparmaintainer@unisys.com
> +Description:	used to set the value of the progress bar on the s-Par automatic
> +		installation progress screen. This field has no effect if not in
> +		installation mode.
> +Users:		sparmaintainer@unisys.com
> +
> +What:		install/textid
> +Date:		7/18/2014
> +KernelVersion: 	TBD
> +Contact:	sparmaintainer@unisys.com
> +Description:	used to send the ID of a string that should be displayed on
> +		s-Par's automatic installation progress screen. Setting this
> +		field when not in installation mode (boottotool was set on
> +		the previous guest boot) has no effect.
> +Users:		sparmaintainer@unisys.com
> +
> +What:		install/boottotool
> +Date:		7/18/2014
> +KernelVersion: 	TBD
> +Contact:	sparmaintainer@unisys.com
> +Description:	The boottotool flag controls s-Par behavior on the next boot of
> +		this guest. Setting the flag will cause the guest to boot from
> +		the utility and installation image, which will use the value in
> +		the toolaction field to determine what operation is being
> +		requested.
> +Users:		sparmaintainer@unisys.com
> +
> +What:		install/toolaction
> +Date:		7/18/2014
> +KernelVersion: 	TBD
> +Contact:	sparmaintainer@unisys.com
> +Description:	This field is used to tell s-Par which type of recovery tool
> +		action to perform on the next guest boot-up. The meaning of the
> +		value is dependent on the type of installation software used to
> +		commission the guest.
> +Users:		sparmaintainer@unisys.com
> +
> +What:		guest/chipsetready
> +Date:		7/18/2014
> +KernelVersion: 	TBD
> +Contact:	sparmaintainer@unisys.com
> +Description:	This entry is used by Unisys application software on the guest
> +		to acknowledge completion of specific events for integration
> +		purposes, but these acknowledgements are not required for the
> +		guest to operate correctly.
> +Users:		sparmaintainer@unisys.com

What is the format of the chipsetready file?  It looks like you want to
write 2 values to it?  Please describe it better, for all of these.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs
  2014-07-22 13:56 ` [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs Benjamin Romer
@ 2014-07-22 23:15   ` Greg KH
  2014-07-22 23:17   ` Greg KH
  1 sibling, 0 replies; 15+ messages in thread
From: Greg KH @ 2014-07-22 23:15 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: jkc, driverdev-devel, sparmaintainer

On Tue, Jul 22, 2014 at 09:56:28AM -0400, Benjamin Romer wrote:
> Move the chipsetready proc entry to sysfs under a new directory guest. This
> entry is used by Unisys application software on the guest to acknowledge
> completion of specific events for integration purposes, but these
> acknowledgements are not required for the guest to operate correctly.
> 
> The store function is simplified as well, to use scanf() instead of copying
> the buffer and using strsep().
> 
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> ---
> v2: attribute creation was fixed and checks for controlvm_channel pointer were
> removed. The off-by-one error in the sscanf() was fixed. Error -1 that was 
> being returned was changed to -EINVAL.
> 
>  .../unisys/visorchipset/visorchipset_main.c        | 88 ++++++++--------------
>  1 file changed, 33 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> index a20e21b..74ab15b 100644
> --- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
> +++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> @@ -129,9 +129,6 @@ static MYPROCTYPE *PartitionType;
>  #define VISORCHIPSET_DIAG_PROC_ENTRY_FN "diagdump"
>  static struct proc_dir_entry *diag_proc_dir;
>  
> -#define VISORCHIPSET_CHIPSET_PROC_ENTRY_FN "chipsetready"
> -static struct proc_dir_entry *chipset_proc_dir;
> -
>  #define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN "parahotplug"
>  static struct proc_dir_entry *parahotplug_proc_dir;
>  
> @@ -323,6 +320,10 @@ static ssize_t remaining_steps_store(struct device *dev,
>  	struct device_attribute *attr, const char *buf, size_t count);
>  static DEVICE_ATTR_RW(remaining_steps);
>  
> +static ssize_t chipsetready_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count);
> +static DEVICE_ATTR_WO(chipsetready);
> +
>  static struct attribute *visorchipset_install_attrs[] = {
>  	&dev_attr_toolaction.attr,
>  	&dev_attr_boottotool.attr,
> @@ -337,8 +338,19 @@ static struct attribute_group visorchipset_install_group = {
>  	.attrs = visorchipset_install_attrs
>  };
>  
> +static struct attribute *visorchipset_guest_attrs[] = {
> +	&dev_attr_chipsetready.attr,
> +	NULL
> +};
> +
> +static struct attribute_group visorchipset_guest_group = {
> +	.name = "guest",
> +	.attrs = visorchipset_guest_attrs
> +};
> +
>  static const struct attribute_group *visorchipset_dev_groups[] = {
>  	&visorchipset_install_group,
> +	&visorchipset_guest_group,
>  	NULL
>  };
>  
> @@ -2369,49 +2381,27 @@ visorchipset_cache_free(struct kmem_cache *pool, void *p, char *fn, int ln)
>  	kmem_cache_free(pool, p);
>  }
>  
> -#define gettoken(bufp) strsep(bufp, " -\t\n")
> -
> -static ssize_t
> -chipset_proc_write(struct file *file, const char __user *buffer,
> -		   size_t count, loff_t *ppos)
> +ssize_t chipsetready_store(struct device *dev, struct device_attribute *attr,
> +		const char *buf, size_t count)
>  {
> -	char buf[512];
> -	char *token, *p;
> -
> -	if (count > sizeof(buf) - 1) {
> -		LOGERR("chipset_proc_write: count (%d) exceeds size of buffer (%d)",
> -		     (int) count, (int) sizeof(buffer));
> -		return -EINVAL;
> -	}
> -	if (copy_from_user(buf, buffer, count)) {
> -		LOGERR("chipset_proc_write: copy_from_user failed");
> -		return -EFAULT;
> -	}
> -	buf[count] = '\0';
> -
> -	p = buf;
> -	token = gettoken(&p);
> -
> -	if (strcmp(token, "CALLHOMEDISK_MOUNTED") == 0) {
> -		token = gettoken(&p);
> -		/* The Call Home Disk has been mounted */
> -		if (strcmp(token, "0") == 0)
> -			chipset_events[0] = 1;
> -	} else if (strcmp(token, "MODULES_LOADED") == 0) {
> -		token = gettoken(&p);
> -		/* All modules for the partition have been loaded */
> -		if (strcmp(token, "0") == 0)
> -			chipset_events[1] = 1;
> -	} else if (token == NULL) {
> -		/* No event specified */
> -		LOGERR("No event was specified to send CHIPSET_READY response");
> -		return -1;
> +	char msgtype[64];
> +	int msgparam;
> +
> +	if (sscanf(buf, "%63s %d", msgtype, &msgparam) == 2) {

2 values in one sysfs file?  Not good :(

Why?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs
  2014-07-22 13:56 ` [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs Benjamin Romer
  2014-07-22 23:15   ` Greg KH
@ 2014-07-22 23:17   ` Greg KH
  1 sibling, 0 replies; 15+ messages in thread
From: Greg KH @ 2014-07-22 23:17 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: jkc, driverdev-devel, sparmaintainer

On Tue, Jul 22, 2014 at 09:56:28AM -0400, Benjamin Romer wrote:
> Move the chipsetready proc entry to sysfs under a new directory guest. This
> entry is used by Unisys application software on the guest to acknowledge
> completion of specific events for integration purposes, but these
> acknowledgements are not required for the guest to operate correctly.
> 
> The store function is simplified as well, to use scanf() instead of copying
> the buffer and using strsep().
> 
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> ---
> v2: attribute creation was fixed and checks for controlvm_channel pointer were
> removed. The off-by-one error in the sscanf() was fixed. Error -1 that was 
> being returned was changed to -EINVAL.
> 
>  .../unisys/visorchipset/visorchipset_main.c        | 88 ++++++++--------------
>  1 file changed, 33 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> index a20e21b..74ab15b 100644
> --- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
> +++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> @@ -129,9 +129,6 @@ static MYPROCTYPE *PartitionType;
>  #define VISORCHIPSET_DIAG_PROC_ENTRY_FN "diagdump"
>  static struct proc_dir_entry *diag_proc_dir;
>  
> -#define VISORCHIPSET_CHIPSET_PROC_ENTRY_FN "chipsetready"
> -static struct proc_dir_entry *chipset_proc_dir;
> -
>  #define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN "parahotplug"
>  static struct proc_dir_entry *parahotplug_proc_dir;
>  
> @@ -323,6 +320,10 @@ static ssize_t remaining_steps_store(struct device *dev,
>  	struct device_attribute *attr, const char *buf, size_t count);
>  static DEVICE_ATTR_RW(remaining_steps);
>  
> +static ssize_t chipsetready_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count);
> +static DEVICE_ATTR_WO(chipsetready);
> +
>  static struct attribute *visorchipset_install_attrs[] = {
>  	&dev_attr_toolaction.attr,
>  	&dev_attr_boottotool.attr,
> @@ -337,8 +338,19 @@ static struct attribute_group visorchipset_install_group = {
>  	.attrs = visorchipset_install_attrs
>  };
>  
> +static struct attribute *visorchipset_guest_attrs[] = {
> +	&dev_attr_chipsetready.attr,
> +	NULL
> +};
> +
> +static struct attribute_group visorchipset_guest_group = {
> +	.name = "guest",
> +	.attrs = visorchipset_guest_attrs
> +};
> +
>  static const struct attribute_group *visorchipset_dev_groups[] = {
>  	&visorchipset_install_group,
> +	&visorchipset_guest_group,
>  	NULL
>  };
>  
> @@ -2369,49 +2381,27 @@ visorchipset_cache_free(struct kmem_cache *pool, void *p, char *fn, int ln)
>  	kmem_cache_free(pool, p);
>  }
>  
> -#define gettoken(bufp) strsep(bufp, " -\t\n")
> -
> -static ssize_t
> -chipset_proc_write(struct file *file, const char __user *buffer,
> -		   size_t count, loff_t *ppos)
> +ssize_t chipsetready_store(struct device *dev, struct device_attribute *attr,
> +		const char *buf, size_t count)

Shouldn't this, and your other sysfs file attribute functions, be
static?   You said they were above, but not here, so, which does the
compiler decide to use?  I think the last one, but please be consistant.

Same goes for all of the patches in this series...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/8 v2] staging: unisys: move parahotplug to sysfs
  2014-07-22 13:56 ` [PATCH 5/8 v2] staging: unisys: move parahotplug " Benjamin Romer
@ 2014-07-22 23:18   ` Greg KH
  0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2014-07-22 23:18 UTC (permalink / raw)
  To: Benjamin Romer; +Cc: driverdev-devel, sparmaintainer

On Tue, Jul 22, 2014 at 09:56:29AM -0400, Benjamin Romer wrote:
> Move the /proc/visorchipset/parahotplug interface to sysfs under
> /sys/devices/platform/visorchipset/guest/parahotplug.
> 
> The parahotplug interface is used to deal with recovery situations on s-Par
> guest partitions. The command service partition will send a message to a guest
> when a device that guest is using needs to be temporarily removed. The message
> triggers a udev event that will cause a recovery script to run. When that
> script has completed its work, it will write to the parahotplug interface to
> send a message back to Command indicating that it is safe to remove the device.
> 
> Moving this interface to sysfs orphans the visorchipset_proc_read_writeonly()
> function, so it is also removed.
> 
> Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>
> ---
> v2: attribute creation was fixed and checks for controlvm_channel pointer were
> removed.
> 
>  .../unisys/visorchipset/visorchipset_main.c        | 59 ++++------------------
>  1 file changed, 11 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> index 74ab15b..c88f95f 100644
> --- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
> +++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
> @@ -129,19 +129,12 @@ static MYPROCTYPE *PartitionType;
>  #define VISORCHIPSET_DIAG_PROC_ENTRY_FN "diagdump"
>  static struct proc_dir_entry *diag_proc_dir;
>  
> -#define VISORCHIPSET_PARAHOTPLUG_PROC_ENTRY_FN "parahotplug"
> -static struct proc_dir_entry *parahotplug_proc_dir;
> -
>  static LIST_HEAD(BusInfoList);
>  static LIST_HEAD(DevInfoList);
>  
>  static struct proc_dir_entry *ProcDir;
>  static VISORCHANNEL *ControlVm_channel;
>  
> -static ssize_t visorchipset_proc_read_writeonly(struct file *file,
> -						char __user *buf,
> -						size_t len, loff_t *offset);
> -
>  typedef struct {
>  	U8 __iomem *ptr;	/* pointer to base address of payload pool */
>  	U64 offset;		/* offset from beginning of controlvm
> @@ -324,6 +317,10 @@ static ssize_t chipsetready_store(struct device *dev,
>  		struct device_attribute *attr, const char *buf, size_t count);
>  static DEVICE_ATTR_WO(chipsetready);
>  
> +static ssize_t parahotplug_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count);
> +static DEVICE_ATTR_WO(parahotplug);
> +
>  static struct attribute *visorchipset_install_attrs[] = {
>  	&dev_attr_toolaction.attr,
>  	&dev_attr_boottotool.attr,
> @@ -340,6 +337,7 @@ static struct attribute_group visorchipset_install_group = {
>  
>  static struct attribute *visorchipset_guest_attrs[] = {
>  	&dev_attr_chipsetready.attr,
> +	&dev_attr_parahotplug.attr,
>  	NULL
>  };
>  
> @@ -1812,30 +1810,17 @@ parahotplug_process_message(CONTROLVM_MESSAGE *inmsg)
>  
>  /*
>   * Gets called when the udev script writes to
> - * /proc/visorchipset/parahotplug.  Expects input in the form of "<id>
> - * <active>" where <id> is the identifier passed to the script that
> - * matches a request on the request list, and <active> is 0 or 1
> - * indicating whether the device is now enabled or not.
> + * /sys/devices/platform/visorchipset/guest/parahotplug.
> + * Expects input in the form of "<id> <active>" where <id> is the identifier
> + * passed to the script that matches a request on the request list, and <active>
> + * is 0 or 1 indicating whether the device is now enabled or not.

Why isn't this information in the ABI file?

Also, 2 values for one sysfs file?  Not acceptable, sorry.

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries
  2014-07-22 23:14   ` Greg KH
@ 2014-07-23 13:46     ` Romer, Benjamin M
  0 siblings, 0 replies; 15+ messages in thread
From: Romer, Benjamin M @ 2014-07-23 13:46 UTC (permalink / raw)
  To: Greg KH; +Cc: driverdev-devel, *S-Par-Maintainer

On Tue, 2014-07-22 at 16:14 -0700, Greg KH wrote:

> What is the format of the chipsetready file?  It looks like you want to
> write 2 values to it?  Please describe it better, for all of these.
> 

I think the intent was to pass in a tuple from a script started by a
udev event, as an indication of completion. I'm getting in touch with
the original author of that function to see how we can fix this to do
things correctly in sysfs. I'll send another set of patches once I know
what I need to change in user space to match whatever we end up deciding
on doing in the kernel.

> thanks,
> 
> greg k-h

-- 
Ben Romer | Software Engineer |
Virtual Systems Development 

Unisys Corporation |  2476
Swedesford Rd |  Malvern, PA 19355
|  610-648-7140



_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-07-23 13:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-22 13:56 [PATCH 0/8 v2] staging: unisys: visorchipset proc fixes Benjamin Romer
2014-07-22 13:56 ` [PATCH 1/8 v2] staging: unisys: add toolaction to sysfs Benjamin Romer
2014-07-22 13:56 ` [PATCH 2/8 v2] staging: unisys: move boottotool out of proc " Benjamin Romer
2014-07-22 13:56 ` [PATCH 3/8 v2] staging: unisys: move installer to sysfs and split fields Benjamin Romer
2014-07-22 23:13   ` Greg KH
2014-07-22 13:56 ` [PATCH 4/8 v2] staging: unisys: move chipsetready to sysfs Benjamin Romer
2014-07-22 23:15   ` Greg KH
2014-07-22 23:17   ` Greg KH
2014-07-22 13:56 ` [PATCH 5/8 v2] staging: unisys: move parahotplug " Benjamin Romer
2014-07-22 23:18   ` Greg KH
2014-07-22 13:56 ` [PATCH 6/8 v2] staging: unisys: clean up diagdump proc entry code Benjamin Romer
2014-07-22 13:56 ` [PATCH 7/8 v2] staging: unisys: remove partition information from proc Benjamin Romer
2014-07-22 13:56 ` [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries Benjamin Romer
2014-07-22 23:14   ` Greg KH
2014-07-23 13:46     ` Romer, Benjamin M

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.