[PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[David Kershner]

This patchset moves drivers/staging/unisys/include to
include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
drivers/virt/visorbus.

David Kershner (3):
  include: linux: visorbus: Add visorbus to include/linux directory
  Documentation: Move visorbus documentation from staging to
    Documentation/
  drivers: virt: Add visorbus to the drivers/virt directory

 .../ABI/stable/sysfs-bus-visorbus                             |  0
 .../Documentation/overview.txt => Documentation/visorbus.txt  |  0
 MAINTAINERS                                                   |  4 ++++
 drivers/staging/unisys/Kconfig                                |  1 -
 drivers/staging/unisys/Makefile                               |  1 -
 drivers/staging/unisys/visorbus/Makefile                      | 11 -----------
 drivers/staging/unisys/visorhba/Makefile                      |  3 ---
 drivers/staging/unisys/visorhba/visorhba_main.c               |  4 ++--
 drivers/staging/unisys/visorinput/Makefile                    |  2 --
 drivers/staging/unisys/visorinput/visorinput.c                |  2 +-
 drivers/staging/unisys/visornic/Makefile                      |  2 --
 drivers/staging/unisys/visornic/visornic_main.c               |  5 ++---
 drivers/virt/Kconfig                                          |  1 +
 drivers/virt/Makefile                                         |  1 +
 drivers/{staging/unisys => virt}/visorbus/Kconfig             |  2 +-
 drivers/virt/visorbus/Makefile                                |  9 +++++++++
 drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h  |  2 +-
 drivers/{staging/unisys => virt}/visorbus/vbuschannel.h       |  2 +-
 drivers/{staging/unisys => virt}/visorbus/visorbus_main.c     |  2 +-
 drivers/{staging/unisys => virt}/visorbus/visorbus_private.h  |  0
 drivers/{staging/unisys => virt}/visorbus/visorchannel.c      |  2 +-
 drivers/{staging/unisys => virt}/visorbus/visorchipset.c      |  2 +-
 drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h   |  0
 .../unisys/include => include/linux/visorbus}/channel.h       |  0
 .../unisys/include => include/linux/visorbus}/iochannel.h     |  2 +-
 .../unisys/include => include/linux/visorbus}/visorbus.h      |  3 +--
 26 files changed, 28 insertions(+), 35 deletions(-)
 rename drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset => Documentation/ABI/stable/sysfs-bus-visorbus (100%)
 rename drivers/staging/unisys/Documentation/overview.txt => Documentation/visorbus.txt (100%)
 delete mode 100644 drivers/staging/unisys/visorbus/Makefile
 rename drivers/{staging/unisys => virt}/visorbus/Kconfig (95%)
 create mode 100644 drivers/virt/visorbus/Makefile
 rename drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h (99%)
 rename drivers/{staging/unisys => virt}/visorbus/vbuschannel.h (98%)
 rename drivers/{staging/unisys => virt}/visorbus/visorbus_main.c (99%)
 rename drivers/{staging/unisys => virt}/visorbus/visorbus_private.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorchannel.c (99%)
 rename drivers/{staging/unisys => virt}/visorbus/visorchipset.c (99%)
 rename drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/channel.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/iochannel.h (99%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h (99%)

-- 
1.9.1

[PATCH 1/3] include: linux: visorbus: Add visorbus to include/linux directory

[David Kershner]

Update include/linux to include the s-Par drivers set's common header
files, which are  needed for the s-Par visorbus.

Since the include directories are now moved to include/linux/visorbus, this
patch makes all the s-Par drivers (visorbus, visorinput, visornic, and
visorhba) use the new include folder.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <timothy.sell@unisys.com>
---
 MAINTAINERS                                                          | 1 +
 drivers/staging/unisys/visorbus/Makefile                             | 2 --
 drivers/staging/unisys/visorbus/controlvmchannel.h                   | 2 +-
 drivers/staging/unisys/visorbus/vbuschannel.h                        | 2 +-
 drivers/staging/unisys/visorbus/visorbus_main.c                      | 2 +-
 drivers/staging/unisys/visorbus/visorchannel.c                       | 2 +-
 drivers/staging/unisys/visorbus/visorchipset.c                       | 2 +-
 drivers/staging/unisys/visorhba/Makefile                             | 3 ---
 drivers/staging/unisys/visorhba/visorhba_main.c                      | 4 ++--
 drivers/staging/unisys/visorinput/Makefile                           | 2 --
 drivers/staging/unisys/visorinput/visorinput.c                       | 2 +-
 drivers/staging/unisys/visornic/Makefile                             | 2 --
 drivers/staging/unisys/visornic/visornic_main.c                      | 5 ++---
 {drivers/staging/unisys/include => include/linux/visorbus}/channel.h | 0
 .../staging/unisys/include => include/linux/visorbus}/iochannel.h    | 2 +-
 .../staging/unisys/include => include/linux/visorbus}/visorbus.h     | 3 +--
 16 files changed, 13 insertions(+), 23 deletions(-)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/channel.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/iochannel.h (99%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e98464..0893d53 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13116,6 +13116,7 @@ M:	David Kershner <david.kershner@unisys.com>
 L:	sparmaintainer@unisys.com (Unisys internal)
 S:	Supported
 F:	drivers/staging/unisys/
+F:	include/linux/visorbus/
 
 UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER
 M:	Vinayak Holikatti <vinholikatti@gmail.com>
diff --git a/drivers/staging/unisys/visorbus/Makefile b/drivers/staging/unisys/visorbus/Makefile
index f3730d8..7f328cc 100644
--- a/drivers/staging/unisys/visorbus/Makefile
+++ b/drivers/staging/unisys/visorbus/Makefile
@@ -7,5 +7,3 @@ obj-$(CONFIG_UNISYS_VISORBUS)	+= visorbus.o
 visorbus-y := visorbus_main.o
 visorbus-y += visorchannel.o
 visorbus-y += visorchipset.o
-
-ccflags-y += -Idrivers/staging/unisys/include
diff --git a/drivers/staging/unisys/visorbus/controlvmchannel.h b/drivers/staging/unisys/visorbus/controlvmchannel.h
index ed045ef..875603b 100644
--- a/drivers/staging/unisys/visorbus/controlvmchannel.h
+++ b/drivers/staging/unisys/visorbus/controlvmchannel.h
@@ -16,7 +16,7 @@
 #define __CONTROLVMCHANNEL_H__
 
 #include <linux/uuid.h>
-#include "channel.h"
+#include <linux/visorbus/channel.h>
 
 /* {2B3C2D10-7EF5-4ad8-B966-3448B7386B3D} */
 #define VISOR_CONTROLVM_CHANNEL_UUID \
diff --git a/drivers/staging/unisys/visorbus/vbuschannel.h b/drivers/staging/unisys/visorbus/vbuschannel.h
index 01d7d51..dd9df0a 100644
--- a/drivers/staging/unisys/visorbus/vbuschannel.h
+++ b/drivers/staging/unisys/visorbus/vbuschannel.h
@@ -24,7 +24,7 @@
  */
 #include <linux/uuid.h>
 #include <linux/ctype.h>
-#include "channel.h"
+#include <linux/visorbus/channel.h>
 
 /* {193b331b-c58f-11da-95a9-00e08161165f} */
 #define VISOR_VBUS_CHANNEL_UUID \
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 1c785dd..bf759e7 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -16,8 +16,8 @@
 
 #include <linux/debugfs.h>
 #include <linux/uuid.h>
+#include <linux/visorbus/visorbus.h>
 
-#include "visorbus.h"
 #include "visorbus_private.h"
 
 #define MYDRVNAME "visorbus"
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
index 6885c2c..8c05af6 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/staging/unisys/visorbus/visorchannel.c
@@ -21,8 +21,8 @@
 
 #include <linux/uuid.h>
 #include <linux/io.h>
+#include <linux/visorbus/visorbus.h>
 
-#include "visorbus.h"
 #include "visorbus_private.h"
 #include "controlvmchannel.h"
 
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 2215056..1324849 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -22,8 +22,8 @@
 #include <linux/netdevice.h>
 #include <linux/uuid.h>
 #include <linux/crash_dump.h>
+#include <linux/visorbus/visorbus.h>
 
-#include "visorbus.h"
 #include "visorbus_private.h"
 #include "vmcallinterface.h"
 
diff --git a/drivers/staging/unisys/visorhba/Makefile b/drivers/staging/unisys/visorhba/Makefile
index a8a8e0e..c321f6a 100644
--- a/drivers/staging/unisys/visorhba/Makefile
+++ b/drivers/staging/unisys/visorhba/Makefile
@@ -5,6 +5,3 @@
 obj-$(CONFIG_UNISYS_VISORHBA)	+= visorhba.o
 
 visorhba-y := visorhba_main.o
-
-ccflags-y += -Idrivers/staging/unisys/include
-
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 2fd31c9..75ca181 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -18,13 +18,13 @@
 #include <linux/kthread.h>
 #include <linux/idr.h>
 #include <linux/seq_file.h>
+#include <linux/visorbus/visorbus.h>
+#include <linux/visorbus/iochannel.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_device.h>
 
-#include "visorbus.h"
-#include "iochannel.h"
 
 /* The Send and Receive Buffers of the IO Queue may both be full */
 
diff --git a/drivers/staging/unisys/visorinput/Makefile b/drivers/staging/unisys/visorinput/Makefile
index beedca7..87426a0 100644
--- a/drivers/staging/unisys/visorinput/Makefile
+++ b/drivers/staging/unisys/visorinput/Makefile
@@ -3,5 +3,3 @@
 #
 
 obj-$(CONFIG_UNISYS_VISORINPUT)	+= visorinput.o
-
-ccflags-y += -Idrivers/staging/unisys/include
diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index 45bc340..7e5594e 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -28,8 +28,8 @@
 #include <linux/uaccess.h>
 #include <linux/kernel.h>
 #include <linux/uuid.h>
+#include <linux/visorbus/visorbus.h>
 
-#include "visorbus.h"
 #include "ultrainputreport.h"
 
 /* Keyboard channel {c73416d0-b0b8-44af-b304-9d2ae99f1b3d} */
diff --git a/drivers/staging/unisys/visornic/Makefile b/drivers/staging/unisys/visornic/Makefile
index 439e95e..43985bb 100644
--- a/drivers/staging/unisys/visornic/Makefile
+++ b/drivers/staging/unisys/visornic/Makefile
@@ -6,5 +6,3 @@ obj-$(CONFIG_UNISYS_VISORNIC)	+= visornic.o
 
 visornic-y := visornic_main.o
 
-ccflags-y += -Idrivers/staging/unisys/include
-
diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 2891622..1663945 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -24,9 +24,8 @@
 #include <linux/kthread.h>
 #include <linux/skbuff.h>
 #include <linux/rtnetlink.h>
-
-#include "visorbus.h"
-#include "iochannel.h"
+#include <linux/visorbus/visorbus.h>
+#include <linux/visorbus/iochannel.h>
 
 #define VISORNIC_INFINITE_RSP_WAIT 0
 
diff --git a/drivers/staging/unisys/include/channel.h b/include/linux/visorbus/channel.h
similarity index 100%
rename from drivers/staging/unisys/include/channel.h
rename to include/linux/visorbus/channel.h
diff --git a/drivers/staging/unisys/include/iochannel.h b/include/linux/visorbus/iochannel.h
similarity index 99%
rename from drivers/staging/unisys/include/iochannel.h
rename to include/linux/visorbus/iochannel.h
index c7cb3fb..c44b9b2 100644
--- a/drivers/staging/unisys/include/iochannel.h
+++ b/include/linux/visorbus/iochannel.h
@@ -32,7 +32,7 @@
 #include <linux/uuid.h>
 
 #include <linux/dma-direction.h>
-#include "channel.h"
+#include <linux/visorbus/channel.h>
 
 #define VISOR_VHBA_CHANNEL_SIGNATURE VISOR_CHANNEL_SIGNATURE
 #define VISOR_VNIC_CHANNEL_SIGNATURE VISOR_CHANNEL_SIGNATURE
diff --git a/drivers/staging/unisys/include/visorbus.h b/include/linux/visorbus/visorbus.h
similarity index 99%
rename from drivers/staging/unisys/include/visorbus.h
rename to include/linux/visorbus/visorbus.h
index de06355..9814b21 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/include/linux/visorbus/visorbus.h
@@ -36,8 +36,7 @@
 #include <linux/uuid.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
-
-#include "channel.h"
+#include <linux/visorbus/channel.h>
 
 struct visor_driver;
 struct visor_device;
-- 
1.9.1

Re: [PATCH 1/3] include: linux: visorbus: Add visorbus to include/linux directory

[Christoph Hellwig]

Please don't send any move patches but the actual code added to the
kernel proper.  And based on what's in linux-next I don't think this
giant pile of junk is anywhere near mergeable.

[PATCH 2/3] Documentation: Move visorbus documentation from staging to Documentation/

[David Kershner]

This patch simply does a git mv of the
drivers/staging/unisys/Documentation directory to Documentation. It also
renames overview.txt to visorbus.txt and renames sysfs-platform-visorchipset
to the correct name sysfs-bus-visorbus.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <timothy.sell@unisys.com>
---
 .../ABI/stable/sysfs-bus-visorbus                                       | 0
 .../unisys/Documentation/overview.txt => Documentation/visorbus.txt     | 0
 MAINTAINERS                                                             | 2 ++
 3 files changed, 2 insertions(+)
 rename drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset => Documentation/ABI/stable/sysfs-bus-visorbus (100%)
 rename drivers/staging/unisys/Documentation/overview.txt => Documentation/visorbus.txt (100%)

diff --git a/drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset b/Documentation/ABI/stable/sysfs-bus-visorbus
similarity index 100%
rename from drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset
rename to Documentation/ABI/stable/sysfs-bus-visorbus
diff --git a/drivers/staging/unisys/Documentation/overview.txt b/Documentation/visorbus.txt
similarity index 100%
rename from drivers/staging/unisys/Documentation/overview.txt
rename to Documentation/visorbus.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 0893d53..7a92b67 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13117,6 +13117,8 @@ L:	sparmaintainer@unisys.com (Unisys internal)
 S:	Supported
 F:	drivers/staging/unisys/
 F:	include/linux/visorbus/
+F:	Documentation/ABI/stable/sysfs-bus-visorbus
+F:	Documentation/visorbus.txt
 
 UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER
 M:	Vinayak Holikatti <vinholikatti@gmail.com>
-- 
1.9.1

Re: [PATCH 2/3] Documentation: Move visorbus documentation from staging to Documentation/

[Jani Nikula]

On Mon, 05 Jun 2017, David Kershner <david.kershner@unisys.com> wrote:
> This patch simply does a git mv of the
> drivers/staging/unisys/Documentation directory to Documentation.

Up to Jon, of course, but I wouldn't add any new files directly under
Documentation, and especially not something as specific as this.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH 3/3] drivers: virt: Add visorbus to the drivers/virt directory

