* Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
From: Hans de Goede @ 2014-01-02 13:45 UTC (permalink / raw)
To: Heiko Stübner,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Dmitry Torokhov, Maxime Ripard,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
In-Reply-To: <1700375.GaI3zFl6RI@phil>
Hi,
On 01/02/2014 12:59 PM, Heiko Stübner wrote:
> Hi Hans, Dmitry,
>
> Am Donnerstag, 2. Januar 2014, 10:37:47 schrieb Hans de Goede:
>> Hi,
>>
>> On 01/01/2014 09:56 PM, Dmitry Torokhov wrote:
>>> Hi Hans,
>>>
>>> On Wed, Jan 01, 2014 at 08:30:07PM +0100, Hans de Goede wrote:
>>>> +Required properties:
>>>> + - compatible: "allwinner,sun4i-lradc-keys"
>>>> + - reg: mmio address range of the chip
>>>> + - interrupts: interrupt to which the chip is connected
>>>> + - allwinner,chan0-step: step in mV between keys must be 150 or 200
>>>> + - allwinner,chan0-keycodes: array of include/uapi/linux/input.h KEY_
>>>> codes>
>>> I think this should be "linux,chan0-keycodes".
>>
>> Right, because the codes are Linux specific, will fix in v2.
>
> but the property with its "chan0-" thingy would be allwinner-specific if I'm
> not mistaken.
Correct, but denoting that this is linux only is more important, so as to
avoid namespace collisions.
>
> Also, instead of inventing yet another vendor-specific property, why not re-use
> a button binding similar to gpio-keys like:
>
> lradc: lradc@01c22800 {
> compatible = "allwinner,sun4i-lradc-keys";
> reg = <0x01c22800 0x100>;
> interrupts = <31>;
> allwinner,chan0-step = <200>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> button@0 {
> reg = <0>; /* your channel index from above */
> linux,code = <115>; /* already used as dt-property */
> };
>
> button@1 {
> reg = <1>;
> linux,code = <114>;
> };
Ugh no. Having a vendor specific property which is KISS certainly beats this,
both wrt ease of writing dts files as well as wrt the dts parsing code in the driver.
Regards,
Hans
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [Xen-devel] [PATCH v3 1/2] xen/pvhvm: If xen_platform_pci=0 is set don't blow up (v3).
From: David Vrabel @ 2014-01-02 15:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Fabio Fantoni, axboe, leosilva, linux-fbdev, dmitry.torokhov,
ian.campbell, stefano.stabellini, linux-pci, netdev, ashley,
tpmdd, linux-kernel, mail, tpmdd-devel, tomi.valkeinen,
david.vrabel, linux-input, bhelgaas, xen-devel, boris.ostrovsky,
plagnioj, peterhuewe
In-Reply-To: <20131231143258.GA3018@phenom.dumpdata.com>
On 31/12/13 14:32, Konrad Rzeszutek Wilk wrote:
>> That is because 'disks' is incorrect. It should have been 'ide-disks'
>>
>> [ 0.000000] unrecognised option 'disks' in parameter 'xen_emul_unplug'
>>
>> With the 'ide-disks' it should work. I will update the description to
>> mention 'ide-disks' instead of 'disks'. Thank you for finding this!
>>
>
> I've v4 with said update and will push it to Linus shortly.
>
> Thanks!
>
> P.S.
> Here is v4:
>
>>From 275a81e7496d3532e5b4752703c50a7c8355a6c7 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Tue, 26 Nov 2013 15:05:40 -0500
> Subject: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up (v4).
>
> The user has the option of disabling the platform driver:
> 00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
>
> which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> and allow the PV drivers to take over. If the user wishes
> to disable that they can set:
>
> xen_platform_pci=0
> (in the guest config file)
>
> or
> xen_emul_unplug=never
> (on the Linux command line)
>
> except it does not work properly. The PV drivers still try to
> load and since the Xen platform driver is not run - and it
> has not initialized the grant tables, most of the PV drivers
> stumble upon:
>
> input: Xen Virtual Keyboard as /devices/virtual/input/input5
> input: Xen Virtual Pointer as /devices/virtual/input/input6M
> ------------[ cut here ]------------
> kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> invalid opcode: 0000 [#1] SMP
> Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> RIP: 0010:[<ffffffff813ddc40>] [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
> Call Trace:
> [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
> [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
> [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
> [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
> [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
> [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
> [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
> [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
> [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
> [<ffffffff8145e7d9>] driver_attach+0x19/0x20
> [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
> [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
> [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
> [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
> [<ffffffffa0015000>] ? 0xffffffffa0014fff
> [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
> [<ffffffff81002049>] do_one_initcall+0x49/0x170
>
> .. snip..
>
> which is hardly nice. This patch fixes this by having each
> PV driver check for:
> - if running in PV, then it is fine to execute (as that is their
> native environment).
> - if running in HVM, check if user wanted 'xen_emul_unplug=never',
> in which case bail out and don't load any PV drivers.
> - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
> does not exist, then bail out and not load PV drivers.
> - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=ide-disks',
> then bail out for all PV devices _except_ the block one.
> Ditto for the network one ('nics').
> - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=unnecessary'
> then load block PV driver, and also setup the legacy IDE paths.
> In (v3) make it actually load PV drivers.
[...]
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -69,6 +69,80 @@ static int check_platform_magic(void)
> return 0;
> }
>
> +bool xen_has_pv_devices()
> +{
> + if (!xen_domain())
> + return false;
> +
> + /* PV domains always have them. */
> + if (xen_pv_domain())
> + return true;
> +
> + /* And user has xen_platform_pci=0 set in guest config as
> + * driver did not modify the value. */
> + if (xen_platform_pci_unplug == 0)
> + return false;
> +
> + if (xen_platform_pci_unplug & XEN_UNPLUG_NEVER)
> + return false;
> +
> + if (xen_platform_pci_unplug & XEN_UNPLUG_ALL)
> + return true;
> +
> + /* This is an odd one - we are going to run legacy
> + * and PV drivers at the same time. */
> + if (xen_platform_pci_unplug & XEN_UNPLUG_UNNECESSARY)
> + return true;
> +
> + /* And the caller has to follow with xen_pv_{disk,nic}_devices
> + * to be certain which driver can load. */
> + return false;
This may result in:
xen_has_pv_devices() == false
xen_has_pv_disk_devices() == true
which looks odd to me. Surely xen_has_pv_*_devices() is a subset of
xen_has_pv_devices()?
David
^ permalink raw reply
* Re: [PATCH] Input: cros_ec_keyb - switch from using uint8_t to u8
From: Doug Anderson @ 2014-01-02 16:12 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Simon Glass, Vincent Palatin, Luigi Semenzato,
linux-kernel@vger.kernel.org
In-Reply-To: <20131231193442.GA25208@core.coreip.homeip.net>
Dmitry,
On Tue, Dec 31, 2013 at 11:34 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> u8 is proper in-kernel type for unsigned byte data.
I won't say that I keep up with all the latest trends here, but this
surprised me so I did some research. My findings don't agree with
your statement. Perhaps there are different standards that are used
for the input subsystem?
Specifically looking at
<https://www.kernel.org/doc/Documentation/CodingStyle>, I see:
Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
signed equivalents which are identical to standard types are
permitted -- although they are not mandatory in new code of your
own.
When editing existing code which already uses one or the other set
of types, you should conform to the existing choices in that code.
That makes it sound like the author of that document would prefer
uint8_t but will accept u8. It also seems like if code is consistent
about using a given type (as this code is) that it shouldn't be
changed.
I'm always happy to be enlightened, though!
-Doug
^ permalink raw reply
* Re: [PATCH] Input: cros_ec_keyb - avoid variable-length arrays on stack
From: Doug Anderson @ 2014-01-02 17:40 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Simon Glass, Vincent Palatin, Luigi Semenzato,
linux-kernel@vger.kernel.org, Olof Johansson
In-Reply-To: <20131231193530.GA25261@core.coreip.homeip.net>
Dmitry,
Thanks for cleaning up cros_eckeyb. :) I'm a little curious about
the motivation here. I can't imagine a keyboard with all that many
columns (ours has 13), so it's really not taking a whole lot off of
the stack. Are you trying to make some sort of automated checker
happy, or just generally trying to keep the kernel consistent?
In any case, I'm not opposed to moving these bytes off the stack.
Comments below, though...
---
On Tue, Dec 31, 2013 at 11:35 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 80 ++++++++++++++++++++---------------
> 1 file changed, 45 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index d44c5d4..03cb960 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -39,6 +39,7 @@
> * @keymap_data: Matrix keymap data used to convert to keyscan values
> * @ghost_filter: true to enable the matrix key-ghosting filter
> * @old_kb_state: bitmap of keys pressed last scan
> + * @kb_state: bitmap of keys currently pressed
> * @dev: Device pointer
> * @idev: Input device
> * @ec: Top level ChromeOS device to use to talk to EC
> @@ -50,17 +51,17 @@ struct cros_ec_keyb {
> int row_shift;
> const struct matrix_keymap_data *keymap_data;
> bool ghost_filter;
> - u8 *old_kb_state;
> -
> struct device *dev;
> struct input_dev *idev;
> struct cros_ec_device *ec;
> struct notifier_block notifier;
> +
> + u8 *old_kb_state;
> + u8 kb_state[];
nit: you've moved old_kb_state to the end of the structure but you
haven't moved the description in the comments above. I'd expect the
ordering in the comment and the ordering in the comment to match.
> };
>
>
> -static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
> - u8 *buf, int row)
> +static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev, int row)
> {
> int pressed_in_row = 0;
> int row_has_teeth = 0;
> @@ -68,9 +69,9 @@ static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
>
> mask = 1 << row;
> for (col = 0; col < ckdev->cols; col++) {
> - if (buf[col] & mask) {
> + if (ckdev->kb_state[col] & mask) {
> pressed_in_row++;
> - row_has_teeth |= buf[col] & ~mask;
> + row_has_teeth |= ckdev->kb_state[col] & ~mask;
> if (pressed_in_row > 1 && row_has_teeth) {
> /* ghosting */
> dev_dbg(ckdev->dev,
> @@ -89,7 +90,7 @@ static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
> * Returns true when there is at least one combination of pressed keys that
> * results in ghosting.
> */
> -static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
> +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev)
> {
> int row;
>
> @@ -120,7 +121,7 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
> * cheat because the number of rows is small.
> */
> for (row = 0; row < ckdev->rows; row++)
> - if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
> + if (cros_ec_keyb_row_has_ghosting(ckdev, row))
> return true;
>
> return false;
> @@ -131,8 +132,7 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
> * press/release events accordingly. The keyboard state is 13 bytes (one byte
> * per column)
> */
> -static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> - u8 *kb_state, int len)
> +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, int len)
> {
> struct input_dev *idev = ckdev->idev;
> int col, row;
> @@ -142,7 +142,7 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>
> num_cols = len;
>
> - if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
> + if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev)) {
> /*
> * Simple-minded solution: ignore this state. The obvious
> * improvement is to only ignore changes to keys involved in
> @@ -157,7 +157,7 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> int pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> const unsigned short *keycodes = idev->keycode;
>
> - new_state = kb_state[col] & (1 << row);
> + new_state = ckdev->kb_state[col] & (1 << row);
> old_state = ckdev->old_kb_state[col] & (1 << row);
> if (new_state != old_state) {
> dev_dbg(ckdev->dev,
> @@ -168,9 +168,12 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> new_state);
> }
> }
> - ckdev->old_kb_state[col] = kb_state[col];
> }
> +
> input_sync(ckdev->idev);
> +
> + memcpy(ckdev->old_kb_state, ckdev->kb_state,
> + ckdev->cols * sizeof(*ckdev->kb_state));
Any motivation for why you've moved this to a memcpy? In my mind the
old code is easier to understand and I'm not convinced about the speed
/ code space gains (introducing a function call to save 13 assignment
operations). Again this is not something I'll NAK, but it seems like
a bit of code churn.
> }
>
> static int cros_ec_keyb_open(struct input_dev *dev)
> @@ -189,10 +192,10 @@ static void cros_ec_keyb_close(struct input_dev *dev)
> &ckdev->notifier);
> }
>
> -static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, u8 *kb_state)
> +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev)
> {
> return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> - kb_state, ckdev->cols);
> + ckdev->kb_state, ckdev->cols);
> }
>
> static int cros_ec_keyb_work(struct notifier_block *nb,
> @@ -201,11 +204,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> int ret;
> struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> notifier);
> - u8 kb_state[ckdev->cols];
>
> - ret = cros_ec_keyb_get_state(ckdev, kb_state);
> + ret = cros_ec_keyb_get_state(ckdev);
> if (ret >= 0)
> - cros_ec_keyb_process(ckdev, kb_state, ret);
> + cros_ec_keyb_process(ckdev, ret);
>
> return NOTIFY_DONE;
> }
> @@ -217,32 +219,40 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> struct cros_ec_keyb *ckdev;
> struct input_dev *idev;
> struct device_node *np;
> + unsigned int rows, cols;
> + size_t size;
> int err;
>
> np = pdev->dev.of_node;
> if (!np)
> return -ENODEV;
>
> - ckdev = devm_kzalloc(&pdev->dev, sizeof(*ckdev), GFP_KERNEL);
> - if (!ckdev)
> - return -ENOMEM;
> - err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
> - &ckdev->cols);
> + err = matrix_keypad_parse_of_params(&pdev->dev, &rows, &cols);
> if (err)
> return err;
> - ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
> - if (!ckdev->old_kb_state)
> - return -ENOMEM;
>
> - idev = devm_input_allocate_device(&pdev->dev);
> - if (!idev)
> + /*
> + * Double memory for keyboard state so we have space for storing
> + * current and previous state.
> + */
> + size = sizeof(*ckdev) + 2 * cols * sizeof(*ckdev->kb_state);
> + ckdev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> + if (!ckdev)
> return -ENOMEM;
This change seems like a lot of complexity to save one memory
allocation. If you insist, I'd be OK with having one allocation for
both buffers (kb_state and old_kb_state) but trying to jam this onto
the end of the structure is non-obvious. It certainly took me a
minute to understand what you were doing and why.
> + ckdev->rows = rows;
> + ckdev->cols = cols;
> + ckdev->old_kb_state = &ckdev->kb_state[cols];
> +
> ckdev->ec = ec;
> - ckdev->notifier.notifier_call = cros_ec_keyb_work;
> ckdev->dev = dev;
> + ckdev->notifier.notifier_call = cros_ec_keyb_work;
> dev_set_drvdata(&pdev->dev, ckdev);
>
> + idev = devm_input_allocate_device(&pdev->dev);
> + if (!idev)
> + return -ENOMEM;
> +
> idev->name = ec->ec_name;
> idev->phys = ec->phys_name;
> __set_bit(EV_REP, idev->evbit);
> @@ -282,8 +292,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> /* Clear any keys in the buffer */
> static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> {
> - u8 old_state[ckdev->cols];
> - u8 new_state[ckdev->cols];
> unsigned long duration;
> int i, ret;
>
> @@ -294,11 +302,13 @@ static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> * Assume that the EC keyscan buffer is at most 32 deep.
> */
> duration = jiffies;
> - ret = cros_ec_keyb_get_state(ckdev, new_state);
> + ret = cros_ec_keyb_get_state(ckdev);
> for (i = 1; !ret && i < 32; i++) {
> - memcpy(old_state, new_state, sizeof(old_state));
> - ret = cros_ec_keyb_get_state(ckdev, new_state);
> - if (0 == memcmp(old_state, new_state, sizeof(old_state)))
> + memcpy(ckdev->old_kb_state, ckdev->kb_state,
> + ckdev->cols * sizeof(*ckdev->kb_state));
> + ret = cros_ec_keyb_get_state(ckdev);
> + if (!memcmp(ckdev->old_kb_state, ckdev->kb_state,
> + ckdev->cols * sizeof(*ckdev->kb_state)))
I'm not necessarily convinced that reusing ckdev->old_kb_state is wise
in cros_ec_keyb_clear_keyboard(). It is a behavior change (though it
might be a benign one).
Before your patch old_kb_state represented the last state that was
reported to the keyboard subsystem. After your patch, old_kb_state
may contain something different than what was reported to the
subsystem, at least after a suspend/resume cycle.
To put it in concrete terms, I _think_ this is what happens (please
correct me if I'm wrong):
Example A: imagine no keys were pressed when we suspended. Then, we
press and hold the "Shift" key to wake up. After waking up, we also
press the "A" key.
The sequence of events before your patch would be:
1. System wakes up and ignores the shift key being pressed.
2. System sees the "A" pressed and notices shift is still pressed.
3. We'll add a keydown for Shift and A at the same time.
4. Everyone thinks both keys down.
After your patch:
1. System wakes up and ignores the shift key being pressed.
2. System sees the "A" pressed; thinks it already told about shift.
3. We'll add a keydown for A.
4. cros_ec thinks both down; OS thinks A down.
Example B: imagine no keys were pressed when we suspended. Then, we
press and hold the "Shift" key to wake up. After waking, we release
Shift.
Before:
1. System wakes up and ignores the shift key being pressed.
2. When shift is released we scan and don't enqueue anything.
After:
1. System wakes up and ignores the shift key being pressed.
2. When shift is released we scan and process a release of the Shift
key (though we never sent a press).
-Doug
^ permalink raw reply
* Re: [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
From: Mark Brown @ 2014-01-02 18:49 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Stephen Boyd, linux-arm-msm, linux-kernel, linux-arm-kernel,
linux-input
In-Reply-To: <20131215113337.GB14818@core.coreip.homeip.net>
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Sun, Dec 15, 2013 at 03:33:37AM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 10, 2013 at 03:43:11PM -0800, Stephen Boyd wrote:
> > + regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > + if (!regmap)
> > + return -ENODEV;
> And you are leaking memory here...
He's not, dev_get_regmap() just gets a pointer to an existing regmap -
no reference is taken and nothing is allocated. It's a helper that's
mainly there so that generic code can be written without needing the
regmap to be passed around. The caller is responsible for ensuring that
it will stick around for as long as it's used (generally by having it
lifetime managed with the device).
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Xen-devel] [PATCH v3 1/2] xen/pvhvm: If xen_platform_pci=0 is set don't blow up (v3).
From: Konrad Rzeszutek Wilk @ 2014-01-02 19:16 UTC (permalink / raw)
To: David Vrabel
Cc: Fabio Fantoni, axboe, leosilva, linux-fbdev, dmitry.torokhov,
ian.campbell, stefano.stabellini, linux-pci, netdev, ashley,
tpmdd, linux-kernel, mail, tpmdd-devel, tomi.valkeinen,
linux-input, bhelgaas, xen-devel, boris.ostrovsky, plagnioj,
peterhuewe
In-Reply-To: <52C58138.1030301@citrix.com>
On Thu, Jan 02, 2014 at 03:09:44PM +0000, David Vrabel wrote:
> On 31/12/13 14:32, Konrad Rzeszutek Wilk wrote:
> >> That is because 'disks' is incorrect. It should have been 'ide-disks'
> >>
> >> [ 0.000000] unrecognised option 'disks' in parameter 'xen_emul_unplug'
> >>
> >> With the 'ide-disks' it should work. I will update the description to
> >> mention 'ide-disks' instead of 'disks'. Thank you for finding this!
> >>
> >
> > I've v4 with said update and will push it to Linus shortly.
> >
> > Thanks!
> >
> > P.S.
> > Here is v4:
> >
> >>From 275a81e7496d3532e5b4752703c50a7c8355a6c7 Mon Sep 17 00:00:00 2001
> > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Date: Tue, 26 Nov 2013 15:05:40 -0500
> > Subject: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up (v4).
> >
> > The user has the option of disabling the platform driver:
> > 00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
> >
> > which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> > and allow the PV drivers to take over. If the user wishes
> > to disable that they can set:
> >
> > xen_platform_pci=0
> > (in the guest config file)
> >
> > or
> > xen_emul_unplug=never
> > (on the Linux command line)
> >
> > except it does not work properly. The PV drivers still try to
> > load and since the Xen platform driver is not run - and it
> > has not initialized the grant tables, most of the PV drivers
> > stumble upon:
> >
> > input: Xen Virtual Keyboard as /devices/virtual/input/input5
> > input: Xen Virtual Pointer as /devices/virtual/input/input6M
> > ------------[ cut here ]------------
> > kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> > invalid opcode: 0000 [#1] SMP
> > Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> > CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> > Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> > RIP: 0010:[<ffffffff813ddc40>] [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
> > Call Trace:
> > [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
> > [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
> > [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
> > [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
> > [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
> > [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
> > [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
> > [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
> > [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> > [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
> > [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
> > [<ffffffff8145e7d9>] driver_attach+0x19/0x20
> > [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
> > [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
> > [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
> > [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
> > [<ffffffffa0015000>] ? 0xffffffffa0014fff
> > [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
> > [<ffffffff81002049>] do_one_initcall+0x49/0x170
> >
> > .. snip..
> >
> > which is hardly nice. This patch fixes this by having each
> > PV driver check for:
> > - if running in PV, then it is fine to execute (as that is their
> > native environment).
> > - if running in HVM, check if user wanted 'xen_emul_unplug=never',
> > in which case bail out and don't load any PV drivers.
> > - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
> > does not exist, then bail out and not load PV drivers.
> > - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=ide-disks',
> > then bail out for all PV devices _except_ the block one.
> > Ditto for the network one ('nics').
> > - (v2) if running in HVM, and if the user wanted 'xen_emul_unplug=unnecessary'
> > then load block PV driver, and also setup the legacy IDE paths.
> > In (v3) make it actually load PV drivers.
> [...]
> > --- a/arch/x86/xen/platform-pci-unplug.c
> > +++ b/arch/x86/xen/platform-pci-unplug.c
> > @@ -69,6 +69,80 @@ static int check_platform_magic(void)
> > return 0;
> > }
> >
> > +bool xen_has_pv_devices()
> > +{
> > + if (!xen_domain())
> > + return false;
> > +
> > + /* PV domains always have them. */
> > + if (xen_pv_domain())
> > + return true;
> > +
> > + /* And user has xen_platform_pci=0 set in guest config as
> > + * driver did not modify the value. */
> > + if (xen_platform_pci_unplug == 0)
> > + return false;
> > +
> > + if (xen_platform_pci_unplug & XEN_UNPLUG_NEVER)
> > + return false;
> > +
> > + if (xen_platform_pci_unplug & XEN_UNPLUG_ALL)
> > + return true;
> > +
> > + /* This is an odd one - we are going to run legacy
> > + * and PV drivers at the same time. */
> > + if (xen_platform_pci_unplug & XEN_UNPLUG_UNNECESSARY)
> > + return true;
> > +
> > + /* And the caller has to follow with xen_pv_{disk,nic}_devices
> > + * to be certain which driver can load. */
> > + return false;
>
> This may result in:
>
> xen_has_pv_devices() == false
> xen_has_pv_disk_devices() == true
Yes.
>
> which looks odd to me. Surely xen_has_pv_*_devices() is a subset of
> xen_has_pv_devices()?
I wish, this thing drives me nuts and I couldn't come up with a sensible
way to make this work for those special ones that have their own
xen_emul_unplug parameter without special casing the
'xen_has_pv_devices'.
Perhaps it should be renamed to 'xen_has_pv_generic_devices' ?
>
> David
^ permalink raw reply
* Re: [PATCH 2/7] Input: pmic8xxx-pwrkey - Migrate to regmap APIs
From: Dmitry Torokhov @ 2014-01-02 19:17 UTC (permalink / raw)
To: Mark Brown
Cc: linux-arm-msm, Stephen Boyd, linux-kernel, linux-arm-kernel,
linux-input
In-Reply-To: <20140102184959.GI31886@sirena.org.uk>
On Thu, Jan 02, 2014 at 06:49:59PM +0000, Mark Brown wrote:
> On Sun, Dec 15, 2013 at 03:33:37AM -0800, Dmitry Torokhov wrote:
> > On Tue, Dec 10, 2013 at 03:43:11PM -0800, Stephen Boyd wrote:
>
> > > + regmap = dev_get_regmap(pdev->dev.parent, NULL);
> > > + if (!regmap)
> > > + return -ENODEV;
>
> > And you are leaking memory here...
>
> He's not, dev_get_regmap() just gets a pointer to an existing regmap -
> no reference is taken and nothing is allocated. It's a helper that's
> mainly there so that generic code can be written without needing the
> regmap to be passed around. The caller is responsible for ensuring that
> it will stick around for as long as it's used (generally by having it
> lifetime managed with the device).
I was not talking about data returned by dev_get_regmap() but all other
memory that was allocated before as this was pre devm conversion of the
driver.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: cros_ec_keyb - switch from using uint8_t to u8
From: Dmitry Torokhov @ 2014-01-02 19:27 UTC (permalink / raw)
To: Doug Anderson
Cc: linux-input, Simon Glass, Vincent Palatin, Luigi Semenzato,
linux-kernel@vger.kernel.org
In-Reply-To: <CAD=FV=VjYtXA5XHiuz-bULpW_MCBWxw2eTvg+SQz9CrgsER4AA@mail.gmail.com>
On Thu, Jan 02, 2014 at 08:12:09AM -0800, Doug Anderson wrote:
> Dmitry,
>
> On Tue, Dec 31, 2013 at 11:34 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > u8 is proper in-kernel type for unsigned byte data.
>
> I won't say that I keep up with all the latest trends here, but this
> surprised me so I did some research. My findings don't agree with
> your statement. Perhaps there are different standards that are used
> for the input subsystem?
>
> Specifically looking at
> <https://www.kernel.org/doc/Documentation/CodingStyle>, I see:
>
> Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
> signed equivalents which are identical to standard types are
> permitted -- although they are not mandatory in new code of your
> own.
>
> When editing existing code which already uses one or the other set
> of types, you should conform to the existing choices in that code.
>
> That makes it sound like the author of that document would prefer
> uint8_t but will accept u8. It also seems like if code is consistent
> about using a given type (as this code is) that it shouldn't be
> changed.
>
> I'm always happy to be enlightened, though!
I prefer uXX in kernel because it matches __uXX that we publish in UAPI.
Also here is Linus's response form the discussion that introduced that
particular wording in CodingStyle [1]:
"The problem with uint32_t is that it's ugly, it used to be unportable,
and you can't use it in header files _anyway_.
In other words, there's no _point_ to the "standard type".
I really object to this whole thing. The fact is, "u8" and friends _are_
the standard types as far as the kernel is concerned. Claiming that
they aren't is just silly.
It's the "uint32_t" kind of thing that isn't standard within the kernel.
You can't use that thing in public header files anyway due to name
scoping rules, so they have basically no redeeming features."
Thanks.
--
Dmitry
[1] http://marc.info/?l=linux-kernel&m=114659539715468&w=2
^ permalink raw reply
* Re: [PATCH] Input: cros_ec_keyb - avoid variable-length arrays on stack
From: Dmitry Torokhov @ 2014-01-02 19:48 UTC (permalink / raw)
To: Doug Anderson
Cc: linux-input, Simon Glass, Vincent Palatin, Luigi Semenzato,
linux-kernel@vger.kernel.org, Olof Johansson
In-Reply-To: <CAD=FV=VZyXs9b+UyzuWNHBwKWEVcEvSZ4xdtsJLD+TFnRJon6A@mail.gmail.com>
Hi Doug,
On Thu, Jan 02, 2014 at 09:40:44AM -0800, Doug Anderson wrote:
> Dmitry,
>
> Thanks for cleaning up cros_eckeyb. :) I'm a little curious about
> the motivation here. I can't imagine a keyboard with all that many
> columns (ours has 13), so it's really not taking a whole lot off of
> the stack. Are you trying to make some sort of automated checker
> happy, or just generally trying to keep the kernel consistent?
I compile most of the code with sparse so I prefer to keep it happy.
>
> In any case, I'm not opposed to moving these bytes off the stack.
> Comments below, though...
>
> ---
>
> On Tue, Dec 31, 2013 at 11:35 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/keyboard/cros_ec_keyb.c | 80 ++++++++++++++++++++---------------
> > 1 file changed, 45 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> > index d44c5d4..03cb960 100644
> > --- a/drivers/input/keyboard/cros_ec_keyb.c
> > +++ b/drivers/input/keyboard/cros_ec_keyb.c
> > @@ -39,6 +39,7 @@
> > * @keymap_data: Matrix keymap data used to convert to keyscan values
> > * @ghost_filter: true to enable the matrix key-ghosting filter
> > * @old_kb_state: bitmap of keys pressed last scan
> > + * @kb_state: bitmap of keys currently pressed
> > * @dev: Device pointer
> > * @idev: Input device
> > * @ec: Top level ChromeOS device to use to talk to EC
> > @@ -50,17 +51,17 @@ struct cros_ec_keyb {
> > int row_shift;
> > const struct matrix_keymap_data *keymap_data;
> > bool ghost_filter;
> > - u8 *old_kb_state;
> > -
> > struct device *dev;
> > struct input_dev *idev;
> > struct cros_ec_device *ec;
> > struct notifier_block notifier;
> > +
> > + u8 *old_kb_state;
> > + u8 kb_state[];
>
> nit: you've moved old_kb_state to the end of the structure but you
> haven't moved the description in the comments above. I'd expect the
> ordering in the comment and the ordering in the comment to match.
>
> > };
> >
> >
> > -static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
> > - u8 *buf, int row)
> > +static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev, int row)
> > {
> > int pressed_in_row = 0;
> > int row_has_teeth = 0;
> > @@ -68,9 +69,9 @@ static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
> >
> > mask = 1 << row;
> > for (col = 0; col < ckdev->cols; col++) {
> > - if (buf[col] & mask) {
> > + if (ckdev->kb_state[col] & mask) {
> > pressed_in_row++;
> > - row_has_teeth |= buf[col] & ~mask;
> > + row_has_teeth |= ckdev->kb_state[col] & ~mask;
> > if (pressed_in_row > 1 && row_has_teeth) {
> > /* ghosting */
> > dev_dbg(ckdev->dev,
> > @@ -89,7 +90,7 @@ static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
> > * Returns true when there is at least one combination of pressed keys that
> > * results in ghosting.
> > */
> > -static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
> > +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev)
> > {
> > int row;
> >
> > @@ -120,7 +121,7 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
> > * cheat because the number of rows is small.
> > */
> > for (row = 0; row < ckdev->rows; row++)
> > - if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
> > + if (cros_ec_keyb_row_has_ghosting(ckdev, row))
> > return true;
> >
> > return false;
> > @@ -131,8 +132,7 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
> > * press/release events accordingly. The keyboard state is 13 bytes (one byte
> > * per column)
> > */
> > -static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> > - u8 *kb_state, int len)
> > +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, int len)
> > {
> > struct input_dev *idev = ckdev->idev;
> > int col, row;
> > @@ -142,7 +142,7 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> >
> > num_cols = len;
> >
> > - if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
> > + if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev)) {
> > /*
> > * Simple-minded solution: ignore this state. The obvious
> > * improvement is to only ignore changes to keys involved in
> > @@ -157,7 +157,7 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> > int pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> > const unsigned short *keycodes = idev->keycode;
> >
> > - new_state = kb_state[col] & (1 << row);
> > + new_state = ckdev->kb_state[col] & (1 << row);
> > old_state = ckdev->old_kb_state[col] & (1 << row);
> > if (new_state != old_state) {
> > dev_dbg(ckdev->dev,
> > @@ -168,9 +168,12 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> > new_state);
> > }
> > }
> > - ckdev->old_kb_state[col] = kb_state[col];
> > }
> > +
> > input_sync(ckdev->idev);
> > +
> > + memcpy(ckdev->old_kb_state, ckdev->kb_state,
> > + ckdev->cols * sizeof(*ckdev->kb_state));
>
> Any motivation for why you've moved this to a memcpy? In my mind the
> old code is easier to understand and I'm not convinced about the speed
> / code space gains (introducing a function call to save 13 assignment
> operations). Again this is not something I'll NAK, but it seems like
> a bit of code churn.
Logically you are saving entire state of keyboard so to me it makes
sense to do it at once.
>
>
> > }
> >
> > static int cros_ec_keyb_open(struct input_dev *dev)
> > @@ -189,10 +192,10 @@ static void cros_ec_keyb_close(struct input_dev *dev)
> > &ckdev->notifier);
> > }
> >
> > -static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, u8 *kb_state)
> > +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev)
> > {
> > return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> > - kb_state, ckdev->cols);
> > + ckdev->kb_state, ckdev->cols);
> > }
> >
> > static int cros_ec_keyb_work(struct notifier_block *nb,
> > @@ -201,11 +204,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> > int ret;
> > struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> > notifier);
> > - u8 kb_state[ckdev->cols];
> >
> > - ret = cros_ec_keyb_get_state(ckdev, kb_state);
> > + ret = cros_ec_keyb_get_state(ckdev);
> > if (ret >= 0)
> > - cros_ec_keyb_process(ckdev, kb_state, ret);
> > + cros_ec_keyb_process(ckdev, ret);
> >
> > return NOTIFY_DONE;
> > }
> > @@ -217,32 +219,40 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> > struct cros_ec_keyb *ckdev;
> > struct input_dev *idev;
> > struct device_node *np;
> > + unsigned int rows, cols;
> > + size_t size;
> > int err;
> >
> > np = pdev->dev.of_node;
> > if (!np)
> > return -ENODEV;
> >
> > - ckdev = devm_kzalloc(&pdev->dev, sizeof(*ckdev), GFP_KERNEL);
> > - if (!ckdev)
> > - return -ENOMEM;
> > - err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
> > - &ckdev->cols);
> > + err = matrix_keypad_parse_of_params(&pdev->dev, &rows, &cols);
> > if (err)
> > return err;
> > - ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
> > - if (!ckdev->old_kb_state)
> > - return -ENOMEM;
> >
> > - idev = devm_input_allocate_device(&pdev->dev);
> > - if (!idev)
> > + /*
> > + * Double memory for keyboard state so we have space for storing
> > + * current and previous state.
> > + */
> > + size = sizeof(*ckdev) + 2 * cols * sizeof(*ckdev->kb_state);
> > + ckdev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> > + if (!ckdev)
> > return -ENOMEM;
>
> This change seems like a lot of complexity to save one memory
> allocation. If you insist, I'd be OK with having one allocation for
> both buffers (kb_state and old_kb_state) but trying to jam this onto
> the end of the structure is non-obvious. It certainly took me a
> minute to understand what you were doing and why.
It is not one additional allocation but more as you need to allocate
devres data structures and add them there. I think we have quite a few
drivers piggy-backing key tables at the end of data structures.
>
>
> > + ckdev->rows = rows;
> > + ckdev->cols = cols;
> > + ckdev->old_kb_state = &ckdev->kb_state[cols];
> > +
> > ckdev->ec = ec;
> > - ckdev->notifier.notifier_call = cros_ec_keyb_work;
> > ckdev->dev = dev;
> > + ckdev->notifier.notifier_call = cros_ec_keyb_work;
> > dev_set_drvdata(&pdev->dev, ckdev);
> >
> > + idev = devm_input_allocate_device(&pdev->dev);
> > + if (!idev)
> > + return -ENOMEM;
> > +
> > idev->name = ec->ec_name;
> > idev->phys = ec->phys_name;
> > __set_bit(EV_REP, idev->evbit);
> > @@ -282,8 +292,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> > /* Clear any keys in the buffer */
> > static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> > {
> > - u8 old_state[ckdev->cols];
> > - u8 new_state[ckdev->cols];
> > unsigned long duration;
> > int i, ret;
> >
> > @@ -294,11 +302,13 @@ static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> > * Assume that the EC keyscan buffer is at most 32 deep.
> > */
> > duration = jiffies;
> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
> > + ret = cros_ec_keyb_get_state(ckdev);
> > for (i = 1; !ret && i < 32; i++) {
> > - memcpy(old_state, new_state, sizeof(old_state));
> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
> > - if (0 == memcmp(old_state, new_state, sizeof(old_state)))
> > + memcpy(ckdev->old_kb_state, ckdev->kb_state,
> > + ckdev->cols * sizeof(*ckdev->kb_state));
> > + ret = cros_ec_keyb_get_state(ckdev);
> > + if (!memcmp(ckdev->old_kb_state, ckdev->kb_state,
> > + ckdev->cols * sizeof(*ckdev->kb_state)))
>
> I'm not necessarily convinced that reusing ckdev->old_kb_state is wise
> in cros_ec_keyb_clear_keyboard(). It is a behavior change (though it
> might be a benign one).
>
> Before your patch old_kb_state represented the last state that was
> reported to the keyboard subsystem. After your patch, old_kb_state
> may contain something different than what was reported to the
> subsystem, at least after a suspend/resume cycle.
Hmm, I think you are right... By the way, why do we have to clear the
buffer?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: quirk for Saitek RAT7 and MMO7 mices' mode button
From: Conan @ 2014-01-02 19:56 UTC (permalink / raw)
To: linux-input
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi all,
this patch adds support for some saitek mice which implement a tristate
button (for switching button mappings in the official windows driver) by
keeping one of three (non-physical) buttons constantly pressed. This
breaks X and probably other userspace software.
The patch below implements a quirk which instantly sends a release event
for the affected buttons.
Disclaimer: this is not entirely my own work, it is based on a patch for
the R.A.T.7. which was posted here:
http://us.generation-nt.com/answer/patch-2-6-38-mode-button-quirk-saitek-cyborg-t-7-help-202649672.html
I merely added two usb ids and kept it up to date.
- --- a/drivers/hid/hid-ids.h 2013-10-08 15:20:58.781792791 +0200
+++ b/drivers/hid/hid-ids.h 2013-10-08 15:22:17.815804729 +0200
@@ -714,6 +714,9 @@
#define USB_VENDOR_ID_SAITEK 0x06a3
#define USB_DEVICE_ID_SAITEK_RUMBLEPAD 0xff17
#define USB_DEVICE_ID_SAITEK_PS1000 0x0621
+#define USB_DEVICE_ID_SAITEK_RAT7 0x0ccb
+#define USB_DEVICE_ID_SAITEK_RAT7TOO 0x0cd7
+#define USB_DEVICE_ID_SAITEK_MMO7 0x0cd0
#define USB_VENDOR_ID_SAMSUNG 0x0419
#define USB_DEVICE_ID_SAMSUNG_IR_REMOTE 0x0001
- --- a/drivers/hid/hid-input.c 2013-01-31 19:09:35.058570485 +0100
+++ b/drivers/hid/hid-input.c 2013-01-31 19:11:57.779833199 +0100
@@ -1039,6 +1039,25 @@
if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type == EV_KEY))
input_event(input, usage->type, usage->code, 0);
+
+ /* hack for Saitek RAT mice which report release events for their
+ * mode button on the NEXT press event: instant release
+ */
+ if ((*quirks & HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE) &&
+ value && usage->type == EV_KEY &&
+ /* RAT7 uses buttons 13,14,15 */
+ ((usage->code >= BTN_MOUSE + 8 && usage->code <= BTN_MOUSE + 10) ||
+ /* MMO7 uses button 20 */
+ (usage->code == BTN_MOUSE + 15)) &&
+ test_bit(usage->code, input->key)) {
+ input_event(input, usage->type, usage->code, 0);
+ /* we'll get a real release event from the mouse anyway, and
+ * userspace should cope with the extra input-layer
+ * button-up events anyway; just re-set the bit to stop
+ * spurious button-down events
+ */
+ set_bit(usage->code, input->key);
+ }
}
void hidinput_report_event(struct hid_device *hid, struct hid_report
*report)
- --- a/drivers/hid/usbhid/hid-quirks.c 2013-01-31 19:09:35.058570485 +0100
+++ b/drivers/hid/usbhid/hid-quirks.c 2013-01-31 19:12:58.195232311 +0100
@@ -82,6 +82,9 @@
{ USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3001,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_REALTEK, USB_DEVICE_ID_REALTEK_READER,
HID_QUIRK_NO_INIT_REPORTS },
+ { USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7,
HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE },
+ { USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7TOO,
HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE },
+ { USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7,
HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE },
{ USB_VENDOR_ID_SENNHEISER, USB_DEVICE_ID_SENNHEISER_BTD500USB,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_SIGMATEL, USB_DEVICE_ID_SIGMATEL_STMP3780,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_SUN, USB_DEVICE_ID_RARITAN_KVM_DONGLE, HID_QUIRK_NOGET },
- --- a/include/linux/hid.h 2013-05-18 03:16:26.713052127 +0200
+++ b/include/linux/hid.h 2013-05-18 03:17:23.407343351 +0200
@@ -283,6 +283,7 @@
#define HID_QUIRK_MULTI_INPUT 0x00000040
#define HID_QUIRK_HIDINPUT_FORCE 0x00000080
#define HID_QUIRK_NO_EMPTY_INPUT 0x00000100
+#define HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE 0x00000200
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x00010000
#define HID_QUIRK_FULLSPEED_INTERVAL 0x10000000
#define HID_QUIRK_NO_INIT_REPORTS 0x20000000
- --
Regards,
C
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJSxcSIAAoJECsSvCKD1CYc5LQH/ROXMVqulZsP1D4RTY8pL14k
D4udVjVfd60PlmqYmzSfijcD1nISUEldSSdOy1lf/DJUHvvfK/Ybjl1fAeqebqdi
pZZQM+2V2czdcdEB7ldT4stSig4MfHGdh/t5EGFdhIre0fXuXBV+dm9q8tcJitpn
UtJqk2TzDXTOwx847Mwo4kiJ4JdgF/oLgfNOtxCClHySSrBHw/kbgCduoIn5ISx7
4JELsD7LpW+ohbOdg7XBzEZZ4XbBd4eCTr8FogJAlBQSvCiMGXO1geCCaRCzB5Xg
HmULc8/zv1bE9kYHvGhRIodL8x8+De/MktFEiNOSJUw729dsO/eyW38vLvi2brQ=
=g13R
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
From: Maxime Ripard @ 2014-01-02 20:20 UTC (permalink / raw)
To: Hans de Goede
Cc: Heiko Stübner,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Dmitry Torokhov, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
In-Reply-To: <52C56D79.1060506-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]
On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
> >Also, instead of inventing yet another vendor-specific property, why not re-use
> >a button binding similar to gpio-keys like:
> >
> > lradc: lradc@01c22800 {
> > compatible = "allwinner,sun4i-lradc-keys";
> > reg = <0x01c22800 0x100>;
> > interrupts = <31>;
> > allwinner,chan0-step = <200>;
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > button@0 {
> > reg = <0>; /* your channel index from above */
> > linux,code = <115>; /* already used as dt-property */
> > };
> >
> > button@1 {
> > reg = <1>;
> > linux,code = <114>;
> > };
>
> Ugh no. Having a vendor specific property which is KISS certainly
> beats this, both wrt ease of writing dts files as well as wrt the
> dts parsing code in the driver.
I'd agree with Heiko here. This is pretty much the same construct
that's already in use in other input drivers, like gpio-keys.
This is also something that can really easily be made generic, since
this is something that is rather common.
Speaking of which. I believe this should actually come in two
different drivers:
- The ADC driver itself, using IIO
- A generic button handler driver on top of IIO.
The fact that on most board this adc is used for buttons doesn't make
any difference, it's actually a hardware designer choice, we should
support that choice, but we should also be able to use it just as an
ADC.
Carlo Caione already started to work on an IIO driver for the LRADC:
https://github.com/carlocaione/linux/tree/sunxi-lradc
maybe you can take over his work.
I also wonder wether it would be possible in that case to use reg as
the "button" voltage, to get rid of both the chan0-step property, and
those big fat arrays in the driver.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
From: Dmitry Torokhov @ 2014-01-02 20:38 UTC (permalink / raw)
To: Maxime Ripard
Cc: Hans de Goede, Heiko Stübner,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
In-Reply-To: <20140102202022.GK3144@lukather>
On Thu, Jan 02, 2014 at 09:20:22PM +0100, Maxime Ripard wrote:
> On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
> > >Also, instead of inventing yet another vendor-specific property, why not re-use
> > >a button binding similar to gpio-keys like:
> > >
> > > lradc: lradc@01c22800 {
> > > compatible = "allwinner,sun4i-lradc-keys";
> > > reg = <0x01c22800 0x100>;
> > > interrupts = <31>;
> > > allwinner,chan0-step = <200>;
> > >
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > button@0 {
> > > reg = <0>; /* your channel index from above */
> > > linux,code = <115>; /* already used as dt-property */
> > > };
> > >
> > > button@1 {
> > > reg = <1>;
> > > linux,code = <114>;
> > > };
> >
> > Ugh no. Having a vendor specific property which is KISS certainly
> > beats this, both wrt ease of writing dts files as well as wrt the
> > dts parsing code in the driver.
>
> I'd agree with Heiko here. This is pretty much the same construct
> that's already in use in other input drivers, like gpio-keys.
>
> This is also something that can really easily be made generic, since
> this is something that is rather common.
Except that button definition from gpio-keys does not use 'reg' property
but rather gpio. I'd rather we did not cram non-applicable attributes
into that definition just to make it "reusable" like that.
I'd be OK with having similar (but not claiming to be the same) mappings
though.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/4] input: Add new sun4i-lradc-keys drivers
From: Hans de Goede @ 2014-01-02 22:36 UTC (permalink / raw)
To: Maxime Ripard
Cc: Heiko Stübner,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Dmitry Torokhov, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell
In-Reply-To: <20140102202022.GK3144@lukather>
Hi,
On 01/02/2014 09:20 PM, Maxime Ripard wrote:
> On Thu, Jan 02, 2014 at 02:45:29PM +0100, Hans de Goede wrote:
>>> Also, instead of inventing yet another vendor-specific property, why not re-use
>>> a button binding similar to gpio-keys like:
>>>
>>> lradc: lradc@01c22800 {
>>> compatible = "allwinner,sun4i-lradc-keys";
>>> reg = <0x01c22800 0x100>;
>>> interrupts = <31>;
>>> allwinner,chan0-step = <200>;
>>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> button@0 {
>>> reg = <0>; /* your channel index from above */
>>> linux,code = <115>; /* already used as dt-property */
>>> };
>>>
>>> button@1 {
>>> reg = <1>;
>>> linux,code = <114>;
>>> };
>>
>> Ugh no. Having a vendor specific property which is KISS certainly
>> beats this, both wrt ease of writing dts files as well as wrt the
>> dts parsing code in the driver.
>
> I'd agree with Heiko here. This is pretty much the same construct
> that's already in use in other input drivers, like gpio-keys.
In the gpio case there is a 1 on 1 relation between a single hw
entity (the gpio-pin) and a single keycode. Here there is 1 hw entity
which maps to an array of key-codes, certainly using an array rather
then a much more complicated construct is the correct data-structure
to represent this.
>
> This is also something that can really easily be made generic, since
> this is something that is rather common.
>
> Speaking of which. I believe this should actually come in two
> different drivers:
> - The ADC driver itself, using IIO
> - A generic button handler driver on top of IIO.
>
> The fact that on most board this adc is used for buttons doesn't make
> any difference, it's actually a hardware designer choice, we should
> support that choice, but we should also be able to use it just as an
> ADC.
No, this is not a generic adc, as mentioned in the commit msg, this
adc is specifically designed to be used this way.
The adc won't start sampling data, and won't generate any interrupts
until a button is pressed. That is until the input voltage drops below
2/3 of Vref, this is checked through a built-in analog comparator, which
hooks into the control logic.
It has button down and button up interrupts, and can detect long
presses (unused) and generate a second type of down interrupt for those.
This really is an input device, which happens to use an adc.
> Carlo Caione already started to work on an IIO driver for the LRADC:
> https://github.com/carlocaione/linux/tree/sunxi-lradc
> maybe you can take over his work.
That won't work because the adc won't sample if the input gets above
2/3 of Vref. There may be some other mode which does not do that, but
that is not clearly documented.
Even if an IIO driver turns out to be doable, I strongly believe that
having a separate input driver for this is best, since this device
was designed to be used as such. Building input on top of IIO would
mean polling the adc, while with my driver it actually generates
button down / up interrupts without any polling being involved.
And no boards I know of are using this as a generic analog input,
where as many boards are using it as designed.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 0/2] input: mt: Add helper function to send end events
From: Felipe Ferreri Tonello @ 2014-01-02 23:20 UTC (permalink / raw)
To: Dmitry Torokhov, Henrik Rydberg; +Cc: linux-input, linux-kernel
In-Reply-To: <20140101191838.GA30798@core.coreip.homeip.net>
Hi Dmitry and Henrik,
On 01/01/2014 11:18 AM, Dmitry Torokhov wrote:
> On Wed, Jan 01, 2014 at 05:19:10PM +0100, Henrik Rydberg wrote:
>> Hi Dmitry,
>>
>>>> What problematic scenario is this supposed to solve?
>>>>
>>>> The 'release on suspend' is only an approximation to the 'close
>>>> laptop' scenario, it is certainly not correct in the coffee table
>>>> case,
>>>
>>> Why would it not be correct for coffee table? Do we expect the touches
>>> to remain valid while the device is asleep?
>>
>> In some scenarios with placed objects (like a cup or a marker), that would be
>> the case, yes.
>>
>>>> for instance. Instead of
>>>> hardcoding this in the kernel, userland can easily detect long intervals
>>>> directly using the event time.
>>>
>>> But with slots it is not only problem with old events being sent out
>>> later, it is that we have mix of old (pre-sleep) and new state.
>>
>> The new state is not really a problem, it will look exactly the same regardless
>> of how 'old' releases are handled.
>>
>>> We do that for keys (release them when we transition to system sleep)
>>> and I think it might be worthwhile to do the same for touch data.
>>
>> I agree that keyboard applications seldom look at time intervals and hence are
>> well helped by such a strategy, even though it is not strictly 'correct'.
>> However, touch interfaces are quite dependent on time intervals, and it
>> therefore makes a lot of sense to resolve touch timeouts directly in the
>> application. It also puts less restrictions on what can be done.
>>
>> Regarding notifications in general, perhaps it would be interesting to be able
>> to send a notification event via the input interface when a device comes back
>> from sleep. It could help resolve other complex issues, if there were any.
>>
>
> OK, fair enough. If there is a demand we could send SYN_SYSTEM_RESUME
> event though the interfaces to allow clients "flush" the state or
> otherwise decide if new touch data is actually old touches that were
> there pre-suspend.
>
> Felipe, will that help in your case?
Yes, I guess. But why notificate only when it comes back from sleep?
Because the way it is today, if a user touches a device and suddenly the
device sleeps, the user-space will not know that that device went sleep.
Then when the device wakes up (resumes) the end touch event will never
be sent even if the user releases his finger from it (because the actual
hardware is in "sleep" mode or something), thus the user-space will
continue to think the finger was not released from the touch screen.
Felipe
^ permalink raw reply
* [PATCH] Input: xpad - added new USB IDs for Logitech F310 and F710
From: Petr Sebor @ 2014-01-02 23:24 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, Petr Sebor
This enables the rumble force feedback on the F710 unit since
it is no longer treated as XTYPE_UNKNOWN type.
Signed-off-by: Petr Sebor <petr@scssoft.com>
---
drivers/input/joystick/xpad.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 75e3b10..958489a 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -125,6 +125,8 @@ static const struct xpad_device {
{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
+ { 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
+ { 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
{ 0x046d, 0xc242, "Logitech Chillstream Controller", 0, XTYPE_XBOX360 },
{ 0x046d, 0xca84, "Logitech Xbox Cordless Controller", 0, XTYPE_XBOX },
{ 0x046d, 0xca88, "Logitech Compact Controller for Xbox", 0, XTYPE_XBOX },
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH] Input: xpad - added new USB IDs for Logitech F310 and F710
From: Dmitry Torokhov @ 2014-01-02 23:35 UTC (permalink / raw)
To: Petr Sebor; +Cc: linux-input
In-Reply-To: <1388705063-6922-1-git-send-email-petr@scssoft.com>
On Fri, Jan 03, 2014 at 12:24:23AM +0100, Petr Sebor wrote:
> This enables the rumble force feedback on the F710 unit since
> it is no longer treated as XTYPE_UNKNOWN type.
>
> Signed-off-by: Petr Sebor <petr@scssoft.com>
Applied, thank you.
> ---
> drivers/input/joystick/xpad.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 75e3b10..958489a 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -125,6 +125,8 @@ static const struct xpad_device {
> { 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
> { 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
> { 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> + { 0x046d, 0xc21d, "Logitech Gamepad F310", 0, XTYPE_XBOX360 },
> + { 0x046d, 0xc21f, "Logitech Gamepad F710", 0, XTYPE_XBOX360 },
> { 0x046d, 0xc242, "Logitech Chillstream Controller", 0, XTYPE_XBOX360 },
> { 0x046d, 0xca84, "Logitech Xbox Cordless Controller", 0, XTYPE_XBOX },
> { 0x046d, 0xca88, "Logitech Compact Controller for Xbox", 0, XTYPE_XBOX },
> --
> 1.8.3.2
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: cros_ec_keyb - switch from using uint8_t to u8
From: Luigi Semenzato @ 2014-01-03 0:13 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Doug Anderson, linux-input, Simon Glass, Vincent Palatin,
linux-kernel@vger.kernel.org
In-Reply-To: <20140102192745.GB2025@core.coreip.homeip.net>
Thank you, this is useful information, and it would be even more
useful if it made it in Documentation/CodingStyle :-)
On Thu, Jan 2, 2014 at 11:27 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Jan 02, 2014 at 08:12:09AM -0800, Doug Anderson wrote:
>> Dmitry,
>>
>> On Tue, Dec 31, 2013 at 11:34 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > u8 is proper in-kernel type for unsigned byte data.
>>
>> I won't say that I keep up with all the latest trends here, but this
>> surprised me so I did some research. My findings don't agree with
>> your statement. Perhaps there are different standards that are used
>> for the input subsystem?
>>
>> Specifically looking at
>> <https://www.kernel.org/doc/Documentation/CodingStyle>, I see:
>>
>> Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
>> signed equivalents which are identical to standard types are
>> permitted -- although they are not mandatory in new code of your
>> own.
>>
>> When editing existing code which already uses one or the other set
>> of types, you should conform to the existing choices in that code.
>>
>> That makes it sound like the author of that document would prefer
>> uint8_t but will accept u8. It also seems like if code is consistent
>> about using a given type (as this code is) that it shouldn't be
>> changed.
>>
>> I'm always happy to be enlightened, though!
>
> I prefer uXX in kernel because it matches __uXX that we publish in UAPI.
> Also here is Linus's response form the discussion that introduced that
> particular wording in CodingStyle [1]:
>
> "The problem with uint32_t is that it's ugly, it used to be unportable,
> and you can't use it in header files _anyway_.
>
> In other words, there's no _point_ to the "standard type".
>
> I really object to this whole thing. The fact is, "u8" and friends _are_
> the standard types as far as the kernel is concerned. Claiming that
> they aren't is just silly.
>
> It's the "uint32_t" kind of thing that isn't standard within the kernel.
> You can't use that thing in public header files anyway due to name
> scoping rules, so they have basically no redeeming features."
>
> Thanks.
>
> --
> Dmitry
>
> [1] http://marc.info/?l=linux-kernel&m=114659539715468&w=2
^ permalink raw reply
* Re: [PATCH] Input: cros_ec_keyb - avoid variable-length arrays on stack
From: Luigi Semenzato @ 2014-01-03 0:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Doug Anderson, linux-input, Simon Glass, Vincent Palatin,
linux-kernel@vger.kernel.org, Olof Johansson
In-Reply-To: <20140102194817.GC2025@core.coreip.homeip.net>
On Thu, Jan 2, 2014 at 11:48 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Doug,
>
> On Thu, Jan 02, 2014 at 09:40:44AM -0800, Doug Anderson wrote:
>> Dmitry,
>>
>> Thanks for cleaning up cros_eckeyb. :) I'm a little curious about
>> the motivation here. I can't imagine a keyboard with all that many
>> columns (ours has 13), so it's really not taking a whole lot off of
>> the stack. Are you trying to make some sort of automated checker
>> happy, or just generally trying to keep the kernel consistent?
>
> I compile most of the code with sparse so I prefer to keep it happy.
>
>>
>> In any case, I'm not opposed to moving these bytes off the stack.
>> Comments below, though...
>>
>> ---
>>
>> On Tue, Dec 31, 2013 at 11:35 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > ---
>> > drivers/input/keyboard/cros_ec_keyb.c | 80 ++++++++++++++++++++---------------
>> > 1 file changed, 45 insertions(+), 35 deletions(-)
>> >
>> > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
>> > index d44c5d4..03cb960 100644
>> > --- a/drivers/input/keyboard/cros_ec_keyb.c
>> > +++ b/drivers/input/keyboard/cros_ec_keyb.c
>> > @@ -39,6 +39,7 @@
>> > * @keymap_data: Matrix keymap data used to convert to keyscan values
>> > * @ghost_filter: true to enable the matrix key-ghosting filter
>> > * @old_kb_state: bitmap of keys pressed last scan
>> > + * @kb_state: bitmap of keys currently pressed
>> > * @dev: Device pointer
>> > * @idev: Input device
>> > * @ec: Top level ChromeOS device to use to talk to EC
>> > @@ -50,17 +51,17 @@ struct cros_ec_keyb {
>> > int row_shift;
>> > const struct matrix_keymap_data *keymap_data;
>> > bool ghost_filter;
>> > - u8 *old_kb_state;
>> > -
>> > struct device *dev;
>> > struct input_dev *idev;
>> > struct cros_ec_device *ec;
>> > struct notifier_block notifier;
>> > +
>> > + u8 *old_kb_state;
>> > + u8 kb_state[];
>>
>> nit: you've moved old_kb_state to the end of the structure but you
>> haven't moved the description in the comments above. I'd expect the
>> ordering in the comment and the ordering in the comment to match.
>>
>> > };
>> >
>> >
>> > -static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
>> > - u8 *buf, int row)
>> > +static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev, int row)
>> > {
>> > int pressed_in_row = 0;
>> > int row_has_teeth = 0;
>> > @@ -68,9 +69,9 @@ static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
>> >
>> > mask = 1 << row;
>> > for (col = 0; col < ckdev->cols; col++) {
>> > - if (buf[col] & mask) {
>> > + if (ckdev->kb_state[col] & mask) {
>> > pressed_in_row++;
>> > - row_has_teeth |= buf[col] & ~mask;
>> > + row_has_teeth |= ckdev->kb_state[col] & ~mask;
>> > if (pressed_in_row > 1 && row_has_teeth) {
>> > /* ghosting */
>> > dev_dbg(ckdev->dev,
>> > @@ -89,7 +90,7 @@ static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
>> > * Returns true when there is at least one combination of pressed keys that
>> > * results in ghosting.
>> > */
>> > -static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
>> > +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev)
>> > {
>> > int row;
>> >
>> > @@ -120,7 +121,7 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
>> > * cheat because the number of rows is small.
>> > */
>> > for (row = 0; row < ckdev->rows; row++)
>> > - if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
>> > + if (cros_ec_keyb_row_has_ghosting(ckdev, row))
>> > return true;
>> >
>> > return false;
>> > @@ -131,8 +132,7 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
>> > * press/release events accordingly. The keyboard state is 13 bytes (one byte
>> > * per column)
>> > */
>> > -static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>> > - u8 *kb_state, int len)
>> > +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, int len)
>> > {
>> > struct input_dev *idev = ckdev->idev;
>> > int col, row;
>> > @@ -142,7 +142,7 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>> >
>> > num_cols = len;
>> >
>> > - if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
>> > + if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev)) {
>> > /*
>> > * Simple-minded solution: ignore this state. The obvious
>> > * improvement is to only ignore changes to keys involved in
>> > @@ -157,7 +157,7 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>> > int pos = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
>> > const unsigned short *keycodes = idev->keycode;
>> >
>> > - new_state = kb_state[col] & (1 << row);
>> > + new_state = ckdev->kb_state[col] & (1 << row);
>> > old_state = ckdev->old_kb_state[col] & (1 << row);
>> > if (new_state != old_state) {
>> > dev_dbg(ckdev->dev,
>> > @@ -168,9 +168,12 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>> > new_state);
>> > }
>> > }
>> > - ckdev->old_kb_state[col] = kb_state[col];
>> > }
>> > +
>> > input_sync(ckdev->idev);
>> > +
>> > + memcpy(ckdev->old_kb_state, ckdev->kb_state,
>> > + ckdev->cols * sizeof(*ckdev->kb_state));
>>
>> Any motivation for why you've moved this to a memcpy? In my mind the
>> old code is easier to understand and I'm not convinced about the speed
>> / code space gains (introducing a function call to save 13 assignment
>> operations). Again this is not something I'll NAK, but it seems like
>> a bit of code churn.
>
> Logically you are saving entire state of keyboard so to me it makes
> sense to do it at once.
>
>>
>>
>> > }
>> >
>> > static int cros_ec_keyb_open(struct input_dev *dev)
>> > @@ -189,10 +192,10 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>> > &ckdev->notifier);
>> > }
>> >
>> > -static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, u8 *kb_state)
>> > +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev)
>> > {
>> > return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>> > - kb_state, ckdev->cols);
>> > + ckdev->kb_state, ckdev->cols);
>> > }
>> >
>> > static int cros_ec_keyb_work(struct notifier_block *nb,
>> > @@ -201,11 +204,10 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
>> > int ret;
>> > struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
>> > notifier);
>> > - u8 kb_state[ckdev->cols];
>> >
>> > - ret = cros_ec_keyb_get_state(ckdev, kb_state);
>> > + ret = cros_ec_keyb_get_state(ckdev);
>> > if (ret >= 0)
>> > - cros_ec_keyb_process(ckdev, kb_state, ret);
>> > + cros_ec_keyb_process(ckdev, ret);
>> >
>> > return NOTIFY_DONE;
>> > }
>> > @@ -217,32 +219,40 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>> > struct cros_ec_keyb *ckdev;
>> > struct input_dev *idev;
>> > struct device_node *np;
>> > + unsigned int rows, cols;
>> > + size_t size;
>> > int err;
>> >
>> > np = pdev->dev.of_node;
>> > if (!np)
>> > return -ENODEV;
>> >
>> > - ckdev = devm_kzalloc(&pdev->dev, sizeof(*ckdev), GFP_KERNEL);
>> > - if (!ckdev)
>> > - return -ENOMEM;
>> > - err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
>> > - &ckdev->cols);
>> > + err = matrix_keypad_parse_of_params(&pdev->dev, &rows, &cols);
>> > if (err)
>> > return err;
>> > - ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
>> > - if (!ckdev->old_kb_state)
>> > - return -ENOMEM;
>> >
>> > - idev = devm_input_allocate_device(&pdev->dev);
>> > - if (!idev)
>> > + /*
>> > + * Double memory for keyboard state so we have space for storing
>> > + * current and previous state.
>> > + */
>> > + size = sizeof(*ckdev) + 2 * cols * sizeof(*ckdev->kb_state);
>> > + ckdev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
>> > + if (!ckdev)
>> > return -ENOMEM;
>>
>> This change seems like a lot of complexity to save one memory
>> allocation. If you insist, I'd be OK with having one allocation for
>> both buffers (kb_state and old_kb_state) but trying to jam this onto
>> the end of the structure is non-obvious. It certainly took me a
>> minute to understand what you were doing and why.
>
> It is not one additional allocation but more as you need to allocate
> devres data structures and add them there. I think we have quite a few
> drivers piggy-backing key tables at the end of data structures.
>
>>
>>
>> > + ckdev->rows = rows;
>> > + ckdev->cols = cols;
>> > + ckdev->old_kb_state = &ckdev->kb_state[cols];
>> > +
>> > ckdev->ec = ec;
>> > - ckdev->notifier.notifier_call = cros_ec_keyb_work;
>> > ckdev->dev = dev;
>> > + ckdev->notifier.notifier_call = cros_ec_keyb_work;
>> > dev_set_drvdata(&pdev->dev, ckdev);
>> >
>> > + idev = devm_input_allocate_device(&pdev->dev);
>> > + if (!idev)
>> > + return -ENOMEM;
>> > +
>> > idev->name = ec->ec_name;
>> > idev->phys = ec->phys_name;
>> > __set_bit(EV_REP, idev->evbit);
>> > @@ -282,8 +292,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>> > /* Clear any keys in the buffer */
>> > static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
>> > {
>> > - u8 old_state[ckdev->cols];
>> > - u8 new_state[ckdev->cols];
>> > unsigned long duration;
>> > int i, ret;
>> >
>> > @@ -294,11 +302,13 @@ static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
>> > * Assume that the EC keyscan buffer is at most 32 deep.
>> > */
>> > duration = jiffies;
>> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
>> > + ret = cros_ec_keyb_get_state(ckdev);
>> > for (i = 1; !ret && i < 32; i++) {
>> > - memcpy(old_state, new_state, sizeof(old_state));
>> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
>> > - if (0 == memcmp(old_state, new_state, sizeof(old_state)))
>> > + memcpy(ckdev->old_kb_state, ckdev->kb_state,
>> > + ckdev->cols * sizeof(*ckdev->kb_state));
>> > + ret = cros_ec_keyb_get_state(ckdev);
>> > + if (!memcmp(ckdev->old_kb_state, ckdev->kb_state,
>> > + ckdev->cols * sizeof(*ckdev->kb_state)))
>>
>> I'm not necessarily convinced that reusing ckdev->old_kb_state is wise
>> in cros_ec_keyb_clear_keyboard(). It is a behavior change (though it
>> might be a benign one).
>>
>> Before your patch old_kb_state represented the last state that was
>> reported to the keyboard subsystem. After your patch, old_kb_state
>> may contain something different than what was reported to the
>> subsystem, at least after a suspend/resume cycle.
>
> Hmm, I think you are right... By the way, why do we have to clear the
> buffer?
This is almost funny. The keyboard is connected to the EC, which
stays awake during sleep. The case of some laptops has enough flex
that keys can be pressed when the laptop is squished in a backpack.
On resume the kernel gets interrupts and loads the key events from the
EC, but we really don't want to see those keys, so we ignore them and
clear the state.
I suppose we could have made the EC smarter about suspend/resume, but
it's got a tiny memory and we don't want to field-update it, so we
keep that code as simple as possible.
Thanks!
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH v2 00/10] Use regmap+devm+DT in pm8xxx input drivers
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input,
Thomas Gleixner, devicetree
These patches move the pm8xxx input drivers over to use devm_* APIs
and regmap. This breaks the dependency of these drivers on the pm8xxx
specific read/write calls and also simplifies the probe code a bit.
Finally we add devicetree support to these drivers so they can be probed
on the platforms that are supported upstream.
There was no devm_request_any_context_irq() available, so I've added
it here.
Changes since v1:
* Picked up Dmitry's version of devm for pwrkey
* Added DT bindings and parsing patches
* Dropped patches picked up by Dmitry
Dmitry Torokhov (1):
Input: pmic8xxx-pwrkey - switch to using managed resources
Stephen Boyd (9):
genirq: Add devm_request_any_context_irq()
Input: pmic8xxx-keypad - Switch to using managed resources
Input: pmic8xxx-keypad - Migrate to regmap APIs
Input: pmic8xxx-pwrkey - Migrate to DT
Input: pm8xxx-vibrator - Add DT match table
Input: pmic8xxx-keypad - Migrate to DT
devicetree: bindings: Document PM8921/8058 keypads
devicetree: bindings: Document PM8921/8058 power keys
devicetree: bindings: Document PM8921/8058 vibrators
.../bindings/input/qcom,pm8xxx-keypad.txt | 72 +++++
.../bindings/input/qcom,pm8xxx-pwrkey.txt | 39 +++
.../devicetree/bindings/input/qcom,pm8xxx-vib.txt | 16 ++
drivers/input/keyboard/pmic8xxx-keypad.c | 291 ++++++++++-----------
drivers/input/misc/pm8xxx-vibrator.c | 8 +
drivers/input/misc/pmic8xxx-pwrkey.c | 107 ++++----
include/linux/input/pmic8xxx-keypad.h | 52 ----
include/linux/input/pmic8xxx-pwrkey.h | 31 ---
include/linux/interrupt.h | 5 +
kernel/irq/devres.c | 45 ++++
10 files changed, 361 insertions(+), 305 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt
delete mode 100644 include/linux/input/pmic8xxx-keypad.h
delete mode 100644 include/linux/input/pmic8xxx-pwrkey.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH v2 01/10] Input: pmic8xxx-pwrkey - switch to using managed resources
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input
In-Reply-To: <1388709460-19222-1-git-send-email-sboyd@codeaurora.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
This simplifies error handling and device removal paths.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/input/misc/pmic8xxx-pwrkey.c | 74 ++++++++++++------------------------
1 file changed, 25 insertions(+), 49 deletions(-)
diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
index ef938405a9c6..aaf332510623 100644
--- a/drivers/input/misc/pmic8xxx-pwrkey.c
+++ b/drivers/input/misc/pmic8xxx-pwrkey.c
@@ -32,7 +32,6 @@
* @key_press_irq: key press irq number
*/
struct pmic8xxx_pwrkey {
- struct input_dev *pwr;
int key_press_irq;
};
@@ -110,22 +109,22 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
return -ENODEV;
}
- pwrkey = kzalloc(sizeof(*pwrkey), GFP_KERNEL);
+ pwrkey = devm_kzalloc(&pdev->dev, sizeof(*pwrkey), GFP_KERNEL);
if (!pwrkey)
return -ENOMEM;
- pwr = input_allocate_device();
+ pwrkey->key_press_irq = key_press_irq;
+
+ pwr = devm_input_allocate_device(&pdev->dev);
if (!pwr) {
dev_dbg(&pdev->dev, "Can't allocate power button\n");
- err = -ENOMEM;
- goto free_pwrkey;
+ return -ENOMEM;
}
input_set_capability(pwr, EV_KEY, KEY_POWER);
pwr->name = "pmic8xxx_pwrkey";
pwr->phys = "pmic8xxx_pwrkey/input0";
- pwr->dev.parent = &pdev->dev;
delay = (pdata->kpd_trigger_delay_us << 10) / USEC_PER_SEC;
delay = 1 + ilog2(delay);
@@ -133,7 +132,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
err = regmap_read(regmap, PON_CNTL_1, &pon_cntl);
if (err < 0) {
dev_err(&pdev->dev, "failed reading PON_CNTL_1 err=%d\n", err);
- goto free_input_dev;
+ return err;
}
pon_cntl &= ~PON_CNTL_TRIG_DELAY_MASK;
@@ -146,66 +145,43 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
err = regmap_write(regmap, PON_CNTL_1, pon_cntl);
if (err < 0) {
dev_err(&pdev->dev, "failed writing PON_CNTL_1 err=%d\n", err);
- goto free_input_dev;
+ return err;
}
- err = input_register_device(pwr);
+ err = devm_request_irq(&pdev->dev, key_press_irq, pwrkey_press_irq,
+ IRQF_TRIGGER_RISING,
+ "pmic8xxx_pwrkey_press", pwr);
if (err) {
- dev_dbg(&pdev->dev, "Can't register power key: %d\n", err);
- goto free_input_dev;
+ dev_err(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
+ key_press_irq, err);
+ return err;
}
- pwrkey->key_press_irq = key_press_irq;
- pwrkey->pwr = pwr;
-
- platform_set_drvdata(pdev, pwrkey);
-
- err = request_irq(key_press_irq, pwrkey_press_irq,
- IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_press", pwr);
- if (err < 0) {
- dev_dbg(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
- key_press_irq, err);
- goto unreg_input_dev;
+ err = devm_request_irq(&pdev->dev, key_release_irq, pwrkey_release_irq,
+ IRQF_TRIGGER_RISING,
+ "pmic8xxx_pwrkey_release", pwr);
+ if (err) {
+ dev_err(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
+ key_release_irq, err);
+ return err;
}
- err = request_irq(key_release_irq, pwrkey_release_irq,
- IRQF_TRIGGER_RISING, "pmic8xxx_pwrkey_release", pwr);
- if (err < 0) {
- dev_dbg(&pdev->dev, "Can't get %d IRQ for pwrkey: %d\n",
- key_release_irq, err);
-
- goto free_press_irq;
+ err = input_register_device(pwr);
+ if (err) {
+ dev_err(&pdev->dev, "Can't register power key: %d\n", err);
+ return err;
}
+ platform_set_drvdata(pdev, pwrkey);
device_init_wakeup(&pdev->dev, pdata->wakeup);
return 0;
-
-free_press_irq:
- free_irq(key_press_irq, pwrkey);
-unreg_input_dev:
- input_unregister_device(pwr);
- pwr = NULL;
-free_input_dev:
- input_free_device(pwr);
-free_pwrkey:
- kfree(pwrkey);
- return err;
}
static int pmic8xxx_pwrkey_remove(struct platform_device *pdev)
{
- struct pmic8xxx_pwrkey *pwrkey = platform_get_drvdata(pdev);
- int key_release_irq = platform_get_irq(pdev, 0);
- int key_press_irq = platform_get_irq(pdev, 1);
-
device_init_wakeup(&pdev->dev, 0);
- free_irq(key_press_irq, pwrkey->pwr);
- free_irq(key_release_irq, pwrkey->pwr);
- input_unregister_device(pwrkey->pwr);
- kfree(pwrkey);
-
return 0;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 02/10] genirq: Add devm_request_any_context_irq()
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input,
Thomas Gleixner
In-Reply-To: <1388709460-19222-1-git-send-email-sboyd@codeaurora.org>
Some drivers use request_any_context_irq() but there isn't a
devm_* function for it. Add one so that these drivers don't need
to explicitly free the irq on driver detach.
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
include/linux/interrupt.h | 5 +++++
kernel/irq/devres.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 0053adde0ed9..a2678d35b5a2 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -158,6 +158,11 @@ devm_request_irq(struct device *dev, unsigned int irq, irq_handler_t handler,
devname, dev_id);
}
+extern int __must_check
+devm_request_any_context_irq(struct device *dev, unsigned int irq,
+ irq_handler_t handler, unsigned long irqflags,
+ const char *devname, void *dev_id);
+
extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);
/*
diff --git a/kernel/irq/devres.c b/kernel/irq/devres.c
index bd8e788d71e0..1ef0606797c9 100644
--- a/kernel/irq/devres.c
+++ b/kernel/irq/devres.c
@@ -73,6 +73,51 @@ int devm_request_threaded_irq(struct device *dev, unsigned int irq,
EXPORT_SYMBOL(devm_request_threaded_irq);
/**
+ * devm_request_any_context_irq - allocate an interrupt line for a managed device
+ * @dev: device to request interrupt for
+ * @irq: Interrupt line to allocate
+ * @handler: Function to be called when the IRQ occurs
+ * @thread_fn: function to be called in a threaded interrupt context. NULL
+ * for devices which handle everything in @handler
+ * @irqflags: Interrupt type flags
+ * @devname: An ascii name for the claiming device
+ * @dev_id: A cookie passed back to the handler function
+ *
+ * Except for the extra @dev argument, this function takes the
+ * same arguments and performs the same function as
+ * request_any_context_irq(). IRQs requested with this function will be
+ * automatically freed on driver detach.
+ *
+ * If an IRQ allocated with this function needs to be freed
+ * separately, devm_free_irq() must be used.
+ */
+int devm_request_any_context_irq(struct device *dev, unsigned int irq,
+ irq_handler_t handler, unsigned long irqflags,
+ const char *devname, void *dev_id)
+{
+ struct irq_devres *dr;
+ int rc;
+
+ dr = devres_alloc(devm_irq_release, sizeof(struct irq_devres),
+ GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ rc = request_any_context_irq(irq, handler, irqflags, devname, dev_id);
+ if (rc) {
+ devres_free(dr);
+ return rc;
+ }
+
+ dr->irq = irq;
+ dr->dev_id = dev_id;
+ devres_add(dev, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL(devm_request_any_context_irq);
+
+/**
* devm_free_irq - free an interrupt
* @dev: device to free interrupt for
* @irq: Interrupt line to free
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 03/10] Input: pmic8xxx-keypad - Switch to using managed resources
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, linux-input
In-Reply-To: <1388709460-19222-1-git-send-email-sboyd@codeaurora.org>
Simplify the error paths and reduce the lines of code in this
driver by using the devm_* APIs.
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/input/keyboard/pmic8xxx-keypad.c | 62 +++++++++-----------------------
1 file changed, 17 insertions(+), 45 deletions(-)
diff --git a/drivers/input/keyboard/pmic8xxx-keypad.c b/drivers/input/keyboard/pmic8xxx-keypad.c
index 2c9f19ac35ea..4e6bfbf94ae4 100644
--- a/drivers/input/keyboard/pmic8xxx-keypad.c
+++ b/drivers/input/keyboard/pmic8xxx-keypad.c
@@ -586,7 +586,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
return -EINVAL;
}
- kp = kzalloc(sizeof(*kp), GFP_KERNEL);
+ kp = devm_kzalloc(&pdev->dev, sizeof(*kp), GFP_KERNEL);
if (!kp)
return -ENOMEM;
@@ -595,32 +595,27 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
kp->pdata = pdata;
kp->dev = &pdev->dev;
- kp->input = input_allocate_device();
+ kp->input = devm_input_allocate_device(&pdev->dev);
if (!kp->input) {
dev_err(&pdev->dev, "unable to allocate input device\n");
- rc = -ENOMEM;
- goto err_alloc_device;
+ return -ENOMEM;
}
kp->key_sense_irq = platform_get_irq(pdev, 0);
if (kp->key_sense_irq < 0) {
dev_err(&pdev->dev, "unable to get keypad sense irq\n");
- rc = -ENXIO;
- goto err_get_irq;
+ return kp->key_sense_irq;
}
kp->key_stuck_irq = platform_get_irq(pdev, 1);
if (kp->key_stuck_irq < 0) {
dev_err(&pdev->dev, "unable to get keypad stuck irq\n");
- rc = -ENXIO;
- goto err_get_irq;
+ return kp->key_stuck_irq;
}
kp->input->name = pdata->input_name ? : "PMIC8XXX keypad";
kp->input->phys = pdata->input_phys_device ? : "pmic8xxx_keypad/input0";
- kp->input->dev.parent = &pdev->dev;
-
kp->input->id.bustype = BUS_I2C;
kp->input->id.version = 0x0001;
kp->input->id.product = 0x0001;
@@ -634,7 +629,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
kp->keycodes, kp->input);
if (rc) {
dev_err(&pdev->dev, "failed to build keymap\n");
- goto err_get_irq;
+ return rc;
}
if (pdata->rep)
@@ -650,7 +645,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
rc = pmic8xxx_kpd_init(kp);
if (rc < 0) {
dev_err(&pdev->dev, "unable to initialize keypad controller\n");
- goto err_get_irq;
+ return rc;
}
rc = pmic8xxx_kp_config_gpio(pdata->cols_gpio_start,
@@ -667,24 +662,26 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
goto err_gpio_config;
}
- rc = request_any_context_irq(kp->key_sense_irq, pmic8xxx_kp_irq,
- IRQF_TRIGGER_RISING, "pmic-keypad", kp);
+ rc = devm_request_any_context_irq(&pdev->dev, kp->key_sense_irq,
+ pmic8xxx_kp_irq, IRQF_TRIGGER_RISING, "pmic-keypad",
+ kp);
if (rc < 0) {
dev_err(&pdev->dev, "failed to request keypad sense irq\n");
- goto err_get_irq;
+ return rc;
}
- rc = request_any_context_irq(kp->key_stuck_irq, pmic8xxx_kp_stuck_irq,
- IRQF_TRIGGER_RISING, "pmic-keypad-stuck", kp);
+ rc = devm_request_any_context_irq(&pdev->dev, kp->key_stuck_irq,
+ pmic8xxx_kp_stuck_irq, IRQF_TRIGGER_RISING,
+ "pmic-keypad-stuck", kp);
if (rc < 0) {
dev_err(&pdev->dev, "failed to request keypad stuck irq\n");
- goto err_req_stuck_irq;
+ return rc;
}
rc = pmic8xxx_kp_read_u8(kp, &ctrl_val, KEYP_CTRL);
if (rc < 0) {
dev_err(&pdev->dev, "failed to read KEYP_CTRL register\n");
- goto err_pmic_reg_read;
+ return rc;
}
kp->ctrl_reg = ctrl_val;
@@ -692,36 +689,12 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
rc = input_register_device(kp->input);
if (rc < 0) {
dev_err(&pdev->dev, "unable to register keypad input device\n");
- goto err_pmic_reg_read;
+ return rc;
}
device_init_wakeup(&pdev->dev, pdata->wakeup);
return 0;
-
-err_pmic_reg_read:
- free_irq(kp->key_stuck_irq, kp);
-err_req_stuck_irq:
- free_irq(kp->key_sense_irq, kp);
-err_gpio_config:
-err_get_irq:
- input_free_device(kp->input);
-err_alloc_device:
- kfree(kp);
- return rc;
-}
-
-static int pmic8xxx_kp_remove(struct platform_device *pdev)
-{
- struct pmic8xxx_kp *kp = platform_get_drvdata(pdev);
-
- device_init_wakeup(&pdev->dev, 0);
- free_irq(kp->key_stuck_irq, kp);
- free_irq(kp->key_sense_irq, kp);
- input_unregister_device(kp->input);
- kfree(kp);
-
- return 0;
}
#ifdef CONFIG_PM_SLEEP
@@ -771,7 +744,6 @@ static SIMPLE_DEV_PM_OPS(pm8xxx_kp_pm_ops,
static struct platform_driver pmic8xxx_kp_driver = {
.probe = pmic8xxx_kp_probe,
- .remove = pmic8xxx_kp_remove,
.driver = {
.name = PM8XXX_KEYPAD_DEV_NAME,
.owner = THIS_MODULE,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 04/10] Input: pmic8xxx-keypad - Migrate to regmap APIs
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input
In-Reply-To: <1388709460-19222-1-git-send-email-sboyd@codeaurora.org>
Use the regmap APIs for this driver instead of custom pm8xxx
APIs. This breaks this driver's dependency on the pm8xxx APIs and
allows us to easily port it to other bus protocols in the future.
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/input/keyboard/pmic8xxx-keypad.c | 81 ++++++++++++--------------------
1 file changed, 29 insertions(+), 52 deletions(-)
diff --git a/drivers/input/keyboard/pmic8xxx-keypad.c b/drivers/input/keyboard/pmic8xxx-keypad.c
index 4e6bfbf94ae4..c6d3d216ffa7 100644
--- a/drivers/input/keyboard/pmic8xxx-keypad.c
+++ b/drivers/input/keyboard/pmic8xxx-keypad.c
@@ -19,8 +19,8 @@
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/regmap.h>
-#include <linux/mfd/pm8xxx/core.h>
#include <linux/mfd/pm8xxx/gpio.h>
#include <linux/input/pmic8xxx-keypad.h>
@@ -87,6 +87,7 @@
* struct pmic8xxx_kp - internal keypad data structure
* @pdata - keypad platform data pointer
* @input - input device pointer for keypad
+ * @regmap - regmap handle
* @key_sense_irq - key press/release irq number
* @key_stuck_irq - key stuck notification irq number
* @keycodes - array to hold the key codes
@@ -98,6 +99,7 @@
struct pmic8xxx_kp {
const struct pm8xxx_keypad_platform_data *pdata;
struct input_dev *input;
+ struct regmap *regmap;
int key_sense_irq;
int key_stuck_irq;
@@ -110,33 +112,6 @@ struct pmic8xxx_kp {
u8 ctrl_reg;
};
-static int pmic8xxx_kp_write_u8(struct pmic8xxx_kp *kp,
- u8 data, u16 reg)
-{
- int rc;
-
- rc = pm8xxx_writeb(kp->dev->parent, reg, data);
- return rc;
-}
-
-static int pmic8xxx_kp_read(struct pmic8xxx_kp *kp,
- u8 *data, u16 reg, unsigned num_bytes)
-{
- int rc;
-
- rc = pm8xxx_read_buf(kp->dev->parent, reg, data, num_bytes);
- return rc;
-}
-
-static int pmic8xxx_kp_read_u8(struct pmic8xxx_kp *kp,
- u8 *data, u16 reg)
-{
- int rc;
-
- rc = pmic8xxx_kp_read(kp, data, reg, 1);
- return rc;
-}
-
static u8 pmic8xxx_col_state(struct pmic8xxx_kp *kp, u8 col)
{
/* all keys pressed on that particular row? */
@@ -161,9 +136,9 @@ static u8 pmic8xxx_col_state(struct pmic8xxx_kp *kp, u8 col)
static int pmic8xxx_chk_sync_read(struct pmic8xxx_kp *kp)
{
int rc;
- u8 scan_val;
+ unsigned int scan_val;
- rc = pmic8xxx_kp_read_u8(kp, &scan_val, KEYP_SCAN);
+ rc = regmap_read(kp->regmap, KEYP_SCAN, &scan_val);
if (rc < 0) {
dev_err(kp->dev, "Error reading KEYP_SCAN reg, rc=%d\n", rc);
return rc;
@@ -171,7 +146,7 @@ static int pmic8xxx_chk_sync_read(struct pmic8xxx_kp *kp)
scan_val |= 0x1;
- rc = pmic8xxx_kp_write_u8(kp, scan_val, KEYP_SCAN);
+ rc = regmap_write(kp->regmap, KEYP_SCAN, scan_val);
if (rc < 0) {
dev_err(kp->dev, "Error writing KEYP_SCAN reg, rc=%d\n", rc);
return rc;
@@ -187,26 +162,24 @@ static int pmic8xxx_kp_read_data(struct pmic8xxx_kp *kp, u16 *state,
u16 data_reg, int read_rows)
{
int rc, row;
- u8 new_data[PM8XXX_MAX_ROWS];
+ unsigned int val;
- rc = pmic8xxx_kp_read(kp, new_data, data_reg, read_rows);
- if (rc)
- return rc;
-
- for (row = 0; row < kp->pdata->num_rows; row++) {
- dev_dbg(kp->dev, "new_data[%d] = %d\n", row,
- new_data[row]);
- state[row] = pmic8xxx_col_state(kp, new_data[row]);
+ for (row = 0; row < read_rows; row++) {
+ rc = regmap_read(kp->regmap, data_reg, &val);
+ if (rc)
+ return rc;
+ dev_dbg(kp->dev, "%d = %d\n", row, val);
+ state[row] = pmic8xxx_col_state(kp, val);
}
- return rc;
+ return 0;
}
static int pmic8xxx_kp_read_matrix(struct pmic8xxx_kp *kp, u16 *new_state,
u16 *old_state)
{
int rc, read_rows;
- u8 scan_val;
+ unsigned int scan_val;
if (kp->pdata->num_rows < PM8XXX_MIN_ROWS)
read_rows = PM8XXX_MIN_ROWS;
@@ -236,14 +209,14 @@ static int pmic8xxx_kp_read_matrix(struct pmic8xxx_kp *kp, u16 *new_state,
/* 4 * 32KHz clocks */
udelay((4 * DIV_ROUND_UP(USEC_PER_SEC, KEYP_CLOCK_FREQ)) + 1);
- rc = pmic8xxx_kp_read_u8(kp, &scan_val, KEYP_SCAN);
+ rc = regmap_read(kp->regmap, KEYP_SCAN, &scan_val);
if (rc < 0) {
dev_err(kp->dev, "Error reading KEYP_SCAN reg, rc=%d\n", rc);
return rc;
}
scan_val &= 0xFE;
- rc = pmic8xxx_kp_write_u8(kp, scan_val, KEYP_SCAN);
+ rc = regmap_write(kp->regmap, KEYP_SCAN, scan_val);
if (rc < 0)
dev_err(kp->dev, "Error writing KEYP_SCAN reg, rc=%d\n", rc);
@@ -379,10 +352,10 @@ static irqreturn_t pmic8xxx_kp_stuck_irq(int irq, void *data)
static irqreturn_t pmic8xxx_kp_irq(int irq, void *data)
{
struct pmic8xxx_kp *kp = data;
- u8 ctrl_val, events;
+ unsigned int ctrl_val, events;
int rc;
- rc = pmic8xxx_kp_read(kp, &ctrl_val, KEYP_CTRL, 1);
+ rc = regmap_read(kp->regmap, KEYP_CTRL, &ctrl_val);
if (rc < 0) {
dev_err(kp->dev, "failed to read keyp_ctrl register\n");
return IRQ_HANDLED;
@@ -421,7 +394,7 @@ static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
ctrl_val |= (bits << KEYP_CTRL_SCAN_ROWS_SHIFT);
- rc = pmic8xxx_kp_write_u8(kp, ctrl_val, KEYP_CTRL);
+ rc = regmap_write(kp->regmap, KEYP_CTRL, ctrl_val);
if (rc < 0) {
dev_err(kp->dev, "Error writing KEYP_CTRL reg, rc=%d\n", rc);
return rc;
@@ -439,7 +412,7 @@ static int pmic8xxx_kpd_init(struct pmic8xxx_kp *kp)
scan_val |= (cycles << KEYP_SCAN_ROW_HOLD_SHIFT);
- rc = pmic8xxx_kp_write_u8(kp, scan_val, KEYP_SCAN);
+ rc = regmap_write(kp->regmap, KEYP_SCAN, scan_val);
if (rc)
dev_err(kp->dev, "Error writing KEYP_SCAN reg, rc=%d\n", rc);
@@ -474,7 +447,7 @@ static int pmic8xxx_kp_enable(struct pmic8xxx_kp *kp)
kp->ctrl_reg |= KEYP_CTRL_KEYP_EN;
- rc = pmic8xxx_kp_write_u8(kp, kp->ctrl_reg, KEYP_CTRL);
+ rc = regmap_write(kp->regmap, KEYP_CTRL, kp->ctrl_reg);
if (rc < 0)
dev_err(kp->dev, "Error writing KEYP_CTRL reg, rc=%d\n", rc);
@@ -487,7 +460,7 @@ static int pmic8xxx_kp_disable(struct pmic8xxx_kp *kp)
kp->ctrl_reg &= ~KEYP_CTRL_KEYP_EN;
- rc = pmic8xxx_kp_write_u8(kp, kp->ctrl_reg, KEYP_CTRL);
+ rc = regmap_write(kp->regmap, KEYP_CTRL, kp->ctrl_reg);
if (rc < 0)
return rc;
@@ -525,7 +498,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
const struct matrix_keymap_data *keymap_data;
struct pmic8xxx_kp *kp;
int rc;
- u8 ctrl_val;
+ unsigned int ctrl_val;
struct pm_gpio kypd_drv = {
.direction = PM_GPIO_DIR_OUT,
@@ -590,6 +563,10 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
if (!kp)
return -ENOMEM;
+ kp->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!kp->regmap)
+ return -ENODEV;
+
platform_set_drvdata(pdev, kp);
kp->pdata = pdata;
@@ -678,7 +655,7 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
return rc;
}
- rc = pmic8xxx_kp_read_u8(kp, &ctrl_val, KEYP_CTRL);
+ rc = regmap_read(kp->regmap, KEYP_CTRL, &ctrl_val);
if (rc < 0) {
dev_err(&pdev->dev, "failed to read KEYP_CTRL register\n");
return rc;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 05/10] Input: pmic8xxx-pwrkey - Migrate to DT
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input
In-Reply-To: <1388709460-19222-1-git-send-email-sboyd@codeaurora.org>
The driver is only supported on DT enabled platforms. Convert the
driver to DT so that it can probe properly.
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/input/misc/pmic8xxx-pwrkey.c | 33 ++++++++++++++++++++-------------
include/linux/input/pmic8xxx-pwrkey.h | 31 -------------------------------
2 files changed, 20 insertions(+), 44 deletions(-)
delete mode 100644 include/linux/input/pmic8xxx-pwrkey.h
diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c b/drivers/input/misc/pmic8xxx-pwrkey.c
index aaf332510623..72f629f37211 100644
--- a/drivers/input/misc/pmic8xxx-pwrkey.c
+++ b/drivers/input/misc/pmic8xxx-pwrkey.c
@@ -20,8 +20,7 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/log2.h>
-
-#include <linux/input/pmic8xxx-pwrkey.h>
+#include <linux/of.h>
#define PON_CNTL_1 0x1C
#define PON_CNTL_PULL_UP BIT(7)
@@ -80,6 +79,13 @@ static int pmic8xxx_pwrkey_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pm8xxx_pwr_key_pm_ops,
pmic8xxx_pwrkey_suspend, pmic8xxx_pwrkey_resume);
+static const struct of_device_id pm8xxx_pwr_key_id_table[] = {
+ { .compatible = "qcom,pm8058-pwrkey" },
+ { .compatible = "qcom,pm8921-pwrkey" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_pwr_key_id_table);
+
static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
{
struct input_dev *pwr;
@@ -90,15 +96,15 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
unsigned int pon_cntl;
struct regmap *regmap;
struct pmic8xxx_pwrkey *pwrkey;
- const struct pm8xxx_pwrkey_platform_data *pdata =
- dev_get_platdata(&pdev->dev);
+ u32 kpd_delay;
+ bool pull_up;
- if (!pdata) {
- dev_err(&pdev->dev, "power key platform data not supplied\n");
- return -EINVAL;
- }
+ if (of_property_read_u32(pdev->dev.of_node, "debounce", &kpd_delay))
+ kpd_delay = 0;
+
+ pull_up = of_property_read_bool(pdev->dev.of_node, "pull-up");
- if (pdata->kpd_trigger_delay_us > 62500) {
+ if (kpd_delay > 62500) {
dev_err(&pdev->dev, "invalid power key trigger delay\n");
return -EINVAL;
}
@@ -126,7 +132,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
pwr->name = "pmic8xxx_pwrkey";
pwr->phys = "pmic8xxx_pwrkey/input0";
- delay = (pdata->kpd_trigger_delay_us << 10) / USEC_PER_SEC;
+ delay = (kpd_delay << 10) / USEC_PER_SEC;
delay = 1 + ilog2(delay);
err = regmap_read(regmap, PON_CNTL_1, &pon_cntl);
@@ -137,7 +143,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
pon_cntl &= ~PON_CNTL_TRIG_DELAY_MASK;
pon_cntl |= (delay & PON_CNTL_TRIG_DELAY_MASK);
- if (pdata->pull_up)
+ if (pull_up)
pon_cntl |= PON_CNTL_PULL_UP;
else
pon_cntl &= ~PON_CNTL_PULL_UP;
@@ -173,7 +179,7 @@ static int pmic8xxx_pwrkey_probe(struct platform_device *pdev)
}
platform_set_drvdata(pdev, pwrkey);
- device_init_wakeup(&pdev->dev, pdata->wakeup);
+ device_init_wakeup(&pdev->dev, 1);
return 0;
}
@@ -189,9 +195,10 @@ static struct platform_driver pmic8xxx_pwrkey_driver = {
.probe = pmic8xxx_pwrkey_probe,
.remove = pmic8xxx_pwrkey_remove,
.driver = {
- .name = PM8XXX_PWRKEY_DEV_NAME,
+ .name = "pm8xxx-pwrkey",
.owner = THIS_MODULE,
.pm = &pm8xxx_pwr_key_pm_ops,
+ .of_match_table = pm8xxx_pwr_key_id_table,
},
};
module_platform_driver(pmic8xxx_pwrkey_driver);
diff --git a/include/linux/input/pmic8xxx-pwrkey.h b/include/linux/input/pmic8xxx-pwrkey.h
deleted file mode 100644
index 6d2974e57109..000000000000
--- a/include/linux/input/pmic8xxx-pwrkey.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/* Copyright (c) 2010-2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#ifndef __PMIC8XXX_PWRKEY_H__
-#define __PMIC8XXX_PWRKEY_H__
-
-#define PM8XXX_PWRKEY_DEV_NAME "pm8xxx-pwrkey"
-
-/**
- * struct pm8xxx_pwrkey_platform_data - platform data for pwrkey driver
- * @pull up: power on register control for pull up/down configuration
- * @kpd_trigger_delay_us: time delay for power key state change interrupt
- * trigger.
- * @wakeup: configure power key as wakeup source
- */
-struct pm8xxx_pwrkey_platform_data {
- bool pull_up;
- u32 kpd_trigger_delay_us;
- u32 wakeup;
-};
-
-#endif /* __PMIC8XXX_PWRKEY_H__ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 06/10] Input: pm8xxx-vibrator - Add DT match table
From: Stephen Boyd @ 2014-01-03 0:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input
In-Reply-To: <1388709460-19222-1-git-send-email-sboyd@codeaurora.org>
The driver is only supported on DT enabled platforms. Convert the
driver to DT so that it can probe properly.
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/input/misc/pm8xxx-vibrator.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 28251560249d..458d51b88be5 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -142,6 +142,13 @@ static int pm8xxx_vib_play_effect(struct input_dev *dev, void *data,
return 0;
}
+static const struct of_device_id pm8xxx_vib_id_table[] = {
+ { .compatible = "qcom,pm8058-vib" },
+ { .compatible = "qcom,pm8921-vib" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
+
static int pm8xxx_vib_probe(struct platform_device *pdev)
{
@@ -221,6 +228,7 @@ static struct platform_driver pm8xxx_vib_driver = {
.name = "pm8xxx-vib",
.owner = THIS_MODULE,
.pm = &pm8xxx_vib_pm_ops,
+ .of_match_table = pm8xxx_vib_id_table,
},
};
module_platform_driver(pm8xxx_vib_driver);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ 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