Re: [RFC PATCH 2/11] input: RMI4 core bus and sensor drivers.

[Dmitry Torokhov]

Hi Christopher,

Please find my [incomplete] comments below.

On Wed, Dec 21, 2011 at 06:09:53PM -0800, Christopher Heiny wrote:
> Signed-off-by: Christopher Heiny <cheiny@xxxxxxxxxxxxx>
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx>
> Cc: Naveen Kumar Gaddipati <naveen.gaddipati@xxxxxxxxxxxxxx>
> Cc: Joeri de Gram <j.de.gram@xxxxxxxxx>
> 
> ---
> 
>  drivers/input/rmi4/rmi_bus.c    |  436 ++++++++++++
>  drivers/input/rmi4/rmi_driver.c | 1488 +++++++++++++++++++++++++++++++++++++++
>  drivers/input/rmi4/rmi_driver.h |   97 +++
>  3 files changed, 2021 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> new file mode 100644
> index 0000000..e32d4ad
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -0,0 +1,436 @@
> +/*
> + * Copyright (c) 2011 Synaptics Incorporated
> + * Copyright (c) 2011 Unixphere
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any 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.  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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/pm.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +#include <linux/rmi.h>
> +
> +DEFINE_MUTEX(rmi_bus_mutex);
> +
> +static struct rmi_function_list {
> +	struct list_head list;
> +	struct rmi_function_handler *fh;
> +} rmi_supported_functions;
> +
> +static struct rmi_character_driver_list {
> +	struct list_head list;
> +	struct rmi_char_driver *cd;
> +} rmi_character_drivers;
> +
> +static int physical_device_count;
> +
> +static int rmi_bus_match(struct device *dev, struct device_driver *driver)
> +{
> +	struct rmi_driver *rmi_driver;
> +	struct rmi_device *rmi_dev;
> +	struct rmi_device_platform_data *pdata;
> +
> +	rmi_driver = to_rmi_driver(driver);
> +	rmi_dev = to_rmi_device(dev);
> +	pdata = to_rmi_platform_data(rmi_dev);
> +	dev_dbg(dev, "Matching %s.\n", pdata->sensor_name);
> +
> +	if (!strcmp(pdata->driver_name, rmi_driver->driver.name)) {
> +		rmi_dev->driver = rmi_driver;
> +		dev_dbg(dev, "%s: Match %s to %s succeeded.\n", __func__,
> +			pdata->driver_name, rmi_driver->driver.name);
> +		return 1;
> +	}
> +
> +	dev_err(dev, "%s: Match %s to %s failed.\n", __func__,
> +		pdata->driver_name, rmi_driver->driver.name);

Why is this an error? dev_vdbg() at most, better yet just remove it.

> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int rmi_bus_suspend(struct device *dev)
> +{
> +#ifdef GENERIC_SUBSYS_PM_OPS
> +	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
> +	if (pm && pm->suspend)
> +		return pm->suspend(dev);
> +#endif
> +
> +	return 0;
> +}
> +
> +static int rmi_bus_resume(struct device *dev)
> +{
> +#ifdef GENERIC_SUBSYS_PM_OPS
> +	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
> +	if (pm && pm->resume)
> +		return pm->resume(dev);
> +#endif
> +
> +	return 0;
> +}
> +#endif

These are not needed if you switch to generic_subsys_pm_ops.

> +
> +static int rmi_bus_probe(struct device *dev)
> +{
> +	struct rmi_driver *driver;
> +	struct rmi_device *rmi_dev = to_rmi_device(dev);
> +
> +	driver = rmi_dev->driver;
> +	if (driver && driver->probe)
> +		return driver->probe(rmi_dev);
> +
> +	return 0;
> +}
> +
> +static int rmi_bus_remove(struct device *dev)
> +{
> +	struct rmi_driver *driver;
> +	struct rmi_device *rmi_dev = to_rmi_device(dev);
> +
> +	driver = rmi_dev->driver;
> +	if (driver && driver->remove)
> +		return driver->remove(rmi_dev);
> +
> +	return 0;
> +}
> +
> +static void rmi_bus_shutdown(struct device *dev)
> +{
> +	struct rmi_driver *driver;
> +	struct rmi_device *rmi_dev = to_rmi_device(dev);
> +
> +	driver = rmi_dev->driver;
> +	if (driver && driver->shutdown)
> +		driver->shutdown(rmi_dev);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(rmi_bus_pm_ops,
> +			 rmi_bus_suspend, rmi_bus_resume);
> +
> +struct bus_type rmi_bus_type = {
> +	.name		= "rmi",
> +	.match		= rmi_bus_match,
> +	.probe		= rmi_bus_probe,
> +	.remove		= rmi_bus_remove,
> +	.shutdown	= rmi_bus_shutdown,
> +	.pm		= &rmi_bus_pm_ops
> +};
> +
> +int rmi_register_phys_device(struct rmi_phys_device *phys)
> +{
> +	struct rmi_device_platform_data *pdata = phys->dev->platform_data;
> +	struct rmi_device *rmi_dev;
> +
> +	if (!pdata) {
> +		dev_err(phys->dev, "no platform data!\n");
> +		return -EINVAL;
> +	}
> +
> +	rmi_dev = kzalloc(sizeof(struct rmi_device), GFP_KERNEL);
> +	if (!rmi_dev)
> +		return -ENOMEM;
> +
> +	rmi_dev->phys = phys;
> +	rmi_dev->dev.bus = &rmi_bus_type;
> +
> +	mutex_lock(&rmi_bus_mutex);
> +	rmi_dev->number = physical_device_count;
> +	physical_device_count++;
> +	mutex_unlock(&rmi_bus_mutex);

Do
	rmi_dev->number = atomic_inc_return(&rmi_no) - 1;

and  stick "static atomic_t rmi_no = ATOMIC_INIT(0)"; at the beginning
of the function. Then you don't need to take mutex here. Do you need
rmi_dev->number?

> +
> +	dev_set_name(&rmi_dev->dev, "sensor%02d", rmi_dev->number);
> +	pr_debug("%s: Registered %s as %s.\n", __func__, pdata->sensor_name,
> +		dev_name(&rmi_dev->dev));
> +
> +	phys->rmi_dev = rmi_dev;
> +	return device_register(&rmi_dev->dev);
> +}
> +EXPORT_SYMBOL(rmi_register_phys_device);
> +
> +void rmi_unregister_phys_device(struct rmi_phys_device *phys)
> +{
> +	struct rmi_device *rmi_dev = phys->rmi_dev;
> +
> +	device_unregister(&rmi_dev->dev);
> +	kfree(rmi_dev);

This is lifetime rules violation; rmi_dev->dev might still be referenced
and you are freeing it. Please provide proper release function.

> +}
> +EXPORT_SYMBOL(rmi_unregister_phys_device);
> +
> +int rmi_register_driver(struct rmi_driver *driver)
> +{
> +	driver->driver.bus = &rmi_bus_type;
> +	return driver_register(&driver->driver);
> +}
> +EXPORT_SYMBOL(rmi_register_driver);
> +
> +static int __rmi_driver_remove(struct device *dev, void *data)
> +{
> +	struct rmi_driver *driver = data;
> +	struct rmi_device *rmi_dev = to_rmi_device(dev);
> +
> +	if (rmi_dev->driver == driver)
> +		rmi_dev->driver = NULL;

No cleanup whatsoever?

> +
> +	return 0;
> +}
> +
> +void rmi_unregister_driver(struct rmi_driver *driver)
> +{
> +	bus_for_each_dev(&rmi_bus_type, NULL, driver, __rmi_driver_remove);

Why don't you rely on driver core to unbind devices upon driver removal
instead of rolling your own (and highly likely broken) implementation.

> +	driver_unregister(&driver->driver);
> +}
> +EXPORT_SYMBOL(rmi_unregister_driver);
> +
> +static int __rmi_bus_fh_add(struct device *dev, void *data)
> +{
> +	struct rmi_driver *driver;
> +	struct rmi_device *rmi_dev = to_rmi_device(dev);
> +
> +	driver = rmi_dev->driver;
> +	if (driver && driver->fh_add)
> +		driver->fh_add(rmi_dev, data);
> +
> +	return 0;
> +}
> +
> +int rmi_register_function_driver(struct rmi_function_handler *fh)
> +{
> +	struct rmi_function_list *entry;
> +	struct rmi_function_handler *fh_dup;
> +
> +	fh_dup = rmi_get_function_handler(fh->func);
> +	if (fh_dup) {
> +		pr_err("%s: function f%.2x already registered!\n", __func__,
> +			fh->func);
> +		return -EINVAL;
> +	}
> +
> +	entry = kzalloc(sizeof(struct rmi_function_list), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->fh = fh;
> +	list_add_tail(&entry->list, &rmi_supported_functions.list);
> +
> +	/* notify devices of the new function handler */
> +	bus_for_each_dev(&rmi_bus_type, NULL, fh, __rmi_bus_fh_add);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(rmi_register_function_driver);
> +
> +static int __rmi_bus_fh_remove(struct device *dev, void *data)
> +{
> +	struct rmi_driver *driver;
> +	struct rmi_device *rmi_dev = to_rmi_device(dev);
> +
> +	driver = rmi_dev->driver;
> +	if (driver && driver->fh_remove)
> +		driver->fh_remove(rmi_dev, data);
> +
> +	return 0;
> +}
> +
> +void rmi_unregister_function_driver(struct rmi_function_handler *fh)
> +{
> +	struct rmi_function_list *entry, *n;
> +
> +	/* notify devices of the removal of the function handler */
> +	bus_for_each_dev(&rmi_bus_type, NULL, fh, __rmi_bus_fh_remove);
> +
> +	list_for_each_entry_safe(entry, n, &rmi_supported_functions.list,
> +									list) {
> +		if (entry->fh->func == fh->func) {
> +			list_del(&entry->list);
> +			kfree(entry);
> +		}
> +	}

You are still rolling partly your own infrastructure. It looks like you
need 2 types of devices on rmi bus - composite RMI device and sensor
device, see struct device_type. Make 2 of those and match drivers
depending on the device type. Then driver core will maintain all the
lists for you.

> +
> +}
> +EXPORT_SYMBOL(rmi_unregister_function_driver);
> +
> +struct rmi_function_handler *rmi_get_function_handler(int id)
> +{
> +	struct rmi_function_list *entry;
> +
> +	list_for_each_entry(entry, &rmi_supported_functions.list, list)
> +		if (entry->fh->func == id)
> +			return entry->fh;
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(rmi_get_function_handler);
> +
> +static void rmi_release_character_device(struct device *dev)
> +{
> +	dev_dbg(dev, "%s: Called.\n", __func__);
> +	return;

No, no, no. Do not try to shut up warnings from device core; they are
there for a reason.

> +}
> +
> +static int rmi_register_character_device(struct device *dev, void *data)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_char_driver *char_driver = data;
> +	struct rmi_char_device *char_dev;
> +	int retval;
> +
> +	dev_dbg(dev, "Attaching character device.\n");
> +	rmi_dev = to_rmi_device(dev);
> +	if (char_driver->match && !char_driver->match(rmi_dev))
> +		return 0;
> +
> +	if (!char_driver->init) {
> +		dev_err(dev, "ERROR: No init() function in %s.\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	char_dev = kzalloc(sizeof(struct rmi_char_device), GFP_KERNEL);
> +	if (!char_dev)
> +		return -ENOMEM;
> +
> +	char_dev->rmi_dev = rmi_dev;
> +	char_dev->driver = char_driver;
> +
> +	char_dev->dev.parent = dev;
> +	char_dev->dev.release = rmi_release_character_device;
> +	char_dev->dev.driver = &char_driver->driver;
> +	retval = device_register(&char_dev->dev);
> +	if (!retval) {
> +		dev_err(dev, "Failed to register character device.\n");
> +		goto error_exit;
> +	}
> +
> +	retval = char_driver->init(char_dev);
> +	if (retval) {
> +		dev_err(dev, "Failed to initialize character device.\n");
> +		goto error_exit;
> +	}
> +
> +	mutex_lock(&rmi_bus_mutex);
> +	list_add_tail(&char_dev->list, &char_driver->devices);
> +	mutex_unlock(&rmi_bus_mutex);
> +	dev_info(&char_dev->dev, "Registered a device.\n");
> +	return retval;
> +
> +error_exit:
> +	kfree(char_dev);
> +	return retval;
> +}
> +
> +int rmi_register_character_driver(struct rmi_char_driver *char_driver)
> +{
> +	struct rmi_character_driver_list *entry;
> +	int retval;
> +
> +	pr_debug("%s: Registering character driver %s.\n", __func__,
> +		char_driver->driver.name);
> +
> +	char_driver->driver.bus = &rmi_bus_type;
> +	INIT_LIST_HEAD(&char_driver->devices);
> +	retval = driver_register(&char_driver->driver);
> +	if (retval) {
> +		pr_err("%s: Failed to register %s, code: %d.\n", __func__,
> +		       char_driver->driver.name, retval);
> +		return retval;
> +	}
> +
> +	entry = kzalloc(sizeof(struct rmi_character_driver_list), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +	entry->cd = char_driver;
> +
> +	mutex_lock(&rmi_bus_mutex);
> +	list_add_tail(&entry->list, &rmi_character_drivers.list);
> +	mutex_unlock(&rmi_bus_mutex);
> +
> +	/* notify devices of the removal of the function handler */
> +	bus_for_each_dev(&rmi_bus_type, NULL, char_driver,
> +			 rmi_register_character_device);

Hmm, thisi is very roundabout way of attaching RMI chardevice... Does it
even work if driver [re]appears after rmi_dev module was loaded?

IFF we agree on keeping rmi_dev interface then I think something more
elegant could be cooked via bus's blocking notifier.

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(rmi_register_character_driver);
> +
> +
> +int rmi_unregister_character_driver(struct rmi_char_driver *char_driver)
> +{
> +	struct rmi_character_driver_list *entry, *n;
> +	struct rmi_char_device *char_dev, *m;
> +	pr_debug("%s: Unregistering character driver %s.\n", __func__,
> +		char_driver->driver.name);
> +
> +	mutex_lock(&rmi_bus_mutex);
> +	list_for_each_entry_safe(char_dev, m, &char_driver->devices,
> +				 list) {
> +		list_del(&char_dev->list);
> +		char_dev->driver->remove(char_dev);
> +	}
> +	list_for_each_entry_safe(entry, n, &rmi_character_drivers.list,
> +				 list) {
> +		if (entry->cd == char_driver) {
> +			list_del(&entry->list);
> +			kfree(entry);
> +		}
> +	}
> +	mutex_unlock(&rmi_bus_mutex);
> +
> +	driver_unregister(&char_driver->driver);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(rmi_unregister_character_driver);
> +
> +static int __init rmi_bus_init(void)
> +{
> +	int error;
> +
> +	mutex_init(&rmi_bus_mutex);
> +	INIT_LIST_HEAD(&rmi_supported_functions.list);
> +	INIT_LIST_HEAD(&rmi_character_drivers.list);
> +
> +	error = bus_register(&rmi_bus_type);
> +	if (error < 0) {
> +		pr_err("%s: error registering the RMI bus: %d\n", __func__,
> +		       error);
> +		return error;
> +	}
> +	pr_info("%s: successfully registered RMI bus.\n", __func__);

This is all useless noise. Just do:

	return bus_register(&rmi_bus_type);

> +
> +	return 0;
> +}
> +
> +static void __exit rmi_bus_exit(void)
> +{
> +	struct rmi_function_list *entry, *n;
> +
> +	list_for_each_entry_safe(entry, n, &rmi_supported_functions.list,
> +				 list) {
> +		list_del(&entry->list);
> +		kfree(entry);
> +	}

How can this list be non-free? Your bus code is reference by function
drivers so module count is non zero until all such drivers are unloaded,
and therefore rmi_bus_exit() can not be called.

> +
> +	bus_unregister(&rmi_bus_type);
> +}
> +
> +module_init(rmi_bus_init);
> +module_exit(rmi_bus_exit);
> +
> +MODULE_AUTHOR("Eric Andersson <eric.andersson@xxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("RMI bus");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> new file mode 100644
> index 0000000..07097bb
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -0,0 +1,1488 @@
> +/*
> + * Copyright (c) 2011 Synaptics Incorporated
> + * Copyright (c) 2011 Unixphere
> + *
> + * This driver adds support for generic RMI4 devices from Synpatics. It
> + * implements the mandatory f01 RMI register and depends on the presence of
> + * other required RMI functions.
> + *
> + * The RMI4 specification can be found here (URL split after files/ for
> + * style reasons):
> + * http://www.synaptics.com/sites/default/files/
> + *           511-000136-01-Rev-E-RMI4%20Intrfacing%20Guide.pdf
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any 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.  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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +#include <linux/pm.h>
> +#include <linux/rmi.h>
> +#include "rmi_driver.h"
> +
> +#define DELAY_DEBUG 0
> +#define REGISTER_DEBUG 0
> +
> +#define PDT_END_SCAN_LOCATION	0x0005
> +#define PDT_PROPERTIES_LOCATION 0x00EF
> +#define BSR_LOCATION 0x00FE
> +#define HAS_BSR_MASK 0x20
> +#define HAS_NONSTANDARD_PDT_MASK 0x40
> +#define RMI4_END_OF_PDT(id) ((id) == 0x00 || (id) == 0xff)
> +#define RMI4_MAX_PAGE 0xff
> +#define RMI4_PAGE_SIZE 0x100
> +
> +#define RMI_DEVICE_RESET_CMD	0x01
> +#define DEFAULT_RESET_DELAY_MS	20
> +
> +#ifdef CONFIG_HAS_EARLYSUSPEND
> +static void rmi_driver_early_suspend(struct early_suspend *h);
> +static void rmi_driver_late_resume(struct early_suspend *h);
> +#endif

Does not appear to be in mainline; please trim.

> +
> +/* sysfs files for attributes for driver values. */
> +static ssize_t rmi_driver_hasbsr_show(struct device *dev,
> +				      struct device_attribute *attr, char *buf);

Well, if it does not have bsr why do you even create bsr attribute?

> +
> +static ssize_t rmi_driver_bsr_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf);
> +
> +static ssize_t rmi_driver_bsr_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t count);
> +
> +static ssize_t rmi_driver_enabled_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf);
> +
> +static ssize_t rmi_driver_enabled_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t count);

Should rely on load/unload or bind/unbind; no need for yet another
mechanism.

> +
> +static ssize_t rmi_driver_phys_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf);
> +
> +static ssize_t rmi_driver_version_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf);