[David Kershner]

This patch moves visorbus from drivers/staging/visorbus to
drivers/virt.

Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: Tim Sell <timothy.sell@unisys.com>
---
 MAINTAINERS                                                  | 1 +
 drivers/staging/unisys/Kconfig                               | 1 -
 drivers/staging/unisys/Makefile                              | 1 -
 drivers/virt/Kconfig                                         | 1 +
 drivers/virt/Makefile                                        | 1 +
 drivers/{staging/unisys => virt}/visorbus/Kconfig            | 2 +-
 drivers/{staging/unisys => virt}/visorbus/Makefile           | 0
 drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h | 0
 drivers/{staging/unisys => virt}/visorbus/vbuschannel.h      | 0
 drivers/{staging/unisys => virt}/visorbus/visorbus_main.c    | 0
 drivers/{staging/unisys => virt}/visorbus/visorbus_private.h | 0
 drivers/{staging/unisys => virt}/visorbus/visorchannel.c     | 0
 drivers/{staging/unisys => virt}/visorbus/visorchipset.c     | 0
 drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h  | 0
 14 files changed, 4 insertions(+), 3 deletions(-)
 rename drivers/{staging/unisys => virt}/visorbus/Kconfig (95%)
 rename drivers/{staging/unisys => virt}/visorbus/Makefile (100%)
 rename drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/vbuschannel.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorbus_main.c (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorbus_private.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorchannel.c (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorchipset.c (100%)
 rename drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a92b67..bfd3bb7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13119,6 +13119,7 @@ F:	drivers/staging/unisys/
 F:	include/linux/visorbus/
 F:	Documentation/ABI/stable/sysfs-bus-visorbus
 F:	Documentation/visorbus.txt
+F:	drivers/virt/visorbus
 
 UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER
 M:	Vinayak Holikatti <vinholikatti@gmail.com>
diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index 4f1f5e6..688b5e3 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -11,7 +11,6 @@ menuconfig UNISYSSPAR
 
 if UNISYSSPAR
 
-source "drivers/staging/unisys/visorbus/Kconfig"
 source "drivers/staging/unisys/visornic/Kconfig"
 source "drivers/staging/unisys/visorinput/Kconfig"
 source "drivers/staging/unisys/visorhba/Kconfig"
diff --git a/drivers/staging/unisys/Makefile b/drivers/staging/unisys/Makefile
index 20eb098..e45f44b 100644
--- a/drivers/staging/unisys/Makefile
+++ b/drivers/staging/unisys/Makefile
@@ -1,7 +1,6 @@
 #
 # Makefile for Unisys SPAR drivers
 #
-obj-$(CONFIG_UNISYS_VISORBUS)		+= visorbus/
 obj-$(CONFIG_UNISYS_VISORNIC)		+= visornic/
 obj-$(CONFIG_UNISYS_VISORINPUT)		+= visorinput/
 obj-$(CONFIG_UNISYS_VISORHBA)		+= visorhba/
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 99ebdde..b13acff 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -30,4 +30,5 @@ config FSL_HV_MANAGER
           4) A kernel interface for receiving callbacks when a managed
 	     partition shuts down.
 
+source "drivers/virt/visorbus/Kconfig"
 endif
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index c47f04d..f3f49f6 100644
--- a/drivers/virt/Makefile
+++ b/drivers/virt/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_FSL_HV_MANAGER)	+= fsl_hypervisor.o
+obj-$(CONFIG_UNISYS_VISORBUS)  += visorbus/
diff --git a/drivers/staging/unisys/visorbus/Kconfig b/drivers/virt/visorbus/Kconfig
similarity index 95%
rename from drivers/staging/unisys/visorbus/Kconfig
rename to drivers/virt/visorbus/Kconfig
index 5113880..49c1c75 100644
--- a/drivers/staging/unisys/visorbus/Kconfig
+++ b/drivers/virt/visorbus/Kconfig
@@ -4,7 +4,7 @@
 
 config UNISYS_VISORBUS
 	tristate "Unisys visorbus driver"
