[PATCH] staging: unisys: add visorclientbus driver

[Greg KH]

On Thu, Nov 06, 2014 at 12:53:32PM -0600, Ken Cox wrote:
> From: Benjamin Romer <benjamin.romer at unisys.com>
> 
> 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 at unisys.com>
> Signed-off-by: Ken Cox <jkc at redhat.com>
> ---
>  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".

>  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.

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?

thanks,

greg k-h