Should be /sys/module/xxx/version already.

> +
> +#if REGISTER_DEBUG
> +static ssize_t rmi_driver_reg_store(struct device *dev,
> +					struct device_attribute *attr,
> +
					const char *buf, size_t count);
debugfs


> +#endif
> +
> +#if DELAY_DEBUG
> +static ssize_t rmi_delay_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf);
> +
> +static ssize_t rmi_delay_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t count);

debugfs

> +#endif
> +
> +static int rmi_driver_process_reset_requests(struct rmi_device *rmi_dev);
> +
> +static int rmi_driver_process_config_requests(struct rmi_device *rmi_dev);
> +
> +static int rmi_driver_irq_restore(struct rmi_device *rmi_dev);
> +
> +static struct device_attribute attrs[] = {
> +	__ATTR(hasbsr, RMI_RO_ATTR,
> +	       rmi_driver_hasbsr_show, rmi_store_error),
> +	__ATTR(bsr, RMI_RW_ATTR,
> +	       rmi_driver_bsr_show, rmi_driver_bsr_store),
> +	__ATTR(enabled, RMI_RW_ATTR,
> +	       rmi_driver_enabled_show, rmi_driver_enabled_store),
> +	__ATTR(phys, RMI_RO_ATTR,
> +	       rmi_driver_phys_show, rmi_store_error),
> +#if REGISTER_DEBUG
> +	__ATTR(reg, RMI_WO_ATTR,
> +	       rmi_show_error, rmi_driver_reg_store),
> +#endif
> +#if DELAY_DEBUG
> +	__ATTR(delay, RMI_RW_ATTR,
> +	       rmi_delay_show, rmi_delay_store),
> +#endif
> +	__ATTR(version, RMI_RO_ATTR,
> +	       rmi_driver_version_show, rmi_store_error),
> +};
> +
> +/* Useful helper functions for u8* */

