On 12/17/2013 08:57 AM, Dmitry Torokhov wrote:
Hi Chris,
On Mon, Dec 16, 2013 at 07:45:06PM -0800, Christopher Heiny wrote:
This patch fixes two bugs in handling of the RMI4 attention line GPIO.
1) in enable_sensor(), make sure the attn_gpio is defined before attempting to
get its value.
2) in rmi_driver_probe(), declare the name of the attn_gpio, then
request the attn_gpio before attempting to export it. As an added bonus,
the code relating to the export is tidied up.
Signed-off-by: Christopher Heiny <cheiny@xxxxxxxxxxxxx>
Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Cc: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
---
This patch implements changes to the synaptics-rmi4 branch of
Dmitry's input tree. The base for the patch is commit
e0c5aec5e6144ae8391d164e2dc659f8ef2b2ba7.
You do not have to mention base commit (and update it all the time),
that's way too much work. If you are the one posting patches I should
be able to figure out how to apply them.
drivers/input/rmi4/rmi_driver.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index a30c7d3..030e8d5 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -169,7 +169,7 @@ static int enable_sensor(struct rmi_device *rmi_dev)
data->enabled = true;
- if (!pdata->level_triggered &&
+ if (pdata->attn_gpio && !pdata->level_triggered &&
gpio_get_value(pdata->attn_gpio) == pdata->attn_polarity)
retval = process_interrupt_requests(rmi_dev);
@@ -807,6 +807,9 @@ static int rmi_driver_remove(struct device *dev)
return 0;
}
+
+static const char *GPIO_LABEL = "attn";
+
This wastes 4 or 8 bytes I believe. If you want to do that then you
should say:
static const char GPIO_LABEL[] = "attn";
Hmmm. Learned something new today!
static int rmi_driver_probe(struct device *dev)
{
struct rmi_driver *rmi_driver;
@@ -959,20 +962,24 @@ static int rmi_driver_probe(struct device *dev)
}
if (IS_ENABLED(CONFIG_RMI4_DEV) && pdata->attn_gpio) {
- retval = gpio_export(pdata->attn_gpio, false);
- if (retval) {
- dev_warn(dev, "WARNING: Failed to export ATTN gpio!\n");
- retval = 0;
- } else {
- retval = gpio_export_link(dev,
- "attn", pdata->attn_gpio);
- if (retval) {
- dev_warn(dev,
- "WARNING: Failed to symlink ATTN gpio!\n");
- retval = 0;
- } else {
- dev_info(dev, "Exported ATTN GPIO %d.",
- pdata->attn_gpio);
+ retval = gpio_request(pdata->attn_gpio, GPIO_LABEL);
+ if (retval)
+ dev_warn(dev, "WARNING: Failed to request ATTN gpio %d, code: %d.\n",
+ pdata->attn_gpio, retval);
+ else {
The rule is: if one branch needs {} then they both should use them:
if (condition) {
statement;
} else {
statement;
...
statement;
}
OK.
+ retval = gpio_export(pdata->attn_gpio, false);
+ if (retval)
+ dev_warn(dev, "WARNING: Failed to export ATTN %d, code: %d.\n",
+ pdata->attn_gpio, retval);
+ else {
+ retval = gpio_export_link(dev, "attn",
Why are we using constant when we request gpio but not here?
It's a leftover that wasn't caught. We'll use the constant.
+ pdata->attn_gpio);
+ if (retval)
+ dev_warn(dev,
+ "WARNING: Failed to symlink ATTN gpio!\n");
+ else
+ dev_info(dev, "Exported ATTN GPIO %d.",
+ pdata->attn_gpio);
}
}
}
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
