Re: [PATCH v3 1/6] staging: unisys: remove DBGINF, DBGVER, DEBUGDEV, and DEBUGDRV macros

2015-03-04 Thread Romer, Benjamin M
On Wed, 2015-03-04 at 18:59 +0300, Dan Carpenter wrote: I don't think you meant to delete this line. No, I didn't, thanks for the catch. I'll fix both of these problems and resubmit. -- Ben ___ devel mailing list de...@linuxdriverproject.org

Re: [PATCH v3 5/6] staging: unisys: remove ERRDEV macros

2015-03-04 Thread Romer, Benjamin M
On Wed, 2015-03-04 at 19:34 +0300, Dan Carpenter wrote: Btw, I'm these patches are quite large. You're going to have to redo them to fix the three behavior changes. I'm hoping I can just diff the two patchsets instead of reviewing everything again so please redo them within the next couple

Re: [PATCH v4 0/6] staging: unisys: remove logging macros from all drivers

2015-03-04 Thread Romer, Benjamin M
On Wed, 2015-03-04 at 22:11 +0300, Dan Carpenter wrote: Looks good to me. Thanks! regards, dan carpenter Thank you for all the help!!! :) -- Ben

Re: [PATCH 1/5] staging: unisys: remove DBGINF, DBGVER, DEBUGDEV, and DEBUGDRV macros

2015-02-26 Thread Romer, Benjamin M
On Thu, 2015-02-26 at 18:07 +0300, Dan Carpenter wrote: Gar... Actually everything is double tabbed. What's the deal with that? There are other examples as well. regards, dan carpenter I'll fix these and resubmit the patch set. -- Ben Romer | Software Engineer | Virtual Systems

Re: [PATCH v2 2/5] staging: unisys: remove LOGINF macros

2015-02-26 Thread Romer, Benjamin M
On Thu, 2015-02-26 at 20:19 +0300, Dan Carpenter wrote: + wake_up_process(thrinfo-task); Please send bug fixes separately. regards, dan carpenter Actually, I think this is a merge error on my part, I'll send a v3 of just this specific patch if that's alright? -- Ben

Re: [PATCH v2 4/5] staging: unisys: remove ERRDEV macros

2015-02-26 Thread Romer, Benjamin M
On Thu, 2015-02-26 at 20:37 +0300, Dan Carpenter wrote: There is only one place which calls this ASSERT() macro. It should probably be changed to: WARN_ON(!pw-is_scheduled); I can do that in a subsequent patch, but my intent was to remove the logging macros, not to fix the

Re: [PATCH] staging: unisys: Rework Kconfig dependencies

2015-02-24 Thread Romer, Benjamin M
On Sat, 2015-02-21 at 13:16 +0100, Jean Delvare wrote: Signed-off-by: Jean Delvare jdelv...@suse.de Cc: Benjamin Romer benjamin.ro...@unisys.com Cc: David Kershner david.kersh...@unisys.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org --- This is an humble proposal if you like it. I

Re: [PATCH 01/30] staging: unisys: serverdown variable change bool to int virthba

2015-02-11 Thread Romer, Benjamin M
On Wed, 2015-02-11 at 11:36 +0300, Dan Carpenter wrote: On Tue, Feb 10, 2015 at 12:58:35PM -0500, Benjamin Romer wrote: From: Erik Arfvidson erik.arfvid...@unisys.com This patch changes serverdown variable to int instead of bool Why? It looks like bool is more appropriate? Hi Dan,

Re: [RFC PATCH 0/5] unisys: kthread cleanup

2015-02-10 Thread Romer, Benjamin M
On Mon, 2015-01-26 at 00:39 -0500, devendra.aaru wrote: Hello, Sorry for the late reply. Would reverting last two patches help ? Thanks, Devendra Devendra, I am really sorry for how long this is taking. I don't think that there's anything wrong with your patches, but rather that

Re: [PATCH v2] staging: unisys: rework signal remove/insert to avoid sparse lock warnings

2015-01-27 Thread Romer, Benjamin M
On Sun, 2015-01-25 at 19:48 +0800, Greg KH wrote: On Sat, Jan 17, 2015 at 10:39:53PM +1100, Zachary Warren wrote: Avoids the following warnings from sparse: visorchannel_funcs.c:457:9: warning: context imbalance in 'visorchannel_signalremove' - different lock contexts for basic block

Re: [PATCH 55/69] staging: unisys: get rid of LOGWRN() macro and uisklog.h