No, they are not. We have *_bit and bitmaps infrastructure in place
already.

> +
> +void u8_set_bit(u8 *target, int pos)
> +{
> +	target[pos/8] |= 1<<pos%8;
> +}
> +
> +void u8_clear_bit(u8 *target, int pos)
> +{
> +	target[pos/8] &= ~(1<<pos%8);
> +}
> +
> +bool u8_is_set(u8 *target, int pos)
> +{
> +	return target[pos/8] & 1<<pos%8;
> +}
> +
> +bool u8_is_any_set(u8 *target, int size)
> +{
> +	int i;
> +	for (i = 0; i < size; i++) {
> +		if (target[i])
> +			return true;
> +	}
> +	return false;
> +}
> +
> +void u8_or(u8 *dest, u8 *target1, u8 *target2, int size)
> +{
> +	int i;
> +	for (i = 0; i < size; i++)
> +		dest[i] = target1[i] | target2[i];
> +}
> +
> +void u8_and(u8 *dest, u8 *target1, u8 *target2, int size)
> +{
> +	int i;
> +	for (i = 0; i < size; i++)
> +		dest[i] = target1[i] & target2[i];
> +}
> +
> +static bool has_bsr(struct rmi_driver_data *data)
> +{
> +	return (data->pdt_props & HAS_BSR_MASK) != 0;
> +}
> +
> +/* Utility routine to set bits in a register. */
> +int rmi_set_bits(struct rmi_device *rmi_dev, unsigned short address,
> +		 unsigned char bits)
> +{
> +	unsigned char reg_contents;
> +	int retval;
> +
> +	retval = rmi_read_block(rmi_dev, address, &reg_contents, 1);
> +	if (retval)
> +		return retval;
> +	reg_contents = reg_contents | bits;
> +	retval = rmi_write_block(rmi_dev, address, &reg_contents, 1);
> +	if (retval == 1)
> +		return 0;
> +	else if (retval == 0)
> +		return -EIO;
> +	return retval;
> +}
> +EXPORT_SYMBOL(rmi_set_bits);
> +
> +/* Utility routine to clear bits in a register. */
> +int rmi_clear_bits(struct rmi_device *rmi_dev, unsigned short address,
> +		   unsigned char bits)
> +{
> +	unsigned char reg_contents;
> +	int retval;
> +
> +	retval = rmi_read_block(rmi_dev, address, &reg_contents, 1);
> +	if (retval)
> +		return retval;
> +	reg_contents = reg_contents & ~bits;
> +	retval = rmi_write_block(rmi_dev, address, &reg_contents, 1);
> +	if (retval == 1)
> +		return 0;
> +	else if (retval == 0)
> +		return -EIO;
> +	return retval;
> +}
> +EXPORT_SYMBOL(rmi_clear_bits);
> +
> +static void rmi_free_function_list(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_function_container *entry, *n;
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +
> +	if (!data) {
> +		dev_err(&rmi_dev->dev, "WTF: No driver data in %s\n", __func__);
> +		return;
> +	}
> +
> +	if (list_empty(&data->rmi_functions.list))
> +		return;
> +
> +	list_for_each_entry_safe(entry, n, &data->rmi_functions.list, list) {
> +		kfree(entry->irq_mask);
> +		list_del(&entry->list);
> +	}
> +}
> +
> +void no_op(struct device *dev)
> +{
> +	dev_dbg(dev, "REMOVING KOBJ!");
> +	kobject_put(&dev->kobj);
> +}
> +
> +static int init_one_function(struct rmi_device *rmi_dev,
> +			     struct rmi_function_container *fc)
> +{
> +	int retval;
> +
> +	if (!fc->fh) {
> +		struct rmi_function_handler *fh =
> +			rmi_get_function_handler(fc->fd.function_number);
> +		if (!fh) {
> +			dev_dbg(&rmi_dev->dev, "No handler for F%02X.\n",
> +				fc->fd.function_number);
> +			return 0;
> +		}
> +		fc->fh = fh;
> +	}
> +
> +	if (!fc->fh->init)
> +		return 0;
> +	/* This memset might not be what we want to do... */
> +	memset(&(fc->dev), 0, sizeof(struct device));
> +	dev_set_name(&(fc->dev), "fn%02x", fc->fd.function_number);
> +	fc->dev.release = no_op;
> +
> +	fc->dev.parent = &rmi_dev->dev;
> +	dev_dbg(&rmi_dev->dev, "%s: Register F%02X.\n", __func__,
> +			fc->fd.function_number);
> +	retval = device_register(&fc->dev);
> +	if (retval) {
> +		dev_err(&rmi_dev->dev, "Failed device_register for F%02X.\n",
> +			fc->fd.function_number);
> +		return retval;
> +	}
> +
> +	retval = fc->fh->init(fc);
> +	if (retval < 0) {
> +		dev_err(&rmi_dev->dev, "Failed to initialize function F%02x\n",
> +			fc->fd.function_number);
> +		goto error_exit;
> +	}
> +
> +	return 0;
> +
> +error_exit:
> +	device_unregister(&fc->dev);
> +	return retval;
> +}
> +
> +static void rmi_driver_fh_add(struct rmi_device *rmi_dev,
> +			      struct rmi_function_handler *fh)
> +{
> +	struct rmi_function_container *entry;
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +
> +	if (!data)
> +		return;
> +	if (fh->func == 0x01) {
> +		if (data->f01_container)
> +			data->f01_container->fh = fh;
> +	} else if (!list_empty(&data->rmi_functions.list)) {
> +		mutex_lock(&data->pdt_mutex);
> +		list_for_each_entry(entry, &data->rmi_functions.list, list)
> +			if (entry->fd.function_number == fh->func) {
> +				entry->fh = fh;
> +				init_one_function(rmi_dev, entry);
> +			}
> +		mutex_unlock(&data->pdt_mutex);
> +	}
> +
> +}
> +
> +static void rmi_driver_fh_remove(struct rmi_device *rmi_dev,
> +				 struct rmi_function_handler *fh)
> +{
> +	struct rmi_function_container *entry, *temp;
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +
> +	list_for_each_entry_safe(entry, temp, &data->rmi_functions.list,
> +									list) {
> +		if (entry->fd.function_number == fh->func) {
> +			if (fh->remove)
> +				fh->remove(entry);
> +
> +			entry->fh = NULL;
> +			device_unregister(&entry->dev);
> +		}
> +	}
> +}
> +
> +
> +static int rmi_driver_process_reset_requests(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct device *dev = &rmi_dev->dev;
> +	struct rmi_function_container *entry;
> +	int error;
> +
> +	/* Device control (F01) is handled before anything else. */
> +
> +	if (data->f01_container->fh->reset) {
> +		error = data->f01_container->fh->reset(data->f01_container);
> +		if (error < 0) {
> +			dev_err(dev, "%s: f%.2x"
> +					" reset handler failed:"
> +					" %d\n", __func__,
> +					data->f01_container->fh->func, error);
> +			return error;
> +		}
> +	}
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list) {
> +		if (entry->fh->reset) {
> +			error = entry->fh->reset(entry);
> +			if (error < 0) {
> +				dev_err(dev, "%s: f%.2x"
> +						" reset handler failed:"
> +						" %d\n", __func__,
> +						entry->fh->func, error);
> +				return error;
> +			}
> +		}
> +
> +	}
> +
> +	return 0;
> +}
> +
> +
> +static int rmi_driver_process_config_requests(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct device *dev = &rmi_dev->dev;
> +	struct rmi_function_container *entry;
> +	int error;
> +
> +	/* Device control (F01) is handled before anything else. */
> +
> +	if (data->f01_container->fh->config) {
> +		error = data->f01_container->fh->config(data->f01_container);
> +		if (error < 0) {
> +			dev_err(dev, "%s: f%.2x"
> +					" config handler failed:"
> +					" %d\n", __func__,
> +					data->f01_container->fh->func, error);
> +			return error;
> +		}
> +	}
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list) {
> +		if (entry->fh->config) {
> +			error = entry->fh->config(entry);
> +			if (error < 0) {
> +				dev_err(dev, "%s: f%.2x"
> +						" config handler failed:"
> +						" %d\n", __func__,
> +						entry->fh->func, error);
> +				return error;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void construct_mask(u8 *mask, int num, int pos)
> +{
> +	int i;
> +
> +	for (i = 0; i < num; i++)
> +		u8_set_bit(mask, pos+i);
> +}
> +
> +static int process_interrupt_requests(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct device *dev = &rmi_dev->dev;
> +	struct rmi_function_container *entry;
> +	u8 irq_status[data->num_of_irq_regs];
> +	u8 irq_bits[data->num_of_irq_regs];
> +	int error;
> +
> +	error = rmi_read_block(rmi_dev,
> +				data->f01_container->fd.data_base_addr + 1,
> +				irq_status, data->num_of_irq_regs);
> +	if (error < 0) {
> +		dev_err(dev, "%s: failed to read irqs.", __func__);
> +		return error;
> +	}
> +	/* Device control (F01) is handled before anything else. */
> +	u8_and(irq_bits, irq_status, data->f01_container->irq_mask,
> +			data->num_of_irq_regs);
> +	if (u8_is_any_set(irq_bits, data->num_of_irq_regs))
> +		data->f01_container->fh->attention(
> +				data->f01_container, irq_bits);
> +
> +	u8_and(irq_status, irq_status, data->current_irq_mask,
> +	       data->num_of_irq_regs);
> +	/* At this point, irq_status has all bits that are set in the
> +	 * interrupt status register and are enabled.
> +	 */
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list)
> +		if (entry->irq_mask && entry->fh && entry->fh->attention) {
> +			u8_and(irq_bits, irq_status, entry->irq_mask,
> +			       data->num_of_irq_regs);
> +			if (u8_is_any_set(irq_bits, data->num_of_irq_regs)) {
> +				error = entry->fh->attention(entry, irq_bits);
> +				if (error < 0)
> +					dev_err(dev, "%s: f%.2x"
> +						" attention handler failed:"
> +						" %d\n", __func__,
> +						entry->fh->func, error);
> +			}
> +		}
> +
> +	return 0;
> +}
> +
> +static int rmi_driver_irq_handler(struct rmi_device *rmi_dev, int irq)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +
> +	/* Can get called before the driver is fully ready to deal with
> +	 * interrupts.
> +	 */
> +	if (!data || !data->f01_container || !data->f01_container->fh) {
> +		dev_warn(&rmi_dev->dev,
> +			 "Not ready to handle interrupts yet!\n");
> +		return 0;
> +	}
> +
> +	return process_interrupt_requests(rmi_dev);
> +}
> +
> +
> +static int rmi_driver_reset_handler(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	int error;
> +
> +	/* Can get called before the driver is fully ready to deal with
> +	 * interrupts.
> +	 */
> +	if (!data || !data->f01_container || !data->f01_container->fh) {
> +		dev_warn(&rmi_dev->dev,
> +			 "Not ready to handle reset yet!\n");
> +		return 0;
> +	}
> +
> +	error = rmi_driver_process_reset_requests(rmi_dev);
> +	if (error < 0)
> +		return error;
> +
> +
> +	error = rmi_driver_process_config_requests(rmi_dev);
> +	if (error < 0)
> +		return error;
> +
> +	if (data->irq_stored) {
> +		error = rmi_driver_irq_restore(rmi_dev);
> +		if (error < 0)
> +			return error;
> +	}
> +
> +	dev_err(&rmi_dev->dev, "DEBUG 5!\n");
> +
> +	return 0;
> +}
> +
> +
> +
> +/*
> + * Construct a function's IRQ mask. This should
> + * be called once and stored.
> + */
> +static u8 *rmi_driver_irq_get_mask(struct rmi_device *rmi_dev,
> +				   struct rmi_function_container *fc) {
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +
> +	u8 *irq_mask = kzalloc(sizeof(u8)*data->num_of_irq_regs, GFP_KERNEL);
> +	if (irq_mask)
> +		construct_mask(irq_mask, fc->num_of_irqs, fc->irq_pos);
> +
> +	return irq_mask;
> +}
> +
> +/*
> + * This pair of functions allows functions like function 54 to request to have
> + * other interupts disabled until the restore function is called. Only one store
> + * happens at a time.
> + */
> +static int rmi_driver_irq_save(struct rmi_device *rmi_dev, u8 * new_ints)
> +{
> +	int retval = 0;
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct device *dev = &rmi_dev->dev;
> +
> +	mutex_lock(&data->irq_mutex);
> +	if (!data->irq_stored) {
> +		/* Save current enabled interupts */
> +		retval = rmi_read_block(rmi_dev,
> +				data->f01_container->fd.control_base_addr+1,
> +				data->irq_mask_store, data->num_of_irq_regs);
> +		if (retval < 0) {
> +			dev_err(dev, "%s: Failed to read enabled interrupts!",
> +								__func__);
> +			goto error_unlock;
> +		}
> +		/*
> +		 * Disable every interupt except for function 54
> +		 * TODO:Will also want to not disable function 1-like functions.
> +		 * No need to take care of this now, since there's no good way
> +		 * to identify them.
> +		 */
> +		retval = rmi_write_block(rmi_dev,
> +				data->f01_container->fd.control_base_addr+1,
> +				new_ints, data->num_of_irq_regs);
> +		if (retval < 0) {
> +			dev_err(dev, "%s: Failed to change enabled interrupts!",
> +								__func__);
> +			goto error_unlock;
> +		}
> +		memcpy(data->current_irq_mask, new_ints,
> +					data->num_of_irq_regs * sizeof(u8));
> +		data->irq_stored = true;
> +	} else {
> +		retval = -ENOSPC; /* No space to store IRQs.*/
> +		dev_err(dev, "%s: Attempted to save values when"
> +						" already stored!", __func__);
> +	}
> +
> +error_unlock:
> +	mutex_unlock(&data->irq_mutex);
> +	return retval;
> +}
> +
> +static int rmi_driver_irq_restore(struct rmi_device *rmi_dev)
> +{
> +	int retval = 0;
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct device *dev = &rmi_dev->dev;
> +	mutex_lock(&data->irq_mutex);
> +
> +	if (data->irq_stored) {
> +		retval = rmi_write_block(rmi_dev,
> +				data->f01_container->fd.control_base_addr+1,
> +				data->irq_mask_store, data->num_of_irq_regs);
> +		if (retval < 0) {
> +			dev_err(dev, "%s: Failed to write enabled interupts!",
> +								__func__);
> +			goto error_unlock;
> +		}
> +		memcpy(data->current_irq_mask, data->irq_mask_store,
> +					data->num_of_irq_regs * sizeof(u8));
> +		data->irq_stored = false;
> +	} else {
> +		retval = -EINVAL;
> +		dev_err(dev, "%s: Attempted to restore values when not stored!",
> +			__func__);
> +	}
> +
> +error_unlock:
> +	mutex_unlock(&data->irq_mutex);
> +	return retval;
> +}
> +
> +static int rmi_driver_fn_generic(struct rmi_device *rmi_dev,
> +				     struct pdt_entry *pdt_ptr,
> +				     int *current_irq_count,
> +				     u16 page_start)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct rmi_function_container *fc = NULL;
> +	int retval = 0;
> +	struct device *dev = &rmi_dev->dev;
> +	struct rmi_device_platform_data *pdata;
> +
> +	pdata = to_rmi_platform_data(rmi_dev);
> +
> +	dev_info(dev, "Initializing F%02X for %s.\n", pdt_ptr->function_number,
> +		pdata->sensor_name);
> +
> +	fc = kzalloc(sizeof(struct rmi_function_container),
> +			GFP_KERNEL);
> +	if (!fc) {
> +		dev_err(dev, "Failed to allocate container for F%02X.\n",
> +			pdt_ptr->function_number);
> +		retval = -ENOMEM;
> +		goto error_free_data;
> +	}
> +
> +	copy_pdt_entry_to_fd(pdt_ptr, &fc->fd, page_start);
> +
> +	fc->rmi_dev = rmi_dev;
> +	fc->num_of_irqs = pdt_ptr->interrupt_source_count;
> +	fc->irq_pos = *current_irq_count;
> +	*current_irq_count += fc->num_of_irqs;
> +
> +	retval = init_one_function(rmi_dev, fc);
> +	if (retval < 0) {
> +		dev_err(dev, "Failed to initialize F%.2x\n",
> +			pdt_ptr->function_number);
> +		kfree(fc);
> +		goto error_free_data;
> +	}
> +
> +	list_add_tail(&fc->list, &data->rmi_functions.list);
> +	return 0;
> +
> +error_free_data:
> +	kfree(fc);
> +	return retval;
> +}
> +
> +/*
> + * F01 was once handled very differently from all other functions.  It is
> + * now only slightly special, and as the driver is refined we expect this
> + * function to go away.
> + */
> +static int rmi_driver_fn_01_specific(struct rmi_device *rmi_dev,
> +				     struct pdt_entry *pdt_ptr,
> +				     int *current_irq_count,
> +				     u16 page_start)
> +{
> +	struct rmi_driver_data *data = NULL;
> +	struct rmi_function_container *fc = NULL;
> +	int retval = 0;
> +	struct device *dev = &rmi_dev->dev;
> +	struct rmi_function_handler *fh =
> +		rmi_get_function_handler(0x01);
> +	struct rmi_device_platform_data *pdata;
> +
> +	pdata = to_rmi_platform_data(rmi_dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	dev_info(dev, "Initializing F01 for %s.\n", pdata->sensor_name);
> +	if (!fh)
> +		dev_dbg(dev, "%s: No function handler for F01?!", __func__);
> +
> +	fc = kzalloc(sizeof(struct rmi_function_container), GFP_KERNEL);
> +	if (!fc) {
> +		retval = -ENOMEM;
> +		return retval;
> +	}
> +
> +	copy_pdt_entry_to_fd(pdt_ptr, &fc->fd, page_start);
> +	fc->num_of_irqs = pdt_ptr->interrupt_source_count;
> +	fc->irq_pos = *current_irq_count;
> +	*current_irq_count += fc->num_of_irqs;
> +
> +	fc->rmi_dev        = rmi_dev;
> +	fc->dev.parent     = &fc->rmi_dev->dev;
> +	fc->fh = fh;
> +
> +	dev_set_name(&(fc->dev), "fn%02x", fc->fd.function_number);
> +	fc->dev.release = no_op;
> +
> +	dev_dbg(dev, "%s: Register F01.\n", __func__);
> +	retval = device_register(&fc->dev);
> +	if (retval) {
> +		dev_err(dev, "%s: Failed device_register for F01.\n", __func__);
> +		goto error_free_data;
> +	}
> +
> +	data->f01_container = fc;
> +
> +	return retval;
> +
> +error_free_data:
> +	kfree(fc);
> +	return retval;
> +}
> +
> +/*
> + * Scan the PDT for F01 so we can force a reset before anything else
> + * is done.  This forces the sensor into a known state, and also
> + * forces application of any pending updates from reflashing the
> + * firmware or configuration.  We have to do this before actually
> + * building the PDT because the reflash might cause various registers
> + * to move around.
> + */
> +static int do_initial_reset(struct rmi_device *rmi_dev)
> +{
> +	struct pdt_entry pdt_entry;
> +	int page;
> +	struct device *dev = &rmi_dev->dev;
> +	bool done = false;
> +	int i;
> +	int retval;
> +	struct rmi_device_platform_data *pdata;
> +
> +	dev_dbg(dev, "Initial reset.\n");
> +	pdata = to_rmi_platform_data(rmi_dev);
> +	for (page = 0; (page <= RMI4_MAX_PAGE) && !done; page++) {
> +		u16 page_start = RMI4_PAGE_SIZE * page;
> +		u16 pdt_start = page_start + PDT_START_SCAN_LOCATION;
> +		u16 pdt_end = page_start + PDT_END_SCAN_LOCATION;
> +
> +		done = true;
> +		for (i = pdt_start; i >= pdt_end; i -= sizeof(pdt_entry)) {
> +			retval = rmi_read_block(rmi_dev, i, (u8 *)&pdt_entry,
> +					       sizeof(pdt_entry));
> +			if (retval != sizeof(pdt_entry)) {
> +				dev_err(dev, "Read PDT entry at 0x%04x"
> +					"failed, code = %d.\n", i, retval);
> +				return retval;
> +			}
> +
> +			if (RMI4_END_OF_PDT(pdt_entry.function_number))
> +				break;
> +			done = false;
> +
> +			if (pdt_entry.function_number == 0x01) {
> +				u16 cmd_addr = page_start +
> +					pdt_entry.command_base_addr;
> +				u8 cmd_buf = RMI_DEVICE_RESET_CMD;
> +				retval = rmi_write_block(rmi_dev, cmd_addr,
> +						&cmd_buf, 1);
> +				if (retval < 0) {
> +					dev_err(dev, "Initial reset failed. "
> +						"Code = %d.\n", retval);
> +					return retval;
> +				}
> +				mdelay(pdata->reset_delay_ms);
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	dev_warn(dev, "WARNING: Failed to find F01 for initial reset.\n");
> +	return -ENODEV;
> +}
> +
> +static int rmi_scan_pdt(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data;
> +	struct pdt_entry pdt_entry;
> +	int page;
> +	struct device *dev = &rmi_dev->dev;
> +	int irq_count = 0;
> +	bool done = false;
> +	int i;
> +	int retval;
> +
> +	dev_info(dev, "Scanning PDT...\n");
> +
> +	data = rmi_get_driverdata(rmi_dev);
> +	mutex_lock(&data->pdt_mutex);
> +
> +	for (page = 0; (page <= RMI4_MAX_PAGE) && !done; page++) {
> +		u16 page_start = RMI4_PAGE_SIZE * page;
> +		u16 pdt_start = page_start + PDT_START_SCAN_LOCATION;
> +		u16 pdt_end = page_start + PDT_END_SCAN_LOCATION;
> +
> +		done = true;
> +		for (i = pdt_start; i >= pdt_end; i -= sizeof(pdt_entry)) {
> +			retval = rmi_read_block(rmi_dev, i, (u8 *)&pdt_entry,
> +					       sizeof(pdt_entry));
> +			if (retval != sizeof(pdt_entry)) {
> +				dev_err(dev, "Read PDT entry at 0x%04x "
> +					"failed.\n", i);
> +				goto error_exit;
> +			}
> +
> +			if (RMI4_END_OF_PDT(pdt_entry.function_number))
> +				break;
> +
> +			dev_dbg(dev, "%s: Found F%.2X on page 0x%02X\n",
> +				__func__, pdt_entry.function_number, page);
> +			done = false;
> +
> +			if (pdt_entry.function_number == 0x01)
> +				retval = rmi_driver_fn_01_specific(rmi_dev,
> +						&pdt_entry, &irq_count,
> +						page_start);
> +			else
> +				retval = rmi_driver_fn_generic(rmi_dev,
> +						&pdt_entry, &irq_count,
> +						page_start);
> +
> +			if (retval)
> +				goto error_exit;
> +		}
> +	}
> +	data->num_of_irq_regs = (irq_count + 7) / 8;
> +	dev_dbg(dev, "%s: Done with PDT scan.\n", __func__);
> +	retval = 0;
> +
> +error_exit:
> +	mutex_unlock(&data->pdt_mutex);
> +	return retval;
> +}
> +
> +static int rmi_driver_probe(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = NULL;
> +	struct rmi_function_container *fc;
> +	struct rmi_device_platform_data *pdata;
> +	int error = 0;
> +	struct device *dev = &rmi_dev->dev;
> +	int attr_count = 0;
> +
> +	dev_dbg(dev, "%s: Starting probe.\n", __func__);
> +
> +	pdata = to_rmi_platform_data(rmi_dev);
> +
> +	data = kzalloc(sizeof(struct rmi_driver_data), GFP_KERNEL);
> +	if (!data) {
> +		dev_err(dev, "%s: Failed to allocate driver data.\n", __func__);
> +		return -ENOMEM;
> +	}
> +	INIT_LIST_HEAD(&data->rmi_functions.list);
> +	rmi_set_driverdata(rmi_dev, data);
> +	mutex_init(&data->pdt_mutex);
> +
> +	if (!pdata->reset_delay_ms)
> +		pdata->reset_delay_ms = DEFAULT_RESET_DELAY_MS;
> +	error = do_initial_reset(rmi_dev);
> +	if (error)
> +		dev_warn(dev, "RMI initial reset failed! Soldiering on.\n");
> +
> +
> +	error = rmi_scan_pdt(rmi_dev);
> +	if (error) {
> +		dev_err(dev, "PDT scan for %s failed with code %d.\n",
> +			pdata->sensor_name, error);
> +		goto err_free_data;
> +	}
> +
> +	if (!data->f01_container) {
> +		dev_err(dev, "missing F01 container!\n");
> +		error = -EINVAL;
> +		goto err_free_data;
> +	}
> +
> +	data->f01_container->irq_mask = kzalloc(
> +			sizeof(u8)*data->num_of_irq_regs, GFP_KERNEL);
> +	if (!data->f01_container->irq_mask) {
> +		dev_err(dev, "Failed to allocate F01 IRQ mask.\n");
> +		error = -ENOMEM;
> +		goto err_free_data;
> +	}
> +	construct_mask(data->f01_container->irq_mask,
> +		       data->f01_container->num_of_irqs,
> +		       data->f01_container->irq_pos);
> +	list_for_each_entry(fc, &data->rmi_functions.list, list)
> +		fc->irq_mask = rmi_driver_irq_get_mask(rmi_dev, fc);
> +
> +	error = rmi_driver_f01_init(rmi_dev);
> +	if (error < 0) {
> +		dev_err(dev, "Failed to initialize F01.\n");
> +		goto err_free_data;
> +	}
> +
> +	error = rmi_read(rmi_dev, PDT_PROPERTIES_LOCATION,
> +			 (char *) &data->pdt_props);
> +	if (error < 0) {
> +		/* we'll print out a warning and continue since
> +		 * failure to get the PDT properties is not a cause to fail
> +		 */
> +		dev_warn(dev, "Could not read PDT properties from 0x%04x. "
> +			 "Assuming 0x00.\n", PDT_PROPERTIES_LOCATION);
> +	}
> +
> +	dev_dbg(dev, "%s: Creating sysfs files.", __func__);
> +	for (attr_count = 0; attr_count < ARRAY_SIZE(attrs); attr_count++) {
> +		error = device_create_file(dev, &attrs[attr_count]);
> +		if (error < 0) {
> +			dev_err(dev, "%s: Failed to create sysfs file %s.\n",
> +				__func__, attrs[attr_count].attr.name);
> +			goto err_free_data;
> +		}
> +	}

Use attribute group or driver infrastructure for registering common
attributes.

> +
> +	__mutex_init(&data->irq_mutex, "irq_mutex", &data->irq_key);

Why not standard mutex_init()?


> +	data->current_irq_mask = kzalloc(sizeof(u8)*data->num_of_irq_regs,

Spaces around arithmetic and other operations are appreciated.

> +					 GFP_KERNEL);
> +	if (!data->current_irq_mask) {
> +		dev_err(dev, "Failed to allocate current_irq_mask.\n");
> +		error = -ENOMEM;
> +		goto err_free_data;
> +	}
> +	error = rmi_read_block(rmi_dev,
> +				data->f01_container->fd.control_base_addr+1,
> +				data->current_irq_mask, data->num_of_irq_regs);
> +	if (error < 0) {
> +		dev_err(dev, "%s: Failed to read current IRQ mask.\n",
> +			__func__);
> +		goto err_free_data;
> +	}
> +	data->irq_mask_store = kzalloc(sizeof(u8)*data->num_of_irq_regs,
> +				       GFP_KERNEL);
> +	if (!data->irq_mask_store) {
> +		dev_err(dev, "Failed to allocate mask store.\n");
> +		error = -ENOMEM;
> +		goto err_free_data;
> +	}
> +
> +#ifdef	CONFIG_PM
> +	data->pm_data = pdata->pm_data;
> +	data->pre_suspend = pdata->pre_suspend;
> +	data->post_resume = pdata->post_resume;

Is it really platform dependent?

> +
> +	mutex_init(&data->suspend_mutex);
> +
> +#ifdef CONFIG_HAS_EARLYSUSPEND
> +	rmi_dev->early_suspend_handler.level =
> +		EARLY_SUSPEND_LEVEL_BLANK_SCREEN + 1;
> +	rmi_dev->early_suspend_handler.suspend = rmi_driver_early_suspend;
> +	rmi_dev->early_suspend_handler.resume = rmi_driver_late_resume;
> +	register_early_suspend(&rmi_dev->early_suspend_handler);

Not in mainline.

> +#endif /* CONFIG_HAS_EARLYSUSPEND */
> +#endif /* CONFIG_PM */
> +	data->enabled = true;
> +
> +	dev_info(dev, "connected RMI device manufacturer: %s product: %s\n",
> +		 data->manufacturer_id == 1 ? "synaptics" : "unknown",
> +		 data->product_id);
> +
> +	return 0;
> +
> + err_free_data:
> +	rmi_free_function_list(rmi_dev);
> +	for (attr_count--; attr_count >= 0; attr_count--)
> +		device_remove_file(dev, &attrs[attr_count]);
> +	if (data) {

You exit earlier if data is NULL.

> +		if (data->f01_container)
> +			kfree(data->f01_container->irq_mask);
> +		kfree(data->irq_mask_store);
> +		kfree(data->current_irq_mask);
> +		kfree(data);
> +		rmi_set_driverdata(rmi_dev, NULL);
> +	}
> +	return error;
> +}
> +
> +#ifdef CONFIG_PM
> +static int rmi_driver_suspend(struct device *dev)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +	struct rmi_function_container *entry;
> +	int retval = 0;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	mutex_lock(&data->suspend_mutex);
> +	if (data->suspended)
> +		goto exit;
> +
> +#ifndef	CONFIG_HAS_EARLYSUSPEND
> +	if (data->pre_suspend) {
> +		retval = data->pre_suspend(data->pm_data);
> +		if (retval)
> +			goto exit;
> +	}
> +#endif  /* !CONFIG_HAS_EARLYSUSPEND */
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list)
> +		if (entry->fh && entry->fh->suspend) {
> +			retval = entry->fh->suspend(entry);
> +			if (retval < 0)
> +				goto exit;
> +		}
> +
> +	if (data->f01_container && data->f01_container->fh
> +				&& data->f01_container->fh->suspend) {
> +		retval = data->f01_container->fh->suspend(data->f01_container);
> +		if (retval < 0)
> +			goto exit;
> +	}
> +	data->suspended = true;
> +
> +exit:
> +	mutex_unlock(&data->suspend_mutex);
> +	return retval;

Once it is better integrated in driver core this will be much simpler.

> +}
> +
> +static int rmi_driver_resume(struct device *dev)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +	struct rmi_function_container *entry;
> +	int retval = 0;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	mutex_lock(&data->suspend_mutex);
> +	if (!data->suspended)
> +		goto exit;
> +
> +	if (data->f01_container && data->f01_container->fh
> +				&& data->f01_container->fh->resume) {
> +		retval = data->f01_container->fh->resume(data->f01_container);
> +		if (retval < 0)
> +			goto exit;
> +	}
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list)
> +		if (entry->fh && entry->fh->resume) {
> +			retval = entry->fh->resume(entry);
> +			if (retval < 0)
> +				goto exit;
> +		}
> +
> +#ifndef	CONFIG_HAS_EARLYSUSPEND
> +	if (data->post_resume) {
> +		retval = data->post_resume(data->pm_data);
> +		if (retval)
> +			goto exit;
> +	}
> +#endif  /* !CONFIG_HAS_EARLYSUSPEND */
> +
> +	data->suspended = false;
> +
> +exit:
> +	mutex_unlock(&data->suspend_mutex);
> +	return retval;

This one too.

> +}
> +
> +#ifdef CONFIG_HAS_EARLYSUSPEND
> +static void rmi_driver_early_suspend(struct early_suspend *h)
> +{
> +	struct rmi_device *rmi_dev =
> +	    container_of(h, struct rmi_device, early_suspend_handler);
> +	struct rmi_driver_data *data;
> +	struct rmi_function_container *entry;
> +	int retval = 0;
> +
> +	data = rmi_get_driverdata(rmi_dev);
> +	dev_dbg(&rmi_dev->dev, "Early suspend.\n");
> +
> +	mutex_lock(&data->suspend_mutex);
> +	if (data->suspended)
> +		goto exit;
> +
> +	if (data->pre_suspend) {
> +		retval = data->pre_suspend(data->pm_data);
> +		if (retval)
> +			goto exit;
> +	}
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list)
> +		if (entry->fh && entry->fh->early_suspend) {
> +			retval = entry->fh->early_suspend(entry);
> +			if (retval < 0)
> +				goto exit;
> +		}
> +
> +	if (data->f01_container && data->f01_container->fh
> +				&& data->f01_container->fh->early_suspend) {
> +		retval = data->f01_container->fh->early_suspend(
> +				data->f01_container);
> +		if (retval < 0)
> +			goto exit;
> +	}
> +	data->suspended = true;
> +
> +exit:
> +	mutex_unlock(&data->suspend_mutex);
> +}
> +
> +static void rmi_driver_late_resume(struct early_suspend *h)
> +{
> +	struct rmi_device *rmi_dev =
> +	    container_of(h, struct rmi_device, early_suspend_handler);
> +	struct rmi_driver_data *data;
> +	struct rmi_function_container *entry;
> +	int retval = 0;
> +
> +	data = rmi_get_driverdata(rmi_dev);
> +	dev_dbg(&rmi_dev->dev, "Late resume.\n");
> +
> +	mutex_lock(&data->suspend_mutex);
> +	if (!data->suspended)
> +		goto exit;
> +
> +	if (data->f01_container && data->f01_container->fh
> +				&& data->f01_container->fh->late_resume) {
> +		retval = data->f01_container->fh->late_resume(
> +				data->f01_container);
> +		if (retval < 0)
> +			goto exit;
> +	}
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list)
> +		if (entry->fh && entry->fh->late_resume) {
> +			retval = entry->fh->late_resume(entry);
> +			if (retval < 0)
> +				goto exit;
> +		}
> +
> +	if (data->post_resume) {
> +		retval = data->post_resume(data->pm_data);
> +		if (retval)
> +			goto exit;
> +	}
> +
> +	data->suspended = false;
> +
> +exit:
> +	mutex_unlock(&data->suspend_mutex);
> +}
> +#endif /* CONFIG_HAS_EARLYSUSPEND */
> +#endif /* CONFIG_PM */
> +
> +static int __devexit rmi_driver_remove(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	struct rmi_function_container *entry;
> +	int i;
> +
> +	list_for_each_entry(entry, &data->rmi_functions.list, list)
> +		if (entry->fh && entry->fh->remove)
> +			entry->fh->remove(entry);
> +
> +	rmi_free_function_list(rmi_dev);
> +	for (i = 0; i < ARRAY_SIZE(attrs); i++)
> +		device_remove_file(&rmi_dev->dev, &attrs[i]);
> +	kfree(data->f01_container->irq_mask);
> +	kfree(data->irq_mask_store);
> +	kfree(data->current_irq_mask);
> +	kfree(data);
> +
> +	return 0;
> +}
> +
> +#ifdef UNIVERSAL_DEV_PM_OPS
> +static UNIVERSAL_DEV_PM_OPS(rmi_driver_pm, rmi_driver_suspend,
> +			    rmi_driver_resume, NULL);
> +#endif
> +
> +static struct rmi_driver sensor_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "rmi-generic",
> +#ifdef UNIVERSAL_DEV_PM_OPS
> +		.pm = &rmi_driver_pm,
> +#endif
> +	},
> +	.probe = rmi_driver_probe,
> +	.irq_handler = rmi_driver_irq_handler,
> +	.reset_handler = rmi_driver_reset_handler,
> +	.fh_add = rmi_driver_fh_add,
> +	.fh_remove = rmi_driver_fh_remove,
> +	.get_func_irq_mask = rmi_driver_irq_get_mask,
> +	.store_irq_mask = rmi_driver_irq_save,
> +	.restore_irq_mask = rmi_driver_irq_restore,
> +	.remove = __devexit_p(rmi_driver_remove)
> +};
> +
> +/* sysfs show and store fns for driver attributes */
> +static ssize_t rmi_driver_hasbsr_show(struct device *dev,
> +				      struct device_attribute *attr,
> +				      char *buf)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u\n", has_bsr(data));
> +}
> +
> +static ssize_t rmi_driver_bsr_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u\n", data->bsr);
> +}
> +
> +static ssize_t rmi_driver_bsr_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	int retval;
> +	unsigned long val;
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	/* need to convert the string data to an actual value */
> +	retval = strict_strtoul(buf, 10, &val);
> +	if (retval < 0) {
> +		dev_err(dev, "Invalid value '%s' written to BSR.\n", buf);
> +		return -EINVAL;
> +	}
> +
> +	retval = rmi_write(rmi_dev, BSR_LOCATION, (unsigned char)val);
> +	if (retval) {
> +		dev_err(dev, "%s : failed to write bsr %u to 0x%x\n",
> +			__func__, (unsigned int)val, BSR_LOCATION);
> +		return retval;
> +	}
> +
> +	data->bsr = val;
> +
> +	return count;
> +}
> +
> +static void disable_sensor(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +
> +	rmi_dev->phys->disable_device(rmi_dev->phys);
> +
> +	data->enabled = false;
> +}
> +
> +static int enable_sensor(struct rmi_device *rmi_dev)
> +{
> +	struct rmi_driver_data *data = rmi_get_driverdata(rmi_dev);
> +	int retval = 0;
> +
> +	retval = rmi_dev->phys->enable_device(rmi_dev->phys);
> +	/* non-zero means error occurred */
> +	if (retval)
> +		return retval;
> +
> +	data->enabled = true;
> +
> +	return 0;
> +}
> +
> +static ssize_t rmi_driver_enabled_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	return snprintf(buf, PAGE_SIZE, "%u\n", data->enabled);
> +}
> +
> +static ssize_t rmi_driver_enabled_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t count)
> +{
> +	int retval;
> +	int new_value;
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	if (sysfs_streq(buf, "0"))
> +		new_value = false;
> +	else if (sysfs_streq(buf, "1"))
> +		new_value = true;
> +	else
> +		return -EINVAL;
> +
> +	if (new_value) {
> +		retval = enable_sensor(rmi_dev);
> +		if (retval) {
> +			dev_err(dev, "Failed to enable sensor, code=%d.\n",
> +				retval);
> +			return -EIO;
> +		}
> +	} else {
> +		disable_sensor(rmi_dev);
> +	}
> +
> +	return count;
> +}
> +
> +#if REGISTER_DEBUG
> +static ssize_t rmi_driver_reg_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t count)
> +{
> +	int retval;
> +	unsigned int address;
> +	unsigned int bytes;
> +	struct rmi_device *rmi_dev;
> +	struct rmi_driver_data *data;
> +	u8 readbuf[128];
> +	unsigned char outbuf[512];
> +	unsigned char *bufptr = outbuf;
> +	int i;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	data = rmi_get_driverdata(rmi_dev);
> +
> +	retval = sscanf(buf, "%x %u", &address, &bytes);
> +	if (retval != 2) {
> +		dev_err(dev, "Invalid input (code %d) for reg store: %s",
> +			retval, buf);
> +		return -EINVAL;
> +	}
> +	if (address < 0 || address > 0xFFFF) {
> +		dev_err(dev, "Invalid address for reg store '%#06x'.\n",
> +			address);
> +		return -EINVAL;
> +	}
> +	if (bytes < 0 || bytes >= sizeof(readbuf) || address+bytes > 65535) {
> +		dev_err(dev, "Invalid byte count for reg store '%d'.\n",
> +			bytes);
> +		return -EINVAL;
> +	}
> +
> +	retval = rmi_read_block(rmi_dev, address, readbuf, bytes);
> +	if (retval != bytes) {
> +		dev_err(dev, "Failed to read %d registers at %#06x, code %d.\n",
> +			bytes, address, retval);
> +		return retval;
> +	}
> +
> +	dev_info(dev, "Reading %d bytes from %#06x.\n", bytes, address);
> +	for (i = 0; i < bytes; i++) {
> +		retval = snprintf(bufptr, 4, "%02X ", readbuf[i]);
> +		if (retval < 0) {
> +			dev_err(dev, "Failed to format string. Code: %d",
> +				retval);
> +			return retval;
> +		}
> +		bufptr += retval;
> +	}
> +	dev_info(dev, "%s\n", outbuf);
> +
> +	return count;
> +}
> +#endif
> +
> +#if DELAY_DEBUG
> +static ssize_t rmi_delay_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t count)
> +{
> +	int retval;
> +	struct rmi_device *rmi_dev;
> +	struct rmi_device_platform_data *pdata;
> +	unsigned int new_read_delay;
> +	unsigned int new_write_delay;
> +	unsigned int new_block_delay;
> +	unsigned int new_pre_delay;
> +	unsigned int new_post_delay;
> +
> +	retval = sscanf(buf, "%u %u %u %u %u", &new_read_delay,
> +			&new_write_delay, &new_block_delay,
> +			&new_pre_delay, &new_post_delay);
> +	if (retval != 5) {
> +		dev_err(dev, "Incorrect number of values provided for delay.");
> +		return -EINVAL;
> +	}
> +	if (new_read_delay < 0) {
> +		dev_err(dev, "Byte delay must be positive microseconds.\n");
> +		return -EINVAL;
> +	}
> +	if (new_write_delay < 0) {
> +		dev_err(dev, "Write delay must be positive microseconds.\n");
> +		return -EINVAL;
> +	}
> +	if (new_block_delay < 0) {
> +		dev_err(dev, "Block delay must be positive microseconds.\n");
> +		return -EINVAL;
> +	}
> +	if (new_pre_delay < 0) {
> +		dev_err(dev,
> +			"Pre-transfer delay must be positive microseconds.\n");
> +		return -EINVAL;
> +	}
> +	if (new_post_delay < 0) {
> +		dev_err(dev,
> +			"Post-transfer delay must be positive microseconds.\n");
> +		return -EINVAL;
> +	}
> +
> +	rmi_dev = to_rmi_device(dev);
> +	pdata = rmi_dev->phys->dev->platform_data;
> +
> +	dev_info(dev, "Setting delays to %u %u %u %u %u.\n", new_read_delay,
> +		 new_write_delay, new_block_delay, new_pre_delay,
> +		 new_post_delay);
> +	pdata->spi_data.read_delay_us = new_read_delay;
> +	pdata->spi_data.write_delay_us = new_write_delay;
> +	pdata->spi_data.block_delay_us = new_block_delay;
> +	pdata->spi_data.pre_delay_us = new_pre_delay;
> +	pdata->spi_data.post_delay_us = new_post_delay;
> +
> +	return count;
> +}
> +
> +static ssize_t rmi_delay_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_device_platform_data *pdata;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	pdata = rmi_dev->phys->dev->platform_data;
> +
> +	return snprintf(buf, PAGE_SIZE, "%d %d %d %d %d\n",
> +		pdata->spi_data.read_delay_us, pdata->spi_data.write_delay_us,
> +		pdata->spi_data.block_delay_us,
> +		pdata->spi_data.pre_delay_us, pdata->spi_data.post_delay_us);