-	depends on UNISYSSPAR
+	depends on VIRT_DRIVERS
 	---help---
 	The visorbus driver is a virtualized bus for the Unisys s-Par firmware.
 	Virtualized devices allow Linux guests on a system to share disks and
diff --git a/drivers/staging/unisys/visorbus/Makefile b/drivers/virt/visorbus/Makefile
similarity index 100%
rename from drivers/staging/unisys/visorbus/Makefile
rename to drivers/virt/visorbus/Makefile
diff --git a/drivers/staging/unisys/visorbus/controlvmchannel.h b/drivers/virt/visorbus/controlvmchannel.h
similarity index 100%
rename from drivers/staging/unisys/visorbus/controlvmchannel.h
rename to drivers/virt/visorbus/controlvmchannel.h
diff --git a/drivers/staging/unisys/visorbus/vbuschannel.h b/drivers/virt/visorbus/vbuschannel.h
similarity index 100%
rename from drivers/staging/unisys/visorbus/vbuschannel.h
rename to drivers/virt/visorbus/vbuschannel.h
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/virt/visorbus/visorbus_main.c
similarity index 100%
rename from drivers/staging/unisys/visorbus/visorbus_main.c
rename to drivers/virt/visorbus/visorbus_main.c
diff --git a/drivers/staging/unisys/visorbus/visorbus_private.h b/drivers/virt/visorbus/visorbus_private.h
similarity index 100%
rename from drivers/staging/unisys/visorbus/visorbus_private.h
rename to drivers/virt/visorbus/visorbus_private.h
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/virt/visorbus/visorchannel.c
similarity index 100%
rename from drivers/staging/unisys/visorbus/visorchannel.c
rename to drivers/virt/visorbus/visorchannel.c
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/virt/visorbus/visorchipset.c
similarity index 100%
rename from drivers/staging/unisys/visorbus/visorchipset.c
rename to drivers/virt/visorbus/visorchipset.c
diff --git a/drivers/staging/unisys/visorbus/vmcallinterface.h b/drivers/virt/visorbus/vmcallinterface.h
similarity index 100%
rename from drivers/staging/unisys/visorbus/vmcallinterface.h
rename to drivers/virt/visorbus/vmcallinterface.h
-- 
1.9.1

