* [patch] Input: edt-ft5x06 - fix an i2c write for M09 support
From: Simon Budig @ 2014-06-07 23:53 UTC (permalink / raw)
To: Dmitry Torokhov, Lothar Waßmann
Cc: Henrik Rydberg, Grant Likely, Rob Herring, Fugang Duan,
Jingoo Han, linux-input, kernel-janitors, Robert Wörle
[-- Attachment #1.1: Type: text/plain, Size: 1016 bytes --]
Hi all.
I received notice from Robert Wörle who spotted a bug in the ft5x06
driver (M09 support) which can result in bogus values being written into
the configuration registers.
Paul wrote:
> I think i found the problem, as the driver sends 3 bytes instead of 2
> when accessing a register on the M09 firmware.
> Therefore , writing to gain seems to overflow into the offset register.
>
> This seems to be even in the lastet upstream driver so i created a patch
> for this driver which you find attached.
>
> @Simon Budig
>
> Find attached a patch to fix this edt driver bug, feel free to comment ,
> test and commit upstream ;-)
He is right, his fix is correct. Please include his patch into the
repository.
Acked-By: Simon Budig <simon.budig@kernelconcepts.de>
Thanks,
Simon
--
Simon Budig kernel concepts GmbH
simon.budig@kernelconcepts.de Sieghuetter Hauptweg 48
+49-271-771091-17 D-57072 Siegen
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Input-edt-ft5x06-fix-i2c-writes-to-2-bytes-on-M09-fi.patch --]
[-- Type: text/x-patch; name="0001-Input-edt-ft5x06-fix-i2c-writes-to-2-bytes-on-M09-fi.patch", Size: 929 bytes --]
From 36b9ac0e8206efefc8795c925b0f3b63ab29fd08 Mon Sep 17 00:00:00 2001
From: Robert Woerle <robert@linuxdevelopment.de>
Date: Wed, 28 May 2014 13:22:09 +0200
Subject: [PATCH] Input: edt-ft5x06 - fix i2c writes to 2 bytes on M09
firmware
Signed-off-by: Robert Woerle <robert@linuxdevelopment.de>
---
drivers/input/touchscreen/edt-ft5x06.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index f8815be..d4f3399 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -271,7 +271,7 @@ static int edt_ft5x06_register_write(struct edt_ft5x06_ts_data *tsdata,
wrbuf[0] = addr;
wrbuf[1] = value;
- return edt_ft5x06_ts_readwrite(tsdata->client, 3,
+ return edt_ft5x06_ts_readwrite(tsdata->client, 2,
wrbuf, 0, NULL);
default:
--
1.7.9.5
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply related
* Re: [PATCH] elantech: Deal with clickpads reporting right button events
From: Dmitry Torokhov @ 2014-06-08 5:36 UTC (permalink / raw)
To: Hans de Goede; +Cc: Elder Marco, linux-input, stable
In-Reply-To: <1402063695-20196-1-git-send-email-hdegoede@redhat.com>
On Fri, Jun 06, 2014 at 04:08:15PM +0200, Hans de Goede wrote:
> At least the Dell Vostro 5470 elantech *clickpad* reports right button
> clicks when clicked in the right bottom area:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1103528
>
> This is different from how (elantech) clickpads normally operate,
> normally no matter where the user clicks on the pad the pad always reports
> a left button event, since there is only 1 hardware button beneath the path.
>
> It looks like Dell has put 2 buttons under the pad, one under each bottom
> corner, causing this.
>
> Since this however still clearly is a real clickpad hardware-wise, we still
> want to report it as such to userspace, so that things like finger movement
> in the bottom area can be properly ignored as it should be on clickpads.
>
> So deal with this weirdness by simply mapping a right click to a left click
> on elantech clickpads. As an added advantage this is something which we can
> simply do on all elantech clickpads, so no need to add special quirks for
> this weird model.
>
> Reported-and-tested-by: Elder Marco <eldermarco@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thank you.
> ---
> drivers/input/mouse/elantech.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 4d79821..846926d 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -473,8 +473,15 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
> input_report_key(dev, BTN_TOOL_FINGER, fingers == 1);
> input_report_key(dev, BTN_TOOL_DOUBLETAP, fingers == 2);
> input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3);
> - input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> - input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> +
> + /* For clickpads map both buttons to BTN_LEFT */
> + if (etd->fw_version & 0x001000) {
> + input_report_key(dev, BTN_LEFT, packet[0] & 0x03);
> + } else {
> + input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> + }
> +
> input_report_abs(dev, ABS_PRESSURE, pres);
> input_report_abs(dev, ABS_TOOL_WIDTH, width);
>
> @@ -484,10 +491,17 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
> static void elantech_input_sync_v4(struct psmouse *psmouse)
> {
> struct input_dev *dev = psmouse->dev;
> + struct elantech_data *etd = psmouse->private;
> unsigned char *packet = psmouse->packet;
>
> - input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> - input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> + /* For clickpads map both buttons to BTN_LEFT */
> + if (etd->fw_version & 0x001000) {
> + input_report_key(dev, BTN_LEFT, packet[0] & 0x03);
> + } else {
> + input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
> + input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
> + }
> +
> input_mt_report_pointer_emulation(dev, true);
> input_sync(dev);
> }
> --
> 2.0.0
>
--
Dmitry
^ permalink raw reply
* Re: [patch] Input: edt-ft5x06 - fix an i2c write for M09 support
From: Dmitry Torokhov @ 2014-06-08 5:36 UTC (permalink / raw)
To: Simon Budig
Cc: Lothar Waßmann, Henrik Rydberg, Grant Likely, Rob Herring,
Fugang Duan, Jingoo Han, linux-input, kernel-janitors,
Robert Wörle
In-Reply-To: <5393A60C.8040207@kernelconcepts.de>
On Sun, Jun 08, 2014 at 01:53:48AM +0200, Simon Budig wrote:
> Hi all.
>
> I received notice from Robert Wörle who spotted a bug in the ft5x06
> driver (M09 support) which can result in bogus values being written into
> the configuration registers.
>
> Paul wrote:
> > I think i found the problem, as the driver sends 3 bytes instead of 2
> > when accessing a register on the M09 firmware.
> > Therefore , writing to gain seems to overflow into the offset register.
> >
> > This seems to be even in the lastet upstream driver so i created a patch
> > for this driver which you find attached.
> >
> > @Simon Budig
> >
> > Find attached a patch to fix this edt driver bug, feel free to comment ,
> > test and commit upstream ;-)
>
> He is right, his fix is correct. Please include his patch into the
> repository.
>
> Acked-By: Simon Budig <simon.budig@kernelconcepts.de>
Applied, thank you.
>
> Thanks,
> Simon
> --
> Simon Budig kernel concepts GmbH
> simon.budig@kernelconcepts.de Sieghuetter Hauptweg 48
> +49-271-771091-17 D-57072 Siegen
>
> From 36b9ac0e8206efefc8795c925b0f3b63ab29fd08 Mon Sep 17 00:00:00 2001
> From: Robert Woerle <robert@linuxdevelopment.de>
> Date: Wed, 28 May 2014 13:22:09 +0200
> Subject: [PATCH] Input: edt-ft5x06 - fix i2c writes to 2 bytes on M09
> firmware
>
> Signed-off-by: Robert Woerle <robert@linuxdevelopment.de>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index f8815be..d4f3399 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -271,7 +271,7 @@ static int edt_ft5x06_register_write(struct edt_ft5x06_ts_data *tsdata,
> wrbuf[0] = addr;
> wrbuf[1] = value;
>
> - return edt_ft5x06_ts_readwrite(tsdata->client, 3,
> + return edt_ft5x06_ts_readwrite(tsdata->client, 2,
> wrbuf, 0, NULL);
>
> default:
> --
> 1.7.9.5
>
--
Dmitry
--
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] Input - synaptics: fix resolution for manually provided min/max
From: Dmitry Torokhov @ 2014-06-08 5:40 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, linux-kernel, Hans de Goede, Peter Hutterer
In-Reply-To: <1401818375-7212-1-git-send-email-benjamin.tissoires@redhat.com>
On Tue, Jun 03, 2014 at 01:59:35PM -0400, Benjamin Tissoires wrote:
> commit 421e08c41fda fixed the reported min/max for the X and Y axis,
> but unfortunately, it broke the resolution of those same axis.
>
> On the t540p, the resolution is the same regarding X and Y. It is not
> a problem for xf86-input-synaptics because this driver is only interested
> in the ratio between X and Y.
> Unfortunately, xf86-input-cmt uses directly the resolution, and having a
> null resolution leads to some divide by 0 errors, which are translated by
> -infinity in the resulting coordinates.
>
> Reported-by: Peter Hutterer <peter.hutterer@who-t.net>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: stable@vger.kernel.org
> ---
>
> Hi Dmitry,
>
> in the original submission, the test for the quirk was at the end of the
> synaptics_resolution() function. However, there was a mixmatch with the config
> in this original submission and you had to change the patch slightly.
> Unfortunately, this change created this bug which was hard to notice, which is
> why it comes that late in the cycle (and also because we are trying to find out
> if xf86-input-cmt could replace xf86-input-synaptics).
Hmm, I guess we are not going to quirk very old hardware so the change
is fine, applied.
>
> Anyway, as mentioned in the commit message, this will not harm a big majority
> of users, because only those using ChromeOS will be impacted (not sure they
> have the Lenovo 40 series in their kernel either).
>
> Again, it's your call to leave or not the stable@ tag, but given that all we
> put regarding those crappy devices are tagged as such, I put it there also.
>
> Cheers,
> Benjamin
>
> drivers/input/mouse/synaptics.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index c5ec703..9cff1f8 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -347,15 +347,6 @@ static int synaptics_resolution(struct psmouse *psmouse)
> unsigned char resp[3];
> int i;
>
> - for (i = 0; min_max_pnpid_table[i].pnp_ids; i++)
> - if (matches_pnp_id(psmouse, min_max_pnpid_table[i].pnp_ids)) {
> - priv->x_min = min_max_pnpid_table[i].x_min;
> - priv->x_max = min_max_pnpid_table[i].x_max;
> - priv->y_min = min_max_pnpid_table[i].y_min;
> - priv->y_max = min_max_pnpid_table[i].y_max;
> - return 0;
> - }
> -
> if (SYN_ID_MAJOR(priv->identity) < 4)
> return 0;
>
> @@ -366,6 +357,15 @@ static int synaptics_resolution(struct psmouse *psmouse)
> }
> }
>
> + for (i = 0; min_max_pnpid_table[i].pnp_ids; i++)
> + if (matches_pnp_id(psmouse, min_max_pnpid_table[i].pnp_ids)) {
> + priv->x_min = min_max_pnpid_table[i].x_min;
> + priv->x_max = min_max_pnpid_table[i].x_max;
> + priv->y_min = min_max_pnpid_table[i].y_min;
> + priv->y_max = min_max_pnpid_table[i].y_max;
> + return 0;
> + }
> +
> if (SYN_EXT_CAP_REQUESTS(priv->capabilities) >= 5 &&
> SYN_CAP_MAX_DIMENSIONS(priv->ext_cap_0c)) {
> if (synaptics_send_cmd(psmouse, SYN_QUE_EXT_MAX_COORDS, resp)) {
> --
> 1.9.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] elantech: Don't set bit 1 of reg_10 when the no_hw_res quirk is set
From: Dmitry Torokhov @ 2014-06-08 6:08 UTC (permalink / raw)
To: Hans de Goede; +Cc: James Lademann, Philipp Wolfer, linux-input, stable
In-Reply-To: <1401784265-18325-1-git-send-email-hdegoede@redhat.com>
On Tue, Jun 03, 2014 at 10:31:05AM +0200, Hans de Goede wrote:
> The touchpad on the GIGABYTE U2442 not only stops communicating when we try
> to set bit 3 (enable real hardware resolution) of reg_10, but on some BIOS
> versions also when we set bit 1 (enable two finger mode auto correct).
>
> I've asked the original reporter of:
> https://bugzilla.kernel.org/show_bug.cgi?id=61151
>
> To check that not setting bit 1 does not lead to any adverse effects on
> his model / BIOS revision, and it does not, so this commit fixes the touchpad
> not working on these versions by simply never setting bit 1 for laptop models
> with the no_hw_res quirk.
>
> Reported-and-tested-by: James Lademann <jwlademann@gmail.com>
> Tested-by: Philipp Wolfer <ph.wolfer@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thank you.
> ---
> drivers/input/mouse/elantech.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index b96e978..4d79821 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -835,7 +835,7 @@ static int elantech_set_absolute_mode(struct psmouse *psmouse)
> if (etd->set_hw_resolution)
> etd->reg_10 = 0x0b;
> else
> - etd->reg_10 = 0x03;
> + etd->reg_10 = 0x01;
>
> if (elantech_write_reg(psmouse, 0x10, etd->reg_10))
> rc = -1;
> @@ -1336,7 +1336,8 @@ static int elantech_reconnect(struct psmouse *psmouse)
> }
>
> /*
> - * Some hw_version 3 models go into error state when we try to set bit 3 of r10
> + * Some hw_version 3 models go into error state when we try to set
> + * bit 3 and/or bit 1 of r10
> */
> static const struct dmi_system_id no_hw_res_dmi_table[] = {
> #if defined(CONFIG_DMI) && defined(CONFIG_X86)
> --
> 2.0.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: max8997_haptic - Check return values
From: Dmitry Torokhov @ 2014-06-08 6:12 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-input, Donggeun Kim
In-Reply-To: <1401704643-27846-1-git-send-email-sachin.kamat@linaro.org>
On Mon, Jun 02, 2014 at 03:54:03PM +0530, Sachin Kamat wrote:
> Check the return value of regulator_enable and pwm_enable to
> avoid errors.
> Fixes the following warning:
> drivers/input/misc/max8997_haptic.c:185:19: warning: ignoring return value of ‘regulator_enable’, declared with attribute warn_unused_result [-Wunused-result]
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Cc: Donggeun Kim <dg77.kim@samsung.com>
> ---
> Changes since v1:
> * checked the return value of pwm_enable as suggested by Dmitry.
> * Make chip->enabled = true only if enablement is
> successful.
> ---
> drivers/input/misc/max8997_haptic.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c
> index 1fea5484941f..11f4d84625b3 100644
> --- a/drivers/input/misc/max8997_haptic.c
> +++ b/drivers/input/misc/max8997_haptic.c
> @@ -181,11 +181,20 @@ static void max8997_haptic_enable(struct max8997_haptic *chip)
> }
>
> if (!chip->enabled) {
> - chip->enabled = true;
> - regulator_enable(chip->regulator);
> + error = regulator_enable(chip->regulator);
> + if (error) {
> + dev_err(chip->dev, "Failed to enable regulator\n");
> + goto out;
> + }
> max8997_haptic_configure(chip);
> - if (chip->mode == MAX8997_EXTERNAL_MODE)
> - pwm_enable(chip->pwm);
> + if (chip->mode == MAX8997_EXTERNAL_MODE) {
> + error = pwm_enable(chip->pwm);
> + if (error) {
> + dev_err(chip->dev, "Failed to enable PWM\n");
I think we should also disable regulator here, I'll add it.
> + goto out;
> + }
> + }
> + chip->enabled = true;
> }
>
> out:
> --
> 1.7.9.5
>
--
Dmitry
--
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 2/2] gpio: gpiolib: set gpiochip_remove retval to void
From: Ben Dooks @ 2014-06-08 23:18 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: David Daney, abdoulaye berthe, Geert Uytterhoeven, Linus Walleij,
Alexandre Courbot, m, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Linux MIPS Mailing List,
linuxppc-dev@lists.ozlabs.org, Linux-sh list, linux-wireless,
patches, linux-input@vger.kernel.org, linux-leds@vger.kernel.org
In-Reply-To: <5388CB1B.3090802@metafoo.de>
On Fri, May 30, 2014 at 08:16:59PM +0200, Lars-Peter Clausen wrote:
> On 05/30/2014 07:33 PM, David Daney wrote:
> >On 05/30/2014 04:39 AM, Geert Uytterhoeven wrote:
> >>On Fri, May 30, 2014 at 1:30 PM, abdoulaye berthe <berthe.ab@gmail.com>
> >>wrote:
> >>>--- a/drivers/gpio/gpiolib.c
> >>>+++ b/drivers/gpio/gpiolib.c
> >>>@@ -1263,10 +1263,9 @@ static void gpiochip_irqchip_remove(struct
> >>>gpio_chip *gpiochip);
> >>> *
> >>> * A gpio_chip with any GPIOs still requested may not be removed.
> >>> */
> >>>-int gpiochip_remove(struct gpio_chip *chip)
> >>>+void gpiochip_remove(struct gpio_chip *chip)
> >>> {
> >>> unsigned long flags;
> >>>- int status = 0;
> >>> unsigned id;
> >>>
> >>> acpi_gpiochip_remove(chip);
> >>>@@ -1278,24 +1277,15 @@ int gpiochip_remove(struct gpio_chip *chip)
> >>> of_gpiochip_remove(chip);
> >>>
> >>> for (id = 0; id < chip->ngpio; id++) {
> >>>- if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
> >>>- status = -EBUSY;
> >>>- break;
> >>>- }
> >>>- }
> >>>- if (status == 0) {
> >>>- for (id = 0; id < chip->ngpio; id++)
> >>>- chip->desc[id].chip = NULL;
> >>>-
> >>>- list_del(&chip->list);
> >>>+ if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags))
> >>>+ panic("gpio: removing gpiochip with gpios still
> >>>requested\n");
> >>
> >>panic?
> >
> >NACK to the patch for this reason. The strongest thing you should do here
> >is WARN.
> >
> >That said, I am not sure why we need this whole patch set in the first place.
>
> Well, what currently happens when you remove a device that is a
> provider of a gpio_chip which is still in use, is that the kernel
> crashes. Probably with a rather cryptic error message. So this patch
> doesn't really change the behavior, but makes it more explicit what
> is actually wrong. And even if you replace the panic() by a WARN()
> it will again just crash slightly later.
>
> This is a design flaw in the GPIO subsystem that needs to be fixed.
Surely then the best way is to error out to the module unload and
stop the driver being unloaded?
--
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/
Large Hadron Colada: A large Pina Colada that makes the universe disappear.
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: max8997_haptic - Check return values
From: Sachin Kamat @ 2014-06-09 4:02 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Sachin Kamat, linux-input, Donggeun Kim
In-Reply-To: <20140608061209.GC6982@core.coreip.homeip.net>
On Sun, Jun 8, 2014 at 11:42 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Jun 02, 2014 at 03:54:03PM +0530, Sachin Kamat wrote:
>> Check the return value of regulator_enable and pwm_enable to
>> avoid errors.
>> Fixes the following warning:
>> drivers/input/misc/max8997_haptic.c:185:19: warning: ignoring return value of ‘regulator_enable’, declared with attribute warn_unused_result [-Wunused-result]
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> Cc: Donggeun Kim <dg77.kim@samsung.com>
>> ---
>> Changes since v1:
>> * checked the return value of pwm_enable as suggested by Dmitry.
>> * Make chip->enabled = true only if enablement is
>> successful.
>> ---
>> drivers/input/misc/max8997_haptic.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c
>> index 1fea5484941f..11f4d84625b3 100644
>> --- a/drivers/input/misc/max8997_haptic.c
>> +++ b/drivers/input/misc/max8997_haptic.c
>> @@ -181,11 +181,20 @@ static void max8997_haptic_enable(struct max8997_haptic *chip)
>> }
>>
>> if (!chip->enabled) {
>> - chip->enabled = true;
>> - regulator_enable(chip->regulator);
>> + error = regulator_enable(chip->regulator);
>> + if (error) {
>> + dev_err(chip->dev, "Failed to enable regulator\n");
>> + goto out;
>> + }
>> max8997_haptic_configure(chip);
>> - if (chip->mode == MAX8997_EXTERNAL_MODE)
>> - pwm_enable(chip->pwm);
>> + if (chip->mode == MAX8997_EXTERNAL_MODE) {
>> + error = pwm_enable(chip->pwm);
>> + if (error) {
>> + dev_err(chip->dev, "Failed to enable PWM\n");
>
> I think we should also disable regulator here, I'll add it.
Right. Thanks for fixing it, Dmitry.
Regards,
Sachin.
--
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
* [git pull] Input updates for 3.16-rc0
From: Dmitry Torokhov @ 2014-06-09 6:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
[-- Attachment #1: Type: text/plain, Size: 11623 bytes --]
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. You will get a big update to
Atmel touchscreen driver, devm support for polled input devices, several
drivers have been converted to using managed resources, and assorted
driver fixes.
Changelog:
---------
Alexander Shiyan (3):
Input: gpio-beeper - simplify GPIO handling
Input: gpio_keys - more conversions to devm-* API
Input: gpio_keys_polled - convert to devm-* API
Andy Shevchenko (2):
Input: gpio_keys - convert to use devm_*
Input: gpio_keys - convert struct descriptions to kernel-doc
Benjamin Tissoires (1):
Input: synaptics - fix resolution for manually provided min/max
Benson Leung (2):
Input: atmel_mxt_ts - wait for CHG assert in mxt_check_bootloader
Input: atmel_mxt_ts - wait for CHG after bootloader resets
Beomho Seo (4):
Input: mcs_touchkey - fix incorrect input device name
Input: mms114 - fix incorrect input device name
Input: mcs5000_ts - fix incorrect input device name
Input: mcs5000_ts - switch to using managed resources
Christian Engelmayer (1):
Input: ims-pcu - fix uninitialized use of 'error' in ims_pcu_buffers_alloc()
Daniel Kurtz (1):
Input: atmel_mxt_ts - define helper functions for size and instances
Daniele Forsi (1):
Input: logips2pp - fix typo in Kconfig help
Dmitry Torokhov (3):
Input: evdev - get rid of old workaround for EVIOCGBIT
Input: remove race when instantiating polled device attributes
Input: implement managed polled input devices
Ezequiel Garcia (1):
Input: gpio-keys - move the gpio-keys bindings documentation
Fabio Estevam (1):
Input: twl6040-vibra - use devm functions
Gabriel FERNANDEZ (1):
Input: add st-keyscan driver
Hans de Goede (4):
Input: add driver for Allwinner sunxi SoC's rtp controller
Input: sun4i-ts - add support for temperature sensor
Input: elantech - deal with clickpads reporting right button events
Input: elantech - don't set bit 1 of reg_10 when the no_hw_res quirk is set
Himangi Saraogi (8):
Input: da9034-ts - switch to using managed resources
Input: adp5520-keys - switch to using managed resources
Input: jornada680_kbd - switch top using managed resources
Input: intel-mid-touch - switch to using managed resources
Input: 88pm860x_onkey - switch to using managed resources
Input: 88pm860x-ts - switch to using managed resources
Input: max8925_onkey - switch to using managed resources
Input: ab8500-ponkey - switch to using managed resources
Iiro Valkonen (1):
Input: atmel_mxt_ts - make wait-after-reset period compatible with all chips
Jason Gerecke (3):
Input: wacom - add support for 0x116 sensor on Win8 Panasonic CF-H2
Input: wacom - use unaligned access where necessary
Input: wacom - add support for three new ISDv4 sensors
Jean Delvare (1):
Input: w90p910_ts - depend on ARCH_W90X900
Jingoo Han (13):
Input: clps711x-keypad - make of_device_id array const
Input: gpio_keys - make of_device_id array const
Input: gpio_keys_polled - make of_device_id array const
Input: imx_keypad - make of_device_id array const
Input: gpio-beeper - make of_device_id array const
Input: rotary_encoder - make of_device_id array const
Input: olpc_apsp - make of_device_id array const
Input: apbps2 - make of_device_id array const
Input: auo-pixcir-ts - make of_device_id array const
Input: egalax_ts - make of_device_id array const
Input: lpc32xx_ts - make of_device_id array const
Input: mms114 - make of_device_id array const
Input: zforce - make of_device_id array const
Joachim Eastwood (2):
ARM: OMAP2+: remove unused omap4-keypad file and code
Input: omap-keypad - remove platform data support
Linus Walleij (1):
Input: tc3589x-keypad - support probing from device tree
Mark Brown (2):
Input: ads7846 - correct log message for spi_sync() errors
Input: ads7877 - remove bitrotted comment
Nick Dyer (16):
Input: atmel_mxt_ts - remove unnecessary platform data
Input: atmel_mxt_ts - improve T19 GPIO keys handling
Input: atmel_mxt_ts - return IRQ_NONE when interrupt handler fails
Input: atmel_mxt_ts - select FW_LOADER for firmware code
Input: atmel_mxt_ts - improve error reporting and debug
Input: atmel_mxt_ts - implement CRC check for configuration data
Input: atmel_mxt_ts - add additional bootloader addresses
Input: atmel_mxt_ts - read and report bootloader version
Input: atmel_mxt_ts - implement bootloader frame retries
Input: atmel_mxt_ts - improve bootloader progress output
Input: atmel_mxt_ts - add check for incorrect firmware file format
Input: atmel_mxt_ts - read screen config from chip
Input: atmel_mxt_ts - rename pressure to amplitude to match spec
Input: atmel_mxt_ts - rename touchscreen defines to include T9
Input: atmel_mxt_ts - handle multiple input reports in one message
Input: atmel_mxt_ts - fix invalid return from mxt_get_bootloader_version
Ping Cheng (2):
Input: wacom - set stylus_in_proximity when pen is in range
Input: wacom - process outbound for newer Cintiqs
Robert Woerle (1):
Input: edt-ft5x06 - fix an i2c write for M09 support
Roger Quadros (4):
Input: pixcir_i2c_ts - use devres managed resource allocations
Input: pixcir_i2c_ts - initialize interrupt mode and power mode
Input: pixcir_i2c_ts - get rid of pdata->attb_read_val()
Input: pixcir_i2c_ts - implement wakeup from suspend
Sachin Kamat (2):
Input: soc_button_array - remove duplicate inclusion of input.h
Input: max8997_haptic - add error handling for regulator and pwm
Sebastian Reichel (5):
Input: tsc2005 - use dev_err for error messages
Input: tsc2005 - convert driver to use devm_*
Input: add common DT binding for touchscreens
Input: tsc2005 - add DT support
DTS: ARM: OMAP3-N900: Add tsc2005 support
Stephen Boyd (1):
Input: pmic8xxx-pwrkey - set sane default for debounce time
Diffstat:
--------
.../{gpio/gpio_keys.txt => input/gpio-keys.txt} | 0
.../devicetree/bindings/input/st-keyscan.txt | 60 ++
.../bindings/input/touchscreen/sun4i.txt | 20 +
.../bindings/input/touchscreen/touchscreen.txt | 27 +
.../bindings/input/touchscreen/tsc2005.txt | 42 +
arch/arm/boot/dts/omap3-n900.dts | 17 +-
arch/arm/mach-omap2/devices.c | 33 -
arch/arm/mach-omap2/omap4-keypad.h | 8 -
arch/arm/mach-s5pv210/mach-goni.c | 8 -
drivers/input/evdev.c | 18 -
drivers/input/input-polldev.c | 131 +++-
drivers/input/keyboard/Kconfig | 13 +-
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/adp5520-keys.c | 32 +-
drivers/input/keyboard/clps711x-keypad.c | 2 +-
drivers/input/keyboard/gpio_keys.c | 145 ++--
drivers/input/keyboard/gpio_keys_polled.c | 100 +--
drivers/input/keyboard/imx_keypad.c | 2 +-
drivers/input/keyboard/jornada680_kbd.c | 36 +-
drivers/input/keyboard/mcs_touchkey.c | 2 +-
drivers/input/keyboard/omap4-keypad.c | 32 +-
drivers/input/keyboard/st-keyscan.c | 274 +++++++
drivers/input/keyboard/tc3589x-keypad.c | 66 +-
drivers/input/misc/88pm860x_onkey.c | 40 +-
drivers/input/misc/Kconfig | 2 +-
drivers/input/misc/ab8500-ponkey.c | 54 +-
drivers/input/misc/gpio-beeper.c | 29 +-
drivers/input/misc/ims-pcu.c | 1 +
drivers/input/misc/max8925_onkey.c | 55 +-
drivers/input/misc/max8997_haptic.c | 18 +-
drivers/input/misc/pmic8xxx-pwrkey.c | 8 +-
drivers/input/misc/rotary_encoder.c | 2 +-
drivers/input/misc/soc_button_array.c | 1 -
drivers/input/misc/twl6040-vibra.c | 83 +-
drivers/input/mouse/Kconfig | 2 +-
drivers/input/mouse/elantech.c | 27 +-
drivers/input/mouse/synaptics.c | 19 +-
drivers/input/serio/apbps2.c | 2 +-
drivers/input/serio/olpc_apsp.c | 2 +-
drivers/input/tablet/wacom_sys.c | 19 +
drivers/input/tablet/wacom_wac.c | 92 ++-
drivers/input/tablet/wacom_wac.h | 6 +
drivers/input/touchscreen/88pm860x-ts.c | 41 +-
drivers/input/touchscreen/Kconfig | 18 +-
drivers/input/touchscreen/Makefile | 2 +
drivers/input/touchscreen/ad7877.c | 5 -
drivers/input/touchscreen/ads7846.c | 2 +-
drivers/input/touchscreen/atmel_mxt_ts.c | 862 ++++++++++++++-------
drivers/input/touchscreen/auo-pixcir-ts.c | 2 +-
drivers/input/touchscreen/da9034-ts.c | 39 +-
drivers/input/touchscreen/edt-ft5x06.c | 2 +-
drivers/input/touchscreen/egalax_ts.c | 2 +-
drivers/input/touchscreen/intel-mid-touch.c | 47 +-
drivers/input/touchscreen/lpc32xx_ts.c | 2 +-
drivers/input/touchscreen/mcs5000_ts.c | 83 +-
drivers/input/touchscreen/mms114.c | 4 +-
drivers/input/touchscreen/of_touchscreen.c | 45 ++
drivers/input/touchscreen/pixcir_i2c_ts.c | 281 ++++++-
drivers/input/touchscreen/sun4i-ts.c | 339 ++++++++
drivers/input/touchscreen/tsc2005.c | 157 +++-
drivers/input/touchscreen/zforce_ts.c | 2 +-
drivers/platform/chrome/chromeos_laptop.c | 33 +-
include/linux/gpio_keys.h | 48 +-
include/linux/i2c/atmel_mxt_ts.h | 27 +-
include/linux/input-polldev.h | 3 +
include/linux/input/pixcir_ts.h | 44 +-
include/linux/input/touchscreen.h | 22 +
include/linux/platform_data/omap4-keypad.h | 13 -
68 files changed, 2517 insertions(+), 1139 deletions(-)
rename Documentation/devicetree/bindings/{gpio/gpio_keys.txt => input/gpio-keys.txt} (100%)
create mode 100644 Documentation/devicetree/bindings/input/st-keyscan.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2005.txt
delete mode 100644 arch/arm/mach-omap2/omap4-keypad.h
create mode 100644 drivers/input/keyboard/st-keyscan.c
create mode 100644 drivers/input/touchscreen/of_touchscreen.c
create mode 100644 drivers/input/touchscreen/sun4i-ts.c
create mode 100644 include/linux/input/touchscreen.h
delete mode 100644 include/linux/platform_data/omap4-keypad.h
--
Dmitry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] gpio: gpiolib: set gpiochip_remove retval to void
From: Lars-Peter Clausen @ 2014-06-09 11:29 UTC (permalink / raw)
To: Ben Dooks
Cc: Linux MIPS Mailing List, m, Linux-sh list, Linus Walleij,
platform-driver-x86, linux-leds@vger.kernel.org, driverdevel,
Alexandre Courbot, patches, linux-samsungsoc, Geert Uytterhoeven,
linux-input@vger.kernel.org, Linux Media Mailing List,
spear-devel, linux-gpio@vger.kernel.org, David Daney,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-wireless, linux-kernel
In-Reply-To: <20140608231823.GB10112@trinity.fluff.org>
On 06/09/2014 01:18 AM, Ben Dooks wrote:
> On Fri, May 30, 2014 at 08:16:59PM +0200, Lars-Peter Clausen wrote:
>> On 05/30/2014 07:33 PM, David Daney wrote:
>>> On 05/30/2014 04:39 AM, Geert Uytterhoeven wrote:
>>>> On Fri, May 30, 2014 at 1:30 PM, abdoulaye berthe <berthe.ab@gmail.com>
>>>> wrote:
>>>>> --- a/drivers/gpio/gpiolib.c
>>>>> +++ b/drivers/gpio/gpiolib.c
>>>>> @@ -1263,10 +1263,9 @@ static void gpiochip_irqchip_remove(struct
>>>>> gpio_chip *gpiochip);
>>>>> *
>>>>> * A gpio_chip with any GPIOs still requested may not be removed.
>>>>> */
>>>>> -int gpiochip_remove(struct gpio_chip *chip)
>>>>> +void gpiochip_remove(struct gpio_chip *chip)
>>>>> {
>>>>> unsigned long flags;
>>>>> - int status = 0;
>>>>> unsigned id;
>>>>>
>>>>> acpi_gpiochip_remove(chip);
>>>>> @@ -1278,24 +1277,15 @@ int gpiochip_remove(struct gpio_chip *chip)
>>>>> of_gpiochip_remove(chip);
>>>>>
>>>>> for (id = 0; id < chip->ngpio; id++) {
>>>>> - if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
>>>>> - status = -EBUSY;
>>>>> - break;
>>>>> - }
>>>>> - }
>>>>> - if (status == 0) {
>>>>> - for (id = 0; id < chip->ngpio; id++)
>>>>> - chip->desc[id].chip = NULL;
>>>>> -
>>>>> - list_del(&chip->list);
>>>>> + if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags))
>>>>> + panic("gpio: removing gpiochip with gpios still
>>>>> requested\n");
>>>>
>>>> panic?
>>>
>>> NACK to the patch for this reason. The strongest thing you should do here
>>> is WARN.
>>>
>>> That said, I am not sure why we need this whole patch set in the first place.
>>
>> Well, what currently happens when you remove a device that is a
>> provider of a gpio_chip which is still in use, is that the kernel
>> crashes. Probably with a rather cryptic error message. So this patch
>> doesn't really change the behavior, but makes it more explicit what
>> is actually wrong. And even if you replace the panic() by a WARN()
>> it will again just crash slightly later.
>>
>> This is a design flaw in the GPIO subsystem that needs to be fixed.
>
> Surely then the best way is to error out to the module unload and
> stop the driver being unloaded?
>
You can't error out on module unload, although that's not really relevant
here. gpiochip_remove() is typically called when the device that registered
the GPIO chip is unbound. And despite some remove() callbacks having a
return type of int you can not abort the removal of a device.
- Lars
^ permalink raw reply
* Re: [PATCH v2] leds: USB: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-09 11:42 UTC (permalink / raw)
To: Janne Kanniainen
Cc: Johan Hovold, Bryan Wu, rpurdie, linux-kernel, linux-leds,
linux-usb, Jiri Kosina, linux-input
In-Reply-To: <CAEEjW04UAsi7yOdmF3FZ3sLE7m7B-VAH0prDPYjaMXarAfNwcw@mail.gmail.com>
On Sat, Jun 07, 2014 at 01:12:39PM +0300, Janne Kanniainen wrote:
> > First of all, please reply to the original thread and make sure to not
> > drop people or lists from CC.
>
> Sorry this is my first patch and i didn't know that. Now I know.
>
> > For arrays you can use the ARRAY_SIZE() macro if that was the reason for
> > this change. I should have mentioned that when I pointed out that you
> > cannot use strlen().
>
> That wasn't the reason. I just thought it might be better to use u64
> than char[8]. I know why I can't use strlen and that was only careless
> error. And there was lot of them :( I will be more careful next time.
No problem, that's what review is for, and the second version was much
cleaner even if there were still a few issues (some hard to know about,
such as the DMA from stack issue, which is also a very common error).
> > Where did you get these (HID report) values from by the way?
>
> I got them by reverse engineering.
Traffic sniffing?
> >> +
> >> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> >> + enum led_brightness brightness)
> >> +{
> >> + struct gt683r_led *led =
> >> + container_of(led_cdev, struct gt683r_led, led_dev);
> >> +
> >> + mutex_lock(&led->lock);
> >
> > You cannot grab a mutex here since this function can be called from
> > interrupt context (if I remember correctly). Either way, you shouldn't
> > be holding the lock until the work task has finished...
>
> I thought use asked me to put some lock there:
>
> >> +
> >> +static void gt683r_brightness_set(struct led_classdev *led_cdev,
> >> + enum led_brightness brightness)
> >> +{
> >> + struct gt683r_led *led =
> >> + container_of(led_cdev, struct gt683r_led, led_dev);
> >> +
> >> + led->brightness = brightness;
> >
> > Missing locking?
I asked if locking was missing and did not specify how you should be
adding it. ;)
In fact, it seems you can get away with not adding any locking here.
Just do the (mutex) locking in gt683r_led_set (or gt683r_led_work).
Johan
^ permalink raw reply
* Re: [RESEND: PATCH 1/2] mfd: tc3589x: Add device tree bindings
From: Linus Walleij @ 2014-06-09 12:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org, Dmitry Torokhov, Linux Input,
Samuel Ortiz, Lee Jones
Cc: linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Mark Rutland, Linus Walleij
In-Reply-To: <1397028480-15413-1-git-send-email-linus.walleij@linaro.org>
On Wed, Apr 9, 2014 at 9:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This defines the device tree bindings for the Toshiba TC3589x
> series of multi-purpose expanders. Only the stuff I can test
> is defined: GPIO and keypad. Others may implement more
> subdevices further down the road.
>
> This is to complement
> commit a435ae1d51e2f18414f2a87219fdbe068231e692
> "mfd: Enable the tc3589x for Device Tree" which left off
> the definition of the device tree bindings.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Fix the keys/rows bindings to be u32 rather than
> /bits/ 8, inconsistency noted by Mark Rutland.
> ChangeLog v1->v2:
> - Include a verbose example in the DT bindings.
> - Explain why this is a stand-alone bindings patch.
Lee, ping on this - it seems to have fallen to the floor at some
point...
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/2] gpio: gpiolib: set gpiochip_remove retval to void
From: Andrzej Hajda @ 2014-06-09 13:15 UTC (permalink / raw)
To: Lars-Peter Clausen, Ben Dooks
Cc: Linux MIPS Mailing List, m, Linux-sh list, Linus Walleij,
platform-driver-x86, linux-leds@vger.kernel.org, driverdevel,
Alexandre Courbot, patches, linux-samsungsoc, Geert Uytterhoeven,
linux-input@vger.kernel.org, Linux Media Mailing List,
spear-devel, linux-gpio@vger.kernel.org, David Daney,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-wireless, linux-kernel
In-Reply-To: <53959A93.7080308@metafoo.de>
On 06/09/2014 01:29 PM, Lars-Peter Clausen wrote:
> On 06/09/2014 01:18 AM, Ben Dooks wrote:
>> On Fri, May 30, 2014 at 08:16:59PM +0200, Lars-Peter Clausen wrote:
>>> On 05/30/2014 07:33 PM, David Daney wrote:
>>>> On 05/30/2014 04:39 AM, Geert Uytterhoeven wrote:
>>>>> On Fri, May 30, 2014 at 1:30 PM, abdoulaye berthe <berthe.ab@gmail.com>
>>>>> wrote:
>>>>>> --- a/drivers/gpio/gpiolib.c
>>>>>> +++ b/drivers/gpio/gpiolib.c
>>>>>> @@ -1263,10 +1263,9 @@ static void gpiochip_irqchip_remove(struct
>>>>>> gpio_chip *gpiochip);
>>>>>> *
>>>>>> * A gpio_chip with any GPIOs still requested may not be removed.
>>>>>> */
>>>>>> -int gpiochip_remove(struct gpio_chip *chip)
>>>>>> +void gpiochip_remove(struct gpio_chip *chip)
>>>>>> {
>>>>>> unsigned long flags;
>>>>>> - int status = 0;
>>>>>> unsigned id;
>>>>>>
>>>>>> acpi_gpiochip_remove(chip);
>>>>>> @@ -1278,24 +1277,15 @@ int gpiochip_remove(struct gpio_chip *chip)
>>>>>> of_gpiochip_remove(chip);
>>>>>>
>>>>>> for (id = 0; id < chip->ngpio; id++) {
>>>>>> - if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
>>>>>> - status = -EBUSY;
>>>>>> - break;
>>>>>> - }
>>>>>> - }
>>>>>> - if (status == 0) {
>>>>>> - for (id = 0; id < chip->ngpio; id++)
>>>>>> - chip->desc[id].chip = NULL;
>>>>>> -
>>>>>> - list_del(&chip->list);
>>>>>> + if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags))
>>>>>> + panic("gpio: removing gpiochip with gpios still
>>>>>> requested\n");
>>>>>
>>>>> panic?
>>>>
>>>> NACK to the patch for this reason. The strongest thing you should do here
>>>> is WARN.
>>>>
>>>> That said, I am not sure why we need this whole patch set in the first place.
>>>
>>> Well, what currently happens when you remove a device that is a
>>> provider of a gpio_chip which is still in use, is that the kernel
>>> crashes. Probably with a rather cryptic error message. So this patch
>>> doesn't really change the behavior, but makes it more explicit what
>>> is actually wrong. And even if you replace the panic() by a WARN()
>>> it will again just crash slightly later.
>>>
>>> This is a design flaw in the GPIO subsystem that needs to be fixed.
>>
>> Surely then the best way is to error out to the module unload and
>> stop the driver being unloaded?
>>
>
> You can't error out on module unload, although that's not really relevant
> here. gpiochip_remove() is typically called when the device that registered
> the GPIO chip is unbound. And despite some remove() callbacks having a
> return type of int you can not abort the removal of a device.
It is a design flaw in many subsystems having providers and consumers,
not only GPIO. The same situation is with clock providers, regulators,
phys, drm_panels, ..., at least it was such last time I have tested it.
The problem is that many frameworks assumes that lifetime of provider is
always bigger than lifetime of its consumers, and this is wrong
assumption - usually it is not possible to prevent unbinding driver from
device, so if the device is a provider there is no way to inform
consumers about his removal.
Some solution for such problems is to use some kind of availability
callbacks for requesting resources (gpios, clocks, regulators,...)
instead of simple 'getters' (clk_get, gpiod_get). Callbacks should
guarantee that the resource is always valid between callback reporting
its availability and callback reporting its removal. Such approach seems
to be complicated at the first sight but it should allow to make the
code safe and as a bonus it will allow to avoid deferred probing.
Btw I have send already RFC for such framework [1].
[1]: https://lkml.org/lkml/2014/4/30/345
Regards
Andrzej
>
> - Lars
>
^ permalink raw reply
* RE: [PATCH 2/2] gpio: gpiolib: set gpiochip_remove retval to void
From: David Laight @ 2014-06-09 13:43 UTC (permalink / raw)
To: 'Andrzej Hajda', Lars-Peter Clausen, Ben Dooks
Cc: driverdevel, Linux MIPS Mailing List, spear-devel@list.st.com,
David Daney, Linux-sh list, netdev@vger.kernel.org, Linus Walleij,
patches@opensource.wolfsonmicro.com, linux-wireless,
linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,
linux-gpio@vger.kernel.org, Geert Uytterhoeven,
linux-arm-kernel@lists.infradead.org, m@bues.ch,
linux-input@vger.kernel.org, linux-samsungsoc
In-Reply-To: <5395B379.2010706@samsung.com>
From: Of Andrzej Hajda
...
> > You can't error out on module unload, although that's not really relevant
> > here. gpiochip_remove() is typically called when the device that registered
> > the GPIO chip is unbound. And despite some remove() callbacks having a
> > return type of int you can not abort the removal of a device.
>
> It is a design flaw in many subsystems having providers and consumers,
> not only GPIO. The same situation is with clock providers, regulators,
> phys, drm_panels, ..., at least it was such last time I have tested it.
>
> The problem is that many frameworks assumes that lifetime of provider is
> always bigger than lifetime of its consumers, and this is wrong
> assumption - usually it is not possible to prevent unbinding driver from
> device, so if the device is a provider there is no way to inform
> consumers about his removal.
>
> Some solution for such problems is to use some kind of availability
> callbacks for requesting resources (gpios, clocks, regulators,...)
> instead of simple 'getters' (clk_get, gpiod_get). Callbacks should
> guarantee that the resource is always valid between callback reporting
> its availability and callback reporting its removal. Such approach seems
> to be complicated at the first sight but it should allow to make the
> code safe and as a bonus it will allow to avoid deferred probing.
> Btw I have send already RFC for such framework [1].
Callbacks for delete are generally a locking nightmare.
A two-way handshake is also usually needed to avoid problems
with concurrent disconnect requests.
David
^ permalink raw reply
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
From: Jonathan Cameron @ 2014-06-09 17:43 UTC (permalink / raw)
To: Srinivas Pandruvada, Reyad Attiyat
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ
In-Reply-To: <538E092F.9040004-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On 03/06/14 18:43, Srinivas Pandruvada wrote:
> On 06/02/2014 04:14 PM, Reyad Attiyat wrote:
>> Updated magn_3d_channel enum for all possible north channels
>>
>> Added functions to setup iio_chan_spec array depending on which hid usage reports are found
>>
>> Renamed magn_val to iio_val to differentiate the index being used
>>
>> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[]
>>
>> Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each
>>
>> Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++---------
>> 1 file changed, 177 insertions(+), 94 deletions(-)
>>
>> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> index 6d162b7..32f4d90 100644
>> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> @@ -34,63 +34,54 @@ enum magn_3d_channel {
>> CHANNEL_SCAN_INDEX_X,
>> CHANNEL_SCAN_INDEX_Y,
>> CHANNEL_SCAN_INDEX_Z,
>> + CHANNEL_SCAN_INDEX_NORTH_MAGN,
>> + CHANNEL_SCAN_INDEX_NORTH_TRUE,
>> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
>> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
>> MAGN_3D_CHANNEL_MAX,
>> };
>>
>> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
>> +
>> struct magn_3d_state {
>> struct hid_sensor_hub_callbacks callbacks;
>> struct hid_sensor_common common_attributes;
>> struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
>> - u32 magn_val[MAGN_3D_CHANNEL_MAX];
>> -};
>> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
>>
>> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
>> + u32 iio_val[IIO_CHANNEL_MAX];
>> + int num_iio_channels;
>> };
>>
>> -/* Channel definitions */
>> -static const struct iio_chan_spec magn_3d_channels[] = {
>> - {
>> - .type = IIO_MAGN,
>> - .modified = 1,
>> - .channel2 = IIO_MOD_X,
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>> - BIT(IIO_CHAN_INFO_SCALE) |
>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>> - .scan_index = CHANNEL_SCAN_INDEX_X,
>> - }, {
>> - .type = IIO_MAGN,
>> - .modified = 1,
>> - .channel2 = IIO_MOD_Y,
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>> - BIT(IIO_CHAN_INFO_SCALE) |
>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>> - .scan_index = CHANNEL_SCAN_INDEX_Y,
>> - }, {
>> - .type = IIO_MAGN,
>> - .modified = 1,
>> - .channel2 = IIO_MOD_Z,
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>> - BIT(IIO_CHAN_INFO_SCALE) |
>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>> - .scan_index = CHANNEL_SCAN_INDEX_Z,
>
> May be we should have a field in const struct iio_chan_spec, so that we
> can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically.
But then it's not const to you have to take a per instance copy.
Once you are doing that you might as well just pull out into the copy
those channels that are actually present, at which point the field
doesn't have any purpose.
>
>
>> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
>> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
>> + int offset = -1;
>> +
>> + switch (usage_id) {
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
>> + offset = CHANNEL_SCAN_INDEX_X;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
>> + offset = CHANNEL_SCAN_INDEX_Y;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
>> + offset = CHANNEL_SCAN_INDEX_Z;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
>> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
>> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
>> + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
>> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
>> + break;
>> }
>> -};
>>
>> -/* Adjust channel real bits based on report descriptor */
>> -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
>> - int channel, int size)
>> -{
>> - channels[channel].scan_type.sign = 's';
>> - /* Real storage bits will change based on the report desc. */
>> - channels[channel].scan_type.realbits = size * 8;
>> - /* Maximum size of a sample to capture is u32 */
>> - channels[channel].scan_type.storagebits = sizeof(u32) * 8;
>> + return offset;
>> }
>>
>> /* Channel read_raw handler */
>> @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
>> {
>> struct magn_3d_state *magn_state = iio_priv(indio_dev);
>> int report_id = -1;
>> - u32 address;
>> + unsigned usage_id;
>> + int chan_index = -1;
>> int ret;
>> int ret_type;
>>
>> + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n");
>> +
>> *val = 0;
>> *val2 = 0;
>> switch (mask) {
>> case 0:
>> + /* We store the HID usage ID of the iio channel
>> + * in its address field
>> + */
>> + usage_id = chan->address;
>> + chan_index = magn_3d_usage_id_to_chan_index(usage_id);
>> + if(chan_index < 0)
>> + return -EINVAL;
>> +
>> report_id =
>> - magn_state->magn[chan->scan_index].report_id;
>> - address = magn_3d_addresses[chan->scan_index];
>> + magn_state->magn[chan_index].report_id;
>> if (report_id >= 0)
>> *val = sensor_hub_input_attr_get_raw_value(
>> magn_state->common_attributes.hsdev,
>> - HID_USAGE_SENSOR_COMPASS_3D, address,
>> + HID_USAGE_SENSOR_COMPASS_3D, usage_id,
>> report_id);
>> else {
>> *val = 0;
>> @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>> magn_state->common_attributes.data_ready);
>> if (magn_state->common_attributes.data_ready)
>> hid_sensor_push_data(indio_dev,
>> - magn_state->magn_val,
>> - sizeof(magn_state->magn_val));
>> + &(magn_state->iio_val),
>> + sizeof(magn_state->iio_val));
>>
>> return 0;
>> }
>>
>> +
>> /* Capture samples in local storage */
>> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
>> unsigned usage_id,
>> @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
>> struct iio_dev *indio_dev = platform_get_drvdata(priv);
>> struct magn_3d_state *magn_state = iio_priv(indio_dev);
>> int offset;
>> + u32 *magn_val;
>> int ret = -EINVAL;
>>
>> - switch (usage_id) {
>> + offset = magn_3d_usage_id_to_chan_index(usage_id);
>> + if(offset < 0)
>> + return ret;
>> +
>> + magn_val = magn_state->magn_val_addr[offset];
>> + if(!magn_val)
>> + return ret;
>> +
>> + *(magn_val) = *(u32 *)raw_data;
>> +
>> + return 0;
>> +}
>> +
>> +/* Setup the iio_chan_spec for HID Usage ID */
>> +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev,
>> + unsigned usage_id,
>> + unsigned attr_usage_id,
>> + struct iio_chan_spec *iio_chans,
>> + struct magn_3d_state *st)
>> +{
>> + int ret = -1;
>> + int iio_index;
>> + int magn_index;
>> + struct iio_chan_spec *channel;
>> +
>> + /* Setup magn_3d_state for new channel */
>> + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id);
>> + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){
>> + return -1;
>> + }
>> +
>> + /* Check if usage attribute exists in the sensor hub device */
>> + ret = sensor_hub_input_get_attribute_info(hsdev,
>> + HID_INPUT_REPORT,
>> + usage_id,
>> + attr_usage_id,
>> + &(st->magn[magn_index]));
>> + if(ret != 0){
>> + /* Usage not found. Nothing to setup.*/
>> + return 0;
>> + }
>> +
>> + iio_index = st->num_iio_channels;
>> + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){
>> + return -2;
>> + }
>> +
>> + /* Check if this usage hasn't been setup */
>> + if(st->magn_val_addr[magn_index] != NULL){
>> + return -3;
>> + }
>> + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]);
>> +
>> + /* Setup IIO Channel spec */
>> + channel = &(iio_chans[iio_index]);
>> +
>> + channel->type = IIO_MAGN;
>> + channel->address = attr_usage_id;
>> + channel->modified = 1;
>> +
>> + switch (attr_usage_id){
>> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
>> + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
>> + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
>> + channel->channel2 = IIO_MOD_NORTH_MAGN;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
>> + channel->channel2 = IIO_MOD_NORTH_TRUE;
>> + break;
>> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
>> + channel->channel2 = IIO_MOD_X;
>> + break;
>> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
>> + channel->channel2 = IIO_MOD_Y;
>> + break;
>> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
>> - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS;
>> - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] =
>> - *(u32 *)raw_data;
>> - ret = 0;
>> - break;
>> - default:
>> + channel->channel2 = IIO_MOD_Z;
>> break;
>> + default:
>> + return -4;
>> }
>>
>> + channel->info_mask_shared_by_type =
>> + BIT(IIO_CHAN_INFO_OFFSET) |
>> + BIT(IIO_CHAN_INFO_SCALE) |
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> + BIT(IIO_CHAN_INFO_HYSTERESIS);
>> +
>> + channel->scan_type.sign = 's';
>> + /* Real storage bits will change based on the report desc. */
>> + channel->scan_type.realbits = st->magn[magn_index].size * 8;
>> + /* Maximum size of a sample to capture is u32 */
>> + channel->scan_type.storagebits = sizeof(u32) * 8;
>> +
>> + (st->num_iio_channels)++;
>> + ret = 0;
>> +
>> return ret;
>> }
>>
>> -/* Parse report which is specific to an usage id*/
>> +/* Read the HID reports and setup IIO Channels */
>> static int magn_3d_parse_report(struct platform_device *pdev,
>> struct hid_sensor_hub_device *hsdev,
>> - struct iio_chan_spec *channels,
>> + struct iio_chan_spec *iio_chans,
>> unsigned usage_id,
>> struct magn_3d_state *st)
>> {
>> - int ret;
>> + int ret = 0;
>> int i;
>>
>> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
>> - ret = sensor_hub_input_get_attribute_info(hsdev,
>> - HID_INPUT_REPORT,
>> - usage_id,
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i,
>> - &st->magn[CHANNEL_SCAN_INDEX_X + i]);
>> - if (ret < 0)
>> - break;
>> - magn_3d_adjust_channel_bit_mask(channels,
>> - CHANNEL_SCAN_INDEX_X + i,
>> - st->magn[CHANNEL_SCAN_INDEX_X + i].size);
>> - }
>> - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n",
>> - st->magn[0].index,
>> - st->magn[0].report_id,
>> - st->magn[1].index, st->magn[1].report_id,
>> - st->magn[2].index, st->magn[2].report_id);
>> -
>> - /* Set Sensitivity field ids, when there is no individual modifier */
>> - if (st->common_attributes.sensitivity.index < 0) {
>> - sensor_hub_input_get_attribute_info(hsdev,
>> - HID_FEATURE_REPORT, usage_id,
>> - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
>> - HID_USAGE_SENSOR_DATA_ORIENTATION,
>> - &st->common_attributes.sensitivity);
>> - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n",
>> - st->common_attributes.sensitivity.index,
>> - st->common_attributes.sensitivity.report_id);
>> - }
>> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id);
>> + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS;
>> + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0;
>> + i++)
>> + ret = magn_3d_setup_new_iio_chan(hsdev,
>> + usage_id,
>> + i,
>> + iio_chans,
>> + st);
>> +
>> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret);
>> + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH;
>> + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0;
>> + i++)
>> + ret = magn_3d_setup_new_iio_chan(hsdev,
>> + usage_id,
>> + i,
>> + iio_chans,
>> + st);
>> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret);
>>
> I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function.
Yes, angles need to be in radians.
>
>> return ret;
>> }
>> @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels),
>> - GFP_KERNEL);
>> + channels = kcalloc(MAGN_3D_CHANNEL_MAX,
>> + sizeof(struct iio_chan_spec),
>> + GFP_KERNEL);
>
> I don't see kfree. Try devm_kcalloc?
>
>> if (!channels) {
>> - dev_err(&pdev->dev, "failed to duplicate channels\n");
>> + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n");
>> return -ENOMEM;
>> }
>>
>> @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
>> }
>>
>> indio_dev->channels = channels;
>> - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels);
>> + indio_dev->num_channels = magn_state->num_iio_channels;
>> indio_dev->dev.parent = &pdev->dev;
>> indio_dev->info = &magn_3d_info;
>> indio_dev->name = name;
>>
> Thanks,
> Srinivas
>
^ permalink raw reply
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
From: Jonathan Cameron @ 2014-06-09 17:47 UTC (permalink / raw)
To: Srinivas Pandruvada, Reyad Attiyat
Cc: linux-kernel, linux-iio, linux-input, Jiri Kosina
In-Reply-To: <538F3E6A.4080905@linux.intel.com>
On 04/06/14 16:42, Srinivas Pandruvada wrote:
> On 06/04/2014 08:23 AM, Reyad Attiyat wrote:
>> Hey Srinivas,
>>
>> On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada
>> <srinivas.pandruvada@linux.intel.com> wrote:
>>
>>>
>>> May be we should have a field in const struct iio_chan_spec, so that we
>>> can dynamically enable disable channels. In this way we can statically
>>> define channels, but can be enabled/disabled dynamically.
>>>
>> Would this require changing iio subsystem to create sysfs entries only
>> when enabled? Would we need to add functions to disable and enable
>> later on?
>
> This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this.
Replied to earlier message. If there is some common code we can factor out into
the core then I'm happy to do so, but I don't see an advantage in using
a field, rather than just tailoring the copy in the first place. e.g.
1) Pass whatever is needed to figure out how many channels are present.
2) Allocate space for that many channels.
3) Copy said channels from predefined versions, perhaps amending as necessary.
This is a reasonably common pattern in complex parts or those with hugely
variable numbers of channels...
>
>>>
>>> I think we need to present angle in radians. Are you basing change present
>>> in linux-next? This will automatically do in this function.
>>>
>> I'll look into this. What function should I use to make the iio chanel
>> to report radians?
> I think it will work if the existing sequence is maintained
> st->scale_precision = hid_sensor_format_scale(
> HID_USAGE_SENSOR_COMPASS_3D,
> &st->magn[CHANNEL_SCAN_INDEX_X],
> &st->scale_pre_decml, &st->scale_post_decml);
>
> So as long as you call this function, the scale value to user space will
> be returned correctly.
>
>>>
>>>
>>> I don't see kfree. Try devm_kcalloc?
>>>
>> I changed kmemdup to kcalloc so there is still a kfree in the exsiting
>> code. I can change this to devm_kcalloc but only if we don't go with
>> static channels that are enabled as found.
>>
> Since you are changing this part, devm_ calls are preferred, I think.
As long as it doesn't add complexity or possibility of race conditions,
devm calls are usually a good idea.
>
> Thanks,
> Srinivas
>
>
>> Thanks,
>> Reyad Attiyat
>>
>
^ permalink raw reply
* Re: [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages
From: Jonathan Cameron @ 2014-06-09 17:51 UTC (permalink / raw)
To: Reyad Attiyat, linux-kernel, linux-iio, srinivas.pandruvada,
linux-input, jkosina
In-Reply-To: <1401750890-31854-3-git-send-email-reyad.attiyat@gmail.com>
On 03/06/14 00:14, Reyad Attiyat wrote:
> Updated iio modifier enum for compass north usages,
> including magnetic/true north with tilt compensation.
>
> Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com>
This looks fine.
> ---
> drivers/iio/industrialio-core.c | 4 ++++
> include/linux/iio/types.h | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index ede16aec..2f523b5 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = {
> [IIO_MOD_LIGHT_RED] = "red",
> [IIO_MOD_LIGHT_GREEN] = "green",
> [IIO_MOD_LIGHT_BLUE] = "blue",
> + [IIO_MOD_NORTH_MAGN] = "north_magnetic",
> + [IIO_MOD_NORTH_TRUE] = "north_true",
> + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp",
> + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp",
> };
>
> /* relies on pairs of these shared then separate */
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 084d882..5bf8847 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -53,6 +53,10 @@ enum iio_modifier {
> IIO_MOD_LIGHT_RED,
> IIO_MOD_LIGHT_GREEN,
> IIO_MOD_LIGHT_BLUE,
> + IIO_MOD_NORTH_MAGN,
> + IIO_MOD_NORTH_TRUE,
> + IIO_MOD_NORTH_MAGN_TILT_COMP,
> + IIO_MOD_NORTH_TRUE_TILT_COMP
> };
>
> enum iio_event_type {
>
^ permalink raw reply
* Re: [PATCHv2 1/3] IIO: Documentation: Add north attribute to ABI docs
From: Jonathan Cameron @ 2014-06-09 17:50 UTC (permalink / raw)
To: Reyad Attiyat, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA,
linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ
In-Reply-To: <1401750890-31854-2-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 03/06/14 00:14, Reyad Attiyat wrote:
> Updated ABI documentation to inculde compass north usages.
>
> Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Amazing how many lines of documentation two small additions can make!
Anyhow, I'm keener on having these as as modifiers on a rotation rather than
there own type.
in_rot_from_north_magnetic_raw and friends (as per your suggestion elsewhere).
Otherwise, whilst obvious, perhaps a line or two in the main _raw attribute
description to state the obvious and say what this is...
J
> ---
> Documentation/ABI/testing/sysfs-bus-iio | 76 +++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 6e02c50..251c2bb 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -165,6 +165,10 @@ Description:
> What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_raw
> What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_raw
> KernelVersion: 2.6.35
> Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Description:
> @@ -249,6 +253,10 @@ What: /sys/bus/iio/devices/iio:deviceX/in_magn_scale
> What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_scale
> What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_scale
> What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_scale
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_scale
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_scale
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_scale
> +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_scale
> What: /sys/bus/iio/devices/iio:deviceX/in_pressureY_scale
> What: /sys/bus/iio/devices/iio:deviceX/in_pressure_scale
> KernelVersion: 2.6.35
> @@ -436,6 +444,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_thresh_rising_en
> What: /sys/.../iio:deviceX/events/in_magn_y_thresh_falling_en
> What: /sys/.../iio:deviceX/events/in_magn_z_thresh_rising_en
> What: /sys/.../iio:deviceX/events/in_magn_z_thresh_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_true_thresh_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_true_thresh_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_falling_en
> What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_rising_en
> What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_falling_en
> What: /sys/.../iio:deviceX/events/in_voltageY_thresh_rising_en
> @@ -481,6 +497,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_roc_rising_en
> What: /sys/.../iio:deviceX/events/in_magn_y_roc_falling_en
> What: /sys/.../iio:deviceX/events/in_magn_z_roc_rising_en
> What: /sys/.../iio:deviceX/events/in_magn_z_roc_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_true_roc_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_true_roc_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_falling_en
> +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_rising_en
> +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_falling_en
> What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_rising_en
> What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_falling_en
> What: /sys/.../iio:deviceX/events/in_voltageY_roc_rising_en
> @@ -527,6 +551,14 @@ What: /sys/.../events/in_magn_y_raw_thresh_rising_value
> What: /sys/.../events/in_magn_y_raw_thresh_falling_value
> What: /sys/.../events/in_magn_z_raw_thresh_rising_value
> What: /sys/.../events/in_magn_z_raw_thresh_falling_value
> +What: /sys/.../events/in_north_magnetic_raw_thresh_rising_value
> +What: /sys/.../events/in_north_magnetic_raw_thresh_falling_value
> +What: /sys/.../events/in_north_true_raw_thresh_rising_value
> +What: /sys/.../events/in_north_true_raw_thresh_falling_value
> +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_rising_value
> +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_falling_value
> +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_rising_value
> +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_falling_value
> What: /sys/.../events/in_voltageY_supply_raw_thresh_rising_value
> What: /sys/.../events/in_voltageY_supply_raw_thresh_falling_value
> What: /sys/.../events/in_voltageY_raw_thresh_rising_value
> @@ -577,6 +609,18 @@ What: /sys/.../events/in_magn_y_thresh_either_hysteresis
> What: /sys/.../events/in_magn_z_thresh_rising_hysteresis
> What: /sys/.../events/in_magn_z_thresh_falling_hysteresis
> What: /sys/.../events/in_magn_z_thresh_either_hysteresis
> +What: /sys/.../events/in_north_magnetic_thresh_rising_hysteresis
> +What: /sys/.../events/in_north_magnetic_thresh_falling_hysteresis
> +What: /sys/.../events/in_north_magnetic_thresh_either_hysteresis
> +What: /sys/.../events/in_north_true_thresh_rising_hysteresis
> +What: /sys/.../events/in_north_true_thresh_falling_hysteresis
> +What: /sys/.../events/in_north_true_thresh_either_hysteresis
> +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_hysteresis
> +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_hysteresis
> +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_either_hysteresis
> +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_hysteresis
> +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_hysteresis
> +What: /sys/.../events/in_north_true_tilt_comp_thresh_either_hysteresis
> What: /sys/.../events/in_voltageY_thresh_rising_hysteresis
> What: /sys/.../events/in_voltageY_thresh_falling_hysteresis
> What: /sys/.../events/in_voltageY_thresh_either_hysteresis
> @@ -624,6 +668,14 @@ What: /sys/.../events/in_magn_y_raw_roc_rising_value
> What: /sys/.../events/in_magn_y_raw_roc_falling_value
> What: /sys/.../events/in_magn_z_raw_roc_rising_value
> What: /sys/.../events/in_magn_z_raw_roc_falling_value
> +What: /sys/.../events/in_north_magnetic_raw_roc_rising_value
> +What: /sys/.../events/in_north_magnetic_raw_roc_falling_value
> +What: /sys/.../events/in_north_true_raw_roc_rising_value
> +What: /sys/.../events/in_north_true_raw_roc_falling_value
> +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_rising_value
> +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_falling_value
> +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_rising_value
> +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_falling_value
> What: /sys/.../events/in_voltageY_supply_raw_roc_rising_value
> What: /sys/.../events/in_voltageY_supply_raw_roc_falling_value
> What: /sys/.../events/in_voltageY_raw_roc_rising_value
> @@ -679,6 +731,22 @@ What: /sys/.../events/in_magn_z_thresh_rising_period
> What: /sys/.../events/in_magn_z_thresh_falling_period
> What: /sys/.../events/in_magn_z_roc_rising_period
> What: /sys/.../events/in_magn_z_roc_falling_period
> +What: /sys/.../events/in_north_magnetic_thresh_rising_period
> +What: /sys/.../events/in_north_magnetic_thresh_falling_period
> +What: /sys/.../events/in_north_magnetic_roc_rising_period
> +What: /sys/.../events/in_north_magnetic_roc_falling_period
> +What: /sys/.../events/in_north_true_thresh_rising_period
> +What: /sys/.../events/in_north_true_thresh_falling_period
> +What: /sys/.../events/in_north_true_roc_rising_period
> +What: /sys/.../events/in_north_true_roc_falling_period
> +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_period
> +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_period
> +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_rising_period
> +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_falling_period
> +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_period
> +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_period
> +What: /sys/.../events/in_north_true_tilt_comp_roc_rising_period
> +What: /sys/.../events/in_north_true_tilt_comp_roc_falling_period
> What: /sys/.../events/in_voltageY_supply_thresh_rising_period
> What: /sys/.../events/in_voltageY_supply_thresh_falling_period
> What: /sys/.../events/in_voltageY_supply_roc_rising_period
> @@ -776,6 +844,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_en
> What: /sys/.../iio:deviceX/scan_elements/in_magn_x_en
> What: /sys/.../iio:deviceX/scan_elements/in_magn_y_en
> What: /sys/.../iio:deviceX/scan_elements/in_magn_z_en
> +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_en
> +What: /sys/.../iio:deviceX/scan_elements/in_north_true_en
> +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_en
> +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_en
> What: /sys/.../iio:deviceX/scan_elements/in_timestamp_en
> What: /sys/.../iio:deviceX/scan_elements/in_voltageY_supply_en
> What: /sys/.../iio:deviceX/scan_elements/in_voltageY_en
> @@ -840,6 +912,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_index
> What: /sys/.../iio:deviceX/scan_elements/in_magn_x_index
> What: /sys/.../iio:deviceX/scan_elements/in_magn_y_index
> What: /sys/.../iio:deviceX/scan_elements/in_magn_z_index
> +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_index
> +What: /sys/.../iio:deviceX/scan_elements/in_north_true_index
> +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_index
> +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_index
> What: /sys/.../iio:deviceX/scan_elements/in_incli_x_index
> What: /sys/.../iio:deviceX/scan_elements/in_incli_y_index
> What: /sys/.../iio:deviceX/scan_elements/in_timestamp_index
>
^ permalink raw reply
* Re: [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages
From: Jonathan Cameron @ 2014-06-09 17:52 UTC (permalink / raw)
To: Reyad Attiyat, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA,
linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ
In-Reply-To: <5395F424.4070407-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 09/06/14 18:51, Jonathan Cameron wrote:
> On 03/06/14 00:14, Reyad Attiyat wrote:
>> Updated iio modifier enum for compass north usages,
>> including magnetic/true north with tilt compensation.
>>
>> Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> This looks fine.
gah. With the from_ prefix you suggested of course!
>> ---
>> drivers/iio/industrialio-core.c | 4 ++++
>> include/linux/iio/types.h | 4 ++++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index ede16aec..2f523b5 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = {
>> [IIO_MOD_LIGHT_RED] = "red",
>> [IIO_MOD_LIGHT_GREEN] = "green",
>> [IIO_MOD_LIGHT_BLUE] = "blue",
>> + [IIO_MOD_NORTH_MAGN] = "north_magnetic",
>> + [IIO_MOD_NORTH_TRUE] = "north_true",
>> + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp",
>> + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp",
>> };
>>
>> /* relies on pairs of these shared then separate */
>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> index 084d882..5bf8847 100644
>> --- a/include/linux/iio/types.h
>> +++ b/include/linux/iio/types.h
>> @@ -53,6 +53,10 @@ enum iio_modifier {
>> IIO_MOD_LIGHT_RED,
>> IIO_MOD_LIGHT_GREEN,
>> IIO_MOD_LIGHT_BLUE,
>> + IIO_MOD_NORTH_MAGN,
>> + IIO_MOD_NORTH_TRUE,
>> + IIO_MOD_NORTH_MAGN_TILT_COMP,
>> + IIO_MOD_NORTH_TRUE_TILT_COMP
>> };
>>
>> enum iio_event_type {
>>
>
^ permalink raw reply
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
From: Jonathan Cameron @ 2014-06-09 19:55 UTC (permalink / raw)
To: Reyad Attiyat, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA,
linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ
In-Reply-To: <1401750890-31854-4-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 03/06/14 00:14, Reyad Attiyat wrote:
> Updated magn_3d_channel enum for all possible north channels
>
> Added functions to setup iio_chan_spec array depending on which hid usage reports are found
>
> Renamed magn_val to iio_val to differentiate the index being used
>
> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[]
>
> Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each
>
> Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Reyad,
I've taken a rather quick look at this. Will probably want to take
one more close look at a newer version.
Various bits and bobs inline - mostly fine!
Jonathan
> ---
> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++---------
> 1 file changed, 177 insertions(+), 94 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index 6d162b7..32f4d90 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> @@ -34,63 +34,54 @@ enum magn_3d_channel {
> CHANNEL_SCAN_INDEX_X,
> CHANNEL_SCAN_INDEX_Y,
> CHANNEL_SCAN_INDEX_Z,
> + CHANNEL_SCAN_INDEX_NORTH_MAGN,
> + CHANNEL_SCAN_INDEX_NORTH_TRUE,
> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
> MAGN_3D_CHANNEL_MAX,
> };
>
> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
Please don't define a generic sounding name for a local
constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
> +
> struct magn_3d_state {
> struct hid_sensor_hub_callbacks callbacks;
> struct hid_sensor_common common_attributes;
> struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
> - u32 magn_val[MAGN_3D_CHANNEL_MAX];
> -};
> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
>
> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
> + u32 iio_val[IIO_CHANNEL_MAX];
> + int num_iio_channels;
> };
>
> -/* Channel definitions */
> -static const struct iio_chan_spec magn_3d_channels[] = {
> - {
> - .type = IIO_MAGN,
> - .modified = 1,
> - .channel2 = IIO_MOD_X,
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> - BIT(IIO_CHAN_INFO_SCALE) |
> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> - BIT(IIO_CHAN_INFO_HYSTERESIS),
> - .scan_index = CHANNEL_SCAN_INDEX_X,
> - }, {
> - .type = IIO_MAGN,
> - .modified = 1,
> - .channel2 = IIO_MOD_Y,
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> - BIT(IIO_CHAN_INFO_SCALE) |
> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> - BIT(IIO_CHAN_INFO_HYSTERESIS),
> - .scan_index = CHANNEL_SCAN_INDEX_Y,
> - }, {
> - .type = IIO_MAGN,
> - .modified = 1,
> - .channel2 = IIO_MOD_Z,
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> - BIT(IIO_CHAN_INFO_SCALE) |
> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> - BIT(IIO_CHAN_INFO_HYSTERESIS),
> - .scan_index = CHANNEL_SCAN_INDEX_Z,
> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
> + int offset = -1;
I'd personally prefer offset = -EINVAL and to have it assigned as
a default element in the switch statement rather that here.
> +
> + switch (usage_id) {
> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
> + offset = CHANNEL_SCAN_INDEX_X;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
> + offset = CHANNEL_SCAN_INDEX_Y;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
> + offset = CHANNEL_SCAN_INDEX_Z;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
> + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
> + break;
> }
> -};
>
> -/* Adjust channel real bits based on report descriptor */
> -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
> - int channel, int size)
> -{
> - channels[channel].scan_type.sign = 's';
> - /* Real storage bits will change based on the report desc. */
> - channels[channel].scan_type.realbits = size * 8;
> - /* Maximum size of a sample to capture is u32 */
> - channels[channel].scan_type.storagebits = sizeof(u32) * 8;
> + return offset;
> }
>
> /* Channel read_raw handler */
> @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
> {
> struct magn_3d_state *magn_state = iio_priv(indio_dev);
> int report_id = -1;
> - u32 address;
> + unsigned usage_id;
> + int chan_index = -1;
> int ret;
> int ret_type;
>
> + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n");
> +
> *val = 0;
> *val2 = 0;
> switch (mask) {
> case 0:
> + /* We store the HID usage ID of the iio channel
> + * in its address field
> + */
> + usage_id = chan->address;
> + chan_index = magn_3d_usage_id_to_chan_index(usage_id);
> + if(chan_index < 0)
> + return -EINVAL;
> +
> report_id =
> - magn_state->magn[chan->scan_index].report_id;
> - address = magn_3d_addresses[chan->scan_index];
> + magn_state->magn[chan_index].report_id;
> if (report_id >= 0)
> *val = sensor_hub_input_attr_get_raw_value(
> magn_state->common_attributes.hsdev,
> - HID_USAGE_SENSOR_COMPASS_3D, address,
> + HID_USAGE_SENSOR_COMPASS_3D, usage_id,
> report_id);
> else {
> *val = 0;
> @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
> magn_state->common_attributes.data_ready);
> if (magn_state->common_attributes.data_ready)
> hid_sensor_push_data(indio_dev,
> - magn_state->magn_val,
> - sizeof(magn_state->magn_val));
> + &(magn_state->iio_val),
> + sizeof(magn_state->iio_val));
>
> return 0;
> }
>
> +
> /* Capture samples in local storage */
> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> unsigned usage_id,
> @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> struct iio_dev *indio_dev = platform_get_drvdata(priv);
> struct magn_3d_state *magn_state = iio_priv(indio_dev);
> int offset;
> + u32 *magn_val;
> int ret = -EINVAL;
>
> - switch (usage_id) {
> + offset = magn_3d_usage_id_to_chan_index(usage_id);
> + if(offset < 0)
> + return ret;
> +
> + magn_val = magn_state->magn_val_addr[offset];
> + if(!magn_val)
> + return ret;
> +
> + *(magn_val) = *(u32 *)raw_data;
> +
> + return 0;
> +}
> +
> +/* Setup the iio_chan_spec for HID Usage ID */
> +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev,
> + unsigned usage_id,
> + unsigned attr_usage_id,
> + struct iio_chan_spec *iio_chans,
> + struct magn_3d_state *st)
> +{
> + int ret = -1;
This is kind of backwards to normal practice for this sort of function.
Better to maintain ret in the correct state at all times. Hence
start with it being 0 and set to negative when there is an error rather
than the other way around.
> + int iio_index;
> + int magn_index;
> + struct iio_chan_spec *channel;
> +
> + /* Setup magn_3d_state for new channel */
> + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id);
> + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){
> + return -1;
> + }
> +
> + /* Check if usage attribute exists in the sensor hub device */
> + ret = sensor_hub_input_get_attribute_info(hsdev,
> + HID_INPUT_REPORT,
> + usage_id,
> + attr_usage_id,
> + &(st->magn[magn_index]));
> + if(ret != 0){
> + /* Usage not found. Nothing to setup.*/
> + return 0;
> + }
> +
> + iio_index = st->num_iio_channels;
> + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){
I'd be tempted to allocate space dynamically but I guess this works.
> + return -2;
> + }
> +
> + /* Check if this usage hasn't been setup */
> + if(st->magn_val_addr[magn_index] != NULL){
Space before that bracket here and elsewhere.
Don't return your own error codes. Use standard ones. I guess thees
are left from debugging.
> + return -3;
> + }
> + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]);
> +
> + /* Setup IIO Channel spec */
> + channel = &(iio_chans[iio_index]);
Just pass this is in as a pointer in the first place?
That is a pointer to the current channel location, that may or may
not be filled by this function.
> +
> + channel->type = IIO_MAGN;
> + channel->address = attr_usage_id;
> + channel->modified = 1;
> +
> + switch (attr_usage_id){
> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
> + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
> + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
> + channel->channel2 = IIO_MOD_NORTH_MAGN;
> + break;
> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
> + channel->channel2 = IIO_MOD_NORTH_TRUE;
> + break;
> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
> + channel->channel2 = IIO_MOD_X;
> + break;
> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
> + channel->channel2 = IIO_MOD_Y;
> + break;
> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
> - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS;
> - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] =
> - *(u32 *)raw_data;
> - ret = 0;
> - break;
> - default:
> + channel->channel2 = IIO_MOD_Z;
> break;
> + default:
> + return -4;
huh? why -4. -EINVAL or -ENOSUP or something similar perhaps.
> }
>
> + channel->info_mask_shared_by_type =
> + BIT(IIO_CHAN_INFO_OFFSET) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_HYSTERESIS);
> +
> + channel->scan_type.sign = 's';
> + /* Real storage bits will change based on the report desc. */
> + channel->scan_type.realbits = st->magn[magn_index].size * 8;
> + /* Maximum size of a sample to capture is u32 */
> + channel->scan_type.storagebits = sizeof(u32) * 8;
> +
I'd keen num_iio_channels outside this function and increment it if
this function does not return an error.
> + (st->num_iio_channels)++;
Don't do this. ret should have been valid throughout... if not something
odd is going on with your use of ret.
Hmm. I see it is in the original code :( feel free to clean this up.
> + ret = 0;
> +
> return ret;
> }
>
> -/* Parse report which is specific to an usage id*/
> +/* Read the HID reports and setup IIO Channels */
> static int magn_3d_parse_report(struct platform_device *pdev,
> struct hid_sensor_hub_device *hsdev,
> - struct iio_chan_spec *channels,
> + struct iio_chan_spec *iio_chans,
> unsigned usage_id,
> struct magn_3d_state *st)
> {
> - int ret;
> + int ret = 0;
> int i;
>
> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> - ret = sensor_hub_input_get_attribute_info(hsdev,
> - HID_INPUT_REPORT,
> - usage_id,
> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i,
> - &st->magn[CHANNEL_SCAN_INDEX_X + i]);
> - if (ret < 0)
> - break;
> - magn_3d_adjust_channel_bit_mask(channels,
> - CHANNEL_SCAN_INDEX_X + i,
> - st->magn[CHANNEL_SCAN_INDEX_X + i].size);
> - }
> - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n",
> - st->magn[0].index,
> - st->magn[0].report_id,
> - st->magn[1].index, st->magn[1].report_id,
> - st->magn[2].index, st->magn[2].report_id);
> -
> - /* Set Sensitivity field ids, when there is no individual modifier */
> - if (st->common_attributes.sensitivity.index < 0) {
> - sensor_hub_input_get_attribute_info(hsdev,
> - HID_FEATURE_REPORT, usage_id,
> - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
> - HID_USAGE_SENSOR_DATA_ORIENTATION,
> - &st->common_attributes.sensitivity);
> - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n",
> - st->common_attributes.sensitivity.index,
> - st->common_attributes.sensitivity.report_id);
> - }
> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id);
> + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS;
> + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0;
> + i++)
> + ret = magn_3d_setup_new_iio_chan(hsdev,
> + usage_id,
> + i,
> + iio_chans,
> + st);
> +
> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret);
> + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH;
> + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0;
> + i++)
> + ret = magn_3d_setup_new_iio_chan(hsdev,
> + usage_id,
> + i,
> + iio_chans,
> + st);
As stated above, I'd use whether this succeeds for a given channel to
increment the location in iio_chans and then pass in only the 'next' channel
location on each pass. Moving the bounds checks out here as well would be
make for a cleaner magn_3d_setup_new_iio_chan function.
> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret);
>
> return ret;
> }
> @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
> return ret;
> }
>
> - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels),
> - GFP_KERNEL);
> + channels = kcalloc(MAGN_3D_CHANNEL_MAX,
> + sizeof(struct iio_chan_spec),
> + GFP_KERNEL);
> if (!channels) {
> - dev_err(&pdev->dev, "failed to duplicate channels\n");
> + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n");
> return -ENOMEM;
> }
>
> @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
> }
>
> indio_dev->channels = channels;
> - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels);
> + indio_dev->num_channels = magn_state->num_iio_channels;
With a bit of rearranging it strikes me that you can fill num_channels
directly rather than having another copy of it....
> indio_dev->dev.parent = &pdev->dev;
> indio_dev->info = &magn_3d_info;
> indio_dev->name = name;
>
^ permalink raw reply
* Re: [PATCH 2/2] gpio: gpiolib: set gpiochip_remove retval to void
From: Andrzej Hajda @ 2014-06-10 6:57 UTC (permalink / raw)
To: David Laight, Lars-Peter Clausen, Ben Dooks
Cc: driverdevel, Linux MIPS Mailing List, spear-devel@list.st.com,
David Daney, Linux-sh list, netdev@vger.kernel.org, Linus Walleij,
patches@opensource.wolfsonmicro.com, linux-wireless,
linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,
linux-gpio@vger.kernel.org, Geert Uytterhoeven,
linux-arm-kernel@lists.infradead.org, m@bues.ch,
linux-input@vger.kernel.org, linux-samsung-soc
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17259A1F@AcuExch.aculab.com>
On 06/09/2014 03:43 PM, David Laight wrote:
> From: Of Andrzej Hajda
> ...
>>> You can't error out on module unload, although that's not really relevant
>>> here. gpiochip_remove() is typically called when the device that registered
>>> the GPIO chip is unbound. And despite some remove() callbacks having a
>>> return type of int you can not abort the removal of a device.
>>
>> It is a design flaw in many subsystems having providers and consumers,
>> not only GPIO. The same situation is with clock providers, regulators,
>> phys, drm_panels, ..., at least it was such last time I have tested it.
>>
>> The problem is that many frameworks assumes that lifetime of provider is
>> always bigger than lifetime of its consumers, and this is wrong
>> assumption - usually it is not possible to prevent unbinding driver from
>> device, so if the device is a provider there is no way to inform
>> consumers about his removal.
>>
>> Some solution for such problems is to use some kind of availability
>> callbacks for requesting resources (gpios, clocks, regulators,...)
>> instead of simple 'getters' (clk_get, gpiod_get). Callbacks should
>> guarantee that the resource is always valid between callback reporting
>> its availability and callback reporting its removal. Such approach seems
>> to be complicated at the first sight but it should allow to make the
>> code safe and as a bonus it will allow to avoid deferred probing.
>> Btw I have send already RFC for such framework [1].
>
> Callbacks for delete are generally a locking nightmare.
> A two-way handshake is also usually needed to avoid problems
> with concurrent disconnect requests.
The framework I have proposed is lock-less[1] and concurrent requests
are serialized so both objections are invalid.
[1]: in fact the framework uses spinlock, but only to protect internal
list simple operations, and even this could be converted to fully
lock-less implementation.
Andrzej
>
> David
>
^ permalink raw reply
* [PATCH] drivers: Kconfig: Add HAS_IOMEM dependency for allmodconfig under score architecture
From: Chen Gang @ 2014-06-10 12:04 UTC (permalink / raw)
To: jkosina, kishon; +Cc: linux-input, linux-next, liqin.linux, lennox.wu
When NO_IOMEM is enabled (e.g. score architecture), some drivers which
need HAS_IOMEM need notice about it, or it will report related warning:
scripts/kconfig/conf --allmodconfig Kconfig
kernel/time/Kconfig:162:warning: range is invalid
warning: (PINCTRL_ROCKCHIP && PINCTRL_DOVE && POWER_RESET_KEYSTONE && S3C2410_WATCHDOG && VIDEO_S5P_FIMC && PHY_SAMSUNG_USB2) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM)
warning: (GPIO_SCH && GPIO_ICH && GPIO_VX855 && GPIO_RDC321X && IE6XX_WDT && RADIO_WL1273 && HID_SENSOR_HUB && MFD_NVEC) selects MFD_CORE which has unmet direct dependencies (HAS_IOMEM)
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
drivers/hid/Kconfig | 2 +-
drivers/phy/Kconfig | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 800c8b6..5e79c6a 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -810,7 +810,7 @@ config HID_ZYDACRON
config HID_SENSOR_HUB
tristate "HID Sensors framework support"
- depends on HID
+ depends on HID && HAS_IOMEM
select MFD_CORE
default n
---help---
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 16a2f06..fcdfe7c 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -122,6 +122,7 @@ config PHY_SUN4I_USB
config PHY_SAMSUNG_USB2
tristate "Samsung USB 2.0 PHY driver"
+ depends on HAS_IOMEM
select GENERIC_PHY
select MFD_SYSCON
help
--
1.9.2.459.g68773ac
^ permalink raw reply related
* Re: [PATCH] drivers: Kconfig: Add HAS_IOMEM dependency for allmodconfig under score architecture
From: Chen Gang @ 2014-06-10 12:20 UTC (permalink / raw)
To: jkosina, kishon; +Cc: linux-input, linux-next, liqin.linux, lennox.wu
In-Reply-To: <5396F46A.30805@gmail.com>
On 06/10/2014 08:04 PM, Chen Gang wrote:
> When NO_IOMEM is enabled (e.g. score architecture), some drivers which
> need HAS_IOMEM need notice about it, or it will report related warning:
>
> scripts/kconfig/conf --allmodconfig Kconfig
> kernel/time/Kconfig:162:warning: range is invalid
^
Oh, sorry, this line above need be removed, it is not for this patch, if
still necessary to send patch v2 for it, please let me know, thanks.
> warning: (PINCTRL_ROCKCHIP && PINCTRL_DOVE && POWER_RESET_KEYSTONE && S3C2410_WATCHDOG && VIDEO_S5P_FIMC && PHY_SAMSUNG_USB2) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM)
> warning: (GPIO_SCH && GPIO_ICH && GPIO_VX855 && GPIO_RDC321X && IE6XX_WDT && RADIO_WL1273 && HID_SENSOR_HUB && MFD_NVEC) selects MFD_CORE which has unmet direct dependencies (HAS_IOMEM)
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> drivers/hid/Kconfig | 2 +-
> drivers/phy/Kconfig | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 800c8b6..5e79c6a 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -810,7 +810,7 @@ config HID_ZYDACRON
>
> config HID_SENSOR_HUB
> tristate "HID Sensors framework support"
> - depends on HID
> + depends on HID && HAS_IOMEM
> select MFD_CORE
> default n
> ---help---
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 16a2f06..fcdfe7c 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -122,6 +122,7 @@ config PHY_SUN4I_USB
>
> config PHY_SAMSUNG_USB2
> tristate "Samsung USB 2.0 PHY driver"
> + depends on HAS_IOMEM
> select GENERIC_PHY
> select MFD_SYSCON
> help
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH] HID: usbhid: remove unneeded initialization of quirks_param[]
From: Jiri Kosina @ 2014-06-10 12:33 UTC (permalink / raw)
To: Mathias Krause; +Cc: linux-usb, linux-input
In-Reply-To: <1401996051-7951-1-git-send-email-minipli@googlemail.com>
On Thu, 5 Jun 2014, Mathias Krause wrote:
> The quirks_param array is located in the BSS, no need to explicitly
> initialize it with NULL.
>
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> ---
> drivers/hid/usbhid/hid-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] HID: hid-sensor-hub: Make all locks of dyn_callback_lock disable interrupts.
From: Reyad Attiyat @ 2014-06-10 20:11 UTC (permalink / raw)
To: linux-kernel, srinivas.pandruvada, jkosina, linux-input; +Cc: Reyad Attiyat
The dynamic callback lock (dyn_callback_lock) must not be interrupted
when locked because the lock is used in the interrupt handler function sensor_hub_raw_event().
Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com>
---
drivers/hid/hid-sensor-hub.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index a8d5c8f..6547138 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -133,10 +133,11 @@ static struct hid_sensor_hub_callbacks *sensor_hub_get_callback(
struct hid_sensor_hub_device **hsdev,
void **priv)
{
+ unsigned long flags;
struct hid_sensor_hub_callbacks_list *callback;
struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
- spin_lock(&pdata->dyn_callback_lock);
+ spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
if (callback->usage_id == usage_id &&
(collection_index >=
@@ -145,10 +146,10 @@ static struct hid_sensor_hub_callbacks *sensor_hub_get_callback(
callback->hsdev->end_collection_index)) {
*priv = callback->priv;
*hsdev = callback->hsdev;
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return callback->usage_callback;
}
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return NULL;
}
@@ -157,19 +158,20 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
u32 usage_id,
struct hid_sensor_hub_callbacks *usage_callback)
{
+ unsigned long flags;
struct hid_sensor_hub_callbacks_list *callback;
struct sensor_hub_data *pdata = hid_get_drvdata(hsdev->hdev);
- spin_lock(&pdata->dyn_callback_lock);
+ spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
if (callback->usage_id == usage_id &&
callback->hsdev == hsdev) {
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return -EINVAL;
}
callback = kzalloc(sizeof(*callback), GFP_ATOMIC);
if (!callback) {
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return -ENOMEM;
}
callback->hsdev = hsdev;
@@ -177,7 +179,7 @@ int sensor_hub_register_callback(struct hid_sensor_hub_device *hsdev,
callback->usage_id = usage_id;
callback->priv = NULL;
list_add_tail(&callback->list, &pdata->dyn_callback_list);
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return 0;
}
@@ -186,10 +188,11 @@ EXPORT_SYMBOL_GPL(sensor_hub_register_callback);
int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
u32 usage_id)
{
+ unsigned long flags;
struct hid_sensor_hub_callbacks_list *callback;
struct sensor_hub_data *pdata = hid_get_drvdata(hsdev->hdev);
- spin_lock(&pdata->dyn_callback_lock);
+ spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
if (callback->usage_id == usage_id &&
callback->hsdev == hsdev) {
@@ -197,7 +200,7 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
kfree(callback);
break;
}
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return 0;
}
@@ -376,34 +379,36 @@ EXPORT_SYMBOL_GPL(sensor_hub_input_get_attribute_info);
#ifdef CONFIG_PM
static int sensor_hub_suspend(struct hid_device *hdev, pm_message_t message)
{
+ unsigned long flags;
struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
struct hid_sensor_hub_callbacks_list *callback;
hid_dbg(hdev, " sensor_hub_suspend\n");
- spin_lock(&pdata->dyn_callback_lock);
+ spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
list_for_each_entry(callback, &pdata->dyn_callback_list, list) {
if (callback->usage_callback->suspend)
callback->usage_callback->suspend(
callback->hsdev, callback->priv);
}
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return 0;
}
static int sensor_hub_resume(struct hid_device *hdev)
{
+ unsigned long flags;
struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
struct hid_sensor_hub_callbacks_list *callback;
hid_dbg(hdev, " sensor_hub_resume\n");
- spin_lock(&pdata->dyn_callback_lock);
+ spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
list_for_each_entry(callback, &pdata->dyn_callback_list, list) {
if (callback->usage_callback->resume)
callback->usage_callback->resume(
callback->hsdev, callback->priv);
}
- spin_unlock(&pdata->dyn_callback_lock);
+ spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
return 0;
}
--
1.9.3
^ permalink raw reply related
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