This violates "one value per attribute" principle. Also it does not look
like it is essential for device operation but rather diagnostic
facility. Switch to debugfs?

> +}
> +#endif
> +
> +static ssize_t rmi_driver_phys_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct rmi_device *rmi_dev;
> +	struct rmi_phys_info *info;
> +
> +	rmi_dev = to_rmi_device(dev);
> +	info = &rmi_dev->phys->info;
> +
> +	return snprintf(buf, PAGE_SIZE, "%-5s %ld %ld %ld %ld %ld %ld %ld\n",
> +		 info->proto ? info->proto : "unk",
> +		 info->tx_count, info->tx_bytes, info->tx_errs,
> +		 info->rx_count, info->rx_bytes, info->rx_errs,
> +		 info->attn_count);

Same as delay.

> +}
> +
> +static ssize_t rmi_driver_version_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE, "%s\n",
> +		 RMI_DRIVER_VERSION_STRING);
> +}
> +
> +static int __init rmi_driver_init(void)
> +{
> +	return rmi_register_driver(&sensor_driver);
> +}
> +
> +static void __exit rmi_driver_exit(void)
> +{
> +	rmi_unregister_driver(&sensor_driver);
> +}
> +
> +module_init(rmi_driver_init);
> +module_exit(rmi_driver_exit);
> +
> +MODULE_AUTHOR("Christopher Heiny <cheiny@xxxxxxxxxxxxx");
> +MODULE_DESCRIPTION("RMI generic driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> new file mode 100644
> index 0000000..dc0d9c5
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -0,0 +1,97 @@
> +/*
> + * Copyright (c) 2011 Synaptics Incorporated
> + * Copyright (c) 2011 Unixphere
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any 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.  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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +#ifndef _RMI_DRIVER_H
> +#define _RMI_DRIVER_H
> +
> +#define RMI_DRIVER_MAJOR_VERSION     1
> +#define RMI_DRIVER_MINOR_VERSION     3
> +#define RMI_DRIVER_SUB_MINOR_VERSION 0
> +
> +/* TODO: Figure out some way to construct this string in the define macro
> + * using the values defined above.
> + */
> +#define RMI_DRIVER_VERSION_STRING "1.3.0"
> +
> +
> +#define RMI_PRODUCT_ID_LENGTH    10
> +#define RMI_PRODUCT_INFO_LENGTH   2
> +#define RMI_DATE_CODE_LENGTH      3
> +
> +struct rmi_driver_data {
> +	struct rmi_function_container rmi_functions;
> +
> +	struct rmi_function_container *f01_container;
> +
> +	int num_of_irq_regs;
> +	u8 *current_irq_mask;
> +	u8 *irq_mask_store;
> +	bool irq_stored;
> +	struct mutex irq_mutex;
> +	struct lock_class_key irq_key;
> +	struct mutex pdt_mutex;
> +
> +	unsigned char pdt_props;
> +	unsigned char bsr;
> +	bool enabled;
> +
> +	u8 manufacturer_id;
> +	/* product id + null termination */
> +	u8 product_id[RMI_PRODUCT_ID_LENGTH + 1];
> +
> +#ifdef CONFIG_PM
> +	bool suspended;
> +	struct mutex suspend_mutex;
> +
> +	void *pm_data;
> +	int (*pre_suspend) (const void *pm_data);
> +	int (*post_resume) (const void *pm_data);
> +#endif
> +
> +	void *data;
> +};
> +
> +struct pdt_entry {
> +	u8 query_base_addr:8;
> +	u8 command_base_addr:8;
> +	u8 control_base_addr:8;
> +	u8 data_base_addr:8;
> +	u8 interrupt_source_count:3;
> +	u8 bits3and4:2;
> +	u8 function_version:2;
> +	u8 bit7:1;
> +	u8 function_number:8;
> +};
> +
> +int rmi_driver_f01_init(struct rmi_device *rmi_dev);
> +
> +static inline void copy_pdt_entry_to_fd(struct pdt_entry *pdt,
> +				 struct rmi_function_descriptor *fd,
> +				 u16 page_start)
> +{
> +	fd->query_base_addr = pdt->query_base_addr + page_start;
> +	fd->command_base_addr = pdt->command_base_addr + page_start;
> +	fd->control_base_addr = pdt->control_base_addr + page_start;
> +	fd->data_base_addr = pdt->data_base_addr + page_start;
> +	fd->function_number = pdt->function_number;
> +	fd->interrupt_source_count = pdt->interrupt_source_count;
> +	fd->function_version = pdt->function_version;
> +}
> +
> +#endif
> +

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/