2015-01-22 Thread Romer, Benjamin M
On Thu, 2015-01-22 at 11:19 +0800, Greg KH wrote: On Wed, Jan 21, 2015 at 06:53:31PM +0300, Dan Carpenter wrote: Generally delete code patches are easy to review. But sometimes you have to change formatting and remove variables and curly braces. $ grep LOG drivers/staging/unisys/ -R |

Re: [RFC PATCH 0/5] unisys: kthread cleanup

2015-01-21 Thread Romer, Benjamin M
On Wed, 2015-01-21 at 04:03 -0500, Devendra Naga wrote: The 1st patch (replace kthread_create..) replace the kthread_create and wake_up_process with a call to kthread_run. The 2nd patch changes the library API uisthread_start and uisthread_stop to use the kthread API. The 3rd patch and

Re: [PATCH 55/69] staging: unisys: get rid of LOGWRN() macro and uisklog.h

2015-01-21 Thread Romer, Benjamin M
On Fri, 2015-01-09 at 17:40 -0800, Greg KH wrote: On Fri, Dec 05, 2014 at 05:09:30PM -0500, Benjamin Romer wrote: Get rid of LOGWRN() and all related macros, and call pr_warn() directly instead. Side note, are you setting pr_fmt() properly so that everything is unified with these

Re: [PATCH v2] staging: unisys: remove duplicate header

2014-12-02 Thread Romer, Benjamin M
On Tue, 2014-12-02 at 16:20 +0530, Sudip Mukherjee wrote: these header files were included multiple times Signed-off-by: Sudip Mukherjee su...@vectorindia.org Looks good to me, thanks for the help. :) Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com

Re: [PATCH 00/10] parser header and c file patches

2014-12-01 Thread Romer, Benjamin M
On Mon, 2014-12-01 at 07:26 -0500, Jeffrey Brown wrote: This series of patches made corrections to the camel casing and typedef problems in parser header and parser.c. There is a camel case in parser header where visorchipset_main.c was affected as well. Smaller problems like space after

Re: [PATCH 04/67] staging: unisys: remove stray blank line in timskmod.h

2014-10-02 Thread Romer, Benjamin M
On Thu, 2014-10-02 at 09:41 -0700, Greg KH wrote: - /* @} */ What is that odd comment? I think it's part of a doxygen block that starts above the #defines: /** * @addtogroup driverlogging * @{ */ To make sure, I'll ask the original author when I get the chance. -- Ben

Re: [PATCH 0/9] staging: unisys: checkpatch strict cleanup in include

2014-09-25 Thread Romer, Benjamin M
On Thu, 2014-09-25 at 00:02 +0200, Greg KH wrote: On Wed, Sep 24, 2014 at 07:12:03PM +0300, Dan Carpenter wrote: Gar. I hate to make you redo a lot of work but these need to be broken up into one thing per patch. That rules is kind of vague so we normally tell people to fix one type of

Re: [PATCH 0/9] staging: unisys: checkpatch strict cleanup in include

2014-09-25 Thread Romer, Benjamin M
On Thu, 2014-09-25 at 16:30 +0300, Dan Carpenter wrote: Probably that's fine, but normally we suggest: patch 1: clean up long lines patch 2: delete extra blank lines patch 3: comments or whatever... OK, thanks. I suppose if I'm not sure, I'll make individual patches and I can just

Re: [PATCH 1/9] staging: unisys: clean up periodic_work.c and periodic_work.h

2014-09-25 Thread Romer, Benjamin M
On Wed, 2014-09-24 at 19:34 +0300, Dan Carpenter wrote: On Wed, Sep 24, 2014 at 11:56:19AM -0400, Benjamin Romer wrote: +struct periodic_work * + visor_periodic_work_create(ulong jiffy_interval, + struct workqueue_struct *workqueue, +

Re: Staging: unisys: base drivers complete

2014-09-15 Thread Romer, Benjamin M
Have you actually ran the checkpatch.pl tool on this code? You still have a lot of cleanup to do (hint, typedefs for drivers are not allowed...) Yes, I've been using checkpatch.pl a lot, though admittedly I did not know about --strict. I'll start addressing the check issues as well as the

Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance

2014-09-10 Thread Romer, Benjamin M
On Tue, 2014-09-09 at 16:11 +0530, Sudip Mukherjee wrote: fixed sparse warning : context imbalance in 'resume_device' unexpected unlock this patch will generate warning from checkpatch for lines over 80 character , but since those are user-visible strings so it was not

Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance

2014-09-08 Thread Romer, Benjamin M
On Fri, 2014-09-05 at 14:52 +0530, Sudip Mukherjee wrote: fixed sparse warning : context imbalance in 'pause_device' unexpected unlock this patch will generate warning from checkpatch for lines over 80 character , but since those are user-visible strings so it was not

Re: [PATCH] unisys: Fix sparse error - accessing __iomem directly

2014-09-08 Thread Romer, Benjamin M
On Mon, 2014-09-08 at 17:04 +0300, Dan Carpenter wrote: On Mon, Sep 08, 2014 at 01:44:49PM +0100, Luke Hart wrote: Copy the channel type into a temporary buffer so that code will work for architectures that don't support MMIO. This now works in same way as other tests in same function.

Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance

2014-09-08 Thread Romer, Benjamin M
On Mon, 2014-09-08 at 21:57 +0530, Sudip Mukherjee wrote: Hi Ben, thanks. the same file is having two more similar warnings. if you want i can resend a patch fixing all the three warnings , or i can send two separate patches. I personally will prefer two separate patches , as that will be

Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance

2014-09-08 Thread Romer, Benjamin M
On Mon, 2014-09-08 at 22:08 +0530, Sudip Mukherjee wrote: Hi Ben, sorry to disturb you again. i got confused , which one is perfect one combined patch or separate patches? thanks sudip Two patches, as you preferred. -- Ben ___ devel mailing

Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance

2014-09-08 Thread Romer, Benjamin M
On Mon, 2014-09-08 at 10:06 -0700, Greg Kroah-Hartman wrote: Traditionally, you would respond with a: Acked-by: Developer Name email@address so I can add it to the patch. Care to do that here? thanks, greg k-h Of course. :) Acked-by: Benjamin Romer benjamin.ro...@unisys.com

Re: [PATCH] staging: unisys: uislib: uislib.c: sparse warning of context imbalance

2014-09-08 Thread Romer, Benjamin M
On Mon, 2014-09-08 at 10:24 -0700, Greg Kroah-Hartman wrote: If you test it, do a tested-by. If you sign off on it (i.e. it flows through you to me), then a signed-off is correct. Hope this helps, greg k-h It does. :) We should add the Tested-by then, too. Acked-by: Benjamin Romer

Re: [PATCH 11/14] staging: unisys: remove do{} while(0) in macros in channel.h

2014-08-06 Thread Romer, Benjamin M
On Wed, 2014-08-06 at 11:18 +0300, Dan Carpenter wrote: On Tue, Aug 05, 2014 at 02:57:55PM -0400, Benjamin Romer wrote: The CHANNEL_*_MISMATCH error message macros should not be inside of do blocks. Why not? We do that so they can be called like a function. These seem to not be

Re: [PATCH v5] staging: unisys: move parahotplug to sysfs

2014-07-29 Thread Romer, Benjamin M
On Tue, 2014-07-29 at 16:58 +0300, Dan Carpenter wrote: This is broken code which clearly hasn't been tested. Wat??? Oh, I think I see what you mean. When I said tested, I meant that the entries appear in sysfs instead of proc, not that they pass data anywhere or work with s-Par correctly. I

Re: [PATCH 1/6 v3] staging: unisys: move installer to sysfs and split fields

2014-07-25 Thread Romer, Benjamin M
On Fri, 2014-07-25 at 14:05 +0300, Dan Carpenter wrote: visorchannel_write() returns either 0 or -EFAULT so this condition is never true. This bug occurs in several places. Thank you for catching my mistake. I'll fix this and change visorchannel_write() to return -EIO as you suggested.

Re: [PATCH 8/8 v2] staging: unisys: ABI documentation for new sysfs entries

2014-07-23 Thread Romer, Benjamin M
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

Re: [PATCH 1/8] staging: unisys: add toolaction to sysfs

2014-07-21 Thread Romer, Benjamin M
On Sat, 2014-07-19 at 09:36 -0700, Greg KH wrote: /* /sys/devices/platform/visorchipset */ static struct platform_device Visorchipset_platform_device = { .name = visorchipset, .id = -1, + .dev.groups = visorchipset_dev_groups, Only create this device when

Re: [PATCH 02/10] staging: unisys: add toolaction to sysfs

2014-07-16 Thread Romer, Benjamin M
On Tue, 2014-07-15 at 21:50 -0700, Greg KH wrote: On Tue, Jul 15, 2014 at 01:30:42PM -0400, Benjamin Romer wrote: All sysfs files need a Documentation/ABI/ entry. As this isn't in the real part of the kernel yet, just create the entries in the unisys/ subdir and then when it moves out, we can

Re: [PATCH 2/2] staging: unisys: added virtpci info entry

2014-07-11 Thread Romer, Benjamin M
On Thu, 2014-07-10 at 07:51 -0700, Greg KH wrote: On Thu, Jul 10, 2014 at 10:34:14AM -0400, Erik Arfvidson wrote: This patch adds the virtpci debugfs directory and the info entry inside of it. Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com Signed-off-by: Benjamin Romer

Re: [PATCH 2/7] staging: unisys: move uislib/platform proc entry to debugfs

2014-05-20 Thread Romer, Benjamin M
On Tue, 2014-05-20 at 10:09 +0900, Greg KH wrote: On Mon, May 19, 2014 at 10:57:08PM +0300, Dan Carpenter wrote: On Mon, May 19, 2014 at 09:42:22AM -0500, Romer, Benjamin M wrote: On Sun, 2014-05-18 at 09:49 -0700, Greg KH wrote: Also, why are these entries moving to debugfs at all? Why

Re: [PATCH 2/7] staging: unisys: move uislib/platform proc entry to debugfs

2014-05-19 Thread Romer, Benjamin M
On Sun, 2014-05-18 at 09:49 -0700, Greg KH wrote: There's no need to keep this dentry around, you can just remove all the debugfs files in your directory recursively when you exit. That will save you code and logic overall. Ah, thanks! I wasn't aware that I could do that. That's an easy fix.

[PATCH] staging: unisys: fix copyright notices

2014-04-10 Thread Romer, Benjamin M
This patch replaces the inconsistent copyright symbols in each source file with (C). Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- drivers/staging/unisys/channels/channel.c | 2 +- drivers/staging/unisys/channels/chanstub.c

Re: [visorchipset] invalid opcode: 0000 [#1] PREEMPT SMP

2014-04-08 Thread Romer, Benjamin M
On Tue, 2014-04-08 at 10:53 +0800, Fengguang Wu wrote: Hi Benjamin, Fengguang, I ran your script against freshly-checked-out source from staging-next, and was not able to reproduce the error with it. My boot log is attached. I noticed that your log did not have Hypervisor detected:

Re: [PATCH 1/1] staging: unisys: Fix MAINTAINERS

2014-03-07 Thread Romer, Benjamin M
On Thu, 2014-03-06 at 12:01 -0800, gre...@linuxfoundation.org wrote: On Thu, Mar 06, 2014 at 10:01:04AM -0600, Romer, Benjamin M wrote: This patch updates the MAINTAINERS file to add the maintainer for the Unisys s-Par driver set. Signed-off-by: Ken Cox j...@redhat.com signed-off

Re: [PATCH 1/1] staging: unisys: Fix MAINTAINERS

2014-03-07 Thread Romer, Benjamin M
On Fri, 2014-03-07 at 17:41 +0300, Dan Carpenter wrote: On Fri, Mar 07, 2014 at 08:18:44AM -0600, Romer, Benjamin M wrote: On Thu, 2014-03-06 at 12:01 -0800, gre...@linuxfoundation.org wrote: On Thu, Mar 06, 2014 at 10:01:04AM -0600, Romer, Benjamin M wrote: This patch updates

[PATCH 1/1] staging: unisys: update MAINTAINERS and TODO

2014-03-07 Thread Romer, Benjamin M
This patch adds the Unisys s-Par driver maintainers to the MAINTAINERS file, changes the state to Supported, modifies TODO to address patches to the Unisys mailing list, and adds Greg Kroah-Hartman to the patch recipients list. Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com --- diff

[PATCH 1/1] staging: unisys: Fix MAINTAINERS

2014-03-06 Thread Romer, Benjamin M
This patch updates the MAINTAINERS file to add the maintainer for the Unisys s-Par driver set. Signed-off-by: Ken Cox j...@redhat.com signed-off-by: Ben Romer sparmaintai...@unisys.com diff --git a/MAINTAINERS b/MAINTAINERS index c3ff623..06dc169 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@

Re: [PATCH 3/3] staging: unisys: Fix MAINTAINERS and TODO

2014-03-06 Thread Romer, Benjamin M
On Thu, 2014-03-06 at 08:47 -0600, Ken Cox wrote: On 03/05/2014 10:40 PM, Greg KH wrote: On Wed, Mar 05, 2014 at 02:52:26PM -0600, Ken Cox wrote: Add the Unisys s-Par maintainer entry to the MAINTAINERS file. Add Greg Kroah-Hartman to the list of patch recipients in the TODO file

Re: [patch 1/8] staging: visorutil driver to provide common functionality to other s-Par drivers

2014-03-03 Thread Romer, Benjamin M
Hi Greg, The copyright text is old boilerplate that we'd been carrying. We'll change it so that it's correctly GPL 2 only, and we'll fix all of the EXPORT_SYMBOL()s also. The majority of our proc entries are switches for debugging or purely informational, so these could be moved to /sys with