[PATCH] staging: unisys: add visorclientbus driver

[Ken Cox]

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