* [PATCH v3 6/6] input: touchscreen: ti_am335x_tsc: Replace delta filtering with median filtering
From: Vignesh R @ 2014-11-11 8:34 UTC (permalink / raw)
To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Benoit Cousson, Tony Lindgren, Russell King, Jonathan Cameron,
Hartmut Knaack, richardcochran, Dmitry Torokhov, Lee Jones,
Sebastian Andrzej Siewior
Cc: Lars-Peter Clausen, Peter Meerwald, Samuel Ortiz, Felipe Balbi,
Vignesh R, Brad Griffis, Sanjeev Sharma, Paul Gortmaker,
Jan Kardell, devicetree, linux-kernel, linux-omap,
linux-arm-kernel, linux-iio, linux-input
In-Reply-To: <1415694844-11230-1-git-send-email-vigneshr@ti.com>
Previously, delta filtering was applied TSC co-ordinate readouts before
reporting a single value to user space. This patch replaces delta filtering
with median filtering. Median filtering sorts co-ordinate readouts, drops min
and max values, and reports the average of remaining values. This method is
more sensible than delta filering. Median filtering is applied only if number
of readouts is greater than 3 else just average of co-ordinate readouts is
reported.
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
drivers/input/touchscreen/ti_am335x_tsc.c | 91 +++++++++++++++++--------------
1 file changed, 51 insertions(+), 40 deletions(-)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 20ce76b1b6e7..e1df470946e6 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -26,6 +26,7 @@
#include <linux/delay.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/sort.h>
#include <linux/mfd/ti_am335x_tscadc.h>
@@ -204,56 +205,61 @@ static void titsc_step_config(struct titsc *ts_dev)
am335x_tsc_se_set_cache(ts_dev->mfd_tscadc, ts_dev->step_mask);
}
+static int titsc_cmp_coord(const void *a, const void *b)
+{
+ return *(int *)a - *(int *)b;
+}
+
static void titsc_read_coordinates(struct titsc *ts_dev,
u32 *x, u32 *y, u32 *z1, u32 *z2)
{
- unsigned int fifocount = titsc_readl(ts_dev, REG_FIFO0CNT);
- unsigned int prev_val_x = ~0, prev_val_y = ~0;
- unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
- unsigned int read, diff;
- unsigned int i, channel;
+ unsigned int yvals[7], xvals[7];
+ unsigned int i, xsum = 0, ysum = 0;
unsigned int creads = ts_dev->coordinate_readouts;
- unsigned int first_step = TOTAL_STEPS - (creads * 2 + 2);
- *z1 = *z2 = 0;
- if (fifocount % (creads * 2 + 2))
- fifocount -= fifocount % (creads * 2 + 2);
- /*
- * Delta filter is used to remove large variations in sampled
- * values from ADC. The filter tries to predict where the next
- * coordinate could be. This is done by taking a previous
- * coordinate and subtracting it form current one. Further the
- * algorithm compares the difference with that of a present value,
- * if true the value is reported to the sub system.
- */
- for (i = 0; i < fifocount; i++) {
- read = titsc_readl(ts_dev, REG_FIFO0);
-
- channel = (read & 0xf0000) >> 16;
- read &= 0xfff;
- if (channel > first_step + creads + 2) {
- diff = abs(read - prev_val_x);
- if (diff < prev_diff_x) {
- prev_diff_x = diff;
- *x = read;
- }
- prev_val_x = read;
+ for (i = 0; i < creads; i++) {
+ yvals[i] = titsc_readl(ts_dev, REG_FIFO0);
+ yvals[i] &= 0xfff;
+ }
- } else if (channel == first_step + creads + 1) {
- *z1 = read;
+ *z1 = titsc_readl(ts_dev, REG_FIFO0);
+ *z1 &= 0xfff;
+ *z2 = titsc_readl(ts_dev, REG_FIFO0);
+ *z2 &= 0xfff;
- } else if (channel == first_step + creads + 2) {
- *z2 = read;
+ for (i = 0; i < creads; i++) {
+ xvals[i] = titsc_readl(ts_dev, REG_FIFO0);
+ xvals[i] &= 0xfff;
+ }
- } else if (channel > first_step) {
- diff = abs(read - prev_val_y);
- if (diff < prev_diff_y) {
- prev_diff_y = diff;
- *y = read;
- }
- prev_val_y = read;
+ /*
+ * If co-ordinates readouts is less than 4 then
+ * report the average. In case of 4 or more
+ * readouts, sort the co-ordinate samples, drop
+ * min and max values and report the average of
+ * remaining values.
+ */
+ if (creads <= 3) {
+ for (i = 0; i < creads; i++) {
+ ysum += yvals[i];
+ xsum += xvals[i];
}
+ ysum /= creads;
+ xsum /= creads;
+ } else {
+ sort(yvals, creads, sizeof(unsigned int),
+ titsc_cmp_coord, NULL);
+ sort(xvals, creads, sizeof(unsigned int),
+ titsc_cmp_coord, NULL);
+ for (i = 1; i < creads - 1; i++) {
+ ysum += yvals[i];
+ xsum += xvals[i];
+ }
+ ysum /= creads - 2;
+ xsum /= creads - 2;
}
+ *y = ysum;
+ *x = xsum;
}
static irqreturn_t titsc_irq(int irq, void *dev)
@@ -362,6 +368,11 @@ static int titsc_parse_dt(struct platform_device *pdev,
if (err < 0)
return err;
+ if (ts_dev->coordinate_readouts <= 0) {
+ dev_warn(&pdev->dev, "invalid co-ordinate readouts, resetting it to 5\n");
+ ts_dev->coordinate_readouts = 5;
+ }
+
err = of_property_read_u32(node, "ti,charge-delay",
&ts_dev->charge_delay);
if (err < 0)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/1] x86: Surface Pro 3 Type Cover 3
From: Jiri Kosina @ 2014-11-11 10:44 UTC (permalink / raw)
To: Alan Wu; +Cc: linux-input, linux-kernel, linux-usb
In-Reply-To: <1415067972-3239-1-git-send-email-alan.c.wu@gmail.com>
On Mon, 3 Nov 2014, Alan Wu wrote:
> Surface Pro 3 Type Cover that works with Ubuntu (and possibly Arch) from this thread. Both trackpad and keyboard work after compiling my own kernel.
> http://ubuntuforums.org/showthread.php?t=2231207&page=2&s=44910e0c56047e4f93dfd9fea58121ef
>
> Also includes Jarrad Whitaker's message which sources
> http://winaero.com/blog/how-to-install-linux-on-surface-pro-3/
> which he says is sourced from a Russian site
>
> Signed-off-by: Alan Wu <alan.c.wu@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] HID: lenovo: Small fixups for compact keyboards
From: Jiri Kosina @ 2014-11-11 10:47 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Antonio Ospite, linux-input, linux-kernel
In-Reply-To: <1415519670-24449-1-git-send-email-jm@lentin.co.uk>
On Sun, 9 Nov 2014, Jamie Lentin wrote:
> Some small problems that have since been picked up since my patchset that
> introduced support for the Thinkpad Compact Keyboards[0]. Both have been tested
> on 3.18-rc1.
Both patches applied.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Lee Jones @ 2014-11-11 12:26 UTC (permalink / raw)
To: Vignesh R
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Benoit Cousson, Tony Lindgren, Russell King, Jonathan Cameron,
Hartmut Knaack, richardcochran, Dmitry Torokhov,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Felipe Balbi, Brad Griffis, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree, linux-kernel, linux-omap
In-Reply-To: <1415694844-11230-4-git-send-email-vigneshr@ti.com>
On Tue, 11 Nov 2014, Vignesh R wrote:
> In one shot mode, sequencer automatically disables all enabled steps at
> the end of each cycle. (both ADC steps and TSC steps) Hence these steps
> need not be saved in reg_se_cache for clearing these steps at a later
> stage.
> Also, when ADC wakes up Sequencer should not be busy executing any of the
> config steps except for the charge step. Previously charge step was 1 ADC
> clock cycle and hence it was ignored.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
You need to provide a changelog here when submitting new versions.
> drivers/mfd/ti_am335x_tscadc.c | 7 +++++--
> include/linux/mfd/ti_am335x_tscadc.h | 1 +
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index d877e777cce6..110038859a8d 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -86,8 +86,12 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
> spin_lock_irq(&tsadc->reg_lock);
> finish_wait(&tsadc->reg_se_wait, &wait);
>
> + /*
> + * Sequencer should either be idle or
> + * busy applying the charge step.
> + */
> reg = tscadc_readl(tsadc, REG_ADCFSM);
> - WARN_ON(reg & SEQ_STATUS);
> + WARN_ON((reg & SEQ_STATUS) && !(reg & CHARGE_STEP));
> tsadc->adc_waiting = false;
> }
> tsadc->adc_in_use = true;
> @@ -96,7 +100,6 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
> void am335x_tsc_se_set_once(struct ti_tscadc_dev *tsadc, u32 val)
> {
> spin_lock_irq(&tsadc->reg_lock);
> - tsadc->reg_se_cache |= val;
You didn't answer my question about this?
> am335x_tscadc_need_adc(tsadc);
>
> tscadc_writel(tsadc, REG_SE, val);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index c99be5dc0f5c..fcce182e4a35 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -128,6 +128,7 @@
>
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> +#define CHARGE_STEP 0x11
>
> #define ADC_CLK 3000000
> #define TOTAL_STEPS 16
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 02/15] GPIO: port LoCoMo gpio support from old driver
From: Dmitry Eremin-Solenikov @ 2014-11-11 13:16 UTC (permalink / raw)
To: Mark Brown
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Linux Input,
linux-leds@vger.kernel.org, linux-spi@vger.kernel.org,
linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org,
Andrea Adami, Russell King, Daniel Mack, Haojian Zhuang,
Robert Jarzmik, Alexandre Courbot, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones
In-Reply-To: <20141106060335.GR8509@sirena.org.uk>
2014-11-06 9:03 GMT+03:00 Mark Brown <broonie@kernel.org>:
> On Thu, Nov 06, 2014 at 01:33:24AM +0400, Dmitry Eremin-Solenikov wrote:
>> 2014-11-03 16:43 GMT+03:00 Linus Walleij <linus.walleij@linaro.org>:
>
>> > Yes that's the point: if you use regmap mmio you get the RMW-locking
>> > for free, with the regmap implementation.
>
>> Just to be more concrete. Currently locomo_gpio_ack_irq() function uses
>> one lock and one unlock for doing 3 consecutive RMW I I convert locomo
>> to regmap, will that be 3 lock/unlock calls or still one? (Or maybe I'm
>> trying to be over-protective here and adding more lock/unlock cycles
>> won't matter that much?)
>
> You'll get three lock/unlocks, we could add an interface for bulk
> updates under one lock if it's important though.
>
>> Next question: if I have to export regmap to several subdrivers, is it better
>> to have one big regmap or to have one-map-per-driver approach?
>
> One regmap for the physical register map which is shared between all the
> users.
Mark, Linus,
Just to better understand your suggestions: do you want me to convert
to regmap only gpio driver or do you suggest to convert all LoCoMo drivers?
That is doable, of course, but the amount of changes is overwhelming.
Also I'm concerned about the performance impact from going through
regmap layers.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 02/15] GPIO: port LoCoMo gpio support from old driver
From: Mark Brown @ 2014-11-11 13:23 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Linus Walleij,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Input,
linux-leds-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org, Andrea Adami,
Russell King, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Alexandre Courbot, Dmitry Torokhov, Bryan Wu, Richard Purdie,
Samuel Ortiz, Lee Jones
In-Reply-To: <CALT56yPr42FV66USngocw=eWPt81d5R2MJxmzBnv02HOMmXAkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 397 bytes --]
On Tue, Nov 11, 2014 at 05:16:38PM +0400, Dmitry Eremin-Solenikov wrote:
> Just to better understand your suggestions: do you want me to convert
> to regmap only gpio driver or do you suggest to convert all LoCoMo drivers?
> That is doable, of course, but the amount of changes is overwhelming.
> Also I'm concerned about the performance impact from going through
> regmap layers.
I don't care.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v4] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Mathias Gottschlag @ 2014-11-11 13:35 UTC (permalink / raw)
To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input
In-Reply-To: <5456B50F.6090700@gmail.com>
The issue seems to be unrelated to the patch, so I'd declare the patch
ready as fas as I can see. Do you need a rebased patch?
Am 02.11.2014 um 23:49 schrieb Mathias Gottschlag:
>
> Apparently this patch breaks the touchpad after suspend/resume on some
> machines - but not on mine. I have no idea how to debug this kind of
> problem, what the source could be, or what debug info could help. Can
> someone maybe lend me a hand here?
>
> The report of the problem has been in the following bug report, at the
> bottom:
> https://bugzilla.redhat.com/show_bug.cgi?id=1110011
>
> Am 01.11.2014 um 10:37 schrieb Mathias Gottschlag:
>> Most of the protocol for these touchpads has been reverse engineered.
This
>> commit adds a basic multitouch-capable driver.
>>
>> A lot of the protocol is still unknown. Especially, we don't know how to
>> identify the device yet apart from the PNP ID.
>>
>> The previous workaround for these devices has been left in place in case
>> the driver is not compiled into the kernel or in case some other device
>> with the same PNP ID is not recognized by the driver yet still has the
> same
>> problems with the device probing code.
>>
>> Signed-off-by: Mathias Gottschlag <mgottschlag@gmail.com>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>
>> Here is v4 of the driver, hopefully without any wrapped lines this
> time. I've
>> received further feedback, the size detection code should be correct
> on Asus
>> X200 and N550 as well.
>>
>> drivers/input/mouse/Kconfig | 10 ++
>> drivers/input/mouse/focaltech.c | 300
> ++++++++++++++++++++++++++++++++++++-
>> drivers/input/mouse/focaltech.h | 60 ++++++++
>> drivers/input/mouse/psmouse-base.c | 32 ++--
>> drivers/input/mouse/psmouse.h | 1 +
>> 5 files changed, 386 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
>> index 366fc7a..db973e5 100644
>> --- a/drivers/input/mouse/Kconfig
>> +++ b/drivers/input/mouse/Kconfig
>> @@ -146,6 +146,16 @@ config MOUSE_PS2_OLPC
>>
>> If unsure, say N.
>>
>> +config MOUSE_PS2_FOCALTECH
>> + bool "FocalTech PS/2 mouse protocol extension" if EXPERT
>> + default y
>> + depends on MOUSE_PS2
>> + help
>> + Say Y here if you have a FocalTech PS/2 TouchPad connected to
>> + your system.
>> +
>> + If unsure, say Y.
>> +
>> config MOUSE_SERIAL
>> tristate "Serial mouse"
>> select SERIO
>> diff --git a/drivers/input/mouse/focaltech.c
> b/drivers/input/mouse/focaltech.c
>> index f4d657e..26bc5b7 100644
>> --- a/drivers/input/mouse/focaltech.c
>> +++ b/drivers/input/mouse/focaltech.c
>> @@ -2,6 +2,7 @@
>> * Focaltech TouchPad PS/2 mouse driver
>> *
>> * Copyright (c) 2014 Red Hat Inc.
>> + * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@gmail.com>
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License as published by
>> @@ -13,15 +14,14 @@
>> * Hans de Goede <hdegoede@redhat.com>
>> */
>>
>> -/*
>> - * The Focaltech PS/2 touchpad protocol is unknown. This drivers
> deals with
>> - * detection only, to avoid further detection attempts confusing the
> touchpad
>> - * this way it at least works in PS/2 mouse compatibility mode.
>> - */
>>
>> #include <linux/device.h>
>> #include <linux/libps2.h>
>> +#include <linux/input/mt.h>
>> +#include <linux/serio.h>
>> +#include <linux/slab.h>
>> #include "psmouse.h"
>> +#include "focaltech.h"
>>
>> static const char * const focaltech_pnp_ids[] = {
>> "FLT0101",
>> @@ -30,6 +30,12 @@ static const char * const focaltech_pnp_ids[] = {
>> NULL
>> };
>>
>> +/*
>> + * Even if the kernel is built without support for Focaltech PS/2
> touchpads (or
>> + * when the real driver fails to recognize the device), we still have
> to detect
>> + * them in order to avoid further detection attempts confusing the
> touchpad.
>> + * This way it at least works in PS/2 mouse compatibility mode.
>> + */
>> int focaltech_detect(struct psmouse *psmouse, bool set_properties)
>> {
>> if (!psmouse_matches_pnp_id(psmouse, focaltech_pnp_ids))
>> @@ -37,16 +43,296 @@ int focaltech_detect(struct psmouse *psmouse,
> bool set_properties)
>>
>> if (set_properties) {
>> psmouse->vendor = "FocalTech";
>> - psmouse->name = "FocalTech Touchpad in mouse emulation mode";
>> + psmouse->name = "FocalTech Touchpad";
>> }
>>
>> return 0;
>> }
>>
>> -int focaltech_init(struct psmouse *psmouse)
>> +static void focaltech_reset(struct psmouse *psmouse)
>> {
>> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
>> psmouse_reset(psmouse);
>> +}
>> +
>> +#ifdef CONFIG_MOUSE_PS2_FOCALTECH
>> +
>> +static void focaltech_report_state(struct psmouse *psmouse)
>> +{
>> + int i;
>> + struct focaltech_data *priv = psmouse->private;
>> + struct focaltech_hw_state *state = &priv->state;
>> + struct input_dev *dev = psmouse->dev;
>> + int finger_count = 0;
>> +
>> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
>> + struct focaltech_finger_state *finger = &state->fingers[i];
>> + int active = finger->active && finger->valid;
>> + input_mt_slot(dev, i);
>> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
>> + if (active) {
>> + finger_count++;
>> + input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
>> + input_report_abs(dev, ABS_MT_POSITION_Y,
>> + focaltech_invert_y(finger->y));
>> + }
>> + }
>> + input_mt_report_pointer_emulation(dev, finger_count);
>> +
>> + input_report_key(psmouse->dev, BTN_LEFT, state->pressed);
>> + input_sync(psmouse->dev);
>> +}
>> +
>> +static void process_touch_packet(struct focaltech_hw_state *state,
>> + unsigned char *packet)
>> +{
>> + int i;
>> + unsigned char fingers = packet[1];
>> +
>> + state->pressed = (packet[0] >> 4) & 1;
>> + /* the second byte contains a bitmap of all fingers touching the
> pad */
>> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
>> + if ((fingers & 0x1) && !state->fingers[i].active) {
>> + /* we do not have a valid position for the finger yet */
>> + state->fingers[i].valid = 0;
>> + }
>> + state->fingers[i].active = fingers & 0x1;
>> + fingers >>= 1;
>> + }
>> +}
>> +
>> +static void process_abs_packet(struct focaltech_hw_state *state,
>> + unsigned char *packet)
>> +{
>> + unsigned int finger = (packet[1] >> 4) - 1;
>> +
>> + state->pressed = (packet[0] >> 4) & 1;
>> + if (finger >= FOC_MAX_FINGERS)
>> + return;
>> + /*
>> + * packet[5] contains some kind of tool size in the most significant
>> + * nibble. 0xff is a special value (latching) that signals a large
>> + * contact area.
>> + */
>> + if (packet[5] == 0xff) {
>> + state->fingers[finger].valid = 0;
>> + return;
>> + }
>> + state->fingers[finger].x = ((packet[1] & 0xf) << 8) | packet[2];
>> + state->fingers[finger].y = (packet[3] << 8) | packet[4];
>> + state->fingers[finger].valid = 1;
>> +}
>> +static void process_rel_packet(struct focaltech_hw_state *state,
>> + unsigned char *packet)
>> +{
>> + int finger1 = ((packet[0] >> 4) & 0x7) - 1;
>> + int finger2 = ((packet[3] >> 4) & 0x7) - 1;
>> +
>> + state->pressed = packet[0] >> 7;
>> + if (finger1 < FOC_MAX_FINGERS) {
>> + state->fingers[finger1].x += (char)packet[1];
>> + state->fingers[finger1].y += (char)packet[2];
>> + }
>> + /*
>> + * If there is an odd number of fingers, the last relative packet
> only
>> + * contains one finger. In this case, the second finger index in the
>> + * packet is 0 (we subtract 1 in the lines above to create array
>> + * indices).
>> + */
>> + if (finger2 != -1 && finger2 < FOC_MAX_FINGERS) {
>> + state->fingers[finger2].x += (char)packet[4];
>> + state->fingers[finger2].y += (char)packet[5];
>> + }
>> +}
>> +
>> +static void focaltech_process_packet(struct psmouse *psmouse)
>> +{
>> + struct focaltech_data *priv = psmouse->private;
>> + unsigned char *packet = psmouse->packet;
>> +
>> + switch (packet[0] & 0xf) {
>> + case FOC_TOUCH:
>> + process_touch_packet(&priv->state, packet);
>> + break;
>> + case FOC_ABS:
>> + process_abs_packet(&priv->state, packet);
>> + break;
>> + case FOC_REL:
>> + process_rel_packet(&priv->state, packet);
>> + break;
>> + default:
>> + psmouse_err(psmouse, "Unknown packet type: %02x", packet[0]);
>> + break;
>> + }
>> +
>> + focaltech_report_state(psmouse);
>> +}
>> +
>> +static psmouse_ret_t focaltech_process_byte(struct psmouse *psmouse)
>> +{
>> + if (psmouse->pktcnt >= 6) { /* Full packet received */
>> + focaltech_process_packet(psmouse);
>> + return PSMOUSE_FULL_PACKET;
>> + }
>> + /*
>> + * we might want to do some validation of the data here, but we
> do not
>> + * know the protocoll well enough
>> + */
>> + return PSMOUSE_GOOD_DATA;
>> +}
>> +
>> +static int focaltech_switch_protocol(struct psmouse *psmouse)
>> +{
>> + struct ps2dev *ps2dev = &psmouse->ps2dev;
>> + unsigned char param[3];
>> +
>> + param[0] = 0;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + param[0] = 1;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
>> + return -EIO;
>> +
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_ENABLE))
>> + return -EIO;
>>
>> return 0;
>> }
>> +
>> +static void focaltech_disconnect(struct psmouse *psmouse)
>> +{
>> + focaltech_reset(psmouse);
>> + kfree(psmouse->private);
>> + psmouse->private = NULL;
>> +}
>> +
>> +static int focaltech_reconnect(struct psmouse *psmouse)
>> +{
>> + focaltech_reset(psmouse);
>> + if (focaltech_switch_protocol(psmouse)) {
>> + psmouse_err(psmouse,
>> + "Unable to initialize the device.");
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> +static void set_input_params(struct psmouse *psmouse)
>> +{
>> + struct input_dev *dev = psmouse->dev;
>> + struct focaltech_data *priv = psmouse->private;
>> +
>> + __set_bit(EV_ABS, dev->evbit);
>> + input_set_abs_params(dev, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
>> + input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
>> + input_mt_init_slots(dev, 5, INPUT_MT_POINTER);
>> + __clear_bit(EV_REL, dev->evbit);
>> + __clear_bit(REL_X, dev->relbit);
>> + __clear_bit(REL_Y, dev->relbit);
>> + __clear_bit(BTN_RIGHT, dev->keybit);
>> + __clear_bit(BTN_MIDDLE, dev->keybit);
>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
>> +}
>> +
>> +static int focaltech_read_register(struct ps2dev *ps2dev, int reg,
>> + unsigned char *param)
>> +{
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
>> + return -1;
>> + param[0] = 0;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + param[0] = reg;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
>> + return -1;
>> + return 0;
>> +}
>> +
>> +static int focaltech_read_size(struct psmouse *psmouse)
>> +{
>> + struct ps2dev *ps2dev = &psmouse->ps2dev;
>> + struct focaltech_data *priv = psmouse->private;
>> + char param[3];
>> +
>> + if (focaltech_read_register(ps2dev, 2, param))
>> + return -EIO;
>> + /* not sure whether this is 100% correct */
>> + priv->x_max = (unsigned char)param[1] * 128;
>> + priv->y_max = (unsigned char)param[2] * 128;
>> +
>> + return 0;
>> +}
>> +int focaltech_init(struct psmouse *psmouse)
>> +{
>> + struct focaltech_data *priv;
>> + int err;
>> +
>> + psmouse->private = priv = kzalloc(sizeof(struct focaltech_data),
> GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + focaltech_reset(psmouse);
>> + if (focaltech_read_size(psmouse)) {
>> + focaltech_reset(psmouse);
>> + psmouse_err(psmouse,
>> + "Unable to read the size of the touchpad.");
>> + err = -ENOSYS;
>> + goto fail;
>> + }
>> + if (focaltech_switch_protocol(psmouse)) {
>> + focaltech_reset(psmouse);
>> + psmouse_err(psmouse,
>> + "Unable to initialize the device.");
>> + err = -ENOSYS;
>> + goto fail;
>> + }
>> +
>> + set_input_params(psmouse);
>> +
>> + psmouse->protocol_handler = focaltech_process_byte;
>> + psmouse->pktsize = 6;
>> + psmouse->disconnect = focaltech_disconnect;
>> + psmouse->reconnect = focaltech_reconnect;
>> + psmouse->cleanup = focaltech_reset;
>> + /* resync is not supported yet */
>> + psmouse->resync_time = 0;
>> +
>> + return 0;
>> +fail:
>> + focaltech_reset(psmouse);
>> + kfree(priv);
>> + return err;
>> +}
>> +
>> +bool focaltech_supported(void)
>> +{
>> + return true;
>> +}
>> +
>> +#else /* CONFIG_MOUSE_PS2_FOCALTECH */
>> +
>> +int focaltech_init(struct psmouse *psmouse)
>> +{
>> + focaltech_reset(psmouse);
>> +
>> + return 0;
>> +}
>> +
>> +bool focaltech_supported(void)
>> +{
>> + return false;
>> +}
>> +
>> +#endif /* CONFIG_MOUSE_PS2_FOCALTECH */
>> diff --git a/drivers/input/mouse/focaltech.h
> b/drivers/input/mouse/focaltech.h
>> index 498650c..68c5cfc 100644
>> --- a/drivers/input/mouse/focaltech.h
>> +++ b/drivers/input/mouse/focaltech.h
>> @@ -2,6 +2,7 @@
>> * Focaltech TouchPad PS/2 mouse driver
>> *
>> * Copyright (c) 2014 Red Hat Inc.
>> + * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@gmail.com>
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License as published by
>> @@ -16,7 +17,66 @@
>> #ifndef _FOCALTECH_H
>> #define _FOCALTECH_H
>>
>> +/*
>> + * Packet types - the numbers are not consecutive, so we might be
missing
>> + * something here.
>> + */
>> +#define FOC_TOUCH 0x3 /* bitmap of active fingers */
>> +#define FOC_ABS 0x6 /* absolute position of one finger */
>> +#define FOC_REL 0x9 /* relative position of 1-2 fingers */
>> +
>> +#define FOC_MAX_FINGERS 5
>> +
>> +#define FOC_MAX_X 2431
>> +#define FOC_MAX_Y 1663
>> +
>> +static inline int focaltech_invert_y(int y)
>> +{
>> + return FOC_MAX_Y - y;
>> +}
>> +
>> +/*
>> + * Current state of a single finger on the touchpad.
>> + */
>> +struct focaltech_finger_state {
>> + /* the touchpad has generated a touch event for the finger */
>> + bool active;
>> + /*
>> + * The touchpad has sent position data for the finger. Touch event
>> + * packages reset this flag for new fingers, and there is a time
>> + * between the first touch event and the following absolute position
>> + * packet for the finger where the touchpad has declared the
> finger to
>> + * be valid, but we do not have any valid position yet.
>> + */
>> + bool valid;
>> + /* absolute position (from the bottom left corner) of the finger */
>> + unsigned int x;
>> + unsigned int y;
>> +};
>> +
>> +/*
>> + * Description of the current state of the touchpad hardware.
>> + */
>> +struct focaltech_hw_state {
>> + /*
>> + * The touchpad tracks the positions of the fingers for us, the
array
>> + * indices correspond to the finger indices returned in the report
>> + * packages.
>> + */
>> + struct focaltech_finger_state fingers[FOC_MAX_FINGERS];
>> + /*
>> + * True if the clickpad has been pressed.
>> + */
>> + bool pressed;
>> +};
>> +
>> +struct focaltech_data {
>> + unsigned int x_max, y_max;
>> + struct focaltech_hw_state state;
>> +};
>> +
>> int focaltech_detect(struct psmouse *psmouse, bool set_properties);
>> int focaltech_init(struct psmouse *psmouse);
>> +bool focaltech_supported(void);
>>
>> #endif
>> diff --git a/drivers/input/mouse/psmouse-base.c
> b/drivers/input/mouse/psmouse-base.c
>> index 26994f6..4a9de33 100644
>> --- a/drivers/input/mouse/psmouse-base.c
>> +++ b/drivers/input/mouse/psmouse-base.c
>> @@ -725,16 +725,19 @@ static int psmouse_extensions(struct psmouse
> *psmouse,
>>
>> /* Always check for focaltech, this is safe as it uses pnp-id
matching */
>> if (psmouse_do_detect(focaltech_detect, psmouse, set_properties)
> == 0) {
>> - if (!set_properties || focaltech_init(psmouse) == 0) {
>> - /*
>> - * Not supported yet, use bare protocol.
>> - * Note that we need to also restrict
>> - * psmouse_max_proto so that psmouse_initialize()
>> - * does not try to reset rate and resolution,
>> - * because even that upsets the device.
>> - */
>> - psmouse_max_proto = PSMOUSE_PS2;
>> - return PSMOUSE_PS2;
>> + if (max_proto > PSMOUSE_IMEX) {
>> + if (!set_properties || focaltech_init(psmouse) == 0) {
>> + if (focaltech_supported())
>> + return PSMOUSE_FOCALTECH;
>> + /*
>> + * Note that we need to also restrict
>> + * psmouse_max_proto so that psmouse_initialize()
>> + * does not try to reset rate and resolution,
>> + * because even that upsets the device.
>> + */
>> + psmouse_max_proto = PSMOUSE_PS2;
>> + return PSMOUSE_PS2;
>> + }
>> }
>> }
>>
>> @@ -1063,6 +1066,15 @@ static const struct psmouse_protocol
> psmouse_protocols[] = {
>> .alias = "cortps",
>> .detect = cortron_detect,
>> },
>> +#ifdef CONFIG_MOUSE_PS2_FOCALTECH
>> + {
>> + .type = PSMOUSE_FOCALTECH,
>> + .name = "FocalTechPS/2",
>> + .alias = "focaltech",
>> + .detect = focaltech_detect,
>> + .init = focaltech_init,
>> + },
>> +#endif
>> {
>> .type = PSMOUSE_AUTO,
>> .name = "auto",
>> diff --git a/drivers/input/mouse/psmouse.h
b/drivers/input/mouse/psmouse.h
>> index f4cf664..c2ff137 100644
>> --- a/drivers/input/mouse/psmouse.h
>> +++ b/drivers/input/mouse/psmouse.h
>> @@ -96,6 +96,7 @@ enum psmouse_type {
>> PSMOUSE_FSP,
>> PSMOUSE_SYNAPTICS_RELATIVE,
>> PSMOUSE_CYPRESS,
>> + PSMOUSE_FOCALTECH,
>> PSMOUSE_AUTO /* This one should always be last */
>> };
>>
>
>
>
^ permalink raw reply
* RE: [PATCH v3 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: R, Vignesh @ 2014-11-11 15:26 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Benoit Cousson, Tony Lindgren, Russell King, Jonathan Cameron,
Hartmut Knaack, richardcochran@gmail.com, Dmitry Torokhov,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Balbi, Felipe, Griffis, Brad, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree@vger.kernel.org
In-Reply-To: <20141111122623.GA24004@x1>
On Tue, 11 Nov 2014, Vignesh R wrote:
> In one shot mode, sequencer automatically disables all enabled steps
> at the end of each cycle. (both ADC steps and TSC steps) Hence these
> steps need not be saved in reg_se_cache for clearing these steps at a
> later stage.
> Also, when ADC wakes up Sequencer should not be busy executing any of
> the config steps except for the charge step. Previously charge step
> was 1 ADC clock cycle and hence it was ignored.
>
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
You need to provide a changelog here when submitting new versions.
I have included changelog in the cover page. Will include them in
individual patches next time.
> drivers/mfd/ti_am335x_tscadc.c | 7 +++++--
> include/linux/mfd/ti_am335x_tscadc.h | 1 +
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/ti_am335x_tscadc.c
> b/drivers/mfd/ti_am335x_tscadc.c index d877e777cce6..110038859a8d
> 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -86,8 +86,12 @@ static void am335x_tscadc_need_adc(struct ti_tscadc_dev *tsadc)
> spin_lock_irq(&tsadc->reg_lock);
> finish_wait(&tsadc->reg_se_wait, &wait);
>
> + /*
> + * Sequencer should either be idle or
> + * busy applying the charge step.
> + */
> reg = tscadc_readl(tsadc, REG_ADCFSM);
> - WARN_ON(reg & SEQ_STATUS);
> + WARN_ON((reg & SEQ_STATUS) && !(reg & CHARGE_STEP));
> tsadc->adc_waiting = false;
> }
> tsadc->adc_in_use = true;
> @@ -96,7 +100,6 @@ static void am335x_tscadc_need_adc(struct
> ti_tscadc_dev *tsadc) void am335x_tsc_se_set_once(struct
> ti_tscadc_dev *tsadc, u32 val) {
> spin_lock_irq(&tsadc->reg_lock);
> - tsadc->reg_se_cache |= val;
You didn't answer my question about this?
I did reply to the question in the previous thread.
Previously, TSC did not reliably re-enable its steps as the TSC irq handler received
false pen up events. Hence, in order to use TSC after ADC operation, it was necessary to
save and re-enable TSC steps (basically, to keep TSC steps enabled always).
The change was more of a workaround to overcome limitation of TSC irq handler. With
this series of patches, TSC irq handler is very reliable and the workaround is no longer required.
> am335x_tscadc_need_adc(tsadc);
>
> tscadc_writel(tsadc, REG_SE, val);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h
> b/include/linux/mfd/ti_am335x_tscadc.h
> index c99be5dc0f5c..fcce182e4a35 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -128,6 +128,7 @@
>
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> +#define CHARGE_STEP 0x11
>
> #define ADC_CLK 3000000
> #define TOTAL_STEPS 16
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Dmitry Torokhov @ 2014-11-11 16:55 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mathias Gottschlag, linux-input
In-Reply-To: <544E1304.2060006@redhat.com>
Hi Hans,
On Mon, Oct 27, 2014 at 10:40:20AM +0100, Hans de Goede wrote:
> Hi Matthias,
>
> On 10/25/2014 02:01 PM, Mathias Gottschlag wrote:
> > +static void focaltech_report_state(struct psmouse *psmouse)
> > +{
> > + int i;
> > + struct focaltech_data *priv = psmouse->private;
> > + struct focaltech_hw_state *state = &priv->state;
> > + struct input_dev *dev = psmouse->dev;
> > + int finger_count = 0;
> > +
> > + for (i = 0; i < FOC_MAX_FINGERS; i++) {
> > + struct focaltech_finger_state *finger = &state->fingers[i];
> > + int active = finger->active && finger->valid;
> > + input_mt_slot(dev, i);
> > + input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
> > + if (active) {
> > + finger_count++;
> > + input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
> > + input_report_abs(dev, ABS_MT_POSITION_Y,
> > + focaltech_invert_y(finger->y));
> > + }
> > + }
> > + input_mt_report_pointer_emulation(dev, false);
> > + input_mt_report_finger_count(dev, finger_count);
>
> These 2 lines should be replace with:
> input_mt_report_pointer_emulation(dev, finger_count);
>
> So that BTN_TOUCH properly gets set when at least one finger is down (this is
> mandatory when not reporting pressure).
I am baffled by this suggestion. input_mt_report_pointer_emulation()
always report BTN_TOUCH, regardless of the value of the 2nd argument.
Also, the name of the 2nd argument is "use_count" and it is boolean, so
I am not sure why you are suggesting that we basically do:
if (finger_count)
input_mt_report_pointer_emulation(dev, true /*count them again */);
else
input_mt_report_pointer_emulation(dve, false);
Did you mean to tell Mathias to call
input_mt_report_pointer_emulation(dev, *true*); to instruct MT core to
count contacts and emit appropriate BTN_TOOL_* based on number of
contacts (if any)?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Dmitry Torokhov @ 2014-11-11 17:15 UTC (permalink / raw)
To: Mathias Gottschlag; +Cc: Hans de Goede, linux-input
In-Reply-To: <1414834638-16285-1-git-send-email-mgottschlag@gmail.com>
On Sat, Nov 01, 2014 at 10:37:18AM +0100, Mathias Gottschlag wrote:
> Most of the protocol for these touchpads has been reverse engineered. This
> commit adds a basic multitouch-capable driver.
>
> A lot of the protocol is still unknown. Especially, we don't know how to
> identify the device yet apart from the PNP ID.
>
> The previous workaround for these devices has been left in place in case
> the driver is not compiled into the kernel or in case some other device
> with the same PNP ID is not recognized by the driver yet still has the same
> problems with the device probing code.
>
> Signed-off-by: Mathias Gottschlag <mgottschlag@gmail.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
>
> Here is v4 of the driver, hopefully without any wrapped lines this time. I've
> received further feedback, the size detection code should be correct on Asus
> X200 and N550 as well.
>
> drivers/input/mouse/Kconfig | 10 ++
> drivers/input/mouse/focaltech.c | 300 ++++++++++++++++++++++++++++++++++++-
> drivers/input/mouse/focaltech.h | 60 ++++++++
> drivers/input/mouse/psmouse-base.c | 32 ++--
> drivers/input/mouse/psmouse.h | 1 +
> 5 files changed, 386 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
> index 366fc7a..db973e5 100644
> --- a/drivers/input/mouse/Kconfig
> +++ b/drivers/input/mouse/Kconfig
> @@ -146,6 +146,16 @@ config MOUSE_PS2_OLPC
>
> If unsure, say N.
>
> +config MOUSE_PS2_FOCALTECH
> + bool "FocalTech PS/2 mouse protocol extension" if EXPERT
> + default y
> + depends on MOUSE_PS2
> + help
> + Say Y here if you have a FocalTech PS/2 TouchPad connected to
> + your system.
> +
> + If unsure, say Y.
> +
> config MOUSE_SERIAL
> tristate "Serial mouse"
> select SERIO
> diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
> index f4d657e..26bc5b7 100644
> --- a/drivers/input/mouse/focaltech.c
> +++ b/drivers/input/mouse/focaltech.c
> @@ -2,6 +2,7 @@
> * Focaltech TouchPad PS/2 mouse driver
> *
> * Copyright (c) 2014 Red Hat Inc.
> + * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@gmail.com>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -13,15 +14,14 @@
> * Hans de Goede <hdegoede@redhat.com>
> */
>
> -/*
> - * The Focaltech PS/2 touchpad protocol is unknown. This drivers deals with
> - * detection only, to avoid further detection attempts confusing the touchpad
> - * this way it at least works in PS/2 mouse compatibility mode.
> - */
>
> #include <linux/device.h>
> #include <linux/libps2.h>
> +#include <linux/input/mt.h>
> +#include <linux/serio.h>
> +#include <linux/slab.h>
> #include "psmouse.h"
> +#include "focaltech.h"
>
> static const char * const focaltech_pnp_ids[] = {
> "FLT0101",
> @@ -30,6 +30,12 @@ static const char * const focaltech_pnp_ids[] = {
> NULL
> };
>
> +/*
> + * Even if the kernel is built without support for Focaltech PS/2 touchpads (or
> + * when the real driver fails to recognize the device), we still have to detect
> + * them in order to avoid further detection attempts confusing the touchpad.
> + * This way it at least works in PS/2 mouse compatibility mode.
> + */
> int focaltech_detect(struct psmouse *psmouse, bool set_properties)
> {
> if (!psmouse_matches_pnp_id(psmouse, focaltech_pnp_ids))
> @@ -37,16 +43,296 @@ int focaltech_detect(struct psmouse *psmouse, bool set_properties)
>
> if (set_properties) {
> psmouse->vendor = "FocalTech";
> - psmouse->name = "FocalTech Touchpad in mouse emulation mode";
> + psmouse->name = "FocalTech Touchpad";
> }
>
> return 0;
> }
>
> -int focaltech_init(struct psmouse *psmouse)
> +static void focaltech_reset(struct psmouse *psmouse)
> {
> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
> psmouse_reset(psmouse);
> +}
> +
> +#ifdef CONFIG_MOUSE_PS2_FOCALTECH
> +
> +static void focaltech_report_state(struct psmouse *psmouse)
> +{
> + int i;
> + struct focaltech_data *priv = psmouse->private;
> + struct focaltech_hw_state *state = &priv->state;
> + struct input_dev *dev = psmouse->dev;
> + int finger_count = 0;
> +
> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
> + struct focaltech_finger_state *finger = &state->fingers[i];
> + int active = finger->active && finger->valid;
bool active;
> + input_mt_slot(dev, i);
> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
> + if (active) {
> + finger_count++;
Why do we need to count contacts ourselves?
> + input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
> + input_report_abs(dev, ABS_MT_POSITION_Y,
> + focaltech_invert_y(finger->y));
> + }
> + }
> + input_mt_report_pointer_emulation(dev, finger_count);
> +
> + input_report_key(psmouse->dev, BTN_LEFT, state->pressed);
> + input_sync(psmouse->dev);
> +}
> +
> +static void process_touch_packet(struct focaltech_hw_state *state,
> + unsigned char *packet)
> +{
> + int i;
> + unsigned char fingers = packet[1];
> +
> + state->pressed = (packet[0] >> 4) & 1;
> + /* the second byte contains a bitmap of all fingers touching the pad */
> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
> + if ((fingers & 0x1) && !state->fingers[i].active) {
I think the test (fingers & 1) is not needed here. If the contact was
not active we do not have valid coordinates for it, so just checking if
!state->fingers[i].active should we enough, right?
> + /* we do not have a valid position for the finger yet */
> + state->fingers[i].valid = 0;
> + }
> + state->fingers[i].active = fingers & 0x1;
> + fingers >>= 1;
> + }
> +}
> +
> +static void process_abs_packet(struct focaltech_hw_state *state,
> + unsigned char *packet)
> +{
> + unsigned int finger = (packet[1] >> 4) - 1;
> +
> + state->pressed = (packet[0] >> 4) & 1;
> + if (finger >= FOC_MAX_FINGERS)
> + return;
Please combine checks with respective assignments:
finger = (packet[1] >> 4) - 1;
if (finger >= FOC_MAX_FINGERS) {
psmouse_err(...);
return;
}
state->pressed = (packet[0] >> 4) & 1;
...
> + /*
> + * packet[5] contains some kind of tool size in the most significant
> + * nibble. 0xff is a special value (latching) that signals a large
> + * contact area.
> + */
> + if (packet[5] == 0xff) {
> + state->fingers[finger].valid = 0;
I wonder if we should report it via ABS_TOOL_WIDTH/ABS_MT_TOUCH_MAJOR.
Can be done in a separate patch though.
> + return;
> + }
> + state->fingers[finger].x = ((packet[1] & 0xf) << 8) | packet[2];
> + state->fingers[finger].y = (packet[3] << 8) | packet[4];
> + state->fingers[finger].valid = 1;
> +}
> +static void process_rel_packet(struct focaltech_hw_state *state,
> + unsigned char *packet)
> +{
> + int finger1 = ((packet[0] >> 4) & 0x7) - 1;
> + int finger2 = ((packet[3] >> 4) & 0x7) - 1;
I think you want unsigned int here, otherwise your checks need to be
if (f >= 0 && f < FOC_MAX_FINGERS)
...
> +
> + state->pressed = packet[0] >> 7;
> + if (finger1 < FOC_MAX_FINGERS) {
> + state->fingers[finger1].x += (char)packet[1];
> + state->fingers[finger1].y += (char)packet[2];
> + }
> + /*
> + * If there is an odd number of fingers, the last relative packet only
> + * contains one finger. In this case, the second finger index in the
> + * packet is 0 (we subtract 1 in the lines above to create array
> + * indices).
> + */
> + if (finger2 != -1 && finger2 < FOC_MAX_FINGERS) {
> + state->fingers[finger2].x += (char)packet[4];
> + state->fingers[finger2].y += (char)packet[5];
> + }
> +}
> +
> +static void focaltech_process_packet(struct psmouse *psmouse)
> +{
> + struct focaltech_data *priv = psmouse->private;
> + unsigned char *packet = psmouse->packet;
> +
> + switch (packet[0] & 0xf) {
> + case FOC_TOUCH:
> + process_touch_packet(&priv->state, packet);
> + break;
> + case FOC_ABS:
> + process_abs_packet(&priv->state, packet);
> + break;
> + case FOC_REL:
> + process_rel_packet(&priv->state, packet);
> + break;
> + default:
> + psmouse_err(psmouse, "Unknown packet type: %02x", packet[0]);
> + break;
> + }
> +
> + focaltech_report_state(psmouse);
> +}
> +
> +static psmouse_ret_t focaltech_process_byte(struct psmouse *psmouse)
> +{
> + if (psmouse->pktcnt >= 6) { /* Full packet received */
> + focaltech_process_packet(psmouse);
> + return PSMOUSE_FULL_PACKET;
> + }
> + /*
> + * we might want to do some validation of the data here, but we do not
> + * know the protocoll well enough
> + */
> + return PSMOUSE_GOOD_DATA;
> +}
> +
> +static int focaltech_switch_protocol(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + unsigned char param[3];
> +
> + param[0] = 0;
> + if (ps2_command(ps2dev, param, 0x10f8))
> + return -EIO;
> + if (ps2_command(ps2dev, param, 0x10f8))
> + return -EIO;
> + if (ps2_command(ps2dev, param, 0x10f8))
> + return -EIO;
> + param[0] = 1;
> + if (ps2_command(ps2dev, param, 0x10f8))
> + return -EIO;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
> + return -EIO;
> +
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_ENABLE))
> + return -EIO;
>
> return 0;
> }
> +
> +static void focaltech_disconnect(struct psmouse *psmouse)
> +{
> + focaltech_reset(psmouse);
> + kfree(psmouse->private);
> + psmouse->private = NULL;
> +}
> +
> +static int focaltech_reconnect(struct psmouse *psmouse)
> +{
> + focaltech_reset(psmouse);
> + if (focaltech_switch_protocol(psmouse)) {
> + psmouse_err(psmouse,
> + "Unable to initialize the device.");
> + return -1;
> + }
> + return 0;
> +}
> +
> +static void set_input_params(struct psmouse *psmouse)
> +{
> + struct input_dev *dev = psmouse->dev;
> + struct focaltech_data *priv = psmouse->private;
> +
> + __set_bit(EV_ABS, dev->evbit);
> + input_set_abs_params(dev, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
> + input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
> + input_mt_init_slots(dev, 5, INPUT_MT_POINTER);
> + __clear_bit(EV_REL, dev->evbit);
> + __clear_bit(REL_X, dev->relbit);
> + __clear_bit(REL_Y, dev->relbit);
> + __clear_bit(BTN_RIGHT, dev->keybit);
> + __clear_bit(BTN_MIDDLE, dev->keybit);
> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
> +}
> +
> +static int focaltech_read_register(struct ps2dev *ps2dev, int reg,
> + unsigned char *param)
> +{
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
> + return -1;
I 'd rather see -EIO here as well.
> + param[0] = 0;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
> + return -1;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
> + return -1;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
> + return -1;
> + param[0] = reg;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
> + return -1;
> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
> + return -1;
> + return 0;
> +}
> +
> +static int focaltech_read_size(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + struct focaltech_data *priv = psmouse->private;
> + char param[3];
> +
> + if (focaltech_read_register(ps2dev, 2, param))
> + return -EIO;
> + /* not sure whether this is 100% correct */
> + priv->x_max = (unsigned char)param[1] * 128;
> + priv->y_max = (unsigned char)param[2] * 128;
> +
> + return 0;
> +}
> +int focaltech_init(struct psmouse *psmouse)
> +{
> + struct focaltech_data *priv;
> + int err;
> +
> + psmouse->private = priv = kzalloc(sizeof(struct focaltech_data), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + focaltech_reset(psmouse);
> + if (focaltech_read_size(psmouse)) {
> + focaltech_reset(psmouse);
> + psmouse_err(psmouse,
> + "Unable to read the size of the touchpad.");
> + err = -ENOSYS;
Why -ENOSYS?
error = focaltech_read_size(psmouse);
if (error) {
...
return error;
}
The fact that the rest of psmouse likes returning -1 does not mean that
the new code should be similarly ugly.
> + goto fail;
> + }
> + if (focaltech_switch_protocol(psmouse)) {
> + focaltech_reset(psmouse);
> + psmouse_err(psmouse,
> + "Unable to initialize the device.");
> + err = -ENOSYS;
> + goto fail;
> + }
> +
> + set_input_params(psmouse);
> +
> + psmouse->protocol_handler = focaltech_process_byte;
> + psmouse->pktsize = 6;
> + psmouse->disconnect = focaltech_disconnect;
> + psmouse->reconnect = focaltech_reconnect;
> + psmouse->cleanup = focaltech_reset;
> + /* resync is not supported yet */
> + psmouse->resync_time = 0;
> +
> + return 0;
> +fail:
> + focaltech_reset(psmouse);
> + kfree(priv);
> + return err;
> +}
> +
> +bool focaltech_supported(void)
> +{
> + return true;
> +}
> +
> +#else /* CONFIG_MOUSE_PS2_FOCALTECH */
> +
> +int focaltech_init(struct psmouse *psmouse)
> +{
> + focaltech_reset(psmouse);
> +
> + return 0;
> +}
> +
> +bool focaltech_supported(void)
> +{
> + return false;
> +}
> +
> +#endif /* CONFIG_MOUSE_PS2_FOCALTECH */
> diff --git a/drivers/input/mouse/focaltech.h b/drivers/input/mouse/focaltech.h
> index 498650c..68c5cfc 100644
> --- a/drivers/input/mouse/focaltech.h
> +++ b/drivers/input/mouse/focaltech.h
> @@ -2,6 +2,7 @@
> * Focaltech TouchPad PS/2 mouse driver
> *
> * Copyright (c) 2014 Red Hat Inc.
> + * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@gmail.com>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -16,7 +17,66 @@
> #ifndef _FOCALTECH_H
> #define _FOCALTECH_H
>
> +/*
> + * Packet types - the numbers are not consecutive, so we might be missing
> + * something here.
> + */
> +#define FOC_TOUCH 0x3 /* bitmap of active fingers */
> +#define FOC_ABS 0x6 /* absolute position of one finger */
> +#define FOC_REL 0x9 /* relative position of 1-2 fingers */
> +
> +#define FOC_MAX_FINGERS 5
> +
> +#define FOC_MAX_X 2431
> +#define FOC_MAX_Y 1663
> +
> +static inline int focaltech_invert_y(int y)
> +{
> + return FOC_MAX_Y - y;
> +}
> +
> +/*
> + * Current state of a single finger on the touchpad.
> + */
> +struct focaltech_finger_state {
> + /* the touchpad has generated a touch event for the finger */
> + bool active;
> + /*
> + * The touchpad has sent position data for the finger. Touch event
> + * packages reset this flag for new fingers, and there is a time
> + * between the first touch event and the following absolute position
> + * packet for the finger where the touchpad has declared the finger to
> + * be valid, but we do not have any valid position yet.
> + */
> + bool valid;
> + /* absolute position (from the bottom left corner) of the finger */
> + unsigned int x;
> + unsigned int y;
> +};
> +
> +/*
> + * Description of the current state of the touchpad hardware.
> + */
> +struct focaltech_hw_state {
> + /*
> + * The touchpad tracks the positions of the fingers for us, the array
> + * indices correspond to the finger indices returned in the report
> + * packages.
> + */
> + struct focaltech_finger_state fingers[FOC_MAX_FINGERS];
> + /*
> + * True if the clickpad has been pressed.
> + */
> + bool pressed;
> +};
> +
> +struct focaltech_data {
> + unsigned int x_max, y_max;
> + struct focaltech_hw_state state;
> +};
> +
> int focaltech_detect(struct psmouse *psmouse, bool set_properties);
> int focaltech_init(struct psmouse *psmouse);
> +bool focaltech_supported(void);
>
> #endif
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 26994f6..4a9de33 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -725,16 +725,19 @@ static int psmouse_extensions(struct psmouse *psmouse,
>
> /* Always check for focaltech, this is safe as it uses pnp-id matching */
> if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
> - if (!set_properties || focaltech_init(psmouse) == 0) {
> - /*
> - * Not supported yet, use bare protocol.
> - * Note that we need to also restrict
> - * psmouse_max_proto so that psmouse_initialize()
> - * does not try to reset rate and resolution,
> - * because even that upsets the device.
> - */
> - psmouse_max_proto = PSMOUSE_PS2;
> - return PSMOUSE_PS2;
> + if (max_proto > PSMOUSE_IMEX) {
> + if (!set_properties || focaltech_init(psmouse) == 0) {
> + if (focaltech_supported())
> + return PSMOUSE_FOCALTECH;
> + /*
> + * Note that we need to also restrict
> + * psmouse_max_proto so that psmouse_initialize()
> + * does not try to reset rate and resolution,
> + * because even that upsets the device.
> + */
> + psmouse_max_proto = PSMOUSE_PS2;
> + return PSMOUSE_PS2;
> + }
> }
> }
>
> @@ -1063,6 +1066,15 @@ static const struct psmouse_protocol psmouse_protocols[] = {
> .alias = "cortps",
> .detect = cortron_detect,
> },
> +#ifdef CONFIG_MOUSE_PS2_FOCALTECH
> + {
> + .type = PSMOUSE_FOCALTECH,
> + .name = "FocalTechPS/2",
> + .alias = "focaltech",
> + .detect = focaltech_detect,
> + .init = focaltech_init,
> + },
> +#endif
> {
> .type = PSMOUSE_AUTO,
> .name = "auto",
> diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
> index f4cf664..c2ff137 100644
> --- a/drivers/input/mouse/psmouse.h
> +++ b/drivers/input/mouse/psmouse.h
> @@ -96,6 +96,7 @@ enum psmouse_type {
> PSMOUSE_FSP,
> PSMOUSE_SYNAPTICS_RELATIVE,
> PSMOUSE_CYPRESS,
> + PSMOUSE_FOCALTECH,
> PSMOUSE_AUTO /* This one should always be last */
> };
>
> --
> 1.9.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] hid: sony: Use kernel allocated buffers for HID reports
From: Frank Praznik @ 2014-11-11 18:01 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dmitry.torokhov, hector, Frank Praznik
Replace stack buffers with kernel allocated buffers when sending
and receiving HID reports to prevent issues with DMA transfers
on certain hardware.
Output report buffers are allocated at initialization time to avoid
excessive kmalloc and kfree calls.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
I'm CC-ing the original bug reporter to see if he can test this patch
on whatever platform was giving him trouble as I can't replicate the
reported error on my end.
drivers/hid/hid-sony.c | 135 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 105 insertions(+), 30 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index bc4269e..28f2849 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -798,6 +798,9 @@ union sixaxis_output_report_01 {
__u8 buf[36];
};
+#define DS4_REPORT5_SIZE 32
+#define DS4_REPORT17_SIZE 78
+
static spinlock_t sony_dev_list_lock;
static LIST_HEAD(sony_device_list);
static DEFINE_IDA(sony_device_id_allocator);
@@ -811,6 +814,7 @@ struct sony_sc {
struct work_struct state_worker;
struct power_supply battery;
int device_id;
+ __u8 *output_report_dmabuf;
#ifdef CONFIG_SONY_FF
__u8 left;
@@ -1142,9 +1146,19 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
static int sixaxis_set_operational_bt(struct hid_device *hdev)
{
- unsigned char buf[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
- return hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
+ static const __u8 report[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
+ __u8 *buf = kmemdup(report, sizeof(report), GFP_KERNEL);
+ int ret;
+
+ if (!buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+
+ kfree(buf);
+
+ return ret;
}
/*
@@ -1153,10 +1167,18 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
*/
static int dualshock4_set_operational_bt(struct hid_device *hdev)
{
- __u8 buf[37] = { 0 };
+ __u8 *buf = kmalloc(37, GFP_KERNEL);
+ int ret;
- return hid_hw_raw_request(hdev, 0x02, buf, sizeof(buf),
- HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, 0x02, buf, 37, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
+
+ kfree(buf);
+
+ return ret;
}
static void sixaxis_set_leds_from_id(int id, __u8 values[MAX_LEDS])
@@ -1471,9 +1493,7 @@ error_leds:
static void sixaxis_state_worker(struct work_struct *work)
{
- struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
- int n;
- union sixaxis_output_report_01 report = {
+ static const union sixaxis_output_report_01 default_report = {
.buf = {
0x01,
0x00, 0xff, 0x00, 0xff, 0x00,
@@ -1485,20 +1505,27 @@ static void sixaxis_state_worker(struct work_struct *work)
0x00, 0x00, 0x00, 0x00, 0x00
}
};
+ struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
+ struct sixaxis_output_report *report =
+ (struct sixaxis_output_report *)sc->output_report_dmabuf;
+ int n;
+
+ /* Initialize the report with default values */
+ memcpy(report, &default_report, sizeof(struct sixaxis_output_report));
#ifdef CONFIG_SONY_FF
- report.data.rumble.right_motor_on = sc->right ? 1 : 0;
- report.data.rumble.left_motor_force = sc->left;
+ report->rumble.right_motor_on = sc->right ? 1 : 0;
+ report->rumble.left_motor_force = sc->left;
#endif
- report.data.leds_bitmap |= sc->led_state[0] << 1;
- report.data.leds_bitmap |= sc->led_state[1] << 2;
- report.data.leds_bitmap |= sc->led_state[2] << 3;
- report.data.leds_bitmap |= sc->led_state[3] << 4;
+ report->leds_bitmap |= sc->led_state[0] << 1;
+ report->leds_bitmap |= sc->led_state[1] << 2;
+ report->leds_bitmap |= sc->led_state[2] << 3;
+ report->leds_bitmap |= sc->led_state[3] << 4;
/* Set flag for all leds off, required for 3rd party INTEC controller */
- if ((report.data.leds_bitmap & 0x1E) == 0)
- report.data.leds_bitmap |= 0x20;
+ if ((report->leds_bitmap & 0x1E) == 0)
+ report->leds_bitmap |= 0x20;
/*
* The LEDs in the report are indexed in reverse order to their
@@ -1511,13 +1538,14 @@ static void sixaxis_state_worker(struct work_struct *work)
*/
for (n = 0; n < 4; n++) {
if (sc->led_delay_on[n] || sc->led_delay_off[n]) {
- report.data.led[3 - n].duty_off = sc->led_delay_off[n];
- report.data.led[3 - n].duty_on = sc->led_delay_on[n];
+ report->led[3 - n].duty_off = sc->led_delay_off[n];
+ report->led[3 - n].duty_on = sc->led_delay_on[n];
}
}
- hid_hw_raw_request(sc->hdev, report.data.report_id, report.buf,
- sizeof(report), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ hid_hw_raw_request(sc->hdev, report->report_id, (__u8 *)report,
+ sizeof(struct sixaxis_output_report),
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
static void dualshock4_state_worker(struct work_struct *work)
@@ -1526,13 +1554,15 @@ static void dualshock4_state_worker(struct work_struct *work)
struct hid_device *hdev = sc->hdev;
int offset;
- __u8 buf[78] = { 0 };
+ __u8 *buf = sc->output_report_dmabuf;
if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ memset(buf, 0, DS4_REPORT5_SIZE);
buf[0] = 0x05;
buf[1] = 0xFF;
offset = 4;
} else {
+ memset(buf, 0, DS4_REPORT17_SIZE);
buf[0] = 0x11;
buf[1] = 0xB0;
buf[3] = 0x0F;
@@ -1560,12 +1590,33 @@ static void dualshock4_state_worker(struct work_struct *work)
buf[offset++] = sc->led_delay_off[3];
if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
- hid_hw_output_report(hdev, buf, 32);
+ hid_hw_output_report(hdev, buf, DS4_REPORT5_SIZE);
else
- hid_hw_raw_request(hdev, 0x11, buf, 78,
+ hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT17_SIZE,
HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
+static int sony_allocate_output_report(struct sony_sc *sc)
+{
+ if (sc->quirks & SIXAXIS_CONTROLLER)
+ sc->output_report_dmabuf =
+ kmalloc(sizeof(union sixaxis_output_report_01),
+ GFP_KERNEL);
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT)
+ sc->output_report_dmabuf = kmalloc(DS4_REPORT17_SIZE,
+ GFP_KERNEL);
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
+ sc->output_report_dmabuf = kmalloc(DS4_REPORT5_SIZE,
+ GFP_KERNEL);
+ else
+ return 0;
+
+ if (!sc->output_report_dmabuf)
+ return -ENOMEM;
+
+ return 0;
+}
+
#ifdef CONFIG_SONY_FF
static int sony_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
@@ -1754,6 +1805,7 @@ static int sony_get_bt_devaddr(struct sony_sc *sc)
static int sony_check_add(struct sony_sc *sc)
{
+ __u8 *buf = NULL;
int n, ret;
if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) ||
@@ -1769,36 +1821,44 @@ static int sony_check_add(struct sony_sc *sc)
return 0;
}
} else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
- __u8 buf[7];
+ buf = kmalloc(7, GFP_KERNEL);
+
+ if (!buf)
+ return -ENOMEM;
/*
* The MAC address of a DS4 controller connected via USB can be
* retrieved with feature report 0x81. The address begins at
* offset 1.
*/
- ret = hid_hw_raw_request(sc->hdev, 0x81, buf, sizeof(buf),
+ ret = hid_hw_raw_request(sc->hdev, 0x81, buf, 7,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
if (ret != 7) {
hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n");
- return ret < 0 ? ret : -EINVAL;
+ ret = ret < 0 ? ret : -EINVAL;
+ goto out_free;
}
memcpy(sc->mac_address, &buf[1], sizeof(sc->mac_address));
} else if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
- __u8 buf[18];
+ buf = kmalloc(18, GFP_KERNEL);
+
+ if (!buf)
+ return -ENOMEM;
/*
* The MAC address of a Sixaxis controller connected via USB can
* be retrieved with feature report 0xf2. The address begins at
* offset 4.
*/
- ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, sizeof(buf),
+ ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, 18,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
if (ret != 18) {
hid_err(sc->hdev, "failed to retrieve feature report 0xf2 with the Sixaxis MAC address\n");
- return ret < 0 ? ret : -EINVAL;
+ ret = ret < 0 ? ret : -EINVAL;
+ goto out_free;
}
/*
@@ -1811,7 +1871,13 @@ static int sony_check_add(struct sony_sc *sc)
return 0;
}
- return sony_check_add_dev_list(sc);
+ ret = sony_check_add_dev_list(sc);
+
+out_free:
+
+ kfree(buf);
+
+ return ret;
}
static int sony_set_device_id(struct sony_sc *sc)
@@ -1895,6 +1961,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ ret = sony_allocate_output_report(sc);
+ if (ret < 0) {
+ hid_err(hdev, "failed to allocate the output report buffer\n");
+ goto err_stop;
+ }
+
ret = sony_set_device_id(sc);
if (ret < 0) {
hid_err(hdev, "failed to allocate the device id\n");
@@ -1984,6 +2056,7 @@ err_stop:
if (sc->quirks & SONY_BATTERY_SUPPORT)
sony_battery_remove(sc);
sony_cancel_work_sync(sc);
+ kfree(sc->output_report_dmabuf);
sony_remove_dev_list(sc);
sony_release_device_id(sc);
hid_hw_stop(hdev);
@@ -2004,6 +2077,8 @@ static void sony_remove(struct hid_device *hdev)
sony_cancel_work_sync(sc);
+ kfree(sc->output_report_dmabuf);
+
sony_remove_dev_list(sc);
sony_release_device_id(sc);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] hid: sony: Use kernel allocated buffers for HID reports
From: Hector Martin @ 2014-11-11 18:20 UTC (permalink / raw)
To: Frank Praznik, linux-input; +Cc: jkosina, dmitry.torokhov
In-Reply-To: <1415728913-2679-1-git-send-email-frank.praznik@oh.rr.com>
On 12/11/14 03:01, Frank Praznik wrote:
> Replace stack buffers with kernel allocated buffers when sending
> and receiving HID reports to prevent issues with DMA transfers
> on certain hardware.
>
> Output report buffers are allocated at initialization time to avoid
> excessive kmalloc and kfree calls.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>
> I'm CC-ing the original bug reporter to see if he can test this patch
> on whatever platform was giving him trouble as I can't replicate the
> reported error on my end.
Seems to work fine with the patch. Thanks!
FWIW, this is 3.17.1-hardened (I wonder if the grsecurity patches have
anything to do with it... maybe they do something to the stack) on an
Intel 7 Series (Panther Point) PCH's EHCI controller (with a couple of
hubs along the way).
--
Hector Martin (hector@marcansoft.com)
Public Key: http://www.marcansoft.com/marcan.asc
^ permalink raw reply
* Re: [PATCH] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Hans de Goede @ 2014-11-11 18:58 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mathias Gottschlag, linux-input
In-Reply-To: <20141111165553.GA27720@dtor-ws>
Hi,
On 11/11/2014 05:55 PM, Dmitry Torokhov wrote:
> Hi Hans,
>
> On Mon, Oct 27, 2014 at 10:40:20AM +0100, Hans de Goede wrote:
>> Hi Matthias,
>>
>> On 10/25/2014 02:01 PM, Mathias Gottschlag wrote:
>>> +static void focaltech_report_state(struct psmouse *psmouse)
>>> +{
>>> + int i;
>>> + struct focaltech_data *priv = psmouse->private;
>>> + struct focaltech_hw_state *state = &priv->state;
>>> + struct input_dev *dev = psmouse->dev;
>>> + int finger_count = 0;
>>> +
>>> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
>>> + struct focaltech_finger_state *finger = &state->fingers[i];
>>> + int active = finger->active && finger->valid;
>>> + input_mt_slot(dev, i);
>>> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
>>> + if (active) {
>>> + finger_count++;
>>> + input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
>>> + input_report_abs(dev, ABS_MT_POSITION_Y,
>>> + focaltech_invert_y(finger->y));
>>> + }
>>> + }
>>> + input_mt_report_pointer_emulation(dev, false);
>>> + input_mt_report_finger_count(dev, finger_count);
>>
>> These 2 lines should be replace with:
>> input_mt_report_pointer_emulation(dev, finger_count);
>>
>> So that BTN_TOUCH properly gets set when at least one finger is down (this is
>> mandatory when not reporting pressure).
>
> I am baffled by this suggestion. input_mt_report_pointer_emulation()
> always report BTN_TOUCH, regardless of the value of the 2nd argument.
My bad, I misread the code (I think I mixed up the use_count and count vars).
> Also, the name of the 2nd argument is "use_count" and it is boolean, so
> I am not sure why you are suggesting that we basically do:
>
> if (finger_count)
> input_mt_report_pointer_emulation(dev, true /*count them again */);
> else
> input_mt_report_pointer_emulation(dve, false);
>
> Did you mean to tell Mathias to call
> input_mt_report_pointer_emulation(dev, *true*); to instruct MT core to
> count contacts and emit appropriate BTN_TOOL_* based on number of
> contacts (if any)?
Right, rereading things, their is no need for the focaltech code to do
its own finger counting, and these 2 calls:
input_mt_report_pointer_emulation(dev, false);
input_mt_report_finger_count(dev, finger_count);
Should be replaced with:
input_mt_report_pointer_emulation(dev, true);
Sorry for the confusion.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] hid: sony: Use kernel allocated buffers for HID reports
From: Dmitry Torokhov @ 2014-11-11 19:04 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, jkosina, hector
In-Reply-To: <1415728913-2679-1-git-send-email-frank.praznik@oh.rr.com>
Hi Frank,
On Tue, Nov 11, 2014 at 01:01:53PM -0500, Frank Praznik wrote:
> Replace stack buffers with kernel allocated buffers when sending
> and receiving HID reports to prevent issues with DMA transfers
> on certain hardware.
>
> Output report buffers are allocated at initialization time to avoid
> excessive kmalloc and kfree calls.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>
> I'm CC-ing the original bug reporter to see if he can test this patch
> on whatever platform was giving him trouble as I can't replicate the
> reported error on my end.
>
> drivers/hid/hid-sony.c | 135 ++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 105 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index bc4269e..28f2849 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -798,6 +798,9 @@ union sixaxis_output_report_01 {
> __u8 buf[36];
> };
>
> +#define DS4_REPORT5_SIZE 32
> +#define DS4_REPORT17_SIZE 78
> +
> static spinlock_t sony_dev_list_lock;
> static LIST_HEAD(sony_device_list);
> static DEFINE_IDA(sony_device_id_allocator);
> @@ -811,6 +814,7 @@ struct sony_sc {
> struct work_struct state_worker;
> struct power_supply battery;
> int device_id;
> + __u8 *output_report_dmabuf;
Just to confirm as I haven't looked at the entire driver: there is no
possibility of 2 requests being submitted at the same time so that one
will overwrite other's data?
>
> #ifdef CONFIG_SONY_FF
> __u8 left;
> @@ -1142,9 +1146,19 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
>
> static int sixaxis_set_operational_bt(struct hid_device *hdev)
> {
> - unsigned char buf[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
> - return hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
> + static const __u8 report[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
> + __u8 *buf = kmemdup(report, sizeof(report), GFP_KERNEL);
> + int ret;
> +
> + if (!buf)
Please keep allocation and check together, i.e.
buf = kmemdup(report, sizeof(report), GFP_KERNEL);
if (!buf)
...
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
sizeof(buf) is either 4 or 8, not 5!
> HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> +
> + kfree(buf);
> +
> + return ret;
> }
>
> /*
> @@ -1153,10 +1167,18 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
> */
> static int dualshock4_set_operational_bt(struct hid_device *hdev)
> {
> - __u8 buf[37] = { 0 };
> + __u8 *buf = kmalloc(37, GFP_KERNEL);
No magic constants please.
> + int ret;
>
> - return hid_hw_raw_request(hdev, 0x02, buf, sizeof(buf),
> - HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = hid_hw_raw_request(hdev, 0x02, buf, 37, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> +
> + kfree(buf);
> +
> + return ret;
> }
>
> static void sixaxis_set_leds_from_id(int id, __u8 values[MAX_LEDS])
> @@ -1471,9 +1493,7 @@ error_leds:
>
> static void sixaxis_state_worker(struct work_struct *work)
> {
> - struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
> - int n;
> - union sixaxis_output_report_01 report = {
> + static const union sixaxis_output_report_01 default_report = {
> .buf = {
> 0x01,
> 0x00, 0xff, 0x00, 0xff, 0x00,
> @@ -1485,20 +1505,27 @@ static void sixaxis_state_worker(struct work_struct *work)
> 0x00, 0x00, 0x00, 0x00, 0x00
> }
> };
> + struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
> + struct sixaxis_output_report *report =
> + (struct sixaxis_output_report *)sc->output_report_dmabuf;
> + int n;
> +
> + /* Initialize the report with default values */
> + memcpy(report, &default_report, sizeof(struct sixaxis_output_report));
>
> #ifdef CONFIG_SONY_FF
> - report.data.rumble.right_motor_on = sc->right ? 1 : 0;
> - report.data.rumble.left_motor_force = sc->left;
> + report->rumble.right_motor_on = sc->right ? 1 : 0;
> + report->rumble.left_motor_force = sc->left;
> #endif
>
> - report.data.leds_bitmap |= sc->led_state[0] << 1;
> - report.data.leds_bitmap |= sc->led_state[1] << 2;
> - report.data.leds_bitmap |= sc->led_state[2] << 3;
> - report.data.leds_bitmap |= sc->led_state[3] << 4;
> + report->leds_bitmap |= sc->led_state[0] << 1;
> + report->leds_bitmap |= sc->led_state[1] << 2;
> + report->leds_bitmap |= sc->led_state[2] << 3;
> + report->leds_bitmap |= sc->led_state[3] << 4;
>
> /* Set flag for all leds off, required for 3rd party INTEC controller */
> - if ((report.data.leds_bitmap & 0x1E) == 0)
> - report.data.leds_bitmap |= 0x20;
> + if ((report->leds_bitmap & 0x1E) == 0)
> + report->leds_bitmap |= 0x20;
>
> /*
> * The LEDs in the report are indexed in reverse order to their
> @@ -1511,13 +1538,14 @@ static void sixaxis_state_worker(struct work_struct *work)
> */
> for (n = 0; n < 4; n++) {
> if (sc->led_delay_on[n] || sc->led_delay_off[n]) {
> - report.data.led[3 - n].duty_off = sc->led_delay_off[n];
> - report.data.led[3 - n].duty_on = sc->led_delay_on[n];
> + report->led[3 - n].duty_off = sc->led_delay_off[n];
> + report->led[3 - n].duty_on = sc->led_delay_on[n];
> }
> }
>
> - hid_hw_raw_request(sc->hdev, report.data.report_id, report.buf,
> - sizeof(report), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> + hid_hw_raw_request(sc->hdev, report->report_id, (__u8 *)report,
> + sizeof(struct sixaxis_output_report),
> + HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> }
>
> static void dualshock4_state_worker(struct work_struct *work)
> @@ -1526,13 +1554,15 @@ static void dualshock4_state_worker(struct work_struct *work)
> struct hid_device *hdev = sc->hdev;
> int offset;
>
> - __u8 buf[78] = { 0 };
> + __u8 *buf = sc->output_report_dmabuf;
>
> if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
> + memset(buf, 0, DS4_REPORT5_SIZE);
> buf[0] = 0x05;
> buf[1] = 0xFF;
> offset = 4;
> } else {
> + memset(buf, 0, DS4_REPORT17_SIZE);
> buf[0] = 0x11;
> buf[1] = 0xB0;
> buf[3] = 0x0F;
> @@ -1560,12 +1590,33 @@ static void dualshock4_state_worker(struct work_struct *work)
> buf[offset++] = sc->led_delay_off[3];
>
> if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
> - hid_hw_output_report(hdev, buf, 32);
> + hid_hw_output_report(hdev, buf, DS4_REPORT5_SIZE);
> else
> - hid_hw_raw_request(hdev, 0x11, buf, 78,
> + hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT17_SIZE,
> HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> }
>
> +static int sony_allocate_output_report(struct sony_sc *sc)
> +{
> + if (sc->quirks & SIXAXIS_CONTROLLER)
> + sc->output_report_dmabuf =
> + kmalloc(sizeof(union sixaxis_output_report_01),
> + GFP_KERNEL);
> + else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT)
> + sc->output_report_dmabuf = kmalloc(DS4_REPORT17_SIZE,
> + GFP_KERNEL);
> + else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
> + sc->output_report_dmabuf = kmalloc(DS4_REPORT5_SIZE,
> + GFP_KERNEL);
> + else
> + return 0;
> +
> + if (!sc->output_report_dmabuf)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_SONY_FF
> static int sony_play_effect(struct input_dev *dev, void *data,
> struct ff_effect *effect)
> @@ -1754,6 +1805,7 @@ static int sony_get_bt_devaddr(struct sony_sc *sc)
>
> static int sony_check_add(struct sony_sc *sc)
> {
> + __u8 *buf = NULL;
> int n, ret;
>
> if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) ||
> @@ -1769,36 +1821,44 @@ static int sony_check_add(struct sony_sc *sc)
> return 0;
> }
> } else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
> - __u8 buf[7];
> + buf = kmalloc(7, GFP_KERNEL);
> +
> + if (!buf)
> + return -ENOMEM;
>
> /*
> * The MAC address of a DS4 controller connected via USB can be
> * retrieved with feature report 0x81. The address begins at
> * offset 1.
> */
> - ret = hid_hw_raw_request(sc->hdev, 0x81, buf, sizeof(buf),
> + ret = hid_hw_raw_request(sc->hdev, 0x81, buf, 7,
> HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
>
> if (ret != 7) {
> hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n");
> - return ret < 0 ? ret : -EINVAL;
> + ret = ret < 0 ? ret : -EINVAL;
> + goto out_free;
> }
>
> memcpy(sc->mac_address, &buf[1], sizeof(sc->mac_address));
> } else if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
> - __u8 buf[18];
> + buf = kmalloc(18, GFP_KERNEL);
> +
> + if (!buf)
> + return -ENOMEM;
>
> /*
> * The MAC address of a Sixaxis controller connected via USB can
> * be retrieved with feature report 0xf2. The address begins at
> * offset 4.
> */
> - ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, sizeof(buf),
> + ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, 18,
> HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
>
> if (ret != 18) {
> hid_err(sc->hdev, "failed to retrieve feature report 0xf2 with the Sixaxis MAC address\n");
> - return ret < 0 ? ret : -EINVAL;
> + ret = ret < 0 ? ret : -EINVAL;
> + goto out_free;
> }
>
> /*
> @@ -1811,7 +1871,13 @@ static int sony_check_add(struct sony_sc *sc)
> return 0;
> }
>
> - return sony_check_add_dev_list(sc);
> + ret = sony_check_add_dev_list(sc);
> +
> +out_free:
> +
> + kfree(buf);
> +
> + return ret;
> }
>
> static int sony_set_device_id(struct sony_sc *sc)
> @@ -1895,6 +1961,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
> }
>
> + ret = sony_allocate_output_report(sc);
> + if (ret < 0) {
> + hid_err(hdev, "failed to allocate the output report buffer\n");
> + goto err_stop;
> + }
> +
> ret = sony_set_device_id(sc);
> if (ret < 0) {
> hid_err(hdev, "failed to allocate the device id\n");
> @@ -1984,6 +2056,7 @@ err_stop:
> if (sc->quirks & SONY_BATTERY_SUPPORT)
> sony_battery_remove(sc);
> sony_cancel_work_sync(sc);
> + kfree(sc->output_report_dmabuf);
> sony_remove_dev_list(sc);
> sony_release_device_id(sc);
> hid_hw_stop(hdev);
> @@ -2004,6 +2077,8 @@ static void sony_remove(struct hid_device *hdev)
>
> sony_cancel_work_sync(sc);
>
> + kfree(sc->output_report_dmabuf);
> +
> sony_remove_dev_list(sc);
>
> sony_release_device_id(sc);
> --
> 2.1.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Hans de Goede @ 2014-11-11 19:06 UTC (permalink / raw)
To: Dmitry Torokhov, Mathias Gottschlag; +Cc: linux-input
In-Reply-To: <20141111171554.GB27720@dtor-ws>
Hi,
On 11/11/2014 06:15 PM, Dmitry Torokhov wrote:
> On Sat, Nov 01, 2014 at 10:37:18AM +0100, Mathias Gottschlag wrote:
>> Most of the protocol for these touchpads has been reverse engineered. This
>> commit adds a basic multitouch-capable driver.
>>
>> A lot of the protocol is still unknown. Especially, we don't know how to
>> identify the device yet apart from the PNP ID.
>>
>> The previous workaround for these devices has been left in place in case
>> the driver is not compiled into the kernel or in case some other device
>> with the same PNP ID is not recognized by the driver yet still has the same
>> problems with the device probing code.
>>
>> Signed-off-by: Mathias Gottschlag <mgottschlag@gmail.com>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>
>> Here is v4 of the driver, hopefully without any wrapped lines this time. I've
>> received further feedback, the size detection code should be correct on Asus
>> X200 and N550 as well.
>>
>> drivers/input/mouse/Kconfig | 10 ++
>> drivers/input/mouse/focaltech.c | 300 ++++++++++++++++++++++++++++++++++++-
>> drivers/input/mouse/focaltech.h | 60 ++++++++
>> drivers/input/mouse/psmouse-base.c | 32 ++--
>> drivers/input/mouse/psmouse.h | 1 +
>> 5 files changed, 386 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
>> index 366fc7a..db973e5 100644
>> --- a/drivers/input/mouse/Kconfig
>> +++ b/drivers/input/mouse/Kconfig
>> @@ -146,6 +146,16 @@ config MOUSE_PS2_OLPC
>>
>> If unsure, say N.
>>
>> +config MOUSE_PS2_FOCALTECH
>> + bool "FocalTech PS/2 mouse protocol extension" if EXPERT
>> + default y
>> + depends on MOUSE_PS2
>> + help
>> + Say Y here if you have a FocalTech PS/2 TouchPad connected to
>> + your system.
>> +
>> + If unsure, say Y.
>> +
>> config MOUSE_SERIAL
>> tristate "Serial mouse"
>> select SERIO
>> diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
>> index f4d657e..26bc5b7 100644
>> --- a/drivers/input/mouse/focaltech.c
>> +++ b/drivers/input/mouse/focaltech.c
>> @@ -2,6 +2,7 @@
>> * Focaltech TouchPad PS/2 mouse driver
>> *
>> * Copyright (c) 2014 Red Hat Inc.
>> + * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@gmail.com>
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License as published by
>> @@ -13,15 +14,14 @@
>> * Hans de Goede <hdegoede@redhat.com>
>> */
>>
>> -/*
>> - * The Focaltech PS/2 touchpad protocol is unknown. This drivers deals with
>> - * detection only, to avoid further detection attempts confusing the touchpad
>> - * this way it at least works in PS/2 mouse compatibility mode.
>> - */
>>
>> #include <linux/device.h>
>> #include <linux/libps2.h>
>> +#include <linux/input/mt.h>
>> +#include <linux/serio.h>
>> +#include <linux/slab.h>
>> #include "psmouse.h"
>> +#include "focaltech.h"
>>
>> static const char * const focaltech_pnp_ids[] = {
>> "FLT0101",
>> @@ -30,6 +30,12 @@ static const char * const focaltech_pnp_ids[] = {
>> NULL
>> };
>>
>> +/*
>> + * Even if the kernel is built without support for Focaltech PS/2 touchpads (or
>> + * when the real driver fails to recognize the device), we still have to detect
>> + * them in order to avoid further detection attempts confusing the touchpad.
>> + * This way it at least works in PS/2 mouse compatibility mode.
>> + */
>> int focaltech_detect(struct psmouse *psmouse, bool set_properties)
>> {
>> if (!psmouse_matches_pnp_id(psmouse, focaltech_pnp_ids))
>> @@ -37,16 +43,296 @@ int focaltech_detect(struct psmouse *psmouse, bool set_properties)
>>
>> if (set_properties) {
>> psmouse->vendor = "FocalTech";
>> - psmouse->name = "FocalTech Touchpad in mouse emulation mode";
>> + psmouse->name = "FocalTech Touchpad";
>> }
>>
>> return 0;
>> }
>>
>> -int focaltech_init(struct psmouse *psmouse)
>> +static void focaltech_reset(struct psmouse *psmouse)
>> {
>> ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
>> psmouse_reset(psmouse);
>> +}
>> +
>> +#ifdef CONFIG_MOUSE_PS2_FOCALTECH
>> +
>> +static void focaltech_report_state(struct psmouse *psmouse)
>> +{
>> + int i;
>> + struct focaltech_data *priv = psmouse->private;
>> + struct focaltech_hw_state *state = &priv->state;
>> + struct input_dev *dev = psmouse->dev;
>> + int finger_count = 0;
>> +
>> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
>> + struct focaltech_finger_state *finger = &state->fingers[i];
>> + int active = finger->active && finger->valid;
>
> bool active;
>
>> + input_mt_slot(dev, i);
>> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
>> + if (active) {
>> + finger_count++;
>
> Why do we need to count contacts ourselves?
We don't.
>
>> + input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
>> + input_report_abs(dev, ABS_MT_POSITION_Y,
>> + focaltech_invert_y(finger->y));
>> + }
>> + }
>> + input_mt_report_pointer_emulation(dev, finger_count);
And this should be:
input_mt_report_pointer_emulation(dev, true);
As already said in my previous mail, sorry about that.
>> +
>> + input_report_key(psmouse->dev, BTN_LEFT, state->pressed);
>> + input_sync(psmouse->dev);
>> +}
>> +
>> +static void process_touch_packet(struct focaltech_hw_state *state,
>> + unsigned char *packet)
>> +{
>> + int i;
>> + unsigned char fingers = packet[1];
>> +
>> + state->pressed = (packet[0] >> 4) & 1;
>> + /* the second byte contains a bitmap of all fingers touching the pad */
>> + for (i = 0; i < FOC_MAX_FINGERS; i++) {
>> + if ((fingers & 0x1) && !state->fingers[i].active) {
>
>
> I think the test (fingers & 1) is not needed here. If the contact was
> not active we do not have valid coordinates for it, so just checking if
> !state->fingers[i].active should we enough, right?
This is checking for fingers[i].active making a transation from false
to true, state->fingers[i].active is still the old value here (it is
re-assigned below) and (fingers & 1) is the new active.
You're right that it does matter to set valid to 0 when going from
inactive to inactive though, so that only testing if fingers[i].active was
false is enough. Either way works for me.
>
>> + /* we do not have a valid position for the finger yet */
>> + state->fingers[i].valid = 0;
>> + }
>> + state->fingers[i].active = fingers & 0x1;
>> + fingers >>= 1;
>> + }
>> +}
>> +
>> +static void process_abs_packet(struct focaltech_hw_state *state,
>> + unsigned char *packet)
>> +{
>> + unsigned int finger = (packet[1] >> 4) - 1;
>> +
>> + state->pressed = (packet[0] >> 4) & 1;
>> + if (finger >= FOC_MAX_FINGERS)
>> + return;
>
> Please combine checks with respective assignments:
>
> finger = (packet[1] >> 4) - 1;
> if (finger >= FOC_MAX_FINGERS) {
> psmouse_err(...);
> return;
> }
>
> state->pressed = (packet[0] >> 4) & 1;
> ...
>
>> + /*
>> + * packet[5] contains some kind of tool size in the most significant
>> + * nibble. 0xff is a special value (latching) that signals a large
>> + * contact area.
>> + */
>> + if (packet[5] == 0xff) {
>> + state->fingers[finger].valid = 0;
>
> I wonder if we should report it via ABS_TOOL_WIDTH/ABS_MT_TOUCH_MAJOR.
> Can be done in a separate patch though.
>
>> + return;
>> + }
>> + state->fingers[finger].x = ((packet[1] & 0xf) << 8) | packet[2];
>> + state->fingers[finger].y = (packet[3] << 8) | packet[4];
>> + state->fingers[finger].valid = 1;
>> +}
>> +static void process_rel_packet(struct focaltech_hw_state *state,
>> + unsigned char *packet)
>> +{
>> + int finger1 = ((packet[0] >> 4) & 0x7) - 1;
>> + int finger2 = ((packet[3] >> 4) & 0x7) - 1;
>
> I think you want unsigned int here, otherwise your checks need to be
>
> if (f >= 0 && f < FOC_MAX_FINGERS)
> ...
>
>> +
>> + state->pressed = packet[0] >> 7;
>> + if (finger1 < FOC_MAX_FINGERS) {
>> + state->fingers[finger1].x += (char)packet[1];
>> + state->fingers[finger1].y += (char)packet[2];
>> + }
>> + /*
>> + * If there is an odd number of fingers, the last relative packet only
>> + * contains one finger. In this case, the second finger index in the
>> + * packet is 0 (we subtract 1 in the lines above to create array
>> + * indices).
>> + */
>> + if (finger2 != -1 && finger2 < FOC_MAX_FINGERS) {
>> + state->fingers[finger2].x += (char)packet[4];
>> + state->fingers[finger2].y += (char)packet[5];
>> + }
>> +}
>> +
>> +static void focaltech_process_packet(struct psmouse *psmouse)
>> +{
>> + struct focaltech_data *priv = psmouse->private;
>> + unsigned char *packet = psmouse->packet;
>> +
>> + switch (packet[0] & 0xf) {
>> + case FOC_TOUCH:
>> + process_touch_packet(&priv->state, packet);
>> + break;
>> + case FOC_ABS:
>> + process_abs_packet(&priv->state, packet);
>> + break;
>> + case FOC_REL:
>> + process_rel_packet(&priv->state, packet);
>> + break;
>> + default:
>> + psmouse_err(psmouse, "Unknown packet type: %02x", packet[0]);
>> + break;
>> + }
>> +
>> + focaltech_report_state(psmouse);
>> +}
>> +
>> +static psmouse_ret_t focaltech_process_byte(struct psmouse *psmouse)
>> +{
>> + if (psmouse->pktcnt >= 6) { /* Full packet received */
>> + focaltech_process_packet(psmouse);
>> + return PSMOUSE_FULL_PACKET;
>> + }
>> + /*
>> + * we might want to do some validation of the data here, but we do not
>> + * know the protocoll well enough
>> + */
>> + return PSMOUSE_GOOD_DATA;
>> +}
>> +
>> +static int focaltech_switch_protocol(struct psmouse *psmouse)
>> +{
>> + struct ps2dev *ps2dev = &psmouse->ps2dev;
>> + unsigned char param[3];
>> +
>> + param[0] = 0;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + param[0] = 1;
>> + if (ps2_command(ps2dev, param, 0x10f8))
>> + return -EIO;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
>> + return -EIO;
>> +
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_ENABLE))
>> + return -EIO;
>>
>> return 0;
>> }
>> +
>> +static void focaltech_disconnect(struct psmouse *psmouse)
>> +{
>> + focaltech_reset(psmouse);
>> + kfree(psmouse->private);
>> + psmouse->private = NULL;
>> +}
>> +
>> +static int focaltech_reconnect(struct psmouse *psmouse)
>> +{
>> + focaltech_reset(psmouse);
>> + if (focaltech_switch_protocol(psmouse)) {
>> + psmouse_err(psmouse,
>> + "Unable to initialize the device.");
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> +static void set_input_params(struct psmouse *psmouse)
>> +{
>> + struct input_dev *dev = psmouse->dev;
>> + struct focaltech_data *priv = psmouse->private;
>> +
>> + __set_bit(EV_ABS, dev->evbit);
>> + input_set_abs_params(dev, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
>> + input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
>> + input_mt_init_slots(dev, 5, INPUT_MT_POINTER);
>> + __clear_bit(EV_REL, dev->evbit);
>> + __clear_bit(REL_X, dev->relbit);
>> + __clear_bit(REL_Y, dev->relbit);
>> + __clear_bit(BTN_RIGHT, dev->keybit);
>> + __clear_bit(BTN_MIDDLE, dev->keybit);
>> + __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
>> +}
>> +
>> +static int focaltech_read_register(struct ps2dev *ps2dev, int reg,
>> + unsigned char *param)
>> +{
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
>> + return -1;
>
> I 'd rather see -EIO here as well.
>
>> + param[0] = 0;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + param[0] = reg;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>> + return -1;
>> + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
>> + return -1;
>> + return 0;
>> +}
>> +
>> +static int focaltech_read_size(struct psmouse *psmouse)
>> +{
>> + struct ps2dev *ps2dev = &psmouse->ps2dev;
>> + struct focaltech_data *priv = psmouse->private;
>> + char param[3];
>> +
>> + if (focaltech_read_register(ps2dev, 2, param))
>> + return -EIO;
>> + /* not sure whether this is 100% correct */
>> + priv->x_max = (unsigned char)param[1] * 128;
>> + priv->y_max = (unsigned char)param[2] * 128;
>> +
>> + return 0;
>> +}
>> +int focaltech_init(struct psmouse *psmouse)
>> +{
>> + struct focaltech_data *priv;
>> + int err;
>> +
>> + psmouse->private = priv = kzalloc(sizeof(struct focaltech_data), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + focaltech_reset(psmouse);
>> + if (focaltech_read_size(psmouse)) {
>> + focaltech_reset(psmouse);
>> + psmouse_err(psmouse,
>> + "Unable to read the size of the touchpad.");
>> + err = -ENOSYS;
>
> Why -ENOSYS?
>
> error = focaltech_read_size(psmouse);
> if (error) {
> ...
> return error;
> }
>
> The fact that the rest of psmouse likes returning -1 does not mean that
> the new code should be similarly ugly.
>
>> + goto fail;
>> + }
>> + if (focaltech_switch_protocol(psmouse)) {
>> + focaltech_reset(psmouse);
>> + psmouse_err(psmouse,
>> + "Unable to initialize the device.");
>> + err = -ENOSYS;
>> + goto fail;
>> + }
>> +
>> + set_input_params(psmouse);
>> +
>> + psmouse->protocol_handler = focaltech_process_byte;
>> + psmouse->pktsize = 6;
>> + psmouse->disconnect = focaltech_disconnect;
>> + psmouse->reconnect = focaltech_reconnect;
>> + psmouse->cleanup = focaltech_reset;
>> + /* resync is not supported yet */
>> + psmouse->resync_time = 0;
>> +
>> + return 0;
>> +fail:
>> + focaltech_reset(psmouse);
>> + kfree(priv);
>> + return err;
>> +}
>> +
>> +bool focaltech_supported(void)
>> +{
>> + return true;
>> +}
>> +
>> +#else /* CONFIG_MOUSE_PS2_FOCALTECH */
>> +
>> +int focaltech_init(struct psmouse *psmouse)
>> +{
>> + focaltech_reset(psmouse);
>> +
>> + return 0;
>> +}
>> +
>> +bool focaltech_supported(void)
>> +{
>> + return false;
>> +}
>> +
>> +#endif /* CONFIG_MOUSE_PS2_FOCALTECH */
>> diff --git a/drivers/input/mouse/focaltech.h b/drivers/input/mouse/focaltech.h
>> index 498650c..68c5cfc 100644
>> --- a/drivers/input/mouse/focaltech.h
>> +++ b/drivers/input/mouse/focaltech.h
>> @@ -2,6 +2,7 @@
>> * Focaltech TouchPad PS/2 mouse driver
>> *
>> * Copyright (c) 2014 Red Hat Inc.
>> + * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@gmail.com>
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License as published by
>> @@ -16,7 +17,66 @@
>> #ifndef _FOCALTECH_H
>> #define _FOCALTECH_H
>>
>> +/*
>> + * Packet types - the numbers are not consecutive, so we might be missing
>> + * something here.
>> + */
>> +#define FOC_TOUCH 0x3 /* bitmap of active fingers */
>> +#define FOC_ABS 0x6 /* absolute position of one finger */
>> +#define FOC_REL 0x9 /* relative position of 1-2 fingers */
>> +
>> +#define FOC_MAX_FINGERS 5
>> +
>> +#define FOC_MAX_X 2431
>> +#define FOC_MAX_Y 1663
>> +
>> +static inline int focaltech_invert_y(int y)
>> +{
>> + return FOC_MAX_Y - y;
>> +}
>> +
>> +/*
>> + * Current state of a single finger on the touchpad.
>> + */
>> +struct focaltech_finger_state {
>> + /* the touchpad has generated a touch event for the finger */
>> + bool active;
>> + /*
>> + * The touchpad has sent position data for the finger. Touch event
>> + * packages reset this flag for new fingers, and there is a time
>> + * between the first touch event and the following absolute position
>> + * packet for the finger where the touchpad has declared the finger to
>> + * be valid, but we do not have any valid position yet.
>> + */
>> + bool valid;
>> + /* absolute position (from the bottom left corner) of the finger */
>> + unsigned int x;
>> + unsigned int y;
>> +};
>> +
>> +/*
>> + * Description of the current state of the touchpad hardware.
>> + */
>> +struct focaltech_hw_state {
>> + /*
>> + * The touchpad tracks the positions of the fingers for us, the array
>> + * indices correspond to the finger indices returned in the report
>> + * packages.
>> + */
>> + struct focaltech_finger_state fingers[FOC_MAX_FINGERS];
>> + /*
>> + * True if the clickpad has been pressed.
>> + */
>> + bool pressed;
>> +};
>> +
>> +struct focaltech_data {
>> + unsigned int x_max, y_max;
>> + struct focaltech_hw_state state;
>> +};
>> +
>> int focaltech_detect(struct psmouse *psmouse, bool set_properties);
>> int focaltech_init(struct psmouse *psmouse);
>> +bool focaltech_supported(void);
>>
>> #endif
>> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
>> index 26994f6..4a9de33 100644
>> --- a/drivers/input/mouse/psmouse-base.c
>> +++ b/drivers/input/mouse/psmouse-base.c
>> @@ -725,16 +725,19 @@ static int psmouse_extensions(struct psmouse *psmouse,
>>
>> /* Always check for focaltech, this is safe as it uses pnp-id matching */
>> if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
>> - if (!set_properties || focaltech_init(psmouse) == 0) {
>> - /*
>> - * Not supported yet, use bare protocol.
>> - * Note that we need to also restrict
>> - * psmouse_max_proto so that psmouse_initialize()
>> - * does not try to reset rate and resolution,
>> - * because even that upsets the device.
>> - */
>> - psmouse_max_proto = PSMOUSE_PS2;
>> - return PSMOUSE_PS2;
>> + if (max_proto > PSMOUSE_IMEX) {
>> + if (!set_properties || focaltech_init(psmouse) == 0) {
>> + if (focaltech_supported())
>> + return PSMOUSE_FOCALTECH;
>> + /*
>> + * Note that we need to also restrict
>> + * psmouse_max_proto so that psmouse_initialize()
>> + * does not try to reset rate and resolution,
>> + * because even that upsets the device.
>> + */
>> + psmouse_max_proto = PSMOUSE_PS2;
>> + return PSMOUSE_PS2;
>> + }
>> }
>> }
>>
>> @@ -1063,6 +1066,15 @@ static const struct psmouse_protocol psmouse_protocols[] = {
>> .alias = "cortps",
>> .detect = cortron_detect,
>> },
>> +#ifdef CONFIG_MOUSE_PS2_FOCALTECH
>> + {
>> + .type = PSMOUSE_FOCALTECH,
>> + .name = "FocalTechPS/2",
>> + .alias = "focaltech",
>> + .detect = focaltech_detect,
>> + .init = focaltech_init,
>> + },
>> +#endif
>> {
>> .type = PSMOUSE_AUTO,
>> .name = "auto",
>> diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
>> index f4cf664..c2ff137 100644
>> --- a/drivers/input/mouse/psmouse.h
>> +++ b/drivers/input/mouse/psmouse.h
>> @@ -96,6 +96,7 @@ enum psmouse_type {
>> PSMOUSE_FSP,
>> PSMOUSE_SYNAPTICS_RELATIVE,
>> PSMOUSE_CYPRESS,
>> + PSMOUSE_FOCALTECH,
>> PSMOUSE_AUTO /* This one should always be last */
>> };
>>
>> --
>> 1.9.1
>>
>
> Thanks.
>
Regards,
Hans
^ permalink raw reply
* [PATCH] HID: wacom - Add support for Intuos Pen Medium
From: Ping Cheng @ 2014-11-11 20:52 UTC (permalink / raw)
To: jkosina; +Cc: linux-input, Ping Cheng
Signed-off-by: Ping Cheng <pingc@wacom.com>
---
drivers/hid/wacom_wac.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 586b240..8ce7fab 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2878,6 +2878,10 @@ static const struct wacom_features wacom_features_0x30C =
{ "Wacom ISDv5 30C", .type = WACOM_24HDT, /* Touch */
.oVid = USB_VENDOR_ID_WACOM, .oPid = 0x30A, .touch_max = 10,
.check_for_hid_type = true, .hid_type = HID_TYPE_USBNONE };
+static const struct wacom_features wacom_features_0x323 =
+ { "Wacom Intuos P M", 21600, 13500, 1023, 31,
+ INTUOSHT, WACOM_INTUOS_RES, WACOM_INTUOS_RES,
+ .check_for_hid_type = true, .hid_type = HID_TYPE_USBNONE };
static const struct wacom_features wacom_features_HID_ANY_ID =
{ "Wacom HID", .type = HID_GENERIC };
@@ -3022,6 +3026,7 @@ const struct hid_device_id wacom_ids[] = {
{ USB_DEVICE_WACOM(0x314) },
{ USB_DEVICE_WACOM(0x315) },
{ USB_DEVICE_WACOM(0x317) },
+ { USB_DEVICE_WACOM(0x323) },
{ USB_DEVICE_WACOM(0x4001) },
{ USB_DEVICE_WACOM(0x4004) },
{ USB_DEVICE_WACOM(0x5000) },
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] HID: wacom - Add support for Intuos Pen Medium
From: Jiri Kosina @ 2014-11-11 23:35 UTC (permalink / raw)
To: Ping Cheng; +Cc: linux-input, Ping Cheng
In-Reply-To: <1415739128-28533-1-git-send-email-pingc@wacom.com>
On Tue, 11 Nov 2014, Ping Cheng wrote:
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
> drivers/hid/wacom_wac.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 586b240..8ce7fab 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -2878,6 +2878,10 @@ static const struct wacom_features wacom_features_0x30C =
> { "Wacom ISDv5 30C", .type = WACOM_24HDT, /* Touch */
> .oVid = USB_VENDOR_ID_WACOM, .oPid = 0x30A, .touch_max = 10,
> .check_for_hid_type = true, .hid_type = HID_TYPE_USBNONE };
> +static const struct wacom_features wacom_features_0x323 =
> + { "Wacom Intuos P M", 21600, 13500, 1023, 31,
> + INTUOSHT, WACOM_INTUOS_RES, WACOM_INTUOS_RES,
> + .check_for_hid_type = true, .hid_type = HID_TYPE_USBNONE };
>
> static const struct wacom_features wacom_features_HID_ANY_ID =
> { "Wacom HID", .type = HID_GENERIC };
> @@ -3022,6 +3026,7 @@ const struct hid_device_id wacom_ids[] = {
> { USB_DEVICE_WACOM(0x314) },
> { USB_DEVICE_WACOM(0x315) },
> { USB_DEVICE_WACOM(0x317) },
> + { USB_DEVICE_WACOM(0x323) },
> { USB_DEVICE_WACOM(0x4001) },
> { USB_DEVICE_WACOM(0x4004) },
> { USB_DEVICE_WACOM(0x5000) },
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v5 0/3] mfd: max8997: add regmap support
From: Robert Baldyga @ 2014-11-12 7:23 UTC (permalink / raw)
To: lee.jones
Cc: sameo, myungjoo.ham, cw00.choi, dmitry.torokhov, cooloney,
rpurdie, sre, dbaryshkov, dwmw2, lgirdwood, broonie, a.zummo,
paul.gortmaker, sachin.kamat, k.kozlowski, linux-kernel,
linux-input, linux-leds, linux-pm, rtc-linux, Robert Baldyga
This patchset modifies max8997 driver and associated function drivers to use
register maps instead of operating directly on i2c bus. This change allowed
to simplify irq handling, and to move some initializations to individual
function drivers. Hence now when some functions are not enabled, their i2c
clients, regmaps and irqs are not registered.
Because patches are under review for very long time and I still don't have
all needed Acks, I'm resending this series rebased to the latest next branch.
Best regards
Robert Baldyga
Samsung R&D Institute Poland
Changelog:
v5:
- rebase patches to last next branch
v4:
- remove patch moving regmap handling to function drivers
v3: https://lkml.org/lkml/2014/3/13/101
- fix error handling
- fix deinitializations order
- move muic irq enum values renaming to separate patch
v2: https://lkml.org/lkml/2014/3/12/237
- rebase patches on Lee Jones' MFD tree
- add missing selects in Kconfig
- add missing deinitializations
- add interrupt disabling when suspend
- few minor changes and typo fixes
v1: https://lkml.org/lkml/2014/3/11/291
Robert Baldyga (3):
mfd: max8997: use regmap to access registers
mfd: max8997: handle IRQs using regmap
mfd: max8997: change irq names to upper case
drivers/extcon/extcon-max8997.c | 66 +++---
drivers/input/misc/max8997_haptic.c | 34 ++--
drivers/leds/leds-max8997.c | 13 +-
drivers/mfd/Kconfig | 3 +-
drivers/mfd/Makefile | 2 +-
drivers/mfd/max8997-irq.c | 387 ------------------------------------
drivers/mfd/max8997.c | 245 +++++++++++++++--------
drivers/power/max8997_charger.c | 33 +--
drivers/regulator/max8997.c | 87 ++++----
drivers/rtc/rtc-max8997.c | 58 +++---
include/linux/mfd/max8997-private.h | 82 ++++++--
11 files changed, 381 insertions(+), 629 deletions(-)
delete mode 100644 drivers/mfd/max8997-irq.c
--
1.9.1
^ permalink raw reply
* [PATCH v5 1/3] mfd: max8997: use regmap to access registers
From: Robert Baldyga @ 2014-11-12 7:23 UTC (permalink / raw)
To: lee.jones
Cc: sameo, myungjoo.ham, cw00.choi, dmitry.torokhov, cooloney,
rpurdie, sre, dbaryshkov, dwmw2, lgirdwood, broonie, a.zummo,
paul.gortmaker, sachin.kamat, k.kozlowski, linux-kernel,
linux-input, linux-leds, linux-pm, rtc-linux, Robert Baldyga
In-Reply-To: <1415776996-11569-1-git-send-email-r.baldyga@samsung.com>
This patch modifies max8997 driver and each associated function driver,
to use regmap instead of operating directly on i2c bus. It will allow to
simplify IRQ handling using regmap-irq.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
[For extcon part]
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
[For leds part]
Acked-by: Bryan Wu <cooloney@gmail.com>
[For the mfd part]
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/extcon/extcon-max8997.c | 31 ++++----
drivers/input/misc/max8997_haptic.c | 34 +++++----
drivers/leds/leds-max8997.c | 13 ++--
drivers/mfd/Kconfig | 1 +
drivers/mfd/max8997-irq.c | 64 +++++++---------
drivers/mfd/max8997.c | 141 ++++++++++++++++--------------------
drivers/power/max8997_charger.c | 33 +++++----
drivers/regulator/max8997.c | 87 +++++++++++-----------
drivers/rtc/rtc-max8997.c | 56 +++++++-------
include/linux/mfd/max8997-private.h | 17 ++---
10 files changed, 228 insertions(+), 249 deletions(-)
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 75e501c..2aafd5b 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -27,6 +27,7 @@
#include <linux/mfd/max8997-private.h>
#include <linux/extcon.h>
#include <linux/irqdomain.h>
+#include <linux/regmap.h>
#define DEV_NAME "max8997-muic"
#define DELAY_MS_DEFAULT 20000 /* unit: millisecond */
@@ -116,7 +117,7 @@ enum max8997_muic_charger_type {
struct max8997_muic_info {
struct device *dev;
- struct i2c_client *muic;
+ struct max8997_dev *max8997;
struct extcon_dev *edev;
int prev_cable_type;
int prev_chg_type;
@@ -190,10 +191,10 @@ static int max8997_muic_set_debounce_time(struct max8997_muic_info *info,
case ADC_DEBOUNCE_TIME_10MS:
case ADC_DEBOUNCE_TIME_25MS:
case ADC_DEBOUNCE_TIME_38_62MS:
- ret = max8997_update_reg(info->muic,
+ ret = regmap_update_bits(info->max8997->regmap_muic,
MAX8997_MUIC_REG_CONTROL3,
- time << CONTROL3_ADCDBSET_SHIFT,
- CONTROL3_ADCDBSET_MASK);
+ CONTROL3_ADCDBSET_MASK,
+ time << CONTROL3_ADCDBSET_SHIFT);
if (ret) {
dev_err(info->dev, "failed to set ADC debounce time\n");
return ret;
@@ -228,8 +229,8 @@ static int max8997_muic_set_path(struct max8997_muic_info *info,
else
ctrl1 = CONTROL1_SW_OPEN;
- ret = max8997_update_reg(info->muic,
- MAX8997_MUIC_REG_CONTROL1, ctrl1, COMP_SW_MASK);
+ ret = regmap_update_bits(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_CONTROL1, COMP_SW_MASK, ctrl1);
if (ret < 0) {
dev_err(info->dev, "failed to update MUIC register\n");
return ret;
@@ -240,9 +241,9 @@ static int max8997_muic_set_path(struct max8997_muic_info *info,
else
ctrl2 |= CONTROL2_LOWPWR_MASK; /* LowPwr=1, CPEn=0 */
- ret = max8997_update_reg(info->muic,
- MAX8997_MUIC_REG_CONTROL2, ctrl2,
- CONTROL2_LOWPWR_MASK | CONTROL2_CPEN_MASK);
+ ret = regmap_update_bits(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_CONTROL2,
+ CONTROL2_LOWPWR_MASK | CONTROL2_CPEN_MASK, ctrl2);
if (ret < 0) {
dev_err(info->dev, "failed to update MUIC register\n");
return ret;
@@ -543,8 +544,8 @@ static void max8997_muic_irq_work(struct work_struct *work)
if (info->irq == muic_irqs[i].virq)
irq_type = muic_irqs[i].irq;
- ret = max8997_bulk_read(info->muic, MAX8997_MUIC_REG_STATUS1,
- 2, info->status);
+ ret = regmap_bulk_read(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_STATUS1, info->status, 2);
if (ret) {
dev_err(info->dev, "failed to read muic register\n");
mutex_unlock(&info->mutex);
@@ -605,8 +606,8 @@ static int max8997_muic_detect_dev(struct max8997_muic_info *info)
mutex_lock(&info->mutex);
/* Read STATUSx register to detect accessory */
- ret = max8997_bulk_read(info->muic,
- MAX8997_MUIC_REG_STATUS1, 2, info->status);
+ ret = regmap_bulk_read(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_STATUS1, info->status, 2);
if (ret) {
dev_err(info->dev, "failed to read MUIC register\n");
mutex_unlock(&info->mutex);
@@ -665,7 +666,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
return -ENOMEM;
info->dev = &pdev->dev;
- info->muic = max8997->muic;
+ info->max8997 = max8997;
platform_set_drvdata(pdev, info);
mutex_init(&info->mutex);
@@ -717,7 +718,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
/* Initialize registers according to platform data */
for (i = 0; i < muic_pdata->num_init_data; i++) {
- max8997_write_reg(info->muic,
+ regmap_write(info->max8997->regmap_muic,
muic_pdata->init_data[i].addr,
muic_pdata->init_data[i].data);
}
diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c
index a363ebb..53b32b5 100644
--- a/drivers/input/misc/max8997_haptic.c
+++ b/drivers/input/misc/max8997_haptic.c
@@ -31,6 +31,7 @@
#include <linux/mfd/max8997-private.h>
#include <linux/mfd/max8997.h>
#include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
/* Haptic configuration 2 register */
#define MAX8997_MOTOR_TYPE_SHIFT 7
@@ -45,7 +46,7 @@
struct max8997_haptic {
struct device *dev;
- struct i2c_client *client;
+ struct max8997_dev *max8997;
struct input_dev *input_dev;
struct regulator *regulator;
@@ -86,19 +87,19 @@ static int max8997_haptic_set_duty_cycle(struct max8997_haptic *chip)
}
switch (chip->internal_mode_pattern) {
case 0:
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGPWMDC1, duty_index);
break;
case 1:
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGPWMDC2, duty_index);
break;
case 2:
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGPWMDC3, duty_index);
break;
case 3:
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGPWMDC4, duty_index);
break;
default:
@@ -115,50 +116,51 @@ static void max8997_haptic_configure(struct max8997_haptic *chip)
value = chip->type << MAX8997_MOTOR_TYPE_SHIFT |
chip->enabled << MAX8997_ENABLE_SHIFT |
chip->mode << MAX8997_MODE_SHIFT | chip->pwm_divisor;
- max8997_write_reg(chip->client, MAX8997_HAPTIC_REG_CONF2, value);
+ regmap_write(chip->max8997->regmap_haptic,
+ MAX8997_HAPTIC_REG_CONF2, value);
if (chip->mode == MAX8997_INTERNAL_MODE && chip->enabled) {
value = chip->internal_mode_pattern << MAX8997_CYCLE_SHIFT |
chip->internal_mode_pattern << MAX8997_SIG_PERIOD_SHIFT |
chip->internal_mode_pattern << MAX8997_SIG_DUTY_SHIFT |
chip->internal_mode_pattern << MAX8997_PWM_DUTY_SHIFT;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_DRVCONF, value);
switch (chip->internal_mode_pattern) {
case 0:
value = chip->pattern_cycle << 4;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_CYCLECONF1, value);
value = chip->pattern_signal_period;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGCONF1, value);
break;
case 1:
value = chip->pattern_cycle;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_CYCLECONF1, value);
value = chip->pattern_signal_period;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGCONF2, value);
break;
case 2:
value = chip->pattern_cycle << 4;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_CYCLECONF2, value);
value = chip->pattern_signal_period;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGCONF3, value);
break;
case 3:
value = chip->pattern_cycle;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_CYCLECONF2, value);
value = chip->pattern_signal_period;
- max8997_write_reg(chip->client,
+ regmap_write(chip->max8997->regmap_haptic,
MAX8997_HAPTIC_REG_SIGCONF4, value);
break;
@@ -277,7 +279,7 @@ static int max8997_haptic_probe(struct platform_device *pdev)
INIT_WORK(&chip->work, max8997_haptic_play_effect_work);
mutex_init(&chip->mutex);
- chip->client = iodev->haptic;
+ chip->max8997 = iodev;
chip->dev = &pdev->dev;
chip->input_dev = input_dev;
chip->pwm_period = haptic_pdata->pwm_period;
diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
index 607bc27..49bb128 100644
--- a/drivers/leds/leds-max8997.c
+++ b/drivers/leds/leds-max8997.c
@@ -18,6 +18,7 @@
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
#include <linux/platform_device.h>
+#include <linux/regmap.h>
#define MAX8997_LED_FLASH_SHIFT 3
#define MAX8997_LED_FLASH_CUR_MASK 0xf8
@@ -53,7 +54,6 @@ static void max8997_led_set_mode(struct max8997_led *led,
enum max8997_led_mode mode)
{
int ret;
- struct i2c_client *client = led->iodev->i2c;
u8 mask = 0, val;
switch (mode) {
@@ -89,8 +89,8 @@ static void max8997_led_set_mode(struct max8997_led *led,
}
if (mask) {
- ret = max8997_update_reg(client, MAX8997_REG_LEN_CNTL, val,
- mask);
+ ret = regmap_update_bits(led->iodev->regmap,
+ MAX8997_REG_LEN_CNTL, mask, val);
if (ret)
dev_err(led->iodev->dev,
"failed to update register(%d)\n", ret);
@@ -102,7 +102,6 @@ static void max8997_led_set_mode(struct max8997_led *led,
static void max8997_led_enable(struct max8997_led *led, bool enable)
{
int ret;
- struct i2c_client *client = led->iodev->i2c;
u8 val = 0, mask = MAX8997_LED_BOOST_ENABLE_MASK;
if (led->enabled == enable)
@@ -110,7 +109,8 @@ static void max8997_led_enable(struct max8997_led *led, bool enable)
val = enable ? MAX8997_LED_BOOST_ENABLE_MASK : 0;
- ret = max8997_update_reg(client, MAX8997_REG_BOOST_CNTL, val, mask);
+ ret = regmap_update_bits(led->iodev->regmap,
+ MAX8997_REG_BOOST_CNTL, mask, val);
if (ret)
dev_err(led->iodev->dev,
"failed to update register(%d)\n", ret);
@@ -122,7 +122,6 @@ static void max8997_led_set_current(struct max8997_led *led,
enum led_brightness value)
{
int ret;
- struct i2c_client *client = led->iodev->i2c;
u8 val = 0, mask = 0, reg = 0;
switch (led->led_mode) {
@@ -143,7 +142,7 @@ static void max8997_led_set_current(struct max8997_led *led,
}
if (mask) {
- ret = max8997_update_reg(client, reg, val, mask);
+ ret = regmap_update_bits(led->iodev->regmap, reg, mask, val);
if (ret)
dev_err(led->iodev->dev,
"failed to update register(%d)\n", ret);
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 72d3808..ca57035 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -463,6 +463,7 @@ config MFD_MAX8997
bool "Maxim Semiconductor MAX8997/8966 PMIC Support"
depends on I2C=y
select MFD_CORE
+ select REGMAP_I2C
select IRQ_DOMAIN
help
Say yes here to add support for Maxim Semiconductor MAX8997/8966.
diff --git a/drivers/mfd/max8997-irq.c b/drivers/mfd/max8997-irq.c
index 43fa614..0e7ff39 100644
--- a/drivers/mfd/max8997-irq.c
+++ b/drivers/mfd/max8997-irq.c
@@ -26,6 +26,7 @@
#include <linux/interrupt.h>
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
+#include <linux/regmap.h>
static const u8 max8997_mask_reg[] = {
[PMIC_INT1] = MAX8997_REG_INT1MSK,
@@ -41,25 +42,6 @@ static const u8 max8997_mask_reg[] = {
[FLASH_STATUS] = MAX8997_REG_INVALID,
};
-static struct i2c_client *get_i2c(struct max8997_dev *max8997,
- enum max8997_irq_source src)
-{
- switch (src) {
- case PMIC_INT1 ... PMIC_INT4:
- return max8997->i2c;
- case FUEL_GAUGE:
- return NULL;
- case MUIC_INT1 ... MUIC_INT3:
- return max8997->muic;
- case GPIO_LOW ... GPIO_HI:
- return max8997->i2c;
- case FLASH_STATUS:
- return max8997->i2c;
- default:
- return ERR_PTR(-EINVAL);
- }
-}
-
struct max8997_irq_data {
int mask;
enum max8997_irq_source group;
@@ -124,15 +106,20 @@ static void max8997_irq_sync_unlock(struct irq_data *data)
int i;
for (i = 0; i < MAX8997_IRQ_GROUP_NR; i++) {
+ struct regmap *map;
u8 mask_reg = max8997_mask_reg[i];
- struct i2c_client *i2c = get_i2c(max8997, i);
+
+ if (i >= MUIC_INT1 && i <= MUIC_INT3)
+ map = max8997->regmap_muic;
+ else
+ map = max8997->regmap;
if (mask_reg == MAX8997_REG_INVALID ||
- IS_ERR_OR_NULL(i2c))
+ IS_ERR_OR_NULL(map))
continue;
max8997->irq_masks_cache[i] = max8997->irq_masks_cur[i];
- max8997_write_reg(i2c, max8997_mask_reg[i],
+ regmap_write(map, max8997_mask_reg[i],
max8997->irq_masks_cur[i]);
}
@@ -181,11 +168,11 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
{
struct max8997_dev *max8997 = data;
u8 irq_reg[MAX8997_IRQ_GROUP_NR] = {};
- u8 irq_src;
+ unsigned int irq_src;
int ret;
int i, cur_irq;
- ret = max8997_read_reg(max8997->i2c, MAX8997_REG_INTSRC, &irq_src);
+ ret = regmap_read(max8997->regmap, MAX8997_REG_INTSRC, &irq_src);
if (ret < 0) {
dev_err(max8997->dev, "Failed to read interrupt source: %d\n",
ret);
@@ -194,8 +181,8 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
if (irq_src & MAX8997_IRQSRC_PMIC) {
/* PMIC INT1 ~ INT4 */
- max8997_bulk_read(max8997->i2c, MAX8997_REG_INT1, 4,
- &irq_reg[PMIC_INT1]);
+ regmap_bulk_read(max8997->regmap, MAX8997_REG_INT1,
+ &irq_reg[PMIC_INT1], 4);
}
if (irq_src & MAX8997_IRQSRC_FUELGAUGE) {
/*
@@ -215,8 +202,8 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
}
if (irq_src & MAX8997_IRQSRC_MUIC) {
/* MUIC INT1 ~ INT3 */
- max8997_bulk_read(max8997->muic, MAX8997_MUIC_REG_INT1, 3,
- &irq_reg[MUIC_INT1]);
+ regmap_bulk_read(max8997->regmap_muic, MAX8997_MUIC_REG_INT1,
+ &irq_reg[MUIC_INT1], 3);
}
if (irq_src & MAX8997_IRQSRC_GPIO) {
/* GPIO Interrupt */
@@ -225,8 +212,8 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
irq_reg[GPIO_LOW] = 0;
irq_reg[GPIO_HI] = 0;
- max8997_bulk_read(max8997->i2c, MAX8997_REG_GPIOCNTL1,
- MAX8997_NUM_GPIO, gpio_info);
+ regmap_bulk_read(max8997->regmap, MAX8997_REG_GPIOCNTL1,
+ gpio_info, MAX8997_NUM_GPIO);
for (i = 0; i < MAX8997_NUM_GPIO; i++) {
bool interrupt = false;
@@ -260,8 +247,10 @@ static irqreturn_t max8997_irq_thread(int irq, void *data)
}
if (irq_src & MAX8997_IRQSRC_FLASH) {
/* Flash Status Interrupt */
- ret = max8997_read_reg(max8997->i2c, MAX8997_REG_FLASHSTATUS,
- &irq_reg[FLASH_STATUS]);
+ unsigned int data;
+ ret = regmap_read(max8997->regmap,
+ MAX8997_REG_FLASHSTATUS, &data);
+ irq_reg[FLASH_STATUS] = data;
}
/* Apply masking */
@@ -312,7 +301,7 @@ int max8997_irq_init(struct max8997_dev *max8997)
struct irq_domain *domain;
int i;
int ret;
- u8 val;
+ unsigned int val;
if (!max8997->irq) {
dev_warn(max8997->dev, "No interrupt specified.\n");
@@ -323,22 +312,19 @@ int max8997_irq_init(struct max8997_dev *max8997)
/* Mask individual interrupt sources */
for (i = 0; i < MAX8997_IRQ_GROUP_NR; i++) {
- struct i2c_client *i2c;
-
max8997->irq_masks_cur[i] = 0xff;
max8997->irq_masks_cache[i] = 0xff;
- i2c = get_i2c(max8997, i);
- if (IS_ERR_OR_NULL(i2c))
+ if (IS_ERR_OR_NULL(max8997->regmap))
continue;
if (max8997_mask_reg[i] == MAX8997_REG_INVALID)
continue;
- max8997_write_reg(i2c, max8997_mask_reg[i], 0xff);
+ regmap_write(max8997->regmap, max8997_mask_reg[i], 0xff);
}
for (i = 0; i < MAX8997_NUM_GPIO; i++) {
- max8997->gpio_status[i] = (max8997_read_reg(max8997->i2c,
+ max8997->gpio_status[i] = (regmap_read(max8997->regmap,
MAX8997_REG_GPIOCNTL1 + i,
&val)
& MAX8997_GPIO_DATA_MASK) ?
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 595364e..c4a1072 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -33,6 +33,7 @@
#include <linux/mfd/core.h>
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
+#include <linux/regmap.h>
#define I2C_ADDR_PMIC (0xCC >> 1)
#define I2C_ADDR_MUIC (0x4A >> 1)
@@ -57,81 +58,29 @@ static const struct of_device_id max8997_pmic_dt_match[] = {
};
#endif
-int max8997_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
-{
- struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- int ret;
-
- mutex_lock(&max8997->iolock);
- ret = i2c_smbus_read_byte_data(i2c, reg);
- mutex_unlock(&max8997->iolock);
- if (ret < 0)
- return ret;
-
- ret &= 0xff;
- *dest = ret;
- return 0;
-}
-EXPORT_SYMBOL_GPL(max8997_read_reg);
-
-int max8997_bulk_read(struct i2c_client *i2c, u8 reg, int count, u8 *buf)
-{
- struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- int ret;
-
- mutex_lock(&max8997->iolock);
- ret = i2c_smbus_read_i2c_block_data(i2c, reg, count, buf);
- mutex_unlock(&max8997->iolock);
- if (ret < 0)
- return ret;
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(max8997_bulk_read);
-
-int max8997_write_reg(struct i2c_client *i2c, u8 reg, u8 value)
-{
- struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- int ret;
-
- mutex_lock(&max8997->iolock);
- ret = i2c_smbus_write_byte_data(i2c, reg, value);
- mutex_unlock(&max8997->iolock);
- return ret;
-}
-EXPORT_SYMBOL_GPL(max8997_write_reg);
-
-int max8997_bulk_write(struct i2c_client *i2c, u8 reg, int count, u8 *buf)
-{
- struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- int ret;
+static const struct regmap_config max8997_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX8997_REG_PMIC_END,
+};
- mutex_lock(&max8997->iolock);
- ret = i2c_smbus_write_i2c_block_data(i2c, reg, count, buf);
- mutex_unlock(&max8997->iolock);
- if (ret < 0)
- return ret;
+static const struct regmap_config max8997_regmap_rtc_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX8997_RTC_REG_END,
+};
- return 0;
-}
-EXPORT_SYMBOL_GPL(max8997_bulk_write);
+static const struct regmap_config max8997_regmap_haptic_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX8997_HAPTIC_REG_END,
+};
-int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
-{
- struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- int ret;
-
- mutex_lock(&max8997->iolock);
- ret = i2c_smbus_read_byte_data(i2c, reg);
- if (ret >= 0) {
- u8 old_val = ret & 0xff;
- u8 new_val = (val & mask) | (old_val & (~mask));
- ret = i2c_smbus_write_byte_data(i2c, reg, new_val);
- }
- mutex_unlock(&max8997->iolock);
- return ret;
-}
-EXPORT_SYMBOL_GPL(max8997_update_reg);
+static const struct regmap_config max8997_regmap_muic_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX8997_MUIC_REG_END,
+};
/*
* Only the common platform data elements for max8997 are parsed here from the
@@ -230,6 +179,41 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
}
i2c_set_clientdata(max8997->muic, max8997);
+ max8997->regmap = devm_regmap_init_i2c(i2c, &max8997_regmap_config);
+ if (IS_ERR(max8997->regmap)) {
+ ret = PTR_ERR(max8997->regmap);
+ dev_err(max8997->dev,
+ "failed to allocate register map: %d\n", ret);
+ return ret;
+ }
+
+ max8997->regmap_rtc = devm_regmap_init_i2c(max8997->rtc,
+ &max8997_regmap_rtc_config);
+ if (IS_ERR(max8997->regmap_rtc)) {
+ ret = PTR_ERR(max8997->regmap_rtc);
+ dev_err(max8997->dev,
+ "failed to allocate register map: %d\n", ret);
+ goto err_regmap;
+ }
+
+ max8997->regmap_haptic = devm_regmap_init_i2c(max8997->haptic,
+ &max8997_regmap_haptic_config);
+ if (IS_ERR(max8997->regmap_haptic)) {
+ ret = PTR_ERR(max8997->regmap_haptic);
+ dev_err(max8997->dev,
+ "failed to allocate register map: %d\n", ret);
+ goto err_regmap;
+ }
+
+ max8997->regmap_muic = devm_regmap_init_i2c(max8997->muic,
+ &max8997_regmap_muic_config);
+ if (IS_ERR(max8997->regmap_muic)) {
+ ret = PTR_ERR(max8997->regmap_muic);
+ dev_err(max8997->dev,
+ "failed to allocate register map: %d\n", ret);
+ goto err_regmap;
+ }
+
pm_runtime_set_active(max8997->dev);
max8997_irq_init(max8997);
@@ -254,6 +238,7 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
err_mfd:
mfd_remove_devices(max8997->dev);
+err_regmap:
i2c_unregister_device(max8997->muic);
err_i2c_muic:
i2c_unregister_device(max8997->haptic);
@@ -441,15 +426,15 @@ static int max8997_freeze(struct device *dev)
int i;
for (i = 0; i < ARRAY_SIZE(max8997_dumpaddr_pmic); i++)
- max8997_read_reg(i2c, max8997_dumpaddr_pmic[i],
+ regmap_read(max8997->regmap, max8997_dumpaddr_pmic[i],
&max8997->reg_dump[i]);
for (i = 0; i < ARRAY_SIZE(max8997_dumpaddr_muic); i++)
- max8997_read_reg(i2c, max8997_dumpaddr_muic[i],
+ regmap_read(max8997->regmap_muic, max8997_dumpaddr_muic[i],
&max8997->reg_dump[i + MAX8997_REG_PMIC_END]);
for (i = 0; i < ARRAY_SIZE(max8997_dumpaddr_haptic); i++)
- max8997_read_reg(i2c, max8997_dumpaddr_haptic[i],
+ regmap_read(max8997->regmap_haptic, max8997_dumpaddr_haptic[i],
&max8997->reg_dump[i + MAX8997_REG_PMIC_END +
MAX8997_MUIC_REG_END]);
@@ -463,15 +448,15 @@ static int max8997_restore(struct device *dev)
int i;
for (i = 0; i < ARRAY_SIZE(max8997_dumpaddr_pmic); i++)
- max8997_write_reg(i2c, max8997_dumpaddr_pmic[i],
+ regmap_write(max8997->regmap, max8997_dumpaddr_pmic[i],
max8997->reg_dump[i]);
for (i = 0; i < ARRAY_SIZE(max8997_dumpaddr_muic); i++)
- max8997_write_reg(i2c, max8997_dumpaddr_muic[i],
+ regmap_write(max8997->regmap_muic, max8997_dumpaddr_muic[i],
max8997->reg_dump[i + MAX8997_REG_PMIC_END]);
for (i = 0; i < ARRAY_SIZE(max8997_dumpaddr_haptic); i++)
- max8997_write_reg(i2c, max8997_dumpaddr_haptic[i],
+ regmap_write(max8997->regmap_haptic, max8997_dumpaddr_haptic[i],
max8997->reg_dump[i + MAX8997_REG_PMIC_END +
MAX8997_MUIC_REG_END]);
diff --git a/drivers/power/max8997_charger.c b/drivers/power/max8997_charger.c
index 4bdedfe..43d8481 100644
--- a/drivers/power/max8997_charger.c
+++ b/drivers/power/max8997_charger.c
@@ -26,6 +26,7 @@
#include <linux/power_supply.h>
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
+#include <linux/regmap.h>
struct charger_data {
struct device *dev;
@@ -46,14 +47,14 @@ static int max8997_battery_get_property(struct power_supply *psy,
{
struct charger_data *charger = container_of(psy,
struct charger_data, battery);
- struct i2c_client *i2c = charger->iodev->i2c;
int ret;
- u8 reg;
+ unsigned int reg;
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
val->intval = 0;
- ret = max8997_read_reg(i2c, MAX8997_REG_STATUS4, ®);
+ ret = regmap_read(charger->iodev->regmap,
+ MAX8997_REG_STATUS4, ®);
if (ret)
return ret;
if ((reg & (1 << 0)) == 0x1)
@@ -62,7 +63,8 @@ static int max8997_battery_get_property(struct power_supply *psy,
break;
case POWER_SUPPLY_PROP_PRESENT:
val->intval = 0;
- ret = max8997_read_reg(i2c, MAX8997_REG_STATUS4, ®);
+ ret = regmap_read(charger->iodev->regmap,
+ MAX8997_REG_STATUS4, ®);
if (ret)
return ret;
if ((reg & (1 << 2)) == 0x0)
@@ -71,7 +73,8 @@ static int max8997_battery_get_property(struct power_supply *psy,
break;
case POWER_SUPPLY_PROP_ONLINE:
val->intval = 0;
- ret = max8997_read_reg(i2c, MAX8997_REG_STATUS4, ®);
+ ret = regmap_read(charger->iodev->regmap,
+ MAX8997_REG_STATUS4, ®);
if (ret)
return ret;
/* DCINOK */
@@ -103,8 +106,8 @@ static int max8997_battery_probe(struct platform_device *pdev)
if (val > 0xf)
val = 0xf;
- ret = max8997_update_reg(iodev->i2c,
- MAX8997_REG_MBCCTRL5, val, 0xf);
+ ret = regmap_update_bits(iodev->regmap,
+ MAX8997_REG_MBCCTRL5, 0xf, val);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot use i2c bus.\n");
return ret;
@@ -113,20 +116,20 @@ static int max8997_battery_probe(struct platform_device *pdev)
switch (pdata->timeout) {
case 5:
- ret = max8997_update_reg(iodev->i2c, MAX8997_REG_MBCCTRL1,
- 0x2 << 4, 0x7 << 4);
+ ret = regmap_update_bits(iodev->regmap,
+ MAX8997_REG_MBCCTRL1, 0x7 << 4, 0x2 << 4);
break;
case 6:
- ret = max8997_update_reg(iodev->i2c, MAX8997_REG_MBCCTRL1,
- 0x3 << 4, 0x7 << 4);
+ ret = regmap_update_bits(iodev->regmap,
+ MAX8997_REG_MBCCTRL1, 0x7 << 4, 0x3 << 4);
break;
case 7:
- ret = max8997_update_reg(iodev->i2c, MAX8997_REG_MBCCTRL1,
- 0x4 << 4, 0x7 << 4);
+ ret = regmap_update_bits(iodev->regmap,
+ MAX8997_REG_MBCCTRL1, 0x7 << 4, 0x4 << 4);
break;
case 0:
- ret = max8997_update_reg(iodev->i2c, MAX8997_REG_MBCCTRL1,
- 0x7 << 4, 0x7 << 4);
+ ret = regmap_update_bits(iodev->regmap,
+ MAX8997_REG_MBCCTRL1, 0x7 << 4, 0x7 << 4);
break;
default:
dev_err(&pdev->dev, "incorrect timeout value (%d)\n",
diff --git a/drivers/regulator/max8997.c b/drivers/regulator/max8997.c
index 9c31e21..faefd1c 100644
--- a/drivers/regulator/max8997.c
+++ b/drivers/regulator/max8997.c
@@ -33,6 +33,7 @@
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
#include <linux/regulator/of_regulator.h>
+#include <linux/regmap.h>
struct max8997_data {
struct device *dev;
@@ -50,7 +51,7 @@ struct max8997_data {
int buck125_gpioindex;
bool ignore_gpiodvs_side_effect;
- u8 saved_states[MAX8997_REG_MAX];
+ unsigned int saved_states[MAX8997_REG_MAX];
};
static const unsigned int safeoutvolt[] = {
@@ -257,15 +258,14 @@ static int max8997_get_enable_register(struct regulator_dev *rdev,
static int max8997_reg_is_enabled(struct regulator_dev *rdev)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int ret, reg, mask, pattern;
- u8 val;
+ unsigned int val;
ret = max8997_get_enable_register(rdev, ®, &mask, &pattern);
if (ret)
return ret;
- ret = max8997_read_reg(i2c, reg, &val);
+ ret = regmap_read(max8997->iodev->regmap, reg, &val);
if (ret)
return ret;
@@ -275,27 +275,25 @@ static int max8997_reg_is_enabled(struct regulator_dev *rdev)
static int max8997_reg_enable(struct regulator_dev *rdev)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int ret, reg, mask, pattern;
ret = max8997_get_enable_register(rdev, ®, &mask, &pattern);
if (ret)
return ret;
- return max8997_update_reg(i2c, reg, pattern, mask);
+ return regmap_update_bits(max8997->iodev->regmap, reg, mask, pattern);
}
static int max8997_reg_disable(struct regulator_dev *rdev)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int ret, reg, mask, pattern;
ret = max8997_get_enable_register(rdev, ®, &mask, &pattern);
if (ret)
return ret;
- return max8997_update_reg(i2c, reg, ~pattern, mask);
+ return regmap_update_bits(max8997->iodev->regmap, reg, mask, ~pattern);
}
static int max8997_get_voltage_register(struct regulator_dev *rdev,
@@ -367,15 +365,14 @@ static int max8997_get_voltage_register(struct regulator_dev *rdev,
static int max8997_get_voltage_sel(struct regulator_dev *rdev)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int reg, shift, mask, ret;
- u8 val;
+ unsigned int val;
ret = max8997_get_voltage_register(rdev, ®, &shift, &mask);
if (ret)
return ret;
- ret = max8997_read_reg(i2c, reg, &val);
+ ret = regmap_read(max8997->iodev->regmap, reg, &val);
if (ret)
return ret;
@@ -412,7 +409,6 @@ static int max8997_set_voltage_charger_cv(struct regulator_dev *rdev,
int min_uV, int max_uV, unsigned *selector)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int rid = rdev_get_id(rdev);
int lb, ub;
int reg, shift = 0, mask, ret = 0;
@@ -454,7 +450,8 @@ static int max8997_set_voltage_charger_cv(struct regulator_dev *rdev,
*selector = val;
- ret = max8997_update_reg(i2c, reg, val << shift, mask);
+ ret = regmap_update_bits(max8997->iodev->regmap,
+ reg, mask, val << shift);
return ret;
}
@@ -467,7 +464,6 @@ static int max8997_set_voltage_ldobuck(struct regulator_dev *rdev,
int min_uV, int max_uV, unsigned *selector)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
const struct voltage_map_desc *desc;
int rid = rdev_get_id(rdev);
int i, reg, shift, mask, ret;
@@ -499,7 +495,8 @@ static int max8997_set_voltage_ldobuck(struct regulator_dev *rdev,
if (ret)
return ret;
- ret = max8997_update_reg(i2c, reg, i << shift, mask << shift);
+ ret = regmap_update_bits(max8997->iodev->regmap,
+ reg, mask << shift, i << shift);
*selector = i;
return ret;
@@ -709,7 +706,6 @@ static int max8997_set_voltage_safeout_sel(struct regulator_dev *rdev,
unsigned selector)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int rid = rdev_get_id(rdev);
int reg, shift = 0, mask, ret;
@@ -720,13 +716,13 @@ static int max8997_set_voltage_safeout_sel(struct regulator_dev *rdev,
if (ret)
return ret;
- return max8997_update_reg(i2c, reg, selector << shift, mask << shift);
+ return regmap_update_bits(max8997->iodev->regmap,
+ reg, mask << shift, selector << shift);
}
static int max8997_reg_disable_suspend(struct regulator_dev *rdev)
{
struct max8997_data *max8997 = rdev_get_drvdata(rdev);
- struct i2c_client *i2c = max8997->iodev->i2c;
int ret, reg, mask, pattern;
int rid = rdev_get_id(rdev);
@@ -734,20 +730,22 @@ static int max8997_reg_disable_suspend(struct regulator_dev *rdev)
if (ret)
return ret;
- max8997_read_reg(i2c, reg, &max8997->saved_states[rid]);
+ regmap_read(max8997->iodev->regmap,
+ reg, &max8997->saved_states[rid]);
if (rid == MAX8997_LDO1 ||
rid == MAX8997_LDO10 ||
rid == MAX8997_LDO21) {
dev_dbg(&rdev->dev, "Conditional Power-Off for %s\n",
rdev->desc->name);
- return max8997_update_reg(i2c, reg, 0x40, mask);
+ return regmap_update_bits(max8997->iodev->regmap,
+ reg, mask, 0x40);
}
dev_dbg(&rdev->dev, "Full Power-Off for %s (%xh -> %xh)\n",
rdev->desc->name, max8997->saved_states[rid] & mask,
(~pattern) & mask);
- return max8997_update_reg(i2c, reg, ~pattern, mask);
+ return regmap_update_bits(max8997->iodev->regmap, reg, mask, ~pattern);
}
static struct regulator_ops max8997_ldo_ops = {
@@ -1030,7 +1028,6 @@ static int max8997_pmic_probe(struct platform_device *pdev)
struct regulator_config config = { };
struct regulator_dev *rdev;
struct max8997_data *max8997;
- struct i2c_client *i2c;
int i, ret, nr_dvs;
u8 max_buck1 = 0, max_buck2 = 0, max_buck5 = 0;
@@ -1054,7 +1051,6 @@ static int max8997_pmic_probe(struct platform_device *pdev)
max8997->iodev = iodev;
max8997->num_regulators = pdata->num_regulators;
platform_set_drvdata(pdev, max8997);
- i2c = max8997->iodev->i2c;
max8997->buck125_gpioindex = pdata->buck125_default_idx;
max8997->buck1_gpiodvs = pdata->buck1_gpiodvs;
@@ -1104,25 +1100,25 @@ static int max8997_pmic_probe(struct platform_device *pdev)
/* For the safety, set max voltage before setting up */
for (i = 0; i < 8; i++) {
- max8997_update_reg(i2c, MAX8997_REG_BUCK1DVS1 + i,
- max_buck1, 0x3f);
- max8997_update_reg(i2c, MAX8997_REG_BUCK2DVS1 + i,
- max_buck2, 0x3f);
- max8997_update_reg(i2c, MAX8997_REG_BUCK5DVS1 + i,
- max_buck5, 0x3f);
+ regmap_update_bits(max8997->iodev->regmap,
+ MAX8997_REG_BUCK1DVS1 + i, 0x3f, max_buck1);
+ regmap_update_bits(max8997->iodev->regmap,
+ MAX8997_REG_BUCK2DVS1 + i, 0x3f, max_buck2);
+ regmap_update_bits(max8997->iodev->regmap,
+ MAX8997_REG_BUCK5DVS1 + i, 0x3f, max_buck5);
}
/* Initialize all the DVS related BUCK registers */
for (i = 0; i < nr_dvs; i++) {
- max8997_update_reg(i2c, MAX8997_REG_BUCK1DVS1 + i,
- max8997->buck1_vol[i],
- 0x3f);
- max8997_update_reg(i2c, MAX8997_REG_BUCK2DVS1 + i,
- max8997->buck2_vol[i],
- 0x3f);
- max8997_update_reg(i2c, MAX8997_REG_BUCK5DVS1 + i,
- max8997->buck5_vol[i],
- 0x3f);
+ regmap_update_bits(max8997->iodev->regmap,
+ MAX8997_REG_BUCK1DVS1 + i,
+ 0x3f, max8997->buck1_vol[i]);
+ regmap_update_bits(max8997->iodev->regmap,
+ MAX8997_REG_BUCK2DVS1 + i,
+ 0x3f, max8997->buck2_vol[i]);
+ regmap_update_bits(max8997->iodev->regmap,
+ MAX8997_REG_BUCK5DVS1 + i,
+ 0x3f, max8997->buck5_vol[i]);
}
/*
@@ -1166,16 +1162,17 @@ static int max8997_pmic_probe(struct platform_device *pdev)
}
/* DVS-GPIO disabled */
- max8997_update_reg(i2c, MAX8997_REG_BUCK1CTRL, (pdata->buck1_gpiodvs) ?
- (1 << 1) : (0 << 1), 1 << 1);
- max8997_update_reg(i2c, MAX8997_REG_BUCK2CTRL, (pdata->buck2_gpiodvs) ?
- (1 << 1) : (0 << 1), 1 << 1);
- max8997_update_reg(i2c, MAX8997_REG_BUCK5CTRL, (pdata->buck5_gpiodvs) ?
- (1 << 1) : (0 << 1), 1 << 1);
+ regmap_update_bits(max8997->iodev->regmap, MAX8997_REG_BUCK1CTRL,
+ 1 << 1, (pdata->buck1_gpiodvs) ? (1 << 1) : (0 << 1));
+ regmap_update_bits(max8997->iodev->regmap, MAX8997_REG_BUCK2CTRL,
+ 1 << 1, (pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1));
+ regmap_update_bits(max8997->iodev->regmap, MAX8997_REG_BUCK5CTRL,
+ 1 << 1, (pdata->buck5_gpiodvs) ? (1 << 1) : (0 << 1));
/* Misc Settings */
max8997->ramp_delay = 10; /* set 10mV/us, which is the default */
- max8997_write_reg(i2c, MAX8997_REG_BUCKRAMP, (0xf << 4) | 0x9);
+ regmap_write(max8997->iodev->regmap,
+ MAX8997_REG_BUCKRAMP, (0xf << 4) | 0x9);
for (i = 0; i < pdata->num_regulators; i++) {
const struct voltage_map_desc *desc;
diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
index 0777c01..b866f7d5 100644
--- a/drivers/rtc/rtc-max8997.c
+++ b/drivers/rtc/rtc-max8997.c
@@ -20,6 +20,7 @@
#include <linux/platform_device.h>
#include <linux/mfd/max8997-private.h>
#include <linux/irqdomain.h>
+#include <linux/regmap.h>
/* Module parameter for WTSR function control */
static int wtsr_en = 1;
@@ -68,7 +69,6 @@ enum {
struct max8997_rtc_info {
struct device *dev;
struct max8997_dev *max8997;
- struct i2c_client *rtc;
struct rtc_device *rtc_dev;
struct mutex lock;
int virq;
@@ -118,8 +118,8 @@ static inline int max8997_rtc_set_update_reg(struct max8997_rtc_info *info)
{
int ret;
- ret = max8997_write_reg(info->rtc, MAX8997_RTC_UPDATE1,
- RTC_UDR_MASK);
+ ret = regmap_write(info->max8997->regmap_rtc,
+ MAX8997_RTC_UPDATE1, RTC_UDR_MASK);
if (ret < 0)
dev_err(info->dev, "%s: fail to write update reg(%d)\n",
__func__, ret);
@@ -140,7 +140,8 @@ static int max8997_rtc_read_time(struct device *dev, struct rtc_time *tm)
int ret;
mutex_lock(&info->lock);
- ret = max8997_bulk_read(info->rtc, MAX8997_RTC_SEC, RTC_NR_TIME, data);
+ ret = regmap_bulk_read(info->max8997->regmap_rtc,
+ MAX8997_RTC_SEC, data, RTC_NR_TIME);
mutex_unlock(&info->lock);
if (ret < 0) {
@@ -166,7 +167,8 @@ static int max8997_rtc_set_time(struct device *dev, struct rtc_time *tm)
mutex_lock(&info->lock);
- ret = max8997_bulk_write(info->rtc, MAX8997_RTC_SEC, RTC_NR_TIME, data);
+ ret = regmap_bulk_write(info->max8997->regmap_rtc,
+ MAX8997_RTC_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s: fail to write time reg(%d)\n", __func__,
ret);
@@ -183,13 +185,13 @@ static int max8997_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
{
struct max8997_rtc_info *info = dev_get_drvdata(dev);
u8 data[RTC_NR_TIME];
- u8 val;
+ unsigned int val;
int i, ret;
mutex_lock(&info->lock);
- ret = max8997_bulk_read(info->rtc, MAX8997_RTC_ALARM1_SEC, RTC_NR_TIME,
- data);
+ ret = regmap_bulk_read(info->max8997->regmap_rtc,
+ MAX8997_RTC_ALARM1_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s:%d fail to read alarm reg(%d)\n",
__func__, __LINE__, ret);
@@ -207,7 +209,8 @@ static int max8997_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
}
alrm->pending = 0;
- ret = max8997_read_reg(info->max8997->i2c, MAX8997_REG_STATUS1, &val);
+ ret = regmap_read(info->max8997->regmap_rtc,
+ MAX8997_REG_STATUS1, &val);
if (ret < 0) {
dev_err(info->dev, "%s:%d fail to read status1 reg(%d)\n",
__func__, __LINE__, ret);
@@ -230,8 +233,8 @@ static int max8997_rtc_stop_alarm(struct max8997_rtc_info *info)
if (!mutex_is_locked(&info->lock))
dev_warn(info->dev, "%s: should have mutex locked\n", __func__);
- ret = max8997_bulk_read(info->rtc, MAX8997_RTC_ALARM1_SEC, RTC_NR_TIME,
- data);
+ ret = regmap_bulk_read(info->max8997->regmap_rtc,
+ MAX8997_RTC_ALARM1_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s: fail to read alarm reg(%d)\n",
__func__, ret);
@@ -241,8 +244,8 @@ static int max8997_rtc_stop_alarm(struct max8997_rtc_info *info)
for (i = 0; i < RTC_NR_TIME; i++)
data[i] &= ~ALARM_ENABLE_MASK;
- ret = max8997_bulk_write(info->rtc, MAX8997_RTC_ALARM1_SEC, RTC_NR_TIME,
- data);
+ ret = regmap_bulk_write(info->max8997->regmap_rtc,
+ MAX8997_RTC_ALARM1_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s: fail to write alarm reg(%d)\n",
__func__, ret);
@@ -262,8 +265,8 @@ static int max8997_rtc_start_alarm(struct max8997_rtc_info *info)
if (!mutex_is_locked(&info->lock))
dev_warn(info->dev, "%s: should have mutex locked\n", __func__);
- ret = max8997_bulk_read(info->rtc, MAX8997_RTC_ALARM1_SEC, RTC_NR_TIME,
- data);
+ ret = regmap_bulk_read(info->max8997->regmap_rtc,
+ MAX8997_RTC_ALARM1_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s: fail to read alarm reg(%d)\n",
__func__, ret);
@@ -281,8 +284,8 @@ static int max8997_rtc_start_alarm(struct max8997_rtc_info *info)
if (data[RTC_DATE] & 0x1f)
data[RTC_DATE] |= (1 << ALARM_ENABLE_SHIFT);
- ret = max8997_bulk_write(info->rtc, MAX8997_RTC_ALARM1_SEC, RTC_NR_TIME,
- data);
+ ret = regmap_bulk_write(info->max8997->regmap_rtc,
+ MAX8997_RTC_ALARM1_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s: fail to write alarm reg(%d)\n",
__func__, ret);
@@ -313,8 +316,8 @@ static int max8997_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
if (ret < 0)
goto out;
- ret = max8997_bulk_write(info->rtc, MAX8997_RTC_ALARM1_SEC, RTC_NR_TIME,
- data);
+ ret = regmap_bulk_write(info->max8997->regmap_rtc,
+ MAX8997_RTC_ALARM1_SEC, data, RTC_NR_TIME);
if (ret < 0) {
dev_err(info->dev, "%s: fail to write alarm reg(%d)\n",
__func__, ret);
@@ -385,7 +388,8 @@ static void max8997_rtc_enable_wtsr(struct max8997_rtc_info *info, bool enable)
dev_info(info->dev, "%s: %s WTSR\n", __func__,
enable ? "enable" : "disable");
- ret = max8997_update_reg(info->rtc, MAX8997_RTC_WTSR_SMPL, val, mask);
+ ret = regmap_update_bits(info->max8997->regmap_rtc,
+ MAX8997_RTC_WTSR_SMPL, mask, val);
if (ret < 0) {
dev_err(info->dev, "%s: fail to update WTSR reg(%d)\n",
__func__, ret);
@@ -398,7 +402,7 @@ static void max8997_rtc_enable_wtsr(struct max8997_rtc_info *info, bool enable)
static void max8997_rtc_enable_smpl(struct max8997_rtc_info *info, bool enable)
{
int ret;
- u8 val, mask;
+ unsigned int val, mask;
if (!smpl_en)
return;
@@ -413,7 +417,8 @@ static void max8997_rtc_enable_smpl(struct max8997_rtc_info *info, bool enable)
dev_info(info->dev, "%s: %s SMPL\n", __func__,
enable ? "enable" : "disable");
- ret = max8997_update_reg(info->rtc, MAX8997_RTC_WTSR_SMPL, val, mask);
+ ret = regmap_update_bits(info->max8997->regmap_rtc,
+ MAX8997_RTC_WTSR_SMPL, mask, val);
if (ret < 0) {
dev_err(info->dev, "%s: fail to update SMPL reg(%d)\n",
__func__, ret);
@@ -423,7 +428,8 @@ static void max8997_rtc_enable_smpl(struct max8997_rtc_info *info, bool enable)
max8997_rtc_set_update_reg(info);
val = 0;
- max8997_read_reg(info->rtc, MAX8997_RTC_WTSR_SMPL, &val);
+ regmap_read(info->max8997->regmap_rtc,
+ MAX8997_RTC_WTSR_SMPL, &val);
pr_info("%s: WTSR_SMPL(0x%02x)\n", __func__, val);
}
@@ -438,7 +444,8 @@ static int max8997_rtc_init_reg(struct max8997_rtc_info *info)
info->rtc_24hr_mode = 1;
- ret = max8997_bulk_write(info->rtc, MAX8997_RTC_CTRLMASK, 2, data);
+ ret = regmap_bulk_write(info->max8997->regmap_rtc,
+ MAX8997_RTC_CTRLMASK, data, 2);
if (ret < 0) {
dev_err(info->dev, "%s: fail to write controlm reg(%d)\n",
__func__, ret);
@@ -463,7 +470,6 @@ static int max8997_rtc_probe(struct platform_device *pdev)
mutex_init(&info->lock);
info->dev = &pdev->dev;
info->max8997 = max8997;
- info->rtc = max8997->rtc;
platform_set_drvdata(pdev, info);
diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h
index 78c76cd..ea80ef8 100644
--- a/include/linux/mfd/max8997-private.h
+++ b/include/linux/mfd/max8997-private.h
@@ -309,6 +309,8 @@ enum max8997_rtc_reg {
MAX8997_RTC_ALARM2_MONTH = 0x22,
MAX8997_RTC_ALARM2_YEAR = 0x23,
MAX8997_RTC_ALARM2_DAY_OF_MONTH = 0x24,
+
+ MAX8997_RTC_REG_END = 0x25,
};
enum max8997_irq_source {
@@ -390,6 +392,11 @@ struct max8997_dev {
unsigned long type;
struct platform_device *battery; /* battery control (not fuel gauge) */
+ struct regmap *regmap;
+ struct regmap *regmap_rtc;
+ struct regmap *regmap_haptic;
+ struct regmap *regmap_muic;
+
int irq;
int ono;
struct irq_domain *irq_domain;
@@ -398,7 +405,7 @@ struct max8997_dev {
int irq_masks_cache[MAX8997_IRQ_GROUP_NR];
/* For hibernation */
- u8 reg_dump[MAX8997_REG_PMIC_END + MAX8997_MUIC_REG_END +
+ unsigned int reg_dump[MAX8997_REG_PMIC_END + MAX8997_MUIC_REG_END +
MAX8997_HAPTIC_REG_END];
bool gpio_status[MAX8997_NUM_GPIO];
@@ -413,14 +420,6 @@ extern int max8997_irq_init(struct max8997_dev *max8997);
extern void max8997_irq_exit(struct max8997_dev *max8997);
extern int max8997_irq_resume(struct max8997_dev *max8997);
-extern int max8997_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest);
-extern int max8997_bulk_read(struct i2c_client *i2c, u8 reg, int count,
- u8 *buf);
-extern int max8997_write_reg(struct i2c_client *i2c, u8 reg, u8 value);
-extern int max8997_bulk_write(struct i2c_client *i2c, u8 reg, int count,
- u8 *buf);
-extern int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask);
-
#define MAX8997_GPIO_INT_BOTH (0x3 << 4)
#define MAX8997_GPIO_INT_RISE (0x2 << 4)
#define MAX8997_GPIO_INT_FALL (0x1 << 4)
--
1.9.1
^ permalink raw reply related
* [PATCH v5 2/3] mfd: max8997: handle IRQs using regmap
From: Robert Baldyga @ 2014-11-12 7:23 UTC (permalink / raw)
To: lee.jones
Cc: sameo, myungjoo.ham, cw00.choi, dmitry.torokhov, cooloney,
rpurdie, sre, dbaryshkov, dwmw2, lgirdwood, broonie, a.zummo,
paul.gortmaker, sachin.kamat, k.kozlowski, linux-kernel,
linux-input, linux-leds, linux-pm, rtc-linux, Robert Baldyga
In-Reply-To: <1415776996-11569-1-git-send-email-r.baldyga@samsung.com>
This patch modifies mfd driver to use regmap for handling interrupts.
It allows to simplify irq handling process. This modifications needed
to make small changes in function drivers, which use interrupts.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
[For extcon part]
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
[For the mfd part]
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/extcon/extcon-max8997.c | 3 +-
drivers/mfd/Kconfig | 2 +-
drivers/mfd/Makefile | 2 +-
drivers/mfd/max8997-irq.c | 373 ------------------------------------
drivers/mfd/max8997.c | 112 ++++++++++-
drivers/rtc/rtc-max8997.c | 2 +-
include/linux/mfd/max8997-private.h | 63 +++++-
7 files changed, 165 insertions(+), 392 deletions(-)
delete mode 100644 drivers/mfd/max8997-irq.c
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 2aafd5b..172dbd6 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -677,7 +677,8 @@ static int max8997_muic_probe(struct platform_device *pdev)
struct max8997_muic_irq *muic_irq = &muic_irqs[i];
unsigned int virq = 0;
- virq = irq_create_mapping(max8997->irq_domain, muic_irq->irq);
+ virq = regmap_irq_get_virq(max8997->irq_data_muic,
+ muic_irq->irq);
if (!virq) {
ret = -EINVAL;
goto err_irq;
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ca57035..8cfc11b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -464,7 +464,7 @@ config MFD_MAX8997
depends on I2C=y
select MFD_CORE
select REGMAP_I2C
- select IRQ_DOMAIN
+ select REGMAP_IRQ
help
Say yes here to add support for Maxim Semiconductor MAX8997/8966.
This is a Power Management IC with RTC, Flash, Fuel Gauge, Haptic,
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 53467e2..e043ee4 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -120,7 +120,7 @@ obj-$(CONFIG_MFD_MAX77693) += max77693.o
obj-$(CONFIG_MFD_MAX8907) += max8907.o
max8925-objs := max8925-core.o max8925-i2c.o
obj-$(CONFIG_MFD_MAX8925) += max8925.o
-obj-$(CONFIG_MFD_MAX8997) += max8997.o max8997-irq.o
+obj-$(CONFIG_MFD_MAX8997) += max8997.o
obj-$(CONFIG_MFD_MAX8998) += max8998.o max8998-irq.o
pcf50633-objs := pcf50633-core.o pcf50633-irq.o
diff --git a/drivers/mfd/max8997-irq.c b/drivers/mfd/max8997-irq.c
deleted file mode 100644
index 0e7ff39..0000000
--- a/drivers/mfd/max8997-irq.c
+++ /dev/null
@@ -1,373 +0,0 @@
-/*
- * max8997-irq.c - Interrupt controller support for MAX8997
- *
- * Copyright (C) 2011 Samsung Electronics Co.Ltd
- * MyungJoo Ham <myungjoo.ham@samsung.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * This driver is based on max8998-irq.c
- */
-
-#include <linux/err.h>
-#include <linux/irq.h>
-#include <linux/interrupt.h>
-#include <linux/mfd/max8997.h>
-#include <linux/mfd/max8997-private.h>
-#include <linux/regmap.h>
-
-static const u8 max8997_mask_reg[] = {
- [PMIC_INT1] = MAX8997_REG_INT1MSK,
- [PMIC_INT2] = MAX8997_REG_INT2MSK,
- [PMIC_INT3] = MAX8997_REG_INT3MSK,
- [PMIC_INT4] = MAX8997_REG_INT4MSK,
- [FUEL_GAUGE] = MAX8997_REG_INVALID,
- [MUIC_INT1] = MAX8997_MUIC_REG_INTMASK1,
- [MUIC_INT2] = MAX8997_MUIC_REG_INTMASK2,
- [MUIC_INT3] = MAX8997_MUIC_REG_INTMASK3,
- [GPIO_LOW] = MAX8997_REG_INVALID,
- [GPIO_HI] = MAX8997_REG_INVALID,
- [FLASH_STATUS] = MAX8997_REG_INVALID,
-};
-
-struct max8997_irq_data {
- int mask;
- enum max8997_irq_source group;
-};
-
-#define DECLARE_IRQ(idx, _group, _mask) \
- [(idx)] = { .group = (_group), .mask = (_mask) }
-static const struct max8997_irq_data max8997_irqs[] = {
- DECLARE_IRQ(MAX8997_PMICIRQ_PWRONR, PMIC_INT1, 1 << 0),
- DECLARE_IRQ(MAX8997_PMICIRQ_PWRONF, PMIC_INT1, 1 << 1),
- DECLARE_IRQ(MAX8997_PMICIRQ_PWRON1SEC, PMIC_INT1, 1 << 3),
- DECLARE_IRQ(MAX8997_PMICIRQ_JIGONR, PMIC_INT1, 1 << 4),
- DECLARE_IRQ(MAX8997_PMICIRQ_JIGONF, PMIC_INT1, 1 << 5),
- DECLARE_IRQ(MAX8997_PMICIRQ_LOWBAT2, PMIC_INT1, 1 << 6),
- DECLARE_IRQ(MAX8997_PMICIRQ_LOWBAT1, PMIC_INT1, 1 << 7),
-
- DECLARE_IRQ(MAX8997_PMICIRQ_JIGR, PMIC_INT2, 1 << 0),
- DECLARE_IRQ(MAX8997_PMICIRQ_JIGF, PMIC_INT2, 1 << 1),
- DECLARE_IRQ(MAX8997_PMICIRQ_MR, PMIC_INT2, 1 << 2),
- DECLARE_IRQ(MAX8997_PMICIRQ_DVS1OK, PMIC_INT2, 1 << 3),
- DECLARE_IRQ(MAX8997_PMICIRQ_DVS2OK, PMIC_INT2, 1 << 4),
- DECLARE_IRQ(MAX8997_PMICIRQ_DVS3OK, PMIC_INT2, 1 << 5),
- DECLARE_IRQ(MAX8997_PMICIRQ_DVS4OK, PMIC_INT2, 1 << 6),
-
- DECLARE_IRQ(MAX8997_PMICIRQ_CHGINS, PMIC_INT3, 1 << 0),
- DECLARE_IRQ(MAX8997_PMICIRQ_CHGRM, PMIC_INT3, 1 << 1),
- DECLARE_IRQ(MAX8997_PMICIRQ_DCINOVP, PMIC_INT3, 1 << 2),
- DECLARE_IRQ(MAX8997_PMICIRQ_TOPOFFR, PMIC_INT3, 1 << 3),
- DECLARE_IRQ(MAX8997_PMICIRQ_CHGRSTF, PMIC_INT3, 1 << 5),
- DECLARE_IRQ(MAX8997_PMICIRQ_MBCHGTMEXPD, PMIC_INT3, 1 << 7),
-
- DECLARE_IRQ(MAX8997_PMICIRQ_RTC60S, PMIC_INT4, 1 << 0),
- DECLARE_IRQ(MAX8997_PMICIRQ_RTCA1, PMIC_INT4, 1 << 1),
- DECLARE_IRQ(MAX8997_PMICIRQ_RTCA2, PMIC_INT4, 1 << 2),
- DECLARE_IRQ(MAX8997_PMICIRQ_SMPL_INT, PMIC_INT4, 1 << 3),
- DECLARE_IRQ(MAX8997_PMICIRQ_RTC1S, PMIC_INT4, 1 << 4),
- DECLARE_IRQ(MAX8997_PMICIRQ_WTSR, PMIC_INT4, 1 << 5),
-
- DECLARE_IRQ(MAX8997_MUICIRQ_ADCError, MUIC_INT1, 1 << 2),
- DECLARE_IRQ(MAX8997_MUICIRQ_ADCLow, MUIC_INT1, 1 << 1),
- DECLARE_IRQ(MAX8997_MUICIRQ_ADC, MUIC_INT1, 1 << 0),
-
- DECLARE_IRQ(MAX8997_MUICIRQ_VBVolt, MUIC_INT2, 1 << 4),
- DECLARE_IRQ(MAX8997_MUICIRQ_DBChg, MUIC_INT2, 1 << 3),
- DECLARE_IRQ(MAX8997_MUICIRQ_DCDTmr, MUIC_INT2, 1 << 2),
- DECLARE_IRQ(MAX8997_MUICIRQ_ChgDetRun, MUIC_INT2, 1 << 1),
- DECLARE_IRQ(MAX8997_MUICIRQ_ChgTyp, MUIC_INT2, 1 << 0),
-
- DECLARE_IRQ(MAX8997_MUICIRQ_OVP, MUIC_INT3, 1 << 2),
-};
-
-static void max8997_irq_lock(struct irq_data *data)
-{
- struct max8997_dev *max8997 = irq_get_chip_data(data->irq);
-
- mutex_lock(&max8997->irqlock);
-}
-
-static void max8997_irq_sync_unlock(struct irq_data *data)
-{
- struct max8997_dev *max8997 = irq_get_chip_data(data->irq);
- int i;
-
- for (i = 0; i < MAX8997_IRQ_GROUP_NR; i++) {
- struct regmap *map;
- u8 mask_reg = max8997_mask_reg[i];
-
- if (i >= MUIC_INT1 && i <= MUIC_INT3)
- map = max8997->regmap_muic;
- else
- map = max8997->regmap;
-
- if (mask_reg == MAX8997_REG_INVALID ||
- IS_ERR_OR_NULL(map))
- continue;
- max8997->irq_masks_cache[i] = max8997->irq_masks_cur[i];
-
- regmap_write(map, max8997_mask_reg[i],
- max8997->irq_masks_cur[i]);
- }
-
- mutex_unlock(&max8997->irqlock);
-}
-
-static const inline struct max8997_irq_data *
-irq_to_max8997_irq(struct max8997_dev *max8997, int irq)
-{
- struct irq_data *data = irq_get_irq_data(irq);
- return &max8997_irqs[data->hwirq];
-}
-
-static void max8997_irq_mask(struct irq_data *data)
-{
- struct max8997_dev *max8997 = irq_get_chip_data(data->irq);
- const struct max8997_irq_data *irq_data = irq_to_max8997_irq(max8997,
- data->irq);
-
- max8997->irq_masks_cur[irq_data->group] |= irq_data->mask;
-}
-
-static void max8997_irq_unmask(struct irq_data *data)
-{
- struct max8997_dev *max8997 = irq_get_chip_data(data->irq);
- const struct max8997_irq_data *irq_data = irq_to_max8997_irq(max8997,
- data->irq);
-
- max8997->irq_masks_cur[irq_data->group] &= ~irq_data->mask;
-}
-
-static struct irq_chip max8997_irq_chip = {
- .name = "max8997",
- .irq_bus_lock = max8997_irq_lock,
- .irq_bus_sync_unlock = max8997_irq_sync_unlock,
- .irq_mask = max8997_irq_mask,
- .irq_unmask = max8997_irq_unmask,
-};
-
-#define MAX8997_IRQSRC_PMIC (1 << 1)
-#define MAX8997_IRQSRC_FUELGAUGE (1 << 2)
-#define MAX8997_IRQSRC_MUIC (1 << 3)
-#define MAX8997_IRQSRC_GPIO (1 << 4)
-#define MAX8997_IRQSRC_FLASH (1 << 5)
-static irqreturn_t max8997_irq_thread(int irq, void *data)
-{
- struct max8997_dev *max8997 = data;
- u8 irq_reg[MAX8997_IRQ_GROUP_NR] = {};
- unsigned int irq_src;
- int ret;
- int i, cur_irq;
-
- ret = regmap_read(max8997->regmap, MAX8997_REG_INTSRC, &irq_src);
- if (ret < 0) {
- dev_err(max8997->dev, "Failed to read interrupt source: %d\n",
- ret);
- return IRQ_NONE;
- }
-
- if (irq_src & MAX8997_IRQSRC_PMIC) {
- /* PMIC INT1 ~ INT4 */
- regmap_bulk_read(max8997->regmap, MAX8997_REG_INT1,
- &irq_reg[PMIC_INT1], 4);
- }
- if (irq_src & MAX8997_IRQSRC_FUELGAUGE) {
- /*
- * TODO: FUEL GAUGE
- *
- * This is to be supported by Max17042 driver. When
- * an interrupt incurs here, it should be relayed to a
- * Max17042 device that is connected (probably by
- * platform-data). However, we do not have interrupt
- * handling in Max17042 driver currently. The Max17042 IRQ
- * driver should be ready to be used as a stand-alone device and
- * a Max8997-dependent device. Because it is not ready in
- * Max17042-side and it is not too critical in operating
- * Max8997, we do not implement this in initial releases.
- */
- irq_reg[FUEL_GAUGE] = 0;
- }
- if (irq_src & MAX8997_IRQSRC_MUIC) {
- /* MUIC INT1 ~ INT3 */
- regmap_bulk_read(max8997->regmap_muic, MAX8997_MUIC_REG_INT1,
- &irq_reg[MUIC_INT1], 3);
- }
- if (irq_src & MAX8997_IRQSRC_GPIO) {
- /* GPIO Interrupt */
- u8 gpio_info[MAX8997_NUM_GPIO];
-
- irq_reg[GPIO_LOW] = 0;
- irq_reg[GPIO_HI] = 0;
-
- regmap_bulk_read(max8997->regmap, MAX8997_REG_GPIOCNTL1,
- gpio_info, MAX8997_NUM_GPIO);
- for (i = 0; i < MAX8997_NUM_GPIO; i++) {
- bool interrupt = false;
-
- switch (gpio_info[i] & MAX8997_GPIO_INT_MASK) {
- case MAX8997_GPIO_INT_BOTH:
- if (max8997->gpio_status[i] != gpio_info[i])
- interrupt = true;
- break;
- case MAX8997_GPIO_INT_RISE:
- if ((max8997->gpio_status[i] != gpio_info[i]) &&
- (gpio_info[i] & MAX8997_GPIO_DATA_MASK))
- interrupt = true;
- break;
- case MAX8997_GPIO_INT_FALL:
- if ((max8997->gpio_status[i] != gpio_info[i]) &&
- !(gpio_info[i] & MAX8997_GPIO_DATA_MASK))
- interrupt = true;
- break;
- default:
- break;
- }
-
- if (interrupt) {
- if (i < 8)
- irq_reg[GPIO_LOW] |= (1 << i);
- else
- irq_reg[GPIO_HI] |= (1 << (i - 8));
- }
-
- }
- }
- if (irq_src & MAX8997_IRQSRC_FLASH) {
- /* Flash Status Interrupt */
- unsigned int data;
- ret = regmap_read(max8997->regmap,
- MAX8997_REG_FLASHSTATUS, &data);
- irq_reg[FLASH_STATUS] = data;
- }
-
- /* Apply masking */
- for (i = 0; i < MAX8997_IRQ_GROUP_NR; i++)
- irq_reg[i] &= ~max8997->irq_masks_cur[i];
-
- /* Report */
- for (i = 0; i < MAX8997_IRQ_NR; i++) {
- if (irq_reg[max8997_irqs[i].group] & max8997_irqs[i].mask) {
- cur_irq = irq_find_mapping(max8997->irq_domain, i);
- if (cur_irq)
- handle_nested_irq(cur_irq);
- }
- }
-
- return IRQ_HANDLED;
-}
-
-int max8997_irq_resume(struct max8997_dev *max8997)
-{
- if (max8997->irq && max8997->irq_domain)
- max8997_irq_thread(0, max8997);
- return 0;
-}
-
-static int max8997_irq_domain_map(struct irq_domain *d, unsigned int irq,
- irq_hw_number_t hw)
-{
- struct max8997_dev *max8997 = d->host_data;
-
- irq_set_chip_data(irq, max8997);
- irq_set_chip_and_handler(irq, &max8997_irq_chip, handle_edge_irq);
- irq_set_nested_thread(irq, 1);
-#ifdef CONFIG_ARM
- set_irq_flags(irq, IRQF_VALID);
-#else
- irq_set_noprobe(irq);
-#endif
- return 0;
-}
-
-static struct irq_domain_ops max8997_irq_domain_ops = {
- .map = max8997_irq_domain_map,
-};
-
-int max8997_irq_init(struct max8997_dev *max8997)
-{
- struct irq_domain *domain;
- int i;
- int ret;
- unsigned int val;
-
- if (!max8997->irq) {
- dev_warn(max8997->dev, "No interrupt specified.\n");
- return 0;
- }
-
- mutex_init(&max8997->irqlock);
-
- /* Mask individual interrupt sources */
- for (i = 0; i < MAX8997_IRQ_GROUP_NR; i++) {
- max8997->irq_masks_cur[i] = 0xff;
- max8997->irq_masks_cache[i] = 0xff;
-
- if (IS_ERR_OR_NULL(max8997->regmap))
- continue;
- if (max8997_mask_reg[i] == MAX8997_REG_INVALID)
- continue;
-
- regmap_write(max8997->regmap, max8997_mask_reg[i], 0xff);
- }
-
- for (i = 0; i < MAX8997_NUM_GPIO; i++) {
- max8997->gpio_status[i] = (regmap_read(max8997->regmap,
- MAX8997_REG_GPIOCNTL1 + i,
- &val)
- & MAX8997_GPIO_DATA_MASK) ?
- true : false;
- }
-
- domain = irq_domain_add_linear(NULL, MAX8997_IRQ_NR,
- &max8997_irq_domain_ops, max8997);
- if (!domain) {
- dev_err(max8997->dev, "could not create irq domain\n");
- return -ENODEV;
- }
- max8997->irq_domain = domain;
-
- ret = request_threaded_irq(max8997->irq, NULL, max8997_irq_thread,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- "max8997-irq", max8997);
-
- if (ret) {
- dev_err(max8997->dev, "Failed to request IRQ %d: %d\n",
- max8997->irq, ret);
- return ret;
- }
-
- if (!max8997->ono)
- return 0;
-
- ret = request_threaded_irq(max8997->ono, NULL, max8997_irq_thread,
- IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING |
- IRQF_ONESHOT, "max8997-ono", max8997);
-
- if (ret)
- dev_err(max8997->dev, "Failed to request ono-IRQ %d: %d\n",
- max8997->ono, ret);
-
- return 0;
-}
-
-void max8997_irq_exit(struct max8997_dev *max8997)
-{
- if (max8997->ono)
- free_irq(max8997->ono, max8997);
-
- if (max8997->irq)
- free_irq(max8997->irq, max8997);
-}
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index c4a1072..8f50716 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -64,6 +64,49 @@ static const struct regmap_config max8997_regmap_config = {
.max_register = MAX8997_REG_PMIC_END,
};
+static const struct regmap_irq max8997_irqs[] = {
+ /* PMIC_INT1 interrupts */
+ { .reg_offset = 0, .mask = PMIC_INT1_PWRONR_MASK, },
+ { .reg_offset = 0, .mask = PMIC_INT1_PWRONF_MASK, },
+ { .reg_offset = 0, .mask = PMIC_INT1_PWRON1SEC_MASK, },
+ { .reg_offset = 0, .mask = PMIC_INT1_JIGONR_MASK, },
+ { .reg_offset = 0, .mask = PMIC_INT1_JIGONF_MASK, },
+ { .reg_offset = 0, .mask = PMIC_INT1_LOWBAT2_MASK, },
+ { .reg_offset = 0, .mask = PMIC_INT1_LOWBAT1_MASK, },
+ /* PMIC_INT2 interrupts */
+ { .reg_offset = 1, .mask = PMIC_INT2_JIGR_MASK, },
+ { .reg_offset = 1, .mask = PMIC_INT2_JIGF_MASK, },
+ { .reg_offset = 1, .mask = PMIC_INT2_MR_MASK, },
+ { .reg_offset = 1, .mask = PMIC_INT2_DVS1OK_MASK, },
+ { .reg_offset = 1, .mask = PMIC_INT2_DVS2OK_MASK, },
+ { .reg_offset = 1, .mask = PMIC_INT2_DVS3OK_MASK, },
+ { .reg_offset = 1, .mask = PMIC_INT2_DVS4OK_MASK, },
+ /* PMIC_INT3 interrupts */
+ { .reg_offset = 2, .mask = PMIC_INT3_CHGINS_MASK, },
+ { .reg_offset = 2, .mask = PMIC_INT3_CHGRM_MASK, },
+ { .reg_offset = 2, .mask = PMIC_INT3_DCINOVP_MASK, },
+ { .reg_offset = 2, .mask = PMIC_INT3_TOPOFFR_MASK, },
+ { .reg_offset = 2, .mask = PMIC_INT3_CHGRSTF_MASK, },
+ { .reg_offset = 2, .mask = PMIC_INT3_MBCHGTMEXPD_MASK, },
+ /* PMIC_INT4 interrupts */
+ { .reg_offset = 3, .mask = PMIC_INT4_RTC60S_MASK, },
+ { .reg_offset = 3, .mask = PMIC_INT4_RTCA1_MASK, },
+ { .reg_offset = 3, .mask = PMIC_INT4_RTCA2_MASK, },
+ { .reg_offset = 3, .mask = PMIC_INT4_SMPL_INT_MASK, },
+ { .reg_offset = 3, .mask = PMIC_INT4_RTC1S_MASK, },
+ { .reg_offset = 3, .mask = PMIC_INT4_WTSR_MASK, },
+};
+
+static const struct regmap_irq_chip max8997_irq_chip = {
+ .name = "max8997",
+ .status_base = MAX8997_REG_INT1,
+ .mask_base = MAX8997_REG_INT1MSK,
+ .mask_invert = false,
+ .num_regs = 4,
+ .irqs = max8997_irqs,
+ .num_irqs = ARRAY_SIZE(max8997_irqs),
+};
+
static const struct regmap_config max8997_regmap_rtc_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -82,6 +125,31 @@ static const struct regmap_config max8997_regmap_muic_config = {
.max_register = MAX8997_MUIC_REG_END,
};
+static const struct regmap_irq max8997_irqs_muic[] = {
+ /* MUIC_INT1 interrupts */
+ { .reg_offset = 0, .mask = MUIC_INT1_ADC_MASK, },
+ { .reg_offset = 0, .mask = MUIC_INT1_ADCLOW_MASK, },
+ { .reg_offset = 0, .mask = MUIC_INT1_ADCERROR_MASK, },
+ /* MUIC_INT2 interrupts */
+ { .reg_offset = 1, .mask = MUIC_INT2_CHGTYP_MASK, },
+ { .reg_offset = 1, .mask = MUIC_INT2_CHGDETRUN_MASK, },
+ { .reg_offset = 1, .mask = MUIC_INT2_DCDTMR_MASK, },
+ { .reg_offset = 1, .mask = MUIC_INT2_DBCHG_MASK, },
+ { .reg_offset = 1, .mask = MUIC_INT2_VBVOLT_MASK, },
+ /* MUIC_INT3 interrupts */
+ { .reg_offset = 2, .mask = MUIC_INT3_OVP_MASK, },
+};
+
+static const struct regmap_irq_chip max8997_irq_chip_muic = {
+ .name = "max8997-muic",
+ .status_base = MAX8997_MUIC_REG_INT1,
+ .mask_base = MAX8997_MUIC_REG_INTMASK1,
+ .mask_invert = true,
+ .num_regs = 3,
+ .irqs = max8997_irqs_muic,
+ .num_irqs = ARRAY_SIZE(max8997_irqs_muic),
+};
+
/*
* Only the common platform data elements for max8997 are parsed here from the
* device tree. Other sub-modules of max8997 such as pmic, rtc and others have
@@ -214,9 +282,26 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
goto err_regmap;
}
- pm_runtime_set_active(max8997->dev);
+ ret = regmap_add_irq_chip(max8997->regmap, max8997->irq,
+ IRQF_ONESHOT | IRQF_SHARED |
+ IRQF_TRIGGER_FALLING, 0,
+ &max8997_irq_chip, &max8997->irq_data);
+ if (ret) {
+ dev_err(max8997->dev, "failed to add irq chip: %d\n", ret);
+ goto err_regmap;
+ }
- max8997_irq_init(max8997);
+ ret = regmap_add_irq_chip(max8997->regmap_muic, max8997->irq,
+ IRQF_ONESHOT | IRQF_SHARED |
+ IRQF_TRIGGER_FALLING, 0,
+ &max8997_irq_chip_muic,
+ &max8997->irq_data_muic);
+ if (ret) {
+ dev_err(max8997->dev, "failed to add irq chip: %d\n", ret);
+ goto err_irq_muic;
+ }
+
+ pm_runtime_set_active(max8997->dev);
ret = mfd_add_devices(max8997->dev, -1, max8997_devs,
ARRAY_SIZE(max8997_devs),
@@ -238,6 +323,9 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
err_mfd:
mfd_remove_devices(max8997->dev);
+ regmap_del_irq_chip(max8997->irq, max8997->irq_data_muic);
+err_irq_muic:
+ regmap_del_irq_chip(max8997->irq, max8997->irq_data);
err_regmap:
i2c_unregister_device(max8997->muic);
err_i2c_muic:
@@ -252,6 +340,10 @@ static int max8997_i2c_remove(struct i2c_client *i2c)
struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
mfd_remove_devices(max8997->dev);
+
+ regmap_del_irq_chip(max8997->irq, max8997->irq_data_muic);
+ regmap_del_irq_chip(max8997->irq, max8997->irq_data);
+
i2c_unregister_device(max8997->muic);
i2c_unregister_device(max8997->haptic);
i2c_unregister_device(max8997->rtc);
@@ -468,8 +560,11 @@ static int max8997_suspend(struct device *dev)
struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- if (device_may_wakeup(dev))
- irq_set_irq_wake(max8997->irq, 1);
+ if (device_may_wakeup(dev)) {
+ enable_irq_wake(max8997->irq);
+ disable_irq(max8997->irq);
+ }
+
return 0;
}
@@ -478,9 +573,12 @@ static int max8997_resume(struct device *dev)
struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
- if (device_may_wakeup(dev))
- irq_set_irq_wake(max8997->irq, 0);
- return max8997_irq_resume(max8997);
+ if (device_may_wakeup(dev)) {
+ disable_irq_wake(max8997->irq);
+ enable_irq(max8997->irq);
+ }
+
+ return 0;
}
static const struct dev_pm_ops max8997_pm = {
diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
index b866f7d5..22769ea 100644
--- a/drivers/rtc/rtc-max8997.c
+++ b/drivers/rtc/rtc-max8997.c
@@ -494,7 +494,7 @@ static int max8997_rtc_probe(struct platform_device *pdev)
return ret;
}
- virq = irq_create_mapping(max8997->irq_domain, MAX8997_PMICIRQ_RTCA1);
+ virq = regmap_irq_get_virq(max8997->irq_data, MAX8997_PMICIRQ_RTCA1);
if (!virq) {
dev_err(&pdev->dev, "Failed to create mapping alarm IRQ\n");
ret = -ENXIO;
diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h
index ea80ef8..f42ea22 100644
--- a/include/linux/mfd/max8997-private.h
+++ b/include/linux/mfd/max8997-private.h
@@ -333,6 +333,48 @@ enum max8997_irq_source {
MAX8997_IRQ_GROUP_NR,
};
+#define PMIC_INT1_PWRONR_MASK (0x1 << 0)
+#define PMIC_INT1_PWRONF_MASK (0x1 << 1)
+#define PMIC_INT1_PWRON1SEC_MASK (0x1 << 3)
+#define PMIC_INT1_JIGONR_MASK (0x1 << 4)
+#define PMIC_INT1_JIGONF_MASK (0x1 << 5)
+#define PMIC_INT1_LOWBAT2_MASK (0x1 << 6)
+#define PMIC_INT1_LOWBAT1_MASK (0x1 << 7)
+
+#define PMIC_INT2_JIGR_MASK (0x1 << 0)
+#define PMIC_INT2_JIGF_MASK (0x1 << 1)
+#define PMIC_INT2_MR_MASK (0x1 << 2)
+#define PMIC_INT2_DVS1OK_MASK (0x1 << 3)
+#define PMIC_INT2_DVS2OK_MASK (0x1 << 4)
+#define PMIC_INT2_DVS3OK_MASK (0x1 << 5)
+#define PMIC_INT2_DVS4OK_MASK (0x1 << 6)
+
+#define PMIC_INT3_CHGINS_MASK (0x1 << 0)
+#define PMIC_INT3_CHGRM_MASK (0x1 << 1)
+#define PMIC_INT3_DCINOVP_MASK (0x1 << 2)
+#define PMIC_INT3_TOPOFFR_MASK (0x1 << 3)
+#define PMIC_INT3_CHGRSTF_MASK (0x1 << 5)
+#define PMIC_INT3_MBCHGTMEXPD_MASK (0x1 << 7)
+
+#define PMIC_INT4_RTC60S_MASK (0x1 << 0)
+#define PMIC_INT4_RTCA1_MASK (0x1 << 1)
+#define PMIC_INT4_RTCA2_MASK (0x1 << 2)
+#define PMIC_INT4_SMPL_INT_MASK (0x1 << 3)
+#define PMIC_INT4_RTC1S_MASK (0x1 << 4)
+#define PMIC_INT4_WTSR_MASK (0x1 << 5)
+
+#define MUIC_INT1_ADC_MASK (0x1 << 0)
+#define MUIC_INT1_ADCLOW_MASK (0x1 << 1)
+#define MUIC_INT1_ADCERROR_MASK (0x1 << 2)
+
+#define MUIC_INT2_CHGTYP_MASK (0x1 << 0)
+#define MUIC_INT2_CHGDETRUN_MASK (0x1 << 1)
+#define MUIC_INT2_DCDTMR_MASK (0x1 << 2)
+#define MUIC_INT2_DBCHG_MASK (0x1 << 3)
+#define MUIC_INT2_VBVOLT_MASK (0x1 << 4)
+
+#define MUIC_INT3_OVP_MASK (0x1 << 2)
+
enum max8997_irq {
MAX8997_PMICIRQ_PWRONR,
MAX8997_PMICIRQ_PWRONF,
@@ -364,19 +406,23 @@ enum max8997_irq {
MAX8997_PMICIRQ_RTC1S,
MAX8997_PMICIRQ_WTSR,
- MAX8997_MUICIRQ_ADCError,
- MAX8997_MUICIRQ_ADCLow,
+ MAX8997_PMICIRQ_NR,
+};
+
+enum max8997_irq_muic {
MAX8997_MUICIRQ_ADC,
+ MAX8997_MUICIRQ_ADCLow,
+ MAX8997_MUICIRQ_ADCError,
- MAX8997_MUICIRQ_VBVolt,
- MAX8997_MUICIRQ_DBChg,
- MAX8997_MUICIRQ_DCDTmr,
- MAX8997_MUICIRQ_ChgDetRun,
MAX8997_MUICIRQ_ChgTyp,
+ MAX8997_MUICIRQ_ChgDetRun,
+ MAX8997_MUICIRQ_DCDTmr,
+ MAX8997_MUICIRQ_DBChg,
+ MAX8997_MUICIRQ_VBVolt,
MAX8997_MUICIRQ_OVP,
- MAX8997_IRQ_NR,
+ MAX8997_MUCIRQ_NR,
};
#define MAX8997_NUM_GPIO 12
@@ -397,9 +443,10 @@ struct max8997_dev {
struct regmap *regmap_haptic;
struct regmap *regmap_muic;
+ struct regmap_irq_chip_data *irq_data;
+ struct regmap_irq_chip_data *irq_data_muic;
int irq;
int ono;
- struct irq_domain *irq_domain;
struct mutex irqlock;
int irq_masks_cur[MAX8997_IRQ_GROUP_NR];
int irq_masks_cache[MAX8997_IRQ_GROUP_NR];
--
1.9.1
^ permalink raw reply related
* [PATCH v5 3/3] mfd: max8997: change irq names to upper case
From: Robert Baldyga @ 2014-11-12 7:23 UTC (permalink / raw)
To: lee.jones
Cc: sameo, myungjoo.ham, cw00.choi, dmitry.torokhov, cooloney,
rpurdie, sre, dbaryshkov, dwmw2, lgirdwood, broonie, a.zummo,
paul.gortmaker, sachin.kamat, k.kozlowski, linux-kernel,
linux-input, linux-leds, linux-pm, rtc-linux, Robert Baldyga
In-Reply-To: <1415776996-11569-1-git-send-email-r.baldyga@samsung.com>
This patch changes naming convention of MUIC interrupts form CamelCase
to upper case. It makes names more readable and consistent with another
interrupt names in max8997 driver.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
[For the mfd part]
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/extcon/extcon-max8997.c | 32 ++++++++++++++++----------------
include/linux/mfd/max8997-private.h | 16 ++++++++--------
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 172dbd6..834b72f 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -46,15 +46,15 @@ struct max8997_muic_irq {
};
static struct max8997_muic_irq muic_irqs[] = {
- { MAX8997_MUICIRQ_ADCError, "muic-ADCERROR" },
- { MAX8997_MUICIRQ_ADCLow, "muic-ADCLOW" },
- { MAX8997_MUICIRQ_ADC, "muic-ADC" },
- { MAX8997_MUICIRQ_VBVolt, "muic-VBVOLT" },
- { MAX8997_MUICIRQ_DBChg, "muic-DBCHG" },
- { MAX8997_MUICIRQ_DCDTmr, "muic-DCDTMR" },
- { MAX8997_MUICIRQ_ChgDetRun, "muic-CHGDETRUN" },
- { MAX8997_MUICIRQ_ChgTyp, "muic-CHGTYP" },
- { MAX8997_MUICIRQ_OVP, "muic-OVP" },
+ { MAX8997_MUICIRQ_ADCERROR, "MUIC-ADCERROR" },
+ { MAX8997_MUICIRQ_ADCLOW, "MUIC-ADCLOW" },
+ { MAX8997_MUICIRQ_ADC, "MUIC-ADC" },
+ { MAX8997_MUICIRQ_VBVOLT, "MUIC-VBVOLT" },
+ { MAX8997_MUICIRQ_DBCHG, "MUIC-DBCHG" },
+ { MAX8997_MUICIRQ_DCDTMR, "MUIC-DCDTMR" },
+ { MAX8997_MUICIRQ_CHGDETRUN, "MUIC-CHGDETRUN" },
+ { MAX8997_MUICIRQ_CHGTYP, "MUIC-CHGTYP" },
+ { MAX8997_MUICIRQ_OVP, "MUIC-OVP" },
};
/* Define supported cable type */
@@ -553,17 +553,17 @@ static void max8997_muic_irq_work(struct work_struct *work)
}
switch (irq_type) {
- case MAX8997_MUICIRQ_ADCError:
- case MAX8997_MUICIRQ_ADCLow:
+ case MAX8997_MUICIRQ_ADCERROR:
+ case MAX8997_MUICIRQ_ADCLOW:
case MAX8997_MUICIRQ_ADC:
/* Handle all of cable except for charger cable */
ret = max8997_muic_adc_handler(info);
break;
- case MAX8997_MUICIRQ_VBVolt:
- case MAX8997_MUICIRQ_DBChg:
- case MAX8997_MUICIRQ_DCDTmr:
- case MAX8997_MUICIRQ_ChgDetRun:
- case MAX8997_MUICIRQ_ChgTyp:
+ case MAX8997_MUICIRQ_VBVOLT:
+ case MAX8997_MUICIRQ_DBCHG:
+ case MAX8997_MUICIRQ_DCDTMR:
+ case MAX8997_MUICIRQ_CHGDETRUN:
+ case MAX8997_MUICIRQ_CHGTYP:
/* Handle charger cable */
ret = max8997_muic_chg_handler(info);
break;
diff --git a/include/linux/mfd/max8997-private.h b/include/linux/mfd/max8997-private.h
index f42ea22..2817fa6 100644
--- a/include/linux/mfd/max8997-private.h
+++ b/include/linux/mfd/max8997-private.h
@@ -411,14 +411,14 @@ enum max8997_irq {
enum max8997_irq_muic {
MAX8997_MUICIRQ_ADC,
- MAX8997_MUICIRQ_ADCLow,
- MAX8997_MUICIRQ_ADCError,
-
- MAX8997_MUICIRQ_ChgTyp,
- MAX8997_MUICIRQ_ChgDetRun,
- MAX8997_MUICIRQ_DCDTmr,
- MAX8997_MUICIRQ_DBChg,
- MAX8997_MUICIRQ_VBVolt,
+ MAX8997_MUICIRQ_ADCLOW,
+ MAX8997_MUICIRQ_ADCERROR,
+
+ MAX8997_MUICIRQ_CHGTYP,
+ MAX8997_MUICIRQ_CHGDETRUN,
+ MAX8997_MUICIRQ_DCDTMR,
+ MAX8997_MUICIRQ_DBCHG,
+ MAX8997_MUICIRQ_VBVOLT,
MAX8997_MUICIRQ_OVP,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v3] Input: evdev - add event-mask API
From: Dmitry Torokhov @ 2014-11-12 7:40 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-input, Peter Hutterer
In-Reply-To: <1415121143-1675-1-git-send-email-dh.herrmann@gmail.com>
Hi David,
On Tue, Nov 04, 2014 at 06:12:23PM +0100, David Herrmann wrote:
> +static int bits_from_user(unsigned long *bits, unsigned int maxbit,
> + unsigned int maxlen, const void __user *p, int compat)
> +{
> + int len;
> +
> +#if IS_ENABLED(CONFIG_COMPAT)
> + if (compat)
I think this can be written as:
if (IS_ENABLED(CONFIG_COMPAT) && compat)
> + len = BITS_TO_LONGS_COMPAT(maxbit) * sizeof(compat_long_t);
> + else
> +#endif
> + len = BITS_TO_LONGS(maxbit) * sizeof(long);
> +
> + if (len > maxlen)
> + len = maxlen;
> +
> +#if IS_ENABLED(CONFIG_COMPAT) && defined(__BIG_ENDIAN)
> + if (compat) {
> + int i;
> +
> + for (i = 0; i < len / sizeof(compat_long_t); i++)
> + if (copy_from_user((compat_long_t *) bits +
> + i + 1 - ((i % 2) << 1),
> + (compat_long_t __user *) p + i,
> + sizeof(compat_long_t)))
Unfortunately this is not quite enough, you need to also zero out the
upper bits of the last partial if userspace did not supply multiple of
64 bits.
Frankly, bitmask APIs that we have in input are god-awful on big endian:
they do not work if usespace uses quantities less than long (or compat
long). I think we'll have to enforce it in the code.
> + } else {
> + /* fake mask with all bits set */
> + out = (u8 __user*)codes;
> + for (i = 0; i < min; ++i) {
> + if (put_user((u8)0xff, out + i))
> + return -EFAULT;
I wonder if we should not replace this with access_ok() + straight
memset instead of doing byte-by-byte copy.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] Input: evdev - add event-mask API
From: David Herrmann @ 2014-11-12 12:41 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER, Peter Hutterer
In-Reply-To: <20141112074020.GA4325@dtor-ws>
Hi
On Wed, Nov 12, 2014 at 8:40 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi David,
>
> On Tue, Nov 04, 2014 at 06:12:23PM +0100, David Herrmann wrote:
>> +static int bits_from_user(unsigned long *bits, unsigned int maxbit,
>> + unsigned int maxlen, const void __user *p, int compat)
>> +{
>> + int len;
>> +
>> +#if IS_ENABLED(CONFIG_COMPAT)
>> + if (compat)
>
> I think this can be written as:
> if (IS_ENABLED(CONFIG_COMPAT) && compat)
BITS_TO_LONGS_COMPAT is only defined if CONFIG_COMPAT is. We'd rely on
compiler-optimizations (dead code elimination) if we'd do that change.
I'm not really fond of making -O0 compiles fail, but there're several
places in the kernel that do. Your call ;)
>> + len = BITS_TO_LONGS_COMPAT(maxbit) * sizeof(compat_long_t);
>> + else
>> +#endif
>> + len = BITS_TO_LONGS(maxbit) * sizeof(long);
>> +
>> + if (len > maxlen)
>> + len = maxlen;
>> +
>> +#if IS_ENABLED(CONFIG_COMPAT) && defined(__BIG_ENDIAN)
>> + if (compat) {
>> + int i;
>> +
>> + for (i = 0; i < len / sizeof(compat_long_t); i++)
>> + if (copy_from_user((compat_long_t *) bits +
>> + i + 1 - ((i % 2) << 1),
>> + (compat_long_t __user *) p + i,
>> + sizeof(compat_long_t)))
>
> Unfortunately this is not quite enough, you need to also zero out the
> upper bits of the last partial if userspace did not supply multiple of
> 64 bits.
>
> Frankly, bitmask APIs that we have in input are god-awful on big endian:
> they do not work if usespace uses quantities less than long (or compat
> long). I think we'll have to enforce it in the code.
Right, I relied on the caller of bits_from_user() to initialize it to
zero and make it long-aligned. I can move it into that helper.
>> + } else {
>> + /* fake mask with all bits set */
>> + out = (u8 __user*)codes;
>> + for (i = 0; i < min; ++i) {
>> + if (put_user((u8)0xff, out + i))
>> + return -EFAULT;
>
> I wonder if we should not replace this with access_ok() + straight
> memset instead of doing byte-by-byte copy.
I'm actually not very familiar with the details of copy_to_user().
That's why I went with put_user(). I can look into it, but if you know
it well, I am open for suggestions.
Thanks
David
^ permalink raw reply
* Re: [PATCH 0/4] Touchscreen performance related fixes
From: Johannes Pointner @ 2014-11-12 13:00 UTC (permalink / raw)
To: Vignesh R
Cc: Richard Cochran, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Benoit Cousson, Tony Lindgren,
Russell King, Jonathan Cameron, Dmitry Torokhov, devicetree,
Lars-Peter Clausen, Samuel Ortiz, Jan Kardell, linux-iio,
Sebastian Andrzej Siewior, linux-input, linux-kernel,
Felipe Balbi, Paul Gortmaker, Peter Meerwald, Hartmut Knaack,
linux-omap
In-Reply-To: <545B2651.4090905@ti.com>
Hello Vignesh,
I tried your patch version 3 on a customized board and had some
behavior I couldn't explain.
If I only use the touchscreen it works fine but if I also read values
from the ADCs then I get a lot of pen_up events even if I am still
touching the screen.
For the test I read via
# cat /sys/bus/iio/devices/iio\:device0/in_voltage5_raw
values from the ADC in an busy loop as you explained in an email
before. Did you also experience such behavior or do you know what
causes it?
Without the patches the touchscreen works fine during the iio test.
Thanks,
Hannes
2014-11-06 8:42 GMT+01:00 Vignesh R <vigneshr@ti.com>:
>
>
> On Monday 03 November 2014 11:39 PM, Richard Cochran wrote:
>> On Mon, Oct 27, 2014 at 04:38:27PM +0530, Vignesh R wrote:
>>> This series of patches fix TSC defects related to lag in touchscreen
>>> performance and cursor jump at touch release. The lag was result of
>>> udelay in TSC interrupt handler. Cursor jump due to false pen-up event.
>>> The patches implement Advisory 1.0.31 in silicon errata of am335x-evm
>>> to avoid false pen-up events and remove udelay.
>>
>> That advisory has two workarounds. You have chosen the second one?
>
> Work around one. Hence 5 wire design is not broken.
>
>>
>> The text of the second workaround says it only works on 4 wire setups,
>> so I wonder how 5 wire designs will be affected.
>>
>>> The advisory says to use
>>> steps 1 to 4 for ADC and 5 to 16 for TSC (assuming 4 wire TSC and 4 channel
>>> ADC).
>>
>> No, it doesn't say that. (sprz360f.pdf)
>
> The pen up event detection happens immediately after charge step. Hence,
> interchanging ADC and TSC steps makes sure that sampling of touch
> co-ordinates and pen events are done one after the other. This
> workaround was suggested by internal hardware folks. Earlier ADC steps
> intervened between sampling of co-ordinates and pen event detection
> which is not desirable.
>
>>
>>> Further the X co-ordinate must be the last one to be sampled just
>>> before charge step. The first two patches implement the required changes.
>>
>> FWIW, I implemented the first workaround and removed the udelay not
>> too long ago. Like Sebastian, I saw the TSC unit hang after about
>> 50000 interrupts when running with the workaround.
>>
>> Did you test you patch for very long?
>
> Yes, I tested for about 200000 interrupts and I didn't see any hang.
> This patch series does not just implement workaround but also does some
> minor changes, such as interchanging ADC and TSC steps etc, which makes
> TSC driver more robust. Let me know if you encounter any issues with my
> patch series.
>
> Regards
> Vignesh
>
>>
>> Thanks,
>> Richard
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] hid: sony: Use kernel allocated buffers for HID reports
From: Frank Praznik @ 2014-11-12 15:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: open list:HID CORE LAYER <linux-input@vger.kernel.org>, Jiri Kosina,
open list:HID CORE LAYER <linux-input@vger.kernel.org>, Jiri Kosina
In-Reply-To: <20141111190412.GC27720@dtor-ws>
On Tue, Nov 11, 2014 at 2:04 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Frank,
>> static spinlock_t sony_dev_list_lock;
>> static LIST_HEAD(sony_device_list);
>> static DEFINE_IDA(sony_device_id_allocator);
>> @@ -811,6 +814,7 @@ struct sony_sc {
>> struct work_struct state_worker;
>> struct power_supply battery;
>> int device_id;
>> + __u8 *output_report_dmabuf;
>
> Just to confirm as I haven't looked at the entire driver: there is no
> possibility of 2 requests being submitted at the same time so that one
> will overwrite other's data?
>
> --
> Dmitry
Output reports are sent using a work request submitted to the kernel
work queue with schedule_work(), so using one buffer is safe since the
work_struct can't be scheduled multiple times or simultaneously
execute across multiple cores.
The other comments you had are addressed in v2 of the patch.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox