Data that is allocated with devm_kzalloc() should not be freed with
kfree(). In fact, we should rely on the fact that memory is managed and let
devres core free it for us.
Reported-by: Courtney Cavin <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 381ad60..92b90d1 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -272,7 +272,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
if (error < 0) {
dev_err(&fn->dev,
"Failed to read F01 control interrupt enable register.\n");
- goto error_exit;
+ return error;
}
ctrl_base_addr += data->num_of_irq_regs;
@@ -307,14 +307,14 @@ static int rmi_f01_initialize(struct rmi_function *fn)
data->device_control.doze_interval);
if (error < 0) {
dev_err(&fn->dev, "Failed to configure F01 doze interval register.\n");
- goto error_exit;
+ return error;
}
} else {
error = rmi_read(rmi_dev, data->doze_interval_addr,
&data->device_control.doze_interval);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
- goto error_exit;
+ return error;
}
}
@@ -328,14 +328,14 @@ static int rmi_f01_initialize(struct rmi_function *fn)
data->device_control.wakeup_threshold);
if (error < 0) {
dev_err(&fn->dev, "Failed to configure F01 wakeup threshold register.\n");
- goto error_exit;
+ return error;
}
} else {
error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
&data->device_control.wakeup_threshold);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
- goto error_exit;
+ return error;
}
}
}
@@ -351,14 +351,14 @@ static int rmi_f01_initialize(struct rmi_function *fn)
data->device_control.doze_holdoff);
if (error < 0) {
dev_err(&fn->dev, "Failed to configure F01 doze holdoff register.\n");
- goto error_exit;
+ return error;
}
} else {
error = rmi_read(rmi_dev, data->doze_holdoff_addr,
&data->device_control.doze_holdoff);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
- goto error_exit;
+ return error;
}
}
}
@@ -367,22 +367,17 @@ static int rmi_f01_initialize(struct rmi_function *fn)
&data->device_status, sizeof(data->device_status));
if (error < 0) {
dev_err(&fn->dev, "Failed to read device status.\n");
- goto error_exit;
+ return error;
}
if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
dev_err(&fn->dev,
"Device was reset during configuration process, status: %#02x!\n",
RMI_F01_STATUS_CODE(data->device_status));
- error = -EINVAL;
- goto error_exit;
+ return -EINVAL;
}
return 0;
-
- error_exit:
- kfree(data);
- return error;
}
static int rmi_f01_config(struct rmi_function *fn)
--
1.8.5.3
When accessing platform data of RMI device let's make sure we do not
accidentally change data that may be shared by returning const pointer.
Also switch to an inline function instead of macro to ensure type safety.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_bus.h | 7 ++++++-
drivers/input/rmi4/rmi_driver.c | 8 ++++----
drivers/input/rmi4/rmi_f01.c | 2 +-
drivers/input/rmi4/rmi_f11.c | 2 +-
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 8d47f52..02a2af8 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -224,7 +224,12 @@ struct rmi_device {
};
#define to_rmi_device(d) container_of(d, struct rmi_device, dev)
-#define to_rmi_platform_data(d) ((d)->xport->dev->platform_data)
+
+static inline const struct rmi_device_platform_data *
+rmi_get_platform_data(struct rmi_device *d)
+{
+ return dev_get_platdata(d->xport->dev);
+}
bool rmi_is_physical_device(struct device *dev);
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 736cfbd..473efbc 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -598,7 +598,7 @@ static int rmi_initial_reset(struct rmi_device *rmi_dev,
u16 cmd_addr = pdt->page_start + pdt->command_base_addr;
u8 cmd_buf = RMI_DEVICE_RESET_CMD;
const struct rmi_device_platform_data *pdata =
- to_rmi_platform_data(rmi_dev);
+ rmi_get_platform_data(rmi_dev);
error = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
if (error) {
@@ -621,7 +621,7 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
{
struct device *dev = &rmi_dev->dev;
struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
- struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
int *current_irq_count = ctx;
struct rmi_function *fn;
int i;
@@ -745,7 +745,7 @@ static int rmi_driver_remove(struct device *dev)
struct rmi_device *rmi_dev = to_rmi_device(dev);
struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
const struct rmi_device_platform_data *pdata =
- to_rmi_platform_data(rmi_dev);
+ rmi_get_platform_data(rmi_dev);
disable_sensor(rmi_dev);
rmi_free_function_list(rmi_dev);
@@ -781,7 +781,7 @@ static int rmi_driver_probe(struct device *dev)
rmi_driver = to_rmi_driver(dev->driver);
rmi_dev->driver = rmi_driver;
- pdata = to_rmi_platform_data(rmi_dev);
+ pdata = rmi_get_platform_data(rmi_dev);
data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
if (!data) {
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 0e21004..36fcf8d 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -184,7 +184,7 @@ static int rmi_f01_probe(struct rmi_function *fn)
{
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
- const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
struct f01_data *f01;
size_t f01_size;
int error;
diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 2aa7d17..5072437 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1176,7 +1176,7 @@ static int rmi_f11_initialize(struct rmi_function *fn)
u16 control_base_addr;
u16 max_x_pos, max_y_pos, temp;
int rc;
- struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
struct f11_2d_sensor *sensor;
u8 buf;
--
1.8.5.3
Let's allocate interrupt mask together with the main structure and combine
rmi_f01_alloc_memory, rmi_f01_initialize and rmi_f01_probe into single
function.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 86 ++++++++++++++++----------------------------
1 file changed, 30 insertions(+), 56 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index a2f05bc..0e21004 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -130,37 +130,15 @@ struct f01_data {
u16 doze_interval_addr;
u16 wakeup_threshold_addr;
u16 doze_holdoff_addr;
- int irq_count;
- int num_of_irq_regs;
#ifdef CONFIG_PM_SLEEP
bool suspended;
bool old_nosleep;
#endif
-};
-
-static int rmi_f01_alloc_memory(struct rmi_function *fn,
- int num_of_irq_regs)
-{
- struct f01_data *f01;
- f01 = devm_kzalloc(&fn->dev, sizeof(struct f01_data), GFP_KERNEL);
- if (!f01) {
- dev_err(&fn->dev, "Failed to allocate fn_01_data.\n");
- return -ENOMEM;
- }
-
- f01->device_control.interrupt_enable = devm_kzalloc(&fn->dev,
- sizeof(u8) * (num_of_irq_regs),
- GFP_KERNEL);
- if (!f01->device_control.interrupt_enable) {
- dev_err(&fn->dev, "Failed to allocate interrupt enable.\n");
- return -ENOMEM;
- }
- fn->data = f01;
-
- return 0;
-}
+ unsigned int num_of_irq_regs;
+ u8 interrupt_enable[];
+};
static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
u16 query_base_addr,
@@ -202,16 +180,28 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
return 0;
}
-static int rmi_f01_initialize(struct rmi_function *fn)
+static int rmi_f01_probe(struct rmi_function *fn)
{
- u8 temp;
- int error;
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
- struct f01_data *f01 = fn->data;
const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ struct f01_data *f01;
+ size_t f01_size;
+ int error;
u16 ctrl_base_addr = fn->fd.control_base_addr;
u8 device_status;
+ u8 temp;
+
+ f01_size = sizeof(struct f01_data) +
+ sizeof(u8) * driver_data->num_of_irq_regs;
+ f01 = devm_kzalloc(&fn->dev, f01_size, GFP_KERNEL);
+ if (!f01) {
+ dev_err(&fn->dev, "Failed to allocate fn01_data.\n");
+ return -ENOMEM;
+ }
+
+ f01->num_of_irq_regs = driver_data->num_of_irq_regs;
+ f01->device_control.interrupt_enable = f01->interrupt_enable;
/*
* Set the configured bit and (optionally) other important stuff
@@ -257,12 +247,11 @@ static int rmi_f01_initialize(struct rmi_function *fn)
return error;
}
- f01->irq_count = driver_data->irq_count;
- f01->num_of_irq_regs = driver_data->num_of_irq_regs;
- ctrl_base_addr += sizeof(u8);
-
+ /* Advance to interrupt control registers */
+ ctrl_base_addr++;
f01->interrupt_enable_addr = ctrl_base_addr;
- error = rmi_read_block(rmi_dev, ctrl_base_addr,
+
+ error = rmi_read_block(rmi_dev, f01->interrupt_enable_addr,
f01->device_control.interrupt_enable,
sizeof(u8) * (f01->num_of_irq_regs));
if (error) {
@@ -272,9 +261,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
return error;
}
- ctrl_base_addr += f01->num_of_irq_regs;
-
- /* dummy read in order to clear irqs */
+ /* Dummy read in order to clear irqs */
error = rmi_read(rmi_dev, fn->fd.data_base_addr + 1, &temp);
if (error < 0) {
dev_err(&fn->dev, "Failed to read Interrupt Status.\n");
@@ -287,11 +274,13 @@ static int rmi_f01_initialize(struct rmi_function *fn)
dev_err(&fn->dev, "Failed to read F01 properties.\n");
return error;
}
+
dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
- f01->properties.manufacturer_id == 1 ?
- "Synaptics" : "unknown",
+ f01->properties.manufacturer_id == 1 ? "Synaptics" : "unknown",
f01->properties.product_id);
+ ctrl_base_addr += f01->num_of_irq_regs;
+
/* read control register */
if (f01->properties.has_adjustable_doze) {
f01->doze_interval_addr = ctrl_base_addr;
@@ -365,6 +354,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
return -EINVAL;
}
+ fn->data = f01;
+
return 0;
}
@@ -424,23 +415,6 @@ static int rmi_f01_config(struct rmi_function *fn)
return 0;
}
-static int rmi_f01_probe(struct rmi_function *fn)
-{
- struct rmi_driver_data *driver_data =
- dev_get_drvdata(&fn->rmi_dev->dev);
- int error;
-
- error = rmi_f01_alloc_memory(fn, driver_data->num_of_irq_regs);
- if (error)
- return error;
-
- error = rmi_f01_initialize(fn);
- if (error)
- return error;
-
- return 0;
-}
-
#ifdef CONFIG_PM_SLEEP
static int rmi_f01_suspend(struct device *dev)
{
--
1.8.5.3
Device core provides way of accessing driver-private data, we should
use it.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_bus.h | 1 -
drivers/input/rmi4/rmi_f01.c | 14 +++++------
drivers/input/rmi4/rmi_f11.c | 57 ++++++++++++++++++++------------------------
3 files changed, 32 insertions(+), 40 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 02a2af8..5ad94c6 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -48,7 +48,6 @@ struct rmi_function {
int num_of_irqs;
int irq_pos;
unsigned long *irq_mask;
- void *data;
struct list_head node;
#ifdef CONFIG_RMI4_DEBUG
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 36fcf8d..e646f60 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -354,14 +354,14 @@ static int rmi_f01_probe(struct rmi_function *fn)
return -EINVAL;
}
- fn->data = f01;
+ dev_set_drvdata(&fn->dev, f01);
return 0;
}
static int rmi_f01_config(struct rmi_function *fn)
{
- struct f01_data *f01 = fn->data;
+ struct f01_data *f01 = dev_get_drvdata(&fn->dev);
int error;
error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
@@ -419,8 +419,7 @@ static int rmi_f01_config(struct rmi_function *fn)
static int rmi_f01_suspend(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
- struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f01_data *f01 = fn->data;
+ struct f01_data *f01 = dev_get_drvdata(&fn->dev);
int error;
f01->old_nosleep =
@@ -430,7 +429,7 @@ static int rmi_f01_suspend(struct device *dev)
f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
- error = rmi_write(rmi_dev, fn->fd.control_base_addr,
+ error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
f01->device_control.ctrl0);
if (error) {
dev_err(&fn->dev, "Failed to write sleep mode: %d.\n", error);
@@ -447,8 +446,7 @@ static int rmi_f01_suspend(struct device *dev)
static int rmi_f01_resume(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
- struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f01_data *f01 = fn->data;
+ struct f01_data *f01 = dev_get_drvdata(&fn->dev);
int error;
if (f01->old_nosleep)
@@ -457,7 +455,7 @@ static int rmi_f01_resume(struct device *dev)
f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
- error = rmi_write(rmi_dev, fn->fd.control_base_addr,
+ error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
f01->device_control.ctrl0);
if (error) {
dev_err(&fn->dev,
diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
index 5072437..a26b944 100644
--- a/drivers/input/rmi4/rmi_f11.c
+++ b/drivers/input/rmi4/rmi_f11.c
@@ -1087,9 +1087,8 @@ static int rmi_f11_get_query_parameters(struct rmi_device *rmi_dev,
/* This operation is done in a number of places, so we have a handy routine
* for it.
*/
-static void f11_set_abs_params(struct rmi_function *fn)
+static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
{
- struct f11_data *f11 = fn->data;
struct f11_2d_sensor *sensor = &f11->sensor;
struct input_dev *input = sensor->input;
/* These two lines are not doing what we want them to. So we use
@@ -1190,7 +1189,6 @@ static int rmi_f11_initialize(struct rmi_function *fn)
if (!f11)
return -ENOMEM;
- fn->data = f11;
f11->rezero_wait_ms = pdata->f11_rezero_wait;
query_base_addr = fn->fd.query_base_addr;
@@ -1280,13 +1278,16 @@ static int rmi_f11_initialize(struct rmi_function *fn)
}
mutex_init(&f11->dev_controls_mutex);
+
+ dev_set_drvdata(&fn->dev, f11);
+
return 0;
}
static int rmi_f11_register_devices(struct rmi_function *fn)
{
struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f11_data *f11 = fn->data;
+ struct f11_data *f11 = dev_get_drvdata(&fn->dev);
struct input_dev *input_dev;
struct input_dev *input_dev_mouse;
struct rmi_driver *driver = rmi_dev->driver;
@@ -1304,8 +1305,8 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
rc = driver->set_input_params(rmi_dev, input_dev);
if (rc < 0) {
dev_err(&fn->dev,
- "%s: Error in setting input device.\n",
- __func__);
+ "%s: Error in setting input device.\n",
+ __func__);
goto error_unregister;
}
}
@@ -1319,7 +1320,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
set_bit(EV_ABS, input_dev->evbit);
input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
- f11_set_abs_params(fn);
+ f11_set_abs_params(fn, f11);
if (sensor->sens_query.has_rel) {
set_bit(EV_REL, input_dev->evbit);
@@ -1327,7 +1328,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
set_bit(REL_Y, input_dev->relbit);
}
rc = input_register_device(input_dev);
- if (rc < 0) {
+ if (rc) {
input_free_device(input_dev);
sensor->input = NULL;
goto error_unregister;
@@ -1347,8 +1348,8 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
input_dev_mouse);
if (rc < 0) {
dev_err(&fn->dev,
- "%s: Error in setting input device.\n",
- __func__);
+ "%s: Error in setting input device.\n",
+ __func__);
goto error_unregister;
}
}
@@ -1366,7 +1367,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
set_bit(BTN_RIGHT, input_dev_mouse->keybit);
rc = input_register_device(input_dev_mouse);
- if (rc < 0) {
+ if (rc) {
input_free_device(input_dev_mouse);
sensor->mouse_input = NULL;
goto error_unregister;
@@ -1390,19 +1391,9 @@ error_unregister:
return rc;
}
-static void rmi_f11_free_devices(struct rmi_function *fn)
-{
- struct f11_data *f11 = fn->data;
-
- if (f11->sensor.input)
- input_unregister_device(f11->sensor.input);
- if (f11->sensor.mouse_input)
- input_unregister_device(f11->sensor.mouse_input);
-}
-
static int rmi_f11_config(struct rmi_function *fn)
{
- struct f11_data *f11 = fn->data;
+ struct f11_data *f11 = dev_get_drvdata(&fn->dev);
int rc;
rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
@@ -1416,7 +1407,7 @@ static int rmi_f11_config(struct rmi_function *fn)
static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
{
struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f11_data *f11 = fn->data;
+ struct f11_data *f11 = dev_get_drvdata(&fn->dev);
u16 data_base_addr = fn->fd.data_base_addr;
u16 data_base_addr_offset = 0;
int error;
@@ -1425,7 +1416,7 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
data_base_addr + data_base_addr_offset,
f11->sensor.data_pkt,
f11->sensor.pkt_size);
- if (error < 0)
+ if (error)
return error;
rmi_f11_finger_handler(f11, &f11->sensor);
@@ -1438,18 +1429,17 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
static int rmi_f11_resume(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
- struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f11_data *data = fn->data;
+ struct f11_data *f11 = dev_get_drvdata(&fn->dev);
int error;
dev_dbg(&fn->dev, "Resuming...\n");
- if (!data->rezero_wait_ms)
+ if (!f11->rezero_wait_ms)
return 0;
- mdelay(data->rezero_wait_ms);
+ mdelay(f11->rezero_wait_ms);
- error = rmi_write(rmi_dev, fn->fd.command_base_addr, RMI_F11_REZERO);
- if (error < 0) {
+ error = rmi_write(fn->rmi_dev, fn->fd.command_base_addr, RMI_F11_REZERO);
+ if (error) {
dev_err(&fn->dev,
"%s: failed to issue rezero command, error = %d.",
__func__, error);
@@ -1479,7 +1469,12 @@ static int rmi_f11_probe(struct rmi_function *fn)
static void rmi_f11_remove(struct rmi_function *fn)
{
- rmi_f11_free_devices(fn);
+ struct f11_data *f11 = dev_get_drvdata(&fn->dev);
+
+ if (f11->sensor.input)
+ input_unregister_device(f11->sensor.input);
+ if (f11->sensor.mouse_input)
+ input_unregister_device(f11->sensor.mouse_input);
}
static struct rmi_function_handler rmi_f11_handler = {
--
1.8.5.3
It is not used by anyone.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 30e0b50..6f90a6c 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -125,7 +125,6 @@ struct f01_data {
struct f01_basic_properties properties;
struct f01_device_control device_control;
- struct mutex control_mutex;
u8 device_status;
@@ -214,8 +213,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
struct f01_data *data = fn->data;
struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
- mutex_init(&data->control_mutex);
-
/*
* Set the configured bit and (optionally) other important stuff
* in the device control register.
--
1.8.5.3
Use rmi_read()/rmi_write() for reading/writing single-byte data. Also print
error code when IO fails.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 170 ++++++++++++++++++++++---------------------
1 file changed, 88 insertions(+), 82 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 1219e0c..a2f05bc 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -171,8 +171,9 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
error = rmi_read_block(rmi_dev, query_base_addr,
basic_query, sizeof(basic_query));
- if (error < 0) {
- dev_err(&rmi_dev->dev, "Failed to read device query registers.\n");
+ if (error) {
+ dev_err(&rmi_dev->dev,
+ "Failed to read device query registers: %d\n", error);
return error;
}
@@ -205,25 +206,25 @@ static int rmi_f01_initialize(struct rmi_function *fn)
{
u8 temp;
int error;
- u16 ctrl_base_addr;
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
struct f01_data *f01 = fn->data;
- struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ u16 ctrl_base_addr = fn->fd.control_base_addr;
u8 device_status;
/*
* Set the configured bit and (optionally) other important stuff
* in the device control register.
*/
- ctrl_base_addr = fn->fd.control_base_addr;
- error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
- &f01->device_control.ctrl0,
- sizeof(f01->device_control.ctrl0));
- if (error < 0) {
- dev_err(&fn->dev, "Failed to read F01 control.\n");
+
+ error = rmi_read(rmi_dev, fn->fd.control_base_addr,
+ &f01->device_control.ctrl0);
+ if (error) {
+ dev_err(&fn->dev, "Failed to read F01 control: %d\n", error);
return error;
}
+
switch (pdata->power_management.nosleep) {
case RMI_F01_NOSLEEP_DEFAULT:
break;
@@ -249,11 +250,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
f01->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
- error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
- &f01->device_control.ctrl0,
- sizeof(f01->device_control.ctrl0));
- if (error < 0) {
- dev_err(&fn->dev, "Failed to write F01 control.\n");
+ error = rmi_write(rmi_dev, fn->fd.control_base_addr,
+ f01->device_control.ctrl0);
+ if (error) {
+ dev_err(&fn->dev, "Failed to write F01 control: %d\n", error);
return error;
}
@@ -265,9 +265,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
error = rmi_read_block(rmi_dev, ctrl_base_addr,
f01->device_control.interrupt_enable,
sizeof(u8) * (f01->num_of_irq_regs));
- if (error < 0) {
+ if (error) {
dev_err(&fn->dev,
- "Failed to read F01 control interrupt enable register.\n");
+ "Failed to read F01 control interrupt enable register: %d\n",
+ error);
return error;
}
@@ -302,8 +303,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
} else {
error = rmi_read(rmi_dev, f01->doze_interval_addr,
&f01->device_control.doze_interval);
- if (error < 0) {
- dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to read F01 doze interval register: %d\n",
+ error);
return error;
}
}
@@ -318,7 +321,9 @@ static int rmi_f01_initialize(struct rmi_function *fn)
error = rmi_read(rmi_dev, f01->wakeup_threshold_addr,
&f01->device_control.wakeup_threshold);
if (error < 0) {
- dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
+ dev_err(&fn->dev,
+ "Failed to read F01 wakeup threshold register: %d\n",
+ error);
return error;
}
}
@@ -337,17 +342,19 @@ static int rmi_f01_initialize(struct rmi_function *fn)
} else {
error = rmi_read(rmi_dev, f01->doze_holdoff_addr,
&f01->device_control.doze_holdoff);
- if (error < 0) {
- dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to read F01 doze holdoff register: %d\n",
+ error);
return error;
}
}
}
- error = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
- &device_status, sizeof(device_status));
+ error = rmi_read(rmi_dev, fn->fd.data_base_addr, &device_status);
if (error < 0) {
- dev_err(&fn->dev, "Failed to read device status.\n");
+ dev_err(&fn->dev,
+ "Failed to read device status: %d\n", error);
return error;
}
@@ -364,50 +371,53 @@ static int rmi_f01_initialize(struct rmi_function *fn)
static int rmi_f01_config(struct rmi_function *fn)
{
struct f01_data *f01 = fn->data;
- int retval;
-
- retval = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
- &f01->device_control.ctrl0,
- sizeof(f01->device_control.ctrl0));
- if (retval < 0) {
- dev_err(&fn->dev, "Failed to write device_control.reg.\n");
- return retval;
+ int error;
+
+ error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
+ f01->device_control.ctrl0);
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to write device_control register: %d\n", error);
+ return error;
}
- retval = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
- f01->device_control.interrupt_enable,
- sizeof(u8) * f01->num_of_irq_regs);
- if (retval < 0) {
- dev_err(&fn->dev, "Failed to write interrupt enable.\n");
- return retval;
+ error = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
+ f01->device_control.interrupt_enable,
+ sizeof(u8) * f01->num_of_irq_regs);
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to write interrupt enable: %d\n", error);
+ return error;
}
if (f01->properties.has_adjustable_doze) {
- retval = rmi_write_block(fn->rmi_dev, f01->doze_interval_addr,
- &f01->device_control.doze_interval,
- sizeof(u8));
- if (retval < 0) {
- dev_err(&fn->dev, "Failed to write doze interval.\n");
- return retval;
+ error = rmi_write(fn->rmi_dev, f01->doze_interval_addr,
+ f01->device_control.doze_interval);
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to write doze interval: %d\n", error);
+ return error;
}
- retval = rmi_write_block(fn->rmi_dev,
+ error = rmi_write_block(fn->rmi_dev,
f01->wakeup_threshold_addr,
&f01->device_control.wakeup_threshold,
sizeof(u8));
- if (retval < 0) {
- dev_err(&fn->dev, "Failed to write wakeup threshold.\n");
- return retval;
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to write wakeup threshold: %d\n",
+ error);
+ return error;
}
}
if (f01->properties.has_adjustable_doze_holdoff) {
- retval = rmi_write_block(fn->rmi_dev, f01->doze_holdoff_addr,
- &f01->device_control.doze_holdoff,
- sizeof(u8));
- if (retval < 0) {
- dev_err(&fn->dev, "Failed to write doze holdoff.\n");
- return retval;
+ error = rmi_write(fn->rmi_dev, f01->doze_holdoff_addr,
+ f01->device_control.doze_holdoff);
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to write doze holdoff: %d\n", error);
+ return error;
}
}
@@ -439,23 +449,19 @@ static int rmi_f01_suspend(struct device *dev)
struct f01_data *f01 = fn->data;
int error;
- f01->old_nosleep = f01->device_control.ctrl0 &
- RMI_F01_CRTL0_NOSLEEP_BIT;
+ f01->old_nosleep =
+ f01->device_control.ctrl0 & RMI_F01_CRTL0_NOSLEEP_BIT;
f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
- error = rmi_write_block(rmi_dev,
- fn->fd.control_base_addr,
- &f01->device_control.ctrl0,
- sizeof(f01->device_control.ctrl0));
- if (error < 0) {
- dev_err(&fn->dev, "Failed to write sleep mode. Code: %d.\n",
- error);
+ error = rmi_write(rmi_dev, fn->fd.control_base_addr,
+ f01->device_control.ctrl0);
+ if (error) {
+ dev_err(&fn->dev, "Failed to write sleep mode: %d.\n", error);
if (f01->old_nosleep)
- f01->device_control.ctrl0 |=
- RMI_F01_CRTL0_NOSLEEP_BIT;
+ f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
return error;
@@ -477,13 +483,11 @@ static int rmi_f01_resume(struct device *dev)
f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
- error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
- &f01->device_control.ctrl0,
- sizeof(f01->device_control.ctrl0));
- if (error < 0) {
+ error = rmi_write(rmi_dev, fn->fd.control_base_addr,
+ f01->device_control.ctrl0);
+ if (error) {
dev_err(&fn->dev,
- "Failed to restore normal operation. Code: %d.\n",
- error);
+ "Failed to restore normal operation: %d.\n", error);
return error;
}
@@ -497,23 +501,25 @@ static int rmi_f01_attention(struct rmi_function *fn,
unsigned long *irq_bits)
{
struct rmi_device *rmi_dev = fn->rmi_dev;
- int retval;
+ int error;
u8 device_status;
- retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
- &device_status, sizeof(device_status));
- if (retval < 0) {
- dev_err(&fn->dev, "Failed to read device status, code: %d.\n",
- retval);
- return retval;
+ error = rmi_read(rmi_dev, fn->fd.data_base_addr, &device_status);
+ if (error) {
+ dev_err(&fn->dev,
+ "Failed to read device status: %d.\n", error);
+ return error;
}
if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
dev_warn(&fn->dev, "Device reset detected.\n");
- retval = rmi_dev->driver->reset_handler(rmi_dev);
- if (retval < 0)
- return retval;
+ error = rmi_dev->driver->reset_handler(rmi_dev);
+ if (error) {
+ dev_err(&fn->dev, "Device reset failed: %d\n", error);
+ return error;
+ }
}
+
return 0;
}
--
1.8.5.3
We have too many "data"s: f01_data, driver_data, pdata, etc. Let's
untangle it a bit.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 135 ++++++++++++++++++++++---------------------
1 file changed, 68 insertions(+), 67 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 1e49ab4..1219e0c 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -208,7 +208,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
u16 ctrl_base_addr;
struct rmi_device *rmi_dev = fn->rmi_dev;
struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
- struct f01_data *data = fn->data;
+ struct f01_data *f01 = fn->data;
struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
u8 device_status;
@@ -218,8 +218,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
*/
ctrl_base_addr = fn->fd.control_base_addr;
error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
- &data->device_control.ctrl0,
- sizeof(data->device_control.ctrl0));
+ &f01->device_control.ctrl0,
+ sizeof(f01->device_control.ctrl0));
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 control.\n");
return error;
@@ -228,10 +228,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
case RMI_F01_NOSLEEP_DEFAULT:
break;
case RMI_F01_NOSLEEP_OFF:
- data->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
+ f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
break;
case RMI_F01_NOSLEEP_ON:
- data->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
+ f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
break;
}
@@ -240,38 +240,38 @@ static int rmi_f01_initialize(struct rmi_function *fn)
* reboot without power cycle. If so, clear it so the sensor
* is certain to function.
*/
- if ((data->device_control.ctrl0 & RMI_F01_CTRL0_SLEEP_MODE_MASK) !=
+ if ((f01->device_control.ctrl0 & RMI_F01_CTRL0_SLEEP_MODE_MASK) !=
RMI_SLEEP_MODE_NORMAL) {
dev_warn(&fn->dev,
"WARNING: Non-zero sleep mode found. Clearing...\n");
- data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
+ f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
}
- data->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
+ f01->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
- &data->device_control.ctrl0,
- sizeof(data->device_control.ctrl0));
+ &f01->device_control.ctrl0,
+ sizeof(f01->device_control.ctrl0));
if (error < 0) {
dev_err(&fn->dev, "Failed to write F01 control.\n");
return error;
}
- data->irq_count = driver_data->irq_count;
- data->num_of_irq_regs = driver_data->num_of_irq_regs;
+ f01->irq_count = driver_data->irq_count;
+ f01->num_of_irq_regs = driver_data->num_of_irq_regs;
ctrl_base_addr += sizeof(u8);
- data->interrupt_enable_addr = ctrl_base_addr;
+ f01->interrupt_enable_addr = ctrl_base_addr;
error = rmi_read_block(rmi_dev, ctrl_base_addr,
- data->device_control.interrupt_enable,
- sizeof(u8) * (data->num_of_irq_regs));
+ f01->device_control.interrupt_enable,
+ sizeof(u8) * (f01->num_of_irq_regs));
if (error < 0) {
dev_err(&fn->dev,
"Failed to read F01 control interrupt enable register.\n");
return error;
}
- ctrl_base_addr += data->num_of_irq_regs;
+ ctrl_base_addr += f01->num_of_irq_regs;
/* dummy read in order to clear irqs */
error = rmi_read(rmi_dev, fn->fd.data_base_addr + 1, &temp);
@@ -281,42 +281,42 @@ static int rmi_f01_initialize(struct rmi_function *fn)
}
error = rmi_f01_read_properties(rmi_dev, fn->fd.query_base_addr,
- &data->properties);
+ &f01->properties);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 properties.\n");
return error;
}
dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
- data->properties.manufacturer_id == 1 ?
+ f01->properties.manufacturer_id == 1 ?
"Synaptics" : "unknown",
- data->properties.product_id);
+ f01->properties.product_id);
/* read control register */
- if (data->properties.has_adjustable_doze) {
- data->doze_interval_addr = ctrl_base_addr;
+ if (f01->properties.has_adjustable_doze) {
+ f01->doze_interval_addr = ctrl_base_addr;
ctrl_base_addr++;
if (pdata->power_management.doze_interval) {
- data->device_control.doze_interval =
+ f01->device_control.doze_interval =
pdata->power_management.doze_interval;
} else {
- error = rmi_read(rmi_dev, data->doze_interval_addr,
- &data->device_control.doze_interval);
+ error = rmi_read(rmi_dev, f01->doze_interval_addr,
+ &f01->device_control.doze_interval);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
return error;
}
}
- data->wakeup_threshold_addr = ctrl_base_addr;
+ f01->wakeup_threshold_addr = ctrl_base_addr;
ctrl_base_addr++;
if (pdata->power_management.wakeup_threshold) {
- data->device_control.wakeup_threshold =
+ f01->device_control.wakeup_threshold =
pdata->power_management.wakeup_threshold;
} else {
- error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
- &data->device_control.wakeup_threshold);
+ error = rmi_read(rmi_dev, f01->wakeup_threshold_addr,
+ &f01->device_control.wakeup_threshold);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
return error;
@@ -324,19 +324,19 @@ static int rmi_f01_initialize(struct rmi_function *fn)
}
}
- if (data->properties.has_lts)
+ if (f01->properties.has_lts)
ctrl_base_addr++;
- if (data->properties.has_adjustable_doze_holdoff) {
- data->doze_holdoff_addr = ctrl_base_addr;
+ if (f01->properties.has_adjustable_doze_holdoff) {
+ f01->doze_holdoff_addr = ctrl_base_addr;
ctrl_base_addr++;
if (pdata->power_management.doze_holdoff) {
- data->device_control.doze_holdoff =
+ f01->device_control.doze_holdoff =
pdata->power_management.doze_holdoff;
} else {
- error = rmi_read(rmi_dev, data->doze_holdoff_addr,
- &data->device_control.doze_holdoff);
+ error = rmi_read(rmi_dev, f01->doze_holdoff_addr,
+ &f01->device_control.doze_holdoff);
if (error < 0) {
dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
return error;
@@ -363,28 +363,28 @@ static int rmi_f01_initialize(struct rmi_function *fn)
static int rmi_f01_config(struct rmi_function *fn)
{
- struct f01_data *data = fn->data;
+ struct f01_data *f01 = fn->data;
int retval;
retval = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
- &data->device_control.ctrl0,
- sizeof(data->device_control.ctrl0));
+ &f01->device_control.ctrl0,
+ sizeof(f01->device_control.ctrl0));
if (retval < 0) {
dev_err(&fn->dev, "Failed to write device_control.reg.\n");
return retval;
}
- retval = rmi_write_block(fn->rmi_dev, data->interrupt_enable_addr,
- data->device_control.interrupt_enable,
- sizeof(u8) * data->num_of_irq_regs);
-
+ retval = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
+ f01->device_control.interrupt_enable,
+ sizeof(u8) * f01->num_of_irq_regs);
if (retval < 0) {
dev_err(&fn->dev, "Failed to write interrupt enable.\n");
return retval;
}
- if (data->properties.has_adjustable_doze) {
- retval = rmi_write_block(fn->rmi_dev, data->doze_interval_addr,
- &data->device_control.doze_interval,
+
+ if (f01->properties.has_adjustable_doze) {
+ retval = rmi_write_block(fn->rmi_dev, f01->doze_interval_addr,
+ &f01->device_control.doze_interval,
sizeof(u8));
if (retval < 0) {
dev_err(&fn->dev, "Failed to write doze interval.\n");
@@ -392,8 +392,8 @@ static int rmi_f01_config(struct rmi_function *fn)
}
retval = rmi_write_block(fn->rmi_dev,
- data->wakeup_threshold_addr,
- &data->device_control.wakeup_threshold,
+ f01->wakeup_threshold_addr,
+ &f01->device_control.wakeup_threshold,
sizeof(u8));
if (retval < 0) {
dev_err(&fn->dev, "Failed to write wakeup threshold.\n");
@@ -401,15 +401,16 @@ static int rmi_f01_config(struct rmi_function *fn)
}
}
- if (data->properties.has_adjustable_doze_holdoff) {
- retval = rmi_write_block(fn->rmi_dev, data->doze_holdoff_addr,
- &data->device_control.doze_holdoff,
+ if (f01->properties.has_adjustable_doze_holdoff) {
+ retval = rmi_write_block(fn->rmi_dev, f01->doze_holdoff_addr,
+ &f01->device_control.doze_holdoff,
sizeof(u8));
if (retval < 0) {
dev_err(&fn->dev, "Failed to write doze holdoff.\n");
return retval;
}
}
+
return 0;
}
@@ -435,28 +436,28 @@ static int rmi_f01_suspend(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f01_data *data = fn->data;
+ struct f01_data *f01 = fn->data;
int error;
- data->old_nosleep = data->device_control.ctrl0 &
+ f01->old_nosleep = f01->device_control.ctrl0 &
RMI_F01_CRTL0_NOSLEEP_BIT;
- data->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
+ f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
- data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
- data->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
+ f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
+ f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
error = rmi_write_block(rmi_dev,
fn->fd.control_base_addr,
- &data->device_control.ctrl0,
- sizeof(data->device_control.ctrl0));
+ &f01->device_control.ctrl0,
+ sizeof(f01->device_control.ctrl0));
if (error < 0) {
dev_err(&fn->dev, "Failed to write sleep mode. Code: %d.\n",
error);
- if (data->old_nosleep)
- data->device_control.ctrl0 |=
+ if (f01->old_nosleep)
+ f01->device_control.ctrl0 |=
RMI_F01_CRTL0_NOSLEEP_BIT;
- data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
- data->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
+ f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
+ f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
return error;
}
@@ -467,18 +468,18 @@ static int rmi_f01_resume(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f01_data *data = fn->data;
+ struct f01_data *f01 = fn->data;
int error;
- if (data->old_nosleep)
- data->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
+ if (f01->old_nosleep)
+ f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
- data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
- data->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
+ f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
+ f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
- &data->device_control.ctrl0,
- sizeof(data->device_control.ctrl0));
+ &f01->device_control.ctrl0,
+ sizeof(f01->device_control.ctrl0));
if (error < 0) {
dev_err(&fn->dev,
"Failed to restore normal operation. Code: %d.\n",
--
1.8.5.3
From: Christopher Heiny <[email protected]>
Both F01_RMI_Ctrl2 and F01_RMI_Ctrl3 (doze_interval and wakeup_threshold)
are controlled by the has_adjustable_doze bit.
Signed-off-by: Christopher Heiny<[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 976aba3..30e0b50 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -328,6 +328,9 @@ static int rmi_f01_initialize(struct rmi_function *fn)
}
}
+ if (data->properties.has_lts)
+ ctrl_base_addr++;
+
if (data->properties.has_adjustable_doze_holdoff) {
data->doze_holdoff_addr = ctrl_base_addr;
ctrl_base_addr++;
@@ -383,7 +386,7 @@ static int rmi_f01_config(struct rmi_function *fn)
dev_err(&fn->dev, "Failed to write interrupt enable.\n");
return retval;
}
- if (data->properties.has_lts) {
+ if (data->properties.has_adjustable_doze) {
retval = rmi_write_block(fn->rmi_dev, data->doze_interval_addr,
&data->device_control.doze_interval,
sizeof(u8));
@@ -391,9 +394,7 @@ static int rmi_f01_config(struct rmi_function *fn)
dev_err(&fn->dev, "Failed to write doze interval.\n");
return retval;
}
- }
- if (data->properties.has_adjustable_doze) {
retval = rmi_write_block(fn->rmi_dev,
data->wakeup_threshold_addr,
&data->device_control.wakeup_threshold,
--
1.8.5.3
We do not need to persist it - we read it when signalled.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 6f90a6c..1e49ab4 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -126,8 +126,6 @@ struct f01_data {
struct f01_device_control device_control;
- u8 device_status;
-
u16 interrupt_enable_addr;
u16 doze_interval_addr;
u16 wakeup_threshold_addr;
@@ -212,6 +210,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
struct f01_data *data = fn->data;
struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
+ u8 device_status;
/*
* Set the configured bit and (optionally) other important stuff
@@ -346,16 +345,16 @@ static int rmi_f01_initialize(struct rmi_function *fn)
}
error = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
- &data->device_status, sizeof(data->device_status));
+ &device_status, sizeof(device_status));
if (error < 0) {
dev_err(&fn->dev, "Failed to read device status.\n");
return error;
}
- if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
+ if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
dev_err(&fn->dev,
"Device was reset during configuration process, status: %#02x!\n",
- RMI_F01_STATUS_CODE(data->device_status));
+ RMI_F01_STATUS_CODE(device_status));
return -EINVAL;
}
@@ -497,18 +496,18 @@ static int rmi_f01_attention(struct rmi_function *fn,
unsigned long *irq_bits)
{
struct rmi_device *rmi_dev = fn->rmi_dev;
- struct f01_data *data = fn->data;
int retval;
+ u8 device_status;
retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
- &data->device_status, sizeof(data->device_status));
+ &device_status, sizeof(device_status));
if (retval < 0) {
dev_err(&fn->dev, "Failed to read device status, code: %d.\n",
retval);
return retval;
}
- if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
+ if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
dev_warn(&fn->dev, "Device reset detected.\n");
retval = rmi_dev->driver->reset_handler(rmi_dev);
if (retval < 0)
--
1.8.5.3
It is an empty stub and is not needed.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 92b90d1..897d8ac 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -451,11 +451,6 @@ static int rmi_f01_probe(struct rmi_function *fn)
return 0;
}
-static void rmi_f01_remove(struct rmi_function *fn)
-{
- /* Placeholder for now. */
-}
-
#ifdef CONFIG_PM_SLEEP
static int rmi_f01_suspend(struct device *dev)
{
@@ -554,7 +549,6 @@ static struct rmi_function_handler rmi_f01_handler = {
},
.func = 0x01,
.probe = rmi_f01_probe,
- .remove = rmi_f01_remove,
.config = rmi_f01_config,
.attention = rmi_f01_attention,
};
--
1.8.5.3
Do not write configuration data in probe(), we have config() for that.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/rmi4/rmi_f01.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 897d8ac..976aba3 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -303,12 +303,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
if (pdata->power_management.doze_interval) {
data->device_control.doze_interval =
pdata->power_management.doze_interval;
- error = rmi_write(rmi_dev, data->doze_interval_addr,
- data->device_control.doze_interval);
- if (error < 0) {
- dev_err(&fn->dev, "Failed to configure F01 doze interval register.\n");
- return error;
- }
} else {
error = rmi_read(rmi_dev, data->doze_interval_addr,
&data->device_control.doze_interval);
@@ -324,12 +318,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
if (pdata->power_management.wakeup_threshold) {
data->device_control.wakeup_threshold =
pdata->power_management.wakeup_threshold;
- error = rmi_write(rmi_dev, data->wakeup_threshold_addr,
- data->device_control.wakeup_threshold);
- if (error < 0) {
- dev_err(&fn->dev, "Failed to configure F01 wakeup threshold register.\n");
- return error;
- }
} else {
error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
&data->device_control.wakeup_threshold);
@@ -347,12 +335,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
if (pdata->power_management.doze_holdoff) {
data->device_control.doze_holdoff =
pdata->power_management.doze_holdoff;
- error = rmi_write(rmi_dev, data->doze_holdoff_addr,
- data->device_control.doze_holdoff);
- if (error < 0) {
- dev_err(&fn->dev, "Failed to configure F01 doze holdoff register.\n");
- return error;
- }
} else {
error = rmi_read(rmi_dev, data->doze_holdoff_addr,
&data->device_control.doze_holdoff);
--
1.8.5.3
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Data that is allocated with devm_kzalloc() should not be freed with
> kfree(). In fact, we should rely on the fact that memory is managed and let
> devres core free it for us.
>
> Reported-by: Courtney Cavin <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 381ad60..92b90d1 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -272,7 +272,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (error < 0) {
> dev_err(&fn->dev,
> "Failed to read F01 control interrupt enable register.\n");
> - goto error_exit;
> + return error;
> }
>
> ctrl_base_addr += data->num_of_irq_regs;
> @@ -307,14 +307,14 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> data->device_control.doze_interval);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to configure F01 doze interval register.\n");
> - goto error_exit;
> + return error;
> }
> } else {
> error = rmi_read(rmi_dev, data->doze_interval_addr,
> &data->device_control.doze_interval);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
> - goto error_exit;
> + return error;
> }
> }
>
> @@ -328,14 +328,14 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> data->device_control.wakeup_threshold);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to configure F01 wakeup threshold register.\n");
> - goto error_exit;
> + return error;
> }
> } else {
> error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
> &data->device_control.wakeup_threshold);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
> - goto error_exit;
> + return error;
> }
> }
> }
> @@ -351,14 +351,14 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> data->device_control.doze_holdoff);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to configure F01 doze holdoff register.\n");
> - goto error_exit;
> + return error;
> }
> } else {
> error = rmi_read(rmi_dev, data->doze_holdoff_addr,
> &data->device_control.doze_holdoff);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
> - goto error_exit;
> + return error;
> }
> }
> }
> @@ -367,22 +367,17 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> &data->device_status, sizeof(data->device_status));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read device status.\n");
> - goto error_exit;
> + return error;
> }
>
> if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
> dev_err(&fn->dev,
> "Device was reset during configuration process, status: %#02x!\n",
> RMI_F01_STATUS_CODE(data->device_status));
> - error = -EINVAL;
> - goto error_exit;
> + return -EINVAL;
> }
>
> return 0;
> -
> - error_exit:
> - kfree(data);
> - return error;
> }
>
> static int rmi_f01_config(struct rmi_function *fn)
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> It is an empty stub and is not needed.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 92b90d1..897d8ac 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -451,11 +451,6 @@ static int rmi_f01_probe(struct rmi_function *fn)
> return 0;
> }
>
> -static void rmi_f01_remove(struct rmi_function *fn)
> -{
> - /* Placeholder for now. */
> -}
> -
> #ifdef CONFIG_PM_SLEEP
> static int rmi_f01_suspend(struct device *dev)
> {
> @@ -554,7 +549,6 @@ static struct rmi_function_handler rmi_f01_handler = {
> },
> .func = 0x01,
> .probe = rmi_f01_probe,
> - .remove = rmi_f01_remove,
> .config = rmi_f01_config,
> .attention = rmi_f01_attention,
> };
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Do not write configuration data in probe(), we have config() for that.
Then we should call config() in rmi_function_probe() to ensure that
any platform data or device tree configuration settings get written
to the device.
Thinking about that, it looks like it's not fatal if the config
write fails in that situation. The device might not function as
intended, but you can hopefully get some use out of it (for
instance, a phone's touchscreen sensitivity might be wacky, but
the user will still be able to dial tech support).
For example:
static int rmi_function_probe(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
struct rmi_function_handler *handler =
to_rmi_function_handler(dev->driver);
int error;
if (handler->probe) {
error = handler->probe(fn);
if (error)
return error;
}
if (handler->config) {
error = handler->config(fn);
if (error)
dev_warn(dev, "Driver config failed.\n");
}
return 0;
}
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 897d8ac..976aba3 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -303,12 +303,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (pdata->power_management.doze_interval) {
> data->device_control.doze_interval =
> pdata->power_management.doze_interval;
> - error = rmi_write(rmi_dev, data->doze_interval_addr,
> - data->device_control.doze_interval);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to configure F01 doze interval register.\n");
> - return error;
> - }
> } else {
> error = rmi_read(rmi_dev, data->doze_interval_addr,
> &data->device_control.doze_interval);
> @@ -324,12 +318,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (pdata->power_management.wakeup_threshold) {
> data->device_control.wakeup_threshold =
> pdata->power_management.wakeup_threshold;
> - error = rmi_write(rmi_dev, data->wakeup_threshold_addr,
> - data->device_control.wakeup_threshold);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to configure F01 wakeup threshold register.\n");
> - return error;
> - }
> } else {
> error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
> &data->device_control.wakeup_threshold);
> @@ -347,12 +335,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (pdata->power_management.doze_holdoff) {
> data->device_control.doze_holdoff =
> pdata->power_management.doze_holdoff;
> - error = rmi_write(rmi_dev, data->doze_holdoff_addr,
> - data->device_control.doze_holdoff);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to configure F01 doze holdoff register.\n");
> - return error;
> - }
> } else {
> error = rmi_read(rmi_dev, data->doze_holdoff_addr,
> &data->device_control.doze_holdoff);
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> From: Christopher Heiny <[email protected]>
>
> Both F01_RMI_Ctrl2 and F01_RMI_Ctrl3 (doze_interval and wakeup_threshold)
> are controlled by the has_adjustable_doze bit.
>
> Signed-off-by: Christopher Heiny<[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Not sure if this need an Ack, but just in case.
Acked-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 976aba3..30e0b50 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -328,6 +328,9 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
> }
>
> + if (data->properties.has_lts)
> + ctrl_base_addr++;
> +
> if (data->properties.has_adjustable_doze_holdoff) {
> data->doze_holdoff_addr = ctrl_base_addr;
> ctrl_base_addr++;
> @@ -383,7 +386,7 @@ static int rmi_f01_config(struct rmi_function *fn)
> dev_err(&fn->dev, "Failed to write interrupt enable.\n");
> return retval;
> }
> - if (data->properties.has_lts) {
> + if (data->properties.has_adjustable_doze) {
> retval = rmi_write_block(fn->rmi_dev, data->doze_interval_addr,
> &data->device_control.doze_interval,
> sizeof(u8));
> @@ -391,9 +394,7 @@ static int rmi_f01_config(struct rmi_function *fn)
> dev_err(&fn->dev, "Failed to write doze interval.\n");
> return retval;
> }
> - }
>
> - if (data->properties.has_adjustable_doze) {
> retval = rmi_write_block(fn->rmi_dev,
> data->wakeup_threshold_addr,
> &data->device_control.wakeup_threshold,
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Let's allocate interrupt mask together with the main structure and combine
> rmi_f01_alloc_memory, rmi_f01_initialize and rmi_f01_probe into single
> function.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 86 ++++++++++++++++----------------------------
> 1 file changed, 30 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index a2f05bc..0e21004 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -130,37 +130,15 @@ struct f01_data {
> u16 doze_interval_addr;
> u16 wakeup_threshold_addr;
> u16 doze_holdoff_addr;
> - int irq_count;
> - int num_of_irq_regs;
>
> #ifdef CONFIG_PM_SLEEP
> bool suspended;
> bool old_nosleep;
> #endif
> -};
> -
> -static int rmi_f01_alloc_memory(struct rmi_function *fn,
> - int num_of_irq_regs)
> -{
> - struct f01_data *f01;
>
> - f01 = devm_kzalloc(&fn->dev, sizeof(struct f01_data), GFP_KERNEL);
> - if (!f01) {
> - dev_err(&fn->dev, "Failed to allocate fn_01_data.\n");
> - return -ENOMEM;
> - }
> -
> - f01->device_control.interrupt_enable = devm_kzalloc(&fn->dev,
> - sizeof(u8) * (num_of_irq_regs),
> - GFP_KERNEL);
> - if (!f01->device_control.interrupt_enable) {
> - dev_err(&fn->dev, "Failed to allocate interrupt enable.\n");
> - return -ENOMEM;
> - }
> - fn->data = f01;
> -
> - return 0;
> -}
> + unsigned int num_of_irq_regs;
> + u8 interrupt_enable[];
> +};
>
> static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
> u16 query_base_addr,
> @@ -202,16 +180,28 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
> return 0;
> }
>
> -static int rmi_f01_initialize(struct rmi_function *fn)
> +static int rmi_f01_probe(struct rmi_function *fn)
> {
> - u8 temp;
> - int error;
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> - struct f01_data *f01 = fn->data;
> const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + struct f01_data *f01;
> + size_t f01_size;
> + int error;
> u16 ctrl_base_addr = fn->fd.control_base_addr;
> u8 device_status;
> + u8 temp;
> +
> + f01_size = sizeof(struct f01_data) +
> + sizeof(u8) * driver_data->num_of_irq_regs;
> + f01 = devm_kzalloc(&fn->dev, f01_size, GFP_KERNEL);
> + if (!f01) {
> + dev_err(&fn->dev, "Failed to allocate fn01_data.\n");
> + return -ENOMEM;
> + }
> +
> + f01->num_of_irq_regs = driver_data->num_of_irq_regs;
> + f01->device_control.interrupt_enable = f01->interrupt_enable;
>
> /*
> * Set the configured bit and (optionally) other important stuff
> @@ -257,12 +247,11 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return error;
> }
>
> - f01->irq_count = driver_data->irq_count;
> - f01->num_of_irq_regs = driver_data->num_of_irq_regs;
> - ctrl_base_addr += sizeof(u8);
> -
> + /* Advance to interrupt control registers */
> + ctrl_base_addr++;
> f01->interrupt_enable_addr = ctrl_base_addr;
> - error = rmi_read_block(rmi_dev, ctrl_base_addr,
> +
> + error = rmi_read_block(rmi_dev, f01->interrupt_enable_addr,
> f01->device_control.interrupt_enable,
> sizeof(u8) * (f01->num_of_irq_regs));
> if (error) {
> @@ -272,9 +261,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return error;
> }
>
> - ctrl_base_addr += f01->num_of_irq_regs;
> -
> - /* dummy read in order to clear irqs */
> + /* Dummy read in order to clear irqs */
> error = rmi_read(rmi_dev, fn->fd.data_base_addr + 1, &temp);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read Interrupt Status.\n");
> @@ -287,11 +274,13 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> dev_err(&fn->dev, "Failed to read F01 properties.\n");
> return error;
> }
> +
> dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
> - f01->properties.manufacturer_id == 1 ?
> - "Synaptics" : "unknown",
> + f01->properties.manufacturer_id == 1 ? "Synaptics" : "unknown",
> f01->properties.product_id);
>
> + ctrl_base_addr += f01->num_of_irq_regs;
> +
> /* read control register */
> if (f01->properties.has_adjustable_doze) {
> f01->doze_interval_addr = ctrl_base_addr;
> @@ -365,6 +354,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return -EINVAL;
> }
>
> + fn->data = f01;
> +
> return 0;
> }
>
> @@ -424,23 +415,6 @@ static int rmi_f01_config(struct rmi_function *fn)
> return 0;
> }
>
> -static int rmi_f01_probe(struct rmi_function *fn)
> -{
> - struct rmi_driver_data *driver_data =
> - dev_get_drvdata(&fn->rmi_dev->dev);
> - int error;
> -
> - error = rmi_f01_alloc_memory(fn, driver_data->num_of_irq_regs);
> - if (error)
> - return error;
> -
> - error = rmi_f01_initialize(fn);
> - if (error)
> - return error;
> -
> - return 0;
> -}
> -
> #ifdef CONFIG_PM_SLEEP
> static int rmi_f01_suspend(struct device *dev)
> {
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> When accessing platform data of RMI device let's make sure we do not
> accidentally change data that may be shared by returning const pointer.
> Also switch to an inline function instead of macro to ensure type safety.
That's a good idea. We'll want to look at some of our other #define
accessors as well, I think.
Signed-off-by: Christopher Heiny <[email protected]>
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> drivers/input/rmi4/rmi_bus.h | 7 ++++++-
> drivers/input/rmi4/rmi_driver.c | 8 ++++----
> drivers/input/rmi4/rmi_f01.c | 2 +-
> drivers/input/rmi4/rmi_f11.c | 2 +-
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
> index 8d47f52..02a2af8 100644
> --- a/drivers/input/rmi4/rmi_bus.h
> +++ b/drivers/input/rmi4/rmi_bus.h
> @@ -224,7 +224,12 @@ struct rmi_device {
> };
>
> #define to_rmi_device(d) container_of(d, struct rmi_device, dev)
> -#define to_rmi_platform_data(d) ((d)->xport->dev->platform_data)
> +
> +static inline const struct rmi_device_platform_data *
> +rmi_get_platform_data(struct rmi_device *d)
> +{
> + return dev_get_platdata(d->xport->dev);
> +}
>
> bool rmi_is_physical_device(struct device *dev);
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 736cfbd..473efbc 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -598,7 +598,7 @@ static int rmi_initial_reset(struct rmi_device *rmi_dev,
> u16 cmd_addr = pdt->page_start + pdt->command_base_addr;
> u8 cmd_buf = RMI_DEVICE_RESET_CMD;
> const struct rmi_device_platform_data *pdata =
> - to_rmi_platform_data(rmi_dev);
> + rmi_get_platform_data(rmi_dev);
>
> error = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
> if (error) {
> @@ -621,7 +621,7 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> {
> struct device *dev = &rmi_dev->dev;
> struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> - struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> int *current_irq_count = ctx;
> struct rmi_function *fn;
> int i;
> @@ -745,7 +745,7 @@ static int rmi_driver_remove(struct device *dev)
> struct rmi_device *rmi_dev = to_rmi_device(dev);
> struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> const struct rmi_device_platform_data *pdata =
> - to_rmi_platform_data(rmi_dev);
> + rmi_get_platform_data(rmi_dev);
>
> disable_sensor(rmi_dev);
> rmi_free_function_list(rmi_dev);
> @@ -781,7 +781,7 @@ static int rmi_driver_probe(struct device *dev)
> rmi_driver = to_rmi_driver(dev->driver);
> rmi_dev->driver = rmi_driver;
>
> - pdata = to_rmi_platform_data(rmi_dev);
> + pdata = rmi_get_platform_data(rmi_dev);
>
> data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
> if (!data) {
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 0e21004..36fcf8d 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -184,7 +184,7 @@ static int rmi_f01_probe(struct rmi_function *fn)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> - const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> struct f01_data *f01;
> size_t f01_size;
> int error;
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 2aa7d17..5072437 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1176,7 +1176,7 @@ static int rmi_f11_initialize(struct rmi_function *fn)
> u16 control_base_addr;
> u16 max_x_pos, max_y_pos, temp;
> int rc;
> - struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> struct f11_2d_sensor *sensor;
> u8 buf;
>
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> We do not need to persist it - we read it when signalled.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Acked-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 6f90a6c..1e49ab4 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -126,8 +126,6 @@ struct f01_data {
>
> struct f01_device_control device_control;
>
> - u8 device_status;
> -
> u16 interrupt_enable_addr;
> u16 doze_interval_addr;
> u16 wakeup_threshold_addr;
> @@ -212,6 +210,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> struct f01_data *data = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + u8 device_status;
>
> /*
> * Set the configured bit and (optionally) other important stuff
> @@ -346,16 +345,16 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
>
> error = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &data->device_status, sizeof(data->device_status));
> + &device_status, sizeof(device_status));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read device status.\n");
> return error;
> }
>
> - if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
> + if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
> dev_err(&fn->dev,
> "Device was reset during configuration process, status: %#02x!\n",
> - RMI_F01_STATUS_CODE(data->device_status));
> + RMI_F01_STATUS_CODE(device_status));
> return -EINVAL;
> }
>
> @@ -497,18 +496,18 @@ static int rmi_f01_attention(struct rmi_function *fn,
> unsigned long *irq_bits)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *data = fn->data;
> int retval;
> + u8 device_status;
>
> retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &data->device_status, sizeof(data->device_status));
> + &device_status, sizeof(device_status));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to read device status, code: %d.\n",
> retval);
> return retval;
> }
>
> - if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
> + if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
> dev_warn(&fn->dev, "Device reset detected.\n");
> retval = rmi_dev->driver->reset_handler(rmi_dev);
> if (retval < 0)
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> We have too many "data"s: f01_data, driver_data, pdata, etc. Let's
> untangle it a bit.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Acked-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 135 ++++++++++++++++++++++---------------------
> 1 file changed, 68 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 1e49ab4..1219e0c 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -208,7 +208,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> u16 ctrl_base_addr;
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> u8 device_status;
>
> @@ -218,8 +218,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> */
> ctrl_base_addr = fn->fd.control_base_addr;
> error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 control.\n");
> return error;
> @@ -228,10 +228,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> case RMI_F01_NOSLEEP_DEFAULT:
> break;
> case RMI_F01_NOSLEEP_OFF:
> - data->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
> break;
> case RMI_F01_NOSLEEP_ON:
> - data->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> break;
> }
>
> @@ -240,38 +240,38 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> * reboot without power cycle. If so, clear it so the sensor
> * is certain to function.
> */
> - if ((data->device_control.ctrl0 & RMI_F01_CTRL0_SLEEP_MODE_MASK) !=
> + if ((f01->device_control.ctrl0 & RMI_F01_CTRL0_SLEEP_MODE_MASK) !=
> RMI_SLEEP_MODE_NORMAL) {
> dev_warn(&fn->dev,
> "WARNING: Non-zero sleep mode found. Clearing...\n");
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> }
>
> - data->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
>
> error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to write F01 control.\n");
> return error;
> }
>
> - data->irq_count = driver_data->irq_count;
> - data->num_of_irq_regs = driver_data->num_of_irq_regs;
> + f01->irq_count = driver_data->irq_count;
> + f01->num_of_irq_regs = driver_data->num_of_irq_regs;
> ctrl_base_addr += sizeof(u8);
>
> - data->interrupt_enable_addr = ctrl_base_addr;
> + f01->interrupt_enable_addr = ctrl_base_addr;
> error = rmi_read_block(rmi_dev, ctrl_base_addr,
> - data->device_control.interrupt_enable,
> - sizeof(u8) * (data->num_of_irq_regs));
> + f01->device_control.interrupt_enable,
> + sizeof(u8) * (f01->num_of_irq_regs));
> if (error < 0) {
> dev_err(&fn->dev,
> "Failed to read F01 control interrupt enable register.\n");
> return error;
> }
>
> - ctrl_base_addr += data->num_of_irq_regs;
> + ctrl_base_addr += f01->num_of_irq_regs;
>
> /* dummy read in order to clear irqs */
> error = rmi_read(rmi_dev, fn->fd.data_base_addr + 1, &temp);
> @@ -281,42 +281,42 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
>
> error = rmi_f01_read_properties(rmi_dev, fn->fd.query_base_addr,
> - &data->properties);
> + &f01->properties);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 properties.\n");
> return error;
> }
> dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
> - data->properties.manufacturer_id == 1 ?
> + f01->properties.manufacturer_id == 1 ?
> "Synaptics" : "unknown",
> - data->properties.product_id);
> + f01->properties.product_id);
>
> /* read control register */
> - if (data->properties.has_adjustable_doze) {
> - data->doze_interval_addr = ctrl_base_addr;
> + if (f01->properties.has_adjustable_doze) {
> + f01->doze_interval_addr = ctrl_base_addr;
> ctrl_base_addr++;
>
> if (pdata->power_management.doze_interval) {
> - data->device_control.doze_interval =
> + f01->device_control.doze_interval =
> pdata->power_management.doze_interval;
> } else {
> - error = rmi_read(rmi_dev, data->doze_interval_addr,
> - &data->device_control.doze_interval);
> + error = rmi_read(rmi_dev, f01->doze_interval_addr,
> + &f01->device_control.doze_interval);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
> return error;
> }
> }
>
> - data->wakeup_threshold_addr = ctrl_base_addr;
> + f01->wakeup_threshold_addr = ctrl_base_addr;
> ctrl_base_addr++;
>
> if (pdata->power_management.wakeup_threshold) {
> - data->device_control.wakeup_threshold =
> + f01->device_control.wakeup_threshold =
> pdata->power_management.wakeup_threshold;
> } else {
> - error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
> - &data->device_control.wakeup_threshold);
> + error = rmi_read(rmi_dev, f01->wakeup_threshold_addr,
> + &f01->device_control.wakeup_threshold);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
> return error;
> @@ -324,19 +324,19 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
> }
>
> - if (data->properties.has_lts)
> + if (f01->properties.has_lts)
> ctrl_base_addr++;
>
> - if (data->properties.has_adjustable_doze_holdoff) {
> - data->doze_holdoff_addr = ctrl_base_addr;
> + if (f01->properties.has_adjustable_doze_holdoff) {
> + f01->doze_holdoff_addr = ctrl_base_addr;
> ctrl_base_addr++;
>
> if (pdata->power_management.doze_holdoff) {
> - data->device_control.doze_holdoff =
> + f01->device_control.doze_holdoff =
> pdata->power_management.doze_holdoff;
> } else {
> - error = rmi_read(rmi_dev, data->doze_holdoff_addr,
> - &data->device_control.doze_holdoff);
> + error = rmi_read(rmi_dev, f01->doze_holdoff_addr,
> + &f01->device_control.doze_holdoff);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
> return error;
> @@ -363,28 +363,28 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>
> static int rmi_f01_config(struct rmi_function *fn)
> {
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> int retval;
>
> retval = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write device_control.reg.\n");
> return retval;
> }
>
> - retval = rmi_write_block(fn->rmi_dev, data->interrupt_enable_addr,
> - data->device_control.interrupt_enable,
> - sizeof(u8) * data->num_of_irq_regs);
> -
> + retval = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
> + f01->device_control.interrupt_enable,
> + sizeof(u8) * f01->num_of_irq_regs);
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write interrupt enable.\n");
> return retval;
> }
> - if (data->properties.has_adjustable_doze) {
> - retval = rmi_write_block(fn->rmi_dev, data->doze_interval_addr,
> - &data->device_control.doze_interval,
> +
> + if (f01->properties.has_adjustable_doze) {
> + retval = rmi_write_block(fn->rmi_dev, f01->doze_interval_addr,
> + &f01->device_control.doze_interval,
> sizeof(u8));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write doze interval.\n");
> @@ -392,8 +392,8 @@ static int rmi_f01_config(struct rmi_function *fn)
> }
>
> retval = rmi_write_block(fn->rmi_dev,
> - data->wakeup_threshold_addr,
> - &data->device_control.wakeup_threshold,
> + f01->wakeup_threshold_addr,
> + &f01->device_control.wakeup_threshold,
> sizeof(u8));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write wakeup threshold.\n");
> @@ -401,15 +401,16 @@ static int rmi_f01_config(struct rmi_function *fn)
> }
> }
>
> - if (data->properties.has_adjustable_doze_holdoff) {
> - retval = rmi_write_block(fn->rmi_dev, data->doze_holdoff_addr,
> - &data->device_control.doze_holdoff,
> + if (f01->properties.has_adjustable_doze_holdoff) {
> + retval = rmi_write_block(fn->rmi_dev, f01->doze_holdoff_addr,
> + &f01->device_control.doze_holdoff,
> sizeof(u8));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write doze holdoff.\n");
> return retval;
> }
> }
> +
> return 0;
> }
>
> @@ -435,28 +436,28 @@ static int rmi_f01_suspend(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> int error;
>
> - data->old_nosleep = data->device_control.ctrl0 &
> + f01->old_nosleep = f01->device_control.ctrl0 &
> RMI_F01_CRTL0_NOSLEEP_BIT;
> - data->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
>
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> - data->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
>
> error = rmi_write_block(rmi_dev,
> fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to write sleep mode. Code: %d.\n",
> error);
> - if (data->old_nosleep)
> - data->device_control.ctrl0 |=
> + if (f01->old_nosleep)
> + f01->device_control.ctrl0 |=
> RMI_F01_CRTL0_NOSLEEP_BIT;
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> - data->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> return error;
> }
>
> @@ -467,18 +468,18 @@ static int rmi_f01_resume(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> int error;
>
> - if (data->old_nosleep)
> - data->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> + if (f01->old_nosleep)
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
>
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> - data->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
>
> error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev,
> "Failed to restore normal operation. Code: %d.\n",
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Use rmi_read()/rmi_write() for reading/writing single-byte data. Also print
> error code when IO fails.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Acked-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 170 ++++++++++++++++++++++---------------------
> 1 file changed, 88 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 1219e0c..a2f05bc 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -171,8 +171,9 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
>
> error = rmi_read_block(rmi_dev, query_base_addr,
> basic_query, sizeof(basic_query));
> - if (error < 0) {
> - dev_err(&rmi_dev->dev, "Failed to read device query registers.\n");
> + if (error) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read device query registers: %d\n", error);
> return error;
> }
>
> @@ -205,25 +206,25 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> {
> u8 temp;
> int error;
> - u16 ctrl_base_addr;
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> struct f01_data *f01 = fn->data;
> - struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + u16 ctrl_base_addr = fn->fd.control_base_addr;
> u8 device_status;
>
> /*
> * Set the configured bit and (optionally) other important stuff
> * in the device control register.
> */
> - ctrl_base_addr = fn->fd.control_base_addr;
> - error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 control.\n");
> +
> + error = rmi_read(rmi_dev, fn->fd.control_base_addr,
> + &f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev, "Failed to read F01 control: %d\n", error);
> return error;
> }
> +
> switch (pdata->power_management.nosleep) {
> case RMI_F01_NOSLEEP_DEFAULT:
> break;
> @@ -249,11 +250,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>
> f01->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
>
> - error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to write F01 control.\n");
> + error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev, "Failed to write F01 control: %d\n", error);
> return error;
> }
>
> @@ -265,9 +265,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> error = rmi_read_block(rmi_dev, ctrl_base_addr,
> f01->device_control.interrupt_enable,
> sizeof(u8) * (f01->num_of_irq_regs));
> - if (error < 0) {
> + if (error) {
> dev_err(&fn->dev,
> - "Failed to read F01 control interrupt enable register.\n");
> + "Failed to read F01 control interrupt enable register: %d\n",
> + error);
> return error;
> }
>
> @@ -302,8 +303,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> } else {
> error = rmi_read(rmi_dev, f01->doze_interval_addr,
> &f01->device_control.doze_interval);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to read F01 doze interval register: %d\n",
> + error);
> return error;
> }
> }
> @@ -318,7 +321,9 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> error = rmi_read(rmi_dev, f01->wakeup_threshold_addr,
> &f01->device_control.wakeup_threshold);
> if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
> + dev_err(&fn->dev,
> + "Failed to read F01 wakeup threshold register: %d\n",
> + error);
> return error;
> }
> }
> @@ -337,17 +342,19 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> } else {
> error = rmi_read(rmi_dev, f01->doze_holdoff_addr,
> &f01->device_control.doze_holdoff);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to read F01 doze holdoff register: %d\n",
> + error);
> return error;
> }
> }
> }
>
> - error = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &device_status, sizeof(device_status));
> + error = rmi_read(rmi_dev, fn->fd.data_base_addr, &device_status);
> if (error < 0) {
> - dev_err(&fn->dev, "Failed to read device status.\n");
> + dev_err(&fn->dev,
> + "Failed to read device status: %d\n", error);
> return error;
> }
>
> @@ -364,50 +371,53 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> static int rmi_f01_config(struct rmi_function *fn)
> {
> struct f01_data *f01 = fn->data;
> - int retval;
> -
> - retval = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write device_control.reg.\n");
> - return retval;
> + int error;
> +
> + error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write device_control register: %d\n", error);
> + return error;
> }
>
> - retval = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
> - f01->device_control.interrupt_enable,
> - sizeof(u8) * f01->num_of_irq_regs);
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write interrupt enable.\n");
> - return retval;
> + error = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
> + f01->device_control.interrupt_enable,
> + sizeof(u8) * f01->num_of_irq_regs);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write interrupt enable: %d\n", error);
> + return error;
> }
>
> if (f01->properties.has_adjustable_doze) {
> - retval = rmi_write_block(fn->rmi_dev, f01->doze_interval_addr,
> - &f01->device_control.doze_interval,
> - sizeof(u8));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write doze interval.\n");
> - return retval;
> + error = rmi_write(fn->rmi_dev, f01->doze_interval_addr,
> + f01->device_control.doze_interval);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write doze interval: %d\n", error);
> + return error;
> }
>
> - retval = rmi_write_block(fn->rmi_dev,
> + error = rmi_write_block(fn->rmi_dev,
> f01->wakeup_threshold_addr,
> &f01->device_control.wakeup_threshold,
> sizeof(u8));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write wakeup threshold.\n");
> - return retval;
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write wakeup threshold: %d\n",
> + error);
> + return error;
> }
> }
>
> if (f01->properties.has_adjustable_doze_holdoff) {
> - retval = rmi_write_block(fn->rmi_dev, f01->doze_holdoff_addr,
> - &f01->device_control.doze_holdoff,
> - sizeof(u8));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write doze holdoff.\n");
> - return retval;
> + error = rmi_write(fn->rmi_dev, f01->doze_holdoff_addr,
> + f01->device_control.doze_holdoff);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write doze holdoff: %d\n", error);
> + return error;
> }
> }
>
> @@ -439,23 +449,19 @@ static int rmi_f01_suspend(struct device *dev)
> struct f01_data *f01 = fn->data;
> int error;
>
> - f01->old_nosleep = f01->device_control.ctrl0 &
> - RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->old_nosleep =
> + f01->device_control.ctrl0 & RMI_F01_CRTL0_NOSLEEP_BIT;
> f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
>
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
>
> - error = rmi_write_block(rmi_dev,
> - fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to write sleep mode. Code: %d.\n",
> - error);
> + error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev, "Failed to write sleep mode: %d.\n", error);
> if (f01->old_nosleep)
> - f01->device_control.ctrl0 |=
> - RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> return error;
> @@ -477,13 +483,11 @@ static int rmi_f01_resume(struct device *dev)
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
>
> - error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> + error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> dev_err(&fn->dev,
> - "Failed to restore normal operation. Code: %d.\n",
> - error);
> + "Failed to restore normal operation: %d.\n", error);
> return error;
> }
>
> @@ -497,23 +501,25 @@ static int rmi_f01_attention(struct rmi_function *fn,
> unsigned long *irq_bits)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - int retval;
> + int error;
> u8 device_status;
>
> - retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &device_status, sizeof(device_status));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to read device status, code: %d.\n",
> - retval);
> - return retval;
> + error = rmi_read(rmi_dev, fn->fd.data_base_addr, &device_status);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to read device status: %d.\n", error);
> + return error;
> }
>
> if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
> dev_warn(&fn->dev, "Device reset detected.\n");
> - retval = rmi_dev->driver->reset_handler(rmi_dev);
> - if (retval < 0)
> - return retval;
> + error = rmi_dev->driver->reset_handler(rmi_dev);
> + if (error) {
> + dev_err(&fn->dev, "Device reset failed: %d\n", error);
> + return error;
> + }
> }
> +
> return 0;
> }
>
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Device core provides way of accessing driver-private data, we should
> use it.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Acked-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_bus.h | 1 -
> drivers/input/rmi4/rmi_f01.c | 14 +++++------
> drivers/input/rmi4/rmi_f11.c | 57 ++++++++++++++++++++------------------------
> 3 files changed, 32 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
> index 02a2af8..5ad94c6 100644
> --- a/drivers/input/rmi4/rmi_bus.h
> +++ b/drivers/input/rmi4/rmi_bus.h
> @@ -48,7 +48,6 @@ struct rmi_function {
> int num_of_irqs;
> int irq_pos;
> unsigned long *irq_mask;
> - void *data;
> struct list_head node;
>
> #ifdef CONFIG_RMI4_DEBUG
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 36fcf8d..e646f60 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -354,14 +354,14 @@ static int rmi_f01_probe(struct rmi_function *fn)
> return -EINVAL;
> }
>
> - fn->data = f01;
> + dev_set_drvdata(&fn->dev, f01);
>
> return 0;
> }
>
> static int rmi_f01_config(struct rmi_function *fn)
> {
> - struct f01_data *f01 = fn->data;
> + struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> int error;
>
> error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> @@ -419,8 +419,7 @@ static int rmi_f01_config(struct rmi_function *fn)
> static int rmi_f01_suspend(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *f01 = fn->data;
> + struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> int error;
>
> f01->old_nosleep =
> @@ -430,7 +429,7 @@ static int rmi_f01_suspend(struct device *dev)
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
>
> - error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> f01->device_control.ctrl0);
> if (error) {
> dev_err(&fn->dev, "Failed to write sleep mode: %d.\n", error);
> @@ -447,8 +446,7 @@ static int rmi_f01_suspend(struct device *dev)
> static int rmi_f01_resume(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *f01 = fn->data;
> + struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> int error;
>
> if (f01->old_nosleep)
> @@ -457,7 +455,7 @@ static int rmi_f01_resume(struct device *dev)
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
>
> - error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> f01->device_control.ctrl0);
> if (error) {
> dev_err(&fn->dev,
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 5072437..a26b944 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1087,9 +1087,8 @@ static int rmi_f11_get_query_parameters(struct rmi_device *rmi_dev,
> /* This operation is done in a number of places, so we have a handy routine
> * for it.
> */
> -static void f11_set_abs_params(struct rmi_function *fn)
> +static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
> {
> - struct f11_data *f11 = fn->data;
> struct f11_2d_sensor *sensor = &f11->sensor;
> struct input_dev *input = sensor->input;
> /* These two lines are not doing what we want them to. So we use
> @@ -1190,7 +1189,6 @@ static int rmi_f11_initialize(struct rmi_function *fn)
> if (!f11)
> return -ENOMEM;
>
> - fn->data = f11;
> f11->rezero_wait_ms = pdata->f11_rezero_wait;
>
> query_base_addr = fn->fd.query_base_addr;
> @@ -1280,13 +1278,16 @@ static int rmi_f11_initialize(struct rmi_function *fn)
> }
>
> mutex_init(&f11->dev_controls_mutex);
> +
> + dev_set_drvdata(&fn->dev, f11);
> +
> return 0;
> }
>
> static int rmi_f11_register_devices(struct rmi_function *fn)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f11_data *f11 = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> struct input_dev *input_dev;
> struct input_dev *input_dev_mouse;
> struct rmi_driver *driver = rmi_dev->driver;
> @@ -1304,8 +1305,8 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> rc = driver->set_input_params(rmi_dev, input_dev);
> if (rc < 0) {
> dev_err(&fn->dev,
> - "%s: Error in setting input device.\n",
> - __func__);
> + "%s: Error in setting input device.\n",
> + __func__);
> goto error_unregister;
> }
> }
> @@ -1319,7 +1320,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> set_bit(EV_ABS, input_dev->evbit);
> input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>
> - f11_set_abs_params(fn);
> + f11_set_abs_params(fn, f11);
>
> if (sensor->sens_query.has_rel) {
> set_bit(EV_REL, input_dev->evbit);
> @@ -1327,7 +1328,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> set_bit(REL_Y, input_dev->relbit);
> }
> rc = input_register_device(input_dev);
> - if (rc < 0) {
> + if (rc) {
> input_free_device(input_dev);
> sensor->input = NULL;
> goto error_unregister;
> @@ -1347,8 +1348,8 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> input_dev_mouse);
> if (rc < 0) {
> dev_err(&fn->dev,
> - "%s: Error in setting input device.\n",
> - __func__);
> + "%s: Error in setting input device.\n",
> + __func__);
> goto error_unregister;
> }
> }
> @@ -1366,7 +1367,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> set_bit(BTN_RIGHT, input_dev_mouse->keybit);
>
> rc = input_register_device(input_dev_mouse);
> - if (rc < 0) {
> + if (rc) {
> input_free_device(input_dev_mouse);
> sensor->mouse_input = NULL;
> goto error_unregister;
> @@ -1390,19 +1391,9 @@ error_unregister:
> return rc;
> }
>
> -static void rmi_f11_free_devices(struct rmi_function *fn)
> -{
> - struct f11_data *f11 = fn->data;
> -
> - if (f11->sensor.input)
> - input_unregister_device(f11->sensor.input);
> - if (f11->sensor.mouse_input)
> - input_unregister_device(f11->sensor.mouse_input);
> -}
> -
> static int rmi_f11_config(struct rmi_function *fn)
> {
> - struct f11_data *f11 = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> int rc;
>
> rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
> @@ -1416,7 +1407,7 @@ static int rmi_f11_config(struct rmi_function *fn)
> static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f11_data *f11 = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> u16 data_base_addr = fn->fd.data_base_addr;
> u16 data_base_addr_offset = 0;
> int error;
> @@ -1425,7 +1416,7 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
> data_base_addr + data_base_addr_offset,
> f11->sensor.data_pkt,
> f11->sensor.pkt_size);
> - if (error < 0)
> + if (error)
> return error;
>
> rmi_f11_finger_handler(f11, &f11->sensor);
> @@ -1438,18 +1429,17 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
> static int rmi_f11_resume(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f11_data *data = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> int error;
>
> dev_dbg(&fn->dev, "Resuming...\n");
> - if (!data->rezero_wait_ms)
> + if (!f11->rezero_wait_ms)
> return 0;
>
> - mdelay(data->rezero_wait_ms);
> + mdelay(f11->rezero_wait_ms);
>
> - error = rmi_write(rmi_dev, fn->fd.command_base_addr, RMI_F11_REZERO);
> - if (error < 0) {
> + error = rmi_write(fn->rmi_dev, fn->fd.command_base_addr, RMI_F11_REZERO);
> + if (error) {
> dev_err(&fn->dev,
> "%s: failed to issue rezero command, error = %d.",
> __func__, error);
> @@ -1479,7 +1469,12 @@ static int rmi_f11_probe(struct rmi_function *fn)
>
> static void rmi_f11_remove(struct rmi_function *fn)
> {
> - rmi_f11_free_devices(fn);
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> +
> + if (f11->sensor.input)
> + input_unregister_device(f11->sensor.input);
> + if (f11->sensor.mouse_input)
> + input_unregister_device(f11->sensor.mouse_input);
> }
>
> static struct rmi_function_handler rmi_f11_handler = {
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> It is not used by anyone.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Acked-by: Christopher Heiny <[email protected]>
> ---
> drivers/input/rmi4/rmi_f01.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 30e0b50..6f90a6c 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -125,7 +125,6 @@ struct f01_data {
> struct f01_basic_properties properties;
>
> struct f01_device_control device_control;
> - struct mutex control_mutex;
>
> u8 device_status;
>
> @@ -214,8 +213,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> struct f01_data *data = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
>
> - mutex_init(&data->control_mutex);
> -
> /*
> * Set the configured bit and (optionally) other important stuff
> * in the device control register.
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
On Thu, Feb 13, 2014 at 11:23:44AM -0800, Christopher Heiny wrote:
> On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> >Do not write configuration data in probe(), we have config() for that.
>
> Then we should call config() in rmi_function_probe() to ensure that
> any platform data or device tree configuration settings get written
> to the device.
Well, yes, we may elect to update device configuration in probe, but
then we should not be doing that 2nd time in ->config(). We shoudl pick
either one or another.
Thanks.
--
Dmitry
On Thu, Feb 13, 2014 at 11:32:30AM -0800, Christopher Heiny wrote:
> On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> >From: Christopher Heiny <[email protected]>
> >
> >Both F01_RMI_Ctrl2 and F01_RMI_Ctrl3 (doze_interval and wakeup_threshold)
> >are controlled by the has_adjustable_doze bit.
> >
> >Signed-off-by: Christopher Heiny<[email protected]>
> >Signed-off-by: Dmitry Torokhov <[email protected]>
>
> Not sure if this need an Ack, but just in case.
>
> Acked-by: Christopher Heiny <[email protected]>
Not the formal ack but I wanted to make sure I split it off correctly.
Thanks for verifying it!
--
Dmitry
On 02/13/2014 01:54 PM, Dmitry Torokhov wrote:
> On Thu, Feb 13, 2014 at 11:23:44AM -0800, Christopher Heiny wrote:
>> >On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
>>> > >Do not write configuration data in probe(), we have config() for that.
>> >
>> >Then we should call config() in rmi_function_probe() to ensure that
>> >any platform data or device tree configuration settings get written
>> >to the device.
>
> Well, yes, we may elect to update device configuration in probe, but
> then we should not be doing that 2nd time in ->config(). We shoudl pick
> either one or another.
But as the code currently stands, config() is only called when a device
reset is detected, not during initialization. So if there's platform
specific configuration data that needs to be written to a function, it
won't get written until after a device reset occurs, which might never
happen. So either we need to write that data (or call config()) in each
function's probe(), or in rmi_function_probe().
On Fri, Feb 14, 2014 at 03:00:43PM -0800, Christopher Heiny wrote:
> On 02/13/2014 01:54 PM, Dmitry Torokhov wrote:
> >On Thu, Feb 13, 2014 at 11:23:44AM -0800, Christopher Heiny wrote:
> >>>On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> >>>> >Do not write configuration data in probe(), we have config() for that.
> >>>
> >>>Then we should call config() in rmi_function_probe() to ensure that
> >>>any platform data or device tree configuration settings get written
> >>>to the device.
> >
> >Well, yes, we may elect to update device configuration in probe, but
> >then we should not be doing that 2nd time in ->config(). We shoudl pick
> >either one or another.
>
> But as the code currently stands, config() is only called when a
> device reset is detected, not during initialization. So if there's
> platform specific configuration data that needs to be written to a
> function, it won't get written until after a device reset occurs,
> which might never happen. So either we need to write that data (or
> call config()) in each function's probe(), or in
> rmi_function_probe().
Ah, I missed the fact that we do not normally call ->config() unless
device was reset. BTW, if device was reset, shouldn't we tear down
everything and then reenumerate all functions?
--
Dmitry
On 02/17/2014 11:23 AM, Dmitry Torokhov wrote:
> On Fri, Feb 14, 2014 at 03:00:43PM -0800, Christopher Heiny wrote:
>> On 02/13/2014 01:54 PM, Dmitry Torokhov wrote:
>>> On Thu, Feb 13, 2014 at 11:23:44AM -0800, Christopher Heiny wrote:
>>>>> On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
>>>>>>> Do not write configuration data in probe(), we have config() for that.
>>>>>
>>>>> Then we should call config() in rmi_function_probe() to ensure that
>>>>> any platform data or device tree configuration settings get written
>>>>> to the device.
>>>
>>> Well, yes, we may elect to update device configuration in probe, but
>>> then we should not be doing that 2nd time in ->config(). We shoudl pick
>>> either one or another.
>>
>> But as the code currently stands, config() is only called when a
>> device reset is detected, not during initialization. So if there's
>> platform specific configuration data that needs to be written to a
>> function, it won't get written until after a device reset occurs,
>> which might never happen. So either we need to write that data (or
>> call config()) in each function's probe(), or in
>> rmi_function_probe().
>
> Ah, I missed the fact that we do not normally call ->config() unless
> device was reset. BTW, if device was reset, shouldn't we tear down
> everything and then reenumerate all functions?
That's only required if the reset is a result of the device being
reflashed. Since we're dropping support for user-space control of
reflash, and will instead use an in-driver reflash, we know which resets
are the result of a reflash and which aren't. The reflash code will do
the following sequence:
- tell the core to tear down the functions
- perform the reflash operation
- reset the device
- tell the core to re-enumerate the functions
Chris