Re: [PATCH 3/3] drivers: virt: Add visorbus to the drivers/virt directory

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 14078 bytes --]

Hi David,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.12-rc4 next-20170606]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/David-Kershner/move-visorbus-out-of-staging-to-drivers-virt-visorbus/20170606-070850
config: m68k-allyesconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/virt/visorbus/visorchipset.c: In function 'save_crash_message':
>> drivers/virt/visorbus/visorchipset.c:479:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:485:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:495:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:508:37: error: dereferencing pointer to incomplete type
       dev_err(&chipset_dev->acpi_device->dev,
                                        ^
   drivers/virt/visorbus/visorchipset.c:519:37: error: dereferencing pointer to incomplete type
       dev_err(&chipset_dev->acpi_device->dev,
                                        ^
   drivers/virt/visorbus/visorchipset.c:525:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorbus_create':
   drivers/virt/visorbus/visorchipset.c:582:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorbus_configure':
   drivers/virt/visorbus/visorchipset.c:734:35: error: dereferencing pointer to incomplete type
     dev_err(&chipset_dev->acpi_device->dev,
                                      ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorbus_device_create':
   drivers/virt/visorbus/visorchipset.c:755:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:762:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:770:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:796:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorbus_device_changestate':
   drivers/virt/visorbus/visorchipset.c:896:35: error: dereferencing pointer to incomplete type
     dev_err(&chipset_dev->acpi_device->dev, "failed: %d\n", err);
                                      ^
   drivers/virt/visorbus/visorchipset.c: In function 'parahotplug_request_kickoff':
   drivers/virt/visorbus/visorchipset.c:1188:53: error: dereferencing pointer to incomplete type
     return kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj,
                                                        ^
   drivers/virt/visorbus/visorchipset.c: In function 'chipset_ready_uevent':
   drivers/virt/visorbus/visorchipset.c:1255:48: error: dereferencing pointer to incomplete type
     res = kobject_uevent(&chipset_dev->acpi_device->dev.kobj,
                                                   ^
   drivers/virt/visorbus/visorchipset.c: In function 'chipset_selftest_uevent':
   drivers/virt/visorbus/visorchipset.c:1279:52: error: dereferencing pointer to incomplete type
     res = kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj,
                                                       ^
   drivers/virt/visorbus/visorchipset.c: In function 'chipset_notready_uevent':
   drivers/virt/visorbus/visorchipset.c:1300:48: error: dereferencing pointer to incomplete type
     res = kobject_uevent(&chipset_dev->acpi_device->dev.kobj,
                                                   ^
   drivers/virt/visorbus/visorchipset.c: In function 'unisys_vmcall':
   drivers/virt/visorbus/visorchipset.c:1316:2: warning: right shift count >= width of type
     reg_ecx = param >> 32;
     ^
   drivers/virt/visorbus/visorchipset.c:1318:2: error: implicit declaration of function 'cpuid' [-Werror=implicit-function-declaration]
     cpuid(0x00000001, &cpuid_eax, &cpuid_ebx, &cpuid_ecx, &cpuid_edx);
     ^
   drivers/virt/visorbus/visorchipset.c: In function 'setup_crash_devices_work_queue':
   drivers/virt/visorbus/visorchipset.c:1388:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:1394:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:1404:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:1414:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:1425:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:1432:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c:1440:36: error: dereferencing pointer to incomplete type
      dev_err(&chipset_dev->acpi_device->dev,
                                       ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorchipset_init':
   drivers/virt/visorbus/visorchipset.c:1831:13: error: dereferencing pointer to incomplete type
     acpi_device->driver_data = chipset_dev;
                ^
   drivers/virt/visorbus/visorchipset.c:1843:53: error: dereferencing pointer to incomplete type
     err = sysfs_create_groups(&chipset_dev->acpi_device->dev.kobj,
                                                        ^
   drivers/virt/visorbus/visorchipset.c:1875:47: error: dereferencing pointer to incomplete type
     sysfs_remove_groups(&chipset_dev->acpi_device->dev.kobj,
                                                  ^
   drivers/virt/visorbus/visorchipset.c:1885:22: error: dereferencing pointer to incomplete type
     dev_err(&acpi_device->dev, "failed with error %d\n", err);
                         ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorchipset_exit':
   drivers/virt/visorbus/visorchipset.c:1894:47: error: dereferencing pointer to incomplete type
     sysfs_remove_groups(&chipset_dev->acpi_device->dev.kobj,
                                                  ^
   drivers/virt/visorbus/visorchipset.c: At top level:
>> drivers/virt/visorbus/visorchipset.c:1903:36: error: array type has incomplete element type
    static const struct acpi_device_id unisys_device_ids[] = {
                                       ^
   drivers/virt/visorbus/visorchipset.c:1908:15: error: variable 'unisys_acpi_driver' has initializer but incomplete type
    static struct acpi_driver unisys_acpi_driver = {
                  ^
   drivers/virt/visorbus/visorchipset.c:1909:2: error: unknown field 'name' specified in initializer
     .name = "unisys_acpi",
     ^
   drivers/virt/visorbus/visorchipset.c:1909:2: warning: excess elements in struct initializer
   drivers/virt/visorbus/visorchipset.c:1909:2: warning: (near initialization for 'unisys_acpi_driver')
   drivers/virt/visorbus/visorchipset.c:1910:2: error: unknown field 'class' specified in initializer
     .class = "unisys_acpi_class",
     ^
   drivers/virt/visorbus/visorchipset.c:1910:2: warning: excess elements in struct initializer
   drivers/virt/visorbus/visorchipset.c:1910:2: warning: (near initialization for 'unisys_acpi_driver')
   drivers/virt/visorbus/visorchipset.c:1911:2: error: unknown field 'owner' specified in initializer
     .owner = THIS_MODULE,
     ^
   In file included from include/linux/linkage.h:6:0,
                    from include/linux/kernel.h:6,
                    from include/linux/list.h:8,
                    from include/linux/resource_ext.h:17,
                    from include/linux/acpi.h:26,
                    from drivers/virt/visorbus/visorchipset.c:17:
   include/linux/export.h:37:30: warning: excess elements in struct initializer
    #define THIS_MODULE ((struct module *)0)
                                 ^
   drivers/virt/visorbus/visorchipset.c:1911:11: note: in expansion of macro 'THIS_MODULE'
     .owner = THIS_MODULE,
              ^
   include/linux/export.h:37:30: warning: (near initialization for 'unisys_acpi_driver')
    #define THIS_MODULE ((struct module *)0)
                                 ^
   drivers/virt/visorbus/visorchipset.c:1911:11: note: in expansion of macro 'THIS_MODULE'
     .owner = THIS_MODULE,
              ^
   drivers/virt/visorbus/visorchipset.c:1912:2: error: unknown field 'ids' specified in initializer
     .ids = unisys_device_ids,
     ^
   drivers/virt/visorbus/visorchipset.c:1912:2: warning: excess elements in struct initializer
   drivers/virt/visorbus/visorchipset.c:1912:2: warning: (near initialization for 'unisys_acpi_driver')
   drivers/virt/visorbus/visorchipset.c:1913:2: error: unknown field 'ops' specified in initializer
     .ops = {
     ^
   drivers/virt/visorbus/visorchipset.c:1913:2: error: extra brace group at end of initializer
   drivers/virt/visorbus/visorchipset.c:1913:2: error: (near initialization for 'unisys_acpi_driver')
   drivers/virt/visorbus/visorchipset.c:1916:2: warning: excess elements in struct initializer
     },
     ^
   drivers/virt/visorbus/visorchipset.c:1916:2: warning: (near initialization for 'unisys_acpi_driver')
   drivers/virt/visorbus/visorchipset.c: In function 'visorutil_spar_detect':
   drivers/virt/visorbus/visorchipset.c:1925:2: error: implicit declaration of function 'boot_cpu_has' [-Werror=implicit-function-declaration]
     if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
     ^
   drivers/virt/visorbus/visorchipset.c:1925:19: error: 'X86_FEATURE_HYPERVISOR' undeclared (first use in this function)
     if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
                      ^
   drivers/virt/visorbus/visorchipset.c:1925:19: note: each undeclared identifier is reported only once for each function it appears in
   drivers/virt/visorbus/visorchipset.c: In function 'init_unisys':
   drivers/virt/visorbus/visorchipset.c:1943:2: error: implicit declaration of function 'acpi_bus_register_driver' [-Werror=implicit-function-declaration]
     result = acpi_bus_register_driver(&unisys_acpi_driver);
     ^
   drivers/virt/visorbus/visorchipset.c: In function 'exit_unisys':
   drivers/virt/visorbus/visorchipset.c:1953:2: error: implicit declaration of function 'acpi_bus_unregister_driver' [-Werror=implicit-function-declaration]
     acpi_bus_unregister_driver(&unisys_acpi_driver);
     ^
   drivers/virt/visorbus/visorchipset.c: At top level:
   drivers/virt/visorbus/visorchipset.c:1903:36: warning: 'unisys_device_ids' defined but not used [-Wunused-variable]
    static const struct acpi_device_id unisys_device_ids[] = {
                                       ^
   drivers/virt/visorbus/visorchipset.c: In function 'visorutil_spar_detect':
   drivers/virt/visorbus/visorchipset.c:1934:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^
   cc1: some warnings being treated as errors

vim +479 drivers/virt/visorbus/visorchipset.c

12c957dc drivers/staging/unisys/visorbus/visorchipset.c Tim Sell          2016-03-01  473  
765b2f82 drivers/staging/unisys/visorbus/visorchipset.c Sameer Wadgaonkar 2017-02-21  474  	err = visorchannel_read(chipset_dev->controlvm_channel,
545f0913 drivers/staging/unisys/visorbus/visorchipset.c Sameer Wadgaonkar 2017-05-19  475  				offsetof(struct visor_controlvm_channel,
12c957dc drivers/staging/unisys/visorbus/visorchipset.c Tim Sell          2016-03-01  476  					 saved_crash_message_count),
8dff01f7 drivers/staging/unisys/visorbus/visorchipset.c David Kershner    2016-11-21  477  				&local_crash_msg_count, sizeof(u16));
8dff01f7 drivers/staging/unisys/visorbus/visorchipset.c David Kershner    2016-11-21  478  	if (err) {
35301b87 drivers/staging/unisys/visorbus/visorchipset.c David Kershner    2017-04-18 @479  		dev_err(&chipset_dev->acpi_device->dev,
35301b87 drivers/staging/unisys/visorbus/visorchipset.c David Kershner    2017-04-18  480  			"failed to read message count\n");
8dff01f7 drivers/staging/unisys/visorbus/visorchipset.c David Kershner    2016-11-21  481  		return err;
12c957dc drivers/staging/unisys/visorbus/visorchipset.c Tim Sell          2016-03-01  482  	}

:::::: The code at line 479 was first introduced by commit
:::::: 35301b87628df68a554d63b4baa864644008a97c staging: unisys: visorbus: get rid of POSTCODEs in save_crash_msg

:::::: TO: David Kershner <david.kershner@unisys.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41153 bytes --]

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> This patchset moves drivers/staging/unisys/include to
> include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> drivers/virt/visorbus.

Um, are you thinking it is ready to be moved?  Have you asked for
another review?

In a totally random chance, I was doing some driver core work today and
I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
have 2 tabs for your 'struct attribute' variables, which is really odd.

Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
of having to "open code" the struct attribute_group lists.

So either you all have horrible luck in that I just happened to find the
only remaining problem, or that you should proabably ask for a good code
audit, I haven't looked at the code before today since the last round of
"fun" I found in just one other random file :)

thanks,

greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > This patchset moves drivers/staging/unisys/include to
> > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > drivers/virt/visorbus.
> 
> Um, are you thinking it is ready to be moved?  Have you asked for
> another review?
> 
> In a totally random chance, I was doing some driver core work today and
> I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> have 2 tabs for your 'struct attribute' variables, which is really odd.
> 
> Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
> of having to "open code" the struct attribute_group lists.
> 
> So either you all have horrible luck in that I just happened to find the
> only remaining problem, or that you should proabably ask for a good code
> audit, I haven't looked at the code before today since the last round of
> "fun" I found in just one other random file :)

Also, many of the attribute callbacks in that file seem to all have
their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
catch that...

partition_handle_show() is one such example that is obviously wrong.

There's also one checkpatch.pl warning for it, which should probably be
resolved as well.

I strongly suggest you go through everything again on your own...

thanks,

greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > This patchset moves drivers/staging/unisys/include to
> > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > drivers/virt/visorbus.
> > 
> > Um, are you thinking it is ready to be moved?  Have you asked for
> > another review?
> > 
> > In a totally random chance, I was doing some driver core work today and
> > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > 
> > Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
> > of having to "open code" the struct attribute_group lists.
> > 
> > So either you all have horrible luck in that I just happened to find the
> > only remaining problem, or that you should proabably ask for a good code
> > audit, I haven't looked at the code before today since the last round of
> > "fun" I found in just one other random file :)
> 
> Also, many of the attribute callbacks in that file seem to all have
> their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> catch that...
> 
> partition_handle_show() is one such example that is obviously wrong.
> 
> There's also one checkpatch.pl warning for it, which should probably be
> resolved as well.

drivers/staging/unisys/visorbus/visorbus_main.c:1035: WARNING: Prefer using '"%s...", __func__' to using 'create_bus_instance', this function's name, in a string

to be specific, something you should have caught, right?

Are you sure this is ready to be moved out of staging?  :(

greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > > This patchset moves drivers/staging/unisys/include to
> > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > > drivers/virt/visorbus.
> > > 
> > > Um, are you thinking it is ready to be moved?  Have you asked for
> > > another review?
> > > 
> > > In a totally random chance, I was doing some driver core work today and
> > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > > 
> > > Also, you should be using the ATTRIBUTE_GROUPS() macro for them instead
> > > of having to "open code" the struct attribute_group lists.
> > > 
> > > So either you all have horrible luck in that I just happened to find the
> > > only remaining problem, or that you should proabably ask for a good code
> > > audit, I haven't looked at the code before today since the last round of
> > > "fun" I found in just one other random file :)
> > 
> > Also, many of the attribute callbacks in that file seem to all have
> > their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> > catch that...
> > 
> > partition_handle_show() is one such example that is obviously wrong.
> > 
> > There's also one checkpatch.pl warning for it, which should probably be
> > resolved as well.
> 
> drivers/staging/unisys/visorbus/visorbus_main.c:1035: WARNING: Prefer using '"%s...", __func__' to using 'create_bus_instance', this function's name, in a string
> 
> to be specific, something you should have caught, right?
> 
> Are you sure this is ready to be moved out of staging?  :(

Eek, I can't look away...

You do this a bunch:
	if (dev->visorchannel) {
		visorchannel_destroy(dev->visorchannel);

yet the first thing that visorchannel_destroy() does is check for null.
So, no need to test this twice, right, only do so in the function, that
will make your code flow a lot "smoother" where ever you are calling
this.

Ok, I'll stop now, gotta go find some dinner...

greg k-h

RE: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Kershner, David A]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, June 6, 2017 11:06 AM
> To: Kershner, David A <David.Kershner@unisys.com>
> Cc: corbet@lwn.net; tglx@linutronix.de; mingo@kernel.org; akpm@linux-
> foundation.org; jes.sorensen@gmail.com; linux-kernel@vger.kernel.org;
> linux-doc@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> Maintainer <SParMaintainer@unisys.com>
> Subject: Re: [PATCH 0/3] move visorbus out of staging to
> drivers/virt/visorbus
> 
> On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > > > This patchset moves drivers/staging/unisys/include to
> > > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > > > drivers/virt/visorbus.
> > > >
> > > > Um, are you thinking it is ready to be moved?  Have you asked for
> > > > another review?
> > > >

Thank you for taking a quick look at our patch series. Part of the motivation
behind this submission was, in fact, to initiate another code review. What is
the formal procedure for initiating a code review?

> > > > In a totally random chance, I was doing some driver core work today
> and
> > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > > >

Sorry I missed that; I guess my eyes glazed over by the time I got to that file,
and I was expecting checkpatch to catch that. Now I know better, and I will be
looking for more things. Thanks for catching.

> > > > Also, you should be using the ATTRIBUTE_GROUPS() macro for them
> instead
> > > > of having to "open code" the struct attribute_group lists.
> > > >
> > > > So either you all have horrible luck in that I just happened to find the
> > > > only remaining problem, or that you should proabably ask for a good
> code
> > > > audit, I haven't looked at the code before today since the last round of
> > > > "fun" I found in just one other random file :)
> > >
> > > Also, many of the attribute callbacks in that file seem to all have
> > > their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> > > catch that...
> > >
> > > partition_handle_show() is one such example that is obviously wrong.
> > >
> > > There's also one checkpatch.pl warning for it, which should probably be
> > > resolved as well.
> >
> > drivers/staging/unisys/visorbus/visorbus_main.c:1035: WARNING: Prefer
> using '"%s...", __func__' to using 'create_bus_instance', this function's name,
> in a string
> >
> > to be specific, something you should have caught, right?
> >
> > Are you sure this is ready to be moved out of staging?  :(
> 
> Eek, I can't look away...
> 
> You do this a bunch:
> 	if (dev->visorchannel) {
> 		visorchannel_destroy(dev->visorchannel);
> 
> yet the first thing that visorchannel_destroy() does is check for null.
> So, no need to test this twice, right, only do so in the function, that
> will make your code flow a lot "smoother" where ever you are calling
> this.
> 
> Ok, I'll stop now, gotta go find some dinner...
> 

We will do some more internal reviews, and send out fixes for things
we find. I hope you enjoyed your dinner.

> greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 06:20:17PM +0000, Kershner, David A wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, June 6, 2017 11:06 AM
> > To: Kershner, David A <David.Kershner@unisys.com>
> > Cc: corbet@lwn.net; tglx@linutronix.de; mingo@kernel.org; akpm@linux-
> > foundation.org; jes.sorensen@gmail.com; linux-kernel@vger.kernel.org;
> > linux-doc@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> > Maintainer <SParMaintainer@unisys.com>
> > Subject: Re: [PATCH 0/3] move visorbus out of staging to
> > drivers/virt/visorbus
> > 
> > On Tue, Jun 06, 2017 at 04:54:30PM +0200, Greg KH wrote:
> > > On Tue, Jun 06, 2017 at 04:53:22PM +0200, Greg KH wrote:
> > > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > > On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > > > > > This patchset moves drivers/staging/unisys/include to
> > > > > > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > > > > > drivers/virt/visorbus.
> > > > >
> > > > > Um, are you thinking it is ready to be moved?  Have you asked for
> > > > > another review?
> > > > >
> 
> Thank you for taking a quick look at our patch series. Part of the motivation
> behind this submission was, in fact, to initiate another code review. What is
> the formal procedure for initiating a code review?

Send an email that says, "Hey Greg, we think the code is ready to be
moved out of staging, can you review it to see if we have missed
anything?"

Of course, do it _AFTER_ you have fixed up the checkpatch.pl issues.
That's not even done yet...

thanks,

greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Joe Perches]

On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > have 2 tabs for your 'struct attribute' variables, which is really odd.
[]
> Also, many of the attribute callbacks in that file seem to all have
> their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> catch that...

checkpatch doesn't really check much about inconsistent
indentation.  I believe the only new statement indentation
check is after an if.

For instance, checkpatch doesn't emit a warning on this code:

struct foo {
		int bar;
};

struct foo *alloc_foo(void)
{
			struct foo *baz = malloc(sizeof(struct foo));
		if (baz)
			baz->bar = 1;
				return baz;
}

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> []
> > Also, many of the attribute callbacks in that file seem to all have
> > their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> > catch that...
> 
> checkpatch doesn't really check much about inconsistent
> indentation.  I believe the only new statement indentation
> check is after an if.
> 
> For instance, checkpatch doesn't emit a warning on this code:
> 
> struct foo {
> 		int bar;
> };
> 
> struct foo *alloc_foo(void)
> {
> 			struct foo *baz = malloc(sizeof(struct foo));
> 		if (baz)
> 			baz->bar = 1;
> 				return baz;
> }

Ok, but the following code in that file should be caught, right:

static ssize_t partition_handle_show(struct device *dev,
                                     struct device_attribute *attr,
                                     char *buf) {
        struct visor_device *vdev = to_visor_device(dev);
        u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);

        return sprintf(buf, "0x%llx\n", handle);
}
static DEVICE_ATTR_RO(partition_handle);


The initial { is in the wrong place...

thanks,

greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Joe Perches]

On Tue, 2017-06-06 at 17:39 +0200, Greg KH wrote:
> On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> > On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > 
> > []
> > > Also, many of the attribute callbacks in that file seem to all have
> > > their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> > > catch that...
[]
> the following code in that file should be caught, right:
> 
> static ssize_t partition_handle_show(struct device *dev,
>                                      struct device_attribute *attr,
>                                      char *buf) {
>         struct visor_device *vdev = to_visor_device(dev);
>         u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
> 
>         return sprintf(buf, "0x%llx\n", handle);
> }
> static DEVICE_ATTR_RO(partition_handle);

Not really.

> The initial { is in the wrong place...

True.

Please understand that checkpatch looks at patches one line
at a time.  It's not very smart about function definitions
or context.

checkpatch's function definition code is pretty limited.
It can miss a lot of style misuses.

Single line function definitions brace tests work well.
Multiple line function definitions do not.

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Greg KH]

On Tue, Jun 06, 2017 at 08:52:27AM -0700, Joe Perches wrote:
> On Tue, 2017-06-06 at 17:39 +0200, Greg KH wrote:
> > On Tue, Jun 06, 2017 at 08:33:49AM -0700, Joe Perches wrote:
> > > On Tue, 2017-06-06 at 16:53 +0200, Greg KH wrote:
> > > > On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> > > > > I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> > > > > have 2 tabs for your 'struct attribute' variables, which is really odd.
> > > 
> > > []
> > > > Also, many of the attribute callbacks in that file seem to all have
> > > > their leading '{' in the wrong place.  Odd that checkpatch.pl doesn't
> > > > catch that...
> []
> > the following code in that file should be caught, right:
> > 
> > static ssize_t partition_handle_show(struct device *dev,
> >                                      struct device_attribute *attr,
> >                                      char *buf) {
> >         struct visor_device *vdev = to_visor_device(dev);
> >         u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
> > 
> >         return sprintf(buf, "0x%llx\n", handle);
> > }
> > static DEVICE_ATTR_RO(partition_handle);
> 
> Not really.
> 
> > The initial { is in the wrong place...
> 
> True.
> 
> Please understand that checkpatch looks at patches one line
> at a time.  It's not very smart about function definitions
> or context.
> 
> checkpatch's function definition code is pretty limited.
> It can miss a lot of style misuses.
> 
> Single line function definitions brace tests work well.
> Multiple line function definitions do not.

Ok, that makes sense why this is missed.  No big deal, a simple visual
inspection shows stuff like this up really easily, which obviously no
one did yet on this file :)

thanks,

greg k-h

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Christoph Hellwig]

On Tue, Jun 06, 2017 at 04:49:09PM +0200, Greg KH wrote:
> On Mon, Jun 05, 2017 at 04:07:29PM -0400, David Kershner wrote:
> > This patchset moves drivers/staging/unisys/include to
> > include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> > drivers/virt/visorbus.
> 
> Um, are you thinking it is ready to be moved?  Have you asked for
> another review?
> 
> In a totally random chance, I was doing some driver core work today and
> I noticed that in drivers/staging/unisys/visorbus/visorbus_main.c, you
> have 2 tabs for your 'struct attribute' variables, which is really odd.

That's the least of the problems.  Just about any function in there is
doing functionally stupid things.  E.g. raw sg_phys() calls for I/O
instead of dma mapping routines or parsing SCSI INQUIRY data in the
driver.

[PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[David Kershner]

This patchset is dependent on the previously-submitted patchset:

	staging: unisys: fix visorbus & visorinput issues raised by tglx

This patchset moves drivers/staging/unisys/include to
include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
drivers/virt/visorbus.

This patchset comprises the last 3 patches of the previously-submitted
patchset (but retracted):

	[PATCH v4 00/29] Fixed issues raised by tglx,
			 then move visorbus to drivers/virt


David Kershner (3):
  include: linux: visorbus: Add visorbus to include/linux directory
  Documentation: Move visorbus documentation from staging to
    Documentation/
  drivers: Add visorbus to the drivers/virt directory

 .../ABI/stable/sysfs-bus-visorbus                             |  0
 .../Documentation/overview.txt => Documentation/visorbus.txt  |  0
 drivers/staging/unisys/Kconfig                                |  3 +--
 drivers/staging/unisys/MAINTAINERS                            |  2 +-
 drivers/staging/unisys/Makefile                               |  1 -
 drivers/staging/unisys/visorbus/Makefile                      | 11 -----------
 drivers/staging/unisys/visorhba/Makefile                      |  2 --
 drivers/staging/unisys/visorhba/visorhba_main.c               |  5 ++---
 drivers/staging/unisys/visorinput/Makefile                    |  2 --
 drivers/staging/unisys/visorinput/visorinput.c                |  6 +++---
 drivers/staging/unisys/visornic/Makefile                      |  2 --
 drivers/staging/unisys/visornic/visornic_main.c               |  5 ++---
 drivers/virt/Kconfig                                          |  2 ++
 drivers/virt/Makefile                                         |  1 +
 drivers/{staging/unisys => virt}/visorbus/Kconfig             |  0
 drivers/virt/visorbus/Makefile                                |  9 +++++++++
 drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h  |  2 +-
 .../unisys => virt}/visorbus/controlvmcompletionstatus.h      |  0
 drivers/{staging/unisys => virt}/visorbus/iovmcall_gnuc.h     |  0
 drivers/{staging/unisys => virt}/visorbus/vbuschannel.h       |  3 ++-
 drivers/{staging/unisys => virt}/visorbus/vbusdeviceinfo.h    |  0
 drivers/{staging/unisys => virt}/visorbus/vbushelper.h        |  0
 drivers/{staging/unisys => virt}/visorbus/visorbus_main.c     |  6 +++---
 drivers/{staging/unisys => virt}/visorbus/visorbus_private.h  |  0
 drivers/{staging/unisys => virt}/visorbus/visorchannel.c      |  4 ++--
 drivers/{staging/unisys => virt}/visorbus/visorchipset.c      |  8 ++++----
 drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h   |  5 ++---
 .../unisys/include => include/linux/visorbus}/channel.h       |  0
 .../unisys/include => include/linux/visorbus}/channel_guid.h  |  0
 .../unisys/include => include/linux/visorbus}/diagchannel.h   |  0
 .../include => include/linux/visorbus}/guestlinuxdebug.h      |  0
 .../unisys/include => include/linux/visorbus}/iochannel.h     |  0
 .../unisys/include => include/linux/visorbus}/version.h       |  0
 .../unisys/include => include/linux/visorbus}/visorbus.h      |  0
 34 files changed, 35 insertions(+), 44 deletions(-)
 rename drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset => Documentation/ABI/stable/sysfs-bus-visorbus (100%)
 rename drivers/staging/unisys/Documentation/overview.txt => Documentation/visorbus.txt (100%)
 delete mode 100644 drivers/staging/unisys/visorbus/Makefile
 rename drivers/{staging/unisys => virt}/visorbus/Kconfig (100%)
 create mode 100644 drivers/virt/visorbus/Makefile
 rename drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h (99%)
 rename drivers/{staging/unisys => virt}/visorbus/controlvmcompletionstatus.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/iovmcall_gnuc.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/vbuschannel.h (99%)
 rename drivers/{staging/unisys => virt}/visorbus/vbusdeviceinfo.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/vbushelper.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorbus_main.c (99%)
 rename drivers/{staging/unisys => virt}/visorbus/visorbus_private.h (100%)
 rename drivers/{staging/unisys => virt}/visorbus/visorchannel.c (99%)
 rename drivers/{staging/unisys => virt}/visorbus/visorchipset.c (99%)
 rename drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h (98%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/channel.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/channel_guid.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/diagchannel.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/guestlinuxdebug.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/iochannel.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/version.h (100%)
 rename {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h (100%)

-- 
1.9.1

Re: [PATCH 0/3] move visorbus out of staging to drivers/virt/visorbus

[Neil Horman]

On Fri, Jun 10, 2016 at 11:23:53PM -0400, David Kershner wrote:
> This patchset is dependent on the previously-submitted patchset:
> 
> 	staging: unisys: fix visorbus & visorinput issues raised by tglx
> 
> This patchset moves drivers/staging/unisys/include to
> include/linux/visorbus, and moves drivers/staging/unisys/visorbus to
> drivers/virt/visorbus.
> 
> This patchset comprises the last 3 patches of the previously-submitted
> patchset (but retracted):
> 
> 	[PATCH v4 00/29] Fixed issues raised by tglx,
> 			 then move visorbus to drivers/virt
> 
> 
> David Kershner (3):
>   include: linux: visorbus: Add visorbus to include/linux directory
>   Documentation: Move visorbus documentation from staging to
>     Documentation/
>   drivers: Add visorbus to the drivers/virt directory
> 
>  .../ABI/stable/sysfs-bus-visorbus                             |  0
>  .../Documentation/overview.txt => Documentation/visorbus.txt  |  0
>  drivers/staging/unisys/Kconfig                                |  3 +--
>  drivers/staging/unisys/MAINTAINERS                            |  2 +-
>  drivers/staging/unisys/Makefile                               |  1 -
>  drivers/staging/unisys/visorbus/Makefile                      | 11 -----------
>  drivers/staging/unisys/visorhba/Makefile                      |  2 --
>  drivers/staging/unisys/visorhba/visorhba_main.c               |  5 ++---
>  drivers/staging/unisys/visorinput/Makefile                    |  2 --
>  drivers/staging/unisys/visorinput/visorinput.c                |  6 +++---
>  drivers/staging/unisys/visornic/Makefile                      |  2 --
>  drivers/staging/unisys/visornic/visornic_main.c               |  5 ++---
>  drivers/virt/Kconfig                                          |  2 ++
>  drivers/virt/Makefile                                         |  1 +
>  drivers/{staging/unisys => virt}/visorbus/Kconfig             |  0
>  drivers/virt/visorbus/Makefile                                |  9 +++++++++
>  drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h  |  2 +-
>  .../unisys => virt}/visorbus/controlvmcompletionstatus.h      |  0
>  drivers/{staging/unisys => virt}/visorbus/iovmcall_gnuc.h     |  0
>  drivers/{staging/unisys => virt}/visorbus/vbuschannel.h       |  3 ++-
>  drivers/{staging/unisys => virt}/visorbus/vbusdeviceinfo.h    |  0
>  drivers/{staging/unisys => virt}/visorbus/vbushelper.h        |  0
>  drivers/{staging/unisys => virt}/visorbus/visorbus_main.c     |  6 +++---
>  drivers/{staging/unisys => virt}/visorbus/visorbus_private.h  |  0
>  drivers/{staging/unisys => virt}/visorbus/visorchannel.c      |  4 ++--
>  drivers/{staging/unisys => virt}/visorbus/visorchipset.c      |  8 ++++----
>  drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h   |  5 ++---
>  .../unisys/include => include/linux/visorbus}/channel.h       |  0
>  .../unisys/include => include/linux/visorbus}/channel_guid.h  |  0
>  .../unisys/include => include/linux/visorbus}/diagchannel.h   |  0
>  .../include => include/linux/visorbus}/guestlinuxdebug.h      |  0
>  .../unisys/include => include/linux/visorbus}/iochannel.h     |  0
>  .../unisys/include => include/linux/visorbus}/version.h       |  0
>  .../unisys/include => include/linux/visorbus}/visorbus.h      |  0
>  34 files changed, 35 insertions(+), 44 deletions(-)
>  rename drivers/staging/unisys/Documentation/ABI/sysfs-platform-visorchipset => Documentation/ABI/stable/sysfs-bus-visorbus (100%)
>  rename drivers/staging/unisys/Documentation/overview.txt => Documentation/visorbus.txt (100%)
>  delete mode 100644 drivers/staging/unisys/visorbus/Makefile
>  rename drivers/{staging/unisys => virt}/visorbus/Kconfig (100%)
>  create mode 100644 drivers/virt/visorbus/Makefile
>  rename drivers/{staging/unisys => virt}/visorbus/controlvmchannel.h (99%)
>  rename drivers/{staging/unisys => virt}/visorbus/controlvmcompletionstatus.h (100%)
>  rename drivers/{staging/unisys => virt}/visorbus/iovmcall_gnuc.h (100%)
>  rename drivers/{staging/unisys => virt}/visorbus/vbuschannel.h (99%)
>  rename drivers/{staging/unisys => virt}/visorbus/vbusdeviceinfo.h (100%)
>  rename drivers/{staging/unisys => virt}/visorbus/vbushelper.h (100%)
>  rename drivers/{staging/unisys => virt}/visorbus/visorbus_main.c (99%)
>  rename drivers/{staging/unisys => virt}/visorbus/visorbus_private.h (100%)
>  rename drivers/{staging/unisys => virt}/visorbus/visorchannel.c (99%)
>  rename drivers/{staging/unisys => virt}/visorbus/visorchipset.c (99%)
>  rename drivers/{staging/unisys => virt}/visorbus/vmcallinterface.h (98%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/channel.h (100%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/channel_guid.h (100%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/diagchannel.h (100%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/guestlinuxdebug.h (100%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/iochannel.h (100%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/version.h (100%)
>  rename {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h (100%)
> 
> -- 
> 1.9.1
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>