Re: [PATCH] staging: unisys: add visorclientbus driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/06/2014 01:05 PM, Greg KH wrote:
On Thu, Nov 06, 2014 at 12:53:32PM -0600, Ken Cox wrote:
From: Benjamin Romer <benjamin.romer@xxxxxxxxxx>

This patch adds the visorclientbus driver to the Unisys s-Par driver set. This
driver is responsible for helping manage virtual devices like keyboards, mice,
serial ports, displays, and any customized drivers for special-purpose guests.

Signed-off-by: Benjamin Romer <benjamin.romer@xxxxxxxxxx>
Signed-off-by: Ken Cox <jkc@xxxxxxxxxx>
---
  drivers/staging/unisys/Kconfig                     |   1 +
  drivers/staging/unisys/Makefile                    |   1 +
  drivers/staging/unisys/visorclientbus/Kconfig      |   9 +
  drivers/staging/unisys/visorclientbus/Makefile     |  13 +
  .../unisys/visorclientbus/visorclientbus_main.c    | 378 +++++++++++++++++++++
I really don't want to accept any "new features" for this codebase,
until you have cleaned up the existing "mess".
OK, we will continue getting the code cleaned up before submitting anything new.
  5 files changed, 402 insertions(+)
  create mode 100644 drivers/staging/unisys/visorclientbus/Kconfig
  create mode 100644 drivers/staging/unisys/visorclientbus/Makefile
  create mode 100644 drivers/staging/unisys/visorclientbus/visorclientbus_main.c

diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index ac080c9..d63b035 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -16,5 +16,6 @@ source "drivers/staging/unisys/channels/Kconfig"
  source "drivers/staging/unisys/uislib/Kconfig"
  source "drivers/staging/unisys/virtpci/Kconfig"
  source "drivers/staging/unisys/virthba/Kconfig"
+source "drivers/staging/unisys/visorclientbus/Kconfig"
endif # UNISYSSPAR
diff --git a/drivers/staging/unisys/Makefile b/drivers/staging/unisys/Makefile
index b988d69..794177b 100644
--- a/drivers/staging/unisys/Makefile
+++ b/drivers/staging/unisys/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_UNISYS_CHANNELSTUB)	+= channels/
  obj-$(CONFIG_UNISYS_UISLIB)		+= uislib/
  obj-$(CONFIG_UNISYS_VIRTPCI)		+= virtpci/
  obj-$(CONFIG_UNISYS_VIRTHBA)		+= virthba/
+obj-$(CONFIG_UNISYS_VISORCLIENTBUS)	+= visorclientbus/
diff --git a/drivers/staging/unisys/visorclientbus/Kconfig b/drivers/staging/unisys/visorclientbus/Kconfig
new file mode 100644
index 0000000..62bdbb9
--- /dev/null
+++ b/drivers/staging/unisys/visorclientbus/Kconfig
@@ -0,0 +1,9 @@
+#
+# Unisys visorclientbus configuration
+#
+
+config UNISYS_VISORCLIENTBUS
+	tristate "Unisys visorclientbus driver"
+	depends on UNISYSSPAR && UNISYS_VISORCHIPSET && UNISYS_UISLIB
+	---help---
+	If you say Y here, you will enable the Unisys visorclientbus driver.
What about if you say 'M'?  :)

A whole subdirectory for a single .c file?  That's not needed, or
acceptable.

diff --git a/drivers/staging/unisys/visorclientbus/Makefile b/drivers/staging/unisys/visorclientbus/Makefile
new file mode 100644
index 0000000..bfacc0d
--- /dev/null
+++ b/drivers/staging/unisys/visorclientbus/Makefile
@@ -0,0 +1,13 @@
+#
+# Makefile for Unisys visorclientbus
+#
+
+obj-$(CONFIG_UNISYS_VISORCLIENTBUS)	+= visorclientbus.o
+
+visorclientbus-y := visorclientbus_main.o
As this is a single-file module, why not just have the .c file be called
visorclientbus.c?

+ccflags-y += -Idrivers/staging/unisys/include
+ccflags-y += -Idrivers/staging/unisys/uislib
+ccflags-y += -Idrivers/staging/unisys/visorchipset
+ccflags-y += -Idrivers/staging/unisys/common-spar/include
+ccflags-y += -Idrivers/staging/unisys/common-spar/include/channels
Here's one example of things that need to be fixed up, you should never
need -I lines in a kernel Makefile, otherwise some testing tools are
broken.
Thanks for the feedback.  I'll get these corrected.

There are also other checkpatch errors in this patch, which makes me not
want to take it at all, as you would now just be required to send more
fixes for the file.  So why not fix things up right the first time?
The only checkpatch warnings I get are:
-a warning about the MAINTAINERS file not being updated when adding a new file. MAINTAINERS already has an entry that covers this file

-warnings about lines over 80 characters. These are strings for error messages that make the line exceed 80 characters. I thought this was OK.


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




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux