Hi Andrew.
A few things noted in the following..
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 2661f6e..fe38c7a 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -517,4 +517,5 @@ source "drivers/misc/lis3lv02d/Kconfig"
> source "drivers/misc/carma/Kconfig"
> source "drivers/misc/altera-stapl/Kconfig"
> source "drivers/misc/mei/Kconfig"
> +source "drivers/misc/vmw_vmci/Kconfig"
> endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 456972f..af9e413 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -51,3 +51,4 @@ obj-y += carma/
> obj-$(CONFIG_USB_SWITCH_FSA9480) += fsa9480.o
> obj-$(CONFIG_ALTERA_STAPL) +=altera-stapl/
> obj-$(CONFIG_INTEL_MEI) += mei/
> +obj-y += vmw_vmci/
Please use obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
like we do in the other cases. This prevents us from visiting the directory
when this feature is not enabled.
> +++ b/drivers/misc/vmw_vmci/Makefile
> @@ -0,0 +1,43 @@
> +################################################################################
> +#
> +# Linux driver for VMware's VMCI device.
> +#
> +# Copyright (C) 2007-2012, VMware, Inc. All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or modify it
> +# under the terms of the GNU General Public License as published by the
> +# Free Software Foundation; version 2 of the License and no later version.
> +#
> +# This program is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> +# NON INFRINGEMENT. See the GNU General Public License for more
> +# details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> +#
> +# The full GNU General Public License is included in this distribution in
> +# the file called "COPYING".
> +#
> +# Maintained by: Andrew Stiegmann <pv-drivers@xxxxxxxxxx>
> +#
> +################################################################################
Lot's of boilerplate noise for such a simple file...
> +
> +#
> +# Makefile for the VMware VMCI
> +#
> +
> +obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci.o
> +
> +vmw_vmci-objs += vmci_context.o
> +vmw_vmci-objs += vmci_datagram.o
> +vmw_vmci-objs += vmci_doorbell.o
> +vmw_vmci-objs += vmci_driver.o
> +vmw_vmci-objs += vmci_event.o
> +vmw_vmci-objs += vmci_handle_array.o
> +vmw_vmci-objs += vmci_hash_table.o
> +vmw_vmci-objs += vmci_queue_pair.o
> +vmw_vmci-objs += vmci_resource.o
> +vmw_vmci-objs += vmci_route.o
please use:
vmw_vmci-y += vmci_context.o
vmw_vmci-y += vmci_datagram.o
vmw_vmci-y += vmci_doorbell.o
This is recommended these days and allows you to enable/disable
single files later using a config option.
> diff --git a/drivers/misc/vmw_vmci/vmci_common_int.h b/drivers/misc/vmw_vmci/vmci_common_int.h
> +
> +#ifndef _VMCI_COMMONINT_H_
> +#define _VMCI_COMMONINT_H_
> +
> +#include <linux/printk.h>
> +#include <linux/vmw_vmci_defs.h>
Use inverse chrismas tree here.
Longer include lines first, and soret alphabetically when
lines are of the same length.
This applies likely in many cases.
> +#include "vmci_handle_array.h"
> +
> +#define ASSERT(cond) BUG_ON(!(cond))
> +
> +#define CAN_BLOCK(_f) (!((_f) & VMCI_QPFLAG_NONBLOCK))
> +#define QP_PINNED(_f) ((_f) & VMCI_QPFLAG_PINNED)
Looks like poor obscufation.
Use a statis inline function if you need a helper for this.
> +
> +/*
> + * Utilility function that checks whether two entities are allowed
> + * to interact. If one of them is restricted, the other one must
> + * be trusted.
> + */
> +static inline bool vmci_deny_interaction(uint32_t partOne,
> + uint32_t partTwo)
The kernel types are u32 not uint32_t - these types belongs in user-space.
> +++ b/include/linux/vmw_vmci_api.h
> +
> +#ifndef __VMW_VMCI_API_H__
> +#define __VMW_VMCI_API_H__
> +
> +#include <linux/vmw_vmci_defs.h>
> +
> +#undef VMCI_KERNEL_API_VERSION
> +#define VMCI_KERNEL_API_VERSION_2 2
> +#define VMCI_KERNEL_API_VERSION VMCI_KERNEL_API_VERSION_2
> +
> +typedef void (VMCI_DeviceShutdownFn) (void *deviceRegistration, void *userData);
> +
> +bool VMCI_DeviceGet(uint32_t *apiVersion,
> + VMCI_DeviceShutdownFn *deviceShutdownCB,
> + void *userData, void **deviceRegistration);
The kernel style is to use lower_case for everything.
So this would become:
vmci_device_get()
This is obviously a very general comment and applies everywhere.
Sam
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
