| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 4 Jan 2012 23:53:39 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Christopher Heiny <cheiny@...aptics.com>
Cc: Jean Delvare <khali@...ux-fr.org>,
Linux Kernel <linux-kernel@...r.kernel.org>,
Linux Input <linux-input@...r.kernel.org>,
Joerie de Gram <j.de.gram@...il.com>,
Linus Walleij <linus.walleij@...ricsson.com>,
Naveen Kumar Gaddipati <naveen.gaddipati@...ricsson.com>
Subject: Re: [RFC PATCH 10/11] input: RMI4 F19 - capacitive buttons
Hi Christopher,
On Wed, Dec 21, 2011 at 06:10:01PM -0800, Christopher Heiny wrote:
> Signed-off-by: Christopher Heiny <cheiny@...aptics.com>
A bunch of comments generally applicable to all patches.
I have not looked at the core thoroughly yet.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Cc: Linus Walleij <linus.walleij@...ricsson.com>
> Cc: Naveen Kumar Gaddipati <naveen.gaddipati@...ricsson.com>
> Cc: Joeri de Gram <j.de.gram@...il.com>
>
> ---
>
> drivers/input/rmi4/rmi_f19.c | 1571 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 1571 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f19.c b/drivers/input/rmi4/rmi_f19.c
> new file mode 100644
> index 0000000..a678f30
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_f19.c
> @@ -0,0 +1,1571 @@
> +/*
> + * Copyright (c) 2011 Synaptics Incorporated
> + *
> + * 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/rmi.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +
> +#define QUERY_BASE_INDEX 1
> +#define MAX_LEN 256
> +
> +struct f19_0d_query {
> + union {
> + struct {
> + u8 configurable:1;
> + u8 has_sensitivity_adjust:1;
> + u8 has_hysteresis_threshold:1;
> + };
> + u8 f19_0d_query0;
> + };
> + u8 f19_0d_query1:5;
> +};
> +
> +struct f19_0d_control_0 {
> + union {
> + struct {
> + u8 button_usage:2;
> + u8 filter_mode:2;
> + };
> + u8 f19_0d_control0;
> + };
Anonymous union in a struct is kind of pointless...
> +};
> +
> +struct f19_0d_control_1 {
> + u8 int_enabled_button;
> +};
> +
> +struct f19_0d_control_2 {
> + u8 single_button;
> +};
> +
> +struct f19_0d_control_3_4 {
> + u8 sensor_map_button:7;
> + /*u8 sensitivity_button;*/
> +};
> +
> +struct f19_0d_control_5 {
> + u8 sensitivity_adj;
> +};
> +struct f19_0d_control_6 {
> + u8 hysteresis_threshold;
> +};
> +
> +struct f19_0d_control {
> + struct f19_0d_control_0 *general_control;
Single instance, does not need to be allocated separately.
> + struct f19_0d_control_1 *button_int_enable;
> + struct f19_0d_control_2 *single_button_participation;
> + struct f19_0d_control_3_4 *sensor_map;
This should probably be an array of
struct f19_button_ctrl {
struct f19_0d_control_1 int_enable;
struct f19_0d_control_2 participation;
struct f19_0d_control_3_4 sensor_map;
};
located at the end of f19_0d_control structure so it can be all
allocated in one shot.
> + struct f19_0d_control_5 *all_button_sensitivity_adj;
Single instance, does not need to be allocated separately.
> + struct f19_0d_control_6 *all_button_hysteresis_threshold;
Single instance, does not need to be allocated separately.
> +};
> +/* data specific to fn $19 that needs to be kept around */
> +struct f19_data {
> + struct f19_0d_control *button_control;
> + struct f19_0d_query button_query;
> + u8 button_rezero;
> + bool *button_down;
Just let input core track this for you.
> + unsigned char button_count;
> + unsigned char button_data_buffer_size;
Your fingers must be hurting by the time you finished writing this
driver...
> + unsigned char *button_data_buffer;
> + unsigned char *button_map;
Linux keycodes are wider than 8 bits, should be unsigned short.
> + char input_name[MAX_LEN];
> + char input_phys[MAX_LEN];
> + struct input_dev *input;
> +};
> +
> +static ssize_t rmi_f19_button_count_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +
> +static ssize_t rmi_f19_button_map_show(struct device *dev,
> + struct device_attribute *attr, char *buf);
> +
> +static ssize_t rmi_f19_button_map_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_rezero_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_rezero_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_has_hysteresis_threshold_show(struct device *dev,
> + struct device_attribute *attr, char *buf);
> +static ssize_t rmi_f19_has_sensitivity_adjust_show(struct device *dev,
> + struct device_attribute *attr, char *buf);
> +static ssize_t rmi_f19_configurable_show(struct device *dev,
> + struct device_attribute *attr, char *buf);
> +static ssize_t rmi_f19_filter_mode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_filter_mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_button_usage_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_button_usage_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_interrupt_enable_button_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_interrupt_enable_button_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_single_button_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_single_button_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_sensor_map_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_sensor_map_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_sensitivity_adjust_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_sensitivity_adjust_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +static ssize_t rmi_f19_hysteresis_threshold_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +static ssize_t rmi_f19_hysteresis_threshold_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count);
> +
> +
> +
> +static inline int rmi_f19_alloc_memory(struct rmi_function_container *fc);
> +
> +static inline void rmi_f19_free_memory(struct rmi_function_container *fc);
> +
> +static inline int rmi_f19_initialize(struct rmi_function_container *fc);
> +
> +static inline int rmi_f19_register_device(struct rmi_function_container *fc);
> +
> +static inline int rmi_f19_create_sysfs(struct rmi_function_container *fc);
> +
> +static int rmi_f19_config(struct rmi_function_container *fc);
> +
> +static int rmi_f19_reset(struct rmi_function_container *fc);
> +
> +static struct device_attribute attrs[] = {
> + __ATTR(button_count, RMI_RO_ATTR,
> + rmi_f19_button_count_show, rmi_store_error),
Why not NULL instead of rmi_store_error?
> + __ATTR(button_map, RMI_RW_ATTR,
> + rmi_f19_button_map_show, rmi_f19_button_map_store),
> + __ATTR(rezero, RMI_RW_ATTR,
> + rmi_f19_rezero_show, rmi_f19_rezero_store),
> + __ATTR(has_hysteresis_threshold, RMI_RO_ATTR,
> + rmi_f19_has_hysteresis_threshold_show, rmi_store_error),
> + __ATTR(has_sensitivity_adjust, RMI_RO_ATTR,
> + rmi_f19_has_sensitivity_adjust_show, rmi_store_error),
> + __ATTR(configurable, RMI_RO_ATTR,
> + rmi_f19_configurable_show, rmi_store_error),
> + __ATTR(filter_mode, RMI_RW_ATTR,
> + rmi_f19_filter_mode_show, rmi_f19_filter_mode_store),
> + __ATTR(button_usage, RMI_RW_ATTR,
> + rmi_f19_button_usage_show, rmi_f19_button_usage_store),
> + __ATTR(interrupt_enable_button, RMI_RW_ATTR,
> + rmi_f19_interrupt_enable_button_show,
> + rmi_f19_interrupt_enable_button_store),
> + __ATTR(single_button, RMI_RW_ATTR,
> + rmi_f19_single_button_show, rmi_f19_single_button_store),
> + __ATTR(sensor_map, RMI_RW_ATTR,
> + rmi_f19_sensor_map_show, rmi_f19_sensor_map_store),
> + __ATTR(sensitivity_adjust, RMI_RW_ATTR,
> + rmi_f19_sensitivity_adjust_show,
> + rmi_f19_sensitivity_adjust_store),
> + __ATTR(hysteresis_threshold, RMI_RW_ATTR,
> + rmi_f19_hysteresis_threshold_show,
> + rmi_f19_hysteresis_threshold_store)
> +};
> +
> +
> +int rmi_f19_read_control_parameters(struct rmi_device *rmi_dev,
> + struct f19_0d_control *button_control,
> + unsigned char button_count,
> + unsigned char int_button_enabled_count,
> + u8 ctrl_base_addr)
> +{
> + int error = 0;
> + int i;
> +
> + if (button_control->general_control) {
It can't be NULL.
> + error = rmi_read_block(rmi_dev, ctrl_base_addr,
> + (u8 *)button_control->general_control,
> + sizeof(struct f19_0d_control_0));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read f19_0d_control_0, code:"
> + " %d.\n", error);
> + return error;
> + }
> + ctrl_base_addr = ctrl_base_addr +
> + sizeof(struct f19_0d_control_0);
> + }
> +
> + if (button_control->button_int_enable) {
Neither can this.
> + for (i = 0; i < int_button_enabled_count; i++) {
> + error = rmi_read_block(rmi_dev, ctrl_base_addr,
> + (u8 *)&button_control->button_int_enable[i],
> + sizeof(struct f19_0d_control_1));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read f19_0d_control_2,"
> + " code: %d.\n", error);
> + return error;
> + }
> + ctrl_base_addr = ctrl_base_addr +
> + sizeof(struct f19_0d_control_1);
> + }
> + }
> +
> + if (button_control->single_button_participation) {
Nor this.
> + for (i = 0; i < int_button_enabled_count; i++) {
> + error = rmi_read_block(rmi_dev, ctrl_base_addr,
> + (u8 *)&button_control->
> + single_button_participation[i],
> + sizeof(struct f19_0d_control_2));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read f19_0d_control_2,"
> + " code: %d.\n", error);
> + return error;
> + }
> + ctrl_base_addr = ctrl_base_addr +
> + sizeof(struct f19_0d_control_2);
> + }
> + }
> +
> + if (button_control->sensor_map) {
Nor this...
> + for (i = 0; i < button_count; i++) {
> + error = rmi_read_block(rmi_dev, ctrl_base_addr,
> + (u8 *)&button_control->sensor_map[i],
> + sizeof(struct f19_0d_control_3_4));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read f19_0d_control_3_4,"
> + " code: %d.\n", error);
> + return error;
> + }
> + ctrl_base_addr = ctrl_base_addr +
> + sizeof(struct f19_0d_control_3_4);
> + }
> + }
> +
> + if (button_control->all_button_sensitivity_adj) {
Nor this.
> + error = rmi_read_block(rmi_dev, ctrl_base_addr,
> + (u8 *)button_control->
> + all_button_sensitivity_adj,
> + sizeof(struct f19_0d_control_5));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read f19_0d_control_5,"
> + " code: %d.\n", error);
> + return error;
> + }
> + ctrl_base_addr = ctrl_base_addr +
> + sizeof(struct f19_0d_control_5);
> + }
> +
> + if (button_control->all_button_hysteresis_threshold) {
...
> + error = rmi_read_block(rmi_dev, ctrl_base_addr,
> + (u8 *)button_control->
> + all_button_hysteresis_threshold,
> + sizeof(struct f19_0d_control_6));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read f19_0d_control_6,"
> + " code: %d.\n", error);
> + return error;
> + }
> + ctrl_base_addr = ctrl_base_addr +
> + sizeof(struct f19_0d_control_6);
> + }
> + return 0;
> +}
> +
> +
> +static int rmi_f19_init(struct rmi_function_container *fc)
> +{
> + int rc;
> +
> + rc = rmi_f19_alloc_memory(fc);
> + if (rc < 0)
> + goto err_free_data;
> +
> + rc = rmi_f19_initialize(fc);
> + if (rc < 0)
> + goto err_free_data;
> +
> + rc = rmi_f19_register_device(fc);
> + if (rc < 0)
> + goto err_free_data;
> +
> + rc = rmi_f19_create_sysfs(fc);
> + if (rc < 0)
> + goto err_free_data;
> +
> + return 0;
> +
> +err_free_data:
> + rmi_f19_free_memory(fc);
> +
> + return rc;
> +}
> +
> +
> +static inline int rmi_f19_alloc_memory(struct rmi_function_container *fc)
Please drop inline throughout, let compiler do its job.
> +{
> + struct f19_data *f19;
> + int rc;
> +
> + f19 = kzalloc(sizeof(struct f19_data), GFP_KERNEL);
> + if (!f19) {
> + dev_err(&fc->dev, "Failed to allocate function data.\n");
> + return -ENOMEM;
> + }
> + fc->data = f19;
> +
> + rc = rmi_read_block(fc->rmi_dev,
> + fc->fd.query_base_addr,
> + (u8 *)&f19->button_query,
> + sizeof(struct f19_0d_query));
> + if (rc < 0) {
> + dev_err(&fc->dev, "Failed to read query register.\n");
> + return rc;
> + }
> +
> + f19->button_count = f19->button_query.f19_0d_query1;
> +
> + f19->button_down = kcalloc(f19->button_count,
> + sizeof(bool), GFP_KERNEL);
> + if (!f19->button_down) {
> + dev_err(&fc->dev, "Failed to allocate button state buffer.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_data_buffer_size = (f19->button_count + 7) / 8;
DIV_ROUND_UP(f19->button_count, 8);
> + f19->button_data_buffer =
> + kcalloc(f19->button_data_buffer_size,
> + sizeof(unsigned char), GFP_KERNEL);
> + if (!f19->button_data_buffer) {
> + dev_err(&fc->dev, "Failed to allocate button data buffer.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_map = kcalloc(f19->button_count,
> + sizeof(unsigned char), GFP_KERNEL);
> + if (!f19->button_map) {
> + dev_err(&fc->dev, "Failed to allocate button map.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_control = kzalloc(sizeof(struct f19_0d_control),
> + GFP_KERNEL);
If you allocate a single copy of this structure why isn't it a member of
struct f19_data to begin with?
> + if (!f19->button_control) {
> + dev_err(&fc->dev, "Failed to allocate button_control.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_control->general_control =
> + kzalloc(sizeof(struct f19_0d_control_0), GFP_KERNEL);
This a single byte, why do you need to allocate it separately?
> + if (!f19->button_control->general_control) {
> + dev_err(&fc->dev, "Failed to allocate"
> + " f19_0d_control_0.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_control->button_int_enable =
> + kzalloc(f19->button_data_buffer_size *
> + sizeof(struct f19_0d_control_2), GFP_KERNEL);
> + if (!f19->button_control->button_int_enable) {
> + dev_err(&fc->dev, "Failed to allocate f19_0d_control_1.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_control->single_button_participation =
> + kzalloc(f19->button_data_buffer_size *
> + sizeof(struct f19_0d_control_2), GFP_KERNEL);
> + if (!f19->button_control->single_button_participation) {
> + dev_err(&fc->dev, "Failed to allocate"
> + " f19_0d_control_2.\n");
Do not split error messages; it's OK if they exceed 80 column limit.
> + return -ENOMEM;
> + }
> +
> + f19->button_control->sensor_map =
> + kzalloc(f19->button_count *
> + sizeof(struct f19_0d_control_3_4), GFP_KERNEL);
> + if (!f19->button_control->sensor_map) {
> + dev_err(&fc->dev, "Failed to allocate"
> + " f19_0d_control_3_4.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_control->all_button_sensitivity_adj =
> + kzalloc(sizeof(struct f19_0d_control_5), GFP_KERNEL);
> + if (!f19->button_control->all_button_sensitivity_adj) {
> + dev_err(&fc->dev, "Failed to allocate"
> + " f19_0d_control_5.\n");
> + return -ENOMEM;
> + }
> +
> + f19->button_control->all_button_hysteresis_threshold =
> + kzalloc(sizeof(struct f19_0d_control_6), GFP_KERNEL);
> + if (!f19->button_control->all_button_hysteresis_threshold) {
> + dev_err(&fc->dev, "Failed to allocate"
> + " f19_0d_control_6.\n");
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +
> +
> +static inline void rmi_f19_free_memory(struct rmi_function_container *fc)
> +{
> + struct f19_data *f19 = fc->data;
> +
> + if (f19) {
Can it be NULL? How?
> + kfree(f19->button_down);
> + kfree(f19->button_data_buffer);
> + kfree(f19->button_map);
> + if (f19->button_control) {
> + kfree(f19->button_control->general_control);
> + kfree(f19->button_control->button_int_enable);
> + kfree(f19->button_control->single_button_participation);
> + kfree(f19->button_control->sensor_map);
> + kfree(f19->button_control->all_button_sensitivity_adj);
> + kfree(f19->button_control->
> + all_button_hysteresis_threshold);
> + kfree(f19->button_control);
> + }
> + kfree(f19);
> + fc->data = NULL;
> + }
> +}
> +
> +
> +static inline int rmi_f19_initialize(struct rmi_function_container *fc)
> +{
> + struct rmi_device *rmi_dev = fc->rmi_dev;
> + struct rmi_device_platform_data *pdata;
> + struct f19_data *f19 = fc->data;
> + int i;
> + int rc;
> +
> + dev_info(&fc->dev, "Intializing F19 values.");
dev_dbg()
> +
> + /* initial all default values for f19 data here */
> + rc = rmi_read(rmi_dev, fc->fd.command_base_addr,
> + (u8 *)&f19->button_rezero);
> + if (rc < 0) {
> + dev_err(&fc->dev, "Failed to read command register.\n");
> + return rc;
> + }
> + f19->button_rezero = f19->button_rezero & 1;
> +
> + pdata = to_rmi_platform_data(rmi_dev);
> + if (pdata) {
> + if (!pdata->button_map) {
> + dev_warn(&fc->dev, "%s - button_map is NULL", __func__);
> + } else if (pdata->button_map->nbuttons != f19->button_count) {
> + dev_warn(&fc->dev,
> + "Platformdata button map size (%d) != number "
> + "of buttons on device (%d) - ignored.\n",
> + pdata->button_map->nbuttons,
> + f19->button_count);
> + } else if (!pdata->button_map->map) {
> + dev_warn(&fc->dev,
> + "Platformdata button map is missing!\n");
> + } else {
> + for (i = 0; i < pdata->button_map->nbuttons; i++)
> + f19->button_map[i] = pdata->button_map->map[i];
> + }
> + }
> +
> + rc = rmi_f19_read_control_parameters(rmi_dev,
> + f19->button_control,
> + f19->button_count,
> + f19->button_data_buffer_size,
> + fc->fd.control_base_addr);
> + if (rc < 0) {
> + dev_err(&fc->dev,
> + "Failed to initialize F19 control params.\n");
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +
> +
> +static inline int rmi_f19_register_device(struct rmi_function_container *fc)
> +{
> + struct rmi_device *rmi_dev = fc->rmi_dev;
> + struct input_dev *input_dev;
> + struct f19_data *f19 = fc->data;
> + int i;
> + int rc;
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + dev_err(&fc->dev, "Failed to allocate input device.\n");
> + return -ENOMEM;
> + }
> +
> + f19->input = input_dev;
> + snprintf(f19->input_name, MAX_LEN, "%sfn%02x", dev_name(&rmi_dev->dev),
> + fc->fd.function_number);
> + input_dev->name = f19->input_name;
> + snprintf(f19->input_phys, MAX_LEN, "%s/input0", input_dev->name);
> + input_dev->phys = f19->input_phys;
> + input_dev->dev.parent = &rmi_dev->dev;
> + input_set_drvdata(input_dev, f19);
> +
> + /* Set up any input events. */
> + set_bit(EV_SYN, input_dev->evbit);
> + set_bit(EV_KEY, input_dev->evbit);
__set_bit(), no need to lock the bus.
> + /* set bits for each button... */
> + for (i = 0; i < f19->button_count; i++)
> + set_bit(f19->button_map[i], input_dev->keybit);
> + rc = input_register_device(input_dev);
> + if (rc < 0) {
> + dev_err(&fc->dev, "Failed to register input device.\n");
> + goto error_free_device;
> + }
> +
> + return 0;
> +
> +error_free_device:
> + input_free_device(input_dev);
> +
> + return rc;
> +}
> +
> +
> +static inline int rmi_f19_create_sysfs(struct rmi_function_container *fc)
> +{
> + int attr_count = 0;
> + int rc;
> +
> + dev_dbg(&fc->dev, "Creating sysfs files.\n");
> + /* Set up sysfs device attributes. */
> + for (attr_count = 0; attr_count < ARRAY_SIZE(attrs); attr_count++) {
> + if (sysfs_create_file
> + (&fc->dev.kobj, &attrs[attr_count].attr) < 0) {
> + dev_err(&fc->dev,
> + "Failed to create sysfs file for %s.",
> + attrs[attr_count].attr.name);
> + rc = -ENODEV;
> + goto err_remove_sysfs;
> + }
> + }
This is called attribute group, please use it.
> +
> + return 0;
> +
> +err_remove_sysfs:
> + for (attr_count--; attr_count >= 0; attr_count--)
> + sysfs_remove_file(&fc->dev.kobj,
> + &attrs[attr_count].attr);
> + return rc;
> +
> +}
> +
> +
> +
> +static int rmi_f19_config(struct rmi_function_container *fc)
> +{
> + struct f19_data *data;
> + int retval;
> + int ctrl_base_addr;
> + int button_reg;
> +
> + data = fc->data;
> +
> + ctrl_base_addr = fc->fd.control_base_addr;
> +
> + button_reg = (data->button_count / 7) + 1;
Please combine variable declarations and initializations - this makes
code a bit less verbose and easier to read.
> +
> + retval = rmi_write_block(fc->rmi_dev, ctrl_base_addr,
> + (u8 *)data->button_control->general_control,
> + sizeof(struct f19_0d_control_0));
> + if (retval < 0) {
> + dev_err(&fc->dev, "%s : Could not write general_control to 0x%x\n",
> + __func__, fc->fd.control_base_addr);
> + return retval;
> + }
> +
> + ctrl_base_addr += sizeof(struct f19_0d_control_0);
> + retval = rmi_write_block(fc->rmi_dev, ctrl_base_addr,
> + (u8 *)data->button_control->button_int_enable,
> + sizeof(struct f19_0d_control_1)*(button_reg + 1));
> + if (retval < 0) {
> + dev_err(&fc->dev, "%s : Could not write interrupt_enable_store"
> + " to 0x%x\n", __func__, ctrl_base_addr);
> + return retval;
> + }
> +
> + ctrl_base_addr += sizeof(struct f19_0d_control_2)*(button_reg + 1);
> + retval = rmi_write_block(fc->rmi_dev, ctrl_base_addr,
> + (u8 *)data->button_control->single_button_participation,
> + sizeof(struct f19_0d_control_2)*(button_reg + 1));
> + if (retval < 0) {
> + dev_err(&fc->dev,
> + "%s : Could not write interrupt_enable_store to"
> + " 0x%x\n", __func__, ctrl_base_addr);
> + return -EINVAL;
> + }
> +
> + ctrl_base_addr = fc->fd.control_base_addr +
> + sizeof(struct f19_0d_control_0) +
> + sizeof(struct f19_0d_control_1)*button_reg +
> + sizeof(struct f19_0d_control_2)*button_reg;
> + retval = rmi_write_block(fc->rmi_dev, ctrl_base_addr,
> + (u8 *)data->button_control->sensor_map,
> + sizeof(struct f19_0d_control_3_4)*data->button_count);
> + if (retval < 0) {
> + dev_err(&fc->dev, "%s : Could not sensor_map_store to 0x%x\n",
> + __func__, ctrl_base_addr);
> + return -EINVAL;
> + }
> + /* write back to the control register */
> + ctrl_base_addr += sizeof(struct f19_0d_control_3_4)*data->button_count;
> + retval = rmi_write_block(fc->rmi_dev, ctrl_base_addr,
> + (u8 *)data->button_control->all_button_sensitivity_adj,
> + sizeof(struct f19_0d_control_5));
> + if (retval < 0) {
> + dev_err(&fc->dev, "%s : Could not sensitivity_adjust_store to"
> + " 0x%x\n", __func__, ctrl_base_addr);
> + return retval;
> + }
> +
> + ctrl_base_addr += sizeof(struct f19_0d_control_5);
> + retval = rmi_write_block(fc->rmi_dev, ctrl_base_addr,
> + (u8 *)data->button_control->all_button_sensitivity_adj,
> + sizeof(struct f19_0d_control_6));
> + if (retval < 0) {
> + dev_err(&fc->dev, "%s : Could not write all_button hysteresis "
> + "threshold to 0x%x\n", __func__, ctrl_base_addr);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +
> +static int rmi_f19_reset(struct rmi_function_container *fc)
> +{
> + /* we do nnothing here */
> + return 0;
> +}
> +
> +
> +static void rmi_f19_remove(struct rmi_function_container *fc)
> +{
> + struct f19_data *f19 = fc->data;
> + int attr_count = 0;
> +
> + for (attr_count = 0; attr_count < ARRAY_SIZE(attrs); attr_count++)
> + sysfs_remove_file(&fc->dev.kobj,
> + &attrs[attr_count].attr);
> +
> + input_unregister_device(f19->input);
> +
> + rmi_f19_free_memory(fc);
> +}
> +
> +int rmi_f19_attention(struct rmi_function_container *fc, u8 *irq_bits)
static.
> +{
> + struct rmi_device *rmi_dev = fc->rmi_dev;
> + struct f19_data *f19 = fc->data;
> + u8 data_base_addr = fc->fd.data_base_addr;
> + int error;
> + int button;
> +
> + /* Read the button data. */
> +
> + error = rmi_read_block(rmi_dev, data_base_addr, f19->button_data_buffer,
> + f19->button_data_buffer_size);
> + if (error < 0) {
> + dev_err(&fc->dev, "%s: Failed to read button data registers.\n",
> + __func__);
> + return error;
> + }
> +
> + /* Generate events for buttons that change state. */
> + for (button = 0; button < f19->button_count;
> + button++) {
> + int button_reg;
> + int button_shift;
> + bool button_status;
> +
> + /* determine which data byte the button status is in */
> + button_reg = button / 7;
> + /* bit shift to get button's status */
> + button_shift = button % 8;
> + button_status =
> + ((f19->button_data_buffer[button_reg] >> button_shift)
> + & 0x01) != 0;
> +
> + /* if the button state changed from the last time report it
> + * and store the new state */
> + if (button_status != f19->button_down[button]) {
> + dev_dbg(&fc->dev, "%s: Button %d (code %d) -> %d.\n",
> + __func__, button, f19->button_map[button],
> + button_status);
> + /* Generate an event here. */
> + input_report_key(f19->input, f19->button_map[button],
> + button_status);
> + f19->button_down[button] = button_status;
> + }
> + }
All of the above could be reduced to:
for (button = 0; button < f19->button_count; button++)
input_report_key(f19->input, f19->button_map[button],
test_bit(button, f19->button_data_buffer);
> +
> + input_sync(f19->input); /* sync after groups of events */
> + return 0;
> +}
> +
> +static struct rmi_function_handler function_handler = {
> + .func = 0x19,
> + .init = rmi_f19_init,
> + .config = rmi_f19_config,
> + .reset = rmi_f19_reset,
> + .attention = rmi_f19_attention,
> + .remove = rmi_f19_remove
> +};
> +
> +static int __init rmi_f19_module_init(void)
> +{
> + int error;
> +
> + error = rmi_register_function_driver(&function_handler);
> + if (error < 0) {
> + pr_err("%s: register failed!\n", __func__);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static void rmi_f19_module_exit(void)
> +{
> + rmi_unregister_function_driver(&function_handler);
> +}
> +
> +static ssize_t rmi_f19_filter_mode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_control->general_control->filter_mode);
> +
> +}
> +
> +static ssize_t rmi_f19_filter_mode_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + unsigned int new_value;
> + int result;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + if (sscanf(buf, "%u", &new_value) != 1) {
> + dev_err(dev,
> + "%s: Error - filter_mode_store has an "
> + "invalid len.\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + if (new_value < 0 || new_value > 4) {
> + dev_err(dev, "%s: Error - filter_mode_store has an "
> + "invalid value %d.\n",
> + __func__, new_value);
> + return -EINVAL;
> + }
> + data->button_control->general_control->filter_mode = new_value;
> + result = rmi_write_block(fc->rmi_dev, fc->fd.control_base_addr,
> + (u8 *)data->button_control->general_control,
> + sizeof(struct f19_0d_control_0));
> + if (result < 0) {
> + dev_err(dev, "%s : Could not write filter_mode_store to 0x%x\n",
> + __func__, fc->fd.control_base_addr);
> + return result;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t rmi_f19_button_usage_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_control->general_control->button_usage);
> +
> +}
> +
> +static ssize_t rmi_f19_button_usage_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + unsigned int new_value;
> + int result;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + if (sscanf(buf, "%u", &new_value) != 1) {
> + dev_err(dev,
> + "%s: Error - button_usage_store has an "
> + "invalid len.\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + if (new_value < 0 || new_value > 4) {
> + dev_err(dev, "%s: Error - button_usage_store has an "
> + "invalid value %d.\n",
> + __func__, new_value);
> + return -EINVAL;
> + }
> + data->button_control->general_control->button_usage = new_value;
> + result = rmi_write_block(fc->rmi_dev, fc->fd.control_base_addr,
> + (u8 *)data->button_control->general_control,
> + sizeof(struct f19_0d_control_0));
> + if (result < 0) {
> + dev_err(dev, "%s: Could not write button_usage_store to 0x%x\n",
> + __func__, fc->fd.control_base_addr);
> + return result;
> + }
> +
> + return count;
> +
> +}
> +
> +static ssize_t rmi_f19_interrupt_enable_button_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int i, len, total_len = 0;
> + char *current_buf = buf;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + /* loop through each button map value and copy its
> + * string representation into buf */
> + for (i = 0; i < data->button_count; i++) {
> + int button_reg;
> + int button_shift;
> + int interrupt_button;
> +
> + button_reg = i / 7;
> + button_shift = i % 8;
> + interrupt_button =
> + ((data->button_control->
> + button_int_enable[button_reg].int_enabled_button >>
> + button_shift) & 0x01);
> +
> + /* get next button mapping value and write it to buf */
> + len = snprintf(current_buf, PAGE_SIZE - total_len,
> + "%u ", interrupt_button);
> + /* bump up ptr to next location in buf if the
> + * snprintf was valid. Otherwise issue an error
> + * and return. */
> + if (len > 0) {
> + current_buf += len;
> + total_len += len;
> + } else {
> + dev_err(dev, "%s: Failed to build interrupt button"
> + " buffer, code = %d.\n", __func__, len);
> + return snprintf(buf, PAGE_SIZE, "unknown\n");
> + }
> + }
> + len = snprintf(current_buf, PAGE_SIZE - total_len, "\n");
> + if (len > 0)
> + total_len += len;
> + else
> + dev_warn(dev, "%s: Failed to append carriage return.\n",
> + __func__);
> + return total_len;
> +
> +}
> +
> +static ssize_t rmi_f19_interrupt_enable_button_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int i;
> + int button_count = 0;
> + int retval = count;
> + int button_reg = 0;
> + int ctrl_bass_addr;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + for (i = 0; i < data->button_count && *buf != 0;
> + i++) {
> + int button_shift;
> + int button;
> +
> + button_reg = i / 7;
> + button_shift = i % 8;
> + /* get next button mapping value and store and bump up to
> + * point to next item in buf */
> + sscanf(buf, "%u", &button);
> +
> + if (button != 0 && button != 1) {
> + dev_err(dev,
> + "%s: Error - interrupt enable button for"
> + " button %d is not a valid value 0x%x.\n",
> + __func__, i, button);
> + return -EINVAL;
> + }
> +
> + if (button_shift == 0)
> + data->button_control->button_int_enable[button_reg].
> + int_enabled_button = 0;
> + data->button_control->button_int_enable[button_reg].
> + int_enabled_button |= (button << button_shift);
> + button_count++;
> + /* bump up buf to point to next item to read */
> + while (*buf != 0) {
> + buf++;
> + if (*(buf - 1) == ' ')
> + break;
> + }
> + }
> +
> + /* Make sure the button count matches */
> + if (button_count != data->button_count) {
> + dev_err(dev,
> + "%s: Error - interrupt enable button count of %d"
> + " doesn't match device button count of %d.\n",
> + __func__, button_count, data->button_count);
> + return -EINVAL;
> + }
> +
> + /* write back to the control register */
> + ctrl_bass_addr = fc->fd.control_base_addr +
> + sizeof(struct f19_0d_control_0);
> + retval = rmi_write_block(fc->rmi_dev, ctrl_bass_addr,
> + (u8 *)data->button_control->button_int_enable,
> + sizeof(struct f19_0d_control_1)*(button_reg + 1));
> + if (retval < 0) {
> + dev_err(dev, "%s : Could not write interrupt_enable_store"
> + " to 0x%x\n", __func__, ctrl_bass_addr);
> + return retval;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t rmi_f19_single_button_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int i, len, total_len = 0;
> + char *current_buf = buf;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + /* loop through each button map value and copy its
> + * string representation into buf */
> + for (i = 0; i < data->button_count; i++) {
> + int button_reg;
> + int button_shift;
> + int single_button;
> +
> + button_reg = i / 7;
> + button_shift = i % 8;
> + single_button = ((data->button_control->
> + single_button_participation[button_reg].single_button
> + >> button_shift) & 0x01);
> +
> + /* get next button mapping value and write it to buf */
> + len = snprintf(current_buf, PAGE_SIZE - total_len,
> + "%u ", single_button);
> + /* bump up ptr to next location in buf if the
> + * snprintf was valid. Otherwise issue an error
> + * and return. */
> + if (len > 0) {
> + current_buf += len;
> + total_len += len;
> + } else {
> + dev_err(dev, "%s: Failed to build signle button buffer"
> + ", code = %d.\n", __func__, len);
> + return snprintf(buf, PAGE_SIZE, "unknown\n");
> + }
> + }
> + len = snprintf(current_buf, PAGE_SIZE - total_len, "\n");
> + if (len > 0)
> + total_len += len;
> + else
> + dev_warn(dev, "%s: Failed to append carriage return.\n",
> + __func__);
> +
> + return total_len;
> +
> +}
> +
> +static ssize_t rmi_f19_single_button_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int i;
> + int button_count = 0;
> + int retval = count;
> + int ctrl_bass_addr;
> + int button_reg = 0;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + for (i = 0; i < data->button_count && *buf != 0;
> + i++) {
> + int button_shift;
> + int button;
> +
> + button_reg = i / 7;
> + button_shift = i % 8;
> + /* get next button mapping value and store and bump up to
> + * point to next item in buf */
> + sscanf(buf, "%u", &button);
> +
> + if (button != 0 && button != 1) {
> + dev_err(dev,
> + "%s: Error - single button for button %d"
> + " is not a valid value 0x%x.\n",
> + __func__, i, button);
> + return -EINVAL;
> + }
> + if (button_shift == 0)
> + data->button_control->
> + single_button_participation[button_reg].
> + single_button = 0;
> + data->button_control->single_button_participation[button_reg].
> + single_button |= (button << button_shift);
> + button_count++;
> + /* bump up buf to point to next item to read */
> + while (*buf != 0) {
> + buf++;
> + if (*(buf - 1) == ' ')
> + break;
> + }
> + }
> +
> + /* Make sure the button count matches */
> + if (button_count != data->button_count) {
> + dev_err(dev,
> + "%s: Error - single button count of %d doesn't match"
> + " device button count of %d.\n", __func__, button_count,
> + data->button_count);
> + return -EINVAL;
> + }
> + /* write back to the control register */
> + ctrl_bass_addr = fc->fd.control_base_addr +
> + sizeof(struct f19_0d_control_0) +
> + sizeof(struct f19_0d_control_2)*(button_reg + 1);
> + retval = rmi_write_block(fc->rmi_dev, ctrl_bass_addr,
> + (u8 *)data->button_control->single_button_participation,
> + sizeof(struct f19_0d_control_2)*(button_reg + 1));
> + if (retval < 0) {
> + dev_err(dev, "%s : Could not write interrupt_enable_store to"
> + " 0x%x\n", __func__, ctrl_bass_addr);
> + return -EINVAL;
> + }
> + return count;
> +}
> +
> +static ssize_t rmi_f19_sensor_map_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int i, len, total_len = 0;
> + char *current_buf = buf;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + for (i = 0; i < data->button_count; i++) {
> + len = snprintf(current_buf, PAGE_SIZE - total_len,
> + "%u ", data->button_control->sensor_map[i].
> + sensor_map_button);
> + /* bump up ptr to next location in buf if the
> + * snprintf was valid. Otherwise issue an error
> + * and return. */
> + if (len > 0) {
> + current_buf += len;
> + total_len += len;
> + } else {
> + dev_err(dev, "%s: Failed to build sensor map buffer, "
> + "code = %d.\n", __func__, len);
> + return snprintf(buf, PAGE_SIZE, "unknown\n");
> + }
> + }
> + len = snprintf(current_buf, PAGE_SIZE - total_len, "\n");
> + if (len > 0)
> + total_len += len;
> + else
> + dev_warn(dev, "%s: Failed to append carriage return.\n",
> + __func__);
> + return total_len;
> +
> +
> +}
> +
> +static ssize_t rmi_f19_sensor_map_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int sensor_map;
> + int i;
> + int retval = count;
> + int button_count = 0;
> + int ctrl_bass_addr;
> + int button_reg;
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + if (data->button_query.configurable == 0) {
> + dev_err(dev,
> + "%s: Error - sensor map is not configuralbe at"
> + " run-time", __func__);
This is not driver error, return error silently.
> + return -EINVAL;
> + }
If sensor is not cinfigurable maybe attributes mode should be adjusted
to be read-only?
> +
> + for (i = 0; i < data->button_count && *buf != 0; i++) {
> + /* get next button mapping value and store and bump up to
> + * point to next item in buf */
> + sscanf(buf, "%u", &sensor_map);
> +
> + /* Make sure the key is a valid key */
> + if (sensor_map < 0 || sensor_map > 127) {
> + dev_err(dev,
> + "%s: Error - sensor map for button %d is"
> + " not a valid value 0x%x.\n",
> + __func__, i, sensor_map);
> + return -EINVAL;
> + }
> +
> + data->button_control->sensor_map[i].sensor_map_button =
> + sensor_map;
> + button_count++;
> +
> + /* bump up buf to point to next item to read */
> + while (*buf != 0) {
> + buf++;
> + if (*(buf - 1) == ' ')
> + break;
> + }
> + }
> +
> + if (button_count != data->button_count) {
> + dev_err(dev,
> + "%s: Error - button map count of %d doesn't match device "
> + "button count of %d.\n", __func__, button_count,
> + data->button_count);
> + return -EINVAL;
> + }
> +
> + /* write back to the control register */
> + button_reg = (button_count / 7) + 1;
> + ctrl_bass_addr = fc->fd.control_base_addr +
> + sizeof(struct f19_0d_control_0) +
> + sizeof(struct f19_0d_control_1)*button_reg +
> + sizeof(struct f19_0d_control_2)*button_reg;
> + retval = rmi_write_block(fc->rmi_dev, ctrl_bass_addr,
> + (u8 *)data->button_control->sensor_map,
> + sizeof(struct f19_0d_control_3_4)*button_count);
> + if (retval < 0) {
> + dev_err(dev, "%s : Could not sensor_map_store to 0x%x\n",
> + __func__, ctrl_bass_addr);
> + return -EINVAL;
> + }
> + return count;
> +}
> +
> +static ssize_t rmi_f19_sensitivity_adjust_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n", data->button_control->
> + all_button_sensitivity_adj->sensitivity_adj);
> +
> +}
> +
> +static ssize_t rmi_f19_sensitivity_adjust_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + unsigned int new_value;
> + int len;
> + int ctrl_bass_addr;
> + int button_reg;
> +
> + fc = to_rmi_function_container(dev);
> +
> + data = fc->data;
> +
> + if (data->button_query.configurable == 0) {
> + dev_err(dev,
> + "%s: Error - sensitivity_adjust is not"
> + " configuralbe at run-time", __func__);
> + return -EINVAL;
> + }
> +
> + len = sscanf(buf, "%u", &new_value);
> + if (new_value < 0 || new_value > 31)
> + return -EINVAL;
> +
> + data->button_control->all_button_sensitivity_adj->sensitivity_adj =
> + new_value;
> + /* write back to the control register */
> + button_reg = (data->button_count / 7) + 1;
> + ctrl_bass_addr = fc->fd.control_base_addr +
> + sizeof(struct f19_0d_control_0) +
> + sizeof(struct f19_0d_control_1)*button_reg +
> + sizeof(struct f19_0d_control_2)*button_reg +
> + sizeof(struct f19_0d_control_3_4)*data->button_count;
> + len = rmi_write_block(fc->rmi_dev, ctrl_bass_addr,
> + (u8 *)data->button_control->all_button_sensitivity_adj,
> + sizeof(struct f19_0d_control_5));
> + if (len < 0) {
> + dev_err(dev, "%s : Could not sensitivity_adjust_store to"
> + " 0x%x\n", __func__, ctrl_bass_addr);
> + return len;
> + }
> +
> + return len;
> +}
> +
> +static ssize_t rmi_f19_hysteresis_threshold_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n", data->button_control->
> + all_button_hysteresis_threshold->hysteresis_threshold);
> +
> +}
> +static ssize_t rmi_f19_hysteresis_threshold_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + unsigned int new_value;
> + int len;
> + int ctrl_bass_addr;
> + int button_reg;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + len = sscanf(buf, "%u", &new_value);
> + if (new_value < 0 || new_value > 15) {
> + dev_err(dev, "%s: Error - hysteresis_threshold_store has an "
> + "invalid value %d.\n",
> + __func__, new_value);
> + return -EINVAL;
> + }
> + data->button_control->all_button_hysteresis_threshold->
> + hysteresis_threshold = new_value;
> + /* write back to the control register */
> + button_reg = (data->button_count / 7) + 1;
> + ctrl_bass_addr = fc->fd.control_base_addr +
> + sizeof(struct f19_0d_control_0) +
> + sizeof(struct f19_0d_control_1)*button_reg +
> + sizeof(struct f19_0d_control_2)*button_reg +
> + sizeof(struct f19_0d_control_3_4)*data->button_count+
> + sizeof(struct f19_0d_control_5);
> + len = rmi_write_block(fc->rmi_dev, ctrl_bass_addr,
> + (u8 *)data->button_control->all_button_sensitivity_adj,
> + sizeof(struct f19_0d_control_6));
> + if (len < 0) {
> + dev_err(dev, "%s : Could not write all_button hysteresis "
> + "threshold to 0x%x\n", __func__, ctrl_bass_addr);
> + return -EINVAL;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t rmi_f19_has_hysteresis_threshold_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_query.has_hysteresis_threshold);
> +}
> +
> +static ssize_t rmi_f19_has_sensitivity_adjust_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_query.has_sensitivity_adjust);
> +}
> +
> +static ssize_t rmi_f19_configurable_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_query.configurable);
> +}
> +
> +static ssize_t rmi_f19_rezero_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_rezero);
> +
> +}
> +
> +static ssize_t rmi_f19_rezero_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + unsigned int new_value;
> + int len;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + len = sscanf(buf, "%u", &new_value);
> + if (new_value != 0 && new_value != 1) {
> + dev_err(dev,
> + "%s: Error - rezero is not a "
> + "valid value 0x%x.\n",
> + __func__, new_value);
> + return -EINVAL;
> + }
> + data->button_rezero = new_value & 1;
> + len = rmi_write(fc->rmi_dev, fc->fd.command_base_addr,
> + data->button_rezero);
> +
> + if (len < 0) {
> + dev_err(dev, "%s : Could not write rezero to 0x%x\n",
> + __func__, fc->fd.command_base_addr);
> + return -EINVAL;
> + }
> + return count;
> +}
> +
> +static ssize_t rmi_f19_button_count_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + return snprintf(buf, PAGE_SIZE, "%u\n",
> + data->button_count);
> +}
> +
> +static ssize_t rmi_f19_button_map_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> +
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + int i, len, total_len = 0;
> + char *current_buf = buf;
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> + /* loop through each button map value and copy its
> + * string representation into buf */
> + for (i = 0; i < data->button_count; i++) {
> + /* get next button mapping value and write it to buf */
> + len = snprintf(current_buf, PAGE_SIZE - total_len,
> + "%u ", data->button_map[i]);
> + /* bump up ptr to next location in buf if the
> + * snprintf was valid. Otherwise issue an error
> + * and return. */
> + if (len > 0) {
> + current_buf += len;
> + total_len += len;
> + } else {
> + dev_err(dev, "%s: Failed to build button map buffer, "
> + "code = %d.\n", __func__, len);
> + return snprintf(buf, PAGE_SIZE, "unknown\n");
> + }
> + }
> + len = snprintf(current_buf, PAGE_SIZE - total_len, "\n");
> + if (len > 0)
> + total_len += len;
> + else
> + dev_warn(dev, "%s: Failed to append carriage return.\n",
> + __func__);
> + return total_len;
> +}
> +
> +static ssize_t rmi_f19_button_map_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct rmi_function_container *fc;
> + struct f19_data *data;
> + unsigned int button;
> + int i;
> + int retval = count;
> + int button_count = 0;
> + unsigned char temp_button_map[KEY_MAX];
> +
> + fc = to_rmi_function_container(dev);
> + data = fc->data;
> +
> + /* Do validation on the button map data passed in. Store button
> + * mappings into a temp buffer and then verify button count and
> + * data prior to clearing out old button mappings and storing the
> + * new ones. */
> + for (i = 0; i < data->button_count && *buf != 0;
> + i++) {
> + /* get next button mapping value and store and bump up to
> + * point to next item in buf */
> + sscanf(buf, "%u", &button);
> +
> + /* Make sure the key is a valid key */
> + if (button > KEY_MAX) {
> + dev_err(dev,
> + "%s: Error - button map for button %d is not a"
> + " valid value 0x%x.\n", __func__, i, button);
> + retval = -EINVAL;
> + goto err_ret;
> + }
> +
> + temp_button_map[i] = button;
> + button_count++;
> +
> + /* bump up buf to point to next item to read */
> + while (*buf != 0) {
> + buf++;
> + if (*(buf - 1) == ' ')
> + break;
> + }
> + }
> +
> + /* Make sure the button count matches */
> + if (button_count != data->button_count) {
> + dev_err(dev,
> + "%s: Error - button map count of %d doesn't match device "
> + "button count of %d.\n", __func__, button_count,
> + data->button_count);
> + retval = -EINVAL;
> + goto err_ret;
> + }
> +
> + /* Clear the key bits for the old button map. */
> + for (i = 0; i < button_count; i++)
> + clear_bit(data->button_map[i], data->input->keybit);
> +
> + /* Switch to the new map. */
> + memcpy(data->button_map, temp_button_map,
> + data->button_count);
> +
> + /* Loop through the key map and set the key bit for the new mapping. */
> + for (i = 0; i < button_count; i++)
> + set_bit(data->button_map[i], data->input->keybit);
> +
> +err_ret:
> + return retval;
> +}
Button map (keymap) should be manipulated with EVIOCGKEYCODE and
EVIOCSKEYCODE ioctls, no need to invent driver-specific way of doing
this.
> +
> +module_init(rmi_f19_module_init);
> +module_exit(rmi_f19_module_exit);
> +
> +MODULE_AUTHOR("Vivian Ly <vly@...aptics.com>");
> +MODULE_DESCRIPTION("RMI F19 module");
> +MODULE_LICENSE("GPL");
> +
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists